From 147a2c057cfcf459391c67e05ce2d0be561f0382 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 14 Dec 2009 17:43:56 +0100 Subject: [PATCH 01/22] s3: Avoid a memset(, 0, ) call --- source3/smbd/chgpasswd.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/source3/smbd/chgpasswd.c b/source3/smbd/chgpasswd.c index 2da36b2..5c46f44 100644 --- a/source3/smbd/chgpasswd.c +++ b/source3/smbd/chgpasswd.c @@ -1017,7 +1017,7 @@ static NTSTATUS check_oem_password(const char *user, static bool check_passwd_history(struct samu *sampass, const char *plaintext) { uchar new_nt_p16[NT_HASH_LEN]; - uchar zero_md5_nt_pw[SALTED_MD5_HASH_LEN]; + static const uint8_t zero_md5_nt_pw[SALTED_MD5_HASH_LEN] = { 0, }; const uint8 *nt_pw; const uint8 *pwhistory; bool found = False; @@ -1051,7 +1051,6 @@ static bool check_passwd_history(struct samu *sampass, const char *plaintext) dump_data(100, new_nt_p16, NT_HASH_LEN); dump_data(100, pwhistory, PW_HISTORY_ENTRY_LEN*pwHisLen); - memset(zero_md5_nt_pw, '\0', SALTED_MD5_HASH_LEN); for (i=0; i Date: Mon, 14 Dec 2009 17:51:39 +0100 Subject: [PATCH 02/22] s3: Fix a typo --- source3/smbd/chgpasswd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/source3/smbd/chgpasswd.c b/source3/smbd/chgpasswd.c index 5c46f44..70ce75c 100644 --- a/source3/smbd/chgpasswd.c +++ b/source3/smbd/chgpasswd.c @@ -1155,7 +1155,7 @@ NTSTATUS change_oem_password(struct samu *hnd, char *old_passwd, char *new_passw } } - /* removed calculation here, becuase passdb now calculates + /* removed calculation here, because passdb now calculates based on policy. jmcd */ if ((can_change_time != 0) && (time(NULL) < can_change_time)) { DEBUG(1, ("user %s cannot change password now, must " -- 1.6.3.3 From ca6c1cdd5faa1c2ff067b53f2ad6b811fe9d79bb Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 14 Dec 2009 18:13:28 +0100 Subject: [PATCH 03/22] s3: Simplify pdb_set_plaintext_passwd() slightly No functional change, this just removes an indentation level by the early "return True;" in + if ((pdb_get_acct_ctrl(sampass) & ACB_NORMAL) == 0) { + /* + * No password history for non-user accounts + */ + return true; + } Volker --- source3/passdb/pdb_get_set.c | 139 +++++++++++++++++++++++++----------------- 1 files changed, 83 insertions(+), 56 deletions(-) diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c index da65440..4c24076 100644 --- a/source3/passdb/pdb_get_set.c +++ b/source3/passdb/pdb_get_set.c @@ -981,6 +981,8 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) { uchar new_lanman_p16[LM_HASH_LEN]; uchar new_nt_p16[NT_HASH_LEN]; + uchar *pwhistory; + uint32 pwHistLen; if (!plaintext) return False; @@ -1010,68 +1012,93 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) if (!pdb_set_pass_last_set_time (sampass, time(NULL), PDB_CHANGED)) return False; - /* Store the password history. */ - if (pdb_get_acct_ctrl(sampass) & ACB_NORMAL) { - uchar *pwhistory; - uint32 pwHistLen; - pdb_get_account_policy(PDB_POLICY_PASSWORD_HISTORY, &pwHistLen); - if (pwHistLen != 0){ - uint32 current_history_len; - /* We need to make sure we don't have a race condition here - the - account policy history length can change between when the pw_history - was first loaded into the struct samu struct and now.... JRA. */ - pwhistory = (uchar *)pdb_get_pw_history(sampass, ¤t_history_len); - - if (current_history_len != pwHistLen) { - /* After closing and reopening struct samu the history - values will sync up. We can't do this here. */ - - /* current_history_len > pwHistLen is not a problem - we - have more history than we need. */ - - if (current_history_len < pwHistLen) { - /* Ensure we have space for the needed history. */ - uchar *new_history = (uchar *)TALLOC(sampass, - pwHistLen*PW_HISTORY_ENTRY_LEN); - if (!new_history) { - return False; - } - - /* And copy it into the new buffer. */ - if (current_history_len) { - memcpy(new_history, pwhistory, - current_history_len*PW_HISTORY_ENTRY_LEN); - } - /* Clearing out any extra space. */ - memset(&new_history[current_history_len*PW_HISTORY_ENTRY_LEN], - '\0', (pwHistLen-current_history_len)*PW_HISTORY_ENTRY_LEN); - /* Finally replace it. */ - pwhistory = new_history; - } - } - if (pwhistory && pwHistLen){ - /* Make room for the new password in the history list. */ - if (pwHistLen > 1) { - memmove(&pwhistory[PW_HISTORY_ENTRY_LEN], - pwhistory, (pwHistLen -1)*PW_HISTORY_ENTRY_LEN ); - } - /* Create the new salt as the first part of the history entry. */ - generate_random_buffer(pwhistory, PW_HISTORY_SALT_LEN); + if ((pdb_get_acct_ctrl(sampass) & ACB_NORMAL) == 0) { + /* + * No password history for non-user accounts + */ + return true; + } - /* Generate the md5 hash of the salt+new password as the second - part of the history entry. */ + pdb_get_account_policy(PDB_POLICY_PASSWORD_HISTORY, &pwHistLen); + if (pwHistLen != 0){ + uint32 current_history_len; + /* + * We need to make sure we don't have a race condition + * here - the account policy history length can change + * between when the pw_history was first loaded into + * the struct samu struct and now.... JRA. + */ + pwhistory = (uchar *)pdb_get_pw_history(sampass, + ¤t_history_len); + + if (current_history_len != pwHistLen) { + /* + * After closing and reopening struct samu the history + * values will sync up. We can't do this here. + */ + + /* + * current_history_len > pwHistLen is not a + * problem - we have more history than we + * need. + */ + + if (current_history_len < pwHistLen) { + /* + * Ensure we have space for the needed history. + */ + uchar *new_history = (uchar *)TALLOC( + sampass, + pwHistLen*PW_HISTORY_ENTRY_LEN); + if (!new_history) { + return False; + } - E_md5hash(pwhistory, new_nt_p16, &pwhistory[PW_HISTORY_SALT_LEN]); - pdb_set_pw_history(sampass, pwhistory, pwHistLen, PDB_CHANGED); - } else { - DEBUG (10,("pdb_get_set.c: pdb_set_plaintext_passwd: pwhistory was NULL!\n")); + /* And copy it into the new buffer. */ + if (current_history_len) { + memcpy(new_history, pwhistory, + current_history_len*PW_HISTORY_ENTRY_LEN); + } + /* Clearing out any extra space. */ + memset(&new_history[current_history_len*PW_HISTORY_ENTRY_LEN], + '\0', (pwHistLen-current_history_len)*PW_HISTORY_ENTRY_LEN); + /* Finally replace it. */ + pwhistory = new_history; + } + } + if (pwhistory && pwHistLen){ + /* + * Make room for the new password in the + * history list. + */ + if (pwHistLen > 1) { + memmove(&pwhistory[PW_HISTORY_ENTRY_LEN], + pwhistory, (pwHistLen -1)*PW_HISTORY_ENTRY_LEN ); } + /* + * Create the new salt as the first part of + * the history entry. + */ + generate_random_buffer(pwhistory, PW_HISTORY_SALT_LEN); + + /* + * Generate the md5 hash of the salt+new + * password as the second part of the history + * entry. + */ + + E_md5hash(pwhistory, new_nt_p16, + &pwhistory[PW_HISTORY_SALT_LEN]); + pdb_set_pw_history(sampass, pwhistory, pwHistLen, + PDB_CHANGED); } else { - /* Set the history length to zero. */ - pdb_set_pw_history(sampass, NULL, 0, PDB_CHANGED); + DEBUG (10,("pdb_get_set.c: pdb_set_plaintext_passwd: " + "pwhistory was NULL!\n")); } + } else { + /* Set the history length to zero. */ + pdb_set_pw_history(sampass, NULL, 0, PDB_CHANGED); } - return True; } -- 1.6.3.3 From 7ba006430f427a1f28300d29a5c045a552d2382c Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 14 Dec 2009 18:24:04 +0100 Subject: [PATCH 04/22] s3: Simplify pdb_set_plaintext_passwd() a bit Remove an indentation by the early return in + if (pwHistLen == 0) { + /* Set the history length to zero. */ + pdb_set_pw_history(sampass, NULL, 0, PDB_CHANGED); + return true; + } --- source3/passdb/pdb_get_set.c | 129 ++++++++++++++++++++--------------------- 1 files changed, 63 insertions(+), 66 deletions(-) diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c index 4c24076..af99e81 100644 --- a/source3/passdb/pdb_get_set.c +++ b/source3/passdb/pdb_get_set.c @@ -983,6 +983,7 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) uchar new_nt_p16[NT_HASH_LEN]; uchar *pwhistory; uint32 pwHistLen; + uint32 current_history_len; if (!plaintext) return False; @@ -1020,84 +1021,80 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) } pdb_get_account_policy(PDB_POLICY_PASSWORD_HISTORY, &pwHistLen); - if (pwHistLen != 0){ - uint32 current_history_len; + + if (pwHistLen == 0) { + /* Set the history length to zero. */ + pdb_set_pw_history(sampass, NULL, 0, PDB_CHANGED); + return true; + } + + /* + * We need to make sure we don't have a race condition here - + * the account policy history length can change between when + * the pw_history was first loaded into the struct samu struct + * and now.... JRA. + */ + pwhistory = (uchar *)pdb_get_pw_history(sampass, ¤t_history_len); + + if (current_history_len != pwHistLen) { /* - * We need to make sure we don't have a race condition - * here - the account policy history length can change - * between when the pw_history was first loaded into - * the struct samu struct and now.... JRA. + * After closing and reopening struct samu the history + * values will sync up. We can't do this here. */ - pwhistory = (uchar *)pdb_get_pw_history(sampass, - ¤t_history_len); - if (current_history_len != pwHistLen) { - /* - * After closing and reopening struct samu the history - * values will sync up. We can't do this here. - */ + /* + * current_history_len > pwHistLen is not a problem - + * we have more history than we need. + */ + if (current_history_len < pwHistLen) { /* - * current_history_len > pwHistLen is not a - * problem - we have more history than we - * need. + * Ensure we have space for the needed history. */ + uchar *new_history = (uchar *)TALLOC( + sampass, pwHistLen*PW_HISTORY_ENTRY_LEN); + if (!new_history) { + return False; + } - if (current_history_len < pwHistLen) { - /* - * Ensure we have space for the needed history. - */ - uchar *new_history = (uchar *)TALLOC( - sampass, - pwHistLen*PW_HISTORY_ENTRY_LEN); - if (!new_history) { - return False; - } - - /* And copy it into the new buffer. */ - if (current_history_len) { - memcpy(new_history, pwhistory, - current_history_len*PW_HISTORY_ENTRY_LEN); - } - /* Clearing out any extra space. */ - memset(&new_history[current_history_len*PW_HISTORY_ENTRY_LEN], - '\0', (pwHistLen-current_history_len)*PW_HISTORY_ENTRY_LEN); - /* Finally replace it. */ - pwhistory = new_history; + /* And copy it into the new buffer. */ + if (current_history_len) { + memcpy(new_history, pwhistory, + current_history_len*PW_HISTORY_ENTRY_LEN); } + /* Clearing out any extra space. */ + memset(&new_history[current_history_len*PW_HISTORY_ENTRY_LEN], + '\0', (pwHistLen-current_history_len)*PW_HISTORY_ENTRY_LEN); + /* Finally replace it. */ + pwhistory = new_history; } - if (pwhistory && pwHistLen){ - /* - * Make room for the new password in the - * history list. - */ - if (pwHistLen > 1) { - memmove(&pwhistory[PW_HISTORY_ENTRY_LEN], - pwhistory, (pwHistLen -1)*PW_HISTORY_ENTRY_LEN ); - } - /* - * Create the new salt as the first part of - * the history entry. - */ - generate_random_buffer(pwhistory, PW_HISTORY_SALT_LEN); - - /* - * Generate the md5 hash of the salt+new - * password as the second part of the history - * entry. - */ + } - E_md5hash(pwhistory, new_nt_p16, - &pwhistory[PW_HISTORY_SALT_LEN]); - pdb_set_pw_history(sampass, pwhistory, pwHistLen, - PDB_CHANGED); - } else { - DEBUG (10,("pdb_get_set.c: pdb_set_plaintext_passwd: " - "pwhistory was NULL!\n")); + if (pwhistory && pwHistLen) { + /* + * Make room for the new password in the history list. + */ + if (pwHistLen > 1) { + memmove(&pwhistory[PW_HISTORY_ENTRY_LEN], pwhistory, + (pwHistLen-1)*PW_HISTORY_ENTRY_LEN ); } + /* + * Create the new salt as the first part of the + * history entry. + */ + generate_random_buffer(pwhistory, PW_HISTORY_SALT_LEN); + + /* + * Generate the md5 hash of the salt+new password as + * the second part of the history entry. + */ + + E_md5hash(pwhistory, new_nt_p16, + &pwhistory[PW_HISTORY_SALT_LEN]); + pdb_set_pw_history(sampass, pwhistory, pwHistLen, PDB_CHANGED); } else { - /* Set the history length to zero. */ - pdb_set_pw_history(sampass, NULL, 0, PDB_CHANGED); + DEBUG (10,("pdb_get_set.c: pdb_set_plaintext_passwd: " + "pwhistory was NULL!\n")); } return True; } -- 1.6.3.3 From e7290255f5ba1dd913bb3d40e71654cff1cfe4cf Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 14 Dec 2009 18:39:19 +0100 Subject: [PATCH 05/22] s3: Make use of talloc_array in pdb_set_plaintext_passwd() --- source3/passdb/pdb_get_set.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c index af99e81..bea4c32 100644 --- a/source3/passdb/pdb_get_set.c +++ b/source3/passdb/pdb_get_set.c @@ -1051,8 +1051,9 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) /* * Ensure we have space for the needed history. */ - uchar *new_history = (uchar *)TALLOC( - sampass, pwHistLen*PW_HISTORY_ENTRY_LEN); + uchar *new_history = talloc_array( + sampass, uchar, + pwHistLen*PW_HISTORY_ENTRY_LEN); if (!new_history) { return False; } -- 1.6.3.3 From 864ed92954315600ddcef69b21face95c06224a4 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 14 Dec 2009 18:43:03 +0100 Subject: [PATCH 06/22] s3: Simplify pdb_set_plaintext_passwd by using talloc_zero_array --- source3/passdb/pdb_get_set.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c index bea4c32..149dde0 100644 --- a/source3/passdb/pdb_get_set.c +++ b/source3/passdb/pdb_get_set.c @@ -1051,7 +1051,7 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) /* * Ensure we have space for the needed history. */ - uchar *new_history = talloc_array( + uchar *new_history = talloc_zero_array( sampass, uchar, pwHistLen*PW_HISTORY_ENTRY_LEN); if (!new_history) { @@ -1063,10 +1063,7 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) memcpy(new_history, pwhistory, current_history_len*PW_HISTORY_ENTRY_LEN); } - /* Clearing out any extra space. */ - memset(&new_history[current_history_len*PW_HISTORY_ENTRY_LEN], - '\0', (pwHistLen-current_history_len)*PW_HISTORY_ENTRY_LEN); - /* Finally replace it. */ + pwhistory = new_history; } } -- 1.6.3.3 From 7633837026d56ee723ffb603c9bd884ff6c69ef3 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 14 Dec 2009 18:44:38 +0100 Subject: [PATCH 07/22] s3: Simplify pdb_set_plaintext_passwd: memcpy deals fine with 0 bytes --- source3/passdb/pdb_get_set.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c index 149dde0..968da9d 100644 --- a/source3/passdb/pdb_get_set.c +++ b/source3/passdb/pdb_get_set.c @@ -1058,11 +1058,8 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) return False; } - /* And copy it into the new buffer. */ - if (current_history_len) { - memcpy(new_history, pwhistory, - current_history_len*PW_HISTORY_ENTRY_LEN); - } + memcpy(new_history, pwhistory, + current_history_len*PW_HISTORY_ENTRY_LEN); pwhistory = new_history; } -- 1.6.3.3 From a3f522202ddc09d444e800ad1da2078975de01c1 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 14 Dec 2009 18:50:38 +0100 Subject: [PATCH 08/22] s3: Simplify pdb_set_plaintext_passwd() by removing a redundant condition if (current_history_len != pwHistLen) { if (current_history_len < pwHistLen) { } } The second "if" is a bit pointless here --- source3/passdb/pdb_get_set.c | 33 +++++++++++---------------------- 1 files changed, 11 insertions(+), 22 deletions(-) diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c index 968da9d..f0c3fb1 100644 --- a/source3/passdb/pdb_get_set.c +++ b/source3/passdb/pdb_get_set.c @@ -1036,33 +1036,22 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) */ pwhistory = (uchar *)pdb_get_pw_history(sampass, ¤t_history_len); - if (current_history_len != pwHistLen) { + if (current_history_len < pwHistLen) { /* - * After closing and reopening struct samu the history - * values will sync up. We can't do this here. + * Ensure we have space for the needed history. */ + uchar *new_history = talloc_zero_array( + sampass, uchar, + pwHistLen*PW_HISTORY_ENTRY_LEN); - /* - * current_history_len > pwHistLen is not a problem - - * we have more history than we need. - */ - - if (current_history_len < pwHistLen) { - /* - * Ensure we have space for the needed history. - */ - uchar *new_history = talloc_zero_array( - sampass, uchar, - pwHistLen*PW_HISTORY_ENTRY_LEN); - if (!new_history) { - return False; - } + if (!new_history) { + return False; + } - memcpy(new_history, pwhistory, - current_history_len*PW_HISTORY_ENTRY_LEN); + memcpy(new_history, pwhistory, + current_history_len*PW_HISTORY_ENTRY_LEN); - pwhistory = new_history; - } + pwhistory = new_history; } if (pwhistory && pwHistLen) { -- 1.6.3.3 From ec0998ada5eebf5cae63719ef14097639ffef258 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 14 Dec 2009 19:12:50 +0100 Subject: [PATCH 09/22] s3: Add a paranoia check to pdb_set_plaintext_passwd() --- source3/passdb/pdb_get_set.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c index f0c3fb1..ba19b6b 100644 --- a/source3/passdb/pdb_get_set.c +++ b/source3/passdb/pdb_get_set.c @@ -1036,6 +1036,11 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) */ pwhistory = (uchar *)pdb_get_pw_history(sampass, ¤t_history_len); + if ((current_history_len != 0) && (pwhistory == NULL)) { + DEBUG(1, ("pdb_set_plaintext_passwd: pwhistory == NULL!\n")); + return false; + } + if (current_history_len < pwHistLen) { /* * Ensure we have space for the needed history. -- 1.6.3.3 From 2a11f3b3d7b4a009ddfa70511ad2ce3b84aa0539 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 14 Dec 2009 19:16:58 +0100 Subject: [PATCH 10/22] s3: Simplify pdb_set_plaintext_passwd: pwHistLen==0 was checked above --- source3/passdb/pdb_get_set.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c index ba19b6b..eed3591 100644 --- a/source3/passdb/pdb_get_set.c +++ b/source3/passdb/pdb_get_set.c @@ -1043,7 +1043,9 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) if (current_history_len < pwHistLen) { /* - * Ensure we have space for the needed history. + * Ensure we have space for the needed history. This + * also takes care of an account which did not have + * any history at all so far, i.e. pwhistory==NULL */ uchar *new_history = talloc_zero_array( sampass, uchar, @@ -1059,7 +1061,7 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) pwhistory = new_history; } - if (pwhistory && pwHistLen) { + if (pwhistory != NULL) { /* * Make room for the new password in the history list. */ -- 1.6.3.3 From 5e2fc28b639a3944c272bbc5664e3da209c81365 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 14 Dec 2009 19:18:09 +0100 Subject: [PATCH 11/22] s3: Simplify pdb_set_plaintext_passwd: pwhistory==NULL can not happen anymore --- source3/passdb/pdb_get_set.c | 43 ++++++++++++++++++----------------------- 1 files changed, 19 insertions(+), 24 deletions(-) diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c index eed3591..005cf46 100644 --- a/source3/passdb/pdb_get_set.c +++ b/source3/passdb/pdb_get_set.c @@ -1061,32 +1061,27 @@ bool pdb_set_plaintext_passwd(struct samu *sampass, const char *plaintext) pwhistory = new_history; } - if (pwhistory != NULL) { - /* - * Make room for the new password in the history list. - */ - if (pwHistLen > 1) { - memmove(&pwhistory[PW_HISTORY_ENTRY_LEN], pwhistory, - (pwHistLen-1)*PW_HISTORY_ENTRY_LEN ); - } - /* - * Create the new salt as the first part of the - * history entry. - */ - generate_random_buffer(pwhistory, PW_HISTORY_SALT_LEN); + /* + * Make room for the new password in the history list. + */ + if (pwHistLen > 1) { + memmove(&pwhistory[PW_HISTORY_ENTRY_LEN], pwhistory, + (pwHistLen-1)*PW_HISTORY_ENTRY_LEN ); + } - /* - * Generate the md5 hash of the salt+new password as - * the second part of the history entry. - */ + /* + * Create the new salt as the first part of the history entry. + */ + generate_random_buffer(pwhistory, PW_HISTORY_SALT_LEN); + + /* + * Generate the md5 hash of the salt+new password as the + * second part of the history entry. + */ + E_md5hash(pwhistory, new_nt_p16, &pwhistory[PW_HISTORY_SALT_LEN]); + + pdb_set_pw_history(sampass, pwhistory, pwHistLen, PDB_CHANGED); - E_md5hash(pwhistory, new_nt_p16, - &pwhistory[PW_HISTORY_SALT_LEN]); - pdb_set_pw_history(sampass, pwhistory, pwHistLen, PDB_CHANGED); - } else { - DEBUG (10,("pdb_get_set.c: pdb_set_plaintext_passwd: " - "pwhistory was NULL!\n")); - } return True; } -- 1.6.3.3 From be05d71b9e3fe3c73ada46f7bb7745bf19633716 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 14 Dec 2009 19:29:36 +0100 Subject: [PATCH 12/22] Simplify E_md5hash a bit --- libcli/auth/smbencrypt.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-) diff --git a/libcli/auth/smbencrypt.c b/libcli/auth/smbencrypt.c index a3182cd..f7c60e7 100644 --- a/libcli/auth/smbencrypt.c +++ b/libcli/auth/smbencrypt.c @@ -100,13 +100,9 @@ bool E_md4hash(const char *passwd, uint8_t p16[16]) void E_md5hash(const uint8_t salt[16], const uint8_t nthash[16], uint8_t hash_out[16]) { struct MD5Context tctx; - uint8_t array[32]; - - memset(hash_out, '\0', 16); - memcpy(array, salt, 16); - memcpy(&array[16], nthash, 16); MD5Init(&tctx); - MD5Update(&tctx, array, 32); + MD5Update(&tctx, salt, 16); + MD5Update(&tctx, nthash, 16); MD5Final(hash_out, &tctx); } -- 1.6.3.3 From 53a1ed9b6caa7b8ea2c5b4f1cae6faba19e09708 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 14 Dec 2009 20:54:33 +0100 Subject: [PATCH 13/22] s3: Factor password_in_history() out of check_passwd_history() --- source3/include/proto.h | 3 ++ source3/smbd/chgpasswd.c | 66 ++++++++++++++++++++++++++++----------------- 2 files changed, 44 insertions(+), 25 deletions(-) diff --git a/source3/include/proto.h b/source3/include/proto.h index 5b16120..c0ca96b 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -6116,6 +6116,9 @@ NTSTATUS pass_oem_change(char *user, uchar password_encrypted_with_nt_hash[516], const uchar old_nt_hash_encrypted[16], uint32 *reject_reason); +bool password_in_history(uint8_t nt_pw[NT_HASH_LEN], + uint32_t pw_history_len, + const uint8_t *pw_history); NTSTATUS change_oem_password(struct samu *hnd, char *old_passwd, char *new_passwd, bool as_root, uint32 *samr_reject_reason); /* The following definitions come from smbd/close.c */ diff --git a/source3/smbd/chgpasswd.c b/source3/smbd/chgpasswd.c index 70ce75c..c858c2d 100644 --- a/source3/smbd/chgpasswd.c +++ b/source3/smbd/chgpasswd.c @@ -1008,6 +1008,41 @@ static NTSTATUS check_oem_password(const char *user, return NT_STATUS_WRONG_PASSWORD; } +bool password_in_history(uint8_t nt_pw[NT_HASH_LEN], + uint32_t pw_history_len, + const uint8_t *pw_history) +{ + static const uint8_t zero_md5_nt_pw[SALTED_MD5_HASH_LEN] = { 0, }; + int i; + + dump_data(100, nt_pw, NT_HASH_LEN); + dump_data(100, pw_history, PW_HISTORY_ENTRY_LEN * pw_history_len); + + for (i=0; i Date: Wed, 30 Dec 2009 12:46:22 +0100 Subject: [PATCH 14/22] s3:check_sam_security: untangle assignment from statement Michael --- source3/auth/auth_sam.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index f0500b3..942f9ca 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -305,7 +305,8 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, /* the returned struct gets kept on the server_info, by means of a steal further down */ - if ( !(sampass = samu_new( mem_ctx )) ) { + sampass = samu_new(mem_ctx); + if (sampass == NULL) { return NT_STATUS_NO_MEMORY; } -- 1.6.3.3 From 7ac18c743b50b8cd63284326bd648675db63c557 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Wed, 30 Dec 2009 15:35:50 +0100 Subject: [PATCH 15/22] s3:auth:sam_password_ok: enhance readability (imho) by adding some pointers and removing bool variables and several checks. Michael --- source3/auth/auth_sam.c | 41 ++++++++++++++++++++++++----------------- 1 files changed, 24 insertions(+), 17 deletions(-) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index 942f9ca..381ad5b 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -40,9 +40,12 @@ static NTSTATUS sam_password_ok(const struct auth_context *auth_context, { uint32 acct_ctrl; const uint8 *lm_pw, *nt_pw; - struct samr_Password lm_hash, nt_hash, client_lm_hash, client_nt_hash; + struct samr_Password _lm_hash, _nt_hash, _client_lm_hash, _client_nt_hash; + struct samr_Password *lm_hash = NULL; + struct samr_Password *nt_hash = NULL; + struct samr_Password *client_lm_hash = NULL; + struct samr_Password *client_nt_hash = NULL; const char *username = pdb_get_username(sampass); - bool got_lm = false, got_nt = false; *user_sess_key = data_blob(NULL, 0); *lm_sess_key = data_blob(NULL, 0); @@ -60,32 +63,36 @@ static NTSTATUS sam_password_ok(const struct auth_context *auth_context, lm_pw = pdb_get_lanman_passwd(sampass); nt_pw = pdb_get_nt_passwd(sampass); + if (lm_pw) { - memcpy(lm_hash.hash, lm_pw, sizeof(lm_hash.hash)); + memcpy(_lm_hash.hash, lm_pw, sizeof(_lm_hash.hash)); + lm_hash = &_lm_hash; } if (nt_pw) { - memcpy(nt_hash.hash, nt_pw, sizeof(nt_hash.hash)); + memcpy(_nt_hash.hash, nt_pw, sizeof(_nt_hash.hash)); + nt_hash = &_nt_hash; } - if (user_info->lm_interactive_pwd.data && sizeof(client_lm_hash.hash) == user_info->lm_interactive_pwd.length) { - memcpy(client_lm_hash.hash, user_info->lm_interactive_pwd.data, sizeof(lm_hash.hash)); - got_lm = true; + if (user_info->lm_interactive_pwd.data && sizeof(_client_lm_hash.hash) == user_info->lm_interactive_pwd.length) { + memcpy(_client_lm_hash.hash, user_info->lm_interactive_pwd.data, sizeof(_lm_hash.hash)); + client_lm_hash = &_client_lm_hash; } - if (user_info->nt_interactive_pwd.data && sizeof(client_nt_hash.hash) == user_info->nt_interactive_pwd.length) { - memcpy(client_nt_hash.hash, user_info->nt_interactive_pwd.data, sizeof(nt_hash.hash)); - got_nt = true; + if (user_info->nt_interactive_pwd.data && sizeof(_client_nt_hash.hash) == user_info->nt_interactive_pwd.length) { + memcpy(_client_nt_hash.hash, user_info->nt_interactive_pwd.data, sizeof(_nt_hash.hash)); + client_nt_hash = &_client_nt_hash; } - if (got_lm || got_nt) { + + if (client_lm_hash || client_nt_hash) { *user_sess_key = data_blob(mem_ctx, 16); if (!user_sess_key->data) { return NT_STATUS_NO_MEMORY; } SMBsesskeygen_ntv1(nt_pw, user_sess_key->data); return hash_password_check(mem_ctx, lp_lanman_auth(), - got_lm ? &client_lm_hash : NULL, - got_nt ? &client_nt_hash : NULL, + client_lm_hash, + client_nt_hash, username, - lm_pw ? &lm_hash: NULL, - nt_pw ? &nt_hash : NULL); + lm_hash, + nt_hash); } else { return ntlm_password_check(mem_ctx, lp_lanman_auth(), lp_ntlm_auth(), @@ -95,8 +102,8 @@ static NTSTATUS sam_password_ok(const struct auth_context *auth_context, username, user_info->smb_name, user_info->client_domain, - lm_pw ? &lm_hash: NULL, - nt_pw ? &nt_hash : NULL, + lm_hash, + nt_hash, user_sess_key, lm_sess_key); } } -- 1.6.3.3 From 0172587d8d56e1163c27014e1e092580d0158e10 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Wed, 30 Dec 2009 15:37:23 +0100 Subject: [PATCH 16/22] s3:auth:sam_password_ok: fix allocation of a data blob. data_blob(mem_ctx, 16) does not use mem_ctx as a talloc ctx but copies 16 bytes from mem_ctx into the newly allocated data blob. This can not have been intentional. A blank uint8_t array of length 16 is allocated by passing NULL instead of mem_ctx. And using data_blob_talloc(mem_ctx, NULL, 16) adds the allocated blank 16 byte array to mem_ctx - so this is what must have been intended. Michael --- source3/auth/auth_sam.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index 381ad5b..42ede64 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -82,7 +82,7 @@ static NTSTATUS sam_password_ok(const struct auth_context *auth_context, } if (client_lm_hash || client_nt_hash) { - *user_sess_key = data_blob(mem_ctx, 16); + *user_sess_key = data_blob_talloc(mem_ctx, NULL, 16); if (!user_sess_key->data) { return NT_STATUS_NO_MEMORY; } -- 1.6.3.3 From c0f404a2e46187424915a073142a0a218b48ec2c Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Mon, 4 Jan 2010 15:37:24 +0100 Subject: [PATCH 17/22] s3:auth: use data_blob_null instead of data_blob(NULL, 0) in sam_password_ok() This way it is more explicit that there is no allocated data here that may leak. Michael --- source3/auth/auth_sam.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index 42ede64..a9f1600 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -47,8 +47,8 @@ static NTSTATUS sam_password_ok(const struct auth_context *auth_context, struct samr_Password *client_nt_hash = NULL; const char *username = pdb_get_username(sampass); - *user_sess_key = data_blob(NULL, 0); - *lm_sess_key = data_blob(NULL, 0); + *user_sess_key = data_blob_null; + *lm_sess_key = data_blob_null; acct_ctrl = pdb_get_acct_ctrl(sampass); if (acct_ctrl & ACB_PWNOTREQ) { -- 1.6.3.3 From 36348594505a5e7934d20d3b614f51023ae5740a Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Mon, 4 Jan 2010 18:15:24 +0100 Subject: [PATCH 18/22] s3:auth:sam_password_ok: take username, acct_ctrl and nt/lm hashes, not sampass This is in preparation to extending check_sam_security to also check against the password history before updating the bad password count. This way, sam_password_ok can more easily be reused for that purpose. Michael --- source3/auth/auth_sam.c | 34 ++++++++++++++++++++-------------- 1 files changed, 20 insertions(+), 14 deletions(-) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index a9f1600..add74f6 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -33,24 +33,23 @@ static NTSTATUS sam_password_ok(const struct auth_context *auth_context, TALLOC_CTX *mem_ctx, - struct samu *sampass, + const char *username, + uint32_t acct_ctrl, + const uint8_t *lm_pw, + const uint8_t *nt_pw, const auth_usersupplied_info *user_info, DATA_BLOB *user_sess_key, DATA_BLOB *lm_sess_key) { - uint32 acct_ctrl; - const uint8 *lm_pw, *nt_pw; struct samr_Password _lm_hash, _nt_hash, _client_lm_hash, _client_nt_hash; struct samr_Password *lm_hash = NULL; struct samr_Password *nt_hash = NULL; struct samr_Password *client_lm_hash = NULL; struct samr_Password *client_nt_hash = NULL; - const char *username = pdb_get_username(sampass); *user_sess_key = data_blob_null; *lm_sess_key = data_blob_null; - acct_ctrl = pdb_get_acct_ctrl(sampass); if (acct_ctrl & ACB_PWNOTREQ) { if (lp_null_passwords()) { DEBUG(3,("Account for user '%s' has no password and null passwords are allowed.\n", username)); @@ -61,9 +60,6 @@ static NTSTATUS sam_password_ok(const struct auth_context *auth_context, } } - lm_pw = pdb_get_lanman_passwd(sampass); - nt_pw = pdb_get_nt_passwd(sampass); - if (lm_pw) { memcpy(_lm_hash.hash, lm_pw, sizeof(_lm_hash.hash)); lm_hash = &_lm_hash; @@ -304,6 +300,10 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, DATA_BLOB user_sess_key = data_blob_null; DATA_BLOB lm_sess_key = data_blob_null; bool updated_autolock = False, updated_badpw = False; + uint32_t acct_ctrl; + const char *username; + const uint8_t *nt_pw; + const uint8_t *lm_pw; if (!user_info || !auth_context) { return NT_STATUS_UNSUCCESSFUL; @@ -330,16 +330,22 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, return NT_STATUS_NO_SUCH_USER; } + acct_ctrl = pdb_get_acct_ctrl(sampass); + username = pdb_get_username(sampass); + nt_pw = pdb_get_nt_passwd(sampass); + lm_pw = pdb_get_lanman_passwd(sampass); + /* see if autolock flag needs to be updated */ - if (pdb_get_acct_ctrl(sampass) & ACB_NORMAL) + if (acct_ctrl & ACB_NORMAL) pdb_update_autolock_flag(sampass, &updated_autolock); /* Quit if the account was locked out. */ - if (pdb_get_acct_ctrl(sampass) & ACB_AUTOLOCK) { - DEBUG(3,("check_sam_security: Account for user %s was locked out.\n", pdb_get_username(sampass))); + if (acct_ctrl & ACB_AUTOLOCK) { + DEBUG(3,("check_sam_security: Account for user %s was locked out.\n", username)); return NT_STATUS_ACCOUNT_LOCKED_OUT; } - nt_status = sam_password_ok(auth_context, mem_ctx, sampass, + nt_status = sam_password_ok(auth_context, mem_ctx, + username, acct_ctrl, lm_pw, nt_pw, user_info, &user_sess_key, &lm_sess_key); /* Notify passdb backend of login success/failure. If not @@ -349,7 +355,7 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, if (!NT_STATUS_IS_OK(nt_status)) { if (NT_STATUS_EQUAL(nt_status,NT_STATUS_WRONG_PASSWORD) && - pdb_get_acct_ctrl(sampass) &ACB_NORMAL && + acct_ctrl & ACB_NORMAL && NT_STATUS_IS_OK(update_login_attempts_status)) { pdb_increment_bad_password_count(sampass); @@ -370,7 +376,7 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, return nt_status; } - if ((pdb_get_acct_ctrl(sampass) & ACB_NORMAL) && + if ((acct_ctrl & 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); -- 1.6.3.3 From de4fb80beec59999dd9ce074d4fff0b310fb08da Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Wed, 6 Jan 2010 12:32:51 +0100 Subject: [PATCH 19/22] s3:auth:check_sam_security: null out sampass after it has been stolen. So that a later talloc_free would not harm. I could have used talloc_move instead of talloc steal in make_server_info_sam(), but this would have required a change of the signature. Michael --- source3/auth/auth_sam.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index add74f6..3573de1 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -402,6 +402,7 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, become_root(); nt_status = make_server_info_sam(server_info, sampass); unbecome_root(); + sampass = NULL; if (!NT_STATUS_IS_OK(nt_status)) { DEBUG(0,("check_sam_security: make_server_info_sam() failed with '%s'\n", nt_errstr(nt_status))); -- 1.6.3.3 From 970317c413eae52af9976e5652362412dd3038e3 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Wed, 6 Jan 2010 12:36:56 +0100 Subject: [PATCH 20/22] s3:auth:check_sam_security: create (and use) a common exit point for use after sam_password_ok() has been called. Michael --- source3/auth/auth_sam.c | 18 +++++++----------- 1 files changed, 7 insertions(+), 11 deletions(-) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index 3573de1..dd4a465 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -370,10 +370,7 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, DEBUG(1, ("Failed to modify entry.\n")); unbecome_root(); } - data_blob_free(&user_sess_key); - data_blob_free(&lm_sess_key); - TALLOC_FREE(sampass); - return nt_status; + goto done; } if ((acct_ctrl & ACB_NORMAL) && @@ -393,10 +390,7 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, nt_status = sam_account_ok(mem_ctx, sampass, user_info); if (!NT_STATUS_IS_OK(nt_status)) { - TALLOC_FREE(sampass); - data_blob_free(&user_sess_key); - data_blob_free(&lm_sess_key); - return nt_status; + goto done; } become_root(); @@ -406,9 +400,7 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, if (!NT_STATUS_IS_OK(nt_status)) { DEBUG(0,("check_sam_security: make_server_info_sam() failed with '%s'\n", nt_errstr(nt_status))); - data_blob_free(&user_sess_key); - data_blob_free(&lm_sess_key); - return nt_status; + goto done; } (*server_info)->user_session_key = @@ -423,6 +415,10 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, (*server_info)->nss_token |= user_info->was_mapped; +done: + TALLOC_FREE(sampass); + data_blob_free(&user_sess_key); + data_blob_free(&lm_sess_key); return nt_status; } -- 1.6.3.3 From 5ad1b7e0c5aa7c8e0a0d55c2456e9d6354dc9bcc Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Wed, 6 Jan 2010 13:40:58 +0100 Subject: [PATCH 21/22] s3:auth:check_sam_security: fix a leading tab/ws mixup Michael --- source3/auth/auth_sam.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index dd4a465..7835b18 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -385,7 +385,7 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, if(!NT_STATUS_IS_OK(pdb_update_sam_account(sampass))) DEBUG(1, ("Failed to modify entry.\n")); unbecome_root(); - } + } nt_status = sam_account_ok(mem_ctx, sampass, user_info); -- 1.6.3.3 From 7248873b48ac28c40809c949da0e7325ca63aef0 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Wed, 6 Jan 2010 13:53:10 +0100 Subject: [PATCH 22/22] s3:auth:check_sam_security: improve calling and logging of pdb_update_sam_account Log what went wrongl, and also call pdb_update_sam_account inside become_root/unbecome_root: do the logging outside. Michael --- source3/auth/auth_sam.c | 20 ++++++++++++++++---- 1 files changed, 16 insertions(+), 4 deletions(-) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index 7835b18..e7b9f2b 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -365,10 +365,16 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, &updated_badpw); } if (updated_autolock || updated_badpw){ + NTSTATUS status; + become_root(); - if(!NT_STATUS_IS_OK(pdb_update_sam_account(sampass))) - DEBUG(1, ("Failed to modify entry.\n")); + status = pdb_update_sam_account(sampass); unbecome_root(); + + if (!NT_STATUS_IS_OK(status)) { + DEBUG(1, ("Failed to modify entry: %s\n", + nt_errstr(status))); + } } goto done; } @@ -381,10 +387,16 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context, } if (updated_autolock || updated_badpw){ + NTSTATUS status; + become_root(); - if(!NT_STATUS_IS_OK(pdb_update_sam_account(sampass))) - DEBUG(1, ("Failed to modify entry.\n")); + status = pdb_update_sam_account(sampass); unbecome_root(); + + if (!NT_STATUS_IS_OK(status)) { + DEBUG(1, ("Failed to modify entry: %s\n", + nt_errstr(status))); + } } nt_status = sam_account_ok(mem_ctx, sampass, user_info); -- 1.6.3.3