[PATCH] Rework patch for bug 6133
Ralph Böhme
slow at samba.org
Thu Sep 7 10:27:53 UTC 2017
Hi!
Attached is a patch that moves ACE4_ADD_FILE -> ACE4_DELETE_CHILD mapping from
the NFSv4 framework to vfs_zfsacl.
This was added in e6a5f11865a55e9644292ae92e4a4b5ec0662ccd to adopt the NFSv4
framework to follow ZFS permission rules. But this is the wrong place, other
filesystems like GPFS do not allow deletion when the user has SEC_DIR_ADD_FILE,
so setting ACE4_DELETE_CHILD when the access_mask has ACE4_ADD_FILE is wrong:
# su -s /bin/bash FOO\\aduser1
bash-4.2$ mmgetacl .
#NFSv4 ACL
#owner:FOO\aduser1
#group:FOO\domain users
special:owner@:rwxc:allow:FileInherit:DirInherit:InheritOnly
(X)READ/LIST (X)WRITE/CREATE (X)APPEND/MKDIR (X)SYNCHRONIZE (X)READ_ACL (X)READ_ATTR (X)READ_NAMED
(X)DELETE (X)DELETE_CHILD (X)CHOWN (X)EXEC/SEARCH (X)WRITE_ACL (X)WRITE_ATTR (X)WRITE_NAMED
user:FOO\aduser1:rwx-:allow:FileInherit:DirInherit
(X)READ/LIST (X)WRITE/CREATE (X)APPEND/MKDIR (X)SYNCHRONIZE (X)READ_ACL (X)READ_ATTR (X)READ_NAMED
(-)DELETE (-)DELETE_CHILD (-)CHOWN (X)EXEC/SEARCH (-)WRITE_ACL (-)WRITE_ATTR (-)WRITE_NAMED
The ACL has an explicit ACE for the user that grants CREATE, but not
DELETE_CHILD, so the user can't delete the file:
bash-4.2$ rm file
rm: cannot remove ‘file’: Permission denied
bash-4.2$
Adding DELETE_CHILD allows the user to delete the file:
bash-4.2$ rm file
bash-4.2$
This patch therefor moves the change from the NFS4 framework into the ZFS
module.
Please review & push if ok. Thanks!
Cheerio!
-slow
-------------- next part --------------
From 9aebdfcc39b48d06e122570186554c5c302006be Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 6 Sep 2017 16:44:12 +0200
Subject: [PATCH 1/3] vfs_zfsacl: pass smb_fname to zfs_get_nt_acl_common
This is in preperation of moving SMB_ACE4_ADD_FILE /
SMB_ACE4_DELETE_CHILD mapping from the common NFSv4 framework into this
module excusively.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=6133
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_zfsacl.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/source3/modules/vfs_zfsacl.c b/source3/modules/vfs_zfsacl.c
index 4cb1b98f01b..97fe2028307 100644
--- a/source3/modules/vfs_zfsacl.c
+++ b/source3/modules/vfs_zfsacl.c
@@ -41,7 +41,7 @@
* using the NFSv4 format conversion
*/
static NTSTATUS zfs_get_nt_acl_common(TALLOC_CTX *mem_ctx,
- const char *name,
+ const struct smb_filename *smb_fname,
struct SMB4ACL_T **ppacl)
{
int naces, i;
@@ -49,13 +49,13 @@ static NTSTATUS zfs_get_nt_acl_common(TALLOC_CTX *mem_ctx,
struct SMB4ACL_T *pacl;
/* read the number of file aces */
- if((naces = acl(name, ACE_GETACLCNT, 0, NULL)) == -1) {
+ 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", name));
+ "reside\n", smb_fname->base_name));
} else {
- DEBUG(9, ("acl(ACE_GETACLCNT, %s): %s ", name,
+ DEBUG(9, ("acl(ACE_GETACLCNT, %s): %s ", smb_fname->base_name,
strerror(errno)));
}
return map_nt_error_from_unix(errno);
@@ -67,8 +67,8 @@ static NTSTATUS zfs_get_nt_acl_common(TALLOC_CTX *mem_ctx,
return NT_STATUS_NO_MEMORY;
}
/* read the aces into the field */
- if(acl(name, ACE_GETACL, naces, acebuf) < 0) {
- DEBUG(9, ("acl(ACE_GETACL, %s): %s ", name,
+ 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);
}
@@ -210,9 +210,7 @@ static NTSTATUS zfsacl_fget_nt_acl(struct vfs_handle_struct *handle,
NTSTATUS status;
TALLOC_CTX *frame = talloc_stackframe();
- status = zfs_get_nt_acl_common(frame,
- fsp->fsp_name->base_name,
- &pacl);
+ status = zfs_get_nt_acl_common(frame, fsp->fsp_name, &pacl);
if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(frame);
return status;
@@ -234,9 +232,7 @@ static NTSTATUS zfsacl_get_nt_acl(struct vfs_handle_struct *handle,
NTSTATUS status;
TALLOC_CTX *frame = talloc_stackframe();
- status = zfs_get_nt_acl_common(frame,
- smb_fname->base_name,
- &pacl);
+ status = zfs_get_nt_acl_common(frame, smb_fname, &pacl);
if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(frame);
return status;
--
2.13.5
From 8d6be0aa8ff81ed7645839ce7a37fd74201abdd0 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 6 Sep 2017 16:53:23 +0200
Subject: [PATCH 2/3] vfs_zfsacl: ensure zfs_get_nt_acl_common() has access to
stat info
We'll need this in the next commit.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=6133
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_zfsacl.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/source3/modules/vfs_zfsacl.c b/source3/modules/vfs_zfsacl.c
index 97fe2028307..da13c4c4908 100644
--- a/source3/modules/vfs_zfsacl.c
+++ b/source3/modules/vfs_zfsacl.c
@@ -40,13 +40,31 @@
* read the local file's acls and return it in NT form
* using the NFSv4 format conversion
*/
-static NTSTATUS zfs_get_nt_acl_common(TALLOC_CTX *mem_ctx,
+static NTSTATUS zfs_get_nt_acl_common(struct connection_struct *conn,
+ TALLOC_CTX *mem_ctx,
const struct smb_filename *smb_fname,
struct SMB4ACL_T **ppacl)
{
int naces, i;
ace_t *acebuf;
struct SMB4ACL_T *pacl;
+ SMB_STRUCT_STAT sbuf;
+ const SMB_STRUCT_STAT *psbuf = NULL;
+ int ret;
+
+ if (VALID_STAT(smb_fname->st)) {
+ psbuf = &smb_fname->st;
+ }
+
+ if (psbuf == NULL) {
+ ret = vfs_stat_smb_basename(conn, smb_fname, &sbuf);
+ if (ret != 0) {
+ DBG_INFO("stat [%s]failed: %s\n",
+ smb_fname_str_dbg(smb_fname), strerror(errno));
+ return map_nt_error_from_unix(errno);
+ }
+ psbuf = &sbuf;
+ }
/* read the number of file aces */
if((naces = acl(smb_fname->base_name, ACE_GETACLCNT, 0, NULL)) == -1) {
@@ -210,7 +228,8 @@ static NTSTATUS zfsacl_fget_nt_acl(struct vfs_handle_struct *handle,
NTSTATUS status;
TALLOC_CTX *frame = talloc_stackframe();
- status = zfs_get_nt_acl_common(frame, fsp->fsp_name, &pacl);
+ status = zfs_get_nt_acl_common(handle->conn, frame,
+ fsp->fsp_name, &pacl);
if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(frame);
return status;
@@ -232,7 +251,7 @@ static NTSTATUS zfsacl_get_nt_acl(struct vfs_handle_struct *handle,
NTSTATUS status;
TALLOC_CTX *frame = talloc_stackframe();
- status = zfs_get_nt_acl_common(frame, smb_fname, &pacl);
+ status = zfs_get_nt_acl_common(handle->conn, frame, smb_fname, &pacl);
if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(frame);
return status;
--
2.13.5
From 216abbcf5ea028a5091722d5d9024fe6728f5794 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 6 Sep 2017 16:56:47 +0200
Subject: [PATCH 3/3] s3/vfs: move ACE4_ADD_FILE/ACE4_DELETE_CHILD mapping from
NFSv4 framework to vfs_zfsacl
This was added in e6a5f11865a55e9644292ae92e4a4b5ec0662ccd to adopt the
NFSv4 framework to follow ZFS permission rules. But this is the wrong
place, other filesystems like GPFS do not allow deletion when the user
has SEC_DIR_ADD_FILE.
This patch therefor moves the change from the NFS4 framework into the
ZFS module.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=6133
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/nfs4_acls.c | 4 ----
source3/modules/vfs_zfsacl.c | 4 ++++
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c
index c715b83390d..19f0fefdb98 100644
--- a/source3/modules/nfs4_acls.c
+++ b/source3/modules/nfs4_acls.c
@@ -351,10 +351,6 @@ static bool smbacl4_nfs42win(TALLOC_CTX *mem_ctx,
DEBUG(10, ("mapped %d to %s\n", ace->who.id,
sid_string_dbg(&sid)));
- if (is_directory && (ace->aceMask & SMB_ACE4_ADD_FILE)) {
- ace->aceMask |= SMB_ACE4_DELETE_CHILD;
- }
-
if (!is_directory && params->map_full_control) {
/*
* Do we have all access except DELETE_CHILD
diff --git a/source3/modules/vfs_zfsacl.c b/source3/modules/vfs_zfsacl.c
index da13c4c4908..dd0f343b8c6 100644
--- a/source3/modules/vfs_zfsacl.c
+++ b/source3/modules/vfs_zfsacl.c
@@ -66,6 +66,10 @@ static NTSTATUS zfs_get_nt_acl_common(struct connection_struct *conn,
psbuf = &sbuf;
}
+ if (S_ISDIR(psbuf->st_ex_mode) && (ace->aceMask & SMB_ACE4_ADD_FILE)) {
+ ace->aceMask |= SMB_ACE4_DELETE_CHILD;
+ }
+
/* read the number of file aces */
if((naces = acl(smb_fname->base_name, ACE_GETACLCNT, 0, NULL)) == -1) {
if(errno == ENOSYS) {
--
2.13.5
More information about the samba-technical
mailing list