On 9.5.2013 14:53, Petr Spacek wrote:
On 9.5.2013 10:59, Tomas Hozza wrote:
On 04/16/2013 12:45 PM, Petr Spacek wrote:
Hello,

Explicitly return SERVFAIL if PTR synchronization is misconfigured.

SERVFAIL will be returned if PTR synchronization is enabled
in forward zone but reverse zone has dynamic updates disabled.


What the patch does little bit differs from what the commit
message says. Explanation follows:

Snip from ldap_helper.c (starting line 2959):

/* Get attribute "idnsAllowDynUpdate" for reverse zone or use default. */
dns_name_free(&zone_name, mctx);
dns_name_init(&zone_name, NULL);
CHECK(dn_to_dnsname(mctx, owner_zone_dn_ptr, &zone_name, NULL));

zone_settings = NULL;
result = zr_get_zone_settings(ldap_inst->zone_register, &zone_name,
                          &zone_settings);
if (result != ISC_R_SUCCESS) {
    if (result == ISC_R_NOTFOUND)
        log_debug(3, "active zone '%s' not found", zone_dn);
    goto cleanup;
    ^
    You replaced this goto with "CLEANUP_WITH(DNS_R_SERVFAIL)" but
    the check if dynamic updates in reverse zone are enabled
    is done in the following IF statement
}

CHECK(setting_get_bool("dyn_update", zone_settings, &zone_dyn_update));
if (!zone_dyn_update) {
    log_debug(3, "dynamic update is not allowed in zone "
             "'%s'", zone_dn);
    CLEANUP_WITH(ISC_R_NOPERM);
}


The patch modifies the plugin to explicitly return SERVFAIL if there was
some error while getting settings of PTR zone (the zone does not exist,
etc).

Maybe it would be good to explicitly return SERVFAIL also if dynamic
updates in PTR zone are disabled and modify the commit message to
better express what this patch does.

You are right. Revised patch is attached.

I sent a bad patch by mistake...

--
Petr^2 Spacek
From 04b48143f592541d3c98e06229987e36dbaf6ec8 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Tue, 16 Apr 2013 11:00:04 +0200
Subject: [PATCH] Explicitly return SERVFAIL if PTR synchronization is
 misconfigured.

SERVFAIL will be returned if PTR synchronization is enabled
in forward zone but the reverse zone is not managed by plugin
or if the reverse zone has dynamic updates disabled.

Signed-off-by: Petr Spacek <pspa...@redhat.com>
---
 src/ldap_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index d6061f247db625326ce09e75b1c7ca5c1f259ba5..af630e53dde36c3eec4d1a286cb096bcd8f3b0ca 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2990,14 +2990,14 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
 		if (result != ISC_R_SUCCESS) {
 			if (result == ISC_R_NOTFOUND)
 				log_debug(3, "active zone '%s' not found", zone_dn);
-			goto cleanup;
+			CLEANUP_WITH(DNS_R_SERVFAIL);
 		}
 
 		CHECK(setting_get_bool("dyn_update", zone_settings, &zone_dyn_update));
 		if (!zone_dyn_update) {
 			log_error("dynamic update is not allowed in zone "
 				  "'%s'", zone_dn);
-			CLEANUP_WITH(ISC_R_NOPERM);
+			CLEANUP_WITH(DNS_R_SERVFAIL);
 		}
 
 		/* 
-- 
1.7.11.7

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

Reply via email to