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.

--
Martin Basti

From 8b14534382cdb95782483c669e6c6cc4057636d8 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Wed, 22 Apr 2015 15:29:21 +0200
Subject: [PATCH 1/2] DNSSEC: Improve global forwarders validation

Validation now provides more detailed information and less false
positives failures.

https://fedorahosted.org/freeipa/ticket/4657
---
 ipalib/messages.py                      |  23 +++++-
 ipalib/plugins/dns.py                   |  67 +++++++++-------
 ipalib/util.py                          | 130 ++++++++++++++++++++++++++------
 ipaserver/install/bindinstance.py       |  32 +++++---
 ipatests/test_xmlrpc/test_dns_plugin.py |   5 +-
 5 files changed, 190 insertions(+), 67 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index b44beca729f5483a7241e4c98a9f724ed663e70f..236b683b30692d88e5257d9189c559dd9f848885 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -179,14 +179,14 @@ class OptionSemanticChangedWarning(PublicMessage):
                u"%(hint)s")
 
 
-class DNSServerNotRespondingWarning(PublicMessage):
+class DNSServerValidationWarning(PublicMessage):
     """
-    **13006**  Used when a DNS server is not responding to queries
+    **13006**  Used when a DNS server is not to able to resolve query
     """
 
     errno = 13006
     type = "warning"
-    format = _(u"DNS server %(server)s not responding.")
+    format = _(u"DNS server %(server)s: %(error)s.")
 
 
 class DNSServerDoesNotSupportDNSSECWarning(PublicMessage):
@@ -196,10 +196,11 @@ class DNSServerDoesNotSupportDNSSECWarning(PublicMessage):
 
     errno = 13007
     type = "warning"
-    format = _(u"DNS server %(server)s does not support DNSSEC. "
+    format = _(u"DNS server %(server)s does not support DNSSEC: %(error)s.\n"
                u"If DNSSEC validation is enabled on IPA server(s), "
                u"please disable it.")
 
+
 class ForwardzoneIsNotEffectiveWarning(PublicMessage):
     """
     **13008** Forwardzone is not effective, forwarding will not work because
@@ -214,6 +215,20 @@ class ForwardzoneIsNotEffectiveWarning(PublicMessage):
                u"\"%(ns_rec)s\" to parent zone \"%(authzone)s\".")
 
 
+class DNSServerDoesNotSupportEDNS0Warning(PublicMessage):
+    """
+    **13009** Used when a DNS server does not support EDNS0, required for
+    DNSSEC support
+    """
+
+    errno = 13009
+    type = "warning"
+    format = _(u"DNS server %(server)s does not support EDNS0 (RFC 6891): "
+               u"%(error)s.\n"
+               u"If DNSSEC validation is enabled on IPA server(s), "
+               u"please disable it.")
+
+
 def iter_messages(variables, base):
     """Return a tuple with all subclasses
     """
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index f589ab5b77a918b75fe6c48b465ecd9f02cb6d42..c9dc1e547f6681731cf96ac7386a197e038e3859 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -43,7 +43,10 @@ from ipalib.util import (normalize_zonemgr,
                          get_dns_forward_zone_update_policy,
                          get_dns_reverse_zone_update_policy,
                          get_reverse_zone_default, REVERSE_DNS_ZONES,
-                         normalize_zone, validate_dnssec_forwarder)
+                         normalize_zone, validate_dnssec_global_forwarder,
+                         DNSSECSignatureMissingError, UnresolvableRecordError,
+                         EDNS0UnsupportedError)
+
 from ipapython.ipautil import CheckedIPAddress, is_host_resolvable
 from ipapython.dnsutil import DNSName
 
@@ -4261,41 +4264,47 @@ class dnsconfig_mod(LDAPUpdate):
     __doc__ = _('Modify global DNS configuration.')
 
     def interactive_prompt_callback(self, kw):
+
+        # show informative message on client side
+        # server cannot send messages asynchronous
         if kw.get('idnsforwarders', False):
-            self.Backend.textui.print_plain("Server will check forwarder(s).")
-            self.Backend.textui.print_plain("This may take some time, please wait ...")
+            self.Backend.textui.print_plain(
+                _("Server will check DNS forwarder(s)."))
+            self.Backend.textui.print_plain(
+                _("This may take some time, please wait ..."))
 
     def execute(self, *keys, **options):
         # test dnssec forwarders
-        non_dnssec_forwarders = []
-        not_responding_forwarders = []
         forwarders = options.get('idnsforwarders')
+
+        result = super(dnsconfig_mod, self).execute(*keys, **options)
+        self.obj.postprocess_result(result)
+
         if forwarders:
             for forwarder in forwarders:
-                dnssec_status = validate_dnssec_forwarder(forwarder)
-                if dnssec_status is None:
-                    not_responding_forwarders.append(forwarder)
-                elif dnssec_status is False:
-                    non_dnssec_forwarders.append(forwarder)
-
-        result = super(dnsconfig_mod, self).execute(*keys, **options)
-        self.obj.postprocess_result(result)
-
-        # add messages
-        for forwarder in not_responding_forwarders:
-            messages.add_message(
-                options['version'],
-                result, messages.DNSServerNotRespondingWarning(
-                    server=forwarder,
-                )
-            )
-        for forwarder in non_dnssec_forwarders:
-            messages.add_message(
-                options['version'],
-                result, messages.DNSServerDoesNotSupportDNSSECWarning(
-                    server=forwarder,
-                )
-            )
+                try:
+                    validate_dnssec_global_forwarder(forwarder, log=self.log)
+                except DNSSECSignatureMissingError as e:
+                    messages.add_message(
+                        options['version'],
+                        result, messages.DNSServerDoesNotSupportDNSSECWarning(
+                            server=forwarder, error=e,
+                        )
+                    )
+                except EDNS0UnsupportedError as e:
+                    messages.add_message(
+                        options['version'],
+                        result, messages.DNSServerDoesNotSupportEDNS0Warning(
+                            server=forwarder, error=e,
+                        )
+                    )
+                except UnresolvableRecordError as e:
+                    messages.add_message(
+                        options['version'],
+                        result, messages.DNSServerValidationWarning(
+                            server=forwarder, error=e
+                        )
+                    )
 
         return result
 
diff --git a/ipalib/util.py b/ipalib/util.py
index 2c17d80a0427a5c7e45a6a0b64fa1f4d39fffa8a..8d7b666386efe61994d640f9ae0f1761f7e07e2b 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -36,7 +36,7 @@ from dns import resolver, rdatatype
 from dns.exception import DNSException
 from netaddr.core import AddrFormatError
 
-from ipalib import errors
+from ipalib import errors, messages
 from ipalib.text import _
 from ipapython.ssh import SSHPublicKey
 from ipapython.dn import DN, RDN
@@ -559,38 +559,126 @@ def validate_hostmask(ugettext, hostmask):
         return _('invalid hostmask')
 
 
-def validate_dnssec_forwarder(ip_addr):
-    """Test DNS forwarder properties.
+class ForwarderValidationError(Exception):
+    format = None
 
-    :returns:
-     True if forwarder works as expected and supports DNSSEC.
-     False if forwarder does not support DNSSEC.
-     None if forwarder does not respond.
+    def __init__(self, format=None, message=None, **kw):
+        messages.process_message_arguments(self, format, message, **kw)
+        super(ForwarderValidationError, self).__init__(self.msg)
+
+
+class UnresolvableRecordError(ForwarderValidationError):
+    format = _("query '%(owner)s %(rtype)s': %(error)s")
+
+
+class EDNS0UnsupportedError(ForwarderValidationError):
+    format = _("query '%(owner)s %(rtype)s' with EDNS0: %(error)s")
+
+
+class DNSSECSignatureMissingError(ForwarderValidationError):
+    format = _("answer to query '%(owner)s %(rtype)s' is missing DNSSEC "
+               "signatures (no RRSIG data)")
+
+
+def _log_response(log, e):
+    """
+    If exception contains response from server, log this response to debug log
+    :param log: if log is None, do not log
+    :param e: DNSException
     """
-    ip_addr = str(ip_addr)
+    assert isinstance(e, DNSException)
+    if log is not None:
+        response = getattr(e, 'kwargs', {}).get('response')
+        if response:
+            log.debug("DNSException: %s; server response: %s", e, response)
+
+
+def _resolve_record(owner, rtype, nameserver_ip=None, edns0=False,
+                    dnssec=False, timeout=10):
+    """
+    :param nameserver_ip: if None, default resolvers will be used
+    :param edns0: enables EDNS0
+    :param dnssec: enabled EDNS0, flags: DO
+    :raise DNSException: if error occurs
+    """
+    assert isinstance(nameserver_ip, basestring)
+    assert isinstance(rtype, basestring)
+
     res = dns.resolver.Resolver()
-    res.nameservers = [ip_addr]
-    res.lifetime = 10  # wait max 10 seconds for reply
+    if nameserver_ip:
+        res.nameservers = [nameserver_ip]
+    res.lifetime = timeout
 
-    # enable Authenticated Data + Checking Disabled flags
-    res.set_flags(dns.flags.AD | dns.flags.CD)
+    # Recursion Desired,
+    # this option prevents to get answers in authority section instead of answer
+    res.set_flags(dns.flags.RD)
 
-    # enable EDNS v0 + enable DNSSEC-Ok flag
-    res.use_edns(0, dns.flags.DO, 0)
+    if dnssec:
+        res.use_edns(0, dns.flags.DO, 4096)
+        res.set_flags(dns.flags.RD)
+    elif edns0:
+        res.use_edns(0, 0, 4096)
+
+    return res.query(owner, rtype)
+
+
+def _validate_edns0_forwarder(owner, rtype, ip_addr, log=None, timeout=10):
+    """
+    Validate if forwarder supports EDNS0
+
+    :raise UnresolvableRecordError: record cannot be resolved
+    :raise EDNS0UnsupportedError: EDNS0 is not supported by forwarder
+    """
+
+    try:
+        _resolve_record(owner, rtype, nameserver_ip=ip_addr, timeout=timeout)
+    except DNSException as e:
+        _log_response(log, e)
+        raise UnresolvableRecordError(owner=owner, rtype=rtype, ip=ip_addr,
+                                      error=e)
+
+    try:
+        _resolve_record(owner, rtype, nameserver_ip=ip_addr, edns0=True,
+                        timeout=timeout)
+    except DNSException as e:
+        _log_response(log, e)
+        raise EDNS0UnsupportedError(owner=owner, rtype=rtype, ip=ip_addr,
+                                    error=e)
+
+
+def validate_dnssec_global_forwarder(ip_addr, log=None, timeout=10):
+    """Test DNS forwarder properties. against root zone.
+
+    Global forwarders should be able return signed root zone
+
+    :raise UnresolvableRecordError: record cannot be resolved
+    :raise EDNS0UnsupportedError: EDNS0 is not supported by forwarder
+    :raise DNSSECSignatureMissingError: did not receive RRSIG for root zone
+    """
+
+    ip_addr = str(ip_addr)
+    owner = "."
+    rtype = "SOA"
+
+    _validate_edns0_forwarder(owner, rtype, ip_addr, log=log, timeout=timeout)
 
     # DNS root has to be signed
     try:
-        ans = res.query('.', 'NS')
-    except DNSException:
-        return None
+        ans = _resolve_record(owner, rtype, nameserver_ip=ip_addr, dnssec=True,
+                              timeout=timeout)
+    except DNSException as e:
+        _log_response(log, e)
+        raise UnresolvableRecordError(owner=owner, rtype=rtype, ip=ip_addr,
+                                      error=e)
 
     try:
-        ans.response.find_rrset(ans.response.answer, dns.name.root,
-                dns.rdataclass.IN, dns.rdatatype.RRSIG, dns.rdatatype.NS)
+        ans.response.find_rrset(
+            ans.response.answer, dns.name.root, dns.rdataclass.IN,
+            dns.rdatatype.RRSIG, dns.rdatatype.SOA
+        )
     except KeyError:
-        return False
+        raise DNSSECSignatureMissingError(owner=owner, rtype=rtype, ip=ip_addr)
 
-    return True
 
 
 def validate_idna_domain(value):
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 4c1bfa600fc887e401bbe818121e3bf5115a3785..77ff342d770ee1bb42521a3373df797663afd7b5 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -42,7 +42,8 @@ from ipaplatform.tasks import tasks
 from ipalib.util import (validate_zonemgr_str, normalize_zonemgr,
         get_dns_forward_zone_update_policy, get_dns_reverse_zone_update_policy,
         normalize_zone, get_reverse_zone_default, zone_is_reverse,
-        validate_dnssec_forwarder)
+        validate_dnssec_global_forwarder, DNSSECSignatureMissingError,
+        EDNS0UnsupportedError, UnresolvableRecordError)
 from ipalib.constants import CACERT
 
 NAMED_CONF = paths.NAMED_CONF
@@ -463,23 +464,32 @@ def check_reverse_zones(ip_addresses, reverse_zones, options, unattended, search
     return ret_reverse_zones
 
 def check_forwarders(dns_forwarders, logger):
-    print "Checking forwarders, please wait ..."
+    print "Checking DNS forwarders, please wait ..."
     forwarders_dnssec_valid = True
     for forwarder in dns_forwarders:
-        logger.debug("Checking forwarder: %s", forwarder)
-        result = validate_dnssec_forwarder(forwarder)
-        if result is None:
-            logger.error("Forwarder %s does not work", forwarder)
-            raise RuntimeError("Forwarder %s does not respond" % forwarder)
-        elif result is False:
+        logger.debug("Checking DNS server: %s", forwarder)
+        try:
+            validate_dnssec_global_forwarder(forwarder, log=logger)
+        except DNSSECSignatureMissingError as e:
             forwarders_dnssec_valid = False
-            logger.warning("DNS forwarder %s does not return DNSSEC signatures in answers", forwarder)
+            logger.warning("DNS server %s does not support DNSSEC: %s",
+                           forwarder, e)
             logger.warning("Please fix forwarder configuration to enable DNSSEC support.\n"
                 "(For BIND 9 add directive \"dnssec-enable yes;\" to \"options {}\")")
-            print ("WARNING: DNS forwarder %s does not return DNSSEC "
-                   "signatures in answers" % forwarder)
+            print "DNS server %s: %s" % (forwarder, e)
             print "Please fix forwarder configuration to enable DNSSEC support."
             print "(For BIND 9 add directive \"dnssec-enable yes;\" to \"options {}\")"
+        except EDNS0UnsupportedError as e:
+            forwarders_dnssec_valid = False
+            logger.warning("DNS server %s does not support ENDS0 "
+                           "(RFC 6891): %s", forwarder, e)
+            logger.warning("Please fix forwarder configuration. "
+                           "DNSSEC support cannot be enabled without EDNS0")
+            print ("WARNING: DNS server %s does not support EDNS0 "
+                   "(RFC 6891): %s" % (forwarder, e))
+        except UnresolvableRecordError as e:
+            logger.error("DNS server %s: %s", forwarder, e)
+            raise RuntimeError("DNS server %s: %s" % (forwarder, e))
 
     return forwarders_dnssec_valid
 
diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index a226c80486e4d44a44714a2f7d03e1049d4d37a8..50f46bc52a1646320628e792d72d8686ffa47d24 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -1751,10 +1751,11 @@ class test_dns(Declarative):
                 'value': None,
                 'summary': None,
                 u'messages': (
-                    {u'message': u'DNS server 172.16.31.80 not responding.',
+                    {u'message': lambda x: x.startswith(
+                        u"DNS server %s: query '. SOA':" % fwd_ip),
                      u'code': 13006,
                      u'type':u'warning',
-                     u'name': u'DNSServerNotRespondingWarning'},
+                     u'name': u'DNSServerValidationWarning'},
                 ),
                 'result': {
                     'idnsforwarders': [fwd_ip],
-- 
2.1.0

From 44fa937b4fb7c49442b10d132b841a8a687744af Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Fri, 24 Apr 2015 13:37:07 +0200
Subject: [PATCH 2/2] DNSSEC: validate forward zone forwarders

Show warning messages if DNSSEC validation is failing for particular FW
zone or if the specified forwarders do not work

https://fedorahosted.org/freeipa/ticket/4657
---
 ipalib/messages.py                      |  12 ++++
 ipalib/plugins/dns.py                   | 112 +++++++++++++++++++++++++++++++-
 ipalib/util.py                          |  60 ++++++++++++++++-
 ipatests/test_xmlrpc/test_dns_plugin.py |  20 ++++++
 4 files changed, 201 insertions(+), 3 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index 236b683b30692d88e5257d9189c559dd9f848885..84f0a722d3c82a9626a9b404a7134305fcd71dfa 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -229,6 +229,18 @@ class DNSServerDoesNotSupportEDNS0Warning(PublicMessage):
                u"please disable it.")
 
 
+class DNSSECValidationFailingWarning(PublicMessage):
+    """
+    **13010** Used when a DNSSEC validation failed on IPA DNS server
+    """
+
+    errno = 13010
+    type = "warning"
+    format = _(u"DNSSEC validation failed: %(error)s.\n"
+               u"Please verify your DNSSEC signatures or disable DNSSEC "
+               u"validation on all IPA servers.")
+
+
 def iter_messages(variables, base):
     """Return a tuple with all subclasses
     """
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index c9dc1e547f6681731cf96ac7386a197e038e3859..5c22c099384cb8580ddb768ba4b8f7ce9788d5d3 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -26,6 +26,7 @@ import re
 import binascii
 import dns.name
 import dns.exception
+import dns.rdatatype
 import dns.resolver
 import encodings.idna
 
@@ -45,7 +46,9 @@ from ipalib.util import (normalize_zonemgr,
                          get_reverse_zone_default, REVERSE_DNS_ZONES,
                          normalize_zone, validate_dnssec_global_forwarder,
                          DNSSECSignatureMissingError, UnresolvableRecordError,
-                         EDNS0UnsupportedError)
+                         EDNS0UnsupportedError, DNSSECValidationError,
+                         validate_dnssec_zone_forwarder_step1,
+                         validate_dnssec_zone_forwarder_step2)
 
 from ipapython.ipautil import CheckedIPAddress, is_host_resolvable
 from ipapython.dnsutil import DNSName
@@ -4340,11 +4343,100 @@ class dnsforwardzone(DNSZoneBase):
         _add_warning_fw_zone_is_not_effective(result, fwzone,
                                               options['version'])
 
+    def _warning_if_forwarders_do_not_work(self, result, new_zone=False,
+                                           *keys, **options):
+        fwzone = keys[-1]
+        forwarders = options.get('idnsforwarders', [])
+        any_forwarder_work = False
+
+        for forwarder in forwarders:
+            try:
+                validate_dnssec_zone_forwarder_step1(forwarder, fwzone,
+                                                     log=self.log)
+            except UnresolvableRecordError as e:
+                messages.add_message(
+                    options['version'],
+                    result, messages.DNSServerValidationWarning(
+                        server=forwarder, error=e
+                    )
+                )
+            except EDNS0UnsupportedError as e:
+                messages.add_message(
+                    options['version'],
+                    result, messages.DNSServerDoesNotSupportEDNS0Warning(
+                        server=forwarder, error=e
+                    )
+                )
+            else:
+                any_forwarder_work = True
+
+        if not any_forwarder_work:
+            # do not test DNSSEC validation if there is no valid forwarder
+            return
+
+        # resolve IP address of any DNS replica
+        # FIXME: https://fedorahosted.org/bind-dyndb-ldap/ticket/143
+        # we currenly should to test all IPA DNS replica, because DNSSEC
+        # validation is configured just in named.conf per replica
+
+        ipa_dns_masters = [normalize_zone(x) for x in
+                           api.Object.dnsrecord.get_dns_masters()]
+
+        if not ipa_dns_masters:
+            # something very bad happened, DNS is installed, but no IPA DNS
+            # servers available
+            self.log.error("No IPA DNS server can be found, but integrated DNS "
+                           "is installed")
+            return
+
+        ipa_dns_ip = None
+        for rdtype in (dns.rdatatype.A, dns.rdatatype.AAAA):
+            try:
+                ans = dns.resolver.query(ipa_dns_masters[0], rdtype)
+            except dns.exception.DNSException:
+                continue
+            else:
+                ipa_dns_ip = str(ans.rrset.items[0])
+                break
+
+        if not ipa_dns_ip:
+            self.log.error("Cannot resolve %s hostname", ipa_dns_masters[0])
+            return
+
+        # sleep a bit, adding new zone to BIND from LDAP may take a while
+        if new_zone:
+            time.sleep(5)
+
+        # Test if IPA is able to receive replies from forwarders
+        try:
+            validate_dnssec_zone_forwarder_step2(ipa_dns_ip, fwzone,
+                                                 log=self.log)
+        except DNSSECValidationError as e:
+            messages.add_message(
+                options['version'],
+                result, messages.DNSSECValidationFailingWarning(error=e)
+            )
+        except UnresolvableRecordError as e:
+            messages.add_message(
+                options['version'],
+                result, messages.DNSServerValidationWarning(
+                    server=ipa_dns_ip, error=e
+                )
+            )
 
 @register()
 class dnsforwardzone_add(DNSZoneBase_add):
     __doc__ = _('Create new DNS forward zone.')
 
+    def interactive_prompt_callback(self, kw):
+        # show informative message on client side
+        # server cannot send messages asynchronous
+        if kw.get('idnsforwarders', False):
+            self.Backend.textui.print_plain(
+                _("Server will check DNS forwarder(s)."))
+            self.Backend.textui.print_plain(
+                _("This may take some time, please wait ..."))
+
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         assert isinstance(dn, DN)
 
@@ -4364,6 +4456,9 @@ class dnsforwardzone_add(DNSZoneBase_add):
     def execute(self, *keys, **options):
         result = super(dnsforwardzone_add, self).execute(*keys, **options)
         self.obj._warning_fw_zone_is_not_effective(result, *keys, **options)
+        if options.get('idnsforwarders'):
+            self.obj._warning_if_forwarders_do_not_work(
+                result, new_zone=True, *keys, **options)
         return result
 
 
@@ -4378,6 +4473,15 @@ class dnsforwardzone_del(DNSZoneBase_del):
 class dnsforwardzone_mod(DNSZoneBase_mod):
     __doc__ = _('Modify DNS forward zone.')
 
+    def interactive_prompt_callback(self, kw):
+        # show informative message on client side
+        # server cannot send messages asynchronous
+        if kw.get('idnsforwarders', False):
+            self.Backend.textui.print_plain(
+                _("Server will check DNS forwarder(s)."))
+            self.Backend.textui.print_plain(
+                _("This may take some time, please wait ..."))
+
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         try:
             entry = ldap.get_entry(dn)
@@ -4406,6 +4510,12 @@ class dnsforwardzone_mod(DNSZoneBase_mod):
 
         return dn
 
+    def execute(self, *keys, **options):
+        result = super(dnsforwardzone_mod, self).execute(*keys, **options)
+        if options.get('idnsforwarders'):
+            self.obj._warning_if_forwarders_do_not_work(result, *keys,
+                                                        **options)
+        return result
 
 @register()
 class dnsforwardzone_find(DNSZoneBase_find):
diff --git a/ipalib/util.py b/ipalib/util.py
index 8d7b666386efe61994d640f9ae0f1761f7e07e2b..5810c774a670d300bf60326ac438e6993d34f611 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -34,6 +34,7 @@ from types import NoneType
 from weakref import WeakKeyDictionary
 from dns import resolver, rdatatype
 from dns.exception import DNSException
+from dns.resolver import NXDOMAIN
 from netaddr.core import AddrFormatError
 
 from ipalib import errors, messages
@@ -580,6 +581,11 @@ class DNSSECSignatureMissingError(ForwarderValidationError):
                "signatures (no RRSIG data)")
 
 
+class DNSSECValidationError(ForwarderValidationError):
+    format = _("requested record '%(owner)s %(rtype)s' was refused by IPA "
+               "server %(ip)s because DNSSEC signature is not valid")
+
+
 def _log_response(log, e):
     """
     If exception contains response from server, log this response to debug log
@@ -594,11 +600,12 @@ def _log_response(log, e):
 
 
 def _resolve_record(owner, rtype, nameserver_ip=None, edns0=False,
-                    dnssec=False, timeout=10):
+                    dnssec=False, flag_cd=False, timeout=10):
     """
     :param nameserver_ip: if None, default resolvers will be used
     :param edns0: enables EDNS0
     :param dnssec: enabled EDNS0, flags: DO
+    :param flag_cd: requires dnssec=True, adds flag CD
     :raise DNSException: if error occurs
     """
     assert isinstance(nameserver_ip, basestring)
@@ -615,7 +622,10 @@ def _resolve_record(owner, rtype, nameserver_ip=None, edns0=False,
 
     if dnssec:
         res.use_edns(0, dns.flags.DO, 4096)
-        res.set_flags(dns.flags.RD)
+        flags = dns.flags.RD
+        if flag_cd:
+            flags = flags | dns.flags.CD
+        res.set_flags(flags)
     elif edns0:
         res.use_edns(0, 0, 4096)
 
@@ -680,6 +690,52 @@ def validate_dnssec_global_forwarder(ip_addr, log=None, timeout=10):
         raise DNSSECSignatureMissingError(owner=owner, rtype=rtype, ip=ip_addr)
 
 
+def validate_dnssec_zone_forwarder_step1(ip_addr, fwzone, log=None, timeout=10):
+    """
+    Only forwarders in forward zones can be validated in this way
+    :raise UnresolvableRecordError: record cannot be resolved
+    :raise EDNS0UnsupportedError: ENDS0 is not supported by forwarder
+    """
+    _validate_edns0_forwarder(fwzone, "SOA", ip_addr, log=log, timeout=timeout)
+
+
+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
+    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)
+    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)
+    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
+        raise DNSSECValidationError(owner=fwzone, rtype=rtype, ip=ipa_ip_addr)
+
 
 def validate_idna_domain(value):
     """
diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index 50f46bc52a1646320628e792d72d8686ffa47d24..e38ea424db6b9216af35a172b7ffc12a3bf890f3 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -3375,6 +3375,14 @@ class test_forward_zones(Declarative):
             expected={
                 'value': fwzone2_dnsname,
                 'summary': None,
+                u'messages': (
+                    {u'message': lambda x: x.startswith(
+                        u"DNS server %s: query '%s SOA':" %
+                        (forwarder1, fwzone2)),
+                     u'code': 13006,
+                     u'type':u'warning',
+                     u'name': u'DNSServerValidationWarning'},
+                ),
                 'result': {
                     'dn': fwzone2_dn,
                     'idnsname': [fwzone2_dnsname],
@@ -3409,6 +3417,7 @@ class test_forward_zones(Declarative):
             expected={
                 'value': fwzone2_dnsname,
                 'summary': None,
+                'messages': lambda x: True,  # fake forwarders - ignore message
                 'result': {
                     'dn': fwzone2_dn,
                     'idnsname': [fwzone2_dnsname],
@@ -3442,6 +3451,7 @@ class test_forward_zones(Declarative):
             expected={
                 'value': fwzone2_dnsname,
                 'summary': None,
+                'messages': lambda x: True,  # fake forwarders - ignore message
                 'result': {
                     'dn': fwzone2_dn,
                     'idnsname': [fwzone2_dnsname],
@@ -3465,6 +3475,7 @@ class test_forward_zones(Declarative):
             expected={
                 'value': fwzone3_dnsname,
                 'summary': None,
+                'messages': lambda x: True,  # fake forwarders - ignore message
                 'result': {
                     'dn': fwzone3_dn,
                     'idnsname': [fwzone3_dnsname],
@@ -3498,6 +3509,7 @@ class test_forward_zones(Declarative):
             expected={
                 'value': fwzone3_dnsname,
                 'summary': None,
+                'messages': lambda x: True,  # fake forwarders - ignore message
                 'result': {
                     'dn': fwzone3_dn,
                     'idnsname': [fwzone3_dnsname],
@@ -3521,6 +3533,7 @@ class test_forward_zones(Declarative):
             expected={
                 'value': fwzone3_dnsname,
                 'summary': None,
+                'messages': lambda x: True,  # fake forwarders - ignore message
                 'result': {
                     'idnsname': [fwzone3_dnsname],
                     'idnszoneactive': [u'TRUE'],
@@ -3541,6 +3554,7 @@ class test_forward_zones(Declarative):
             expected={
                 'value': fwzone3_dnsname,
                 'summary': None,
+                'messages': lambda x: True,  # fake forwarders - ignore message
                 'result': {
                     'idnsname': [fwzone3_dnsname],
                     'idnszoneactive': [u'TRUE'],
@@ -3561,6 +3575,7 @@ class test_forward_zones(Declarative):
             expected={
                 'value': fwzone3_dnsname,
                 'summary': None,
+                'messages': lambda x: True,  # fake forwarders - ignore message
                 'result': {
                     'idnsname': [fwzone3_dnsname],
                     'idnszoneactive': [u'TRUE'],
@@ -3581,6 +3596,7 @@ class test_forward_zones(Declarative):
             expected={
                 'value': fwzone3_dnsname,
                 'summary': None,
+                'messages': lambda x: True,  # fake forwarders - ignore message
                 'result': {
                     'idnsname': [fwzone3_dnsname],
                     'idnszoneactive': [u'TRUE'],
@@ -3602,6 +3618,7 @@ class test_forward_zones(Declarative):
             expected={
                 'value': fwzone1_dnsname,
                 'summary': None,
+                'messages': lambda x: True,  # fake forwarders - ignore message
                 'result': {
                     'idnsname': [fwzone1_dnsname],
                     'idnszoneactive': [u'TRUE'],
@@ -3663,6 +3680,7 @@ class test_forward_zones(Declarative):
             expected={
                 'value': fwzone1_dnsname,
                 'summary': None,
+                'messages': lambda x: True,  # fake forwarders - ignore message
                 'result': {
                     'idnsname': [fwzone1_dnsname],
                     'idnszoneactive': [u'TRUE'],
@@ -3704,6 +3722,7 @@ class test_forward_zones(Declarative):
             expected={
                 'value': fwzone1_dnsname,
                 'summary': None,
+                'messages': lambda x: True,  # fake forwarders - ignore message
                 'result': {
                     'idnsname': [fwzone1_dnsname],
                     'idnszoneactive': [u'TRUE'],
@@ -4616,6 +4635,7 @@ class test_forward_master_zones_mutual_exlusion(Declarative):
             expected={
                 'value': zone_findtest_forward_dnsname,
                 'summary': None,
+                'messages': lambda x: True,  # fake forwarders - ignore message
                 'result': {
                     'dn': zone_findtest_forward_dn,
                     'idnsname': [zone_findtest_forward_dnsname],
-- 
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

Reply via email to