Re: [Freeipa-devel] [PATCH 0148] Explicitly return SERVFAIL if PTR synchronization is misconfigured.
On 05/09/2013 05:23 PM, Petr Spacek wrote:
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...
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
I tested the patch. Works ok, ACK.
Tomas
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0148] Explicitly return SERVFAIL if PTR synchronization is misconfigured.
On 14.5.2013 15:07, Tomas Babej wrote:
On 05/09/2013 05:23 PM, Petr Spacek wrote:
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...
Pushed to master: 04b48143f592541d3c98e06229987e36dbaf6ec8
--
Petr^2 Spacek
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0148] Explicitly return SERVFAIL if PTR synchronization is misconfigured.
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.
Regards,
Tomas Hozza
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0148] Explicitly return SERVFAIL if PTR synchronization is misconfigured.
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.
--
Petr^2 Spacek
From c7f6f7283ea289c74b69c25da1bcff8e3ad61ef2 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 16 Apr 2013 16:10:09 +0200
Subject: [PATCH] Clean up PTR record synchronization code and make it more
robust.
PTR record synchronization was split to smaller functions.
Input validation, error handling and logging was improved
significantly.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 507 --
1 file changed, 342 insertions(+), 165 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index af630e53dde36c3eec4d1a286cb096bcd8f3b0ca..52d706a8ee36aabb1b05bb83bdc42d85225d7104 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2830,35 +2830,360 @@ cleanup:
#undef SET_LDAP_MOD
}
+
+#define SYNCPTR_PREFPTR record synchronization
+#define SYNCPTR_FMTPRE SYNCPTR_PREF (%s) for A/ '%s'
+#define SYNCPTR_FMTPOST ldap_modop_str(mod_op), a_name_str
+
+static const char *
+ldap_modop_str(unsigned int mod_op) {
+ static const char add[] = addition;
+ static const char del[] = deletion;
+
+ switch (mod_op) {
+ case LDAP_MOD_ADD:
+ return add;
+
+ case LDAP_MOD_DELETE:
+ return del;
+
+ default:
+ INSIST(unsupported LDAP mod_op == NULL);
+ return NULL;
+ }
+}
+
+static void
+append_trailing_dot(char *str, unsigned int size) {
+ unsigned int length = strlen(str);
+ if (str[length] != '.') {
+ REQUIRE(length + 1 size);
+ str[length] = '.';
+ str[length+1] = '\0';
+ }
+}
+
+static isc_result_t
+ldap_find_ptr(ldap_instance_t *ldap_inst, const char *ip_str,
+ dns_name_t *ptr_name, ld_string_t *ptr_dn,
+ dns_name_t *zone_name) {
+ isc_result_t result;
+ isc_mem_t *mctx = ldap_inst-mctx;
+
+ in_addr_t ip;
+
+ /* Get string with IP address from change request
+ * and convert it to in_addr structure. */
+ if ((ip = inet_addr(ip_str)) == 0) {
+ log_bug(SYNCPTR_PREF could not convert IP address
+ from string '%s', ip_str);
+ CLEANUP_WITH(ISC_R_UNEXPECTED);
+ }
+
+ /* Use internal net address representation. */
+ isc_netaddr_t isc_ip;
+ /* Only copy data to isc_ip stucture. */
+ isc_netaddr_fromin(isc_ip,(struct in_addr *) ip);
+
+ /*
+ * Convert IP address to PTR record.
+ *
+ * @example
+ * 192.168.0.1 - 1.0.168.192.in-addr.arpa
+ *
+ * @todo Check if it works for IPv6 correctly.
+ */
+ CHECK(dns_byaddr_createptrname2(isc_ip, 0, ptr_name));
+
+ /* Get LDAP entry indentifier. */
+ CHECK(dnsname_to_dn(ldap_inst-zone_register, ptr_name, ptr_dn));
+
+ /*
+ * @example
+ * owner_dn_ptr = idnsName=100.0.168, idnsname=192.in-addr.arpa,cn=dns,$SUFFIX
+ * owner_zone_dn_ptr = idnsname=192.in-addr.arpa,cn=dns,$SUFFIX
+ */
+ char *owner_zone_dn_ptr = strstr(str_buf(ptr_dn),, ) + 1;
+
+ /* Get attribute idnsAllowDynUpdate for reverse zone or use default. */
+ CHECK(dn_to_dnsname(mctx, owner_zone_dn_ptr, zone_name, NULL));
+
+cleanup:
+ return result;
+}
+
+/**
+ * Check if PTR record's value in LDAP == name of the modified A/ record.
+ * Update will be refused if the PTR name contains multiple PTR records or
+ * if the value in LDAP != expected name.
+ *
+ * @param[in] a_name Name of modified A/ record.
+ * @param[in] a_name_str Name of modified A/
Re: [Freeipa-devel] [PATCH 0148] Explicitly return SERVFAIL if PTR synchronization is misconfigured.
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
Re: [Freeipa-devel] [PATCH 0148] Explicitly return SERVFAIL if PTR synchronization is misconfigured.
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. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0148] Explicitly return SERVFAIL if PTR synchronization is misconfigured.
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.
--
Petr Spacek
From fe5f710b212b2cbe144f8132de5df3672ad479a5 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 reverse zone has dynamic updates disabled.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index d6061f247db625326ce09e75b1c7ca5c1f259ba5..21dbed1b81af74cd3c8f28e3a78db7a878cfdeb4 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2990,7 +2990,7 @@ 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));
--
1.7.11.7
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
