summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJoseph Sutton <josephsutton@catalyst.net.nz>2022-08-02 14:43:19 +1200
committerJule Anger <janger@samba.org>2022-09-19 05:03:03 +0000
commitbb86d2f3a10edbe27aa2edeafce0475b9cd79feb (patch)
tree58ae679e8b3d08c36723e16c7e1bb7ded8a94af5
parent9aabf78216f91aee6abcd401b10f1ca01f544be0 (diff)
downloadsamba-bb86d2f3a10edbe27aa2edeafce0475b9cd79feb.tar.gz
CVE-2021-20251 s3: Ensure bad password count atomic updates for SAMR AES password change
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. We also update the bad password count if the password is wrong, which we did not previously do. Derived from a similar patch to source3/auth/check_samsec.c by Jeremy Allison <jra@samba.org> 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> Autobuild-User(master): Andrew Bartlett <abartlet@samba.org> Autobuild-Date(master): Tue Sep 13 00:08:07 UTC 2022 on sn-devel-184 (cherry picked from commit 8ae0c38d54f065915e927bbfe1b656400a79eb13) Autobuild-User(v4-17-test): Jule Anger <janger@samba.org> Autobuild-Date(v4-17-test): Mon Sep 19 05:03:03 UTC 2022 on sn-devel-184
-rw-r--r--source3/rpc_server/samr/srv_samr_nt.c117
1 files changed, 117 insertions, 0 deletions
diff --git a/source3/rpc_server/samr/srv_samr_nt.c b/source3/rpc_server/samr/srv_samr_nt.c
index 1036410f534..5f93d4287ad 100644
--- a/source3/rpc_server/samr/srv_samr_nt.c
+++ b/source3/rpc_server/samr/srv_samr_nt.c
@@ -7697,6 +7697,10 @@ NTSTATUS _samr_ChangePasswordUser4(struct pipes_struct *p,
.length = sizeof(cdk_data),
};
char *new_passwd = NULL;
+ bool updated_badpw = false;
+ NTSTATUS update_login_attempts_status;
+ char *mutex_name_by_user = NULL;
+ struct named_mutex *mtx = NULL;
NTSTATUS status = NT_STATUS_WRONG_PASSWORD;
bool ok;
int rc;
@@ -7770,6 +7774,119 @@ NTSTATUS _samr_ChangePasswordUser4(struct pipes_struct *p,
r->in.password,
&new_passwd);
BURN_DATA(cdk_data);
+
+ /*
+ * 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);
+
+ sampass = samu_new(frame);
+ if (sampass == NULL) {
+ status = NT_STATUS_NO_MEMORY;
+ goto done;
+ }
+
+ mutex_name_by_user = talloc_asprintf(frame,
+ "check_sam_security_mutex_%s",
+ username);
+ if (mutex_name_by_user == NULL) {
+ status = NT_STATUS_NO_MEMORY;
+ goto done;
+ }
+
+ /* Grab the named mutex under root with 30 second timeout. */
+ become_root();
+ mtx = grab_named_mutex(frame, mutex_name_by_user, 30);
+ if (mtx != NULL) {
+ /* Re-load the account information if we got the mutex. */
+ ok = pdb_getsampwnam(sampass, username);
+ }
+ 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,
+ username);
+ status = NT_STATUS_INTERNAL_ERROR;
+ goto done;
+ }
+
+ if (!ok) {
+ /*
+ * 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",
+ username);
+ status = NT_STATUS_NO_SUCH_USER;
+ goto done;
+ }
+
+ /*
+ * 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 (pdb_get_acct_ctrl(sampass) & ACB_AUTOLOCK) {
+ DBG_NOTICE("Account for user %s was locked out.\n", username);
+ 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
+ */
+ update_login_attempts_status = pdb_update_login_attempts(
+ sampass, NT_STATUS_IS_OK(status));
+
+ if (!NT_STATUS_IS_OK(status)) {
+ bool increment_bad_pw_count = false;
+
+ if (NT_STATUS_EQUAL(status, NT_STATUS_WRONG_PASSWORD) &&
+ (pdb_get_acct_ctrl(sampass) & ACB_NORMAL) &&
+ NT_STATUS_IS_OK(update_login_attempts_status))
+ {
+ increment_bad_pw_count = true;
+ }
+
+ if (increment_bad_pw_count) {
+ pdb_increment_bad_password_count(sampass);
+ updated_badpw = true;
+ } else {
+ pdb_update_bad_password_count(sampass,
+ &updated_badpw);
+ }
+ } else {
+ if ((pdb_get_acct_ctrl(sampass) & ACB_NORMAL) &&
+ (pdb_get_bad_password_count(sampass) > 0))
+ {
+ pdb_set_bad_password_count(sampass, 0, PDB_CHANGED);
+ pdb_set_bad_password_time(sampass, 0, PDB_CHANGED);
+ updated_badpw = true;
+ }
+ }
+
+ if (updated_badpw) {
+ NTSTATUS update_status;
+ become_root();
+ update_status = pdb_update_sam_account(sampass);
+ unbecome_root();
+
+ if (!NT_STATUS_IS_OK(update_status)) {
+ DEBUG(1, ("Failed to modify entry: %s\n",
+ nt_errstr(update_status)));
+ }
+ }
+
if (!NT_STATUS_IS_OK(status)) {
goto done;
}