From 6951ac05d32096586453cba9d7c3640e90432de4 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 20 Aug 2020 16:18:35 +0200 Subject: [PATCH 1/2] vfs_zfsacl: use handle based facl() call to query ZFS filesytem ACL BUG: https://bugzilla.samba.org/show_bug.cgi?id=14470 Pair-Programmed-With: Andrew Walker Signed-off-by: Ralph Boehme Signed-off-by: Andrew Walker Reviewed-by: Jeremy Allison (cherry picked from commit f763b1e43640082af80c855a4a519f7747a6c87c) --- source3/modules/vfs_zfsacl.c | 159 +++++++++++++++++++++++++++-------- 1 file changed, 122 insertions(+), 37 deletions(-) diff --git a/source3/modules/vfs_zfsacl.c b/source3/modules/vfs_zfsacl.c index 9142a07aadd..b0a93aa410e 100644 --- a/source3/modules/vfs_zfsacl.c +++ b/source3/modules/vfs_zfsacl.c @@ -49,11 +49,12 @@ struct zfsacl_config_data { static NTSTATUS zfs_get_nt_acl_common(struct connection_struct *conn, TALLOC_CTX *mem_ctx, const struct smb_filename *smb_fname, + const ace_t *acebuf, + int naces, struct SMB4ACL_T **ppacl, struct zfsacl_config_data *config) { - int naces, i; - ace_t *acebuf; + int i; struct SMB4ACL_T *pacl; SMB_STRUCT_STAT sbuf; const SMB_STRUCT_STAT *psbuf = NULL; @@ -76,30 +77,8 @@ static NTSTATUS zfs_get_nt_acl_common(struct connection_struct *conn, } is_dir = S_ISDIR(psbuf->st_ex_mode); - /* read the number of file aces */ - if((naces = acl(smb_fname->base_name, ACE_GETACLCNT, 0, NULL)) == -1) { - if(errno == ENOSYS) { - DEBUG(9, ("acl(ACE_GETACLCNT, %s): Operation is not " - "supported on the filesystem where the file " - "reside\n", smb_fname->base_name)); - } else { - DEBUG(9, ("acl(ACE_GETACLCNT, %s): %s ", smb_fname->base_name, - strerror(errno))); - } - return map_nt_error_from_unix(errno); - } - /* allocate the field of ZFS aces */ mem_ctx = talloc_tos(); - acebuf = (ace_t *) talloc_size(mem_ctx, sizeof(ace_t)*naces); - if(acebuf == NULL) { - return NT_STATUS_NO_MEMORY; - } - /* read the aces into the field */ - if(acl(smb_fname->base_name, ACE_GETACL, naces, acebuf) < 0) { - DEBUG(9, ("acl(ACE_GETACL, %s): %s ", smb_fname->base_name, - strerror(errno))); - return map_nt_error_from_unix(errno); - } + /* create SMB4ACL data */ if((pacl = smb_create_smb4acl(mem_ctx)) == NULL) { return NT_STATUS_NO_MEMORY; @@ -163,7 +142,7 @@ static NTSTATUS zfs_get_nt_acl_common(struct connection_struct *conn, static bool zfs_process_smbacl(vfs_handle_struct *handle, files_struct *fsp, struct SMB4ACL_T *smbacl) { - int naces = smb_get_naces(smbacl), i; + int naces = smb_get_naces(smbacl), i, rv; ace_t *acebuf; struct SMB4ACE_T *smbace; TALLOC_CTX *mem_ctx; @@ -222,7 +201,13 @@ static bool zfs_process_smbacl(vfs_handle_struct *handle, files_struct *fsp, SMB_ASSERT(i == naces); /* store acl */ - if(acl(fsp->fsp_name->base_name, ACE_SETACL, naces, acebuf)) { + if (fsp->fh->fd != -1) { + rv = facl(fsp->fh->fd, ACE_SETACL, naces, acebuf); + } + else { + rv = acl(fsp->fsp_name->base_name, ACE_SETACL, naces, acebuf); + } + if (rv != 0) { if(errno == ENOSYS) { DEBUG(9, ("acl(ACE_SETACL, %s): Operation is not " "supported on the filesystem where the file " @@ -231,7 +216,7 @@ static bool zfs_process_smbacl(vfs_handle_struct *handle, files_struct *fsp, DEBUG(9, ("acl(ACE_SETACL, %s): %s ", fsp_str_dbg(fsp), strerror(errno))); } - return 0; + return false; } return True; @@ -259,6 +244,81 @@ static NTSTATUS zfs_set_nt_acl(vfs_handle_struct *handle, files_struct *fsp, zfs_process_smbacl); } +static int get_zfsacl(TALLOC_CTX *mem_ctx, + const struct smb_filename *smb_fname, + ace_t **outbuf) +{ + int naces, rv; + ace_t *acebuf = NULL; + + naces = acl(smb_fname->base_name, ACE_GETACLCNT, 0, NULL); + if (naces == -1) { + int dbg_level = 10; + + if (errno == ENOSYS) { + dbg_level = 1; + } + DEBUG(dbg_level, ("acl(ACE_GETACLCNT, %s): %s ", + smb_fname->base_name, strerror(errno))); + return naces; + } + acebuf = talloc_size(mem_ctx, sizeof(ace_t)*naces); + if (acebuf == NULL) { + errno = ENOMEM; + return -1; + } + + rv = acl(smb_fname->base_name, ACE_GETACL, naces, acebuf); + if (rv == -1) { + DBG_DEBUG("acl(ACE_GETACL, %s) failed: %s ", + smb_fname->base_name, strerror(errno)); + return -1; + } + + *outbuf = acebuf; + return naces; +} + +static int fget_zfsacl(TALLOC_CTX *mem_ctx, + struct files_struct *fsp, + ace_t **outbuf) +{ + int naces, rv; + ace_t *acebuf = NULL; + + if (fsp->fh->fd == -1) { + return get_zfsacl(mem_ctx, fsp->fsp_name, outbuf); + } + + naces = facl(fsp->fh->fd, ACE_GETACLCNT, 0, NULL); + if (naces == -1) { + int dbg_level = 10; + + if (errno == ENOSYS) { + dbg_level = 1; + } + DEBUG(dbg_level, ("facl(ACE_GETACLCNT, %s): %s ", + fsp_str_dbg(fsp), strerror(errno))); + return naces; + } + + acebuf = talloc_size(mem_ctx, sizeof(ace_t)*naces); + if (acebuf == NULL) { + errno = ENOMEM; + return -1; + } + + rv = facl(fsp->fh->fd, ACE_GETACL, naces, acebuf); + if (rv == -1) { + DBG_DEBUG("acl(ACE_GETACL, %s): %s ", + fsp_str_dbg(fsp), strerror(errno)); + return -1; + } + + *outbuf = acebuf; + return naces; +} + static NTSTATUS zfsacl_fget_nt_acl(struct vfs_handle_struct *handle, struct files_struct *fsp, uint32_t security_info, @@ -268,6 +328,8 @@ static NTSTATUS zfsacl_fget_nt_acl(struct vfs_handle_struct *handle, struct SMB4ACL_T *pacl; NTSTATUS status; struct zfsacl_config_data *config = NULL; + ace_t *acebuf = NULL; + int naces; SMB_VFS_HANDLE_GET_DATA(handle, config, struct zfsacl_config_data, @@ -275,9 +337,9 @@ static NTSTATUS zfsacl_fget_nt_acl(struct vfs_handle_struct *handle, TALLOC_CTX *frame = talloc_stackframe(); - status = zfs_get_nt_acl_common(handle->conn, frame, - fsp->fsp_name, &pacl, config); - if (!NT_STATUS_IS_OK(status)) { + naces = fget_zfsacl(talloc_tos(), fsp, &acebuf); + if (naces == -1) { + status = map_nt_error_from_unix(errno); TALLOC_FREE(frame); if (!NT_STATUS_EQUAL(status, NT_STATUS_NOT_SUPPORTED)) { return status; @@ -295,6 +357,18 @@ static NTSTATUS zfsacl_fget_nt_acl(struct vfs_handle_struct *handle, return NT_STATUS_OK; } + status = zfs_get_nt_acl_common(handle->conn, + frame, + fsp->fsp_name, + acebuf, + naces, + &pacl, + config); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return status; + } + status = smb_fget_nt_acl_nfs4(fsp, NULL, security_info, mem_ctx, ppdesc, pacl); TALLOC_FREE(frame); @@ -312,6 +386,8 @@ static NTSTATUS zfsacl_get_nt_acl_at(struct vfs_handle_struct *handle, NTSTATUS status; struct zfsacl_config_data *config = NULL; TALLOC_CTX *frame = NULL; + int naces; + ace_t *acebuf = NULL; SMB_ASSERT(dirfsp == handle->conn->cwd_fsp); @@ -322,12 +398,9 @@ static NTSTATUS zfsacl_get_nt_acl_at(struct vfs_handle_struct *handle, frame = talloc_stackframe(); - status = zfs_get_nt_acl_common(handle->conn, - frame, - smb_fname, - &pacl, - config); - if (!NT_STATUS_IS_OK(status)) { + naces = get_zfsacl(frame, smb_fname, &acebuf); + if (naces == -1) { + status = map_nt_error_from_unix(errno); TALLOC_FREE(frame); if (!NT_STATUS_EQUAL(status, NT_STATUS_NOT_SUPPORTED)) { return status; @@ -351,6 +424,18 @@ static NTSTATUS zfsacl_get_nt_acl_at(struct vfs_handle_struct *handle, return NT_STATUS_OK; } + status = zfs_get_nt_acl_common(handle->conn, + frame, + smb_fname, + acebuf, + naces, + &pacl, + config); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return status; + } + status = smb_get_nt_acl_nfs4(handle->conn, smb_fname, NULL, -- 2.26.2 From 6fec6c4bba292ab6b6fa3f5c7ddad54fa7da02b3 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Thu, 24 Sep 2020 11:42:16 -0400 Subject: [PATCH 2/2] vfs_zfsacl: Add new parameter to stop automatic addition of special entries Prevent ZFS from automatically adding NFSv4 special entries (owner@, group@, everyone@). ZFS will automatically add these these entries when calculating the inherited ACL of new files if the ACL of the parent directory lacks an inheriting special entry. This may result in user confusion and unexpected change in permissions of files and directories as the inherited ACL is generated. Blocking this behavior is achieved by setting an inheriting everyone@ that grants no permissions and not adding the entry to the file's Security Descriptor. This change also updates behavior so that the fd-based syscall facl() is used where possible. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14470 RN: vfs_zfsacl: Add new parameter to stop automatic addition of special entries Signed-off-by: Andrew Walker Reviewed-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit c10ae30c1185463eb937f69c1fc9914558087167) --- docs-xml/manpages/vfs_zfsacl.8.xml | 20 ++++++++++++++++++++ source3/modules/vfs_zfsacl.c | 24 ++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/docs-xml/manpages/vfs_zfsacl.8.xml b/docs-xml/manpages/vfs_zfsacl.8.xml index ae583409fe1..1ac954b9429 100644 --- a/docs-xml/manpages/vfs_zfsacl.8.xml +++ b/docs-xml/manpages/vfs_zfsacl.8.xml @@ -140,6 +140,26 @@ + + zfsacl:block_special = [yes|no] + + Prevent ZFS from automatically adding NFSv4 special + entries (owner@, group@, everyone@). ZFS will automatically + generate these these entries when calculating the inherited ACL + of new files if the ACL of the parent directory lacks an + inheriting special entry. This may result in user confusion and + unexpected change in permissions of files and directories as the + inherited ACL is generated. Blocking this behavior is achieved + by setting an inheriting everyone@ that grants no permissions + and not adding the entry to the file's Security + Descriptor + + yes (default) + no + + + + zfsacl:map_dacl_protected = [yes|no] diff --git a/source3/modules/vfs_zfsacl.c b/source3/modules/vfs_zfsacl.c index b0a93aa410e..c5c4718d6ce 100644 --- a/source3/modules/vfs_zfsacl.c +++ b/source3/modules/vfs_zfsacl.c @@ -40,6 +40,7 @@ struct zfsacl_config_data { struct smbacl4_vfs_params nfs4_params; bool zfsacl_map_dacl_protected; bool zfsacl_denymissingspecial; + bool zfsacl_block_special; }; /* zfs_get_nt_acl() @@ -57,6 +58,7 @@ static NTSTATUS zfs_get_nt_acl_common(struct connection_struct *conn, int i; struct SMB4ACL_T *pacl; SMB_STRUCT_STAT sbuf; + SMB_ACE4PROP_T blocking_ace; const SMB_STRUCT_STAT *psbuf = NULL; int ret; bool inherited_is_present = false; @@ -91,6 +93,13 @@ static NTSTATUS zfs_get_nt_acl_common(struct connection_struct *conn, aceprop.aceMask = (uint32_t) acebuf[i].a_access_mask; aceprop.who.id = (uint32_t) acebuf[i].a_who; + if (config->zfsacl_block_special && + (aceprop.aceMask == 0) && + (aceprop.aceFlags & ACE_EVERYONE) && + (aceprop.aceFlags & ACE_INHERITED_ACE)) + { + continue; + } /* * Windows clients expect SYNC on acls to correctly allow * rename, cf bug #7909. But not on DENY ace entries, cf bug @@ -147,12 +156,17 @@ static bool zfs_process_smbacl(vfs_handle_struct *handle, files_struct *fsp, struct SMB4ACE_T *smbace; TALLOC_CTX *mem_ctx; bool have_special_id = false; + bool must_add_empty_ace = false; struct zfsacl_config_data *config = NULL; SMB_VFS_HANDLE_GET_DATA(handle, config, struct zfsacl_config_data, return False); + if (config->zfsacl_block_special && S_ISDIR(fsp->fsp_name->st.st_ex_mode)) { + naces++; + must_add_empty_ace = true; + } /* allocate the field of ZFS aces */ mem_ctx = talloc_tos(); acebuf = (ace_t *) talloc_size(mem_ctx, sizeof(ace_t)*naces); @@ -192,6 +206,13 @@ static bool zfs_process_smbacl(vfs_handle_struct *handle, files_struct *fsp, have_special_id = true; } } + if (must_add_empty_ace) { + acebuf[i].a_type = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE; + acebuf[i].a_flags = SMB_ACE4_DIRECTORY_INHERIT_ACE| \ + SMB_ACE4_FILE_INHERIT_ACE|ACE_EVERYONE; + acebuf[i].a_access_mask = 0; + i++; + } if (!have_special_id && config->zfsacl_denymissingspecial) { errno = EACCES; @@ -560,6 +581,9 @@ static int zfsacl_connect(struct vfs_handle_struct *handle, config->zfsacl_denymissingspecial = lp_parm_bool(SNUM(handle->conn), "zfsacl", "denymissingspecial", false); + config->zfsacl_block_special = lp_parm_bool(SNUM(handle->conn), + "zfsacl", "block_special", true); + ret = smbacl4_get_vfs_params(handle->conn, &config->nfs4_params); if (ret < 0) { TALLOC_FREE(config); -- 2.26.2