From 5b8e1e8c62bb899145e3572dc8bfa7cb0570fd05 Mon Sep 17 00:00:00 2001 From: Martin Kosek Date: Thu, 22 Mar 2012 17:35:39 +0100 Subject: [PATCH] Harden raw record processing in DNS plugin There were cases where DNS plugin was too tolerant in a raw DNS record option (---rec option. This patch hardens the processing and returns error in both described cases to make the processes clearer and more robust. All these use cases were also covered by new unit tests. https://fedorahosted.org/freeipa/ticket/2551 --- ipalib/plugins/dns.py | 30 ++++++++++-------- tests/test_xmlrpc/test_dns_plugin.py | 46 ++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index 7b22f2f7b..eef6ab1de 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -2131,6 +2131,7 @@ class dnsrecord_add(LDAPCreate): def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): precallback_attrs = [] + processed_attrs = [] for option in options: try: param = self.params[option] @@ -2142,13 +2143,19 @@ class dnsrecord_add(LDAPCreate): continue if 'dnsrecord_part' in param.flags: - if rrparam.name in entry_attrs: + if rrparam.name in processed_attrs: # this record was already entered continue + if rrparam.name in entry_attrs: + # this record is entered both via parts and raw records + raise errors.ValidationError(name=param.cli_name or param.name, + error=_('Raw value of a DNS record was already set by "%(name)s" option') \ + % dict(name=rrparam.cli_name or rrparam.name)) parts = rrparam.get_parts_from_kw(options) dnsvalue = [rrparam._convert_scalar(parts)] entry_attrs[rrparam.name] = dnsvalue + processed_attrs.append(rrparam.name) continue if 'dnsrecord_extra' in param.flags: @@ -2193,6 +2200,8 @@ class dnsrecord_add(LDAPCreate): for attr in old_entry.keys(): if attr not in _record_attributes: continue + if entry_attrs[attr] is None: + entry_attrs[attr] = [] if not isinstance(entry_attrs[attr], (tuple, list)): vals = [entry_attrs[attr]] else: @@ -2244,26 +2253,23 @@ class dnsrecord_mod(LDAPUpdate): # check if any attr should be updated using structured instead of replaced # format is recordname : (old_value, new_parts) updated_attrs = {} - for attr in entry_attrs: - param = self.params[attr] - if not isinstance(param, DNSRecord): - continue - + for param in self.obj.iterate_rrparams_by_parts(options, skip_extra=True): parts = param.get_parts_from_kw(options, raise_on_none=False) if parts is None: # old-style modification continue - if isinstance(entry_attrs[attr], (tuple, list)): - if len(entry_attrs[attr]) > 1: + old_value = entry_attrs.get(param.name) + if not old_value: + raise errors.RequirementError(name=param.name) + if isinstance(old_value, (tuple, list)): + if len(old_value) > 1: raise errors.ValidationError(name=param.name, error=_('DNS records can be only updated one at a time')) - old_value = entry_attrs[attr][0] - else: - old_value = entry_attrs[attr] + old_value = old_value[0] - updated_attrs[attr] = (old_value, parts) + updated_attrs[param.name] = (old_value, parts) # Run pre_callback validators self.obj.run_precallback_validators(dn, entry_attrs, *keys, **options) diff --git a/tests/test_xmlrpc/test_dns_plugin.py b/tests/test_xmlrpc/test_dns_plugin.py index 93093ec8a..1e5ab7917 100644 --- a/tests/test_xmlrpc/test_dns_plugin.py +++ b/tests/test_xmlrpc/test_dns_plugin.py @@ -621,6 +621,18 @@ class test_dns(Declarative): expected=errors.ValidationError(name='srv_part_target', error=''), ), + dict( + desc='Try to add SRV record to zone %r both via parts and a raw value' % (dnszone1), + command=('dnsrecord_add', [dnszone1, u'_foo._tcp'], {'srv_part_priority': 0, + 'srv_part_weight' : 0, + 'srv_part_port' : 123, + 'srv_part_target' : u'foo.bar.', + 'srvrecord': [u"1 100 1234 %s" \ + % dnszone1_mname]}), + expected=errors.ValidationError(name='srv_target', + error='Raw value of a DNS record was already set by a_rec option'), + ), + dict( desc='Add SRV record to zone %r using dnsrecord_add' % (dnszone1), command=('dnsrecord_add', [dnszone1, u'_foo._tcp'], {'srvrecord': u"0 100 1234 %s" % dnszone1_mname}), @@ -636,6 +648,40 @@ class test_dns(Declarative): }, ), + dict( + desc='Try to modify SRV record in zone %r without specifying modified value' % (dnszone1), + command=('dnsrecord_mod', [dnszone1, u'_foo._tcp'], {'srv_part_priority': 1,}), + expected=errors.RequirementError(name='srvrecord'), + ), + + dict( + desc='Try to modify SRV record in zone %r with non-existent modified value' % (dnszone1), + command=('dnsrecord_mod', [dnszone1, u'_foo._tcp'], {'srv_part_priority': 1, + 'srvrecord' : [u"0 100 1234 does.not.exist."] }), + expected=errors.AttrValueNotFound(attr='SRV', value=u'0 100 1234 ns1.dnszone.test.'), + ), + + dict( + desc='Try to modify SRV record in zone %r with invalid part value' % (dnszone1), + command=('dnsrecord_mod', [dnszone1, u'_foo._tcp'], {'srv_part_priority': 100000, + 'srvrecord' : [u"0 100 1234 %s" % dnszone1_mname] }), + expected=errors.ValidationError(name='srv_priority', error=u'can be at most 65535'), + ), + + dict( + desc='Modify SRV record in zone %r using parts' % (dnszone1), + command=('dnsrecord_mod', [dnszone1, u'_foo._tcp'], {'srv_part_priority': 1, + 'srvrecord' : [u"0 100 1234 %s" % dnszone1_mname] }), + expected={ + 'value': u'_foo._tcp', + 'summary': None, + 'result': { + 'idnsname': [u'_foo._tcp'], + 'srvrecord': [u"1 100 1234 %s" % dnszone1_mname], + }, + }, + ), + dict( desc='Try to add invalid LOC record to zone %r using dnsrecord_add' % (dnszone1), command=('dnsrecord_add', [dnszone1, u'@'], {'locrecord': u"91 11 42.4 N 16 36 29.6 E 227.64" }),