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>
(backported from commit 02ed99343d19fd0845531ad99a46b1dd5b8a7a4f)
[slow@samba.org: vfs_acl_common.c: different chown_needed check]

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 81e1116b20b02551b9c948411415f118b1052a2c..cd987b47f0a3dfd447c6aead9bde8117fc80fe14 100644 (file)
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -742,10 +742,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 == false ||
-           !(fsp->access_mask & SEC_STD_WRITE_OWNER)) {
+       if (get_current_uid(handle->conn) == 0 || 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/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 eb263132adf7f2cbb1cafb0bb4c3d047c5e67201..4f4ea42405b3965df318918458f869b074900ec6 100644 (file)
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -233,11 +233,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 5a88cd059b040a7f3fb53fb9b6cda12d0e7d5c6c..f0d66557ded8ef23ea483599c16b3537075699bb 100644 (file)
--- a/source3/smbd/dosmode.c
+++ b/source3/smbd/dosmode.c
@@ -1086,15 +1086,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 c18ad0027b67703d584d79db0aa3bd6157107930..68f8539beeee284334fd061f09a03dc60fa234b5 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 9129005dcaeb4dfa250cfdd3068a9b8fc8a1fad5..740fb5fa9f787f43ce9f57cdb16b37e0b18301e2 100644 (file)
--- a/source3/smbd/smb1_reply.c
+++ b/source3/smbd/smb1_reply.c
@@ -6657,8 +6657,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 6d1c3022c21e58917cad01d7539d5657fe2fc381..3bd0b2a93bd6436b17cf4406340a1c4038617c5d 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 f5156147bc3cf681114b8804111524d7c06f2903..cc6f39548a9aa863bff631b927bd15696787adbd 100644 (file)
--- a/source3/smbd/smb2_getinfo.c
+++ b/source3/smbd/smb2_getinfo.c
@@ -316,15 +316,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 84defa3f05288b28be067de04d36179d7536bf95..54186f8508739d453b8ca48319f230712e9be74e 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 a4055c00a312add93927c715c66ebdc6722b1336..8d8d8e0bc715655ccb68cf62f1269d91ddcda589 100644 (file)
--- a/source3/smbd/smb2_reply.c
+++ b/source3/smbd/smb2_reply.c
@@ -1156,6 +1156,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;
@@ -1199,11 +1201,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 03639f218db79935e51b94cecc1fec4a9d92f520..b5d73bdf1fa5573a78b17c5638e32b11928b5188 100644 (file)
--- a/source3/smbd/smb2_trans2.c
+++ b/source3/smbd/smb2_trans2.c
@@ -4167,8 +4167,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) {
@@ -4981,8 +4982,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. */