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

[Martin Basti]

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.
1.0/25.1.168.192.in-addr.arpa. is also valid, it could be used to CNAME
records
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.
> 
> Honza
> 


-- 
Martin^2 Basti