Re: [Freeipa-devel] [PATCH 0181] Replace LDAP persistent search with syncrepl (RFC 4533)

2014-02-21 Thread Petr Spacek 

On 13.12.2013 17:44, Petr Spacek wrote:

On 7.10.2013 15:19, Tomas Hozza wrote:

On 07/22/2013 03:16 PM, Petr Spacek wrote:

On 22.7.2013 13:23, Petr Spacek wrote:

Hello,

Replace LDAP persistent search with syncrepl (RFC 4533).

All direct operations with LDAP Persistent Search control are replaced
by ldap_sync_* calls.

Syncrepl code works in exactly same way as old psearch code:
Only the DN of the modified object is re-used from the message,
data from the object are fetched via separate LDAP search.

Current code is not able to detect object renaming because we don't have
UUID->DN mapping yet.

Another limitation is that current code can't detect unchanged entries,
so serial is incremented after each parsed LDAP object.


Clang noticed potential NULL dereference in cleanup section of
ldap_syncrepl_watcher(). Fixed patch is attached.



ACK.

Tested Patch bundle 181 - 185. Common tasks like
adding/deleting/updating records work fine. Also PTR sync, zone serial
number
incrementation is OK.


I have found that patch 181-2 doesn't handle reconnection to LDAP.

This new version should handle reconnections better.

This patch should go to master branch only.


It is known limitation that zones and records deleted when connection is down
are not refreshed properly after reconnection. This will be fixed some future
version.

I use this command for testing:
socat tcp-listen:3899,fork,reuseaddr tcp-connect:localhost:389

It is necessary to modify port in /etc/named.conf to connect via socat. Then I
can kill & restart socat to simulate connection breakage.


Pushed to master branch: 9c8aa4fb7d798015d8f889a008b5807b73c55341

--
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0181] Replace LDAP persistent search with syncrepl (RFC 4533)

2013-12-13 Thread Petr Spacek 

On 7.10.2013 15:19, Tomas Hozza wrote:

On 07/22/2013 03:16 PM, Petr Spacek wrote:

On 22.7.2013 13:23, Petr Spacek wrote:

Hello,

Replace LDAP persistent search with syncrepl (RFC 4533).

All direct operations with LDAP Persistent Search control are replaced
by ldap_sync_* calls.

Syncrepl code works in exactly same way as old psearch code:
Only the DN of the modified object is re-used from the message,
data from the object are fetched via separate LDAP search.

Current code is not able to detect object renaming because we don't have
UUID->DN mapping yet.

Another limitation is that current code can't detect unchanged entries,
so serial is incremented after each parsed LDAP object.


Clang noticed potential NULL dereference in cleanup section of
ldap_syncrepl_watcher(). Fixed patch is attached.



ACK.

Tested Patch bundle 181 - 185. Common tasks like
adding/deleting/updating records work fine. Also PTR sync, zone serial
number
incrementation is OK.


I have found that patch 181-2 doesn't handle reconnection to LDAP.

This new version should handle reconnections better.

This patch should go to master branch only.


It is known limitation that zones and records deleted when connection is down 
are not refreshed properly after reconnection. This will be fixed some future 
version.


I use this command for testing:
socat tcp-listen:3899,fork,reuseaddr tcp-connect:localhost:389

It is necessary to modify port in /etc/named.conf to connect via socat. Then I 
can kill & restart socat to simulate connection breakage.


--
Petr^2 Spacek

From b83d2d67b11fe9fc6cf8a1b36bc5e5dccaec65dd Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Mon, 22 Jul 2013 12:59:38 +0200
Subject: [PATCH] Replace LDAP persistent search with syncrepl (RFC 4533).

All direct operations with LDAP Persistent Search control are replaced
by ldap_sync_* calls.

Syncrepl code works in exactly same way as old psearch code:
Only the DN of the modified object is re-used from the message,
data from the object are fetched via separate LDAP search.

Current code is not able to detect object renaming because we don't have
UUID->DN mapping yet.

Another limitation is that current code can't detect unchanged entries,
so serial is incremented after each parsed LDAP object.

Signed-off-by: Petr Spacek 
---
 README|   2 +-
 src/ldap_entry.c  |  20 +-
 src/ldap_helper.c | 576 ++
 3 files changed, 276 insertions(+), 322 deletions(-)

diff --git a/README b/README
index 74e6a40b8601907f0a699d46da40a6608dde0e02..bd6af49e8176ef0d69971a255c97e49b73d15a28 100644
--- a/README
+++ b/README
@@ -17,7 +17,7 @@ is required.
 ===
 * support for dynamic updates
 * SASL authentication
-* persistent search for zones (experimental)
+* SyncRepl (RFC 4533) for run-time synchronization with LDAP server
 
 
 3. Installation
diff --git a/src/ldap_entry.c b/src/ldap_entry.c
index 1a5673428ce2fecc24d0566de99d8f39a26c3e86..fb62032dbe2c0ad126278ba6d152fe4708b31f7d 100644
--- a/src/ldap_entry.c
+++ b/src/ldap_entry.c
@@ -248,9 +248,11 @@ ldap_entry_destroy(isc_mem_t *mctx, ldap_entry_t **entryp)
 {
 	ldap_entry_t *entry;
 
-	REQUIRE(entryp != NULL && *entryp != NULL);
+	REQUIRE(entryp != NULL);
 
 	entry = *entryp;
+	if (entry == NULL)
+		return;
 
 	ldap_attributelist_destroy(mctx, &entry->attrs);
 	if (entry->dn != NULL)
@@ -451,22 +453,6 @@ ldap_entry_getclass(ldap_entry_t *entry, ldap_entryclass_t *class)
 
 	*class = entryclass;
 	return ISC_R_SUCCESS;
-
-#if 0
-	/* Preserve current attribute iterator */
-	lastattr = = entry->lastattr;
-	entry->lastattr = NULL;
-
-	while ((attr = ldap_entry_nextattr(entry, "objectClass")) != NULL) {
-		if (!strcasecmp(attr->ldap_values[0], "idnsrecord")) {
-			entryclass |= LDAP_ENTRYCLASS_RR;
-		} else if (!strcasecmp(attr->ldap_values[0], "idnszone")) {
-			entryclass |= LDAP_ENTRYCLASS_ZONE;
-		}
-	}
-
-	entry->lastattr = lastattr;
-#endif
 }
 
 isc_result_t
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 80ea0b31501fdbce9445e696c4f40b4d630dea17..86dbef02d48556fad63d06a123c5fcb4f982fbcf 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -178,7 +178,6 @@ struct ldap_connection {
 	isc_mutex_t		lock;
 
 	LDAP			*handle;
-	LDAPControl		*serverctrls[2]; /* psearch/NULL or NULL/NULL */
 	int			msgid;
 
 	/* For reconnection logic. */
@@ -211,11 +210,11 @@ const ldap_auth_pair_t supported_ldap_auth[] = {
 };
 
 #define LDAPDB_EVENTCLASS 	ISC_EVENTCLASS(0x)
-#define LDAPDB_EVENT_PSEARCH	(LDAPDB_EVENTCLASS + 0)
+#define LDAPDB_EVENT_SYNCREPL	(LDAPDB_EVENTCLASS + 0)
 
-typedef struct ldap_psearchevent ldap_psearchevent_t;
-struct ldap_psearchevent {
-	ISC_EVENT_COMMON(ldap_psearchevent_t);
+typedef struct ldap_syncreplevent ldap_syncreplevent_t;
+struct ldap_syncreplevent {
+	ISC_EVENT_COMMON(ldap_syncreplevent_t);
 	isc_mem_t *mctx;
 	char *dbname;
 	char *dn;
@@ -323,17 +322,9 @@ static void ldap_pool_putconnection(ldap_pool_t *pool,
 static isc_result_t ldap

Re: [Freeipa-devel] [PATCH 0181] Replace LDAP persistent search with syncrepl (RFC 4533)

2013-10-07 Thread Tomas Hozza 
On 07/22/2013 03:16 PM, Petr Spacek wrote:
> On 22.7.2013 13:23, Petr Spacek wrote:
>> Hello,
>>
>> Replace LDAP persistent search with syncrepl (RFC 4533).
>>
>> All direct operations with LDAP Persistent Search control are replaced
>> by ldap_sync_* calls.
>>
>> Syncrepl code works in exactly same way as old psearch code:
>> Only the DN of the modified object is re-used from the message,
>> data from the object are fetched via separate LDAP search.
>>
>> Current code is not able to detect object renaming because we don't have
>> UUID->DN mapping yet.
>>
>> Another limitation is that current code can't detect unchanged entries,
>> so serial is incremented after each parsed LDAP object.
> 
> Clang noticed potential NULL dereference in cleanup section of
> ldap_syncrepl_watcher(). Fixed patch is attached.
> 

ACK.

Tested Patch bundle 181 - 185. Common tasks like
adding/deleting/updating records work fine. Also PTR sync, zone serial
number
incrementation is OK.


Regards,

Tomas

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

Re: [Freeipa-devel] [PATCH 0181] Replace LDAP persistent search with syncrepl (RFC 4533)

2013-07-22 Thread Petr Spacek 

On 22.7.2013 13:23, Petr Spacek wrote:

Hello,

Replace LDAP persistent search with syncrepl (RFC 4533).

All direct operations with LDAP Persistent Search control are replaced
by ldap_sync_* calls.

Syncrepl code works in exactly same way as old psearch code:
Only the DN of the modified object is re-used from the message,
data from the object are fetched via separate LDAP search.

Current code is not able to detect object renaming because we don't have
UUID->DN mapping yet.

Another limitation is that current code can't detect unchanged entries,
so serial is incremented after each parsed LDAP object.


Clang noticed potential NULL dereference in cleanup section of 
ldap_syncrepl_watcher(). Fixed patch is attached.


--
Petr^2 Spacek
From e5abf7bcacfb6596fc23dd68dad6f4b4684903b4 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Mon, 22 Jul 2013 12:59:38 +0200
Subject: [PATCH] Replace LDAP persistent search with syncrepl (RFC 4533).

All direct operations with LDAP Persistent Search control are replaced
by ldap_sync_* calls.

Syncrepl code works in exactly same way as old psearch code:
Only the DN of the modified object is re-used from the message,
data from the object are fetched via separate LDAP search.

Current code is not able to detect object renaming because we don't have
UUID->DN mapping yet.

Another limitation is that current code can't detect unchanged entries,
so serial is incremented after each parsed LDAP object.

Signed-off-by: Petr Spacek 
---
 src/ldap_entry.c  |  20 +--
 src/ldap_helper.c | 472 ++
 2 files changed, 197 insertions(+), 295 deletions(-)

diff --git a/src/ldap_entry.c b/src/ldap_entry.c
index 9a3cdf2183b4bbd22663f78440559b573d946bc4..986126dd10fdfdd93666362b87433138615d9bca 100644
--- a/src/ldap_entry.c
+++ b/src/ldap_entry.c
@@ -241,9 +241,11 @@ ldap_entry_destroy(isc_mem_t *mctx, ldap_entry_t **entryp)
 {
 	ldap_entry_t *entry;
 
-	REQUIRE(entryp != NULL && *entryp != NULL);
+	REQUIRE(entryp != NULL);
 
 	entry = *entryp;
+	if (entry == NULL)
+		return;
 
 	ldap_attributelist_destroy(mctx, &entry->attrs);
 	if (entry->dn != NULL)
@@ -440,22 +442,6 @@ ldap_entry_getclass(ldap_entry_t *entry, ldap_entryclass_t *class)
 
 	*class = entryclass;
 	return ISC_R_SUCCESS;
-
-#if 0
-	/* Preserve current attribute iterator */
-	lastattr = = entry->lastattr;
-	entry->lastattr = NULL;
-
-	while ((attr = ldap_entry_nextattr(entry, "objectClass")) != NULL) {
-		if (!strcasecmp(attr->ldap_values[0], "idnsrecord")) {
-			entryclass |= LDAP_ENTRYCLASS_RR;
-		} else if (!strcasecmp(attr->ldap_values[0], "idnszone")) {
-			entryclass |= LDAP_ENTRYCLASS_ZONE;
-		}
-	}
-
-	entry->lastattr = lastattr;
-#endif
 }
 
 ld_string_t*
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 6a04d2b7da3977e3517fd7f449b7d218be6e1e93..15f2afd2edf5a4094a4ac9a61e93292f12da3f45 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -184,7 +184,6 @@ struct ldap_connection {
 	isc_mutex_t		lock;
 
 	LDAP			*handle;
-	LDAPControl		*serverctrls[2]; /* psearch/NULL or NULL/NULL */
 	int			msgid;
 
 	/* For reconnection logic. */
@@ -222,11 +221,11 @@ const ldap_auth_pair_t supported_ldap_auth[] = {
 };
 
 #define LDAPDB_EVENTCLASS 	ISC_EVENTCLASS(0x)
-#define LDAPDB_EVENT_PSEARCH	(LDAPDB_EVENTCLASS + 0)
+#define LDAPDB_EVENT_SYNCREPL	(LDAPDB_EVENTCLASS + 0)
 
-typedef struct ldap_psearchevent ldap_psearchevent_t;
-struct ldap_psearchevent {
-	ISC_EVENT_COMMON(ldap_psearchevent_t);
+typedef struct ldap_syncreplevent ldap_syncreplevent_t;
+struct ldap_syncreplevent {
+	ISC_EVENT_COMMON(ldap_syncreplevent_t);
 	isc_mem_t *mctx;
 	char *dbname;
 	char *dn;
@@ -338,17 +337,9 @@ static void ldap_pool_putconnection(ldap_pool_t *pool,
 static isc_result_t ldap_pool_connect(ldap_pool_t *pool,
 		ldap_instance_t *ldap_inst) ATTR_NONNULLS;
 
-/* Functions for manipulating LDAP persistent search control */
-static isc_result_t ldap_pscontrol_create(LDAPControl **ctrlp) ATTR_NONNULLS;
-
-static isc_threadresult_t ldap_psearch_watcher(isc_threadarg_t arg) ATTR_NONNULLS;
-static void psearch_update(ldap_instance_t *inst, ldap_entry_t *entry,
-		LDAPControl **ctrls) ATTR_NONNULL(1, 2);
-
-
 /* Persistent updates watcher */
 static isc_threadresult_t
-ldap_psearch_watcher(isc_threadarg_t arg);
+ldap_syncrepl_watcher(isc_threadarg_t arg) ATTR_NONNULLS;
 
 #define PRINT_BUFF_SIZE 10 /* for unsigned int 2^32 */
 isc_result_t
@@ -573,10 +564,10 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
 	CHECK(ldap_pool_connect(ldap_inst->pool, ldap_inst));
 
 	/* Start the watcher thread */
-	result = isc_thread_create(ldap_psearch_watcher, ldap_inst,
+	result = isc_thread_create(ldap_syncrepl_watcher, ldap_inst,
    &ldap_inst->watcher);
 	if (result != ISC_R_SUCCESS) {
-		log_error("Failed to create psearch watcher thread");
+		log_error("Failed to create syncrepl watcher thread");
 		goto cleanup;
 	}
 
@@ -665,8 +656,6 @@ new_ldap_connection(ldap_pool_t *pool, ldap_c