On 25.5.2016 12:30, Martin Basti wrote:
> 
> 
> On 04.05.2016 10:43, Petr Spacek wrote:
>> Hello,
>>
>> DNS: Warn if forwarding policy conflicts with automatic empty zones
>>
>> Forwarding policy "first" or "none" may conflicts with some automatic empty
>> zones. Queries for zones specified by RFC 6303 will ignore
>> forwarding and recursion and always result in NXDOMAIN answers.
>>
>> This is not detected and warned about. Global forwarding is equivalent
>> to forward zone ".".
>>
>> Example:
>> Forward zone 1.10.in-addr.arpa with policy "first"
>> will not forward anything because BIND will automatically prefer
>> automatic empty zone "10.in-addr.arpa." which is authoritative.
>>
>> https://fedorahosted.org/freeipa/ticket/5710
>>
>>
>> This is last patch in the series so the ticket can be closed when all 
>> relevant
>> patches are pushed.
>>
>>
>>
> 
> 
> You forgot to update tests
> 
> _____________________________________________________________________
> test_dns.test_command[0087: dnsconfig_mod: Update global DNS settings]
> ______________________________________________________________________
> 
> self = <ipatests.test_xmlrpc.test_dns_plugin.test_dns object at
> 0x7fcef3ef2510>, index = 87
> declarative_test_definition = {'command': ('dnsconfig_mod', [],
> {'idnsforwarders': ['172.16.31.80'], 'version': '2.166'}), 'desc': 'Update
> global DN...arders': ['172.16.31.80']}, 'summary': None, 'value': None},
> 'nice': '0087: dnsconfig_mod: Update global DNS settings'}
> 
>     def test_command(self, index, declarative_test_definition):
>         """Run an individual test
> 
>             The arguments are provided by the pytest plugin.
>             """
>         if callable(declarative_test_definition):
>             declarative_test_definition(self)
>         else:
>>           self.check(**declarative_test_definition)
> 
> test_xmlrpc/xmlrpc_test.py:313:
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> test_xmlrpc/xmlrpc_test.py:325: in check
>     self.check_output(nice, cmd, args, options, expected, extra_check)
> test_xmlrpc/xmlrpc_test.py:368: in check_output
>     assert_deepequal(expected, got, nice)
> util.py:361: in assert_deepequal
>     assert_deepequal(e_sub, g_sub, doc, stack + (key,))
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> 
> expected = [{'code': 13006, 'message': <function <lambda> at 0x7fcef426c758>,
> 'name': 'DNSServerValidationWarning', 'type': 'warning'}]
> got = [{'code': 13021, 'message': "Forwarding policy conflicts with some
> automatic empty zones. Queries for zones specified ...': The DNS operation
> timed out after 10.0008428097 seconds.", 'name': 'DNSServerValidationWarning',
> 'type': 'warning'}]
> doc = '0087: dnsconfig_mod: Update global DNS settings', stack = ('messages',)
> 
>     def assert_deepequal(expected, got, doc='', stack=tuple()):
>         """
>         Recursively check for type and equality.
> 
>         If a value in expected is callable then it will used as a callback to
>         test for equality on the got value. The callback is passed the got
>         value and returns True if equal, False otherwise.
> 
>         If the tests fails, it will raise an ``AssertionError`` with detailed
>         information, including the path to the offending value.  For example:
> 
>         >>> expected = [u'Hello', dict(world=u'how are you?')]
>         >>> got = [u'Hello', dict(world='how are you?')]
>         >>> expected == got
>         True
>         >>> assert_deepequal(expected, got, doc='Testing my nested data')
>         Traceback (most recent call last):
>           ...
>         AssertionError: assert_deepequal: type(expected) is not type(got).
>           Testing my nested data
>           type(expected) = <type 'unicode'>
>           type(got) = <type 'str'>
>           expected = u'how are you?'
>           got = 'how are you?'
>           path = (0, 'world')
> 
>         Note that lists and tuples are considered equivalent, and the order of
>         their elements does not matter.
>         """
>         if isinstance(expected, tuple):
>             expected = list(expected)
>         if isinstance(got, tuple):
>             got = list(got)
>         if isinstance(expected, DN):
>             if isinstance(got, six.string_types):
>                 got = DN(got)
>         if not (isinstance(expected, Fuzzy) or callable(expected) or
> type(expected) is type(got)):
>             raise AssertionError(
>                 TYPE % (doc, type(expected), type(got), expected, got, stack)
>             )
>         if isinstance(expected, (list, tuple)):
>             if len(expected) != len(got):
>                 raise AssertionError(
>>                   LEN % (doc, len(expected), len(got), expected, got, stack)
>                 )
> E               AssertionError: assert_deepequal: list length mismatch.
> E                 0087: dnsconfig_mod: Update global DNS settings
> E                 len(expected) = 1
> E                 len(got) = 2
> E                 expected = [{u'message': <function <lambda> at
> 0x7fcef426c758>, u'code': 13006, u'type': u'warning', u'name':
> u'DNSServerValidationWarning'}]
> E                 got = [{u'message': u"Forwarding policy conflicts with some
> automatic empty zones. Queries for zones specified by RFC 6303 will ignore
> forwarding and recursion and always result in NXDOMAIN answers. To override
> this behavior use forward policy 'only'.", u'code': 13021, u'type':
> u'warning', u'name': u'DNSForwardPolicyConflictWithEmptyZone'}, {u'message':
> u"DNS server 172.16.31.80: query '. SOA': The DNS operation timed out after
> 10.0008428097 seconds.", u'code': 13006, u'type': u'warning', u'name':
> u'DNSServerValidationWarning'}]
> E                 path = (u'messages',)
> 
> util.py:332: AssertionError

Fixed patch is attached. It depends on newest patches 113-132.

-- 
Petr^2 Spacek
From ca48b7979439c5940a4fd20ea12c772a4a81a1c0 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Wed, 4 May 2016 10:30:18 +0200
Subject: [PATCH] DNS: Warn if forwarding policy conflicts with automatic empty
 zones

Forwarding policy "first" or "none" may conflicts with some automatic empty
zones. Queries for zones specified by RFC 6303 will ignore
forwarding and recursion and always result in NXDOMAIN answers.

This is not detected and warned about. Global forwarding is equivalent
to forward zone ".".

Example:
Forward zone 1.10.in-addr.arpa with policy "first"
will not forward anything because BIND will automatically prefer
automatic empty zone "10.in-addr.arpa." which is authoritative.

https://fedorahosted.org/freeipa/ticket/5710
---
 ipalib/messages.py                      | 17 +++++++++++++++++
 ipalib/plugins/dns.py                   | 26 ++++++++++++++++++++++++++
 ipatests/test_xmlrpc/test_dns_plugin.py | 14 ++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index c760e9d3746a0e46a5b67f801ecddd76f1ce6406..e863bdd495b55921c9e487794f5c9573a6166038 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -378,6 +378,23 @@ class FailedToRemoveHostDNSRecords(PublicMessage):
                "(%(reason)s)")
 
 
+class DNSForwardPolicyConflictWithEmptyZone(PublicMessage):
+    """
+    **13021** Forward zone 1.10.in-addr.arpa with policy "first"
+    will not forward anything because BIND automatically prefers
+    empty zone "10.in-addr.arpa.".
+    """
+
+    errno = 13021
+    type = "warning"
+    format = _(
+        "Forwarding policy conflicts with some automatic empty zones. "
+        "Queries for zones specified by RFC 6303 will ignore "
+        "forwarding and recursion and always result in NXDOMAIN answers. "
+        "To override this behavior use forward policy 'only'."
+    )
+
+
 def iter_messages(variables, base):
     """Return a tuple with all subclasses
     """
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index f0a44a862ecda941ab5257b197ecae83b343d692..ef0d9d8193aecd0421712840b08998a19372cfac 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -67,6 +67,7 @@ from ipapython.dn import DN
 from ipapython.ipautil import CheckedIPAddress
 from ipapython.dnsutil import check_zone_overlap
 from ipapython.dnsutil import DNSName
+from ipapython.dnsutil import related_to_auto_empty_zone
 
 if six.PY3:
     unicode = str
@@ -2080,6 +2081,20 @@ def _add_warning_fw_zone_is_not_effective(api, result, fwzone, version):
         )
 
 
+def _add_warning_fw_policy_conflict_aez(result, fwzone, **options):
+    """Warn if forwarding policy conflicts with an automatic empty zone."""
+    fwd_policy = result['result'].get(u'idnsforwardpolicy',
+                                      dnsforwardzone.default_forward_policy)
+    if (
+        fwd_policy != [u'only']
+        and related_to_auto_empty_zone(DNSName(fwzone))
+    ):
+        messages.add_message(
+            options['version'], result,
+            messages.DNSForwardPolicyConflictWithEmptyZone()
+        )
+
+
 class DNSZoneBase(LDAPObject):
     """
     Base class for DNS Zone
@@ -4428,7 +4443,13 @@ class dnsconfig_mod(LDAPUpdate):
         result = super(dnsconfig_mod, self).execute(*keys, **options)
         self.obj.postprocess_result(result)
 
+        # this check makes sense only when resulting forwarders are non-empty
+        if result['result'].get('idnsforwarders'):
+            fwzone = DNSName('.')
+            _add_warning_fw_policy_conflict_aez(result, fwzone, **options)
+
         if forwarders:
+            # forwarders were changed
             for forwarder in forwarders:
                 try:
                     validate_dnssec_global_forwarder(forwarder, log=self.log)
@@ -4570,6 +4591,7 @@ class dnsforwardzone(DNSZoneBase):
                 )
             )
 
+
 @register()
 class dnsforwardzone_add(DNSZoneBase_add):
     __doc__ = _('Create new DNS forward zone.')
@@ -4600,8 +4622,10 @@ class dnsforwardzone_add(DNSZoneBase_add):
         return dn
 
     def execute(self, *keys, **options):
+        fwzone = keys[-1]
         result = super(dnsforwardzone_add, self).execute(*keys, **options)
         self.obj._warning_fw_zone_is_not_effective(result, *keys, **options)
+        _add_warning_fw_policy_conflict_aez(result, fwzone, **options)
         if options.get('idnsforwarders'):
             self.obj._warning_if_forwarders_do_not_work(
                 result, True, *keys, **options)
@@ -4657,7 +4681,9 @@ class dnsforwardzone_mod(DNSZoneBase_mod):
         return dn
 
     def execute(self, *keys, **options):
+        fwzone = keys[-1]
         result = super(dnsforwardzone_mod, self).execute(*keys, **options)
+        _add_warning_fw_policy_conflict_aez(result, fwzone, **options)
         if options.get('idnsforwarders'):
             self.obj._warning_if_forwarders_do_not_work(result, False, *keys,
                                                         **options)
diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index 0953de8b54a04478540eade56b86f09af820d736..7351d0b1d5f5498c1721a8cea4e3711fc4147e73 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -1763,6 +1763,13 @@ class test_dns(Declarative):
                 'summary': None,
                 u'messages': (
                     {u'message': lambda x: x.startswith(
+                        u"Forwarding policy conflicts with some "
+                        "automatic empty zones."),
+                     u'code': 13021,
+                     u'type': u'warning',
+                     u'name': u'DNSForwardPolicyConflictWithEmptyZone',
+                     u'data': {}},
+                    {u'message': lambda x: x.startswith(
                         u"DNS server %s: query '. SOA':" % fwd_ip),
                      u'code': 13006,
                      u'type':u'warning',
@@ -3438,6 +3445,13 @@ class test_forward_zones(Declarative):
                 'summary': None,
                 u'messages': (
                     {u'message': lambda x: x.startswith(
+                        u"Forwarding policy conflicts with some "
+                        "automatic empty zones."),
+                     u'code': 13021,
+                     u'type': u'warning',
+                     u'name': u'DNSForwardPolicyConflictWithEmptyZone',
+                     u'data': {}},
+                    {u'message': lambda x: x.startswith(
                         u"DNS server %s: query '%s SOA':" %
                         (forwarder1, fwzone2)),
                      u'code': 13006,
-- 
2.5.5

-- 
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