On Fri, Oct 26, 2012 at 02:47:17PM +0200, Petr Spacek wrote:
> Hello,
> 
>     Add forward policy "none".
> 
>     This policy adds ability to disable forwarding on per-zone basics.
>     Now it is possible to forward all queries to global forwarders
>     and selectively disable forwarding for specific master zones.
> 
>     It will be handy in situation where global forwarder is not able
>     to resolve records from some sub-zones served by internal DNS
>     servers.
> 
>     Logging and error handling related to forwarders was cleaned up.
> 
>     https://fedorahosted.org/bind-dyndb-ldap/ticket/98
> 
> Please review changes in README also, it can be clearer I guess.

Ack

> From ba54f3f900abc877004bf76a62efa92321976313 Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspa...@redhat.com>
> Date: Thu, 25 Oct 2012 16:36:29 +0200
> Subject: [PATCH] Add forward policy "none".
> 
> This policy adds ability to disable forwarding on per-zone basics.
> Now it is possible to forward all queries to global forwarders
> and selectively disable forwarding for specific master zones.
> 
> It will be handy in situation where global forwarder is not able
> to resolve records from some sub-zones served by internal DNS
> servers.
> 
> Logging and error handling related to forwarders was cleaned up.
> 
> https://fedorahosted.org/bind-dyndb-ldap/ticket/98
> 
> Signed-off-by: Petr Spacek <pspa...@redhat.com>
> ---
>  NEWS              |   2 +
>  README            |  22 +++++++--
>  src/ldap_helper.c | 141 
> +++++++++++++++++++++++++++++++++++-------------------
>  3 files changed, 113 insertions(+), 52 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 
> 431274111486eaa82c7961f98d0153b252709623..3bbd344ce1603401c75b430c7a63b67fceac8574
>  100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,3 +1,5 @@
> +[1] Forward policy "none" was introduced.
> +
>  2.0
>  ======
>  [1] SOA serial number can be incremented automatically after each change
> diff --git a/README b/README
> index 
> 7539e7642054ffbc3e9d36904627198ffd43322c..e4805fb2b3272d6f736178abd3c9f77dece2ed12
>  100644
> --- a/README
> +++ b/README
> @@ -86,18 +86,32 @@ example zone ldif is available in the doc directory.
>       zone. Reverse zone must have Dynamic update allowed. 
>       (See idnsAllowDynUpdate attribute and dyn_update configuration 
> parameter.)
>  
> -* idnsForwardPolicy
> -     Specifies BIND9 zone forward policy. Only relevant in conjunction 
> -     with a valid idnsForwarders attribute.
> +* idnsForwardPolicy (default "first")
> +     Specifies BIND9 zone forward policy. Proprietary value "none"
> +     is equivalent to "forwarders {};" in BIND configuration,
> +     i.e. effectively disables forwarding and ignores idnsForwarders
> +     attribute.
> +
> +     Value "none" disables forwarding for given zone and ignores
> +     global forwarders. Zone with forward policy "none" is considered
> +     as type "master", not "forward".
> +     Values "first" and "only" are relevant in conjunction with a valid
> +     idnsForwarders attribute. Their meaning is same as in BIND9.
>  
>  * idnsForwarders
> -     Defines multiple IP addresses to which queries will be forwarded.
> +     Defines multiple IP addresses to which queries will be forwarded and
> +     effectively creates "forward" zones.
>       It is multi-value attribute: Each IP address (and optional port) has to
>       be in own value. BIND9 syntax for "forwarders" is required.
>       Optional port can be specified by adding " port <number>" after IP 
>       address. IPv4 and IPv6 addresses are supported.
>       Examples: "1.2.3.4" or "1.2.3.4 port 553" or "A::B" or "A::B port 553"
>  
> +     Zones with idnsForwarders attribute specified and forward policy other
> +     than "none" are considered as "forward" zones. All records in LDAP
> +     belonging to those zones are ignored and all queries are forwarded.
> +
> +
>  5. Configuration
>  ================
>  
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index 
> 8534362ae119e51931af375658bcdddd99d8e88a..dce1943484cfeb1519ead67c28c9377b155c275c
>  100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -877,70 +877,117 @@ cleanup:
>       return result;
>  }
>  
> -
> -
> +/**
> + * Read forwarding policy (from idnsForwardingPolicy attribute) and
> + * list of forwarders (from idnsForwarders multi-value attribute)
> + * and update forwarding settings for given zone.
> + *
> + * Enable forwarding if forwarders are specified and policy is not 'none'.
> + * Disable forwarding if forwarding policy is 'none' or list of forwarders
> + * is empty.
> + *
> + * Invalid forwarders are skipped, forwarding will be enabled if at least
> + * one valid forwarder is defined. Global forwarders will be used if all
> + * defined forwarders are invalid or list of forwarders is not present at 
> all.
> + *
> + * @retval ISC_R_SUCCESS  Forwarding was enabled.
> + * @retval ISC_R_DISABLED Forwarding was disabled.
> + * @retval ISC_R_UNEXPECTEDTOKEN Forwarding policy is invalid
> + *                               or all specified forwarders are invalid.
> + * @retval ISC_R_NOMEMORY
> + * @retval others      Some RBT manipulation errors including ISC_R_FAILURE.
> + */
>  static isc_result_t
>  configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst, 
> -                          dns_name_t *name, ldap_valuelist_t *values)
> +                          dns_name_t *name)
>  {
>       const char *dn = entry->dn;
>       isc_result_t result;
> +     ldap_valuelist_t values;
>       ldap_value_t *value;
>       isc_sockaddrlist_t addrs;
> +     /**
> +      * BIND forward policies are "first" (default) or "only".
> +      * We invented option "none" which disables forwarding for zone
> +      * regardless idnsForwarders attribute and global forwarders.
> +      */
> +     dns_fwdpolicy_t fwdpolicy = dns_fwdpolicy_first;
> +     const char msg_use_global_fwds[] = "global forwarders will be used "
> +                                        "(if they are configured)";
>  
> -     REQUIRE(entry != NULL && inst != NULL && name != NULL && values != 
> NULL);
> +     REQUIRE(entry != NULL && inst != NULL && name != NULL);
>  
>       /* Clean old fwdtable. */
>       result = dns_fwdtable_delete(inst->view->fwdtable, name);
>       if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND) {
> -             log_error("Failed to update forwarders");
> +             log_error("zone '%s': failed to delete forwarders", dn);
>               return ISC_R_FAILURE;
>       }
>  
> -     if (EMPTY(*values)) {
> -             log_debug(1, "Forwarders are empty for '%s'", dn);
> -             return ISC_R_SUCCESS;
> +     /*
> +      * Fetch forward policy.
> +      */
> +     result = ldap_entry_getvalues(entry, "idnsForwardPolicy", &values);
> +     if (result == ISC_R_SUCCESS) {
> +             value = HEAD(values);
> +             if (value != NULL && value->value != NULL) {
> +                     if (strcasecmp(value->value, "only") == 0)
> +                             fwdpolicy = dns_fwdpolicy_only;
> +                     else if (strcasecmp(value->value, "first") == 0)
> +                             fwdpolicy = dns_fwdpolicy_first;
> +                     else if (strcasecmp(value->value, "none") == 0)
> +                             fwdpolicy = dns_fwdpolicy_none;
> +                     else {
> +                             log_error("zone '%s': invalid value '%s' in "
> +                                       "idnsForwardPolicy attribute; "
> +                                       "valid values: first, only, none; "
> +                                       "%s",
> +                                       dn, value->value, 
> msg_use_global_fwds);
> +                             return ISC_R_UNEXPECTEDTOKEN;
> +                     }
> +             }
> +     }
> +     log_debug(5, "zone '%s': forward policy is '%s'", dn,
> +               (fwdpolicy == dns_fwdpolicy_first) ? "first" : value->value);
> +
> +     if (fwdpolicy == dns_fwdpolicy_none) {
> +             ISC_LIST_INIT(values); /* ignore idnsForwarders in LDAP */
> +     } else {
> +             result = ldap_entry_getvalues(entry, "idnsForwarders", &values);
> +             if (result == ISC_R_NOTFOUND || EMPTY(values)) {
> +                     log_debug(5, "zone '%s': idnsForwarders attribute is "
> +                               "not present, %s", dn, msg_use_global_fwds);
> +                     return ISC_R_DISABLED;
> +             }
>       }
>  
>       ISC_LIST_INIT(addrs);
> -     for (value = HEAD(*values); value != NULL; value = NEXT(value, link)) {
> +     for (value = HEAD(values); value != NULL; value = NEXT(value, link)) {
>               isc_sockaddr_t *addr = NULL;
>               char forwarder_txt[ISC_SOCKADDR_FORMATSIZE];
>  
> -             if( acl_parse_forwarder(value->value, inst->mctx, &addr)
> +             if (acl_parse_forwarder(value->value, inst->mctx, &addr)
>                               != ISC_R_SUCCESS) {
> -                     log_error("Could not parse forwarder '%s'", 
> value->value);
> +                     log_error("zone '%s': could not parse forwarder '%s'",
> +                               dn, value->value);
>                       continue;
>               }
>  
>               ISC_LINK_INIT(addr, link);
>               ISC_LIST_APPEND(addrs, addr, link);
>               isc_sockaddr_format(addr, forwarder_txt, 
> ISC_SOCKADDR_FORMATSIZE);
> -             log_debug(5, "Adding forwarder %s for %s", forwarder_txt, dn);
> +             log_debug(5, "zone '%s': adding forwarder '%s'", dn, 
> forwarder_txt);
>       }
>  
> -     /*
> -      * Fetch forward policy.
> -      */
> -     dns_fwdpolicy_t fwdpolicy = dns_fwdpolicy_first; /* "first" is default 
> option. */
> -     result = ldap_entry_getvalues(entry, "idnsForwardPolicy", values);
> -     if (result == ISC_R_SUCCESS) {
> -             value = HEAD(*values);
> -             /*
> -              * fwdpolicy: "only" or "first" (default)
> -              */
> -             if (value != NULL && value->value != NULL &&
> -                 strcmp(value->value, "only") == 0)
> -                     fwdpolicy = dns_fwdpolicy_only;
> +     if (fwdpolicy != dns_fwdpolicy_none && ISC_LIST_EMPTY(addrs)) {
> +             log_debug(5, "zone '%s': all idnsForwarders are invalid, %s",
> +                       dn, msg_use_global_fwds);
> +             return ISC_R_UNEXPECTEDTOKEN;
> +     } else if (fwdpolicy == dns_fwdpolicy_none) {
> +             log_debug(5, "zone '%s': forwarding explicitly disabled "
> +                       "(policy 'none', ignoring global forwarders)", dn);
>       }
>  
> -     if (ISC_LIST_EMPTY(addrs)) {
> -             log_debug(5, "No forwarders seen; disabling forwarding");
> -             fwdpolicy = dns_fwdpolicy_none;
> -     }
> -
> -     log_debug(5, "Forward policy: %d", fwdpolicy);
> -             
>       /* Set forward table up. */
>       result = dns_fwdtable_add(inst->view->fwdtable, name, &addrs, 
> fwdpolicy);
>  
> @@ -951,6 +998,12 @@ configure_zone_forwarders(ldap_entry_t *entry, 
> ldap_instance_t *inst,
>               isc_mem_put(inst->mctx, addr, sizeof(*addr));
>       }
>  
> +     if (result != ISC_R_SUCCESS)
> +             log_error_r("zone '%s': forwarding table update failed", dn);
> +
> +     if (fwdpolicy == dns_fwdpolicy_none && result == ISC_R_SUCCESS)
> +             result = ISC_R_DISABLED;
> +
>       return result;
>  }
>  
> @@ -971,13 +1024,9 @@ ldap_parse_configentry(ldap_entry_t *entry, 
> ldap_instance_t *inst)
>       log_debug(3, "Parsing configuration object");
>  
>       /* idnsForwardPolicy change is handled by configure_zone_forwarders() */
> -     result = ldap_entry_getvalues(entry, "idnsForwarders", &values);
> -     if (result == ISC_R_SUCCESS) {
> -             log_debug(2, "Setting global forwarders");
> -             result = configure_zone_forwarders(entry, inst, dns_rootname, 
> &values);
> -             if (result != ISC_R_SUCCESS) {
> -                     log_error("Global forwarder could not be set up.");
> -             }
> +     result = configure_zone_forwarders(entry, inst, dns_rootname);
> +     if (result != ISC_R_SUCCESS && result != ISC_R_DISABLED) {
> +             log_error("Global forwarder could not be set up.");
>       }
>  
>       result = ldap_entry_getvalues(entry, "idnsAllowSyncPTR", &values);
> @@ -1060,21 +1109,17 @@ ldap_parse_zoneentry(ldap_entry_t *entry, 
> ldap_instance_t *inst)
>  
>       CHECK(discard_from_cache(inst->cache, &name));
>  
> -     /* 
> -      * Fetch forwarders. 
> +     /*
>        * Forwarding has top priority hence when the forwarders are properly
>        * set up all others attributes are ignored.
>        */
> -     result = ldap_entry_getvalues(entry, "idnsForwarders", &values);
> -     if (result == ISC_R_SUCCESS) {
> -             log_debug(2, "Setting forwarders for %s", dn);
> -             CHECK(configure_zone_forwarders(entry, inst, &name, &values));
> +     result = configure_zone_forwarders(entry, inst, &name);
> +     if (result != ISC_R_DISABLED) {
>               /* DO NOT CHANGE ANYTHING ELSE after forwarders are set up! */
>               goto cleanup;
> -     } else {
> -             /* No forwarders are used. Remove zone from fwdtable. */
> -             CHECK(configure_zone_forwarders(entry, inst, &name, &values));
>       }
> +     /* No forwarders are used. Zone was removed from fwdtable.
> +      * Load the zone. */
>  
>       /* Check if we are already serving given zone */
>       result = zr_get_zone_ptr(inst->zone_register, &name, &zone);
> -- 
> 1.7.11.7
> 


-- 
Adam Tkac, Red Hat, Inc.

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

Reply via email to