On 02/27/2013 10:58 AM, Martin Kosek wrote: > On 02/22/2013 04:02 PM, Ana Krivokapic wrote: >> On 02/22/2013 10:19 AM, Petr Spacek wrote: >>> On 20.2.2013 11:03, Ana Krivokapic wrote: >>>> On 02/18/2013 01:08 PM, Martin Kosek wrote: >>>>> On 02/18/2013 12:47 PM, Sumit Bose wrote: >>>>>> On Mon, Feb 18, 2013 at 12:27:35PM +0100, Petr Spacek wrote: >>>>>>> On 15.2.2013 15:22, Ana Krivokapic wrote: >>>>>>>> Hello, >>>>>>>> >>>>>>>> The .isalpha() check in validate_domain_name() was too strict, >>>>>>>> causing some commands like ipa dnsrecord-add to fail. >>>>>>>> >>>>>>>> https://fedorahosted.org/freeipa/ticket/3385 >>>>>>> I would add --force option rather than removing whole check, if >>>>>>> it's possible. >>>>>>> >>>>>>> Would it be possible to mention RFC in the error message? Something >>>>>>> like _('top level domain label must be alphabetic (RFC 1123 section >>>>>>> 2.1)') >>>>>>> ? >>>>>>> >>>>>>> IMHO it is handy, because it educates users. >>>>>> The problem is that this check is always done on the last component of >>>>>> the domain_name even if it is just a sub-domain of the FreeIPA domain, >>>>>> where e.g. numbers are valid characters. >>>>>> >>>>>> At the beginning of validate_domain_name() a trailing '.' is stripped >>>>>> away. iirc the trailing '.' is an indication for a complete, fully >>>>>> qualified name. Would it work if the presence of the trailing '.' is >>>>>> saved and the check is only done if there was a '.'? >>>>>> >>>>>> bye, >>>>>> Sumit >>>>>> >>>>> Sure. Though I am now not 100% sure that some IPA functions do not >>>>> use this >>>>> validator with a fqdn hostname without trailing dot. If not, I am >>>>> for fixing >>>>> this function as Sumit and Petr suggested. >>>>> >>>>> Martin >>>>> >>>>> _______________________________________________ >>>>> Freeipa-devel mailing list >>>>> Freeipa-devel@redhat.com >>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel >>>> After some thought, I decided to change the approach. >>>> >>>> As pointed out by Sumit, the problem was that the validate_domain_name() >>>> function did not distinguish between fqdn and non-fqdn domains >>>> (subdomains of the IPA domain). The trailing dot is not a clear >>>> indication either, because some IPA functions use this validator with an >>>> fqdn without the trailing dot. >>>> >>>> To fix this, I introduced an additional parameter to this function - a >>>> flag which indicates whether the domain name is an fqdn or not. The is >>>> .isalpha() check is then performed only in the case of an fqdn. >>>> >>>> I also improved the error message to mention the relevant RFC, as >>>> suggested by Petr. >>> Please don't forget to add --force switch. It could be handy. >>> >> I added the --force switch to ipa dnsrecord-add and opened a new ticket >> to handle the rest of the ipa commands that use domain name validation: >> https://fedorahosted.org/freeipa/ticket/3455 >> >> Updated patch is attached. >> > This patch fixed validation only partially. The --force flag you made > available > will not allow admin to for example add a zone "example.zone1" which > technically will be resolvable, it is just not a good practice: > > # ipa dnszone-add example.zone1 --name-server `hostname`. --force > ipa: ERROR: invalid 'name': top level domain label must be alphabetic (RFC > 1123 > section 2.1) > > To enable this, I think you would need to not postpone the validation to DNS > zone pre_callback as you could not check --force flag presence right in the > idnsname parameter validator. > > We may also want to change --force flag label, it now talks only about NS > record validation, but we now expanded it a bit, so the label would need to be > more general. > > Martin
I added the fix for dnszone-add and edited the label of the --force flag to make it more general. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc.
From 76360400f4eca52f27e27d5d79f0c1eaa4220c42 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic <akriv...@redhat.com> Date: Mon, 11 Mar 2013 07:04:05 -0400 Subject: [PATCH] Improve domain name validation Check for alphabetic only characters is now performed only in the case of fully qualified domain names. This fixes the bug that caused some ipa commands (e.g. ipa dnsrecord-add) to fail due to the presence of numbers in the domain name. This patch also adds --force option to dnsrecord-add and dnszone-add to allow these commands to skip domain name validation. https://fedorahosted.org/freeipa/ticket/3385 --- ipalib/plugins/dns.py | 9 ++++----- ipalib/util.py | 10 +++++----- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index a23d1b8233eec14825ac6b43f509de51ad0ff1f7..640ad05d836a9cc6f011148b1aa20a9b525ab649 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -410,7 +410,7 @@ def _validate_bind_forwarder(ugettext, forwarder): def _domain_name_validator(ugettext, value): try: - validate_domain_name(value) + validate_domain_name(value, fqdn=False) except ValueError, e: return unicode(e) @@ -1777,7 +1777,7 @@ class dnszone_add(LDAPCreate): takes_options = LDAPCreate.takes_options + ( Flag('force', label=_('Force'), - doc=_('Force DNS zone creation even if nameserver is not resolvable.'), + doc=_('Skip DNS validation check.'), ), Str('ip_address?', _validate_ipaddr, doc=_('Add forward record for nameserver located in the created zone'), @@ -1866,7 +1866,7 @@ class dnszone_mod(LDAPUpdate): takes_options = LDAPUpdate.takes_options + ( Flag('force', label=_('Force'), - doc=_('Force nameserver change even if nameserver not in DNS'), + doc=_('Skip DNS validation check.'), ), ) @@ -2296,8 +2296,7 @@ class dnsrecord_add(LDAPCreate): takes_options = LDAPCreate.takes_options + ( Flag('force', label=_('Force'), - flags=['no_option', 'no_output'], - doc=_('force NS record creation even if its hostname is not in DNS'), + doc=_('Skip DNS validation check.'), ), dnsrecord.structured_flag, ) diff --git a/ipalib/util.py b/ipalib/util.py index 9ed27c84ec8d189507410ba141d8968fc38f674c..5a5af69eac68adc51ee5d55a8c570275f64c6aee 100644 --- a/ipalib/util.py +++ b/ipalib/util.py @@ -235,7 +235,7 @@ def validate_dns_label(dns_label, allow_underscore=False): 'DNS label may not start or end with -') \ % dict(underscore=underscore_err_msg)) -def validate_domain_name(domain_name, allow_underscore=False): +def validate_domain_name(domain_name, allow_underscore=False, fqdn=True): if domain_name.endswith('.'): domain_name = domain_name[:-1] @@ -244,9 +244,9 @@ def validate_domain_name(domain_name, allow_underscore=False): # apply DNS name validator to every name part map(lambda label:validate_dns_label(label,allow_underscore), domain_name) - if not domain_name[-1].isalpha(): + if fqdn and not domain_name[-1].isalpha(): # see RFC 1123 - raise ValueError(_('top level domain label must be alphabetic')) + raise ValueError(_('top level domain label must be alphabetic (RFC 1123 section 2.1)')) def validate_zonemgr(zonemgr): """ See RFC 1033, 1035 """ @@ -308,9 +308,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) else: - validate_domain_name(hostname,allow_underscore) + validate_domain_name(hostname, allow_underscore, check_fqdn) def normalize_sshpubkey(value): return SSHPublicKey(value).openssh() -- 1.8.1.4
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
