[Freeipa-devel] [PATCH 0086] Respect global forwarders from named.conf if they are not overridden by LDAP configuration
Adam Tkac
atkac at redhat.com
Tue Nov 6 14:38:16 UTC 2012
On Fri, Nov 02, 2012 at 07:02:29PM +0100, Petr Spacek wrote:
> On 11/02/2012 06:52 PM, Petr Spacek wrote:
> >On 11/02/2012 06:45 PM, Petr Spacek wrote:
> >>Hello,
> >>
> >>Respect global forwarders from named.conf if they are not overridden by LDAP
> >>configuration.
> >>
> >>Original configuration from named.conf is saved on load.
> >>Original configuration will be restored if configuration in LDAP is not
> >>present or is invalid.
> >>
> >>
> >>Another patch for cache flushing will follow.
> >
> >Well, another Friday - another forgotten patch.
> >
> For my mail client: The patch is *attached*.
Ack
> From 6439769095d018f1487e53e7dc1a597e411cf33a Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Fri, 2 Nov 2012 17:48:04 +0100
> Subject: [PATCH] Respect global forwarders from named.conf if they are not
> overridden by LDAP configuration.
>
> Original configuration from named.conf is saved on load.
> Original configuration will be restored if configuration in LDAP
> is not present or is invalid.
>
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
> src/ldap_helper.c | 173 ++++++++++++++++++++++++++++++++++++++++++------------
> src/settings.c | 19 ++++++
> src/settings.h | 4 ++
> src/types.h | 5 ++
> 4 files changed, 163 insertions(+), 38 deletions(-)
>
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index 5d7138b1861a85ad8f3d8027e94f4f807355f48a..2d52bfaf894de8a5591f966b0c9197d14a1e73f7 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -84,6 +84,13 @@
> #define MINTSIZ (65535 - 12 - 1 - 2 - 2 - 4 - 2)
> #define TOKENSIZ (8*1024)
>
> +const enum_txt_assoc_t forwarder_policy_txts[] = {
> + { dns_fwdpolicy_none, "none" },
> + { dns_fwdpolicy_first, "first" },
> + { dns_fwdpolicy_only, "only" },
> + { -1, NULL } /* end marker */
> +};
> +
> #define LDAP_OPT_CHECK(r, ...) \
> do { \
> if ((r) != LDAP_OPT_SUCCESS) { \
> @@ -177,6 +184,7 @@ struct ldap_instance {
> isc_boolean_t sync_ptr;
> isc_boolean_t dyn_update;
> isc_boolean_t serial_autoincrement;
> + dns_forwarders_t orig_global_forwarders; /* from named.conf */
> };
>
> struct ldap_pool {
> @@ -335,6 +343,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
> ldap_instance_t *ldap_inst;
> dns_view_t *view = NULL;
> ld_string_t *auth_method_str = NULL;
> + dns_forwarders_t *orig_global_forwarders = NULL;
> setting_t ldap_settings[] = {
> { "uri", no_default_string },
> { "connections", default_uint(2) },
> @@ -373,6 +382,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
> view = dns_dyndb_get_view(dyndb_args);
> dns_view_attach(view, &ldap_inst->view);
> ldap_inst->zmgr = dns_dyndb_get_zonemgr(dyndb_args);
> + ISC_LIST_INIT(ldap_inst->orig_global_forwarders.addrs);
>
> CHECK(zr_create(mctx, &ldap_inst->zone_register));
>
> @@ -490,6 +500,35 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
> CHECK(ldap_pool_create(mctx, ldap_inst->connections, &ldap_inst->pool));
> CHECK(ldap_pool_connect(ldap_inst->pool, ldap_inst));
>
> + /* copy global forwarders setting for configuration roll back in
> + * configure_zone_forwarders() */
> + result = dns_fwdtable_find(ldap_inst->view->fwdtable, dns_rootname,
> + &orig_global_forwarders);
> + if (result == ISC_R_SUCCESS) {
> + isc_sockaddr_t *addr;
> + isc_sockaddr_t *new_addr;
> + for (addr = ISC_LIST_HEAD(orig_global_forwarders->addrs);
> + addr != NULL;
> + addr = ISC_LIST_NEXT(addr, link)) {
> + new_addr = isc_mem_get(mctx, sizeof(isc_sockaddr_t));
> + if (new_addr == NULL)
> + CLEANUP_WITH(ISC_R_NOMEMORY);
> + *new_addr = *addr;
> + ISC_LINK_INIT(new_addr, link);
> + ISC_LIST_APPEND(ldap_inst->orig_global_forwarders.addrs,
> + new_addr, link);
> + }
> + ldap_inst->orig_global_forwarders.fwdpolicy =
> + orig_global_forwarders->fwdpolicy;
> +
> + } else if (result == ISC_R_NOTFOUND) {
> + /* global forwarders are not configured */
> + ldap_inst->orig_global_forwarders.fwdpolicy = dns_fwdpolicy_none;
> + result = ISC_R_SUCCESS;
> + } else {
> + goto cleanup;
> + }
> +
> if (ldap_inst->psearch) {
> /* Start the watcher thread */
> result = isc_thread_create(ldap_psearch_watcher, ldap_inst,
> @@ -519,6 +558,7 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
> dns_rbt_t *rbt = NULL;
> isc_result_t result = ISC_R_SUCCESS;
> const char *db_name;
> + isc_sockaddr_t *addr;
>
> REQUIRE(ldap_instp != NULL && *ldap_instp != NULL);
>
> @@ -628,6 +668,12 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
>
> zr_destroy(&ldap_inst->zone_register);
>
> + while (!ISC_LIST_EMPTY(ldap_inst->orig_global_forwarders.addrs)) {
> + addr = ISC_LIST_HEAD(ldap_inst->orig_global_forwarders.addrs);
> + ISC_LIST_UNLINK(ldap_inst->orig_global_forwarders.addrs, addr, link);
> + isc_mem_put(ldap_inst->mctx, addr, sizeof(isc_sockaddr_t));
> + }
> +
> isc_mem_putanddetach(&ldap_inst->mctx, ldap_inst, sizeof(ldap_instance_t));
>
> *ldap_instp = NULL;
> @@ -882,6 +928,22 @@ cleanup:
> return result;
> }
>
> +static isc_result_t
> +delete_forwarding_table(ldap_instance_t *inst, dns_name_t *name,
> + const char *msg_obj_type, const char *dn) {
> + isc_result_t result;
> +
> + /* Clean old fwdtable. */
> + result = dns_fwdtable_delete(inst->view->fwdtable, name);
> + if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND) {
> + log_error_r("%s '%s': failed to delete forwarders",
> + msg_obj_type, dn);
> + return result;
> + } else {
> + return ISC_R_SUCCESS; /* ISC_R_NOTFOUND = nothing to delete */
> + }
> +}
> +
> /**
> * Read forwarding policy (from idnsForwardingPolicy attribute) and
> * list of forwarders (from idnsForwarders multi-value attribute)
> @@ -911,22 +973,33 @@ configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst,
> ldap_valuelist_t values;
> ldap_value_t *value;
> isc_sockaddrlist_t addrs;
> + isc_boolean_t is_global_config;
> + isc_boolean_t fwdtbl_deletion_requested = ISC_TRUE;
> + const char *msg_use_global_fwds;
> + const char *msg_obj_type;
> + const char *msg_forwarders_not_def;
> + const char *msg_forward_policy = NULL;
> /**
> * 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);
> -
> - /* Clean old fwdtable. */
> - result = dns_fwdtable_delete(inst->view->fwdtable, name);
> - if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND) {
> - log_error("zone '%s': failed to delete forwarders", dn);
> - return ISC_R_FAILURE;
> + ISC_LIST_INIT(addrs);
> + if (dns_name_equal(name, dns_rootname)) {
> + is_global_config = ISC_TRUE;
> + msg_obj_type = "global configuration";
> + msg_use_global_fwds = "; global forwarders will be disabled";
> + msg_forwarders_not_def = "; global forwarders from "
> + "configuration file will be used";
> + } else {
> + is_global_config = ISC_FALSE;
> + msg_obj_type = "zone";
> + msg_use_global_fwds = "; global forwarders will be used "
> + "(if they are configured)";
> + msg_forwarders_not_def = msg_use_global_fwds;
> }
>
> /*
> @@ -943,72 +1016,96 @@ configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst,
> else if (strcasecmp(value->value, "none") == 0)
> fwdpolicy = dns_fwdpolicy_none;
> else {
> - log_error("zone '%s': invalid value '%s' in "
> + log_error("%s '%s': invalid value '%s' in "
> "idnsForwardPolicy attribute; "
> - "valid values: first, only, none; "
> + "valid values: first, only, none"
> "%s",
> - dn, value->value, msg_use_global_fwds);
> - return ISC_R_UNEXPECTEDTOKEN;
> + msg_obj_type, dn, value->value,
> + msg_use_global_fwds);
> + CLEANUP_WITH(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;
> + log_debug(5, "%s '%s': idnsForwarders attribute is "
> + "not present%s", msg_obj_type, dn,
> + msg_forwarders_not_def);
> + if (is_global_config) {
> + ISC_LIST_INIT(values);
> + addrs = inst->orig_global_forwarders.addrs;
> + fwdpolicy = inst->orig_global_forwarders.fwdpolicy;
> + } else {
> + CLEANUP_WITH(ISC_R_DISABLED);
> + }
> }
> }
>
> - ISC_LIST_INIT(addrs);
> + CHECK(get_enum_description(forwarder_policy_txts, fwdpolicy,
> + &msg_forward_policy));
> + log_debug(5, "%s '%s': forward policy is '%s'", msg_obj_type, dn,
> + msg_forward_policy);
> +
> 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)
> != ISC_R_SUCCESS) {
> - log_error("zone '%s': could not parse forwarder '%s'",
> - dn, value->value);
> + log_error("%s '%s': could not parse forwarder '%s'",
> + msg_obj_type, 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, "zone '%s': adding forwarder '%s'", dn, forwarder_txt);
> + log_debug(5, "%s '%s': adding forwarder '%s'", msg_obj_type,
> + dn, forwarder_txt);
> }
>
> 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;
> + log_debug(5, "%s '%s': all idnsForwarders are invalid%s",
> + msg_obj_type, dn, msg_use_global_fwds);
> + CLEANUP_WITH(ISC_R_UNEXPECTEDTOKEN);
> } else if (fwdpolicy == dns_fwdpolicy_none) {
> - log_debug(5, "zone '%s': forwarding explicitly disabled "
> - "(policy 'none', ignoring global forwarders)", dn);
> + log_debug(5, "%s '%s': forwarding explicitly disabled "
> + "(policy 'none', ignoring global forwarders)",
> + msg_obj_type, dn);
> }
>
> /* Set forward table up. */
> + CHECK(delete_forwarding_table(inst, name, msg_obj_type, dn));
> result = dns_fwdtable_add(inst->view->fwdtable, name, &addrs, fwdpolicy);
> -
> - while (!ISC_LIST_EMPTY(addrs)) {
> - isc_sockaddr_t *addr = NULL;
> - addr = ISC_LIST_HEAD(addrs);
> - ISC_LIST_UNLINK(addrs, addr, link);
> - isc_mem_put(inst->mctx, addr, sizeof(*addr));
> + if (result == ISC_R_SUCCESS) {
> + fwdtbl_deletion_requested = ISC_FALSE;
> + if (fwdpolicy == dns_fwdpolicy_none)
> + result = ISC_R_DISABLED;
> + } else {
> + log_error_r("%s '%s': forwarding table update failed",
> + msg_obj_type, dn);
> }
>
> - 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;
> -
> +cleanup:
> + if (ISC_LIST_HEAD(addrs) !=
> + ISC_LIST_HEAD(inst->orig_global_forwarders.addrs)) {
> + while(!ISC_LIST_EMPTY(addrs)) {
> + isc_sockaddr_t *addr = NULL;
> + addr = ISC_LIST_HEAD(addrs);
> + ISC_LIST_UNLINK(addrs, addr, link);
> + isc_mem_put(inst->mctx, addr, sizeof(*addr));
> + }
> + }
> + if (fwdtbl_deletion_requested) {
> + isc_result_t orig_result = result;
> + result = delete_forwarding_table(inst, name, msg_obj_type, dn);
> + if (result == ISC_R_SUCCESS)
> + result = orig_result;
> + }
> return result;
> }
>
> diff --git a/src/settings.c b/src/settings.c
> index de9c7738d68954e9cb2f141d10963aa09da6a19f..25578ce2687bf12e3a2d387caf0b26ed1a3083b6 100644
> --- a/src/settings.c
> +++ b/src/settings.c
> @@ -30,6 +30,7 @@
> #include "settings.h"
> #include "str.h"
> #include "util.h"
> +#include "types.h"
>
>
> /*
> @@ -195,3 +196,21 @@ get_value_str(const char *arg)
>
> return arg;
> }
> +
> +isc_result_t
> +get_enum_description(const enum_txt_assoc_t *map, int value, const char **desc) {
> + const enum_txt_assoc_t *record;
> +
> + REQUIRE(map != NULL);
> + REQUIRE(desc != NULL && *desc == NULL);
> +
> + for (record = map;
> + record->description != NULL && record->value != -1;
> + record++) {
> + if (record->value == value) {
> + *desc = record->description;
> + return ISC_R_SUCCESS;
> + }
> + }
> + return ISC_R_NOTFOUND;
> +}
> diff --git a/src/settings.h b/src/settings.h
> index f1846695e1d1433cd310c6b9fba3076171fca2eb..53910ee11c2ac1f87db25fac8f24f5743f4312e4 100644
> --- a/src/settings.h
> +++ b/src/settings.h
> @@ -22,6 +22,7 @@
> #define _LD_SETTINGS_H_
>
> #include <isc/types.h>
> +#include "types.h"
>
> typedef struct setting setting_t;
>
> @@ -77,4 +78,7 @@ struct setting {
> isc_result_t
> set_settings(setting_t *settings, const char * const* argv);
>
> +isc_result_t
> +get_enum_description(const enum_txt_assoc_t *map, int value, const char **desc);
> +
> #endif /* !_LD_SETTINGS_H_ */
> diff --git a/src/types.h b/src/types.h
> index fe8dc62dda60a79ab753a7d32482f122c466c1bf..7d3bce4a26c99627bb1183dfd37814c4a7507d6e 100644
> --- a/src/types.h
> +++ b/src/types.h
> @@ -49,6 +49,11 @@ struct ldapdb_node {
> ISC_LINK(ldapdb_node_t) link;
> };
>
> +typedef struct enum_txt_assoc {
> + int value;
> + const char *description;
> +} enum_txt_assoc_t;
> +
> isc_result_t
> ldapdbnode_create(isc_mem_t *mctx, dns_name_t *owner, ldapdb_node_t **nodep);
> #endif /* !_LD_TYPES_H_ */
> --
> 1.7.11.7
>
--
Adam Tkac, Red Hat, Inc.
More information about the Freeipa-devel
mailing list