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

2014-02-11 Thread Martin Basti
On Mon, 2014-02-10 at 14:28 +0100, Jan Cholasta wrote:
 On 10.2.2014 13:14, Martin Basti wrote:
  On Mon, 2014-02-10 at 12:22 +0100, Jan Cholasta wrote:
  On 10.2.2014 08:50, Petr Spacek wrote:
  On 7.2.2014 10:42, Martin Basti wrote:
  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
 
  For the record, we discussed this off-line with Martin and Petr and
  figured out it would be best to allow slashes in IPv6 reverse zones
  after all.
 
 
  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
 
  Point taken.
 
 
  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
  Multiple slashes has to be allowed, without limitation to last octet.
  Imagine situation when split subnet is later split to even smaller pieces.
 
  Guys, do not over-engineer it. IMHO this validator should produce a
  warning is something is not as we expect but it should not block user
  from adding a record.
 
  We have had enough problems with too strict validators in the past and
  IMHO warning is the way to go.
 
  I agree, but it's too late to get such change into 3.3.x.
 
 
  Petr^2 Spacek
 
  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.
 
  OK then. (But I still don't like it.)
 
 
 
  Updated patch attached.
  Patch for tests is the same as previos.
 
  +validate_domain_name(value, allow_slash=True)
  +
  +#classless reverse zones can contain slash '/'
  +normalized_zone = normalize_zone(value)
  +if not zone_is_reverse(normalized_zone) and ('/' in
  normalized_zone):
  +return uOnly reverse zones can contain '/' in labels
 
  You don't need to enclose x in y in parentheses. Also I don't think
  there is any value in pointing out that slash can be used for reverse
  zones when giving an error for non-reverse zones. I would prefer
  something like this instead:
 
normalized_zone = normalize_zone(value)
validate_domain_mame(value,
 

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

2014-02-11 Thread Jan Cholasta

On 11.2.2014 14:29, Martin Basti wrote:

On Mon, 2014-02-10 at 14:28 +0100, Jan Cholasta wrote:

On 10.2.2014 13:14, Martin Basti wrote:

On Mon, 2014-02-10 at 12:22 +0100, Jan Cholasta wrote:

On 10.2.2014 08:50, Petr Spacek wrote:

On 7.2.2014 10:42, Martin Basti wrote:

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


For the record, we discussed this off-line with Martin and Petr and
figured out it would be best to allow slashes in IPv6 reverse zones
after all.




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


Point taken.



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

Multiple slashes has to be allowed, without limitation to last octet.
Imagine situation when split subnet is later split to even smaller pieces.

Guys, do not over-engineer it. IMHO this validator should produce a
warning is something is not as we expect but it should not block user
from adding a record.

We have had enough problems with too strict validators in the past and
IMHO warning is the way to go.


I agree, but it's too late to get such change into 3.3.x.



Petr^2 Spacek


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.


OK then. (But I still don't like it.)





Updated patch attached.
Patch for tests is the same as previos.


+validate_domain_name(value, allow_slash=True)
+
+#classless reverse zones can contain slash '/'
+normalized_zone = normalize_zone(value)
+if not zone_is_reverse(normalized_zone) and ('/' in
normalized_zone):
+return uOnly reverse zones can contain '/' in labels

You don't need to enclose x in y in parentheses. Also I don't think
there is any value in pointing out that slash can be used for reverse
zones when giving an error for non-reverse zones. I would prefer
something like this instead:

   normalized_zone = normalize_zone(value)
   validate_domain_mame(value,
allow_slash=zone_is_reverse(normalized_zone))


+def _idnsname_pre_callback(self, ldap, dn, entry_attrs, *keys,
**options):
+#in reverse zone can a record name 

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

2014-02-11 Thread Martin Basti
On Tue, 2014-02-11 at 15:42 +0100, Jan Cholasta wrote:
 On 11.2.2014 14:29, Martin Basti wrote:
  On Mon, 2014-02-10 at 14:28 +0100, Jan Cholasta wrote:
  On 10.2.2014 13:14, Martin Basti wrote:
  On Mon, 2014-02-10 at 12:22 +0100, Jan Cholasta wrote:
  On 10.2.2014 08:50, Petr Spacek wrote:
  On 7.2.2014 10:42, Martin Basti wrote:
  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
 
  For the record, we discussed this off-line with Martin and Petr and
  figured out it would be best to allow slashes in IPv6 reverse zones
  after all.
 
 
  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
 
  Point taken.
 
 
  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
  Multiple slashes has to be allowed, without limitation to last octet.
  Imagine situation when split subnet is later split to even smaller 
  pieces.
 
  Guys, do not over-engineer it. IMHO this validator should produce a
  warning is something is not as we expect but it should not block user
  from adding a record.
 
  We have had enough problems with too strict validators in the past and
  IMHO warning is the way to go.
 
  I agree, but it's too late to get such change into 3.3.x.
 
 
  Petr^2 Spacek
 
  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.
 
  OK then. (But I still don't like it.)
 
 
 
  Updated patch attached.
  Patch for tests is the same as previos.
 
  +validate_domain_name(value, allow_slash=True)
  +
  +#classless reverse zones can contain slash '/'
  +normalized_zone = normalize_zone(value)
  +if not zone_is_reverse(normalized_zone) and ('/' in
  normalized_zone):
  +return uOnly reverse zones can contain '/' in labels
 
  You don't need to enclose x in y in parentheses. Also I don't think
  there is any value in pointing out that slash can be used for reverse
  zones when giving an error for non-reverse zones. I would prefer
  

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

2014-02-11 Thread Jan Cholasta

On 11.2.2014 16:23, Martin Basti wrote:

On Tue, 2014-02-11 at 15:42 +0100, Jan Cholasta wrote:

On 11.2.2014 14:29, Martin Basti wrote:

On Mon, 2014-02-10 at 14:28 +0100, Jan Cholasta wrote:

On 10.2.2014 13:14, Martin Basti wrote:

On Mon, 2014-02-10 at 12:22 +0100, Jan Cholasta wrote:

On 10.2.2014 08:50, Petr Spacek wrote:

On 7.2.2014 10:42, Martin Basti wrote:

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


For the record, we discussed this off-line with Martin and Petr and
figured out it would be best to allow slashes in IPv6 reverse zones
after all.




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


Point taken.



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

Multiple slashes has to be allowed, without limitation to last octet.
Imagine situation when split subnet is later split to even smaller pieces.

Guys, do not over-engineer it. IMHO this validator should produce a
warning is something is not as we expect but it should not block user
from adding a record.

We have had enough problems with too strict validators in the past and
IMHO warning is the way to go.


I agree, but it's too late to get such change into 3.3.x.



Petr^2 Spacek


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.


OK then. (But I still don't like it.)





Updated patch attached.
Patch for tests is the same as previos.


+validate_domain_name(value, allow_slash=True)
+
+#classless reverse zones can contain slash '/'
+normalized_zone = normalize_zone(value)
+if not zone_is_reverse(normalized_zone) and ('/' in
normalized_zone):
+return uOnly reverse zones can contain '/' in labels

You don't need to enclose x in y in parentheses. Also I don't think
there is any value in pointing out that slash can be used for reverse
zones when giving an error for non-reverse zones. I would prefer
something like this instead:

normalized_zone = normalize_zone(value)
validate_domain_mame(value,
allow_slash=zone_is_reverse(normalized_zone))


+def 

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

2014-02-11 Thread Martin Kosek
On 02/11/2014 05:11 PM, Jan Cholasta wrote:
 On 11.2.2014 16:23, Martin Basti wrote:
 On Tue, 2014-02-11 at 15:42 +0100, Jan Cholasta wrote:
 On 11.2.2014 14:29, Martin Basti wrote:
 On Mon, 2014-02-10 at 14:28 +0100, Jan Cholasta wrote:
 On 10.2.2014 13:14, Martin Basti wrote:
 On Mon, 2014-02-10 at 12:22 +0100, Jan Cholasta wrote:
 On 10.2.2014 08:50, Petr Spacek wrote:
 On 7.2.2014 10:42, Martin Basti wrote:
 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

 For the record, we discussed this off-line with Martin and Petr and
 figured out it would be best to allow slashes in IPv6 reverse zones
 after all.


 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

 Point taken.


 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
 Multiple slashes has to be allowed, without limitation to last octet.
 Imagine situation when split subnet is later split to even smaller 
 pieces.

 Guys, do not over-engineer it. IMHO this validator should produce a
 warning is something is not as we expect but it should not block user
 from adding a record.

 We have had enough problems with too strict validators in the past and
 IMHO warning is the way to go.

 I agree, but it's too late to get such change into 3.3.x.


 Petr^2 Spacek

 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.

 OK then. (But I still don't like it.)



 Updated patch attached.
 Patch for tests is the same as previos.

 +validate_domain_name(value, allow_slash=True)
 +
 +#classless reverse zones can contain slash '/'
 +normalized_zone = normalize_zone(value)
 +if not zone_is_reverse(normalized_zone) and ('/' in
 normalized_zone):
 +return uOnly reverse zones can contain '/' in labels

 You don't need to enclose x in y in parentheses. Also I don't think
 there is any value in pointing out that slash can be used for reverse
 zones when giving an error for non-reverse zones. I would prefer
 something like this instead:

 normalized_zone = 

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

2014-02-10 Thread Jan Cholasta

On 10.2.2014 08:50, Petr Spacek wrote:

On 7.2.2014 10:42, Martin Basti wrote:

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


For the record, we discussed this off-line with Martin and Petr and 
figured out it would be best to allow slashes in IPv6 reverse zones 
after all.





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


Point taken.



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

Multiple slashes has to be allowed, without limitation to last octet.
Imagine situation when split subnet is later split to even smaller pieces.

Guys, do not over-engineer it. IMHO this validator should produce a
warning is something is not as we expect but it should not block user
from adding a record.

We have had enough problems with too strict validators in the past and
IMHO warning is the way to go.


I agree, but it's too late to get such change into 3.3.x.



Petr^2 Spacek


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.


OK then. (But I still don't like it.)





Updated patch attached.
Patch for tests is the same as previos.


+validate_domain_name(value, allow_slash=True)
+
+#classless reverse zones can contain slash '/'
+normalized_zone = normalize_zone(value)
+if not zone_is_reverse(normalized_zone) and ('/' in 
normalized_zone):

+return uOnly reverse zones can contain '/' in labels

You don't need to enclose x in y in parentheses. Also I don't think 
there is any value in pointing out that slash can be used for reverse 
zones when giving an error for non-reverse zones. I would prefer 
something like this instead:


normalized_zone = normalize_zone(value)
validate_domain_mame(value, 
allow_slash=zone_is_reverse(normalized_zone))



+def _idnsname_pre_callback(self, ldap, dn, entry_attrs, *keys, 
**options):

+#in reverse zone can a record name contains /, (and -)
+
+if self.is_pkey_zone_record(*keys):
+addr = u''
+else:
+addr = keys[-1]
+
+zone = keys[-2]
+zone = 

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

2014-02-10 Thread Martin Basti
On Mon, 2014-02-10 at 12:22 +0100, Jan Cholasta wrote:
 On 10.2.2014 08:50, Petr Spacek wrote:
  On 7.2.2014 10:42, Martin Basti wrote:
  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
 
 For the record, we discussed this off-line with Martin and Petr and 
 figured out it would be best to allow slashes in IPv6 reverse zones 
 after all.
 
 
  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
 
 Point taken.
 
 
  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
  Multiple slashes has to be allowed, without limitation to last octet.
  Imagine situation when split subnet is later split to even smaller pieces.
 
  Guys, do not over-engineer it. IMHO this validator should produce a
  warning is something is not as we expect but it should not block user
  from adding a record.
 
  We have had enough problems with too strict validators in the past and
  IMHO warning is the way to go.
 
 I agree, but it's too late to get such change into 3.3.x.
 
 
  Petr^2 Spacek
 
  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.
 
 OK then. (But I still don't like it.)
 
 
 
  Updated patch attached.
  Patch for tests is the same as previos.
 
 +validate_domain_name(value, allow_slash=True)
 +
 +#classless reverse zones can contain slash '/'
 +normalized_zone = normalize_zone(value)
 +if not zone_is_reverse(normalized_zone) and ('/' in 
 normalized_zone):
 +return uOnly reverse zones can contain '/' in labels
 
 You don't need to enclose x in y in parentheses. Also I don't think 
 there is any value in pointing out that slash can be used for reverse 
 zones when giving an error for non-reverse zones. I would prefer 
 something like this instead:
 
  normalized_zone = normalize_zone(value)
  validate_domain_mame(value, 
 allow_slash=zone_is_reverse(normalized_zone))
 
 
 +def _idnsname_pre_callback(self, ldap, dn, entry_attrs, 

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

2014-02-10 Thread Jan Cholasta

On 10.2.2014 13:14, Martin Basti wrote:

On Mon, 2014-02-10 at 12:22 +0100, Jan Cholasta wrote:

On 10.2.2014 08:50, Petr Spacek wrote:

On 7.2.2014 10:42, Martin Basti wrote:

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


For the record, we discussed this off-line with Martin and Petr and
figured out it would be best to allow slashes in IPv6 reverse zones
after all.




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


Point taken.



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

Multiple slashes has to be allowed, without limitation to last octet.
Imagine situation when split subnet is later split to even smaller pieces.

Guys, do not over-engineer it. IMHO this validator should produce a
warning is something is not as we expect but it should not block user
from adding a record.

We have had enough problems with too strict validators in the past and
IMHO warning is the way to go.


I agree, but it's too late to get such change into 3.3.x.



Petr^2 Spacek


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.


OK then. (But I still don't like it.)





Updated patch attached.
Patch for tests is the same as previos.


+validate_domain_name(value, allow_slash=True)
+
+#classless reverse zones can contain slash '/'
+normalized_zone = normalize_zone(value)
+if not zone_is_reverse(normalized_zone) and ('/' in
normalized_zone):
+return uOnly reverse zones can contain '/' in labels

You don't need to enclose x in y in parentheses. Also I don't think
there is any value in pointing out that slash can be used for reverse
zones when giving an error for non-reverse zones. I would prefer
something like this instead:

  normalized_zone = normalize_zone(value)
  validate_domain_mame(value,
allow_slash=zone_is_reverse(normalized_zone))


+def _idnsname_pre_callback(self, ldap, dn, entry_attrs, *keys,
**options):
+#in reverse zone can a record name contains /, (and -)
+
+if self.is_pkey_zone_record(*keys):
+addr = u''
+

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

2014-02-09 Thread Petr Spacek

On 7.2.2014 10:42, Martin Basti wrote:

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
Multiple slashes has to be allowed, without limitation to last octet. Imagine 
situation when split subnet is later split to even smaller pieces.


Guys, do not over-engineer it. IMHO this validator should produce a warning is 
something is not as we expect but it should not block user from adding a record.


We have had enough problems with too strict validators in the past and IMHO 
warning is the way to go.


Petr^2 Spacek


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.



--
Petr^2 Spacek

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


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

2014-02-07 Thread 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
From e753d0c38384ee245ecbd350caa74fa54dcce6fd Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Fri, 31 Jan 2014 15:42:31 +0100
Subject: [PATCH] DNS classless support for reverse domains

Now users can adding reverse zones in classless form:
0/25.1.168.192.in-addr.arpa.
0-25.1.168.192.in-addr.arpa.

128/25 NS ns.example.com.
10 CNAME 10.128/25.1.168.192.in-addr.arpa.

Ticket: https://fedorahosted.org/freeipa/ticket/4143
---
 ipalib/plugins/dns.py | 55 ---
 ipalib/util.py| 32 --
 2 files changed, 66 insertions(+), 21 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index afd47be316b8b84fe281698af206b39fc1e1bf55..b8cd6a6355fbdc17d86b24f132b7abfa58752a01 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -368,25 +368,31 @@ def _normalize_bind_aci(bind_acis):
 acis += u';'
 return acis
 
-def _bind_hostname_validator(ugettext, value):
+def _bind_hostname_validator(ugettext, value, allow_slash=False):
 if value == _dns_zone_record:
 return
 try:
 # Allow domain name which is not fully qualified. These are supported
 # in bind and then translated as non-fqdn-name.domain.
-validate_hostname(value, check_fqdn=False, allow_underscore=True)
+validate_hostname(value, check_fqdn=False, allow_underscore=True, allow_slash=allow_slash)
 except ValueError, e:
 return _('invalid domain-name: %s') \
 % unicode(e)
 
 return None
 

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

2014-02-06 Thread Jan Cholasta

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 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.



+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?



+#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.



+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.



+if addr.count('/')  0:

I think if '/' in addr: would be better.


-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.



Honza

--
Jan Cholasta

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


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

2014-02-06 Thread 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

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


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

2014-02-06 Thread Jan Cholasta

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.



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.


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.


--
Jan Cholasta

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


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

2014-02-06 Thread Martin Basti
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.

-- 
Martin^2 Basti

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


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

2014-01-31 Thread Martin Basti
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.
-- 
Martin^2 Basti
From 052462c2aba165737d7fffe0a3dc2a846a008f5b Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Fri, 31 Jan 2014 15:42:31 +0100
Subject: [PATCH] DNS classless support for reverse domains

Now users can adding reverse zones in classless form:
0/25.1.168.192.in-addr.arpa.
0-25.1.168.192.in-addr.arpa.

128/25 NS ns.example.com.
10 CNAME 10.128/25.1.168.192.in-addr.arpa.

Ticket: https://fedorahosted.org/freeipa/ticket/4143
---
 ipalib/plugins/dns.py | 57 +--
 ipalib/util.py| 32 -
 2 files changed, 68 insertions(+), 21 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 57322e9bbd1ab9a9f09effc8a54fd73b5875c781..b00a84a4dcdea7fb87e67688f931108740914bcf 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -368,25 +368,31 @@ def _normalize_bind_aci(bind_acis):
 acis += u';'
 return acis
 
-def _bind_hostname_validator(ugettext, value):
+def _bind_hostname_validator(ugettext, value, allow_slash=False):
 if value == _dns_zone_record:
 return
 try:
 # Allow domain name which is not fully qualified. These are supported
 # in bind and then translated as non-fqdn-name.domain.
-validate_hostname(value, check_fqdn=False, allow_underscore=True)
+validate_hostname(value, check_fqdn=False, allow_underscore=True, allow_slash=allow_slash)
 except ValueError, e:
 return _('invalid domain-name: %s') \
 % unicode(e)
 
 return None
 
+def _cname_hostname_validator(ugettext, value):
+
+Validator for CNAME allows classless domain names (25/0.0.10.in-addr.arpa.)
+
+return _bind_hostname_validator(ugettext, value, allow_slash=True)
+
 def _dns_record_name_validator(ugettext, value):
 if value == _dns_zone_record:
 return
 
 try:
-map(lambda label:validate_dns_label(label, allow_underscore=True), \
+map(lambda label:validate_dns_label(label, allow_underscore=True, allow_slash=True), \
 value.split(u'.'))
 except ValueError, e:
 return unicode(e)
@@ -411,7 +417,7 @@ def _validate_bind_forwarder(ugettext, forwarder):
 
 def _domain_name_validator(ugettext, value):
 try:
-validate_domain_name(value)
+validate_domain_name(value, allow_slash=True)
 except ValueError, e:
 return unicode(e)
 
@@ -939,7 +945,7 @@ class CNAMERecord(DNSRecord):
 rfc = 1035
 parts = (
 Str('hostname',
-_bind_hostname_validator,
+_cname_hostname_validator,
 label=_('Hostname'),
 doc=_('A hostname which this alias hostname points to'),
 ),
@@ -960,7 +966,7 @@ class DNAMERecord(DNSRecord):
 rfc = 2672
 parts = (
 Str('target',
-_bind_hostname_validator,
+_cname_hostname_validator,
 label=_('Target'),
 ),
 )
@@ -1818,6 +1824,11 @@ class dnszone_add(LDAPCreate):
 else:
 record_in_zone = nameserver
 
+#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))
+
 if zone_is_reverse(normalized_zone):
 if not nameserver.endswith('.'):
 raise errors.ValidationError(name='name-server',
@@ -2131,6 +2142,22 @@ class dnsrecord(LDAPObject):
doc=_('Parse all raw DNS records and return them in a structured way'),
)
 
+def _reverse_zone_pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
+#in reverse zone can record name contain /, (and -)
+
+if self.is_pkey_zone_record(*keys):
+addr = u''
+else:
+addr = keys[-1]
+
+zone = keys[-2]
+zone = normalize_zone(zone)
+if not zone_is_reverse(zone):
+if addr.count('/')  0:
+raise errors.ValidationError(name='name',
+error=unicode(_('Only domain names in reverse zones can contain \'/\'')) )
+
+
 def _nsrecord_pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
 assert isinstance(dn, DN)
 nsrecords = entry_attrs.get('nsrecord')
@@ -2144,6 +2171,7 @@ class dnsrecord(LDAPObject):
 ptrrecords = entry_attrs.get('ptrrecord')
 if ptrrecords is None:
 return
+
 zone = keys[-2]
 if self.is_pkey_zone_record(*keys):
 addr = u''
@@ -2162,16 +2190,23 @@ class dnsrecord(LDAPObject):
 error=unicode(_('Reverse zone for PTR record should be a sub-zone of one the following fully