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

Fri Mar 15 11:08:57 UTC 2013

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.

-- 
Petr^2 Spacek