From bd847e40448a0046c731dcdc02596df269ba2c87 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Wed, 27 Jan 2021 14:24:58 +1300 Subject: [PATCH] s4 auth: make bad password count increment atomic Ensure that the bad password count is incremented atomically. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Gary Lockyer --- source4/auth/sam.c | 121 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 101 insertions(+), 20 deletions(-) diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 39e48c26b52..75e03aa3e08 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -823,10 +823,12 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, "pwdProperties", NULL }; int ret; - NTSTATUS status; - struct ldb_result *domain_res; + NTSTATUS status = NT_STATUS_OK; + struct ldb_result *domain_res = NULL; struct ldb_message *msg_mod = NULL; struct ldb_message *pso_msg = NULL; + struct ldb_result *res = NULL; + uint16_t acct_flags = 0; TALLOC_CTX *mem_ctx; mem_ctx = talloc_new(msg); @@ -851,14 +853,68 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, ret, ldb_dn_get_linearized(msg->dn)); } - status = dsdb_update_bad_pwd_count(mem_ctx, sam_ctx, - msg, domain_res->msgs[0], pso_msg, - &msg_mod); + /* + * To ensure that the bad password count is updated atomically, + * we need to: + * begin a transaction + * re-read the account details + * update the bad password count + * commit the transaction. + */ + + /* + * Start a new transaction + */ + ret = ldb_transaction_start(sam_ctx); + if (ret != LDB_SUCCESS) { + goto error; + } + + /* + * Re-read the account details + */ + ret = dsdb_search_dn( + sam_ctx, + mem_ctx, + &res, + msg->dn, + user_attrs, + 0); + if (ret != LDB_SUCCESS) { + goto error; + } + + /* + * Ensure the account has not been locked out by another request + */ + acct_flags = samdb_result_acct_flags( + res->msgs[0], + "msDS-User-Account-Control-Computed"); + if (acct_flags & ACB_AUTOLOCK) { + DBG_WARNING( + "Account for user %s was locked out.\n", + ldb_dn_get_linearized(msg->dn)); + status = NT_STATUS_ACCOUNT_LOCKED_OUT; + goto cleanup; + } + + /* + * Update the bad password count and if required lock the account + */ + status = dsdb_update_bad_pwd_count( + mem_ctx, + sam_ctx, + res->msgs[0], + domain_res->msgs[0], + pso_msg, + &msg_mod); if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(mem_ctx); - return status; + goto error; } + /* + * Write the data back to disk if required. + */ if (msg_mod != NULL) { struct ldb_request *req; @@ -869,35 +925,60 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, ldb_op_default_callback, NULL); if (ret != LDB_SUCCESS) { - goto done; + goto error; } - ret = ldb_request_add_control(req, - DSDB_CONTROL_FORCE_RODC_LOCAL_CHANGE, - false, NULL); + ret = ldb_request_add_control( + req, + DSDB_CONTROL_FORCE_RODC_LOCAL_CHANGE, + false, + NULL); if (ret != LDB_SUCCESS) { talloc_free(req); - goto done; + goto error; } ret = dsdb_autotransaction_request(sam_ctx, req); talloc_free(req); + if (ret != LDB_SUCCESS) { + goto error; + } } -done: + TALLOC_FREE(mem_ctx); + ret = ldb_transaction_commit(sam_ctx); if (ret != LDB_SUCCESS) { - DBG_ERR("Failed to update badPwdCount, badPasswordTime or " - "set lockoutTime on %s: %s\n", - ldb_dn_get_linearized(msg->dn), - ldb_errstring(sam_ctx)); - TALLOC_FREE(mem_ctx); + DBG_ERR("Error (%d) %s, committing transaction," + " while updating bad password count" + " for (%s)\n", + ret, + ldb_errstring(sam_ctx), + ldb_dn_get_linearized(msg->dn)); return NT_STATUS_INTERNAL_ERROR; } + return NT_STATUS_OK; +error: + DBG_ERR("Failed to update badPwdCount, badPasswordTime or " + "set lockoutTime on %s: %s\n", + ldb_dn_get_linearized(msg->dn), + ldb_errstring(sam_ctx)); +cleanup: + ret = ldb_transaction_cancel(sam_ctx); + if (ret != LDB_SUCCESS) { + DBG_ERR("Error rolling back transaction," + " while updating bad password count" + " on %s: %s\n", + ldb_dn_get_linearized(msg->dn), + ldb_errstring(sam_ctx)); + } TALLOC_FREE(mem_ctx); - return NT_STATUS_OK; -} + if (NT_STATUS_EQUAL(NT_STATUS_ACCOUNT_LOCKED_OUT, status)) { + return NT_STATUS_ACCOUNT_LOCKED_OUT; + } + return NT_STATUS_INTERNAL_ERROR; +} static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, struct ldb_message *msg_mod, -- 2.25.1