[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Thu Aug 1 06:21:02 UTC 2019


The branch, master has been updated
       via  464fef34d1d tests/ldap: Use TLDAP to check the extended DN return
       via  85a7b594c56 tests/tldap: Actually check the paging return code
       via  bff466943e0 tldap: Paged searches fail when they get to the end
       via  e5452a37425 tldap: Make memcpy of no controls safe
       via  1a7f2a230d5 dsdb: Quiet CID 1452117 1452119 1452114 (STRAY_SEMICOLON)
      from  0c001a7bf64 CID 1452121: dsdb/mod/partition: protect whole function with NULL check

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 464fef34d1d047d73be347cd446b74e0f5eb2370
Author: Garming Sam <garming at catalyst.net.nz>
Date:   Wed Jul 31 01:14:42 2019 +0000

    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 at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Thu Aug  1 06:20:28 UTC 2019 on sn-devel-184

commit 85a7b594c56f7729bdfa194fee9299a08f6b4785
Author: Garming Sam <garming at catalyst.net.nz>
Date:   Wed Jul 31 15:29:07 2019 +1200

    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 at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029

commit bff466943e01540b4d3210392e0fd5b1c882c0b9
Author: Garming Sam <garming at catalyst.net.nz>
Date:   Wed Jul 31 13:39:13 2019 +1200

    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 at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029

commit e5452a37425484a95f90604a3e58e8a731460793
Author: Garming Sam <garming at catalyst.net.nz>
Date:   Wed Jul 31 01:08:23 2019 +0000

    tldap: Make memcpy of no controls safe
    
    Static analyzers sometimes complain about this case.
    
    Signed-off-by: Garming Sam <garming at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029

commit 1a7f2a230d509df6f1f91bfea80c9b4b2c0df1bd
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Jul 31 12:08:58 2019 +1200

    dsdb: Quiet CID 1452117 1452119 1452114 (STRAY_SEMICOLON)
    
    Try to make clear what is being done here, we are trying to count the partitions so that
    we can then walk them in reverse.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>

-----------------------------------------------------------------------

Summary of changes:
 source3/lib/tldap_util.c                   |   7 +-
 source3/torture/torture.c                  | 178 +++++++++++++++++++++++++++++
 source4/dsdb/samdb/ldb_modules/partition.c |  15 ++-
 3 files changed, 195 insertions(+), 5 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/lib/tldap_util.c b/source3/lib/tldap_util.c
index 152942dab2c..1b86962a32e 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;
 }
@@ -808,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);
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 17020328eaf..ad6b3458f3c 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"
@@ -46,6 +47,9 @@
 #include "lib/util/time.h"
 #include "lib/gencache.h"
 #include "lib/util/sys_rw.h"
+#include "lib/util/asn1.h"
+#include "lib/param/param.h"
+#include "auth/gensec/gensec.h"
 
 #include <gnutls/gnutls.h>
 #include <gnutls/crypto.h>
@@ -11555,6 +11559,8 @@ static bool run_shortname_test(int dummy)
 	return correct;
 }
 
+TLDAPRC callback_code;
+
 static void pagedsearch_cb(struct tevent_req *req)
 {
 	TLDAPRC rc;
@@ -11565,6 +11571,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) {
@@ -11579,6 +11586,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;
@@ -11629,6 +11764,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,
@@ -11643,6 +11790,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))))";
@@ -11656,6 +11811,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;
 }
diff --git a/source4/dsdb/samdb/ldb_modules/partition.c b/source4/dsdb/samdb/ldb_modules/partition.c
index 6a65726c181..6b0fbe728bc 100644
--- a/source4/dsdb/samdb/ldb_modules/partition.c
+++ b/source4/dsdb/samdb/ldb_modules/partition.c
@@ -1185,7 +1185,10 @@ int partition_end_trans(struct ldb_module *module)
 	 * partition_start_trans. See comment in that function for detail.
 	 */
 	if (data && data->partitions) {
-		for (i=0; data->partitions[i]; i++);;
+		/* Just counting the partitions */
+		for (i=0; data->partitions[i]; i++) {}
+
+		/* now walk them backwards */
 		for (i--; i>=0; i--) {
 			struct dsdb_partition *p = data->partitions[i];
 			if (trace) {
@@ -1241,7 +1244,10 @@ int partition_del_trans(struct ldb_module *module)
 	 * partition_start_trans. See comment in that function for detail.
 	 */
 	if (data->partitions) {
-		for (i=0; data->partitions[i]; i++);;
+		/* Just counting the partitions */
+		for (i=0; data->partitions[i]; i++) {}
+
+		/* now walk them backwards */
 		for (i--; i>=0; i--) {
 			struct dsdb_partition *p = data->partitions[i];
 			if (trace) {
@@ -1595,7 +1601,10 @@ int partition_read_unlock(struct ldb_module *module)
 	 * partition_start_trans. See comment in that function for detail.
 	 */
 	if (data && data->partitions) {
-		for (i=0; data->partitions[i]; i++);;
+		/* Just counting the partitions */
+		for (i=0; data->partitions[i]; i++) {}
+
+		/* now walk them backwards */
 		for (i--; i>=0; i--) {
 			struct dsdb_partition *p = data->partitions[i];
 			if (trace) {


-- 
Samba Shared Repository



More information about the samba-cvs mailing list