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 (file)
index 54ed56c..0000000
--- 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 b9f7fb77e6f151f6ab3e6ab305907339a79be286..b308226a9f9a42247d8641de21d2e070d5160017 100644 (file)
--- 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!",