From f4e8e4aa6a226a6889248eff0e14d1cf4ceb263b Mon Sep 17 00:00:00 2001 From: Michael Adam 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 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 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 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