[Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation
Petr Vobornik
pvoborni at redhat.com
Thu Jun 11 11:13:30 UTC 2015
On 06/10/2015 03:25 PM, Martin Basti wrote:
> 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 :-)
>>
> Patches 233-244 can be pushed.
>
233.6, 234.6 pushed to
master:
* 9aa6124b39267148c4c1b9a8ee4209fb859b9c42 DNSSEC: Improve global
forwarders validation
* f8c8c360f1957a39ce98df61752abbfa1df9864b DNSSEC: validate forward zone
forwarders
ipa-4-1:
* e8f39566eb8bf73ac907f7db74fbc8fc78ce9e12 DNSSEC: Improve global
forwarders validation
* 9a90ef2982573db216fac1c23406aa70bc4f32e4 DNSSEC: validate forward zone
forwarders
--
Petr Vobornik
More information about the Freeipa-devel
mailing list