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>

diff --git a/source3/include/smb_macros.h b/source3/include/smb_macros.h
index de39bf616e1e37b192624fbad0428fce21de204d..f1191ac011e7adfae9f919c14418df622d3f41ba 100644 (file)
--- 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 0bb34be454f76c47218adbf330d26ec377dc5255..bf2ea5a913814750a578b90940bbd784cdb70e49 100644 (file)
--- 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 180fac2b44b7ee5c66d341c6bbb1c871bc31db00..b90a8b3fe11040bd77c132b5f2bab4fe81d2154f 100644 (file)
--- 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 1c85840137e678570eeadb0e357b0bb4be84bfc6..d639bbf9285841ad729fc06ed90a2ca155aaeed8 100644 (file)
--- 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);
        }