From 60e3e10e84983da073fcc539378ec50f88e9f41c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 9 Jun 2017 13:02:49 +0200 Subject: [PATCH] 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 Reviewed-by: Stefan Metzmacher --- source3/include/smb_macros.h | 16 ++++++++++++++++ source3/smbd/smb2_glue.c | 15 --------------- source3/smbd/smb2_ioctl_network_fs.c | 2 +- source3/smbd/smb2_read.c | 2 +- 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); } -- 2.11.4.GIT