CVE-2021-20251 s3: ensure bad password count atomic updates

The bad password count is supposed to limit the number of failed login
attempt a user can make before being temporarily locked out, but race
conditions between processes have allowed determined attackers to make
many more than the specified number of attempts.  This is especially
bad on constrained or overcommitted hardware.

To fix this, once a bad password is detected, we reload the sam account
information under a user-specific mutex, ensuring we have an up to
date bad password count.

Discovered by Nathaniel W. Turner.

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

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/source3/auth/check_samsec.c b/source3/auth/check_samsec.c
index 25620aef20a9eacd3ea4b8cf36de0c717d143547..8c64456c582d7e2be3bd54d581ee5eaf5de74cdc 100644 (file)
--- a/source3/auth/check_samsec.c
+++ b/source3/auth/check_samsec.c
@@ -385,6 +385,8 @@ NTSTATUS check_sam_security(const DATA_BLOB *challenge,
        const uint8_t *nt_pw;
        const uint8_t *lm_pw;
        uint32_t acct_ctrl;
+       char *mutex_name_by_user = NULL;
+       struct named_mutex *mtx = NULL;
 
        /* the returned struct gets kept on the server_info, by means
           of a steal further down */
@@ -424,6 +426,79 @@ NTSTATUS check_sam_security(const DATA_BLOB *challenge,
                                    challenge, lm_pw, nt_pw,
                                    user_info, &user_sess_key, &lm_sess_key);
 
+       /*
+        * We must re-load the sam acount information under a mutex
+        * lock to ensure we don't miss any concurrent account lockout
+        * changes.
+        */
+
+       /* Clear out old sampass info. */
+       TALLOC_FREE(sampass);
+       acct_ctrl = 0;
+       username = NULL;
+       nt_pw = NULL;
+       lm_pw = NULL;
+
+       sampass = samu_new(mem_ctx);
+       if (sampass == NULL) {
+               return NT_STATUS_NO_MEMORY;
+       }
+
+       mutex_name_by_user = talloc_asprintf(mem_ctx,
+                                            "check_sam_security_mutex_%s",
+                                            user_info->mapped.account_name);
+       if (mutex_name_by_user == NULL) {
+               nt_status = NT_STATUS_NO_MEMORY;
+               goto done;
+       }
+
+       /* Grab the named mutex under root with 30 second timeout. */
+       become_root();
+       mtx = grab_named_mutex(mem_ctx, mutex_name_by_user, 30);
+       if (mtx != NULL) {
+               /* Re-load the account information if we got the mutex. */
+               ret = pdb_getsampwnam(sampass, user_info->mapped.account_name);
+       }
+       unbecome_root();
+
+       /* Everything from here on until mtx is freed is done under the mutex.*/
+
+       if (mtx == NULL) {
+               DBG_ERR("Acquisition of mutex %s failed "
+                       "for user %s\n",
+                       mutex_name_by_user,
+                       user_info->mapped.account_name);
+               nt_status = NT_STATUS_INTERNAL_ERROR;
+               goto done;
+       }
+
+       if (!ret) {
+               /*
+                * Re-load of account failed. This could only happen if the
+                * user was deleted in the meantime.
+                */
+               DBG_NOTICE("reload of user '%s' in passdb failed.\n",
+                          user_info->mapped.account_name);
+               nt_status = NT_STATUS_NO_SUCH_USER;
+               goto done;
+       }
+
+       /* Re-load the account control info. */
+       acct_ctrl = pdb_get_acct_ctrl(sampass);
+       username = pdb_get_username(sampass);
+
+       /*
+        * Check if the account is now locked out - now under the mutex.
+        * This can happen if the server is under
+        * a password guess attack and the ACB_AUTOLOCK is set by
+        * another process.
+        */
+       if (acct_ctrl & ACB_AUTOLOCK) {
+               DBG_NOTICE("Account for user %s was locked out.\n", username);
+               nt_status = NT_STATUS_ACCOUNT_LOCKED_OUT;
+               goto done;
+       }
+
        /* Notify passdb backend of login success/failure. If not
           NT_STATUS_OK the backend doesn't like the login */
 
@@ -516,6 +591,8 @@ done:
        TALLOC_FREE(sampass);
        data_blob_free(&user_sess_key);
        data_blob_free(&lm_sess_key);
+       TALLOC_FREE(mutex_name_by_user);
+       TALLOC_FREE(mtx);
        return nt_status;
 }