On 02/09/14 17:16, Petr Spacek wrote:
On 20.8.2014 19:26, Martin Basti wrote:
Part of DNSSEC
Patches attached.
NACK
# ipa dnsrecord-add ipa.example. ds '--ds-rec=1 2 3 4'
ipa: ERROR: invalid 'dsrecord': DS record requires to coexist with an
NS record (RFC 4529, section 4.6)
RFC number is incorrect. IMHO it should also reference 'RFC 4035
section 2.4'.
Also, there is one hole:
Current code allows you to add DS RR to existing NS and then to remove
NS.
Let me know if adding a check to -del is too hard, maybe we can live
without it...
dnsrecord-del validation added
Updated patch attached
Required in ipa 4.1 but this could be pushed to 4.0.x too
--
Martin Basti
From 072206c859f52b801926fab6a75f4c4f574040ea Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Wed, 20 Aug 2014 18:51:25 +0200
Subject: [PATCH 1/2] DNSSEC: fix DS record validation
Part of: https://fedorahosted.org/freeipa/ticket/3801
---
ipalib/plugins/dns.py | 99 ++++++++++++++++++++++++++++++++-------------------
1 file changed, 63 insertions(+), 36 deletions(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index aba30dd3f3ca2f06058a05f5c0350e1a3e8eb2e5..e4822003fd8ca2b2caa9c4db9b812488b2023bdc 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -2610,6 +2610,14 @@ class dnsrecord(LDAPObject):
doc=_('Parse all raw DNS records and return them in a structured way'),
)
+ def _dsrecord_pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
+ assert isinstance(dn, DN)
+ dsrecords = entry_attrs.get('dsrecord')
+ if dsrecords and self.is_pkey_zone_record(*keys):
+ raise errors.ValidationError(
+ name='dsrecord',
+ error=unicode(_('DS record must not be in zone apex (RFC 4035 section 2.4)')))
+
def _nsrecord_pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
assert isinstance(dn, DN)
nsrecords = entry_attrs.get('nsrecord')
@@ -2868,8 +2876,9 @@ class dnsrecord(LDAPObject):
processed.append(rrparam.name)
yield rrparam
- def check_record_type_collisions(self, keys, old_entry, entry_attrs):
- # Test that only allowed combination of record types was created
+ def updated_rrattrs(self, old_entry, entry_attrs):
+ """Returns updated RR attributes
+ """
rrattrs = {}
if old_entry is not None:
old_rrattrs = dict((key, value) for key, value in old_entry.iteritems()
@@ -2880,42 +2889,53 @@ class dnsrecord(LDAPObject):
if key in self.params and
isinstance(self.params[key], DNSRecord))
rrattrs.update(new_rrattrs)
+ return rrattrs
+
+ def check_record_type_collisions(self, keys, rrattrs):
+ # Test that only allowed combination of record types was created
# CNAME record validation
- try:
- cnames = rrattrs['cnamerecord']
- except KeyError:
- pass
- else:
- 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 (RFC 1034, section 3.6.2)'))
+ cnames = rrattrs.get('cnamerecord')
+ 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 (RFC 1034, section 3.6.2)'))
# 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 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)'))
+ dnames = rrattrs.get('dnamerecord')
+ 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 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)'))
+
+ def check_record_type_dependencies(self, keys, rrattrs):
+ # Test that all record type dependencies are satisfied
+
+ # DS record validation
+ # DS record requires to coexists with NS record
+ dsrecords = rrattrs.get('dsrecord')
+ nsrecords = rrattrs.get('nsrecord')
+ # DS record cannot be in zone apex, checked in pre-callback validators
+ if dsrecords and not nsrecords:
+ raise errors.ValidationError(
+ name='dsrecord',
+ 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}.
@@ -3260,7 +3280,9 @@ 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(keys, old_entry, entry_attrs)
+ rrattrs = self.obj.updated_rrattrs(old_entry, entry_attrs)
+ self.obj.check_record_type_dependencies(keys, rrattrs)
+ self.obj.check_record_type_collisions(keys, rrattrs)
context.dnsrecord_entry_mods = getattr(context, 'dnsrecord_entry_mods',
{})
context.dnsrecord_entry_mods[(keys[0], keys[1])] = entry_attrs.copy()
@@ -3368,7 +3390,9 @@ 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(keys, old_entry, entry_attrs)
+ rrattrs = self.obj.updated_rrattrs(old_entry, entry_attrs)
+ self.obj.check_record_type_dependencies(keys, rrattrs)
+ self.obj.check_record_type_collisions(keys, rrattrs)
context.dnsrecord_entry_mods = getattr(context, 'dnsrecord_entry_mods',
{})
@@ -3533,6 +3557,9 @@ class dnsrecord_del(LDAPUpdate):
raise errors.AttrValueNotFound(attr=attr_name, value=val)
entry_attrs[attr] = list(set(old_entry[attr]))
+ rrattrs = self.obj.updated_rrattrs(old_entry, entry_attrs)
+ self.obj.check_record_type_dependencies(keys, rrattrs)
+
del_all = False
if not self.obj.is_pkey_zone_record(*keys):
record_found = False
--
1.8.3.1
From 80426ae4a2a17485cf02e1654c468712c7b8eb10 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Wed, 20 Aug 2014 18:53:49 +0200
Subject: [PATCH 2/2] Tests: DNS dsrecord validation
Part of: https://fedorahosted.org/freeipa/ticket/3801
---
ipatests/test_xmlrpc/test_dns_plugin.py | 83 +++++++++++++++++++++++++++++++++
1 file changed, 83 insertions(+)
diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index 1876e1440eda6b357810765fe92712d750ce36f3..4b4595580b0058b25bb0552bd4f8f72a3d606d03 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -147,6 +147,12 @@ dlv_dn = DN(('idnsname', dlv), zone1_dn)
dlvrec = u'60485 5 1 2BB183AF5F22588179A53B0A98631FAD1A292118'
+ds = u'ds'
+ds_dnsname = DNSName(ds)
+ds_dn = DN(('idnsname', ds), zone1_dn)
+
+ds_rec = u'0 0 0 00'
+
tlsa = u'tlsa'
tlsa_dnsname = DNSName(tlsa)
tlsa_dn = DN(('idnsname', tlsa), zone1_dn)
@@ -1323,6 +1329,83 @@ class test_dns(Declarative):
dict(
+ desc='Try to add DS record to zone %r apex, using dnsrecord_add' % (zone1),
+ command=('dnsrecord_add', [zone1, zone1_absolute], {'dsrecord': ds_rec}),
+ expected=errors.ValidationError(
+ name="dsrecord",
+ error=u'DS record must not be in zone apex (RFC 4035 section 2.4)'
+ ),
+ ),
+
+
+ dict(
+ desc='Try to add DS record %r without NS record in RRset, using dnsrecord_add' % (ds),
+ command=('dnsrecord_add', [zone1, ds], {'dsrecord': ds_rec}),
+ expected=errors.ValidationError(
+ name="dsrecord",
+ error=u'DS record requires to coexist with an NS record (RFC 4592 section 4.6, RFC 4035 section 2.4)'
+ ),
+ ),
+
+
+ dict(
+ desc='Add NS record to %r using dnsrecord_add' % (ds),
+ command=('dnsrecord_add', [zone1, ds],
+ {'nsrecord': zone1_ns}),
+ expected={
+ 'value': ds_dnsname,
+ 'summary': None,
+ 'result': {
+ 'objectclass': objectclasses.dnsrecord,
+ 'dn': ds_dn,
+ 'idnsname': [ds_dnsname],
+ 'nsrecord': [zone1_ns],
+ },
+ },
+ ),
+
+
+ dict(
+ desc='Add DS record to %r using dnsrecord_add' % (ds),
+ command=('dnsrecord_add', [zone1, ds],
+ {'dsrecord': ds_rec}),
+ expected={
+ 'value': ds_dnsname,
+ 'summary': None,
+ 'result': {
+ 'objectclass': objectclasses.dnsrecord,
+ 'dn': ds_dn,
+ 'idnsname': [ds_dnsname],
+ 'nsrecord': [zone1_ns],
+ 'dsrecord': [ds_rec],
+ },
+ },
+ ),
+
+
+ dict(
+ desc='Try to delete NS record (with DS record) %r using dnsrecord_del' % (ds),
+ command=('dnsrecord_del', [zone1, ds],
+ {'nsrecord': zone1_ns}),
+ expected=errors.ValidationError(
+ name="dsrecord",
+ error=u'DS record requires to coexist with an NS record (RFC 4592 section 4.6, RFC 4035 section 2.4)'
+ ),
+ ),
+
+
+ dict(
+ desc='Delete NS+DS record %r in zone %r' % (ds, zone1),
+ command=('dnsrecord_del', [zone1, ds], {'nsrecord': zone1_ns, 'dsrecord': ds_rec}),
+ expected={
+ 'value': [ds_dnsname],
+ 'summary': u'Deleted record "%s"' % ds,
+ 'result': {'failed': []},
+ },
+ ),
+
+
+ dict(
desc='Delete record %r in zone %r' % (dlv, zone1),
command=('dnsrecord_del', [zone1, dlv], {'del_all': True}),
expected={
--
1.8.3.1
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
