From 9ffd594eed5315722ecbe2e6c1ed84cbc1ea6ba8 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher 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 --- 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 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 --- 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 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 --- 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 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 --- .../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 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 --- 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 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 --- 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 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 --- .../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 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 --- 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 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 --- 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 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 --- 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 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 --- 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