1
0
mirror of https://github.com/samba-team/samba.git synced 2025-01-06 13:18:07 +03:00

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 02ed99343d)
[slow@samba.org: vfs_acl_common.c: different chown_needed check]
This commit is contained in:
Ralph Boehme 2023-12-20 18:01:57 +01:00 committed by Jule Anger
parent 77a71bc993
commit d3f062e212
12 changed files with 82 additions and 54 deletions

View File

@ -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, /* We got access denied here. If we're already root,
or we didn't need to do a chown, or the fsp isn't or we didn't need to do a chown, or the fsp isn't
open with WRITE_OWNER access, just return. */ open with WRITE_OWNER access, just return. */
if (get_current_uid(handle->conn) == 0 || chown_needed == false || if (get_current_uid(handle->conn) == 0 || chown_needed == false) {
!(fsp->access_mask & SEC_STD_WRITE_OWNER)) {
return NT_STATUS_ACCESS_DENIED; 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 * Only allow take-ownership, not give-ownership. That's the way Windows

View File

@ -368,11 +368,14 @@ static NTSTATUS nfs4acl_xattr_fset_nt_acl(vfs_handle_struct *handle,
} }
if (get_current_uid(handle->conn) == 0 || if (get_current_uid(handle->conn) == 0 ||
chown_needed == false || chown_needed == false)
!(fsp->access_mask & SEC_STD_WRITE_OWNER))
{ {
return NT_STATUS_ACCESS_DENIED; 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 * Only allow take-ownership, not give-ownership. That's the way Windows

View File

@ -233,11 +233,12 @@ NTSTATUS dptr_create(connection_struct *conn,
return NT_STATUS_INVALID_PARAMETER; 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 " DBG_INFO("dptr_create: directory %s "
"not open for LIST access\n", "not open for LIST access\n",
fsp_str_dbg(fsp)); fsp_str_dbg(fsp));
return NT_STATUS_ACCESS_DENIED; return status;
} }
status = OpenDir_fsp(NULL, conn, fsp, wcard, attr, &dir_hnd); status = OpenDir_fsp(NULL, conn, fsp, wcard, attr, &dir_hnd);
if (!NT_STATUS_IS_OK(status)) { if (!NT_STATUS_IS_OK(status)) {

View File

@ -1086,15 +1086,17 @@ NTSTATUS file_set_sparse(connection_struct *conn,
* Windows Server 2008 & 2012 permit FSCTL_SET_SPARSE if any of the * Windows Server 2008 & 2012 permit FSCTL_SET_SPARSE if any of the
* following access flags are granted. * following access flags are granted.
*/ */
if ((fsp->access_mask & (FILE_WRITE_DATA status = check_any_access_fsp(fsp,
| FILE_WRITE_ATTRIBUTES FILE_WRITE_DATA
| SEC_FILE_APPEND_DATA)) == 0) { | FILE_WRITE_ATTRIBUTES
DEBUG(9,("file_set_sparse: fname[%s] set[%u] " | SEC_FILE_APPEND_DATA);
"access_mask[0x%08X] - access denied\n", if (!NT_STATUS_IS_OK(status)) {
smb_fname_str_dbg(fsp->fsp_name), DBG_DEBUG("fname[%s] set[%u] "
sparse, "access_mask[0x%08X] - access denied\n",
fsp->access_mask)); smb_fname_str_dbg(fsp->fsp_name),
return NT_STATUS_ACCESS_DENIED; sparse,
fsp->access_mask);
return status;
} }
if (fsp->fsp_flags.is_directory) { if (fsp->fsp_flags.is_directory) {

View File

@ -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 can_set_delete_on_close(files_struct *fsp, uint32_t dosmode)
{ {
NTSTATUS status;
/* /*
* Only allow delete on close for writable files. * 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. * intent.
*/ */
if (!(fsp->access_mask & DELETE_ACCESS)) { status = check_any_access_fsp(fsp, DELETE_ACCESS);
DEBUG(10,("can_set_delete_on_close: file %s delete on " if (!NT_STATUS_IS_OK(status)) {
DBG_DEBUG("file %s delete on "
"close flag set but delete access denied.\n", "close flag set but delete access denied.\n",
fsp_str_dbg(fsp))); fsp_str_dbg(fsp));
return NT_STATUS_ACCESS_DENIED; return status;
} }
/* Don't allow delete on close for non-empty directories. */ /* Don't allow delete on close for non-empty directories. */

View File

@ -295,8 +295,9 @@ NTSTATUS change_notify_create(struct files_struct *fsp,
* Setting a changenotify needs READ/LIST access * Setting a changenotify needs READ/LIST access
* on the directory handle. * on the directory handle.
*/ */
if (!(fsp->access_mask & SEC_DIR_LIST)) { status = check_any_access_fsp(fsp, SEC_DIR_LIST);
return NT_STATUS_ACCESS_DENIED; if (!NT_STATUS_IS_OK(status)) {
return status;
} }
if (fsp->notify != NULL) { if (fsp->notify != NULL) {

View File

@ -6657,8 +6657,9 @@ void reply_setattrE(struct smb_request *req)
goto out; goto out;
} }
if (!(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { status = check_any_access_fsp(fsp, FILE_WRITE_ATTRIBUTES);
reply_nterror(req, NT_STATUS_ACCESS_DENIED); if (!NT_STATUS_IS_OK(status)) {
reply_nterror(req, status);
goto out; goto out;
} }

View File

@ -128,6 +128,7 @@ static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx,
struct smb_request *smbreq; struct smb_request *smbreq;
bool is_compound = false; bool is_compound = false;
bool is_last_in_compound = false; bool is_last_in_compound = false;
NTSTATUS status;
req = tevent_req_create(mem_ctx, &state, req = tevent_req_create(mem_ctx, &state,
struct smbd_smb2_flush_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. * 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; allow_dir_flush = true;
} }

View File

@ -316,15 +316,15 @@ static struct tevent_req *smbd_smb2_getinfo_send(TALLOC_CTX *mem_ctx,
case FSCC_FILE_ALL_INFORMATION: case FSCC_FILE_ALL_INFORMATION:
case FSCC_FILE_NETWORK_OPEN_INFORMATION: case FSCC_FILE_NETWORK_OPEN_INFORMATION:
case FSCC_FILE_ATTRIBUTE_TAG_INFORMATION: case FSCC_FILE_ATTRIBUTE_TAG_INFORMATION:
if (!(fsp->access_mask & SEC_FILE_READ_ATTRIBUTE)) { status = check_any_access_fsp(fsp, SEC_FILE_READ_ATTRIBUTE);
tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); if (tevent_req_nterror(req, status)) {
return tevent_req_post(req, ev); return tevent_req_post(req, ev);
} }
break; break;
case FSCC_FILE_FULL_EA_INFORMATION: case FSCC_FILE_FULL_EA_INFORMATION:
if (!(fsp->access_mask & SEC_FILE_READ_EA)) { status = check_any_access_fsp(fsp, SEC_FILE_READ_EA);
tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); if (tevent_req_nterror(req, status)) {
return tevent_req_post(req, ev); return tevent_req_post(req, ev);
} }
break; break;

View File

@ -114,20 +114,23 @@ NTSTATUS set_sd(files_struct *fsp, struct security_descriptor *psd,
/* Ensure we have the rights to do this. */ /* Ensure we have the rights to do this. */
if (security_info_sent & SECINFO_OWNER) { if (security_info_sent & SECINFO_OWNER) {
if (!(fsp->access_mask & SEC_STD_WRITE_OWNER)) { status = check_any_access_fsp(fsp, SEC_STD_WRITE_OWNER);
return NT_STATUS_ACCESS_DENIED; if (!NT_STATUS_IS_OK(status)) {
return status;
} }
} }
if (security_info_sent & SECINFO_GROUP) { if (security_info_sent & SECINFO_GROUP) {
if (!(fsp->access_mask & SEC_STD_WRITE_OWNER)) { status = check_any_access_fsp(fsp, SEC_STD_WRITE_OWNER);
return NT_STATUS_ACCESS_DENIED; if (!NT_STATUS_IS_OK(status)) {
return status;
} }
} }
if (security_info_sent & SECINFO_DACL) { if (security_info_sent & SECINFO_DACL) {
if (!(fsp->access_mask & SEC_STD_WRITE_DAC)) { status = check_any_access_fsp(fsp, SEC_STD_WRITE_DAC);
return NT_STATUS_ACCESS_DENIED; if (!NT_STATUS_IS_OK(status)) {
return status;
} }
/* Convert all the generic bits. */ /* Convert all the generic bits. */
if (psd->dacl) { if (psd->dacl) {
@ -136,15 +139,17 @@ NTSTATUS set_sd(files_struct *fsp, struct security_descriptor *psd,
} }
if (security_info_sent & SECINFO_SACL) { if (security_info_sent & SECINFO_SACL) {
if (!(fsp->access_mask & SEC_FLAG_SYSTEM_SECURITY)) { status = check_any_access_fsp(fsp, SEC_FLAG_SYSTEM_SECURITY);
return NT_STATUS_ACCESS_DENIED; if (!NT_STATUS_IS_OK(status)) {
return status;
} }
/* /*
* Setting a SACL also requires WRITE_DAC. * Setting a SACL also requires WRITE_DAC.
* See the smbtorture3 SMB2-SACL test. * See the smbtorture3 SMB2-SACL test.
*/ */
if (!(fsp->access_mask & SEC_STD_WRITE_DAC)) { status = check_any_access_fsp(fsp, SEC_STD_WRITE_DAC);
return NT_STATUS_ACCESS_DENIED; if (!NT_STATUS_IS_OK(status)) {
return status;
} }
/* Convert all the generic bits. */ /* Convert all the generic bits. */
if (psd->sacl) { if (psd->sacl) {
@ -407,16 +412,20 @@ static NTSTATUS smbd_fetch_security_desc(connection_struct *conn,
* Get the permissions to return. * Get the permissions to return.
*/ */
if ((security_info_wanted & SECINFO_SACL) && if (security_info_wanted & SECINFO_SACL) {
!(fsp->access_mask & SEC_FLAG_SYSTEM_SECURITY)) { status = check_any_access_fsp(fsp, SEC_FLAG_SYSTEM_SECURITY);
DEBUG(10, ("Access to SACL denied.\n")); if (!NT_STATUS_IS_OK(status)) {
return NT_STATUS_ACCESS_DENIED; DBG_DEBUG("Access to SACL denied.\n");
return status;
}
} }
if ((security_info_wanted & (SECINFO_DACL|SECINFO_OWNER|SECINFO_GROUP)) && if (security_info_wanted & (SECINFO_DACL|SECINFO_OWNER|SECINFO_GROUP)) {
!(fsp->access_mask & SEC_STD_READ_CONTROL)) { status = check_any_access_fsp(fsp, SEC_STD_READ_CONTROL);
DEBUG(10, ("Access to DACL, OWNER, or GROUP denied.\n")); if (!NT_STATUS_IS_OK(status)) {
return NT_STATUS_ACCESS_DENIED; DBG_DEBUG("Access to DACL, OWNER, or GROUP denied.\n");
return status;
}
} }
status = refuse_symlink_fsp(fsp); status = refuse_symlink_fsp(fsp);

View File

@ -1156,6 +1156,8 @@ ssize_t sendfile_short_send(struct smbXsrv_connection *xconn,
static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp, static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp,
uint16_t dirtype) uint16_t dirtype)
{ {
NTSTATUS status;
if (fsp->fsp_name->twrp != 0) { if (fsp->fsp_name->twrp != 0) {
/* Get the error right, this is what Windows returns. */ /* Get the error right, this is what Windows returns. */
return NT_STATUS_NOT_SAME_DEVICE; return NT_STATUS_NOT_SAME_DEVICE;
@ -1199,11 +1201,11 @@ static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp,
return NT_STATUS_OK; return NT_STATUS_OK;
} }
if (fsp->access_mask & (DELETE_ACCESS|FILE_WRITE_ATTRIBUTES)) { status = check_any_access_fsp(fsp, DELETE_ACCESS | FILE_WRITE_ATTRIBUTES);
return NT_STATUS_OK; if (!NT_STATUS_IS_OK(status)) {
return status;
} }
return NT_STATUS_OK;
return NT_STATUS_ACCESS_DENIED;
} }
/**************************************************************************** /****************************************************************************

View File

@ -4167,8 +4167,9 @@ NTSTATUS smb_set_file_size(connection_struct *conn,
fsp_get_io_fd(fsp) != -1) fsp_get_io_fd(fsp) != -1)
{ {
/* Handle based call. */ /* Handle based call. */
if (!(fsp->access_mask & FILE_WRITE_DATA)) { status = check_any_access_fsp(fsp, FILE_WRITE_DATA);
return NT_STATUS_ACCESS_DENIED; if (!NT_STATUS_IS_OK(status)) {
return status;
} }
if (vfs_set_filelen(fsp, size) == -1) { 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) fsp_get_io_fd(fsp) != -1)
{ {
/* Open file handle. */ /* Open file handle. */
if (!(fsp->access_mask & FILE_WRITE_DATA)) { status = check_any_access_fsp(fsp, FILE_WRITE_DATA);
return NT_STATUS_ACCESS_DENIED; if (!NT_STATUS_IS_OK(status)) {
return status;
} }
/* Only change if needed. */ /* Only change if needed. */