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.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From d322f309f9e646489d5328f7bb417dc2387f33b6 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic <akriv...@redhat.com>
Date: Fri, 22 Feb 2013 09:25:39 -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.

This patch also adds --force option to ipa dnsrecord-add to allow
this command to skip domain name validation.

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

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 61c2de3217e206bfcd3c68fa6a3f4d2611d0815e..5531f96687fdaabe39798f0cc2a72f4f28fdba68 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -2296,7 +2296,6 @@ 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'),
         ),
         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.2

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

Reply via email to