Ticket: https://fedorahosted.org/freeipa/ticket/4721
Patch attached

--
Martin Basti

From a5a19137e3ddf2d2d48cfbdb2968b6f68ac8f772 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Fri, 21 Nov 2014 16:54:09 +0100
Subject: [PATCH] Detect and warn about invalid DNS forward zone configuration

Shows warning if forward and parent authoritative zone do not have
proper NS record delegation, which can cause the forward zone will be
ineffective and forwarding will not work.

The chande in the test was required due python changes order of elemnts
in dict (python doesnt guarantee an order in dict)

Ticket: https://fedorahosted.org/freeipa/ticket/4721
---
 ipalib/messages.py                      |  12 +++
 ipalib/plugins/dns.py                   | 147 +++++++++++++++++++++++++++++---
 ipatests/test_xmlrpc/test_dns_plugin.py |   2 +-
 3 files changed, 149 insertions(+), 12 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index 102e35275dbe37328c84ecb3cd5b2a8d8578056f..cc9bae3d6e5c7d3a7401dab89fafb1b60dc1e15f 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -200,6 +200,18 @@ class DNSServerDoesNotSupportDNSSECWarning(PublicMessage):
                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
+    there is authoritative parent zone, without proper NS delegation
+    """
+
+    errno = 13008
+    type = "warning"
+    format = _(u"forward zone \"%(fwzone)s\" is not effective (missing proper "
+               u"NS delegation in authoritative zone \"%(authzone)s\"). "
+               u"Forwarding may not work.")
+
 
 def iter_messages(variables, base):
     """Return a tuple with all subclasses
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index c5d96a8c4fcdf101254ecefb60cb83d63bee6310..4f92da645e0faf784c7deb4b8ce386c1440f4229 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -1725,6 +1725,79 @@ def _normalize_zone(zone):
     return zone
 
 
+def _find_zone_which_makes_fw_zone_ineffective(fwzonename):
+    """
+    Check if forward zone is effective.
+
+    If parent zone exists as authoritative zone, forward zone will not
+    forward queries. It is necessary to delegate authority of forward zone
+    to another server (non IPA DNS server).
+
+    Example:
+
+    Forward zone: sub.example.com
+    Zone: example.com
+
+    Forwarding will not work, because server thinks it is authoritative
+    and returns NXDOMAIN
+
+    Adding record: sub.example.com NS nameserver.out.of.ipa.domain.
+    will delegate authority to another server, and IPA DNS server will
+    forward DNS queries.
+
+    :param fwzonename: forwardzone
+    :return: None if effective, name of authoritative zone otherwise
+    """
+    assert isinstance(fwzonename, DNSName)
+
+    # get all zones
+    zones = api.Command.dnszone_find(pkey_only=True,
+                                        sizelimit=0)['result']
+    zonenames = [zone['idnsname'][0].make_absolute() for zone in zones]
+    zonenames.sort(reverse=True, key=len)  # sort, we need longest match
+
+    # check if is there a zone which can cause the forward zone will
+    # be ineffective
+    sub_of_auth_zone = None
+    for name in zonenames:
+        if fwzonename.is_subdomain(name):
+            sub_of_auth_zone = name
+            break
+
+    if sub_of_auth_zone is None:
+        return None
+
+    # check if forwardzone is delegated in authoritative zone
+    # test if exists and get NS record
+    try:
+        ns_records = api.Command.dnsrecord_show(
+            sub_of_auth_zone, fwzonename, raw=True)['result']['nsrecord']
+    except (errors.NotFound, KeyError):
+        return sub_of_auth_zone
+
+    # make sure the target is not IPA DNS server
+    # FIXME: what if aliases are used?
+    normalized_ns_records = set()
+    for record in ns_records:
+        if record.endswith('.'):
+            normalized_ns_records.add(record)
+        else:
+            normalized_record = "%s.%s" % (record, sub_of_auth_zone)
+            normalized_ns_records.add(normalized_record)
+
+    nameservers = [normalize_zone(x) for x in
+                   api.Object.dnsrecord.get_dns_masters()]
+
+    if any(nameserver in normalized_ns_records
+           for nameserver in nameservers):
+        # NS record is pointing to IPA DNS server
+        return sub_of_auth_zone
+
+    # authoritative zone contains NS records which delegates forwardzone to
+    # different server, forwardzone is effective
+    return None
+
+
 class DNSZoneBase(LDAPObject):
     """
     Base class for DNS Zone
@@ -3164,6 +3237,29 @@ class dnsrecord(LDAPObject):
             dns_name = entry_name[1].derelativize(dns_domain)
             self.wait_for_modified_attrs(entry, dns_name, dns_domain)
 
+    def warning_if_ns_change_cause_fwzone_ineffective(self, keys, result,
+                                                      options):
+        """after execute"""
+        record_name_absolute = keys[-1]
+        if not keys[-1].is_absolute():
+            record_name_absolute = keys[-1].derelativize(keys[-2])
+
+        try:
+            api.Command.dnsforwardzone_show(record_name_absolute)
+        except errors.NotFound:
+            # no forward zone
+            return
+
+        authoritative_zone = \
+            _find_zone_which_makes_fw_zone_ineffective(record_name_absolute)
+        if authoritative_zone:
+            # forward zone is not effective and forwarding will not work
+            messages.add_message(
+                options['version'], result,
+                messages.ForwardzoneIsNotEffectiveWarning(
+                    fwzone=record_name_absolute, authzone=authoritative_zone
+                )
+            )
 
 
 @register()
@@ -3358,6 +3454,14 @@ class dnsrecord_add(LDAPCreate):
                 return
         raise exc
 
+    def execute(self, *keys, **options):
+        result = super(dnsrecord_add, self).execute(*keys, **options)
+        if 'nsrecord' in options:
+            self.obj.warning_if_ns_change_cause_fwzone_ineffective(keys,
+                                                                   result,
+                                                                   options)
+        return result
+
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
         assert isinstance(dn, DN)
         for attr in getattr(context, 'dnsrecord_precallback_attrs', []):
@@ -3487,7 +3591,10 @@ class dnsrecord_mod(LDAPUpdate):
 
         if self.api.env['wait_for_dns']:
             self.obj.wait_for_modified_entries(context.dnsrecord_entry_mods)
-
+        if 'nsrecord' in options:
+            self.obj.warning_if_ns_change_cause_fwzone_ineffective(keys,
+                                                                   result,
+                                                                   options)
         return result
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
@@ -3654,19 +3761,23 @@ class dnsrecord_del(LDAPUpdate):
             if self.api.env['wait_for_dns']:
                 entries = {(keys[0], keys[1]): None}
                 self.obj.wait_for_modified_entries(entries)
-            return result
+        else:
+            result = super(dnsrecord_del, self).execute(*keys, **options)
+            result['value'] = pkey_to_value([keys[-1]], options)
 
-        result = super(dnsrecord_del, self).execute(*keys, **options)
-        result['value'] = pkey_to_value([keys[-1]], options)
+            if getattr(context, 'del_all', False) and not \
+                    self.obj.is_pkey_zone_record(*keys):
+                result = self.obj.methods.delentry(*keys,
+                                                   version=options['version'])
+                context.dnsrecord_entry_mods[(keys[0], keys[1])] = None
 
-        if getattr(context, 'del_all', False) and not \
-                self.obj.is_pkey_zone_record(*keys):
-            result = self.obj.methods.delentry(*keys,
-                                               version=options['version'])
-            context.dnsrecord_entry_mods[(keys[0], keys[1])] = None
+            if self.api.env['wait_for_dns']:
+                self.obj.wait_for_modified_entries(context.dnsrecord_entry_mods)
 
-        if self.api.env['wait_for_dns']:
-            self.obj.wait_for_modified_entries(context.dnsrecord_entry_mods)
+        if 'nsrecord' in options or options.get('del_all', False):
+            self.obj.warning_if_ns_change_cause_fwzone_ineffective(keys,
+                                                                   result,
+                                                                   options)
         return result
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
@@ -4019,6 +4130,20 @@ class dnsforwardzone_add(DNSZoneBase_add):
 
         return dn
 
+    def execute(self, *keys, **options):
+        result = super(dnsforwardzone_add, self).execute(*keys, **options)
+        authoritative_zone = _find_zone_which_makes_fw_zone_ineffective(keys[-1])
+        if authoritative_zone:
+            # forward zone is not effective and forwarding will not work
+            messages.add_message(
+                options['version'], result,
+                messages.ForwardzoneIsNotEffectiveWarning(
+                    fwzone=keys[-1], authzone=authoritative_zone
+                )
+            )
+
+        return result
+
 
 @register()
 class dnsforwardzone_del(DNSZoneBase_del):
diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index fb53853147ecf663cf7015867131445f32364cfb..224a80d98273480f40fd78dea0f27e34ea36492f 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -1060,7 +1060,7 @@ class test_dns(Declarative):
                                                                  'srv_part_target' : u'foo.bar.',
                                                                  'srvrecord': [u"1 100 1234 %s" \
                                                                      % zone1_ns]}),
-            expected=errors.ValidationError(name='srv_target',
+            expected=errors.ValidationError(name='srv_weight',
                 error=u'Raw value of a DNS record was already set by ' +
                     u'"srv_rec" option'),
         ),
-- 
1.8.3.1

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to