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

[Martin Kosek]

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?

Martin