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>
(cherry picked from commit 02ed99343d19fd0845531ad99a46b1dd5b8a7a4f)

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 341806b09a40309dc2f1f75b395c80798fe895f2..8f611c485aef9c250be360ed837b17067203eb79 100644 (file)
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -738,10 +738,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 ||
-           !(fsp->access_mask & SEC_STD_WRITE_OWNER)) {
+       if (get_current_uid(handle->conn) == 0 || !chown_needed) {
                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 cecafcf50b8d85b9ad4fa273060a3fc97ffa2c16..1fd3519ca020c889828dfb4edf2d53d461101651 100644 (file)
--- 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 59fbe54850136962891dbbdfe9bfaa2e8e604b5a..6cdbec19a54fff30a8c81c3c0a228fa549d10ff8 100644 (file)
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -218,11 +218,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 18d2322cf0e7d22e24124ed59a2ee5529c073693..5402180cf268055c16351f465f52919e66275a54 100644 (file)
--- a/source3/smbd/dosmode.c
+++ b/source3/smbd/dosmode.c
@@ -1068,15 +1068,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 476aead590dcd3399e67aab21ba790b2914d1aa7..7c4c7156d2458df125f969222f6eb00d463e0446 100644 (file)
--- 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 399d7800249c74018e90160eefd25d47de7f8bd0..156fbf700c3e718c605d03f3da4db6bca08cd72b 100644 (file)
--- 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 92cc7bf7dd118f264fac1e6200b2e320ba5b588a..9fbfc39c69fcc31464a4a11d79720b27a6a0838a 100644 (file)
--- a/source3/smbd/smb1_reply.c
+++ b/source3/smbd/smb1_reply.c
@@ -6965,8 +6965,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 3fec7908a46284e3f6c6452a016b0703e647ed2e..35f545ae3bdee44c231daabee8b9ac265b07b074 100644 (file)
--- 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 51283eb174e79f31ff86f7de256c881ad5ba4d73..d3067a53ab942f3bccc979b944ec854a2854deff 100644 (file)
--- a/source3/smbd/smb2_getinfo.c
+++ b/source3/smbd/smb2_getinfo.c
@@ -315,15 +315,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 1312d9bfc363fec5719750d255f9b74be63532d1..c58cfadcf925dec88a2bb6ae6e6bb133c7294adc 100644 (file)
--- 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 a7c63c0df58e3999f437e5ddf7f27987112a86e6..4d92e83c57aecfdc140c80ce8147264ae0b9722f 100644 (file)
--- a/source3/smbd/smb2_reply.c
+++ b/source3/smbd/smb2_reply.c
@@ -1140,6 +1140,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;
@@ -1183,11 +1185,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 2a90629ce61f9df2224cf79d776e507fdeeb7c9f..d39f5880efab85540c26702f82b5b1e6a1d85977 100644 (file)
--- a/source3/smbd/smb2_trans2.c
+++ b/source3/smbd/smb2_trans2.c
@@ -4051,8 +4051,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) {
@@ -4962,8 +4963,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. */