From 3ca77e3edc0ba2c9dd3f2c0394f8c2f799d989b9 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 5 May 2020 12:54:59 +1200
Subject: [PATCH 01/22] CVE-2020-10730: vlv: Use strcmp(), not strncmp()
 checking the NULL terminated control OIDs

The end result is the same, as sizeof() includes the trailing NUL, but this
avoids having to think about that.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
 source4/dsdb/samdb/ldb_modules/vlv_pagination.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
index 980177cb05e..31e64b4bd78 100644
--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
@@ -682,8 +682,8 @@ vlv_copy_down_controls(TALLOC_CTX *mem_ctx, struct ldb_control **controls)
 		if (control->oid == NULL) {
 			break;
 		}
-		if (strncmp(control->oid, LDB_CONTROL_VLV_REQ_OID, sizeof(LDB_CONTROL_VLV_REQ_OID)) == 0 ||
-		    strncmp(control->oid, LDB_CONTROL_SERVER_SORT_OID, sizeof(LDB_CONTROL_SERVER_SORT_OID)) == 0) {
+		if (strcmp(control->oid, LDB_CONTROL_VLV_REQ_OID) == 0 ||
+		    strcmp(control->oid, LDB_CONTROL_SERVER_SORT_OID) == 0) {
 			continue;
 		}
 		new_controls[j] = talloc_steal(new_controls, control);
-- 
2.17.1


From c745a495415d77fd3ebfb8a658a1dad7cce114a5 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 5 May 2020 12:55:57 +1200
Subject: [PATCH 02/22] CVE-2020-10730: vlv: Do not re-ASQ search the results
 of an ASQ search with VLV

This is a silly combination, but at least try and keep the results sensible
and avoid a double-dereference.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
 source4/dsdb/samdb/ldb_modules/vlv_pagination.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
index 31e64b4bd78..d58a62482c9 100644
--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
@@ -682,10 +682,21 @@ vlv_copy_down_controls(TALLOC_CTX *mem_ctx, struct ldb_control **controls)
 		if (control->oid == NULL) {
 			break;
 		}
+		/*
+		 * Do not re-use VLV, nor the server-sort, both are
+		 * already handled here.
+		 */
 		if (strcmp(control->oid, LDB_CONTROL_VLV_REQ_OID) == 0 ||
 		    strcmp(control->oid, LDB_CONTROL_SERVER_SORT_OID) == 0) {
 			continue;
 		}
+		/*
+		 * ASQ changes everything, do not copy it down for the
+		 * per-GUID search
+		 */
+		if (strcmp(control->oid, LDB_CONTROL_ASQ_OID) == 0) {
+			continue;
+		}
 		new_controls[j] = talloc_steal(new_controls, control);
 		j++;
 	}
-- 
2.17.1


From 9d4b98d75ea6a283afc45e9e4aef91fdfb95a189 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 5 May 2020 13:16:48 +1200
Subject: [PATCH 03/22] CVE-2020-10730: selftest: Add test to confirm VLV
 interaction with ASQ

Tested against Windows 1709.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
 source4/dsdb/tests/python/asq.py | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/source4/dsdb/tests/python/asq.py b/source4/dsdb/tests/python/asq.py
index a32c9f40cd3..1c93a45f131 100644
--- a/source4/dsdb/tests/python/asq.py
+++ b/source4/dsdb/tests/python/asq.py
@@ -162,6 +162,33 @@ class ASQLDAPTest(samba.tests.TestCase):
                 self.assertIn(ldb.Dn(self.ldb, str(group)),
                               self.members)
 
+    def test_asq_vlv(self):
+        """Testing ASQ behaviour with VLV set.
+
+        ASQ is very strange, it turns a BASE search into a search for
+        all the objects pointed to by the specified attribute,
+        returning multiple entries!
+
+        """
+
+        sort_control = "server_sort:1:0:cn"
+
+        msgs = self.ldb.search(base=self.top_dn,
+                               scope=ldb.SCOPE_BASE,
+                               attrs=["objectGUID", "cn", "member"],
+                               controls=["asq:1:member",
+                                         sort_control,
+                                         "vlv:1:20:20:11:0"])
+
+        self.assertEqual(len(msgs), 20)
+
+        for msg in msgs:
+            self.assertNotEqual(msg.dn, self.top_dn)
+            self.assertIn(msg.dn, self.members2)
+            for group in msg["member"]:
+                self.assertIn(ldb.Dn(self.ldb, str(group)),
+                              self.members)
+
 if "://" not in url:
     if os.path.isfile(url):
         url = "tdb://%s" % url
-- 
2.17.1


From 80144c53d5546314f929b48f40c704f7cff083a8 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 5 May 2020 16:34:11 +1200
Subject: [PATCH 04/22] CVE-2020-10730: vlv: Another workaround for mixing ASQ
 and VLV

This is essentially an alternative patch, but without the correct
behaviour.  Instead this just avoids a segfault.

Included in case we have something simialr again in
another module.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
 .../dsdb/samdb/ldb_modules/vlv_pagination.c   | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
index d58a62482c9..720b5e95638 100644
--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
@@ -442,10 +442,21 @@ static int vlv_results(struct vlv_context *ac)
 			ret = vlv_search_by_dn_guid(ac->module, ac, &result, guid,
 						    ac->req->op.search.attrs);
 
-			if (ret == LDAP_NO_SUCH_OBJECT) {
-				/* The thing isn't there, which we quietly
-				   ignore and go on to send an extra one
-				   instead. */
+			if (ret == LDAP_NO_SUCH_OBJECT
+			    || result->count != 1) {
+				/*
+				 * The thing isn't there, which we quietly
+				 * ignore and go on to send an extra one
+				 * instead.
+				 *
+				 * result->count == 0 or > 1 can only
+				 * happen if ASQ (which breaks all the
+				 * rules) is somehow invoked (as this
+				 * is a BASE search).
+				 *
+				 * (We skip the ASQ cookie for the
+				 * GUID searches)
+				 */
 				if (last_i < ac->store->num_entries - 1) {
 					last_i++;
 				}
-- 
2.17.1


From ca2be7c95bb3a76c125cc816a8fe23a5557b6e63 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Wed, 6 May 2020 16:19:01 +1200
Subject: [PATCH 05/22] CVE-2020-10730: selftest: Add test to show that VLV and
 paged_results are incompatible

As tested against Windows Server 1709

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
 source4/dsdb/tests/python/asq.py | 27 +++++++++++++++++++++++++++
 source4/dsdb/tests/python/vlv.py | 23 +++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/source4/dsdb/tests/python/asq.py b/source4/dsdb/tests/python/asq.py
index 1c93a45f131..33973d66c37 100644
--- a/source4/dsdb/tests/python/asq.py
+++ b/source4/dsdb/tests/python/asq.py
@@ -189,6 +189,33 @@ class ASQLDAPTest(samba.tests.TestCase):
                 self.assertIn(ldb.Dn(self.ldb, str(group)),
                               self.members)
 
+    def test_asq_vlv_paged(self):
+        """Testing ASQ behaviour with VLV and paged_results set.
+
+        ASQ is very strange, it turns a BASE search into a search for
+        all the objects pointed to by the specified attribute,
+        returning multiple entries!
+
+        Thankfully combining both of these gives
+        unavailable-critical-extension against Windows 1709
+
+        """
+
+        sort_control = "server_sort:1:0:cn"
+
+        try:
+            msgs = self.ldb.search(base=self.top_dn,
+                                   scope=ldb.SCOPE_BASE,
+                                   attrs=["objectGUID", "cn", "member"],
+                                   controls=["asq:1:member",
+                                             sort_control,
+                                             "vlv:1:20:20:11:0",
+                                             "paged_results:1:1024"])
+            self.fail("should have failed with LDAP_UNAVAILABLE_CRITICAL_EXTENSION")
+        except ldb.LdbError as e:
+            (enum, estr) = e.args
+            self.assertEqual(enum, ldb.ERR_UNSUPPORTED_CRITICAL_EXTENSION)
+
 if "://" not in url:
     if os.path.isfile(url):
         url = "tdb://%s" % url
diff --git a/source4/dsdb/tests/python/vlv.py b/source4/dsdb/tests/python/vlv.py
index bc07a53d575..ce7aa213c36 100644
--- a/source4/dsdb/tests/python/vlv.py
+++ b/source4/dsdb/tests/python/vlv.py
@@ -1644,6 +1644,29 @@ class PagedResultsTests(TestsWithUserOU):
                                        page_size=len(self.users))
         self.assertEqual(results, set_2[ps*2:])
 
+    def test_vlv_paged(self):
+        """Testing behaviour with VLV and paged_results set.
+
+        A strange combination, certainly
+
+        Thankfully combining both of these gives
+        unavailable-critical-extension against Windows 1709
+
+        """
+        sort_control = "server_sort:1:0:cn"
+
+        try:
+            msgs = self.ldb.search(base=self.base_dn,
+                                   scope=ldb.SCOPE_SUBTREE,
+                                   attrs=["objectGUID", "cn", "member"],
+                                   controls=["vlv:1:20:20:11:0",
+                                             sort_control,
+                                             "paged_results:1:1024"])
+            self.fail("should have failed with LDAP_UNAVAILABLE_CRITICAL_EXTENSION")
+        except ldb.LdbError as e:
+            (enum, estr) = e.args
+            self.assertEqual(enum, ldb.ERR_UNSUPPORTED_CRITICAL_EXTENSION)
+
 
 if "://" not in host:
     if os.path.isfile(host):
-- 
2.17.1


From 04f059fa7ec543122e72f77d3fbd6e9cc45f6947 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Wed, 6 May 2020 17:05:30 +1200
Subject: [PATCH 06/22] CVE-2020-10730: dsdb: Fix crash when vlv and
 paged_results are combined

The GUID is not returned in the DN for some reason in this (to be banned)
combination.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
 source4/dsdb/samdb/ldb_modules/paged_results.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/source4/dsdb/samdb/ldb_modules/paged_results.c b/source4/dsdb/samdb/ldb_modules/paged_results.c
index dff247a9a55..c442a967792 100644
--- a/source4/dsdb/samdb/ldb_modules/paged_results.c
+++ b/source4/dsdb/samdb/ldb_modules/paged_results.c
@@ -416,6 +416,10 @@ static int paged_search_callback(struct ldb_request *req,
 
 		guid_blob = ldb_dn_get_extended_component(ares->message->dn,
 							  "GUID");
+		if (guid_blob == NULL) {
+			return ldb_module_done(ac->req, NULL, NULL,
+					       LDB_ERR_OPERATIONS_ERROR);
+		}
 		status = GUID_from_ndr_blob(guid_blob, &guid);
 		if (!NT_STATUS_IS_OK(status)) {
 			return ldb_module_done(ac->req, NULL, NULL,
-- 
2.17.1


From fff5a35ba4ee43c50691fc178c386dd2d9e460e7 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Wed, 6 May 2020 16:18:19 +1200
Subject: [PATCH 07/22] CVE-2020-10730: dsdb: Ban the combination of
 paged_results and VLV

This (two different paging controls) makes no sense and fails against
Windows Server 1709.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
 source4/dsdb/samdb/ldb_modules/paged_results.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/source4/dsdb/samdb/ldb_modules/paged_results.c b/source4/dsdb/samdb/ldb_modules/paged_results.c
index c442a967792..e9221099325 100644
--- a/source4/dsdb/samdb/ldb_modules/paged_results.c
+++ b/source4/dsdb/samdb/ldb_modules/paged_results.c
@@ -589,6 +589,7 @@ static int paged_search(struct ldb_module *module, struct ldb_request *req)
 {
 	struct ldb_context *ldb;
 	struct ldb_control *control;
+	struct ldb_control *vlv_control;
 	struct private_data *private_data;
 	struct ldb_paged_control *paged_ctrl;
 	struct ldb_request *search_req;
@@ -612,6 +613,15 @@ static int paged_search(struct ldb_module *module, struct ldb_request *req)
 	private_data = talloc_get_type(ldb_module_get_private(module),
 					struct private_data);
 
+	vlv_control = ldb_request_get_control(req, LDB_CONTROL_VLV_REQ_OID);
+	if (vlv_control != NULL) {
+		/*
+		 * VLV and paged_results are not allowed at the same
+		 * time
+		 */
+		return LDB_ERR_UNSUPPORTED_CRITICAL_EXTENSION;
+	}
+
 	ac = talloc_zero(req, struct paged_context);
 	if (ac == NULL) {
 		ldb_set_errstring(ldb, "Out of Memory");
-- 
2.17.1


From 9c5c2c845caf7f03a6b47abcd5e1bc0092d628c2 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Mon, 18 May 2020 12:36:57 +1200
Subject: [PATCH 08/22] CVE-2020-10730: s4 dsdb paged_results: Prevent repeat
 call of ldb_module_done

Check the return code from paged_results, if it is not LDB_SUCCESS
ldb_module_done has already been called, and SHOULD NOT be called again.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 .../dsdb/samdb/ldb_modules/paged_results.c    | 43 +++++++++++++++----
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/paged_results.c b/source4/dsdb/samdb/ldb_modules/paged_results.c
index e9221099325..c4b538f2208 100644
--- a/source4/dsdb/samdb/ldb_modules/paged_results.c
+++ b/source4/dsdb/samdb/ldb_modules/paged_results.c
@@ -237,14 +237,16 @@ static int paged_search_by_dn_guid(struct ldb_module *module,
 	return ret;
 }
 
-static int paged_results(struct paged_context *ac)
+static int paged_results(struct paged_context *ac, struct ldb_reply *ares)
 {
 	struct ldb_paged_control *paged;
 	unsigned int i, num_ctrls;
 	int ret;
 
 	if (ac->store == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 
 	while (ac->store->last_i < ac->store->num_entries && ac->size > 0) {
@@ -273,12 +275,17 @@ static int paged_results(struct paged_context *ac)
 			   instead. */
 			continue;
 		} else if (ret != LDB_SUCCESS) {
-			return ret;
+			return ldb_module_done(
+				ac->req, ac->controls, ares->response, ret);
 		}
 
 		ret = ldb_module_send_entry(ac->req, result->msgs[0],
 					    NULL);
 		if (ret != LDB_SUCCESS) {
+			/*
+			 * ldb_module_send_entry will have called
+			 * ldb_module_done if an error occurred.
+			 */
 			return ret;
 		}
 	}
@@ -289,6 +296,10 @@ static int paged_results(struct paged_context *ac)
 		*/
 		ret = send_referrals(ac->store, ac->req);
 		if (ret != LDB_SUCCESS) {
+			/*
+			 * send_referrals will have called ldb_module_done
+			 * if an error occurred.
+			 */
 			return ret;
 		}
 	}
@@ -305,7 +316,9 @@ static int paged_results(struct paged_context *ac)
 
 	ac->controls = talloc_array(ac, struct ldb_control *, num_ctrls +1);
 	if (ac->controls == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 	ac->controls[num_ctrls] = NULL;
 
@@ -316,20 +329,26 @@ static int paged_results(struct paged_context *ac)
 
 	ac->controls[i] = talloc(ac->controls, struct ldb_control);
 	if (ac->controls[i] == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 
 	ac->controls[i]->oid = talloc_strdup(ac->controls[i],
 						LDB_CONTROL_PAGED_RESULTS_OID);
 	if (ac->controls[i]->oid == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 
 	ac->controls[i]->critical = 0;
 
 	paged = talloc(ac->controls[i], struct ldb_paged_control);
 	if (paged == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 
 	ac->controls[i]->data = paged;
@@ -456,7 +475,13 @@ static int paged_search_callback(struct ldb_request *req,
 		store->result_array_size = store->num_entries;
 
 		ac->store->controls = talloc_move(ac->store, &ares->controls);
-		ret = paged_results(ac);
+		ret = paged_results(ac, ares);
+		if (ret != LDB_SUCCESS) {
+			/* paged_results will have called ldb_module_done
+			 * if an error occurred
+			 */
+			return ret;
+		}
 		return ldb_module_done(ac->req, ac->controls,
 					ares->response, ret);
 	}
@@ -769,7 +794,7 @@ static int paged_search(struct ldb_module *module, struct ldb_request *req)
 								LDB_SUCCESS);
 		}
 
-		ret = paged_results(ac);
+		ret = paged_results(ac, NULL);
 		if (ret != LDB_SUCCESS) {
 			return ldb_module_done(req, NULL, NULL, ret);
 		}
-- 
2.17.1


From d400d7d8ac66ab889c3f1a6572fec64f20f0861e Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Mon, 18 May 2020 12:37:39 +1200
Subject: [PATCH 09/22] CVE-2020-10730: s4 dsdb vlv_pagination: Prevent repeat
 call of ldb_module_done

Check the return code from vlv_results, if it is not LDB_SUCCESS
ldb_module_done has already been called, and SHOULD NOT be called again.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 .../dsdb/samdb/ldb_modules/vlv_pagination.c   | 61 +++++++++++++++----
 1 file changed, 49 insertions(+), 12 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
index 720b5e95638..b103bda5f52 100644
--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
@@ -387,7 +387,7 @@ static int vlv_calc_real_offset(int offset, int denominator, int n_entries)
    has been prepared earlier and saved -- or by vlv_search_callback() when a
    search has just been completed. */
 
-static int vlv_results(struct vlv_context *ac)
+static int vlv_results(struct vlv_context *ac, struct ldb_reply *ares)
 {
 	struct ldb_vlv_resp_control *vlv;
 	unsigned int num_ctrls;
@@ -397,7 +397,9 @@ static int vlv_results(struct vlv_context *ac)
 	int target = 0;
 
 	if (ac->store == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 
 	if (ac->store->first_ref) {
@@ -406,6 +408,10 @@ static int vlv_results(struct vlv_context *ac)
 		*/
 		ret = send_referrals(ac->store, ac->req);
 		if (ret != LDB_SUCCESS) {
+			/*
+			 * send_referrals will have called ldb_module_done
+			 * if there was an error.
+			 */
 			return ret;
 		}
 	}
@@ -419,14 +425,23 @@ static int vlv_results(struct vlv_context *ac)
 						    vlv_details,
 						    sort_details, &ret);
 			if (ret != LDB_SUCCESS) {
-				return ret;
+				return ldb_module_done(
+					ac->req,
+					ac->controls,
+					ares->response,
+					ret);
 			}
 		} else {
 			target = vlv_calc_real_offset(vlv_details->match.byOffset.offset,
 						      vlv_details->match.byOffset.contentCount,
 						      ac->store->num_entries);
 			if (target == -1) {
-				return LDB_ERR_OPERATIONS_ERROR;
+				ret = LDB_ERR_OPERATIONS_ERROR;
+				return ldb_module_done(
+					ac->req,
+					ac->controls,
+					ares->response,
+					ret);
 			}
 		}
 
@@ -462,12 +477,20 @@ static int vlv_results(struct vlv_context *ac)
 				}
 				continue;
 			} else if (ret != LDB_SUCCESS) {
-				return ret;
+				return ldb_module_done(
+					ac->req,
+					ac->controls,
+					ares->response,
+					ret);
 			}
 
 			ret = ldb_module_send_entry(ac->req, result->msgs[0],
 						    NULL);
 			if (ret != LDB_SUCCESS) {
+				/*
+				 * ldb_module_send_entry will have called
+				 * ldb_module_done if there was an error
+				 */
 				return ret;
 			}
 		}
@@ -488,7 +511,9 @@ static int vlv_results(struct vlv_context *ac)
 
 	ac->controls = talloc_array(ac, struct ldb_control *, num_ctrls + 1);
 	if (ac->controls == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 	ac->controls[num_ctrls] = NULL;
 
@@ -498,20 +523,26 @@ static int vlv_results(struct vlv_context *ac)
 
 	ac->controls[i] = talloc(ac->controls, struct ldb_control);
 	if (ac->controls[i] == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 
 	ac->controls[i]->oid = talloc_strdup(ac->controls[i],
 					     LDB_CONTROL_VLV_RESP_OID);
 	if (ac->controls[i]->oid == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 
 	ac->controls[i]->critical = 0;
 
 	vlv = talloc(ac->controls[i], struct ldb_vlv_resp_control);
 	if (vlv == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 	ac->controls[i]->data = vlv;
 
@@ -600,7 +631,13 @@ static int vlv_search_callback(struct ldb_request *req, struct ldb_reply *ares)
 		store->result_array_size = store->num_entries;
 
 		ac->store->controls = talloc_move(ac->store, &ares->controls);
-		ret = vlv_results(ac);
+		ret = vlv_results(ac, ares);
+		if (ret != LDB_SUCCESS) {
+			/* vlv_results will have called ldb_module_done
+			 * if there was an error.
+			 */
+			return ret;
+		}
 		return ldb_module_done(ac->req, ac->controls,
 					ares->response, ret);
 	}
@@ -845,9 +882,9 @@ static int vlv_search(struct ldb_module *module, struct ldb_request *req)
 			return ret;
 		}
 
-		ret = vlv_results(ac);
+		ret = vlv_results(ac, NULL);
 		if (ret != LDB_SUCCESS) {
-			return ldb_module_done(req, NULL, NULL, ret);
+			return ret;
 		}
 		return ldb_module_done(req, ac->controls, NULL,
 				       LDB_SUCCESS);
-- 
2.17.1


From d6848d2f3f281956d8401f11ed1a6b609f802e21 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Wed, 13 May 2020 10:56:56 +1200
Subject: [PATCH 10/22] CVE-2020-10730: lib ldb: Check if
 ldb_lock_backend_callback called twice

Prevent use after free issues if ldb_lock_backend_callback is called
twice, usually due to ldb_module_done being called twice. This can happen if a
module ignores the return value from function a function that calls
ldb_module_done as part of it's error handling.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 lib/ldb/common/ldb.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index 44a487ba987..090d41dde69 100644
--- a/lib/ldb/common/ldb.c
+++ b/lib/ldb/common/ldb.c
@@ -1009,6 +1009,13 @@ static int ldb_lock_backend_callback(struct ldb_request *req,
 	struct ldb_db_lock_context *lock_context;
 	int ret;
 
+	if (req->context == NULL) {
+		/*
+		 * The usual way to get here is to ignore the return codes
+		 * and continuing processing after an error.
+		 */
+		abort();
+	}
 	lock_context = talloc_get_type(req->context,
 				       struct ldb_db_lock_context);
 
@@ -1023,7 +1030,7 @@ static int ldb_lock_backend_callback(struct ldb_request *req,
 		 * If this is a LDB_REPLY_DONE or an error, unlock the
 		 * DB by calling the destructor on this context
 		 */
-		talloc_free(lock_context);
+		TALLOC_FREE(req->context);
 		return ret;
 	}
 
-- 
2.17.1


From ec11a9adbf989add9a77c68fe8175bd587400d4b Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Fri, 22 May 2020 10:53:29 +1200
Subject: [PATCH 11/22] ldb: Bump version to 1.5.8

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 lib/ldb/ABI/ldb-1.5.8.sigs            | 281 ++++++++++++++++++++++++++
 lib/ldb/ABI/pyldb-util-1.5.8.sigs     |   2 +
 lib/ldb/ABI/pyldb-util.py3-1.5.8.sigs |   2 +
 lib/ldb/wscript                       |   2 +-
 4 files changed, 286 insertions(+), 1 deletion(-)
 create mode 100644 lib/ldb/ABI/ldb-1.5.8.sigs
 create mode 100644 lib/ldb/ABI/pyldb-util-1.5.8.sigs
 create mode 100644 lib/ldb/ABI/pyldb-util.py3-1.5.8.sigs

diff --git a/lib/ldb/ABI/ldb-1.5.8.sigs b/lib/ldb/ABI/ldb-1.5.8.sigs
new file mode 100644
index 00000000000..9bf06ce6e93
--- /dev/null
+++ b/lib/ldb/ABI/ldb-1.5.8.sigs
@@ -0,0 +1,281 @@
+ldb_add: int (struct ldb_context *, const struct ldb_message *)
+ldb_any_comparison: int (struct ldb_context *, void *, ldb_attr_handler_t, const struct ldb_val *, const struct ldb_val *)
+ldb_asprintf_errstring: void (struct ldb_context *, const char *, ...)
+ldb_attr_casefold: char *(TALLOC_CTX *, const char *)
+ldb_attr_dn: int (const char *)
+ldb_attr_in_list: int (const char * const *, const char *)
+ldb_attr_list_copy: const char **(TALLOC_CTX *, const char * const *)
+ldb_attr_list_copy_add: const char **(TALLOC_CTX *, const char * const *, const char *)
+ldb_base64_decode: int (char *)
+ldb_base64_encode: char *(TALLOC_CTX *, const char *, int)
+ldb_binary_decode: struct ldb_val (TALLOC_CTX *, const char *)
+ldb_binary_encode: char *(TALLOC_CTX *, struct ldb_val)
+ldb_binary_encode_string: char *(TALLOC_CTX *, const char *)
+ldb_build_add_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_del_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_extended_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, const char *, void *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_mod_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_rename_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, struct ldb_dn *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_search_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, enum ldb_scope, const char *, const char * const *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_search_req_ex: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, enum ldb_scope, struct ldb_parse_tree *, const char * const *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_casefold: char *(struct ldb_context *, TALLOC_CTX *, const char *, size_t)
+ldb_casefold_default: char *(void *, TALLOC_CTX *, const char *, size_t)
+ldb_check_critical_controls: int (struct ldb_control **)
+ldb_comparison_binary: int (struct ldb_context *, void *, const struct ldb_val *, const struct ldb_val *)
+ldb_comparison_fold: int (struct ldb_context *, void *, const struct ldb_val *, const struct ldb_val *)
+ldb_connect: int (struct ldb_context *, const char *, unsigned int, const char **)
+ldb_control_to_string: char *(TALLOC_CTX *, const struct ldb_control *)
+ldb_controls_except_specified: struct ldb_control **(struct ldb_control **, TALLOC_CTX *, struct ldb_control *)
+ldb_debug: void (struct ldb_context *, enum ldb_debug_level, const char *, ...)
+ldb_debug_add: void (struct ldb_context *, const char *, ...)
+ldb_debug_end: void (struct ldb_context *, enum ldb_debug_level)
+ldb_debug_set: void (struct ldb_context *, enum ldb_debug_level, const char *, ...)
+ldb_delete: int (struct ldb_context *, struct ldb_dn *)
+ldb_dn_add_base: bool (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_add_base_fmt: bool (struct ldb_dn *, const char *, ...)
+ldb_dn_add_child: bool (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_add_child_fmt: bool (struct ldb_dn *, const char *, ...)
+ldb_dn_add_child_val: bool (struct ldb_dn *, const char *, struct ldb_val)
+ldb_dn_alloc_casefold: char *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_alloc_linearized: char *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_canonical_ex_string: char *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_canonical_string: char *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_check_local: bool (struct ldb_module *, struct ldb_dn *)
+ldb_dn_check_special: bool (struct ldb_dn *, const char *)
+ldb_dn_compare: int (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_compare_base: int (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_copy: struct ldb_dn *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_escape_value: char *(TALLOC_CTX *, struct ldb_val)
+ldb_dn_extended_add_syntax: int (struct ldb_context *, unsigned int, const struct ldb_dn_extended_syntax *)
+ldb_dn_extended_filter: void (struct ldb_dn *, const char * const *)
+ldb_dn_extended_syntax_by_name: const struct ldb_dn_extended_syntax *(struct ldb_context *, const char *)
+ldb_dn_from_ldb_val: struct ldb_dn *(TALLOC_CTX *, struct ldb_context *, const struct ldb_val *)
+ldb_dn_get_casefold: const char *(struct ldb_dn *)
+ldb_dn_get_comp_num: int (struct ldb_dn *)
+ldb_dn_get_component_name: const char *(struct ldb_dn *, unsigned int)
+ldb_dn_get_component_val: const struct ldb_val *(struct ldb_dn *, unsigned int)
+ldb_dn_get_extended_comp_num: int (struct ldb_dn *)
+ldb_dn_get_extended_component: const struct ldb_val *(struct ldb_dn *, const char *)
+ldb_dn_get_extended_linearized: char *(TALLOC_CTX *, struct ldb_dn *, int)
+ldb_dn_get_ldb_context: struct ldb_context *(struct ldb_dn *)
+ldb_dn_get_linearized: const char *(struct ldb_dn *)
+ldb_dn_get_parent: struct ldb_dn *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_get_rdn_name: const char *(struct ldb_dn *)
+ldb_dn_get_rdn_val: const struct ldb_val *(struct ldb_dn *)
+ldb_dn_has_extended: bool (struct ldb_dn *)
+ldb_dn_is_null: bool (struct ldb_dn *)
+ldb_dn_is_special: bool (struct ldb_dn *)
+ldb_dn_is_valid: bool (struct ldb_dn *)
+ldb_dn_map_local: struct ldb_dn *(struct ldb_module *, void *, struct ldb_dn *)
+ldb_dn_map_rebase_remote: struct ldb_dn *(struct ldb_module *, void *, struct ldb_dn *)
+ldb_dn_map_remote: struct ldb_dn *(struct ldb_module *, void *, struct ldb_dn *)
+ldb_dn_minimise: bool (struct ldb_dn *)
+ldb_dn_new: struct ldb_dn *(TALLOC_CTX *, struct ldb_context *, const char *)
+ldb_dn_new_fmt: struct ldb_dn *(TALLOC_CTX *, struct ldb_context *, const char *, ...)
+ldb_dn_remove_base_components: bool (struct ldb_dn *, unsigned int)
+ldb_dn_remove_child_components: bool (struct ldb_dn *, unsigned int)
+ldb_dn_remove_extended_components: void (struct ldb_dn *)
+ldb_dn_replace_components: bool (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_set_component: int (struct ldb_dn *, int, const char *, const struct ldb_val)
+ldb_dn_set_extended_component: int (struct ldb_dn *, const char *, const struct ldb_val *)
+ldb_dn_update_components: int (struct ldb_dn *, const struct ldb_dn *)
+ldb_dn_validate: bool (struct ldb_dn *)
+ldb_dump_results: void (struct ldb_context *, struct ldb_result *, FILE *)
+ldb_error_at: int (struct ldb_context *, int, const char *, const char *, int)
+ldb_errstring: const char *(struct ldb_context *)
+ldb_extended: int (struct ldb_context *, const char *, void *, struct ldb_result **)
+ldb_extended_default_callback: int (struct ldb_request *, struct ldb_reply *)
+ldb_filter_from_tree: char *(TALLOC_CTX *, const struct ldb_parse_tree *)
+ldb_get_config_basedn: struct ldb_dn *(struct ldb_context *)
+ldb_get_create_perms: unsigned int (struct ldb_context *)
+ldb_get_default_basedn: struct ldb_dn *(struct ldb_context *)
+ldb_get_event_context: struct tevent_context *(struct ldb_context *)
+ldb_get_flags: unsigned int (struct ldb_context *)
+ldb_get_opaque: void *(struct ldb_context *, const char *)
+ldb_get_root_basedn: struct ldb_dn *(struct ldb_context *)
+ldb_get_schema_basedn: struct ldb_dn *(struct ldb_context *)
+ldb_global_init: int (void)
+ldb_handle_get_event_context: struct tevent_context *(struct ldb_handle *)
+ldb_handle_new: struct ldb_handle *(TALLOC_CTX *, struct ldb_context *)
+ldb_handle_use_global_event_context: void (struct ldb_handle *)
+ldb_handler_copy: int (struct ldb_context *, void *, const struct ldb_val *, struct ldb_val *)
+ldb_handler_fold: int (struct ldb_context *, void *, const struct ldb_val *, struct ldb_val *)
+ldb_init: struct ldb_context *(TALLOC_CTX *, struct tevent_context *)
+ldb_ldif_message_redacted_string: char *(struct ldb_context *, TALLOC_CTX *, enum ldb_changetype, const struct ldb_message *)
+ldb_ldif_message_string: char *(struct ldb_context *, TALLOC_CTX *, enum ldb_changetype, const struct ldb_message *)
+ldb_ldif_parse_modrdn: int (struct ldb_context *, const struct ldb_ldif *, TALLOC_CTX *, struct ldb_dn **, struct ldb_dn **, bool *, struct ldb_dn **, struct ldb_dn **)
+ldb_ldif_read: struct ldb_ldif *(struct ldb_context *, int (*)(void *), void *)
+ldb_ldif_read_file: struct ldb_ldif *(struct ldb_context *, FILE *)
+ldb_ldif_read_file_state: struct ldb_ldif *(struct ldb_context *, struct ldif_read_file_state *)
+ldb_ldif_read_free: void (struct ldb_context *, struct ldb_ldif *)
+ldb_ldif_read_string: struct ldb_ldif *(struct ldb_context *, const char **)
+ldb_ldif_write: int (struct ldb_context *, int (*)(void *, const char *, ...), void *, const struct ldb_ldif *)
+ldb_ldif_write_file: int (struct ldb_context *, FILE *, const struct ldb_ldif *)
+ldb_ldif_write_redacted_trace_string: char *(struct ldb_context *, TALLOC_CTX *, const struct ldb_ldif *)
+ldb_ldif_write_string: char *(struct ldb_context *, TALLOC_CTX *, const struct ldb_ldif *)
+ldb_load_modules: int (struct ldb_context *, const char **)
+ldb_map_add: int (struct ldb_module *, struct ldb_request *)
+ldb_map_delete: int (struct ldb_module *, struct ldb_request *)
+ldb_map_init: int (struct ldb_module *, const struct ldb_map_attribute *, const struct ldb_map_objectclass *, const char * const *, const char *, const char *)
+ldb_map_modify: int (struct ldb_module *, struct ldb_request *)
+ldb_map_rename: int (struct ldb_module *, struct ldb_request *)
+ldb_map_search: int (struct ldb_module *, struct ldb_request *)
+ldb_match_message: int (struct ldb_context *, const struct ldb_message *, const struct ldb_parse_tree *, enum ldb_scope, bool *)
+ldb_match_msg: int (struct ldb_context *, const struct ldb_message *, const struct ldb_parse_tree *, struct ldb_dn *, enum ldb_scope)
+ldb_match_msg_error: int (struct ldb_context *, const struct ldb_message *, const struct ldb_parse_tree *, struct ldb_dn *, enum ldb_scope, bool *)
+ldb_match_msg_objectclass: int (const struct ldb_message *, const char *)
+ldb_mod_register_control: int (struct ldb_module *, const char *)
+ldb_modify: int (struct ldb_context *, const struct ldb_message *)
+ldb_modify_default_callback: int (struct ldb_request *, struct ldb_reply *)
+ldb_module_call_chain: char *(struct ldb_request *, TALLOC_CTX *)
+ldb_module_connect_backend: int (struct ldb_context *, const char *, const char **, struct ldb_module **)
+ldb_module_done: int (struct ldb_request *, struct ldb_control **, struct ldb_extended *, int)
+ldb_module_flags: uint32_t (struct ldb_context *)
+ldb_module_get_ctx: struct ldb_context *(struct ldb_module *)
+ldb_module_get_name: const char *(struct ldb_module *)
+ldb_module_get_ops: const struct ldb_module_ops *(struct ldb_module *)
+ldb_module_get_private: void *(struct ldb_module *)
+ldb_module_init_chain: int (struct ldb_context *, struct ldb_module *)
+ldb_module_load_list: int (struct ldb_context *, const char **, struct ldb_module *, struct ldb_module **)
+ldb_module_new: struct ldb_module *(TALLOC_CTX *, struct ldb_context *, const char *, const struct ldb_module_ops *)
+ldb_module_next: struct ldb_module *(struct ldb_module *)
+ldb_module_popt_options: struct poptOption **(struct ldb_context *)
+ldb_module_send_entry: int (struct ldb_request *, struct ldb_message *, struct ldb_control **)
+ldb_module_send_referral: int (struct ldb_request *, char *)
+ldb_module_set_next: void (struct ldb_module *, struct ldb_module *)
+ldb_module_set_private: void (struct ldb_module *, void *)
+ldb_modules_hook: int (struct ldb_context *, enum ldb_module_hook_type)
+ldb_modules_list_from_string: const char **(struct ldb_context *, TALLOC_CTX *, const char *)
+ldb_modules_load: int (const char *, const char *)
+ldb_msg_add: int (struct ldb_message *, const struct ldb_message_element *, int)
+ldb_msg_add_empty: int (struct ldb_message *, const char *, int, struct ldb_message_element **)
+ldb_msg_add_fmt: int (struct ldb_message *, const char *, const char *, ...)
+ldb_msg_add_linearized_dn: int (struct ldb_message *, const char *, struct ldb_dn *)
+ldb_msg_add_steal_string: int (struct ldb_message *, const char *, char *)
+ldb_msg_add_steal_value: int (struct ldb_message *, const char *, struct ldb_val *)
+ldb_msg_add_string: int (struct ldb_message *, const char *, const char *)
+ldb_msg_add_value: int (struct ldb_message *, const char *, const struct ldb_val *, struct ldb_message_element **)
+ldb_msg_canonicalize: struct ldb_message *(struct ldb_context *, const struct ldb_message *)
+ldb_msg_check_string_attribute: int (const struct ldb_message *, const char *, const char *)
+ldb_msg_copy: struct ldb_message *(TALLOC_CTX *, const struct ldb_message *)
+ldb_msg_copy_attr: int (struct ldb_message *, const char *, const char *)
+ldb_msg_copy_shallow: struct ldb_message *(TALLOC_CTX *, const struct ldb_message *)
+ldb_msg_diff: struct ldb_message *(struct ldb_context *, struct ldb_message *, struct ldb_message *)
+ldb_msg_difference: int (struct ldb_context *, TALLOC_CTX *, struct ldb_message *, struct ldb_message *, struct ldb_message **)
+ldb_msg_element_compare: int (struct ldb_message_element *, struct ldb_message_element *)
+ldb_msg_element_compare_name: int (struct ldb_message_element *, struct ldb_message_element *)
+ldb_msg_element_equal_ordered: bool (const struct ldb_message_element *, const struct ldb_message_element *)
+ldb_msg_find_attr_as_bool: int (const struct ldb_message *, const char *, int)
+ldb_msg_find_attr_as_dn: struct ldb_dn *(struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, const char *)
+ldb_msg_find_attr_as_double: double (const struct ldb_message *, const char *, double)
+ldb_msg_find_attr_as_int: int (const struct ldb_message *, const char *, int)
+ldb_msg_find_attr_as_int64: int64_t (const struct ldb_message *, const char *, int64_t)
+ldb_msg_find_attr_as_string: const char *(const struct ldb_message *, const char *, const char *)
+ldb_msg_find_attr_as_uint: unsigned int (const struct ldb_message *, const char *, unsigned int)
+ldb_msg_find_attr_as_uint64: uint64_t (const struct ldb_message *, const char *, uint64_t)
+ldb_msg_find_common_values: int (struct ldb_context *, TALLOC_CTX *, struct ldb_message_element *, struct ldb_message_element *, uint32_t)
+ldb_msg_find_duplicate_val: int (struct ldb_context *, TALLOC_CTX *, const struct ldb_message_element *, struct ldb_val **, uint32_t)
+ldb_msg_find_element: struct ldb_message_element *(const struct ldb_message *, const char *)
+ldb_msg_find_ldb_val: const struct ldb_val *(const struct ldb_message *, const char *)
+ldb_msg_find_val: struct ldb_val *(const struct ldb_message_element *, struct ldb_val *)
+ldb_msg_new: struct ldb_message *(TALLOC_CTX *)
+ldb_msg_normalize: int (struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, struct ldb_message **)
+ldb_msg_remove_attr: void (struct ldb_message *, const char *)
+ldb_msg_remove_element: void (struct ldb_message *, struct ldb_message_element *)
+ldb_msg_rename_attr: int (struct ldb_message *, const char *, const char *)
+ldb_msg_sanity_check: int (struct ldb_context *, const struct ldb_message *)
+ldb_msg_sort_elements: void (struct ldb_message *)
+ldb_next_del_trans: int (struct ldb_module *)
+ldb_next_end_trans: int (struct ldb_module *)
+ldb_next_init: int (struct ldb_module *)
+ldb_next_prepare_commit: int (struct ldb_module *)
+ldb_next_read_lock: int (struct ldb_module *)
+ldb_next_read_unlock: int (struct ldb_module *)
+ldb_next_remote_request: int (struct ldb_module *, struct ldb_request *)
+ldb_next_request: int (struct ldb_module *, struct ldb_request *)
+ldb_next_start_trans: int (struct ldb_module *)
+ldb_op_default_callback: int (struct ldb_request *, struct ldb_reply *)
+ldb_options_find: const char *(struct ldb_context *, const char **, const char *)
+ldb_pack_data: int (struct ldb_context *, const struct ldb_message *, struct ldb_val *)
+ldb_parse_control_from_string: struct ldb_control *(struct ldb_context *, TALLOC_CTX *, const char *)
+ldb_parse_control_strings: struct ldb_control **(struct ldb_context *, TALLOC_CTX *, const char **)
+ldb_parse_tree: struct ldb_parse_tree *(TALLOC_CTX *, const char *)
+ldb_parse_tree_attr_replace: void (struct ldb_parse_tree *, const char *, const char *)
+ldb_parse_tree_copy_shallow: struct ldb_parse_tree *(TALLOC_CTX *, const struct ldb_parse_tree *)
+ldb_parse_tree_walk: int (struct ldb_parse_tree *, int (*)(struct ldb_parse_tree *, void *), void *)
+ldb_qsort: void (void * const, size_t, size_t, void *, ldb_qsort_cmp_fn_t)
+ldb_register_backend: int (const char *, ldb_connect_fn, bool)
+ldb_register_extended_match_rule: int (struct ldb_context *, const struct ldb_extended_match_rule *)
+ldb_register_hook: int (ldb_hook_fn)
+ldb_register_module: int (const struct ldb_module_ops *)
+ldb_rename: int (struct ldb_context *, struct ldb_dn *, struct ldb_dn *)
+ldb_reply_add_control: int (struct ldb_reply *, const char *, bool, void *)
+ldb_reply_get_control: struct ldb_control *(struct ldb_reply *, const char *)
+ldb_req_get_custom_flags: uint32_t (struct ldb_request *)
+ldb_req_is_untrusted: bool (struct ldb_request *)
+ldb_req_location: const char *(struct ldb_request *)
+ldb_req_mark_trusted: void (struct ldb_request *)
+ldb_req_mark_untrusted: void (struct ldb_request *)
+ldb_req_set_custom_flags: void (struct ldb_request *, uint32_t)
+ldb_req_set_location: void (struct ldb_request *, const char *)
+ldb_request: int (struct ldb_context *, struct ldb_request *)
+ldb_request_add_control: int (struct ldb_request *, const char *, bool, void *)
+ldb_request_done: int (struct ldb_request *, int)
+ldb_request_get_control: struct ldb_control *(struct ldb_request *, const char *)
+ldb_request_get_status: int (struct ldb_request *)
+ldb_request_replace_control: int (struct ldb_request *, const char *, bool, void *)
+ldb_request_set_state: void (struct ldb_request *, int)
+ldb_reset_err_string: void (struct ldb_context *)
+ldb_save_controls: int (struct ldb_control *, struct ldb_request *, struct ldb_control ***)
+ldb_schema_attribute_add: int (struct ldb_context *, const char *, unsigned int, const char *)
+ldb_schema_attribute_add_with_syntax: int (struct ldb_context *, const char *, unsigned int, const struct ldb_schema_syntax *)
+ldb_schema_attribute_by_name: const struct ldb_schema_attribute *(struct ldb_context *, const char *)
+ldb_schema_attribute_fill_with_syntax: int (struct ldb_context *, TALLOC_CTX *, const char *, unsigned int, const struct ldb_schema_syntax *, struct ldb_schema_attribute *)
+ldb_schema_attribute_remove: void (struct ldb_context *, const char *)
+ldb_schema_attribute_remove_flagged: void (struct ldb_context *, unsigned int)
+ldb_schema_attribute_set_override_handler: void (struct ldb_context *, ldb_attribute_handler_override_fn_t, void *)
+ldb_schema_set_override_GUID_index: void (struct ldb_context *, const char *, const char *)
+ldb_schema_set_override_indexlist: void (struct ldb_context *, bool)
+ldb_search: int (struct ldb_context *, TALLOC_CTX *, struct ldb_result **, struct ldb_dn *, enum ldb_scope, const char * const *, const char *, ...)
+ldb_search_default_callback: int (struct ldb_request *, struct ldb_reply *)
+ldb_sequence_number: int (struct ldb_context *, enum ldb_sequence_type, uint64_t *)
+ldb_set_create_perms: void (struct ldb_context *, unsigned int)
+ldb_set_debug: int (struct ldb_context *, void (*)(void *, enum ldb_debug_level, const char *, va_list), void *)
+ldb_set_debug_stderr: int (struct ldb_context *)
+ldb_set_default_dns: void (struct ldb_context *)
+ldb_set_errstring: void (struct ldb_context *, const char *)
+ldb_set_event_context: void (struct ldb_context *, struct tevent_context *)
+ldb_set_flags: void (struct ldb_context *, unsigned int)
+ldb_set_modules_dir: void (struct ldb_context *, const char *)
+ldb_set_opaque: int (struct ldb_context *, const char *, void *)
+ldb_set_require_private_event_context: void (struct ldb_context *)
+ldb_set_timeout: int (struct ldb_context *, struct ldb_request *, int)
+ldb_set_timeout_from_prev_req: int (struct ldb_context *, struct ldb_request *, struct ldb_request *)
+ldb_set_utf8_default: void (struct ldb_context *)
+ldb_set_utf8_fns: void (struct ldb_context *, void *, char *(*)(void *, void *, const char *, size_t))
+ldb_setup_wellknown_attributes: int (struct ldb_context *)
+ldb_should_b64_encode: int (struct ldb_context *, const struct ldb_val *)
+ldb_standard_syntax_by_name: const struct ldb_schema_syntax *(struct ldb_context *, const char *)
+ldb_strerror: const char *(int)
+ldb_string_to_time: time_t (const char *)
+ldb_string_utc_to_time: time_t (const char *)
+ldb_timestring: char *(TALLOC_CTX *, time_t)
+ldb_timestring_utc: char *(TALLOC_CTX *, time_t)
+ldb_transaction_cancel: int (struct ldb_context *)
+ldb_transaction_cancel_noerr: int (struct ldb_context *)
+ldb_transaction_commit: int (struct ldb_context *)
+ldb_transaction_prepare_commit: int (struct ldb_context *)
+ldb_transaction_start: int (struct ldb_context *)
+ldb_unpack_data: int (struct ldb_context *, const struct ldb_val *, struct ldb_message *)
+ldb_unpack_data_only_attr_list: int (struct ldb_context *, const struct ldb_val *, struct ldb_message *, const char * const *, unsigned int, unsigned int *)
+ldb_unpack_data_only_attr_list_flags: int (struct ldb_context *, const struct ldb_val *, struct ldb_message *, const char * const *, unsigned int, unsigned int, unsigned int *)
+ldb_unpack_get_format: int (const struct ldb_val *, uint32_t *)
+ldb_val_dup: struct ldb_val (TALLOC_CTX *, const struct ldb_val *)
+ldb_val_equal_exact: int (const struct ldb_val *, const struct ldb_val *)
+ldb_val_map_local: struct ldb_val (struct ldb_module *, void *, const struct ldb_map_attribute *, const struct ldb_val *)
+ldb_val_map_remote: struct ldb_val (struct ldb_module *, void *, const struct ldb_map_attribute *, const struct ldb_val *)
+ldb_val_string_cmp: int (const struct ldb_val *, const char *)
+ldb_val_to_time: int (const struct ldb_val *, time_t *)
+ldb_valid_attr_name: int (const char *)
+ldb_vdebug: void (struct ldb_context *, enum ldb_debug_level, const char *, va_list)
+ldb_wait: int (struct ldb_handle *, enum ldb_wait_type)
diff --git a/lib/ldb/ABI/pyldb-util-1.5.8.sigs b/lib/ldb/ABI/pyldb-util-1.5.8.sigs
new file mode 100644
index 00000000000..74d6719d2bc
--- /dev/null
+++ b/lib/ldb/ABI/pyldb-util-1.5.8.sigs
@@ -0,0 +1,2 @@
+pyldb_Dn_FromDn: PyObject *(struct ldb_dn *)
+pyldb_Object_AsDn: bool (TALLOC_CTX *, PyObject *, struct ldb_context *, struct ldb_dn **)
diff --git a/lib/ldb/ABI/pyldb-util.py3-1.5.8.sigs b/lib/ldb/ABI/pyldb-util.py3-1.5.8.sigs
new file mode 100644
index 00000000000..74d6719d2bc
--- /dev/null
+++ b/lib/ldb/ABI/pyldb-util.py3-1.5.8.sigs
@@ -0,0 +1,2 @@
+pyldb_Dn_FromDn: PyObject *(struct ldb_dn *)
+pyldb_Object_AsDn: bool (TALLOC_CTX *, PyObject *, struct ldb_context *, struct ldb_dn **)
diff --git a/lib/ldb/wscript b/lib/ldb/wscript
index 0f760a9bc80..58240222d5f 100644
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 
 APPNAME = 'ldb'
-VERSION = '1.5.7'
+VERSION = '1.5.8'
 
 import sys, os
 
-- 
2.17.1


From 6253d590d2330eadd740fdde79551f9a50f4b52f Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Thu, 11 Jun 2020 17:38:51 +1200
Subject: [PATCH 12/22] CVE-2020-10745: pytests: hand-rolled invalid dns/nbt
 packet tests

The client libraries don't allow us to make packets that are broken in
certain ways, so we need to construct them as byte strings.

These tests all fail at present, proving the server is rendered
unresponsive, which is the crux of CVE-2020-10745.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>

(backported from patch for master)
[abartlet@samba.org: f"" strings are not in Python 3.4 and
bytes cannot be formatted in python 3.4]

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
---
 python/samba/tests/dns_packet.py | 210 +++++++++++++++++++++++++++++++
 selftest/knownfail.d/dns_packet  |   2 +
 source4/selftest/tests.py        |  10 ++
 3 files changed, 222 insertions(+)
 create mode 100644 python/samba/tests/dns_packet.py
 create mode 100644 selftest/knownfail.d/dns_packet

diff --git a/python/samba/tests/dns_packet.py b/python/samba/tests/dns_packet.py
new file mode 100644
index 00000000000..a9996664e57
--- /dev/null
+++ b/python/samba/tests/dns_packet.py
@@ -0,0 +1,210 @@
+# Tests of malformed DNS packets
+# Copyright (C) Catalyst.NET ltd
+#
+# written by Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Sanity tests for DNS and NBT server parsing.
+
+We don't use a proper client library so we can make improper packets.
+"""
+
+import os
+import struct
+import socket
+import select
+from samba.dcerpc import dns, nbt
+
+from samba.tests import TestCase
+
+def _msg_id():
+    while True:
+        for i in range(1, 0xffff):
+            yield i
+
+
+SERVER = os.environ['SERVER_IP']
+SERVER_NAME = os.environ['SERVER'] + "." + os.environ['REALM']
+TIMEOUT = 0.5
+
+
+def encode_netbios_bytes(chars):
+    """Even RFC 1002 uses distancing quotes when calling this "compression"."""
+    out = []
+    chars = (chars + b'                   ')[:16]
+    for c in chars:
+        out.append((c >> 4) + 65)
+        out.append((c & 15) + 65)
+    return bytes(out)
+
+
+class TestDnsPacketBase(TestCase):
+    msg_id = _msg_id()
+
+    def tearDown(self):
+        # we need to ensure the DNS server is responsive before
+        # continuing.
+        for i in range(40):
+            ok = self._known_good_query()
+            if ok:
+                return
+        print("the server is STILL unresponsive after %d seconds" % (40 * TIMEOUT))
+
+    def decode_reply(self, data):
+        header = data[:12]
+        id, flags, n_q, n_a, n_rec, n_exta = struct.unpack('!6H',
+                                                           header)
+        return {
+            'rcode': flags & 0xf
+        }
+
+    def construct_query(self, names):
+        """Create a query packet containing one query record.
+
+        *names* is either a single string name in the usual dotted
+        form, or a list of names. In the latter case, each name can
+        be a dotted string or a list of byte components, which allows
+        dots in components. Where I say list, I mean non-string
+        iterable.
+
+        Examples:
+
+        # these 3 are all the same
+        "example.com"
+        ["example.com"]
+        [[b"example", b"com"]]
+
+        # this is three names in the same request
+        ["example.com",
+         [b"example", b"com", b"..!"],
+         (b"first component", b" 2nd component")]
+        """
+        header = struct.pack('!6H',
+                             next(self.msg_id),
+                             0x0100,       # query, with recursion
+                             len(names),   # number of queries
+                             0x0000,       # no answers
+                             0x0000,       # no records
+                             0x0000,       # no extra records
+        )
+        tail = struct.pack('!BHH',
+                           0x00,         # root node
+                           self.qtype,
+                           0x0001,       # class IN-ternet
+        )
+        encoded_bits = []
+        for name in names:
+            if isinstance(name, str):
+                bits = name.encode('utf8').split(b'.')
+            else:
+                bits = name
+
+            for b in bits:
+                encoded_bits.append(bytes([len(b)]) +  b)
+            encoded_bits.append(tail)
+
+        return header + b''.join(encoded_bits)
+
+    def _test_query(self, names=(), expected_rcode=None):
+
+        if isinstance(names, str):
+            names = [names]
+
+        packet = self.construct_query(names)
+        s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+        s.sendto(packet, self.server)
+        r, _, _ = select.select([s], [], [], TIMEOUT)
+        s.close()
+        # It is reasonable to not reply to these packets (Windows
+        # doesn't), but it is not reasonable to render the server
+        # unresponsive.
+        if r != [s]:
+            ok = self._known_good_query()
+            self.assertTrue(ok, "the server is unresponsive")
+
+    def _known_good_query(self):
+        if self.server[1] == 53:
+            name = SERVER_NAME
+            expected_rcode = dns.DNS_RCODE_OK
+        else:
+            name = [encode_netbios_bytes(b'nxdomain'), b'nxdomain']
+            expected_rcode = nbt.NBT_RCODE_NAM
+
+        packet = self.construct_query([name])
+        s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+        s.sendto(packet, self.server)
+        r, _, _ = select.select([s], [], [], TIMEOUT)
+        if r != [s]:
+            s.close()
+            return False
+
+        data, addr = s.recvfrom(4096)
+        s.close()
+        rcode = self.decode_reply(data)['rcode']
+        return expected_rcode == rcode
+
+
+class TestDnsPackets(TestDnsPacketBase):
+    server = (SERVER, 53)
+    qtype = 1     # dns type A
+
+    def _test_many_repeated_components(self, label, n, expected_rcode=None):
+        name = [label] * n
+        self._test_query([name],
+                         expected_rcode=expected_rcode)
+
+    def test_127_very_dotty_components(self):
+        label = b'.' * 63
+        self._test_many_repeated_components(label, 127)
+
+    def test_127_half_dotty_components(self):
+        label = b'x.' * 31 + b'x'
+        self._test_many_repeated_components(label, 127)
+
+
+class TestNbtPackets(TestDnsPacketBase):
+    server = (SERVER, 137)
+    qtype = 0x20  # NBT_QTYPE_NETBIOS
+
+    def _test_nbt_encode_query(self, names, *args, **kwargs):
+        if isinstance(names, str):
+            names = [names]
+
+        nbt_names = []
+        for name in names:
+            if isinstance(name, str):
+                bits = name.encode('utf8').split(b'.')
+            else:
+                bits = name
+
+            encoded = [encode_netbios_bytes(bits[0])]
+            encoded.extend(bits[1:])
+            nbt_names.append(encoded)
+
+        self._test_query(nbt_names, *args, **kwargs)
+
+    def _test_many_repeated_components(self, label, n, expected_rcode=None):
+        name = [label] * n
+        name[0] = encode_netbios_bytes(label)
+        self._test_query([name],
+                         expected_rcode=expected_rcode)
+
+    def test_127_very_dotty_components(self):
+        label = b'.' * 63
+        self._test_many_repeated_components(label, 127)
+
+    def test_127_half_dotty_components(self):
+        label = b'x.' * 31 + b'x'
+        self._test_many_repeated_components(label, 127)
diff --git a/selftest/knownfail.d/dns_packet b/selftest/knownfail.d/dns_packet
new file mode 100644
index 00000000000..6e2e5a699de
--- /dev/null
+++ b/selftest/knownfail.d/dns_packet
@@ -0,0 +1,2 @@
+samba.tests.dns_packet.samba.tests.dns_packet.TestDnsPackets.test_127_very_dotty_components
+samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 19c81b46ea4..23416df221c 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -416,6 +416,16 @@ plantestsuite_loadlist("samba.tests.dns_wildcard", "ad_dc", [python, os.path.joi
 
 plantestsuite_loadlist("samba.tests.dns_invalid", "ad_dc", [python, os.path.join(srcdir(), "python/samba/tests/dns_invalid.py"), '$SERVER_IP', '--machine-pass', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
 
+plantestsuite_loadlist("samba.tests.dns_packet",
+                       "ad_dc",
+                       [python,
+                        '-msamba.subunit.run',
+                        '$LOADLIST',
+                        "$LISTOPT"
+                        "samba.tests.dns_packet"
+                       ])
+
+
 for t in smbtorture4_testsuites("dns_internal."):
     plansmbtorture4testsuite(t, "ad_dc_ntvfs:local", '//$SERVER/whavever')
 
-- 
2.17.1


From b9ba23d41e0bc5f781e8a391f71b3be5f8017155 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Fri, 12 Jun 2020 14:26:38 +1200
Subject: [PATCH 13/22] CVE-2020-10745: librpc/tests: cmocka tests of dns and
 ndr strings

These time the push and pull function in isolation.

Timing should be under 0.0001 seconds on even quite old hardware; we
assert it must be under 0.2 seconds.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378

(backported from master commit)
[abartlet@samba.org: backported due to differences in pre-existing
tests - eg test_ndr - mentioned in wscript_build and tests.py]

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
---
 librpc/tests/test_ndr_dns_nbt.c  | 236 +++++++++++++++++++++++++++++++
 librpc/wscript_build             |  13 ++
 selftest/knownfail.d/ndr_dns_nbt |   4 +
 source4/selftest/tests.py        |   2 +
 4 files changed, 255 insertions(+)
 create mode 100644 librpc/tests/test_ndr_dns_nbt.c
 create mode 100644 selftest/knownfail.d/ndr_dns_nbt

diff --git a/librpc/tests/test_ndr_dns_nbt.c b/librpc/tests/test_ndr_dns_nbt.c
new file mode 100644
index 00000000000..1e2ef45c10d
--- /dev/null
+++ b/librpc/tests/test_ndr_dns_nbt.c
@@ -0,0 +1,236 @@
+/*
+ * Tests for librpc ndr functions
+ *
+ * Copyright (C) Catalyst.NET Ltd 2020
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "replace.h"
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include "includes.h"
+#include "librpc/ndr/libndr.h"
+#include "librpc/gen_ndr/ndr_dns.h"
+#include "librpc/gen_ndr/ndr_nbt.h"
+#include "lib/util/time.h"
+
+#define NBT_NAME "EOGFGLGPCACACACACACACACACACACACA" /* "neko" */
+
+
+static DATA_BLOB generate_obnoxious_dns_name(TALLOC_CTX *mem_ctx,
+					     size_t n_labels,
+					     size_t dot_every,
+					     bool is_nbt)
+{
+	size_t i, j;
+	char *s;
+	DATA_BLOB name = data_blob_talloc(mem_ctx, NULL, 64 * n_labels + 1);
+	assert_non_null(name.data);
+
+	s = (char*)name.data;
+	if (is_nbt) {
+		size_t len = strlen(NBT_NAME);
+		*s = len;
+		s++;
+		memcpy(s, NBT_NAME, len);
+		s += len;
+		n_labels--;
+	}
+
+	for (i = 0; i < n_labels; i++) {
+		*s = 63;
+		s++;
+		for (j = 0; j < 63; j++) {
+			if (j % dot_every == (dot_every - 1)) {
+				*s = '.';
+			} else {
+				*s = 'x';
+			}
+			s++;
+		}
+	}
+	*s = 0;
+	s++;
+	name.length = s - (char*)name.data;
+	return name;
+}
+
+
+static char *_test_ndr_pull_dns_string_list(TALLOC_CTX *mem_ctx,
+					    size_t n_labels,
+					    size_t dot_every,
+					    bool is_nbt)
+{
+	enum ndr_err_code ndr_err;
+	DATA_BLOB blob = generate_obnoxious_dns_name(mem_ctx,
+						     n_labels,
+						     dot_every,
+						     is_nbt);
+
+	char *name;
+	ndr_pull_flags_fn_t fn;
+
+	if (is_nbt) {
+		fn = (ndr_pull_flags_fn_t)ndr_pull_nbt_string;
+	} else {
+		fn = (ndr_pull_flags_fn_t)ndr_pull_dns_string;
+	}
+
+	ndr_err = ndr_pull_struct_blob(&blob,
+				       mem_ctx,
+				       &name,
+				       fn);
+	/* Success here is not expected, but we let it go to measure timing. */
+	if (ndr_err == NDR_ERR_SUCCESS) {
+		printf("pull succeed\n");
+	} else {
+		assert_int_equal(ndr_err, NDR_ERR_STRING);
+	}
+
+	TALLOC_FREE(blob.data);
+	return name;
+}
+
+
+static void _test_ndr_push_dns_string_list(TALLOC_CTX *mem_ctx,
+					   char *name,
+					   bool is_nbt)
+{
+	DATA_BLOB blob;
+	enum ndr_err_code ndr_err;
+	ndr_push_flags_fn_t fn;
+
+	if (is_nbt) {
+		fn = (ndr_push_flags_fn_t)ndr_push_nbt_string;
+	} else {
+		fn = (ndr_push_flags_fn_t)ndr_push_dns_string;
+	}
+
+	ndr_err = ndr_push_struct_blob(&blob,
+				       mem_ctx,
+				       name,
+				       fn);
+
+	/* Success here is not expected, but we let it go to measure timing. */
+	if (ndr_err == NDR_ERR_SUCCESS) {
+		printf("push succeed\n");
+	} else {
+		assert_int_equal(ndr_err, NDR_ERR_STRING);
+	}
+}
+
+
+static uint64_t elapsed_time(struct timespec start, const char *print)
+{
+	struct timespec end;
+	unsigned long long microsecs;
+	clock_gettime_mono(&end);
+	end.tv_sec -= start.tv_sec;
+	if (end.tv_nsec < start.tv_nsec) {
+		/* we need to borrow */
+		end.tv_nsec += 1000 * 1000 * 1000;
+		end.tv_sec -= 1;
+	}
+	end.tv_nsec -= start.tv_nsec;
+	microsecs = end.tv_sec * 1000000;
+	microsecs += end.tv_nsec / 1000;
+
+	if (print != NULL) {
+		printf(" %s: %llu microseconds\n", print, microsecs);
+	}
+	return microsecs;
+}
+
+
+static void test_ndr_dns_string_half_dots(void **state)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(NULL);
+	char *name;
+	struct timespec start;
+	uint64_t elapsed;
+
+	clock_gettime_mono(&start);
+	name =_test_ndr_pull_dns_string_list(mem_ctx, 127, 2, false);
+	elapsed_time(start, "pull");
+	_test_ndr_push_dns_string_list(mem_ctx, name, false);
+	elapsed = elapsed_time(start, "total");
+	assert_in_range(elapsed, 0, 200000);
+	talloc_free(mem_ctx);
+}
+
+static void test_ndr_nbt_string_half_dots(void **state)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(NULL);
+	char *name;
+	struct timespec start;
+	uint64_t elapsed;
+
+	clock_gettime_mono(&start);
+	name =_test_ndr_pull_dns_string_list(mem_ctx, 127, 2, true);
+	elapsed_time(start, "pull");
+	_test_ndr_push_dns_string_list(mem_ctx, name, true);
+	elapsed = elapsed_time(start, "total");
+	assert_in_range(elapsed, 0, 200000);
+	talloc_free(mem_ctx);
+}
+
+static void test_ndr_dns_string_all_dots(void **state)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(NULL);
+	char *name;
+	struct timespec start;
+	uint64_t elapsed;
+
+	clock_gettime_mono(&start);
+	name =_test_ndr_pull_dns_string_list(mem_ctx, 127, 1, false);
+	elapsed_time(start, "pull");
+	_test_ndr_push_dns_string_list(mem_ctx, name, false);
+	elapsed = elapsed_time(start, "total");
+	assert_in_range(elapsed, 0, 200000);
+	talloc_free(mem_ctx);
+}
+
+static void test_ndr_nbt_string_all_dots(void **state)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(NULL);
+	char *name;
+	struct timespec start;
+	uint64_t elapsed;
+
+	clock_gettime_mono(&start);
+	name =_test_ndr_pull_dns_string_list(mem_ctx, 127, 1, true);
+	elapsed_time(start, "pull");
+	_test_ndr_push_dns_string_list(mem_ctx, name, true);
+	elapsed = elapsed_time(start, "total");
+	assert_in_range(elapsed, 0, 200000);
+	talloc_free(mem_ctx);
+}
+
+
+
+int main(int argc, const char **argv)
+{
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(test_ndr_nbt_string_half_dots),
+		cmocka_unit_test(test_ndr_dns_string_half_dots),
+		cmocka_unit_test(test_ndr_nbt_string_all_dots),
+		cmocka_unit_test(test_ndr_dns_string_all_dots),
+	};
+
+	cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+	return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/librpc/wscript_build b/librpc/wscript_build
index 8e113c422b2..788efcf57a6 100644
--- a/librpc/wscript_build
+++ b/librpc/wscript_build
@@ -756,3 +756,16 @@ bld.SAMBA_SUBSYSTEM('NDR_FSRVP_STATE',
     source='gen_ndr/ndr_fsrvp_state.c',
     public_deps='ndr'
     )
+#
+# Cmocka tests
+#
+
+bld.SAMBA_BINARY('test_ndr_dns_nbt',
+                 source='tests/test_ndr_dns_nbt.c',
+                 deps='''
+                      cmocka
+                      ndr
+                      ndr_nbt
+                      NDR_DNS
+                      ''',
+                 install=False)
diff --git a/selftest/knownfail.d/ndr_dns_nbt b/selftest/knownfail.d/ndr_dns_nbt
new file mode 100644
index 00000000000..f30217c4033
--- /dev/null
+++ b/selftest/knownfail.d/ndr_dns_nbt
@@ -0,0 +1,4 @@
+librpc.ndr.ndr_dns_nbt.test_ndr_dns_string_all_dots
+librpc.ndr.ndr_dns_nbt.test_ndr_dns_string_half_dots
+librpc.ndr.ndr_dns_nbt.test_ndr_nbt_string_all_dots
+librpc.ndr.ndr_dns_nbt.test_ndr_nbt_string_half_dots
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 23416df221c..8cf54841e86 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -1303,6 +1303,8 @@ plantestsuite("samba4.dsdb.samdb.ldb_modules.group_audit.errors", "none",
               [os.path.join(bindir(), "test_group_audit_errors")])
 plantestsuite("samba4.dcerpc.dnsserver.dnsutils", "none",
               [os.path.join(bindir(), "test_rpc_dns_server_dnsutils")])
+plantestsuite("librpc.ndr.ndr_dns_nbt", "none",
+              [os.path.join(bindir(), "test_ndr_dns_nbt")])
 plantestsuite("libcli.ldap.ldap_message", "none",
               [os.path.join(bindir(), "test_ldap_message")])
 
-- 
2.17.1


From b8788a04ebd6ca9c2f9f72e11049d770ff54afac Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Sat, 25 Apr 2020 11:02:08 +1200
Subject: [PATCH 14/22] CVE-2020-10745: ndr_dns: move ndr_push_dns_string core
 into sharable function

This is because ndr_nbt.c does almost exactly the same thing with
almost exactly the same code, and they both do it wrong. Soon they
will both be using the better version that this will become. Though in
this patch we just move the code, not fix it.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
---
 librpc/ndr/ndr_dns.c       | 79 +++-------------------------------
 librpc/ndr/ndr_dns_utils.c | 88 ++++++++++++++++++++++++++++++++++++++
 librpc/ndr/ndr_dns_utils.h |  5 +++
 librpc/wscript_build       |  2 +-
 4 files changed, 99 insertions(+), 75 deletions(-)
 create mode 100644 librpc/ndr/ndr_dns_utils.c
 create mode 100644 librpc/ndr/ndr_dns_utils.h

diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c
index d37c8cc2ece..68a3c9de782 100644
--- a/librpc/ndr/ndr_dns.c
+++ b/librpc/ndr/ndr_dns.c
@@ -33,6 +33,7 @@
 #include "librpc/gen_ndr/ndr_dnsp.h"
 #include "system/locale.h"
 #include "lib/util/util_net.h"
+#include "ndr_dns_utils.h"
 
 /* don't allow an unlimited number of name components */
 #define MAX_COMPONENTS 128
@@ -159,80 +160,10 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr,
 					       int ndr_flags,
 					       const char *s)
 {
-	if (!(ndr_flags & NDR_SCALARS)) {
-		return NDR_ERR_SUCCESS;
-	}
-
-	while (s && *s) {
-		enum ndr_err_code ndr_err;
-		char *compname;
-		size_t complen;
-		uint32_t offset;
-
-		if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) {
-			/* see if we have pushed the remaining string already,
-			 * if so we use a label pointer to this string
-			 */
-			ndr_err = ndr_token_retrieve_cmp_fn(&ndr->dns_string_list, s,
-							    &offset,
-							    (comparison_fn_t)strcmp,
-							    false);
-			if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-				uint8_t b[2];
-
-				if (offset > 0x3FFF) {
-					return ndr_push_error(ndr, NDR_ERR_STRING,
-							      "offset for dns string " \
-							      "label pointer " \
-							      "%u[%08X] > 0x00003FFF",
-							      offset, offset);
-				}
-
-				b[0] = 0xC0 | (offset>>8);
-				b[1] = (offset & 0xFF);
-
-				return ndr_push_bytes(ndr, b, 2);
-			}
-		}
-
-		complen = strcspn(s, ".");
-
-		/* we need to make sure the length fits into 6 bytes */
-		if (complen > 0x3F) {
-			return ndr_push_error(ndr, NDR_ERR_STRING,
-					      "component length %u[%08X] > " \
-					      "0x0000003F",
-					      (unsigned)complen,
-					      (unsigned)complen);
-		}
-
-		compname = talloc_asprintf(ndr, "%c%*.*s",
-						(unsigned char)complen,
-						(unsigned char)complen,
-						(unsigned char)complen, s);
-		NDR_ERR_HAVE_NO_MEMORY(compname);
-
-		/* remember the current component + the rest of the string
-		 * so it can be reused later
-		 */
-		if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) {
-			NDR_CHECK(ndr_token_store(ndr, &ndr->dns_string_list, s,
-						  ndr->offset));
-		}
-
-		/* push just this component into the blob */
-		NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname,
-					 complen+1));
-		talloc_free(compname);
-
-		s += complen;
-		if (*s == '.') s++;
-	}
-
-	/* if we reach the end of the string and have pushed the last component
-	 * without using a label pointer, we need to terminate the string
-	 */
-	return ndr_push_bytes(ndr, (const uint8_t *)"", 1);
+	return ndr_push_dns_string_list(ndr,
+					&ndr->dns_string_list,
+					ndr_flags,
+					s);
 }
 
 _PUBLIC_ enum ndr_err_code ndr_pull_dns_txt_record(struct ndr_pull *ndr, int ndr_flags, struct dns_txt_record *r)
diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c
new file mode 100644
index 00000000000..2d9b5f1bc1e
--- /dev/null
+++ b/librpc/ndr/ndr_dns_utils.c
@@ -0,0 +1,88 @@
+#include "includes.h"
+#include "../librpc/ndr/libndr.h"
+#include "ndr_dns_utils.h"
+
+
+/**
+  push a dns/nbt string list to the wire
+*/
+enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
+					   struct ndr_token_list *string_list,
+					   int ndr_flags,
+					   const char *s)
+{
+	if (!(ndr_flags & NDR_SCALARS)) {
+		return NDR_ERR_SUCCESS;
+	}
+
+	while (s && *s) {
+		enum ndr_err_code ndr_err;
+		char *compname;
+		size_t complen;
+		uint32_t offset;
+
+		if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) {
+			/* see if we have pushed the remaining string already,
+			 * if so we use a label pointer to this string
+			 */
+			ndr_err = ndr_token_retrieve_cmp_fn(string_list, s,
+							    &offset,
+							    (comparison_fn_t)strcmp,
+							    false);
+			if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+				uint8_t b[2];
+
+				if (offset > 0x3FFF) {
+					return ndr_push_error(ndr, NDR_ERR_STRING,
+							      "offset for dns string " \
+							      "label pointer " \
+							      "%u[%08X] > 0x00003FFF",
+							      offset, offset);
+				}
+
+				b[0] = 0xC0 | (offset>>8);
+				b[1] = (offset & 0xFF);
+
+				return ndr_push_bytes(ndr, b, 2);
+			}
+		}
+
+		complen = strcspn(s, ".");
+
+		/* we need to make sure the length fits into 6 bytes */
+		if (complen > 0x3F) {
+			return ndr_push_error(ndr, NDR_ERR_STRING,
+					      "component length %u[%08X] > " \
+					      "0x0000003F",
+					      (unsigned)complen,
+					      (unsigned)complen);
+		}
+
+		compname = talloc_asprintf(ndr, "%c%*.*s",
+						(unsigned char)complen,
+						(unsigned char)complen,
+						(unsigned char)complen, s);
+		NDR_ERR_HAVE_NO_MEMORY(compname);
+
+		/* remember the current component + the rest of the string
+		 * so it can be reused later
+		 */
+		if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) {
+			NDR_CHECK(ndr_token_store(ndr, string_list, s,
+						  ndr->offset));
+		}
+
+		/* push just this component into the blob */
+		NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname,
+					 complen+1));
+		talloc_free(compname);
+
+		s += complen;
+		if (*s == '.') s++;
+	}
+
+	/* if we reach the end of the string and have pushed the last component
+	 * without using a label pointer, we need to terminate the string
+	 */
+	return ndr_push_bytes(ndr, (const uint8_t *)"", 1);
+}
diff --git a/librpc/ndr/ndr_dns_utils.h b/librpc/ndr/ndr_dns_utils.h
new file mode 100644
index 00000000000..823e3201112
--- /dev/null
+++ b/librpc/ndr/ndr_dns_utils.h
@@ -0,0 +1,5 @@
+
+enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
+					   struct ndr_token_list *string_list,
+					   int ndr_flags,
+					   const char *s);
diff --git a/librpc/wscript_build b/librpc/wscript_build
index 788efcf57a6..62200ea0524 100644
--- a/librpc/wscript_build
+++ b/librpc/wscript_build
@@ -31,7 +31,7 @@ bld.SAMBA_SUBSYSTEM('NDR_DNSSERVER',
     )
 
 bld.SAMBA_SUBSYSTEM('NDR_DNS',
-    source='gen_ndr/ndr_dns.c ndr/ndr_dns.c',
+    source='gen_ndr/ndr_dns.c ndr/ndr_dns.c ndr/ndr_dns_utils.c',
     public_deps='ndr NDR_DNSP'
     )
 
-- 
2.17.1


From 0c6bbd701926dfc16754fc5ea523d6f21d0ea740 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Sat, 25 Apr 2020 11:03:30 +1200
Subject: [PATCH 15/22] CVE-2020-10745: ndr/dns_utils: correct a comment

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
---
 librpc/ndr/ndr_dns_utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c
index 2d9b5f1bc1e..2ce300863bc 100644
--- a/librpc/ndr/ndr_dns_utils.c
+++ b/librpc/ndr/ndr_dns_utils.c
@@ -49,7 +49,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
 
 		complen = strcspn(s, ".");
 
-		/* we need to make sure the length fits into 6 bytes */
+		/* the length must fit into 6 bits (i.e. <= 63) */
 		if (complen > 0x3F) {
 			return ndr_push_error(ndr, NDR_ERR_STRING,
 					      "component length %u[%08X] > " \
-- 
2.17.1


From dbde3431f70ec0cf9c0da7abe7bc53fd4e5d3a63 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Sat, 25 Apr 2020 11:10:18 +1200
Subject: [PATCH 16/22] CVE-2020-10745: ndr_dns: do not allow consecutive dots

The empty subdomain component is reserved for the root domain, which we
should only (and always) see at the end of the list. That is, we expect
"example.com.", but never "example..com".

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
---
 librpc/ndr/ndr_dns_utils.c       | 6 ++++++
 selftest/knownfail.d/dns_packet  | 1 -
 selftest/knownfail.d/ndr_dns_nbt | 1 -
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c
index 2ce300863bc..6931dac422d 100644
--- a/librpc/ndr/ndr_dns_utils.c
+++ b/librpc/ndr/ndr_dns_utils.c
@@ -58,6 +58,12 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
 					      (unsigned)complen);
 		}
 
+		if (complen == 0 && s[complen] == '.') {
+			return ndr_push_error(ndr, NDR_ERR_STRING,
+					      "component length is 0 "
+					      "(consecutive dots)");
+		}
+
 		compname = talloc_asprintf(ndr, "%c%*.*s",
 						(unsigned char)complen,
 						(unsigned char)complen,
diff --git a/selftest/knownfail.d/dns_packet b/selftest/knownfail.d/dns_packet
index 6e2e5a699de..0662266f689 100644
--- a/selftest/knownfail.d/dns_packet
+++ b/selftest/knownfail.d/dns_packet
@@ -1,2 +1 @@
-samba.tests.dns_packet.samba.tests.dns_packet.TestDnsPackets.test_127_very_dotty_components
 samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components
diff --git a/selftest/knownfail.d/ndr_dns_nbt b/selftest/knownfail.d/ndr_dns_nbt
index f30217c4033..e11c121b7a7 100644
--- a/selftest/knownfail.d/ndr_dns_nbt
+++ b/selftest/knownfail.d/ndr_dns_nbt
@@ -1,4 +1,3 @@
-librpc.ndr.ndr_dns_nbt.test_ndr_dns_string_all_dots
 librpc.ndr.ndr_dns_nbt.test_ndr_dns_string_half_dots
 librpc.ndr.ndr_dns_nbt.test_ndr_nbt_string_all_dots
 librpc.ndr.ndr_dns_nbt.test_ndr_nbt_string_half_dots
-- 
2.17.1


From d266802a3fd75b91848b41f2b347de2e27fee5f9 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Fri, 15 May 2020 00:06:08 +1200
Subject: [PATCH 17/22] CVE-2020-10745: dns_util/push: forbid names longer than
 255 bytes

As per RFC 1035.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
---
 librpc/ndr/ndr_dns_utils.c       | 10 +++++++++-
 selftest/knownfail.d/ndr_dns_nbt |  1 -
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c
index 6931dac422d..b7f11dbab4e 100644
--- a/librpc/ndr/ndr_dns_utils.c
+++ b/librpc/ndr/ndr_dns_utils.c
@@ -11,6 +11,8 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
 					   int ndr_flags,
 					   const char *s)
 {
+	const char *start = s;
+
 	if (!(ndr_flags & NDR_SCALARS)) {
 		return NDR_ERR_SUCCESS;
 	}
@@ -84,7 +86,13 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
 		talloc_free(compname);
 
 		s += complen;
-		if (*s == '.') s++;
+		if (*s == '.') {
+			s++;
+		}
+		if (s - start > 255) {
+			return ndr_push_error(ndr, NDR_ERR_STRING,
+					      "name > 255 character long");
+		}
 	}
 
 	/* if we reach the end of the string and have pushed the last component
diff --git a/selftest/knownfail.d/ndr_dns_nbt b/selftest/knownfail.d/ndr_dns_nbt
index e11c121b7a7..603395c8c50 100644
--- a/selftest/knownfail.d/ndr_dns_nbt
+++ b/selftest/knownfail.d/ndr_dns_nbt
@@ -1,3 +1,2 @@
-librpc.ndr.ndr_dns_nbt.test_ndr_dns_string_half_dots
 librpc.ndr.ndr_dns_nbt.test_ndr_nbt_string_all_dots
 librpc.ndr.ndr_dns_nbt.test_ndr_nbt_string_half_dots
-- 
2.17.1


From 21a449f491be33f7cc2dd54491abf17dae041c21 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Fri, 15 May 2020 10:52:45 +1200
Subject: [PATCH 18/22] CVE-2020-10745: ndr/dns-utils: prepare for NBT
 compatibility

NBT has a funny thing where it sometimes needs to send a trailing dot as
part of the last component, because the string representation is a user
name. In DNS, "example.com", and "example.com." are the same, both
having three components ("example", "com", ""); in NBT, we want to treat
them differently, with the second form having the three components
("example", "com.", "").

This retains the logic of e6e2ec0001fe3c010445e26cc0efddbc1f73416b.

Also DNS compression cannot be turned off for NBT.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
---
 librpc/ndr/ndr_dns.c             |  3 +-
 librpc/ndr/ndr_dns_utils.c       | 42 ++++++++++++++++---
 librpc/ndr/ndr_dns_utils.h       |  3 +-
 librpc/ndr/ndr_nbt.c             | 72 ++++----------------------------
 librpc/wscript_build             |  3 +-
 selftest/knownfail.d/dns_packet  |  1 -
 selftest/knownfail.d/ndr_dns_nbt |  2 -
 7 files changed, 49 insertions(+), 77 deletions(-)
 delete mode 100644 selftest/knownfail.d/ndr_dns_nbt

diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c
index 68a3c9de782..966e0b59786 100644
--- a/librpc/ndr/ndr_dns.c
+++ b/librpc/ndr/ndr_dns.c
@@ -163,7 +163,8 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr,
 	return ndr_push_dns_string_list(ndr,
 					&ndr->dns_string_list,
 					ndr_flags,
-					s);
+					s,
+					false);
 }
 
 _PUBLIC_ enum ndr_err_code ndr_pull_dns_txt_record(struct ndr_pull *ndr, int ndr_flags, struct dns_txt_record *r)
diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c
index b7f11dbab4e..325d9c68bea 100644
--- a/librpc/ndr/ndr_dns_utils.c
+++ b/librpc/ndr/ndr_dns_utils.c
@@ -9,9 +9,32 @@
 enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
 					   struct ndr_token_list *string_list,
 					   int ndr_flags,
-					   const char *s)
+					   const char *s,
+					   bool is_nbt)
 {
 	const char *start = s;
+	bool use_compression;
+	size_t max_length;
+	if (is_nbt) {
+		use_compression = true;
+		/*
+		 * Max length is longer in NBT/Wins, because Windows counts
+		 * the semi-decompressed size of the netbios name (16 bytes)
+		 * rather than the wire size of 32, which is what you'd expect
+		 * if it followed RFC1002 (it uses the short form in
+		 * [MS-WINSRA]). In other words the maximum size of the
+		 * "scope" is 237, not 221.
+		 *
+		 * We make the size limit slightly larger than 255 + 16,
+		 * because the 237 scope limit is already enforced in the
+		 * winsserver code with a specific return value; bailing out
+		 * here would muck with that.
+		 */
+		max_length = 274;
+	} else {
+		use_compression = !(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION);
+		max_length = 255;
+	}
 
 	if (!(ndr_flags & NDR_SCALARS)) {
 		return NDR_ERR_SUCCESS;
@@ -23,7 +46,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
 		size_t complen;
 		uint32_t offset;
 
-		if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) {
+		if (use_compression) {
 			/* see if we have pushed the remaining string already,
 			 * if so we use a label pointer to this string
 			 */
@@ -66,6 +89,14 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
 					      "(consecutive dots)");
 		}
 
+		if (is_nbt && s[complen] == '.' && s[complen + 1] == '\0') {
+			/* nbt names are sometimes usernames, and we need to
+			 * keep a trailing dot to ensure it is byte-identical,
+			 * (not just semantically identical given DNS
+			 * semantics). */
+			complen++;
+		}
+
 		compname = talloc_asprintf(ndr, "%c%*.*s",
 						(unsigned char)complen,
 						(unsigned char)complen,
@@ -75,7 +106,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
 		/* remember the current component + the rest of the string
 		 * so it can be reused later
 		 */
-		if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) {
+		if (use_compression) {
 			NDR_CHECK(ndr_token_store(ndr, string_list, s,
 						  ndr->offset));
 		}
@@ -89,9 +120,10 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
 		if (*s == '.') {
 			s++;
 		}
-		if (s - start > 255) {
+		if (s - start > max_length) {
 			return ndr_push_error(ndr, NDR_ERR_STRING,
-					      "name > 255 character long");
+					      "name > %zu character long",
+					      max_length);
 		}
 	}
 
diff --git a/librpc/ndr/ndr_dns_utils.h b/librpc/ndr/ndr_dns_utils.h
index 823e3201112..71a65433bbb 100644
--- a/librpc/ndr/ndr_dns_utils.h
+++ b/librpc/ndr/ndr_dns_utils.h
@@ -2,4 +2,5 @@
 enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
 					   struct ndr_token_list *string_list,
 					   int ndr_flags,
-					   const char *s);
+					   const char *s,
+					   bool is_nbt);
diff --git a/librpc/ndr/ndr_nbt.c b/librpc/ndr/ndr_nbt.c
index 838f947a168..e8dd7549a53 100644
--- a/librpc/ndr/ndr_nbt.c
+++ b/librpc/ndr/ndr_nbt.c
@@ -25,6 +25,8 @@
 #include "includes.h"
 #include "../libcli/nbt/libnbt.h"
 #include "../libcli/netlogon/netlogon.h"
+#include "ndr_dns_utils.h"
+
 
 /* don't allow an unlimited number of name components */
 #define MAX_COMPONENTS 128
@@ -141,71 +143,11 @@ _PUBLIC_ enum ndr_err_code ndr_pull_nbt_string(struct ndr_pull *ndr, int ndr_fla
 */
 _PUBLIC_ enum ndr_err_code ndr_push_nbt_string(struct ndr_push *ndr, int ndr_flags, const char *s)
 {
-	if (!(ndr_flags & NDR_SCALARS)) {
-		return NDR_ERR_SUCCESS;
-	}
-
-	while (s && *s) {
-		enum ndr_err_code ndr_err;
-		char *compname;
-		size_t complen;
-		uint32_t offset;
-
-		/* see if we have pushed the remaining string already,
-		 * if so we use a label pointer to this string
-		 */
-		ndr_err = ndr_token_retrieve_cmp_fn(&ndr->nbt_string_list, s, &offset, (comparison_fn_t)strcmp, false);
-		if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-			uint8_t b[2];
-
-			if (offset > 0x3FFF) {
-				return ndr_push_error(ndr, NDR_ERR_STRING,
-						      "offset for nbt string label pointer %u[%08X] > 0x00003FFF",
-						      offset, offset);
-			}
-
-			b[0] = 0xC0 | (offset>>8);
-			b[1] = (offset & 0xFF);
-
-			return ndr_push_bytes(ndr, b, 2);
-		}
-
-		complen = strcspn(s, ".");
-
-		/* we need to make sure the length fits into 6 bytes */
-		if (complen > 0x3F) {
-			return ndr_push_error(ndr, NDR_ERR_STRING,
-					      "component length %u[%08X] > 0x0000003F",
-					      (unsigned)complen, (unsigned)complen);
-		}
-
-		if (s[complen] == '.' && s[complen+1] == '\0') {
-			complen++;
-		}
-
-		compname = talloc_asprintf(ndr, "%c%*.*s",
-						(unsigned char)complen,
-						(unsigned char)complen,
-						(unsigned char)complen, s);
-		NDR_ERR_HAVE_NO_MEMORY(compname);
-
-		/* remember the current componemt + the rest of the string
-		 * so it can be reused later
-		 */
-		NDR_CHECK(ndr_token_store(ndr, &ndr->nbt_string_list, s, ndr->offset));
-
-		/* push just this component into the blob */
-		NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, complen+1));
-		talloc_free(compname);
-
-		s += complen;
-		if (*s == '.') s++;
-	}
-
-	/* if we reach the end of the string and have pushed the last component
-	 * without using a label pointer, we need to terminate the string
-	 */
-	return ndr_push_bytes(ndr, (const uint8_t *)"", 1);
+	return ndr_push_dns_string_list(ndr,
+					&ndr->dns_string_list,
+					ndr_flags,
+					s,
+					true);
 }
 
 
diff --git a/librpc/wscript_build b/librpc/wscript_build
index 62200ea0524..b560a08a7e2 100644
--- a/librpc/wscript_build
+++ b/librpc/wscript_build
@@ -411,7 +411,7 @@ bld.SAMBA_SUBSYSTEM('NDR_SCHANNEL',
 
 bld.SAMBA_LIBRARY('ndr_nbt',
     source='gen_ndr/ndr_nbt.c ndr/ndr_nbt.c',
-    public_deps='ndr NDR_NBT_BUF NDR_SECURITY',
+    public_deps='ndr NDR_NBT_BUF NDR_SECURITY NDR_DNS',
     public_headers='gen_ndr/nbt.h gen_ndr/ndr_nbt.h ndr/ndr_nbt.h',
     header_path=[ ('gen_ndr*', 'gen_ndr'), ('ndr*', 'ndr')],
     pc_files='ndr_nbt.pc',
@@ -766,6 +766,5 @@ bld.SAMBA_BINARY('test_ndr_dns_nbt',
                       cmocka
                       ndr
                       ndr_nbt
-                      NDR_DNS
                       ''',
                  install=False)
diff --git a/selftest/knownfail.d/dns_packet b/selftest/knownfail.d/dns_packet
index 0662266f689..e69de29bb2d 100644
--- a/selftest/knownfail.d/dns_packet
+++ b/selftest/knownfail.d/dns_packet
@@ -1 +0,0 @@
-samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components
diff --git a/selftest/knownfail.d/ndr_dns_nbt b/selftest/knownfail.d/ndr_dns_nbt
deleted file mode 100644
index 603395c8c50..00000000000
--- a/selftest/knownfail.d/ndr_dns_nbt
+++ /dev/null
@@ -1,2 +0,0 @@
-librpc.ndr.ndr_dns_nbt.test_ndr_nbt_string_all_dots
-librpc.ndr.ndr_dns_nbt.test_ndr_nbt_string_half_dots
-- 
2.17.1


From 9773231e3a53291214a914ed168065f5ed5ea1e6 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Thu, 25 Jun 2020 11:59:54 +1200
Subject: [PATCH 19/22] CVE-2020-14303 Ensure an empty packet will not DoS the
 NBT server

Signed-off-by: Andrew Bartlett <abartlet@samba.org>

(backported from master commit)
[abartlet@samba.org: Remove f"" format string not supported in
 Python 3.4]
---
 python/samba/tests/dns_packet.py | 19 +++++++++++++++++++
 selftest/knownfail.d/empty-nbt   |  1 +
 2 files changed, 20 insertions(+)
 create mode 100644 selftest/knownfail.d/empty-nbt

diff --git a/python/samba/tests/dns_packet.py b/python/samba/tests/dns_packet.py
index a9996664e57..68e4d154cad 100644
--- a/python/samba/tests/dns_packet.py
+++ b/python/samba/tests/dns_packet.py
@@ -155,6 +155,19 @@ class TestDnsPacketBase(TestCase):
         rcode = self.decode_reply(data)['rcode']
         return expected_rcode == rcode
 
+    def _test_empty_packet(self):
+
+        packet = b""
+        s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+        s.sendto(packet, self.server)
+        s.close()
+
+        # It is reasonable not to reply to an empty packet
+        # but it is not reasonable to render the server
+        # unresponsive.
+        ok = self._known_good_query()
+        self.assertTrue(ok, "the server is unresponsive")
+
 
 class TestDnsPackets(TestDnsPacketBase):
     server = (SERVER, 53)
@@ -173,6 +186,9 @@ class TestDnsPackets(TestDnsPacketBase):
         label = b'x.' * 31 + b'x'
         self._test_many_repeated_components(label, 127)
 
+    def test_empty_packet(self):
+        self._test_empty_packet()
+
 
 class TestNbtPackets(TestDnsPacketBase):
     server = (SERVER, 137)
@@ -208,3 +224,6 @@ class TestNbtPackets(TestDnsPacketBase):
     def test_127_half_dotty_components(self):
         label = b'x.' * 31 + b'x'
         self._test_many_repeated_components(label, 127)
+
+    def test_empty_packet(self):
+        self._test_empty_packet()
diff --git a/selftest/knownfail.d/empty-nbt b/selftest/knownfail.d/empty-nbt
new file mode 100644
index 00000000000..e4bcccab4e5
--- /dev/null
+++ b/selftest/knownfail.d/empty-nbt
@@ -0,0 +1 @@
+^samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_empty_packet
\ No newline at end of file
-- 
2.17.1


From 2e190d5c766d3487223ccdd4dc1e2ad0e160bb3f Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Wed, 24 Jun 2020 14:27:08 +1200
Subject: [PATCH 20/22] CVE-2020-14303: s4 nbt: fix busy loop on empty UDP
 packet

An empty UDP packet put the nbt server into a busy loop that consumes
100% of a cpu.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14417

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
---
 libcli/nbt/nbtsocket.c         | 17 ++++++++++++++++-
 selftest/knownfail.d/empty-nbt |  1 -
 2 files changed, 16 insertions(+), 2 deletions(-)
 delete mode 100644 selftest/knownfail.d/empty-nbt

diff --git a/libcli/nbt/nbtsocket.c b/libcli/nbt/nbtsocket.c
index 33d53fba993..8aecaf73247 100644
--- a/libcli/nbt/nbtsocket.c
+++ b/libcli/nbt/nbtsocket.c
@@ -167,8 +167,23 @@ static void nbt_name_socket_recv(struct nbt_name_socket *nbtsock)
 		return;
 	}
 
+	/*
+	 * Given a zero length, data_blob_talloc() returns the
+	 * NULL blob {NULL, 0}.
+	 *
+	 * We only want to error return here on a real out of memory condition
+	 * (i.e. dsize != 0, so the UDP packet has data, but the return of the
+	 * allocation failed, so blob.data==NULL).
+	 *
+	 * Given an actual zero length UDP packet having blob.data == NULL
+	 * isn't an out of memory error condition, that's the defined semantics
+	 * of data_blob_talloc() when asked for zero bytes.
+	 *
+	 * We still need to continue to do the zero-length socket_recvfrom()
+	 * read in order to clear the "read pending" condition on the socket.
+	 */
 	blob = data_blob_talloc(tmp_ctx, NULL, dsize);
-	if (blob.data == NULL) {
+	if (blob.data == NULL && dsize != 0) {
 		talloc_free(tmp_ctx);
 		return;
 	}
diff --git a/selftest/knownfail.d/empty-nbt b/selftest/knownfail.d/empty-nbt
deleted file mode 100644
index e4bcccab4e5..00000000000
--- a/selftest/knownfail.d/empty-nbt
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_empty_packet
\ No newline at end of file
-- 
2.17.1


From 4bc0ada8d99425ad8d8933e0c3b1abecaa185edf Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Fri, 5 Jun 2020 22:14:48 +1200
Subject: [PATCH 21/22] CVE-2020-10760 dsdb: Ensure a proper talloc tree for
 saved controls

Otherwise a paged search on the GC port will fail as the ->data was
not kept around for the second page of searches.

An example command to produce this is
 bin/ldbsearch --paged -H ldap://$SERVER:3268 -U$USERNAME%$PASSWORD

This shows up later in the partition module as:

ERROR: AddressSanitizer: heap-use-after-free on address 0x60b00151ef20 at pc 0x7fec3f801aac bp 0x7ffe8472c270 sp 0x7ffe8472c260
READ of size 4 at 0x60b00151ef20 thread T0 (ldap(0))
    #0 0x7fec3f801aab in talloc_chunk_from_ptr ../../lib/talloc/talloc.c:526
    #1 0x7fec3f801aab in __talloc_get_name ../../lib/talloc/talloc.c:1559
    #2 0x7fec3f801aab in talloc_check_name ../../lib/talloc/talloc.c:1582
    #3 0x7fec1b86b2e1 in partition_search ../../source4/dsdb/samdb/ldb_modules/partition.c:780

or

smb_panic_default: PANIC (pid 13287): Bad talloc magic value - unknown value
(from source4/dsdb/samdb/ldb_modules/partition.c:780)

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14402

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
---
 source4/dsdb/samdb/ldb_modules/paged_results.c  | 8 ++++++++
 source4/dsdb/samdb/ldb_modules/vlv_pagination.c | 7 +++++++
 2 files changed, 15 insertions(+)

diff --git a/source4/dsdb/samdb/ldb_modules/paged_results.c b/source4/dsdb/samdb/ldb_modules/paged_results.c
index c4b538f2208..bc4996880e0 100644
--- a/source4/dsdb/samdb/ldb_modules/paged_results.c
+++ b/source4/dsdb/samdb/ldb_modules/paged_results.c
@@ -523,6 +523,14 @@ paged_results_copy_down_controls(TALLOC_CTX *mem_ctx,
 			continue;
 		}
 		new_controls[j] = talloc_steal(new_controls, control);
+
+		/*
+		 * Sadly the caller is not obliged to make this a
+		 * proper talloc tree, so we do so here.
+		 */
+		if (control->data) {
+			talloc_steal(control, control->data);
+		}
 		j++;
 	}
 	new_controls[j] = NULL;
diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
index b103bda5f52..d6d6039e849 100644
--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
@@ -746,6 +746,13 @@ vlv_copy_down_controls(TALLOC_CTX *mem_ctx, struct ldb_control **controls)
 			continue;
 		}
 		new_controls[j] = talloc_steal(new_controls, control);
+		/*
+		 * Sadly the caller is not obliged to make this a
+		 * proper talloc tree, so we do so here.
+		 */
+		if (control->data) {
+			talloc_steal(control, control->data);
+		}
 		j++;
 	}
 	new_controls[j] = NULL;
-- 
2.17.1


From ca38b0eecddbfab0ff1b80d7e588b19beb299084 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Mon, 8 Jun 2020 16:32:14 +1200
Subject: [PATCH 22/22] CVE-2020-10760 dsdb: Add tests for paged_results and
 VLV over the Global Catalog port

This should avoid a regression.

(backported from master patch)
[abartlet@samba.org: sort=True parameter on test_paged_delete_during_search
 is not in 4.10]

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
---
 selftest/knownfail.d/vlv         |   2 +-
 source4/dsdb/tests/python/vlv.py | 171 +++++++++++++++++++------------
 2 files changed, 107 insertions(+), 66 deletions(-)

diff --git a/selftest/knownfail.d/vlv b/selftest/knownfail.d/vlv
index f187a2ed55e..7ae02baf17b 100644
--- a/selftest/knownfail.d/vlv
+++ b/selftest/knownfail.d/vlv
@@ -1,2 +1,2 @@
 samba4.ldap.vlv.python.*__main__.VLVTests.test_vlv_change_search_expr
-samba4.ldap.vlv.python.*__main__.PagedResultsTests.test_paged_cant_change_controls_data
+samba4.ldap.vlv.python.*__main__.PagedResultsTestsRW.test_paged_cant_change_controls_data
diff --git a/source4/dsdb/tests/python/vlv.py b/source4/dsdb/tests/python/vlv.py
index ce7aa213c36..acaf64d0faa 100644
--- a/source4/dsdb/tests/python/vlv.py
+++ b/source4/dsdb/tests/python/vlv.py
@@ -152,7 +152,7 @@ class TestsWithUserOU(samba.tests.TestCase):
         super(TestsWithUserOU, self).setUp()
         self.ldb = SamDB(host, credentials=creds,
                          session_info=system_session(lp), lp=lp)
-
+        self.ldb_ro = self.ldb
         self.base_dn = self.ldb.domain_dn()
         self.ou = "ou=vlv,%s" % self.base_dn
         if opts.delete_in_setup:
@@ -195,8 +195,60 @@ class TestsWithUserOU(samba.tests.TestCase):
             self.ldb.delete(self.ou, ['tree_delete:1'])
 
 
-class VLVTests(TestsWithUserOU):
+class VLVTestsBase(TestsWithUserOU):
+
+    # Run a vlv search and return important fields of the response control
+    def vlv_search(self, attr, expr, cookie="", after_count=0, offset=1):
+        sort_ctrl = "server_sort:1:0:%s" % attr
+        ctrl = "vlv:1:0:%d:%d:0" % (after_count, offset)
+        if cookie:
+            ctrl += ":" + cookie
+
+        res = self.ldb_ro.search(self.ou,
+                              expression=expr,
+                              scope=ldb.SCOPE_ONELEVEL,
+                              attrs=[attr],
+                              controls=[ctrl, sort_ctrl])
+        results = [str(x[attr][0]) for x in res]
+
+        ctrls = [str(c) for c in res.controls if
+                 str(c).startswith('vlv')]
+        self.assertEqual(len(ctrls), 1)
+
+        spl = ctrls[0].rsplit(':')
+        cookie = ""
+        if len(spl) == 6:
+            cookie = spl[-1]
+
+        return results, cookie
+
+
+class VLVTestsRO(VLVTestsBase):
+    def test_vlv_simple_double_run(self):
+        """Do the simplest possible VLV query to confirm if VLV
+           works at all.  Useful for showing VLV as a whole works
+           on Global Catalog (for example)"""
+        attr = 'roomNumber'
+        expr = "(objectclass=user)"
 
+        # Start new search
+        full_results, cookie = self.vlv_search(attr, expr,
+                                               after_count=len(self.users))
+
+        results, cookie = self.vlv_search(attr, expr, cookie=cookie,
+                                          after_count=len(self.users))
+        expected_results = full_results
+        self.assertEqual(results, expected_results)
+
+
+class VLVTestsGC(VLVTestsRO):
+    def setUp(self):
+        super(VLVTestsRO, self).setUp()
+        self.ldb_ro = SamDB(host + ":3268", credentials=creds,
+                            session_info=system_session(lp), lp=lp)
+
+
+class VLVTests(VLVTestsBase):
     def get_full_list(self, attr, include_cn=False):
         """Fetch the whole list sorted on the attribute, using the VLV.
         This way you get a VLV cookie."""
@@ -1077,31 +1129,6 @@ class VLVTests(TestsWithUserOU):
                        controls=[sort_control,
                                  "vlv:0:1:1:1:0:%s" % vlv_cookies[-1]])
 
-    # Run a vlv search and return important fields of the response control
-    def vlv_search(self, attr, expr, cookie="", after_count=0, offset=1):
-        sort_ctrl = "server_sort:1:0:%s" % attr
-        ctrl = "vlv:1:0:%d:%d:0" % (after_count, offset)
-        if cookie:
-            ctrl += ":" + cookie
-
-        res = self.ldb.search(self.ou,
-                              expression=expr,
-                              scope=ldb.SCOPE_ONELEVEL,
-                              attrs=[attr],
-                              controls=[ctrl, sort_ctrl])
-        results = [str(x[attr][0]) for x in res]
-
-        ctrls = [str(c) for c in res.controls if
-                 str(c).startswith('vlv')]
-        self.assertEqual(len(ctrls), 1)
-
-        spl = ctrls[0].rsplit(':')
-        cookie = ""
-        if len(spl) == 6:
-            cookie = spl[-1]
-
-        return results, cookie
-
     def test_vlv_modify_during_view(self):
         attr = 'roomNumber'
         expr = "(objectclass=user)"
@@ -1214,11 +1241,11 @@ class PagedResultsTests(TestsWithUserOU):
         if subtree:
             scope = ldb.SCOPE_SUBTREE
 
-        res = self.ldb.search(ou,
-                              expression=expr,
-                              scope=scope,
-                              controls=controls,
-                              **kwargs)
+        res = self.ldb_ro.search(ou,
+                                 expression=expr,
+                                 scope=scope,
+                                 controls=controls,
+                                 **kwargs)
         results = [str(r['cn'][0]) for r in res]
 
         ctrls = [str(c) for c in res.controls if
@@ -1231,6 +1258,53 @@ class PagedResultsTests(TestsWithUserOU):
             cookie = spl[-1]
         return results, cookie
 
+
+class PagedResultsTestsRO(PagedResultsTests):
+
+    def test_paged_search_lockstep(self):
+        expr = "(objectClass=*)"
+        ps = 3
+
+        all_results, _ = self.paged_search(expr, page_size=len(self.users)+1)
+
+        # Run two different but overlapping paged searches simultaneously.
+        set_1_index = int((len(all_results))//3)
+        set_2_index = int((2*len(all_results))//3)
+        set_1 = all_results[set_1_index:]
+        set_2 = all_results[:set_2_index+1]
+        set_1_expr = "(cn>=%s)" % (all_results[set_1_index])
+        set_2_expr = "(cn<=%s)" % (all_results[set_2_index])
+
+        results, cookie1 = self.paged_search(set_1_expr, page_size=ps)
+        self.assertEqual(results, set_1[:ps])
+        results, cookie2 = self.paged_search(set_2_expr, page_size=ps)
+        self.assertEqual(results, set_2[:ps])
+
+        results, cookie1 = self.paged_search(set_1_expr, cookie=cookie1,
+                                             page_size=ps)
+        self.assertEqual(results, set_1[ps:ps*2])
+        results, cookie2 = self.paged_search(set_2_expr, cookie=cookie2,
+                                             page_size=ps)
+        self.assertEqual(results, set_2[ps:ps*2])
+
+        results, _ = self.paged_search(set_1_expr, cookie=cookie1,
+                                       page_size=len(self.users))
+        self.assertEqual(results, set_1[ps*2:])
+        results, _ = self.paged_search(set_2_expr, cookie=cookie2,
+                                       page_size=len(self.users))
+        self.assertEqual(results, set_2[ps*2:])
+
+
+class PagedResultsTestsGC(PagedResultsTestsRO):
+
+    def setUp(self):
+        super(PagedResultsTestsRO, self).setUp()
+        self.ldb_ro = SamDB(host + ":3268", credentials=creds,
+                            session_info=system_session(lp), lp=lp)
+
+
+class PagedResultsTestsRW(PagedResultsTests):
+
     def test_paged_delete_during_search(self):
         expr = "(objectClass=*)"
 
@@ -1611,39 +1685,6 @@ class PagedResultsTests(TestsWithUserOU):
                                      cookie, attrs=changed_attrs,
                                      extra_ctrls=[])
 
-    def test_paged_search_lockstep(self):
-        expr = "(objectClass=*)"
-        ps = 3
-
-        all_results, _ = self.paged_search(expr, page_size=len(self.users)+1)
-
-        # Run two different but overlapping paged searches simultaneously.
-        set_1_index = int((len(all_results))//3)
-        set_2_index = int((2*len(all_results))//3)
-        set_1 = all_results[set_1_index:]
-        set_2 = all_results[:set_2_index+1]
-        set_1_expr = "(cn>=%s)" % (all_results[set_1_index])
-        set_2_expr = "(cn<=%s)" % (all_results[set_2_index])
-
-        results, cookie1 = self.paged_search(set_1_expr, page_size=ps)
-        self.assertEqual(results, set_1[:ps])
-        results, cookie2 = self.paged_search(set_2_expr, page_size=ps)
-        self.assertEqual(results, set_2[:ps])
-
-        results, cookie1 = self.paged_search(set_1_expr, cookie=cookie1,
-                                             page_size=ps)
-        self.assertEqual(results, set_1[ps:ps*2])
-        results, cookie2 = self.paged_search(set_2_expr, cookie=cookie2,
-                                             page_size=ps)
-        self.assertEqual(results, set_2[ps:ps*2])
-
-        results, _ = self.paged_search(set_1_expr, cookie=cookie1,
-                                       page_size=len(self.users))
-        self.assertEqual(results, set_1[ps*2:])
-        results, _ = self.paged_search(set_2_expr, cookie=cookie2,
-                                       page_size=len(self.users))
-        self.assertEqual(results, set_2[ps*2:])
-
     def test_vlv_paged(self):
         """Testing behaviour with VLV and paged_results set.
 
-- 
2.17.1