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>

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 692e776d10cd1f261c8c5ff76cfea507a6195a18..314fc79a3a68a4274f164ba0d067e214b3b2c5f1 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 800b7221cfd0b323ac886f64efd18b89222b95fc..49c37cbb4d9de9b7bbfa8489a05ecf272a127768 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 6fbcac40d2ec789efeb05de6f509c82ba272a6ad..1472af7d0591ad891f9e4b4f881061e6a72c39fa 100644 (file)
--- a/source3/smbd/dosmode.c
+++ b/source3/smbd/dosmode.c
@@ -1076,15 +1076,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 9ad9173e49ca186524e72b6870c03e7349f94aad..9928eb997507d175a1853560b75af024e979e1d0 100644 (file)
--- a/source3/smbd/file_access.c
+++ b/source3/smbd/file_access.c
@@ -192,6 +192,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.
         */
@@ -220,11 +221,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 e4994d2a79f4c5ab0ef33748e939dc3651105e87..b2f317a1db92c79befdcef7fa7710e99c3811859 100644 (file)
--- a/source3/smbd/smb1_reply.c
+++ b/source3/smbd/smb1_reply.c
@@ -6970,8 +6970,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 55dca5e8031858269fc34298d95530392b69234f..7c43a4e923d0c5568c7483dd55cfde4761654af2 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 bf5170f07d417bb59f490d82e283d389399655b2..49bddf5d0e2bfa17cc4d5b5e5f8a1ddf816a4e74 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) {
@@ -474,16 +479,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 f3cc48b7d56b0a0db24f827b2b2a7e8568fc5475..ab12399956913deb03ffe33b08a3f9044997dbbf 100644 (file)
--- a/source3/smbd/smb2_reply.c
+++ b/source3/smbd/smb2_reply.c
@@ -1142,6 +1142,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;
@@ -1185,11 +1187,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 b0383b3cc19cc65ca4cf05b6a1c60e3d4fb90abf..22e0f2f30200209cbd03cf90c0909d485fad27fd 100644 (file)
--- a/source3/smbd/smb2_trans2.c
+++ b/source3/smbd/smb2_trans2.c
@@ -4018,8 +4018,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) {
@@ -4922,8 +4923,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. */