[Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation

Mon May 18 11:48:33 UTC 2015

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.

-- 
Martin Basti

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0233.5-DNSSEC-Improve-global-forwarders-validation.patch
Type: text/x-patch
Size: 16323 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150518/832dd8ce/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0234.5-DNSSEC-validate-forward-zone-forwarders.patch
Type: text/x-patch
Size: 17608 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150518/832dd8ce/attachment-0001.bin>