From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Sat, 9 Jul 2022 03:54:12 +0000 (+1200)
Subject: CVE-2021-20251 s4:dsdb: Make badPwdCount update atomic
X-Git-Tag: talloc-2.4.0~1073
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=96479747bdb5bc5f33d903085f5f69793f369e3a;p=thirdparty%2Fsamba.git

CVE-2021-20251 s4:dsdb: Make badPwdCount update atomic

We reread the account details inside the transaction in case the account
has been locked out in the meantime. If it has, we return the
appropriate error code.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---

diff --git a/selftest/flapping.d/ldap-pwd-change-race b/selftest/flapping.d/ldap-pwd-change-race
deleted file mode 100644
index 54ed56c1134..00000000000
--- a/selftest/flapping.d/ldap-pwd-change-race
+++ /dev/null
@@ -1,5 +0,0 @@
-# This test currently depends on a race. The password_hash dsdb module
-# relinquishes and immediately reacquires a transaction lock, and another
-# process may be able to acquire it during the short period of time in which it
-# is not held.
-^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_ldap_pw_change.ad_dc:local
diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c
index b9f7fb77e6f..b308226a9f9 100644
--- a/source4/dsdb/samdb/ldb_modules/password_hash.c
+++ b/source4/dsdb/samdb/ldb_modules/password_hash.c
@@ -45,6 +45,7 @@
 #include "lib/crypto/md4.h"
 #include "param/param.h"
 #include "lib/krb5_wrap/krb5_samba.h"
+#include "auth/auth_sam.h"
 #include "auth/common_auth.h"
 #include "lib/messaging/messaging.h"
 #include "lib/param/loadparm.h"
@@ -2729,7 +2730,8 @@ static int make_error_and_update_badPwdCount(struct setup_password_fields_io *io
 	struct ldb_context *ldb = ldb_module_get_ctx(io->ac->module);
 	struct ldb_message *mod_msg = NULL;
 	struct ldb_message *pso_msg = NULL;
-	NTSTATUS status;
+	struct ldb_message *current = NULL;
+	NTSTATUS status = NT_STATUS_OK;
 	int ret; /* The errors we will actually return */
 	int dbg_ret; /* The errors we can only complain about in logs */
 
@@ -2775,13 +2777,28 @@ static int make_error_and_update_badPwdCount(struct setup_password_fields_io *io
 		goto done;
 	}
 
+	/*
+	 * Re-read the account details, using the GUID in case the DN
+	 * is being changed.
+	 */
+	status = authsam_reread_user_logon_data(
+		ldb, io->ac,
+		io->ac->search_res->message,
+		&current);
+	if (!NT_STATUS_IS_OK(status)) {
+		/* The re-read can return account locked out, as well
+		 * as an internal error
+		 */
+		goto end_transaction;
+	}
+
 	/* PSO search result is optional (NULL if no PSO applies) */
 	if (io->ac->pso_res != NULL) {
 		pso_msg = io->ac->pso_res->message;
 	}
 
 	status = dsdb_update_bad_pwd_count(io->ac, ldb,
-					   io->ac->search_res->message,
+					   current,
 					   io->ac->dom_res->message,
 					   pso_msg,
 					   &mod_msg);
@@ -2831,7 +2848,11 @@ end_transaction:
 
 done:
 	ret = LDB_ERR_CONSTRAINT_VIOLATION;
-	*werror = WERR_INVALID_PASSWORD;
+	if (NT_STATUS_EQUAL(status, NT_STATUS_ACCOUNT_LOCKED_OUT)) {
+		*werror = WERR_ACCOUNT_LOCKED_OUT;
+	} else {
+		*werror = WERR_INVALID_PASSWORD;
+	}
 	ldb_asprintf_errstring(ldb,
 			       "%08X: %s - check_password_restrictions: "
 			       "The old password specified doesn't match!",