On 03/09/14 12:30, Martin Kosek wrote:
On 09/02/2014 05:38 PM, Petr Spacek wrote:
On 21.8.2014 19:21, Martin Basti wrote:
During work on DNSSEC we found a wrong validation of NS records
Patch 0113 fixes an error in tests caused by bind-dyndb-ldap bug
https://fedorahosted.org/bind-dyndb-ldap/ticket/123
Patches attached.
Functional ACK. It can be pushed if Python gurus don't see any problem.

I think the patches will need a rebase before push, I cannot apply them to my
tree. The Python part itself looked good to me.

Martin

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Rebased patch attached, due changes in freeipa-mbasti-0109,
patches mbasti-0109.2, mbasti-0110.2 are required.

--
Martin Basti

From be5dd565252ccf089e52a378656f070e889b3eaa Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Thu, 21 Aug 2014 18:08:10 +0200
Subject: [PATCH 1/3] DNS fix NS record coexistence validator

NS can coexistent only with A, AAAA, DS, NS record
---
 ipalib/plugins/dns.py | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index e4822003fd8ca2b2caa9c4db9b812488b2023bdc..c7039bbf9c4c52a5a12ba6b806d3a0d414144fb8 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -2916,11 +2916,23 @@ class dnsrecord(LDAPObject):
                     error=_('only one DNAME record is allowed per name '
                             '(RFC 6672, section 2.4)'))
             # DNAME must not coexist with CNAME, but this is already checked earlier
-            if rrattrs.get('nsrecord') and not keys[1].is_empty():
-                raise errors.ValidationError(name='dnamerecord',
-                      error=_('DNAME record is not allowed to coexist with an '
-                              'NS record except when located in a zone root '
-                              'record (RFC 6672, section 2.3)'))
+
+        # NS record validation
+        # NS record can coexist only with A, AAAA, DS, and other NS records (except zone apex)
+        # RFC 2181 section 6.1,
+        allowed_records = ['AAAA', 'A', 'DS', 'NS']
+        nsrecords = rrattrs.get('nsrecord')
+        if nsrecords and not self.is_pkey_zone_record(*keys):
+            for r_type in _record_types:
+                if (r_type not in allowed_records
+                    and rrattrs.get('%srecord' % r_type.lower())
+                ):
+                    raise errors.ValidationError(
+                        name='nsrecord',
+                        error=_('NS record is not allowed to coexist with an '
+                                '%(type)s record except when located in a '
+                                'zone root record (RFC 2181, section 6.1)') %
+                                {'type': r_type})
 
     def check_record_type_dependencies(self, keys, rrattrs):
         # Test that all record type dependencies are satisfied
@@ -2936,7 +2948,6 @@ class dnsrecord(LDAPObject):
                 error=_('DS record requires to coexist with an '
                          'NS record (RFC 4592 section 4.6, RFC 4035 section 2.4)'))
 
-
     def _entry2rrsets(self, entry_attrs, dns_name, dns_domain):
         '''Convert entry_attrs to a dictionary {rdtype: rrset}.
 
-- 
1.8.3.1

From 02b3613c6cc42ad741293442f50fc7a9a8425a09 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Thu, 21 Aug 2014 18:09:22 +0200
Subject: [PATCH 2/3] Test: DNS NS validation

---
 ipatests/test_xmlrpc/test_dns_plugin.py | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index 4b4595580b0058b25bb0552bd4f8f72a3d606d03..1784e0b17e87cffc037184c1ab06d2dfe0454ac1 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -123,6 +123,10 @@ name1_dn = DN(('idnsname',name1), zone1_dn)
 name1_renamed = u'testdnsres-renamed'
 name1_renamed_dnsname = DNSName(name1_renamed)
 
+name_ns = u'testdnsres-ns'
+name_ns_dnsname = DNSName(name_ns)
+name_ns_dn = DN(('idnsname',name_ns), zone1_dn)
+
 revname1 = u'80'
 revname1_dnsname = DNSName(revname1)
 revname1_ip = revzone1_ipprefix + revname1
@@ -1168,9 +1172,10 @@ class test_dns(Declarative):
             desc='Try to add NS record to %r using dnsrecord_add' % (dname),
             command=('dnsrecord_add', [zone1, dname],
                 {'nsrecord': u'%s.%s.' % (name1, zone1)}),
-            expected=errors.ValidationError(name='dnamerecord',
-                error=u'DNAME record is not allowed to coexist with an NS '
-                      u'record except when located in a zone root record (RFC 6672, section 2.3)'),
+            expected=errors.ValidationError(name='nsrecord',
+                error=u'NS record is not allowed to coexist with an DNAME '
+                      u'record except when located in a zone root record '
+                      '(RFC 2181, section 6.1'),
         ),
 
         dict(
@@ -1245,32 +1250,29 @@ class test_dns(Declarative):
 
 
         dict(
-            desc='Try to add unresolvable absolute NS record to %r using dnsrecord_add' % (name1),
-            command=('dnsrecord_add', [zone1, name1], {'nsrecord': absnxname}),
+            desc='Try to add unresolvable absolute NS record to %r using dnsrecord_add' % (name_ns),
+            command=('dnsrecord_add', [zone1, name_ns], {'nsrecord': absnxname}),
             expected=errors.NotFound(reason=u"Nameserver '%s' does not have a corresponding A/AAAA record" % absnxname),
         ),
 
         dict(
-            desc='Try to add unresolvable relative NS record to %r using dnsrecord_add' % (name1),
-            command=('dnsrecord_add', [zone1, name1], {'nsrecord': relnxname}),
+            desc='Try to add unresolvable relative NS record to %r using dnsrecord_add' % (name_ns),
+            command=('dnsrecord_add', [zone1, name_ns], {'nsrecord': relnxname}),
             expected=errors.NotFound(reason=u"Nameserver '%s.%s.' does not "
                 "have a corresponding A/AAAA record" % (relnxname, zone1)),
         ),
 
         dict(
-            desc='Add unresolvable NS record with --force to %r using dnsrecord_add' % (name1),
-            command=('dnsrecord_add', [zone1, name1], {'nsrecord': absnxname,
+            desc='Add unresolvable NS record with --force to %r using dnsrecord_add' % (name_ns),
+            command=('dnsrecord_add', [zone1, name_ns], {'nsrecord': absnxname,
                                                             'force' : True}),
             expected={
-                'value': name1_dnsname,
+                'value': name_ns_dnsname,
                 'summary': None,
                 'result': {
                     'objectclass': objectclasses.dnsrecord,
-                    'dn': name1_dn,
-                    'idnsname': [name1_dnsname],
-                    'arecord': [arec3],
-                    'kxrecord': [u'1 foo-1'],
-                    'txtrecord': [u'foo bar'],
+                    'dn': name_ns_dn,
+                    'idnsname': [name_ns_dnsname],
                     'nsrecord': [absnxname],
                 },
             },
@@ -1294,7 +1296,6 @@ class test_dns(Declarative):
                     'arecord': [arec3],
                     'kxrecord': [u'1 foo-1'],
                     'txtrecord': [u'foo bar'],
-                    'nsrecord': [absnxname],
                 },
             },
         ),
-- 
1.8.3.1

From 997cf2e3e65bc39c5db3fce6507ff49752f07025 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Thu, 21 Aug 2014 19:11:27 +0200
Subject: [PATCH 3/3] Fix DNS record rename test

bind-dyndb-ldap's bug caused test failure
https://fedorahosted.org/bind-dyndb-ldap/ticket/123

Owners with NS record works with the bug
---
 ipatests/test_xmlrpc/test_dns_plugin.py | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index 1784e0b17e87cffc037184c1ab06d2dfe0454ac1..74f4d4a74432d410fa9a80e1206224a7c6115344 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -120,12 +120,12 @@ revzone3_classless2_permission_dn = DN(('cn', revzone3_classless2_permission),
 name1 = u'testdnsres'
 name1_dnsname = DNSName(name1)
 name1_dn = DN(('idnsname',name1), zone1_dn)
-name1_renamed = u'testdnsres-renamed'
-name1_renamed_dnsname = DNSName(name1_renamed)
 
 name_ns = u'testdnsres-ns'
 name_ns_dnsname = DNSName(name_ns)
 name_ns_dn = DN(('idnsname',name_ns), zone1_dn)
+name_ns_renamed = u'testdnsres-ns-renamed'
+name_ns_renamed_dnsname = DNSName(name_ns_renamed)
 
 revname1 = u'80'
 revname1_dnsname = DNSName(revname1)
@@ -1175,7 +1175,7 @@ class test_dns(Declarative):
             expected=errors.ValidationError(name='nsrecord',
                 error=u'NS record is not allowed to coexist with an DNAME '
                       u'record except when located in a zone root record '
-                      '(RFC 2181, section 6.1'),
+                      '(RFC 2181, section 6.1)'),
         ),
 
         dict(
@@ -1280,34 +1280,33 @@ class test_dns(Declarative):
 
         dict(
             desc='Try to to rename DNS zone %r root record' % (zone1),
-            command=('dnsrecord_mod', [zone1, u'@'], {'rename': name1_renamed,}),
+            command=('dnsrecord_mod', [zone1, u'@'], {'rename': u'renamed-zone',}),
             expected=errors.ValidationError(name='rename',
                            error=u'DNS zone root record cannot be renamed')
         ),
 
+
         dict(
-            desc='Rename DNS record %r to %r' % (name1, name1_renamed),
-            command=('dnsrecord_mod', [zone1, name1], {'rename': name1_renamed,}),
+            desc='Rename DNS record %r to %r' % (name_ns, name_ns_renamed),
+            command=('dnsrecord_mod', [zone1, name_ns], {'rename': name_ns_renamed,}),
             expected={
-                'value': name1_dnsname,
+                'value': name_ns_dnsname,
                 'summary': None,
                 'result': {
-                    'idnsname': [name1_renamed_dnsname],
-                    'arecord': [arec3],
-                    'kxrecord': [u'1 foo-1'],
-                    'txtrecord': [u'foo bar'],
+                'idnsname': [name_ns_renamed_dnsname],
+                'nsrecord': [absnxname],
                 },
             },
         ),
 
 
         dict(
-            desc='Delete record %r in zone %r' % (name1_renamed, zone1),
-            command=('dnsrecord_del', [zone1, name1_renamed],
+            desc='Delete record %r in zone %r' % (name1, zone1),
+            command=('dnsrecord_del', [zone1, name1],
                      {'del_all': True}),
             expected={
-                'value': [name1_renamed_dnsname],
-                'summary': u'Deleted record "%s"' % name1_renamed,
+                'value': [name1_dnsname],
+                'summary': u'Deleted record "%s"' % name1,
                 'result': {'failed': []},
             },
         ),
-- 
1.8.3.1

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

Reply via email to