From: Joseph Sutton Date: Tue, 2 Aug 2022 02:39:43 +0000 (+1200) Subject: CVE-2021-20251 s4-rpc_server: Extend scope of transaction for ChangePasswordUser3 X-Git-Tag: talloc-2.4.0~1060 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fcabcb326d385c1e1daaa8dae9820e33a3868f56;p=thirdparty%2Fsamba.git CVE-2021-20251 s4-rpc_server: Extend scope of transaction for ChangePasswordUser3 Now the initial account search is performed under the transaction, ensuring the overall password change is atomic. We set DSDB_SESSION_INFO to drop our privileges to those of the user before we perform the actual password change, and restore them afterwards if we need to update the bad password count. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett --- diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index c6cca985d8e..4691f9a47a9 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -330,6 +330,8 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, = lpcfg_ntlm_auth(lp_ctx); gnutls_cipher_hd_t cipher_hnd = NULL; gnutls_datum_t nt_session_key; + struct auth_session_info *call_session_info = NULL; + struct auth_session_info *old_session_info = NULL; int rc; *r->out.dominfo = NULL; @@ -354,6 +356,12 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, return NT_STATUS_INVALID_SYSTEM_SERVICE; } + ret = ldb_transaction_start(sam_ctx); + if (ret != LDB_SUCCESS) { + DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx))); + return NT_STATUS_TRANSACTION_ABORTED; + } + /* * We use authsam_search_account() to be consistent with the * other callers in the bad password and audit log handling @@ -365,6 +373,7 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, ldb_get_default_basedn(sam_ctx), &msg); if (!NT_STATUS_IS_OK(status)) { + ldb_transaction_cancel(sam_ctx); goto failed; } @@ -375,11 +384,13 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, status = samdb_result_passwords(mem_ctx, lp_ctx, msg, &nt_pwd); if (!NT_STATUS_IS_OK(status) ) { + ldb_transaction_cancel(sam_ctx); goto failed; } if (!nt_pwd) { status = NT_STATUS_WRONG_PASSWORD; + ldb_transaction_cancel(sam_ctx); goto failed; } @@ -395,6 +406,7 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, NULL); if (rc < 0) { status = gnutls_error_to_ntstatus(rc, NT_STATUS_CRYPTO_SYSTEM_INVALID); + ldb_transaction_cancel(sam_ctx); goto failed; } @@ -404,17 +416,20 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, gnutls_cipher_deinit(cipher_hnd); if (rc < 0) { status = gnutls_error_to_ntstatus(rc, NT_STATUS_CRYPTO_SYSTEM_INVALID); + ldb_transaction_cancel(sam_ctx); goto failed; } if (!extract_pw_from_buffer(mem_ctx, r->in.nt_password->data, &new_password)) { DEBUG(3,("samr: failed to decode password buffer\n")); status = NT_STATUS_WRONG_PASSWORD; + ldb_transaction_cancel(sam_ctx); goto failed; } if (r->in.nt_verifier == NULL) { status = NT_STATUS_WRONG_PASSWORD; + ldb_transaction_cancel(sam_ctx); goto failed; } @@ -424,23 +439,25 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, rc = E_old_pw_hash(new_nt_hash, nt_pwd->hash, nt_verifier.hash); if (rc != 0) { status = gnutls_error_to_ntstatus(rc, NT_STATUS_ACCESS_DISABLED_BY_POLICY_OTHER); + ldb_transaction_cancel(sam_ctx); goto failed; } if (!mem_equal_const_time(nt_verifier.hash, r->in.nt_verifier->hash, 16)) { status = NT_STATUS_WRONG_PASSWORD; + ldb_transaction_cancel(sam_ctx); goto failed; } - /* Connect to a SAMDB with user privileges for the password change */ - sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call); - if (sam_ctx == NULL) { - return NT_STATUS_INVALID_SYSTEM_SERVICE; - } + /* Drop to user privileges for the password change */ - ret = ldb_transaction_start(sam_ctx); + old_session_info = ldb_get_opaque(sam_ctx, DSDB_SESSION_INFO); + call_session_info = dcesrv_call_session_info(dce_call); + + ret = ldb_set_opaque(sam_ctx, DSDB_SESSION_INFO, call_session_info); if (ret != LDB_SUCCESS) { - DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx))); - return NT_STATUS_TRANSACTION_ABORTED; + status = NT_STATUS_INVALID_SYSTEM_SERVICE; + ldb_transaction_cancel(sam_ctx); + goto failed; } /* Performs the password modification. We pass the old hashes read out @@ -454,6 +471,11 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, &reason, &dominfo); + /* Restore our privileges to system level */ + if (old_session_info != NULL) { + ldb_set_opaque(sam_ctx, DSDB_SESSION_INFO, old_session_info); + } + if (!NT_STATUS_IS_OK(status)) { ldb_transaction_cancel(sam_ctx); goto failed; @@ -489,7 +511,9 @@ failed: /* Only update the badPwdCount if we found the user */ if (NT_STATUS_EQUAL(status, NT_STATUS_WRONG_PASSWORD)) { - NTSTATUS bad_pwd_status = authsam_update_bad_pwd_count( + NTSTATUS bad_pwd_status; + + bad_pwd_status = authsam_update_bad_pwd_count( sam_ctx, msg, ldb_get_default_basedn(sam_ctx)); if (NT_STATUS_EQUAL(bad_pwd_status, NT_STATUS_ACCOUNT_LOCKED_OUT)) { status = bad_pwd_status;