On 03/05/2013 01:04 PM, Petr Spacek wrote:
> Hello,
> 
> please see my comments in-line.
> 
> On 5.3.2013 12:23, Martin Kosek wrote:
>> These relatively straightforward patches depend on each other, so I am 
>> sending
>> them in bulk. Details can be found in commit messages.
>>
>> Martin
>>
>>
>>
>> freeipa-mkosek-379-improve-cname-record-validation.patch
>>
>>
>>  From 5afde12a1a3d46a89af340e060fd1c687c7f4948 Mon Sep 17 00:00:00 2001
>> From: Martin Kosek<[email protected]>
>> Date: Mon, 4 Mar 2013 15:05:49 +0100
>> Subject: [PATCH 2/3] Improve CNAME record validation
>>
>> Refacto DNS RR conflict validator so that it is better extensible in
>> the future. Also check that there is only one CNAME defined for
>> a DNS record.
>>
>> https://fedorahosted.org/freeipa/ticket/3450
>> ---
>>   ipalib/plugins/dns.py                | 43 
>> ++++++++++++++++++++++--------------
>>   tests/test_xmlrpc/test_dns_plugin.py |  8 +++++++
>>   2 files changed, 35 insertions(+), 16 deletions(-)
>>
>> diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
>> index
>> a23d1b8233eec14825ac6b43f509de51ad0ff1f7..a70d69fad181c90466467482a6ac604a166d728b
>> 100644
>> --- a/ipalib/plugins/dns.py
>> +++ b/ipalib/plugins/dns.py
>> @@ -2267,23 +2267,34 @@ class dnsrecord(LDAPObject):
>>
>>       def check_record_type_collisions(self, old_entry, entry_attrs):
>>           # Test that only allowed combination of record types was created
>> -        attrs = set(attr for attr in entry_attrs.keys() if attr in
>> _record_attributes
>> -                        and entry_attrs[attr])
>> -        attrs.update(attr for attr in old_entry.keys() if attr not in
>> entry_attrs)
>> +        rrattrs = {}
>> +        if old_entry is not None:
>> +            old_rrattrs = dict((key, value) for key, value in
>> old_entry.iteritems()
>> +                            if key in self.params and
>> +                            isinstance(self.params[key], DNSRecord))
>> +            rrattrs.update(old_rrattrs)
>> +        new_rrattrs = dict((key, value) for key, value in
>> entry_attrs.iteritems()
>> +                        if key in self.params and
>> +                        isinstance(self.params[key], DNSRecord))
>> +        rrattrs.update(new_rrattrs)
>> +
>> +        # CNAME record validation
>>           try:
>> -            attrs.remove('cnamerecord')
>> +            cnames = rrattrs['cnamerecord']
>>           except KeyError:
>> -            rec_has_cname = False
>> +            pass
>>           else:
>> -            rec_has_cname = True
>> -        # CNAME and PTR record combination is allowed
> I remember some discussion about PTR and CNAMEs, but now I see that was silly.
> CNAME can't coexist with any other record (under same name).
> 
>> -        attrs.discard('ptrrecord')
>> -        rec_has_other_types = True if attrs else False
>> -
>> -        if rec_has_cname and rec_has_other_types:
>> -            raise errors.ValidationError(name='cnamerecord',
>> -                      error=_('CNAME record is not allowed to coexist with
>> any other '
>> -                              'records except PTR'))
>> +            if cnames is not None:
>> +                if len(cnames) > 1:
>> +                    raise errors.ValidationError(name='cnamerecord',
>> +                        error=_('only one CNAME record is allowed per name
>> (RFC 6672)'))
> RFC 6672 defines DNAME, not CNAME. For CNAME please use "RFC 2136 section
> 1.1.5". RFCs are huge, so section numbers are really handy!
> 
>> +                if any(rrvalue is not None
>> +                       and rrattr != 'cnamerecord'
>> +                       and rrattr != 'ptrrecord'
>> +                       for rrattr, rrvalue in rrattrs.iteritems()):
>> +                    raise errors.ValidationError(name='cnamerecord',
>> +                          error=_('CNAME record is not allowed to coexist
>> with any other '
>> +                                  'records except PTR'))
> The same applies here - CNAME is not allowed to co-exist with any other type.
> 
>>
>>   api.register(dnsrecord)
>>
>> @@ -2433,7 +2444,7 @@ class dnsrecord_add(LDAPCreate):
>>           try:
>>               (dn_, old_entry) = ldap.get_entry(dn, _record_attributes)
>>           except errors.NotFound:
>> -            pass
>> +            old_entry = None
>>           else:
>>               for attr in entry_attrs.keys():
>>                   if attr not in _record_attributes:
>> @@ -2446,7 +2457,7 @@ class dnsrecord_add(LDAPCreate):
>>                       vals = list(entry_attrs[attr])
>>                   entry_attrs[attr] = list(set(old_entry.get(attr, []) + 
>> vals))
>>
>> -            self.obj.check_record_type_collisions(old_entry, entry_attrs)
>> +        self.obj.check_record_type_collisions(old_entry, entry_attrs)
>>           return dn
>>
>>       def exc_callback(self, keys, options, exc, call_func, *call_args,
>> **call_kwargs):
>> diff --git a/tests/test_xmlrpc/test_dns_plugin.py
>> b/tests/test_xmlrpc/test_dns_plugin.py
>> index
>> 1902484949aeb0c96a0f2cda294fd3e6ae6e086f..7b2e5731395a52d26603d1d8fb2f061b7b7e1f8a
>> 100644
>> --- a/tests/test_xmlrpc/test_dns_plugin.py
>> +++ b/tests/test_xmlrpc/test_dns_plugin.py
>> @@ -785,6 +785,14 @@ class test_dns(Declarative):
>>           ),
>>
>>           dict(
>> +            desc='Try to add multiple CNAME record %r using dnsrecord_add' %
>> (dnsrescname),
>> +            command=('dnsrecord_add', [dnszone1, dnsrescname], 
>> {'cnamerecord':
>> +                [u'1.example.com.', u'2.example.com.']}),
>> +            expected=errors.ValidationError(name='cnamerecord',
>> +                error=u'only one CNAME record is allowed per name (RFC 
>> 6672)'),
> Please replace "RFC 6672" with "RFC 2136 section 1.1.5".
> 
>> +        ),
>> +
>> +        dict(
>>               desc='Add CNAME record to %r using dnsrecord_add' % 
>> (dnsrescname),
>>               command=('dnsrecord_add', [dnszone1, dnsrescname],
>> {'cnamerecord': u'foo-1.example.com.'}),
>>               expected={
>> -- 1.8.1.4
> 
> 
>> freeipa-mkosek-380-improve-dname-record-validation.patch
>> +        # DNAME record validation
>> +        try:
>> +            dnames = rrattrs['dnamerecord']
>> +        except KeyError:
>> +            pass
>> +        else:
>> +            if dnames is not None:
>> +                if len(dnames) > 1:
>> +                    raise errors.ValidationError(name='dnamerecord',
>> +                        error=_('only one DNAME record is allowed per name '
>> +                                '(RFC 6672)'))
> Please replace "RFC 6672" with "​RFC 6672 section 2.4".
> 
>> +                # DNAME must not coexist with CNAME, but this is already
> checked earlier
>> +                if rrattrs.get('nsrecord') and keys[1] != _dns_zone_record:
>> +                    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)'))
> Please replace "RFC 6672" with "​RFC 6672 section 2.3".
> 
> Sorry for nitpicking :-)
> 


Thanks, fixed version attached.

Martin
From 4b3eaa57705eecfc0bc6161316fb3f7ced49eab0 Mon Sep 17 00:00:00 2001
From: Martin Kosek <[email protected]>
Date: Mon, 4 Mar 2013 12:48:05 +0100
Subject: [PATCH 1/3] Change CNAME and DNAME attributes to single valued

These DNS attributeTypes are of a singleton type, update LDAP schema
to reflect it.

https://fedorahosted.org/freeipa/ticket/3440
https://fedorahosted.org/freeipa/ticket/3450
---
 install/share/60ipadns.ldif           | 4 ++--
 install/updates/10-bind-schema.update | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/install/share/60ipadns.ldif b/install/share/60ipadns.ldif
index 9697227fb7166b3711568ddea3e5c345277befa3..6293385d62ce10dd3020ad291a947ff0f0d67c6e 100644
--- a/install/share/60ipadns.ldif
+++ b/install/share/60ipadns.ldif
@@ -21,14 +21,14 @@ attributeTypes: (1.3.6.1.4.1.2428.20.1.35 NAME 'nAPTRRecord' DESC 'Naming Author
 attributeTypes: (1.3.6.1.4.1.2428.20.1.36 NAME 'kXRecord' DESC 'Key Exchange Delegation, RFC 2230' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (1.3.6.1.4.1.2428.20.1.37 NAME 'certRecord' DESC 'certificate, RFC 2538' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (1.3.6.1.4.1.2428.20.1.38 NAME 'a6Record' DESC 'A6 Record Type, RFC 2874' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
-attributeTypes: (1.3.6.1.4.1.2428.20.1.39 NAME 'dNameRecord' DESC 'Non-Terminal DNS Name Redirection, RFC 2672' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+attributeTypes: (1.3.6.1.4.1.2428.20.1.39 NAME 'dNameRecord' DESC 'Non-Terminal DNS Name Redirection, RFC 2672' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 SINGLE-VALUE )
 attributeTypes: (1.3.6.1.4.1.2428.20.1.43 NAME 'dSRecord' DESC 'Delegation Signer, RFC 3658' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (1.3.6.1.4.1.2428.20.1.44 NAME 'sSHFPRecord' DESC 'SSH Key Fingerprint, draft-ietf-secsh-dns-05.txt' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (1.3.6.1.4.1.2428.20.1.46 NAME 'rRSIGRecord' DESC 'RRSIG, RFC 3755' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (1.3.6.1.4.1.2428.20.1.47 NAME 'nSECRecord' DESC 'NSEC, RFC 3755' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (0.9.2342.19200300.100.1.26 NAME 'aRecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (0.9.2342.19200300.100.1.29 NAME 'nSRecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
-attributeTypes: (0.9.2342.19200300.100.1.31 NAME 'cNAMERecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+attributeTypes: (0.9.2342.19200300.100.1.31 NAME 'cNAMERecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 SINGLE-VALUE )
 attributeTypes: (0.9.2342.19200300.100.1.28 NAME 'mXRecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (0.9.2342.19200300.100.1.27 NAME 'mDRecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (2.16.840.1.113730.3.8.5.0 NAME 'idnsName' DESC 'DNS FQDN' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 SINGLE-VALUE X-ORIGIN 'IPA v2' )
diff --git a/install/updates/10-bind-schema.update b/install/updates/10-bind-schema.update
index 3c43c8ec79fe6cb9830a27fb2880b6ed0cf0d8e4..cbe7a672b5300d5b945bf996a596909008dda5aa 100644
--- a/install/updates/10-bind-schema.update
+++ b/install/updates/10-bind-schema.update
@@ -78,3 +78,5 @@ add:objectClasses:
 
 dn: cn=schema
 replace:objectClasses:( 2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC 'Zone class' SUP idnsRecord STRUCTURAL MUST ( idnsZoneActive $$ idnsSOAmName $$ idnsSOArName $$ idnsSOAserial $$ idnsSOArefresh $$ idnsSOAretry $$ idnsSOAexpire $$ idnsSOAminimum ) MAY idnsUpdatePolicy )::( 2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC 'Zone class' SUP idnsRecord    STRUCTURAL MUST ( idnsName $$ idnsZoneActive $$ idnsSOAmName $$ idnsSOArName $$ idnsSOAserial $$ idnsSOArefresh $$ idnsSOAretry $$ idnsSOAexpire $$ idnsSOAminimum ) MAY ( idnsUpdatePolicy $$ idnsAllowQuery $$ idnsAllowTransfer $$ idnsAllowSyncPTR $$ idnsForwardPolicy $$ idnsForwarders ) )
+replace:attributeTypes:"(1.3.6.1.4.1.2428.20.1.39 NAME 'dNameRecord' DESC 'Non-Terminal DNS Name Redirection, RFC 2672' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26)::( 1.3.6.1.4.1.2428.20.1.39 NAME 'dNameRecord' DESC 'Non-Terminal DNS Name Redirection, RFC 2672' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 SINGLE-VALUE )"
+replace:attributeTypes: (0.9.2342.19200300.100.1.31 NAME 'cNAMERecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26)::( 0.9.2342.19200300.100.1.31 NAME 'cNAMERecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 SINGLE-VALUE )
-- 
1.8.1.4

From f8426acc9b0792ccc7406f25b3b637e49b832b61 Mon Sep 17 00:00:00 2001
From: Martin Kosek <[email protected]>
Date: Mon, 4 Mar 2013 15:05:49 +0100
Subject: [PATCH 2/3] Improve CNAME record validation

Refactor DNS RR conflict validator so that it is better extensible in
the future. Also check that there is only one CNAME defined for
a DNS record.

PTR+CNAME record combination is no longer allowed as we found out it
does not make sense to have this combination.

https://fedorahosted.org/freeipa/ticket/3450
---
 ipalib/plugins/dns.py                | 43 ++++++++++++++++++++++--------------
 tests/test_xmlrpc/test_dns_plugin.py | 30 +++++++++----------------
 2 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index a23d1b8233eec14825ac6b43f509de51ad0ff1f7..2ea5e706146dc44620f5a97365df4c3c8a604222 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -2267,23 +2267,34 @@ class dnsrecord(LDAPObject):
 
     def check_record_type_collisions(self, old_entry, entry_attrs):
         # Test that only allowed combination of record types was created
-        attrs = set(attr for attr in entry_attrs.keys() if attr in _record_attributes
-                        and entry_attrs[attr])
-        attrs.update(attr for attr in old_entry.keys() if attr not in entry_attrs)
+        rrattrs = {}
+        if old_entry is not None:
+            old_rrattrs = dict((key, value) for key, value in old_entry.iteritems()
+                            if key in self.params and
+                            isinstance(self.params[key], DNSRecord))
+            rrattrs.update(old_rrattrs)
+        new_rrattrs = dict((key, value) for key, value in entry_attrs.iteritems()
+                        if key in self.params and
+                        isinstance(self.params[key], DNSRecord))
+        rrattrs.update(new_rrattrs)
+
+        # CNAME record validation
         try:
-            attrs.remove('cnamerecord')
+            cnames = rrattrs['cnamerecord']
         except KeyError:
-            rec_has_cname = False
+            pass
         else:
-            rec_has_cname = True
-        # CNAME and PTR record combination is allowed
-        attrs.discard('ptrrecord')
-        rec_has_other_types = True if attrs else False
-
-        if rec_has_cname and rec_has_other_types:
-            raise errors.ValidationError(name='cnamerecord',
-                      error=_('CNAME record is not allowed to coexist with any other '
-                              'records except PTR'))
+            if cnames is not None:
+                if len(cnames) > 1:
+                    raise errors.ValidationError(name='cnamerecord',
+                        error=_('only one CNAME record is allowed per name '
+                                '(RFC 2136, section 1.1.5)'))
+                if any(rrvalue is not None
+                       and rrattr != 'cnamerecord'
+                       for rrattr, rrvalue in rrattrs.iteritems()):
+                    raise errors.ValidationError(name='cnamerecord',
+                          error=_('CNAME record is not allowed to coexist '
+                                  'with any other record'))
 
 api.register(dnsrecord)
 
@@ -2433,7 +2444,7 @@ class dnsrecord_add(LDAPCreate):
         try:
             (dn_, old_entry) = ldap.get_entry(dn, _record_attributes)
         except errors.NotFound:
-            pass
+            old_entry = None
         else:
             for attr in entry_attrs.keys():
                 if attr not in _record_attributes:
@@ -2446,7 +2457,7 @@ class dnsrecord_add(LDAPCreate):
                     vals = list(entry_attrs[attr])
                 entry_attrs[attr] = list(set(old_entry.get(attr, []) + vals))
 
-            self.obj.check_record_type_collisions(old_entry, entry_attrs)
+        self.obj.check_record_type_collisions(old_entry, entry_attrs)
         return dn
 
     def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs):
diff --git a/tests/test_xmlrpc/test_dns_plugin.py b/tests/test_xmlrpc/test_dns_plugin.py
index 1902484949aeb0c96a0f2cda294fd3e6ae6e086f..5482c94741f2208163a3700353e4e994cbe3006a 100644
--- a/tests/test_xmlrpc/test_dns_plugin.py
+++ b/tests/test_xmlrpc/test_dns_plugin.py
@@ -773,7 +773,7 @@ class test_dns(Declarative):
             desc='Try to add CNAME record to %r using dnsrecord_add' % (dnsres1),
             command=('dnsrecord_add', [dnszone1, dnsres1], {'cnamerecord': u'foo-1.example.com.'}),
             expected=errors.ValidationError(name='cnamerecord',
-                error=u'CNAME record is not allowed to coexist with any other records except PTR'),
+                error=u'CNAME record is not allowed to coexist with any other record'),
         ),
 
         dict(
@@ -785,6 +785,14 @@ class test_dns(Declarative):
         ),
 
         dict(
+            desc='Try to add multiple CNAME record %r using dnsrecord_add' % (dnsrescname),
+            command=('dnsrecord_add', [dnszone1, dnsrescname], {'cnamerecord':
+                [u'1.example.com.', u'2.example.com.']}),
+            expected=errors.ValidationError(name='cnamerecord',
+                error=u'only one CNAME record is allowed per name (RFC 2136, section 1.1.5)'),
+        ),
+
+        dict(
             desc='Add CNAME record to %r using dnsrecord_add' % (dnsrescname),
             command=('dnsrecord_add', [dnszone1, dnsrescname], {'cnamerecord': u'foo-1.example.com.'}),
             expected={
@@ -803,14 +811,14 @@ class test_dns(Declarative):
             desc='Try to add other record to CNAME record %r using dnsrecord_add' % (dnsrescname),
             command=('dnsrecord_add', [dnszone1, dnsrescname], {'arecord': u'10.0.0.1'}),
             expected=errors.ValidationError(name='cnamerecord',
-                error=u'CNAME record is not allowed to coexist with any other records except PTR'),
+                error=u'CNAME record is not allowed to coexist with any other record'),
         ),
 
         dict(
             desc='Try to add other record to CNAME record %r using dnsrecord_mod' % (dnsrescname),
             command=('dnsrecord_mod', [dnszone1, dnsrescname], {'arecord': u'10.0.0.1'}),
             expected=errors.ValidationError(name='cnamerecord',
-                error=u'CNAME record is not allowed to coexist with any other records except PTR'),
+                error=u'CNAME record is not allowed to coexist with any other record'),
         ),
 
         dict(
@@ -1063,22 +1071,6 @@ class test_dns(Declarative):
         ),
 
         dict(
-            desc='Test that CNAME/PTR record type combination in record %r is allowed' % (dnsrev1),
-            command=('dnsrecord_add', [revdnszone1, dnsrev1], {'cnamerecord': u'foo-1.example.com.' }),
-            expected={
-                'value': dnsrev1,
-                'summary': None,
-                'result': {
-                    'objectclass': objectclasses.dnsrecord,
-                    'dn': dnsrev1_dn,
-                    'idnsname': [dnsrev1],
-                    'ptrrecord': [u'foo-1.example.com.'],
-                    'cnamerecord': [u'foo-1.example.com.'],
-                },
-            },
-        ),
-
-        dict(
             desc='Update global DNS settings',
             command=('dnsconfig_mod', [], {'idnsforwarders' : [u'80.142.15.80'],}),
             expected={
-- 
1.8.1.4

From 151b9c792273dcc272167747cdd51f6911df9b9b Mon Sep 17 00:00:00 2001
From: Martin Kosek <[email protected]>
Date: Wed, 6 Mar 2013 09:27:22 +0100
Subject: [PATCH 3/3] Improve DNAME record validation

Extend DNS RR conflict check and forbid DNAME+NS combination unless
it is done in root DNS zone record.

Add tests to verify this enforced check.

https://fedorahosted.org/freeipa/ticket/3449
---
 ipalib/plugins/dns.py                | 24 ++++++++--
 tests/test_xmlrpc/test_dns_plugin.py | 89 ++++++++++++++++++++++++++++++++----
 2 files changed, 101 insertions(+), 12 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 2ea5e706146dc44620f5a97365df4c3c8a604222..c7d516f4520a2e82506617812c1dce9357f748d5 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -2265,7 +2265,7 @@ class dnsrecord(LDAPObject):
                 processed.append(rrparam.name)
                 yield rrparam
 
-    def check_record_type_collisions(self, old_entry, entry_attrs):
+    def check_record_type_collisions(self, keys, old_entry, entry_attrs):
         # Test that only allowed combination of record types was created
         rrattrs = {}
         if old_entry is not None:
@@ -2296,6 +2296,24 @@ class dnsrecord(LDAPObject):
                           error=_('CNAME record is not allowed to coexist '
                                   'with any other record'))
 
+        # DNAME record validation
+        try:
+            dnames = rrattrs['dnamerecord']
+        except KeyError:
+            pass
+        else:
+            if dnames is not None:
+                if len(dnames) > 1:
+                    raise errors.ValidationError(name='dnamerecord',
+                        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 keys[1] != _dns_zone_record:
+                    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)'))
+
 api.register(dnsrecord)
 
 
@@ -2457,7 +2475,7 @@ class dnsrecord_add(LDAPCreate):
                     vals = list(entry_attrs[attr])
                 entry_attrs[attr] = list(set(old_entry.get(attr, []) + vals))
 
-        self.obj.check_record_type_collisions(old_entry, entry_attrs)
+        self.obj.check_record_type_collisions(keys, old_entry, entry_attrs)
         return dn
 
     def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs):
@@ -2558,7 +2576,7 @@ class dnsrecord_mod(LDAPUpdate):
                 new_dnsvalue = [param._convert_scalar(modified_parts)]
                 entry_attrs[attr] = list(set(old_entry[attr] + new_dnsvalue))
 
-        self.obj.check_record_type_collisions(old_entry, entry_attrs)
+        self.obj.check_record_type_collisions(keys, old_entry, entry_attrs)
         return dn
 
     def execute(self, *keys, **options):
diff --git a/tests/test_xmlrpc/test_dns_plugin.py b/tests/test_xmlrpc/test_dns_plugin.py
index 5482c94741f2208163a3700353e4e994cbe3006a..4b095bd81a669b71823b6382f50a5821c7145435 100644
--- a/tests/test_xmlrpc/test_dns_plugin.py
+++ b/tests/test_xmlrpc/test_dns_plugin.py
@@ -53,6 +53,8 @@ dnsrev2 = u'81'
 dnsrev2_dn = DN(('idnsname',dnsrev2), revdnszone1_dn)
 dnsrescname = u'testcnamerec'
 dnsrescname_dn = DN(('idnsname',dnsrescname), dnszone1_dn)
+dnsresdname = u'testdns-dname'
+dnsresdname_dn = DN(('idnsname',dnsresdname), dnszone1_dn)
 
 class test_dns(Declarative):
 
@@ -223,15 +225,6 @@ class test_dns(Declarative):
             },
         ),
 
-        dict(
-            desc='Delete zone %r' % dnszone2,
-            command=('dnszone_del', [dnszone2], {}),
-            expected={
-                'value': dnszone2,
-                'summary': None,
-                'result': {'failed': u''},
-            },
-        ),
 
         dict(
             desc='Retrieve zone %r' % dnszone1,
@@ -836,6 +829,84 @@ class test_dns(Declarative):
         ),
 
         dict(
+            desc='Try to add multiple DNAME records to %r using dnsrecord_add' % (dnsresdname),
+            command=('dnsrecord_add', [dnszone1, dnsres1], {'dnamerecord':
+                [u'foo-1.example.com.', u'foo-2.example.com.']}),
+            expected=errors.ValidationError(name='dnamerecord',
+                error=u'only one DNAME record is allowed per name (RFC 6672, section 2.4)'),
+        ),
+
+        dict(
+            desc='Try to add invalid DNAME record %r using dnsrecord_add' % (dnsresdname),
+            command=('dnsrecord_add', [dnszone1, dnsresdname], {'dnamerecord': u'-.example.com.'}),
+            expected=errors.ValidationError(name='target',
+                error=u'invalid domain-name: only letters, numbers, and - ' +
+                    u'are allowed. DNS label may not start or end with -'),
+        ),
+
+        dict(
+            desc='Add DNAME record to %r using dnsrecord_add' % (dnsresdname),
+            command=('dnsrecord_add', [dnszone1, dnsresdname],
+                {'dnamerecord': u'd.example.com.', 'arecord': u'10.0.0.1'}),
+            expected={
+                'value': dnsresdname,
+                'summary': None,
+                'result': {
+                    'objectclass': objectclasses.dnsrecord,
+                    'dn': dnsresdname_dn,
+                    'idnsname': [dnsresdname],
+                    'dnamerecord': [u'd.example.com.'],
+                    'arecord': [u'10.0.0.1'],
+                },
+            },
+        ),
+
+        dict(
+            desc='Try to add CNAME record to %r using dnsrecord_add' % (dnsresdname),
+            command=('dnsrecord_add', [dnszone1, dnsresdname], {'cnamerecord': u'foo-1.example.com.'}),
+            expected=errors.ValidationError(name='cnamerecord',
+                error=u'CNAME record is not allowed to coexist with any other record'),
+        ),
+
+        dict(
+            desc='Try to add NS record to %r using dnsrecord_add' % (dnsresdname),
+            command=('dnsrecord_add', [dnszone1, dnsresdname],
+                {'nsrecord': u'%s.%s.' % (dnsres1, dnszone1)}),
+            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)'),
+        ),
+
+        dict(
+            desc='Add NS+DNAME record to %r zone record using dnsrecord_add' % (dnszone2),
+            command=('dnsrecord_add', [dnszone2, u'@'],
+                {'dnamerecord': u'd.example.com.',
+                 'nsrecord': dnszone1_mname}),
+            expected = {
+                'value': u'@',
+                'summary': None,
+                'result': {
+                    'objectclass': objectclasses.dnszone,
+                    'dnamerecord': [u'd.example.com.'],
+                    'dn': dnszone2_dn,
+                    'nsrecord': [dnszone2_mname, dnszone1_mname],
+                    'idnsname': [u'@']
+                }
+            },
+        ),
+
+
+        dict(
+            desc='Delete zone %r' % dnszone2,
+            command=('dnszone_del', [dnszone2], {}),
+            expected={
+                'value': dnszone2,
+                'summary': None,
+                'result': {'failed': u''},
+            },
+        ),
+
+        dict(
             desc='Try to add invalid KX record %r using dnsrecord_add' % (dnsres1),
             command=('dnsrecord_add', [dnszone1, dnsres1], {'kxrecord': u'foo-1.example.com' }),
             expected=errors.ValidationError(name='kx_rec',
-- 
1.8.1.4

_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to