Questions about smbd option "strict rename"

Ralph Boehme rb at sernet.de
Sun Nov 29 15:08:53 UTC 2015



On Sun, Nov 29, 2015 at 10:59:16AM +0100, Stefan Metzmacher wrote:
> Hi Ralph,
> 
> >> Yeah, that looks much closer !
> >>
> >>> I'm not fully happy with the flag names yet, I think we should prefix
> >>> them with VFS_ so they won't collide with flags we may add later on
> >>> for SMB2/3 UNIX extensions.
> >>
> >> Hmmm. Don't like VFS_ as they're not really VFS. How about
> >> FSP_ prefix as they're more to do with fsp open handles ?
> > 
> > perfect, thanks!
> > 
> > Updated patchset attached. I've added a test that verifies both
> > possibilities: deny rename if POSIX rename cap is not enabled, permit
> > rename if POSIX rename cap is enabled via AAPL/vfs_fruit.
> 
> Can we make change from bool posix_open to uint8_t posix_flags first?
> Maybe with a #define posix_open posix_flags.

excellent idea! Something like the attached patch?

> Can we also have a test with 2 connections, one with aapl and one
> without and test the interaction between both against an apple
> server.

Apple's server doesn't care and allows this even without AAPL. I had
verified this previously with smbclient.

Only allowing this for AAPL (or UNIX) clients would be our way of
preserving protocol conformance.

Thanks!
-Ralph

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de
-------------- next part --------------
From bbcc4ccc185729b7629d7a196934fa922351f633 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 27 Nov 2015 18:29:55 +0100
Subject: [PATCH 1/6] s3:smbd: convert file_struct.posix_open to a bitmap with
 flags

This is in preperation of a more fine grained control of POSIX behaviour
in the SMB and VFS layers.

Inititally we use an uint8_t for the flags bitmap and add a define
posix_flags as posix_open in order to avoid breaking the VFS ABI.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11065

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/include/vfs.h            | 10 +++++++++-
 source3/locking/locking.c        |  3 ++-
 source3/modules/vfs_acl_common.c |  4 ++--
 source3/param/loadparm.c         |  3 ++-
 source3/smbd/close.c             |  6 ++++--
 source3/smbd/fileio.c            |  8 ++++----
 source3/smbd/open.c              | 10 ++++++----
 source3/smbd/reply.c             |  2 +-
 source3/smbd/smb2_close.c        |  2 +-
 source3/smbd/vfs.c               |  4 ++--
 10 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 9945375..56a9049 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -218,6 +218,9 @@ struct fsp_lease {
 	struct smb2_lease lease;
 };
 
+/* VFS ABI stability hack */
+#define posix_flags posix_open
+
 typedef struct files_struct {
 	struct files_struct *next, *prev;
 	uint64_t fnum;
@@ -255,7 +258,7 @@ typedef struct files_struct {
 	bool aio_write_behind;
 	bool initial_delete_on_close; /* Only set at NTCreateX if file was created. */
 	bool delete_on_close;
-	bool posix_open;
+	uint8_t posix_flags;
 	bool is_sparse;
 	bool backup_intent; /* Handle was successfully opened with backup intent
 				and opener has privilege to do so. */
@@ -297,6 +300,11 @@ typedef struct files_struct {
 	struct tevent_req *deferred_close;
 } files_struct;
 
+#define FSP_POSIX_FLAGS_OPEN		0x01
+
+#define FSP_POSIX_FLAGS_ALL			\
+	(FSP_POSIX_FLAGS_OPEN)
+
 struct vuid_cache_entry {
 	struct auth_session_info *session_info;
 	uint64_t vuid; /* SMB2 compat */
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 7b8bc84..5a97460 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -847,7 +847,8 @@ bool set_share_mode(struct share_mode_lock *lck, struct files_struct *fsp,
 	e->id = fsp->file_id;
 	e->share_file_id = fsp->fh->gen_id;
 	e->uid = (uint32_t)uid;
-	e->flags = fsp->posix_open ? SHARE_MODE_FLAG_POSIX_OPEN : 0;
+	e->flags = (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) ?
+		SHARE_MODE_FLAG_POSIX_OPEN : 0;
 	e->name_hash = fsp->name_hash;
 
 	return true;
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 625e7cb..f73e80c 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -1089,7 +1089,7 @@ static int chmod_acl_module_common(struct vfs_handle_struct *handle,
 static int fchmod_acl_module_common(struct vfs_handle_struct *handle,
 			struct files_struct *fsp, mode_t mode)
 {
-	if (fsp->posix_open) {
+	if (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) {
 		/* Only allow this on POSIX opens. */
 		return SMB_VFS_NEXT_FCHMOD(handle, fsp, mode);
 	}
@@ -1109,7 +1109,7 @@ static int chmod_acl_acl_module_common(struct vfs_handle_struct *handle,
 static int fchmod_acl_acl_module_common(struct vfs_handle_struct *handle,
 			struct files_struct *fsp, mode_t mode)
 {
-	if (fsp->posix_open) {
+	if (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) {
 		/* Only allow this on POSIX opens. */
 		return SMB_VFS_NEXT_FCHMOD_ACL(handle, fsp, mode);
 	}
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 9f40e65..d5c4f32 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -4316,7 +4316,8 @@ enum brl_flavour lp_posix_cifsu_locktype(files_struct *fsp)
 	if (posix_default_lock_was_set) {
 		return posix_cifsx_locktype;
 	} else {
-		return fsp->posix_open ? POSIX_LOCK : WINDOWS_LOCK;
+		return (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) ?
+			POSIX_LOCK : WINDOWS_LOCK;
 	}
 }
 
diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index 0e75bf0..1cb5460 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -327,7 +327,7 @@ static NTSTATUS close_remove_share_mode(files_struct *fsp,
 			if (e->name_hash != fsp->name_hash) {
 				continue;
 			}
-			if (fsp->posix_open
+			if ((fsp->posix_flags & FSP_POSIX_FLAGS_OPEN)
 			    && (e->flags & SHARE_MODE_FLAG_POSIX_OPEN)) {
 				continue;
 			}
@@ -1103,7 +1103,9 @@ static NTSTATUS close_directory(struct smb_request *req, files_struct *fsp,
 			struct share_mode_entry *e = &lck->data->share_modes[i];
 			if (is_valid_share_mode_entry(e) &&
 					e->name_hash == fsp->name_hash) {
-				if (fsp->posix_open && (e->flags & SHARE_MODE_FLAG_POSIX_OPEN)) {
+				if ((fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) &&
+				    (e->flags & SHARE_MODE_FLAG_POSIX_OPEN))
+				{
 					continue;
 				}
 				if (serverid_equal(&self, &e->pid) &&
diff --git a/source3/smbd/fileio.c b/source3/smbd/fileio.c
index 2fe0432..156c139 100644
--- a/source3/smbd/fileio.c
+++ b/source3/smbd/fileio.c
@@ -193,7 +193,7 @@ void trigger_write_time_update(struct files_struct *fsp)
 {
 	int delay;
 
-	if (fsp->posix_open) {
+	if (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) {
 		/* Don't use delayed writes on POSIX files. */
 		return;
 	}
@@ -238,7 +238,7 @@ void trigger_write_time_update_immediate(struct files_struct *fsp)
 {
 	struct smb_file_time ft;
 
-	if (fsp->posix_open) {
+	if (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) {
 		/* Don't use delayed writes on POSIX files. */
 		return;
 	}
@@ -284,7 +284,7 @@ void mark_file_modified(files_struct *fsp)
 	}
 	trigger_write_time_update(fsp);
 
-	if (fsp->posix_open) {
+	if (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) {
 		return;
 	}
 	if (!(lp_store_dos_attributes(SNUM(fsp->conn)) ||
@@ -1047,7 +1047,7 @@ NTSTATUS sync_file(connection_struct *conn, files_struct *fsp, bool write_throug
 int fsp_stat(files_struct *fsp)
 {
 	if (fsp->fh->fd == -1) {
-		if (fsp->posix_open) {
+		if (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) {
 			return SMB_VFS_LSTAT(fsp->conn, fsp->fsp_name);
 		} else {
 			return SMB_VFS_STAT(fsp->conn, fsp->fsp_name);
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index c34742e..9cd415b 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -362,7 +362,7 @@ NTSTATUS fd_open(struct connection_struct *conn,
 	 * client should be doing this.
 	 */
 
-	if (fsp->posix_open || !lp_follow_symlinks(SNUM(conn))) {
+	if ((fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) || !lp_follow_symlinks(SNUM(conn))) {
 		flags |= O_NOFOLLOW;
 	}
 #endif
@@ -945,7 +945,7 @@ static NTSTATUS open_file(files_struct *fsp,
 				access_mask);
 
 		if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) &&
-				fsp->posix_open &&
+				(fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) &&
 				S_ISLNK(smb_fname->st.st_ex_mode)) {
 			/* This is a POSIX stat open for delete
 			 * or rename on a symlink that points
@@ -2703,7 +2703,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	fsp->access_mask = open_access_mask; /* We change this to the
 					      * requested access_mask after
 					      * the open is done. */
-	fsp->posix_open = posix_open;
+	fsp->posix_flags |= posix_open ? FSP_POSIX_FLAGS_OPEN : 0;
 
 	if (timeval_is_zero(&request_time)) {
 		request_time = fsp->open_time;
@@ -3569,7 +3569,9 @@ static NTSTATUS open_directory(connection_struct *conn,
 	fsp->oplock_type = NO_OPLOCK;
 	fsp->sent_oplock_break = NO_BREAK_SENT;
 	fsp->is_directory = True;
-	fsp->posix_open = (file_attributes & FILE_FLAG_POSIX_SEMANTICS) ? True : False;
+	if (file_attributes & FILE_FLAG_POSIX_SEMANTICS) {
+		fsp->posix_flags |= FSP_POSIX_FLAGS_ALL;
+	}
 	status = fsp_set_smb_fname(fsp, smb_dname);
 	if (!NT_STATUS_IS_OK(status)) {
 		file_free(req, fsp);
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index efef613..81d9c4d 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -2669,7 +2669,7 @@ static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp,
 	}
 
 	if (S_ISDIR(fsp->fsp_name->st.st_ex_mode)) {
-		if (fsp->posix_open) {
+		if (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) {
 			return NT_STATUS_OK;
 		}
 
diff --git a/source3/smbd/smb2_close.c b/source3/smbd/smb2_close.c
index ed53e1b..367c9fa 100644
--- a/source3/smbd/smb2_close.c
+++ b/source3/smbd/smb2_close.c
@@ -231,7 +231,7 @@ static NTSTATUS smbd_smb2_close(struct smbd_smb2_request *req,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	posix_open = fsp->posix_open;
+	posix_open = (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN);
 	smb_fname = cp_smb_filename(talloc_tos(), fsp->fsp_name);
 	if (smb_fname == NULL) {
 		return NT_STATUS_NO_MEMORY;
diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index 9f3ba6d..0c272a7 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -1309,7 +1309,7 @@ NTSTATUS vfs_stat_fsp(files_struct *fsp)
 	int ret;
 
 	if(fsp->fh->fd == -1) {
-		if (fsp->posix_open) {
+		if (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) {
 			ret = SMB_VFS_LSTAT(fsp->conn, fsp->fsp_name);
 		} else {
 			ret = SMB_VFS_STAT(fsp->conn, fsp->fsp_name);
@@ -1939,7 +1939,7 @@ NTSTATUS vfs_chown_fsp(files_struct *fsp, uid_t uid, gid_t gid)
                 path = fsp->fsp_name->base_name;
         }
 
-	if (fsp->posix_open || as_root) {
+	if ((fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) || as_root) {
 		ret = SMB_VFS_LCHOWN(fsp->conn,
 			path,
 			uid, gid);
-- 
2.5.0


From 562715b7dc12eebfac35a367f887b6f2bd267544 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 22 Jan 2015 10:00:15 +0100
Subject: [PATCH 2/6] s3:smbd: file_struct: seperate POSIX directory rename cap
 from POSIX open

We need more fine grained control over which POSIX semantics we'd like
to enable per file handle. Currently POSIX_FLAGS_OPEN is a kitchensink
for all kinds of stuff like:

- POSIX unlink
- POSIX byte-range locks
- POSIX rename
- delayed writetime update
- more...

For CIFS UNIX extensions we use POSIX_FLAGS_ALL so semantics are
preserved. OS X clients will enable POSIX rename via AAPL.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11065

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/include/vfs.h | 4 +++-
 source3/smbd/reply.c  | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 56a9049..d839be4 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -301,9 +301,11 @@ typedef struct files_struct {
 } files_struct;
 
 #define FSP_POSIX_FLAGS_OPEN		0x01
+#define FSP_POSIX_FLAGS_RENAME		0x02
 
 #define FSP_POSIX_FLAGS_ALL			\
-	(FSP_POSIX_FLAGS_OPEN)
+	(FSP_POSIX_FLAGS_OPEN |			\
+	 FSP_POSIX_FLAGS_RENAME)
 
 struct vuid_cache_entry {
 	struct auth_session_info *session_info;
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 81d9c4d..6dae63c 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -2669,7 +2669,7 @@ static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp,
 	}
 
 	if (S_ISDIR(fsp->fsp_name->st.st_ex_mode)) {
-		if (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) {
+		if (fsp->posix_flags & FSP_POSIX_FLAGS_RENAME) {
 			return NT_STATUS_OK;
 		}
 
-- 
2.5.0


From 2d88fabb243cf7d5449fdb24dc5d2abf84400de0 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 25 Nov 2015 09:12:55 +0100
Subject: [PATCH 3/6] vfs_fruit: add a flag that tracks whether use of AAPL was
 negotiated

Add a flag that tracks whether use of AAPL was negotiated, ensuring we
don't enable AAPL features for clients that didn't negotiate it.

Torture test that need AAPL must call the new function enable_aapl().

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11065

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index bd71ff1..79c6651 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -125,7 +125,8 @@ struct fruit_config_data {
 	enum fruit_meta meta;
 	enum fruit_locking locking;
 	enum fruit_encoding encoding;
-	bool use_aapl;
+	bool use_aapl;		/* config from smb.conf */
+	bool nego_aapl;		/* client negotiated AAPL */
 	bool use_copyfile;
 	bool readdir_attr_enabled;
 	bool unix_info_enabled;
@@ -1926,6 +1927,9 @@ static NTSTATUS check_aapl(vfs_handle_struct *handle,
 				      out_context_blobs,
 				      SMB2_CREATE_TAG_AAPL,
 				      blob);
+	if (NT_STATUS_IS_OK(status)) {
+		config->nego_aapl = true;
+	}
 
 	return status;
 }
@@ -3386,16 +3390,20 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle,
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
+
 	fsp = *result;
 
-	if (config->copyfile_enabled) {
-		/*
-		 * Set a flag in the fsp. Gets used in copychunk to
-		 * check whether the special Apple copyfile semantics
-		 * for copychunk should be allowed in a copychunk
-		 * request with a count of 0.
-		 */
-		fsp->aapl_copyfile_supported = true;
+	if (config->nego_aapl) {
+		if (config->copyfile_enabled) {
+			/*
+			 * Set a flag in the fsp. Gets used in
+			 * copychunk to check whether the special
+			 * Apple copyfile semantics for copychunk
+			 * should be allowed in a copychunk request
+			 * with a count of 0.
+			 */
+			fsp->aapl_copyfile_supported = true;
+		}
 	}
 
 	/*
-- 
2.5.0


From 74d1ee86d93bfc726b1d294182ca87813bf572e7 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 22 Jan 2015 10:07:56 +0100
Subject: [PATCH 4/6] vfs_fruit: enable POSIX directory rename semantics

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11065

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 79c6651..e9a54b0 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -3404,6 +3404,13 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle,
 			 */
 			fsp->aapl_copyfile_supported = true;
 		}
+
+		if (fsp->is_directory) {
+			/*
+			 * Enable POSIX directory rename behaviour
+			 */
+			fsp->posix_flags |= FSP_POSIX_FLAGS_RENAME;
+		}
 	}
 
 	/*
-- 
2.5.0


From b4399e4b8721b83b0ce06405248cce15a2155a1d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 28 Nov 2015 19:26:47 +0100
Subject: [PATCH 5/6] s4:torture:vfs_fruit: add a test for POSIX rename

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source4/torture/vfs/fruit.c | 201 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 201 insertions(+)

diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
index 313635e..2488b7a 100644
--- a/source4/torture/vfs/fruit.c
+++ b/source4/torture/vfs/fruit.c
@@ -1188,6 +1188,73 @@ static bool torture_setup_file(TALLOC_CTX *mem_ctx, struct smb2_tree *tree,
 	return true;
 }
 
+static bool enable_aapl(struct torture_context *tctx,
+			struct smb2_tree *tree1)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
+	NTSTATUS status;
+	bool ret = true;
+	struct smb2_create io;
+	DATA_BLOB data;
+	struct smb2_create_blob *aapl = NULL;
+	uint32_t aapl_server_caps;
+	uint32_t expexted_scaps = (SMB2_CRTCTX_AAPL_UNIX_BASED |
+				   SMB2_CRTCTX_AAPL_SUPPORTS_READ_DIR_ATTR |
+				   SMB2_CRTCTX_AAPL_SUPPORTS_NFS_ACE |
+				   SMB2_CRTCTX_AAPL_SUPPORTS_OSX_COPYFILE);
+
+	ZERO_STRUCT(io);
+	io.in.desired_access     = SEC_FLAG_MAXIMUM_ALLOWED;
+	io.in.file_attributes    = FILE_ATTRIBUTE_DIRECTORY;
+	io.in.create_disposition = NTCREATEX_DISP_OPEN;
+	io.in.share_access = (NTCREATEX_SHARE_ACCESS_DELETE |
+			      NTCREATEX_SHARE_ACCESS_READ |
+			      NTCREATEX_SHARE_ACCESS_WRITE);
+	io.in.fname = "";
+
+	/*
+	 * Issuing an SMB2/CREATE with a suitably formed AAPL context,
+	 * controls behaviour of Apple's SMB2 extensions for the whole
+	 * session!
+	 */
+
+	data = data_blob_talloc(mem_ctx, NULL, 3 * sizeof(uint64_t));
+	SBVAL(data.data, 0, SMB2_CRTCTX_AAPL_SERVER_QUERY);
+	SBVAL(data.data, 8, (SMB2_CRTCTX_AAPL_SERVER_CAPS |
+			     SMB2_CRTCTX_AAPL_VOLUME_CAPS |
+			     SMB2_CRTCTX_AAPL_MODEL_INFO));
+	SBVAL(data.data, 16, (SMB2_CRTCTX_AAPL_SUPPORTS_READ_DIR_ATTR |
+			      SMB2_CRTCTX_AAPL_UNIX_BASED |
+			      SMB2_CRTCTX_AAPL_SUPPORTS_NFS_ACE));
+
+	status = smb2_create_blob_add(tctx, &io.in.blobs, "AAPL", data);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_create_blob_add");
+
+	status = smb2_create(tree1, tctx, &io);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_create");
+
+	status = smb2_util_close(tree1, io.out.file.handle);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_util_close");
+
+	/*
+	 * Now check returned AAPL context
+	 */
+	torture_comment(tctx, "Comparing returned AAPL capabilities\n");
+
+	aapl = smb2_create_blob_find(&io.out.blobs,
+				     SMB2_CREATE_TAG_AAPL);
+	torture_assert_goto(tctx, aapl != NULL, ret, done, "missing AAPL context");
+	torture_assert_goto(tctx, aapl->data.length == 50, ret, done, "bad AAPL size");
+
+	aapl_server_caps = BVAL(aapl->data.data, 16);
+	torture_assert_goto(tctx, aapl_server_caps == expexted_scaps,
+			    ret, done, "bad AAPL caps");
+
+done:
+	talloc_free(mem_ctx);
+	return ret;
+}
+
 static bool test_read_atalk_metadata(struct torture_context *tctx,
 				     struct smb2_tree *tree1,
 				     struct smb2_tree *tree2)
@@ -2548,6 +2615,139 @@ done:
 	return ret;
 }
 
+/* Renaming a directory with open file, should work for OS X AAPL clients */
+static bool test_rename_dir_openfile(struct torture_context *torture,
+				     struct smb2_tree *tree1,
+				     struct smb2_tree *tree2)
+{
+	bool ret = true;
+	NTSTATUS status;
+	union smb_open io;
+	union smb_close cl;
+	union smb_setfileinfo sinfo;
+	struct smb2_handle d1, h1;
+	const char *renamedir = BASEDIR "-new";
+
+	smb2_deltree(tree1, BASEDIR);
+	smb2_util_rmdir(tree1, BASEDIR);
+	smb2_deltree(tree1, renamedir);
+
+	ZERO_STRUCT(io.smb2);
+	io.generic.level = RAW_OPEN_SMB2;
+	io.smb2.in.create_flags = 0;
+	io.smb2.in.desired_access = 0x0017019f;
+	io.smb2.in.create_options = NTCREATEX_OPTIONS_DIRECTORY;
+	io.smb2.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY;
+	io.smb2.in.share_access = 0;
+	io.smb2.in.alloc_size = 0;
+	io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE;
+	io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+	io.smb2.in.security_flags = 0;
+	io.smb2.in.fname = BASEDIR;
+
+	status = smb2_create(tree1, torture, &(io.smb2));
+	torture_assert_ntstatus_ok(torture, status, "smb2_create dir");
+	d1 = io.smb2.out.file.handle;
+
+	ZERO_STRUCT(io.smb2);
+	io.generic.level = RAW_OPEN_SMB2;
+	io.smb2.in.create_flags = 0;
+	io.smb2.in.desired_access = 0x0017019f;
+	io.smb2.in.create_options = NTCREATEX_OPTIONS_NON_DIRECTORY_FILE;
+	io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+	io.smb2.in.share_access = 0;
+	io.smb2.in.alloc_size = 0;
+	io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE;
+	io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+	io.smb2.in.security_flags = 0;
+	io.smb2.in.fname = BASEDIR "\\file.txt";
+
+	status = smb2_create(tree1, torture, &(io.smb2));
+	torture_assert_ntstatus_ok(torture, status, "smb2_create file");
+	h1 = io.smb2.out.file.handle;
+
+	torture_comment(torture, "Renaming directory without AAPL, must fail\n");
+
+	ZERO_STRUCT(sinfo);
+	sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION;
+	sinfo.rename_information.in.file.handle = d1;
+	sinfo.rename_information.in.overwrite = 0;
+	sinfo.rename_information.in.root_fid = 0;
+	sinfo.rename_information.in.new_name = renamedir;
+	status = smb2_setinfo_file(tree1, &sinfo);
+	torture_assert_ntstatus_equal(torture, status, NT_STATUS_ACCESS_DENIED,
+				      "smb2_setinfo_file");
+
+	ZERO_STRUCT(cl.smb2);
+	cl.smb2.level = RAW_CLOSE_SMB2;
+	cl.smb2.in.file.handle = d1;
+	status = smb2_close(tree1, &(cl.smb2));
+	torture_assert_ntstatus_ok(torture, status, "smb2_close");
+	ZERO_STRUCT(d1);
+
+	torture_comment(torture, "Enabling AAPL\n");
+
+	ret = enable_aapl(torture, tree1);
+	torture_assert(torture, ret == true, "enable_aapl failed");
+
+	torture_comment(torture, "Renaming directory with AAPL\n");
+
+	ZERO_STRUCT(io.smb2);
+	io.generic.level = RAW_OPEN_SMB2;
+	io.smb2.in.desired_access = 0x0017019f;
+	io.smb2.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY;
+	io.smb2.in.share_access = 0;
+	io.smb2.in.alloc_size = 0;
+	io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN;
+	io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+	io.smb2.in.security_flags = 0;
+	io.smb2.in.fname = BASEDIR;
+
+	status = smb2_create(tree1, torture, &(io.smb2));
+	torture_assert_ntstatus_ok(torture, status, "smb2_create dir");
+	d1 = io.smb2.out.file.handle;
+
+	ZERO_STRUCT(io.smb2);
+	io.generic.level = RAW_OPEN_SMB2;
+	io.smb2.in.desired_access = 0x0017019f;
+	io.smb2.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY;
+	io.smb2.in.share_access = 0;
+	io.smb2.in.alloc_size = 0;
+	io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN;
+	io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+	io.smb2.in.security_flags = 0;
+	io.smb2.in.fname = BASEDIR;
+	sinfo.rename_information.in.file.handle = d1;
+
+	status = smb2_setinfo_file(tree1, &sinfo);
+	torture_assert_ntstatus_ok(torture, status, "smb2_setinfo_file");
+
+	ZERO_STRUCT(cl.smb2);
+	cl.smb2.level = RAW_CLOSE_SMB2;
+	cl.smb2.in.file.handle = d1;
+	status = smb2_close(tree1, &(cl.smb2));
+	torture_assert_ntstatus_ok(torture, status, "smb2_close");
+	ZERO_STRUCT(d1);
+
+	cl.smb2.in.file.handle = h1;
+	status = smb2_close(tree1, &(cl.smb2));
+	torture_assert_ntstatus_ok(torture, status, "smb2_close");
+	ZERO_STRUCT(h1);
+
+	torture_comment(torture, "Cleaning up\n");
+
+	if (h1.data) {
+		ZERO_STRUCT(cl.smb2);
+		cl.smb2.level = RAW_CLOSE_SMB2;
+		cl.smb2.in.file.handle = h1;
+		status = smb2_close(tree1, &(cl.smb2));
+	}
+
+	smb2_deltree(tree1, BASEDIR);
+	smb2_deltree(tree1, renamedir);
+	return ret;
+}
+
 /*
  * Note: This test depends on "vfs objects = catia fruit
  * streams_xattr".  Note: To run this test, use
@@ -2571,6 +2771,7 @@ struct torture_suite *torture_vfs_fruit(void)
 	torture_suite_add_2ns_smb2_test(suite, "stream names", test_stream_names);
 	torture_suite_add_2ns_smb2_test(suite, "truncate resource fork to 0 bytes", test_rfork_truncate);
 	torture_suite_add_2ns_smb2_test(suite, "opening and creating resource fork", test_rfork_create);
+	torture_suite_add_2ns_smb2_test(suite, "rename_dir_openfile", test_rename_dir_openfile);
 
 	return suite;
 }
-- 
2.5.0


From ea668519a95c619b7364e200c3e5e201f9e22352 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 29 Nov 2015 15:55:43 +0100
Subject: [PATCH 6/6] vfs: remove posix_flags hack, bump interface version to
 34

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/include/vfs.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index d839be4..17bd8fa 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -167,8 +167,10 @@
 /* Version 33 - Add snapshot create/delete calls */
 /* Version 33 - Add OS X SMB2 AAPL copyfile extension flag to fsp */
 /* Version 33 - Remove notify_watch_fn */
+/* Bump to version 34 - Samba 4.4 will ship with that */
+/* Version 34 - Remove bool posix_open, add uint64_t posix_flags */
 
-#define SMB_VFS_INTERFACE_VERSION 33
+#define SMB_VFS_INTERFACE_VERSION 34
 
 /*
     All intercepted VFS operations must be declared as static functions inside module source
@@ -218,9 +220,6 @@ struct fsp_lease {
 	struct smb2_lease lease;
 };
 
-/* VFS ABI stability hack */
-#define posix_flags posix_open
-
 typedef struct files_struct {
 	struct files_struct *next, *prev;
 	uint64_t fnum;
@@ -258,7 +257,7 @@ typedef struct files_struct {
 	bool aio_write_behind;
 	bool initial_delete_on_close; /* Only set at NTCreateX if file was created. */
 	bool delete_on_close;
-	uint8_t posix_flags;
+	uint64_t posix_flags;
 	bool is_sparse;
 	bool backup_intent; /* Handle was successfully opened with backup intent
 				and opener has privilege to do so. */
-- 
2.5.0



More information about the samba-technical mailing list