smbd: look only at handle readability for COPYCHUNK dest

This commits sets the stage for a change of behavior
in a later commit.

When checking FILE_READ_DATA on the COPYCHUNK dest handle,
only check the handle readability and not the extra right
that may have been added due to the FILE_EXECUTE right.

The check for FILE_READ_DATA always seemed strange for the
dest handle, which is not read. It turns out that in Windows,
this check is not done at the SMB layer, but at a lower layer
that processes the IOCTL request - the IOCTL code has bits
that specify what type of access check needs to be done.

Therefore, this lower layer is unaware of the SMB layer's
practice of granting READ access based on the FILE_EXECUTE
right, and it only checks the handle's readability.

This subtle difference has observable behavior - the
COPYCHUNK source handle can have FILE_EXECUTE right instead
of FILE_READ_DATA, but the dest handle cannot.

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

Signed-off-by: Uri Simchoni <uri@samba.org>
Reviewed-by: David Disseldorp <ddiss@samba.org>
Autobuild-User(master): David Disseldorp <ddiss@samba.org>
Autobuild-Date(master): Tue Aug 16 15:21:03 CEST 2016 on sn-devel-144

(cherry picked from commit 3e42b69d5e1216b6af570a09d58040d281bbbf17)

diff --git a/source3/include/smb_macros.h b/source3/include/smb_macros.h
index 42a97569e253ed5011c860443dc4bc9cc4b09aba..f8656c7301462f5cd5745dc6ad709e524f6100ef 100644 (file)
--- a/source3/include/smb_macros.h
+++ b/source3/include/smb_macros.h
@@ -56,6 +56,14 @@
                        ((req->flags2 & FLAGS2_READ_PERMIT_EXECUTE) && \
                         (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
+ * ). On Windows servers, this is done by the IO manager, which is unaware of
+ * the "if execute is granted then also grant read" arrangement.
+ */
+#define CHECK_READ_IOCTL(fsp, req) (((fsp)->fh->fd != -1) && ((fsp)->can_read))
+
 #define CHECK_WRITE(fsp) ((fsp)->can_write && ((fsp)->fh->fd != -1))
 
 #define ERROR_WAS_LOCK_DENIED(status) (NT_STATUS_EQUAL((status), NT_STATUS_LOCK_NOT_GRANTED) || \

diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c
index 01798ac8f829cb478553879d831c53409092f4af..9051bed59e674187bd543d20db8586af23711338 100644 (file)
--- a/source3/smbd/smb2_ioctl_network_fs.c
+++ b/source3/smbd/smb2_ioctl_network_fs.c
@@ -116,8 +116,8 @@ static NTSTATUS copychunk_check_handles(uint32_t ctl_code,
         * - The Open.GrantedAccess of the destination file does not include
         *   FILE_READ_DATA, and the CtlCode is FSCTL_SRV_COPYCHUNK.
         */
-       if ((ctl_code == FSCTL_SRV_COPYCHUNK)
-         && !CHECK_READ(dst_fsp, smb1req)) {
+       if ((ctl_code == FSCTL_SRV_COPYCHUNK) &&
+           !CHECK_READ_IOCTL(dst_fsp, smb1req)) {
                DEBUG(5, ("copy chunk no read on dest handle (%s).\n",
                        smb_fname_str_dbg(dst_fsp->fsp_name) ));
                return NT_STATUS_ACCESS_DENIED;