From c53a9ed4ff567f02f6a9843efbef247f20646f10 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Thu, 21 Dec 2023 19:40:21 +0100
Subject: [PATCH 01/13] selftest: remove error_inject from shadow_write share

Frankly, I can't remember why I added this as part of bug 13688. The
goal of the corresponding test is to verify a write on a read-only
file handle fails. As the file is opened O_RDONLY, the write will fail
anyway and there's no need to inject the error.

To make things worse, having the error injected meant we didn't notice
when the underlying logic of forcing the open to be done with O_RDONLY
was done as O_RDWR, resulting in the write on the handle to succeed.

This happened when we introduced reopen_from_fsp(): the initial
pathref open of a path with a twrp value was correctly detected and
handled by shadow_copy2_openat(). However, when converting the pathref
open to a real one via reopen_from_fsp(), shadow_copy2_openat() only
sees the magic /proc/fd path and has no way of inferring that this was
originating from a prevous version open with a twrp value.

Tl;dr: we can just remove this error injection, it is not needed, the
correct fix is to implement this in the SMB layer which is done in the
subsequent commits.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit 78119edba013583555069271bb61134c12c2c135)
---
 selftest/knownfail.d/samba3.blackbox.shadow_copy_torture | 1 +
 selftest/target/Samba3.pm                                | 4 +---
 2 files changed, 2 insertions(+), 3 deletions(-)
 create mode 100644 selftest/knownfail.d/samba3.blackbox.shadow_copy_torture

diff --git a/selftest/knownfail.d/samba3.blackbox.shadow_copy_torture b/selftest/knownfail.d/samba3.blackbox.shadow_copy_torture
new file mode 100644
index 000000000000..16537e58aeb7
--- /dev/null
+++ b/selftest/knownfail.d/samba3.blackbox.shadow_copy_torture
@@ -0,0 +1 @@
+^samba3.blackbox.shadow_copy_torture.writing to shadow copy of a file\(fileserver\)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 60775433de26..acee7dc68f08 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -3408,9 +3408,7 @@ sub provision($$)
 [shadow_write]
 	path = $shadow_tstdir
 	comment = previous versions snapshots under mount point
-	vfs objects = shadow_copy2 streams_xattr error_inject
-	aio write size = 0
-	error_inject:pwrite = EBADF
+	vfs objects = shadow_copy2 streams_xattr
 	shadow:mountpoint = $shadow_tstdir
 	shadow:fixinodes = yes
 	smbd async dosmode = yes
-- 
2.43.0


From 8bd06518af51ad85d5f1611d4b7f46660d3dfcea Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Fri, 22 Dec 2023 10:40:39 +0100
Subject: [PATCH 02/13] s4/libcli/raw: implemement
 RAW_SFILEINFO_LINK_INFORMATION

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit c62484bc2c60ebac42635793d94cb8e62629acbf)
---
 source4/libcli/raw/rawsetfileinfo.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/source4/libcli/raw/rawsetfileinfo.c b/source4/libcli/raw/rawsetfileinfo.c
index fec99d30b957..0ef509405091 100644
--- a/source4/libcli/raw/rawsetfileinfo.c
+++ b/source4/libcli/raw/rawsetfileinfo.c
@@ -119,6 +119,20 @@ bool smb_raw_setfileinfo_passthru(TALLOC_CTX *mem_ctx,
 				    parms->full_ea_information.in.eas.eas, 4);
 		return true;
 
+	case RAW_SFILEINFO_LINK_INFORMATION:
+		NEED_BLOB(20);
+		memset(blob->data, 0, blob->length);
+
+		PUSH_LE_U8(blob->data, 0, parms->link_information.in.overwrite);
+		PUSH_LE_U64(blob->data, 8, parms->link_information.in.root_fid);
+
+		len = smbcli_blob_append_string(
+			NULL, mem_ctx, blob,
+			parms->link_information.in.new_name,
+			STR_UNICODE | STR_TERMINATE);
+		PUSH_LE_U32(blob->data, 16, len - 2);
+		return true;
+
 		/* Unhandled levels */
 	case RAW_SFILEINFO_PIPE_INFORMATION:
 	case RAW_SFILEINFO_VALID_DATA_INFORMATION:
-- 
2.43.0


From 37afa92882cbc31738602e2306bdf4d0b1ca9334 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Fri, 15 Dec 2023 19:55:23 +0100
Subject: [PATCH 03/13] smbtorture: expand smb2.twrp.write test

Test more modifying operations are blocked and access masks are correct.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit 537eedfe2a79fba2e1f062f14ba7a0c5f8f70a88)
---
 source4/torture/smb2/create.c | 245 +++++++++++++++++++++++++++++++++-
 1 file changed, 242 insertions(+), 3 deletions(-)

diff --git a/source4/torture/smb2/create.c b/source4/torture/smb2/create.c
index d9fa2217953c..24c3cace3c41 100644
--- a/source4/torture/smb2/create.c
+++ b/source4/torture/smb2/create.c
@@ -2080,6 +2080,18 @@ static bool test_twrp_write(struct torture_context *tctx, struct smb2_tree *tree
 	uint64_t nttime;
 	const char *file = NULL;
 	const char *snapshot = NULL;
+	uint32_t expected_access;
+	union smb_fileinfo getinfo;
+	union smb_setfileinfo setinfo;
+	struct security_descriptor *sd = NULL, *sd_orig = NULL;
+	const char *owner_sid = NULL;
+	struct create_disps_tests {
+		const char *file;
+		uint32_t create_disposition;
+		uint32_t create_options;
+		NTSTATUS expected_status;
+	};
+	struct create_disps_tests *cd_test = NULL;
 
 	file = torture_setting_string(tctx, "twrp_file", NULL);
 	if (file == NULL) {
@@ -2121,11 +2133,102 @@ static bool test_twrp_write(struct torture_context *tctx, struct smb2_tree *tree
 					"smb2_create\n");
 	smb2_util_close(tree, io.out.file.handle);
 
-	ret = io.out.maximal_access & (SEC_FILE_READ_DATA | SEC_FILE_WRITE_DATA);
-	torture_assert_goto(tctx, ret, ret, done, "Bad access\n");
+	expected_access = SEC_RIGHTS_FILE_ALL &
+				~(SEC_FILE_EXECUTE | SEC_DIR_DELETE_CHILD);
+
+	torture_assert_int_equal_goto(tctx,
+				      io.out.maximal_access & expected_access,
+				      expected_access,
+				      ret, done, "Bad access\n");
+
+	{
+		/*
+		 * Test create dispositions
+		 */
+		struct create_disps_tests cd_tests[] = {
+			{
+				.file = file,
+				.create_disposition = NTCREATEX_DISP_OPEN,
+				.expected_status = NT_STATUS_OK,
+			},
+			{
+				.file = file,
+				.create_disposition = NTCREATEX_DISP_OPEN_IF,
+				.expected_status = NT_STATUS_OK,
+			},
+			{
+				.file = file,
+				.create_disposition = NTCREATEX_DISP_OVERWRITE,
+				.expected_status = NT_STATUS_MEDIA_WRITE_PROTECTED,
+			},
+			{
+				.file = file,
+				.create_disposition = NTCREATEX_DISP_OVERWRITE_IF,
+				.expected_status = NT_STATUS_MEDIA_WRITE_PROTECTED,
+			},
+			{
+				.file = file,
+				.create_disposition = NTCREATEX_DISP_SUPERSEDE,
+				.expected_status = NT_STATUS_MEDIA_WRITE_PROTECTED,
+			},
+			{
+				.file = "newfile",
+				.create_disposition = NTCREATEX_DISP_OPEN_IF,
+				.expected_status = NT_STATUS_MEDIA_WRITE_PROTECTED,
+			},
+			{
+				.file = "newfile",
+				.create_disposition = NTCREATEX_DISP_OVERWRITE_IF,
+				.expected_status = NT_STATUS_MEDIA_WRITE_PROTECTED,
+			},
+			{
+				.file = "newfile",
+				.create_disposition = NTCREATEX_DISP_CREATE,
+				.expected_status = NT_STATUS_MEDIA_WRITE_PROTECTED,
+			},
+			{
+				.file = "newfile",
+				.create_disposition = NTCREATEX_DISP_SUPERSEDE,
+				.expected_status = NT_STATUS_MEDIA_WRITE_PROTECTED,
+			},
+			{
+				.file = "newdir",
+				.create_disposition = NTCREATEX_DISP_OPEN_IF,
+				.create_options = NTCREATEX_OPTIONS_DIRECTORY,
+				.expected_status = NT_STATUS_MEDIA_WRITE_PROTECTED,
+			},
+			{
+				.file = "newdir",
+				.create_disposition = NTCREATEX_DISP_CREATE,
+				.create_options = NTCREATEX_OPTIONS_DIRECTORY,
+				.expected_status = NT_STATUS_MEDIA_WRITE_PROTECTED,
+			},
+			{
+				.file = NULL,
+			},
+		};
+
+		for (cd_test = &cd_tests[0]; cd_test->file != NULL; cd_test++) {
+			io = (struct smb2_create) {
+				.in.fname = cd_test->file,
+				.in.create_disposition = cd_test->create_disposition,
+				.in.create_options = cd_test->create_options,
+
+				.in.desired_access = SEC_FILE_READ_DATA,
+				.in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+				.in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
+				.in.timewarp = nttime,
+			};
+
+			status = smb2_create(tree, tctx, &io);
+			torture_assert_ntstatus_equal_goto(
+				tctx, status, cd_test->expected_status, ret, done,
+				"Bad status\n");
+		}
+	}
 
 	io = (struct smb2_create) {
-		.in.desired_access = SEC_FILE_READ_DATA|SEC_FILE_WRITE_DATA,
+		.in.desired_access = expected_access,
 		.in.file_attributes = FILE_ATTRIBUTE_NORMAL,
 		.in.create_disposition = NTCREATEX_DISP_OPEN,
 		.in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
@@ -2143,9 +2246,145 @@ static bool test_twrp_write(struct torture_context *tctx, struct smb2_tree *tree
 					   NT_STATUS_MEDIA_WRITE_PROTECTED,
 					   ret, done, "smb2_create\n");
 
+	/*
+	 * Verify access mask
+	 */
+
+	ZERO_STRUCT(getinfo);
+	getinfo.generic.level = RAW_FILEINFO_ACCESS_INFORMATION;
+	getinfo.generic.in.file.handle = h1;
+
+	status = smb2_getinfo_file(tree, tree, &getinfo);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_getinfo_file\n");
+
+	torture_assert_int_equal_goto(
+		tctx,
+		getinfo.access_information.out.access_flags,
+		expected_access,
+		ret, done,
+		"Bad access mask\n");
+
+	/*
+	 * Check we can't set various things
+	 */
+
+	ZERO_STRUCT(getinfo);
+	getinfo.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
+	getinfo.query_secdesc.in.file.handle = h1;
+	getinfo.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER;
+
+	status = smb2_getinfo_file(tree, tctx, &getinfo);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_getinfo_file\n");
+
+	sd_orig = getinfo.query_secdesc.out.sd;
+	owner_sid = dom_sid_string(tctx, sd_orig->owner_sid);
+
+	sd = security_descriptor_dacl_create(tctx,
+					     0, NULL, NULL,
+					     owner_sid,
+					     SEC_ACE_TYPE_ACCESS_ALLOWED,
+					     SEC_FILE_WRITE_DATA,
+					     0,
+					     NULL);
+
+	/* Try to set ACL */
+
+	ZERO_STRUCT(setinfo);
+	setinfo.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
+	setinfo.set_secdesc.in.file.handle = h1;
+	setinfo.set_secdesc.in.secinfo_flags = SECINFO_DACL;
+	setinfo.set_secdesc.in.sd = sd;
+
+	status = smb2_setinfo_file(tree, &setinfo);
+	torture_assert_ntstatus_equal_goto(
+		tctx,
+		status,
+		NT_STATUS_MEDIA_WRITE_PROTECTED,
+		ret, done,
+		"smb2_setinfo_file\n");
+
+	/* Try to delete */
+
+	ZERO_STRUCT(setinfo);
+	setinfo.generic.level = RAW_SFILEINFO_DISPOSITION_INFORMATION;
+	setinfo.disposition_info.in.delete_on_close = 1;
+	setinfo.generic.in.file.handle = h1;
+
+	status = smb2_setinfo_file(tree, &setinfo);
+	torture_assert_ntstatus_equal_goto(
+		tctx,
+		status,
+		NT_STATUS_MEDIA_WRITE_PROTECTED,
+		ret, done,
+		"smb2_setinfo_file\n");
+
+	ZERO_STRUCT(setinfo);
+	setinfo.basic_info.in.attrib = FILE_ATTRIBUTE_HIDDEN;
+	setinfo.generic.level = RAW_SFILEINFO_BASIC_INFORMATION;
+	setinfo.generic.in.file.handle = h1;
+
+	status = smb2_setinfo_file(tree, &setinfo);
+	torture_assert_ntstatus_equal_goto(
+		tctx,
+		status,
+		NT_STATUS_MEDIA_WRITE_PROTECTED,
+		ret, done,
+		"smb2_setinfo_file\n");
+
+	/* Try to truncate */
+
+	ZERO_STRUCT(setinfo);
+	setinfo.generic.level = SMB_SFILEINFO_END_OF_FILE_INFORMATION;
+	setinfo.generic.in.file.handle = h1;
+	setinfo.end_of_file_info.in.size = 0x100000;
+
+	status = smb2_setinfo_file(tree, &setinfo);
+	torture_assert_ntstatus_equal_goto(
+		tctx,
+		status,
+		NT_STATUS_MEDIA_WRITE_PROTECTED,
+		ret, done,
+		"smb2_setinfo_file\n");
+
+	/* Try to set a hardlink */
+
+	ZERO_STRUCT(setinfo);
+	setinfo.generic.level = RAW_SFILEINFO_LINK_INFORMATION;
+	setinfo.generic.in.file.handle = h1;
+	setinfo.link_information.in.new_name = "hardlink";
+
+	status = smb2_setinfo_file(tree, &setinfo);
+	torture_assert_ntstatus_equal_goto(
+		tctx,
+		status,
+		NT_STATUS_NOT_SAME_DEVICE,
+		ret, done,
+		"smb2_setinfo_file\n");
+
+	/* Try to rename */
+
+	ZERO_STRUCT(setinfo);
+	setinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION;
+	setinfo.rename_information.in.file.handle = h1;
+	setinfo.rename_information.in.new_name = "renamed";
+
+	status = smb2_setinfo_file(tree, &setinfo);
+	torture_assert_ntstatus_equal_goto(
+		tctx,
+		status,
+		NT_STATUS_NOT_SAME_DEVICE,
+		ret, done,
+		"smb2_setinfo_file\n");
+
 	smb2_util_close(tree, h1);
+	ZERO_STRUCT(h1);
 
 done:
+	if (!smb2_util_handle_empty(h1)) {
+		smb2_util_close(tree, h1);
+	}
 	return ret;
 }
 
-- 
2.43.0


From 45a967ae3eed4db66757fa49a1a009b838dc86bd Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 19 Dec 2023 13:06:55 +0100
Subject: [PATCH 04/13] smbd: return the correct error in can_rename()

This is what Windows returns for this case.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit 276c5bd851ab6ab818a49d9c47f6b96de8024778)
---
 source3/smbd/smb2_reply.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/source3/smbd/smb2_reply.c b/source3/smbd/smb2_reply.c
index 9476c69b73ca..a4055c00a312 100644
--- a/source3/smbd/smb2_reply.c
+++ b/source3/smbd/smb2_reply.c
@@ -1156,6 +1156,11 @@ ssize_t sendfile_short_send(struct smbXsrv_connection *xconn,
 static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp,
 			uint16_t dirtype)
 {
+	if (fsp->fsp_name->twrp != 0) {
+		/* Get the error right, this is what Windows returns. */
+		return NT_STATUS_NOT_SAME_DEVICE;
+	}
+
 	if (!CAN_WRITE(conn)) {
 		return NT_STATUS_MEDIA_WRITE_PROTECTED;
 	}
-- 
2.43.0


From a071390bc6ce86885ef09b4c14beb4e5d551a6a2 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Thu, 21 Dec 2023 16:27:42 +0100
Subject: [PATCH 05/13] smbd: set fsp_flags.is_fsa to true on printer file
 handles

Printer file handles went through SMB_VFS_CREATE_FILE() and are network
callable, so it makes sense to set this on them.

This ensures that check_access_fsp() doesn't take the codepath calling
smbd_check_access_rights_fsp(), but just checks the request rights from
fsp->access_mask.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit 76c8fe16bff36a29fa326355256b50737d04bd85)
---
 source3/printing/printspoolss.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/printing/printspoolss.c b/source3/printing/printspoolss.c
index 31117a4b7438..94404f7682ab 100644
--- a/source3/printing/printspoolss.c
+++ b/source3/printing/printspoolss.c
@@ -244,6 +244,7 @@ NTSTATUS print_spool_open(files_struct *fsp,
 	fsp->sent_oplock_break = NO_BREAK_SENT;
 	fsp->fsp_flags.is_directory = false;
 	fsp->fsp_flags.delete_on_close = false;
+	fsp->fsp_flags.is_fsa = true;
 
 	fsp->print_file = pf;
 
-- 
2.43.0


From bf44a38daaf168c930746d8a4bce202d07523a59 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Thu, 21 Dec 2023 10:58:09 +0100
Subject: [PATCH 06/13] smbd: rename check_access_fsp() to
 check_any_access_fsp()

The semantics of the access check in check_access_fsp() itself is to
allow access if *at least* one or more rights of the rights in
access_mask are allowed. The name check_any_access_fsp() better
reflects this.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit 96b577c380fa914eb1ffa95849c82bdb88aa1ec6)
---
 source3/smbd/proto.h              |  4 ++--
 source3/smbd/smb2_ioctl_filesys.c |  6 +++---
 source3/smbd/smb2_trans2.c        | 15 ++++++++++-----
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 26cafeba091f..f3f998a9c0f2 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1114,8 +1114,8 @@ NTSTATUS smb_set_file_disposition_info(connection_struct *conn,
 				       files_struct *fsp,
 				       struct smb_filename *smb_fname);
 NTSTATUS refuse_symlink_fsp(const struct files_struct *fsp);
-NTSTATUS check_access_fsp(struct files_struct *fsp,
-			  uint32_t access_mask);
+NTSTATUS check_any_access_fsp(struct files_struct *fsp,
+			      uint32_t access_mask);
 uint64_t smb_roundup(connection_struct *conn, uint64_t val);
 bool samba_private_attr_name(const char *unix_ea_name);
 NTSTATUS get_ea_value_fsp(TALLOC_CTX *mem_ctx,
diff --git a/source3/smbd/smb2_ioctl_filesys.c b/source3/smbd/smb2_ioctl_filesys.c
index 36429b8fd352..6cc53d4828ed 100644
--- a/source3/smbd/smb2_ioctl_filesys.c
+++ b/source3/smbd/smb2_ioctl_filesys.c
@@ -378,7 +378,7 @@ static NTSTATUS fsctl_set_cmprn(TALLOC_CTX *mem_ctx,
 	}
 
 	/* WRITE_DATA permission is required, WRITE_ATTRIBUTES is not */
-	status = check_access_fsp(fsp, FILE_WRITE_DATA);
+	status = check_any_access_fsp(fsp, FILE_WRITE_DATA);
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
@@ -426,7 +426,7 @@ static NTSTATUS fsctl_zero_data(TALLOC_CTX *mem_ctx,
 	}
 
 	/* WRITE_DATA permission is required */
-	status = check_access_fsp(fsp, FILE_WRITE_DATA);
+	status = check_any_access_fsp(fsp, FILE_WRITE_DATA);
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
@@ -616,7 +616,7 @@ static NTSTATUS fsctl_qar(TALLOC_CTX *mem_ctx,
 	}
 
 	/* READ_DATA permission is required */
-	status = check_access_fsp(fsp, FILE_READ_DATA);
+	status = check_any_access_fsp(fsp, FILE_READ_DATA);
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
diff --git a/source3/smbd/smb2_trans2.c b/source3/smbd/smb2_trans2.c
index fa9e8b4509b3..80a932566b28 100644
--- a/source3/smbd/smb2_trans2.c
+++ b/source3/smbd/smb2_trans2.c
@@ -72,8 +72,13 @@ NTSTATUS refuse_symlink_fsp(const files_struct *fsp)
 	return NT_STATUS_OK;
 }
 
-NTSTATUS check_access_fsp(struct files_struct *fsp,
-			  uint32_t access_mask)
+/**
+ * Check that one or more of the rights in access_mask are
+ * allowed. Iow, access_mask can contain more then one right and
+ * it is sufficient having only one of those granted to pass.
+ **/
+NTSTATUS check_any_access_fsp(struct files_struct *fsp,
+			      uint32_t access_mask)
 {
 	if (!fsp->fsp_flags.is_fsa) {
 		return smbd_check_access_rights_fsp(fsp->conn->cwd_fsp,
@@ -677,7 +682,7 @@ NTSTATUS set_ea(connection_struct *conn, files_struct *fsp,
 		return status;
 	}
 
-	status = check_access_fsp(fsp, FILE_WRITE_EA);
+	status = check_any_access_fsp(fsp, FILE_WRITE_EA);
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
@@ -4822,7 +4827,7 @@ static NTSTATUS smb_set_file_basic_info(connection_struct *conn,
 		return NT_STATUS_INVALID_HANDLE;
 	}
 
-	status = check_access_fsp(fsp, FILE_WRITE_ATTRIBUTES);
+	status = check_any_access_fsp(fsp, FILE_WRITE_ATTRIBUTES);
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
@@ -4893,7 +4898,7 @@ static NTSTATUS smb_set_info_standard(connection_struct *conn,
 	DEBUG(10,("smb_set_info_standard: file %s\n",
 		smb_fname_str_dbg(smb_fname)));
 
-	status = check_access_fsp(fsp, FILE_WRITE_ATTRIBUTES);
+	status = check_any_access_fsp(fsp, FILE_WRITE_ATTRIBUTES);
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
-- 
2.43.0


From 5a898712c17c507e36b555dfee566da17f7920e2 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Thu, 21 Dec 2023 10:58:09 +0100
Subject: [PATCH 07/13] smbd: fix check_any_access_fsp() for non-fsa fsps

smbd_check_access_rights_fsp() requires *all* rights in access_mask to
be granted by the underlying ACL, but the semantics of this function
is supposed to grant access if any one of the rights in
access_requested is allowed.

Fix this by looping over the requested access mask. If
smbd_check_access_rights_fsp() returns sucess, mask will be non-null
and when assigned to access_granted, the subsequent check will pass,
fail otherwise.

I'm not doing an early exit on purpose because a subsequent commit
adds additional security checks that are done in the subsequent code
path common for fsa and non-fsa fsps.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit bf497819e61131cfa6469971596af3aa9bd4bb49)
---
 source3/smbd/proto.h       |  2 +-
 source3/smbd/smb2_trans2.c | 38 +++++++++++++++++++++++++++++---------
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index f3f998a9c0f2..eea56f6e0a82 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1115,7 +1115,7 @@ NTSTATUS smb_set_file_disposition_info(connection_struct *conn,
 				       struct smb_filename *smb_fname);
 NTSTATUS refuse_symlink_fsp(const struct files_struct *fsp);
 NTSTATUS check_any_access_fsp(struct files_struct *fsp,
-			      uint32_t access_mask);
+			      uint32_t access_requested);
 uint64_t smb_roundup(connection_struct *conn, uint64_t val);
 bool samba_private_attr_name(const char *unix_ea_name);
 NTSTATUS get_ea_value_fsp(TALLOC_CTX *mem_ctx,
diff --git a/source3/smbd/smb2_trans2.c b/source3/smbd/smb2_trans2.c
index 80a932566b28..f6a3166719e5 100644
--- a/source3/smbd/smb2_trans2.c
+++ b/source3/smbd/smb2_trans2.c
@@ -73,20 +73,40 @@ NTSTATUS refuse_symlink_fsp(const files_struct *fsp)
 }
 
 /**
- * Check that one or more of the rights in access_mask are
- * allowed. Iow, access_mask can contain more then one right and
+ * Check that one or more of the rights in access mask are
+ * allowed. Iow, access_requested can contain more then one right and
  * it is sufficient having only one of those granted to pass.
  **/
 NTSTATUS check_any_access_fsp(struct files_struct *fsp,
-			      uint32_t access_mask)
+			      uint32_t access_requested)
 {
-	if (!fsp->fsp_flags.is_fsa) {
-		return smbd_check_access_rights_fsp(fsp->conn->cwd_fsp,
-						    fsp,
-						    false,
-						    access_mask);
+	uint32_t access_granted = 0;
+	NTSTATUS status;
+
+	if (fsp->fsp_flags.is_fsa) {
+		access_granted = fsp->access_mask;
+	} else {
+		uint32_t mask = 1;
+
+		while (mask != 0) {
+			if (!(mask & access_requested)) {
+				mask <<= 1;
+				continue;
+			}
+
+			status = smbd_check_access_rights_fsp(
+							fsp->conn->cwd_fsp,
+							fsp,
+							false,
+							mask);
+			if (NT_STATUS_IS_OK(status)) {
+				break;
+			}
+			mask <<= 1;
+		}
+		access_granted = mask;
 	}
-	if (!(fsp->access_mask & access_mask)) {
+	if ((access_granted & access_requested) == 0) {
 		return NT_STATUS_ACCESS_DENIED;
 	}
 	return NT_STATUS_OK;
-- 
2.43.0


From 4eba27394d7f55c2c4e4ff78cb82b6890c27be7b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Fri, 22 Dec 2023 11:19:38 +0100
Subject: [PATCH 08/13] smbd: return correct error when trying to create a
 hardlink to a VSS file

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit a0ae45be770a13373c148a689b9761f14c4f942c)
---
 source3/smbd/smb2_trans2.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/source3/smbd/smb2_trans2.c b/source3/smbd/smb2_trans2.c
index f6a3166719e5..03639f218db7 100644
--- a/source3/smbd/smb2_trans2.c
+++ b/source3/smbd/smb2_trans2.c
@@ -3904,6 +3904,11 @@ NTSTATUS hardlink_internals(TALLOC_CTX *ctx,
 		goto out;
 	}
 
+	if (smb_fname_old->twrp != 0) {
+		status = NT_STATUS_NOT_SAME_DEVICE;
+		goto out;
+	}
+
 	status = parent_pathref(talloc_tos(),
 				conn->cwd_fsp,
 				smb_fname_old,
-- 
2.43.0


From 5b390fb1cc59345a538f505066da05ae90ad5bea Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Wed, 20 Dec 2023 18:03:22 +0100
Subject: [PATCH 09/13] smbd: set fsp->fsp_flags.can_write to false for access
 to previous-versions

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit ee3035218df4cfd68b6aab6825c78f2b85234c6c)
---
 source3/smbd/files.c | 3 +++
 source3/smbd/open.c  | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index d3f6b629264a..32388f133f37 100644
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -1977,6 +1977,9 @@ NTSTATUS dup_file_fsp(
 	to->fsp_flags.can_write =
 		CAN_WRITE(from->conn) &&
 		((access_mask & (FILE_WRITE_DATA | FILE_APPEND_DATA)) != 0);
+	if (from->fsp_name->twrp != 0) {
+		to->fsp_flags.can_write = false;
+	}
 	to->fsp_flags.modified = from->fsp_flags.modified;
 	to->fsp_flags.is_directory = from->fsp_flags.is_directory;
 	to->fsp_flags.aio_write_behind = from->fsp_flags.aio_write_behind;
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index c848243c9b58..e804d0969077 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1678,6 +1678,9 @@ static NTSTATUS open_file(struct smb_request *req,
 	fsp->fsp_flags.can_write =
 		CAN_WRITE(conn) &&
 		((access_mask & (FILE_WRITE_DATA | FILE_APPEND_DATA)) != 0);
+	if (fsp->fsp_name->twrp != 0) {
+		fsp->fsp_flags.can_write = false;
+	}
 	fsp->print_file = NULL;
 	fsp->fsp_flags.modified = false;
 	fsp->sent_oplock_break = NO_BREAK_SENT;
-- 
2.43.0


From 1fe96c45b3f27b6e6a0bafae482c3db2923b0b15 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Wed, 20 Dec 2023 18:32:25 +0100
Subject: [PATCH 10/13] smbd: replace CHECK_WRITE() macro with calls to
 check_any_access_fsp()

The additional check if fd underlying fd is valid and not -1 should not be done
at this place. I actually would prefer an write to fail with EBADF if this
happens, as it's likely easier to debug why this happened. These days we should
always have a valid fd.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit 995a31c8d4c1789c16bae6b8196f2565d4b1dfdb)
---
 source3/include/smb_macros.h    |  5 -----
 source3/modules/offload_token.c |  7 +++++--
 source3/smbd/smb1_reply.c       | 32 ++++++++++++++++++++------------
 source3/smbd/smb2_flush.c       |  3 ++-
 source3/smbd/smb2_write.c       |  6 ++++--
 5 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/source3/include/smb_macros.h b/source3/include/smb_macros.h
index 42ff9ffb0d45..1851709774a4 100644
--- a/source3/include/smb_macros.h
+++ b/source3/include/smb_macros.h
@@ -74,11 +74,6 @@
 	 (fsp_get_io_fd(fsp) != -1) && \
 	 (((fsp)->fsp_flags.can_read)))
 
-#define CHECK_WRITE(fsp) \
-	((fsp)->fsp_flags.can_write && \
-	(!(fsp)->fsp_flags.is_pathref) && \
-	 (fsp_get_io_fd(fsp) != -1))
-
 #define ERROR_WAS_LOCK_DENIED(status) (NT_STATUS_EQUAL((status), NT_STATUS_LOCK_NOT_GRANTED) || \
 				NT_STATUS_EQUAL((status), NT_STATUS_FILE_LOCK_CONFLICT) )
 
diff --git a/source3/modules/offload_token.c b/source3/modules/offload_token.c
index 901991daf28d..3b71a0028fb7 100644
--- a/source3/modules/offload_token.c
+++ b/source3/modules/offload_token.c
@@ -259,6 +259,8 @@ NTSTATUS vfs_offload_token_check_handles(uint32_t fsctl,
 					 files_struct *src_fsp,
 					 files_struct *dst_fsp)
 {
+	NTSTATUS status;
+
 	if (src_fsp->vuid != dst_fsp->vuid) {
 		DBG_INFO("copy chunk handles not in the same session.\n");
 		return NT_STATUS_ACCESS_DENIED;
@@ -317,10 +319,11 @@ NTSTATUS vfs_offload_token_check_handles(uint32_t fsctl,
 	 *
 	 * A non writable dst handle also doesn't make sense for other fsctls.
 	 */
-	if (!CHECK_WRITE(dst_fsp)) {
+	status = check_any_access_fsp(dst_fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+	if (!NT_STATUS_IS_OK(status)) {
 		DBG_INFO("dest handle not writable (%s).\n",
 			smb_fname_str_dbg(dst_fsp->fsp_name));
-		return NT_STATUS_ACCESS_DENIED;
+		return status;
 	}
 	/*
 	 * - The Open.GrantedAccess of the destination file does not include
diff --git a/source3/smbd/smb1_reply.c b/source3/smbd/smb1_reply.c
index abdc40fd2b28..9129005dcaeb 100644
--- a/source3/smbd/smb1_reply.c
+++ b/source3/smbd/smb1_reply.c
@@ -3626,8 +3626,9 @@ void reply_writebraw(struct smb_request *req)
 		return;
 	}
 
-	if (!CHECK_WRITE(fsp)) {
-		reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+	status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+	if (!NT_STATUS_IS_OK(status)) {
+		reply_nterror(req, status);
 		error_to_writebrawerr(req);
 		END_PROFILE(SMBwritebraw);
 		return;
@@ -3857,8 +3858,9 @@ void reply_writeunlock(struct smb_request *req)
 		return;
 	}
 
-	if (!CHECK_WRITE(fsp)) {
-		reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+	status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+	if (!NT_STATUS_IS_OK(status)) {
+		reply_nterror(req, status);
 		END_PROFILE(SMBwriteunlock);
 		return;
 	}
@@ -3992,8 +3994,9 @@ void reply_write(struct smb_request *req)
 		return;
 	}
 
-	if (!CHECK_WRITE(fsp)) {
-		reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+	status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+	if (!NT_STATUS_IS_OK(status)) {
+		reply_nterror(req, status);
 		END_PROFILE(SMBwrite);
 		return;
 	}
@@ -4271,8 +4274,9 @@ void reply_write_and_X(struct smb_request *req)
 		goto out;
 	}
 
-	if (!CHECK_WRITE(fsp)) {
-		reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+	status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+	if (!NT_STATUS_IS_OK(status)) {
+		reply_nterror(req, status);
 		goto out;
 	}
 
@@ -4983,6 +4987,7 @@ void reply_writeclose(struct smb_request *req)
 	struct timespec mtime;
 	files_struct *fsp;
 	struct lock_struct lock;
+	NTSTATUS status;
 
 	START_PROFILE(SMBwriteclose);
 
@@ -4998,8 +5003,9 @@ void reply_writeclose(struct smb_request *req)
 		END_PROFILE(SMBwriteclose);
 		return;
 	}
-	if (!CHECK_WRITE(fsp)) {
-		reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+	status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+	if (!NT_STATUS_IS_OK(status)) {
+		reply_nterror(req, status);
 		END_PROFILE(SMBwriteclose);
 		return;
 	}
@@ -5799,6 +5805,7 @@ void reply_printwrite(struct smb_request *req)
 	int numtowrite;
 	const char *data;
 	files_struct *fsp;
+	NTSTATUS status;
 
 	START_PROFILE(SMBsplwr);
 
@@ -5821,8 +5828,9 @@ void reply_printwrite(struct smb_request *req)
 		return;
 	}
 
-	if (!CHECK_WRITE(fsp)) {
-		reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+	status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+	if (!NT_STATUS_IS_OK(status)) {
+		reply_nterror(req, status);
 		END_PROFILE(SMBsplwr);
 		return;
 	}
diff --git a/source3/smbd/smb2_flush.c b/source3/smbd/smb2_flush.c
index 5c8c171e418e..6d1c3022c21e 100644
--- a/source3/smbd/smb2_flush.c
+++ b/source3/smbd/smb2_flush.c
@@ -150,7 +150,8 @@ static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx,
 		return tevent_req_post(req, ev);
 	}
 
-	if (!CHECK_WRITE(fsp)) {
+	status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+	if (!NT_STATUS_IS_OK(status)) {
 		bool allow_dir_flush = false;
 		uint32_t flush_access = FILE_ADD_FILE | FILE_ADD_SUBDIRECTORY;
 
diff --git a/source3/smbd/smb2_write.c b/source3/smbd/smb2_write.c
index 9fa87765b8f3..b5a5aeba80bc 100644
--- a/source3/smbd/smb2_write.c
+++ b/source3/smbd/smb2_write.c
@@ -24,6 +24,7 @@
 #include "../libcli/smb/smb_common.h"
 #include "../lib/util/tevent_ntstatus.h"
 #include "rpc_server/srv_pipe_hnd.h"
+#include "libcli/security/security.h"
 
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_SMB2
@@ -339,8 +340,9 @@ static struct tevent_req *smbd_smb2_write_send(TALLOC_CTX *mem_ctx,
 		return req;
 	}
 
-	if (!CHECK_WRITE(fsp)) {
-		tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
+	status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+	if (!NT_STATUS_IS_OK(status)) {
+		tevent_req_nterror(req, status);
 		return tevent_req_post(req, ev);
 	}
 
-- 
2.43.0


From b72f6c17a335a2f187a737f9b3a53f6727db68c6 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Wed, 20 Dec 2023 18:01:57 +0100
Subject: [PATCH 11/13] smbd: use check_any_access_fsp() for all access checks

Replaces the direct access to fsp->access_mask with a call to
check_any_access_fsp() which allows doing additional checks if needed.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(backported from commit 02ed99343d19fd0845531ad99a46b1dd5b8a7a4f)
[slow@samba.org: vfs_acl_common.c: different chown_needed check]
---
 source3/modules/vfs_acl_common.c    |  7 +++--
 source3/modules/vfs_nfs4acl_xattr.c |  7 +++--
 source3/smbd/dir.c                  |  5 ++--
 source3/smbd/dosmode.c              | 20 +++++++------
 source3/smbd/file_access.c          | 10 ++++---
 source3/smbd/notify.c               |  5 ++--
 source3/smbd/smb1_reply.c           |  5 ++--
 source3/smbd/smb2_flush.c           |  4 ++-
 source3/smbd/smb2_getinfo.c         |  8 ++---
 source3/smbd/smb2_nttrans.c         | 45 +++++++++++++++++------------
 source3/smbd/smb2_reply.c           | 10 ++++---
 source3/smbd/smb2_trans2.c          | 10 ++++---
 12 files changed, 82 insertions(+), 54 deletions(-)

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 81e1116b20b0..cd987b47f0a3 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -742,10 +742,13 @@ static NTSTATUS set_underlying_acl(vfs_handle_struct *handle, files_struct *fsp,
 	/* We got access denied here. If we're already root,
 	   or we didn't need to do a chown, or the fsp isn't
 	   open with WRITE_OWNER access, just return. */
-	if (get_current_uid(handle->conn) == 0 || chown_needed == false ||
-	    !(fsp->access_mask & SEC_STD_WRITE_OWNER)) {
+	if (get_current_uid(handle->conn) == 0 || chown_needed == false) {
 		return NT_STATUS_ACCESS_DENIED;
 	}
+	status = check_any_access_fsp(fsp, SEC_STD_WRITE_OWNER);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
 
 	/*
 	 * Only allow take-ownership, not give-ownership. That's the way Windows
diff --git a/source3/modules/vfs_nfs4acl_xattr.c b/source3/modules/vfs_nfs4acl_xattr.c
index cecafcf50b8d..1fd3519ca020 100644
--- a/source3/modules/vfs_nfs4acl_xattr.c
+++ b/source3/modules/vfs_nfs4acl_xattr.c
@@ -368,11 +368,14 @@ static NTSTATUS nfs4acl_xattr_fset_nt_acl(vfs_handle_struct *handle,
 	}
 
 	if (get_current_uid(handle->conn) == 0 ||
-	    chown_needed == false ||
-	    !(fsp->access_mask & SEC_STD_WRITE_OWNER))
+	    chown_needed == false)
 	{
 		return NT_STATUS_ACCESS_DENIED;
 	}
+	status = check_any_access_fsp(fsp, SEC_STD_WRITE_OWNER);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
 
 	/*
 	 * Only allow take-ownership, not give-ownership. That's the way Windows
diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index eb263132adf7..4f4ea42405b3 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -233,11 +233,12 @@ NTSTATUS dptr_create(connection_struct *conn,
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
-	if (!(fsp->access_mask & SEC_DIR_LIST)) {
+	status = check_any_access_fsp(fsp, SEC_DIR_LIST);
+	if (!NT_STATUS_IS_OK(status)) {
 		DBG_INFO("dptr_create: directory %s "
 			"not open for LIST access\n",
 			fsp_str_dbg(fsp));
-		return NT_STATUS_ACCESS_DENIED;
+		return status;
 	}
 	status = OpenDir_fsp(NULL, conn, fsp, wcard, attr, &dir_hnd);
 	if (!NT_STATUS_IS_OK(status)) {
diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c
index 5a88cd059b04..f0d66557ded8 100644
--- a/source3/smbd/dosmode.c
+++ b/source3/smbd/dosmode.c
@@ -1086,15 +1086,17 @@ NTSTATUS file_set_sparse(connection_struct *conn,
 	 * Windows Server 2008 & 2012 permit FSCTL_SET_SPARSE if any of the
 	 * following access flags are granted.
 	 */
-	if ((fsp->access_mask & (FILE_WRITE_DATA
-				| FILE_WRITE_ATTRIBUTES
-				| SEC_FILE_APPEND_DATA)) == 0) {
-		DEBUG(9,("file_set_sparse: fname[%s] set[%u] "
-			"access_mask[0x%08X] - access denied\n",
-			smb_fname_str_dbg(fsp->fsp_name),
-			sparse,
-			fsp->access_mask));
-		return NT_STATUS_ACCESS_DENIED;
+	status = check_any_access_fsp(fsp,
+				  FILE_WRITE_DATA
+				  | FILE_WRITE_ATTRIBUTES
+				  | SEC_FILE_APPEND_DATA);
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_DEBUG("fname[%s] set[%u] "
+			  "access_mask[0x%08X] - access denied\n",
+			  smb_fname_str_dbg(fsp->fsp_name),
+			  sparse,
+			  fsp->access_mask);
+		return status;
 	}
 
 	if (fsp->fsp_flags.is_directory) {
diff --git a/source3/smbd/file_access.c b/source3/smbd/file_access.c
index 476aead590dc..7c4c7156d245 100644
--- a/source3/smbd/file_access.c
+++ b/source3/smbd/file_access.c
@@ -191,6 +191,7 @@ bool directory_has_default_acl_fsp(struct files_struct *fsp)
 
 NTSTATUS can_set_delete_on_close(files_struct *fsp, uint32_t dosmode)
 {
+	NTSTATUS status;
 	/*
 	 * Only allow delete on close for writable files.
 	 */
@@ -219,11 +220,12 @@ NTSTATUS can_set_delete_on_close(files_struct *fsp, uint32_t dosmode)
 	 * intent.
 	 */
 
-	if (!(fsp->access_mask & DELETE_ACCESS)) {
-		DEBUG(10,("can_set_delete_on_close: file %s delete on "
+	status = check_any_access_fsp(fsp, DELETE_ACCESS);
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_DEBUG("file %s delete on "
 			  "close flag set but delete access denied.\n",
-			  fsp_str_dbg(fsp)));
-		return NT_STATUS_ACCESS_DENIED;
+			  fsp_str_dbg(fsp));
+		return status;
 	}
 
 	/* Don't allow delete on close for non-empty directories. */
diff --git a/source3/smbd/notify.c b/source3/smbd/notify.c
index c18ad0027b67..68f8539beeee 100644
--- a/source3/smbd/notify.c
+++ b/source3/smbd/notify.c
@@ -295,8 +295,9 @@ NTSTATUS change_notify_create(struct files_struct *fsp,
 	 * Setting a changenotify needs READ/LIST access
 	 * on the directory handle.
 	 */
-	if (!(fsp->access_mask & SEC_DIR_LIST)) {
-		return NT_STATUS_ACCESS_DENIED;
+	status = check_any_access_fsp(fsp, SEC_DIR_LIST);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
 	}
 
 	if (fsp->notify != NULL) {
diff --git a/source3/smbd/smb1_reply.c b/source3/smbd/smb1_reply.c
index 9129005dcaeb..740fb5fa9f78 100644
--- a/source3/smbd/smb1_reply.c
+++ b/source3/smbd/smb1_reply.c
@@ -6657,8 +6657,9 @@ void reply_setattrE(struct smb_request *req)
 		goto out;
 	}
 
-	if (!(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) {
-		reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+	status = check_any_access_fsp(fsp, FILE_WRITE_ATTRIBUTES);
+	if (!NT_STATUS_IS_OK(status)) {
+		reply_nterror(req, status);
 		goto out;
 	}
 
diff --git a/source3/smbd/smb2_flush.c b/source3/smbd/smb2_flush.c
index 6d1c3022c21e..3bd0b2a93bd6 100644
--- a/source3/smbd/smb2_flush.c
+++ b/source3/smbd/smb2_flush.c
@@ -128,6 +128,7 @@ static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx,
 	struct smb_request *smbreq;
 	bool is_compound = false;
 	bool is_last_in_compound = false;
+	NTSTATUS status;
 
 	req = tevent_req_create(mem_ctx, &state,
 				struct smbd_smb2_flush_state);
@@ -167,7 +168,8 @@ static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx,
 		 * they can be flushed.
 		 */
 
-		if ((fsp->access_mask & flush_access) != 0) {
+		status = check_any_access_fsp(fsp, flush_access);
+		if (NT_STATUS_IS_OK(status)) {
 			allow_dir_flush = true;
 		}
 
diff --git a/source3/smbd/smb2_getinfo.c b/source3/smbd/smb2_getinfo.c
index f5156147bc3c..cc6f39548a9a 100644
--- a/source3/smbd/smb2_getinfo.c
+++ b/source3/smbd/smb2_getinfo.c
@@ -316,15 +316,15 @@ static struct tevent_req *smbd_smb2_getinfo_send(TALLOC_CTX *mem_ctx,
 		case FSCC_FILE_ALL_INFORMATION:
 		case FSCC_FILE_NETWORK_OPEN_INFORMATION:
 		case FSCC_FILE_ATTRIBUTE_TAG_INFORMATION:
-			if (!(fsp->access_mask & SEC_FILE_READ_ATTRIBUTE)) {
-				tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
+			status = check_any_access_fsp(fsp, SEC_FILE_READ_ATTRIBUTE);
+			if (tevent_req_nterror(req, status)) {
 				return tevent_req_post(req, ev);
 			}
 			break;
 
 		case FSCC_FILE_FULL_EA_INFORMATION:
-			if (!(fsp->access_mask & SEC_FILE_READ_EA)) {
-				tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
+			status = check_any_access_fsp(fsp, SEC_FILE_READ_EA);
+			if (tevent_req_nterror(req, status)) {
 				return tevent_req_post(req, ev);
 			}
 			break;
diff --git a/source3/smbd/smb2_nttrans.c b/source3/smbd/smb2_nttrans.c
index 84defa3f0528..54186f850873 100644
--- a/source3/smbd/smb2_nttrans.c
+++ b/source3/smbd/smb2_nttrans.c
@@ -114,20 +114,23 @@ NTSTATUS set_sd(files_struct *fsp, struct security_descriptor *psd,
 
 	/* Ensure we have the rights to do this. */
 	if (security_info_sent & SECINFO_OWNER) {
-		if (!(fsp->access_mask & SEC_STD_WRITE_OWNER)) {
-			return NT_STATUS_ACCESS_DENIED;
+		status = check_any_access_fsp(fsp, SEC_STD_WRITE_OWNER);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
 		}
 	}
 
 	if (security_info_sent & SECINFO_GROUP) {
-		if (!(fsp->access_mask & SEC_STD_WRITE_OWNER)) {
-			return NT_STATUS_ACCESS_DENIED;
+		status = check_any_access_fsp(fsp, SEC_STD_WRITE_OWNER);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
 		}
 	}
 
 	if (security_info_sent & SECINFO_DACL) {
-		if (!(fsp->access_mask & SEC_STD_WRITE_DAC)) {
-			return NT_STATUS_ACCESS_DENIED;
+		status = check_any_access_fsp(fsp, SEC_STD_WRITE_DAC);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
 		}
 		/* Convert all the generic bits. */
 		if (psd->dacl) {
@@ -136,15 +139,17 @@ NTSTATUS set_sd(files_struct *fsp, struct security_descriptor *psd,
 	}
 
 	if (security_info_sent & SECINFO_SACL) {
-		if (!(fsp->access_mask & SEC_FLAG_SYSTEM_SECURITY)) {
-			return NT_STATUS_ACCESS_DENIED;
+		status = check_any_access_fsp(fsp, SEC_FLAG_SYSTEM_SECURITY);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
 		}
 		/*
 		 * Setting a SACL also requires WRITE_DAC.
 		 * See the smbtorture3 SMB2-SACL test.
 		 */
-		if (!(fsp->access_mask & SEC_STD_WRITE_DAC)) {
-			return NT_STATUS_ACCESS_DENIED;
+		status = check_any_access_fsp(fsp, SEC_STD_WRITE_DAC);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
 		}
 		/* Convert all the generic bits. */
 		if (psd->sacl) {
@@ -407,16 +412,20 @@ static NTSTATUS smbd_fetch_security_desc(connection_struct *conn,
 	 * Get the permissions to return.
 	 */
 
-	if ((security_info_wanted & SECINFO_SACL) &&
-			!(fsp->access_mask & SEC_FLAG_SYSTEM_SECURITY)) {
-		DEBUG(10, ("Access to SACL denied.\n"));
-		return NT_STATUS_ACCESS_DENIED;
+	if (security_info_wanted & SECINFO_SACL) {
+		status = check_any_access_fsp(fsp, SEC_FLAG_SYSTEM_SECURITY);
+		if (!NT_STATUS_IS_OK(status)) {
+			DBG_DEBUG("Access to SACL denied.\n");
+			return status;
+		}
 	}
 
-	if ((security_info_wanted & (SECINFO_DACL|SECINFO_OWNER|SECINFO_GROUP)) &&
-			!(fsp->access_mask & SEC_STD_READ_CONTROL)) {
-		DEBUG(10, ("Access to DACL, OWNER, or GROUP denied.\n"));
-		return NT_STATUS_ACCESS_DENIED;
+	if (security_info_wanted & (SECINFO_DACL|SECINFO_OWNER|SECINFO_GROUP)) {
+		status = check_any_access_fsp(fsp, SEC_STD_READ_CONTROL);
+		if (!NT_STATUS_IS_OK(status)) {
+			DBG_DEBUG("Access to DACL, OWNER, or GROUP denied.\n");
+			return status;
+		}
 	}
 
 	status = refuse_symlink_fsp(fsp);
diff --git a/source3/smbd/smb2_reply.c b/source3/smbd/smb2_reply.c
index a4055c00a312..8d8d8e0bc715 100644
--- a/source3/smbd/smb2_reply.c
+++ b/source3/smbd/smb2_reply.c
@@ -1156,6 +1156,8 @@ ssize_t sendfile_short_send(struct smbXsrv_connection *xconn,
 static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp,
 			uint16_t dirtype)
 {
+	NTSTATUS status;
+
 	if (fsp->fsp_name->twrp != 0) {
 		/* Get the error right, this is what Windows returns. */
 		return NT_STATUS_NOT_SAME_DEVICE;
@@ -1199,11 +1201,11 @@ static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp,
 		return NT_STATUS_OK;
 	}
 
-	if (fsp->access_mask & (DELETE_ACCESS|FILE_WRITE_ATTRIBUTES)) {
-		return NT_STATUS_OK;
+	status = check_any_access_fsp(fsp, DELETE_ACCESS | FILE_WRITE_ATTRIBUTES);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
 	}
-
-	return NT_STATUS_ACCESS_DENIED;
+	return NT_STATUS_OK;
 }
 
 /****************************************************************************
diff --git a/source3/smbd/smb2_trans2.c b/source3/smbd/smb2_trans2.c
index 03639f218db7..b5d73bdf1fa5 100644
--- a/source3/smbd/smb2_trans2.c
+++ b/source3/smbd/smb2_trans2.c
@@ -4167,8 +4167,9 @@ NTSTATUS smb_set_file_size(connection_struct *conn,
 	    fsp_get_io_fd(fsp) != -1)
 	{
 		/* Handle based call. */
-		if (!(fsp->access_mask & FILE_WRITE_DATA)) {
-			return NT_STATUS_ACCESS_DENIED;
+		status = check_any_access_fsp(fsp, FILE_WRITE_DATA);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
 		}
 
 		if (vfs_set_filelen(fsp, size) == -1) {
@@ -4981,8 +4982,9 @@ static NTSTATUS smb_set_file_allocation_info(connection_struct *conn,
 	    fsp_get_io_fd(fsp) != -1)
 	{
 		/* Open file handle. */
-		if (!(fsp->access_mask & FILE_WRITE_DATA)) {
-			return NT_STATUS_ACCESS_DENIED;
+		status = check_any_access_fsp(fsp, FILE_WRITE_DATA);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
 		}
 
 		/* Only change if needed. */
-- 
2.43.0


From 9c4b78a7dc1a2b18fc8c683f14a782395b48c7cb Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Wed, 20 Dec 2023 15:09:59 +0100
Subject: [PATCH 12/13] smbd: check for previous versions in
 check_any_access_fsp()

Now that check_any_access_fsp() is broadly used consistently to
restrict access for all modifying operations, we can add a check for
previous versions to check_any_access_fsp() and it gets enforced
consistently.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit fd4e41144a819b4403340e4a28664ac586722b41)
---
 .../samba3.blackbox.shadow_copy_torture       |  1 -
 source3/smbd/smb2_trans2.c                    | 22 +++++++++++++++++--
 2 files changed, 20 insertions(+), 3 deletions(-)
 delete mode 100644 selftest/knownfail.d/samba3.blackbox.shadow_copy_torture

diff --git a/selftest/knownfail.d/samba3.blackbox.shadow_copy_torture b/selftest/knownfail.d/samba3.blackbox.shadow_copy_torture
deleted file mode 100644
index 16537e58aeb7..000000000000
--- a/selftest/knownfail.d/samba3.blackbox.shadow_copy_torture
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.blackbox.shadow_copy_torture.writing to shadow copy of a file\(fileserver\)
diff --git a/source3/smbd/smb2_trans2.c b/source3/smbd/smb2_trans2.c
index b5d73bdf1fa5..57ee3512581f 100644
--- a/source3/smbd/smb2_trans2.c
+++ b/source3/smbd/smb2_trans2.c
@@ -80,6 +80,8 @@ NTSTATUS refuse_symlink_fsp(const files_struct *fsp)
 NTSTATUS check_any_access_fsp(struct files_struct *fsp,
 			      uint32_t access_requested)
 {
+	const uint32_t ro_access = SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
+	uint32_t ro_access_granted = 0;
 	uint32_t access_granted = 0;
 	NTSTATUS status;
 
@@ -100,15 +102,31 @@ NTSTATUS check_any_access_fsp(struct files_struct *fsp,
 							false,
 							mask);
 			if (NT_STATUS_IS_OK(status)) {
-				break;
+				access_granted |= mask;
+				if (fsp->fsp_name->twrp == 0) {
+					/*
+					 * We can only optimize
+					 * the non-snapshot case
+					 */
+					break;
+				}
 			}
 			mask <<= 1;
 		}
-		access_granted = mask;
 	}
 	if ((access_granted & access_requested) == 0) {
 		return NT_STATUS_ACCESS_DENIED;
 	}
+
+	if (fsp->fsp_name->twrp == 0) {
+		return NT_STATUS_OK;
+	}
+
+	ro_access_granted = access_granted & ro_access;
+	if ((ro_access_granted & access_requested) == 0) {
+		return NT_STATUS_MEDIA_WRITE_PROTECTED;
+	}
+
 	return NT_STATUS_OK;
 }
 
-- 
2.43.0


From dfdcbcaa4d060484337b72d8da5c72c7d869863c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Fri, 15 Dec 2023 11:59:36 +0100
Subject: [PATCH 13/13] smbd: move access override for previous versions to the
 SMB layer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Doing the previous version access checks and semantics at the SMB
layer means we can simplify the shadow_copy2 and remove the kludge.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Mon Jan  8 16:58:26 UTC 2024 on atb-devel-224

(backported from commit f14a7065690b00e3c6af2c1f0b0aec51c1e0b372)
[slow@samba.org: vfs_shadow_copy2.c: no TALLOC_FREE() in context]
[slow@samba.org: open.c: assign result from calculate_open_access_flags()]
---
 source3/modules/vfs_shadow_copy2.c | 30 ++++------------------
 source3/smbd/open.c                | 40 ++++++++++++++++++++++++++++--
 2 files changed, 43 insertions(+), 27 deletions(-)

diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c
index a2c9d3ce4c98..5bd5b87fb235 100644
--- a/source3/modules/vfs_shadow_copy2.c
+++ b/source3/modules/vfs_shadow_copy2.c
@@ -1556,7 +1556,6 @@ static int shadow_copy2_openat(vfs_handle_struct *handle,
 	struct smb_filename *smb_fname = NULL;
 	time_t timestamp = 0;
 	char *stripped = NULL;
-	bool is_converted = false;
 	int saved_errno = 0;
 	int ret;
 	bool ok;
@@ -1573,26 +1572,15 @@ static int shadow_copy2_openat(vfs_handle_struct *handle,
 		return -1;
 	}
 
-	ok = shadow_copy2_strip_snapshot_converted(talloc_tos(),
-						   handle,
-						   smb_fname,
-						   &timestamp,
-						   &stripped,
-						   &is_converted);
+	ok = shadow_copy2_strip_snapshot(talloc_tos(),
+					 handle,
+					 smb_fname,
+					 &timestamp,
+					 &stripped);
 	if (!ok) {
 		return -1;
 	}
 	if (timestamp == 0) {
-		if (is_converted) {
-			/*
-			 * Just pave over the user requested mode and use
-			 * O_RDONLY. Later attempts by the client to write on
-			 * the handle will fail in the pwrite() syscall with
-			 * EINVAL which we carefully map to EROFS. In sum, this
-			 * matches Windows behaviour.
-			 */
-			how.flags &= ~(O_WRONLY | O_RDWR | O_CREAT);
-		}
 		return SMB_VFS_NEXT_OPENAT(handle,
 					   dirfsp,
 					   smb_fname_in,
@@ -1613,14 +1601,6 @@ static int shadow_copy2_openat(vfs_handle_struct *handle,
 	}
 	TALLOC_FREE(stripped);
 
-	/*
-	 * Just pave over the user requested mode and use O_RDONLY. Later
-	 * attempts by the client to write on the handle will fail in the
-	 * pwrite() syscall with EINVAL which we carefully map to EROFS. In sum,
-	 * this matches Windows behaviour.
-	 */
-	how.flags &= ~(O_WRONLY | O_RDWR | O_CREAT);
-
 	ret = SMB_VFS_NEXT_OPENAT(handle,
 				  dirfsp,
 				  smb_fname,
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index e804d0969077..edc72bb03f07 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -3672,7 +3672,8 @@ static int disposition_to_open_flags(uint32_t create_disposition)
 }
 
 static int calculate_open_access_flags(uint32_t access_mask,
-				       uint32_t private_flags)
+				       uint32_t private_flags,
+				       NTTIME twrp)
 {
 	bool need_write, need_read;
 
@@ -3681,6 +3682,15 @@ static int calculate_open_access_flags(uint32_t access_mask,
 	 * mean the same thing under DOS and Unix.
 	 */
 
+	if (twrp != 0) {
+		/*
+		 * Pave over the user requested mode and force O_RDONLY for the
+		 * file handle. Windows allows opening a VSS file with O_RDWR,
+		 * even though actual writes on the handle will fail.
+		 */
+		return O_RDONLY;
+	}
+
 	need_write = (access_mask & (FILE_WRITE_DATA | FILE_APPEND_DATA));
 	if (!need_write) {
 		return O_RDONLY;
@@ -3811,6 +3821,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	bool posix_open = False;
 	bool new_file_created = False;
 	bool first_open_attempt = true;
+	bool is_twrp = (smb_fname_atname->twrp != 0);
 	NTSTATUS fsp_open = NT_STATUS_ACCESS_DENIED;
 	mode_t new_unx_mode = (mode_t)0;
 	mode_t unx_mode = (mode_t)0;
@@ -3968,6 +3979,9 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 					 smb_fname_str_dbg(smb_fname) ));
 				return NT_STATUS_OBJECT_NAME_NOT_FOUND;
 			}
+			if (is_twrp) {
+				return NT_STATUS_MEDIA_WRITE_PROTECTED;
+			}
 			break;
 
 		case FILE_CREATE:
@@ -3983,11 +3997,24 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 				}
 				return NT_STATUS_OBJECT_NAME_COLLISION;
 			}
+			if (is_twrp) {
+				return NT_STATUS_MEDIA_WRITE_PROTECTED;
+			}
 			break;
 
 		case FILE_SUPERSEDE:
 		case FILE_OVERWRITE_IF:
+			if (is_twrp) {
+				return NT_STATUS_MEDIA_WRITE_PROTECTED;
+			}
+			break;
 		case FILE_OPEN_IF:
+			if (is_twrp) {
+				if (!file_existed) {
+					return NT_STATUS_MEDIA_WRITE_PROTECTED;
+				}
+				create_disposition = FILE_OPEN;
+			}
 			break;
 		default:
 			return NT_STATUS_INVALID_PARAMETER;
@@ -4060,7 +4087,9 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	 * mean the same thing under DOS and Unix.
 	 */
 
-	flags = calculate_open_access_flags(access_mask, private_flags);
+	flags = calculate_open_access_flags(access_mask,
+					    private_flags,
+					    smb_fname->twrp);
 
 	/*
 	 * Currently we only look at FILE_WRITE_THROUGH for create options.
@@ -4788,6 +4817,10 @@ static NTSTATUS open_directory(connection_struct *conn,
 				return status;
 			}
 
+			if (smb_fname_atname->twrp != 0) {
+				return NT_STATUS_MEDIA_WRITE_PROTECTED;
+			}
+
 			status = mkdir_internal(conn,
 						parent_dir_fname,
 						smb_fname_atname,
@@ -4816,6 +4849,9 @@ static NTSTATUS open_directory(connection_struct *conn,
 				status = NT_STATUS_OK;
 				info = FILE_WAS_OPENED;
 			} else {
+				if (smb_fname_atname->twrp != 0) {
+					return NT_STATUS_MEDIA_WRITE_PROTECTED;
+				}
 				status = mkdir_internal(conn,
 							parent_dir_fname,
 							smb_fname_atname,
-- 
2.43.0