From 38c85f5471cc367bce2018e65af346ffb91f3540 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Aug 2021 11:24:26 +0200 Subject: [PATCH 01/11] CVE-2021-3738 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 Reviewed-by: Andrew Bartlett --- 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 2ae2ba031e9..862c5f592b7 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 f1a5bba05b8..e81b2fe3746 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 f23c33fed7f..78b8a2e0564 100644 --- a/source4/torture/rpc/drsuapi_cracknames.c +++ b/source4/torture/rpc/drsuapi_cracknames.c @@ -803,7 +803,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 9165ea7352b2d56a7aabed40b4442c99f239a9be Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Aug 2021 09:58:37 +0200 Subject: [PATCH 02/11] CVE-2021-3738 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 Reviewed-by: Andrew Bartlett --- 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 862c5f592b7..1cd595e5d8e 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 e81b2fe3746..f327c54cda4 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 7ba6379e642f6c55b0980f476e25bb9c97bf67bb Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Aug 2021 10:34:06 +0200 Subject: [PATCH 03/11] CVE-2021-3738 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 Reviewed-by: Andrew Bartlett --- 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 1cd595e5d8e..799a641dbb9 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 f327c54cda4..3cc4be49d99 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 552f31d6362190b614fc1a8c87743627f4ea5da1 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Aug 2021 11:26:16 +0200 Subject: [PATCH 04/11] CVE-2021-3738 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 Reviewed-by: Andrew Bartlett --- .../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 00000000000..9af5a904fdd --- /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 799a641dbb9..d3e18ca246b 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 3cc18d43a2a462127f560a0e3cc22e20f7a6b25b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Aug 2021 13:30:41 +0200 Subject: [PATCH 05/11] CVE-2021-3738 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 Reviewed-by: Andrew Bartlett --- 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 f3586f1fc1e..fe01babd107 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 8e9ecec541c5333dba5dedd66c253e583843c7aa Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Aug 2021 14:22:32 +0200 Subject: [PATCH 06/11] CVE-2021-3738 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 Reviewed-by: Andrew Bartlett --- 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 6e475bcc796..a2af37653ef 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 ecb650f3846335f52f1bd11c72e417bee423bd17 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Aug 2021 13:31:29 +0200 Subject: [PATCH 07/11] CVE-2021-3738 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 Reviewed-by: Andrew Bartlett --- .../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 9af5a904fdd..00000000000 --- 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 d0fc3d2e4c3..8bb4fb23227 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 0c47b46bd8424a96ff714886c764f7f098a748d7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Aug 2021 14:22:47 +0200 Subject: [PATCH 08/11] CVE-2021-3738 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 Reviewed-by: Andrew Bartlett --- 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 a5948c7969b..be56c7ad7a2 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 62c461992d78c9d69ab8d3447aca9fd66beb63d9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Aug 2021 14:24:25 +0200 Subject: [PATCH 09/11] CVE-2021-3738 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 Reviewed-by: Andrew Bartlett --- 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 f33b61c4035..400c5093079 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 17f5970ca650019eb45ffbd8aa443384caa42f0e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Aug 2021 15:09:04 +0200 Subject: [PATCH 10/11] CVE-2021-3738 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 Reviewed-by: Andrew Bartlett --- 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 ac8f2eab657..03a8f585c41 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" @@ -286,12 +287,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; } @@ -759,12 +755,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; } @@ -828,12 +819,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; } @@ -1719,8 +1705,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; @@ -1747,12 +1731,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; } @@ -1954,13 +1933,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; } @@ -2157,8 +2131,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; @@ -2172,12 +2144,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; } @@ -2319,17 +2286,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; } @@ -2528,12 +2487,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; } @@ -2951,12 +2905,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; } @@ -3067,8 +3016,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; @@ -3091,12 +3038,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; } @@ -3551,11 +3493,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 @@ -3568,12 +3507,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; } @@ -3685,18 +3619,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; } @@ -3822,8 +3748,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; @@ -3865,12 +3789,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; } @@ -3980,7 +3899,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 = @@ -4004,12 +3922,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; } @@ -4136,9 +4049,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; @@ -4162,12 +4072,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; } @@ -4261,12 +4166,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 6e0b7a5f1a6ad962bfea97a000d72b819c1a141c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Aug 2021 14:24:40 +0200 Subject: [PATCH 11/11] CVE-2021-3738 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 Reviewed-by: Andrew Bartlett --- 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 cda887d45ee..878e1eddaf2 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 83b104fbd0e..437e8f66275 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