The Samba-Bugzilla – Attachment 5145 Details for
Bug 4347
check password history before incrasing badPasswordCount
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
second patchset containing the actual fixes
bad-password-count-master.patchset (text/plain), 8.06 KB, created by
Michael Adam
on 2010-01-07 05:11:42 UTC
(
hide
)
Description:
second patchset containing the actual fixes
Filename:
MIME Type:
Creator:
Michael Adam
Created:
2010-01-07 05:11:42 UTC
Size:
8.06 KB
patch
obsolete
>From f4e8e4aa6a226a6889248eff0e14d1cf4ceb263b Mon Sep 17 00:00:00 2001 >From: Michael Adam <obnox@samba.org> >Date: Tue, 5 Jan 2010 16:58:30 +0100 >Subject: [PATCH 1/4] s3:smbd:password_in_history: treat entry with 0 salt as 0 + plain nt hash > >This is to introduce a new format of the password history, maintaining backwards >compatibility: The old format was 16 byte hash + 16 byte md5(salt + nt hash). >The new format is 16 zero bytes and 16 bytes nt hash. > >This will allow us to respect the last X entries of the nt password history >when deciding whether to increment the bad password count. > >This is part of the fix for bug #4347 . > >Michael >--- > source3/smbd/chgpasswd.c | 30 ++++++++++++++++++++++++------ > 1 files changed, 24 insertions(+), 6 deletions(-) > >diff --git a/source3/smbd/chgpasswd.c b/source3/smbd/chgpasswd.c >index c858c2d..dcefc82 100644 >--- a/source3/smbd/chgpasswd.c >+++ b/source3/smbd/chgpasswd.c >@@ -1031,13 +1031,31 @@ bool password_in_history(uint8_t nt_pw[NT_HASH_LEN], > /* Ignore zero valued entries. */ > continue; > } >- /* Create salted versions of new to compare. */ >- E_md5hash(current_salt, nt_pw, new_nt_pw_salted_md5_hash); > >- if (memcmp(new_nt_pw_salted_md5_hash, >- old_nt_pw_salted_md5_hash, >- SALTED_MD5_HASH_LEN) == 0) { >- return true; >+ if (memcmp(zero_md5_nt_pw, current_salt, >+ PW_HISTORY_SALT_LEN) == 0) >+ { >+ /* >+ * New format: zero salt and then plain nt hash. >+ * Directly compare the hashes. >+ */ >+ if (memcmp(nt_pw, old_nt_pw_salted_md5_hash, >+ SALTED_MD5_HASH_LEN) == 0) >+ { >+ return true; >+ } >+ } else { >+ /* >+ * Old format: md5sum of salted nt hash. >+ * Create salted version of new pw to compare. >+ */ >+ E_md5hash(current_salt, nt_pw, new_nt_pw_salted_md5_hash); >+ >+ if (memcmp(new_nt_pw_salted_md5_hash, >+ old_nt_pw_salted_md5_hash, >+ SALTED_MD5_HASH_LEN) == 0) { >+ return true; >+ } > } > } > return false; >-- >1.6.3.3 > > >From 0178978acccfbe0de3cf49a084e04f627052e7fe Mon Sep 17 00:00:00 2001 >From: Michael Adam <obnox@samba.org> >Date: Tue, 5 Jan 2010 18:28:48 +0100 >Subject: [PATCH 2/4] s3:passdb: store the plain nt passwords hashes in history, not salted md5 > >This is in order to be able to do challenge response with the history, >so that this can be checked when an invalid password was entered: >If the given password is wrong but in the history, then the bad password >count should not be updated... > >The "lucky" bit here is that the md5 has and the nt hash (md4) both are >16 bytes long. > >This is part of the fix for bug #4347 . > >Michael >--- > source3/passdb/pdb_get_set.c | 15 ++++++++++----- > 1 files changed, 10 insertions(+), 5 deletions(-) > >diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c >index 005cf46..7fc9f92 100644 >--- a/source3/passdb/pdb_get_set.c >+++ b/source3/passdb/pdb_get_set.c >@@ -1070,15 +1070,20 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) > } > > /* >- * Create the new salt as the first part of the history entry. >+ * Fill the salt area with 0-s: this indicates that >+ * a plain nt hash is stored in the has area. >+ * The old format was to store a 16 byte salt and >+ * then an md5hash of the nt_hash concatenated with >+ * the salt. > */ >- generate_random_buffer(pwhistory, PW_HISTORY_SALT_LEN); >+ memset(pwhistory, 0, PW_HISTORY_SALT_LEN); > > /* >- * Generate the md5 hash of the salt+new password as the >- * second part of the history entry. >+ * Store the plain nt hash in the second 16 bytes. >+ * The old format was to store the md5 hash of >+ * the salt+newpw. > */ >- E_md5hash(pwhistory, new_nt_p16, &pwhistory[PW_HISTORY_SALT_LEN]); >+ memcpy(&pwhistory[PW_HISTORY_SALT_LEN], new_nt_p16, SALTED_MD5_HASH_LEN); > > pdb_set_pw_history(sampass, pwhistory, pwHistLen, PDB_CHANGED); > >-- >1.6.3.3 > > >From 694fb611a4934929d3738fa49672cde94e04b0eb Mon Sep 17 00:00:00 2001 >From: Michael Adam <obnox@samba.org> >Date: Wed, 6 Jan 2010 16:35:44 +0100 >Subject: [PATCH 3/4] s3:auth:check_sam_security: introduce a bool var to control pad_pw_count incrementation > >This is a preparatory patch for the last part in fixing bug #4347 . > >Michael >--- > source3/auth/auth_sam.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > >diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c >index e7b9f2b..4c3f552 100644 >--- a/source3/auth/auth_sam.c >+++ b/source3/auth/auth_sam.c >@@ -354,10 +354,16 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, > update_login_attempts_status = pdb_update_login_attempts(sampass, NT_STATUS_IS_OK(nt_status)); > > if (!NT_STATUS_IS_OK(nt_status)) { >+ bool increment_bad_pw_count = false; >+ > if (NT_STATUS_EQUAL(nt_status,NT_STATUS_WRONG_PASSWORD) && > acct_ctrl & 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 { >-- >1.6.3.3 > > >From bb905aa38f752e29eaec1f8b0a6442e6849187ac Mon Sep 17 00:00:00 2001 >From: Michael Adam <obnox@samba.org> >Date: Wed, 6 Jan 2010 17:29:04 +0100 >Subject: [PATCH 4/4] s3:auth: don't update the bad pw count if pw is among last 2 history entries > >This conforms to the behaviour of Windows 2003: >http://www.microsoft.com/technet/prodtechnol/windowsserver2003/technologies/security/bpactlck.mspx > >This is supposed to fixes Bug #4347 . > >Michael >--- > source3/auth/auth_sam.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 73 insertions(+), 1 deletions(-) > >diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c >index 4c3f552..ef0cd97 100644 >--- a/source3/auth/auth_sam.c >+++ b/source3/auth/auth_sam.c >@@ -281,6 +281,75 @@ static NTSTATUS sam_account_ok(TALLOC_CTX *mem_ctx, > return NT_STATUS_OK; > } > >+/** >+ * Check whether the given password is one of the last two >+ * password history entries. If so, the bad pwcount should >+ * not be incremented even thought the actual password check >+ * failed. >+ */ >+static bool need_to_increment_bad_pw_count( >+ const struct auth_context *auth_context, >+ struct samu* sampass, >+ const auth_usersupplied_info *user_info) >+{ >+ uint8_t i; >+ const uint8_t *pwhistory; >+ uint32_t pwhistory_len; >+ uint32_t policy_pwhistory_len; >+ uint32_t acct_ctrl; >+ const char *username; >+ TALLOC_CTX *mem_ctx = talloc_stackframe(); >+ bool result = true; >+ >+ pdb_get_account_policy(PDB_POLICY_PASSWORD_HISTORY, >+ &policy_pwhistory_len); >+ if (policy_pwhistory_len == 0) { >+ goto done; >+ } >+ >+ pwhistory = pdb_get_pw_history(sampass, &pwhistory_len); >+ if (!pwhistory || pwhistory_len == 0) { >+ goto done; >+ } >+ >+ acct_ctrl = pdb_get_acct_ctrl(sampass); >+ username = pdb_get_username(sampass); >+ >+ for (i=1; i < MIN(MIN(3, policy_pwhistory_len), pwhistory_len); i++) { >+ static const uint8_t zero16[SALTED_MD5_HASH_LEN]; >+ const uint8_t *salt; >+ const uint8_t *nt_pw; >+ NTSTATUS status; >+ DATA_BLOB user_sess_key = data_blob_null; >+ DATA_BLOB lm_sess_key = data_blob_null; >+ >+ salt = &pwhistory[i*PW_HISTORY_ENTRY_LEN]; >+ nt_pw = salt + PW_HISTORY_SALT_LEN; >+ >+ if (memcmp(zero16, nt_pw, NT_HASH_LEN) == 0) { >+ /* skip zero password hash */ >+ continue; >+ } >+ >+ if (memcmp(zero16, salt, PW_HISTORY_SALT_LEN) != 0) { >+ /* skip nonzero salt (old format entry) */ >+ continue; >+ } >+ >+ status = sam_password_ok(auth_context, mem_ctx, >+ username, acct_ctrl, NULL, nt_pw, >+ user_info, &user_sess_key, &lm_sess_key); >+ if (NT_STATUS_IS_OK(status)) { >+ result = false; >+ break; >+ } >+ } >+ >+done: >+ TALLOC_FREE(mem_ctx); >+ return result; >+} >+ > /**************************************************************************** > check if a username/password is OK assuming the password is a 24 byte > SMB hash supplied in the user_info structure >@@ -360,7 +429,10 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, > acct_ctrl & ACB_NORMAL && > NT_STATUS_IS_OK(update_login_attempts_status)) > { >- increment_bad_pw_count = true; >+ increment_bad_pw_count = >+ need_to_increment_bad_pw_count(auth_context, >+ sampass, >+ user_info); > } > > if (increment_bad_pw_count) { >-- >1.6.3.3 >
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:
bjacke
:
review+
Actions:
View
Attachments on
bug 4347
:
5144
| 5145 |
5169
|
5179