[Freeipa-devel] [PATCH 0063] Notify DNS slaves if zone serial number modification was detected.

[Adam Tkac]

On Mon, Sep 17, 2012 at 02:55:06PM +0200, Petr Spacek wrote:
> Hello,
> 
> this patch adds missing notification to DNS slaves if zone serial
> number modification was detected.

Hi,

please check my comment below.

> From eb8d7fc0c02e03b9c7c90e497225536c449fab1c Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Mon, 17 Sep 2012 14:29:45 +0200
> Subject: [PATCH] Notify DNS slaves if zone serial number modification was
>  detected.
> 
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
>  src/ldap_helper.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index 658b960f50b461fa602edf51e955f3bdd4769e1d..d22e8714df57edaad4cf45e6cc26ec0dbbd59108 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -1035,9 +1035,10 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
>  	isc_task_t *task = inst->task;
>  	isc_uint32_t ldap_serial;
>  	isc_uint32_t zr_serial;	/* SOA serial value from in-memory zone register */
> -	unsigned char ldap_digest[RDLIST_DIGESTLENGTH];
> +	unsigned char ldap_digest[RDLIST_DIGESTLENGTH] = {0};
>  	unsigned char *zr_digest = NULL;
>  	ldapdb_rdatalist_t rdatalist;
> +	isc_boolean_t zone_dynamic = ISC_FALSE;
>  
>  	REQUIRE(entry != NULL);
>  	REQUIRE(inst != NULL);
> @@ -1131,13 +1132,13 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
>  		&& result != DNS_R_DYNAMIC && result != DNS_R_CONTINUE)
>  		goto cleanup;
>  
> +	zone_dynamic = (result == DNS_R_DYNAMIC);
>  	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) {
> -		memset(ldap_digest, 0, RDLIST_DIGESTLENGTH);
>  		CHECK(ldap_get_zone_serial(inst, &name, &ldap_serial));
>  		CHECK(zr_set_zone_serial_digest(inst->zone_register, &name, ldap_serial,
>  				ldap_digest));
> @@ -1154,23 +1155,28 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
>  	 * 3c) The old and new serials are same: autoincrement only if something
>  	 *     else was changed.
>  	 */
> +	CHECK(ldap_get_zone_serial(inst, &name, &ldap_serial));
> +	CHECK(zr_get_zone_serial_digest(inst->zone_register, &name, &zr_serial,
> +			&zr_digest));
>  	if (inst->serial_autoincrement) {
> -		CHECK(ldap_get_zone_serial(inst, &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));
>  		}
>  	}
> +	if (ldap_serial != zr_serial) {
> +		/* serial in LDAP was changed - update zone register */
> +		CHECK(zr_set_zone_serial_digest(inst->zone_register, &name,
> +				ldap_serial, ldap_digest));
> +
> +		if (zone_dynamic)
> +			dns_zone_notify(zone);

This if() doesn't seem fully correct to me. Although in most FreeIPA scenarios
it will work, consider situation when someone disables DDNS updates for zone and
modifies records in LDAP. In this case zone_dynamic is FALSE but SOA serial
changes.

I would recommend to simpy send notify every time when serial changes.

Regards, Adam

> +	}
>  
>  cleanup:
>  	if (unlock)
> -- 
> 1.7.11.4
> 


-- 
Adam Tkac, Red Hat, Inc.