From dedc6a69450fbacd01b18cf18d241acb2502d184 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 15 Jul 2016 17:56:02 +0200 Subject: [PATCH 1/2] s3/smbd: move make_default_filesystem_acl() to vfs_acl_common.c This function is only used in vfs_acl_common.c and will be modified in the next commit. Bug: https://bugzilla.samba.org/show_bug.cgi?id=12028 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit afc2417b107af572081974ff9d013ddec890d31f) --- source3/modules/vfs_acl_common.c | 108 ++++++++++++++++++++++++++++++++++++++ source3/smbd/posix_acls.c | 110 --------------------------------------- source3/smbd/proto.h | 4 -- 3 files changed, 108 insertions(+), 114 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index 76ac598..dc96761 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -358,6 +358,114 @@ static NTSTATUS add_directory_inheritable_components(vfs_handle_struct *handle, return NT_STATUS_OK; } +static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, + const char *name, + SMB_STRUCT_STAT *psbuf, + struct security_descriptor **ppdesc) +{ + struct dom_sid owner_sid, group_sid; + size_t size = 0; + struct security_ace aces[4]; + uint32_t access_mask = 0; + mode_t mode = psbuf->st_ex_mode; + struct security_acl *new_dacl = NULL; + int idx = 0; + + DEBUG(10,("make_default_filesystem_acl: file %s mode = 0%o\n", + name, (int)mode )); + + uid_to_sid(&owner_sid, psbuf->st_ex_uid); + gid_to_sid(&group_sid, psbuf->st_ex_gid); + + /* + We provide up to 4 ACEs + - Owner + - Group + - Everyone + - NT System + */ + + if (mode & S_IRUSR) { + if (mode & S_IWUSR) { + access_mask |= SEC_RIGHTS_FILE_ALL; + } else { + access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE; + } + } + if (mode & S_IWUSR) { + access_mask |= SEC_RIGHTS_FILE_WRITE | SEC_STD_DELETE; + } + + init_sec_ace(&aces[idx], + &owner_sid, + SEC_ACE_TYPE_ACCESS_ALLOWED, + access_mask, + 0); + idx++; + + access_mask = 0; + if (mode & S_IRGRP) { + access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE; + } + if (mode & S_IWGRP) { + /* note that delete is not granted - this matches posix behaviour */ + access_mask |= SEC_RIGHTS_FILE_WRITE; + } + if (access_mask) { + init_sec_ace(&aces[idx], + &group_sid, + SEC_ACE_TYPE_ACCESS_ALLOWED, + access_mask, + 0); + idx++; + } + + access_mask = 0; + if (mode & S_IROTH) { + access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE; + } + if (mode & S_IWOTH) { + access_mask |= SEC_RIGHTS_FILE_WRITE; + } + if (access_mask) { + init_sec_ace(&aces[idx], + &global_sid_World, + SEC_ACE_TYPE_ACCESS_ALLOWED, + access_mask, + 0); + idx++; + } + + init_sec_ace(&aces[idx], + &global_sid_System, + SEC_ACE_TYPE_ACCESS_ALLOWED, + SEC_RIGHTS_FILE_ALL, + 0); + idx++; + + new_dacl = make_sec_acl(ctx, + NT4_ACL_REVISION, + idx, + aces); + + if (!new_dacl) { + return NT_STATUS_NO_MEMORY; + } + + *ppdesc = make_sec_desc(ctx, + SECURITY_DESCRIPTOR_REVISION_1, + SEC_DESC_SELF_RELATIVE|SEC_DESC_DACL_PRESENT, + &owner_sid, + &group_sid, + NULL, + new_dacl, + &size); + if (!*ppdesc) { + return NT_STATUS_NO_MEMORY; + } + return NT_STATUS_OK; +} + /******************************************************************* Pull a DATA_BLOB from an xattr given a pathname. If the hash doesn't match, or doesn't exist - return the underlying diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 0c9c749..d87253d 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -4662,116 +4662,6 @@ NTSTATUS get_nt_acl_no_snum(TALLOC_CTX *ctx, const char *fname, return status; } -/* Stolen shamelessly from pvfs_default_acl() in source4 :-). */ - -NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, - const char *name, - SMB_STRUCT_STAT *psbuf, - struct security_descriptor **ppdesc) -{ - struct dom_sid owner_sid, group_sid; - size_t size = 0; - struct security_ace aces[4]; - uint32_t access_mask = 0; - mode_t mode = psbuf->st_ex_mode; - struct security_acl *new_dacl = NULL; - int idx = 0; - - DEBUG(10,("make_default_filesystem_acl: file %s mode = 0%o\n", - name, (int)mode )); - - uid_to_sid(&owner_sid, psbuf->st_ex_uid); - gid_to_sid(&group_sid, psbuf->st_ex_gid); - - /* - We provide up to 4 ACEs - - Owner - - Group - - Everyone - - NT System - */ - - if (mode & S_IRUSR) { - if (mode & S_IWUSR) { - access_mask |= SEC_RIGHTS_FILE_ALL; - } else { - access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE; - } - } - if (mode & S_IWUSR) { - access_mask |= SEC_RIGHTS_FILE_WRITE | SEC_STD_DELETE; - } - - init_sec_ace(&aces[idx], - &owner_sid, - SEC_ACE_TYPE_ACCESS_ALLOWED, - access_mask, - 0); - idx++; - - access_mask = 0; - if (mode & S_IRGRP) { - access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE; - } - if (mode & S_IWGRP) { - /* note that delete is not granted - this matches posix behaviour */ - access_mask |= SEC_RIGHTS_FILE_WRITE; - } - if (access_mask) { - init_sec_ace(&aces[idx], - &group_sid, - SEC_ACE_TYPE_ACCESS_ALLOWED, - access_mask, - 0); - idx++; - } - - access_mask = 0; - if (mode & S_IROTH) { - access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE; - } - if (mode & S_IWOTH) { - access_mask |= SEC_RIGHTS_FILE_WRITE; - } - if (access_mask) { - init_sec_ace(&aces[idx], - &global_sid_World, - SEC_ACE_TYPE_ACCESS_ALLOWED, - access_mask, - 0); - idx++; - } - - init_sec_ace(&aces[idx], - &global_sid_System, - SEC_ACE_TYPE_ACCESS_ALLOWED, - SEC_RIGHTS_FILE_ALL, - 0); - idx++; - - new_dacl = make_sec_acl(ctx, - NT4_ACL_REVISION, - idx, - aces); - - if (!new_dacl) { - return NT_STATUS_NO_MEMORY; - } - - *ppdesc = make_sec_desc(ctx, - SECURITY_DESCRIPTOR_REVISION_1, - SEC_DESC_SELF_RELATIVE|SEC_DESC_DACL_PRESENT, - &owner_sid, - &group_sid, - NULL, - new_dacl, - &size); - if (!*ppdesc) { - return NT_STATUS_NO_MEMORY; - } - return NT_STATUS_OK; -} - int posix_sys_acl_blob_get_file(vfs_handle_struct *handle, const char *path_p, TALLOC_CTX *mem_ctx, diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 914951e..1ef84a0 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -764,10 +764,6 @@ bool set_unix_posix_acl(connection_struct *conn, files_struct *fsp, const char * NTSTATUS get_nt_acl_no_snum( TALLOC_CTX *ctx, const char *fname, uint32_t security_info_wanted, struct security_descriptor **sd); -NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, - const char *name, - SMB_STRUCT_STAT *psbuf, - struct security_descriptor **ppdesc); int posix_sys_acl_blob_get_file(vfs_handle_struct *handle, const char *path_p, TALLOC_CTX *mem_ctx, -- 2.7.4 From 562646812856e98c14fb004d741d433d7f55d6c7 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 15 Jul 2016 17:48:19 +0200 Subject: [PATCH 2/2] vfs_acl_xattr: objects without NT ACL xattr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even with "ignore system acls" set to "yes", for objects without NT ACL xattr we use the underlying filesystem permissions to construct an NT ACL. This can result in *very* unexpected permissions, eg: - a directory with the following ACL: $ ./bin/smbcacls -Uslow%pass //localhost/normal "" REVISION:1 CONTROL:SR|DP OWNER:SLOW\slow GROUP:Unix Group\root ACL:SLOW\slow:ALLOWED/0x0/FULL So only one non-inheritable(!) ACE. - creating a subdirectory: $ ./bin/smbclient -Uslow%pass //localhost/normal -c "mkdir dir1" - checking whether there's an ACL xattr: $ getfattr -m "" /Volumes/normal/dir1 getfattr: Removing leading '/' from absolute path names system.posix_acl_access system.posix_acl_default user.DOSATTRIB So there isn't an ACL xattr, because there where no inheritable ACEs on the parent folder. - reading the new subdirectories ACL: $ ./bin/smbcacls -Uslow%pass //localhost/normal "dir1" REVISION:1 CONTROL:SR|DP OWNER:SLOW\slow GROUP:Unix Group\slow ACL:SLOW\slow:ALLOWED/0x0/FULL ACL:Unix Group\slow:ALLOWED/0x0/READ ACL:Everyone:ALLOWED/0x0/READ ACL:NT Authority\SYSTEM:ALLOWED/0x0/FULL The ACES for "SLOW\slow", "Unix Group\slow" and "Everyone" are coming from the underlying filesystem. This is the problem. - Windows assigns the following ACL in this situation: $ ./bin/smbcacls -UAdministrator%Passw0rd //10.10.10.14/data "dir" REVISION:1 CONTROL:SR|PD|DI|DP OWNER:VORDEFINIERT\Administratoren GROUP:WIN2008R2\Domänen-Benutzer ACL:WIN2008R2\Administrator:ALLOWED/0x0/FULL $ ./bin/smbclient -UAdministrator%Passw0rd //10.10.10.14/data -c "mkdir dir\dir1" $ ./bin/smbcacls -UAdministrator%Passw0rd //10.10.10.14/data "dir\dir1" REVISION:1 CONTROL:SR|DI|DP OWNER:VORDEFINIERT\Administratoren GROUP:WIN2008R2\Domänen-Benutzer ACL:VORDEFINIERT\Administratoren:ALLOWED/0x0/FULL ACL:NT-AUTORITÄT\SYSTEM:ALLOWED/0x0/FULL By changing make_default_filesystem_acl() to only adds user and system ACE to the ACL of objects that lack an ACL xattr, we match Windows behaviour: $ ./bin/smbclient -Uslow%pass //localhost/normal -c "mkdir dir2" $ ./bin/smbcacls -Uslow%pass //localhost/normal "dir2" REVISION:1 CONTROL:SR|DP OWNER:SLOW\slow GROUP:Unix Group\slow ACL:SLOW\slow:ALLOWED/0x0/FULL ACL:NT Authority\SYSTEM:ALLOWED/0x0/FULL Bug: https://bugzilla.samba.org/show_bug.cgi?id=12028 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison Autobuild-User(master): Ralph Böhme Autobuild-Date(master): Tue Jul 19 10:22:05 CEST 2016 on sn-devel-144 (cherry picked from commit 961c4b591bb102751079d9cc92d7aa1c37f1958c) --- source3/modules/vfs_acl_common.c | 44 +++++----------------------------------- 1 file changed, 5 insertions(+), 39 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index dc96761..f5af666 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -24,6 +24,7 @@ #include "../libcli/security/security.h" #include "../librpc/gen_ndr/ndr_security.h" #include "../lib/util/bitmap.h" +#include "passdb/lookup_sid.h" static NTSTATUS create_acl_blob(const struct security_descriptor *psd, DATA_BLOB *pblob, @@ -378,12 +379,10 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, gid_to_sid(&group_sid, psbuf->st_ex_gid); /* - We provide up to 4 ACEs - - Owner - - Group - - Everyone - - NT System - */ + * We provide 2 ACEs: + * - Owner + * - NT System + */ if (mode & S_IRUSR) { if (mode & S_IWUSR) { @@ -403,39 +402,6 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, 0); idx++; - access_mask = 0; - if (mode & S_IRGRP) { - access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE; - } - if (mode & S_IWGRP) { - /* note that delete is not granted - this matches posix behaviour */ - access_mask |= SEC_RIGHTS_FILE_WRITE; - } - if (access_mask) { - init_sec_ace(&aces[idx], - &group_sid, - SEC_ACE_TYPE_ACCESS_ALLOWED, - access_mask, - 0); - idx++; - } - - access_mask = 0; - if (mode & S_IROTH) { - access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE; - } - if (mode & S_IWOTH) { - access_mask |= SEC_RIGHTS_FILE_WRITE; - } - if (access_mask) { - init_sec_ace(&aces[idx], - &global_sid_World, - SEC_ACE_TYPE_ACCESS_ALLOWED, - access_mask, - 0); - idx++; - } - init_sec_ace(&aces[idx], &global_sid_System, SEC_ACE_TYPE_ACCESS_ALLOWED, -- 2.7.4