Attachment 15363 Details for Bug 14029 – Patch for 4.11 (with test)

[patch] Patch for 4.11 (with test)

patch-for-4-11.txt (text/plain), 11.45 KB, created by Garming Sam on 2019-08-01 22:57:36 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Garming Sam

Created: 2019-08-01 22:57:36 UTC

Size: 11.45 KB

patch

obsolete

>From fc2ebae7f4364fe73975d7af74c72ddedc071a06 Mon Sep 17 00:00:00 2001
>From: Garming Sam <garming@catalyst.net.nz>
>Date: Mon, 8 Jul 2019 16:59:33 +1200
>Subject: [PATCH 1/5] ldap_server: Regression in
> 0559430ab6e5c48d6e853fda0d8b63f2e149015c
>
>Extended DN requests seem to have been incorrectly handled.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029
>
>Signed-off-by: Garming Sam <garming@catalyst.net.nz>
>Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
>
>Autobuild-User(master): Gary Lockyer <gary@samba.org>
>Autobuild-Date(master): Thu Jul 11 05:25:26 UTC 2019 on sn-devel-184
>
>(cherry picked from commit 9f6b87d3f6cc9930d75c1f8d38ad4f5a37da34ab)
>---
> source4/ldap_server/ldap_backend.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source4/ldap_server/ldap_backend.c b/source4/ldap_server/ldap_backend.c
>index c6a65122ab0..bf724335a25 100644
>--- a/source4/ldap_server/ldap_backend.c
>+++ b/source4/ldap_server/ldap_backend.c
>@@ -826,6 +826,7 @@ static NTSTATUS ldapsrv_SearchRequest(struct ldapsrv_call *call)
> 		} else {
> 			extended_type = 0;
> 		}
>+		callback_ctx->extended_type = extended_type;
> 	}
> 
> 	notification_control = ldb_request_get_control(lreq, LDB_CONTROL_NOTIFICATION_OID);
>-- 
>2.17.1
>
>
>From 86957a756ec7910f3a511feb0b776737602c2fae Mon Sep 17 00:00:00 2001
>From: Garming Sam <garming@catalyst.net.nz>
>Date: Wed, 31 Jul 2019 01:08:23 +0000
>Subject: [PATCH 2/5] tldap: Make memcpy of no controls safe
>
>Static analyzers sometimes complain about this case.
>
>Signed-off-by: Garming Sam <garming@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029
>(cherry picked from commit e5452a37425484a95f90604a3e58e8a731460793)
>---
> source3/lib/tldap_util.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/source3/lib/tldap_util.c b/source3/lib/tldap_util.c
>index 152942dab2c..bdf8eb031a5 100644
>--- a/source3/lib/tldap_util.c
>+++ b/source3/lib/tldap_util.c
>@@ -588,7 +588,9 @@ struct tldap_control *tldap_add_control(TALLOC_CTX *mem_ctx,
> 	if (result == NULL) {
> 		return NULL;
> 	}
>-	memcpy(result, ctrls, sizeof(struct tldap_control) * num_ctrls);
>+	if (num_ctrls > 0) {
>+		memcpy(result, ctrls, sizeof(struct tldap_control) * num_ctrls);
>+	}
> 	result[num_ctrls] = *ctrl;
> 	return result;
> }
>-- 
>2.17.1
>
>
>From a726b72fd9a1ab6dd419e6f6176ba0d0a03b8b2a Mon Sep 17 00:00:00 2001
>From: Garming Sam <garming@catalyst.net.nz>
>Date: Wed, 31 Jul 2019 13:39:13 +1200
>Subject: [PATCH 3/5] tldap: Paged searches fail when they get to the end
>
>The normal case hit the goto label, and should have just returned.
>
>Signed-off-by: Garming Sam <garming@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029
>(cherry picked from commit bff466943e01540b4d3210392e0fd5b1c882c0b9)
>---
> source3/lib/tldap_util.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source3/lib/tldap_util.c b/source3/lib/tldap_util.c
>index bdf8eb031a5..1b86962a32e 100644
>--- a/source3/lib/tldap_util.c
>+++ b/source3/lib/tldap_util.c
>@@ -810,7 +810,8 @@ static void tldap_search_paged_done(struct tevent_req *subreq)
> 	}
> 	tevent_req_set_callback(subreq, tldap_search_paged_done, req);
> 
>-  err:
>+	return;
>+err:
> 
> 	TALLOC_FREE(asn1);
> 	tevent_req_ldap_error(req, TLDAP_DECODING_ERROR);
>-- 
>2.17.1
>
>
>From 9fb2f77adfd1a1236983e9842eef822f9a8c0ee8 Mon Sep 17 00:00:00 2001
>From: Garming Sam <garming@catalyst.net.nz>
>Date: Wed, 31 Jul 2019 15:29:07 +1200
>Subject: [PATCH 4/5] tests/tldap: Actually check the paging return code
>
>The test never worked correctly because the code was overlooked. It was
>also the case that the connection was never authenticated, and so an
>LDAP BIND call has now been added.
>
>Signed-off-by: Garming Sam <garming@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029
>(cherry picked from commit 85a7b594c56f7729bdfa194fee9299a08f6b4785)
>---
> source3/torture/torture.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
>diff --git a/source3/torture/torture.c b/source3/torture/torture.c
>index 2cb32efea46..20a7459b4db 100644
>--- a/source3/torture/torture.c
>+++ b/source3/torture/torture.c
>@@ -11286,6 +11286,8 @@ static bool run_shortname_test(int dummy)
> 	return correct;
> }
> 
>+TLDAPRC callback_code;
>+
> static void pagedsearch_cb(struct tevent_req *req)
> {
> 	TLDAPRC rc;
>@@ -11296,6 +11298,7 @@ static void pagedsearch_cb(struct tevent_req *req)
> 	if (!TLDAP_RC_IS_SUCCESS(rc)) {
> 		d_printf("tldap_search_paged_recv failed: %s\n",
> 			 tldap_rc2string(rc));
>+		callback_code = rc;
> 		return;
> 	}
> 	if (tldap_msg_type(msg) != TLDAP_RES_SEARCH_ENTRY) {
>@@ -11360,6 +11363,18 @@ static bool run_tldap(int dummy)
> 		return false;
> 	}
> 
>+	rc = tldap_gensec_bind(ld, torture_creds, "ldap", host, NULL,
>+			       loadparm_init_s3(talloc_tos(),
>+						loadparm_s3_helpers()),
>+			       GENSEC_FEATURE_SIGN | GENSEC_FEATURE_SEAL);
>+
>+	if (!TLDAP_RC_IS_SUCCESS(rc)) {
>+		d_printf("tldap_gensec_bind failed\n");
>+		return false;
>+	}
>+
>+	callback_code = TLDAP_SUCCESS;
>+
> 	req = tldap_search_paged_send(talloc_tos(), ev, ld, basedn,
> 				      TLDAP_SCOPE_SUB, "(objectclass=*)",
> 				      NULL, 0, 0,
>@@ -11374,6 +11389,14 @@ static bool run_tldap(int dummy)
> 
> 	TALLOC_FREE(req);
> 
>+	rc = callback_code;
>+
>+	if (!TLDAP_RC_IS_SUCCESS(rc)) {
>+		d_printf("tldap_search with paging failed: %s\n",
>+			 tldap_errstr(talloc_tos(), ld, rc));
>+		return false;
>+	}
>+
> 	/* test search filters against rootDSE */
> 	filter = "(&(|(name=samba)(nextRid<=10000000)(usnChanged>=10)(samba~=ambas)(!(name=s*m*a)))"
> 		   "(|(name:=samba)(name:dn:2.5.13.5:=samba)(:dn:2.5.13.5:=samba)(!(name=*samba))))";
>-- 
>2.17.1
>
>
>From 0b16851dc6d1a31ed05924bb0ca41b9bb2b43db1 Mon Sep 17 00:00:00 2001
>From: Garming Sam <garming@catalyst.net.nz>
>Date: Wed, 31 Jul 2019 01:14:42 +0000
>Subject: [PATCH 5/5] tests/ldap: Use TLDAP to check the extended DN return
>
>Tests commit 9f6b87d3f6cc9930d75c1f8d38ad4f5a37da34ab
>
>To run: make test TESTS="samba3.smbtorture_s3.plain.TLDAP"
>
>Reverting the above commit makes this test fail:
>
>'GUID format in control (no hyphens) doesn't match output
>tldap_search with extended dn (no val) failed: LDAP error 0 (TLDAP_SUCCESS),
>TEST TLDAP FAILED!'
>
>This behaviour couldn't be tested via LDB libraries because they never
>deal with the underlying DN string.
>
>Signed-off-by: Garming Sam <garming@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029
>
>Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
>Autobuild-Date(master): Thu Aug  1 06:20:28 UTC 2019 on sn-devel-184
>
>(adapted from commit 464fef34d1d047d73be347cd446b74e0f5eb2370)
>---
> source3/torture/torture.c | 155 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 155 insertions(+)
>
>diff --git a/source3/torture/torture.c b/source3/torture/torture.c
>index 20a7459b4db..f26c634b7a7 100644
>--- a/source3/torture/torture.c
>+++ b/source3/torture/torture.c
>@@ -26,6 +26,7 @@
> #include "libcli/security/security.h"
> #include "tldap.h"
> #include "tldap_util.h"
>+#include "tldap_gensec_bind.h"
> #include "../librpc/gen_ndr/svcctl.h"
> #include "../lib/util/memcache.h"
> #include "nsswitch/winbind_client.h"
>@@ -45,6 +46,9 @@
> #include "lib/util/base64.h"
> #include "lib/util/time.h"
> #include "lib/gencache.h"
>+#include "lib/util/asn1.h"
>+#include "lib/param/param.h"
>+#include "auth/gensec/gensec.h"
> 
> #include <gnutls/gnutls.h>
> #include <gnutls/crypto.h>
>@@ -11313,6 +11317,134 @@ static void pagedsearch_cb(struct tevent_req *req)
> 	TALLOC_FREE(msg);
> }
> 
>+enum tldap_extended_val {
>+	EXTENDED_ZERO = 0,
>+	EXTENDED_ONE = 1,
>+	EXTENDED_NONE = 2,
>+};
>+
>+/*
>+ * Construct an extended dn control with either no value, 0 or 1
>+ *
>+ * No value and 0 are equivalent (non-hyphenated GUID)
>+ * 1 has the hyphenated GUID
>+ */
>+static struct tldap_control *
>+tldap_build_extended_control(enum tldap_extended_val val)
>+{
>+	struct tldap_control empty_control;
>+	struct asn1_data *data;
>+
>+	ZERO_STRUCT(empty_control);
>+
>+	if (val != EXTENDED_NONE) {
>+		data = asn1_init(talloc_tos());
>+
>+		if (!data) {
>+			return NULL;
>+		}
>+
>+		if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) {
>+			return NULL;
>+		}
>+
>+		if (!asn1_write_Integer(data, (int)val)) {
>+			return NULL;
>+		}
>+
>+		if (!asn1_pop_tag(data)) {
>+			return NULL;
>+		}
>+
>+		if (!asn1_blob(data, &empty_control.value)) {
>+			return NULL;
>+		}
>+	}
>+
>+	empty_control.oid = "1.2.840.113556.1.4.529";
>+	empty_control.critical = true;
>+
>+	return tldap_add_control(talloc_tos(), NULL, 0, &empty_control);
>+
>+}
>+
>+static bool tldap_test_dn_guid_format(struct tldap_context *ld, const char *basedn,
>+				      enum tldap_extended_val control_val)
>+{
>+	struct tldap_control *control = tldap_build_extended_control(control_val);
>+	char *dn = NULL;
>+	struct tldap_message **msg;
>+	TLDAPRC rc;
>+
>+	rc = tldap_search(ld, basedn, TLDAP_SCOPE_BASE,
>+			  "(objectClass=*)", NULL, 0, 0,
>+			  control, 1, NULL,
>+			  0, 0, 0, 0, talloc_tos(), &msg);
>+	if (!TLDAP_RC_IS_SUCCESS(rc)) {
>+		d_printf("tldap_search for domain DN failed: %s\n",
>+			 tldap_errstr(talloc_tos(), ld, rc));
>+		return false;
>+	}
>+
>+	if (!tldap_entry_dn(msg[0], &dn)) {
>+		d_printf("tldap_search domain DN fetch failed: %s\n",
>+			 tldap_errstr(talloc_tos(), ld, rc));
>+		return false;
>+	}
>+
>+	d_printf("%s\n", dn);
>+	{
>+		uint32_t time_low;
>+		uint32_t time_mid, time_hi_and_version;
>+		uint32_t clock_seq[2];
>+		uint32_t node[6];
>+		char next;
>+
>+		switch (control_val) {
>+		case EXTENDED_NONE:
>+		case EXTENDED_ZERO:
>+			/*
>+			 * When reading GUIDs with hyphens, scanf will treat
>+			 * hyphen as a hex character (and counts as part of the
>+			 * width). This creates leftover GUID string which we
>+			 * check will for with 'next' and closing '>'.
>+			 */
>+			if (12 == sscanf(dn, "<GUID=%08x%04x%04x%02x%02x%02x%02x%02x%02x%02x%02x>%c",
>+					 &time_low, &time_mid,
>+					 &time_hi_and_version, &clock_seq[0],
>+					 &clock_seq[1], &node[0], &node[1],
>+					 &node[2], &node[3], &node[4],
>+					 &node[5], &next)) {
>+				/* This GUID is good */
>+			} else {
>+				d_printf("GUID format in control (no hyphens) doesn't match output\n");
>+				return false;
>+			}
>+
>+			break;
>+		case EXTENDED_ONE:
>+			if (12 == sscanf(dn,
>+					 "<GUID=%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x>%c",
>+					 &time_low, &time_mid,
>+					 &time_hi_and_version, &clock_seq[0],
>+					 &clock_seq[1], &node[0], &node[1],
>+					 &node[2], &node[3], &node[4],
>+					 &node[5], &next)) {
>+				/* This GUID is good */
>+			} else {
>+				d_printf("GUID format in control (with hyphens) doesn't match output\n");
>+				return false;
>+			}
>+
>+			break;
>+		default:
>+			return false;
>+		}
>+	}
>+
>+	return true;
>+}
>+
> static bool run_tldap(int dummy)
> {
> 	struct tldap_context *ld;
>@@ -11410,6 +11542,29 @@ static bool run_tldap(int dummy)
> 		return false;
> 	}
> 
>+	/*
>+	 * Tests to check for regression of:
>+	 *
>+	 * https://bugzilla.samba.org/show_bug.cgi?id=14029
>+	 *
>+	 * TLDAP used here to pick apart the original string DN (with GUID)
>+	 */
>+	if (!tldap_test_dn_guid_format(ld, basedn, EXTENDED_NONE)) {
>+		d_printf("tldap_search with extended dn (no val) failed: %s\n",
>+			 tldap_errstr(talloc_tos(), ld, rc));
>+		return false;
>+	}
>+	if (!tldap_test_dn_guid_format(ld, basedn, EXTENDED_ZERO)) {
>+		d_printf("tldap_search with extended dn (0) failed: %s\n",
>+			 tldap_errstr(talloc_tos(), ld, rc));
>+		return false;
>+	}
>+	if (!tldap_test_dn_guid_format(ld, basedn, EXTENDED_ONE)) {
>+		d_printf("tldap_search with extended dn (1) failed: %s\n",
>+			 tldap_errstr(talloc_tos(), ld, rc));
>+		return false;
>+	}
>+
> 	TALLOC_FREE(ld);
> 	return true;
> }
>-- 
>2.17.1
>

Flags:

abartlet: review+

Actions: View

Attachments on bug 14029: 15293 | 15314 | 15363