Attachment 16718 Details for Bug 14468 – WIP patches for master

[patch] WIP patches for master

bug-14468-master.patches.txt (text/plain), 46.39 KB, created by Stefan Metzmacher on 2021-08-05 14:05:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2021-08-05 14:05:00 UTC

Size: 46.39 KB

patch

obsolete

>From 9ffd594eed5315722ecbe2e6c1ed84cbc1ea6ba8 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 5 Aug 2021 11:24:26 +0200
>Subject: [PATCH 01/11] s4:torture/drsuapi: don't pass DsPrivate to
> test_DsBind()
>
>This will make it easier to reuse.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14468
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source4/torture/rpc/drsuapi.c            | 24 +++++++++++++++---------
> source4/torture/rpc/drsuapi.h            |  1 -
> source4/torture/rpc/drsuapi_cracknames.c |  2 +-
> 3 files changed, 16 insertions(+), 11 deletions(-)
>
>diff --git a/source4/torture/rpc/drsuapi.c b/source4/torture/rpc/drsuapi.c
>index 2ae2ba031e96..862c5f592b7b 100644
>--- a/source4/torture/rpc/drsuapi.c
>+++ b/source4/torture/rpc/drsuapi.c
>@@ -28,12 +28,14 @@
> 
> #define TEST_MACHINE_NAME "torturetest"
> 
>-bool test_DsBind(struct dcerpc_pipe *p,
>-		 struct torture_context *tctx,
>-		 struct DsPrivate *priv)
>+static bool test_DsBind(struct dcerpc_pipe *p,
>+			struct torture_context *tctx,
>+			struct policy_handle *bind_handle,
>+			struct drsuapi_DsBindInfo28 *srv_info28)
> {
> 	NTSTATUS status;
> 	struct drsuapi_DsBind r;
>+	struct GUID bind_guid;
> 	struct drsuapi_DsBindInfo28 *bind_info28;
> 	struct drsuapi_DsBindInfoCtr bind_info_ctr;
> 
>@@ -70,19 +72,20 @@ bool test_DsBind(struct dcerpc_pipe *p,
> 	bind_info28->supported_extensions	|= DRSUAPI_SUPPORTED_EXTENSION_GETCHGREPLY_V7;
> 	bind_info28->supported_extensions	|= DRSUAPI_SUPPORTED_EXTENSION_VERIFY_OBJECT;
> 
>-	GUID_from_string(DRSUAPI_DS_BIND_GUID, &priv->bind_guid);
>+	GUID_from_string(DRSUAPI_DS_BIND_GUID, &bind_guid);
> 
>-	r.in.bind_guid = &priv->bind_guid;
>+	r.in.bind_guid = &bind_guid;
> 	r.in.bind_info = &bind_info_ctr;
>-	r.out.bind_handle = &priv->bind_handle;
>+	r.out.bind_handle = bind_handle;
> 
> 	torture_comment(tctx, "Testing DsBind\n");
> 
> 	status = dcerpc_drsuapi_DsBind_r(p->binding_handle, tctx, &r);
> 	torture_drsuapi_assert_call(tctx, p, status, &r, "dcerpc_drsuapi_DsBind");
> 
>-	/* cache server supported extensions, i.e. bind_info */
>-	priv->srv_bind_info = r.out.bind_info->info.info28;
>+	if (srv_info28 != NULL) {
>+		*srv_info28 = r.out.bind_info->info.info28;
>+	}
> 
> 	return true;
> }
>@@ -786,7 +789,10 @@ bool torture_drsuapi_tcase_setup_common(struct torture_context *tctx, struct DsP
> 					 &machine_credentials);
> 	torture_assert(tctx, priv->join, "Failed to join as BDC");
> 
>-	if (!test_DsBind(priv->drs_pipe, tctx, priv)) {
>+	if (!test_DsBind(priv->drs_pipe, tctx,
>+			 &priv->bind_handle,
>+			 &priv->srv_bind_info))
>+	{
> 		/* clean up */
> 		torture_drsuapi_tcase_teardown_common(tctx, priv);
> 		torture_fail(tctx, "Failed execute test_DsBind()");
>diff --git a/source4/torture/rpc/drsuapi.h b/source4/torture/rpc/drsuapi.h
>index f1a5bba05b86..e81b2fe37469 100644
>--- a/source4/torture/rpc/drsuapi.h
>+++ b/source4/torture/rpc/drsuapi.h
>@@ -29,7 +29,6 @@
> struct DsPrivate {
> 	struct dcerpc_pipe *drs_pipe;
> 	struct policy_handle bind_handle;
>-	struct GUID bind_guid;
> 	struct drsuapi_DsBindInfo28 srv_bind_info;
> 
> 	const char *domain_obj_dn;
>diff --git a/source4/torture/rpc/drsuapi_cracknames.c b/source4/torture/rpc/drsuapi_cracknames.c
>index 102f9664b3a5..511309f17da1 100644
>--- a/source4/torture/rpc/drsuapi_cracknames.c
>+++ b/source4/torture/rpc/drsuapi_cracknames.c
>@@ -801,7 +801,7 @@ bool test_DsCrackNames(struct torture_context *tctx,
> 				.format_offered	= DRSUAPI_DS_NAME_FORMAT_GUID,
> 				.format_desired	= DRSUAPI_DS_NAME_FORMAT_FQDN_1779,
> 				.comment = "BIND GUID (ie, not in the directory)",
>-				.str = GUID_string2(mem_ctx, &priv->bind_guid),
>+				.str = DRSUAPI_DS_BIND_GUID,
> 				.status = DRSUAPI_DS_NAME_STATUS_NOT_FOUND
> 			},
> 			{
>-- 
>2.25.1
>
>
>From f69f0bd032a459d4d7764582790c8553ebd8ffb5 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 5 Aug 2021 09:58:37 +0200
>Subject: [PATCH 02/11] s4:torture/drsuapi: maintain priv->dc_credentials
>
>We want to use the credentials of the joined dc account
>in future tests.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14468
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source4/torture/rpc/drsuapi.c | 3 +--
> source4/torture/rpc/drsuapi.h | 1 +
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source4/torture/rpc/drsuapi.c b/source4/torture/rpc/drsuapi.c
>index 862c5f592b7b..1cd595e5d8e9 100644
>--- a/source4/torture/rpc/drsuapi.c
>+++ b/source4/torture/rpc/drsuapi.c
>@@ -774,7 +774,6 @@ bool torture_drsuapi_tcase_setup_common(struct torture_context *tctx, struct DsP
> 	NTSTATUS status;
> 	int rnd = rand() % 1000;
> 	char *name = talloc_asprintf(tctx, "%s%d", TEST_MACHINE_NAME, rnd);
>-	struct cli_credentials *machine_credentials;
> 
> 	torture_assert(tctx, priv, "Invalid argument");
> 
>@@ -786,7 +785,7 @@ bool torture_drsuapi_tcase_setup_common(struct torture_context *tctx, struct DsP
> 
> 	torture_comment(tctx, "About to join domain with name %s\n", name);
> 	priv->join = torture_join_domain(tctx, name, ACB_SVRTRUST,
>-					 &machine_credentials);
>+					 &priv->dc_credentials);
> 	torture_assert(tctx, priv->join, "Failed to join as BDC");
> 
> 	if (!test_DsBind(priv->drs_pipe, tctx,
>diff --git a/source4/torture/rpc/drsuapi.h b/source4/torture/rpc/drsuapi.h
>index e81b2fe37469..f327c54cda44 100644
>--- a/source4/torture/rpc/drsuapi.h
>+++ b/source4/torture/rpc/drsuapi.h
>@@ -37,6 +37,7 @@ struct DsPrivate {
> 	struct GUID domain_guid;
> 	struct drsuapi_DsGetDCInfo2 dcinfo;
> 	struct test_join *join;
>+	struct cli_credentials *dc_credentials;
> };
> 
> /**
>-- 
>2.25.1
>
>
>From 3d725dbb1bef1db9885283e8fca99ab7620195e5 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 5 Aug 2021 10:34:06 +0200
>Subject: [PATCH 03/11] s4:torture/drsuapi: maintain priv->admin_credentials
>
>This will be used in the next commits.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14468
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source4/torture/rpc/drsuapi.c | 3 +++
> source4/torture/rpc/drsuapi.h | 1 +
> 2 files changed, 4 insertions(+)
>
>diff --git a/source4/torture/rpc/drsuapi.c b/source4/torture/rpc/drsuapi.c
>index 1cd595e5d8e9..799a641dbb94 100644
>--- a/source4/torture/rpc/drsuapi.c
>+++ b/source4/torture/rpc/drsuapi.c
>@@ -22,6 +22,7 @@
> */
> 
> #include "includes.h"
>+#include "lib/cmdline/cmdline.h"
> #include "librpc/gen_ndr/ndr_drsuapi_c.h"
> #include "torture/rpc/torture_rpc.h"
> #include "param/param.h"
>@@ -777,6 +778,8 @@ bool torture_drsuapi_tcase_setup_common(struct torture_context *tctx, struct DsP
> 
> 	torture_assert(tctx, priv, "Invalid argument");
> 
>+	priv->admin_credentials = samba_cmdline_get_creds();
>+
> 	torture_comment(tctx, "Create DRSUAPI pipe\n");
> 	status = torture_rpc_connection(tctx,
> 					&priv->drs_pipe,
>diff --git a/source4/torture/rpc/drsuapi.h b/source4/torture/rpc/drsuapi.h
>index f327c54cda44..3cc4be49d999 100644
>--- a/source4/torture/rpc/drsuapi.h
>+++ b/source4/torture/rpc/drsuapi.h
>@@ -27,6 +27,7 @@
>  * Data structure common for most of DRSUAPI tests
>  */
> struct DsPrivate {
>+	struct cli_credentials *admin_credentials;
> 	struct dcerpc_pipe *drs_pipe;
> 	struct policy_handle bind_handle;
> 	struct drsuapi_DsBindInfo28 srv_bind_info;
>-- 
>2.25.1
>
>
>From 779d586358bf4fcd64eb15bd02eaef8e7ba48a75 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 5 Aug 2021 11:26:16 +0200
>Subject: [PATCH 04/11] s4:torture/drsuapi: DsBindAssocGroup* tests
>
>This adds a reproducer for an invalid memory access, when
>using the context handle from DsBind across multiple connections
>within an association group.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14468
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> .../knownfail.d/drsuapi.DsBindAssocGroupWS    |   1 +
> source4/torture/rpc/drsuapi.c                 | 172 ++++++++++++++++++
> 2 files changed, 173 insertions(+)
> create mode 100644 selftest/knownfail.d/drsuapi.DsBindAssocGroupWS
>
>diff --git a/selftest/knownfail.d/drsuapi.DsBindAssocGroupWS b/selftest/knownfail.d/drsuapi.DsBindAssocGroupWS
>new file mode 100644
>index 000000000000..9af5a904fdd3
>--- /dev/null
>+++ b/selftest/knownfail.d/drsuapi.DsBindAssocGroupWS
>@@ -0,0 +1 @@
>+^samba4.rpc.drsuapi.*drsuapi.DsBindAssocGroupWS
>diff --git a/source4/torture/rpc/drsuapi.c b/source4/torture/rpc/drsuapi.c
>index 799a641dbb94..d3e18ca246bb 100644
>--- a/source4/torture/rpc/drsuapi.c
>+++ b/source4/torture/rpc/drsuapi.c
>@@ -25,6 +25,7 @@
> #include "lib/cmdline/cmdline.h"
> #include "librpc/gen_ndr/ndr_drsuapi_c.h"
> #include "torture/rpc/torture_rpc.h"
>+#include "libcli/security/dom_sid.h"
> #include "param/param.h"
> 
> #define TEST_MACHINE_NAME "torturetest"
>@@ -845,6 +846,173 @@ static bool torture_drsuapi_tcase_teardown(struct torture_context *tctx, void *d
> 	return ret;
> }
> 
>+static bool __test_DsBind_assoc_group(struct torture_context *tctx,
>+				      const char *testname,
>+				      struct DsPrivate *priv,
>+				      struct cli_credentials *creds)
>+{
>+	NTSTATUS status;
>+	const char *err_msg;
>+	struct drsuapi_DsCrackNames r;
>+	union drsuapi_DsNameRequest req;
>+	uint32_t level_out;
>+	union drsuapi_DsNameCtr ctr;
>+	struct drsuapi_DsNameString names[1];
>+	const char *dom_sid = NULL;
>+	struct dcerpc_pipe *p1 = NULL;
>+	struct dcerpc_pipe *p2 = NULL;
>+	TALLOC_CTX *mem_ctx = priv;
>+	struct dcerpc_binding *binding = NULL;
>+	struct policy_handle ds_bind_handle = { .handle_type = 0, };
>+
>+	torture_comment(tctx, "%s: starting...\n", testname);
>+
>+	torture_assert_ntstatus_ok(tctx,
>+				   torture_rpc_binding(tctx, &binding),
>+				   "torture_rpc_binding");
>+
>+	torture_assert_ntstatus_ok(tctx,
>+				   dcerpc_pipe_connect_b(tctx,
>+							 &p1,
>+							 binding,
>+							 &ndr_table_drsuapi,
>+							 creds,
>+							 tctx->ev,
>+							 tctx->lp_ctx),
>+				   "connect p1");
>+
>+	torture_assert_ntstatus_ok(tctx,
>+				   dcerpc_pipe_connect_b(tctx,
>+							 &p2,
>+							 p1->binding,
>+							 &ndr_table_drsuapi,
>+							 creds,
>+							 tctx->ev,
>+							 tctx->lp_ctx),
>+				   "connect p2");
>+
>+	torture_assert(tctx, test_DsBind(p1, tctx, &ds_bind_handle, NULL), "DsBind");
>+
>+	ZERO_STRUCT(r);
>+	r.in.bind_handle		= &ds_bind_handle;
>+	r.in.level			= 1;
>+	r.in.req			= &req;
>+	r.in.req->req1.codepage		= 1252; /* german */
>+	r.in.req->req1.language		= 0x00000407; /* german */
>+	r.in.req->req1.count		= 1;
>+	r.in.req->req1.names		= names;
>+	r.in.req->req1.format_flags	= DRSUAPI_DS_NAME_FLAG_NO_FLAGS;
>+
>+	r.in.req->req1.format_offered	= DRSUAPI_DS_NAME_FORMAT_SID_OR_SID_HISTORY;
>+	r.in.req->req1.format_desired	= DRSUAPI_DS_NAME_FORMAT_NT4_ACCOUNT;
>+
>+	r.out.level_out			= &level_out;
>+	r.out.ctr			= &ctr;
>+
>+	dom_sid = dom_sid_string(mem_ctx, torture_join_sid(priv->join));
>+
>+	names[0].str = dom_sid;
>+
>+	torture_comment(tctx, "Testing DsCrackNames on p1 with name '%s'"
>+			      " offered format: %d desired format:%d\n",
>+			names[0].str,
>+			r.in.req->req1.format_offered,
>+			r.in.req->req1.format_desired);
>+	status = dcerpc_drsuapi_DsCrackNames_r(p1->binding_handle, mem_ctx, &r);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		const char *errstr = nt_errstr(status);
>+		err_msg = talloc_asprintf(mem_ctx, "dcerpc_drsuapi_DsCrackNames failed - %s", errstr);
>+		torture_fail(tctx, err_msg);
>+	} else if (!W_ERROR_IS_OK(r.out.result)) {
>+		err_msg = talloc_asprintf(mem_ctx, "DsCrackNames failed - %s", win_errstr(r.out.result));
>+		torture_fail(tctx, err_msg);
>+	} else if (r.out.ctr->ctr1->array[0].status != DRSUAPI_DS_NAME_STATUS_OK) {
>+		err_msg = talloc_asprintf(mem_ctx, "DsCrackNames failed on name - %d",
>+					  r.out.ctr->ctr1->array[0].status);
>+		torture_fail(tctx, err_msg);
>+	}
>+
>+	torture_comment(tctx, "Testing DsCrackNames on p2 with name '%s'"
>+			      " offered format: %d desired format:%d\n",
>+			names[0].str,
>+			r.in.req->req1.format_offered,
>+			r.in.req->req1.format_desired);
>+	status = dcerpc_drsuapi_DsCrackNames_r(p2->binding_handle, mem_ctx, &r);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		const char *errstr = nt_errstr(status);
>+		err_msg = talloc_asprintf(mem_ctx, "dcerpc_drsuapi_DsCrackNames failed - %s", errstr);
>+		torture_fail(tctx, err_msg);
>+	} else if (!W_ERROR_IS_OK(r.out.result)) {
>+		err_msg = talloc_asprintf(mem_ctx, "DsCrackNames failed - %s", win_errstr(r.out.result));
>+		torture_fail(tctx, err_msg);
>+	} else if (r.out.ctr->ctr1->array[0].status != DRSUAPI_DS_NAME_STATUS_OK) {
>+		err_msg = talloc_asprintf(mem_ctx, "DsCrackNames failed on name - %d",
>+					  r.out.ctr->ctr1->array[0].status);
>+		torture_fail(tctx, err_msg);
>+	}
>+
>+	TALLOC_FREE(p1);
>+
>+	torture_comment(tctx, "Testing DsCrackNames on p2 (with p1 closed) with name '%s'"
>+			      " offered format: %d desired format:%d\n",
>+			names[0].str,
>+			r.in.req->req1.format_offered,
>+			r.in.req->req1.format_desired);
>+	status = dcerpc_drsuapi_DsCrackNames_r(p2->binding_handle, mem_ctx, &r);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		const char *errstr = nt_errstr(status);
>+		err_msg = talloc_asprintf(mem_ctx, "dcerpc_drsuapi_DsCrackNames failed - %s", errstr);
>+		torture_fail(tctx, err_msg);
>+	} else if (!W_ERROR_IS_OK(r.out.result)) {
>+		err_msg = talloc_asprintf(mem_ctx, "DsCrackNames failed - %s", win_errstr(r.out.result));
>+		torture_fail(tctx, err_msg);
>+	} else if (r.out.ctr->ctr1->array[0].status != DRSUAPI_DS_NAME_STATUS_OK) {
>+		err_msg = talloc_asprintf(mem_ctx, "DsCrackNames failed on name - %d",
>+					  r.out.ctr->ctr1->array[0].status);
>+		torture_fail(tctx, err_msg);
>+	}
>+
>+	torture_comment(tctx, "%s: ... finished\n", testname);
>+	return true;
>+}
>+
>+static bool test_DsBindAssocGroupAdmin(struct torture_context *tctx,
>+				       struct DsPrivate *priv,
>+				       struct cli_credentials *creds)
>+{
>+	return __test_DsBind_assoc_group(tctx, __func__, priv,
>+					 priv->admin_credentials);
>+}
>+
>+static bool test_DsBindAssocGroupDC(struct torture_context *tctx,
>+				    struct DsPrivate *priv,
>+				    struct cli_credentials *creds)
>+{
>+	return __test_DsBind_assoc_group(tctx, __func__, priv,
>+					 priv->dc_credentials);
>+}
>+
>+static bool test_DsBindAssocGroupWS(struct torture_context *tctx,
>+				    struct DsPrivate *priv,
>+				    struct cli_credentials *creds)
>+{
>+	struct test_join *wks_join = NULL;
>+	struct cli_credentials *wks_credentials = NULL;
>+	int rnd = rand() % 1000;
>+	char *wks_name = talloc_asprintf(tctx, "WKS%s%d", TEST_MACHINE_NAME, rnd);
>+	bool ret;
>+
>+	torture_comment(tctx, "%s: About to join workstation with name %s\n",
>+			__func__, wks_name);
>+	wks_join = torture_join_domain(tctx, wks_name, ACB_WSTRUST,
>+				       &wks_credentials);
>+	torture_assert(tctx, wks_join, "Failed to join as WORKSTATION");
>+	ret = __test_DsBind_assoc_group(tctx, __func__, priv,
>+					wks_credentials);
>+	torture_leave_domain(tctx, wks_join);
>+	return ret;
>+}
>+
> /**
>  * DRSUAPI test case implementation
>  */
>@@ -874,4 +1042,8 @@ void torture_rpc_drsuapi_tcase(struct torture_suite *suite)
> 	torture_tcase_add_simple_test(tcase, "DsReplicaUpdateRefs", (run_func)test_DsReplicaUpdateRefs);
> 
> 	torture_tcase_add_simple_test(tcase, "DsGetNCChanges", (run_func)test_DsGetNCChanges);
>+
>+	torture_tcase_add_simple_test(tcase, "DsBindAssocGroupAdmin", (run_func)test_DsBindAssocGroupAdmin);
>+	torture_tcase_add_simple_test(tcase, "DsBindAssocGroupDC", (run_func)test_DsBindAssocGroupDC);
>+	torture_tcase_add_simple_test(tcase, "DsBindAssocGroupWS", (run_func)test_DsBindAssocGroupWS);
> }
>-- 
>2.25.1
>
>
>From f56c5f6cc144e3b36ca54de2be991a2ad9bc8aef Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 5 Aug 2021 13:30:41 +0200
>Subject: [PATCH 05/11] auth_util: avoid talloc_tos() in copy_session_info()
>
>We want to use this also in code without existing
>stackframe.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14468
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> auth/auth_util.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/auth/auth_util.c b/auth/auth_util.c
>index f3586f1fc1e0..fe01babd1079 100644
>--- a/auth/auth_util.c
>+++ b/auth/auth_util.c
>@@ -26,26 +26,28 @@
> struct auth_session_info *copy_session_info(TALLOC_CTX *mem_ctx,
> 					    const struct auth_session_info *src)
> {
>+	TALLOC_CTX *frame = talloc_stackframe();
> 	struct auth_session_info *dst;
> 	DATA_BLOB blob;
> 	enum ndr_err_code ndr_err;
> 
> 	ndr_err = ndr_push_struct_blob(
> 		&blob,
>-		talloc_tos(),
>+		frame,
> 		src,
> 		(ndr_push_flags_fn_t)ndr_push_auth_session_info);
> 	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
> 		DBG_ERR("copy_session_info(): ndr_push_auth_session_info "
> 			"failed: %s\n",
> 			ndr_errstr(ndr_err));
>+		TALLOC_FREE(frame);
> 		return NULL;
> 	}
> 
> 	dst = talloc(mem_ctx, struct auth_session_info);
> 	if (dst == NULL) {
> 		DBG_ERR("talloc failed\n");
>-		TALLOC_FREE(blob.data);
>+		TALLOC_FREE(frame);
> 		return NULL;
> 	}
> 
>@@ -54,15 +56,16 @@ struct auth_session_info *copy_session_info(TALLOC_CTX *mem_ctx,
> 		dst,
> 		dst,
> 		(ndr_pull_flags_fn_t)ndr_pull_auth_session_info);
>-	TALLOC_FREE(blob.data);
> 
> 	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
> 		DBG_ERR("copy_session_info(): ndr_pull_auth_session_info "
> 			"failed: %s\n",
> 			ndr_errstr(ndr_err));
> 		TALLOC_FREE(dst);
>+		TALLOC_FREE(frame);
> 		return NULL;
> 	}
> 
>+	TALLOC_FREE(frame);
> 	return dst;
> }
>-- 
>2.25.1
>
>
>From 4c43e56925332e6b334ecdeb503148e81e5dbc29 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 5 Aug 2021 14:22:32 +0200
>Subject: [PATCH 06/11] s4:rpc_server/common: provide assoc_group aware
> dcesrv_samdb_connect_as_{system,user}() helpers
>
>We already had dcesrv_samdb_connect_as_system(), but it uses the per
>connection memory of auth_session_info and remote_address.
>
>But in order to use the samdb connection on a per association group
>context/policy handle, we need to make copies, which last for the
>whole lifetime of the 'samdb' context.
>
>We need the same logic also for all cases we make use of
>the almost same logic where we want to create a samdb context
>on behalf of the authenticated user (without allowing system access),
>so we introduce dcesrv_samdb_connect_as_user().
>
>In the end we need to replace all direct callers to samdb_connect()
>from source4/rpc_server.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14468
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source4/rpc_server/common/server_info.c | 121 ++++++++++++++++++++----
> 1 file changed, 105 insertions(+), 16 deletions(-)
>
>diff --git a/source4/rpc_server/common/server_info.c b/source4/rpc_server/common/server_info.c
>index 6e475bcc7964..a2af37653ef0 100644
>--- a/source4/rpc_server/common/server_info.c
>+++ b/source4/rpc_server/common/server_info.c
>@@ -28,6 +28,8 @@
> #include "param/param.h"
> #include "rpc_server/common/common.h"
> #include "libds/common/roles.h"
>+#include "auth/auth_util.h"
>+#include "lib/tsocket/tsocket.h"
> 
> /* 
>     Here are common server info functions used by some dcerpc server interfaces
>@@ -188,30 +190,117 @@ bool dcesrv_common_validate_share_name(TALLOC_CTX *mem_ctx, const char *share_na
> 	return true;
> }
> 
>-/*
>- * Open an ldb connection under the system session and save the remote users
>- * session details in a ldb_opaque. This will allow the audit logging to
>- * log the original session for operations performed in the system session.
>- */
>-struct ldb_context *dcesrv_samdb_connect_as_system(
>+static struct ldb_context *dcesrv_samdb_connect_common(
> 	TALLOC_CTX *mem_ctx,
>-	struct dcesrv_call_state *dce_call)
>+	struct dcesrv_call_state *dce_call,
>+	bool as_system)
> {
> 	struct ldb_context *samdb = NULL;
>+	struct auth_session_info *system_session_info = NULL;
>+	const struct auth_session_info *call_session_info =
>+		dcesrv_call_session_info(dce_call);
>+	struct auth_session_info *user_session_info = NULL;
>+	struct auth_session_info *ldb_session_info = NULL;
>+	struct auth_session_info *audit_session_info = NULL;
>+	struct tsocket_address *remote_address = NULL;
>+
>+	if (as_system) {
>+		system_session_info = system_session(dce_call->conn->dce_ctx->lp_ctx);
>+		if (system_session_info == NULL) {
>+			return NULL;
>+		}
>+	}
>+
>+	user_session_info = copy_session_info(mem_ctx, call_session_info);
>+	if (user_session_info == NULL) {
>+		return NULL;
>+	}
>+
>+	if (dce_call->conn->remote_address != NULL) {
>+		remote_address = tsocket_address_copy(dce_call->conn->remote_address,
>+						      user_session_info);
>+		if (remote_address == NULL) {
>+			return NULL;
>+		}
>+	}
>+
>+	if (system_session_info != NULL) {
>+		ldb_session_info = system_session_info;
>+		audit_session_info = user_session_info;
>+	} else {
>+		ldb_session_info = user_session_info;
>+		audit_session_info = NULL;
>+	}
>+
>+	/*
>+	 * We need to make sure every argument
>+	 * stays arround for the lifetime of 'samdb',
>+	 * typically it is allocated on the scope of
>+	 * an assoc group, so we can't reference dce_call->conn,
>+	 * as the assoc group may stay when the current connection
>+	 * gets disconnected.
>+	 *
>+	 * The following are global per process:
>+	 * - dce_call->conn->dce_ctx->lp_ctx
>+	 * - dce_call->event_ctx
>+	 * - system_session
>+	 *
>+	 * We make a copy of:
>+	 * - dce_call->conn->remote_address
>+	 * - dce_call->auth_state->session_info
>+	 */
> 	samdb = samdb_connect(
> 		mem_ctx,
> 		dce_call->event_ctx,
> 		dce_call->conn->dce_ctx->lp_ctx,
>-		system_session(dce_call->conn->dce_ctx->lp_ctx),
>-		dce_call->conn->remote_address,
>+		ldb_session_info,
>+		remote_address,
> 		0);
>-	if (samdb) {
>-		struct auth_session_info *session_info =
>-			dcesrv_call_session_info(dce_call);
>-		ldb_set_opaque(
>-			samdb,
>-			DSDB_NETWORK_SESSION_INFO,
>-			session_info);
>+	if (samdb == NULL) {
>+		talloc_free(user_session_info);
>+		return NULL;
> 	}
>+	talloc_move(samdb, &user_session_info);
>+
>+	if (audit_session_info != NULL) {
>+		int ret;
>+
>+		ret = ldb_set_opaque(samdb,
>+				     DSDB_NETWORK_SESSION_INFO,
>+				     audit_session_info);
>+		if (ret != LDB_SUCCESS) {
>+			talloc_free(samdb);
>+			return NULL;
>+		}
>+	}
>+
> 	return samdb;
> }
>+
>+/*
>+ * Open an ldb connection under the system session and save the remote users
>+ * session details in a ldb_opaque. This will allow the audit logging to
>+ * log the original session for operations performed in the system session.
>+ *
>+ * Access checks are required by the caller!
>+ */
>+struct ldb_context *dcesrv_samdb_connect_as_system(
>+	TALLOC_CTX *mem_ctx,
>+	struct dcesrv_call_state *dce_call)
>+{
>+	return dcesrv_samdb_connect_common(mem_ctx, dce_call,
>+					   true /* as_system */);
>+}
>+
>+/*
>+ * Open an ldb connection under the remote users session details.
>+ *
>+ * Access checks are done at the ldb level.
>+ */
>+struct ldb_context *dcesrv_samdb_connect_as_user(
>+	TALLOC_CTX *mem_ctx,
>+	struct dcesrv_call_state *dce_call)
>+{
>+	return dcesrv_samdb_connect_common(mem_ctx, dce_call,
>+					   false /* not as_system */);
>+}
>-- 
>2.25.1
>
>
>From 52752363badfb163bd208ef0e9fec569ecca487c Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 5 Aug 2021 13:31:29 +0200
>Subject: [PATCH 07/11] s4:rpc_server/drsuapi: make use of assoc_group aware
> dcesrv_samdb_connect_as_*() helpers
>
>This avoids a crash that's triggered by windows clients using
>DsCrackNames across multiple connections within an association group
>on the same DsBind context(policy) handle.
>
>It also improves the auditing for the dcesrv_samdb_connect_as_system() case.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14468
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> .../knownfail.d/drsuapi.DsBindAssocGroupWS    |  1 -
> source4/rpc_server/drsuapi/dcesrv_drsuapi.c   | 55 +++++++------------
> 2 files changed, 19 insertions(+), 37 deletions(-)
> delete mode 100644 selftest/knownfail.d/drsuapi.DsBindAssocGroupWS
>
>diff --git a/selftest/knownfail.d/drsuapi.DsBindAssocGroupWS b/selftest/knownfail.d/drsuapi.DsBindAssocGroupWS
>deleted file mode 100644
>index 9af5a904fdd3..000000000000
>--- a/selftest/knownfail.d/drsuapi.DsBindAssocGroupWS
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba4.rpc.drsuapi.*drsuapi.DsBindAssocGroupWS
>diff --git a/source4/rpc_server/drsuapi/dcesrv_drsuapi.c b/source4/rpc_server/drsuapi/dcesrv_drsuapi.c
>index d0fc3d2e4c3a..8bb4fb232278 100644
>--- a/source4/rpc_server/drsuapi/dcesrv_drsuapi.c
>+++ b/source4/rpc_server/drsuapi/dcesrv_drsuapi.c
>@@ -74,9 +74,7 @@ static WERROR dcesrv_drsuapi_DsBind(struct dcesrv_call_state *dce_call, TALLOC_C
> 	uint32_t supported_extensions;
> 	uint32_t req_length;
> 	int ret;
>-	struct auth_session_info *auth_info;
> 	WERROR werr;
>-	bool connected_as_system = false;
> 
> 	r->out.bind_info = NULL;
> 	ZERO_STRUCTP(r->out.bind_handle);
>@@ -87,45 +85,30 @@ static WERROR dcesrv_drsuapi_DsBind(struct dcesrv_call_state *dce_call, TALLOC_C
> 	/* if this is a DC connecting, give them system level access */
> 	werr = drs_security_level_check(dce_call, NULL, SECURITY_DOMAIN_CONTROLLER, NULL);
> 	if (W_ERROR_IS_OK(werr)) {
>-		DEBUG(3,(__location__ ": doing DsBind with system_session\n"));
>-		auth_info = system_session(dce_call->conn->dce_ctx->lp_ctx);
>-		connected_as_system = true;
>+		DBG_NOTICE("doing DsBind with system_session\n");
>+		b_state->sam_ctx_system = dcesrv_samdb_connect_as_system(b_state, dce_call);
>+		if (b_state->sam_ctx_system == NULL) {
>+			return WERR_DS_UNAVAILABLE;
>+		}
>+		b_state->sam_ctx = b_state->sam_ctx_system;
> 	} else {
>-		auth_info = dcesrv_call_session_info(dce_call);
>-	}
>-
>-	/*
>-	 * connect to the samdb
>-	 */
>-	b_state->sam_ctx = samdb_connect(
>-		b_state,
>-		dce_call->event_ctx,
>-		dce_call->conn->dce_ctx->lp_ctx,
>-		auth_info,
>-		dce_call->conn->remote_address,
>-		0);
>-	if (!b_state->sam_ctx) {
>-		return WERR_FOOBAR;
>-	}
>+		b_state->sam_ctx = dcesrv_samdb_connect_as_user(b_state, dce_call);
>+		if (b_state->sam_ctx == NULL) {
>+			return WERR_DS_UNAVAILABLE;
>+		}
> 
>-	if (connected_as_system) {
>-		b_state->sam_ctx_system = b_state->sam_ctx;
>-	} else {
>-		/* an RODC also needs system samdb access for secret
>-		   attribute replication */
>+		/*
>+		 * an RODC also needs system samdb access for secret
>+		 * attribute replication
>+		 */
> 		werr = drs_security_level_check(dce_call, NULL, SECURITY_RO_DOMAIN_CONTROLLER,
> 						samdb_domain_sid(b_state->sam_ctx));
> 		if (W_ERROR_IS_OK(werr)) {
>-			b_state->sam_ctx_system
>-			= samdb_connect(
>-				b_state,
>-				dce_call->event_ctx,
>-				dce_call->conn->dce_ctx->lp_ctx,
>-				system_session(dce_call->conn->dce_ctx->lp_ctx),
>-				dce_call->conn->remote_address,
>-				0);
>-			if (!b_state->sam_ctx_system) {
>-				return WERR_FOOBAR;
>+			DBG_NOTICE("doing DsBind as RODC\n");
>+			b_state->sam_ctx_system =
>+				dcesrv_samdb_connect_as_system(b_state, dce_call);
>+			if (b_state->sam_ctx_system == NULL) {
>+				return WERR_DS_UNAVAILABLE;
> 			}
> 		}
> 	}
>-- 
>2.25.1
>
>
>From 3ec544d35b9e8cb1f988a26f28a6d2ef527f9538 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 5 Aug 2021 14:22:47 +0200
>Subject: [PATCH 08/11] s4:rpc_server/dnsserver: make use of
> dcesrv_samdb_connect_as_user() helper
>
>This is not strictly required, but it makes it easier to audit that
>source4/rpc_server no longer calls samdb_connect() directly.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14468
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source4/rpc_server/dnsserver/dcerpc_dnsserver.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
>diff --git a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
>index a7b705e6e017..ea4d86d3c24e 100644
>--- a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
>+++ b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
>@@ -22,6 +22,7 @@
> #include "includes.h"
> #include "talloc.h"
> #include "rpc_server/dcerpc_server.h"
>+#include "rpc_server/common/common.h"
> #include "dsdb/samdb/samdb.h"
> #include "lib/util/dlinklist.h"
> #include "librpc/gen_ndr/ndr_dnsserver.h"
>@@ -106,8 +107,6 @@ static void dnsserver_reload_zones(struct dnsserver_state *dsstate)
> 
> static struct dnsserver_state *dnsserver_connect(struct dcesrv_call_state *dce_call)
> {
>-	struct auth_session_info *session_info =
>-		dcesrv_call_session_info(dce_call);
> 	struct dnsserver_state *dsstate;
> 	struct dnsserver_zone *zones, *z, *znext;
> 	struct dnsserver_partition *partitions, *p;
>@@ -127,13 +126,7 @@ static struct dnsserver_state *dnsserver_connect(struct dcesrv_call_state *dce_c
> 
> 	dsstate->lp_ctx = dce_call->conn->dce_ctx->lp_ctx;
> 
>-	/* FIXME: create correct auth_session_info for connecting user */
>-	dsstate->samdb = samdb_connect(dsstate,
>-				       dce_call->event_ctx,
>-				       dsstate->lp_ctx,
>-				       session_info,
>-				       dce_call->conn->remote_address,
>-				       0);
>+	dsstate->samdb = dcesrv_samdb_connect_as_user(dsstate, dce_call);
> 	if (dsstate->samdb == NULL) {
> 		DEBUG(0,("dnsserver: Failed to open samdb"));
> 		goto failed;
>-- 
>2.25.1
>
>
>From 6cf4afd6e21dc1fbe6df4d64e4d6c0671267bd65 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 5 Aug 2021 14:24:25 +0200
>Subject: [PATCH 09/11] s4:rpc_server/lsa: make use of
> dcesrv_samdb_connect_as_user() helper
>
>This avoids a crash that's triggered by windows clients using
>handles from OpenPolicy[2]() on across multiple connections within
>an association group.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14468
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source4/rpc_server/lsa/lsa_init.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
>diff --git a/source4/rpc_server/lsa/lsa_init.c b/source4/rpc_server/lsa/lsa_init.c
>index f33b61c40357..400c50930792 100644
>--- a/source4/rpc_server/lsa/lsa_init.c
>+++ b/source4/rpc_server/lsa/lsa_init.c
>@@ -71,12 +71,7 @@ NTSTATUS dcesrv_lsa_get_policy_state(struct dcesrv_call_state *dce_call,
> 	}
> 
> 	/* make sure the sam database is accessible */
>-	state->sam_ldb = samdb_connect(state,
>-				       dce_call->event_ctx,
>-				       dce_call->conn->dce_ctx->lp_ctx,
>-				       session_info,
>-				       dce_call->conn->remote_address,
>-				       0);
>+	state->sam_ldb = dcesrv_samdb_connect_as_user(state, dce_call);
> 	if (state->sam_ldb == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>-- 
>2.25.1
>
>
>From 643cd4da264a83be2ac6da26475c85f7e4b40b70 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 5 Aug 2021 15:09:04 +0200
>Subject: [PATCH 10/11] s4:rpc_server/netlogon: make use of
> dcesrv_samdb_connect_as_*() helper
>
>This is not strictly required, but it makes it easier to audit that
>source4/rpc_server no longer calls samdb_connect() directly and
>also improves auditing for the dcesrv_samdb_connect_as_system() case.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14468
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source4/rpc_server/netlogon/dcerpc_netlogon.c | 136 +++---------------
> 1 file changed, 18 insertions(+), 118 deletions(-)
>
>diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
>index 6860202a985b..c39124b6d3b8 100644
>--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
>+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
>@@ -23,6 +23,7 @@
> 
> #include "includes.h"
> #include "rpc_server/dcerpc_server.h"
>+#include "rpc_server/common/common.h"
> #include "auth/auth.h"
> #include "auth/auth_sam_reply.h"
> #include "dsdb/samdb/samdb.h"
>@@ -295,12 +296,7 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3_helper(
> 		return NT_STATUS_INVALID_PARAMETER;
> 	}
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				dce_call->conn->dce_ctx->lp_ctx,
>-				system_session(dce_call->conn->dce_ctx->lp_ctx),
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_system(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>@@ -768,12 +764,7 @@ static NTSTATUS dcesrv_netr_ServerPasswordSet(struct dcesrv_call_state *dce_call
> 							&creds);
> 	NT_STATUS_NOT_OK_RETURN(nt_status);
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				dce_call->conn->dce_ctx->lp_ctx,
>-				system_session(dce_call->conn->dce_ctx->lp_ctx),
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_system(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>@@ -837,12 +828,7 @@ static NTSTATUS dcesrv_netr_ServerPasswordSet2(struct dcesrv_call_state *dce_cal
> 							&creds);
> 	NT_STATUS_NOT_OK_RETURN(nt_status);
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				dce_call->conn->dce_ctx->lp_ctx,
>-				system_session(dce_call->conn->dce_ctx->lp_ctx),
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_system(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>@@ -1729,8 +1715,6 @@ static NTSTATUS dcesrv_netr_AccountSync(struct dcesrv_call_state *dce_call, TALL
> static WERROR dcesrv_netr_GetDcName(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
> 		       struct netr_GetDcName *r)
> {
>-	struct auth_session_info *session_info =
>-		dcesrv_call_session_info(dce_call);
> 	const char * const attrs[] = { NULL };
> 	struct ldb_context *sam_ctx;
> 	struct ldb_message **res;
>@@ -1757,12 +1741,7 @@ static WERROR dcesrv_netr_GetDcName(struct dcesrv_call_state *dce_call, TALLOC_C
> 		 */
> 	}
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				dce_call->conn->dce_ctx->lp_ctx,
>-				session_info,
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return WERR_DS_UNAVAILABLE;
> 	}
>@@ -1964,13 +1943,8 @@ static WERROR dcesrv_netr_LogonControl_base_call(struct dcesrv_netr_LogonControl
> 		if (!ok) {
> 			struct ldb_context *sam_ctx;
> 
>-			sam_ctx = samdb_connect(
>-				state,
>-				state->dce_call->event_ctx,
>-				lp_ctx,
>-				system_session(lp_ctx),
>-				state->dce_call->conn->remote_address,
>-				0);
>+			sam_ctx = dcesrv_samdb_connect_as_system(state,
>+								 state->dce_call);
> 			if (sam_ctx == NULL) {
> 				return WERR_DS_UNAVAILABLE;
> 			}
>@@ -2167,8 +2141,6 @@ static WERROR fill_trusted_domains_array(TALLOC_CTX *mem_ctx,
> static WERROR dcesrv_netr_GetAnyDCName(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
> 		       struct netr_GetAnyDCName *r)
> {
>-	struct auth_session_info *session_info =
>-		dcesrv_call_session_info(dce_call);
> 	struct netr_DomainTrustList *trusts;
> 	struct ldb_context *sam_ctx;
> 	struct loadparm_context *lp_ctx = dce_call->conn->dce_ctx->lp_ctx;
>@@ -2182,12 +2154,7 @@ static WERROR dcesrv_netr_GetAnyDCName(struct dcesrv_call_state *dce_call, TALLO
> 		r->in.domainname = lpcfg_workgroup(lp_ctx);
> 	}
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				lp_ctx,
>-				session_info,
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return WERR_DS_UNAVAILABLE;
> 	}
>@@ -2329,17 +2296,9 @@ static WERROR dcesrv_netr_NETRLOGONCOMPUTECLIENTDIGEST(struct dcesrv_call_state
> static WERROR dcesrv_netr_DsRGetSiteName(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
> 				  struct netr_DsRGetSiteName *r)
> {
>-	struct auth_session_info *session_info =
>-		dcesrv_call_session_info(dce_call);
> 	struct ldb_context *sam_ctx;
>-	struct loadparm_context *lp_ctx = dce_call->conn->dce_ctx->lp_ctx;
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				lp_ctx,
>-				session_info,
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return WERR_DS_UNAVAILABLE;
> 	}
>@@ -2538,12 +2497,7 @@ static NTSTATUS dcesrv_netr_LogonGetDomainInfo(struct dcesrv_call_state *dce_cal
> 	}
> 	NT_STATUS_NOT_OK_RETURN(status);
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				dce_call->conn->dce_ctx->lp_ctx,
>-				system_session(dce_call->conn->dce_ctx->lp_ctx),
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_system(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>@@ -2961,12 +2915,7 @@ static NTSTATUS dcesrv_netr_NetrLogonSendToSam(struct dcesrv_call_state *dce_cal
> 		return NT_STATUS_INVALID_PARAMETER;
> 	}
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				dce_call->conn->dce_ctx->lp_ctx,
>-				system_session(dce_call->conn->dce_ctx->lp_ctx),
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_system(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>@@ -3077,8 +3026,6 @@ static void dcesrv_netr_DsRGetDCName_base_done(struct tevent_req *subreq);
> static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName_base_state *state)
> {
> 	struct dcesrv_call_state *dce_call = state->dce_call;
>-	struct auth_session_info *session_info =
>-		dcesrv_call_session_info(dce_call);
> 	struct imessaging_context *imsg_ctx =
> 		dcesrv_imessaging_context(dce_call->conn);
> 	TALLOC_CTX *mem_ctx = state->mem_ctx;
>@@ -3101,12 +3048,7 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
> 
> 	ZERO_STRUCTP(r->out.info);
> 
>-	sam_ctx = samdb_connect(state,
>-				dce_call->event_ctx,
>-				lp_ctx,
>-				session_info,
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return WERR_DS_UNAVAILABLE;
> 	}
>@@ -3561,11 +3503,8 @@ static WERROR dcesrv_netr_NetrEnumerateTrustedDomainsEx(struct dcesrv_call_state
> static WERROR dcesrv_netr_DsRAddressToSitenamesExW(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
> 						   struct netr_DsRAddressToSitenamesExW *r)
> {
>-	struct auth_session_info *session_info =
>-		dcesrv_call_session_info(dce_call);
> 	struct ldb_context *sam_ctx;
> 	struct netr_DsRAddressToSitenamesExWCtr *ctr;
>-	struct loadparm_context *lp_ctx = dce_call->conn->dce_ctx->lp_ctx;
> 	sa_family_t sin_family;
> 	struct sockaddr_in *addr;
> #ifdef HAVE_IPV6
>@@ -3578,12 +3517,7 @@ static WERROR dcesrv_netr_DsRAddressToSitenamesExW(struct dcesrv_call_state *dce
> 	const char *res;
> 	uint32_t i;
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				lp_ctx,
>-				session_info,
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return WERR_DS_UNAVAILABLE;
> 	}
>@@ -3695,18 +3629,10 @@ static WERROR dcesrv_netr_DsRAddressToSitenamesW(struct dcesrv_call_state *dce_c
> static WERROR dcesrv_netr_DsrGetDcSiteCoverageW(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
> 		       struct netr_DsrGetDcSiteCoverageW *r)
> {
>-	struct auth_session_info *session_info =
>-		dcesrv_call_session_info(dce_call);
> 	struct ldb_context *sam_ctx;
> 	struct DcSitesCtr *ctr;
>-	struct loadparm_context *lp_ctx = dce_call->conn->dce_ctx->lp_ctx;
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				lp_ctx,
>-				session_info,
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return WERR_DS_UNAVAILABLE;
> 	}
>@@ -3832,8 +3758,6 @@ static WERROR dcesrv_netr_DsrEnumerateDomainTrusts(struct dcesrv_call_state *dce
> 						   TALLOC_CTX *mem_ctx,
> 						   struct netr_DsrEnumerateDomainTrusts *r)
> {
>-	struct auth_session_info *session_info =
>-		dcesrv_call_session_info(dce_call);
> 	struct netr_DomainTrustList *trusts;
> 	struct ldb_context *sam_ctx;
> 	int ret;
>@@ -3875,12 +3799,7 @@ static WERROR dcesrv_netr_DsrEnumerateDomainTrusts(struct dcesrv_call_state *dce
> 	trusts->count = 0;
> 	r->out.trusts = trusts;
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				lp_ctx,
>-				session_info,
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return WERR_GEN_FAILURE;
> 	}
>@@ -3990,7 +3909,6 @@ static WERROR dcesrv_netr_DsRGetForestTrustInformation(struct dcesrv_call_state
> 						       TALLOC_CTX *mem_ctx,
> 						       struct netr_DsRGetForestTrustInformation *r)
> {
>-	struct loadparm_context *lp_ctx = dce_call->conn->dce_ctx->lp_ctx;
> 	struct auth_session_info *session_info =
> 		dcesrv_call_session_info(dce_call);
> 	struct imessaging_context *imsg_ctx =
>@@ -4014,12 +3932,7 @@ static WERROR dcesrv_netr_DsRGetForestTrustInformation(struct dcesrv_call_state
> 		return WERR_INVALID_FLAGS;
> 	}
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				lp_ctx,
>-				session_info,
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return WERR_GEN_FAILURE;
> 	}
>@@ -4146,9 +4059,6 @@ static NTSTATUS dcesrv_netr_GetForestTrustInformation(struct dcesrv_call_state *
> 						      TALLOC_CTX *mem_ctx,
> 						      struct netr_GetForestTrustInformation *r)
> {
>-	struct auth_session_info *session_info =
>-		dcesrv_call_session_info(dce_call);
>-	struct loadparm_context *lp_ctx = dce_call->conn->dce_ctx->lp_ctx;
> 	struct netlogon_creds_CredentialState *creds = NULL;
> 	struct ldb_context *sam_ctx = NULL;
> 	struct ldb_dn *domain_dn = NULL;
>@@ -4172,12 +4082,7 @@ static NTSTATUS dcesrv_netr_GetForestTrustInformation(struct dcesrv_call_state *
> 		return NT_STATUS_NOT_IMPLEMENTED;
> 	}
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				lp_ctx,
>-				session_info,
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INTERNAL_ERROR;
> 	}
>@@ -4271,12 +4176,7 @@ static NTSTATUS dcesrv_netr_ServerGetTrustInfo(struct dcesrv_call_state *dce_cal
> 		return NT_STATUS_INVALID_PARAMETER;
> 	}
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				lp_ctx,
>-				system_session(lp_ctx),
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_system(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>-- 
>2.25.1
>
>
>From aef404bc7861f769c44def268a0ff33cca43b764 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 5 Aug 2021 14:24:40 +0200
>Subject: [PATCH 11/11] s4:rpc_server/samr: make use of
> dcesrv_samdb_connect_as_*() helper
>
>This avoids a crash that's triggered by windows clients using
>handles from samr_Connect*() on across multiple connections within
>an association group.
>
>In other cases is not strictly required, but it makes it easier to audit that
>source4/rpc_server no longer calls samdb_connect() directly and also
>improves the auditing for the dcesrv_samdb_connect_as_system() case.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14468
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source4/rpc_server/samr/dcesrv_samr.c   | 19 ++-------------
> source4/rpc_server/samr/samr_password.c | 31 ++++---------------------
> 2 files changed, 7 insertions(+), 43 deletions(-)
>
>diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
>index cda887d45eee..878e1eddaf25 100644
>--- a/source4/rpc_server/samr/dcesrv_samr.c
>+++ b/source4/rpc_server/samr/dcesrv_samr.c
>@@ -212,8 +212,6 @@ exit:
> static NTSTATUS dcesrv_samr_Connect(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
> 			     struct samr_Connect *r)
> {
>-	struct auth_session_info *session_info =
>-		dcesrv_call_session_info(dce_call);
> 	struct samr_connect_state *c_state;
> 	struct dcesrv_handle *handle;
> 
>@@ -225,18 +223,12 @@ static NTSTATUS dcesrv_samr_Connect(struct dcesrv_call_state *dce_call, TALLOC_C
> 	}
> 
> 	/* make sure the sam database is accessible */
>-	c_state->sam_ctx = samdb_connect(c_state,
>-					 dce_call->event_ctx,
>-					 dce_call->conn->dce_ctx->lp_ctx,
>-					 session_info,
>-					 dce_call->conn->remote_address,
>-					 0);
>+	c_state->sam_ctx = dcesrv_samdb_connect_as_user(c_state, dce_call);
> 	if (c_state->sam_ctx == NULL) {
> 		talloc_free(c_state);
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
> 
>-
> 	handle = dcesrv_handle_create(dce_call, SAMR_HANDLE_CONNECT);
> 	if (!handle) {
> 		talloc_free(c_state);
>@@ -4807,8 +4799,6 @@ static NTSTATUS dcesrv_samr_RemoveMultipleMembersFromAlias(struct dcesrv_call_st
> static NTSTATUS dcesrv_samr_GetDomPwInfo(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
> 				  struct samr_GetDomPwInfo *r)
> {
>-	struct auth_session_info *session_info =
>-		dcesrv_call_session_info(dce_call);
> 	struct ldb_message **msgs;
> 	int ret;
> 	const char * const attrs[] = {"minPwdLength", "pwdProperties", NULL };
>@@ -4816,12 +4806,7 @@ static NTSTATUS dcesrv_samr_GetDomPwInfo(struct dcesrv_call_state *dce_call, TAL
> 
> 	ZERO_STRUCTP(r->out.info);
> 
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				dce_call->conn->dce_ctx->lp_ctx,
>-				session_info,
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c
>index 0161b7522a17..eee4c7f3135b 100644
>--- a/source4/rpc_server/samr/samr_password.c
>+++ b/source4/rpc_server/samr/samr_password.c
>@@ -22,6 +22,7 @@
> 
> #include "includes.h"
> #include "rpc_server/dcerpc_server.h"
>+#include "rpc_server/common/common.h"
> #include "rpc_server/samr/dcesrv_samr.h"
> #include "system/time.h"
> #include "lib/crypto/md4.h"
>@@ -156,12 +157,7 @@ NTSTATUS dcesrv_samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call,
> 
> 	/* Connect to a SAMDB with system privileges for fetching the old pw
> 	 * hashes. */
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				dce_call->conn->dce_ctx->lp_ctx,
>-				system_session(dce_call->conn->dce_ctx->lp_ctx),
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_system(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>@@ -262,12 +258,7 @@ NTSTATUS dcesrv_samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call,
> 	}
> 
> 	/* Connect to a SAMDB with user privileges for the password change */
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				dce_call->conn->dce_ctx->lp_ctx,
>-				session_info,
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>@@ -340,8 +331,6 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call,
> 					 TALLOC_CTX *mem_ctx,
> 					 struct samr_ChangePasswordUser3 *r)
> {
>-	struct auth_session_info *session_info =
>-		dcesrv_call_session_info(dce_call);
> 	struct imessaging_context *imsg_ctx =
> 		dcesrv_imessaging_context(dce_call->conn);
> 	NTSTATUS status = NT_STATUS_WRONG_PASSWORD;
>@@ -387,12 +376,7 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call,
> 
> 	/* Connect to a SAMDB with system privileges for fetching the old pw
> 	 * hashes. */
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				dce_call->conn->dce_ctx->lp_ctx,
>-				system_session(dce_call->conn->dce_ctx->lp_ctx),
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_system(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>@@ -498,12 +482,7 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call,
> 	}
> 
> 	/* Connect to a SAMDB with user privileges for the password change */
>-	sam_ctx = samdb_connect(mem_ctx,
>-				dce_call->event_ctx,
>-				dce_call->conn->dce_ctx->lp_ctx,
>-				session_info,
>-				dce_call->conn->remote_address,
>-				0);
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>-- 
>2.25.1
>

Flags:

abartlet: review+
metze: ci-passed+

Actions: View

Attachments on bug 14468: 16718 | 16820 | 16821 | 16822 | 16823 | 16824 | 16827 | 16828 | 16829 | 16830 | 16886 | 16971 | 16975