On 03/15/2013 12:11 PM, Martin Kosek wrote:
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@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


As Ana pointed out on IRC, this is actually what her first revision did.

ACK for the first incarnation of the patch, pushed to master, ipa-3-1.

Thanks,
Martin

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

Reply via email to