The Samba-Bugzilla – Attachment 16411 Details for
Bug 14611
CVE-2021-20251 [SECURITY] Bad password count not incremented atomically
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Proposed patch for bad password path
bug-14611-bad-password-proposed-fix.patch (text/plain), 4.53 KB, created by
Gary Lockyer
on 2021-01-28 02:30:32 UTC
(
hide
)
Description:
Proposed patch for bad password path
Filename:
MIME Type:
Creator:
Gary Lockyer
Created:
2021-01-28 02:30:32 UTC
Size:
4.53 KB
patch
obsolete
>From bd847e40448a0046c731dcdc02596df269ba2c87 Mon Sep 17 00:00:00 2001 >From: Gary Lockyer <gary@catalyst.net.nz> >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 <gary@catalyst.net.nz> >--- > 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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
gary
:
ci-passed+
Actions:
View
Attachments on
bug 14611
:
16386
|
16390
|
16391
|
16392
|
16393
|
16411
|
16474
|
16483
|
16484
|
16485
|
16486
|
16487
|
16573
|
17410
|
17411
|
17413
|
17436
|
17452
|
17453
|
17456
|
17513
|
17521
|
17733
|
17734