Ignore the primaryGruoupSID attribute in struct samu*

Gerald (Jerry) Carter jerry at samba.org
Wed Feb 22 02:51:24 GMT 2006


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Volker,

Here's a patch (tested on standalone, dc, and domain members)
for ignoring the primaryGroupSID attribute in a passdb.
Instead pdb_get_group_sid() builds the group SID from
the Unix primary gid.  It's wrong to try to keep the
primary group SID in sync with the real Unix primary group.
The fall back to setting the primary group RID to be 513
is still there which seems to be ok.

If this patch is agreeable, then we are basically down
to the security descriptor scenario.  I've got some
thoughts on that as well and will send them in a followup
mail.

The patch is also on-line at http://www.samba.org/~/jerry/patches/





cheers, jerry
=====================================================================
I live in a Reply-to-All world                -----------------------
Samba                                    ------- http://www.samba.org
Centeris                         -----------  http://www.centeris.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFD+9GrIR7qMdg1EfYRAv3iAKDHPLEPt74J3NgyahSIpmSwQgsizwCfYnFG
ZGt9bWho8/aQyrhIJgrnT4g=
=M/of
-----END PGP SIGNATURE-----
-------------- next part --------------
Index: source/auth/auth_util.c
===================================================================
--- source/auth/auth_util.c	(revision 13592)
+++ source/auth/auth_util.c	(working copy)
@@ -1103,7 +1103,7 @@
 		return NT_STATUS_NO_MEMORY;
 	}
 		
-	status = samu_set_unix( sampass, pwd );
+	status = samu_set_unix( sampass, pwd, False );
 	if ( !NT_STATUS_IS_OK(status) ) {		
 		return status;
 	}
@@ -1163,7 +1163,7 @@
 		return NT_STATUS_NO_MEMORY;
 	}
 	
-	status = samu_set_unix( sampass, pwd );
+	status = samu_set_unix( sampass, pwd, False );
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
@@ -1273,14 +1273,18 @@
 					    sizeof(gid_t)*dst->n_groups);
 	else
 		dst->groups = NULL;
+		
 	dst->ptok = dup_nt_token(dst, src->ptok);
-	dst->user_session_key = data_blob_talloc(
-		dst, src->user_session_key.data,
+	
+	dst->user_session_key = data_blob_talloc( dst, src->user_session_key.data,
 		src->user_session_key.length);
-	dst->lm_session_key = data_blob_talloc(
-		dst, src->lm_session_key.data,
+		
+	dst->lm_session_key = data_blob_talloc(dst, src->lm_session_key.data,
 		src->lm_session_key.length);
-	pdb_copy_sam_account(src->sam_account, &dst->sam_account);
+		
+	if ( (dst->sam_account = samu_new( NULL )) != NULL )
+		pdb_copy_sam_account(dst->sam_account, src->sam_account);
+	
 	dst->pam_handle = NULL;
 	dst->unix_name = talloc_strdup(dst, src->unix_name);
 
@@ -1350,7 +1354,7 @@
 	
 	DEBUG(5,("fill_sam_account: located username was [%s]\n", *found_username));
 
-	nt_status = samu_set_unix( account, passwd );
+	nt_status = samu_set_unix( account, passwd, False );
 	
 	TALLOC_FREE(passwd);
 	
Index: source/auth/auth_rhosts.c
===================================================================
--- source/auth/auth_rhosts.c	(revision 13592)
+++ source/auth/auth_rhosts.c	(working copy)
@@ -49,7 +49,7 @@
 			return NT_STATUS_NO_SUCH_USER;
 		}
 
-		nt_status = samu_set_unix( *account, pass );
+		nt_status = samu_set_unix( *account, pass, False );
 	}
 
 	return nt_status;
Index: source/rpc_server/srv_samr_nt.c
===================================================================
--- source/rpc_server/srv_samr_nt.c	(revision 13592)
+++ source/rpc_server/srv_samr_nt.c	(working copy)
@@ -3087,18 +3087,17 @@
 	 * id21.  I don't know if they need to be set.    --jerry
 	 */
  
-	if (IS_SAM_CHANGED(pwd, PDB_GROUPSID) &&
-	    !NT_STATUS_IS_OK(status = pdb_set_unix_primary_group(mem_ctx,
-								 pwd))) {
-		return status;
+	if ( IS_SAM_CHANGED(pwd, PDB_GROUPSID) ) {
+		status = pdb_set_unix_primary_group(mem_ctx, pwd);
+		if ( !NT_STATUS_IS_OK(status) ) {
+			return status;
+		}
 	}
+	
+	/* Don't worry about writing out the user account since the
+	   primary group SID is generated solely from the user's Unix 
+	   primary group. */
 
-	/* write the change out */
-	if(!NT_STATUS_IS_OK(status = pdb_update_sam_account(pwd))) {
-		TALLOC_FREE(pwd);
-		return status;
- 	}
-
 	TALLOC_FREE(pwd);
 
 	return NT_STATUS_OK;
Index: source/passdb/pdb_tdb.c
===================================================================
--- source/passdb/pdb_tdb.c	(revision 13600)
+++ source/passdb/pdb_tdb.c	(working copy)
@@ -216,7 +216,6 @@
 
 	pdb_set_pw_history(sampass, NULL, 0, PDB_SET);
 	pdb_set_user_sid_from_rid(sampass, user_rid, PDB_SET);
-	pdb_set_group_sid_from_rid(sampass, group_rid, PDB_SET);
 	pdb_set_hours_len(sampass, hours_len, PDB_SET);
 	pdb_set_bad_password_count(sampass, bad_password_count, PDB_SET);
 	pdb_set_logon_count(sampass, logon_count, PDB_SET);
@@ -404,7 +403,6 @@
 	pdb_set_pw_history(sampass, NULL, 0, PDB_SET);
 
 	pdb_set_user_sid_from_rid(sampass, user_rid, PDB_SET);
-	pdb_set_group_sid_from_rid(sampass, group_rid, PDB_SET);
 	pdb_set_hours_len(sampass, hours_len, PDB_SET);
 	pdb_set_bad_password_count(sampass, bad_password_count, PDB_SET);
 	pdb_set_logon_count(sampass, logon_count, PDB_SET);
@@ -447,7 +445,7 @@
  Intialize a BYTE buffer from a struct samu struct
  *********************************************************************/
 
-static uint32 init_buffer_from_sam (uint8 **buf, const struct samu *sampass, BOOL size_only)
+static uint32 init_buffer_from_sam (uint8 **buf, struct samu *sampass, BOOL size_only)
 {
 	return init_buffer_from_sam_v2(buf, sampass, size_only);
 }
@@ -1171,7 +1169,11 @@
 	
 	tdbsam_endsampwent( my_methods );
 
-	if ( !pdb_copy_sam_account(old_acct, &new_acct) 
+	if ( !(new_acct = samu_new( NULL )) ) {
+		return NT_STATUS_NO_MEMORY;
+	}
+	
+	if ( !pdb_copy_sam_account(new_acct, old_acct) 
 		|| !pdb_set_username(new_acct, newname, PDB_CHANGED)) 
 	{
 		TALLOC_FREE(new_acct );
Index: source/passdb/pdb_ldap.c
===================================================================
--- source/passdb/pdb_ldap.c	(revision 13601)
+++ source/passdb/pdb_ldap.c	(working copy)
@@ -552,36 +552,12 @@
 				get_userattr_key2string(ldap_state->schema_ver, LDAP_ATTR_USER_SID), temp)) {
 			pdb_set_user_sid_from_string(sampass, temp, PDB_SET);
 		}
-		
-		if (smbldap_get_single_pstring(ldap_state->smbldap_state->ldap_struct, entry, 
-				get_userattr_key2string(ldap_state->schema_ver, LDAP_ATTR_PRIMARY_GROUP_SID), temp)) {
-			pdb_set_group_sid_from_string(sampass, temp, PDB_SET);			
-		} else {
-			pdb_set_group_sid_from_rid(sampass, DOMAIN_GROUP_RID_USERS, PDB_DEFAULT);
-		}
 	} else {
 		if (smbldap_get_single_pstring(ldap_state->smbldap_state->ldap_struct, entry,
 				get_userattr_key2string(ldap_state->schema_ver, LDAP_ATTR_USER_RID), temp)) {
 			user_rid = (uint32)atol(temp);
 			pdb_set_user_sid_from_rid(sampass, user_rid, PDB_SET);
 		}
-		
-		if (!smbldap_get_single_pstring(ldap_state->smbldap_state->ldap_struct, entry, 
-				get_userattr_key2string(ldap_state->schema_ver, LDAP_ATTR_PRIMARY_GROUP_RID), temp)) {
-			pdb_set_group_sid_from_rid(sampass, DOMAIN_GROUP_RID_USERS, PDB_DEFAULT);
-		} else {
-			uint32 group_rid;
-			
-			group_rid = (uint32)atol(temp);
-			
-			/* for some reason, we often have 0 as a primary group RID.
-			   Make sure that we treat this just as a 'default' value */
-			   
-			if ( group_rid > 0 )
-				pdb_set_group_sid_from_rid(sampass, group_rid, PDB_SET);
-			else
-				pdb_set_group_sid_from_rid(sampass, DOMAIN_GROUP_RID_USERS, PDB_DEFAULT);
-		}
 	}
 
 	if (pdb_get_init_flags(sampass,PDB_USERSID) == PDB_DEFAULT) {
Index: source/passdb/pdb_compat.c
===================================================================
--- source/passdb/pdb_compat.c	(revision 13592)
+++ source/passdb/pdb_compat.c	(working copy)
@@ -38,7 +38,7 @@
 	return (0);
 }
 
-uint32 pdb_get_group_rid (const struct samu *sampass)
+uint32 pdb_get_group_rid (struct samu *sampass)
 {
 	uint32 g_rid;
 
Index: source/passdb/passdb.c
===================================================================
--- source/passdb/passdb.c	(revision 13601)
+++ source/passdb/passdb.c	(working copy)
@@ -47,13 +47,39 @@
 	return lp_workgroup();
 }
 
-/************************************************************
- Fill the struct samu with default values.
- ***********************************************************/
+/**********************************************************************
+***********************************************************************/
 
-static void samu_init( struct samu *user )
+static int samu_destroy(void *p) 
 {
+	struct samu *user = p;
+
+	data_blob_clear_free( &user->lm_pw );
+	data_blob_clear_free( &user->nt_pw );
+
+	if ( user->plaintext_pw )
+		memset( user->plaintext_pw, 0x0, strlen(user->plaintext_pw) );
+
+	return 0;
+}
+
+/**********************************************************************
+ generate a new struct samuser
+***********************************************************************/
+
+struct samu* samu_new( TALLOC_CTX *ctx )
+{
+	struct samu *user;
+	
+	if ( !(user = TALLOC_ZERO_P( ctx, struct samu )) ) {
+		DEBUG(0,("samuser_new: Talloc failed!\n"));
+		return NULL;
+	}
+	
+	talloc_set_destructor( user, samu_destroy );
+	
 	/* no initial methods */
+	
 	user->methods = NULL;
 
         /* Don't change these timestamp settings without a good reason.
@@ -90,132 +116,53 @@
 
 	user->plaintext_pw = NULL;
 
-	/* 
-	   Unless we know otherwise have a Account Control Bit
+	/* Unless we know otherwise have a Account Control Bit
 	   value of 'normal user'.  This helps User Manager, which
-	   asks for a filtered list of users.
-	*/
+	   asks for a filtered list of users. */
 
 	user->acct_ctrl = ACB_NORMAL;
-}	
-
-/**********************************************************************
-***********************************************************************/
-
-static int samu_destroy(void *p) 
-{
-	struct samu *user = p;
-
-	data_blob_clear_free( &user->lm_pw );
-	data_blob_clear_free( &user->nt_pw );
-
-	if ( user->plaintext_pw )
-		memset( user->plaintext_pw, 0x0, strlen(user->plaintext_pw) );
-
-	return 0;
-}
-
-/**********************************************************************
- generate a new struct samuser
-***********************************************************************/
-
-struct samu* samu_new( TALLOC_CTX *ctx )
-{
-	struct samu *user;
 	
-	if ( !(user = TALLOC_ZERO_P( ctx, struct samu )) ) {
-		DEBUG(0,("samuser_new: Talloc failed!\n"));
-		return NULL;
-	}
 	
-	samu_init( user );
-	
-	talloc_set_destructor( user, samu_destroy );
-	
 	return user;
 }
 
 /*********************************************************************
  Initialize a struct samu from a struct passwd including the user 
- and group SIDs
+ and group SIDs.  The *user structure is filled out with the Unix
+ attributes and a user SID.
 *********************************************************************/
 
-NTSTATUS samu_set_unix(struct samu *user, const struct passwd *pwd)
+NTSTATUS samu_set_unix(struct samu *user, const struct passwd *pwd, BOOL create)
 {
 	const char *guest_account = lp_guestaccount();
-	GROUP_MAP map;
-	BOOL ret;
+	const char *domain = global_myname();
+	uint32 urid;
 
-	/* Set the Unix attributes */
-	
 	if ( !pwd ) {
 		return NT_STATUS_NO_SUCH_USER;
 	}
 
 	/* Basic properties based upon the Unix account information */
-
+	
 	pdb_set_username(user, pwd->pw_name, PDB_SET);
 	pdb_set_fullname(user, pwd->pw_gecos, PDB_SET);
 	pdb_set_domain (user, get_global_sam_name(), PDB_DEFAULT);
-
+	
 	/* save the password structure for later use */
-
+	
 	user->unix_pw = tcopy_passwd( user, pwd );
 
-	/* Special case for the guest account which must have a RID of 501.
-	   By default the guest account is a member of of the domain users 
-	   group as well as the domain guests group.  Verified against 
-	   Windows NT - 2003 */
+	/* Special case for the guest account which must have a RID of 501 */
 	
-	if ( !guest_account ) {
-		DEBUG(0,("samu_set_unix: No guest user defined!\n"));
-		return NT_STATUS_INVALID_ACCOUNT_NAME;
-	}
-	
-	if ( strequal( pwd->pw_name, guest_account ) ) 
-	{
+	if ( strequal( pwd->pw_name, guest_account ) ) {
 		if ( !pdb_set_user_sid_from_rid(user, DOMAIN_USER_RID_GUEST, PDB_DEFAULT)) {
 			return NT_STATUS_NO_SUCH_USER;
 		}
-			   
-		if ( !pdb_set_group_sid_from_rid(user, DOMAIN_GROUP_RID_USERS, PDB_DEFAULT) ) {
-			return NT_STATUS_NO_SUCH_USER;
-		}
 		return NT_STATUS_OK;
 	}
-
-	/* normal user setup -- we really need to throw away the mapping algorithm here */
 	
-	if (!pdb_set_user_sid_from_rid(user, algorithmic_pdb_uid_to_user_rid(pwd->pw_uid), PDB_SET)) {
-		DEBUG(0,("Can't set User SID from RID!\n"));
-		return NT_STATUS_INVALID_PARAMETER;
-	}
+	/* Non-guest accounts...Check for a workstation or user account */
 
-#if 1	/* I think we could throw away the primaryGroupSID attribute altogether
-	   and just build it from the UNIX_TOKEN.   --jerry */
-	   
-	/* call the mapping code here */
-	
-	become_root();
-	ret = pdb_getgrgid(&map, pwd->pw_gid);
-	unbecome_root();
-
-	/* We do not want to fall back to the rid mapping algorithm.   Windows 
-	   standalone servers set the 0x201 rid as the primary group and 
-	   LookupSid( S-1...-513 ) returns SERVER\None.   Do something similar.  
-	   Use the Domain Users RID as a a placeholder. This is a workaround only.  */
-		   
-	if( ret ) {
-		if ( !pdb_set_group_sid(user, &map.sid, PDB_SET) ) {
-			DEBUG(0,("Can't set Group SID!\n"));
-			return NT_STATUS_INVALID_PARAMETER;
-		}
-	} else {		
-		if ( !pdb_set_group_sid_from_rid(user, DOMAIN_GROUP_RID_USERS, PDB_SET)) 
-			return NT_STATUS_INVALID_PARAMETER;
-	}
-#endif
-	
 	if (pwd->pw_name[strlen(pwd->pw_name)-1] == '$') {
 		/* workstation */
 		
@@ -223,11 +170,7 @@
 			DEBUG(1, ("Failed to set 'workstation account' flags for user %s.\n", 
 				pwd->pw_name));
 			return NT_STATUS_INVALID_COMPUTER_NAME;
-		}
-	
-		/* we're done here for a machine account */
-			
-		return NT_STATUS_OK;
+		}	
 	} 
 	else {
 		/* user */
@@ -237,161 +180,59 @@
 				pwd->pw_name));
 			return NT_STATUS_INVALID_ACCOUNT_NAME;
 		}
-	}
-	
-	/* set some basic attributes */
-	
-	pdb_set_profile_path(user, talloc_sub_specified(user, 
-		lp_logon_path(), pwd->pw_name, global_myname(), pwd->pw_uid, pwd->pw_gid), 
-		PDB_DEFAULT);		
-	pdb_set_homedir(user, talloc_sub_specified(user, 
-		lp_logon_home(), pwd->pw_name, global_myname(), pwd->pw_uid, pwd->pw_gid),
-		PDB_DEFAULT);
-	pdb_set_dir_drive(user, talloc_sub_specified(user, 
-		lp_logon_drive(), pwd->pw_name, global_myname(), pwd->pw_uid, pwd->pw_gid),
-		PDB_DEFAULT);
-	pdb_set_logon_script(user, talloc_sub_specified(user, 
-		lp_logon_script(), pwd->pw_name, global_myname(), pwd->pw_uid, pwd->pw_gid), 
-		PDB_DEFAULT);
 		
-	return NT_STATUS_OK;
-}
-
-/*************************************************************
- Initialises a struct samu ready to add a new account, based
- on the UNIX user. 
- ************************************************************/
-
-NTSTATUS pdb_init_sam_new(struct samu **new_sam_acct, const char *username)
-{
-	NTSTATUS 	result;
-	struct passwd 	*pwd;
-	uint32 user_rid;
-	DOM_SID user_sid, group_sid;
-	TALLOC_CTX *mem_ctx;
-	enum SID_NAME_USE type;
-
-	mem_ctx = talloc_new(NULL);
-	if (mem_ctx == NULL) {
-		DEBUG(0, ("talloc_new failed\n"));
-		return NT_STATUS_NO_MEMORY;
-	}
+		/* set some basic attributes */
 	
-	if ( !(pwd = Get_Pwnam_alloc(mem_ctx, username)) ) {
-		DEBUG(10, ("Could not find user %s\n", username));
-		result = NT_STATUS_NO_SUCH_USER;
-		goto done;
+		pdb_set_profile_path(user, talloc_sub_specified(user, 
+			lp_logon_path(), pwd->pw_name, domain, pwd->pw_uid, pwd->pw_gid), 
+			PDB_DEFAULT);		
+		pdb_set_homedir(user, talloc_sub_specified(user, 
+			lp_logon_home(), pwd->pw_name, domain, pwd->pw_uid, pwd->pw_gid),
+			PDB_DEFAULT);
+		pdb_set_dir_drive(user, talloc_sub_specified(user, 
+			lp_logon_drive(), pwd->pw_name, domain, pwd->pw_uid, pwd->pw_gid),
+			PDB_DEFAULT);
+		pdb_set_logon_script(user, talloc_sub_specified(user, 
+			lp_logon_script(), pwd->pw_name, domain, pwd->pw_uid, pwd->pw_gid), 
+			PDB_DEFAULT);
 	}
+	
+	/* Now deal with the user SID.  If we have a backend that can generate 
+	   RIDs, then do so.  But sometimes the caller just wanted a structure 
+	   initialized and will fill in these fields later (such as from a 
+	   NET_USER_INFO_3 structure) */
 
-	if ( !(*new_sam_acct = samu_new( NULL )) ) {
-		result = NT_STATUS_NO_MEMORY;
-		goto done;
-	}
-
-	result = samu_set_unix( *new_sam_acct, pwd );
-
-	if (!NT_STATUS_IS_OK(result)) {
-		DEBUG(10, ("samu_set_unix failed: %s\n", nt_errstr(result)));
-		goto done;
-	}
+	if ( create && !pdb_rid_algorithm() ) {
+		uint32 user_rid;
+		DOM_SID user_sid;
 		
-	if (pdb_rid_algorithm()) {
-		if (!pdb_set_user_sid_from_rid(
-			    *new_sam_acct,
-			    algorithmic_pdb_uid_to_user_rid(pwd->pw_uid),
-			    PDB_SET)) {
-			result = NT_STATUS_INTERNAL_ERROR;
-			goto done;
+		if ( !pdb_new_rid( &user_rid ) ) {
+			DEBUG(3, ("Could not allocate a new RID\n"));
+			return NT_STATUS_ACCESS_DENIED;
 		}
-		if (!pdb_set_group_sid_from_rid(
-			    *new_sam_acct, pdb_gid_to_group_rid(pwd->pw_gid),
-			    PDB_SET)) {
-			result = NT_STATUS_INTERNAL_ERROR;
-			goto done;
-		}
-		result = NT_STATUS_OK;
-		goto done;
-	}
 
-	/* No algorithmic mapping, meaning that we have to figure out the
-	 * primary group SID according to group mapping and the user SID must
-	 * be a newly allocated one */
+		sid_copy( &user_sid, get_global_sam_sid() );
+		sid_append_rid( &user_sid, user_rid );
 
-	if (!pdb_gid_to_sid(pwd->pw_gid, &group_sid)) {
-		struct group *grp;
-		GROUP_MAP map;
-
-		grp = getgrgid(pwd->pw_gid);
-		if (grp == NULL) {
-			DEBUG(1, ("Primary group %d of user %s does not "
-				  "exist.\n", pwd->pw_gid, username));
-			result = NT_STATUS_INVALID_PRIMARY_GROUP;
-			goto done;
+		if ( !pdb_set_user_sid(user, &user_sid, PDB_SET) ) {
+			DEBUG(3, ("pdb_set_user_sid failed\n"));
+			return NT_STATUS_INTERNAL_ERROR;
 		}
-
-		DEBUG(5, ("Primary group %s of user %s is not mapped to "
-			  "a domain group, auto-mapping it\n",
-			  grp->gr_name, username));
-		result = map_unix_group(grp, &map);
-		if (!NT_STATUS_IS_OK(result)) {
-			DEBUG(1, ("Failed to map group %s\n", grp->gr_name));
-			goto done;
-		}
-		sid_copy(&group_sid, &map.sid);
-		DEBUG(5, ("Mapped unix group %s to SID %s\n",
-			  grp->gr_name, sid_string_static(&group_sid)));
+		
+		return NT_STATUS_OK;
 	}
 
-	/* Now check that it's actually a domain group and not something
-	 * else */
-
-	if (!lookup_sid(mem_ctx, &group_sid, NULL, NULL, &type)) {
-		DEBUG(3, ("Could not lookup %s's primary group sid %s\n",
-			  username, sid_string_static(&group_sid)));
-		result = NT_STATUS_INVALID_PRIMARY_GROUP;
-		goto done;
+	/* generate a SID for the user ewith the RID algorithm */
+	
+	urid = algorithmic_pdb_uid_to_user_rid( user->unix_pw->pw_uid );
+		
+	if ( !pdb_set_user_sid_from_rid( user, urid, PDB_SET) ) {
+		return NT_STATUS_INTERNAL_ERROR;
 	}
-
-	if (type != SID_NAME_DOM_GRP) {
-		DEBUG(3, ("Primary group for user %s is a %s and not a domain "
-			  "group\n", username, sid_type_lookup(type)));
-		result = NT_STATUS_INVALID_PRIMARY_GROUP;
-		goto done;
-	}
-
-	if (!pdb_set_group_sid(*new_sam_acct, &group_sid, PDB_SET)) {
-		DEBUG(3, ("Could not set group SID\n"));
-		result = NT_STATUS_INTERNAL_ERROR;
-		goto done;
-	}
-
-	if (!pdb_new_rid(&user_rid)) {
-		DEBUG(3, ("Could not allocate a new RID\n"));
-		result = NT_STATUS_ACCESS_DENIED;
-		goto done;
-	}
-
-	sid_copy(&user_sid, get_global_sam_sid());
-	sid_append_rid(&user_sid, user_rid);
-
-	if (!pdb_set_user_sid(*new_sam_acct, &user_sid, PDB_SET)) {
-		DEBUG(3, ("pdb_set_user_sid failed\n"));
-		result = NT_STATUS_INTERNAL_ERROR;
-		goto done;
-	}
-
-	result = NT_STATUS_OK;
-
- done:
-	if (!NT_STATUS_IS_OK(result) && (*new_sam_acct != NULL)) {
-		TALLOC_FREE(new_sam_acct);
-	}
-
-	TALLOC_FREE(mem_ctx);
-	return result;
+	
+	return NT_STATUS_OK;
 }
 
-
 /**********************************************************
  Encode the account control bits into a string.
  length = length of string to encode into (including terminating
@@ -574,6 +415,9 @@
 	return (True);
 }
 
+/********************************************************************
+********************************************************************/
+
 int algorithmic_rid_base(void)
 {
 	static int rid_offset = 0;
@@ -654,7 +498,7 @@
  Decides if a RID is a well known RID.
  ********************************************************************/
 
-static BOOL pdb_rid_is_well_known(uint32 rid)
+static BOOL rid_is_well_known(uint32 rid)
 {
 	/* Not using rid_offset here, because this is the actual
 	   NT fixed value (1000) */
@@ -668,7 +512,7 @@
 
 BOOL algorithmic_pdb_rid_is_user(uint32 rid)
 {
-	if(pdb_rid_is_well_known(rid)) {
+	if ( rid_is_well_known(rid) ) {
 		/*
 		 * The only well known user RIDs are DOMAIN_USER_RID_ADMIN
 		 * and DOMAIN_USER_RID_GUEST.
@@ -781,6 +625,7 @@
 		
 		if ((local_flags & LOCAL_ADD_USER) || (local_flags & LOCAL_DELETE_USER)) {
 			int tmp_debug = DEBUGLEVEL;
+			struct passwd *pwd;
 
 			/* Might not exist in /etc/passwd. */
 
@@ -788,16 +633,29 @@
 				DEBUGLEVEL = 1;
 			}
 
-			result = pdb_init_sam_new(&sam_pass, user_name);
+			if ( !(pwd = getpwnam_alloc( NULL, user_name)) ) {
+				return NT_STATUS_NO_SUCH_USER;
+			}
+
+			/* create the struct samu and initialize the basic Unix properties */
+
+			if ( !(sam_pass = samu_new( NULL )) ) {
+				return NT_STATUS_NO_MEMORY;
+			}
+
+			result = samu_set_unix( sam_pass, pwd, True );
+
 			DEBUGLEVEL = tmp_debug;
+
+			TALLOC_FREE( pwd );
+
 			if (NT_STATUS_EQUAL(result, NT_STATUS_INVALID_PRIMARY_GROUP)) {
 				return result;
 			}
 
 			if (!NT_STATUS_IS_OK(result)) {
-				slprintf(err_str, err_str_len-1, "Failed to "
-					 "initialize account for user %s: %s\n",
-					 user_name, nt_errstr(result));
+				slprintf(err_str, err_str_len-1, "Failed to " "initialize account for user %s: %s\n",
+					user_name, nt_errstr(result));
 				return result;
 			}
 		} else {
@@ -1130,7 +988,6 @@
 	}
 
 	pdb_set_user_sid_from_rid(sampass, user_rid, PDB_SET);
-	pdb_set_group_sid_from_rid(sampass, group_rid, PDB_SET);
 	pdb_set_hours_len(sampass, hours_len, PDB_SET);
 	pdb_set_bad_password_count(sampass, bad_password_count, PDB_SET);
 	pdb_set_logon_count(sampass, logon_count, PDB_SET);
@@ -1164,7 +1021,7 @@
 /*********************************************************************
 *********************************************************************/
 
-uint32 init_buffer_from_sam_v2 (uint8 **buf, const struct samu *sampass, BOOL size_only)
+uint32 init_buffer_from_sam_v2 (uint8 **buf, struct samu *sampass, BOOL size_only)
 {
 	size_t len, buflen;
 
@@ -1440,13 +1297,13 @@
 /*********************************************************************
 *********************************************************************/
 
-BOOL pdb_copy_sam_account(const struct samu *src, struct samu **dst)
+BOOL pdb_copy_sam_account(struct samu *dst, struct samu *src )
 {
 	BOOL result;
 	uint8 *buf;
 	int len;
 
-	if ( !*dst && !(*dst = samu_new(NULL)) )
+	if ( !dst )
 		return False;
 
 	len = init_buffer_from_sam_v2(&buf, src, False);
@@ -1454,8 +1311,11 @@
 	if (len == -1)
 		return False;
 
-	result = init_sam_from_buffer_v2(*dst, buf, len);
-	(*dst)->methods = src->methods;
+	result = init_sam_from_buffer_v2( dst, buf, len );
+	dst->methods = src->methods;
+	
+	if ( src->unix_pw )
+		dst->unix_pw = tcopy_passwd( dst, src->unix_pw );
 
 	free(buf);
 
Index: source/passdb/pdb_get_set.c
===================================================================
--- source/passdb/pdb_get_set.c	(revision 13601)
+++ source/passdb/pdb_get_set.c	(working copy)
@@ -181,16 +181,75 @@
 {
 	if (sampass) 
 		return &sampass->user_sid;
-	else
-		return (NULL);
+		
+	return NULL;
 }
 
-const DOM_SID *pdb_get_group_sid(const struct samu *sampass)
+const DOM_SID *pdb_get_group_sid(struct samu *sampass)
 {
-	if (sampass)
-		return &sampass->group_sid;
-	else	
-		return (NULL);
+	DOM_SID *gsid;
+	struct passwd *pwd;
+	
+	/* sanity check */
+	
+	if ( !sampass ) {
+		return NULL;
+	}
+		
+	/* Return the cached group SID if we have that */
+	    
+	if ( sampass->group_sid ) {
+		return sampass->group_sid;
+	}
+		
+	/* generate the group SID from the user's primary Unix group */
+	
+	if ( !(gsid  = TALLOC_P( sampass, DOM_SID )) ) {
+		return NULL;
+	}
+	
+	/* No algorithmic mapping, meaning that we have to figure out the
+	   primary group SID according to group mapping and the user SID must
+	   be a newly allocated one.  We rely on the user's Unix primary gid.
+	   We have no choice but to fail if we can't find it. */
+
+	if ( sampass->unix_pw )
+		pwd = sampass->unix_pw;
+	else
+		pwd = getpwnam_alloc( sampass, pdb_get_username(sampass) );
+
+	if ( !pwd ) {
+		DEBUG(0,("pdb_get_group_sid: Failed to find Unix account for %s\n", pdb_get_username(sampass) ));
+		return NULL;
+	}
+	
+	if ( pdb_gid_to_sid(pwd->pw_gid, gsid) ) {
+		enum SID_NAME_USE type = SID_NAME_UNKNOWN;
+		TALLOC_CTX *mem_ctx = talloc_init("pdb_get_group_sid");
+		
+		/* Now check that it's actually a domain group and not something else */
+
+		if ( lookup_sid(mem_ctx, gsid, NULL, NULL, &type) 
+			&& (type != SID_NAME_DOM_GRP) ) 
+		{
+			sampass->group_sid = gsid;
+			
+			return sampass->group_sid;
+		}
+
+		DEBUG(3, ("Primary group for user %s is a %s and not a domain group\n", 
+			pwd->pw_name, sid_type_lookup(type)));
+	}
+
+	/* Just set it to the 'Domain Users' RID of 512 which will 
+	   always resolve to a name */
+		   
+	sid_copy( gsid, get_global_sam_sid() );
+	sid_append_rid( gsid, DOMAIN_GROUP_RID_USERS );
+		
+	sampass->group_sid = gsid;
+		
+	return sampass->group_sid;
 }	
 
 /**
@@ -569,6 +628,14 @@
 	return True;
 }
 
+/********************************************************************
+ We never fill this in from a passdb backend but rather set is 
+ based on the user's primary group membership.  However, the 
+ struct samu* is overloaded and reused in domain memship code 
+ as well and built from the NET_USER_INFO_3 or PAC so we 
+ have to allow the explicitly setting of a group SID here.
+********************************************************************/
+
 BOOL pdb_set_group_sid (struct samu *sampass, const DOM_SID *g_sid, enum pdb_value_state flag)
 {
 	gid_t gid;
@@ -576,43 +643,26 @@
 	if (!sampass || !g_sid)
 		return False;
 
+	if ( !(sampass->group_sid = TALLOC_P( sampass, DOM_SID )) ) {
+		return False;
+	}
+
 	/* if we cannot resolve the SID to gid, then just ignore it and 
 	   store DOMAIN_USERS as the primary groupSID */
 
 	if ( sid_to_gid( g_sid, &gid ) ) {
-		sid_copy(&sampass->group_sid, g_sid);
+		sid_copy(sampass->group_sid, g_sid);
 	} else {
-		sid_copy( &sampass->group_sid, get_global_sam_sid() );
-		sid_append_rid( &sampass->group_sid, DOMAIN_GROUP_RID_USERS );
+		sid_copy( sampass->group_sid, get_global_sam_sid() );
+		sid_append_rid( sampass->group_sid, DOMAIN_GROUP_RID_USERS );
 	}
 
 	DEBUG(10, ("pdb_set_group_sid: setting group sid %s\n", 
-		    sid_string_static(&sampass->group_sid)));
+		sid_string_static(sampass->group_sid)));
 
 	return pdb_set_init_flags(sampass, PDB_GROUPSID, flag);
 }
 
-BOOL pdb_set_group_sid_from_string (struct samu *sampass, fstring g_sid, enum pdb_value_state flag)
-{
-	DOM_SID new_sid;
-	if (!sampass || !g_sid)
-		return False;
-
-	DEBUG(10, ("pdb_set_group_sid_from_string: setting group sid %s\n",
-		   g_sid));
-
-	if (!string_to_sid(&new_sid, g_sid)) { 
-		DEBUG(1, ("pdb_set_group_sid_from_string: %s isn't a valid SID!\n", g_sid));
-		return False;
-	}
-	 
-	if (!pdb_set_group_sid(sampass, &new_sid, flag)) {
-		DEBUG(1, ("pdb_set_group_sid_from_string: could not set sid %s on struct samu!\n", g_sid));
-		return False;
-	}
-	return True;
-}
-
 /*********************************************************************
  Set the user's UNIX name.
  ********************************************************************/
Index: source/passdb/pdb_interface.c
===================================================================
--- source/passdb/pdb_interface.c	(revision 13592)
+++ source/passdb/pdb_interface.c	(working copy)
@@ -274,7 +274,9 @@
 	}
 
 	pdb_force_pw_initialization( sam_acct );
-	pdb_copy_sam_account(sam_acct, &csamuser);
+	
+	if ( (csamuser = samu_new( NULL )) != NULL )
+		pdb_copy_sam_account(csamuser, sam_acct);
 
 	return True;
 }
@@ -294,12 +296,11 @@
 		return False;
 	}
 	
-	result = samu_set_unix(user, pwd);
+	result = samu_set_unix(user, pwd, False);
 
 	TALLOC_FREE( pwd );
 
 	return NT_STATUS_IS_OK( result );
-
 }
 
 /**********************************************************************
@@ -327,7 +328,7 @@
 	/* check the cache first */
 	
 	if ( csamuser && sid_equal(sid, pdb_get_user_sid(csamuser) ) )
-		return pdb_copy_sam_account(csamuser, &sam_acct);
+		return pdb_copy_sam_account(sam_acct, csamuser);
 
 	return NT_STATUS_IS_OK(pdb->getsampwsid(pdb, sam_acct, sid));
 }
@@ -338,8 +339,9 @@
 {
 	struct samu *sam_pass = NULL;
 	NTSTATUS status;
+	struct passwd *pwd;
 
-	if (Get_Pwnam_alloc(tmp_ctx, name) == NULL) {
+	if ( !(pwd = Get_Pwnam_alloc(tmp_ctx, name)) ) {
 		pstring add_script;
 		int add_ret;
 
@@ -357,19 +359,21 @@
 
 		all_string_sub(add_script, "%u", name, sizeof(add_script));
 		add_ret = smbrun(add_script,NULL);
-		DEBUG(add_ret ? 0 : 3, ("_samr_create_user: Running the "
-					"command `%s' gave %d\n",
+		DEBUG(add_ret ? 0 : 3, ("_samr_create_user: Running the command `%s' gave %d\n",
 					add_script, add_ret));
+		flush_pwnam_cache();
+
+		pwd = Get_Pwnam_alloc(tmp_ctx, name);
 	}
 
-	/* implicit call to getpwnam() next.  we have a valid SID coming out
-	 * of this call */
+	/* we have a valid SID coming out of this call */
 
-	flush_pwnam_cache();
-	status = pdb_init_sam_new(&sam_pass, name);
+	status = samu_set_unix( sam_pass, pwd, True );
 
+	TALLOC_FREE( pwd );
+
 	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(3, ("pdb_init_sam_new failed: %s\n", nt_errstr(status)));
+		DEBUG(3, ("pdb_default_create_user: failed to create a new user structure: %s\n", nt_errstr(status)));
 		return status;
 	}
 
@@ -379,8 +383,7 @@
 		return NT_STATUS_INTERNAL_ERROR;
 	}
 
-	/* Disable the account on creation, it does not have a reasonable
-	 * password yet. */
+	/* Disable the account on creation, it does not have a reasonable password yet. */
 
 	acb_info |= ACB_DISABLED;
 
Index: source/passdb/pdb_smbpasswd.c
===================================================================
--- source/passdb/pdb_smbpasswd.c	(revision 13592)
+++ source/passdb/pdb_smbpasswd.c	(working copy)
@@ -1203,7 +1203,7 @@
 			return False;
 	}
 	
-	if ( !NT_STATUS_IS_OK( samu_set_unix(sam_pass, pwfile)) )
+	if ( !NT_STATUS_IS_OK( samu_set_unix(sam_pass, pwfile, False)) )
 		return False;
 		
 	TALLOC_FREE(pwfile);
@@ -1474,10 +1474,16 @@
 	if (!*(lp_renameuser_script()))
 		goto done;
 
-	if (!pdb_copy_sam_account(old_acct, &new_acct) ||
-	    !pdb_set_username(new_acct, newname, PDB_CHANGED))
+	if ( !(new_acct = samu_new( NULL )) ) {
+		return NT_STATUS_NO_MEMORY;
+	}
+	
+	if ( !pdb_copy_sam_account( new_acct, old_acct ) 
+		|| !pdb_set_username(new_acct, newname, PDB_CHANGED)) 
+	{
 		goto done;
-
+	}
+	
 	ret = smbpasswd_add_sam_account(my_methods, new_acct);
 	if (!NT_STATUS_IS_OK(ret))
 		goto done;
Index: source/include/passdb.h
===================================================================
--- source/include/passdb.h	(revision 13601)
+++ source/include/passdb.h	(working copy)
@@ -163,8 +163,8 @@
 	const char *unknown_str;  /* don't know what this is, yet. */
 	const char *munged_dial;  /* munged path name and dial-back tel number */
 		
-	DOM_SID user_sid;    /* Primary User SID */
-	DOM_SID group_sid;   /* Primary Group SID */
+	DOM_SID user_sid;  
+	DOM_SID *group_sid;
 		
 	DATA_BLOB lm_pw; /* .data is Null if no password */
 	DATA_BLOB nt_pw; /* .data is Null if no password */
@@ -185,6 +185,7 @@
 	uint32 unknown_6; /* 0x0000 04ec */
 
 	/* a tag for who added the private methods */
+
 	const struct pdb_methods *backend_private_methods;
 	void *backend_private_data; 
 	void (*backend_private_data_free_fn)(void **);
Index: source/utils/pdbedit.c
===================================================================
--- source/utils/pdbedit.c	(revision 13592)
+++ source/utils/pdbedit.c	(working copy)
@@ -277,8 +277,7 @@
 			  const char *acct_desc, 
 			  const char *drive, const char *script, 
 			  const char *profile, const char *account_control,
-			  const char *user_sid, const char *group_sid,
-			  const char *user_domain,
+			  const char *user_sid, const char *user_domain,
 			  const BOOL badpw, const BOOL hours,
 			  time_t pwd_can_change, time_t pwd_must_change)
 {
@@ -369,21 +368,6 @@
 		}
 		pdb_set_user_sid (sam_pwent, &u_sid, PDB_CHANGED);
 	}
-	if (group_sid) {
-		DOM_SID g_sid;
-		if (!string_to_sid(&g_sid, group_sid)) {
-			/* not a complete sid, may be a RID, try building a SID */
-			int g_rid;
-			
-			if (sscanf(group_sid, "%d", &g_rid) != 1) {
-				fprintf(stderr, "Error passed string is not a complete group SID or RID!\n");
-				return -1;
-			}
-			sid_copy(&g_sid, get_global_sam_sid());
-			sid_append_rid(&g_sid, g_rid);
-		}
-		pdb_set_group_sid (sam_pwent, &g_sid, PDB_CHANGED);
-	}
 
 	if (badpw) {
 		pdb_set_bad_password_count(sam_pwent, 0, PDB_CHANGED);
@@ -407,17 +391,28 @@
 static int new_user (struct pdb_methods *in, const char *username,
 			const char *fullname, const char *homedir,
 			const char *drive, const char *script,
-			const char *profile, char *user_sid, char *group_sid,
-			BOOL stdin_get)
+			const char *profile, char *user_sid, BOOL stdin_get)
 {
-	struct samu *sam_pwent=NULL;
-
+	struct samu *sam_pwent;
 	char *password1, *password2;
 	int rc_pwd_cmp;
+	struct passwd *pwd;
 
 	get_global_sam_sid();
 
-	if (!NT_STATUS_IS_OK(pdb_init_sam_new(&sam_pwent, username))) {
+	if ( !(pwd = getpwnam_alloc( NULL, username )) ) {
+		DEBUG(0,("Cannot locate Unix account for %s\n", username));
+		return -1;
+	}
+
+	if ( !(sam_pwent = samu_new( NULL )) ) {
+		DEBUG(0, ("Memory allocation failure!\n"));
+		return -1;
+	}
+
+	if (!NT_STATUS_IS_OK(samu_set_unix(sam_pwent, pwd, True))) {
+		TALLOC_FREE( sam_pwent );
+		TALLOC_FREE( pwd );
 		DEBUG(0, ("could not create account to add new user %s\n", username));
 		return -1;
 	}
@@ -465,21 +460,6 @@
 		}
 		pdb_set_user_sid (sam_pwent, &u_sid, PDB_CHANGED);
 	}
-	if (group_sid) {
-		DOM_SID g_sid;
-		if (!string_to_sid(&g_sid, group_sid)) {
-			/* not a complete sid, may be a RID, try building a SID */
-			int g_rid;
-			
-			if (sscanf(group_sid, "%d", &g_rid) != 1) {
-				fprintf(stderr, "Error passed string is not a complete group SID or RID!\n");
-				return -1;
-			}
-			sid_copy(&g_sid, get_global_sam_sid());
-			sid_append_rid(&g_sid, g_rid);
-		}
-		pdb_set_group_sid (sam_pwent, &g_sid, PDB_CHANGED);
-	}
 	
 	pdb_set_acct_ctrl (sam_pwent, ACB_NORMAL, PDB_CHANGED);
 	
@@ -526,7 +506,7 @@
 			return -1;
 		}
 
-		if ( !NT_STATUS_IS_OK(samu_set_unix(sam_pwent, pwd)) ) {
+		if ( !NT_STATUS_IS_OK(samu_set_unix(sam_pwent, pwd, False)) ) {
 			fprintf(stderr, "Could not init sam from pw\n");
 			TALLOC_FREE(pwd);
 			return -1;
@@ -541,13 +521,9 @@
 	}
 
 	pdb_set_plaintext_passwd (sam_pwent, machinename);
-
-	pdb_set_username (sam_pwent, machineaccount, PDB_CHANGED);
-	
+	pdb_set_username (sam_pwent, machineaccount, PDB_CHANGED);	
 	pdb_set_acct_ctrl (sam_pwent, ACB_WSTRUST, PDB_CHANGED);
 	
-	pdb_set_group_sid_from_rid(sam_pwent, DOMAIN_GROUP_RID_COMPUTERS, PDB_CHANGED);
-	
 	if (NT_STATUS_IS_OK(in->add_sam_account (in, sam_pwent))) {
 		print_user_info (in, machineaccount, True, False);
 	} else {
@@ -647,7 +623,6 @@
 	static char *account_control = NULL;
 	static char *account_policy = NULL;
 	static char *user_sid = NULL;
-	static char *group_sid = NULL;
 	static long int account_policy_value = 0;
 	BOOL account_policy_value_set = False;
 	static BOOL badpw_reset = False;
@@ -673,7 +648,6 @@
 		{"profile",	'p', POPT_ARG_STRING, &profile_path, 0, "set profile path", NULL},
 		{"domain",	'I', POPT_ARG_STRING, &user_domain, 0, "set a users' domain", NULL},
 		{"user SID",	'U', POPT_ARG_STRING, &user_sid, 0, "set user SID or RID", NULL},
-		{"group SID",	'G', POPT_ARG_STRING, &group_sid, 0, "set group SID or RID", NULL},
 		{"create",	'a', POPT_ARG_NONE, &add_user, 0, "create user", NULL},
 		{"modify",	'r', POPT_ARG_NONE, &modify_user, 0, "modify user", NULL},
 		{"machine",	'm', POPT_ARG_NONE, &machine, 0, "account is a machine account", NULL},
@@ -743,7 +717,6 @@
 			(list_users ? BIT_LIST : 0) +
 			(force_initialised_password ? BIT_FIX_INIT : 0) +
 			(user_sid ? BIT_USERSIDS : 0) +
-			(group_sid ? BIT_USERSIDS : 0) +
 			(modify_user ? BIT_MODIFY : 0) +
 			(add_user ? BIT_CREATE : 0) +
 			(delete_user ? BIT_DELETE : 0) +
@@ -868,9 +841,7 @@
 				return new_machine (bdef, user_name);
 			} else {
 				return new_user (bdef, user_name, full_name, home_dir, 
-						 home_drive, logon_script, 
-						 profile_path, user_sid, group_sid,
-						 pw_from_stdin);
+					home_drive, logon_script, profile_path, user_sid, pw_from_stdin);
 			}
 		}
 
@@ -939,16 +910,10 @@
 					}
 				}	
 			}
-			return set_user_info (bdef, user_name, full_name,
-					      home_dir,
-					      acct_desc,
-					      home_drive,
-					      logon_script,
-					      profile_path, account_control,
-					      user_sid, group_sid,
-					      user_domain,
-					      badpw_reset, hours_reset,
-					      pwd_can_change, pwd_must_change);
+			return set_user_info (bdef, user_name, full_name, home_dir,
+				acct_desc, home_drive, logon_script, profile_path, account_control,
+				user_sid, user_domain, badpw_reset, hours_reset, pwd_can_change, 
+				pwd_must_change);
 error:
 			fprintf (stderr, "Error parsing the time in pwd-%s-change-time!\n", errstr);
 			return -1;


More information about the samba-technical mailing list