s3/smbd: remove flags2 FLAGS2_READ_PERMIT_EXECUTE hack in the SMB2 code

By adding a SMB2 specific CHECK_READ_SMB2 macro called that always
grants read access if execute was granted, we can get rid of the flags2
hack.

All callers in the SMB2 code are converted to use the CHECK_READ_SMB2
macro.

Amongs other things, this later allows moving the handle checks in
copychunk_check_handles() down into the VFS layer where we don't have
access to the smbreq.

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

4 files changed, 18 insertions, 17 deletions

diff --git a/source3/include/smb_macros.h b/source3/include/smb_macros.h
index de39bf616e1..f1191ac011e 100644
--- a/source3/include/smb_macros.h
+++ b/source3/include/smb_macros.h
@@ -56,6 +56,22 @@
 			((req->flags2 & FLAGS2_READ_PERMIT_EXECUTE) && \
 			 (fsp->access_mask & FILE_EXECUTE))))
 
+/*
+ * This is not documented in revision 49 of [MS-SMB2] but should be added in a
+ * later revision (and torture test smb2.read.access as well as
+ * smb2.ioctl_copy_chunk_bad_access against Server 2012R2 confirms this)
+ *
+ * If FILE_EXECUTE is granted to a handle then the SMB2 server acts as if
+ * FILE_READ_DATA has also been granted. We must still keep the original granted
+ * mask, because with ioctl requests, access checks are made on the file handle,
+ * "below" the SMB2 server, and the object store below the SMB layer is not
+ * aware of this arrangement (see smb2.ioctl.copy_chunk_bad_access torture
+ * test).
+ */
+#define CHECK_READ_SMB2(fsp) \
+	(((fsp)->fh->fd != -1) && \
+	 ((fsp)->can_read || (fsp->access_mask & FILE_EXECUTE)))
+
 /* An IOCTL readability check (validating read access
  * when the IOCTL code requires it)
  * http://social.technet.microsoft.com/wiki/contents/articles/24653.decoding-io-control-codes-ioctl-fsctl-and-deviceiocodes-with-table-of-known-values.aspx
diff --git a/source3/smbd/smb2_glue.c b/source3/smbd/smb2_glue.c
index 0bb34be454f..bf2ea5a9138 100644
--- a/source3/smbd/smb2_glue.c
+++ b/source3/smbd/smb2_glue.c
@@ -49,21 +49,6 @@ struct smb_request *smbd_smb2_fake_smb_request(struct smbd_smb2_request *req)
 			 FLAGS2_LONG_PATH_COMPONENTS |
 			 FLAGS2_IS_LONG_NAME;
 
-	/* This is not documented in revision 49 of [MS-SMB2] but should be
-	 * added in a later revision (and torture test smb2.read.access
-	 * as well as smb2.ioctl_copy_chunk_bad_access against
-	 * Server 2012R2 confirms this)
-	 *
-	 * If FILE_EXECUTE is granted to a handle then the SMB2 server
-	 * acts as if FILE_READ_DATA has also been granted. We must still
-	 * keep the original granted mask, because with ioctl requests,
-	 * access checks are made on the file handle, "below" the SMB2
-	 * server, and the object store below the SMB layer is not aware
-	 * of this arrangement (see smb2.ioctl.copy_chunk_bad_access
-	 * torture test).
-	 */
-	smbreq->flags2 |= FLAGS2_READ_PERMIT_EXECUTE;
-
 	if (IVAL(inhdr, SMB2_HDR_FLAGS) & SMB2_HDR_FLAG_DFS) {
 		smbreq->flags2 |= FLAGS2_DFS_PATHNAMES;
 	}
diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c
index 180fac2b44b..b90a8b3fe11 100644
--- a/source3/smbd/smb2_ioctl_network_fs.c
+++ b/source3/smbd/smb2_ioctl_network_fs.c
@@ -125,7 +125,7 @@ static NTSTATUS copychunk_check_handles(uint32_t ctl_code,
 	 * - The Open.GrantedAccess of the source file does not include
 	 *   FILE_READ_DATA access.
 	 */
-	if (!CHECK_READ(src_fsp, smb1req)) {
+	if (!CHECK_READ_SMB2(src_fsp)) {
 		DEBUG(5, ("copy chunk no read on src handle (%s).\n",
 			smb_fname_str_dbg(src_fsp->fsp_name) ));
 		return NT_STATUS_ACCESS_DENIED;
diff --git a/source3/smbd/smb2_read.c b/source3/smbd/smb2_read.c
index 1c85840137e..d639bbf9285 100644
--- a/source3/smbd/smb2_read.c
+++ b/source3/smbd/smb2_read.c
@@ -508,7 +508,7 @@ static struct tevent_req *smbd_smb2_read_send(TALLOC_CTX *mem_ctx,
 		return req;
 	}
 
-	if (!CHECK_READ(fsp, smbreq)) {
+	if (!CHECK_READ_SMB2(fsp)) {
 		tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
 		return tevent_req_post(req, ev);
 	}