[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Fri Oct 28 15:05:02 MDT 2011


The branch, master has been updated
       via  f30f71c Fix bug #8548 - winbind_samlogon_retry_loop ignores logon_parameters flags.
       via  8c6ff21 The xcopy test is used in unusual ways (via a different uid). Ensure we can cope with this.
       via  3bd6513 Remove the order dependency in parent_override_delete(), just check for & not ==.
       via  80c3aa7 The xcopy test requires "dos filemode=yes" as it opens with WRITE_OWNER.
       via  30a5996 Remove the mkdir and open functions from the ACL modules - main code paths now handle this.
       via  8a65e2c Remove unused "struct security_descriptor" parameter from check_parent_access()
       via  ea195b6 Finally do all the open checks inside open_file(). Checks inside vfs_acl_common can now be removed.
       via  8a3070a Simplify smbd_check_open_rights() and move all the special casing inside it.
       via  18df3ae Move parent_override_delete() to before I need to use it.
       via  1619de3 Make smbd_check_open_rights() static.
      from  151bb29 s3-net: Make sure to always re-use the "good" dc for the DNS updates as well.

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


- Log -----------------------------------------------------------------
commit f30f71c14a0b89dea296910ac9b92d3ae4016613
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Oct 28 12:29:54 2011 -0700

    Fix bug #8548 - winbind_samlogon_retry_loop ignores logon_parameters flags.
    
    Fix confirmed by reporter.
    
    Autobuild-User: Jeremy Allison <jra at samba.org>
    Autobuild-Date: Fri Oct 28 23:04:47 CEST 2011 on sn-devel-104

commit 8c6ff21782b141571dde64e80cc42540e9177a23
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Oct 28 12:15:51 2011 -0700

    The xcopy test is used in unusual ways (via a different uid). Ensure we can cope with this.

commit 3bd6513884f1f02fe5638a424bcb1948f0921853
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Oct 27 16:48:13 2011 -0700

    Remove the order dependency in parent_override_delete(), just check for & not ==.

commit 80c3aa7d2991302a2280dbfe6df14040347fdc52
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Oct 27 16:41:18 2011 -0700

    The xcopy test requires "dos filemode=yes" as it opens with WRITE_OWNER.

commit 30a599684a0a8c1f9af2d5b8adf8302172b49ae3
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Oct 26 16:02:40 2011 -0700

    Remove the mkdir and open functions from the ACL modules - main code paths now handle this.

commit 8a65e2c747c17be023d8c1285e0c8b2394fd4354
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Oct 26 15:30:00 2011 -0700

    Remove unused "struct security_descriptor" parameter from check_parent_access()

commit ea195b6cd2152a7f09847dba9c0c2288cc9a862d
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Oct 26 15:03:28 2011 -0700

    Finally do all the open checks inside open_file(). Checks inside
    vfs_acl_common can now be removed.

commit 8a3070a7c9fe3fad35103435c5c74188866057eb
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Oct 26 14:58:32 2011 -0700

    Simplify smbd_check_open_rights() and move all the special casing inside it.

commit 18df3aedb9dc0b7af0cc4046efb23026708f5d71
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Oct 26 14:47:52 2011 -0700

    Move parent_override_delete() to before I need to use it.

commit 1619de30805e57adc8bf063a9ccf6f5ba245bc5a
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Oct 26 14:06:41 2011 -0700

    Make smbd_check_open_rights() static.

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

Summary of changes:
 selftest/target/Samba3.pm        |    6 +
 selftest/target/Samba4.pm        |    7 +
 source3/modules/vfs_acl_common.c |  140 +--------------------
 source3/modules/vfs_acl_tdb.c    |    2 -
 source3/modules/vfs_acl_xattr.c  |    2 -
 source3/smbd/globals.h           |    4 -
 source3/smbd/open.c              |  251 +++++++++++++++++++-------------------
 source3/winbindd/winbindd_pam.c  |    4 +-
 source4/selftest/tests.py        |   18 ++--
 9 files changed, 153 insertions(+), 281 deletions(-)


Changeset truncated at 500 lines:

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 2f23ae3..3c0fbe9 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -916,6 +916,7 @@ sub provision($$$$$$$)
 	map readonly = no
 	store dos attributes = yes
 	create mask = 755
+	dos filemode = yes
 	vfs objects = $vfs_modulesdir_abs/xattr_tdb.so $vfs_modulesdir_abs/streams_depot.so
 
 	printing = vlp
@@ -1002,6 +1003,11 @@ sub provision($$$$$$$)
 	copy = print1
 [lp]
 	copy = print1
+[xcopy_share]
+	path = $shrdir
+	comment = smb username is [%U]
+	create mask = 777
+	force create mode = 777
 [print\$]
 	copy = tmp
 	";
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 6d67229..506bbee 100644
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -751,6 +751,13 @@ sub provision($$$$$$$$$)
 	posix:oplocktimeout = 3
 	posix:writetimeupdatedelay = 500000
 
+[xcopy_share]
+	path = $ctx->{tmpdir}
+	read only = no
+	posix:sharedelay = 10000
+	posix:oplocktimeout = 3
+	posix:writetimeupdatedelay = 500000
+
 [test1]
 	path = $ctx->{tmpdir}/test1
 	read only = no
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 1947a77..14ac6f7 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -600,124 +600,6 @@ static NTSTATUS check_parent_acl_common(vfs_handle_struct *handle,
 }
 
 /*********************************************************************
- Check ACL on open. For new files inherit from parent directory.
-*********************************************************************/
-
-static int open_acl_common(vfs_handle_struct *handle,
-			struct smb_filename *smb_fname,
-			files_struct *fsp,
-			int flags,
-			mode_t mode)
-{
-	uint32_t access_granted = 0;
-	struct security_descriptor *pdesc = NULL;
-	bool file_existed = true;
-	char *fname = NULL;
-	NTSTATUS status;
-
-	if (fsp->base_fsp) {
-		/* Stream open. Base filename open already did the ACL check. */
-		DEBUG(10,("open_acl_common: stream open on %s\n",
-			fsp_str_dbg(fsp) ));
-		return SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, flags, mode);
-	}
-
-	status = get_full_smb_filename(talloc_tos(), smb_fname,
-				       &fname);
-	if (!NT_STATUS_IS_OK(status)) {
-		goto err;
-	}
-
-	status = get_nt_acl_internal(handle,
-				NULL,
-				fname,
-				(SECINFO_OWNER |
-				 SECINFO_GROUP |
-				 SECINFO_DACL),
-				&pdesc);
-        if (NT_STATUS_IS_OK(status)) {
-		/* See if we can access it. */
-		status = smb1_file_se_access_check(handle->conn,
-					pdesc,
-					get_current_nttok(handle->conn),
-					fsp->access_mask,
-					&access_granted);
-		if (!NT_STATUS_IS_OK(status)) {
-			DEBUG(10,("open_acl_xattr: %s open "
-				"refused with error %s\n",
-				fsp_str_dbg(fsp),
-				nt_errstr(status) ));
-			goto err;
-		}
-        } else if (NT_STATUS_EQUAL(status,NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
-		file_existed = false;
-		/*
-		 * If O_CREAT is true then we're trying to create a file.
-		 * Check the parent directory ACL will allow this.
-		 */
-		if (flags & O_CREAT) {
-			struct security_descriptor *parent_desc = NULL;
-			struct security_descriptor **pp_psd = NULL;
-
-			status = check_parent_acl_common(handle, fname,
-					SEC_DIR_ADD_FILE, &parent_desc);
-			if (!NT_STATUS_IS_OK(status)) {
-				goto err;
-			}
-
-			/* Cache the parent security descriptor for
-			 * later use. */
-
-			pp_psd = (struct security_descriptor **)
-				VFS_ADD_FSP_EXTENSION(handle,
-					fsp,
-					struct security_descriptor *,
-					NULL);
-			if (!pp_psd) {
-				status = NT_STATUS_NO_MEMORY;
-				goto err;
-			}
-
-			*pp_psd = parent_desc;
-			status = NT_STATUS_OK;
-		}
-	}
-
-	DEBUG(10,("open_acl_xattr: get_nt_acl_attr_internal for "
-		"%s returned %s\n",
-		fsp_str_dbg(fsp),
-		nt_errstr(status) ));
-
-	fsp->fh->fd = SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, flags, mode);
-	return fsp->fh->fd;
-
-  err:
-
-	errno = map_errno_from_nt_status(status);
-	return -1;
-}
-
-static int mkdir_acl_common(vfs_handle_struct *handle, const char *path, mode_t mode)
-{
-	int ret;
-	NTSTATUS status;
-	SMB_STRUCT_STAT sbuf;
-
-	ret = vfs_stat_smb_fname(handle->conn, path, &sbuf);
-	if (ret == -1 && errno == ENOENT) {
-		/* We're creating a new directory. */
-		status = check_parent_acl_common(handle, path,
-				SEC_DIR_ADD_SUBDIR, NULL);
-		if (!NT_STATUS_IS_OK(status)) {
-			errno = map_errno_from_nt_status(status);
-			return -1;
-		}
-	}
-
-	return SMB_VFS_NEXT_MKDIR(handle, path, mode);
-}
-
-/*********************************************************************
  Fetch a security descriptor given an fsp.
 *********************************************************************/
 
@@ -965,7 +847,6 @@ static NTSTATUS create_file_acl_common(struct vfs_handle_struct *handle,
 	files_struct *fsp = NULL;
 	int info;
 	struct security_descriptor *parent_sd = NULL;
-	struct security_descriptor **pp_parent_sd = NULL;
 
 	status = SMB_VFS_NEXT_CREATE_FILE(handle,
 					req,
@@ -1010,18 +891,11 @@ static NTSTATUS create_file_acl_common(struct vfs_handle_struct *handle,
 		goto out;
 	}
 
-	/* See if we have a cached parent sd, if so, use it. */
-	pp_parent_sd = (struct security_descriptor **)VFS_FETCH_FSP_EXTENSION(handle, fsp);
-	if (!pp_parent_sd) {
-		/* Must be a directory, fetch again (sigh). */
-		status = get_parent_acl_common(handle,
-				fsp->fsp_name->base_name,
-				&parent_sd);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto out;
-		}
-	} else {
-		parent_sd = *pp_parent_sd;
+	status = get_parent_acl_common(handle,
+			fsp->fsp_name->base_name,
+			&parent_sd);
+	if (!NT_STATUS_IS_OK(status)) {
+		goto out;
 	}
 
 	if (!parent_sd) {
@@ -1040,9 +914,7 @@ static NTSTATUS create_file_acl_common(struct vfs_handle_struct *handle,
 
   out:
 
-	if (fsp) {
-		VFS_REMOVE_FSP_EXTENSION(handle, fsp);
-	}
+	TALLOC_FREE(parent_sd);
 
 	if (NT_STATUS_IS_OK(status) && pinfo) {
 		*pinfo = info;
diff --git a/source3/modules/vfs_acl_tdb.c b/source3/modules/vfs_acl_tdb.c
index 778e837..a4869c0 100644
--- a/source3/modules/vfs_acl_tdb.c
+++ b/source3/modules/vfs_acl_tdb.c
@@ -401,9 +401,7 @@ static struct vfs_fn_pointers vfs_acl_tdb_fns = {
 	.connect_fn = connect_acl_tdb,
 	.disconnect = disconnect_acl_tdb,
 	.opendir = opendir_acl_common,
-	.mkdir = mkdir_acl_common,
 	.rmdir = rmdir_acl_tdb,
-	.open_fn = open_acl_common,
 	.create_file = create_file_acl_common,
 	.unlink = unlink_acl_tdb,
 	.chmod = chmod_acl_module_common,
diff --git a/source3/modules/vfs_acl_xattr.c b/source3/modules/vfs_acl_xattr.c
index b522b33..473c2fc 100644
--- a/source3/modules/vfs_acl_xattr.c
+++ b/source3/modules/vfs_acl_xattr.c
@@ -202,9 +202,7 @@ static int connect_acl_xattr(struct vfs_handle_struct *handle,
 static struct vfs_fn_pointers vfs_acl_xattr_fns = {
 	.connect_fn = connect_acl_xattr,
 	.opendir = opendir_acl_common,
-	.mkdir = mkdir_acl_common,
 	.rmdir = rmdir_acl_common,
-	.open_fn = open_acl_common,
 	.create_file = create_file_acl_common,
 	.unlink = unlink_acl_common,
 	.chmod = chmod_acl_module_common,
diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index 472aeee..14337e0 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -221,10 +221,6 @@ NTSTATUS smbd_calculate_access_mask(connection_struct *conn,
 				    bool file_existed,
 				    uint32_t access_mask,
 				    uint32_t *access_mask_out);
-NTSTATUS smbd_check_open_rights(struct connection_struct *conn,
-				const struct smb_filename *smb_fname,
-				uint32_t access_mask,
-				uint32_t *access_granted);
 
 void smbd_notify_cancel_by_smbreq(const struct smb_request *smbreq);
 
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 6ad85b7..42edddc 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -65,33 +65,54 @@ NTSTATUS smb1_file_se_access_check(struct connection_struct *conn,
 }
 
 /****************************************************************************
+ If the requester wanted DELETE_ACCESS and was rejected because
+ the file ACL didn't include DELETE_ACCESS, see if the parent ACL
+ ovverrides this.
+****************************************************************************/
+
+static bool parent_override_delete(connection_struct *conn,
+					struct smb_filename *smb_fname,
+					uint32_t access_mask,
+					uint32_t rejected_mask)
+{
+	if ((access_mask & DELETE_ACCESS) &&
+		    (rejected_mask & DELETE_ACCESS) &&
+		    can_delete_file_in_directory(conn, smb_fname)) {
+		return true;
+	}
+	return false;
+}
+
+/****************************************************************************
  Check if we have open rights.
 ****************************************************************************/
 
-NTSTATUS smbd_check_open_rights(struct connection_struct *conn,
-				const struct smb_filename *smb_fname,
-				uint32_t access_mask,
-				uint32_t *access_granted)
+static NTSTATUS smbd_check_open_rights(struct connection_struct *conn,
+				struct smb_filename *smb_fname,
+				uint32_t access_mask)
 {
 	/* Check if we have rights to open. */
 	NTSTATUS status;
 	struct security_descriptor *sd = NULL;
 	uint32_t rejected_share_access;
+	uint32_t rejected_mask = 0;
 
 	rejected_share_access = access_mask & ~(conn->share_access);
 
 	if (rejected_share_access) {
-		*access_granted = rejected_share_access;
+		DEBUG(10, ("smbd_check_open_rights: rejected share access 0x%x "
+			"on %s (0x%x)\n",
+			(unsigned int)access_mask,
+			smb_fname_str_dbg(smb_fname),
+			(unsigned int)rejected_share_access ));
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
 	if ((access_mask & DELETE_ACCESS) && !lp_acl_check_permissions(SNUM(conn))) {
-		*access_granted = access_mask;
-
 		DEBUG(10,("smbd_check_open_rights: not checking ACL "
 			"on DELETE_ACCESS on file %s. Granting 0x%x\n",
 			smb_fname_str_dbg(smb_fname),
-			(unsigned int)*access_granted ));
+			(unsigned int)access_mask ));
 		return NT_STATUS_OK;
 	}
 
@@ -112,13 +133,13 @@ NTSTATUS smbd_check_open_rights(struct connection_struct *conn,
 				sd,
 				get_current_nttok(conn),
 				access_mask,
-				access_granted);
+				&rejected_mask);
 
 	DEBUG(10,("smbd_check_open_rights: file %s requesting "
 		"0x%x returning 0x%x (%s)\n",
 		smb_fname_str_dbg(smb_fname),
 		(unsigned int)access_mask,
-		(unsigned int)*access_granted,
+		(unsigned int)rejected_mask,
 		nt_errstr(status) ));
 
 	if (!NT_STATUS_IS_OK(status)) {
@@ -131,14 +152,59 @@ NTSTATUS smbd_check_open_rights(struct connection_struct *conn,
 
 	TALLOC_FREE(sd);
 
-	return status;
+	if (NT_STATUS_IS_OK(status) ||
+			!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
+		return status;
+	}
+
+	/* Here we know status == NT_STATUS_ACCESS_DENIED. */
+	if ((access_mask & FILE_WRITE_ATTRIBUTES) &&
+			(rejected_mask & FILE_WRITE_ATTRIBUTES) &&
+			(lp_map_readonly(SNUM(conn)) ||
+			lp_map_archive(SNUM(conn)) ||
+			lp_map_hidden(SNUM(conn)) ||
+			lp_map_system(SNUM(conn)))) {
+		rejected_mask &= ~FILE_WRITE_ATTRIBUTES;
+
+		DEBUG(10,("smbd_check_open_rights: "
+			"overrode "
+			"FILE_WRITE_ATTRIBUTES "
+			"on file %s\n",
+			smb_fname_str_dbg(smb_fname)));
+	}
+
+	if (parent_override_delete(conn,
+				smb_fname,
+				access_mask,
+				rejected_mask)) {
+		/* Were we trying to do an open
+		 * for delete and didn't get DELETE
+		 * access (only) ? Check if the
+		 * directory allows DELETE_CHILD.
+		 * See here:
+		 * http://blogs.msdn.com/oldnewthing/archive/2004/06/04/148426.aspx
+		 * for details. */
+
+		rejected_mask &= ~DELETE_ACCESS;
+
+		DEBUG(10,("smbd_check_open_rights: "
+			"overrode "
+			"DELETE_ACCESS on "
+			"file %s\n",
+			smb_fname_str_dbg(smb_fname)));
+	}
+
+	if (rejected_mask != 0) {
+		return NT_STATUS_ACCESS_DENIED;
+	} else {
+		return NT_STATUS_OK;
+	}
 }
 
 static NTSTATUS check_parent_access(struct connection_struct *conn,
 				struct smb_filename *smb_fname,
 				uint32_t access_mask,
-				char **pp_parent_dir,
-				struct security_descriptor **pp_parent_sd)
+				char **pp_parent_dir)
 {
 	NTSTATUS status;
 	char *parent_dir = NULL;
@@ -185,32 +251,10 @@ static NTSTATUS check_parent_access(struct connection_struct *conn,
 	if (pp_parent_dir) {
 		*pp_parent_dir = parent_dir;
 	}
-	if (pp_parent_sd) {
-		*pp_parent_sd = parent_sd;
-	}
 	return NT_STATUS_OK;
 }
 
 /****************************************************************************
- If the requester wanted DELETE_ACCESS and was only rejected because
- the file ACL didn't include DELETE_ACCESS, see if the parent ACL
- ovverrides this.
-****************************************************************************/
-
-static bool parent_override_delete(connection_struct *conn,
-					struct smb_filename *smb_fname,
-					uint32_t access_mask,
-					uint32_t rejected_mask)
-{
-	if ((access_mask & DELETE_ACCESS) &&
-		    (rejected_mask == DELETE_ACCESS) &&
-		    can_delete_file_in_directory(conn, smb_fname)) {
-		return true;
-	}
-	return false;
-}
-
-/****************************************************************************
  fd support routines - attempt to do a dos_open.
 ****************************************************************************/
 
@@ -565,6 +609,35 @@ static NTSTATUS open_file(files_struct *fsp,
 			return NT_STATUS_OBJECT_NAME_INVALID;
 		}
 
+		/* Can we access this file ? */
+		if (!fsp->base_fsp) {
+			/* Only do this check on non-stream open. */
+			if (file_existed) {
+				status = smbd_check_open_rights(conn,
+						smb_fname,
+						access_mask);
+			} else if (local_flags & O_CREAT){
+				status = check_parent_access(conn,
+						smb_fname,
+						SEC_DIR_ADD_FILE,
+						NULL);
+			} else {
+				/* File didn't exist and no O_CREAT. */
+				return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+			}
+			if (!NT_STATUS_IS_OK(status)) {
+				DEBUG(10,("open_file: "
+					"%s on file "
+					"%s returned %s\n",
+					file_existed ?
+						"smbd_check_open_rights" :
+						"check_parent_access",
+					smb_fname_str_dbg(smb_fname),
+					nt_errstr(status) ));
+				return status;
+			}
+		}
+
 		/* Actually do the open */
 		status = fd_open(conn, fsp, local_flags, unx_mode);
 		if (!NT_STATUS_IS_OK(status)) {
@@ -579,8 +652,6 @@ static NTSTATUS open_file(files_struct *fsp,
 		}
 
 	} else {
-		uint32_t access_granted = 0;
-
 		fsp->fh->fd = -1; /* What we used to call a stat open. */
 		if (!file_existed) {
 			/* File must exist for a stat open. */
@@ -589,79 +660,26 @@ static NTSTATUS open_file(files_struct *fsp,
 
 		status = smbd_check_open_rights(conn,
 				smb_fname,
-				access_mask,
-				&access_granted);
-		if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
-			/*
-			 * On NT_STATUS_ACCESS_DENIED, access_granted
-			 * contains the denied bits.
-			 */
+				access_mask);
 
-			if ((access_mask & FILE_WRITE_ATTRIBUTES) &&
-					(access_granted & FILE_WRITE_ATTRIBUTES) &&
-					(lp_map_readonly(SNUM(conn)) ||
-					 lp_map_archive(SNUM(conn)) ||
-					 lp_map_hidden(SNUM(conn)) ||
-					 lp_map_system(SNUM(conn)))) {


-- 
Samba Shared Repository


More information about the samba-cvs mailing list