[SCM] Samba Shared Repository - branch master updated

Volker Lendecke vlendec at samba.org
Fri Dec 2 08:03:02 UTC 2022


The branch, master has been updated
       via  cffe96ef613 nfs4_acl: Add comment for setting ACL as root
       via  154a0613f89 posix_acls: Make try_chown and unpack_nt_owners static
       via  bfb4b368e10 nfs4_acls: Call chown_if_needed function to remove duplicate code
       via  eeb8a66bf76 posix_acl: Move chown checks to new function
       via  1f3826a7f65 posix_acls: Remove redundant call to save mode
      from  d9c192546fa lib/compression/lzxpress: fix our slow compression

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


- Log -----------------------------------------------------------------
commit cffe96ef6132966305c640a329ed91f0f9514452
Author: Christof Schmitt <cs at samba.org>
Date:   Tue Nov 29 16:51:10 2022 -0700

    nfs4_acl: Add comment for setting ACL as root
    
    Signed-off-by: Christof Schmitt <cs at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    
    Autobuild-User(master): Volker Lendecke <vl at samba.org>
    Autobuild-Date(master): Fri Dec  2 08:02:13 UTC 2022 on sn-devel-184

commit 154a0613f89a84becd6461e36d61a80509b9a9ef
Author: Christof Schmitt <cs at samba.org>
Date:   Tue Jul 12 16:35:37 2022 -0700

    posix_acls: Make try_chown and unpack_nt_owners static
    
    These functions are now only called from check_chown in posix_acls.c
    
    Signed-off-by: Christof Schmitt <cs at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit bfb4b368e1031c9c61274572fe8a453c055267a7
Author: Christof Schmitt <cs at samba.org>
Date:   Tue Jul 12 16:32:08 2022 -0700

    nfs4_acls: Call chown_if_needed function to remove duplicate code
    
    Signed-off-by: Christof Schmitt <cs at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit eeb8a66bf76e4cc095532887cf2532b10e31b23f
Author: Christof Schmitt <cs at samba.org>
Date:   Tue Nov 29 16:46:24 2022 -0700

    posix_acl: Move chown checks to new function
    
    Signed-off-by: Christof Schmitt <cs at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 1f3826a7f65a9123be6ebe3f9cc234ca691b28ec
Author: Christof Schmitt <cs at samba.org>
Date:   Tue Jul 12 16:08:07 2022 -0700

    posix_acls: Remove redundant call to save mode
    
    The same assignment is already done earlier, and nothing is changed in
    between.
    
    Signed-off-by: Christof Schmitt <cs at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

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

Summary of changes:
 source3/modules/nfs4_acls.c |  51 ++++----------------
 source3/smbd/posix_acls.c   | 111 +++++++++++++++++++++++++-------------------
 source3/smbd/proto.h        |   5 +-
 3 files changed, 77 insertions(+), 90 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c
index ff446bb1166..2daae990042 100644
--- a/source3/modules/nfs4_acls.c
+++ b/source3/modules/nfs4_acls.c
@@ -978,8 +978,6 @@ NTSTATUS smb_set_nt_acl_nfs4(vfs_handle_struct *handle, files_struct *fsp,
 	bool	result, is_directory;
 
 	bool set_acl_as_root = false;
-	uid_t newUID = (uid_t)-1;
-	gid_t newGID = (gid_t)-1;
 	int saved_errno;
 	NTSTATUS status;
 	TALLOC_CTX *frame = talloc_stackframe();
@@ -1019,49 +1017,20 @@ NTSTATUS smb_set_nt_acl_nfs4(vfs_handle_struct *handle, files_struct *fsp,
 	is_directory = S_ISDIR(fsp->fsp_name->st.st_ex_mode);
 
 	if (pparams->do_chown) {
-		/* chown logic is a copy/paste from posix_acl.c:set_nt_acl */
-
-		uid_t old_uid = fsp->fsp_name->st.st_ex_uid;
-		gid_t old_gid = fsp->fsp_name->st.st_ex_gid;
-		status = unpack_nt_owners(fsp->conn, &newUID, &newGID,
-					  security_info_sent, psd);
+		/*
+		 * When the chown succeeds, the special entries in the
+		 * file system ACL refer to the new owner. In order to
+		 * apply the complete information from the DACL,
+		 * setting the ACL then has to succeed. Track this
+		 * case with set_acl_as_root and set the ACL as root
+		 * accordingly.
+		 */
+		status = chown_if_needed(fsp, security_info_sent, psd,
+					 &set_acl_as_root);
 		if (!NT_STATUS_IS_OK(status)) {
-			DEBUG(8, ("unpack_nt_owners failed"));
 			TALLOC_FREE(frame);
 			return status;
 		}
-		if (((newUID != (uid_t)-1) && (old_uid != newUID)) ||
-		    ((newGID != (gid_t)-1) && (old_gid != newGID)))
-		{
-			status = try_chown(fsp, newUID, newGID);
-			if (!NT_STATUS_IS_OK(status)) {
-				DEBUG(3,("chown %s, %u, %u failed. Error = "
-					 "%s.\n", fsp_str_dbg(fsp),
-					 (unsigned int)newUID,
-					 (unsigned int)newGID,
-					 nt_errstr(status)));
-				TALLOC_FREE(frame);
-				return status;
-			}
-
-			DEBUG(10,("chown %s, %u, %u succeeded.\n",
-				  fsp_str_dbg(fsp), (unsigned int)newUID,
-				  (unsigned int)newGID));
-
-			/*
-			 * Owner change, need to update stat info.
-			 */
-			status = vfs_stat_fsp(fsp);
-			if (!NT_STATUS_IS_OK(status)) {
-				TALLOC_FREE(frame);
-				return status;
-			}
-
-			/* If we successfully chowned, we know we must
-			 * be able to set the acl, so do it as root.
-			 */
-			set_acl_as_root = true;
-		}
 	}
 
 	if (!(security_info_sent & SECINFO_DACL) || psd->dacl ==NULL) {
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index 8cef0fee024..15c7f14062e 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -1081,10 +1081,10 @@ static mode_t map_nt_perms( uint32_t *mask, int type)
  Unpack a struct security_descriptor into a UNIX owner and group.
 ****************************************************************************/
 
-NTSTATUS unpack_nt_owners(struct connection_struct *conn,
-			uid_t *puser, gid_t *pgrp,
-			uint32_t security_info_sent, const struct
-			security_descriptor *psd)
+static NTSTATUS unpack_nt_owners(struct connection_struct *conn,
+				 uid_t *puser, gid_t *pgrp,
+				 uint32_t security_info_sent,
+				 const struct security_descriptor *psd)
 {
 	*puser = (uid_t)-1;
 	*pgrp = (gid_t)-1;
@@ -3388,7 +3388,7 @@ NTSTATUS posix_fget_nt_acl(struct files_struct *fsp, uint32_t security_info,
      then allow chown to the currently authenticated user.
 ****************************************************************************/
 
-NTSTATUS try_chown(files_struct *fsp, uid_t uid, gid_t gid)
+static NTSTATUS try_chown(files_struct *fsp, uid_t uid, gid_t gid)
 {
 	NTSTATUS status;
 	int ret;
@@ -3475,6 +3475,59 @@ NTSTATUS try_chown(files_struct *fsp, uid_t uid, gid_t gid)
 	return status;
 }
 
+/*
+ * Check whether a chown is needed and if so, attempt the chown
+ * A returned error indicates that the chown failed.
+ * NT_STATUS_OK with did_chown == false indicates that the chown was skipped.
+ * NT_STATUS_OK with did_chown == true indicates that the chown succeeded
+ */
+NTSTATUS chown_if_needed(files_struct *fsp, uint32_t security_info_sent,
+			 const struct security_descriptor *psd,
+			 bool *did_chown)
+{
+	NTSTATUS status;
+	uid_t uid = (uid_t)-1;
+	gid_t gid = (gid_t)-1;
+
+	status = unpack_nt_owners(fsp->conn, &uid, &gid, security_info_sent, psd);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
+
+	if (((uid == (uid_t)-1) || (fsp->fsp_name->st.st_ex_uid == uid)) &&
+	    ((gid == (gid_t)-1) || (fsp->fsp_name->st.st_ex_gid == gid))) {
+		/*
+		 * Skip chown
+		 */
+		*did_chown = false;
+		return NT_STATUS_OK;
+	}
+
+	DBG_NOTICE("chown %s. uid = %u, gid = %u.\n",
+		   fsp_str_dbg(fsp), (unsigned int) uid, (unsigned int)gid);
+
+	status = try_chown(fsp, uid, gid);
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_INFO("chown %s, %u, %u failed. Error = %s.\n",
+			 fsp_str_dbg(fsp), (unsigned int) uid,
+			 (unsigned int)gid, nt_errstr(status));
+		return status;
+	}
+
+	/*
+	 * Recheck the current state of the file, which may have changed.
+	 * (owner and suid/sgid bits, for instance)
+	 */
+
+	status = vfs_stat_fsp(fsp);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
+
+	*did_chown = true;
+	return NT_STATUS_OK;
+}
+
 /****************************************************************************
  Reply to set a security descriptor on an fsp. security_info_sent is the
  description of the following NT ACL.
@@ -3486,8 +3539,6 @@ NTSTATUS try_chown(files_struct *fsp, uid_t uid, gid_t gid)
 NTSTATUS set_nt_acl(files_struct *fsp, uint32_t security_info_sent, const struct security_descriptor *psd_orig)
 {
 	connection_struct *conn = fsp->conn;
-	uid_t user = (uid_t)-1;
-	gid_t grp = (gid_t)-1;
 	struct dom_sid file_owner_sid;
 	struct dom_sid file_grp_sid;
 	canon_ace *file_ace_list = NULL;
@@ -3559,51 +3610,17 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32_t security_info_sent, const struct
 		security_info_sent &= ~SECINFO_OWNER;
 	}
 
-	status = unpack_nt_owners( conn, &user, &grp, security_info_sent, psd);
-	if (!NT_STATUS_IS_OK(status)) {
-		return status;
-	}
-
 	/*
 	 * Do we need to chown ? If so this must be done first as the incoming
 	 * CREATOR_OWNER acl will be relative to the *new* owner, not the old.
 	 * Noticed by Simo.
+	 *
+	 * If we successfully chowned, we know we must be able to set
+	 * the acl, so do it as root (set_acl_as_root).
 	 */
-
-	if (((user != (uid_t)-1) && (fsp->fsp_name->st.st_ex_uid != user)) ||
-	    (( grp != (gid_t)-1) && (fsp->fsp_name->st.st_ex_gid != grp))) {
-
-		DEBUG(3,("set_nt_acl: chown %s. uid = %u, gid = %u.\n",
-			 fsp_str_dbg(fsp), (unsigned int)user,
-			 (unsigned int)grp));
-
-		status = try_chown(fsp, user, grp);
-		if(!NT_STATUS_IS_OK(status)) {
-			DEBUG(3,("set_nt_acl: chown %s, %u, %u failed. Error "
-				"= %s.\n", fsp_str_dbg(fsp),
-				(unsigned int)user,
-				(unsigned int)grp,
-				nt_errstr(status)));
-			return status;
-		}
-
-		/*
-		 * Recheck the current state of the file, which may have changed.
-		 * (suid/sgid bits, for instance)
-		 */
-
-		status = vfs_stat_fsp(fsp);
-		if (!NT_STATUS_IS_OK(status)) {
-			return status;
-		}
-
-		/* Save the original element we check against. */
-		orig_mode = fsp->fsp_name->st.st_ex_mode;
-
-		/* If we successfully chowned, we know we must
-		 * be able to set the acl, so do it as root.
-		 */
-		set_acl_as_root = true;
+	status = chown_if_needed(fsp, security_info_sent, psd, &set_acl_as_root);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
 	}
 
 	create_file_sids(&fsp->fsp_name->st, &file_owner_sid, &file_grp_sid);
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 91bce9c7203..9335ae476f7 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -819,13 +819,14 @@ uint32_t map_canon_ace_perms(int snum,
                                 enum security_ace_type *pacl_type,
                                 mode_t perms,
                                 bool directory_ace);
-NTSTATUS unpack_nt_owners(connection_struct *conn, uid_t *puser, gid_t *pgrp, uint32_t security_info_sent, const struct security_descriptor *psd);
 bool current_user_in_group(connection_struct *conn, gid_t gid);
 SMB_ACL_T free_empty_sys_acl(connection_struct *conn, SMB_ACL_T the_acl);
 NTSTATUS posix_fget_nt_acl(struct files_struct *fsp, uint32_t security_info,
 			   TALLOC_CTX *mem_ctx,
 			   struct security_descriptor **ppdesc);
-NTSTATUS try_chown(files_struct *fsp, uid_t uid, gid_t gid);
+NTSTATUS chown_if_needed(files_struct *fsp, uint32_t security_info_sent,
+			 const struct security_descriptor *psd,
+			 bool *did_chown);
 NTSTATUS set_nt_acl(files_struct *fsp, uint32_t security_info_sent, const struct security_descriptor *psd);
 int get_acl_group_bits(connection_struct *conn,
 		       struct files_struct *fsp,


-- 
Samba Shared Repository



More information about the samba-cvs mailing list