[SCM] Samba Shared Repository - branch master updated - release-4-0-0alpha7-1623-g57e03b6

Jeremy Allison jra at samba.org
Thu May 14 22:12:41 GMT 2009


The branch, master has been updated
       via  57e03b6a1d98219d5adafff570d1cb01d8b5758e (commit)
      from  d35a3952f091f4eaad43d1a3756c24e35b34c5bd (commit)

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 57e03b6a1d98219d5adafff570d1cb01d8b5758e
Author: Jeremy Allison <jra at samba.org>
Date:   Thu May 14 15:11:50 2009 -0700

    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.

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

Summary of changes:
 source3/rpc_server/srv_samr_nt.c |  353 +++++++++++++++++---------------------
 1 files changed, 156 insertions(+), 197 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/rpc_server/srv_samr_nt.c b/source3/rpc_server/srv_samr_nt.c
index dea1a8f..d27924f 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();


-- 
Samba Shared Repository


More information about the samba-cvs mailing list