On 3.6.2015 17:14, Martin Basti wrote: > On 03/06/15 14:57, Petr Spacek wrote: >> On 18.5.2015 13:48, Martin Basti wrote: >>> On 15/05/15 18:11, Petr Spacek wrote: >>>> On 7.5.2015 18:12, Martin Basti wrote: >>>>> On 07/05/15 12:19, Petr Spacek wrote: >>>>>> On 7.5.2015 08:59, David Kupka wrote: >>>>>>> On 05/06/2015 03:20 PM, Martin Basti wrote: >>>>>>>> On 05/05/15 15:00, Martin Basti wrote: >>>>>>>>> On 30/04/15 15:37, David Kupka wrote: >>>>>>>>>> On 04/24/2015 02:56 PM, Martin Basti wrote: >>>>>>>>>>> Patches attached. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> Hi, >>>>>>>>>> thanks for patches. >>>>>>>>>> >>>>>>>>>> 1. You changed message in DNSServerNotRespondingWarning class but not >>>>>>>>>> the test in ipatest/test_xmlrpc/test_dns_plugin.py >>>>>>>>>> >>>>>>>>>> nitpick. Please spell 'edns' correctly. I've seen several instances >>>>>>>>>> of 'ends'. >>>>>>>>>> >>>>>>>>> Thank you, >>>>>>>>> >>>>>>>>> updated patches attached: >>>>>>>>> * new error messages >>>>>>>>> * logging to debug log server output if exception was raised >>>>>>>>> * fixed test >>>>>>>>> * fixed spelling >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> Fixed tests (again) >>>>>>>> >>>>>>>> Updated patches attached >>>>>>>> >>>>>>> The code looks good to me and tests are no longer broken. (I would >>>>>>> prefer >>>>>>> better fix of the tests but given that the priorities are different now >>>>>>> it can >>>>>>> wait.) >>>>>>> >>>>>>> Petr, can you please confirm that the patch set works for you? >>>>>> Sorry, NACK: >>>>>> >>>>>> $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 >>>>>> Server will check DNS forwarder(s). >>>>>> This may take some time, please wait ... >>>>>> ipa: ERROR: an internal error has occurred >>>>>> >>>>>> # /var/log/httpd/error_log >>>>>> ipa: ERROR: non-public: AssertionError: >>>>>> Traceback (most recent call last): >>>>>> File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line >>>>>> 350, in >>>>>> wsgi_execute >>>>>> result = self.Command[name](*args, **options) >>>>>> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line >>>>>> 443, in >>>>>> __call__ >>>>>> ret = self.run(*args, **options) >>>>>> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line >>>>>> 760, >>>>>> in run >>>>>> return self.execute(*args, **options) >>>>>> File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line >>>>>> 4444, in >>>>>> execute >>>>>> **options) >>>>>> File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line >>>>>> 4405, in >>>>>> _warning_if_forwarders_do_not_work >>>>>> log=self.log) >>>>>> File "/usr/lib/python2.7/site-packages/ipalib/util.py", line 715, in >>>>>> validate_dnssec_zone_forwarder_step2 >>>>>> timeout=timeout) >>>>>> File "/usr/lib/python2.7/site-packages/ipalib/util.py", line 610, in >>>>>> _resolve_record >>>>>> assert isinstance(nameserver_ip, basestring) >>>>>> AssertionError >>>>>> ipa: INFO: [jsonserver_session] admin@IPA.EXAMPLE: >>>>>> dnsforwardzone_add(<DNS >>>>>> name ptr.test.>, idnsforwarders=(u'10.34.47.236',), all=False, raw=False, >>>>>> version=u'2.116'): AssertionError >>>>>> >>>>>> This is constantly reproducible in my vm-090.abc. Let me know if you >>>>>> want to >>>>>> take a look. >>>>>> >>>>>> >>>>>> I'm attaching little response.patch which improves compatibility with >>>>>> older >>>>>> python-dns packages. This patch allows IPA to work while error messages >>>>>> are >>>>>> simply not as nice as they could be with latest python-dns :-) >>>>>> >>>>>> check_fwd_msg.patch is a little nitpick, just to make sure everyone >>>>>> understands the message. >>>>>> >>>>>> BTW why some messages in check_forwarders() are printed using 'print' and >>>>>> others using logger? I would prefer to use logger for everything to make >>>>>> sure >>>>>> that logs contain all the information, including warnings. >>>>>> >>>>>> Thank you for your time! >>>>>> >>>>> Thank you, fixed. >>>>> >>>>> I added missing except block after forwarders validation step2. >>>> I confirm that this works but I just discovered another deficiency. >>>> >>>> Setup: >>>> - DNSSEC validation is enabled on IPA server >>>> - forwarders uses fake TLD, e.g. 'test.' >>>> - remote DNS server is responding, supports EDNS0 and so on >>>> >>>> $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 >>>> Server will check DNS forwarder(s). >>>> This may take some time, please wait ... >>>> ipa: WARNING: DNS server 10.34.78.90: query 'ptr.test. SOA': The DNS query >>>> name does not exist: ptr.test.. >>>> >>>> Huh? Let's check named log: >>>> forward zone 'ptr.test': loaded >>>> validating ./SOA: got insecure response; parent indicates it should be >>>> secure >>>> >>>> Sometimes I get SERVFAIL from IPA server, too. >>>> >>>> >>>> Unfortunately this check was the main reason for writing this patchset so >>>> we >>>> need to improve it. >>>> >>>> Maybe validate_dnssec_zone_forwarder_step2() could special-case NXDOMAIN >>>> and >>>> print the DNSSEC-validation-failed error, too? The problem is that it could >>>> trigger some false positives because NXDOMAIN may simply be caused by a >>>> delay >>>> somewhere. >>>> >>>> Any ideas? >>> I add catch block for NXDOMAIN >>>> By the way, this is also weird: >>>> >>>> $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 >>>> Server will check DNS forwarder(s). >>>> This may take some time, please wait ... >>>> ipa: ERROR: DNS forward zone with name "ptr.test." already exists >>>> >>>> Is it actually doing the check even if the forward zone exists already? >>>> (This >>>> is just nitpick, not a blocker!) >>>> >>> The first part is written by IPA client, it is not response from server. >>> It is just written when user use --forwarder option. >>> >>> Updated patch attached. >> NACK, it does not work for me - it explodes when I try to add a forward zone: >> >> $ ipa dnsforwardzone-add ptr.test. --forwarder=192.0.2.1 >> >> ipa: ERROR: non-public: TypeError: _warning_if_forwarders_do_not_work() got >> multiple values for keyword argument 'new_zone' >> Traceback (most recent call last): >> File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 350, >> in >> wsgi_execute >> result = self.Command[name](*args, **options) >> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 443, in >> __call__ >> ret = self.run(*args, **options) >> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 760, in >> run >> return self.execute(*args, **options) >> File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 4461, >> in >> execute >> result, new_zone=True, *keys, **options) >> TypeError: _warning_if_forwarders_do_not_work() got multiple values for >> keyword argument 'new_zone' >> ipa: INFO: [jsonserver_session] admin@IPA.EXAMPLE: dnsforwardzone_add(<DNS >> name ptr.test.>, idnsforwarders=(u'192.0.2.1',), all=False, raw=False, >> version=u'2.123'): TypeError >> > updated patch attached.
Attached patch fixes the case where one domain is shadowed by another domain. ACK for your patches, please review my patch :-) -- Petr^2 Spacek
From b91af8979c9fd06a7c522d56e1638bafc95a25ea Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Thu, 4 Jun 2015 17:27:03 +0200 Subject: [PATCH] DNSSEC: Detect zone shadowing with incorrect DNSSEC signatures. https://fedorahosted.org/freeipa/ticket/4657 --- ipalib/messages.py | 2 +- ipalib/util.py | 26 ++++++++++++++------------ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/ipalib/messages.py b/ipalib/messages.py index 84f0a722d3c82a9626a9b404a7134305fcd71dfa..58ae1f3ecbbf139f6f584c0ea2ebea6eb92e6e2b 100644 --- a/ipalib/messages.py +++ b/ipalib/messages.py @@ -237,7 +237,7 @@ class DNSSECValidationFailingWarning(PublicMessage): errno = 13010 type = "warning" format = _(u"DNSSEC validation failed: %(error)s.\n" - u"Please verify your DNSSEC signatures or disable DNSSEC " + u"Please verify your DNSSEC configuration or disable DNSSEC " u"validation on all IPA servers.") diff --git a/ipalib/util.py b/ipalib/util.py index 5810c774a670d300bf60326ac438e6993d34f611..44478a2d1eed6d66e54949e0840e6d62310830c5 100644 --- a/ipalib/util.py +++ b/ipalib/util.py @@ -582,8 +582,8 @@ class DNSSECSignatureMissingError(ForwarderValidationError): class DNSSECValidationError(ForwarderValidationError): - format = _("requested record '%(owner)s %(rtype)s' was refused by IPA " - "server %(ip)s because DNSSEC signature is not valid") + format = _("record '%(owner)s %(rtype)s' " + "failed DNSSEC validation on server %(ip)s") def _log_response(log, e): @@ -702,38 +702,40 @@ def validate_dnssec_zone_forwarder_step1(ip_addr, fwzone, log=None, timeout=10): def validate_dnssec_zone_forwarder_step2(ipa_ip_addr, fwzone, log=None, timeout=10): """ - This step must be executed after forwarders is added into LDAP, and only + This step must be executed after forwarders are added into LDAP, and only when we are sure the forwarders work. Query will be send to IPA DNS server, to verify if reply passed, or DNSSEC validation failed. Only forwarders in forward zones can be validated in this way :raise UnresolvableRecordError: record cannot be resolved :raise DNSSECValidationError: response from forwarder is not DNSSEC valid """ rtype = "SOA" try: - _resolve_record(fwzone, rtype, nameserver_ip=ipa_ip_addr, edns0=True, - timeout=timeout) + ans_cd = _resolve_record(fwzone, rtype, nameserver_ip=ipa_ip_addr, + edns0=True, dnssec=True, flag_cd=True, + timeout=timeout) except DNSException as e: _log_response(log, e) - else: - return try: - _resolve_record(fwzone, rtype, nameserver_ip=ipa_ip_addr, dnssec=True, - flag_cd=True, timeout=timeout) + ans_do = _resolve_record(fwzone, rtype, nameserver_ip=ipa_ip_addr, + edns0=True, dnssec=True, timeout=timeout) except NXDOMAIN as e: # sometimes CD flag is ignored and NXDomain is returned - # this may cause false positive detection _log_response(log, e) raise DNSSECValidationError(owner=fwzone, rtype=rtype, ip=ipa_ip_addr) except DNSException as e: _log_response(log, e) raise UnresolvableRecordError(owner=fwzone, rtype=rtype, ip=ipa_ip_addr, error=e) else: - # record is not DNSSEC valid, because it can be received with CD flag - # only + if (ans_do.canonical_name == ans_cd.canonical_name + and ans_do.rrset == ans_cd.rrset): + return + # records received with and without CD flag are not equivalent: + # this might be caused by an DNSSEC validation failure in cases where + # existing zone id being 'shadowed' by another zone on forwarder raise DNSSECValidationError(owner=fwzone, rtype=rtype, ip=ipa_ip_addr) -- 2.1.0
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
