[Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation
Petr Spacek
pspacek at redhat.com
Thu Jun 4 15:28:22 UTC 2015
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 :-)
--
Petr^2 Spacek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pspacek-0030-DNSSEC-Detect-zone-shadowing-with-incorrect-DNSSEC-s.patch
Type: text/x-patch
Size: 4106 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150604/2e5dd4e1/attachment.bin>
More information about the Freeipa-devel
mailing list