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.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 9859fa0a4a3e48833c48e22c4be518f28f623a60 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic <akriv...@redhat.com>
Date: Wed, 20 Feb 2013 04:25:32 -0500
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.

https://fedorahosted.org/freeipa/ticket/3385
---
 ipalib/util.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

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.2

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to