[SCM] Samba Shared Repository - branch v3-2-test updated - release-3-2-0pre2-3106-ge38436f

Jeremy Allison jra at samba.org
Wed Oct 22 20:20:52 GMT 2008


The branch, v3-2-test has been updated
       via  e38436f731ff09333588cc0751c79029a569f390 (commit)
      from  79222e476edbccf81e70cf1c0d1f40db0b88e20b (commit)

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-2-test


- Log -----------------------------------------------------------------
commit e38436f731ff09333588cc0751c79029a569f390
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Oct 22 13:18:58 2008 -0700

    Fix net rpc vampire, based on an *amazing* piece of debugging work by "Cooper S. Blake" <the_analogkid at yahoo.com>.
    
    "I believe I have found two bugs in the 3.2 code and one bug that
    carried on to the 3.3 branch.  In the 3.2 code, everything is
    located in the utils/net_rpc_samsync.c file.  What I believe is the
    first problem is that fetch_database() is calling
    samsync_fix_delta_array() with rid_crypt set to true, which means
    the password hashes are unencrypted from the RID encryption.
    However, I believe this call is redundant, and the corresponding
    call for samdump has rid_crypt set to false.  So I think the
    rid_crypt param should be false in fetch_database().
    
    If you follow the code, it makes its way to sam_account_from_delta()
    where the password hashes are decrypted a second time by calling
    sam_pwd_hash().  I believe this is what is scrambling my passwords.
    
    These methods were refactored somewhere in the 3.3 branch.  Now the
    net_rpc_samsync.c class calls rpc_vampire_internals, which calls
    libnet/libnet_samsync.c, which calls samsync_fix_delta_array() with
    rid_crypt always set to false.  I think that's correct.  But the
    second bug has carried through in the sam_account_from_delta()
    function:
    
     208         if (memcmp(r->ntpassword.hash, zero_buf, 16) != 0) {
     209                 sam_pwd_hash(r->rid, r->ntpassword.hash, lm_passwd, 0);
     210                 pdb_set_lanman_passwd(account, lm_passwd, PDB_CHANGED);
     211         }
     212
     213         if (memcmp(r->lmpassword.hash, zero_buf, 16) != 0) {
     214                 sam_pwd_hash(r->rid, r->lmpassword.hash, nt_passwd, 0);
     215                 pdb_set_nt_passwd(account, nt_passwd, PDB_CHANGED);
    
    If you look closely you'll see that the nt hash is going into the
    lm_passwd variable and the decrypted value is being set in the lanman
    hash, and the lanman hash is being decrypted and put into the nt hash
    field.  So the LanMan and NT hashes look like they're being put in
    the opposite fields."
    
    Fix this by removing the rid_crypt parameter.
    Jeremy.

-----------------------------------------------------------------------

Summary of changes:
 source/libnet/libnet_samsync.c |   48 +++++++++++++++++++++++++---------------
 source/utils/net_rpc_samsync.c |   39 +++++++++++++-------------------
 2 files changed, 46 insertions(+), 41 deletions(-)


Changeset truncated at 500 lines:

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);
 


-- 
Samba Shared Repository


More information about the samba-cvs mailing list