Fix the core of the SAMR access functions. This passes make test, but

usrmgr fails against it. The core of this patch is to move all the
access mask setup into the _samr_OpenXXX functions, and then have
each specific function check the attached access_mask against the
required bits. We can then go through the MS-SAMR doc and match
things up. Signed off by Guenther, and writespace cleanup removal
by Volker.
Jeremy.

1 files changed, 156 insertions, 197 deletions

diff --git a/source3/rpc_server/srv_samr_nt.c b/source3/rpc_server/srv_samr_nt.c
index dea1a8f2b1..d27924f2d3 100644
--- a/source3/rpc_server/srv_samr_nt.c
+++ b/source3/rpc_server/srv_samr_nt.c
@@ -185,7 +185,8 @@ static NTSTATUS access_check_samr_object( SEC_DESC *psd, NT_USER_TOKEN *token,
 	   by privileges (mostly having to do with creating/modifying/deleting
 	   users and groups) */
 
-	if ( rights && user_has_any_privilege( token, rights ) ) {
+	if (rights && !se_priv_equal(rights, &se_priv_none) &&
+			user_has_any_privilege(token, rights)) {
 
 		saved_mask = (des_access & rights_mask);
 		des_access &= ~saved_mask;
@@ -552,11 +553,15 @@ NTSTATUS _samr_OpenDomain(pipes_struct *p,
 	make_samr_object_sd( p->mem_ctx, &psd, &sd_size, &dom_generic_mapping, NULL, 0 );
 	se_map_generic( &des_access, &dom_generic_mapping );
 
+	/*
+	 * Users with SeMachineAccount or SeAddUser get additional
+	 * SAMR_DOMAIN_ACCESS_CREATE_USER access, but no more.
+	 */
 	se_priv_copy( &se_rights, &se_machine_account );
 	se_priv_add( &se_rights, &se_add_users );
 
 	status = access_check_samr_object( psd, p->server_info->ptok,
-		&se_rights, GENERIC_RIGHTS_DOMAIN_WRITE, des_access,
+		&se_rights, SAMR_DOMAIN_ACCESS_CREATE_USER, des_access,
 		&acc_granted, "_samr_OpenDomain" );
 
 	if ( !NT_STATUS_IS_OK(status) )
@@ -2207,6 +2212,7 @@ NTSTATUS _samr_OpenUser(pipes_struct *p,
 	SEC_DESC *psd = NULL;
 	uint32    acc_granted;
 	uint32    des_access = r->in.access_mask;
+	uint32_t extra_access = 0;
 	size_t    sd_size;
 	bool ret;
 	NTSTATUS nt_status;
@@ -2236,8 +2242,70 @@ NTSTATUS _samr_OpenUser(pipes_struct *p,
 	make_samr_object_sd(p->mem_ctx, &psd, &sd_size, &usr_generic_mapping, &sid, SAMR_USR_RIGHTS_WRITE_PW);
 	se_map_generic(&des_access, &usr_generic_mapping);
 
-	se_priv_copy( &se_rights, &se_machine_account );
-	se_priv_add( &se_rights, &se_add_users );
+	/*
+	 * Get the sampass first as we need to check privilages
+	 * based on what kind of user object this is.
+	 * But don't reveal info too early if it didn't exist.
+	 */
+
+	become_root();
+	ret=pdb_getsampwsid(sampass, &sid);
+	unbecome_root();
+
+	se_priv_copy(&se_rights, &se_priv_none);
+
+	/*
+	 * We do the override access checks on *open*, not at
+	 * SetUserInfo time.
+	 */
+	if (ret) {
+		uint32_t acb_info = pdb_get_acct_ctrl(sampass);
+
+		if ((acb_info & ACB_WSTRUST) &&
+				user_has_any_privilege(p->server_info->ptok,
+						&se_machine_account)) {
+			/*
+			 * SeMachineAccount is needed to add
+			 * GENERIC_RIGHTS_USER_WRITE to a machine
+			 * account.
+			 */
+			se_priv_add(&se_rights, &se_machine_account);
+			DEBUG(10,("_samr_OpenUser: adding machine account "
+				"rights to handle for user %s\n",
+				pdb_get_username(sampass) ));
+		}
+		if ((acb_info & ACB_NORMAL) &&
+				user_has_any_privilege(p->server_info->ptok,
+						&se_add_users)) {
+			/*
+			 * SeAddUsers is needed to add
+			 * GENERIC_RIGHTS_USER_WRITE to a normal
+			 * account.
+			 */
+			se_priv_add(&se_rights, &se_add_users);
+			DEBUG(10,("_samr_OpenUser: adding add user "
+				"rights to handle for user %s\n",
+				pdb_get_username(sampass) ));
+		}
+		/*
+		 * Cheat - allow GENERIC_RIGHTS_USER_WRITE if pipe user is
+		 * in DOMAIN_GROUP_RID_ADMINS. This is almost certainly not
+		 * what Windows does but is a hack for people who haven't
+		 * set up privilages on groups in Samba.
+		 */
+		if (acb_info & (ACB_SVRTRUST|ACB_DOMTRUST)) {
+			if (lp_enable_privileges() && nt_token_check_domain_rid(p->server_info->ptok,
+							DOMAIN_GROUP_RID_ADMINS)) {
+				des_access &= ~GENERIC_RIGHTS_USER_WRITE;
+				extra_access = GENERIC_RIGHTS_USER_WRITE;
+				DEBUG(4,("_samr_OpenUser: Allowing "
+					"GENERIC_RIGHTS_USER_WRITE for "
+					"rid admins\n"));
+			}
+		}
+	}
+
+	TALLOC_FREE(sampass);
 
 	nt_status = access_check_samr_object(psd, p->server_info->ptok,
 		&se_rights, GENERIC_RIGHTS_USER_WRITE, des_access,
@@ -2246,16 +2314,13 @@ NTSTATUS _samr_OpenUser(pipes_struct *p,
 	if ( !NT_STATUS_IS_OK(nt_status) )
 		return nt_status;
 
-	become_root();
-	ret=pdb_getsampwsid(sampass, &sid);
-	unbecome_root();
-
 	/* check that the SID exists in our domain. */
 	if (ret == False) {
         	return NT_STATUS_NO_SUCH_USER;
 	}
 
-	TALLOC_FREE(sampass);
+	/* If we did the rid admins hack above, allow access. */
+	acc_granted |= extra_access;
 
 	uinfo = policy_handle_create(p, r->out.user_handle, acc_granted,
 				     struct samr_user_info, &nt_status);
@@ -2665,7 +2730,7 @@ static NTSTATUS get_user_info_18(pipes_struct *p,
 	if (ret == False) {
 		DEBUG(4, ("User %s not found\n", sid_string_dbg(user_sid)));
 		TALLOC_FREE(smbpass);
-		return (geteuid() == (uid_t)0) ? NT_STATUS_NO_SUCH_USER : NT_STATUS_ACCESS_DENIED;
+		return (geteuid() == sec_initial_uid()) ? NT_STATUS_NO_SUCH_USER : NT_STATUS_ACCESS_DENIED;
 	}
 
 	DEBUG(3,("User:[%s] 0x%x\n", pdb_get_username(smbpass), pdb_get_acct_ctrl(smbpass) ));
@@ -3483,47 +3548,44 @@ NTSTATUS _samr_CreateUser2(pipes_struct *p,
 
 	/* determine which user right we need to check based on the acb_info */
 
-	if ( acb_info & ACB_WSTRUST )
-	{
-		se_priv_copy( &se_rights, &se_machine_account );
+	if (geteuid() == sec_initial_uid()) {
+		se_priv_copy(&se_rights, &se_priv_none);
+		can_add_account = true;
+	} else if (acb_info & ACB_WSTRUST) {
+		se_priv_copy(&se_rights, &se_machine_account);
 		can_add_account = user_has_privileges(
 			p->server_info->ptok, &se_rights );
-	}
-	/* usrmgr.exe (and net rpc trustdom grant) creates a normal user
-	   account for domain trusts and changes the ACB flags later */
-	else if ( acb_info & ACB_NORMAL &&
-		  (account[strlen(account)-1] != '$') )
-	{
-		se_priv_copy( &se_rights, &se_add_users );
+	} else if (acb_info & ACB_NORMAL &&
+		  (account[strlen(account)-1] != '$')) {
+		/* usrmgr.exe (and net rpc trustdom grant) creates a normal user
+		   account for domain trusts and changes the ACB flags later */
+		se_priv_copy(&se_rights, &se_add_users);
 		can_add_account = user_has_privileges(
 			p->server_info->ptok, &se_rights );
-	}
-	else 	/* implicit assumption of a BDC or domain trust account here
+	} else if (lp_enable_privileges()) {
+		/* implicit assumption of a BDC or domain trust account here
 		 * (we already check the flags earlier) */
-	{
-		if ( lp_enable_privileges() ) {
-			/* only Domain Admins can add a BDC or domain trust */
-			se_priv_copy( &se_rights, &se_priv_none );
-			can_add_account = nt_token_check_domain_rid(
-				p->server_info->ptok,
-				DOMAIN_GROUP_RID_ADMINS );
-		}
+		/* only Domain Admins can add a BDC or domain trust */
+		se_priv_copy(&se_rights, &se_priv_none);
+		can_add_account = nt_token_check_domain_rid(
+			p->server_info->ptok,
+			DOMAIN_GROUP_RID_ADMINS );
 	}
 
 	DEBUG(5, ("_samr_CreateUser2: %s can add this account : %s\n",
 		  uidtoname(p->server_info->utok.uid),
 		  can_add_account ? "True":"False" ));
 
-	/********** BEGIN Admin BLOCK **********/
+	if (!can_add_account) {
+		return NT_STATUS_ACCESS_DENIED;
+	}
 
-	if ( can_add_account )
-		become_root();
+	/********** BEGIN Admin BLOCK **********/
 
+	become_root();
 	nt_status = pdb_create_user(p->mem_ctx, account, acb_info,
 				    r->out.rid);
-
-	if ( can_add_account )
-		unbecome_root();
+	unbecome_root();
 
 	/********** END Admin BLOCK **********/
 
@@ -3542,6 +3604,13 @@ NTSTATUS _samr_CreateUser2(pipes_struct *p,
 			    &sid, SAMR_USR_RIGHTS_WRITE_PW);
 	se_map_generic(&des_access, &usr_generic_mapping);
 
+	/*
+	 * JRA - TESTME. We just created this user so we
+	 * had rights to create them. Do we need to check
+	 * any further access on this object ? Can't we
+	 * just assume we have all the rights we need ?
+	 */
+
 	nt_status = access_check_samr_object(psd, p->server_info->ptok,
 		&se_rights, GENERIC_RIGHTS_USER_WRITE, des_access,
 		&acc_granted, "_samr_CreateUser2");
@@ -3889,10 +3958,9 @@ NTSTATUS _samr_OpenAlias(pipes_struct *p,
 
 	se_priv_copy( &se_rights, &se_add_users );
 
-
 	status = access_check_samr_object(psd, p->server_info->ptok,
-		&se_rights, GENERIC_RIGHTS_ALIAS_WRITE, des_access,
-		&acc_granted, "_samr_OpenAlias");
+		&se_rights, SAMR_ALIAS_ACCESS_ADD_MEMBER,
+		des_access, &acc_granted, "_samr_OpenAlias");
 
 	if ( !NT_STATUS_IS_OK(status) )
 		return status;
@@ -4660,8 +4728,6 @@ NTSTATUS _samr_SetUserInfo(pipes_struct *p,
 	uint16_t switch_value = r->in.level;
 	uint32_t acc_required;
 	bool ret;
-	bool has_enough_rights = False;
-	uint32_t acb_info;
 
 	DEBUG(5,("_samr_SetUserInfo: %d\n", __LINE__));
 
@@ -4716,32 +4782,9 @@ NTSTATUS _samr_SetUserInfo(pipes_struct *p,
 		return NT_STATUS_NO_SUCH_USER;
  	}
 
-	/* deal with machine password changes differently from userinfo changes */
-	/* check to see if we have the sufficient rights */
-
-	acb_info = pdb_get_acct_ctrl(pwd);
-	if (acb_info & ACB_WSTRUST)
-		has_enough_rights = user_has_privileges(p->server_info->ptok,
-							&se_machine_account);
-	else if (acb_info & ACB_NORMAL)
-		has_enough_rights = user_has_privileges(p->server_info->ptok,
-							&se_add_users);
-	else if (acb_info & (ACB_SVRTRUST|ACB_DOMTRUST)) {
-		if (lp_enable_privileges()) {
-			has_enough_rights = nt_token_check_domain_rid(p->server_info->ptok,
-								      DOMAIN_GROUP_RID_ADMINS);
-		}
-	}
+	/* ================ BEGIN Privilege BLOCK ================ */
 
-	DEBUG(5, ("_samr_SetUserInfo: %s does%s possess sufficient rights\n",
-		  uidtoname(p->server_info->utok.uid),
-		  has_enough_rights ? "" : " not"));
-
-	/* ================ BEGIN SeMachineAccountPrivilege BLOCK ================ */
-
-	if (has_enough_rights) {
-		become_root();
-	}
+	become_root();
 
 	/* ok!  user info levels (lots: see MSDEV help), off we go... */
 
@@ -4888,11 +4931,9 @@ NTSTATUS _samr_SetUserInfo(pipes_struct *p,
 
 	TALLOC_FREE(pwd);
 
-	if (has_enough_rights) {
-		unbecome_root();
-	}
+	unbecome_root();
 
-	/* ================ END SeMachineAccountPrivilege BLOCK ================ */
+	/* ================ END Privilege BLOCK ================ */
 
 	if (NT_STATUS_IS_OK(status)) {
 		force_flush_samr_cache(&uinfo->sid);
@@ -5108,8 +5149,6 @@ NTSTATUS _samr_AddAliasMember(pipes_struct *p,
 			      struct samr_AddAliasMember *r)
 {
 	struct samr_alias_info *ainfo;
-	SE_PRIV se_rights;
-	bool can_add_accounts;
 	NTSTATUS status;
 
 	ainfo = policy_handle_find(p, r->in.alias_handle,
@@ -5121,18 +5160,11 @@ NTSTATUS _samr_AddAliasMember(pipes_struct *p,
 
 	DEBUG(10, ("sid is %s\n", sid_string_dbg(&ainfo->sid)));
 
-	se_priv_copy( &se_rights, &se_add_users );
-	can_add_accounts = user_has_privileges( p->server_info->ptok, &se_rights );
-
 	/******** BEGIN SeAddUsers BLOCK *********/
 
-	if ( can_add_accounts )
-		become_root();
-
+	become_root();
 	status = pdb_add_aliasmem(&ainfo->sid, r->in.sid);
-
-	if ( can_add_accounts )
-		unbecome_root();
+	unbecome_root();
 
 	/******** END SeAddUsers BLOCK *********/
 
@@ -5151,8 +5183,6 @@ NTSTATUS _samr_DeleteAliasMember(pipes_struct *p,
 				 struct samr_DeleteAliasMember *r)
 {
 	struct samr_alias_info *ainfo;
-	SE_PRIV se_rights;
-	bool can_add_accounts;
 	NTSTATUS status;
 
 	ainfo = policy_handle_find(p, r->in.alias_handle,
@@ -5165,18 +5195,11 @@ NTSTATUS _samr_DeleteAliasMember(pipes_struct *p,
 	DEBUG(10, ("_samr_del_aliasmem:sid is %s\n",
 		   sid_string_dbg(&ainfo->sid)));
 
-	se_priv_copy( &se_rights, &se_add_users );
-	can_add_accounts = user_has_privileges( p->server_info->ptok, &se_rights );
-
 	/******** BEGIN SeAddUsers BLOCK *********/
 
-	if ( can_add_accounts )
-		become_root();
-
+	become_root();
 	status = pdb_del_aliasmem(&ainfo->sid, r->in.sid);
-
-	if ( can_add_accounts )
-		unbecome_root();
+	unbecome_root();
 
 	/******** END SeAddUsers BLOCK *********/
 
@@ -5197,8 +5220,6 @@ NTSTATUS _samr_AddGroupMember(pipes_struct *p,
 	struct samr_group_info *ginfo;
 	NTSTATUS status;
 	uint32 group_rid;
-	SE_PRIV se_rights;
-	bool can_add_accounts;
 
 	ginfo = policy_handle_find(p, r->in.group_handle,
 				   SAMR_GROUP_ACCESS_ADD_MEMBER, NULL,
@@ -5214,18 +5235,11 @@ NTSTATUS _samr_AddGroupMember(pipes_struct *p,
 		return NT_STATUS_INVALID_HANDLE;
 	}
 
-	se_priv_copy( &se_rights, &se_add_users );
-	can_add_accounts = user_has_privileges( p->server_info->ptok, &se_rights );
-
 	/******** BEGIN SeAddUsers BLOCK *********/
 
-	if ( can_add_accounts )
-		become_root();
-
+	become_root();
 	status = pdb_add_groupmem(p->mem_ctx, group_rid, r->in.rid);
-
-	if ( can_add_accounts )
-		unbecome_root();
+	unbecome_root();
 
 	/******** END SeAddUsers BLOCK *********/
 
@@ -5245,8 +5259,6 @@ NTSTATUS _samr_DeleteGroupMember(pipes_struct *p,
 	struct samr_group_info *ginfo;
 	NTSTATUS status;
 	uint32 group_rid;
-	SE_PRIV se_rights;
-	bool can_add_accounts;
 
 	/*
 	 * delete the group member named r->in.rid
@@ -5266,18 +5278,11 @@ NTSTATUS _samr_DeleteGroupMember(pipes_struct *p,
 		return NT_STATUS_INVALID_HANDLE;
 	}
 
-	se_priv_copy( &se_rights, &se_add_users );
-	can_add_accounts = user_has_privileges( p->server_info->ptok, &se_rights );
-
 	/******** BEGIN SeAddUsers BLOCK *********/
 
-	if ( can_add_accounts )
-		become_root();
-
+	become_root();
 	status = pdb_del_groupmem(p->mem_ctx, group_rid, r->in.rid);
-
-	if ( can_add_accounts )
-		unbecome_root();
+	unbecome_root();
 
 	/******** END SeAddUsers BLOCK *********/
 
@@ -5296,8 +5301,8 @@ NTSTATUS _samr_DeleteUser(pipes_struct *p,
 	struct samr_user_info *uinfo;
 	NTSTATUS status;
 	struct samu *sam_pass=NULL;
-	bool can_add_accounts;
-	uint32 acb_info;
+	bool can_del_accounts = false;
+	uint32 acb_info = 0;
 	bool ret;
 
 	DEBUG(5, ("_samr_DeleteUser: %d\n", __LINE__));
@@ -5321,31 +5326,36 @@ NTSTATUS _samr_DeleteUser(pipes_struct *p,
 	ret = pdb_getsampwsid(sam_pass, &uinfo->sid);
 	unbecome_root();
 
-	if( !ret ) {
-		DEBUG(5,("_samr_DeleteUser: User %s doesn't exist.\n",
-			sid_string_dbg(&uinfo->sid)));
-		TALLOC_FREE(sam_pass);
-		return NT_STATUS_NO_SUCH_USER;
+	if (ret) {
+		acb_info = pdb_get_acct_ctrl(sam_pass);
 	}
 
-	acb_info = pdb_get_acct_ctrl(sam_pass);
-
 	/* For machine accounts it's the SeMachineAccountPrivilege that counts. */
-	if ( acb_info & ACB_WSTRUST ) {
-		can_add_accounts = user_has_privileges( p->server_info->ptok, &se_machine_account );
+	if (geteuid() == sec_initial_uid()) {
+		can_del_accounts = true;
+	} else if (acb_info & ACB_WSTRUST) {
+		can_del_accounts = user_has_privileges( p->server_info->ptok, &se_machine_account );
 	} else {
-		can_add_accounts = user_has_privileges( p->server_info->ptok, &se_add_users );
+		can_del_accounts = user_has_privileges( p->server_info->ptok, &se_add_users );
 	}
 
-	/******** BEGIN SeAddUsers BLOCK *********/
+	if (!can_del_accounts) {
+		TALLOC_FREE(sam_pass);
+		return NT_STATUS_ACCESS_DENIED;
+	}
 
-	if ( can_add_accounts )
-		become_root();
+	if(!ret) {
+		DEBUG(5,("_samr_DeleteUser: User %s doesn't exist.\n",
+			sid_string_dbg(&uinfo->sid)));
+		TALLOC_FREE(sam_pass);
+		return NT_STATUS_NO_SUCH_USER;
+	}
 
-	status = pdb_delete_user(p->mem_ctx, sam_pass);
+	/******** BEGIN SeAddUsers BLOCK *********/
 
-	if ( can_add_accounts )
-		unbecome_root();
+	become_root();
+	status = pdb_delete_user(p->mem_ctx, sam_pass);
+	unbecome_root();
 
 	/******** END SeAddUsers BLOCK *********/
 
@@ -5380,8 +5390,6 @@ NTSTATUS _samr_DeleteDomainGroup(pipes_struct *p,
 	struct samr_group_info *ginfo;
 	NTSTATUS status;
 	uint32 group_rid;
-	SE_PRIV se_rights;
-	bool can_add_accounts;
 
 	DEBUG(5, ("samr_DeleteDomainGroup: %d\n", __LINE__));
 
@@ -5399,18 +5407,11 @@ NTSTATUS _samr_DeleteDomainGroup(pipes_struct *p,
 		return NT_STATUS_NO_SUCH_GROUP;
 	}
 
-	se_priv_copy( &se_rights, &se_add_users );
-	can_add_accounts = user_has_privileges( p->server_info->ptok, &se_rights );
-
 	/******** BEGIN SeAddUsers BLOCK *********/
 
-	if ( can_add_accounts )
-		become_root();
-
+	become_root();
 	status = pdb_delete_dom_group(p->mem_ctx, group_rid);
-
-	if ( can_add_accounts )
-		unbecome_root();
+	unbecome_root();
 
 	/******** END SeAddUsers BLOCK *********/
 
@@ -5438,8 +5439,6 @@ NTSTATUS _samr_DeleteDomAlias(pipes_struct *p,
 			      struct samr_DeleteDomAlias *r)
 {
 	struct samr_alias_info *ainfo;
-	SE_PRIV se_rights;
-	bool can_add_accounts;
 	NTSTATUS status;
 
 	DEBUG(5, ("_samr_DeleteDomAlias: %d\n", __LINE__));
@@ -5464,19 +5463,12 @@ NTSTATUS _samr_DeleteDomAlias(pipes_struct *p,
 
 	DEBUG(10, ("lookup on Local SID\n"));
 
-	se_priv_copy( &se_rights, &se_add_users );
-	can_add_accounts = user_has_privileges( p->server_info->ptok, &se_rights );
-
 	/******** BEGIN SeAddUsers BLOCK *********/
 
-	if ( can_add_accounts )
-		become_root();
-
+	become_root();
 	/* Have passdb delete the alias */
 	status = pdb_delete_alias(&ainfo->sid);
-
-	if ( can_add_accounts )
-		unbecome_root();
+	unbecome_root();
 
 	/******** END SeAddUsers BLOCK *********/
 
@@ -5503,8 +5495,6 @@ NTSTATUS _samr_CreateDomainGroup(pipes_struct *p,
 	const char *name;
 	struct samr_domain_info *dinfo;
 	struct samr_group_info *ginfo;
-	SE_PRIV se_rights;
-	bool can_add_accounts;
 
 	dinfo = policy_handle_find(p, r->in.domain_handle,
 				   SAMR_DOMAIN_ACCESS_CREATE_GROUP, NULL,
@@ -5526,20 +5516,12 @@ NTSTATUS _samr_CreateDomainGroup(pipes_struct *p,
 		return status;
 	}
 
-	se_priv_copy( &se_rights, &se_add_users );
-	can_add_accounts = user_has_privileges( p->server_info->ptok, &se_rights );
-
 	/******** BEGIN SeAddUsers BLOCK *********/
 
-	if ( can_add_accounts )
-		become_root();
-
+	become_root();
 	/* check that we successfully create the UNIX group */
-
 	status = pdb_create_dom_group(p->mem_ctx, name, r->out.rid);
-
-	if ( can_add_accounts )
-		unbecome_root();
+	unbecome_root();
 
 	/******** END SeAddUsers BLOCK *********/
 
@@ -5574,8 +5556,6 @@ NTSTATUS _samr_CreateDomAlias(pipes_struct *p,
 	struct samr_alias_info *ainfo;
 	gid_t gid;
 	NTSTATUS result;
-	SE_PRIV se_rights;
-	bool can_add_accounts;
 
 	dinfo = policy_handle_find(p, r->in.domain_handle,
 				   SAMR_DOMAIN_ACCESS_CREATE_ALIAS, NULL,
@@ -5589,9 +5569,6 @@ NTSTATUS _samr_CreateDomAlias(pipes_struct *p,
 
 	name = r->in.alias_name->string;
 
-	se_priv_copy( &se_rights, &se_add_users );
-	can_add_accounts = user_has_privileges( p->server_info->ptok, &se_rights );
-
 	result = can_create(p->mem_ctx, name);
 	if (!NT_STATUS_IS_OK(result)) {
 		return result;
@@ -5599,14 +5576,10 @@ NTSTATUS _samr_CreateDomAlias(pipes_struct *p,
 
 	/******** BEGIN SeAddUsers BLOCK *********/
 
-	if ( can_add_accounts )
-		become_root();
-
+	become_root();
 	/* Have passdb create the alias */
 	result = pdb_create_alias(name, r->out.rid);
-
-	if ( can_add_accounts )
-		unbecome_root();
+	unbecome_root();
 
 	/******** END SeAddUsers BLOCK *********/
 
@@ -5757,7 +5730,6 @@ NTSTATUS _samr_SetGroupInfo(pipes_struct *p,
 	GROUP_MAP map;
 	NTSTATUS status;
 	bool ret;
-	bool can_mod_accounts;
 
 	ginfo = policy_handle_find(p, r->in.group_handle,
 				   SAMR_GROUP_ACCESS_SET_INFO, NULL,
@@ -5786,17 +5758,11 @@ NTSTATUS _samr_SetGroupInfo(pipes_struct *p,
 			return NT_STATUS_INVALID_INFO_CLASS;
 	}
 
-	can_mod_accounts = user_has_privileges( p->server_info->ptok, &se_add_users );
-
 	/******** BEGIN SeAddUsers BLOCK *********/
 
-	if ( can_mod_accounts )
-		become_root();
-
+	become_root();
 	status = pdb_update_group_mapping_entry(&map);
-
-	if ( can_mod_accounts )
-		unbecome_root();
+	unbecome_root();
 
 	/******** End SeAddUsers BLOCK *********/
 
@@ -5816,7 +5782,6 @@ NTSTATUS _samr_SetAliasInfo(pipes_struct *p,
 {
 	struct samr_alias_info *ainfo;
 	struct acct_info info;
-	bool can_mod_accounts;
 	NTSTATUS status;
 
 	ainfo = policy_handle_find(p, r->in.alias_handle,
@@ -5883,17 +5848,11 @@ NTSTATUS _samr_SetAliasInfo(pipes_struct *p,
 			return NT_STATUS_INVALID_INFO_CLASS;
 	}
 
-        can_mod_accounts = user_has_privileges( p->server_info->ptok, &se_add_users );
-
         /******** BEGIN SeAddUsers BLOCK *********/
 
-        if ( can_mod_accounts )
-                become_root();
-
+	become_root();
         status = pdb_set_aliasinfo( &ainfo->sid, &info );
-
-        if ( can_mod_accounts )
-                unbecome_root();
+	unbecome_root();
 
         /******** End SeAddUsers BLOCK *********/
 
@@ -5975,8 +5934,8 @@ NTSTATUS _samr_OpenGroup(pipes_struct *p,
 	se_priv_copy( &se_rights, &se_add_users );
 
 	status = access_check_samr_object(psd, p->server_info->ptok,
-		&se_rights, GENERIC_RIGHTS_GROUP_WRITE, des_access,
-		&acc_granted, "_samr_OpenGroup");
+		&se_rights, SAMR_GROUP_ACCESS_ADD_MEMBER,
+		des_access, &acc_granted, "_samr_OpenGroup");
 
 	if ( !NT_STATUS_IS_OK(status) )
 		return status;