This is the passdb section of the previously mentioned commit.

Of particular note is the change to pdb_free_sam() to take its sam argument by
reference, allowing it to be NULLified by the SAFE_FREE() macro, and the
changed to local_password_change() both to make it work and to remove the
duplicate code that caused so much breakage over the last few days.

 - Small change in behaviour:  when LOCAL_ADD_USER is set, the user doesn't
actually exist locally but does exist in the passdb we don't attempt to do a
GetPwnam().  (How the entry got there is another matter, and most passdbs won't
allow this anyway).

Andrew Bartlett

2 files changed, 111 insertions, 65 deletions

diff --git a/source/passdb/passdb.c b/source/passdb/passdb.c
index b3efebe5ff0..75f2d432f2a 100644
--- a/source/passdb/passdb.c
+++ b/source/passdb/passdb.c
@@ -160,7 +160,7 @@ static BOOL pdb_free_sam_contents(SAM_ACCOUNT *user)
 	if (user == NULL) {
 		DEBUG(0,("pdb_free_sam_contents: SAM_ACCOUNT was NULL\n"));
 #if 0
-		smb_panic("NULL pointer passed to pdb_free_sam\n");
+		smb_panic("NULL pointer passed to pdb_free_sam_contents\n");
 #endif
 		return False;
 	}
@@ -184,6 +184,9 @@ BOOL pdb_reset_sam(SAM_ACCOUNT *user)
 {
 	if (user == NULL) {
 		DEBUG(0,("pdb_reset_sam: SAM_ACCOUNT was NULL\n"));
+#if 0
+		smb_panic("NULL pointer passed to pdb_free_sam\n");
+#endif
 		return False;
 	}
 	
@@ -203,9 +206,9 @@ BOOL pdb_reset_sam(SAM_ACCOUNT *user)
  Free the SAM_ACCOUNT and the NT/LM hashes.
  ***********************************************************/
 
-BOOL pdb_free_sam(SAM_ACCOUNT *user)
+BOOL pdb_free_sam(SAM_ACCOUNT **user)
 {
-	if (user == NULL) {
+	if (*user == NULL) {
 		DEBUG(0,("pdb_free_sam: SAM_ACCOUNT was NULL\n"));
 #if 0
 		smb_panic("NULL pointer passed to pdb_free_sam\n");
@@ -213,11 +216,11 @@ BOOL pdb_free_sam(SAM_ACCOUNT *user)
 		return False;
 	}
 
-	if (!pdb_free_sam_contents(user)) {
+	if (!pdb_free_sam_contents(*user)) {
 		return False;
 	}
 
-	SAFE_FREE(user);
+	SAFE_FREE(*user);
 	
 	return True;	
 }
@@ -835,7 +838,8 @@ void copy_sam_passwd(SAM_ACCOUNT *to, const SAM_ACCOUNT *from)
 
  FIXME!!  The function needs to be abstracted into the
  passdb interface or something.  It is currently being called
- by _api_samr_create_user() in rpc_server/srv_samr.c
+ by _api_samr_create_user() in rpc_server/srv_samr.c,
+ in SWAT and by smbpasswd/pdbedit.
  
  --jerry
  *************************************************************/
@@ -851,55 +855,36 @@ BOOL local_password_change(char *user_name, int local_flags,
 	*err_str = '\0';
 	*msg_str = '\0';
 
-	if (local_flags & LOCAL_ADD_USER) {
-	
-		/*
-		 * Check for a local account - if we're adding only.
-		 */
-	
-		if(!(pwd = sys_getpwnam(user_name))) {
-			slprintf(err_str, err_str_len - 1, "User %s does not \
-exist in system password file (usually /etc/passwd). Cannot add \
-account without a valid local system user.\n", user_name);
-			return False;
-		}
-	}
-
 	/* Get the smb passwd entry for this user */
 	pdb_init_sam(&sam_pass);
 	if(!pdb_getsampwnam(sam_pass, user_name)) {
-		pdb_free_sam(sam_pass);
+		pdb_free_sam(&sam_pass);
 		
-		if(!(local_flags & LOCAL_ADD_USER)) {
+		if (local_flags & LOCAL_ADD_USER) {
+			/*
+			 * Check for a local account - if we're adding only.
+			 */
+			
+			if(!(pwd = sys_getpwnam(user_name))) {
+				slprintf(err_str, err_str_len - 1, "User %s does not \
+exist in system password file (usually /etc/passwd). Cannot add \
+account without a valid local system user.\n", user_name);
+				return False;
+			}
+		} else {
 			slprintf(err_str, err_str_len-1,"Failed to find entry for user %s.\n", user_name);
 			return False;
 		}
 
 		if (!pdb_init_sam_pw(&sam_pass, pwd)) {
+			slprintf(err_str, err_str_len-1, "Failed initialise SAM_ACCOUNT for user %s.\n", user_name);
 			return False;
 		}
 
 		/* set account flags */
-		pdb_set_acct_ctrl(sam_pass,((local_flags & LOCAL_TRUST_ACCOUNT) ? ACB_WSTRUST : ACB_NORMAL) );
-
-		if (local_flags & LOCAL_DISABLE_USER)
-			pdb_set_acct_ctrl (sam_pass, pdb_get_acct_ctrl(sam_pass)|ACB_DISABLED);
-
-		if (local_flags & LOCAL_SET_NO_PASSWORD) {
-			pdb_set_acct_ctrl (sam_pass, pdb_get_acct_ctrl(sam_pass)|ACB_PWNOTREQ);
-		} else {
-			/* set the passwords here.  if we get to here it means
-			   we have a valid, active account */
-			pdb_set_plaintext_passwd (sam_pass, new_passwd);
-		}
-
-		if (pdb_add_sam_account(sam_pass)) {
-			slprintf(msg_str, msg_str_len-1, "Added user %s.\n", user_name);
-			pdb_free_sam(sam_pass);
-			return True;
-		} else {
-			slprintf(err_str, err_str_len-1, "Failed to add entry for user %s.\n", user_name);
-			pdb_free_sam(sam_pass);
+		if (!pdb_set_acct_ctrl(sam_pass,((local_flags & LOCAL_TRUST_ACCOUNT) ? ACB_WSTRUST : ACB_NORMAL) )) {
+			slprintf(err_str, err_str_len-1, "Failed to set 'trust account' flags for user %s.\n", user_name);
+			pdb_free_sam(&sam_pass);
 			return False;
 		}
 	} else {
@@ -912,19 +897,45 @@ account without a valid local system user.\n", user_name);
 	 * and the valid last change time.
 	 */
 
-	if(local_flags & LOCAL_DISABLE_USER) {
-		pdb_set_acct_ctrl (sam_pass, pdb_get_acct_ctrl(sam_pass)|ACB_DISABLED);
+	if (local_flags & LOCAL_DISABLE_USER) {
+		if (!pdb_set_acct_ctrl (sam_pass, pdb_get_acct_ctrl(sam_pass)|ACB_DISABLED)) {
+			slprintf(err_str, err_str_len-1, "Failed to set 'disabled' flag for user %s.\n", user_name);
+			pdb_free_sam(&sam_pass);
+			return False;
+		}
 	} else if (local_flags & LOCAL_ENABLE_USER) {
-		if(pdb_get_lanman_passwd(sam_pass) == NULL) {
-			pdb_set_plaintext_passwd (sam_pass, new_passwd);
+		if (pdb_get_lanman_passwd(sam_pass) == NULL) {
+			if (!pdb_set_plaintext_passwd (sam_pass, new_passwd)) {
+				slprintf(err_str, err_str_len-1, "Failed to set password for user %s.\n", user_name);
+				pdb_free_sam(&sam_pass);
+				return False;
+			}
+		}
+		if (!pdb_set_acct_ctrl (sam_pass, pdb_get_acct_ctrl(sam_pass)&(~ACB_DISABLED))) {
+			slprintf(err_str, err_str_len-1, "Failed to unset 'disabled' flag for user %s.\n", user_name);
+			pdb_free_sam(&sam_pass);
+			return False;
+		}
+	}
+	
+	if (local_flags & LOCAL_SET_NO_PASSWORD) {
+		if (!pdb_set_acct_ctrl (sam_pass, pdb_get_acct_ctrl(sam_pass)|ACB_PWNOTREQ)) {
+			slprintf(err_str, err_str_len-1, "Failed to set 'no password required' flag for user %s.\n", user_name);
+			pdb_free_sam(&sam_pass);
+			return False;
 		}
-		pdb_set_acct_ctrl (sam_pass, pdb_get_acct_ctrl(sam_pass)&(~ACB_DISABLED));
-	} else if (local_flags & LOCAL_SET_NO_PASSWORD) {
-		pdb_set_acct_ctrl (sam_pass, pdb_get_acct_ctrl(sam_pass)|ACB_PWNOTREQ);
 		
 		/* This is needed to preserve ACB_PWNOTREQ in mod_smbfilepwd_entry */
-		pdb_set_lanman_passwd (sam_pass, NULL);
-		pdb_set_nt_passwd     (sam_pass, NULL);
+		if (!pdb_set_lanman_passwd (sam_pass, NULL)) {
+			slprintf(err_str, err_str_len-1, "Failed to set NULL lanman password for user %s.\n", user_name);
+			pdb_free_sam(&sam_pass);
+			return False;
+		}
+		if (!pdb_set_nt_passwd (sam_pass, NULL)) {
+			slprintf(err_str, err_str_len-1, "Failed to set NULL NT password for user %s.\n", user_name);
+			pdb_free_sam(&sam_pass);
+			return False;
+		}
 	} else {
 		/*
 		 * If we're dealing with setting a completely empty user account
@@ -935,23 +946,47 @@ account without a valid local system user.\n", user_name);
 		 * and the decision hasn't really been made to disable them (ie.
 		 * don't create them disabled). JRA.
 		 */
-		if ((pdb_get_lanman_passwd(sam_pass)==NULL) && (pdb_get_acct_ctrl(sam_pass)&ACB_DISABLED))
-			pdb_set_acct_ctrl (sam_pass, pdb_get_acct_ctrl(sam_pass)&(~ACB_DISABLED));
-		pdb_set_acct_ctrl (sam_pass, pdb_get_acct_ctrl(sam_pass)&(~ACB_PWNOTREQ));
-		pdb_set_plaintext_passwd (sam_pass, new_passwd);
-	}
-	
-	if(local_flags & LOCAL_DELETE_USER) {
+		if ((pdb_get_lanman_passwd(sam_pass)==NULL) && (pdb_get_acct_ctrl(sam_pass)&ACB_DISABLED)) {
+			if (!pdb_set_acct_ctrl (sam_pass, pdb_get_acct_ctrl(sam_pass)&(~ACB_DISABLED))) {
+				slprintf(err_str, err_str_len-1, "Failed to unset 'disabled' flag for user %s.\n", user_name);
+				pdb_free_sam(&sam_pass);
+				return False;
+			}
+		}
+		if (!pdb_set_acct_ctrl (sam_pass, pdb_get_acct_ctrl(sam_pass)&(~ACB_PWNOTREQ))) {
+			slprintf(err_str, err_str_len-1, "Failed to unset 'no password required' flag for user %s.\n", user_name);
+			pdb_free_sam(&sam_pass);
+			return False;
+		}
+		
+		if (!pdb_set_plaintext_passwd (sam_pass, new_passwd)) {
+			slprintf(err_str, err_str_len-1, "Failed to set password for user %s.\n", user_name);
+			pdb_free_sam(&sam_pass);
+			return False;
+		}
+	}	
+
+	if (local_flags & LOCAL_ADD_USER) {
+		if (pdb_add_sam_account(sam_pass)) {
+			slprintf(msg_str, msg_str_len-1, "Added user %s.\n", user_name);
+			pdb_free_sam(&sam_pass);
+			return True;
+		} else {
+			slprintf(err_str, err_str_len-1, "Failed to add entry for user %s.\n", user_name);
+			pdb_free_sam(&sam_pass);
+			return False;
+		}
+	} else if (local_flags & LOCAL_DELETE_USER) {
 		if (!pdb_delete_sam_account(user_name)) {
 			slprintf(err_str,err_str_len-1, "Failed to delete entry for user %s.\n", user_name);
-			pdb_free_sam(sam_pass);
+			pdb_free_sam(&sam_pass);
 			return False;
 		}
 		slprintf(msg_str, msg_str_len-1, "Deleted user %s.\n", user_name);
 	} else {
 		if(!pdb_update_sam_account(sam_pass, True)) {
 			slprintf(err_str, err_str_len-1, "Failed to modify entry for user %s.\n", user_name);
-			pdb_free_sam(sam_pass);
+			pdb_free_sam(&sam_pass);
 			return False;
 		}
 		if(local_flags & LOCAL_DISABLE_USER)
@@ -962,7 +997,7 @@ account without a valid local system user.\n", user_name);
 			slprintf(msg_str, msg_str_len-1, "User %s password set to none.\n", user_name);
 	}
 
-	pdb_free_sam(sam_pass);
+	pdb_free_sam(&sam_pass);
 	return True;
 }
 
@@ -1528,9 +1563,15 @@ BOOL pdb_set_munged_dial (SAM_ACCOUNT *sampass, char *munged_dial)
 
 BOOL pdb_set_nt_passwd (SAM_ACCOUNT *sampass, uint8 *pwd)
 {
-	if (!sampass || !pwd)
+	if (!sampass)
 		return False;
 	
+	if (!pwd) {
+		/* Allow setting to NULL */
+		SAFE_FREE(sampass->nt_pw);
+		return True;
+	}
+
 	if (sampass->nt_pw!=NULL)
 		DEBUG(4,("pdb_set_nt_passwd: NT hash non NULL overwritting ?\n"));
 	else
@@ -1550,9 +1591,15 @@ BOOL pdb_set_nt_passwd (SAM_ACCOUNT *sampass, uint8 *pwd)
 
 BOOL pdb_set_lanman_passwd (SAM_ACCOUNT *sampass, uint8 *pwd)
 {
-	if (!sampass || !pwd)
+	if (!sampass)
 		return False;
 	
+	if (!pwd) {
+		/* Allow setting to NULL */
+		SAFE_FREE(sampass->lm_pw);
+		return True;
+	}
+
 	if (sampass->lm_pw!=NULL)
 		DEBUG(4,("pdb_set_lanman_passwd: LM hash non NULL overwritting ?\n"));
 	else
diff --git a/source/passdb/pdb_tdb.c b/source/passdb/pdb_tdb.c
index 990d0077b23..9b932b7821c 100644
--- a/source/passdb/pdb_tdb.c
+++ b/source/passdb/pdb_tdb.c
@@ -689,7 +689,7 @@ BOOL pdb_delete_sam_account(char *sname)
 
 	rid = pdb_get_user_rid(sam_pass);
 
-	pdb_free_sam (sam_pass);
+	pdb_free_sam (&sam_pass);
 	
 	/* it's outaa here!  8^) */
 	if (tdb_delete(pwd_tdb, key) != TDB_SUCCESS) {
@@ -732,7 +732,6 @@ static BOOL tdb_update_sam(SAM_ACCOUNT* newpwd, BOOL override, int flag)
 	pstring		tdbfile;
 	fstring		name;
 	BOOL		ret = True;
-	int		newtdb = FALSE;
 	
 	get_private_directory(tdbfile);
 	pstrcat (tdbfile, PASSDB_FILE_NAME);