[Samba] BUG: Bad passwords from Vampire / NT migration

Jeremy Allison jra at samba.org
Wed Oct 22 19:51:53 GMT 2008


On Wed, Oct 22, 2008 at 12:34:48PM -0700, Jeremy Allison wrote:
> On Wed, Oct 22, 2008 at 12:15:00PM -0700, Jeremy Allison wrote:
> 
> > Great catch. Both look valid to me. I think the best fix for
> > 3.2 is to always set rid_crypt to true, and remove all the
> > other sam_pwd_hash() calls - just do it in the one place.
> > 
> > Ok, here is a quick patch for 3.2. It removes some silly
> > static buffers and changes all calls to samsync_fix_delta_array()
> > to set rid_crypt = true and then removes all the extra
> > crypto sam_pwd_hash() calls that are no longer needed.
> > 
> > Can you confirm it works for you and I'll check it in
> > with your credit, and then fix 3.3 and master in the
> > same way.
> 
> Sorry, missed the ntpasswd <--> lmpasswd swap.
> 
> Here's the correct version for 3.2.

Ok, third time lucky :-). As we're always using
rid_crypt = true, remove the argument. Also cope
with detecting the zero'ed nt and lm passwords
at the correct place (before the sam_pwd_hash()
call).

3.2 patch (final :-).

Jeremy.
-------------- next part --------------
diff --git a/source/libnet/libnet_samsync.c b/source/libnet/libnet_samsync.c
index 61d53c3..8a14338 100644
--- a/source/libnet/libnet_samsync.c
+++ b/source/libnet/libnet_samsync.c
@@ -32,7 +32,6 @@
 
 static NTSTATUS fix_user(TALLOC_CTX *mem_ctx,
 			 DATA_BLOB *session_key,
-			 bool rid_crypt,
 			 enum netr_SamDatabaseID database_id,
 			 struct netr_DELTA_ENUM *delta)
 {
@@ -41,17 +40,29 @@ static NTSTATUS fix_user(TALLOC_CTX *mem_ctx,
 	struct netr_DELTA_USER *user = delta->delta_union.user;
 	struct samr_Password lm_hash;
 	struct samr_Password nt_hash;
+	unsigned char zero_buf[16];
 
-	if (rid_crypt) {
-		if (user->lm_password_present) {
+	memset(zero_buf, '\0', sizeof(zero_buf));
+
+	/* Note that win2000 may send us all zeros
+	 * for the hashes if it doesn't
+	 * think this channel is secure enough. */
+	if (user->lm_password_present) {
+		if (memcmp(user->lmpassword.hash, zero_buf, 16) != 0) {
 			sam_pwd_hash(rid, user->lmpassword.hash, lm_hash.hash, 0);
-			user->lmpassword = lm_hash;
+		} else {
+			memset(lm_hash.hash, '\0', sizeof(lm_hash.hash));
 		}
+		user->lmpassword = lm_hash;
+	}
 
-		if (user->nt_password_present) {
+	if (user->nt_password_present) {
+		if (memcmp(user->ntpassword.hash, zero_buf, 16) != 0) {
 			sam_pwd_hash(rid, user->ntpassword.hash, nt_hash.hash, 0);
-			user->ntpassword = nt_hash;
+		} else {
+			memset(nt_hash.hash, '\0', sizeof(nt_hash.hash));
 		}
+		user->ntpassword = nt_hash;
 	}
 
 	if (user->user_private_info.SensitiveData) {
@@ -71,26 +82,31 @@ static NTSTATUS fix_user(TALLOC_CTX *mem_ctx,
 			return ndr_map_error2ntstatus(ndr_err);
 		}
 
+		/* Note that win2000 may send us all zeros
+		 * for the hashes if it doesn't
+		 * think this channel is secure enough. */
 		if (keys.keys.keys2.lmpassword.length == 16) {
-			if (rid_crypt) {
+			if (memcmp(keys.keys.keys2.lmpassword.pwd.hash,
+						zero_buf, 16) != 0) {
 				sam_pwd_hash(rid,
 					     keys.keys.keys2.lmpassword.pwd.hash,
 					     lm_hash.hash, 0);
-				user->lmpassword = lm_hash;
 			} else {
-				user->lmpassword = keys.keys.keys2.lmpassword.pwd;
+				memset(lm_hash.hash, '\0', sizeof(lm_hash.hash));
 			}
+			user->lmpassword = lm_hash;
 			user->lm_password_present = true;
 		}
 		if (keys.keys.keys2.ntpassword.length == 16) {
-			if (rid_crypt) {
+			if (memcmp(keys.keys.keys2.ntpassword.pwd.hash,
+						zero_buf, 16) != 0) {
 				sam_pwd_hash(rid,
-					     keys.keys.keys2.ntpassword.pwd.hash,
-					     nt_hash.hash, 0);
-				user->ntpassword = nt_hash;
+					keys.keys.keys2.ntpassword.pwd.hash,
+					nt_hash.hash, 0);
 			} else {
-				user->ntpassword = keys.keys.keys2.ntpassword.pwd;
+				memset(nt_hash.hash, '\0', sizeof(nt_hash.hash));
 			}
+			user->ntpassword = nt_hash;
 			user->nt_password_present = true;
 		}
 		/* TODO: rid decrypt history fields */
@@ -128,7 +144,6 @@ static NTSTATUS fix_secret(TALLOC_CTX *mem_ctx,
 
 static NTSTATUS samsync_fix_delta(TALLOC_CTX *mem_ctx,
 				  DATA_BLOB *session_key,
-				  bool rid_crypt,
 				  enum netr_SamDatabaseID database_id,
 				  struct netr_DELTA_ENUM *delta)
 {
@@ -139,7 +154,6 @@ static NTSTATUS samsync_fix_delta(TALLOC_CTX *mem_ctx,
 
 			status = fix_user(mem_ctx,
 					  session_key,
-					  rid_crypt,
 					  database_id,
 					  delta);
 			break;
@@ -164,7 +178,6 @@ static NTSTATUS samsync_fix_delta(TALLOC_CTX *mem_ctx,
 
 NTSTATUS samsync_fix_delta_array(TALLOC_CTX *mem_ctx,
 				 DATA_BLOB *session_key,
-				 bool rid_crypt,
 				 enum netr_SamDatabaseID database_id,
 				 struct netr_DELTA_ENUM_ARRAY *r)
 {
@@ -175,7 +188,6 @@ NTSTATUS samsync_fix_delta_array(TALLOC_CTX *mem_ctx,
 
 		status = samsync_fix_delta(mem_ctx,
 					   session_key,
-					   rid_crypt,
 					   database_id,
 					   &r->delta_enum[i]);
 		if (!NT_STATUS_IS_OK(status)) {
diff --git a/source/utils/net_rpc_samsync.c b/source/utils/net_rpc_samsync.c
index 13a7bce..49e5c1a 100644
--- a/source/utils/net_rpc_samsync.c
+++ b/source/utils/net_rpc_samsync.c
@@ -65,21 +65,19 @@ static void display_account_info(uint32_t rid,
 				 struct netr_DELTA_USER *r)
 {
 	fstring hex_nt_passwd, hex_lm_passwd;
-	uchar lm_passwd[16], nt_passwd[16];
-	static uchar zero_buf[16];
+	uchar zero_buf[16];
 
+	memset(zero_buf, '\0', sizeof(zero_buf));
 	/* Decode hashes from password hash (if they are not NULL) */
 
 	if (memcmp(r->lmpassword.hash, zero_buf, 16) != 0) {
-		sam_pwd_hash(r->rid, r->lmpassword.hash, lm_passwd, 0);
-		pdb_sethexpwd(hex_lm_passwd, lm_passwd, r->acct_flags);
+		pdb_sethexpwd(hex_lm_passwd, r->lmpassword.hash, r->acct_flags);
 	} else {
 		pdb_sethexpwd(hex_lm_passwd, NULL, 0);
 	}
 
 	if (memcmp(r->ntpassword.hash, zero_buf, 16) != 0) {
-		sam_pwd_hash(r->rid, r->ntpassword.hash, nt_passwd, 0);
-		pdb_sethexpwd(hex_nt_passwd, nt_passwd, r->acct_flags);
+		pdb_sethexpwd(hex_nt_passwd, r->ntpassword.hash, r->acct_flags);
 	} else {
 		pdb_sethexpwd(hex_nt_passwd, NULL, 0);
 	}
@@ -391,7 +389,6 @@ static void dump_database(struct rpc_pipe_client *pipe_hnd,
 
 		samsync_fix_delta_array(mem_ctx,
 					&session_key,
-					false,
 					database_id,
 					delta_enum_array);
 
@@ -466,8 +463,9 @@ static NTSTATUS sam_account_from_delta(struct samu *account,
 {
 	const char *old_string, *new_string;
 	time_t unix_time, stored_time;
-	uchar lm_passwd[16], nt_passwd[16];
-	static uchar zero_buf[16];
+	uchar zero_buf[16];
+
+	memset(zero_buf, '\0', sizeof(zero_buf));
 
 	/* Username, fullname, home dir, dir drive, logon script, acct
 	   desc, workstations, profile. */
@@ -631,14 +629,12 @@ static NTSTATUS sam_account_from_delta(struct samu *account,
 	   think this channel is secure enough - don't set the passwords at all
 	   in that case
 	*/
-	if (memcmp(r->ntpassword.hash, zero_buf, 16) != 0) {
-		sam_pwd_hash(r->rid, r->ntpassword.hash, lm_passwd, 0);
-		pdb_set_lanman_passwd(account, lm_passwd, PDB_CHANGED);
+	if (memcmp(r->lmpassword.hash, zero_buf, 16) != 0) {
+		pdb_set_lanman_passwd(account, r->lmpassword.hash, PDB_CHANGED);
 	}
 
-	if (memcmp(r->lmpassword.hash, zero_buf, 16) != 0) {
-		sam_pwd_hash(r->rid, r->lmpassword.hash, nt_passwd, 0);
-		pdb_set_nt_passwd(account, nt_passwd, PDB_CHANGED);
+	if (memcmp(r->ntpassword.hash, zero_buf, 16) != 0) {
+		pdb_set_nt_passwd(account, r->ntpassword.hash, PDB_CHANGED);
 	}
 
 	/* TODO: account expiry time */
@@ -1257,7 +1253,6 @@ static NTSTATUS fetch_database(struct rpc_pipe_client *pipe_hnd, uint32 db_type,
 
 		samsync_fix_delta_array(mem_ctx,
 					&session_key,
-					true,
 					database_id,
 					delta_enum_array);
 
@@ -1755,15 +1750,16 @@ static NTSTATUS fetch_account_info_to_ldif(struct netr_DELTA_USER *r,
 	fstring username, logonscript, homedrive, homepath = "", homedir = "";
 	fstring hex_nt_passwd, hex_lm_passwd;
 	fstring description, profilepath, fullname, sambaSID;
-	uchar lm_passwd[16], nt_passwd[16];
 	char *flags, *user_rdn;
 	const char *ou;
 	const char* nopasswd = "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX";
-	static uchar zero_buf[16];
+	uchar zero_buf[16];
 	uint32 rid = 0, group_rid = 0, gidNumber = 0;
 	time_t unix_time;
 	int i;
 
+	memset(zero_buf, '\0', sizeof(zero_buf));
+
 	/* Get the username */
 	fstrcpy(username, r->account_name.string);
 
@@ -1808,14 +1804,12 @@ static NTSTATUS fetch_account_info_to_ldif(struct netr_DELTA_USER *r,
 
 	/* Get lm and nt password data */
 	if (memcmp(r->lmpassword.hash, zero_buf, 16) != 0) {
-		sam_pwd_hash(r->rid, r->lmpassword.hash, lm_passwd, 0);
-		pdb_sethexpwd(hex_lm_passwd, lm_passwd, r->acct_flags);
+		pdb_sethexpwd(hex_lm_passwd, r->lmpassword.hash, r->acct_flags);
 	} else {
 		pdb_sethexpwd(hex_lm_passwd, NULL, 0);
 	}
 	if (memcmp(r->ntpassword.hash, zero_buf, 16) != 0) {
-		sam_pwd_hash(r->rid, r->ntpassword.hash, nt_passwd, 0);
-		pdb_sethexpwd(hex_nt_passwd, nt_passwd, r->acct_flags);
+		pdb_sethexpwd(hex_nt_passwd, r->ntpassword.hash, r->acct_flags);
 	} else {
 		pdb_sethexpwd(hex_nt_passwd, NULL, 0);
 	}
@@ -2173,7 +2167,6 @@ static NTSTATUS fetch_database_to_ldif(struct rpc_pipe_client *pipe_hnd,
 
 		samsync_fix_delta_array(mem_ctx,
 					&session_key,
-					true,
 					database_id,
 					delta_enum_array);
 


More information about the samba mailing list