[Freeipa-devel] [PATCH 0170] Detect and warn about invalid forwardzone configuration
Petr Spacek
pspacek at redhat.com
Thu Dec 11 15:28:16 UTC 2014
Hello,
I have only few nitpicks and one minor non-nitpick. Rest is in-line.
On 10.12.2014 18:20, Martin Basti wrote:
> freeipa-mbasti-0170.4-Detect-and-warn-about-invalid-DNS-forward-zone-confi.patch
>
>
> From a1b70e7a12ffdb08941d43587a05d7e36b57ab2b Mon Sep 17 00:00:00 2001
> From: Martin Basti <mbasti at 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.
>
> Ticket: https://fedorahosted.org/freeipa/ticket/4721
> ---
> ipalib/messages.py | 13 ++
> ipalib/plugins/dns.py | 332 ++++++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 334 insertions(+), 11 deletions(-)
>
> diff --git a/ipalib/messages.py b/ipalib/messages.py
> index 102e35275dbe37328c84ecb3cd5b2a8d8578056f..b44beca729f5483a7241e4c98a9f724ed663e70f 100644
> --- a/ipalib/messages.py
> +++ b/ipalib/messages.py
> @@ -200,6 +200,19 @@ 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 because of "
> + u"missing proper NS delegation in authoritative zone "
> + u"\"%(authzone)s\". Please add NS record "
> + u"\"%(ns_rec)s\" to parent zone \"%(authzone)s\".")
> +
>
> 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..5c3a017989b23a1c6076d9dc4d93be65dd66cc67 100644
> --- a/ipalib/plugins/dns.py
> +++ b/ipalib/plugins/dns.py
> @@ -1725,6 +1725,241 @@ def _normalize_zone(zone):
> return zone
>
>
> +def _get_auth_zone_ldap(name):
> + """
> + Find authoritative zone in LDAP for name
Nitpick: Please add this note:
. Only active zones are considered.
> + :param name:
> + :return: (zone, truncated)
> + zone: authoritative zone, or None if authoritative zone is not in LDAP
> + """
> + assert isinstance(name, DNSName)
> + ldap = api.Backend.ldap2
> +
> + # Create all possible parent zone names
> + search_name = name.make_absolute()
> + zone_names = []
> + for i in xrange(len(search_name)):
> + zone_name_abs = DNSName(search_name[i:]).ToASCII()
> + zone_names.append(zone_name_abs)
> + # compatibility with IPA < 4.0, zone name can be relative
> + zone_names.append(zone_name_abs[:-1])
> +
> + # Create filters
> + objectclass_filter = ldap.make_filter({'objectclass':'idnszone'})
> + zonenames_filter = ldap.make_filter({'idnsname': zone_names})
> + zoneactive_filter = ldap.make_filter({'idnsZoneActive': 'true'})
> + complete_filter = ldap.combine_filters(
> + [objectclass_filter, zonenames_filter, zoneactive_filter],
> + rules=ldap.MATCH_ALL
> + )
> +
> + try:
> + entries, truncated = ldap.find_entries(
> + filter=complete_filter,
> + attrs_list=['idnsname'],
> + base_dn=DN(api.env.container_dns, api.env.basedn),
> + scope=ldap.SCOPE_ONELEVEL
> + )
> + except errors.NotFound:
> + return None, False
> +
> + # always use absolute zones
> + matched_auth_zones = [entry.single_value['idnsname'].make_absolute()
> + for entry in entries]
> +
> + # return longest match
> + return max(matched_auth_zones, key=len), truncated
> +
> +
> +def _get_longest_match_ns_delegation_ldap(zone, name):
> + """
> + Finds record in LDAP which has the longest match with name.
> +
> + NOTE: does not search in zone apex, returns None if there is no NS
> + delegation outside of zone apex
Nitpick:
Searches for deepest delegation for name in LDAP zone.
NOTE: NS record in zone apex is not considered as delegation. It returns None
if there is no delegation outside of zone apex.
> +
> + Example:
> + zone: example.com.
> + name: ns.sub.example.com.
> +
> + records:
> + extra.ns.sub.example.com.
> + sub.example.com.
> + example.com
> +
> + result: sub.example.com.
> +
> + :param zone: zone name
> + :param name:
> + :return: (record, truncated);
> + record: record name if success, or None if no such record exists, or
> + record is zone apex record
Nitpick:
:return: (match, truncated);
match: delegation name if success, or None if no delegation record exists
> + """
> + assert isinstance(zone, DNSName)
> + assert isinstance(name, DNSName)
> +
> + ldap = api.Backend.ldap2
> +
> + # get zone DN
> + zone_dn = api.Object.dnszone.get_dn(zone)
> +
> + if name.is_absolute():
> + relative_record_name = name.relativize(zone.make_absolute())
> + else:
> + relative_record_name = name
> +
> + # Name is zone apex
> + if relative_record_name.is_empty():
> + return None, False
> +
> + # create list of possible record names
> + possible_record_names = [DNSName(relative_record_name[i:]).ToASCII()
> + for i in xrange(len(relative_record_name))]
> +
> + # search filters
> + name_filter = ldap.make_filter({'idnsname': [possible_record_names]})
> + objectclass_filter = ldap.make_filter({'objectclass': 'idnsrecord'})
> + complete_filter = ldap.combine_filters(
> + [name_filter, objectclass_filter],
> + rules=ldap.MATCH_ALL
> + )
> +
> + try:
> + entries, truncated = ldap.find_entries(
> + filter=complete_filter,
> + attrs_list=['idnsname', 'nsrecord'],
> + base_dn=zone_dn,
> + scope=ldap.SCOPE_ONELEVEL
> + )
> + except errors.NotFound:
> + return None, False
> +
> + matched_records = []
> +
> + # test if entry contains NS records
> + for entry in entries:
> + print entry
Not-a-nitpick :-) ^^^
> + if entry.get('nsrecord'):
> + matched_records.append(entry.single_value['idnsname'])
> +
> + if not matched_records:
> + return None, truncated
> +
> + # return longest match
> + return max(matched_records, key=len), truncated
> +
> +
> +def _find_subtree_forward_zones_ldap(name, child_zones_only=False):
> + """
> + Search for forwardzone <name> and all child forwardzones
> + Filter: (|(*.<name>.)(<name>.))
> + :param name:
> + :param child_zones_only: search only for child zones
> + :return: (list of zonenames, truncated), list is empty if no zone found
> + """
> + assert isinstance(name, DNSName)
> + ldap = api.Backend.ldap2
> +
> + # prepare for filter "*.<name>."
> + search_name = u".%s" % name.make_absolute().ToASCII()
> +
> + # we need to search zone with and without last dot, due compatibility
> + # with IPA < 4.0
> + search_names = [search_name, search_name[:-1]]
> +
> + # Create filters
> + objectclass_filter = ldap.make_filter({'objectclass':'idnsforwardzone'})
> + zonenames_filter = ldap.make_filter({'idnsname': search_names}, exact=False,
> + trailing_wildcard=False)
> + if not child_zones_only:
> + # find also zone with exact name
> + exact_name = name.make_absolute().ToASCII()
> + # we need to search zone with and without last dot, due compatibility
> + # with IPA < 4.0
> + exact_names = [exact_name, exact_name[-1]]
> + exact_name_filter = ldap.make_filter({'idnsname': exact_names})
> + zonenames_filter = ldap.combine_filters([zonenames_filter,
> + exact_name_filter])
> +
> + zoneactive_filter = ldap.make_filter({'idnsZoneActive': 'true'})
> + complete_filter = ldap.combine_filters(
> + [objectclass_filter, zonenames_filter, zoneactive_filter],
> + rules=ldap.MATCH_ALL
> + )
> +
> + try:
> + entries, truncated = ldap.find_entries(
> + filter=complete_filter,
> + attrs_list=['idnsname'],
> + base_dn=DN(api.env.container_dns, api.env.basedn),
> + scope=ldap.SCOPE_ONELEVEL
> + )
> + except errors.NotFound:
> + return [], False
> +
> + result = [entry.single_value['idnsname'].make_absolute()
> + for entry in entries]
> +
> + return result, truncated
> +
> +
> +def _get_zone_which_makes_fw_zone_ineffective(fwzonename):
> + """
> + Check if forward zone is effective.
> +
> + If parent zone exists as authoritative zone, the forward zone will not
> + forward queries by default. It is necessary to delegate authority
> + to forward zone with a NS record.
> +
> + Example:
> +
> + Forward zone: sub.example.com
> + Zone: example.com
> +
> + Forwarding will not work, because the server thinks it is authoritative
> + for zone and will return NXDOMAIN
> +
> + Adding record: sub.example.com NS ns.sub.example.com.
> + will delegate authority, and IPA DNS server will forward DNS queries.
> +
> + :param fwzonename: forwardzone
> + :return: (zone, truncated)
> + zone: None if effective, name of authoritative zone otherwise
> + """
> + assert isinstance(fwzonename, DNSName)
> +
> + auth_zone, truncated_zone = _get_auth_zone_ldap(fwzonename)
> + if not auth_zone:
> + return None, truncated_zone
> +
> + delegation_record_name, truncated_ns =\
> + _get_longest_match_ns_delegation_ldap(auth_zone, fwzonename)
> +
> + truncated = truncated_ns or truncated_zone
> +
> + if delegation_record_name:
> + return None, truncated
> +
> + return auth_zone, truncated
> +
> +
> +def _add_warning_fw_zone_is_not_effective(result, fwzone, version):
> + """
> + Adds warning message to result, if required
> + """
> + authoritative_zone, truncated = \
> + _get_zone_which_makes_fw_zone_ineffective(fwzone)
> + if authoritative_zone:
> + # forward zone is not effective and forwarding will not work
> + messages.add_message(
> + version, result,
> + messages.ForwardzoneIsNotEffectiveWarning(
> + fwzone=fwzone, authzone=authoritative_zone,
> + ns_rec=fwzone.relativize(authoritative_zone)
> + )
> + )
> +
> +
> class DNSZoneBase(LDAPObject):
> """
> Base class for DNS Zone
> @@ -2376,6 +2611,22 @@ class dnszone(DNSZoneBase):
> )
> )
>
> + def _warning_fw_zone_is_not_effective(self, result, *keys, **options):
> + """
> + Warning if any operation with zone causes, a child forward zone is
> + not effective
> + """
> + zone = keys[-1]
> + affected_fw_zones, truncated = _find_subtree_forward_zones_ldap(
> + zone, child_zones_only=True)
> + if not affected_fw_zones:
> + return
> +
> + for fwzone in affected_fw_zones:
> + _add_warning_fw_zone_is_not_effective(result, fwzone,
> + options['version'])
> +
> +
> @register()
> class dnszone_add(DNSZoneBase_add):
> __doc__ = _('Create new DNS zone (SOA record).')
> @@ -2445,6 +2696,7 @@ class dnszone_add(DNSZoneBase_add):
> self.obj._warning_forwarding(result, **options)
> self.obj._warning_dnssec_experimental(result, *keys, **options)
> self.obj._warning_name_server_option(result, context, **options)
> + self.obj._warning_fw_zone_is_not_effective(result, *keys, **options)
> return result
>
> def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
> @@ -2475,6 +2727,13 @@ class dnszone_del(DNSZoneBase_del):
>
> msg_summary = _('Deleted DNS zone "%(value)s"')
>
> + def execute(self, *keys, **options):
> + result = super(dnszone_del, self).execute(*keys, **options)
> + nkeys = keys[-1] # we can delete more zones
> + for key in nkeys:
> + self.obj._warning_fw_zone_is_not_effective(result, key, **options)
> + return result
> +
> def post_callback(self, ldap, dn, *keys, **options):
> super(dnszone_del, self).post_callback(ldap, dn, *keys, **options)
>
> @@ -2595,12 +2854,22 @@ class dnszone_disable(DNSZoneBase_disable):
> __doc__ = _('Disable DNS Zone.')
> msg_summary = _('Disabled DNS zone "%(value)s"')
>
> + def execute(self, *keys, **options):
> + result = super(dnszone_disable, self).execute(*keys, **options)
> + self.obj._warning_fw_zone_is_not_effective(result, *keys, **options)
> + return result
> +
>
> @register()
> class dnszone_enable(DNSZoneBase_enable):
> __doc__ = _('Enable DNS Zone.')
> msg_summary = _('Enabled DNS zone "%(value)s"')
>
> + def execute(self, *keys, **options):
> + result = super(dnszone_enable, self).execute(*keys, **options)
> + self.obj._warning_fw_zone_is_not_effective(result, *keys, **options)
> + return result
> +
>
> @register()
> class dnszone_add_permission(DNSZoneBase_add_permission):
> @@ -3164,6 +3433,25 @@ 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, result, *keys,
> + **options):
> + """Detect if NS record change can make forward zones ineffective due
> + missing delegation. Run after parent's execute method method.
Nitpick: method.
> + """
> + record_name_absolute = keys[-1]
> + zone = keys[-2]
> +
> + if not record_name_absolute.is_absolute():
> + record_name_absolute = record_name_absolute.derelativize(zone)
> +
> + affected_fw_zones, truncated = _find_subtree_forward_zones_ldap(
> + record_name_absolute)
> + if not affected_fw_zones:
> + return
> +
> + for fwzone in affected_fw_zones:
> + _add_warning_fw_zone_is_not_effective(result, fwzone,
> + options['version'])
>
>
> @register()
> @@ -3487,7 +3775,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(result,
> + *keys,
> + **options)
> return result
>
> def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
> @@ -3654,19 +3945,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(result,
> + *keys,
> + **options)
> return result
>
> def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
> @@ -3998,6 +4293,11 @@ class dnsforwardzone(DNSZoneBase):
> # managed_permissions: permissions was apllied in dnszone class, do NOT
> # add them here, they should not be applied twice.
>
> + def _warning_fw_zone_is_not_effective(self, result, *keys, **options):
> + fwzone = keys[-1]
> + _add_warning_fw_zone_is_not_effective(result, fwzone,
> + options['version'])
> +
>
> @register()
> class dnsforwardzone_add(DNSZoneBase_add):
> @@ -4019,6 +4319,11 @@ class dnsforwardzone_add(DNSZoneBase_add):
>
> return dn
>
> + def execute(self, *keys, **options):
> + result = super(dnsforwardzone_add, self).execute(*keys, **options)
> + self.obj._warning_fw_zone_is_not_effective(result, *keys, **options)
> + return result
> +
>
> @register()
> class dnsforwardzone_del(DNSZoneBase_del):
> @@ -4083,6 +4388,11 @@ class dnsforwardzone_enable(DNSZoneBase_enable):
> __doc__ = _('Enable DNS Forward Zone.')
> msg_summary = _('Enabled DNS forward zone "%(value)s"')
>
> + def execute(self, *keys, **options):
> + result = super(dnsforwardzone_enable, self).execute(*keys, **options)
> + self.obj._warning_fw_zone_is_not_effective(result, *keys, **options)
> + return result
> +
>
> @register()
> class dnsforwardzone_add_permission(DNSZoneBase_add_permission):
> -- 1.8.3.1
Thank you for patience!
--
Petr^2 Spacek
More information about the Freeipa-devel
mailing list