[Freeipa-devel] [PATCH] 0006 Remove check for alphabetic only characters from domain name validation

Fri Mar 15 11:11:36 UTC 2013

On 03/15/2013 12:08 PM, Petr Spacek wrote:
> On 14.3.2013 15:08, Martin Kosek wrote:
>> On 03/11/2013 01:02 PM, Ana Krivokapic wrote:
>>> 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 at 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.
>>>
>>
>> The zone with this name can be created even without the --force flag.
>>
>> # ipa dnszone-add example.zone1 --name-server `hostname`.
>> Administrator e-mail address [hostmaster.example.zone1.]:
>>>>> Administrator e-mail address: top level domain label must be alphabetic
>> (RFC 1123 section 2.1)
>> Administrator e-mail address [hostmaster.example.zone1.]:
>> hostmaster.example.zone.
>>    Zone name: example.zone1
>>    Authoritative nameserver: vm-037.idm.lab.bos.redhat.com.
>>    Administrator e-mail address: hostmaster.example.zone.
>>    SOA serial: 1363268183
>>    SOA refresh: 3600
>>    SOA retry: 900
>>    SOA expire: 1209600
>>    SOA minimum: 3600
>>    BIND update policy: grant IDM.LAB.BOS.REDHAT.COM krb5-self * A; grant
>> IDM.LAB.BOS.REDHAT.COM krb5-self
>>                        * AAAA; grant IDM.LAB.BOS.REDHAT.COM krb5-self * SSHFP;
>>    Active zone: TRUE
>>    Dynamic update: FALSE
>>    Allow query: any;
>>    Allow transfer: none;
>>
>> I thought that the check would only be surpassed with the --force flag.
>>
>> All this struggle with this patch makes me thinking if we are not making it too
>> complicated. I am rather inclined to drop this particular check altogether. I
>> do not see that much harm if user creates an IPA managed zone "example.zone1".
>> Petr, what is your opinion on dropping this check in IPA? Is it OK to drop it?
>
> IMHO we can drop the whole check, it should be safe. I did some basic tests
> with latest bind-dyndb-ldap and it worked for me.
>

Ok, I agree. Ana, can you please send a revised patch which rather drops this 
check altogether?

Thanks,
Martin