[Freeipa-devel] [PATCH] 0025-0028 Implement SOA serial number increments for external changes
Adam Tkac
atkac at redhat.com
Fri Jul 13 13:42:46 UTC 2012
On Tue, Jul 10, 2012 at 03:57:24PM +0200, Petr Spacek wrote:
> Hello,
>
> these patches provides SOA serial auto-increment feature for external changes.
> Related ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/67
>
> It is necessary to set "psearch" AND "serial_autoincrement" to "yes"
> in /etc/named.conf to enable this feature.
>
> In replicated environment idnsSOAserial attribute has to be declared
> as non-replicated. It is done by mkosek's patch 281 for 389 DS &
> FreeIPA.
>
> For testing purposes it is enough to add "idnsSOAserial" to end of
> exclude list in nsDS5ReplicatedAttributeList attribute for each
> replication agreement located in cn=mapping tree,cn=config subtree.
>
>
> My patch 28 contains "trick" necessary for replicated environments
> with 389 DS. 389 sends entry change notification (ECN) in cases when
> non-replicated attribute idnsSOAserial was changed on *other side*.
> In that case no change is visible in DNS attributes, but ECN is sent
> by 389. (Attribute modifyTimestamp is changed also.)
>
> Patch 28 computes digest/hash from all resource records in idnsZone
> object and compares old and new digest after each received ECN. This
> approach eliminates "false changes".
>
> Each patch depends on all preceding patches, but each patch
> implements visible (and testable) part of functionality.
Hello Peter,
please check my comments below.
Regards, Adam
> From 0ed1d3dd4910e1c94617b0209420b1c6598de68e Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Wed, 27 Jun 2012 10:36:26 +0200
> Subject: [PATCH] Increment SOA serial for each ordinary record received
> through psearch
>
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
> src/ldap_helper.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 97 insertions(+), 1 deletions(-)
>
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index 7f0a6f4b37171a6fa4db79cd32fdd8bc62288e0f..5c509443dfb176607d8ee4714d196efaa4afa66b 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -34,6 +34,8 @@
> #include <dns/zt.h>
> #include <dns/byaddr.h>
> #include <dns/forward.h>
> +#include <dns/soa.h>
> +#include <isc/serial.h>
>
> #include <isc/buffer.h>
> #include <isc/lex.h>
> @@ -172,6 +174,7 @@ struct ldap_instance {
> isc_boolean_t exiting;
> isc_boolean_t sync_ptr;
> isc_boolean_t dyn_update;
> + isc_boolean_t serial_autoincrement;
> };
>
> struct ldap_pool {
> @@ -343,6 +346,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
> { "ldap_hostname", default_string("") },
> { "sync_ptr", default_boolean(ISC_FALSE) },
> { "dyn_update", default_boolean(ISC_FALSE) },
> + { "serial_autoincrement", default_boolean(ISC_FALSE) },
> end_of_settings
> };
>
> @@ -401,6 +405,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
> ldap_settings[i++].target = ldap_inst->ldap_hostname;
> ldap_settings[i++].target = &ldap_inst->sync_ptr;
> ldap_settings[i++].target = &ldap_inst->dyn_update;
> + ldap_settings[i++].target = &ldap_inst->serial_autoincrement;
> CHECK(set_settings(ldap_settings, argv));
>
> /* Set timer for deadlock detection inside semaphore_wait_timed . */
> @@ -463,6 +468,13 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
> "increasing limit");
> ldap_inst->connections = 3;
> }
> + if (ldap_inst->serial_autoincrement == ISC_TRUE
> + && ldap_inst->psearch != ISC_TRUE) {
> + log_error("SOA serial number auto-increment feature requires "
> + "persistent search");
> + result = ISC_R_FAILURE;
> + goto cleanup;
> + }
>
> CHECK(new_ldap_cache(mctx, argv, &ldap_inst->cache, ldap_inst->psearch));
> CHECK(ldap_pool_create(mctx, ldap_inst->connections, &ldap_inst->pool));
> @@ -2741,6 +2753,86 @@ ldap_pscontrol_destroy(isc_mem_t *mctx, LDAPControl **ctrlp)
> *ctrlp = NULL;
> }
>
> +isc_result_t
Better is "static inline isc_result_t"
> +soa_serial_get(ldap_instance_t *inst, dns_name_t *zone_name,
> + isc_uint32_t *serial) {
> + isc_result_t result;
> + dns_zone_t *zone = NULL;
> +
> + REQUIRE(inst != NULL);
> + REQUIRE(zone_name != NULL);
> + REQUIRE(serial != NULL);
All those REQUIREs are redundant, please remove them. The "inst" parameter is
checked before function call in soa_serial_increment and last two params are
checked by functions below.
> +
> + CHECK(zr_get_zone_ptr(inst->zone_register, zone_name, &zone));
> + CHECK(dns_zone_getserial2(zone, serial));
> + dns_zone_detach(&zone);
> +
> +cleanup:
> + return result;
> +}
> +
> +isc_result_t
> +soa_serial_increment(isc_mem_t *mctx, ldap_instance_t *inst,
> + dns_name_t *zone_name) {
> + isc_result_t result = ISC_R_FAILURE;
> + ldap_connection_t * conn = NULL;
> + ld_string_t *zone_dn = NULL;
> + ldapdb_rdatalist_t rdatalist;
> + dns_rdatalist_t *rdlist = NULL;
> + dns_rdata_t *soa_rdata = NULL;
> + isc_uint32_t old_serial;
> + isc_uint32_t new_serial;
> + isc_time_t curr_time;
> +
> + REQUIRE(inst != NULL);
> + REQUIRE(zone_name != NULL);
> +
> + CHECK(str_new(mctx, &zone_dn));
> + CHECK(dnsname_to_dn(inst->zone_register, zone_name, zone_dn));
> + log_debug(5, "incrementing SOA serial number in zone '%s'",
> + str_buf(zone_dn));
> +
> + /* get original SOA rdata and serial value */
> + INIT_LIST(rdatalist);
rdatalist variable should be initialized before the first CHECK call, othewise
we will crash in the "cleanup" path in ldapdb_rdatalist_destroy when earlier
CHECK() fails.
> + CHECK(ldapdb_rdatalist_get(mctx, inst, zone_name, zone_name, &rdatalist));
> + CHECK(ldapdb_rdatalist_findrdatatype(&rdatalist, dns_rdatatype_soa, &rdlist));
> + soa_rdata = ISC_LIST_HEAD(rdlist->rdata);
> + CHECK(soa_serial_get(inst, zone_name, &old_serial));
> +
> + /* Compute the new SOA serial - use actual timestamp.
> + * If timestamp <= oldSOAserial then increment old serial by one. */
> + isc_time_now(&curr_time);
> + new_serial = isc_time_seconds(&curr_time) & 0xFFFFFFFF;
> + if (!isc_serial_gt(new_serial, old_serial)) {
> + /* increment by one, RFC1982, from bind-9.8.2/bin/named/update.c */
> + new_serial = (old_serial + 1) & 0xFFFFFFFF;
> + }
> + if (new_serial == 0)
> + new_serial = 1;
> + log_debug(5,"zone '%s': old serial %u, new serial %u",
> + str_buf(zone_dn), old_serial, new_serial);
> + dns_soa_setserial(new_serial, soa_rdata);
> +
> + /* write the new serial back to DB */
> + CHECK(ldap_pool_getconnection(inst->pool, &conn));
> + CHECK(modify_soa_record(conn, str_buf(zone_dn), soa_rdata));
> + CHECK(discard_from_cache(ldap_instance_getcache(inst), zone_name));
> +
> + /* put the new SOA to inst->cache and compare old and new serials */
> + CHECK(soa_serial_get(inst, zone_name, &new_serial));
> + INSIST(isc_serial_gt(new_serial, old_serial) == ISC_TRUE);
> +
> +cleanup:
> + if (result != ISC_R_SUCCESS)
> + log_error("SOA serial number incrementation failed in zone '%s'",
> + str_buf(zone_dn));
> +
> + ldap_pool_putconnection(inst->pool, &conn);
> + str_destroy(&zone_dn);
> + ldapdb_rdatalist_destroy(mctx, &rdatalist);
> + return result;
> +}
> +
> /*
> * update_action routine is processed asynchronously so it cannot assume
> * anything about state of ldap_inst from where it was sent. The ldap_inst
> @@ -2892,7 +2984,7 @@ update_record(isc_task_t *task, isc_event_t *event)
> if (PSEARCH_DEL(pevent->chgtype)) {
> log_debug(5, "psearch_update: Removing item from cache (%s)",
> pevent->dn);
> - }
> + }
>
> /* Get cache instance & clean old record */
> cache = ldap_instance_getcache(inst);
> @@ -2916,6 +3008,10 @@ update_record(isc_task_t *task, isc_event_t *event)
> /* Destroy rdatalist, it is now in the cache. */
> ldapdb_rdatalist_destroy(mctx, &rdatalist);
> }
> +
> + if (inst->serial_autoincrement) {
> + CHECK(soa_serial_increment(mctx, inst, &origin));
> + }
> cleanup:
> if (result != ISC_R_SUCCESS)
> log_error("update_record (psearch) failed for %s. "
> --
> 1.7.7.6
>
> From b82c951ad09c72e524be93ba06b0f46897c85f71 Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Mon, 2 Jul 2012 11:01:58 +0200
> Subject: [PATCH] Do not bump serial for each record during initial database
> dump.
>
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
> src/ldap_helper.c | 18 ++++++++++++++----
> 1 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index 5c509443dfb176607d8ee4714d196efaa4afa66b..d744642364ba70919f52abd5ea31998f8e91ccd4 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -2677,6 +2677,7 @@ cleanup:
> #define LDAP_CONTROL_PERSISTENTSEARCH "2.16.840.1.113730.3.4.3"
> #define LDAP_CONTROL_ENTRYCHANGE "2.16.840.1.113730.3.4.7"
>
> +#define LDAP_ENTRYCHANGE_NONE 0 /* entry change control is not present */
> #define LDAP_ENTRYCHANGE_ADD 1
> #define LDAP_ENTRYCHANGE_DEL 2
> #define LDAP_ENTRYCHANGE_MOD 4
> @@ -2687,6 +2688,7 @@ cleanup:
> #define PSEARCH_DEL(chgtype) ((chgtype & LDAP_ENTRYCHANGE_DEL) != 0)
> #define PSEARCH_MOD(chgtype) ((chgtype & LDAP_ENTRYCHANGE_MOD) != 0)
> #define PSEARCH_MODDN(chgtype) ((chgtype & LDAP_ENTRYCHANGE_MODDN) != 0)
> +#define PSEARCH_ANY(chgtype) ((chgtype & LDAP_ENTRYCHANGE_ALL) != 0)
> /*
> * Creates persistent search (aka psearch,
> * http://tools.ietf.org/id/draft-ietf-ldapext-psearch-03.txt) control.
> @@ -2990,9 +2992,11 @@ update_record(isc_task_t *task, isc_event_t *event)
> cache = ldap_instance_getcache(inst);
> CHECK(discard_from_cache(cache, &name));
>
> - if (PSEARCH_ADD(pevent->chgtype) || PSEARCH_MOD(pevent->chgtype)) {
> + if (PSEARCH_ADD(pevent->chgtype) || PSEARCH_MOD(pevent->chgtype) ||
> + !PSEARCH_ANY(pevent->chgtype)) {
> /*
> - * Find new data in LDAP.
> + * Find new data in LDAP. !PSEARCH_ANY indicates unchanged entry
> + * found during initial lookup (i.e. database dump).
> *
> * @todo Change this to convert ldap_entry_t to ldapdb_rdatalist_t.
> */
> @@ -3009,7 +3013,13 @@ update_record(isc_task_t *task, isc_event_t *event)
> ldapdb_rdatalist_destroy(mctx, &rdatalist);
> }
>
> - if (inst->serial_autoincrement) {
> + log_debug(20,"psearch change type: none%d, add%d, del%d, mod%d, moddn%d",
> + !PSEARCH_ANY(pevent->chgtype), PSEARCH_ADD(pevent->chgtype),
> + PSEARCH_DEL(pevent->chgtype), PSEARCH_MOD(pevent->chgtype),
> + PSEARCH_MODDN(pevent->chgtype));
> +
> + /* Do not bump serial during initial database dump. */
> + if (inst->serial_autoincrement && PSEARCH_ANY(pevent->chgtype)) {
> CHECK(soa_serial_increment(mctx, inst, &origin));
> }
> cleanup:
> @@ -3092,7 +3102,7 @@ psearch_update(ldap_instance_t *inst, ldap_entry_t *entry, LDAPControl **ctrls)
> ldap_entryclass_t class;
> isc_result_t result = ISC_R_SUCCESS;
> ldap_psearchevent_t *pevent = NULL;
> - int chgtype = LDAP_ENTRYCHANGE_ADD;
> + int chgtype = LDAP_ENTRYCHANGE_NONE;
> char *moddn = NULL;
> char *dn = NULL;
> char *dbname = NULL;
> --
> 1.7.7.6
>
> From 9e85fd45baff2dd6fb696ce416af8ae8a8ecccbe Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Mon, 2 Jul 2012 16:40:23 +0200
> Subject: [PATCH] Maintain SOA serial for zone record changes also. Bump
> serial after each BIND startup. Manual changes to zone
> serial are allowed.
>
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
> src/ldap_helper.c | 61 ++++++++++++++++++++++++++++++++++++---------------
> src/ldap_helper.h | 6 +++++
> src/zone_register.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
> src/zone_register.h | 6 +++++
> 4 files changed, 114 insertions(+), 18 deletions(-)
>
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index d744642364ba70919f52abd5ea31998f8e91ccd4..da6f8562ef59dadc8882743ee87bf84305a1de82 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -1000,8 +1000,9 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
> isc_result_t result;
> isc_boolean_t unlock = ISC_FALSE;
> isc_boolean_t publish = ISC_FALSE;
> - isc_boolean_t load = ISC_FALSE;
> isc_task_t *task = inst->task;
> + isc_uint32_t ldap_serial;
> + isc_uint32_t zr_serial;
>
> dns_name_init(&name, NULL);
>
> @@ -1020,11 +1021,11 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
> * 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));
> + CHECK(configure_zone_forwarders(entry, inst, &name, &values));
> /* DO NOT CHANGE ANYTHING ELSE after forwarders are set up! */
> goto cleanup;
> } else {
> @@ -1080,25 +1081,49 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
> /* Everything is set correctly, publish zone */
> CHECK(publish_zone(inst, zone));
> }
> - load = ISC_TRUE;
> +
> + /*
> + * Don't bother if load fails, server will return
> + * SERVFAIL for queries beneath this zone. This is
> + * admin's problem.
> + */
> + result = dns_zone_load(zone);
> + if (result != ISC_R_SUCCESS && result != DNS_R_UPTODATE
> + && result != DNS_R_DYNAMIC && result != DNS_R_CONTINUE)
> + goto cleanup;
> +
> + result = ISC_R_SUCCESS;
> +
> + /* initialize serial in zone register and always increment serial
> + * for a new zone (typically after BIND start)
> + * - the zone was possibly changed in meanwhile */
> + if (publish) {
> + CHECK(ldap_get_zone_serial(inst, &name, &ldap_serial));
> + CHECK(zr_set_zone_serial(inst->zone_register, &name, ldap_serial));
> + }
> +
> + /* SOA serial autoincrement feature for SOA record:
> + * 1) remember old SOA serial from zone register
> + * 2) compare old and new SOA serial from LDAP DB
> + * 3) if (old == new) then do autoincrement, remember new serial otherwise */
> + if (inst->serial_autoincrement) {
> + CHECK(ldap_get_zone_serial(inst, &name, &ldap_serial));
> + CHECK(zr_get_zone_serial(inst->zone_register, &name, &zr_serial));
> + if (ldap_serial == zr_serial)
> + CHECK(soa_serial_increment(inst->mctx, inst, &name));
> + else
> + CHECK(zr_set_zone_serial(inst->zone_register, &name, ldap_serial));
> + }
>
> cleanup:
> - if (load) {
> - /*
> - * Don't bother if load fails, server will return
> - * SERVFAIL for queries beneath this zone. This is
> - * admin's problem.
> - */
> - (void) dns_zone_load(zone);
> - }
> if (unlock)
> isc_task_endexclusive(task);
> if (dns_name_dynamic(&name))
> dns_name_free(&name, inst->mctx);
> if (zone != NULL)
> dns_zone_detach(&zone);
>
> - return ISC_R_SUCCESS;
> + return result;
> }
>
> /*
> @@ -2756,7 +2781,7 @@ ldap_pscontrol_destroy(isc_mem_t *mctx, LDAPControl **ctrlp)
> }
>
> isc_result_t
> -soa_serial_get(ldap_instance_t *inst, dns_name_t *zone_name,
> +ldap_get_zone_serial(ldap_instance_t *inst, dns_name_t *zone_name,
> isc_uint32_t *serial) {
> isc_result_t result;
> dns_zone_t *zone = NULL;
> @@ -2799,7 +2824,7 @@ soa_serial_increment(isc_mem_t *mctx, ldap_instance_t *inst,
> CHECK(ldapdb_rdatalist_get(mctx, inst, zone_name, zone_name, &rdatalist));
> CHECK(ldapdb_rdatalist_findrdatatype(&rdatalist, dns_rdatatype_soa, &rdlist));
> soa_rdata = ISC_LIST_HEAD(rdlist->rdata);
> - CHECK(soa_serial_get(inst, zone_name, &old_serial));
> + CHECK(ldap_get_zone_serial(inst, zone_name, &old_serial));
>
> /* Compute the new SOA serial - use actual timestamp.
> * If timestamp <= oldSOAserial then increment old serial by one. */
> @@ -2821,7 +2846,7 @@ soa_serial_increment(isc_mem_t *mctx, ldap_instance_t *inst,
> CHECK(discard_from_cache(ldap_instance_getcache(inst), zone_name));
>
> /* put the new SOA to inst->cache and compare old and new serials */
> - CHECK(soa_serial_get(inst, zone_name, &new_serial));
> + CHECK(ldap_get_zone_serial(inst, zone_name, &new_serial));
> INSIST(isc_serial_gt(new_serial, old_serial) == ISC_TRUE);
>
> cleanup:
> @@ -2887,9 +2912,9 @@ update_action(isc_task_t *task, isc_event_t *event)
>
> cleanup:
> if (result != ISC_R_SUCCESS)
> - log_error("update_action (psearch) failed for %s. "
> + log_error("update_action (psearch) failed for '%s': %s. "
> "Zones can be outdated, run `rndc reload`",
> - pevent->dn);
> + pevent->dn, isc_result_totext(result));
>
> ldap_pool_putconnection(inst->pool, &conn);
> isc_mem_free(mctx, pevent->dbname);
> diff --git a/src/ldap_helper.h b/src/ldap_helper.h
> index bc784106abefe15787841578687469539c8052c3..b6e46e307fa7e2f0af1b71600fcabdb0c13c5649 100644
> --- a/src/ldap_helper.h
> +++ b/src/ldap_helper.h
> @@ -93,4 +93,10 @@ isc_result_t remove_from_ldap(dns_name_t *owner, ldap_instance_t *ldap_inst,
> /* Get cache associated with ldap_inst */
> ldap_cache_t *ldap_instance_getcache(ldap_instance_t *ldap_inst);
>
> +isc_result_t ldap_get_zone_serial(ldap_instance_t *inst, dns_name_t *zone_name,
> + isc_uint32_t *serial);
> +
> +isc_result_t soa_serial_increment(isc_mem_t *mctx, ldap_instance_t *inst,
> + dns_name_t *zone_name);
> +
Why do you need to export those functions out of the ldap_helper.c? I prefer to
mark them as static and don't export them.
> #endif /* !_LD_LDAP_HELPER_H_ */
> diff --git a/src/zone_register.c b/src/zone_register.c
> index 81d208fc6e3c0dba6efb72ae10db54a79c336eca..1de6bf1ac121dc02c54f187cd36f8b588ee14d18 100644
> --- a/src/zone_register.c
> +++ b/src/zone_register.c
> @@ -50,6 +50,7 @@ struct zone_register {
> typedef struct {
> dns_zone_t *zone;
> char *dn;
> + isc_uint32_t serial; /* last value processed by plugin (!= value in DB) */
> } zone_info_t;
>
> /* Callback for dns_rbt_create(). */
> @@ -136,6 +137,7 @@ create_zone_info(isc_mem_t *mctx, dns_zone_t *zone, const char *dn,
>
> CHECKED_MEM_GET_PTR(mctx, zinfo);
> CHECKED_MEM_STRDUP(mctx, dn, zinfo->dn);
> + zinfo->serial = 0;
> zinfo->zone = NULL;
> dns_zone_attach(zone, &zinfo->zone);
>
> @@ -312,3 +314,60 @@ zr_get_zone_ptr(zone_register_t *zr, dns_name_t *name, dns_zone_t **zonep)
>
> return result;
> }
> +
> +/**
> + * Return last SOA serial value processed by autoincrement feature.
> + */
> +isc_result_t
> +zr_get_zone_serial(zone_register_t *zr, dns_name_t *name, isc_uint32_t *serialp)
> +{
> + isc_result_t result;
> + void *zinfo = NULL;
> +
> + REQUIRE(zr != NULL);
> + REQUIRE(name != NULL);
^^^ This REQUIRE is redundant.
> + REQUIRE(serialp != NULL);
> +
> + if (!dns_name_isabsolute(name)) {
> + log_bug("trying to find zone with a relative name");
> + return ISC_R_FAILURE;
> + }
> +
> + RWLOCK(&zr->rwlock, isc_rwlocktype_read);
> +
> + result = dns_rbt_findname(zr->rbt, name, 0, NULL, &zinfo);
> + if (result == ISC_R_SUCCESS)
> + *serialp = ((zone_info_t *)zinfo)->serial;
> +
> + RWUNLOCK(&zr->rwlock, isc_rwlocktype_read);
> +
> + return result;
> +}
> +
> +/**
> + * Set last SOA serial value processed by autoincrement feature.
> + */
> +isc_result_t
> +zr_set_zone_serial(zone_register_t *zr, dns_name_t *name, isc_uint32_t serial)
> +{
> + isc_result_t result;
> + void *zinfo = NULL;
> +
> + REQUIRE(zr != NULL);
> + REQUIRE(name != NULL);
> +
> + if (!dns_name_isabsolute(name)) {
> + log_bug("trying to find zone with a relative name");
> + return ISC_R_FAILURE;
> + }
> +
> + RWLOCK(&zr->rwlock, isc_rwlocktype_read);
> +
> + result = dns_rbt_findname(zr->rbt, name, 0, NULL, &zinfo);
> + if (result == ISC_R_SUCCESS)
> + ((zone_info_t *)zinfo)->serial = serial;
> +
> + RWUNLOCK(&zr->rwlock, isc_rwlocktype_read);
> +
> + return result;
> +}
> diff --git a/src/zone_register.h b/src/zone_register.h
> index fa8ef255ef9cf212bca04aaafba0e6450d40a5c6..6ac3a92fbe755fcff2d9af3e6e26b32ae83d4133 100644
> --- a/src/zone_register.h
> +++ b/src/zone_register.h
> @@ -48,4 +48,10 @@ zr_get_rbt(zone_register_t *zr);
> isc_mem_t *
> zr_get_mctx(zone_register_t *zr);
>
> +isc_result_t
> +zr_set_zone_serial(zone_register_t *zr, dns_name_t *name, isc_uint32_t serial);
> +
> +isc_result_t
> +zr_get_zone_serial(zone_register_t *zr, dns_name_t *name, isc_uint32_t *serialp);
> +
> #endif /* !_LD_ZONE_REGISTER_H_ */
> --
> 1.7.7.6
>
> From 6918535ee7abd3121db7f31c0c7ecba6a5e847cf Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Tue, 10 Jul 2012 14:23:46 +0200
> Subject: [PATCH] Add support for replicated environments to SOA serial
> autoincrement feature. 389 DS sends entry change
> notification even if modifyTimestamp was modified because
> of replication from another DS. This code computes digest
> from resource records in zone object and compares these
> digests for each received entry change notification. False
> entry change notifications are revealed this way and no
> incrementation is done in that case.
> http://fedorahosted.org/bind-dyndb-ldap/ticket/67
>
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
> src/ldap_driver.c | 16 +------
> src/ldap_helper.c | 47 +++++++++++++++----
> src/rdlist.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> src/rdlist.h | 13 +++++
> src/zone_register.c | 35 +++++++++-----
> src/zone_register.h | 6 ++-
> 6 files changed, 204 insertions(+), 40 deletions(-)
>
> diff --git a/src/ldap_driver.c b/src/ldap_driver.c
> index a87db180b0aa72c97aa51dd178ecdab7b90a0afd..cae45d4f6cc1f201c40ca3413d1f626e03a0318e 100644
> --- a/src/ldap_driver.c
> +++ b/src/ldap_driver.c
> @@ -110,7 +110,6 @@ static void *ldapdb_version = &dummy;
>
> static void free_ldapdb(ldapdb_t *ldapdb);
> static void detachnode(dns_db_t *db, dns_dbnode_t **targetp);
> -static unsigned int rdatalist_length(const dns_rdatalist_t *rdlist);
> static isc_result_t clone_rdatalist_to_rdataset(isc_mem_t *mctx,
> dns_rdatalist_t *rdlist,
> dns_rdataset_t *rdataset);
> @@ -545,6 +544,7 @@ found:
> goto skipfind;
>
> result = ldapdb_rdatalist_findrdatatype(&rdatalist, type, &rdlist);
> +
> if (result != ISC_R_SUCCESS) {
> /* No exact rdtype match. Check CNAME */
>
> @@ -1006,20 +1006,6 @@ cleanup:
> return result;
> }
>
> -static unsigned int
> -rdatalist_length(const dns_rdatalist_t *rdlist)
> -{
> - dns_rdata_t *ptr = HEAD(rdlist->rdata);
> - unsigned int length = 0;
> -
> - while (ptr != NULL) {
> - length++;
> - ptr = NEXT(ptr, link);
> - }
> -
> - return length;
> -}
> -
> static isc_result_t
> subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
> dns_rdataset_t *rdataset, unsigned int options,
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index da6f8562ef59dadc8882743ee87bf84305a1de82..323ee81e70bd3e4e0ffd59a024f047bbcbde9d89 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -71,6 +71,7 @@
> #include "ldap_entry.h"
> #include "ldap_helper.h"
> #include "log.h"
> +#include "rdlist.h"
> #include "semaphore.h"
> #include "settings.h"
> #include "str.h"
> @@ -1002,9 +1003,16 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
> isc_boolean_t publish = ISC_FALSE;
> isc_task_t *task = inst->task;
> isc_uint32_t ldap_serial;
> - isc_uint32_t zr_serial;
> + isc_uint32_t zr_serial; /* SOA serial value from in-memory zone register */
> + unsigned char ldap_digest[RDLIST_DIGESTLENGTH];
> + unsigned char *zr_digest = NULL;
> + ldapdb_rdatalist_t rdatalist;
> +
> + REQUIRE(entry != NULL);
> + REQUIRE(inst != NULL);
>
> dns_name_init(&name, NULL);
> + INIT_LIST(rdatalist);
>
> /* Derive the dns name of the zone from the DN. */
> dn = entry->dn;
> @@ -1098,30 +1106,49 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
> * for a new zone (typically after BIND start)
> * - the zone was possibly changed in meanwhile */
> if (publish) {
> + memset(ldap_digest, 0, RDLIST_DIGESTLENGTH);
> CHECK(ldap_get_zone_serial(inst, &name, &ldap_serial));
> - CHECK(zr_set_zone_serial(inst->zone_register, &name, ldap_serial));
> + CHECK(zr_set_zone_serial_digest(inst->zone_register, &name, ldap_serial,
> + ldap_digest));
> }
>
> /* SOA serial autoincrement feature for SOA record:
> - * 1) remember old SOA serial from zone register
> - * 2) compare old and new SOA serial from LDAP DB
> - * 3) if (old == new) then do autoincrement, remember new serial otherwise */
> + * 1) Remember old (already processed) SOA serial and digest computed from
> + * zone root records in zone register.
> + * 2) After each change notification compare the old and new SOA serials
> + * and recomputed digests. If:
> + * 3a) Nothing was changed (false change notification received) - do nothing
> + * 3b) Serial was changed - remember the new serial and recompute digest,
> + * do not autoincrement (respect external change).
> + * 3c) The old and new serials are same: autoincrement only if something
> + * else was changed.
> + */
> if (inst->serial_autoincrement) {
> CHECK(ldap_get_zone_serial(inst, &name, &ldap_serial));
> - CHECK(zr_get_zone_serial(inst->zone_register, &name, &zr_serial));
> - if (ldap_serial == zr_serial)
> - CHECK(soa_serial_increment(inst->mctx, inst, &name));
> - else
> - CHECK(zr_set_zone_serial(inst->zone_register, &name, ldap_serial));
> + CHECK(ldapdb_rdatalist_get(inst->mctx, inst, &name,
> + &name, &rdatalist));
> + CHECK(rdatalist_digest(inst->mctx, &rdatalist, ldap_digest));
> +
> + CHECK(zr_get_zone_serial_digest(inst->zone_register, &name, &zr_serial,
> + &zr_digest));
> + if (ldap_serial == zr_serial) {
> + /* serials are same - increment only if something was changed */
> + if (memcmp(zr_digest, ldap_digest, RDLIST_DIGESTLENGTH) != 0)
> + CHECK(soa_serial_increment(inst->mctx, inst, &name));
> + } else { /* serial in LDAP was changed - update zone register */
> + CHECK(zr_set_zone_serial_digest(inst->zone_register, &name,
> + ldap_serial, ldap_digest));
> + }
> }
>
> cleanup:
> if (unlock)
> isc_task_endexclusive(task);
> if (dns_name_dynamic(&name))
> dns_name_free(&name, inst->mctx);
> if (zone != NULL)
> dns_zone_detach(&zone);
> + ldapdb_rdatalist_destroy(inst->mctx, &rdatalist);
>
> return result;
> }
> diff --git a/src/rdlist.c b/src/rdlist.c
> index 903c948f401a9bd82cbc0eb06ac55aa05452c976..a16de45bcc49d00a1eacf42b23c87f24be8d3b2f 100644
> --- a/src/rdlist.c
> +++ b/src/rdlist.c
> @@ -2,7 +2,7 @@
> * Authors: Adam Tkac <atkac at redhat.com>
> * Martin Nagy <mnagy at redhat.com>
> *
> - * Copyright (C) 2009 Red Hat
> + * Copyright (C) 2009-2012 Red Hat
> * see file 'COPYING' for use and warranty information
> *
> * This program is free software; you can redistribute it and/or
> @@ -22,17 +22,27 @@
> #include <isc/mem.h>
> #include <isc/result.h>
> #include <isc/util.h>
> +#include <isc/buffer.h>
> +#include <isc/md5.h>
>
> #include <dns/rdata.h>
> #include <dns/rdatalist.h>
>
> #include <string.h>
> +#include <stdlib.h>
>
> #include "ldap_helper.h" /* TODO: Move things from ldap_helper here? */
> #include "rdlist.h"
> #include "util.h"
>
>
> +/* useful only for RR sorting purposes */
> +typedef struct rr_sort rr_sort_t;
> +struct rr_sort {
> + dns_rdatalist_t *rdatalist; /* contains RR class, type, TTL */
> + isc_region_t rdatareg; /* handle to binary area with RR data */
> +};
> +
> static isc_result_t
> rdata_clone(isc_mem_t *mctx, dns_rdata_t *source, dns_rdata_t **targetp)
> {
> @@ -106,6 +116,121 @@ cleanup:
> return result;
> }
>
> +unsigned int
> +rdatalist_length(const dns_rdatalist_t *rdlist)
There is no reason to have this function exported, please mark it as static (and
probably also as inline).
> +{
> + dns_rdata_t *ptr = HEAD(rdlist->rdata);
> + unsigned int length = 0;
> +
> + while (ptr != NULL) {
> + length++;
> + ptr = NEXT(ptr, link);
> + }
> +
> + return length;
> +}
> +
> +static int
> +rr_sort_compare(const void *rdl1, const void *rdl2) {
> + const rr_sort_t *r1 = rdl1;
> + const rr_sort_t *r2 = rdl2;
> + int res;
> +
> + res = r1->rdatalist->rdclass - r2->rdatalist->rdclass;
> + if (res != 0)
> + return res;
> +
> + res = r1->rdatalist->type - r2->rdatalist->type;
> + if (res != 0)
> + return res;
> +
> + res = r1->rdatalist->ttl - r2->rdatalist->ttl;
> + if (res != 0)
> + return res;
> +
> + res = isc_region_compare((isc_region_t *)&r1->rdatareg,
> + (isc_region_t *)&r2->rdatareg);
> +
> + return res;
> +}
> +
> +/**
> + * Compute MD5 digest from all resource records in input rrdatalist.
> + * All RRs are sorted by class, type, ttl and data respectively. For this reason
> + * digest should be unambigous.
> + *
> + * @param rdlist[in] List of RRsets. Each RRset contains a list of individual RR
> + * @param digest[out] Pointer to unsigned char[RDLIST_DIGESTLENGTH] array
> + * @return ISC_R_SUCCESS and MD5 digest in unsigned char array "digest"
> + * In case of any error the array will stay untouched.
> + */
> +isc_result_t
> +rdatalist_digest(isc_mem_t *mctx, ldapdb_rdatalist_t *rdlist,
> + unsigned char *digest) {
> + isc_result_t result;
> + isc_buffer_t *rrs = NULL; /* array of all resource records from input rdlist */
> + unsigned int rrs_len = 0;
> + isc_md5_t md5ctx;
> +
> + REQUIRE(rdlist != NULL);
> + REQUIRE(digest != NULL);
> +
> + /* Compute count of RRs to avoid dynamic reallocations.
> + * The count is expected to be small number (< 20). */
> + for (dns_rdatalist_t *rrset = HEAD(*rdlist);
> + rrset != NULL;
> + rrset = NEXT(rrset, link)) {
> +
> + rrs_len += rdatalist_length(rrset);
> + }
> + CHECK(isc_buffer_allocate(mctx, &rrs, rrs_len*sizeof(rr_sort_t)));
> +
> + /* Fill each rr_sort structure in array rrs with pointer to RRset
> + * and coresponding data region from each RR. rrs array will be sorted. */
> + for (dns_rdatalist_t *rrset = HEAD(*rdlist);
> + rrset != NULL;
> + rrset = NEXT(rrset, link)) {
> +
> + for (dns_rdata_t *rr = HEAD(rrset->rdata);
> + rr != NULL;
> + rr = NEXT(rr, link)) {
> +
> + rr_sort_t rr_sort_rec;
> + rr_sort_rec.rdatalist = rrset;
> + dns_rdata_toregion(rr, &rr_sort_rec.rdatareg);
> +
> + isc_buffer_putmem(rrs, (const unsigned char *)(&rr_sort_rec),
> + sizeof(rr_sort_t));
> + }
> + }
> + qsort(isc_buffer_base(rrs), rrs_len, sizeof(rr_sort_t), rr_sort_compare);
> +
> + isc_md5_init(&md5ctx);
> + for (unsigned int i = 0; i < rrs_len; i++ ) {
> + rr_sort_t *rr_rec = (rr_sort_t *)isc_buffer_base(rrs) + i;
> + isc_md5_update(&md5ctx,
> + (const unsigned char *)&rr_rec->rdatalist->rdclass,
> + sizeof(rr_rec->rdatalist->rdclass));
> + isc_md5_update(&md5ctx,
> + (const unsigned char *)&rr_rec->rdatalist->type,
> + sizeof(rr_rec->rdatalist->type));
> + isc_md5_update(&md5ctx,
> + (const unsigned char *)&rr_rec->rdatalist->ttl,
> + sizeof(rr_rec->rdatalist->ttl));
> + isc_md5_update(&md5ctx,
> + (const unsigned char *)(rr_rec->rdatareg.base),
> + rr_rec->rdatareg.length);
> + }
> + isc_md5_final(&md5ctx, digest);
> + isc_md5_invalidate(&md5ctx);
> +
> +cleanup:
> + if (rrs != NULL)
> + isc_buffer_free(&rrs);
> +
> + return result;
> +}
> +
> isc_result_t
> ldap_rdatalist_copy(isc_mem_t *mctx, ldapdb_rdatalist_t source,
> ldapdb_rdatalist_t *target)
> diff --git a/src/rdlist.h b/src/rdlist.h
> index 04b991508e47ae320df32a60adeb5384e68951ae..465197a63933f2ec8fa1835bf0b34eeabf108d1b 100644
> --- a/src/rdlist.h
> +++ b/src/rdlist.h
> @@ -22,12 +22,25 @@
> #ifndef _LD_RDLIST_H_
> #define _LD_RDLIST_H_
>
> +#include <isc/md5.h>
> +
> +#include "types.h"
> +
> +#define RDLIST_DIGESTLENGTH ISC_MD5_DIGESTLENGTH
> +
> isc_result_t
> rdatalist_clone(isc_mem_t *mctx, dns_rdatalist_t *source,
> dns_rdatalist_t **targetp);
>
> isc_result_t
> ldap_rdatalist_copy(isc_mem_t *mctx, ldapdb_rdatalist_t source,
> ldapdb_rdatalist_t *target);
>
> +unsigned int
> +rdatalist_length(const dns_rdatalist_t *rdlist);
> +
> +isc_result_t
> +rdatalist_digest(isc_mem_t *mctx, ldapdb_rdatalist_t *rdlist,
> + unsigned char *digest);
> +
> #endif /* !_LD_RDLIST_H_ */
> diff --git a/src/zone_register.c b/src/zone_register.c
> index 1de6bf1ac121dc02c54f187cd36f8b588ee14d18..b2b938f3336b23a40d43c85062c9389a2190f3cb 100644
> --- a/src/zone_register.c
> +++ b/src/zone_register.c
> @@ -21,6 +21,7 @@
> #include <isc/mem.h>
> #include <isc/rwlock.h>
> #include <isc/util.h>
> +#include <isc/md5.h>
>
> #include <dns/rbt.h>
> #include <dns/result.h>
> @@ -31,6 +32,7 @@
> #include "log.h"
> #include "util.h"
> #include "zone_register.h"
> +#include "rdlist.h"
>
> /*
> * The zone register is a red-black tree that maps a dns name of a zone to the
> @@ -51,6 +53,7 @@ typedef struct {
> dns_zone_t *zone;
> char *dn;
> isc_uint32_t serial; /* last value processed by plugin (!= value in DB) */
> + unsigned char digest[RDLIST_DIGESTLENGTH]; /* MD5 digest from all RRs in zone record */
> } zone_info_t;
>
> /* Callback for dns_rbt_create(). */
> @@ -316,56 +319,64 @@ zr_get_zone_ptr(zone_register_t *zr, dns_name_t *name, dns_zone_t **zonep)
> }
>
> /**
> - * Return last SOA serial value processed by autoincrement feature.
> + * Return last values processed by autoincrement feature.
> */
> isc_result_t
> -zr_get_zone_serial(zone_register_t *zr, dns_name_t *name, isc_uint32_t *serialp)
> +zr_get_zone_serial_digest(zone_register_t *zr, dns_name_t *name,
> + isc_uint32_t *serialp, unsigned char ** digestp)
> {
> isc_result_t result;
> - void *zinfo = NULL;
> + zone_info_t *zinfo = NULL;
>
> REQUIRE(zr != NULL);
> REQUIRE(name != NULL);
> REQUIRE(serialp != NULL);
> + REQUIRE(digestp != NULL && *digestp == NULL);
>
> if (!dns_name_isabsolute(name)) {
> log_bug("trying to find zone with a relative name");
> return ISC_R_FAILURE;
> }
>
> RWLOCK(&zr->rwlock, isc_rwlocktype_read);
>
> - result = dns_rbt_findname(zr->rbt, name, 0, NULL, &zinfo);
> - if (result == ISC_R_SUCCESS)
> - *serialp = ((zone_info_t *)zinfo)->serial;
> + result = dns_rbt_findname(zr->rbt, name, 0, NULL, (void *)&zinfo);
> + if (result == ISC_R_SUCCESS) {
> + *serialp = zinfo->serial;
> + *digestp = zinfo->digest;
> + }
>
> RWUNLOCK(&zr->rwlock, isc_rwlocktype_read);
>
> return result;
> }
>
> /**
> - * Set last SOA serial value processed by autoincrement feature.
> + * Set last SOA serial and digest from RRs processed by autoincrement feature.
> */
> isc_result_t
> -zr_set_zone_serial(zone_register_t *zr, dns_name_t *name, isc_uint32_t serial)
> +zr_set_zone_serial_digest(zone_register_t *zr, dns_name_t *name,
> + isc_uint32_t serial, unsigned char *digest)
> {
> isc_result_t result;
> - void *zinfo = NULL;
> + zone_info_t *zinfo = NULL;
>
> REQUIRE(zr != NULL);
> REQUIRE(name != NULL);
> + REQUIRE(digest != NULL);
>
> if (!dns_name_isabsolute(name)) {
> log_bug("trying to find zone with a relative name");
> return ISC_R_FAILURE;
> }
>
> RWLOCK(&zr->rwlock, isc_rwlocktype_read);
>
> - result = dns_rbt_findname(zr->rbt, name, 0, NULL, &zinfo);
> - if (result == ISC_R_SUCCESS)
> - ((zone_info_t *)zinfo)->serial = serial;
> + result = dns_rbt_findname(zr->rbt, name, 0, NULL, (void *)&zinfo);
> + if (result == ISC_R_SUCCESS) {
> + zinfo->serial = serial;
> + memcpy(zinfo->digest, digest, RDLIST_DIGESTLENGTH);
> + }
>
> RWUNLOCK(&zr->rwlock, isc_rwlocktype_read);
>
> diff --git a/src/zone_register.h b/src/zone_register.h
> index 6ac3a92fbe755fcff2d9af3e6e26b32ae83d4133..dea2c9dce054daf1764ba31154627419acada27d 100644
> --- a/src/zone_register.h
> +++ b/src/zone_register.h
> @@ -49,9 +49,11 @@ isc_mem_t *
> zr_get_mctx(zone_register_t *zr);
>
> isc_result_t
> -zr_set_zone_serial(zone_register_t *zr, dns_name_t *name, isc_uint32_t serial);
> +zr_set_zone_serial_digest(zone_register_t *zr, dns_name_t *name,
> + isc_uint32_t serial, unsigned char *digest);
>
> isc_result_t
> -zr_get_zone_serial(zone_register_t *zr, dns_name_t *name, isc_uint32_t *serialp);
> +zr_get_zone_serial_digest(zone_register_t *zr, dns_name_t *name,
> + isc_uint32_t *serialp, unsigned char ** digestp);
>
> #endif /* !_LD_ZONE_REGISTER_H_ */
> --
> 1.7.7.6
>
--
Adam Tkac, Red Hat, Inc.
More information about the Freeipa-devel
mailing list