[Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation
Martin Basti
mbasti at redhat.com
Thu Jun 11 13:18:46 UTC 2015
On 04/06/15 17:28, Petr Spacek wrote:
> 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 at 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 at 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 :-)
>
ACK for pspacek-0030
--
Martin Basti
More information about the Freeipa-devel
mailing list