Re: [Freeipa-devel] [PATCH 0123-0126] Separate master and forward zones (add idnsForwardZone object class)
.
+ *
+ * @param[in] iter valid iterator
+ * @param[out] nodename dns_name with pre-allocated storage
+ *
+ * @pre Nodename has pre-allocated storage space.
+ *
+ * @retval ISC_R_SUCCESS Nodename holds independent copy of RBT node name,
+ * RBT is in locked state.
+ * @retval ISC_R_NOMORE Iteration ended, RBT is in unlocked state,
+ * iterator is no longer valid.
+ * @retval othersErrors from dns_name_concatenate() and others.
+ */
+isc_result_t
+rbt_iter_next(rbt_iterator_t *iter, dns_name_t *nodename) {
+ isc_result_t result;
+
+ REQUIRE(iter != NULL);
+ REQUIRE(ISC_MAGIC_VALID(iter, LDAPDB_RBTITER_MAGIC));
+ REQUIRE(iter-locktype != isc_rwlocktype_none);
+
+ do {
+ result = dns_rbtnodechain_next(iter-chain, NULL, NULL);
+ if (result != ISC_R_SUCCESS result != DNS_R_NEWORIGIN)
+ goto cleanup;
+
+ result = rbt_iter_getnodename(iter, nodename);
+ } while (result == DNS_R_EMPTYNAME);
+
+cleanup:
+ if (result != ISC_R_SUCCESS)
+ rbt_iter_stop(iter);
I think rbt_iter_stop() shouldn't be called here. Imagine this piece of code:
CHECK(rbt_iter_first(..));
for (..) {
CHECK(isc_mem_alloc(..));
CHECK(rbt_iter_next(..));
}
cleanup:
if (rbt_iter_first_was_called)
// Now you should call rbt_iter_stop() only when isc_mem_alloc
fails
// but not when rbt_iter_next() fails. However how do you
figure out
// which function failed?
Due to this reason, I recommend to drop rbt_iter_stop() call from the
rbt_iter_next().
+
+ return result;
+}
+
+/**
+ * Stop RBT iteration and unlock RBT.
+ */
+void
+rbt_iter_stop(rbt_iterator_t *iter) {
+ REQUIRE(iter != NULL);
+ REQUIRE(ISC_MAGIC_VALID(iter, LDAPDB_RBTITER_MAGIC));
+
+ if (iter-locktype != isc_rwlocktype_none)
+ isc_rwlock_unlock(iter-rwlock, iter-locktype);
+
+ dns_rbtnodechain_invalidate(iter-chain);
+ isc_mem_detach((iter-mctx));
+ ZERO_PTR(iter);
+}
diff --git a/src/rbt_helper.h b/src/rbt_helper.h
new file mode 100644
index
..9c9bcd202cafafb39a3018bbafff6bbd3c9189a9
--- /dev/null
+++ b/src/rbt_helper.h
@@ -0,0 +1,29 @@
+#ifndef _LD_RBT_HELPER_H_
+#define _LD_RBT_HELPER_H_
+
+#include isc/rwlock.h
+#include dns/rbt.h
+#include util.h
+
+struct rbt_iterator {
+ unsigned intmagic;
+ isc_mem_t *mctx;
+ dns_rbt_t *rbt;
+ isc_rwlock_t*rwlock;
+ isc_rwlocktype_tlocktype;
+ dns_rbtnodechain_t chain;
+};
There is no reason to have struct rbt_iterator exported in header if I read
patchset correctly. Please move it into rbt_helper.c and leave only typedef
below here.
+
+typedef struct rbt_iterator rbt_iterator_t;
+
+isc_result_t
+rbt_iter_first(isc_mem_t *mctx, dns_rbt_t *rbt, isc_rwlock_t *rwlock,
+rbt_iterator_t *iter, dns_name_t *nodename);
+
+isc_result_t
+rbt_iter_next(rbt_iterator_t *iter, dns_name_t *nodename);
+
+void
+rbt_iter_stop(rbt_iterator_t *iter);
+
+#endif /* !_LD_RBT_HELPER_H_ */
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0123-0126] Separate master and forward zones (add idnsForwardZone object class)
On Fri, Mar 22, 2013 at 01:03:12PM +0100, Petr Spacek wrote:
Hello,
this patch set separates master zones (idnsZone objectClass) from
forward zones (idnsForwardZone objectClass). Support for forward
zones in idnsZone objectClass is still present to ease upgrades.
See each commit message for all the gory details.
--
Petr^2 Spacek
Ack for patch 0124.
From 005707761a5b99d50871de91252f9f23a7441d19 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 22 Mar 2013 12:19:02 +0100
Subject: [PATCH] Add missing includes to util.h.
Now include util.h should be enough for util.h usage.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/util.h | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/util.h b/src/util.h
index
d6d3c73e6d25657805eee904e6047c542e52a656..17a3f3b4ca65ab4a80c4e4fcc9ea909bb7a9178c
100644
--- a/src/util.h
+++ b/src/util.h
@@ -21,10 +21,17 @@
#ifndef _LD_UTIL_H_
#define _LD_UTIL_H_
-extern isc_boolean_t verbose_checks; /* from settings.c */
+#include string.h
+
+#include isc/mem.h
+#include isc/buffer.h
+#include dns/types.h
+#include dns/name.h
#include log.h
+extern isc_boolean_t verbose_checks; /* from settings.c */
+
#define CLEANUP_WITH(result_code)\
do {\
result = (result_code); \
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0123-0126] Separate master and forward zones (add idnsForwardZone object class)
, link)) {
- psearch_update(inst, entry_record, NULL);
+ psearch_update(inst, entry_record,
NULL);
+ }
}
}
@@ -3345,6 +3438,8 @@ cleanup:
ldap_query_free(ISC_FALSE, ldap_qresult_zone);
ldap_query_free(ISC_FALSE, ldap_qresult_record);
+ if (dns_name_dynamic(currname))
+ dns_name_free(currname, inst-mctx);
if (dns_name_dynamic(prevname))
dns_name_free(prevname, inst-mctx);
isc_mem_free(mctx, pevent-dbname);
@@ -3647,12 +3742,7 @@ psearch_update(ldap_instance_t *inst, ldap_entry_t
*entry, LDAPControl **ctrls)
isc_mem_t *mctx = NULL;
isc_taskaction_t action = NULL;
- class = ldap_entry_getclass(entry);
- if (class == LDAP_ENTRYCLASS_NONE) {
- log_error(psearch_update: ignoring entry with unknown class,
dn '%s',
- entry-dn);
- return; /* ignore it, it's OK */
- }
+ CHECK(ldap_entry_getclass(entry, class));
if (ctrls != NULL)
CHECK(ldap_parse_entrychangectrl(ctrls, chgtype,
prevdn_ldap));
@@ -3681,7 +3771,9 @@ psearch_update(ldap_instance_t *inst, ldap_entry_t
*entry, LDAPControl **ctrls)
if ((class LDAP_ENTRYCLASS_CONFIG) != 0)
action = update_config;
- else if ((class LDAP_ENTRYCLASS_ZONE) != 0)
+ else if ((class LDAP_ENTRYCLASS_MASTER) != 0)
+ action = update_zone;
+ else if ((class LDAP_ENTRYCLASS_FORWARD) != 0)
action = update_zone;
else if ((class LDAP_ENTRYCLASS_RR) != 0)
action = update_record;
@@ -3851,14 +3943,18 @@ restart:
ret = ldap_search_ext(conn-handle,
base,
LDAP_SCOPE_SUBTREE,
- /*
-* (objectClass==idnsZone AND
idnsZoneActive==TRUE)
-* OR (objectClass == idnsRecord)
-* OR (objectClass ==
idnsConfigObject)
-*/
-
(|((objectClass=idnsZone)(idnsZoneActive=TRUE))
- (objectClass=idnsRecord)
- (objectClass=idnsConfigObject)),
+ /*class = record
+* OR class = config
+* OR class = zone
+* OR class = forward
+*
+* Inactive zones are handled
+* in update_zone. */
+ (|
+ (objectClass=idnsRecord)
+ (objectClass=idnsConfigObject)
+ (objectClass=idnsZone)
+ (objectClass=idnsForwardZone)),
NULL, 0, conn-serverctrls, NULL, NULL,
LDAP_NO_LIMIT, conn-msgid);
if (ret != LDAP_SUCCESS) {
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0134] Make RBT iterators more resilient.
; if (zrp == NULL || *zrp == NULL) diff --git a/src/zone_register.h b/src/zone_register.h index 7ef40f7abdf4ed17cb32414907b5b7283456bb22..091b3d53aba168f54ac60b0c9e86e8fff3347950 100644 --- a/src/zone_register.h +++ b/src/zone_register.h @@ -58,7 +58,7 @@ isc_result_t zr_get_zone_settings(zone_register_t *zr, dns_name_t *name, settings_set_t **set); isc_result_t -zr_rbt_iter_init(zone_register_t *zr, rbt_iterator_t *iter, +zr_rbt_iter_init(zone_register_t *zr, rbt_iterator_t **iter, dns_name_t *nodename); dns_rbt_t * -- 1.7.11.7 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0131] Log plugin version during startup
On Tue, Mar 26, 2013 at 03:45:02PM +0100, Petr Spacek wrote:
Hello,
Log plugin version during startup.
Good idea, ack.
From ab91b26ee8ede6b2909f8dd8b305ed960752c55e Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 26 Mar 2013 15:43:52 +0100
Subject: [PATCH] Log plugin version during startup.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/zone_manager.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/zone_manager.c b/src/zone_manager.c
index
c19c3b6c91ff8114fcb15eacba0f74ec46047986..c0006237ffbec8bf08caaa2e729c2808be65cf75
100644
--- a/src/zone_manager.c
+++ b/src/zone_manager.c
@@ -32,6 +32,7 @@
#include string.h
+#include config.h
#include ldap_convert.h
#include ldap_helper.h
#include log.h
@@ -61,6 +62,9 @@ initialize_manager(void)
{
INIT_LIST(instance_list);
isc_mutex_init(instance_list_lock);
+ log_info(bind-dyndb-ldap version VERSION
+ compiled at __TIME__ __DATE__
+ , compiler __VERSION__);
}
void
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0132] Update NEWS file for upcoming 2.6 release
On Tue, Mar 26, 2013 at 03:46:52PM +0100, Petr Spacek wrote: Hello, Update NEWS file for upcoming 2.6 release. Ack From cd2a02874950ea9e045763c4a316069a00894d5b Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Mon, 25 Mar 2013 17:22:36 +0100 Subject: [PATCH] Update NEWS file for upcoming 2.6 release. Signed-off-by: Petr Spacek pspa...@redhat.com --- NEWS | 24 +++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 6dd09c118c86c4f5daf44bfbc5978600092b3d7f..78e16942ed551d0ec1a71f4865d02134c46ada87 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,28 @@ -[1] Automatically reload invalid zone after each change in zone data. +2.6 += +[1] Invalid zones are automatically reloaded after each change in zone data. https://fedorahosted.org/bind-dyndb-ldap/ticket/102 +[2] Plugin periodically reconnects when KDC is unavailable. +https://fedorahosted.org/bind-dyndb-ldap/ticket/100 + +[3] Invalid wildcard name in update-policy no longer crashes BIND. +https://fedorahosted.org/bind-dyndb-ldap/ticket/108 + +[4] Crash caused by idnsAllowSyncPTR attribute in global configuration object +in LDAP was fixed. +https://fedorahosted.org/bind-dyndb-ldap/ticket/110 + +[5] Crash caused by invalid query/transfer policy was fixed. +https://fedorahosted.org/bind-dyndb-ldap/ticket/109 + +[6] Crash caused by 'zonesub' match-type in update ACL was fixed. +https://fedorahosted.org/bind-dyndb-ldap/ticket/111 + +[7] Support for update-policy match type 'external' was added. + +[8] Various improvements related to logging. + 2.5 = [1] Fix crash during per-zone cache flush. -- 1.7.11.7 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0078] Use automatic connection management in LDAP modification code to prevent potential deadlock
On Tue, Mar 05, 2013 at 05:24:26PM +0100, Petr Spacek wrote:
On 15.10.2012 13:10, Petr Spacek wrote:
On 10/09/2012 03:49 PM, Petr Spacek wrote:
On 10/09/2012 01:21 PM, Adam Tkac wrote:
On Mon, Oct 08, 2012 at 04:46:54PM +0200, Petr Spacek wrote:
Hello,
Use automatic connection management in LDAP modification code to
prevent potential deadlock.
Without this patch the plugin will deadlock when modify_ldap_common()
is called with PTR synchronization enabled and only single
connection is available in the connection pool.
Nack
If I read the patch correctly, it leaves unused ldap_conn parameters in
ldap_modify_do() and modify_soa_record() functions.
Those params are always NULL so they can be safely removed. Please also
remove
the autoconn variable from ldap_modify_do()
My intent was to keep the same connection management abilities as are in
ldap_query(): You can avoid repetitive ldap_pool_get/putconnection() calls
by
passing connection via parameter.
I can remove it if it isn't worth. (Actually *_modify_*() functions do not
use
this capability now.)
I forgot to send the patch after our discussion on IRC. Attached patch
removes
unused parameters.
Patch rebased on top of master branch. No functional changes.
Ack.
From 9af7dae883b6f014fdcb5aee8394ad5d8f87aab9 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 5 Mar 2013 17:10:06 +0100
Subject: [PATCH] Use automatic connection management in LDAP modification
code to prevent potential deadlock.
Without this patch the plugin could deadlock when modify_ldap_common()
is called with PTR synchronization enabled and only single
connection is available in the connection pool.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 54 +++---
1 file changed, 23 insertions(+), 31 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
c5c8245ecd664f038781fc98f4b5756ceff87c2e..3ed4a44e043ce98a5503ad351f4e8a91116d5ac3
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -312,8 +312,7 @@ static void ldap_query_free(isc_boolean_t prepare_reuse,
ldap_qresult_t **ldap_q
/* Functions for writing to LDAP. */
static isc_result_t ldap_modify_do(ldap_instance_t *ldap_inst,
- ldap_connection_t *ldap_conn, const char *dn, LDAPMod **mods,
- isc_boolean_t delete_node);
+ const char *dn, LDAPMod **mods, isc_boolean_t delete_node);
static isc_result_t ldap_rdttl_to_ldapmod(isc_mem_t *mctx,
dns_rdatalist_t *rdlist, LDAPMod **changep);
static isc_result_t ldap_rdatalist_to_ldapmod(isc_mem_t *mctx,
@@ -2472,31 +2471,19 @@ reconnect:
}
static isc_result_t
-ldap_modify_do(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn,
- const char *dn, LDAPMod **mods, isc_boolean_t delete_node)
+ldap_modify_do(ldap_instance_t *ldap_inst, const char *dn, LDAPMod **mods,
+ isc_boolean_t delete_node)
{
int ret;
int err_code;
const char *operation_str;
isc_result_t result;
- isc_boolean_t autoconn = (ldap_conn == NULL);
+ ldap_connection_t *ldap_conn = NULL;
REQUIRE(dn != NULL);
REQUIRE(mods != NULL);
REQUIRE(ldap_inst != NULL);
- if (autoconn)
- CHECK(ldap_pool_getconnection(ldap_inst-pool, ldap_conn));
-
- if (ldap_conn-handle == NULL) {
- /*
- * handle can be NULL when the first connection to LDAP wasn't
- * successful
- * TODO: handle this case inside ldap_pool_getconnection()?
- */
- CHECK(ldap_connect(ldap_inst, ldap_conn, ISC_FALSE));
- }
-
/* Any mod_op can be ORed with LDAP_MOD_BVALUES. */
if ((mods[0]-mod_op ~LDAP_MOD_BVALUES) == LDAP_MOD_ADD)
operation_str = modifying(add);
@@ -2507,7 +2494,17 @@ ldap_modify_do(ldap_instance_t *ldap_inst,
ldap_connection_t *ldap_conn,
else {
operation_str = modifying(unknown operation);
log_bug(%s: 0x%x, operation_str, mods[0]-mod_op);
- CHECK(ISC_R_NOTIMPLEMENTED);
+ CLEANUP_WITH(ISC_R_NOTIMPLEMENTED);
+ }
+
+ CHECK(ldap_pool_getconnection(ldap_inst-pool, ldap_conn));
+ if (ldap_conn-handle == NULL) {
+ /*
+ * handle can be NULL when the first connection to LDAP wasn't
+ * successful
+ * TODO: handle this case inside ldap_pool_getconnection()?
+ */
+ CHECK(ldap_connect(ldap_inst, ldap_conn, ISC_FALSE));
}
if (delete_node) {
@@ -2569,8 +2566,7 @@ ldap_modify_do(ldap_instance_t *ldap_inst,
ldap_connection_t *ldap_conn,
result = ISC_R_FAILURE;
}
cleanup:
- if (autoconn)
- ldap_pool_putconnection(ldap_inst-pool, ldap_conn
Re: [Freeipa-devel] [PATCH 0121] Fix crash during invalid zone reload process
On Thu, Mar 21, 2013 at 02:19:10PM +0100, Petr Spacek wrote:
Hello,
Fix crash during invalid zone reload process.
This bug was created during settings refactoring and is present only
in master, not in v2 branch.
Ack
From 79594a484f30c6677dd901e7f8285719e31bab6b Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 21 Mar 2013 14:13:57 +0100
Subject: [PATCH] Fix crash during invalid zone reload process.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
6f21b8407e8c01a98ae5b6f916c964432c651fd5..7ac5ceda26cd9d734f94d9195388db879be1959e
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -3503,7 +3503,7 @@ update_record(isc_task_t *task, isc_event_t *event)
ldapdb_rdatalist_t rdatalist;
/* Convert domain name from text to struct dns_name_t. */
- settings_set_t *zone_settings = NULL;
+ settings_set_t *zone_settings;
dns_name_t name;
dns_name_t origin;
dns_name_t prevname;
@@ -3569,6 +3569,7 @@ update_restart:
/* Do not bump serial during initial database dump. */
if (PSEARCH_ANY(pevent-chgtype)) {
+ zone_settings = NULL;
CHECK(zr_get_zone_settings(inst-zone_register, origin,
zone_settings));
CHECK(setting_get_bool(serial_autoincrement, zone_settings,
serial_autoincrement));
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0120] Fix automatic reloading of invalid zone after each change in zone data
On Thu, Mar 21, 2013 at 01:38:42PM +0100, Petr Spacek wrote:
Hello,
Fix automatic reloading of invalid zone after each change in zone data.
Reload wasn't done when serial_autoincrement feature was disabled.
https://fedorahosted.org/bind-dyndb-ldap/ticket/102
Ack.
But before the push, please add explicit comment to ldap_get_zone_serial()
call that the only reason of this call is to return ISC_R_SUCCESS in case the
zone is loaded or DNS_R_NOTLOADED in case it isn't. I studied the patch for more
then 15 minutes before I figured this.
Thanks, Adam
From 1700a5d7dbf6c36ce235091a449e13a5e18fbb8b Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 21 Mar 2013 13:35:17 +0100
Subject: [PATCH] Fix automatic reloading of invalid zone after each change in
zone data.
Reload wasn't done when serial_autoincrement feature was disabled.
https://fedorahosted.org/bind-dyndb-ldap/ticket/102
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
c10a23929c1536961a37d18e68d0669aa26539de..6f21b8407e8c01a98ae5b6f916c964432c651fd5
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -3572,8 +3572,15 @@ update_restart:
CHECK(zr_get_zone_settings(inst-zone_register, origin,
zone_settings));
CHECK(setting_get_bool(serial_autoincrement, zone_settings,
serial_autoincrement));
+
+ /* Serial autoincrement does zone state check implicitly.
+ * Do explicit state check if serial autoincrement is disabled.
*/
if (serial_autoincrement)
CHECK(soa_serial_increment(mctx, inst, origin));
+ else {
+ isc_uint32_t dummy;
+ CHECK(ldap_get_zone_serial(inst, origin, dummy));
+ }
}
cleanup:
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0122] Log successful zone reload after record change.
On Thu, Mar 21, 2013 at 02:45:11PM +0100, Petr Spacek wrote:
Hello,
Log successful zone reload after record change.
This should be last piece of
https://fedorahosted.org/bind-dyndb-ldap/ticket/102
Ack
From 06c414c2922bb09c18afd9fadc52b2b0f4529f90 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 21 Mar 2013 14:43:56 +0100
Subject: [PATCH] Log successful zone reload after record change.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
7ac5ceda26cd9d734f94d9195388db879be1959e..72105e6093cea7b0bc9fdfc96229afded7650dce
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -3494,6 +3494,7 @@ update_record(isc_task_t *task, isc_event_t *event)
dns_zone_t *zone_ptr = NULL;
isc_boolean_t zone_found = ISC_FALSE;
isc_boolean_t zone_reloaded = ISC_FALSE;
+ isc_uint32_t serial;
mctx = pevent-mctx;
UNUSED(task);
@@ -3579,8 +3580,7 @@ update_restart:
if (serial_autoincrement)
CHECK(soa_serial_increment(mctx, inst, origin));
else {
- isc_uint32_t dummy;
- CHECK(ldap_get_zone_serial(inst, origin, dummy));
+ CHECK(ldap_get_zone_serial(inst, origin, serial));
}
}
@@ -3594,16 +3594,23 @@ cleanup:
result = zr_get_zone_ptr(inst-zone_register, origin,
zone_ptr);
if (result == ISC_R_SUCCESS)
result = dns_zone_load(zone_ptr);
- if (zone_ptr != NULL)
- dns_zone_detach(zone_ptr);
if (result == ISC_R_SUCCESS || result == DNS_R_UPTODATE ||
result == DNS_R_DYNAMIC || result == DNS_R_CONTINUE) {
/* zone reload succeeded, fire current event again */
log_debug(1, restarting update_record after zone
reload
caused by change in '%s', pevent-dn);
zone_reloaded = ISC_TRUE;
- goto update_restart;
+ result = dns_zone_getserial2(zone_ptr, serial);
+ if (result == ISC_R_SUCCESS) {
+ dns_zone_log(zone_ptr, ISC_LOG_INFO,
+ reloaded serial %u, serial);
+ goto update_restart;
+ } else {
+ dns_zone_log(zone_ptr, ISC_LOG_ERROR,
+ could not get serial after
+ reload);
+ }
} else {
log_error_r(unable to reload invalid zone;
reload triggered by change in '%s',
@@ -3617,6 +3624,8 @@ cleanup:
pevent-dn, pevent-chgtype);
}
+ if (zone_ptr != NULL)
+ dns_zone_detach(zone_ptr);
if (dns_name_dynamic(name))
dns_name_free(name, inst-mctx);
if (dns_name_dynamic(prevname))
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0127] Remove orphaned function declaration from ldap_helper.h
On Fri, Mar 22, 2013 at 01:06:12PM +0100, Petr Spacek wrote: Hello, Remove orphaned function declaration from ldap_helper.h. Ack From 9129f3963c8a7d603c02c5a8ea1ce3f08182541f Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Fri, 22 Mar 2013 13:04:29 +0100 Subject: [PATCH] Remove orphaned function declaration from ldap_helper.h. Signed-off-by: Petr Spacek pspa...@redhat.com --- src/ldap_helper.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ldap_helper.h b/src/ldap_helper.h index fe54687fa1e8ec16d83105e08e1a17cdec68614e..2eb7c7600f45542d92f01dbc878f4862606ade8a 100644 --- a/src/ldap_helper.h +++ b/src/ldap_helper.h @@ -94,9 +94,6 @@ isc_result_t write_to_ldap(dns_name_t *owner, ldap_instance_t *ldap_inst, isc_result_t remove_from_ldap(dns_name_t *owner, ldap_instance_t *ldap_inst, dns_rdatalist_t *rdlist, isc_boolean_t delete_node); -/* Get cache associated with ldap_inst */ -ldap_cache_t *ldap_instance_getcache(ldap_instance_t *ldap_inst); - settings_set_t * ldap_instance_getsettings_local(ldap_instance_t *ldap_inst); #endif /* !_LD_LDAP_HELPER_H_ */ -- 1.7.11.7 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0117] Fix crash caused by invalid query/transfer policy
On Mon, Mar 04, 2013 at 02:22:34PM +0100, Petr Spacek wrote:
Hello,
Fix crash caused by invalid query/transfer policy.
Please double-check correctness. The ISC parser is really complex beast!
Thank you.
Ack
From 41061726684211924e453f74d1db3bec6c2e32d6 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Mon, 4 Mar 2013 14:20:56 +0100
Subject: [PATCH] Fix crash caused by invalid query/transfer policy.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/acl.c | 45 +++--
1 file changed, 35 insertions(+), 10 deletions(-)
diff --git a/src/acl.c b/src/acl.c
index
f95cf431b6363d82085e9cfec7e6c1d6ddd45d7a..076a50375ae1fd132c143aa96379f7c80cc78cb8
100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -71,6 +71,19 @@ static cfg_type_t *allow_query;
static cfg_type_t *allow_transfer;
static cfg_type_t *forwarders;
+/* Following definitions are necessary for context (map configuration
object)
+ * required during ACL parsing. */
+static cfg_clausedef_t * empty_map_clausesets[] = {
+ NULL
+};
+
+static cfg_type_t cfg_type_empty_map = {
+ empty_map, cfg_parse_map, cfg_print_map, cfg_doc_map, cfg_rep_map,
+ empty_map_clausesets
+};
+
+static cfg_type_t *empty_map_p = cfg_type_empty_map;
+
static cfg_type_t *
get_type_from_tuplefield(const cfg_type_t *cfg_type, const char *name)
{
@@ -469,44 +482,56 @@ acl_from_ldap(isc_mem_t *mctx, const char *aclstr,
acl_type_t type,
cfg_parser_t *parser = NULL;
cfg_obj_t *aclobj = NULL;
cfg_aclconfctx_t *aclctx = NULL;
+ /* ACL parser requires configuration context. The parser looks for
+ * undefined names in this context. We create empty context (map
type),
+ * i.e. only built-in named lists any, none etc. are supported. */
+ cfg_obj_t *cctx = NULL;
+ cfg_parser_t *parser_empty = NULL;
REQUIRE(aclp != NULL *aclp == NULL);
CHECK(bracket_str(mctx, aclstr, new_aclstr));
CHECK(cfg_parser_create(mctx, dns_lctx, parser));
+ CHECK(cfg_parser_create(mctx, dns_lctx, parser_empty));
+ CHECK(parse(parser_empty, {}, empty_map_p, cctx));
+
switch (type) {
case acl_type_query:
- result = parse(parser, str_buf(new_aclstr), allow_query,
-aclobj);
+ CHECK(parse(parser, str_buf(new_aclstr), allow_query,
+ aclobj));
break;
case acl_type_transfer:
- result = parse(parser, str_buf(new_aclstr), allow_transfer,
-aclobj);
+ CHECK(parse(parser, str_buf(new_aclstr), allow_transfer,
+ aclobj));
break;
default:
/* This is a bug */
REQUIRE(Unhandled ACL type in acl_from_ldap == NULL);
}
- if (result != ISC_R_SUCCESS) {
- log_error(Failed to parse ACL (%s), aclstr);
- goto cleanup;
- }
-
CHECK(cfg_aclconfctx_create(mctx, aclctx));
- CHECK(cfg_acl_fromconfig(aclobj, NULL, dns_lctx, aclctx, mctx, 0,
acl));
+ CHECK(cfg_acl_fromconfig(aclobj, cctx, dns_lctx, aclctx, mctx, 0,
acl));
*aclp = acl;
result = ISC_R_SUCCESS;
cleanup:
+ if (result != ISC_R_SUCCESS)
+ log_error_r(%s ACL parsing failed: '%s',
+ type == acl_type_query ? query : transfer,
+ aclstr);
+
if (aclctx != NULL)
cfg_aclconfctx_detach(aclctx);
if (aclobj != NULL)
cfg_obj_destroy(parser, aclobj);
if (parser != NULL)
cfg_parser_destroy(parser);
+ if (cctx != NULL)
+ cfg_obj_destroy(parser_empty, cctx);
+ if (parser_empty != NULL)
+ cfg_parser_destroy(parser_empty);
str_destroy(new_aclstr);
return result;
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0128] Fix crash caused by 'zonesub' match-type in update ACL
On Fri, Mar 22, 2013 at 02:51:03PM +0100, Petr Spacek wrote:
On 22.3.2013 14:26, Petr Spacek wrote:
Hello,
Fix crash caused by 'zonesub' match-type in update ACL.
Next patchset will improve overall error handling in ACL processing.
I forgot to check return value from dns_name_copy(). Fixed patch is attached.
Ack
From a76a7a2899e1e8b4335c012271f607e438ef0218 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 22 Mar 2013 13:54:39 +0100
Subject: [PATCH] Fix crash caused by 'zonesub' match-type in update ACL.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/acl.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/src/acl.c b/src/acl.c
index
f95cf431b6363d82085e9cfec7e6c1d6ddd45d7a..ed3bdebcc027f3f5b7b2e9e084cf328ed4f6b1dd
100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -208,6 +208,7 @@ get_match_type(const cfg_obj_t *obj)
MATCH(name, DNS_SSUMATCHTYPE_NAME);
MATCH(subdomain, DNS_SSUMATCHTYPE_SUBDOMAIN);
+ MATCH(zonesub, DNS_SSUMATCHTYPE_SUBDOMAIN);
MATCH(wildcard, DNS_SSUMATCHTYPE_WILDCARD);
MATCH(self, DNS_SSUMATCHTYPE_SELF);
#if defined(DNS_SSUMATCHTYPE_SELFSUB) defined(DNS_SSUMATCHTYPE_SELFWILD)
@@ -246,8 +247,16 @@ get_fixed_name(const cfg_obj_t *obj, const char *name,
dns_fixedname_t *fname)
REQUIRE(fname != NULL);
+ if (!cfg_obj_istuple(obj)) {
+ log_bug(configuration object is not a tuple);
+ return ISC_R_UNEXPECTED;
+ }
obj = cfg_tuple_get(obj, name);
+
+ if (!cfg_obj_isstring(obj))
+ return ISC_R_NOTFOUND;
str = cfg_obj_asstring(obj);
+
len = strlen(str);
isc_buffer_init(buf, str, len);
@@ -417,7 +426,19 @@ acl_configure_zone_ssutable(const char *policy_str,
dns_zone_t *zone)
match_type = get_match_type(stmt);
CHECK(get_fixed_name(stmt, identity, fident));
- CHECK(get_fixed_name(stmt, name, fname));
+
+ /* Use zone name for 'zonesub' match type */
+ result = get_fixed_name(stmt, name, fname);
+ if (result == ISC_R_NOTFOUND
+ match_type == DNS_SSUMATCHTYPE_SUBDOMAIN) {
+ dns_fixedname_init(fname);
+ CHECK(dns_name_copy(dns_zone_getorigin(zone),
+ dns_fixedname_name(fname),
+ fname.buffer));
+ }
+ else if (result != ISC_R_SUCCESS)
+ goto cleanup;
+
CHECK(get_types(mctx, stmt, types, n));
if (match_type == DNS_SSUMATCHTYPE_WILDCARD
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0130] Add support for 'external' match-type in update-policy
On Mon, Mar 25, 2013 at 12:13:26PM +0100, Petr Spacek wrote: Hello, Add support for 'external' match-type in update-policy. Ack From 00ce71c81d2f486ca193acfd3174dc04e612c901 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Mon, 25 Mar 2013 12:12:52 +0100 Subject: [PATCH] Add support for 'external' match-type in update-policy. Signed-off-by: Petr Spacek pspa...@redhat.com --- src/acl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/acl.c b/src/acl.c index 3b5de00f8a40cbc1a876ea2b74e9c2093e48774c..4614d39a38655377a90a588357e82539ffc00330 100644 --- a/src/acl.c +++ b/src/acl.c @@ -247,6 +247,9 @@ get_match_type(const cfg_obj_t *obj, unsigned int *value) MATCH(tcp-self, DNS_SSUMATCHTYPE_TCPSELF); MATCH(6to4-self, DNS_SSUMATCHTYPE_6TO4SELF); #endif +#if defined(DNS_SSUMATCHTYPE_EXTERNAL) + MATCH(external, DNS_SSUMATCHTYPE_EXTERNAL); +#endif log_bug(unsupported match type '%s', str); return ISC_R_NOTIMPLEMENTED; -- 1.7.11.7 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0129] Harden update-policy processing
On Mon, Mar 25, 2013 at 10:56:05AM +0100, Petr Spacek wrote:
Hello,
Harden update-policy processing.
https://fedorahosted.org/bind-dyndb-ldap/ticket/111
This patch should prevent crashes similar to 'zonesub' problem
described in the ticket #111.
Ack
From 05d73392dc6c0f9f6f7a9e570e4382ccb3c66022 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Mon, 25 Mar 2013 10:52:50 +0100
Subject: [PATCH] Harden update-policy processing.
https://fedorahosted.org/bind-dyndb-ldap/ticket/111
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/acl.c | 41 -
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/src/acl.c b/src/acl.c
index
ed3bdebcc027f3f5b7b2e9e084cf328ed4f6b1dd..3b5de00f8a40cbc1a876ea2b74e9c2093e48774c
100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -178,32 +178,48 @@ parse(cfg_parser_t *parser, const char *string,
cfg_type_t **type,
#define MATCH(string_rep, return_val)
\
do {\
if (!strcasecmp(str, string_rep)) { \
- return return_val; \
+ *value = return_val;\
+ return ISC_R_SUCCESS; \
} \
} while (0)
-static isc_boolean_t
-get_mode(const cfg_obj_t *obj)
+static isc_result_t
+get_mode(const cfg_obj_t *obj, isc_boolean_t *value)
{
const char *str;
+ if (!cfg_obj_istuple(obj)) {
+ log_bug(tuple is expected);
+ return ISC_R_UNEXPECTED;
+ }
obj = cfg_tuple_get(obj, mode);
+ if (!cfg_obj_isstring(obj)) {
+ log_bug(mode is not defined);
+ return ISC_R_UNEXPECTED;
+ }
str = cfg_obj_asstring(obj);
MATCH(grant, ISC_TRUE);
MATCH(deny, ISC_FALSE);
- INSIST(0);
- /* Not reached. */
- return ISC_FALSE;
+ log_bug(unsupported ACL mode '%s', str);
+ return ISC_R_NOTIMPLEMENTED;
}
-static unsigned int
-get_match_type(const cfg_obj_t *obj)
+static isc_result_t
+get_match_type(const cfg_obj_t *obj, unsigned int *value)
{
const char *str;
+ if (!cfg_obj_istuple(obj)) {
+ log_bug(tuple is expected);
+ return ISC_R_UNEXPECTED;
+ }
obj = cfg_tuple_get(obj, matchtype);
+ if (!cfg_obj_isstring(obj)) {
+ log_bug(matchtype is not defined);
+ return ISC_R_UNEXPECTED;
+ }
str = cfg_obj_asstring(obj);
MATCH(name, DNS_SSUMATCHTYPE_NAME);
@@ -232,9 +248,8 @@ get_match_type(const cfg_obj_t *obj)
MATCH(6to4-self, DNS_SSUMATCHTYPE_6TO4SELF);
#endif
- INSIST(0);
- /* Not reached. */
- return DNS_SSUMATCHTYPE_NAME;
+ log_bug(unsupported match type '%s', str);
+ return ISC_R_NOTIMPLEMENTED;
}
static isc_result_t
@@ -422,8 +437,8 @@ acl_configure_zone_ssutable(const char *policy_str,
dns_zone_t *zone)
types = NULL;
stmt = cfg_listelt_value(el);
- grant = get_mode(stmt);
- match_type = get_match_type(stmt);
+ CHECK(get_mode(stmt, grant));
+ CHECK(get_match_type(stmt, match_type));
CHECK(get_fixed_name(stmt, identity, fident));
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0119] Prevent crash caused by race condition during plugin initialization
On Wed, Mar 20, 2013 at 01:10:58PM +0100, Petr Spacek wrote:
Hello,
Prevent crash caused by race condition during plugin initialization.
Processing of global configuration was postponed. Now the persistent
search watcher thread doesn't change configuration directly.
The problem was that isc_task_beginexclusive()
was called before task associated with psearch watcher thread
was fully initialized.
Now the watcher enqueues a new configuration change event
and this event is processed later by another thread.
https://fedorahosted.org/bind-dyndb-ldap/ticket/110
Ack
From edd7efd2dbea3f40ec4c2c849a0646d47abf201b Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 20 Mar 2013 12:58:09 +0100
Subject: [PATCH] Prevent crash caused by race condition during plugin
initialization.
Processing of global configuration was postponed. Now the persistent
search watcher thread doesn't change configuration directly.
The problem was that isc_task_beginexclusive()
was called before task associated with psearch watcher thread
was fully initialized.
Now the watcher enqueues a new configuration change event
and this event is processed later by another thread.
https://fedorahosted.org/bind-dyndb-ldap/ticket/110
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
0c9e87911ef196cf89bb1f2022235b7b19609b14..b4162bbabd294224513e4957e5eee01100dae70c
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1458,18 +1458,26 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst,
isc_boolean_t delete_only)
* before processing them. It prevents deadlock in situations where
* ldap_parse_zoneentry() requests another connection. */
CHECK(ldap_pool_getconnection(ldap_inst-pool, ldap_conn));
- CHECK(ldap_query(ldap_inst, ldap_conn, ldap_config_qresult,
str_buf(ldap_inst-base),
- LDAP_SCOPE_SUBTREE, config_attrs, 0,
- (objectClass=idnsConfigObject)));
CHECK(ldap_query(ldap_inst, ldap_conn, ldap_zones_qresult,
str_buf(ldap_inst-base),
LDAP_SCOPE_SUBTREE, zone_attrs, 0,
((objectClass=idnsZone)(idnsZoneActive=TRUE;
+
+ /* Do not touch configuration from psearch watcher thread, otherwise
+ * BIND will crash. The problem is that isc_task_beginexclusive()
+ * is called before task associated with psearch watcher thread
+ * is fully initialized. */
+ if (!delete_only)
+ CHECK(ldap_query(ldap_inst, ldap_conn, ldap_config_qresult,
+ str_buf(ldap_inst-base), LDAP_SCOPE_SUBTREE,
+ config_attrs, 0,
(objectClass=idnsConfigObject)));
+
ldap_pool_putconnection(ldap_inst-pool, ldap_conn);
- for (entry = HEAD(ldap_config_qresult-ldap_entries);
- entry != NULL;
- entry = NEXT(entry, link)) {
- CHECK(ldap_parse_configentry(entry, ldap_inst));
+ if (!delete_only) {
+ for (entry = HEAD(ldap_config_qresult-ldap_entries);
+ entry != NULL;
+ entry = NEXT(entry, link))
+ CHECK(ldap_parse_configentry(entry, ldap_inst));
}
/*
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0114] Log name of the zone if zone cannot be created
On Wed, Feb 20, 2013 at 04:57:13PM +0100, Petr Spacek wrote:
Hello,
Log name of the zone if zone cannot be created.
Ack
From 9f97b85ada8d67f5422dedfb936bc75cb02dfa2c Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 20 Feb 2013 16:55:30 +0100
Subject: [PATCH] Log name of the zone if zone cannot be created.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
71b96ce5eef2a249f7d16c448c70e2c2068d271d..19d9399ada3358c94b7d68f36b8e5147c4ae432d
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -750,12 +750,17 @@ create_zone(ldap_instance_t *ldap_inst, dns_name_t
*name, dns_zone_t **zonep)
argv[1] = ldap_inst-db_name;
result = dns_view_findzone(ldap_inst-view, name, zone);
- if (result == ISC_R_SUCCESS) {
- result = ISC_R_EXISTS;
- log_error_r(failed to create new zone);
- goto cleanup;
- } else if (result != ISC_R_NOTFOUND) {
- log_error_r(dns_view_findzone() failed);
+ if (result != ISC_R_NOTFOUND) {
+ char zone_name[DNS_NAME_FORMATSIZE];
+ dns_name_format(name, zone_name, DNS_NAME_FORMATSIZE);
+
+ if (result == ISC_R_SUCCESS) {
+ result = ISC_R_EXISTS;
+ log_error_r(failed to create new zone '%s',
zone_name);
+ } else {
+ log_error_r(dns_view_findzone() failed while
+ searching for zone '%s', zone_name);
+ }
goto cleanup;
}
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0115] Add support for DNAME substitution
On Thu, Feb 21, 2013 at 04:27:03PM +0100, Petr Spacek wrote:
On 21.2.2013 16:21, Petr Spacek wrote:
Hello,
Add support for DNAME substitution.
https://fedorahosted.org/bind-dyndb-ldap/ticket/63
And now the patch :-)
Ack
From dc1215e8a82d3993f69436b4de9ff91ea16f4369 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 21 Feb 2013 13:34:52 +0100
Subject: [PATCH] Add support for DNAME substitution.
https://fedorahosted.org/bind-dyndb-ldap/ticket/63
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_driver.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index
cde09ee8aa3c9332f3766a031030a95b0cff3229..9cae66b3950323221d3319649fc7b86ef25a5d68
100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -457,7 +457,6 @@ cleanup:
return result;
}
-/* XXX add support for DNAME redirection */
static isc_result_t
find(dns_db_t *db, dns_name_t *name, dns_dbversion_t *version,
dns_rdatatype_t type, unsigned int options, isc_stdtime_t now,
@@ -469,6 +468,7 @@ find(dns_db_t *db, dns_name_t *name, dns_dbversion_t
*version,
ldapdb_node_t *node = NULL;
dns_rdatalist_t *rdlist = NULL;
isc_boolean_t is_cname = ISC_FALSE;
+ isc_boolean_t is_dname = ISC_FALSE;
isc_boolean_t is_delegation = ISC_FALSE;
ldapdb_rdatalist_t rdatalist;
unsigned int labels, qlabels;
@@ -515,7 +515,20 @@ find(dns_db_t *db, dns_name_t *name, dns_dbversion_t
*version,
continue;
}
- /* TODO: We should check for DNAME records right here */
+ /* RFC 6672 section 2.3.:
+Unlike a CNAME RR, a DNAME RR redirects DNS names
+subordinate to its owner name; the owner name of a DNAME
+is not redirected itself. */
+ if (qlabels dns_name_countlabels(traversename)) {
+ rdlist = NULL;
+ result = ldapdb_rdatalist_findrdatatype(rdatalist,
+
dns_rdatatype_dname,
+ rdlist);
+ if (result == ISC_R_SUCCESS) {
+ is_dname = ISC_TRUE;
+ goto skipfind;
+ }
+ }
/*
* Check if there is at least one NS RR. If yes and this is not
NS
@@ -527,6 +540,7 @@ find(dns_db_t *db, dns_name_t *name, dns_dbversion_t
*version,
if (dns_name_countlabels(db-origin)
dns_name_countlabels(traversename)
(options DNS_DBFIND_GLUEOK) == 0) {
+ rdlist = NULL;
result = ldapdb_rdatalist_findrdatatype(rdatalist,
dns_rdatatype_ns,
rdlist);
@@ -582,7 +596,7 @@ found:
skipfind:
CHECK(dns_name_copy(traversename, foundname, NULL));
- if (rdataset != NULL type != dns_rdatatype_any) {
+ if (rdataset != NULL (type != dns_rdatatype_any || is_dname)) {
/* dns_rdatalist_tordataset returns success only */
CHECK(clone_rdatalist_to_rdataset(ldapdb-common.mctx, rdlist,
rdataset));
@@ -600,6 +614,8 @@ skipfind:
return DNS_R_DELEGATION;
else if (is_cname)
return DNS_R_CNAME;
+ else if (is_dname)
+ return DNS_R_DNAME;
else
return ISC_R_SUCCESS;
--
1.7.11.7
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0116] Fix crash caused by invalid wildcard in update policy string
On Mon, Feb 25, 2013 at 03:28:57PM +0100, Petr Spacek wrote:
Hello,
Fix crash caused by invalid wildcard in update policy string.
https://fedorahosted.org/bind-dyndb-ldap/ticket/108
Question:
What we should do if update policy string contains an error?
Should we disable all updates?
Or let the old policy in place?
I vote for disallowing all updates.
+1. In my opinion disallowing all updates is correct.
Ack for the patch.
From 9265430d94cb4997188583b8e4c2befe7b28ba4b Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Mon, 25 Feb 2013 15:24:07 +0100
Subject: [PATCH] Fix crash caused by invalid wildcard in update policy
string.
https://fedorahosted.org/bind-dyndb-ldap/ticket/108
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/acl.c | 12
1 file changed, 12 insertions(+)
diff --git a/src/acl.c b/src/acl.c
index
c62a8cb9e867b658b65ce05a07fc31377b2356c2..f95cf431b6363d82085e9cfec7e6c1d6ddd45d7a
100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -420,6 +420,18 @@ acl_configure_zone_ssutable(const char *policy_str,
dns_zone_t *zone)
CHECK(get_fixed_name(stmt, name, fname));
CHECK(get_types(mctx, stmt, types, n));
+ if (match_type == DNS_SSUMATCHTYPE_WILDCARD
+ !dns_name_iswildcard(dns_fixedname_name(fname))) {
+ char name[DNS_NAME_FORMATSIZE];
+ dns_name_format(dns_fixedname_name(fname), name,
+ DNS_NAME_FORMATSIZE);
+ dns_zone_log(zone, ISC_LOG_ERROR,
+ invalid update policy:
+ name '%s' is expected to be a wildcard,
+ name);
+ CLEANUP_WITH(DNS_R_BADNAME);
+ }
+
result = dns_ssutable_addrule(table, grant,
dns_fixedname_name(fident),
match_type,
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0112] Make log messages related to Kerberos more verbose
); + CHECK_KRB5(context, krberr, Failed to get initial credentials (TGT) + using principal '%s' and keytab '%s', + principal, keyfile); my_creds_ptr = my_creds; /* store credentials in cache */ krberr = krb5_cc_initialize(context, ccache, kprincpw); - CHECK_KRB5(context, krberr, Failed to initialize ccache); + CHECK_KRB5(context, krberr, Failed to initialize credentials cache + '%s', str_buf(ccname)); krberr = krb5_cc_store_cred(context, ccache, my_creds); - CHECK_KRB5(context, krberr, Failed to store ccache); + CHECK_KRB5(context, krberr, Failed to store credentials + in credentials cache '%s', str_buf(ccname)); result = ISC_R_SUCCESS; -- 1.7.11.7 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0111] Automatically reload invalid zone after each change in zone data
. Records can be outdated, run `rndc reload`, pevent-dn, pevent-chgtype); + } if (dns_name_dynamic(name)) dns_name_free(name, inst-mctx); -- 1.7.11.7 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0113] Periodically reconnect if Kerberos KDC is unavailable
On Fri, Feb 15, 2013 at 09:53:46AM +0100, Petr Spacek wrote: Hello, Periodically reconnect if Kerberos KDC is unavailable. https://fedorahosted.org/bind-dyndb-ldap/ticket/100 At the moment, Kerberos libraries contain a memory leak which will be triggered by periodical reconnecting implemented in this ticket. https://bugzilla.redhat.com/show_bug.cgi?id=90 https://bugzilla.redhat.com/show_bug.cgi?id=911147 -- Petr^2 Spacek Ack From bed25248a8c08c2f13027f1dcffecd5ffc60f2dd Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Fri, 15 Feb 2013 09:42:41 +0100 Subject: [PATCH] Periodically reconnect if Kerberos KDC is unavailable. https://fedorahosted.org/bind-dyndb-ldap/ticket/100 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 491817813229f988161f3cedc6c83055040ad3ae..509b5019d1c4b8e330cf21aeefc7ef686e747c19 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -2286,7 +2286,7 @@ force_reconnect: str_buf(ldap_inst-krb5_keytab)); UNLOCK(ldap_inst-kinit_lock); if (result != ISC_R_SUCCESS) - return result; + return ISC_R_NOTCONNECTED; } log_debug(4, trying interactive bind using %s mechanism, -- 1.7.11.7 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0110] Fix crash during per-zone cache flush
On Mon, Feb 04, 2013 at 05:37:05PM +0100, Petr Spacek wrote:
Hello,
Fix crash during per-zone cache flush.
https://fedorahosted.org/bind-dyndb-ldap/ticket/107
Ack, thanks for the patch.
Regards, Adam
From 42edc724eedd401bd1b1e06f1b1c6ef2ad878ed2 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Mon, 4 Feb 2013 17:35:24 +0100
Subject: [PATCH] Fix crash during per-zone cache flush.
https://fedorahosted.org/bind-dyndb-ldap/ticket/107
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/zone_register.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/zone_register.c b/src/zone_register.c
index
03eb1d0765de371420e4da6beb1a7b2e2e52db94..e52397357f2ef482d8dfee33f7564cd9157afa56
100644
--- a/src/zone_register.c
+++ b/src/zone_register.c
@@ -267,8 +267,10 @@ zr_flush_all_caches(zone_register_t *zr) {
ldap_cache_t *cache;
CHECK(dns_rbtnodechain_current(chain, NULL, NULL, node));
- cache = ((zone_info_t *)(node-data))-cache;
- CHECK(flush_ldap_cache(cache));
+ if (node-data != NULL) { /* skip auxiliary RBT nodes */
+ cache = ((zone_info_t *)(node-data))-cache;
+ CHECK(flush_ldap_cache(cache));
+ }
result = dns_rbtnodechain_next(chain, NULL, NULL);
}
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] A new proopsal for Location Based Discovery
On Tue, Jan 22, 2013 at 07:33:53PM -0500, Simo Sorce wrote: On Tue, 2013-01-22 at 17:46 +0100, Adam Tkac wrote: On Tue, Jan 22, 2013 at 11:19:30AM -0500, Simo Sorce wrote: On Tue, 2013-01-22 at 17:02 +0100, Adam Tkac wrote: On Tue, Jan 22, 2013 at 10:25:21AM -0500, Simo Sorce wrote: On Tue, 2013-01-22 at 16:18 +0100, Adam Tkac wrote: Before we start talking about using DNS for this purpose, have you considered to use IP anycast for this? You can simply create multiple servers with same IP address on different places over the world. After that you announce this IP address from multiple places simultaneounsly via BGP and BGP automatically routes all clients to the closest node. Advantage is that this is already implemented, used and nothing have to be modified. Regards, Adam We cannot assume our customers can influence or have access to change BGP routing, so I excluded multicast solutions from the get go. Also it requires more changes on the clients which is another heavy minus. If I understand correctly, target customers of IPA are companies and they use IPA to maintain resources in their internal networks, aren't they? In this case I see two basic solutions how to solve the location issue. 1. BGP routing between multiple internal networks Sorry Adam, I do not want to be dismissive, and I know that in an ideal world this would be an awesome solution. Just trust me that for most cases asking someone to change their network architecture is simply impossible. This is definitely right. However please read my previous post - I don't propose to change network architecture. Do you how to interconnect multiple networks without routers? I don't. So routers are already present in customer's networks. It can be even static routing, not BGP, and admin can simply set rule on router which physical server clients should use. We have users telling us their network admins don't even want change firewall configurations in some cases, so you can well see how they would respond to someone asking them to change their routing or enabling and using multicast. I think it's same amount of work to add record to DNS or to add record to the static or dynamic routing tables. Adding a record to a DNS server is quite different from changing routing and starting routing multicast packets. Please note anycast != multicast. Anycast is unicast so no multicast is involved. Sorry but it simply is not a solution we can consider. Why? Which setup cannot be achieved with routing configuration and can be achieved with location information in DNS? Queries from clients behind a VPN that doesn't do multicast ? In general multicast cannot be assumed to be available/configured. And it requires support in clients as well as services. Also 'location' doesn't mean necessarily 'local'. My client in NYC may be configured to be bound to servers in Boston for whatever administrative reason. Boston is in no way local to me but is my 'location'. How do you deliver that information in a schema like the one you had in mind ? This is not possible with my anycast proposal. Thanks for explanation, I just didn't imagine which schema cannot be configured on routing level and this is the one. Regards, Adam -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] A new proopsal for Location Based Discovery
on the available host records on the fly at DNS server startup, and then keep them cached (and updated) by the bind-dyndb-ldap plugin, which will include these records in AXFR transfers but will not write them back to the LDAP server keeping them local. This solution might be the golden egg, as it might allow all the advantages of dynamic generation, as well as response performance and solve the slave server issue and perhaps even DNSSEC related issues. It has a major drawback, it would make the code a lot more compicated and critical. Overall implementation proposal Given that the basic solution is relatively simple and require minimal if no client changes we should consider implementing at least part of this proposal as soon as possible. Implementing DNAME record support in bind-dyndb-ldap seem a prerequisite and adding client support in the SSSD IPA provider would allow to test at least with the basic setup. This basic support should be implemented sooner rather than later so that full dynamic support can lately be easily added to bind-dyndb-ldap support as well as adding the necessary additional schema and UI to the freeipa framework to mark and group clients and locations. Security Considerations Client Implementation As always DNS replies can be spoofed relatively easily. We recommend that SRV records resolution is used only for those clients that normally use an additional security protocol to talk to network resources and can use additional mechanisms to authenticate these resources. For example a client that uses an LDAP server for security related information like user identity information should only trust SRV record discovery for the LDAP service if LDAPS or STARTTLS over LDAP are mandatory and certificate verification is fully turned on, or if SASL/GSSAPI is used with mutual authentication, integrity and confidentiality options required. Use of DNSSEC and full DNS signature verification may be considered an additional requirement in some cases. Server Implementation TODO: interaction with DNSSEC References SRV Records: RFC 2782 DNAME Records: RFC 6672 -- Simo Sorce * Red Hat, Inc * New York -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] A new proopsal for Location Based Discovery
On Tue, Jan 22, 2013 at 10:25:21AM -0500, Simo Sorce wrote: On Tue, 2013-01-22 at 16:18 +0100, Adam Tkac wrote: Before we start talking about using DNS for this purpose, have you considered to use IP anycast for this? You can simply create multiple servers with same IP address on different places over the world. After that you announce this IP address from multiple places simultaneounsly via BGP and BGP automatically routes all clients to the closest node. Advantage is that this is already implemented, used and nothing have to be modified. Regards, Adam We cannot assume our customers can influence or have access to change BGP routing, so I excluded multicast solutions from the get go. Also it requires more changes on the clients which is another heavy minus. If I understand correctly, target customers of IPA are companies and they use IPA to maintain resources in their internal networks, aren't they? In this case I see two basic solutions how to solve the location issue. 1. BGP routing between multiple internal networks If customer wants to interconnect multiple networks (for example networks in different offices) so resources in network 1 will be accessible from network 2, he must use some kind of routing. All traffic from network 1 must go through border router and is accepted by border router in network 2: network1 - router1 - router2 - network2 This can be extended to multiple offices and all border routers will talk to each other. In this scenario customer can specify set of rules on each router and route traffic to services to specific locations. Please note that there is no need to announce anything to the Internet via BGP. 2. No routing between internal networks In this case networks aren't interconnected so no routing is involved. In this case location discovery doesn't make sense because machine in network 1 cannot access resources in network 2. So it will also use the closest service. To summarize my idea, as long as services have _same_ IP addresses in all cooperating IPA installations, which definitely make sense, you don't need to use DNS for location because routing protocol will automatically pick the closest location. I don't see any reason for modifications on clients. Everything what will be modified is routing rules on border routers. Please note that anycast != multicast. Regards, Adam -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] A new proopsal for Location Based Discovery
On Tue, Jan 22, 2013 at 11:19:30AM -0500, Simo Sorce wrote: On Tue, 2013-01-22 at 17:02 +0100, Adam Tkac wrote: On Tue, Jan 22, 2013 at 10:25:21AM -0500, Simo Sorce wrote: On Tue, 2013-01-22 at 16:18 +0100, Adam Tkac wrote: Before we start talking about using DNS for this purpose, have you considered to use IP anycast for this? You can simply create multiple servers with same IP address on different places over the world. After that you announce this IP address from multiple places simultaneounsly via BGP and BGP automatically routes all clients to the closest node. Advantage is that this is already implemented, used and nothing have to be modified. Regards, Adam We cannot assume our customers can influence or have access to change BGP routing, so I excluded multicast solutions from the get go. Also it requires more changes on the clients which is another heavy minus. If I understand correctly, target customers of IPA are companies and they use IPA to maintain resources in their internal networks, aren't they? In this case I see two basic solutions how to solve the location issue. 1. BGP routing between multiple internal networks Sorry Adam, I do not want to be dismissive, and I know that in an ideal world this would be an awesome solution. Just trust me that for most cases asking someone to change their network architecture is simply impossible. This is definitely right. However please read my previous post - I don't propose to change network architecture. Do you how to interconnect multiple networks without routers? I don't. So routers are already present in customer's networks. It can be even static routing, not BGP, and admin can simply set rule on router which physical server clients should use. We have users telling us their network admins don't even want change firewall configurations in some cases, so you can well see how they would respond to someone asking them to change their routing or enabling and using multicast. I think it's same amount of work to add record to DNS or to add record to the static or dynamic routing tables. Sorry but it simply is not a solution we can consider. Why? Which setup cannot be achieved with routing configuration and can be achieved with location information in DNS? Regards, Adam -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0107] Don't fail if idnsSOAserial attribute is missing in LDAP
On Fri, Jan 11, 2013 at 06:47:52PM +0100, Petr Spacek wrote:
Hello,
Don't fail if idnsSOAserial attribute is missing in LDAP.
DNS zones created on remote IPA 3.0 server don't have
idnsSOAserial attribute present in LDAP.
https://bugzilla.redhat.com/show_bug.cgi?id=894131
Attached patch contains the minimal set of changes need for resurrecting BIND.
In configurations with serial auto-increment:
- enabled (IPA 3.0+ default) - some new serial is written back to
LDAP nearly immediately
- disabled - the attribute will be missing forever
Ack
From 958f46a5ceee336e2466686bafbb203082e2ccc1 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 11 Jan 2013 17:30:03 +0100
Subject: [PATCH] Don't fail if idnsSOAserial attribute is missing in LDAP.
DNS zones created on remote IPA 3.0 server don't have
idnsSOAserial attribute present in LDAP.
https://bugzilla.redhat.com/show_bug.cgi?id=894131
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_entry.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/ldap_entry.c b/src/ldap_entry.c
index
1e165ca696ccafa177f17b97bda08ed9cc344c7d..52b927d410300eb6df98ea058c3a08b426d66a70
100644
--- a/src/ldap_entry.c
+++ b/src/ldap_entry.c
@@ -350,8 +350,9 @@ ldap_entry_getfakesoa(ldap_entry_t *entry, const
ld_string_t *fake_mname,
ldap_valuelist_t values;
int i = 0;
+ const char *soa_serial_attr = idnsSOAserial;
const char *soa_attrs[] = {
- idnsSOAmName, idnsSOArName, idnsSOAserial,
+ idnsSOAmName, idnsSOArName, soa_serial_attr,
idnsSOArefresh, idnsSOAretry, idnsSOAexpire,
idnsSOAminimum, NULL
};
@@ -366,12 +367,25 @@ ldap_entry_getfakesoa(ldap_entry_t *entry, const
ld_string_t *fake_mname,
CHECK(str_cat_char(target, ));
}
for (; soa_attrs[i] != NULL; i++) {
- CHECK(ldap_entry_getvalues(entry, soa_attrs[i], values));
+ result = ldap_entry_getvalues(entry, soa_attrs[i], values);
+ /** Workaround for
+ * https://bugzilla.redhat.com/show_bug.cgi?id=894131
+ * DNS zones created on remote IPA 3.0 server don't have
+ * idnsSOAserial attribute present in LDAP. */
+ if (result == ISC_R_NOTFOUND
+ soa_attrs[i] == soa_serial_attr) {
+ /* idnsSOAserial is missing! Read it as 1. */
+ CHECK(str_cat_char(target, 1 ));
+ continue;
+ } else if (result != ISC_R_SUCCESS)
+ goto cleanup;
+
CHECK(str_cat_char(target, HEAD(values)-value));
CHECK(str_cat_char(target, ));
}
cleanup:
+ /* TODO: check for memory leaks */
return result;
}
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0098] Log failures detected in CHECK() macro
On Tue, Dec 04, 2012 at 01:24:39PM +0100, Petr Spacek wrote:
On 11/22/2012 02:05 PM, Adam Tkac wrote:
On Tue, Nov 20, 2012 at 02:44:49PM +0100, Petr Spacek wrote:
Hello,
Log failures detected in CHECK() macro.
Function ldap_query() can return ISC_R_NOTFOUND legitimately.
For this and similar cases CHECK_CONDLOG macro was introduced.
It will not log if result != ISC_R_SUCCESS but == ignored_code.
Nested condition will be eliminated by optimizing compiler
in cases where ignored_code == ISC_R_SUCCESS.
Function add_soa_record() is now called only for zones to prevent
false error messages.
Nack.
I don't like second part of the patch much, it adds huge amount of logging
and now we will log every error twice because we already log errors
explicitly.
In my opinion better will be to add new configuration option, for example
debug, and with this option we can emit log messages from CHECK macros (I
haven't though about implementation details, yet). Otherwise we should avoid
logging because it's useless to log all errors, they are expected in
production
environment.
I also don't like CHECK_CONDLOG macro, it's not intuitive and with it we can
end
with so called spaghetti code... As I wrote above I would log every CHECK
failure with debugging on.
However the first patch of the patch is fine (the add_soa_record part).
Ok, reworked patch is attached. Logging is enabled only if
configuration option 'verbose_checks yes' is present. I
decommissioned CHECK_CONDLOG(), so each request for non-existing
record will log failure: not found (when verbose mode is enabled).
This looks fine for me. In future we might consider to add some rate-limiting
patch for log_error_position() calls because in production environment debug log
can be too huge but this is not blocker for the patch.
Ack
Regards, Adam
From 0efaa684d8c536805762d9a835897889cac87d25 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 4 Dec 2012 13:12:53 +0100
Subject: [PATCH] Add option to log all failures detected in CHECK() macro.
Logging will be enabled if 'verbose_checks' option is set to 'yes'.
Function add_soa_record() is now called only for zones to prevent
false error messages.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 7 +++
src/settings.c | 1 +
src/util.h | 15 +++
src/zone_manager.c | 2 ++
4 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
7d1febb68a4773d4e1127e8135d30fd855ded6a6..436985247803240f9ec4f2c3e5118adf8466beec
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1694,10 +1694,9 @@ ldap_parse_rrentry(isc_mem_t *mctx, ldap_entry_t
*entry,
const char *dn = NULL entry;
const char *data = NULL data;
- result = add_soa_record(mctx, qresult, origin, entry,
- rdatalist, fake_mname);
- if (result != ISC_R_SUCCESS result != ISC_R_NOTFOUND)
- goto cleanup;
+ if ((ldap_entry_getclass(entry) LDAP_ENTRYCLASS_ZONE) != 0)
+ CHECK(add_soa_record(mctx, qresult, origin, entry,
+ rdatalist, fake_mname));
rdclass = ldap_entry_getrdclass(entry);
ttl = ldap_entry_getttl(entry);
diff --git a/src/settings.c b/src/settings.c
index
25578ce2687bf12e3a2d387caf0b26ed1a3083b6..08164766172f5f915584ae51b43e3d64798eed71
100644
--- a/src/settings.c
+++ b/src/settings.c
@@ -32,6 +32,7 @@
#include util.h
#include types.h
+isc_boolean_t verbose_checks = ISC_FALSE; /* log each failure in CHECK()
macro */
/*
* Forward declarations.
diff --git a/src/util.h b/src/util.h
index
c61f4e7a4930717cfd7729caa2c2f36854d1903f..d6d3c73e6d25657805eee904e6047c542e52a656
100644
--- a/src/util.h
+++ b/src/util.h
@@ -21,6 +21,8 @@
#ifndef _LD_UTIL_H_
#define _LD_UTIL_H_
+extern isc_boolean_t verbose_checks; /* from settings.c */
+
#include log.h
#define CLEANUP_WITH(result_code)\
@@ -32,15 +34,12 @@
#define CHECK(op)\
do {\
result = (op); \
- if (result != ISC_R_SUCCESS)\
+ if (result != ISC_R_SUCCESS) { \
+ if (verbose_checks == ISC_TRUE) \
+ log_error_position(check failed: %s,
\
+dns_result_totext(result));
\
goto cleanup; \
- } while (0)
-
-#define CHECK_NEXT(op) \
- do {\
- result = (op
Re: [Freeipa-devel] [PATCH 101] Fix error handling in ldap_entry_create()
On Thu, Nov 22, 2012 at 09:56:12AM +0100, Petr Spacek wrote:
Hello,
apparently I was very tired yesterday ... Cleaned version of the
patch is attached.
Petr^2 Spacek
On 11/21/2012 05:04 PM, Petr Spacek wrote:
Hello,
fixed fix is attached. Clang found bug in the fix but I didn't notice that
because of other warnings ...
On 11/21/2012 04:04 PM, Petr Spacek wrote:
Hello,
I noticed this problem during investigation of dead code found by Clang.
Fix error handling in ldap_entry_create().
Jump to cleanup section after first memory allocation created memory
leak
which crashed BIND on reload.
Missing return value check after ldap_get_dn() call can lead to crash.
Ack
From eddb9db446610e79e4852b15cb2be420090364b7 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 21 Nov 2012 15:53:28 +0100
Subject: [PATCH] Fix error handling in ldap_entry_create().
Jump to cleanup section after first memory allocation created memory leak
which crashed BIND on reload.
Missing return value check after ldap_get_dn() call can lead to crash.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_entry.c | 26 --
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/src/ldap_entry.c b/src/ldap_entry.c
index
9436b895913b2eb1a711d9343e43e695ea7e6ae4..7d8b29bd0f1206df6b4fcdcdb08c4c9e82630527
100644
--- a/src/ldap_entry.c
+++ b/src/ldap_entry.c
@@ -183,9 +183,9 @@ ldap_entry_create(isc_mem_t *mctx, LDAP *ld, LDAPMessage
*ldap_entry,
ldap_entry_t **entryp)
{
isc_result_t result;
- ldap_attribute_t *attr;
+ ldap_attribute_t *attr = NULL;
char *attribute;
- BerElement *ber;
+ BerElement *ber = NULL;
ldap_entry_t *entry = NULL;
REQUIRE(ld != NULL);
@@ -213,19 +213,25 @@ ldap_entry_create(isc_mem_t *mctx, LDAP *ld,
LDAPMessage *ldap_entry,
APPEND(entry-attrs, attr, link);
}
+ attr = NULL;
entry-dn = ldap_get_dn(ld, ldap_entry);
+ if (entry-dn == NULL) {
+ log_ldap_error(ld);
+ CLEANUP_WITH(ISC_R_FAILURE);
+ }
+ *entryp = entry;
+
+cleanup:
if (ber != NULL)
ber_free(ber, 0);
-
- *entryp = entry;
-
- return ISC_R_SUCCESS;
-
-cleanup:
- if (entry != NULL)
- ldap_attributelist_destroy(mctx, entry-attrs);
+ if (result != ISC_R_SUCCESS) {
+ if (entry != NULL)
+ ldap_attributelist_destroy(mctx, entry-attrs);
+ SAFE_MEM_PUT_PTR(mctx, entry);
+ SAFE_MEM_PUT_PTR(mctx, attr);
+ }
return result;
}
--
1.7.11.7
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0106] Fix error handling for initial zone refresh in persistent search
On Thu, Nov 22, 2012 at 02:55:51PM +0100, Petr Spacek wrote:
Hello,
Fix error handling for initial zone refresh in persistent search.
Old code terminates watcher thread in case of error. Now initial lookup
is restarted after reconnect_interval seconds.
Ack
From 99a820736eab9ad597b193fe504ca965263b6655 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 22 Nov 2012 14:52:08 +0100
Subject: [PATCH] Fix error handling for initial zone refresh in persistent
search.
Old code terminates watcher thread in case of error. Now initial lookup
is restarted after reconnect_interval seconds.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
57235de7da158b7a05de9e35fff2cdaaff74276c..28b2eb45c116572d76fefcfb9f5c1fba5411dc12
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -3757,6 +3757,7 @@ ldap_psearch_watcher(isc_threadarg_t arg)
restart:
/* Perform initial lookup */
+ ldap_query_free(ISC_TRUE, ldap_qresult);
flush_required = ISC_TRUE;
if (inst-psearch) {
log_debug(1, Sending initial psearch lookup);
@@ -3794,15 +3795,30 @@ restart:
if (!sane_sleep(inst, inst-reconnect_interval))
goto cleanup;
}
- ldap_query_free(ISC_TRUE, ldap_qresult);
goto restart;
} else if (flush_required == ISC_TRUE) {
+ isc_boolean_t restart_needed = ISC_FALSE;
/* First LDAP result after (re)start was received
successfully:
* Unload old zones and flush record cache.
* We want to save cache in case of search timeout
during restart.
*/
- CHECK(refresh_zones_from_ldap(inst, ISC_TRUE));
- CHECK(flush_ldap_cache(inst-cache));
+ if ((result = refresh_zones_from_ldap(inst, ISC_TRUE))
+ != ISC_R_SUCCESS) {
+ log_error_r(zone refresh after initial psearch
lookup failed);
+ restart_needed = ISC_TRUE;
+ } else if ((result = flush_ldap_cache(inst-cache))
+ != ISC_R_SUCCESS) {
+ log_error_r(cache flush after initial psearch
lookup failed);
+ restart_needed = ISC_TRUE;
+ }
+
+ if (restart_needed) {
+ if (!sane_sleep(inst, inst-reconnect_interval))
+ goto cleanup;
+
+ goto restart;
+ }
+
flush_required = ISC_FALSE;
}
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 92] Flush whole zone from cache during zone renaming/removal.
On Wed, Dec 05, 2012 at 01:25:56PM +0100, Petr Spacek wrote:
On 12/04/2012 02:36 PM, Adam Tkac wrote:
On Thu, Nov 15, 2012 at 07:06:37PM +0100, Petr Spacek wrote:
Hello,
attached patch is preliminary implementation of selective zone flush.
Implementation is not so straight-forward as I want to see. Before
discussing the patch itself - can we consider per-zone caches? In
that case, we can simply deallocate whole per-zone RBT and we are
done.
Pros:
* Potentially better concurrency, simpler code, much less corner cases.
Cons:
* We have to look into Zone register before searching the cache.
* It can limit concurrency ... with many extra small zones? I'm not sure.
Hi Peter,
In my opinion per-zone caches are better. Look into zone register isn't
costly operation.
Second version of the patch with per-zone caches is attached. I cut
all debugging code so this version could be considered as final.
Ack
From 9b1fe8c26049d0aeff8cc48c36b4faa4aca12b30 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 5 Dec 2012 12:52:41 +0100
Subject: [PATCH] Create separate record cache for each zone.
This separation should solve all problems with stale records
after zone deletion and renaming.
https://fedorahosted.org/bind-dyndb-ldap/ticket/91
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/cache.c | 45 +
src/cache.h | 3 ++-
src/ldap_helper.c | 72
+
src/ldap_helper.h | 3 ---
src/zone_register.c | 69 ++
src/zone_register.h | 11 +++-
6 files changed, 139 insertions(+), 64 deletions(-)
diff --git a/src/cache.c b/src/cache.c
index
898d48b291a83da7f77dbcf79e2bd3e7ff8281aa..fd57fd92eb35dcbbd5c1e1911a93730624e30002
100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -38,11 +38,11 @@
#include util.h
struct ldap_cache {
- isc_mutex_t mutex; /* TODO: RWLOCK? */
- isc_mem_t *mctx;
- dns_rbt_t *rbt;
- isc_interval_t cache_ttl;
- isc_boolean_t psearch;
+ isc_mutex_t mutex; /* TODO: RWLOCK? */
+ isc_mem_t *mctx;
+ dns_rbt_t *rbt;
+ const isc_interval_t*cache_ttl; /* pointer to LDAP instance */
+ const isc_boolean_t *psearch; /* pointer to LDAP instance */
};
typedef struct {
@@ -78,8 +78,9 @@ cache_node_create(ldap_cache_t *cache, cache_node_t **nodep)
isc_mem_attach(cache-mctx, node-mctx);
ZERO_PTR(node-rdatalist);
/* Do not set the ttl when psearch is enabled. */
- if (!cache-psearch)
- CHECK(isc_time_nowplusinterval(node-valid_until,
cache-cache_ttl));
+ if (*cache-psearch == ISC_FALSE)
+ CHECK(isc_time_nowplusinterval(node-valid_until,
+cache-cache_ttl));
*nodep = node;
return ISC_R_SUCCESS;
@@ -90,29 +91,27 @@ cleanup:
return result;
}
+/**
+ * @param[in] cache_ttl ISC interval in LDAP instance shared by all caches
+ * @param[in] psearch boolean in LDAP instance shared by all caches
+ */
isc_result_t
-new_ldap_cache(isc_mem_t *mctx, const char *const *argv, ldap_cache_t
**cachep, isc_boolean_t psearch)
+new_ldap_cache(isc_mem_t *mctx, const isc_interval_t *cache_ttl,
+const isc_boolean_t *psearch, ldap_cache_t **cachep)
{
isc_result_t result;
ldap_cache_t *cache = NULL;
- unsigned int cache_ttl;
- setting_t cache_settings[] = {
- { cache_ttl, default_uint(120) },
- end_of_settings
- };
+ REQUIRE(cache_ttl != NULL);
+ REQUIRE(psearch != NULL);
REQUIRE(cachep != NULL *cachep == NULL);
- cache_settings[0].target = cache_ttl;
- CHECK(set_settings(cache_settings, argv));
-
CHECKED_MEM_GET_PTR(mctx, cache);
ZERO_PTR(cache);
isc_mem_attach(mctx, cache-mctx);
- isc_interval_set(cache-cache_ttl, cache_ttl, 0);
-
- if (cache_ttl) {
+ cache-cache_ttl = cache_ttl;
+ if (!isc_interval_iszero(cache_ttl)) {
CHECK(dns_rbt_create(mctx, cache_node_deleter, NULL,
cache-rbt));
CHECK(isc_mutex_init(cache-mutex));
@@ -123,21 +122,23 @@ new_ldap_cache(isc_mem_t *mctx, const char *const
*argv, ldap_cache_t **cachep,
return ISC_R_SUCCESS;
cleanup:
- if (cache != NULL)
- destroy_ldap_cache(cache);
+ destroy_ldap_cache(cache);
return result;
}
void
destroy_ldap_cache(ldap_cache_t **cachep)
{
ldap_cache_t *cache;
- REQUIRE(cachep != NULL *cachep != NULL);
+ REQUIRE(cachep != NULL);
cache = *cachep;
+ if (cache == NULL)
+ return;
+
if (cache-rbt) {
LOCK(cache-mutex
Re: [Freeipa-devel] [PATCH 0093] Log memory allocation failures
On Tue, Nov 20, 2012 at 10:11:42AM +0100, Petr Spacek wrote:
On 11/19/2012 06:33 PM, Adam Tkac wrote:
On Fri, Nov 16, 2012 at 03:15:44PM +0100, Petr Spacek wrote:
On 11/16/2012 02:24 PM, Simo Sorce wrote:
On Fri, 2012-11-16 at 13:31 +0100, Petr Spacek wrote:
+#define FILE_LINE_FUNC_MSG %-15s: %4d: %-20s
+#define FILE_LINE_FUNC_ARG __FILE__, __LINE__, __func__
snip
Thanks for the patch, please read my comments below.
snip
diff --git a/src/log.h b/src/log.h
index
9062a4e80786b5bab806d6c9ceba1d1a9917a3e2..d6a40151d25b6f67cf6735ec955d45e4ebe4106c
100644
--- a/src/log.h
+++ b/src/log.h
snip
+#define log_error_position(format, ...)\
+ log_error([%-15s: %4d: %-21s] format,\
Do we really need to format the format string? I would prefer to drop the
[...] stuff at all.
This format will align file names, line numbers and most of function
names in columns. It can be handy if some error propagates through
call stack, but I can drop it if you want.
Real world example:
[ldap_entry.c : 370: ldap_entry_getfakesoa] FAILURE: not found
[ldap_helper.c : 1869: add_soa_record ] FAILURE: not found
My intention is to extend logging to failures detected inside CHECK() macros.
Currently, any failure inside CHECK() will jump to cleanup label,
so information about source of the failure is lost.
As a result it produces error message like:
update_record (psearch) failed, dn
'idnsName=q.sub,idnsname=test,cn=dns,dc=e,dc=test' change type 0x1.
Records can be outdated, run `rndc reload`: not found
Huh? What was not found? Those error messages are not useful for
post mortem analysis.
With logging inside CHECK() it produces:
[ldap_helper.c : 3467: update_record] FAILURE: not found
update_record (psearch) failed, dn
'idnsName=q.sub,idnsname=test,cn=dns,dc=e,dc=test' change type 0x1.
Records can be outdated, run `rndc reload`: not found
File ldap_helper.c, line 3467, return code ISC_R_NOTFOUND is much
better information.
Ok, you convinced me.
Ack.
snip
@@ -46,15 +48,17 @@
(target_ptr) = isc_mem_allocate((m), (s)); \
if ((target_ptr) == NULL) { \
result = ISC_R_NOMEMORY;\
+ log_error_position(MEMORY ALLOCATION FAILURE);
\
Such huge msg is not needed, I think. What about more conservative
log_error_position(Memory allocation failed)
You are right, I shouldn't scream too much. Amended patch is attached.
From e9e4b8aa38cb612cf39a1e917c74fd0022de6fd4 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 16 Nov 2012 13:17:44 +0100
Subject: [PATCH] Log memory allocation failures.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/log.h | 7 ++-
src/util.h | 5 +
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/log.h b/src/log.h
index
9062a4e80786b5bab806d6c9ceba1d1a9917a3e2..d6a40151d25b6f67cf6735ec955d45e4ebe4106c
100644
--- a/src/log.h
+++ b/src/log.h
@@ -23,22 +23,27 @@
#include isc/error.h
#include dns/log.h
+#include dns/result.h
#ifdef LOG_AS_ERROR
#define GET_LOG_LEVEL(level) ISC_LOG_ERROR
#else
#define GET_LOG_LEVEL(level) (level)
#endif
#define fatal_error(...) \
-isc_error_fatal(__FILE__, __LINE__, __VA_ARGS__)
+ isc_error_fatal(__FILE__, __LINE__, __VA_ARGS__)
#define log_bug(fmt, ...) \
log_error(bug in %s(): fmt, __func__,##__VA_ARGS__)
#define log_error_r(fmt, ...) \
log_error(fmt : %s, ##__VA_ARGS__, dns_result_totext(result))
+#define log_error_position(format, ...) \
+ log_error([%-15s: %4d: %-21s] format,\
+ __FILE__, __LINE__, __func__, ##__VA_ARGS__)
+
/* Basic logging functions */
#define log_error(format, ...) \
log_write(GET_LOG_LEVEL(ISC_LOG_ERROR), format, ##__VA_ARGS__)
diff --git a/src/util.h b/src/util.h
index
2b8f10e59ddacdb1d0e49cf0de3e9ab8a3786090..c61f4e7a4930717cfd7729caa2c2f36854d1903f
100644
--- a/src/util.h
+++ b/src/util.h
@@ -21,6 +21,8 @@
#ifndef _LD_UTIL_H_
#define _LD_UTIL_H_
+#include log.h
+
#define CLEANUP_WITH(result_code)\
do {\
result = (result_code); \
@@ -46,15 +48,17 @@
(target_ptr) = isc_mem_allocate((m), (s)); \
if ((target_ptr) == NULL) { \
result = ISC_R_NOMEMORY;\
+ log_error_position(Memory allocation failed); \
goto cleanup; \
} \
} while (0)
#define CHECKED_MEM_GET(m, target_ptr, s)\
do
Re: [Freeipa-devel] [PATCH 0090] Fix origin handling in dn_to_dnsname() for zone DNs
On Tue, Nov 13, 2012 at 05:46:20PM +0100, Petr Spacek wrote:
Hello,
I found the bug in dn_to_dnsname() during my work on
https://fedorahosted.org/bind-dyndb-ldap/ticket/91
Cache is not flushed properly if renamed/deleted zone has superior zone in
LDAP
Consider this change as part of solution for ticket #91:
Fix origin handling in dn_to_dnsname() for zone DNs.
This patch fixes case where DN is zone (i.e. DN with single idnsName
component) and origin is non-NULL.
Function str_to_isc_buffer() was fixed to not truncate last character.
Ack
From 840dd9d6ee6eefde0baf00930590bb279e73db75 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 13 Nov 2012 17:29:05 +0100
Subject: [PATCH] Fix origin handling in dn_to_dnsname() for zone DNs.
This patch fixes case where DN is zone (i.e. DN with single idnsName
component) and origin is non-NULL.
Function str_to_isc_buffer() was fixed to not truncate last character.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_convert.c | 39 +--
src/str.c | 2 +-
2 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/src/ldap_convert.c b/src/ldap_convert.c
index
3352c573cafc54421c77b0b770657841b90ebd71..394d8d036c1d4e0f8a82499282d1acef7d442bcd
100644
--- a/src/ldap_convert.c
+++ b/src/ldap_convert.c
@@ -91,12 +91,11 @@ dn_to_dnsname(isc_mem_t *mctx, const char *dn, dns_name_t
*target,
CHECK(dn_to_text(dn, str, ostr));
str_to_isc_buffer(str, buffer);
- CHECK(dns_name_fromtext(name, buffer, dns_rootname, 0, NULL));
+ CHECK(dns_name_fromtext(name, buffer, NULL, 0, NULL));
if (otarget != NULL) {
str_to_isc_buffer(ostr, buffer);
- CHECK(dns_name_fromtext(origin, buffer, dns_rootname, 0,
- NULL));
+ CHECK(dns_name_fromtext(origin, buffer, NULL, 0, NULL));
}
cleanup:
@@ -124,14 +123,26 @@ cleanup:
return result;
}
-/*
- * Convert LDAP dn to DNS name.
+/**
+ * Convert LDAP DN to absolute DNS name.
*
- * Example:
- * dn = idnsName=foo, idnsName=bar, idnsName=example.org, cn=dns,
- * dc=example, dc=org
+ * @param[out] target Absolute DNS name derived from the all idnsNames.
+ * @param[out] origin Absolute DNS name derived from the last idnsName
+ *component of DN, i.e. zone. Can be NULL.
*
- * The resulting string will be foo.bar.example.org.
+ * @code
+ * Examples:
+ * dn = idnsName=foo, idnsName=bar, idnsName=example.org,
+ * cn=dns, dc=example, dc=org
+ * target = foo.bar.example.org.
+ * origin = example.org.
+ *
+ * dn = idnsname=89, idnsname=4.34.10.in-addr.arpa.,
+ * cn=dns, dc=example, dc=org
+ * target = 89.4.34.10.in-addr.arpa.
+ * origin = 4.34.10.in-addr.arpa.
+ * (The dot at the end is not doubled when it's already present.)
+ * @endcode
*/
isc_result_t
dn_to_text(const char *dn, ld_string_t *target, ld_string_t *origin)
@@ -159,24 +170,24 @@ dn_to_text(const char *dn, ld_string_t *target,
ld_string_t *origin)
CHECK(explode_rdn(exploded_dn[i], exploded_rdn, 1));
CHECK(str_cat_char(target, exploded_rdn[0]));
- CHECK(str_cat_char(target, .));
+ if (str_buf(target)[str_len(target)-1] != '.')
+ CHECK(str_cat_char(target, .));
}
if (origin != NULL) {
str_clear(origin);
/*
* If we have DNs with only one idnsName part,
- * treat them as absolute.
+ * treat them as absolute zone name.
*/
-
if (i 2)
CHECK(str_init_char(origin, .));
else {
CHECK(str_cat_char(origin, exploded_rdn[0]));
- CHECK(str_cat_char(origin, .));
+ if (str_buf(origin)[str_len(origin)-1] != '.')
+ CHECK(str_cat_char(origin, .));
}
-
}
if (str_len(target) == 0)
diff --git a/src/str.c b/src/str.c
index
83645365ee6eff7bda5fbeda6837f30d4dec41ae..1be3f5b61250cb6900820dd4bf1375a3ed77359c
100644
--- a/src/str.c
+++ b/src/str.c
@@ -465,7 +465,7 @@ str_to_isc_buffer(const ld_string_t *src, isc_buffer_t
*dest)
REQUIRE(src != NULL);
REQUIRE(dest != NULL);
- len = str_len_internal(src) - 1;
+ len = str_len_internal(src);
isc_buffer_init(dest, src-data, len);
isc_buffer_add(dest, len);
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0091] Harden dn_to_text() conversion
On Wed, Nov 14, 2012 at 06:02:43PM +0100, Petr Spacek wrote:
Hello,
Harden dn_to_text() conversion.
All cases covered by new checks will silently cause errors
in other parts of plugin. This code should catch problematic
inputs and stop processing at the beginning.
Ack
From 71feac726d814469291eeb413fa8c90aca02cd2d Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 14 Nov 2012 17:57:13 +0100
Subject: [PATCH] Harden dn_to_text() conversion.
All cases covered by new checks will silently cause errors
in other parts of plugin. This code should catch problematic
inputs and stop processing at the beginning.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_convert.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/src/ldap_convert.c b/src/ldap_convert.c
index
394d8d036c1d4e0f8a82499282d1acef7d442bcd..360c7f2df976bc992213a01d652ce264202ce89e
100644
--- a/src/ldap_convert.c
+++ b/src/ldap_convert.c
@@ -126,6 +126,8 @@ cleanup:
/**
* Convert LDAP DN to absolute DNS name.
*
+ * @param[in] dn LDAP DN with one or two idnsName components at the
+ *beginning.
* @param[out] target Absolute DNS name derived from the all idnsNames.
* @param[out] origin Absolute DNS name derived from the last idnsName
*component of DN, i.e. zone. Can be NULL.
@@ -169,17 +171,40 @@ dn_to_text(const char *dn, ld_string_t *target,
ld_string_t *origin)
}
CHECK(explode_rdn(exploded_dn[i], exploded_rdn, 1));
+ if (exploded_rdn[0] == NULL || exploded_rdn[1] != NULL) {
+ log_error(idnsName component of DN has to have
+ exactly one value: DN '%s', dn);
+ CLEANUP_WITH(ISC_R_NOTIMPLEMENTED);
+ }
CHECK(str_cat_char(target, exploded_rdn[0]));
if (str_buf(target)[str_len(target)-1] != '.')
CHECK(str_cat_char(target, .));
}
+ /* filter out unsupported cases */
+ if (i = 0) {
+ log_error(no idnsName component found in DN '%s', dn);
+ CLEANUP_WITH(ISC_R_UNEXPECTEDEND);
+ } else if (i == 1) { /* zone only - nothing to check */
+ ;
+ } else if (i == 2) {
+ if (exploded_dn[0][strlen(exploded_dn[0])-1] == '.') {
+ log_error(absolute record name in DN
+ is not supported: DN '%s', dn);
+ CLEANUP_WITH(ISC_R_NOTIMPLEMENTED);
+ }
+ } else {
+ log_error(unsupported number of idnsName components in DN
+ '%s': %u components found, dn, i);
+ CLEANUP_WITH(ISC_R_NOTIMPLEMENTED);
+ }
+
if (origin != NULL) {
str_clear(origin);
/*
* If we have DNs with only one idnsName part,
- * treat them as absolute zone name.
+ * treat them as absolute zone name, i.e. origin is root.
*/
if (i 2)
CHECK(str_init_char(origin, .));
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0096] Remove unused str_*split*() functions
); -isc_result_t str_split(const ld_string_t *src, const char delimiter, ld_split_t *split); -size_t str_split_count(const ld_split_t *split); -const char * str_split_get(const ld_split_t *split, unsigned int split_number); - /* These are pseudo-private functions and shouldn't be called directly. */ isc_result_t str__new(isc_mem_t *mctx, ld_string_t **new_str _STR_MEM_FLARG); void str__destroy(ld_string_t **str _STR_MEM_FLARG); -- 1.7.11.7 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0095] Use memory allocation macros more extensively - part 2
:
isc_sockaddr_t *addr = NULL;
addr = ISC_LIST_HEAD(addrs);
ISC_LIST_UNLINK(addrs, addr, link);
- isc_mem_put(inst-mctx, addr, sizeof(*addr));
+ SAFE_MEM_PUT_PTR(inst-mctx, addr);
}
}
if (fwdtbl_deletion_requested) {
@@ -1654,7 +1654,7 @@ ldapdb_rdatalist_destroy(isc_mem_t *mctx,
ldapdb_rdatalist_t *rdatalist)
rdlist = HEAD(*rdatalist);
free_rdatalist(mctx, rdlist);
UNLINK(*rdatalist, rdlist, link);
- isc_mem_put(mctx, rdlist, sizeof(*rdlist));
+ SAFE_MEM_PUT_PTR(mctx, rdlist);
}
}
@@ -1671,7 +1671,7 @@ free_rdatalist(isc_mem_t *mctx, dns_rdatalist_t *rdlist)
UNLINK(rdlist-rdata, rdata, link);
dns_rdata_toregion(rdata, r);
isc_mem_put(mctx, r.base, r.length);
- isc_mem_put(mctx, rdata, sizeof(*rdata));
+ SAFE_MEM_PUT_PTR(mctx, rdata);
}
}
@@ -1931,8 +1931,7 @@ parse_rdata(isc_mem_t *mctx, ldap_qresult_t *qresult,
cleanup:
isc_lex_close(qresult-lex);
- if (rdata != NULL)
- isc_mem_put(mctx, rdata, sizeof(*rdata));
+ SAFE_MEM_PUT_PTR(mctx, rdata);
if (rdatamem.base != NULL)
isc_mem_put(mctx, rdatamem.base, rdatamem.length);
@@ -3589,23 +3588,11 @@ psearch_update(ldap_instance_t *inst, ldap_entry_t
*entry, LDAPControl **ctrls)
isc_mem_attach(inst-mctx, mctx);
- dn = isc_mem_strdup(mctx, entry-dn);
- if (dn == NULL) {
- result = ISC_R_NOMEMORY;
- goto cleanup;
- }
- dbname = isc_mem_strdup(mctx, inst-db_name);
- if (dbname == NULL) {
- result = ISC_R_NOMEMORY;
- goto cleanup;
- }
+ CHECKED_MEM_STRDUP(mctx, entry-dn, dn);
+ CHECKED_MEM_STRDUP(mctx, inst-db_name, dbname);
if (PSEARCH_MODDN(chgtype)) {
- prevdn = isc_mem_strdup(mctx, prevdn_ldap);
- if (prevdn == NULL) {
- result = ISC_R_NOMEMORY;
- goto cleanup;
- }
+ CHECKED_MEM_STRDUP(mctx, prevdn_ldap, prevdn);
}
/*
diff --git a/src/zone_manager.c b/src/zone_manager.c
index
ca3edd010e5f6ea94adb57e5ae5e915a834e52a0..2988cfd0598de57efbc4f7a42447ad1dadce6d65
100644
--- a/src/zone_manager.c
+++ b/src/zone_manager.c
@@ -98,7 +98,7 @@ destroy_db_instance(db_instance_t **db_instp)
if (db_inst-name != NULL)
isc_mem_free(db_inst-mctx, db_inst-name);
- isc_mem_putanddetach(db_inst-mctx, db_inst, sizeof(*db_inst));
+ MEM_PUT_AND_DETACH(db_inst);
*db_instp = NULL;
}
diff --git a/src/zone_register.c b/src/zone_register.c
index
76305730b2e19686568f8a1bc6ac703ed3898fcc..18438bf937a6482ddf058adbecdc21e7cf2e7f26
100644
--- a/src/zone_register.c
+++ b/src/zone_register.c
@@ -167,7 +167,7 @@ delete_zone_info(void *arg1, void *arg2)
isc_mem_free(mctx, zinfo-dn);
dns_zone_detach(zinfo-zone);
- isc_mem_put(mctx, zinfo, sizeof(*zinfo));
+ SAFE_MEM_PUT_PTR(mctx, zinfo);
}
/*
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0097] Deallocate LDAP connections in safe way
On Tue, Nov 20, 2012 at 01:06:59PM +0100, Petr Spacek wrote:
Hello,
Deallocate LDAP connections in safe way.
Current code does isc_mem_detach() before isc_mem_put() so
memory context reference counter is wrong for a small time frame.
Macro MEM_PUT_AND_DETACH will handle it properly including
pointer zeroing.
Ack
From 15bc2ce1c854dab3ddf0a0521f59390aa8871fae Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 20 Nov 2012 13:02:26 +0100
Subject: [PATCH] Deallocate LDAP connections in safe way.
Current code does isc_mem_detach() before isc_mem_put() so
memory context reference counter is wrong for a small time frame.
Macro MEM_PUT_AND_DETACH will handle it properly including
pointer zeroing.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
84ac06be7c7d4451e59844dd7c447f0f3d557f61..ef0fb69413d0febe8334bb5156d879fa1f0b9f16
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -258,8 +258,7 @@ struct ldap_psearchevent {
/* TODO: reorganize this stuff clean it up. */
static isc_result_t new_ldap_connection(ldap_pool_t *pool,
ldap_connection_t **ldap_connp);
-static void destroy_ldap_connection(ldap_pool_t *pool,
- ldap_connection_t **ldap_connp);
+static void destroy_ldap_connection(ldap_connection_t **ldap_connp);
static isc_result_t findrdatatype_or_create(isc_mem_t *mctx,
ldapdb_rdatalist_t *rdatalist, dns_rdataclass_t rdclass,
@@ -706,13 +705,13 @@ new_ldap_connection(ldap_pool_t *pool,
ldap_connection_t **ldap_connp)
return ISC_R_SUCCESS;
cleanup:
- destroy_ldap_connection(pool, ldap_conn);
+ destroy_ldap_connection(ldap_conn);
return result;
}
static void
-destroy_ldap_connection(ldap_pool_t *pool, ldap_connection_t **ldap_connp)
+destroy_ldap_connection(ldap_connection_t **ldap_connp)
{
ldap_connection_t *ldap_conn;
@@ -730,10 +729,7 @@ destroy_ldap_connection(ldap_pool_t *pool,
ldap_connection_t **ldap_connp)
ldap_control_free(ldap_conn-serverctrls[0]);
}
- isc_mem_detach(ldap_conn-mctx);
-
- isc_mem_put(pool-mctx, *ldap_connp, sizeof(ldap_connection_t));
- *ldap_connp = NULL;
+ MEM_PUT_AND_DETACH(*ldap_connp);
}
/*
@@ -3028,7 +3024,7 @@ ldap_pool_destroy(ldap_pool_t **poolp)
for (i = 0; i pool-connections; i++) {
ldap_conn = pool-conns[i];
if (ldap_conn != NULL)
- destroy_ldap_connection(pool, ldap_conn);
+ destroy_ldap_connection(ldap_conn);
}
SAFE_MEM_PUT(pool-mctx, pool-conns,
@@ -3109,7 +3105,7 @@ ldap_pool_connect(ldap_pool_t *pool, ldap_instance_t
*ldap_inst)
cleanup:
for (i = 0; i pool-connections; i++) {
- destroy_ldap_connection(pool, pool-conns[i]);
+ destroy_ldap_connection(pool-conns[i]);
}
return result;
}
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0098] Log failures detected in CHECK() macro
cleanup; \ + } \ } while (0) #define CHECKED_MEM_ALLOCATE(m, target_ptr, s) \ -- 1.7.11.7 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0094] Use memory allocation macros more extensively - part 1
On Tue, Nov 20, 2012 at 10:18:09AM +0100, Petr Spacek wrote:
Hello,
Use memory allocation macros more extensively.
Some scattered occurences of isc_mem_* functions was replaced by macros.
Destroy_ldap_instance() now accepts NULL LDAP instance.
String functions from str.c are still calling isc_mem_directly.
This patch contain small subset of changes which affect
new_ldap_instance() and destroy_ldap_instance() calls. Remaining
changes do not require changes in new_*/destroy_* logic and will
follow in separate patch.
Ack
From 47605bb5c605729224f43a01fcef94c93b7166d3 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 16 Nov 2012 15:34:39 +0100
Subject: [PATCH] Use memory allocation macros more extensively.
Some scattered occurences of isc_mem_* functions was replaced by macros.
Destroy_ldap_instance() now accepts NULL LDAP instance.
String functions from str.c are still calling isc_mem_directly.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 21 -
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
b36892b4e8180fb2a5f335e3fa1b5589dae8bf14..904bae5259cccd9ce5cfd27deead3a0d23864f84
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -372,13 +372,10 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
REQUIRE(ldap_instp != NULL *ldap_instp == NULL);
- ldap_inst = isc_mem_get(mctx, sizeof(ldap_instance_t));
- if (ldap_inst == NULL)
- return ISC_R_NOMEMORY;
-
+ CHECKED_MEM_GET_PTR(mctx, ldap_inst);
ZERO_PTR(ldap_inst);
-
isc_mem_attach(mctx, ldap_inst-mctx);
+
ldap_inst-db_name = db_name;
view = dns_dyndb_get_view(dyndb_args);
dns_view_attach(view, ldap_inst-view);
@@ -511,9 +508,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
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);
+ CHECKED_MEM_GET_PTR(mctx, new_addr);
*new_addr = *addr;
ISC_LINK_INIT(new_addr, link);
ISC_LIST_APPEND(ldap_inst-orig_global_forwarders.addrs,
@@ -561,9 +556,12 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
const char *db_name;
isc_sockaddr_t *addr;
- REQUIRE(ldap_instp != NULL *ldap_instp != NULL);
+ REQUIRE(ldap_instp != NULL);
ldap_inst = *ldap_instp;
+ if (ldap_inst == NULL)
+ return;
+
db_name = ldap_inst-db_name; /* points to DB instance: outside
ldap_inst */
/*
@@ -690,10 +688,7 @@ new_ldap_connection(ldap_pool_t *pool, ldap_connection_t
**ldap_connp)
REQUIRE(pool != NULL);
REQUIRE(ldap_connp != NULL *ldap_connp == NULL);
- ldap_conn = isc_mem_get(pool-mctx, sizeof(ldap_connection_t));
- if (ldap_conn == NULL)
- return ISC_R_NOMEMORY;
-
+ CHECKED_MEM_GET_PTR(pool-mctx, ldap_conn);
ZERO_PTR(ldap_conn);
result = isc_mutex_init(ldap_conn-lock);
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0103] Remove dead assignments in manager_create_db_instance() and make log messages verbose
On Thu, Nov 22, 2012 at 10:28:27AM +0100, Petr Spacek wrote:
Hello,
Remove dead assignments in manager_create_db_instance() and make
log messages verbose.
Dead assignments were reported by Clang static code analysis.
Ack
From 20b87c908c3ae2c05c10f0ee72fe17f34ff2b35b Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 22 Nov 2012 10:03:21 +0100
Subject: [PATCH] Remove dead assignments in manager_create_db_instance() and
make log messages verbose.
Dead assignments were reported by Clang static code analysis.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/zone_manager.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/src/zone_manager.c b/src/zone_manager.c
index
2988cfd0598de57efbc4f7a42447ad1dadce6d65..08ef91907a35564520b8ccb8d9993b49fc88a391
100644
--- a/src/zone_manager.c
+++ b/src/zone_manager.c
@@ -132,11 +132,8 @@ manager_create_db_instance(isc_mem_t *mctx, const char
*name,
result = find_db_instance(name, db_inst);
if (result == ISC_R_SUCCESS) {
db_inst = NULL;
- result = ISC_R_FAILURE;
- log_error('%s' already exists, name);
- goto cleanup;
- } else {
- result = ISC_R_SUCCESS;
+ log_error(LDAP instance '%s' already exists, name);
+ CLEANUP_WITH(ISC_R_EXISTS);
}
/* Parse settings. */
@@ -184,8 +181,7 @@ manager_create_db_instance(isc_mem_t *mctx, const char
*name,
if (result != ISC_R_SUCCESS) {
/* In case we don't find any zones, we at least return
* ISC_R_SUCCESS so BIND won't exit because of this. */
- result = ISC_R_SUCCESS;
- log_error(no valid zones found);
+ log_error_r(no valid zones found in LDAP);
/*
* Do not jump to cleanup. Rather start timer for zone refresh.
* This is just a workaround when the LDAP server is not
available
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0104] Remove dead assignment and add logging to ldap_pool_connect()
On Thu, Nov 22, 2012 at 11:01:47AM +0100, Petr Spacek wrote:
One day I will forgot my head somewhere ... the patch is attached.
On 11/22/2012 10:57 AM, Petr Spacek wrote:
Hello,
Remove dead assignment and add logging to ldap_pool_connect().
Dead assignment was reported by Clang static code analysis.
--
Petr^2 Spacek
Ack
From 0a10518a20e5ef0f0a8767d47d3306644cf2c636 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 22 Nov 2012 10:48:45 +0100
Subject: [PATCH] Remove dead assignment and add logging to
ldap_pool_connect().
Dead assignment was reported by Clang static code analysis.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
407875a4f7e5952272e496ed70b18b772172bfb3..e537d9b3b76c6a7144ee7df1ddda26ed95b4ab0d
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -3094,18 +3094,18 @@ ldap_pool_connect(ldap_pool_t *pool, ldap_instance_t
*ldap_inst)
ldap_conn = NULL;
CHECK(new_ldap_connection(pool, ldap_conn));
result = ldap_connect(ldap_inst, ldap_conn, ISC_FALSE);
- if (result == ISC_R_NOTCONNECTED || result == ISC_R_TIMEDOUT) {
- /* LDAP server is down which can happen, continue */
- result = ISC_R_SUCCESS;
- } else if (result != ISC_R_SUCCESS) {
+ /* Continue even if LDAP server is down */
+ if (result != ISC_R_NOTCONNECTED result != ISC_R_TIMEDOUT
+ result != ISC_R_SUCCESS) {
goto cleanup;
}
pool-conns[i] = ldap_conn;
}
return ISC_R_SUCCESS;
cleanup:
+ log_error_r(couldn't establish connection in LDAP connection pool);
for (i = 0; i pool-connections; i++) {
destroy_ldap_connection(pool-conns[i]);
}
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0105] Remove dead assignment in ldap_parse_zoneentry()
On Thu, Nov 22, 2012 at 12:30:00PM +0100, Petr Spacek wrote: Hello, this patch fixes the last warning from Clang. Any new warning should attract our attention. Commit message: Remove dead assignment in ldap_parse_zoneentry(). Assigned result value was never read because it is overwritten by following CHECK()s in any case. Dead assignment was reported by Clang static code analysis. -- Petr^2 Spacek Ack From 850efaf7a1fce473216d5c0e07d5738c9819b01d Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Thu, 22 Nov 2012 12:23:15 +0100 Subject: [PATCH] Remove dead assignment in ldap_parse_zoneentry(). Assigned result value was never read because it is overwritten by following CHECK()s in any case. Dead assignment was reported by Clang static code analysis. Signed-off-by: Petr Spacek pspa...@redhat.com --- src/ldap_helper.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ldap_helper.c b/src/ldap_helper.c index e537d9b3b76c6a7144ee7df1ddda26ed95b4ab0d..57235de7da158b7a05de9e35fff2cdaaff74276c 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -1334,7 +1334,6 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst) 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) -- 1.7.11.7 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0102] Fix error reporting for different TTLs in findrdatatype_or_create()
On Thu, Nov 22, 2012 at 09:36:34AM +0100, Petr Spacek wrote:
Hello,
Fix error reporting for different TTLs in findrdatatype_or_create().
Old code returned ISC_R_SUCCESS even in unsupported case.
Bug found by Clang static code analysis.
--
Petr^2 Spacek
Ack
From db7de384c4f047555275c164fc9dbe0a5d9269c6 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 22 Nov 2012 09:23:45 +0100
Subject: [PATCH] Fix error reporting for different TTLs in
findrdatatype_or_create().
Old code returned ISC_R_SUCCESS even in unsupported case.
Bug found by Clang static code analysis.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
8a7765d5a650279db0417f6fb5be01a3dd2b82d4..8d23b6251b77a63cb22c484663a1169c0b1a31e3
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1600,13 +1600,15 @@ findrdatatype_or_create(isc_mem_t *mctx,
ldapdb_rdatalist_t *rdatalist,
rdlist-type = rdtype;
rdlist-ttl = ttl;
APPEND(*rdatalist, rdlist, link);
- result = ISC_R_SUCCESS;
} else {
/*
* No support for different TTLs yet.
*/
- if (rdlist-ttl != ttl)
- result = ISC_R_FAILURE;
+ if (rdlist-ttl != ttl) {
+ log_error(different TTLs in single rdata list
+ are not supported);
+ CLEANUP_WITH(ISC_R_NOTIMPLEMENTED);
+ }
}
*rdlistp = rdlist;
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 100] Add macro for LDAP error logging
On Wed, Nov 21, 2012 at 04:00:24PM +0100, Petr Spacek wrote:
Hello,
Add macro for LDAP error logging.
--
Petr^2 Spacek
Ack
From 1d2c1d3b5024beca92417c0dc65cbfc0f4ddff25 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 21 Nov 2012 15:52:39 +0100
Subject: [PATCH] Add macro for LDAP error logging.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/log.h | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/log.h b/src/log.h
index
d6a40151d25b6f67cf6735ec955d45e4ebe4106c..312f24322fd0c6f9943c6beb810ac0bcd8f3896c
100644
--- a/src/log.h
+++ b/src/log.h
@@ -54,6 +54,17 @@
#define log_debug(level, format, ...)\
log_write(GET_LOG_LEVEL(level), format, ##__VA_ARGS__)
+/* LDAP logging functions */
+#define log_ldap_error(ld) \
+ do {\
+ int err;\
+ char *errmsg = UNKNOWN; \
+ if (ldap_get_option(ld, LDAP_OPT_RESULT_CODE, err) \
+ == LDAP_OPT_SUCCESS)\
+ errmsg = ldap_err2string(err); \
+ log_error_position(LDAP error: %s, errmsg); \
+ } while (0);\
+
void
log_write(int level, const char *format, ...) ISC_FORMAT_PRINTF(2, 3);
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0093] Log memory allocation failures
) { \
result = ISC_R_NOMEMORY;\
+ log_error_position(MEMORY ALLOCATION FAILURE);
\
goto cleanup; \
} \
} while (0)
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0087] Unload master zone if forwarders are specified
(inst, pevent-prevdn,
+ ISC_TRUE, ISC_FALSE));
} else {
log_debug(5, update_action: old zone wasn't
managed
by plugin, dn '%s',
pevent-prevdn);
@@ -3274,7 +3286,7 @@ update_zone(isc_task_t *task, isc_event_t *event)
INSIST(NEXT(entry_zone, link) == NULL); /* no multiple zones
with same DN */
} else {
- CHECK(ldap_delete_zone(inst, pevent-dn, ISC_TRUE));
+ CHECK(ldap_delete_zone(inst, pevent-dn, ISC_TRUE, ISC_FALSE));
}
cleanup:
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0088] Flush BIND caches if forwarder configuration was changed
,
- msg_obj_type, dn);
}
cleanup:
@@ -1106,11 +1144,19 @@ cleanup:
}
}
if (fwdtbl_deletion_requested) {
- isc_result_t orig_result = result;
+ orig_result = result;
result = delete_forwarding_table(inst, name, msg_obj_type, dn);
if (result == ISC_R_SUCCESS)
result = orig_result;
}
+ if (fwdtbl_deletion_requested || fwdtbl_update_requested) {
+ log_debug(5, %s '%s': forwarder table was updated: %s,
+ msg_obj_type, dn, dns_result_totext(result));
+ orig_result = result;
+ result = dns_view_flushcache(inst-view);
+ if (result == ISC_R_SUCCESS)
+ result = orig_result;
+ }
return result;
}
@@ -1133,7 +1179,7 @@ ldap_parse_configentry(ldap_entry_t *entry,
ldap_instance_t *inst)
/* idnsForwardPolicy change is handled by configure_zone_forwarders() */
result = configure_zone_forwarders(entry, inst, dns_rootname);
if (result != ISC_R_SUCCESS result != ISC_R_DISABLED) {
- log_error(Global forwarder could not be set up.);
+ log_error_r(global forwarder could not be set up);
}
result = ldap_entry_getvalues(entry, idnsAllowSyncPTR, values);
@@ -1228,11 +1274,6 @@ ldap_parse_zoneentry(ldap_entry_t *entry,
ldap_instance_t *inst)
* = delete master zone */
CHECK(ldap_delete_zone2(inst, name, ISC_FALSE,
ISC_TRUE));
-#if LIBDNS_VERSION_MAJOR 90
- result = dns_view_flushcache(inst-view);
-#else
- result = dns_view_flushnode(inst-view, name,
ISC_TRUE);
-#endif
}
/* DO NOT CHANGE ANYTHING ELSE after forwarders are set up! */
goto cleanup;
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0086] Respect global forwarders from named.conf if they are not overridden by LDAP configuration
);
+ 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.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0084] Restore compatibility with BIND 9.8
On Wed, Oct 31, 2012 at 10:58:10AM +0100, Petr Spacek wrote:
Hello,
this patch restores compatibility with BIND 9.8.
Ack
From f06e5e21025a1187dfe223682e99bfb8c8193be3 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 31 Oct 2012 10:57:01 +0100
Subject: [PATCH] Restore compatibility with BIND 9.8.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
NEWS | 4
src/ldap_helper.c | 7 ++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/NEWS b/NEWS
index
714c58b940c8c22eca44d9bd0c6fe3f3156fed53..e0d6abfafc0e62eb48789c66345fe9c77caa5591
100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,7 @@
+2.2
+==
+[1] Compatibility with BIND 9.8 was restored.
+
2.1
==
[1] Forward policy none was introduced.
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
b7be970202f625dd43422c3d37695fbf9303e27b..5d7138b1861a85ad8f3d8027e94f4f807355f48a
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1120,8 +1120,13 @@ ldap_parse_zoneentry(ldap_entry_t *entry,
ldap_instance_t *inst)
*/
result = configure_zone_forwarders(entry, inst, name);
if (result != ISC_R_DISABLED) {
- if (result == ISC_R_SUCCESS)
+ if (result == ISC_R_SUCCESS) {
+#if LIBDNS_VERSION_MAJOR 90
+ result = dns_view_flushcache(inst-view);
+#else
result = dns_view_flushnode(inst-view, name,
ISC_TRUE);
+#endif
+ }
/* DO NOT CHANGE ANYTHING ELSE after forwarders are set up! */
goto cleanup;
}
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0085] Bump NVR to 2.2
On Wed, Oct 31, 2012 at 11:04:19AM +0100, Petr Spacek wrote:
Hello,
Bump NVR to 2.2.
Ack
From 249b46aefeee0253fd8c287e0e52f508d9130a00 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 31 Oct 2012 11:02:35 +0100
Subject: [PATCH] Bump NVR to 2.2.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
configure.ac | 2 +-
contrib/bind-dyndb-ldap.spec | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index
b989d0c0e1e9c1941c806bac84135e45e06a23f5..bedc65289fc5a0437b9371ceb6fef03065100d7f
100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
AC_PREREQ([2.59])
-AC_INIT([bind-dyndb-ldap], [2.1], [freeipa-devel@redhat.com])
+AC_INIT([bind-dyndb-ldap], [2.2], [freeipa-devel@redhat.com])
AM_INIT_AUTOMAKE([-Wall foreign dist-bzip2])
diff --git a/contrib/bind-dyndb-ldap.spec b/contrib/bind-dyndb-ldap.spec
index
fdcdfde564eebce4d1db6a79d24967ee6d974fb9..8e9e6af029f99d056f4fe5e3d6afe0d365bf5c5f
100644
--- a/contrib/bind-dyndb-ldap.spec
+++ b/contrib/bind-dyndb-ldap.spec
@@ -1,7 +1,7 @@
%define VERSION %{version}
Name: bind-dyndb-ldap
-Version:2.1
+Version:2.2
Release:0%{?dist}
Summary:LDAP back-end plug-in for BIND
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0081] Add forward policy none
,
+ dn, msg_use_global_fwds);
+ return ISC_R_UNEXPECTEDTOKEN;
+ } else if (fwdpolicy == dns_fwdpolicy_none) {
+ log_debug(5, zone '%s': forwarding explicitly disabled
+ (policy 'none', ignoring global forwarders), dn);
}
- if (ISC_LIST_EMPTY(addrs)) {
- log_debug(5, No forwarders seen; disabling forwarding);
- fwdpolicy = dns_fwdpolicy_none;
- }
-
- log_debug(5, Forward policy: %d, fwdpolicy);
-
/* Set forward table up. */
result = dns_fwdtable_add(inst-view-fwdtable, name, addrs,
fwdpolicy);
@@ -951,6 +998,12 @@ configure_zone_forwarders(ldap_entry_t *entry,
ldap_instance_t *inst,
isc_mem_put(inst-mctx, addr, sizeof(*addr));
}
+ 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;
+
return result;
}
@@ -971,13 +1024,9 @@ ldap_parse_configentry(ldap_entry_t *entry,
ldap_instance_t *inst)
log_debug(3, Parsing configuration object);
/* idnsForwardPolicy change is handled by configure_zone_forwarders() */
- result = ldap_entry_getvalues(entry, idnsForwarders, values);
- if (result == ISC_R_SUCCESS) {
- log_debug(2, Setting global forwarders);
- result = configure_zone_forwarders(entry, inst, dns_rootname,
values);
- if (result != ISC_R_SUCCESS) {
- log_error(Global forwarder could not be set up.);
- }
+ result = configure_zone_forwarders(entry, inst, dns_rootname);
+ if (result != ISC_R_SUCCESS result != ISC_R_DISABLED) {
+ log_error(Global forwarder could not be set up.);
}
result = ldap_entry_getvalues(entry, idnsAllowSyncPTR, values);
@@ -1060,21 +1109,17 @@ ldap_parse_zoneentry(ldap_entry_t *entry,
ldap_instance_t *inst)
CHECK(discard_from_cache(inst-cache, name));
- /*
- * 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));
+ result = configure_zone_forwarders(entry, inst, name);
+ if (result != ISC_R_DISABLED) {
/* DO NOT CHANGE ANYTHING ELSE after forwarders are set up! */
goto cleanup;
- } else {
- /* No forwarders are used. Remove zone from fwdtable. */
- CHECK(configure_zone_forwarders(entry, inst, name, values));
}
+ /* No forwarders are used. Zone was removed from fwdtable.
+ * Load the zone. */
/* Check if we are already serving given zone */
result = zr_get_zone_ptr(inst-zone_register, name, zone);
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0082] Disable forwarding when forward zones are removed
On Mon, Oct 29, 2012 at 02:24:48PM +0100, Petr Spacek wrote:
On 10/26/2012 03:33 PM, Petr Spacek wrote:
Hello,
Disable forwarding when forward zones are removed.
https://fedorahosted.org/bind-dyndb-ldap/ticket/96
Hmm ... Apparently, I was sleeping on Friday :-)
There is a promised patch.
Ack
From d1936f0fbd62aedc8b4c4d21aea44df7d2a11fa4 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 26 Oct 2012 15:19:07 +0200
Subject: [PATCH] Disable forwarding when forward zones are removed.
https://fedorahosted.org/bind-dyndb-ldap/ticket/96
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
dce1943484cfeb1519ead67c28c9377b155c275c..19470d557afe5d44eba3d8b37502d296ccfd67b1
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -812,11 +812,16 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t
*name, isc_boolean_t lock)
result == ISC_R_LOCKBUSY);
if (result == ISC_R_SUCCESS)
unlock = ISC_TRUE;
-
- /* TODO: flush cache records belonging to deleted zone */
- CHECK(discard_from_cache(inst-cache, name));
}
+ /* Disable forwarding. */
+ result = dns_fwdtable_delete(inst-view-fwdtable, name);
+ if (result != ISC_R_SUCCESS result != ISC_R_NOTFOUND)
+ log_error_r(zone '%s': failed to delete forwarders,
zone_name_char);
+
+ /* TODO: flush cache records belonging to deleted zone */
+ CHECK(discard_from_cache(inst-cache, name));
+
result = zr_get_zone_ptr(inst-zone_register, name, zone);
if (result == ISC_R_NOTFOUND || result == DNS_R_PARTIALMATCH) {
log_debug(1, zone '%s' not found in zone register,
zone_name_char);
--
1.7.11.7
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0083] Flush cache after creating a new forward zone
On Fri, Oct 26, 2012 at 04:03:08PM +0200, Petr Spacek wrote:
On 10/26/2012 03:55 PM, Martin Kosek wrote:
On 10/26/2012 03:53 PM, Petr Spacek wrote:
Hello,
Flush cache after creating a new forward zone.
https://fedorahosted.org/bind-dyndb-ldap/ticket/97
I guess the patch is missing :-)
Martin
Hmm, apparently two-lines wasn't worth for Thunderbird to attach the file :-D
Ack
From 72110d81b3c3325f9cb68ad2c345272f9a9e9cec Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 26 Oct 2012 15:52:02 +0200
Subject: [PATCH] Flush cache after creating a new forward zone.
https://fedorahosted.org/bind-dyndb-ldap/ticket/97
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
19470d557afe5d44eba3d8b37502d296ccfd67b1..37281456bf136bc050828331383932bae7d67c81
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1120,6 +1120,8 @@ ldap_parse_zoneentry(ldap_entry_t *entry,
ldap_instance_t *inst)
*/
result = configure_zone_forwarders(entry, inst, name);
if (result != ISC_R_DISABLED) {
+ if (result == ISC_R_SUCCESS)
+ result = dns_view_flushnode(inst-view, name,
ISC_TRUE);
/* DO NOT CHANGE ANYTHING ELSE after forwarders are set up! */
goto cleanup;
}
--
1.7.11.7
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0080] Prevent false 'zone serial (2012060301) unchanged' error messages
On Mon, Oct 22, 2012 at 04:18:19PM +0200, Petr Spacek wrote: Hello, this patch prevents false 'zone serial (2012060301) unchanged' error messages coming from zone_postload(), which is called after each zone change from dns_zone_load(). I found zone_postload() unnecessary for our plugin except initial load. Adam, please, check that information, I'm not 100 % sure about BIND internals. I examined only zone_load() and zone_postload() functions and I'm not sure about consequences in other parts of BIND. Attached patch creates empty SSU table which is enough to pass dns_zone_isdynamic() check - as a result zone_postload() is not called for our zones anymore. This patch closes https://fedorahosted.org/bind-dyndb-ldap/ticket/79 Ack From 8aaf2edbf7cc57b61ee48f649d23bf5ef575f5dc Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Mon, 22 Oct 2012 16:07:32 +0200 Subject: [PATCH] Prevent false 'zone serial (2012060301) unchanged' error messages. This patch prevents zone_postload() calls for all zones managed by bind-dyndb-ldap. https://fedorahosted.org/bind-dyndb-ldap/ticket/79 Signed-off-by: Petr Spacek pspa...@redhat.com --- src/ldap_helper.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/ldap_helper.c b/src/ldap_helper.c index ca08afcfdd9b68cd19997f1b263674bc90c89b20..8534362ae119e51931af375658bc99d8e88a 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -1092,7 +1092,13 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst) if (result == ISC_R_SUCCESS) CHECK(configure_zone_ssutable(zone, HEAD(values)-value)); else - CHECK(configure_zone_ssutable(zone, NULL)); + /* We need to declare zone as 'dynamic' + * for dns_zone_isdynamic() to prevent unwanted + * zone_postload() calls and warnings about serial and so on. + * + * Created SSU table contains no rules = + * dns_ssutable_checkrules() will return deny. */ + CHECK(configure_zone_ssutable(zone, )); /* Fetch allow-query and allow-transfer ACLs */ log_debug(2, Setting allow-query for %p: %s, zone, dn); -- 1.7.11.7 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] Hide private symbols in the bind-dyndb-ldap
Hello,
attached patch hides all symbols except dynamic_driver_{init,destroy}. Feedback
is appreciated.
Regards, Adam
--
Adam Tkac, Red Hat, Inc.
From 126929489baf4f69fe0444860776f7e76c1411f2 Mon Sep 17 00:00:00 2001
From: Adam Tkac von...@gmail.com
Date: Wed, 17 Oct 2012 13:00:31 +0200
Subject: [PATCH] Hide all symbols except dynamic_driver_{init,destroy}
Signed-off-by: Adam Tkac von...@gmail.com
---
configure.ac | 15 +++
src/ldap_driver.c | 10 --
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index cff5526..30a7374 100644
--- a/configure.ac
+++ b/configure.ac
@@ -30,6 +30,21 @@ AC_TYPE_SIZE_T
# Checks for library functions.
AC_CHECK_FUNCS([memset strcasecmp strncasecmp])
+# Check if build chain supports symbol visibility
+AC_MSG_CHECKING([for -fvisibility=hidden compiler flag])
+SAVED_CFLAGS=$CFLAGS
+CFLAGS=$CFLAGS -fvisibility=hidden
+AC_TRY_COMPILE([
+ extern __attribute__((__visibility__(hidden))) int hidden;
+ extern __attribute__((__visibility__(default))) int def;
+ extern __attribute__((__visibility__(hidden))) int fhidden(void);
+ extern __attribute__((__visibility__(default))) int fdef(void);
+],[],
+[AC_MSG_RESULT([yes])
+ AC_DEFINE([HAVE_VISIBILITY], 1, [Define if compiler supports -fvisibility])],
+[CFLAGS=$SAVED_CFLAGS
+ AC_MSG_RESULT([no])])
+
# Check version of libdns
AC_MSG_CHECKING([libdns version])
AC_TRY_RUN([
diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index 3a80223..99a8421 100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -52,6 +52,12 @@
#include util.h
#include zone_manager.h
+#ifdef HAVE_VISIBILITY
+#define VISIBLE __attribute__((__visibility__(default)))
+#else
+#define VISIBLE
+#endif
+
#define LDAPDB_MAGIC ISC_MAGIC('L', 'D', 'P', 'D')
#define VALID_LDAPDB(ldapdb) \
((ldapdb) != NULL (ldapdb)-common.impmagic == LDAPDB_MAGIC)
@@ -1315,7 +1321,7 @@ static dns_dbimplementation_t *ldapdb_imp;
const char *ldapdb_impname = dynamic-ldap;
-isc_result_t
+VISIBLE isc_result_t
dynamic_driver_init(isc_mem_t *mctx, const char *name, const char * const
*argv,
dns_dyndb_arguments_t *dyndb_args)
{
@@ -1360,7 +1366,7 @@ dynamic_driver_init(isc_mem_t *mctx, const char *name,
const char * const *argv,
return result;
}
-void
+VISIBLE void
dynamic_driver_destroy(void)
{
/* Only unregister the implementation if it was registered by us. */
--
1.7.11.7
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Hide private symbols in the bind-dyndb-ldap
On Wed, Oct 17, 2012 at 09:58:36AM -0400, Simo Sorce wrote:
On Wed, 2012-10-17 at 13:04 +0200, Adam Tkac wrote:
Hello,
attached patch hides all symbols except dynamic_driver_{init,destroy}.
Feedback
is appreciated.
Any reason not to use a simple export file ?
This is also possible solution. However if I understand GNU build chain
correctly, using export file only affects linker and doesn't allow compiler to
perform more aggressive optimisations (i.e. inline hidden functions etc), does
it?
Anyway strong ACK, keeping private symbols private is good hygiene.
Thanks, pushed to master.
Regards, Adam
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] broken bind-dyndb-ldap in ipa-devel repo
On Tue, Oct 16, 2012 at 11:46:17AM +0300, Alexander Bokovoy wrote: Hi, It looks like bind has moved its ABI in F17 updates-testing: $ rpm -ql bind-libs /usr/lib64/libbind9.so.90 /usr/lib64/libbind9.so.90.0.5 /usr/lib64/libdns.so.95 /usr/lib64/libdns.so.95.1.1 /usr/lib64/libisc.so.92 /usr/lib64/libisc.so.92.1.0 /usr/lib64/libisccc.so.90 /usr/lib64/libisccc.so.90.0.2 /usr/lib64/libisccfg.so.90 /usr/lib64/libisccfg.so.90.0.2 /usr/lib64/liblwres.so.90 /usr/lib64/liblwres.so.90.0.1 Note libdns.so.95 -- this does not play well with bind-dyndb-ldap in ipa-devel repository as that one is built against libdns.so.93. Yum then is unable to install the packages as the only repo where bind 9.9.2 is available is updates-testing. Petr, Adam, could any of you rebuild bind-dyndb-ldap in ipa-devel repo against bind in updates-testing for F17? Without this rebuild managed DNS is not possible to use in F17+ipa-devel. Hi Alexander, I'm not sure if I have permissions to rebuild pkgs in ipa-devel repo, I've neved did it before. Is there some manual how to do it? Thanks in advance. Regards, Adam -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0079] Update NEWS file for 2.0 release
On Mon, Oct 15, 2012 at 10:38:41AM +0200, Petr Spacek wrote: Hello, Update NEWS file for 2.0 release Hi Peter, are you OK with this version of NEWS? (patch attached) A -- Adam Tkac, Red Hat, Inc. From bcd017c75978e2f78976bb8a2b6d47af26df429a Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Mon, 15 Oct 2012 10:37:01 +0200 Subject: [PATCH] Update NEWS file for 2.0 release. Signed-off-by: Petr Spacek pspa...@redhat.com Signed-off-by: Adam Tkac at...@redhat.com --- NEWS | 34 ++ 1 file changed, 34 insertions(+) diff --git a/NEWS b/NEWS index 02e4845..4312741 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,37 @@ +2.0 +== +[1] SOA serial number can be incremented automatically after each change +in LDAP database. (Configuration option serial_autoincrement.) + +[2] It was possible to DoS named service via quiery which contained +$ character. CVE-2012-3429 was fixed. + +[3] DNS Dynamic Update returns codes NOTAUTH and REFUSED properly. + +[4] BIND doesn't refuse to start if initial connection times out. + +[5] Object renaming (LDAP moddn) in persistent mode is handled properly. + +[6] Internal record cache is flushed properly after reconnection +to the LDAP server (in configurations with persistent search). + +[7] Simple time-based deadlock detection code was added. Error message +is printed after 10*(timeout) seconds. +Some deadlocks in various situations with low connection count were fixed. + +[8] Libdns interface version = 90 is supported properly. + +[9] Zone transfers were fixed. Records with non-FQDNs are handled properly. + +[10] Logging was improved. + +[11] Memory leaks in dynamic update, persistent search, ldap_query + and configurations with multiple plugin instances were fixed. + +[12] Version numbering format changed to: [features].[bugfixes] + +[13] Many other bugfixes + 1.1.0rc1 == -- 1.7.11.7 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Fix zone removal in persistent search update_zone()
On Fri, Oct 05, 2012 at 01:15:13PM +0200, Petr Spacek wrote:
Hello,
Fix zone removal in persistent search update_zone().
Without this patch any zone removed through ipa dnszone-del will
remain active and will return SERVFAILs.
Ack
From e09eebf3c370ff4106013cdeda10a80782e26611 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 5 Oct 2012 13:02:37 +0200
Subject: [PATCH] Fix zone removal in persistent search update_zone() handler.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
68b635f84b4c9015752667510c0497e5f56bc7ab..199b345bb604c30bfa8a3690afc844ca8b264e89
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -3056,7 +3056,6 @@ update_zone(isc_task_t *task, isc_event_t *event)
ldap_qresult_t *ldap_qresult_record = NULL;
ldap_entry_t *entry_zone = NULL;
ldap_entry_t *entry_record = NULL;
- isc_boolean_t delete = ISC_TRUE;
isc_mem_t *mctx;
dns_name_t prevname;
char *attrs_zone[] = {
@@ -3073,14 +3072,16 @@ update_zone(isc_task_t *task, isc_event_t *event)
dns_name_init(prevname, NULL);
CHECK(manager_get_ldap_instance(pevent-dbname, inst));
- CHECK(ldap_query(inst, NULL, ldap_qresult_zone, pevent-dn,
+
+ result = ldap_query(inst, NULL, ldap_qresult_zone, pevent-dn,
LDAP_SCOPE_BASE, attrs_zone, 0,
- ((objectClass=idnsZone)(idnsZoneActive=TRUE;
+ ((objectClass=idnsZone)(idnsZoneActive=TRUE)));
+ if (result != ISC_R_SUCCESS result != ISC_R_NOTFOUND)
+ CLEANUP_WITH(result);
- for (entry_zone = HEAD(ldap_qresult_zone-ldap_entries);
- entry_zone != NULL;
- entry_zone = NEXT(entry_zone, link)) {
- delete = ISC_FALSE;
+ if (result == ISC_R_SUCCESS
+ HEAD(ldap_qresult_zone-ldap_entries) != NULL) {
+ entry_zone = HEAD(ldap_qresult_zone-ldap_entries);
CHECK(ldap_parse_zoneentry(entry_zone, inst));
if (PSEARCH_MODDN(pevent-chgtype)) {
@@ -3106,10 +3107,9 @@ update_zone(isc_task_t *task, isc_event_t *event)
}
INSIST(NEXT(entry_zone, link) == NULL); /* no multiple zones
with same DN */
- }
-
- if (delete)
+ } else {
CHECK(ldap_delete_zone(inst, pevent-dn, ISC_TRUE));
+ }
cleanup:
if (result != ISC_R_SUCCESS)
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0075] Prevent misleading partial match error messages for disabled zones
On Fri, Oct 05, 2012 at 01:45:42PM +0200, Petr Spacek wrote:
Hello,
Prevent misleading partial match error messages for disabled zones.
Following message was printed if zone e.test was disabled and LDAP
contained zones test and e.test:
update_zone (psearch) failed for
'idnsName=e.test,cn=dns,dc=e,dc=org'. Zones can be outdated, run
`rndc reload`: partial match
Ack
From e7f7a7d6e4fb3c9e47bac22d3291d09aa3d885ab Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 5 Oct 2012 13:37:18 +0200
Subject: [PATCH] Prevent misleading partial match error messages for
disabled zones.
Following message was printed if zone e.test was disabled and LDAP
contained zones test and e.test:
update_zone (psearch) failed for 'idnsName=e.test,cn=dns,dc=e,dc=org'. Zones
can be outdated, run `rndc reload`: partial match
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
199b345bb604c30bfa8a3690afc844ca8b264e89..d4bb6db10b0e79f8777fde3c5f344298af87ce56
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -815,10 +815,9 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t
*name, isc_boolean_t lock)
}
result = zr_get_zone_ptr(inst-zone_register, name, zone);
- if (result == ISC_R_NOTFOUND) {
+ if (result == ISC_R_NOTFOUND || result == DNS_R_PARTIALMATCH) {
log_debug(1, zone '%s' not found in zone register,
zone_name_char);
- result = ISC_R_SUCCESS;
- goto cleanup;
+ CLEANUP_WITH(ISC_R_SUCCESS);
} else if (result != ISC_R_SUCCESS)
goto cleanup;
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0071] Fix potential crash caused by failing zone_register allocation.
On Tue, Oct 02, 2012 at 03:21:08PM +0200, Petr Spacek wrote:
Hello,
Fix potential crash caused by failing zone_register allocation.
Problematic call flow:
new_ldap_instance - zr_create (returns failure) -
destroy_ldap_instance - zr_get_rbt (*crash*)
Ack
From 9d96a9c4a4ac5b592ed5874132e0618b1b259de0 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 2 Oct 2012 15:16:27 +0200
Subject: [PATCH] Fix potential crash caused by failing zone_register
allocation.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
38d86afa521dcf0e6b58ebb38635ff0fffbedd2a..629c76732c86af2a614e589a5afff18136068a66
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -516,7 +516,7 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
{
ldap_instance_t *ldap_inst;
dns_rbtnodechain_t chain;
- dns_rbt_t *rbt;
+ dns_rbt_t *rbt = NULL;
isc_result_t result = ISC_R_SUCCESS;
const char *db_name;
@@ -530,7 +530,10 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
*
* NOTE: This should be probably done in zone_register.c
*/
- rbt = zr_get_rbt(ldap_inst-zone_register);
+ if (ldap_inst-zone_register != NULL)
+ rbt = zr_get_rbt(ldap_inst-zone_register);
+ if (rbt == NULL)
+ result = ISC_R_NOTFOUND;
/* Potentially ISC_R_NOSPACE can occur. Destroy codepath has no way to
* return errors, so kill BIND.
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0076] Fix crashes on BIND reload caused by improper error handling during new zone addition
On Fri, Oct 05, 2012 at 05:00:14PM +0200, Petr Spacek wrote:
Hello,
Fix crashes on BIND reload caused by improper error handling
during new zone addition.
Crash can be triggered by invalid query/transfer/update ACLs
or potentially by error in zr_get_zone_ptr().
I found this problem during PATCH 75 testing, so there is a new ticket:
https://fedorahosted.org/bind-dyndb-ldap/ticket/93
Ack
From d0e958cac75035b212f87f00fade080b025d0a23 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 5 Oct 2012 14:41:57 +0200
Subject: [PATCH] Fix crashes on BIND reload caused by improper error handling
during new zone addition.
Crash can be triggered by invalid query/transfer/update ACLs
or potentially by error in zr_get_zone_ptr().
https://fedorahosted.org/bind-dyndb-ldap/ticket/93
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
d4bb6db10b0e79f8777fde3c5f344298af87ce56..0e1cf6f7a6986db126aaa5329dbe9abbc98c8bf4
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1031,6 +1031,7 @@ 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 published = ISC_FALSE;
isc_task_t *task = inst-task;
isc_uint32_t ldap_serial;
isc_uint32_t zr_serial; /* SOA serial value from in-memory zone
register */
@@ -1074,12 +1075,13 @@ ldap_parse_zoneentry(ldap_entry_t *entry,
ldap_instance_t *inst)
/* Check if we are already serving given zone */
result = zr_get_zone_ptr(inst-zone_register, name, zone);
- if (result != ISC_R_SUCCESS) { /* TODO: What about other errors? */
+ if (result == ISC_R_NOTFOUND || result == DNS_R_PARTIALMATCH) {
CHECK(create_zone(inst, name, zone));
CHECK(zr_add_zone(inst-zone_register, zone, dn));
publish = ISC_TRUE;
log_debug(2, created zone %p: %s, zone, dn);
- }
+ } else if (result != ISC_R_SUCCESS)
+ CLEANUP_WITH(result);
log_debug(2, Setting SSU table for %p: %s, zone, dn);
/* Get the update policy and update the zone with it. */
@@ -1119,6 +1121,7 @@ ldap_parse_zoneentry(ldap_entry_t *entry,
ldap_instance_t *inst)
if (publish) {
/* Everything is set correctly, publish zone */
CHECK(publish_zone(inst, zone));
+ published = ISC_TRUE;
}
/*
@@ -1178,6 +1181,13 @@ ldap_parse_zoneentry(ldap_entry_t *entry,
ldap_instance_t *inst)
}
cleanup:
+ if (publish !published) { /* Failure in ACL parsing or so. */
+ log_error_r(zone '%s': publishing failed, rolling back due to,
+ entry-dn);
+ result = zr_del_zone(inst-zone_register, name);
+ if (result != ISC_R_SUCCESS)
+ log_error_r(zone '%s': rollback failed, entry-dn);
+ }
if (unlock)
isc_task_endexclusive(task);
if (dns_name_dynamic(name))
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0078] Use automatic connection management in LDAP modification code to prevent potential deadlock
On Mon, Oct 08, 2012 at 04:46:54PM +0200, Petr Spacek wrote:
Hello,
Use automatic connection management in LDAP modification code to
prevent potential deadlock.
Without this patch the plugin will deadlock when modify_ldap_common()
is called with PTR synchronization enabled and only single
connection is available in the connection pool.
Nack
If I read the patch correctly, it leaves unused ldap_conn parameters in
ldap_modify_do() and modify_soa_record() functions.
Those params are always NULL so they can be safely removed. Please also remove
the autoconn variable from ldap_modify_do()
Regards, Adam
From 5ad686a95510b1584c85d672ec845b27504f746c Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Mon, 8 Oct 2012 16:41:40 +0200
Subject: [PATCH] Use automatic connection management in LDAP modification
code to prevent potential deadlock.
Without this patch the plugin will deadlock when modify_ldap_common()
is called with PTR synchronization enabled and only single
connection is available in the connection pool.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
f8df1b29871c28a1eeecfa93d5d91edd1aee3944..03adb83bdc34593ec527facd3e3fc60755a4de22
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2593,7 +2593,6 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t
*ldap_inst,
{
isc_result_t result;
isc_mem_t *mctx = ldap_inst-mctx;
- ldap_connection_t *ldap_conn = NULL;
ld_string_t *owner_dn = NULL;
LDAPMod *change[3] = { NULL };
LDAPMod *change_ptr = NULL;
@@ -2630,8 +2629,6 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t
*ldap_inst,
CHECK(dn_to_dnsname(mctx, zone_dn, zone_name, NULL));
- CHECK(ldap_pool_getconnection(ldap_inst-pool, ldap_conn));
-
result = zr_get_zone_settings(ldap_inst-zone_register, zone_name,
zone_settings);
if (result != ISC_R_SUCCESS) {
@@ -2655,7 +2652,7 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t
*ldap_inst,
CHECK(discard_from_cache(cache, owner));
if (rdlist-type == dns_rdatatype_soa) {
- result = modify_soa_record(ldap_inst, ldap_conn,
str_buf(owner_dn),
+ result = modify_soa_record(ldap_inst, NULL, str_buf(owner_dn),
HEAD(rdlist-rdata));
goto cleanup;
}
@@ -2666,7 +2663,7 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t
*ldap_inst,
CHECK(ldap_rdttl_to_ldapmod(mctx, rdlist, change[1]));
}
- CHECK(ldap_modify_do(ldap_inst, ldap_conn, str_buf(owner_dn), change,
delete_node));
+ CHECK(ldap_modify_do(ldap_inst, NULL, str_buf(owner_dn), change,
delete_node));
/* Keep the PTR of corresponding A/ record synchronized. */
if (rdlist-type == dns_rdatatype_a || rdlist-type ==
dns_rdatatype_) {
@@ -2824,13 +2821,13 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t
*ldap_inst,
change_ptr = NULL;
/* Modify PTR record. */
- CHECK(ldap_modify_do(ldap_inst, ldap_conn,
str_buf(owner_dn_ptr), change, delete_node));
+ CHECK(ldap_modify_do(ldap_inst, NULL, str_buf(owner_dn_ptr),
+ change, delete_node));
(void) discard_from_cache(ldap_instance_getcache(ldap_inst),
dns_fixedname_name(ptr_name));
}
cleanup:
- ldap_pool_putconnection(ldap_inst-pool, ldap_conn);
str_destroy(owner_dn_ptr);
str_destroy(owner_dn);
str_destroy(str_ptr);
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0073] Use NOTAUTH and REFUSED response codes for dynamic updates rather than SERVFAIL
On Thu, Oct 04, 2012 at 10:31:24AM +0200, Petr Spacek wrote:
Hello,
Use NOTAUTH and REFUSED response codes for dynamic updates rather
than SERVFAIL.
SERVFAIL is still sent if PTR synchronization is enabled but
impossible for some reason.
This change should make dynamic updates debugging simpler.
Ack
From bff8bc688c61717df67de2968492f76b4be65d2a Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 4 Oct 2012 10:26:38 +0200
Subject: [PATCH] Use NOTAUTH and REFUSED response codes for dynamic updates
rather than SERVFAIL.
SERVFAIL is still sent if PTR synchronization is enabled but
impossible for some reason.
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
a7f492e1169c36a321c240fd6ff321a9ef63c2c4..24d469a562b96176ac8fffcf443b9b063096d58c
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2525,7 +2525,7 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t
*ldap_inst,
entry = HEAD(ldap_qresult-ldap_entries);
if (entry == NULL) {
log_debug(3, Active zone %s not found, zone_dn);
- result = ISC_R_NOTFOUND;
+ result = DNS_R_NOTAUTH;
goto cleanup;
}
@@ -2537,7 +2537,7 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t
*ldap_inst,
if (!zone_dyn_update) {
log_debug(3, Dynamic Update is not allowed in zone %s,
zone_dn);
- result = ISC_R_NOPERM;
+ result = DNS_R_REFUSED;
goto cleanup;
}
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Fix memory leaks in dynamic update PTR synchronization
On Thu, Oct 04, 2012 at 09:49:03AM +0200, Petr Spacek wrote:
Hello,
Fix memory leaks in dynamic update PTR synchronization.
During settings code refactoring I found several ugly memory leaks
in sync_ptr handling.
Ack
From 690008eb1fb6f340c735150f21a8d30a244e14bf Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 4 Oct 2012 09:46:25 +0200
Subject: [PATCH] Fix memory leaks in dynamic update PTR synchronization.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
629c76732c86af2a614e589a5afff18136068a66..a7f492e1169c36a321c240fd6ff321a9ef63c2c4
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2496,11 +2496,16 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t
*ldap_inst,
isc_boolean_t zone_sync_ptr = ldap_inst-sync_ptr;
ld_string_t *owner_dn_ptr = NULL;
char *attrs[] = {idnsAllowSyncPTR, idnsAllowDynUpdate, NULL};
+ ld_string_t *str_ptr = NULL;
+ ldapdb_rdatalist_t rdlist_search;
+ dns_rdatalist_t *rdlist_ptr = NULL;
+ char **vals = NULL;
/*
* Find parent zone entry and check if Dynamic Update is allowed.
* @todo Try the cache first and improve split: SOA records are
problematic.
*/
+ ISC_LIST_INIT(rdlist_search);
CHECK(str_new(mctx, owner_dn));
CHECK(dnsname_to_dn(ldap_inst-zone_register, owner, owner_dn));
char *zone_dn = strstr(str_buf(owner_dn),, );
@@ -2606,8 +2611,6 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t
*ldap_inst,
CHECK(dns_byaddr_createptrname2(isc_ip, 0,
dns_fixedname_name(name)));
/* Find PTR entry in LDAP. */
- ldapdb_rdatalist_t rdlist_search;
- dns_rdatalist_t *rdlist_ptr = NULL;
result = ldapdb_rdatalist_get(mctx, ldap_inst,
dns_fixedname_name(name),
NULL,
rdlist_search);
@@ -2682,7 +2685,6 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t
*ldap_inst,
*
* @example str_ptr = host.example.com.
*/
- ld_string_t *str_ptr = NULL;
CHECK(str_new(mctx, str_ptr));
CHECK(dn_to_text(str_buf(owner_dn), str_ptr, NULL));
@@ -2699,7 +2701,6 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t
*ldap_inst,
*
*/
if (mod_op == LDAP_MOD_DELETE) {
- char **vals = NULL;
CHECK(ldap_rdata_to_char_array(mctx,
HEAD(rdlist_ptr-rdata), vals));
if (vals != NULL vals[0] != NULL strcmp(vals[0],
str_buf(str_ptr)) != 0) {
log_debug(3,Cannot delete PTR record,
unexpected value found
@@ -2746,9 +2747,12 @@ cleanup:
ldap_pool_putconnection(ldap_inst-pool, ldap_conn);
str_destroy(owner_dn_ptr);
str_destroy(owner_dn);
+ str_destroy(str_ptr);
free_ldapmod(mctx, change[0]);
free_ldapmod(mctx, change[1]);
if (change_ptr != NULL) free_ldapmod(mctx, change_ptr);
+ ldapdb_rdatalist_destroy(mctx, rdlist_search);
+ free_char_array(mctx, vals);
return result;
}
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0070] Fix zone register locking in zr_set_zone_serial_digest()
On Wed, Sep 26, 2012 at 12:57:33PM +0200, Petr Spacek wrote:
Hello,
Fix zone register locking in zr_set_zone_serial_digest().
Zone register has to be locked against simultaneous writes.
Ack
From ad51025a35efe47542f4379049c8e23d1054726c Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 26 Sep 2012 12:51:06 +0200
Subject: [PATCH] Fix zone register locking in zr_set_zone_serial_digest().
Zone register has to be locked against simultaneous writes.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/zone_register.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/zone_register.c b/src/zone_register.c
index
b2b938f3336b23a40d43c85062c9389a2190f3cb..76305730b2e19686568f8a1bc6ac703ed3898fcc
100644
--- a/src/zone_register.c
+++ b/src/zone_register.c
@@ -370,15 +370,15 @@ zr_set_zone_serial_digest(zone_register_t *zr,
dns_name_t *name,
return ISC_R_FAILURE;
}
- RWLOCK(zr-rwlock, isc_rwlocktype_read);
+ RWLOCK(zr-rwlock, isc_rwlocktype_write);
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);
+ RWUNLOCK(zr-rwlock, isc_rwlocktype_write);
return result;
}
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0062] Prevent memory read outside allocated space in str_alloc()
On Fri, Sep 14, 2012 at 10:50:56AM +0200, Petr Spacek wrote:
Hello,
Prevent memory read outside allocated space in str_alloc().
Found by Valgrind during nsupdate stress test.
Ack
From c53ec9cf2cc22e29630767b6b2259d145192ff62 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 14 Sep 2012 10:48:04 +0200
Subject: [PATCH] Prevent memory read outside allocated space in str_alloc().
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/str.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/str.c b/src/str.c
index
0096536de071f7d0dd2ff327e4d6ac72e81dab5f..83645365ee6eff7bda5fbeda6837f30d4dec41ae
100644
--- a/src/str.c
+++ b/src/str.c
@@ -102,7 +102,7 @@ str_alloc(ld_string_t *str, size_t len)
return ISC_R_NOMEMORY;
if (str-data != NULL) {
- memcpy(new_buffer, str-data, len);
+ memcpy(new_buffer, str-data, str-allocated);
new_buffer[len] = '\0';
isc_mem_put(str-mctx, str-data, str-allocated);
} else {
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0054] Allow BIND to start if LDAP connection times out
On Tue, Aug 28, 2012 at 02:00:57PM +0200, Petr Spacek wrote:
Hello,
this patch allows BIND to start if LDAP connection times out. BIND
will reconnect in same way as after connection refused errors.
The patch closes https://fedorahosted.org/bind-dyndb-ldap/ticket/84 .
Ack
From eaa35060fc47c1422ca7b577fe0096aadd2f8c0a Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 28 Aug 2012 13:54:51 +0200
Subject: [PATCH] Allow BIND to start if LDAP connection times out.
https://fedorahosted.org/bind-dyndb-ldap/ticket/84
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
da083d2e65032e650cfbbeb863262e0141403407..d533d1985c9fb8f56f333a99208d3f60f0b4c5bf
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2797,7 +2797,7 @@ ldap_pool_connect(ldap_pool_t *pool, ldap_instance_t
*ldap_inst)
ldap_conn = NULL;
CHECK(new_ldap_connection(pool, ldap_conn));
result = ldap_connect(ldap_inst, ldap_conn, ISC_FALSE);
- if (result == ISC_R_NOTCONNECTED) {
+ if (result == ISC_R_NOTCONNECTED || result == ISC_R_TIMEDOUT) {
/* LDAP server is down which can happen, continue */
result = ISC_R_SUCCESS;
} else if (result != ISC_R_SUCCESS) {
--
1.7.11.2
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0060] Fix zone delete in ldap_zone_delete2()
),
+dns_fixedname_name(concat),
ISC_TRUE);
-RUNTIME_CHECK(result == ISC_R_SUCCESS);
-
- result = dns_rbtnodechain_next(chain, NULL, NULL);
- RUNTIME_CHECK(result == ISC_R_SUCCESS ||
- result == DNS_R_NEWORIGIN ||
- result == ISC_R_NOMORE);
+ RUNTIME_CHECK(result == ISC_R_SUCCESS);
}
dns_rbtnodechain_invalidate(chain);
@@ -606,6 +629,7 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
isc_mem_putanddetach(ldap_inst-mctx, ldap_inst,
sizeof(ldap_instance_t));
*ldap_instp = NULL;
+ log_debug(1, LDAP instance '%s' destroyed, db_name);
}
static isc_result_t
@@ -776,7 +800,10 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t
*name, isc_boolean_t lock)
isc_boolean_t freeze = ISC_FALSE;
dns_zone_t *zone = NULL;
dns_zone_t *foundzone = NULL;
+ char zone_name_char[DNS_NAME_FORMATSIZE];
+ dns_name_format(name, zone_name_char, DNS_NAME_FORMATSIZE);
+ log_debug(1, deleting zone '%s', zone_name_char);
if (lock) {
result = isc_task_beginexclusive(inst-task);
RUNTIME_CHECK(result == ISC_R_SUCCESS ||
@@ -790,6 +817,7 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t
*name, isc_boolean_t lock)
result = zr_get_zone_ptr(inst-zone_register, name, zone);
if (result == ISC_R_NOTFOUND) {
+ log_debug(1, zone '%s' not found in zone register,
zone_name_char);
result = ISC_R_SUCCESS;
goto cleanup;
} else if (result != ISC_R_SUCCESS)
@@ -810,7 +838,11 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t
*name, isc_boolean_t lock)
if (dns_zone_getdb(zone, dbp) == ISC_R_SUCCESS) {
dns_db_detach(dbp); /* dns_zone_getdb() attaches DB implicitly
*/
dns_zone_unload(zone);
+ log_debug(1, zone '%s' unloaded, zone_name_char);
+ } else {
+ log_debug(1, zone '%s' not loaded - unload skipped,
zone_name_char);
}
+
CHECK(dns_zt_unmount(inst-view-zonetable, zone));
CHECK(zr_del_zone(inst-zone_register, name));
dns_zonemgr_releasezone(inst-zmgr, zone);
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0063] Notify DNS slaves if zone serial number modification was detected.
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 pspa...@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 pspa...@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.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0064] Improve log message about improperly formated Resource Records
On Mon, Sep 17, 2012 at 05:07:44PM +0200, Petr Spacek wrote:
Hello,
this patch adds DN to log message about improperly formated Resource Records.
Hi,
please check my comment below, otherwise ack.
Regards, Adam
From d36ae54c593c33a45ef9936720357ff7de30c8b5 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Mon, 17 Sep 2012 17:01:41 +0200
Subject: [PATCH] Improve log message about improperly formated Resource
Records.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
d22e8714df57edaad4cf45e6cc26ec0dbbd59108..2245cb982f26eab165a327b4ad72046f9eb4024e
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1473,6 +1473,8 @@ ldap_parse_rrentry(isc_mem_t *mctx, ldap_entry_t *entry,
dns_rdata_t *rdata = NULL;
dns_rdatalist_t *rdlist = NULL;
ldap_attribute_t *attr;
+ const char *dn = NULL entry;
+ const char *data = NULL data;
result = add_soa_record(mctx, qresult, origin, entry,
rdatalist, fake_mname);
@@ -1501,6 +1503,11 @@ ldap_parse_rrentry(isc_mem_t *mctx, ldap_entry_t
*entry,
return ISC_R_SUCCESS;
cleanup:
+ if (entry != NULL)
+ dn = entry-dn;
+ if (buf != NULL str_buf(buf) != NULL)
+ data = str_buf(buf);
+ log_error_r(failed to parse RR entry: dn '%s': data '%s', dn, data);
return result;
}
@@ -1539,7 +1546,6 @@ ldapdb_nodelist_get(isc_mem_t *mctx, ldap_instance_t
*ldap_inst, dns_name_t *nam
dns_name_init(node_name, NULL);
if (dn_to_dnsname(mctx, entry-dn, node_name, NULL)
!= ISC_R_SUCCESS) {
- log_error(Failed to parse dn %s, entry-dn);
I prefer to keep this error in place. Otherwise bad DNs will be silently
skipped.
continue;
}
@@ -1551,7 +1557,6 @@ ldapdb_nodelist_get(isc_mem_t *mctx, ldap_instance_t
*ldap_inst, dns_name_t *nam
string, node-rdatalist);
}
if (result != ISC_R_SUCCESS) {
- log_error(Failed to parse RR entry (%s),
str_buf(string));
/* node cleaning */
dns_name_reset(node-owner);
ldapdb_rdatalist_destroy(mctx, node-rdatalist);
@@ -1609,11 +1614,9 @@ ldapdb_rdatalist_get(isc_mem_t *mctx, ldap_instance_t
*ldap_inst, dns_name_t *na
for (entry = HEAD(ldap_qresult-ldap_entries);
entry != NULL;
entry = NEXT(entry, link)) {
- if (ldap_parse_rrentry(mctx, entry, ldap_qresult,
-origin, ldap_inst-fake_mname,
-string, rdatalist) != ISC_R_SUCCESS ) {
- log_error(Failed to parse RR entry (%s),
str_buf(string));
- }
+ CHECK(ldap_parse_rrentry(mctx, entry, ldap_qresult,
+origin, ldap_inst-fake_mname,
+string, rdatalist));
}
if (!EMPTY(*rdatalist)) {
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0063] Notify DNS slaves if zone serial number modification was detected.
On Mon, Sep 24, 2012 at 03:21:23PM +0200, Petr Spacek wrote:
On 09/24/2012 03:09 PM, Adam Tkac wrote:
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 pspa...@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 pspa...@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.
Two lines above start of the patch is following code:
result = dns_zone_load(zone);
if (result != ISC_R_SUCCESS result != DNS_R_UPTODATE
result != DNS_R_DYNAMIC result != DNS_R_CONTINUE)
goto cleanup;
zone_dynamic = (result == DNS_R_DYNAMIC);
For non-dynamic zones dns_zone_load() sends notifies. Zone_dynamic
condition only prevents doubled dns_zone_notify() calls for
non-dynamic zones. I can remove this condition if doubled calls are
not problem.
Ah, good point. Then ack for the original version.
Petr^2 Spacek
Regards, Adam
+ }
cleanup:
if (unlock)
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0064] Improve log message about improperly formated Resource Records
On Mon, Sep 24, 2012 at 03:27:13PM +0200, Petr Spacek wrote:
On 09/24/2012 03:15 PM, Adam Tkac wrote:
On Mon, Sep 17, 2012 at 05:07:44PM +0200, Petr Spacek wrote:
Hello,
this patch adds DN to log message about improperly formated Resource
Records.
Hi,
please check my comment below, otherwise ack.
Regards, Adam
From d36ae54c593c33a45ef9936720357ff7de30c8b5 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Mon, 17 Sep 2012 17:01:41 +0200
Subject: [PATCH] Improve log message about improperly formated Resource
Records.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
d22e8714df57edaad4cf45e6cc26ec0dbbd59108..2245cb982f26eab165a327b4ad72046f9eb4024e
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1473,6 +1473,8 @@ ldap_parse_rrentry(isc_mem_t *mctx, ldap_entry_t
*entry,
dns_rdata_t *rdata = NULL;
dns_rdatalist_t *rdlist = NULL;
ldap_attribute_t *attr;
+ const char *dn = NULL entry;
+ const char *data = NULL data;
result = add_soa_record(mctx, qresult, origin, entry,
rdatalist, fake_mname);
@@ -1501,6 +1503,11 @@ ldap_parse_rrentry(isc_mem_t *mctx, ldap_entry_t
*entry,
return ISC_R_SUCCESS;
cleanup:
+ if (entry != NULL)
+ dn = entry-dn;
+ if (buf != NULL str_buf(buf) != NULL)
+ data = str_buf(buf);
+ log_error_r(failed to parse RR entry: dn '%s': data '%s', dn, data);
return result;
}
@@ -1539,7 +1546,6 @@ ldapdb_nodelist_get(isc_mem_t *mctx, ldap_instance_t
*ldap_inst, dns_name_t *nam
dns_name_init(node_name, NULL);
if (dn_to_dnsname(mctx, entry-dn, node_name, NULL)
!= ISC_R_SUCCESS) {
- log_error(Failed to parse dn %s, entry-dn);
I prefer to keep this error in place. Otherwise bad DNs will be silently
skipped.
There is another log_error_r() call in dn_to_dnsname(). Actually old
code logs error two times and the message from dn_to_dnsname() is
more verbose.
Thanks for clarification, ack for the original version.
continue;
}
@@ -1551,7 +1557,6 @@ ldapdb_nodelist_get(isc_mem_t *mctx, ldap_instance_t
*ldap_inst, dns_name_t *nam
string, node-rdatalist);
}
if (result != ISC_R_SUCCESS) {
- log_error(Failed to parse RR entry (%s),
str_buf(string));
/* node cleaning */
dns_name_reset(node-owner);
ldapdb_rdatalist_destroy(mctx, node-rdatalist);
@@ -1609,11 +1614,9 @@ ldapdb_rdatalist_get(isc_mem_t *mctx,
ldap_instance_t *ldap_inst, dns_name_t *na
for (entry = HEAD(ldap_qresult-ldap_entries);
entry != NULL;
entry = NEXT(entry, link)) {
- if (ldap_parse_rrentry(mctx, entry, ldap_qresult,
- origin, ldap_inst-fake_mname,
- string, rdatalist) != ISC_R_SUCCESS ) {
- log_error(Failed to parse RR entry (%s),
str_buf(string));
- }
+ CHECK(ldap_parse_rrentry(mctx, entry, ldap_qresult,
+ origin, ldap_inst-fake_mname,
+ string, rdatalist));
}
if (!EMPTY(*rdatalist)) {
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0068] Fix unable to concatenate DNS names during zone_refresh error
On Mon, Sep 24, 2012 at 01:22:32PM +0200, Petr Spacek wrote:
Hello,
Fix unable to concatenate DNS names during zone_refresh error.
This fixes zone refresh for cases where concatenation of all
zone names is longer than 255 characters.
Ack
From e9764d49e8c3ecd6985cd35d7ef1991ee569a98a Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Mon, 24 Sep 2012 10:39:49 +0200
Subject: [PATCH] Fix unable to concatenate DNS names during zone_refresh
error.
This fixes zone refresh for cases where concatenation of all
zone names is longer than 255 characters.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
cbb818e7a7510155804a40bc242d7245b1faea90..373d57ac385d4db7394959c23c511e1734503989
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1300,10 +1300,10 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst,
isc_boolean_t delete_only)
result = dns_rbtnodechain_first(chain,
zr_get_rbt(ldap_inst-zone_register), NULL, NULL);
while (result == DNS_R_NEWORIGIN || result == ISC_R_SUCCESS) {
-
+ dns_name_reset(aname);
delete = ISC_FALSE;
node = NULL;
-
+
result = dns_rbtnodechain_current(chain, fname, forig,
node);
if (result != ISC_R_SUCCESS) {
if (result != ISC_R_NOTFOUND)
@@ -1315,7 +1315,7 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst,
isc_boolean_t delete_only)
result = dns_name_concatenate(fname, forig, aname,
aname.buffer);
if (result != ISC_R_SUCCESS) {
- log_error_r(unable to concatenate DNS names
+ log_error_r(unable to concatenate DNS names
during zone_refresh);
goto next;
}
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0069] Fix crash caused by empty zone renaming
On Mon, Sep 24, 2012 at 03:45:25PM +0200, Petr Spacek wrote:
Hello,
Fix crash caused by empty zone renaming.
LDAP query uses LDAP_SCOPE_ONELEVEL scope so original condition
in INSIST is incorrect, because zone is not required to have child
names (i.e. names other than zone itself).
Ack
From 6d59d4beacf5339aec260bac6bd3c69839efb1f7 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Mon, 24 Sep 2012 15:34:11 +0200
Subject: [PATCH] Fix crash caused by empty zone renaming.
LDAP query uses LDAP_SCOPE_ONELEVEL scope so original condition
in INSIST is incorrect, because zone is not required to have child
names (i.e. names other than zone itself).
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
373d57ac385d4db7394959c23c511e1734503989..33eab72f2b37a792a97a6c85f21a3501d399380c
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -3093,8 +3093,6 @@ update_zone(isc_task_t *task, isc_event_t *event)
LDAP_SCOPE_ONELEVEL, attrs_record, 0,
(objectClass=idnsRecord)));
- /* LDAP schema requires SOA record (at least) */
- INSIST(HEAD(ldap_qresult_record-ldap_entries) != NULL);
for (entry_record =
HEAD(ldap_qresult_record-ldap_entries);
entry_record != NULL;
entry_record = NEXT(entry_record,
link)) {
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0061] Add missing DNS view attach/detach to LDAP instance management code
On Thu, Sep 13, 2012 at 01:37:37PM +0200, Petr Spacek wrote:
Hello,
Add missing DNS view attach/detach to LDAP instance management code.
This fixes race condition in BIND shutdown after SIGINT:
- failing assert caused by use-after-free in dns_zt_find():
(((zt) != ((void *)0)) (((const isc__magic_t *)(zt))-magic
== ((('Z') 24 | ('T') 16 | ('b') 8 | ('l')
Ack.
From cc612198a0b7d662557a7c4f71732135e8f43025 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 13 Sep 2012 13:08:36 +0200
Subject: [PATCH] Add missing DNS view attach/detach to LDAP instance
management code. This fixes race condition in BIND shutdown
after SIGINT: - failing assert caused by use-after-free in
dns_zt_find(): (((zt) != ((void *)0)) (((const
isc__magic_t *)(zt))-magic == ((('Z') 24 | ('T') 16
| ('b') 8 | ('l')
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
3b49de809738fef18cae10c38fd3d9c33eef5141..658b960f50b461fa602edf51e955f3bdd4769e1d
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -333,6 +333,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
unsigned int i;
isc_result_t result;
ldap_instance_t *ldap_inst;
+ dns_view_t *view = NULL;
ld_string_t *auth_method_str = NULL;
setting_t ldap_settings[] = {
{ uri, no_default_string },
@@ -369,10 +370,9 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
isc_mem_attach(mctx, ldap_inst-mctx);
ldap_inst-db_name = db_name;
- ldap_inst-view = dns_dyndb_get_view(dyndb_args);
+ view = dns_dyndb_get_view(dyndb_args);
+ dns_view_attach(view, ldap_inst-view);
ldap_inst-zmgr = dns_dyndb_get_zonemgr(dyndb_args);
- /* commented out for now, cause named to hang */
- //dns_view_attach(view, ldap_inst-view);
CHECK(zr_create(mctx, ldap_inst-zone_register));
@@ -616,8 +616,7 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
str_destroy(ldap_inst-fake_mname);
str_destroy(ldap_inst-ldap_hostname);
- /* commented out for now, causes named to hang */
- //dns_view_detach(ldap_inst-view);
+ dns_view_detach(ldap_inst-view);
DESTROYLOCK(ldap_inst-kinit_lock);
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0066] Log errors from dns_name_concatenate() in zone_refresh() properly
On Fri, Sep 21, 2012 at 10:13:37AM +0200, Petr Spacek wrote:
Hello,
Log errors from dns_name_concatenate() in zone_refresh() properly.
Ack.
From b8abc442b0d483366f002a8364a333577042d652 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 21 Sep 2012 10:10:55 +0200
Subject: [PATCH] Log errors from dns_name_concatenate() in zone_refresh()
properly.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
2245cb982f26eab165a327b4ad72046f9eb4024e..3301f8d872239dcc48506d1f935dacad7bf5990f
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1312,9 +1312,11 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst,
isc_boolean_t delete_only)
goto next;
}
- if (dns_name_concatenate(fname, forig, aname, aname.buffer)
- != ISC_R_SUCCESS) {
- log_error_r(unable to concatenate DNS names during
zone_refresh);
+ result = dns_name_concatenate(fname, forig, aname,
+ aname.buffer);
+ if (result != ISC_R_SUCCESS) {
+ log_error_r(unable to concatenate DNS names
+ during zone_refresh);
goto next;
}
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0065] Bump version in .spec file to 2.0
On Thu, Sep 20, 2012 at 04:16:41PM +0200, Petr Spacek wrote:
Hello,
this patch bumps version in .spec file to 2.0.
Ack
From b4fc1e119e5d602c196af47bde07d3cfe3091a3d Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 20 Sep 2012 16:14:05 +0200
Subject: [PATCH] Bump version in .spec file to 2.0.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
contrib/bind-dyndb-ldap.spec | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/contrib/bind-dyndb-ldap.spec b/contrib/bind-dyndb-ldap.spec
index
664e6a9af9629f07193c82382debac028c425fe1..5f1e0262e4a4707a6bb5a5820f807a98390581ce
100644
--- a/contrib/bind-dyndb-ldap.spec
+++ b/contrib/bind-dyndb-ldap.spec
@@ -1,12 +1,8 @@
-#%define PATCHVER P4
-%define PREVER rc1
-#%define VERSION %{version}
-#%define VERSION %{version}-%{PATCHVER}
-%define VERSION %{version}%{PREVER}
+%define VERSION %{version}
Name: bind-dyndb-ldap
-Version:1.1.0
-Release:0.1.%{PREVER}%{?dist}
+Version:2.0
+Release:0%{?dist}
Summary:LDAP back-end plug-in for BIND
Group: System Environment/Libraries
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0055] Fix race condition in addrdataset() during SOA serial update
); cleanup: - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS || + isc_serial_gt(new_serial, old_serial) != ISC_TRUE) log_error(SOA serial number incrementation failed in zone '%s', str_buf(zone_dn)); -- 1.7.11.4 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0056] Fix crash caused by zone deletion vs. SOA serial increment race condition
On Wed, Sep 12, 2012 at 12:33:47PM +0200, Petr Spacek wrote:
Hello,
The patch fixes crash caused by stupid bug in logging code.
Ack.
From 01aa00f9ba4feac9f97b34b81c3697b2b7f8122f Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 7 Sep 2012 16:21:27 +0200
Subject: [PATCH] Fix crash caused by zone deletion vs. SOA serial increment
race condition.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
e636a84b35d0bcdc8573c6e7146f38ee21a42076..058048f41485999be0d8ffeadea02f2e25879370
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2931,6 +2931,7 @@ soa_serial_increment(isc_mem_t *mctx, ldap_instance_t
*inst,
dns_name_t *zone_name) {
isc_result_t result = ISC_R_FAILURE;
ld_string_t *zone_dn = NULL;
+ const char *zone_dn_char = INACTIVE/UNKNOWN;
ldapdb_rdatalist_t rdatalist;
dns_rdatalist_t *rdlist = NULL;
dns_rdata_t *soa_rdata = NULL;
@@ -2944,6 +2945,7 @@ soa_serial_increment(isc_mem_t *mctx, ldap_instance_t
*inst,
INIT_LIST(rdatalist);
CHECK(str_new(mctx, zone_dn));
CHECK(dnsname_to_dn(inst-zone_register, zone_name, zone_dn));
+ zone_dn_char = str_buf(zone_dn);
log_debug(5, incrementing SOA serial number in zone '%s',
str_buf(zone_dn));
@@ -2978,7 +2980,7 @@ cleanup:
if (result != ISC_R_SUCCESS ||
isc_serial_gt(new_serial, old_serial) != ISC_TRUE)
log_error(SOA serial number incrementation failed in zone
'%s',
- str_buf(zone_dn));
+ zone_dn_char);
str_destroy(zone_dn);
ldapdb_rdatalist_destroy(mctx, rdatalist);
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0057] Fix LDAP operation selection logic in ldap_modify_do()
On Wed, Sep 12, 2012 at 12:35:25PM +0200, Petr Spacek wrote:
Hello,
There is a fix for LDAP operation selection logic in ldap_modify_do().
Each operation code in LDAPMod structure can be ORed
with LDAP_MOD_BVALUES.
Ack
From ab11e62ec2496f2c7245c4d8d80c2fd189b68aa9 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 11 Sep 2012 16:23:18 +0200
Subject: [PATCH] Fix LDAP operation selection logic in ldap_modify_do().
Each operation code in LDAPMod structure can be ORed
with LDAP_MOD_BVALUES.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 29 +
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
058048f41485999be0d8ffeadea02f2e25879370..d9c7ce5d84c3944a86ff1865ff6be073ddc294c8
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2149,33 +2149,38 @@ ldap_modify_do(ldap_instance_t *ldap_inst,
ldap_connection_t *ldap_conn,
CHECK(ldap_connect(ldap_inst, ldap_conn, ISC_FALSE));
}
+ /* Any mod_op can be ORed with LDAP_MOD_BVALUES. */
+ if ((mods[0]-mod_op ~LDAP_MOD_BVALUES) == LDAP_MOD_ADD)
+ operation_str = modifying(add);
+ else if ((mods[0]-mod_op ~LDAP_MOD_BVALUES) == LDAP_MOD_DELETE)
+ operation_str = modifying(del);
+ else if ((mods[0]-mod_op ~LDAP_MOD_BVALUES) == LDAP_MOD_REPLACE)
+ operation_str = modifying(replace);
+ else {
+ operation_str = modifying(unknown operation);
+ log_bug(%s: 0x%x, operation_str, mods[0]-mod_op);
+ CHECK(ISC_R_NOTIMPLEMENTED);
+ }
+
if (delete_node) {
log_debug(2, deleting whole node: '%s', dn);
ret = ldap_delete_ext_s(ldap_conn-handle, dn, NULL, NULL);
} else {
- log_debug(2, writing to '%s', dn);
+ log_debug(2, writing to '%s': %s, dn, operation_str);
ret = ldap_modify_ext_s(ldap_conn-handle, dn, mods, NULL,
NULL);
}
result = (ret == LDAP_SUCCESS) ? ISC_R_SUCCESS : ISC_R_FAILURE;
if (ret == LDAP_SUCCESS)
goto cleanup;
- if (mods[0]-mod_op == LDAP_MOD_ADD)
- operation_str = modifying(add);
- else if (mods[0]-mod_op == LDAP_MOD_DELETE)
- operation_str = modifying(del);
- else {
- operation_str = modifying(unknown operation);
- CHECK(ISC_R_NOTIMPLEMENTED);
- }
-
LDAP_OPT_CHECK(ldap_get_option(ldap_conn-handle, LDAP_OPT_RESULT_CODE,
err_code), ldap_modify_do(%s) failed to obtain ldap
error code,
operation_str);
/* If there is no object yet, create it with an ldap add operation. */
- if (mods[0]-mod_op == LDAP_MOD_ADD err_code == LDAP_NO_SUCH_OBJECT)
{
+ if ((mods[0]-mod_op ~LDAP_MOD_BVALUES) == LDAP_MOD_ADD
+ err_code == LDAP_NO_SUCH_OBJECT) {
int i;
LDAPMod **new_mods;
char *obj_str[] = { idnsRecord, NULL };
@@ -2211,7 +2216,7 @@ ldap_modify_do(ldap_instance_t *ldap_inst,
ldap_connection_t *ldap_conn,
/* do not error out if we are trying to delete an
* unexisting attribute */
- if (mods[0]-mod_op != LDAP_MOD_DELETE ||
+ if ((mods[0]-mod_op ~LDAP_MOD_BVALUES) != LDAP_MOD_DELETE ||
err_code != LDAP_NO_SUCH_ATTRIBUTE) {
result = ISC_R_FAILURE;
}
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0058] Improve persistent search logging
On Wed, Sep 12, 2012 at 12:36:38PM +0200, Petr Spacek wrote: Hello, this patch adds result codes to error messages in persistent search code. Ack. From f6cb53278d8f39ac6da4fb8e26820f6ee02ae6e3 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Wed, 12 Sep 2012 12:27:51 +0200 Subject: [PATCH] Improve persistent search logging. Signed-off-by: Petr Spacek pspa...@redhat.com --- src/ldap_helper.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ldap_helper.c b/src/ldap_helper.c index d9c7ce5d84c3944a86ff1865ff6be073ddc294c8..92edbe7159272772e1c993d46da7c93382cbc5d4 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -3069,9 +3069,9 @@ update_zone(isc_task_t *task, isc_event_t *event) cleanup: if (result != ISC_R_SUCCESS) - log_error(update_action (psearch) failed for '%s': %s. + log_error_r(update_zone (psearch) failed for '%s'. Zones can be outdated, run `rndc reload`, - pevent-dn, isc_result_totext(result)); + pevent-dn); ldap_query_free(ISC_FALSE, ldap_qresult_zone); ldap_query_free(ISC_FALSE, ldap_qresult_record); @@ -3125,7 +3125,7 @@ update_config(isc_task_t *task, isc_event_t *event) cleanup: if (result != ISC_R_SUCCESS) - log_error(update_config (psearch) failed for %s. + log_error_r(update_config (psearch) failed for '%s'. Configuration can be outdated, run `rndc reload`, pevent-dn); @@ -3221,9 +3221,9 @@ update_record(isc_task_t *task, isc_event_t *event) } cleanup: if (result != ISC_R_SUCCESS) - log_error(update_record (psearch) failed, dn '%s'. + log_error_r(update_record (psearch) failed, dn '%s' change type 0x%x. Records can be outdated, run `rndc reload`, - pevent-dn); + pevent-dn, pevent-chgtype); if (dns_name_dynamic(name)) dns_name_free(name, inst-mctx); @@ -3400,7 +3400,7 @@ cleanup: if (prevdn_ldap != NULL) ldap_memfree(prevdn); - log_error(psearch_update failed for %s zone. + log_error_r(psearch_update failed for '%s' zone. Zone can be outdated, run `rndc reload`, entry-dn); } @@ -3586,7 +3586,7 @@ restart: * Error means inconsistency of our zones * data. */ - log_error(ldap_psearch_watcher failed, zones + log_error_r(ldap_psearch_watcher failed, zones might be outdated. Run `rndc reload`); goto soft_err; } -- 1.7.11.4 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0059] Fix potential crash after free(uninitialized variable)
On Wed, Sep 12, 2012 at 01:07:56PM +0200, Petr Spacek wrote:
Hello,
This patch fixes potential crash after free(uninitialized variable)
in persistent search code.
Coverity CID 13088.
Ack
From 3197b4ace3e852495bf4f9fdc32192459160027c Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 12 Sep 2012 13:04:39 +0200
Subject: [PATCH] Fix potential crash after free(uninitialized variable) in
persistent search code.
Coverity CID 13088.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
92edbe7159272772e1c993d46da7c93382cbc5d4..67a64b79159983c83cb1bfc73c4b02a9bce986a7
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2878,8 +2878,8 @@ cleanup:
static isc_result_t
ldap_pscontrol_create(LDAPControl **ctrlp)
{
- BerElement *ber;
- struct berval *berval;
+ BerElement *ber = NULL;
+ struct berval *berval = NULL;
isc_result_t result = ISC_R_FAILURE;
REQUIRE(ctrlp != NULL *ctrlp == NULL);
@@ -2905,14 +2905,13 @@ ldap_pscontrol_create(LDAPControl **ctrlp)
!= LDAP_SUCCESS)
goto cleanup;
- ber_free(ber, 1);
- ber_bvfree(berval);
-
- return ISC_R_SUCCESS;
+ result = ISC_R_SUCCESS;
cleanup:
- ber_free(ber, 1);
- ber_bvfree(berval);
+ if (ber != NULL)
+ ber_free(ber, 1);
+ if (berval != NULL)
+ ber_bvfree(berval);
return result;
}
--
1.7.11.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0050] Fix memory leak in configuration with multiple LDAP instances
On Tue, Aug 14, 2012 at 04:00:21PM +0200, Petr Spacek wrote:
Hello,
this patch fixes $SUBJ$.
Adam, please double-check correctness of this change.
I had two assumptions:
- all locking is done inside dns_db_(un)register() functions
- LDAP instances are decommissioned before dynamic_driver_destroy() call
Ack
From e314eb7da7bfbbb2ae9d4ce1252d886c9a744e7f Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 14 Aug 2012 15:53:42 +0200
Subject: [PATCH] Fix memory leak in configuration with multiple LDAP
instances.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_driver.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index
d958d15bdebe5e89dc4948655ffba655073d53e0..117215ae480cdcabb2977037af9a89bb25578243
100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -1278,6 +1278,7 @@ isc_result_t
dynamic_driver_init(isc_mem_t *mctx, const char *name, const char * const
*argv,
dns_dyndb_arguments_t *dyndb_args)
{
+ dns_dbimplementation_t *ldapdb_imp_new = NULL;
isc_result_t result;
REQUIRE(name != NULL);
@@ -1305,11 +1306,12 @@ dynamic_driver_init(isc_mem_t *mctx, const char
*name, const char * const *argv,
}
/* Register new DNS DB implementation. */
- ldapdb_imp = NULL;
result = dns_db_register(ldapdb_impname, ldapdb_create, NULL, mctx,
- ldapdb_imp);
+ ldapdb_imp_new);
if (result != ISC_R_SUCCESS result != ISC_R_EXISTS)
return result;
+ else if (result == ISC_R_SUCCESS)
+ ldapdb_imp = ldapdb_imp_new;
/* Finally, create the instance. */
result = manager_create_db_instance(mctx, name, argv, dyndb_args);
--
1.7.11.2
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 302 Stricter IP network validator in dnszone-add command
On Wed, Sep 05, 2012 at 01:02:35PM +0200, Jan Cholasta wrote: Dne 5.9.2012 12:48, Martin Kosek napsal(a): On 09/05/2012 12:36 PM, Jan Cholasta wrote: Dne 5.9.2012 12:22, Petr Spacek napsal(a): On 09/05/2012 11:30 AM, Jan Cholasta wrote: Dne 5.9.2012 10:04, Martin Kosek napsal(a): We allowed IP addresses without network specification which lead to unexpected results when the zone was being created. We should rather strictly require the prefix/netmask specifying the IP network that the reverse zone should be created for. This is already done in Web UI. A unit test exercising this new validation was added. https://fedorahosted.org/freeipa/ticket/2461 I don't like this much. I would suggest using CheckedIPAddress and not forcing the user to enter the prefix length instead. CheckedIPAddress uses a sensible default prefix length if one is not specified (class-based for IPv4, /64 for IPv6) as opposed to IPNetwork (/32 for IPv4, /128 for IPv6 - this causes the erroneous reverse zones to be created as described in the ticket). Hello, I don't like automatic netmask guessing. I have met class-based guessing in Windows (XP?) and I was forced to overwrite default mask all the time ... If there was no guessing, you would have to write the netmask anyway, so I don't see any harm in guessing here. IMHO there is no sensible default prefix in real world. I sitting on network with /23 prefix right now. Also, I have never seen 10.x network with /8 prefix. While this might be true for IPv4 in some cases, /64 is perfectly sensible for IPv6. Also, I have never seen 192.168.x.x network with non-/24 prefix. Honza While this may be true for 192.168.x.x, it does not apply for 10.x.x.x networks as Petr already pointed out. I don't think that there will be many people expecting that a reverse zone of 10.0.0.0/24 would be created. And they would be correct, because the default prefix length for a class A network is /8, not /24. And since FreeIPA is mainly deployed to internal networks, I assume this will be the case of most users. Martin OK, but what about IPv6? Correct me if I'm wrong, but the prefix length is going to be /64 99% of the time for IPv6. You are right, IPv6 networks could have default /64 prefix. However as I wrote in different mail, I don't recommend to use default IPv4 prefix at all because FreeIPA targets for company environments where /24 is not so common, not for home environments. The installer uses /24 for IPv4 addresses and /64 for IPv6 addresses, maybe this should be used as a default here as well. Regards, Adam -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0051-0052] Log successful reconnection to LDAP server
error code);
+ }
+ }
+ if (ldap_conn-handle == NULL || ret != LDAP_OPT_SUCCESS) {
goto reconnect;
}
I think this is more readable:
if (ldap_conn-handle == NULL)
goto reconnect;
ret = ldap_get_option(..);
if (ret != LDAP_OPT_SUCCESS) {
log_error
goto reconnect;
}
isn't it?
@@ -2090,11 +2099,13 @@ handle_connection_error(ldap_instance_t *ldap_inst,
ldap_connection_t *ldap_conn
reconnect:
if (ldap_conn-tries == 0)
log_error(connection to the LDAP server was lost);
- return ldap_connect(ldap_inst, ldap_conn, force);
-
+ result = ldap_connect(ldap_inst, ldap_conn, force);
+ if (result == ISC_R_SUCCESS)
+ log_info(successfully reconnected to LDAP server);
+ break;
}
- return ISC_R_FAILURE;
+ return result;
}
/* FIXME: Handle the case where the LDAP handle is NULL - try to reconnect.
*/
@@ -3476,7 +3487,7 @@ ldap_psearch_watcher(isc_threadarg_t arg)
Next try in %ds, inst-reconnect_interval);
if (!sane_sleep(inst, inst-reconnect_interval))
goto cleanup;
- ldap_connect(inst, conn, ISC_TRUE);
+ handle_connection_error(inst, conn, ISC_TRUE);
}
CHECK(ldap_query_create(conn-mctx, ldap_qresult));
--
1.7.11.2
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0053] Use richer set of return codes for LDAP connection error handling code
On Wed, Aug 15, 2012 at 01:23:45PM +0200, Petr Spacek wrote:
Hello,
current code return very generic ISC_R_FAILURE code in nearly all (error)
cases.
This patch distinguishes between different LDAP errors and returns
richer set of return codes from LDAP connection error handling code.
It should lead to clearer log messages.
Petr^2 Spacek
Ack
From 15d6b38c9eda5b05d799c145ede8341f359e8633 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 15 Aug 2012 13:01:48 +0200
Subject: [PATCH] Use richer set of return codes for LDAP connection error
handling code.
It should lead to clear log messages.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
798aeadfef27d7071a1dd4133b7f08a21918ef78..da083d2e65032e650cfbbeb863262e0141403407
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1971,7 +1971,7 @@ ldap_reconnect(ldap_instance_t *ldap_inst,
ldap_connection_t *ldap_conn,
result = isc_time_now(now);
time_cmp = isc_time_compare(now, ldap_conn-next_reconnect);
if (result == ISC_R_SUCCESS time_cmp 0)
- return ISC_R_FAILURE;
+ return ISC_R_SOFTQUOTA;
}
/* If either bind_dn or the password is not set, we will use
@@ -2050,6 +2050,8 @@ force_reconnect:
return ISC_R_NOPERM;
case LDAP_SERVER_DOWN:
return ISC_R_NOTCONNECTED;
+ case LDAP_TIMEOUT:
+ return ISC_R_TIMEDOUT;
default:
return ISC_R_FAILURE;
}
@@ -2085,13 +2087,16 @@ handle_connection_error(ldap_instance_t *ldap_inst,
ldap_connection_t *ldap_conn
switch (err_code) {
case LDAP_NO_SUCH_OBJECT:
ldap_conn-tries = 0;
- return ISC_R_SUCCESS;
+ result = ISC_R_SUCCESS;
+ break;
case LDAP_TIMEOUT:
log_error(LDAP query timed out. Try to adjust \timeout\
parameter);
+ result = ISC_R_TIMEDOUT;
break;
case LDAP_INVALID_DN_SYNTAX:
case LDAP_INVALID_SYNTAX:
log_bug(Invalid syntax in handle_connection_error indicates a
bug);
+ result = ISC_R_UNEXPECTEDTOKEN;
break;
default:
/* Try to reconnect on other errors. */
--
1.7.11.2
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0051-0052] Log successful reconnection to LDAP server
On Wed, Sep 05, 2012 at 03:53:36PM +0200, Petr Spacek wrote: On 09/05/2012 01:29 PM, Adam Tkac wrote: On Wed, Aug 15, 2012 at 01:20:08PM +0200, Petr Spacek wrote: Hello, this two patches solves upstream ticket https://fedorahosted.org/bind-dyndb-ldap/ticket/71 Log successful reconnect Patch 51: Adds log_info(): logging facility with log level INFO. Ack. Patch 52: Logs successful reconnection to LDAP server. LDAP connection error handling was modified: Errors are handled exclusively by handle_connection_error() now. Direct calls to ldap_connect() and ldap_reconnect() should be avoided. Nack, please check my comments below. Thanks for your comments! Corrected patches are attached + I replied in-line. Ack -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0047] Avoid manual connection management outside ldap_query()
(discard_from_cache(ldap_instance_getcache(inst), zone_name)); /* put the new SOA to inst-cache and compare old and new serials */ @@ -2999,7 +3028,6 @@ cleanup: 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; @@ -3019,7 +3047,6 @@ update_zone(isc_task_t *task, isc_event_t *event) ldap_psearchevent_t *pevent = (ldap_psearchevent_t *)event; isc_result_t result ; ldap_instance_t *inst = NULL; - ldap_connection_t *conn = NULL; ldap_qresult_t *ldap_qresult_zone = NULL; ldap_qresult_t *ldap_qresult_record = NULL; ldap_entry_t *entry_zone = NULL; @@ -3041,8 +3068,7 @@ update_zone(isc_task_t *task, isc_event_t *event) dns_name_init(prevname, NULL); CHECK(manager_get_ldap_instance(pevent-dbname, inst)); - CHECK(ldap_pool_getconnection(inst-pool, conn)); - CHECK(ldap_query(inst, conn, ldap_qresult_zone, pevent-dn, + CHECK(ldap_query(inst, NULL, ldap_qresult_zone, pevent-dn, LDAP_SCOPE_BASE, attrs_zone, 0, ((objectClass=idnsZone)(idnsZoneActive=TRUE; @@ -3062,7 +3088,7 @@ update_zone(isc_task_t *task, isc_event_t *event) } /* fill the cache with records from renamed zone */ - CHECK(ldap_query(inst, conn, ldap_qresult_record, pevent-dn, + CHECK(ldap_query(inst, NULL, ldap_qresult_record, pevent-dn, LDAP_SCOPE_ONELEVEL, attrs_record, 0, (objectClass=idnsRecord))); @@ -3090,7 +3116,6 @@ cleanup: ldap_query_free(ISC_FALSE, ldap_qresult_zone); ldap_query_free(ISC_FALSE, ldap_qresult_record); - ldap_pool_putconnection(inst-pool, conn); if (dns_name_dynamic(prevname)) dns_name_free(prevname, inst-mctx); isc_mem_free(mctx, pevent-dbname); @@ -3107,7 +3132,6 @@ update_config(isc_task_t *task, isc_event_t *event) ldap_psearchevent_t *pevent = (ldap_psearchevent_t *)event; isc_result_t result ; ldap_instance_t *inst = NULL; - ldap_connection_t *conn = NULL; ldap_qresult_t *ldap_qresult = NULL; ldap_entry_t *entry; isc_mem_t *mctx; @@ -3124,9 +3148,7 @@ update_config(isc_task_t *task, isc_event_t *event) if (result != ISC_R_SUCCESS) goto cleanup; - CHECK(ldap_pool_getconnection(inst-pool, conn)); - - CHECK(ldap_query(inst, conn, ldap_qresult, pevent-dn, + CHECK(ldap_query(inst, NULL, ldap_qresult, pevent-dn, LDAP_SCOPE_BASE, attrs, 0, (objectClass=idnsConfigObject))); @@ -3149,7 +3171,6 @@ cleanup: pevent-dn); ldap_query_free(ISC_FALSE, ldap_qresult); - ldap_pool_putconnection(inst-pool, conn); isc_mem_free(mctx, pevent-dbname); isc_mem_free(mctx, pevent-dn); isc_mem_detach(mctx); -- 1.7.11.2 -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0046] Separate RR data parsing from LDAP connections
On Wed, Aug 15, 2012 at 04:04:26PM +0200, Petr Spacek wrote: On 08/15/2012 03:31 PM, Adam Tkac wrote: On Wed, Aug 01, 2012 at 04:19:11PM +0200, Petr Spacek wrote: Hello, this patch finishes LDAP connection vs. LDAP result separation. It is first step necessary for: https://fedorahosted.org/bind-dyndb-ldap/ticket/68 Avoid manual connection management outside ldap_query() It should prevent deadlocks in future. Petr^2 Spacek ... I recommend to use ZERO_PTR(ldap_qresult) instead of many = NULL assignments. It's safer and used in other *_create functions. Amended patch is attached. Ack -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0042] Flush zones and RRs cache when handling persistent search reconnection
On Wed, Aug 15, 2012 at 03:55:01PM +0200, Petr Spacek wrote:
On 08/15/2012 03:11 PM, Adam Tkac wrote:
On Fri, Jul 27, 2012 at 12:16:07PM +0200, Petr Spacek wrote:
Hello,
this patch implements Flush zones and RRs cache when handling
persistent search reconnection behaviour as requested
in ticket https://fedorahosted.org/bind-dyndb-ldap/ticket/44 .
Petr^2 Spacek
+isc_result_t
+flush_ldap_cache(ldap_cache_t *cache)
+{
+ isc_result_t result;
+
+ REQUIRE(cache != NULL);
+
+ LOCK(cache-mutex);
+ if (!ldap_cache_enabled(cache)) {
+ result = ISC_R_SUCCESS;
+ } else {
+ dns_rbt_destroy(cache-rbt);
+ CHECK(dns_rbt_create(cache-mctx, cache_node_deleter, NULL,
+ cache-rbt));
In my opinion usage of dns_rbt_deletename(cache-rbt, dns_rootname,
ISC_TRUE) is
better, isn't it?
I looked into implementation of both functions. dns_rbt_deletenode()
does horribly complicated magic for simple task as delete whole
tree is.
For this reason I called dns_rbt_destroy() - it is much simpler and
should be faster (it doesn't try to maintain RBT invariants during
whole process).
Sounds fine, ack for the patch as is.
A
Otherwise OK.
+ }
+
+cleanup:
+ if (result != ISC_R_SUCCESS)
+ log_error_r(cache flush failed);
+ UNLOCK(cache-mutex);
+ return result;
+}
Regards, Adam
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0043] Extend API to be compatible with libdns interface = 90
On Fri, Jul 27, 2012 at 02:23:49PM +0200, Petr Spacek wrote:
Hello,
this patch prevents compiler warning on systems with libdns
interface version = 90. This libdns version comes with BIND 9.0.0.
Both new methods are not obligatory, see in bind/lib/dns/db.c,
functions dns_db_findext() and dns_db_findnodeext().
Petr^2 Spacek
Ack
From 9481fc6f6032f236d7e5e48f651906b25fd49b61 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 27 Jul 2012 14:18:15 +0200
Subject: [PATCH] Extend API to be compatible with libdns interface = 90.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_driver.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index
cae45d4f6cc1f201c40ca3413d1f626e03a0318e..51d618c5a2395c58b362a047096b1cf1fc40fbfd
100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -1213,8 +1213,12 @@ static dns_dbmethods_t ldapdb_methods = {
#endif /* LIBDNS_VERSION_MAJOR = 45 */
#if LIBDNS_VERSION_MAJOR = 82
NULL, /* rpz_enabled */
- NULL/* rpz_findips */
+ NULL, /* rpz_findips */
#endif /* LIBDNS_VERSION_MAJOR = 82 */
+#if LIBDNS_VERSION_MAJOR = 90
+ NULL, /* findnodeext */
+ NULL/* findext */
+#endif /* LIBDNS_VERSION_MAJOR = 90 */
};
static isc_result_t
--
1.7.11.2
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0042] Flush zones and RRs cache when handling persistent search reconnection
On Fri, Jul 27, 2012 at 12:16:07PM +0200, Petr Spacek wrote:
Hello,
this patch implements Flush zones and RRs cache when handling
persistent search reconnection behaviour as requested
in ticket https://fedorahosted.org/bind-dyndb-ldap/ticket/44 .
Petr^2 Spacek
+isc_result_t
+flush_ldap_cache(ldap_cache_t *cache)
+{
+ isc_result_t result;
+
+ REQUIRE(cache != NULL);
+
+ LOCK(cache-mutex);
+ if (!ldap_cache_enabled(cache)) {
+ result = ISC_R_SUCCESS;
+ } else {
+ dns_rbt_destroy(cache-rbt);
+ CHECK(dns_rbt_create(cache-mctx, cache_node_deleter, NULL,
+ cache-rbt));
In my opinion usage of dns_rbt_deletename(cache-rbt, dns_rootname, ISC_TRUE) is
better, isn't it?
Otherwise OK.
+ }
+
+cleanup:
+ if (result != ISC_R_SUCCESS)
+ log_error_r(cache flush failed);
+ UNLOCK(cache-mutex);
+ return result;
+}
Regards, Adam
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0044] Fix and comment ispersistent() call in LDAP driver interface
On Fri, Jul 27, 2012 at 03:06:05PM +0200, Petr Spacek wrote:
Hello,
this patch fixes ispersistent() call in LDAP driver interface.
We were lucky, because ISC_R_NOTIMPLEMENTED is evaluated as ISC_TRUE
every time, but I want to be sure.
Petr^2 Spacek
Ack
From bfa32f2fa7d880a5c137cf1705202e939f1928e5 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 27 Jul 2012 14:58:22 +0200
Subject: [PATCH] Fix and comment ispersistent() call in LDAP driver
interface.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_driver.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index
51d618c5a2395c58b362a047096b1cf1fc40fbfd..470b6f315f0f4483eb60703b369891892368548a
100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -309,6 +309,11 @@ free_ldapdb(ldapdb_t *ldapdb)
isc_mem_putanddetach(ldapdb-common.mctx, ldapdb, sizeof(*ldapdb));
}
+
+/**
+ * This method should never be called, because LDAP DB is persistent.
+ * See ispersistent() function.
+ */
static isc_result_t
beginload(dns_db_t *db, dns_addrdatasetfunc_t *addp, dns_dbload_t **dbloadp)
{
@@ -323,6 +328,10 @@ beginload(dns_db_t *db, dns_addrdatasetfunc_t *addp,
dns_dbload_t **dbloadp)
return ISC_R_SUCCESS;
}
+/**
+ * This method should never be called, because LDAP DB is persistent.
+ * See ispersistent() function.
+ */
static isc_result_t
endload(dns_db_t *db, dns_dbload_t **dbloadp)
{
@@ -1114,12 +1123,16 @@ nodecount(dns_db_t *db)
return ISC_R_NOTIMPLEMENTED;
}
+/**
+ * Return TRUE, because database does not need to be loaded from disk
+ * or written to disk.
+ */
static isc_boolean_t
ispersistent(dns_db_t *db)
{
UNUSED(db);
- return ISC_R_NOTIMPLEMENTED;
+ return ISC_TRUE;
}
static void
--
1.7.11.2
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0046] Separate RR data parsing from LDAP connections
;
ldap_qresult-query_string = NULL;
+ ldap_qresult-lex = NULL;
I recommend to use ZERO_PTR(ldap_qresult) instead of many = NULL assignments.
It's safer and used in other *_create functions.
INIT_LIST(ldap_qresult-ldap_entries);
CHECK(str_new(mctx, ldap_qresult-query_string));
+ CHECKED_MEM_GET(ldap_qresult-mctx, ldap_qresult-rdata_target_mem,
MINTSIZ);
+ CHECK(isc_lex_create(ldap_qresult-mctx, TOKENSIZ, ldap_qresult-lex));
+
*ldap_qresultp = ldap_qresult;
return ISC_R_SUCCESS;
cleanup:
- SAFE_MEM_PUT_PTR(mctx, ldap_qresult);
+ if (ldap_qresult != NULL) {
+ str_destroy(ldap_qresult-query_string);
+ SAFE_MEM_PUT(ldap_qresult-mctx,
ldap_qresult-rdata_target_mem, MINTSIZ);
+ if (ldap_qresult-lex != NULL)
+ isc_lex_destroy(ldap_qresult-lex);
+ SAFE_MEM_PUT_PTR(mctx, ldap_qresult);
+ }
+
return result;
}
@@ -1833,8 +1836,13 @@ ldap_query_free(isc_boolean_t prepare_reuse,
ldap_qresult_t **ldap_qresultp)
if (prepare_reuse) {
str_clear(qresult-query_string);
INIT_LIST(qresult-ldap_entries);
+ isc_lex_close(qresult-lex);
} else { /* free the whole structure */
str_destroy(qresult-query_string);
+ if (qresult-lex != NULL)
+ isc_lex_destroy(qresult-lex);
+ if (qresult-rdata_target_mem != NULL)
+ isc_mem_put(qresult-mctx, qresult-rdata_target_mem,
MINTSIZ);
SAFE_MEM_PUT_PTR(qresult-mctx, qresult);
*ldap_qresultp = NULL;
}
--
1.7.11.2
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0041] Cleanup in logging code
On Wed, Jul 25, 2012 at 03:31:34PM +0200, Petr Spacek wrote:
Hello,
this patch clears logging code a bit. Adding functions like
log_info() and similar will be trivial from now.
It will be necessary for ticket #71: Log successful reconnect
https://fedorahosted.org/bind-dyndb-ldap/ticket/71
Ack.
From 26136d6fe5fce5ac4f3138063bcf4774f268bd3c Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 19 Jul 2012 14:13:12 +0200
Subject: [PATCH] Cleanup in logging code.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/log.c | 22 ++
src/log.h | 19 ---
2 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/src/log.c b/src/log.c
index
b23e4720a8dd484a65d8a7e6c58baf257fc9ce50..f731df706b58e1f894659811dae32d4148a8620c
100644
--- a/src/log.c
+++ b/src/log.c
@@ -28,31 +28,13 @@
#include log.h
void
-log_debug(int level, const char *format, ...)
+log_write(int level, const char *format, ...)
{
va_list args;
va_start(args, format);
-#ifdef LOG_AS_ERROR
- UNUSED(level);
isc_log_vwrite(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_DYNDB,
-ISC_LOG_ERROR, format, args);
-#else
- isc_log_vwrite(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_DYNDB,
-ISC_LOG_DEBUG(level), format, args);
-#endif
-
- va_end(args);
-}
-
-void
-log_error(const char *format, ...)
-{
- va_list args;
-
- va_start(args, format);
- isc_log_vwrite(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_DYNDB,
-ISC_LOG_ERROR, format, args);
+level, format, args);
va_end(args);
}
diff --git a/src/log.h b/src/log.h
index
0df4e25618fab932bdec97c276580d1b9d31bf08..898639be144dbf6049a1440493c3358e01a5c2dd
100644
--- a/src/log.h
+++ b/src/log.h
@@ -22,18 +22,31 @@
#define _LD_LOG_H_
#include isc/error.h
+#include dns/log.h
+
+#ifdef LOG_AS_ERROR
+#define GET_LOG_LEVEL(level) ISC_LOG_ERROR
+#else
+#define GET_LOG_LEVEL(level) (level)
+#endif
#define fatal_error(...) \
isc_error_fatal(__FILE__, __LINE__, __VA_ARGS__)
#define log_bug(fmt, ...) \
log_error(bug in %s(): fmt, __func__,##__VA_ARGS__)
#define log_error_r(fmt, ...) \
- log_error(fmt : %s, ##__VA_ARGS__, isc_result_totext(result))
+ log_error(fmt : %s, ##__VA_ARGS__, dns_result_totext(result))
/* Basic logging functions */
-void log_debug(int level, const char *format, ...) ISC_FORMAT_PRINTF(2, 3);
-void log_error(const char *format, ...) ISC_FORMAT_PRINTF(1, 2);
+#define log_error(format, ...) \
+ log_write(GET_LOG_LEVEL(ISC_LOG_ERROR), format, ##__VA_ARGS__)
+
+#define log_debug(level, format, ...)\
+ log_write(GET_LOG_LEVEL(level), format, ##__VA_ARGS__)
+
+void
+log_write(int level, const char *format, ...) ISC_FORMAT_PRINTF(2, 3);
#endif /* !_LD_LOG_H_ */
--
1.7.10.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0040] Handle incomplete/invalid zone unload in same way as BIND's ns_server_del_zone()
On Wed, Jul 25, 2012 at 10:18:01AM +0200, Petr Spacek wrote:
Hello,
this patch prevents potential failure during invalid zone unload.
Error handling was changed to the same way as in
bind/bin/named/server.c ns_server_del_zone().
Ack.
From 02e232632a8a04fcd17f1089553961c18c0b175a Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 25 Jul 2012 10:07:20 +0200
Subject: [PATCH] Handle incomplete/invalid zone unload in same way as
ns_server_del_zone().
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
daffac7c7825a99a07c333217638d3beaddfaad2..cc7003a6cdcd2d27404fec936623ed8a3e8fa7f8
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -823,7 +823,7 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t
*name, isc_boolean_t lock)
/* Do not unload partially loaded zones, they have incomplete
structures. */
dns_db_t *dbp = NULL;
- if (dns_zone_getdb(zone, dbp) != DNS_R_NOTLOADED) {
+ if (dns_zone_getdb(zone, dbp) == ISC_R_SUCCESS) {
dns_db_detach(dbp); /* dns_zone_getdb() attaches DB implicitly
*/
dns_zone_unload(zone);
}
--
1.7.10.4
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0038] Fix two memory leaks in ldap_query()
On Fri, Jul 20, 2012 at 02:28:09PM +0200, Petr Spacek wrote:
Hello,
this patch fixes two memory leaks in ldap_query(). Both memory leaks
occurs after non-success queries.
It effectively re-implements fix for ldap_query can incorrectly
return ISC_R_SUCCESS even when failed:
http://git.fedorahosted.org/git/?p=bind-dyndb-ldap.git;a=commitdiff;h=a7cd8ae747b3a81a02ab9e5dbefe1c595aa24ff6
Please double-check this approach.
Ack, please push it to master.
A
From c8718b98641e7537b2350a625b03b0b7fec6f206 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 20 Jul 2012 14:18:41 +0200
Subject: [PATCH] Fix two memory leaks in ldap_query().
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 14 --
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index
6ac76faecbc250deb28203256fa13d83ae6c80f4..daffac7c7825a99a07c333217638d3beaddfaad2
100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1753,28 +1753,30 @@ retry:
ldap_qresult-ldap_entries);
if (result != ISC_R_SUCCESS) {
log_error(failed to save LDAP query results);
- return result;
+ goto cleanup;
}
*ldap_qresultp = ldap_qresult;
return ISC_R_SUCCESS;
+ } else {
+ result = ISC_R_FAILURE;
}
ret = ldap_get_option(ldap_conn-handle, LDAP_OPT_RESULT_CODE,
(void *)ldap_err_code);
- if (ret == LDAP_OPT_SUCCESS ldap_err_code == LDAP_NO_SUCH_OBJECT)
- return ISC_R_NOTFOUND;
- /* some error happened during ldap_search, try to recover */
- else if (!once) {
+ if (ret == LDAP_OPT_SUCCESS ldap_err_code == LDAP_NO_SUCH_OBJECT) {
+ result = ISC_R_NOTFOUND;
+ } else if (!once) {
+ /* some error happened during ldap_search, try to recover */
once++;
result = handle_connection_error(ldap_inst, ldap_conn,
ISC_FALSE);
if (result == ISC_R_SUCCESS)
goto retry;
}
cleanup:
ldap_query_free(ISC_FALSE, ldap_qresult);
- return ISC_R_FAILURE;
+ return result;
}
/**
--
1.7.7.6
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0032-0035]
(pevent-chgtype)) {
+ log_debug(5, psearch_update: removing name from cache, dn:
'%s',
pevent-dn);
}
@@ -3164,7 +3164,7 @@ update_record(isc_task_t *task, isc_event_t *event)
*
* @todo Change this to convert ldap_entry_t to
ldapdb_rdatalist_t.
*/
- log_debug(5, psearch_update: Updating item in cache (%s),
+ log_debug(5, psearch_update: updating name in cache, dn: '%s',
pevent-dn);
CHECK(ldapdb_rdatalist_get(mctx, inst, name, origin,
rdatalist));
@@ -3177,18 +3177,13 @@ update_record(isc_task_t *task, isc_event_t *event)
ldapdb_rdatalist_destroy(mctx, rdatalist);
}
- 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:
if (result != ISC_R_SUCCESS)
- log_error(update_record (psearch) failed for %s.
+ log_error(update_record (psearch) failed, dn '%s'.
Records can be outdated, run `rndc reload`,
pevent-dn);
@@ -3282,14 +3277,20 @@ psearch_update(ldap_instance_t *inst, ldap_entry_t
*entry, LDAPControl **ctrls)
class = ldap_entry_getclass(entry);
if (class == LDAP_ENTRYCLASS_NONE) {
- log_error(psearch_update: ignoring unknown entry [dn %s],
+ log_error(psearch_update: ignoring entry with unknown class,
dn '%s',
entry-dn);
return; /* ignore it, it's OK */
}
if (ctrls != NULL)
CHECK(ldap_parse_entrychangectrl(ctrls, chgtype,
prevdn_ldap));
+
+ log_debug(20,psearch change type: none%d, add%d, del%d, mod%d,
moddn%d,
+ !PSEARCH_ANY(chgtype), PSEARCH_ADD(chgtype),
+ PSEARCH_DEL(chgtype), PSEARCH_MOD(chgtype),
+ PSEARCH_MODDN(chgtype));
+
isc_mem_attach(inst-mctx, mctx);
dn = isc_mem_strdup(mctx, entry-dn);
--
1.7.7.6
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
