On Tue, 2014-02-11 at 15:42 +0100, Jan Cholasta wrote: > On 11.2.2014 14:29, Martin Basti wrote: > > On Mon, 2014-02-10 at 14:28 +0100, Jan Cholasta wrote: > >> On 10.2.2014 13:14, Martin Basti wrote: > >>> On Mon, 2014-02-10 at 12:22 +0100, Jan Cholasta wrote: > >>>> On 10.2.2014 08:50, Petr Spacek wrote: > >>>>> On 7.2.2014 10:42, Martin Basti wrote: > >>>>>> On Thu, 2014-02-06 at 17:04 +0100, Martin Basti wrote: > >>>>>>> On Thu, 2014-02-06 at 16:37 +0100, Jan Cholasta wrote: > >>>>>>>> On 6.2.2014 15:57, Martin Basti wrote: > >>>>>>>>> On Thu, 2014-02-06 at 10:59 +0100, Jan Cholasta wrote: > >>>>>>>>>> Hi, > >>>>>>>>>> > >>>>>>>>>> On 31.1.2014 16:06, Martin Basti wrote: > >>>>>>>>>>> Reverse domain names in form "0/28.0.10.10.in-addr.arpa." are now > >>>>>>>>>>> allowed. > >>>>>>>>>>> > >>>>>>>>>>> Ticket: https://fedorahosted.org/freeipa/ticket/4143 > >>>>>>>>>>> Patches attached. > >>>>>>>>>> > >>>>>>>>> I add Petr2 to CC, to inspect RFC issues, with allowing '/' in IPv6 > >>>>>>>>> > >>>>>>>>>> I think the validation should be more strict. IPv4 reverse zones > >>>>>>>>>> should > >>>>>>>>>> allow slash only in the label for the last octet (i.e. > >>>>>>>>>> 0/25.1.168.192 is > >>>>>>>>>> valid, 0.1/25.168.192 is not). IPv6 reverse zones should not allow > >>>>>>>>>> slash > >>>>>>>>>> at all. > >>>>>>>>>> > >>>>>>>>> I havent found anything about IPv6, RFCs don't forbids it. > >>>>>>>> > >>>>>>>> AFAIK the RFCs do not forbid anything, but we do validation anyway, > >>>>>>>> so > >>>>>>>> we might as well do it right, otherwise there is no point in doing > >>>>>>>> it. > >>>>>>>> > >>>>>>> OK, I leave there only IPv4 > >>>> > >>>> For the record, we discussed this off-line with Martin and Petr and > >>>> figured out it would be best to allow slashes in IPv6 reverse zones > >>>> after all. > >>>> > >>>>>>> > >>>>>>>>> 1.0/25.1.168.192.in-addr.arpa. is also valid, it could be used to > >>>>>>>>> CNAME > >>>>>>>>> records > >>>>>>>> > >>>>>>>> Yes, obviously. It's 1.0.1/25.168.192.in-addr.arpa. I'm concerned > >>>>>>>> about. > >>>>>>>> > >>>>>>> > >>>>>>> http://tools.ietf.org/html/rfc6672#section-6.2 > >>>>>>> This can give a very strange positions of / in FQDN > >>>> > >>>> Point taken. > >>>> > >>>>>>> > >>>>>>> Optionally, I could permit only 1 slash in domain name, but I have to > >>>>>>> inspect first if user can do something useful with subnet of subnet in > >>>>>>> DNS, like 1.0/25.128/25.168.192.in-addr.arpa > >>>>> Multiple slashes has to be allowed, without limitation to last octet. > >>>>> Imagine situation when split subnet is later split to even smaller > >>>>> pieces. > >>>>> > >>>>> Guys, do not over-engineer it. IMHO this validator should produce a > >>>>> warning is something is not as we expect but it should not block user > >>>>> from adding a record. > >>>>> > >>>>> We have had enough problems with too strict validators in the past and > >>>>> IMHO warning is the way to go. > >>>> > >>>> I agree, but it's too late to get such change into 3.3.x. > >>>> > >>>>> > >>>>> Petr^2 Spacek > >>>>> > >>>>>>>>> The slashes in domain names are referenced as the best practise in > >>>>>>>>> RFC, > >>>>>>>>> there are not strict rules. > >>>>>>>>>> > >>>>>>>>>> +def _cname_hostname_validator(ugettext, value): > >>>>>>>>>> > >>>>>>>>>> Can you name this _bind_cname_hostname_validator, so that it is > >>>>>>>>>> clear it > >>>>>>>>>> is related to _bind_hostname_validator? > >>>>>>>>>> > >>>>>>>>> I will rename it > >>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> + #classless reverse zones can contain slash '/' > >>>>>>>>>> + if not zone_is_reverse(normalized_zone) and > >>>>>>>>>> (normalized_zone.count('/') > 0): > >>>>>>>>>> + raise errors.ValidationError(name='name', > >>>>>>>>>> + error=_("Only reverse zones can contain > >>>>>>>>>> '/' in > >>>>>>>>>> labels")) > >>>>>>>>>> > >>>>>>>>>> This should be handled in _domain_name_validator. Validation in > >>>>>>>>>> pre_callback should be done only when the validation depends on > >>>>>>>>>> values > >>>>>>>>>> of multiple parameters, which is not this case. > >>>>>>>>>> > >>>>>>>>> I will move it > >>>>>>>>>> > >>>>>>>>>> + def _reverse_zone_pre_callback(self, ldap, dn, entry_attrs, > >>>>>>>>>> *keys, > >>>>>>>>>> **options): > >>>>>>>>>> > >>>>>>>>>> Rename this to _idnsname_pre_callback and you won't have to call it > >>>>>>>>>> explicitly in run_precallback_validators. > >>>>>>>>>> > >>>>>>>>> I will rename it > >>>>>>>>>> > >>>>>>>>>> + if addr.count('/') > 0: > >>>>>>>>>> > >>>>>>>>>> I think "if '/' in addr:" would be better. > >>>>>>>>>> > >>>>>>>>> I will change it > >>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> -def validate_dns_label(dns_label, allow_underscore=False): > >>>>>>>>>> +def validate_dns_label(dns_label, allow_underscore=False, > >>>>>>>>>> allow_slash=False): > >>>>>>>>>> > >>>>>>>>>> IMO instead of adding a new boolean argument, it would be nicer to > >>>>>>>>>> replace allow_underscore with an argument (e.g. allowed_chars) > >>>>>>>>>> which > >>>>>>>>>> takes a string of extra allowed characters. > >>>>>>>>>> > >>>>>>>>> But I have to handle not only allowed chars, but position of the > >>>>>>>>> chars > >>>>>>>>> in the label string too. > >>>>>>>> > >>>>>>>> Why? Is there a RFC that forbids it? > >>>>>>>> > >>>>>>>> My point is, adding a new argument for each extra character is bad, > >>>>>>>> there should be a better way of doing that. > >>>>>>>> > >>>>>>> I agree, but for example: "_" should be at start (it is not required > >>>>>>> be > >>>>>>> at the start in IPA), "/" and "-" in the middle. > >>>> > >>>> OK then. (But I still don't like it.) > >>>> > >>>>>>> > >>>>>> > >>>>>> Updated patch attached. > >>>>>> Patch for tests is the same as previos. > >>>> > >>>> + validate_domain_name(value, allow_slash=True) > >>>> + > >>>> + #classless reverse zones can contain slash '/' > >>>> + normalized_zone = normalize_zone(value) > >>>> + if not zone_is_reverse(normalized_zone) and ('/' in > >>>> normalized_zone): > >>>> + return u"Only reverse zones can contain '/' in labels" > >>>> > >>>> You don't need to enclose "x in y" in parentheses. Also I don't think > >>>> there is any value in pointing out that slash can be used for reverse > >>>> zones when giving an error for non-reverse zones. I would prefer > >>>> something like this instead: > >>>> > >>>> normalized_zone = normalize_zone(value) > >>>> validate_domain_mame(value, > >>>> allow_slash=zone_is_reverse(normalized_zone)) > >>>> > >>>> > >>>> + def _idnsname_pre_callback(self, ldap, dn, entry_attrs, *keys, > >>>> **options): > >>>> + #in reverse zone can a record name contains /, (and -) > >>>> + > >>>> + if self.is_pkey_zone_record(*keys): > >>>> + addr = u'' > >>>> + else: > >>>> + addr = keys[-1] > >>>> + > >>>> + zone = keys[-2] > >>>> + zone = normalize_zone(zone) > >>>> + if (not zone_is_reverse(zone)) and ('/' in addr): > >>>> + raise errors.ValidationError(name='name', > >>>> + error=unicode(_('Only domain names in reverse zones > >>>> can contain \'/\'')) ) > >>>> > >>>> I think you can do better (from the top of my head and untested): > >>>> > >>>> if not self.is_pkey_zone_record(*keys): > >>>> zone, addr = normalize_zone(keys[-2]), keys[-1] > >>>> try: > >>>> validate_domain_name(addr + zone, > >>>> allow_slash=zone_is_reverse(zone)) > >>>> except ValueError, e: > >>>> raise errors.ValidationError(name='idnsname', > >>>> error=str(e)) > >>>> > >>>> > >>>> + #Classless zones (0/25.0.0.10.in-addr.arpa.) -> skip check > >>>> + #zone has to be checked without reverse domain suffix > >>>> (in-addr.arpa.) > >>>> + if '/' in addr or '/' in zone or '-' in addr or '-' in zone: > >>>> + pass > >>>> + else: > >>>> + ip_addr_comp_count = addr_len + len(zone.split('.')) > >>>> + if ip_addr_comp_count != zone_len: > >>>> + raise errors.ValidationError(name='ptrrecord', > >>>> + error=unicode(_('Reverse zone %(name)s requires > >>>> exactly %(count)d IP address components, %(user_count)d given') > >>>> + % dict(name=zone_name, count=zone_len, > >>>> user_count=ip_addr_comp_count))) > >>>> > >>>> Is there anything preventing us from dropping this whole check? I don't > >>>> think it makes sense anymore. > >>>> > >>> IMO it doesnt have sense with classless reverse domains, but it is > >>> useful with classfull reverse domains, which are used more, to prevent > >>> users making mistakes. > >> > >> OK, but please use a simple if instead of if/pass/else. > >> > >>> > >>>> > >>>> IMHO validate_dns_label is very ugly. I would prefer if it looked > >>>> something like this instead (again, from the top of my head and > >>>> untested): > >>>> > >>>> def validate_dns_label(dns_label, allow_underscore=False, > >>>> allow_slash=False): > >>>> base_chars = 'a-z0-9' > >>>> extra_chars = '' > >>>> middle_chars = '' > >>>> > >>>> if allow_underscore: > >>>> extra_chars += "_" > >>>> if allow_slash: > >>>> middle_chars += '/' > >>>> > >>>> middle_chars += '-' > >>>> > >>>> label_regex = > >>>> r'^[%(base)s%(extra)s]([%(base)s%(extra)%(middle)s]?[%(base)s%(extra)s])*$' > >>>> % dict(base=base_chars, extra=extra_chars, middle=middle_chars) > >>>> regex = re.compile(label_regex, re.IGNORECASE) > >>>> > >>>> if not dns_label: > >>>> raise ValueError(_('empty DNS label')) > >>>> > >>>> if len(dns_label) > 63: > >>>> raise ValueError(_('DNS label cannot be longer that 63 > >>>> characters')) > >>>> > >>>> if not regex.match(dns_label): > >>>> chars = ', '.join('"%s"' % c for c in base_chars + extra_chars > >>>> + middle_chars) > >>>> chars2 = ', '.join('"%s"' % c for c in middle_chars) > >>>> raise ValueError(_('only letters, numbers, %(chars)s are > >>>> allowed. ' \ > >>>> 'DNS label may not start or end with > >>>> %(chars2)s') \ > >>>> % dict(chars=chars, chars2=chars2)) > >>>> > >>>> > >>> > >>> Thank you for the review, I will fix it. > >>> > >> > >> > > > > All fixed. Patches attached. > > > > Thanks! > > > I see some new test failures caused by the error message change: > > ====================================================================== > FAIL: test_netgroup[17]: netgroup_add_member: Add invalid host > u'+invalid&host' to netgroup u'netgroup1' > ---------------------------------------------------------------------- > Traceback (most recent call last): > File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in > runTest > self.test(*self.arg) > File > "/usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/xmlrpc_test.py", > line 284, in <lambda> > func = lambda: self.check(nice, **test) > File > "/usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/xmlrpc_test.py", > line 298, in check > self.check_exception(nice, cmd, args, options, expected) > File > "/usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/xmlrpc_test.py", > line 324, in check_exception > assert_deepequal(expected.strerror, e.strerror) > File "/usr/lib/python2.7/site-packages/ipatests/util.py", line 352, > in assert_deepequal > VALUE % (doc, expected, got, stack) > AssertionError: assert_deepequal: expected != got. > > expected = u"invalid 'host': only letters, numbers, _, and - are > allowed. DNS label may not start or end with -" > got = u"invalid 'host': only letters, numbers, '_', '-' are allowed. > DNS label may not start or end with '-'" > path = () > > ====================================================================== > FAIL: test_netgroup[32]: netgroup_mod: Add invalid host u'+invalid&host' > to netgroup u'netgroup1' using setattr > ---------------------------------------------------------------------- > Traceback (most recent call last): > File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in > runTest > self.test(*self.arg) > File > "/usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/xmlrpc_test.py", > line 284, in <lambda> > func = lambda: self.check(nice, **test) > File > "/usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/xmlrpc_test.py", > line 298, in check > self.check_exception(nice, cmd, args, options, expected) > File > "/usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/xmlrpc_test.py", > line 324, in check_exception > assert_deepequal(expected.strerror, e.strerror) > File "/usr/lib/python2.7/site-packages/ipatests/util.py", line 352, > in assert_deepequal > VALUE % (doc, expected, got, stack) > AssertionError: assert_deepequal: expected != got. > > expected = u"invalid 'externalhost': only letters, numbers, _, and - > are allowed. DNS label may not start or end with -" > got = u"invalid 'externalhost': only letters, numbers, '_', '-' are > allowed. DNS label may not start or end with '-'" > path = () > > ====================================================================== > FAIL: test_raduisproxy[21]: radiusproxy_mod: Set server string of > testradius to testradius.test:1:2:3 (invalid) > ---------------------------------------------------------------------- > Traceback (most recent call last): > File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in > runTest > self.test(*self.arg) > File > "/usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/xmlrpc_test.py", > line 284, in <lambda> > func = lambda: self.check(nice, **test) > File > "/usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/xmlrpc_test.py", > line 298, in check > self.check_exception(nice, cmd, args, options, expected) > File > "/usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/xmlrpc_test.py", > line 324, in check_exception > assert_deepequal(expected.strerror, e.strerror) > File "/usr/lib/python2.7/site-packages/ipatests/util.py", line 352, > in assert_deepequal > VALUE % (doc, expected, got, stack) > AssertionError: assert_deepequal: expected != got. > > expected = u"invalid 'ipatokenradiusserver': only letters, numbers, > _, and - are allowed. DNS label may not start or end with -" > got = u"invalid 'ipatokenradiusserver': only letters, numbers, '_', > '-' are allowed. DNS label may not start or end with '-'" > path = () > > ====================================================================== > FAIL: Test adding an invalid external host to Sudo rule using > ---------------------------------------------------------------------- > Traceback (most recent call last): > File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in > runTest > self.test(*self.arg) > File > "/usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/test_sudorule_plugin.py", > > line 500, in test_a_sudorule_mod_externalhost_invalid_addattr > "DNS label may not start or end with -") > AssertionError > > > And one nitpick: please don't add the extra newlines around > _domain_name_validator. > > > Honza >
Thank you for review. Patches attached. Extra blank lines removed. The others test are fixed now. -- Martin^2 Basti
>From 2d13e298d180b24d2e650c746e0b0fd231fe90a2 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Fri, 31 Jan 2014 15:42:31 +0100 Subject: [PATCH] DNS classless support for reverse domains Now users can adding reverse zones in classless form: 0/25.1.168.192.in-addr.arpa. 0-25.1.168.192.in-addr.arpa. 128/25 NS ns.example.com. 10 CNAME 10.128/25.1.168.192.in-addr.arpa. Ticket: https://fedorahosted.org/freeipa/ticket/4143 --- ipalib/plugins/dns.py | 45 +++++++++++++++++++++++++++---------- ipalib/util.py | 61 ++++++++++++++++++++++++++++++--------------------- 2 files changed, 70 insertions(+), 36 deletions(-) diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index afd47be316b8b84fe281698af206b39fc1e1bf55..caaeaf1679113c1378944a1c7a19fc0dfffb0525 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -368,25 +368,31 @@ def _normalize_bind_aci(bind_acis): acis += u';' return acis -def _bind_hostname_validator(ugettext, value): +def _bind_hostname_validator(ugettext, value, allow_slash=False): if value == _dns_zone_record: return try: # Allow domain name which is not fully qualified. These are supported # in bind and then translated as <non-fqdn-name>.<domain>. - validate_hostname(value, check_fqdn=False, allow_underscore=True) + validate_hostname(value, check_fqdn=False, allow_underscore=True, allow_slash=allow_slash) except ValueError, e: return _('invalid domain-name: %s') \ % unicode(e) return None +def _bind_cname_hostname_validator(ugettext, value): + """ + Validator for CNAME allows classless domain names (25/0.0.10.in-addr.arpa.) + """ + return _bind_hostname_validator(ugettext, value, allow_slash=True) + def _dns_record_name_validator(ugettext, value): if value == _dns_zone_record: return try: - map(lambda label:validate_dns_label(label, allow_underscore=True), \ + map(lambda label:validate_dns_label(label, allow_underscore=True, allow_slash=True), \ value.split(u'.')) except ValueError, e: return unicode(e) @@ -411,7 +417,10 @@ def _validate_bind_forwarder(ugettext, forwarder): def _domain_name_validator(ugettext, value): try: - validate_domain_name(value) + #classless reverse zones can contain slash '/' + normalized_zone = normalize_zone(value) + validate_domain_name(value, allow_slash=zone_is_reverse(normalized_zone)) + except ValueError, e: return unicode(e) @@ -939,7 +948,7 @@ class CNAMERecord(DNSRecord): rfc = 1035 parts = ( Str('hostname', - _bind_hostname_validator, + _bind_cname_hostname_validator, label=_('Hostname'), doc=_('A hostname which this alias hostname points to'), ), @@ -960,7 +969,7 @@ class DNAMERecord(DNSRecord): rfc = 2672 parts = ( Str('target', - _bind_hostname_validator, + _bind_cname_hostname_validator, label=_('Target'), ), ) @@ -2131,6 +2140,14 @@ class dnsrecord(LDAPObject): doc=_('Parse all raw DNS records and return them in a structured way'), ) + def _idnsname_pre_callback(self, ldap, dn, entry_attrs, *keys, **options): + if not self.is_pkey_zone_record(*keys): + zone, addr = normalize_zone(keys[-2]), keys[-1] + try: + validate_domain_name(addr, allow_underscore=True, allow_slash=zone_is_reverse(zone)) + except ValueError, e: + raise errors.ValidationError(name='idnsname', error=unicode(e)) + def _nsrecord_pre_callback(self, ldap, dn, entry_attrs, *keys, **options): assert isinstance(dn, DN) nsrecords = entry_attrs.get('nsrecord') @@ -2144,6 +2161,7 @@ class dnsrecord(LDAPObject): ptrrecords = entry_attrs.get('ptrrecord') if ptrrecords is None: return + zone = keys[-2] if self.is_pkey_zone_record(*keys): addr = u'' @@ -2165,11 +2183,16 @@ class dnsrecord(LDAPObject): error=unicode(_('Reverse zone for PTR record should be a sub-zone of one the following fully qualified domains: %s') % allowed_zones)) addr_len = len(addr.split('.')) if addr else 0 - ip_addr_comp_count = addr_len + len(zone.split('.')) - if ip_addr_comp_count != zone_len: - raise errors.ValidationError(name='ptrrecord', - error=unicode(_('Reverse zone %(name)s requires exactly %(count)d IP address components, %(user_count)d given') - % dict(name=zone_name, count=zone_len, user_count=ip_addr_comp_count))) + + #Classless zones (0/25.0.0.10.in-addr.arpa.) -> skip check + #zone has to be checked without reverse domain suffix (in-addr.arpa.) + if ('/' not in addr and '/' not in zone and + '-' not in addr and '-' not in zone): + ip_addr_comp_count = addr_len + len(zone.split('.')) + if ip_addr_comp_count != zone_len: + raise errors.ValidationError(name='ptrrecord', + error=unicode(_('Reverse zone %(name)s requires exactly %(count)d IP address components, %(user_count)d given') + % dict(name=zone_name, count=zone_len, user_count=ip_addr_comp_count))) def run_precallback_validators(self, dn, entry_attrs, *keys, **options): assert isinstance(dn, DN) diff --git a/ipalib/util.py b/ipalib/util.py index 1701fbdd18670cfb920044895956d302aa53964c..fcc4b1591e11365a98094019e8e9f5b87d214f75 100644 --- a/ipalib/util.py +++ b/ipalib/util.py @@ -215,34 +215,45 @@ def normalize_zone(zone): else: return zone -def validate_dns_label(dns_label, allow_underscore=False): - label_chars = r'a-z0-9' - underscore_err_msg = '' - if allow_underscore: - label_chars += "_" - underscore_err_msg = u' _,' - label_regex = r'^[%(chars)s]([%(chars)s-]?[%(chars)s])*$' % dict(chars=label_chars) - regex = re.compile(label_regex, re.IGNORECASE) - - if not dns_label: - raise ValueError(_('empty DNS label')) - - if len(dns_label) > 63: - raise ValueError(_('DNS label cannot be longer that 63 characters')) - - if not regex.match(dns_label): - raise ValueError(_('only letters, numbers,%(underscore)s and - are allowed. ' \ - 'DNS label may not start or end with -') \ - % dict(underscore=underscore_err_msg)) - -def validate_domain_name(domain_name, allow_underscore=False): + +def validate_dns_label(dns_label, allow_underscore=False, allow_slash=False): + base_chars = 'a-z0-9' + extra_chars = '' + middle_chars = '' + + if allow_underscore: + extra_chars += '_' + if allow_slash: + middle_chars += '/' + + middle_chars = middle_chars + '-' #has to be always the last in the regex [....-] + + label_regex = r'^[%(base)s%(extra)s]([%(base)s%(extra)s%(middle)s]?[%(base)s%(extra)s])*$' \ + % dict(base=base_chars, extra=extra_chars, middle=middle_chars) + regex = re.compile(label_regex, re.IGNORECASE) + + if not dns_label: + raise ValueError(_('empty DNS label')) + + if len(dns_label) > 63: + raise ValueError(_('DNS label cannot be longer that 63 characters')) + + if not regex.match(dns_label): + chars = ', '.join("'%s'" % c for c in extra_chars + middle_chars) + chars2 = ', '.join("'%s'" % c for c in middle_chars) + raise ValueError(_("only letters, numbers, %(chars)s are allowed. " \ + "DNS label may not start or end with %(chars2)s") \ + % dict(chars=chars, chars2=chars2)) + + +def validate_domain_name(domain_name, allow_underscore=False, allow_slash=False): if domain_name.endswith('.'): domain_name = domain_name[:-1] domain_name = domain_name.split(".") # apply DNS name validator to every name part - map(lambda label:validate_dns_label(label,allow_underscore), domain_name) + map(lambda label:validate_dns_label(label, allow_underscore, allow_slash), domain_name) def validate_zonemgr(zonemgr): @@ -287,7 +298,7 @@ def validate_zonemgr(zonemgr): local_part.split(local_part_sep)): raise ValueError(local_part_errmsg) -def validate_hostname(hostname, check_fqdn=True, allow_underscore=False): +def validate_hostname(hostname, check_fqdn=True, allow_underscore=False, allow_slash=False): """ See RFC 952, 1123 :param hostname Checked value @@ -305,9 +316,9 @@ def validate_hostname(hostname, check_fqdn=True, allow_underscore=False): if '.' not in hostname: if check_fqdn: raise ValueError(_('not fully qualified')) - validate_dns_label(hostname,allow_underscore) + validate_dns_label(hostname, allow_underscore, allow_slash) else: - validate_domain_name(hostname,allow_underscore) + validate_domain_name(hostname, allow_underscore, allow_slash) def normalize_sshpubkey(value): return SSHPublicKey(value).openssh() -- 1.8.3.1
>From c1e04b859ef7f508e319c111be29e139ef86eba7 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Fri, 31 Jan 2014 15:52:35 +0100 Subject: [PATCH] DNS tests for classless reverse domains Ticket: https://fedorahosted.org/freeipa/ticket/4143 --- ipatests/test_xmlrpc/test_dns_plugin.py | 251 ++++++++++++++++++++++-- ipatests/test_xmlrpc/test_netgroup_plugin.py | 8 +- ipatests/test_xmlrpc/test_radiusproxy_plugin.py | 4 +- ipatests/test_xmlrpc/test_sudorule_plugin.py | 4 +- 4 files changed, 246 insertions(+), 21 deletions(-) diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py index 2cc54de504745fa9a98d132d00554e0e54d8eb57..e8b791fd9895467af2352eb7f94e0b03e7f42162 100644 --- a/ipatests/test_xmlrpc/test_dns_plugin.py +++ b/ipatests/test_xmlrpc/test_dns_plugin.py @@ -42,6 +42,17 @@ zone2_dn = DN(('idnsname', zone2), api.env.container_dns, api.env.basedn) zone2_ns = u'ns1.%s.' % zone2 zone2_rname = u'root.%s.' % zone2 +zone3 = u'zone3.test' +zone3_ip = u'192.168.1.1' +zone3_ip2 = u'192.168.1.129' +zone3_dn = DN(('idnsname', zone3), api.env.container_dns, api.env.basedn) +zone3_ns = u'ns1.%s.' % zone3 +zone3_ns2 = u'ns2.%s.' % zone3 +zone3_rname = u'root.%s.' % zone3 + +zone3_ns2_arec = u'ns2' +zone3_ns2_arec_dn = DN(('idnsname',zone3_ns2_arec), zone3_dn) + revzone1 = u'31.16.172.in-addr.arpa.' revzone1_ip = u'172.16.31.0' revzone1_ipprefix = u'172.16.31.' @@ -51,6 +62,16 @@ revzone2 = u'30.15.172.in-addr.arpa.' revzone2_ip = u'172.15.30.0/24' revzone2_dn = DN(('idnsname',revzone2), api.env.container_dns, api.env.basedn) +revzone3_classless1 = u'1.168.192.in-addr.arpa.' +revzone3_classless1_ip = u'192.168.1.0' +revzone3_classless1_ipprefix = u'192.168.1.' +revzone3_classless1_dn = DN(('idnsname', revzone3_classless1), api.env.container_dns, api.env.basedn) + +revzone3_classless2 = u'128/25.1.168.192.in-addr.arpa.' +revzone3_classless2_ip = u'192.168.1.128' +revzone3_classless2_ipprefix = u'192.168.1.' +revzone3_classless2_dn = DN(('idnsname', revzone3_classless2), api.env.container_dns, api.env.basedn) + name1 = u'testdnsres' name1_dn = DN(('idnsname',name1), zone1_dn) name1_renamed = u'testdnsres-renamed' @@ -69,6 +90,17 @@ cname_dn = DN(('idnsname',cname), zone1_dn) dname = u'testdns-dname' dname_dn = DN(('idnsname',dname), zone1_dn) +nsrev = u'128/25' +nsrev_dn = DN(('idnsname',nsrev), revzone3_classless1_dn) + +cnamerev = u'129' +cnamerev_dn = DN(('idnsname',cnamerev), revzone3_classless1_dn) +cnamerev_hostname = u'129.128/25.1.168.192.in-addr.arpa.' + +ptr_revzone3 = u'129' +ptr_revzone3_dn = DN(('idnsname',cnamerev), revzone3_classless2_dn) +ptr_revzone3_hostname = zone3_ns2; + relnxname = u'does-not-exist-test' absnxname = u'does.not.exist.test.' @@ -103,7 +135,8 @@ class test_dns(Declarative): pass cleanup_commands = [ - ('dnszone_del', [zone1, zone2, revzone1, revzone2], + ('dnszone_del', [zone1, zone2, zone3, revzone1, revzone2, + revzone3_classless1, revzone3_classless2], {'continue': True}), ('dnsconfig_mod', [], {'idnsforwarders' : None, 'idnsforwardpolicy' : None, @@ -148,8 +181,8 @@ class test_dns(Declarative): } ), expected=errors.ValidationError(name='name', - error=u'only letters, numbers, and - are allowed. ' + - u'DNS label may not start or end with -'), + error=u"only letters, numbers, '-' are allowed." + + u" DNS label may not start or end with '-'"), ), @@ -525,8 +558,8 @@ class test_dns(Declarative): desc='Try to create record with invalid name in zone %r' % zone1, command=('dnsrecord_add', [zone1, u'invalid record'], {'arecord': arec2}), expected=errors.ValidationError(name='name', - error=u'only letters, numbers, _, and - are allowed. ' + - u'DNS label may not start or end with -'), + error=u"only letters, numbers, '_', '/', '-' are allowed." + + u" DNS label may not start or end with '/', '-'"), ), @@ -701,8 +734,8 @@ class test_dns(Declarative): 'srv_part_port' : 123, 'srv_part_target' : u'foo bar'}), expected=errors.ValidationError(name='srv_target', - error=u'invalid domain-name: only letters, numbers, _, and - ' + - u'are allowed. DNS label may not start or end with -'), + error=u"invalid domain-name: only letters, numbers, '_', '-' are allowed." + + u" DNS label may not start or end with '-'"), ), dict( @@ -804,8 +837,8 @@ class test_dns(Declarative): desc='Try to add invalid CNAME record %r using dnsrecord_add' % (cname), command=('dnsrecord_add', [zone1, cname], {'cnamerecord': u'-.%s' % relnxname}), expected=errors.ValidationError(name='hostname', - error=u'invalid domain-name: only letters, numbers, _, and - ' + - u'are allowed. DNS label may not start or end with -'), + error=u"invalid domain-name: only letters, numbers, '_', '/', '-' are allowed." + + u" DNS label may not start or end with '/', '-'"), ), dict( @@ -874,8 +907,8 @@ class test_dns(Declarative): command=('dnsrecord_add', [zone1, dname], {'dnamerecord': u'-.%s' % absnxname}), expected=errors.ValidationError(name='target', - error=u'invalid domain-name: only letters, numbers, _, and - ' + - u'are allowed. DNS label may not start or end with -'), + error=u"invalid domain-name: only letters, numbers, '_', '/', '-' are allowed." + + u" DNS label may not start or end with '/', '-'"), ), dict( @@ -1159,8 +1192,8 @@ class test_dns(Declarative): desc='Try to add invalid PTR %r to %r using dnsrecord_add' % (revname1, revzone1), command=('dnsrecord_add', [revzone1, revname1], {'ptrrecord': u'-.%s' % relnxname}), expected=errors.ValidationError(name='hostname', - error=u'invalid domain-name: only letters, numbers, and - ' + - u'are allowed. DNS label may not start or end with -'), + error=u"invalid domain-name: only letters, numbers, '-' " + + u"are allowed. DNS label may not start or end with '-'"), ), dict( @@ -1551,4 +1584,196 @@ class test_dns(Declarative): }, ), + dict( + desc='Create zone %r' % zone3, + command=( + 'dnszone_add', [zone3], { + 'idnssoamname': zone3_ns, + 'idnssoarname': zone3_rname, + 'ip_address' : zone3_ip, + } + ), + expected={ + 'value': zone3, + 'summary': None, + 'result': { + 'dn': zone3_dn, + 'idnsname': [zone3], + 'idnszoneactive': [u'TRUE'], + 'idnssoamname': [zone3_ns], + 'nsrecord': [zone3_ns], + 'idnssoarname': [zone3_rname], + 'idnssoaserial': [fuzzy_digits], + 'idnssoarefresh': [fuzzy_digits], + 'idnssoaretry': [fuzzy_digits], + 'idnssoaexpire': [fuzzy_digits], + 'idnssoaminimum': [fuzzy_digits], + 'idnsallowdynupdate': [u'FALSE'], + 'idnsupdatepolicy': [u'grant %(realm)s krb5-self * A; ' + u'grant %(realm)s krb5-self * AAAA; ' + u'grant %(realm)s krb5-self * SSHFP;' + % dict(realm=api.env.realm)], + 'idnsallowtransfer': [u'none;'], + 'idnsallowquery': [u'any;'], + 'objectclass': objectclasses.dnszone, + }, + }, + ), + + dict( + desc='Add A record to %r in zone %r' % (zone3_ns2_arec, zone3), + command=('dnsrecord_add', [zone3, zone3_ns2_arec], {'arecord': zone3_ip2}), + expected={ + 'value': zone3_ns2_arec, + 'summary': None, + 'result': { + 'dn': zone3_ns2_arec_dn, + 'idnsname': [zone3_ns2_arec], + 'arecord': [zone3_ip2], + 'objectclass': objectclasses.dnsrecord, + }, + }, + ), + + dict( + desc='Create reverse zone %r' % revzone3_classless1, + command=( + 'dnszone_add', [revzone3_classless1], { + 'idnssoamname': zone3_ns, + 'idnssoarname': zone3_rname, + } + ), + expected={ + 'value': revzone3_classless1, + 'summary': None, + 'result': { + 'dn': revzone3_classless1_dn, + 'idnsname': [revzone3_classless1], + 'idnszoneactive': [u'TRUE'], + 'idnssoamname': [zone3_ns], + 'nsrecord': [zone3_ns], + 'idnssoarname': [zone3_rname], + 'idnssoaserial': [fuzzy_digits], + 'idnssoarefresh': [fuzzy_digits], + 'idnssoaretry': [fuzzy_digits], + 'idnssoaexpire': [fuzzy_digits], + 'idnssoaminimum': [fuzzy_digits], + 'idnsallowdynupdate': [u'FALSE'], + 'idnsupdatepolicy': [u'grant %(realm)s krb5-subdomain %(zone)s PTR;' + % dict(realm=api.env.realm, zone=revzone3_classless1)], + 'idnsallowtransfer': [u'none;'], + 'idnsallowquery': [u'any;'], + 'objectclass': objectclasses.dnszone, + }, + }, + ), + + dict( + desc='Create classless reverse zone %r' % revzone3_classless2, + command=( + 'dnszone_add', [revzone3_classless2], { + 'idnssoamname': zone3_ns2, + 'idnssoarname': zone3_rname, + } + ), + expected={ + 'value': revzone3_classless2, + 'summary': None, + 'result': { + 'dn': revzone3_classless2_dn, + 'idnsname': [revzone3_classless2], + 'idnszoneactive': [u'TRUE'], + 'idnssoamname': [zone3_ns2], + 'nsrecord': [zone3_ns2], + 'idnssoarname': [zone3_rname], + 'idnssoaserial': [fuzzy_digits], + 'idnssoarefresh': [fuzzy_digits], + 'idnssoaretry': [fuzzy_digits], + 'idnssoaexpire': [fuzzy_digits], + 'idnssoaminimum': [fuzzy_digits], + 'idnsallowdynupdate': [u'FALSE'], + 'idnsupdatepolicy': [u'grant %(realm)s krb5-subdomain %(zone)s PTR;' + % dict(realm=api.env.realm, zone=revzone3_classless2)], + 'idnsallowtransfer': [u'none;'], + 'idnsallowquery': [u'any;'], + 'objectclass': objectclasses.dnszone, + }, + }, + ), + + dict( + desc='Add NS record to %r in revzone %r' % (nsrev, revzone3_classless1), + command=('dnsrecord_add', [revzone3_classless1, nsrev], {'nsrecord': zone3_ns2}), + expected={ + 'value': nsrev, + 'summary': None, + 'result': { + 'dn': nsrev_dn, + 'idnsname': [nsrev], + 'nsrecord': [zone3_ns2], + 'objectclass': objectclasses.dnsrecord, + }, + }, + ), + + dict( + desc='Add CNAME record to %r in revzone %r' % (cnamerev, revzone3_classless1), + command=('dnsrecord_add', [revzone3_classless1, cnamerev], {'cnamerecord': cnamerev_hostname}), + expected={ + 'value': cnamerev, + 'summary': None, + 'result': { + 'dn': cnamerev_dn, + 'idnsname': [cnamerev], + 'cnamerecord': [cnamerev_hostname], + 'objectclass': objectclasses.dnsrecord, + }, + }, + ), + + dict( + desc='Add PTR record to %r in revzone %r' % (ptr_revzone3, revzone3_classless2), + command=('dnsrecord_add', [revzone3_classless2, cnamerev], + {'ptrrecord': ptr_revzone3_hostname}), + expected={ + 'value': ptr_revzone3, + 'summary': None, + 'result': { + 'dn': ptr_revzone3_dn, + 'idnsname': [ptr_revzone3], + 'ptrrecord': [ptr_revzone3_hostname], + 'objectclass': objectclasses.dnsrecord, + }, + }, + ), + + dict( + desc='Try to create zone with invalid name', + command=( + 'dnszone_add', [u'invalid/zone'], { + 'idnssoamname': zone1_ns, + 'idnssoarname': zone1_rname, + 'ip_address' : zone1_ip, + } + ), + expected=errors.ValidationError(name='name', + error=u"only letters, numbers, '-' are allowed." + + u" DNS label may not start or end with '-'"), + ), + + dict( + desc='Try to add NS record %r to non-reverse zone %r using dnsrecord_add' % (nsrev, zone1), + command=('dnsrecord_add', [zone1, nsrev], {'nsrecord': zone3_ns2}), + expected=errors.ValidationError(name='idnsname', + error=u"only letters, numbers, '_', '-' are allowed." + + u" DNS label may not start or end with '-'"), + ), + + dict( + desc='Try to add invalid PTR hostname %r to %r using dnsrecord_add' % (cnamerev_hostname, revzone1), + command=('dnsrecord_add', [revzone1, revname1], {'ptrrecord': cnamerev_hostname }), + expected=errors.ValidationError(name='hostname', + error=u"invalid domain-name: only letters, numbers, '-' are allowed." + + u" DNS label may not start or end with '-'"), + ), ] diff --git a/ipatests/test_xmlrpc/test_netgroup_plugin.py b/ipatests/test_xmlrpc/test_netgroup_plugin.py index da9a809bb2122e760bca49416a098cc8f0c7309e..dd3a8b84c5aa811826187e6f39b1db4b7899e646 100644 --- a/ipatests/test_xmlrpc/test_netgroup_plugin.py +++ b/ipatests/test_xmlrpc/test_netgroup_plugin.py @@ -341,8 +341,8 @@ class test_netgroup(Declarative): desc='Add invalid host %r to netgroup %r' % (invalidhost, netgroup1), command=('netgroup_add_member', [netgroup1], dict(host=invalidhost)), expected=errors.ValidationError(name='host', - error='only letters, numbers, _, and - are allowed. ' + - u'DNS label may not start or end with -'), + error=u"only letters, numbers, '_', '-' are allowed. " + + u"DNS label may not start or end with '-'"), ), @@ -782,8 +782,8 @@ class test_netgroup(Declarative): dict(setattr='externalhost=%s' % invalidhost) ), expected=errors.ValidationError(name='externalhost', - error='only letters, numbers, _, and - are allowed. ' + - 'DNS label may not start or end with -'), + error=u"only letters, numbers, '_', '-' are allowed. " + + u"DNS label may not start or end with '-'"), ), dict( diff --git a/ipatests/test_xmlrpc/test_radiusproxy_plugin.py b/ipatests/test_xmlrpc/test_radiusproxy_plugin.py index c3cb9af2a1f66a3321a5e934f5a2bfcd26f3f49a..d3be060e98f44f3f652e93da68b35384239b9823 100644 --- a/ipatests/test_xmlrpc/test_radiusproxy_plugin.py +++ b/ipatests/test_xmlrpc/test_radiusproxy_plugin.py @@ -240,8 +240,8 @@ class test_raduisproxy(Declarative): for fqdn, error in ( (radius1_fqdn + u':0x5a', 'invalid port number'), (radius1_fqdn + u':1:2:3', - 'only letters, numbers, _, and - are allowed. DNS label may not ' - 'start or end with -'), + "only letters, numbers, '_', '-' are allowed. DNS label may not " + "start or end with '-'"), (u'bogus', 'not fully qualified'), ) ] + [ diff --git a/ipatests/test_xmlrpc/test_sudorule_plugin.py b/ipatests/test_xmlrpc/test_sudorule_plugin.py index ec5d16d62cc38b0d9ef439de96267dda88525fa8..7dc3cb1e458b6f5d145dc315d977e85a1f5b6431 100644 --- a/ipatests/test_xmlrpc/test_sudorule_plugin.py +++ b/ipatests/test_xmlrpc/test_sudorule_plugin.py @@ -496,8 +496,8 @@ class test_sudorule(XMLRPC_test): ) except errors.ValidationError, e: assert unicode(e) == ("invalid 'externalhost': only letters, " + - "numbers, _, and - are allowed. " + - "DNS label may not start or end with -") + "numbers, '_', '-' are allowed. " + + "DNS label may not start or end with '-'") else: assert False -- 1.8.3.1
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
