[Freeipa-devel] [PATCHES 0024, 0025] Classless support for reverse domains

[Martin Basti]

On Thu, 2014-02-06 at 17:04 +0100, Martin Basti wrote:
> On Thu, 2014-02-06 at 16:37 +0100, Jan Cholasta wrote:
> > On 6.2.2014 15:57, Martin Basti wrote:
> > > On Thu, 2014-02-06 at 10:59 +0100, Jan Cholasta wrote:
> > >> Hi,
> > >>
> > >> On 31.1.2014 16:06, Martin Basti wrote:
> > >>> Reverse domain names in form "0/28.0.10.10.in-addr.arpa." are now
> > >>> allowed.
> > >>>
> > >>> Ticket: https://fedorahosted.org/freeipa/ticket/4143
> > >>> Patches attached.
> > >>
> > > I add Petr2 to CC, to inspect RFC issues, with allowing '/' in IPv6
> > >
> > >> I think the validation should be more strict. IPv4 reverse zones should
> > >> allow slash only in the label for the last octet (i.e. 0/25.1.168.192 is
> > >> valid, 0.1/25.168.192 is not). IPv6 reverse zones should not allow slash
> > >> at all.
> > >>
> > > I havent found anything about IPv6, RFCs don't forbids it.
> > 
> > AFAIK the RFCs do not forbid anything, but we do validation anyway, so 
> > we might as well do it right, otherwise there is no point in doing it.
> > 
> OK, I leave there only IPv4
> 
> > > 1.0/25.1.168.192.in-addr.arpa. is also valid, it could be used to CNAME
> > > records
> > 
> > Yes, obviously. It's 1.0.1/25.168.192.in-addr.arpa. I'm concerned about.
> > 
> 
> http://tools.ietf.org/html/rfc6672#section-6.2 
> This can give a very strange positions of / in FQDN
> 
> Optionally, I could permit only 1 slash in domain name, but I have to
> inspect first if user can do something useful with subnet of subnet in
> DNS, like 1.0/25.128/25.168.192.in-addr.arpa
> > > The slashes in domain names are referenced as the best practise in RFC,
> > > there are not strict rules.
> > >>
> > >> +def _cname_hostname_validator(ugettext, value):
> > >>
> > >> Can you name this _bind_cname_hostname_validator, so that it is clear it
> > >> is related to _bind_hostname_validator?
> > >>
> > > I will rename it
> > >
> > >>
> > >> +        #classless reverse zones can contain slash '/'
> > >> +        if not zone_is_reverse(normalized_zone) and
> > >> (normalized_zone.count('/') > 0):
> > >> +            raise errors.ValidationError(name='name',
> > >> +                        error=_("Only reverse zones can contain '/' in
> > >> labels"))
> > >>
> > >> This should be handled in _domain_name_validator. Validation in
> > >> pre_callback should be done only when the validation depends on values
> > >> of multiple parameters, which is not this case.
> > >>
> > > I will move it
> > >>
> > >> +    def _reverse_zone_pre_callback(self, ldap, dn, entry_attrs, *keys,
> > >> **options):
> > >>
> > >> Rename this to _idnsname_pre_callback and you won't have to call it
> > >> explicitly in run_precallback_validators.
> > >>
> > > I will rename it
> > >>
> > >> +            if addr.count('/') > 0:
> > >>
> > >> I think "if '/' in addr:" would be better.
> > >>
> > > I will change it
> > >
> > >>
> > >> -def validate_dns_label(dns_label, allow_underscore=False):
> > >> +def validate_dns_label(dns_label, allow_underscore=False,
> > >> allow_slash=False):
> > >>
> > >> IMO instead of adding a new boolean argument, it would be nicer to
> > >> replace allow_underscore with an argument (e.g. allowed_chars) which
> > >> takes a string of extra allowed characters.
> > >>
> > > But I have to handle not only allowed chars, but position of the chars
> > > in the label string too.
> > 
> > Why? Is there a RFC that forbids it?
> > 
> > My point is, adding a new argument for each extra character is bad, 
> > there should be a better way of doing that.
> > 
> I agree, but for example: "_" should be at start (it is not required be
> at the start in IPA), "/" and "-" in the middle.
> 

Updated patch attached.
Patch for tests is the same as previos.
-- 
Martin^2 Basti
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0024-2-DNS-classless-support-for-reverse-domains.patch
Type: text/x-patch
Size: 9118 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140207/321c3b25/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0025-DNS-tests-for-classless-reverse-domains.patch
Type: text/x-patch
Size: 14149 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140207/321c3b25/attachment-0001.bin>