The Samba-Bugzilla – Attachment 12448 Details for
Bug 12177
Unexpected synthesized default ACL from vfs_acl_xattr
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for 4.3, backported from master
v43-bug12177.patch (text/plain), 74.82 KB, created by
Ralph Böhme
on 2016-09-06 16:13:30 UTC
(
hide
)
Description:
Patch for 4.3, backported from master
Filename:
MIME Type:
Creator:
Ralph Böhme
Created:
2016-09-06 16:13:30 UTC
Size:
74.82 KB
patch
obsolete
>From 6ce9125af72dad28c03e1a6f71d1403905223448 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 24 Aug 2016 10:04:24 +0200 >Subject: [PATCH 01/13] Revert "vfs_acl_xattr: objects without NT ACL xattr" > >This reverts commit 961c4b591bb102751079d9cc92d7aa1c37f1958c. > >Subsequent commits will add the same functionality as an optional >feature. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit 590b80490c00587b5a4035856891e10defb654f6) >--- > source3/modules/vfs_acl_common.c | 43 ++++++++++++++++++++++++++++++++++++---- > 1 file changed, 39 insertions(+), 4 deletions(-) > >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index f5af666..85f6c65 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -379,10 +379,12 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, > gid_to_sid(&group_sid, psbuf->st_ex_gid); > > /* >- * We provide 2 ACEs: >- * - Owner >- * - NT System >- */ >+ We provide up to 4 ACEs >+ - Owner >+ - Group >+ - Everyone >+ - NT System >+ */ > > if (mode & S_IRUSR) { > if (mode & S_IWUSR) { >@@ -402,6 +404,39 @@ 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 > > >From edf046cf00e60da67916d48ffa855c72544b597d Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Tue, 23 Aug 2016 13:08:12 +0200 >Subject: [PATCH 02/13] vfs_acl_common: rename psd to psd_blob in > get_nt_acl_internal() > >This makes it explicit where the SD is originating from. No change in >behaviour. > >This just paves the way for a later change that will simplify the whole >logic and talloc hierarchy, therefor this also strictly renames the >occurences after the out label. > >Logically, behind the out label, we're dealing with a variable that >points to what we're going to return, so the name psd_blob is >misleading, but I'm desperately trying to avoid logic changes in this >commit and therefor I'm just strictly renaming. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(backported from commit 2367eea928593f12f8914f7e7ba613b1b15516de) >--- > source3/modules/vfs_acl_common.c | 60 ++++++++++++++++++++-------------------- > 1 file changed, 30 insertions(+), 30 deletions(-) > >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index 85f6c65..420d909 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -488,7 +488,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > uint8_t sys_acl_hash[XATTR_SD_HASH_SIZE]; > uint8_t hash_tmp[XATTR_SD_HASH_SIZE]; > uint8_t sys_acl_hash_tmp[XATTR_SD_HASH_SIZE]; >- struct security_descriptor *psd = NULL; >+ struct security_descriptor *psd_blob = NULL; > struct security_descriptor *pdesc_next = NULL; > bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn), > ACL_MODULE_NAME, >@@ -506,25 +506,25 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > if (!NT_STATUS_IS_OK(status)) { > DEBUG(10, ("get_nt_acl_internal: get_acl_blob returned %s\n", > nt_errstr(status))); >- psd = NULL; >+ psd_blob = NULL; > goto out; > } else { >- status = parse_acl_blob(&blob, mem_ctx, &psd, >+ status = parse_acl_blob(&blob, mem_ctx, &psd_blob, > &hash_type, &xattr_version, &hash[0], &sys_acl_hash[0]); > if (!NT_STATUS_IS_OK(status)) { > DEBUG(10, ("parse_acl_blob returned %s\n", > nt_errstr(status))); >- psd = NULL; >+ psd_blob = NULL; > goto out; > } > } > >- /* Ensure we don't leak psd if we don't choose it. >+ /* Ensure we don't leak psd_blob if we don't choose it. > * > * We don't allocate it onto frame as it is preferred not to > * steal from a talloc pool. > */ >- talloc_steal(frame, psd); >+ talloc_steal(frame, psd_blob); > > /* determine which type of xattr we got */ > switch (xattr_version) { >@@ -547,8 +547,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > "mismatch (%u) for file %s\n", > (unsigned int)hash_type, > name)); >- TALLOC_FREE(psd); >- psd = NULL; >+ TALLOC_FREE(psd_blob); >+ psd_blob = NULL; > goto out; > } > >@@ -558,8 +558,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > "(%u) unexpected for file %s\n", > (unsigned int)hash_type, > name)); >- TALLOC_FREE(psd); >- psd = NULL; >+ TALLOC_FREE(psd_blob); >+ psd_blob = NULL; > goto out; > } > >@@ -642,8 +642,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > > status = hash_sd_sha256(pdesc_next, hash_tmp); > if (!NT_STATUS_IS_OK(status)) { >- TALLOC_FREE(psd); >- psd = pdesc_next; >+ TALLOC_FREE(psd_blob); >+ psd_blob = pdesc_next; > goto out; > } > >@@ -667,12 +667,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > NDR_PRINT_DEBUG(security_descriptor, pdesc_next); > } > >- TALLOC_FREE(psd); >- psd = pdesc_next; >+ TALLOC_FREE(psd_blob); >+ psd_blob = pdesc_next; > } > out: > >- if (psd == NULL) { >+ if (psd_blob == NULL) { > /* Get the full underlying sd, as we failed to get the > * blob for the hash, or the revision/hash type wasn't > * known */ >@@ -705,10 +705,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > * steal from a talloc pool. > */ > talloc_steal(frame, pdesc_next); >- psd = pdesc_next; >+ psd_blob = pdesc_next; > } > >- if (psd != pdesc_next) { >+ if (psd_blob != pdesc_next) { > /* We're returning the blob, throw > * away the filesystem SD. */ > TALLOC_FREE(pdesc_next); >@@ -761,20 +761,20 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > status = make_default_filesystem_acl(mem_ctx, > name, > psbuf, >- &psd); >+ &psd_blob); > if (!NT_STATUS_IS_OK(status)) { > TALLOC_FREE(frame); > return status; > } > } else { > if (is_directory && >- !sd_has_inheritable_components(psd, >+ !sd_has_inheritable_components(psd_blob, > true)) { > status = add_directory_inheritable_components( > handle, > name, > psbuf, >- psd); >+ psd_blob); > if (!NT_STATUS_IS_OK(status)) { > TALLOC_FREE(frame); > return status; >@@ -784,35 +784,35 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > the ~SEC_DESC_DACL_PROTECTED bit, as ACLs > can't be inherited in this way under POSIX. > Remove it for Windows-style ACLs. */ >- psd->type &= ~SEC_DESC_DACL_PROTECTED; >+ psd_blob->type &= ~SEC_DESC_DACL_PROTECTED; > } > } > > if (!(security_info & SECINFO_OWNER)) { >- psd->owner_sid = NULL; >+ psd_blob->owner_sid = NULL; > } > if (!(security_info & SECINFO_GROUP)) { >- psd->group_sid = NULL; >+ psd_blob->group_sid = NULL; > } > if (!(security_info & SECINFO_DACL)) { >- psd->type &= ~SEC_DESC_DACL_PRESENT; >- psd->dacl = NULL; >+ psd_blob->type &= ~SEC_DESC_DACL_PRESENT; >+ psd_blob->dacl = NULL; > } > if (!(security_info & SECINFO_SACL)) { >- psd->type &= ~SEC_DESC_SACL_PRESENT; >- psd->sacl = NULL; >+ psd_blob->type &= ~SEC_DESC_SACL_PRESENT; >+ psd_blob->sacl = NULL; > } > > TALLOC_FREE(blob.data); > > if (DEBUGLEVEL >= 10) { > DEBUG(10,("get_nt_acl_internal: returning acl for %s is:\n", >- name )); >- NDR_PRINT_DEBUG(security_descriptor, psd); >+ name)); >+ NDR_PRINT_DEBUG(security_descriptor, psd_blob); > } > > /* The VFS API is that the ACL is expected to be on mem_ctx */ >- *ppdesc = talloc_move(mem_ctx, &psd); >+ *ppdesc = talloc_move(mem_ctx, &psd_blob); > > TALLOC_FREE(frame); > return NT_STATUS_OK; >-- >2.7.4 > > >From 569bf46d14479ef606861d06a9950fc2b6a850f5 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Tue, 23 Aug 2016 13:11:24 +0200 >Subject: [PATCH 03/13] vfs_acl_common: rename pdesc_next to psd_fs > >In most realistic cases the "next" VFS op will return the permissions >from the filesystem. This rename makes it explicit where the SD is >originating from. No change in behaviour. > >This just paves the way for a later change that will simplify the whole >logic and talloc hierarchy. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(backported from commit 9f79084f166208820f586c8e43e1e315d32cd5ce) >--- > source3/modules/vfs_acl_common.c | 36 ++++++++++++++++++------------------ > 1 file changed, 18 insertions(+), 18 deletions(-) > >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index 420d909..8f89068 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -489,7 +489,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > uint8_t hash_tmp[XATTR_SD_HASH_SIZE]; > uint8_t sys_acl_hash_tmp[XATTR_SD_HASH_SIZE]; > struct security_descriptor *psd_blob = NULL; >- struct security_descriptor *pdesc_next = NULL; >+ struct security_descriptor *psd_fs = NULL; > bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn), > ACL_MODULE_NAME, > "ignore system acls", >@@ -615,13 +615,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > fsp, > HASH_SECURITY_INFO, > mem_ctx, >- &pdesc_next); >+ &psd_fs); > } else { > status = SMB_VFS_NEXT_GET_NT_ACL(handle, > name, > HASH_SECURITY_INFO, > mem_ctx, >- &pdesc_next); >+ &psd_fs); > } > > if (!NT_STATUS_IS_OK(status)) { >@@ -633,17 +633,17 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > return status; > } > >- /* Ensure we don't leak psd_next if we don't choose it. >+ /* Ensure we don't leak psd_fs if we don't choose it. > * > * We don't allocate it onto frame as it is preferred not to > * steal from a talloc pool. > */ >- talloc_steal(frame, pdesc_next); >+ talloc_steal(frame, psd_fs); > >- status = hash_sd_sha256(pdesc_next, hash_tmp); >+ status = hash_sd_sha256(psd_fs, hash_tmp); > if (!NT_STATUS_IS_OK(status)) { > TALLOC_FREE(psd_blob); >- psd_blob = pdesc_next; >+ psd_blob = psd_fs; > goto out; > } > >@@ -663,12 +663,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > > if (DEBUGLEVEL >= 10) { > DEBUG(10,("get_nt_acl_internal: acl for blob hash for %s is:\n", >- name )); >- NDR_PRINT_DEBUG(security_descriptor, pdesc_next); >+ name)); >+ NDR_PRINT_DEBUG(security_descriptor, psd_fs); > } > > TALLOC_FREE(psd_blob); >- psd_blob = pdesc_next; >+ psd_blob = psd_fs; > } > out: > >@@ -681,13 +681,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > fsp, > security_info, > mem_ctx, >- &pdesc_next); >+ &psd_fs); > } else { > status = SMB_VFS_NEXT_GET_NT_ACL(handle, > name, > security_info, > mem_ctx, >- &pdesc_next); >+ &psd_fs); > } > > if (!NT_STATUS_IS_OK(status)) { >@@ -699,19 +699,19 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > return status; > } > >- /* Ensure we don't leak psd_next if we don't choose it. >+ /* Ensure we don't leak psd_fs if we don't choose it. > * > * We don't allocate it onto frame as it is preferred not to > * steal from a talloc pool. > */ >- talloc_steal(frame, pdesc_next); >- psd_blob = pdesc_next; >+ talloc_steal(frame, psd_fs); >+ psd_blob = psd_fs; > } > >- if (psd_blob != pdesc_next) { >+ if (psd_blob != psd_fs) { > /* We're returning the blob, throw > * away the filesystem SD. */ >- TALLOC_FREE(pdesc_next); >+ TALLOC_FREE(psd_fs); > } else { > SMB_STRUCT_STAT sbuf; > SMB_STRUCT_STAT *psbuf = &sbuf; >@@ -757,7 +757,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > is_directory = S_ISDIR(psbuf->st_ex_mode); > > if (ignore_file_system_acl) { >- TALLOC_FREE(pdesc_next); >+ TALLOC_FREE(psd_fs); > status = make_default_filesystem_acl(mem_ctx, > name, > psbuf, >-- >2.7.4 > > >From ac99361ef3e8d1d41e8481a623069b38334b692d Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Tue, 23 Aug 2016 13:14:50 +0200 >Subject: [PATCH 04/13] vfs_acl_common: remove redundant NULL assignment > >The variables are already set to NULL by TALLOC_FREE. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit e6f1254a00a6bf85b8d95bfbafef7d3e39ce1dde) >--- > source3/modules/vfs_acl_common.c | 2 -- > 1 file changed, 2 deletions(-) > >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index 8f89068..64ee1ec 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -548,7 +548,6 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > (unsigned int)hash_type, > name)); > TALLOC_FREE(psd_blob); >- psd_blob = NULL; > goto out; > } > >@@ -559,7 +558,6 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > (unsigned int)hash_type, > name)); > TALLOC_FREE(psd_blob); >- psd_blob = NULL; > goto out; > } > >-- >2.7.4 > > >From 0730ce5248d1966e27d7dd25cfd78aeba5f9c2c7 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Tue, 23 Aug 2016 17:07:20 +0200 >Subject: [PATCH 05/13] vfs_acl_common: simplify ACL logic, cleanup and talloc > hierarchy > >No change in behaviour (hopefully! :-). This paves the way for moving >the ACL blob validation to a helper function in the next commit. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(backported from commit 335527c647331148927feea2a7ae2f2c88986bc6) >--- > source3/modules/vfs_acl_common.c | 135 ++++++++++++++++++--------------------- > 1 file changed, 63 insertions(+), 72 deletions(-) > >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index 64ee1ec..112874f 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -488,13 +488,16 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > uint8_t sys_acl_hash[XATTR_SD_HASH_SIZE]; > uint8_t hash_tmp[XATTR_SD_HASH_SIZE]; > uint8_t sys_acl_hash_tmp[XATTR_SD_HASH_SIZE]; >+ struct security_descriptor *psd = NULL; > struct security_descriptor *psd_blob = NULL; > struct security_descriptor *psd_fs = NULL; > bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn), > ACL_MODULE_NAME, > "ignore system acls", > false); >- TALLOC_CTX *frame = talloc_stackframe(); >+ char *sys_acl_blob_description = NULL; >+ DATA_BLOB sys_acl_blob = { 0 }; >+ bool psd_is_from_fs = false; > > if (fsp && name == NULL) { > name = fsp->fsp_name->base_name; >@@ -502,11 +505,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > > DEBUG(10, ("get_nt_acl_internal: name=%s\n", name)); > >- status = get_acl_blob(frame, handle, fsp, name, &blob); >+ status = get_acl_blob(mem_ctx, handle, fsp, name, &blob); > if (!NT_STATUS_IS_OK(status)) { > DEBUG(10, ("get_nt_acl_internal: get_acl_blob returned %s\n", > nt_errstr(status))); >- psd_blob = NULL; > goto out; > } else { > status = parse_acl_blob(&blob, mem_ctx, &psd_blob, >@@ -514,17 +516,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > if (!NT_STATUS_IS_OK(status)) { > DEBUG(10, ("parse_acl_blob returned %s\n", > nt_errstr(status))); >- psd_blob = NULL; >+ TALLOC_FREE(blob.data); > goto out; > } > } > >- /* Ensure we don't leak psd_blob if we don't choose it. >- * >- * We don't allocate it onto frame as it is preferred not to >- * steal from a talloc pool. >- */ >- talloc_steal(frame, psd_blob); >+ TALLOC_FREE(blob.data); > > /* determine which type of xattr we got */ > switch (xattr_version) { >@@ -534,10 +531,14 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > * require confirmation of the hash. In particular, > * the NTVFS file server uses version 1, but > * 'samba-tool ntacl' can set these as well */ >+ psd = psd_blob; >+ psd_blob = NULL; > goto out; > case 3: > case 4: > if (ignore_file_system_acl) { >+ psd = psd_blob; >+ psd_blob = NULL; > goto out; > } > >@@ -566,20 +567,18 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > case 4: > { > int ret; >- char *sys_acl_blob_description; >- DATA_BLOB sys_acl_blob; > if (fsp) { > /* Get the full underlying sd, then hash. */ > ret = SMB_VFS_NEXT_SYS_ACL_BLOB_GET_FD(handle, > fsp, >- frame, >+ mem_ctx, > &sys_acl_blob_description, > &sys_acl_blob); > } else { > /* Get the full underlying sd, then hash. */ > ret = SMB_VFS_NEXT_SYS_ACL_BLOB_GET_FILE(handle, > name, >- frame, >+ mem_ctx, > &sys_acl_blob_description, > &sys_acl_blob); > } >@@ -589,16 +588,20 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > if (ret == 0) { > status = hash_blob_sha256(sys_acl_blob, sys_acl_hash_tmp); > if (!NT_STATUS_IS_OK(status)) { >- TALLOC_FREE(frame); >- return status; >+ goto fail; > } > >+ TALLOC_FREE(sys_acl_blob_description); >+ TALLOC_FREE(sys_acl_blob.data); >+ > if (memcmp(&sys_acl_hash[0], &sys_acl_hash_tmp[0], > XATTR_SD_HASH_SIZE) == 0) { > /* Hash matches, return blob sd. */ > DEBUG(10, ("get_nt_acl_internal: blob hash " > "matches for file %s\n", >- name )); >+ name)); >+ psd = psd_blob; >+ psd_blob = NULL; > goto out; > } > } >@@ -627,21 +630,15 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > "returned %s\n", > name, > nt_errstr(status))); >- TALLOC_FREE(frame); >- return status; >+ goto fail; > } > >- /* Ensure we don't leak psd_fs if we don't choose it. >- * >- * We don't allocate it onto frame as it is preferred not to >- * steal from a talloc pool. >- */ >- talloc_steal(frame, psd_fs); >- > status = hash_sd_sha256(psd_fs, hash_tmp); > if (!NT_STATUS_IS_OK(status)) { > TALLOC_FREE(psd_blob); >- psd_blob = psd_fs; >+ psd = psd_fs; >+ psd_fs = NULL; >+ psd_is_from_fs = true; > goto out; > } > >@@ -649,7 +646,9 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > /* Hash matches, return blob sd. */ > DEBUG(10, ("get_nt_acl_internal: blob hash " > "matches for file %s\n", >- name )); >+ name)); >+ psd = psd_blob; >+ psd_blob = NULL; > goto out; > } > >@@ -666,11 +665,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > } > > TALLOC_FREE(psd_blob); >- psd_blob = psd_fs; >+ psd = psd_fs; >+ psd_fs = NULL; >+ psd_is_from_fs = true; > } >- out: > >- if (psd_blob == NULL) { >+out: >+ if (psd == NULL) { > /* Get the full underlying sd, as we failed to get the > * blob for the hash, or the revision/hash type wasn't > * known */ >@@ -679,13 +680,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > fsp, > security_info, > mem_ctx, >- &psd_fs); >+ &psd); > } else { > status = SMB_VFS_NEXT_GET_NT_ACL(handle, > name, > security_info, > mem_ctx, >- &psd_fs); >+ &psd); > } > > if (!NT_STATUS_IS_OK(status)) { >@@ -693,24 +694,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > "returned %s\n", > name, > nt_errstr(status))); >- TALLOC_FREE(frame); >- return status; >+ goto fail; > } > >- /* Ensure we don't leak psd_fs if we don't choose it. >- * >- * We don't allocate it onto frame as it is preferred not to >- * steal from a talloc pool. >- */ >- talloc_steal(frame, psd_fs); >- psd_blob = psd_fs; >+ psd_is_from_fs = true; > } > >- if (psd_blob != psd_fs) { >- /* We're returning the blob, throw >- * away the filesystem SD. */ >- TALLOC_FREE(psd_fs); >- } else { >+ if (psd_is_from_fs) { > SMB_STRUCT_STAT sbuf; > SMB_STRUCT_STAT *psbuf = &sbuf; > bool is_directory = false; >@@ -722,8 +712,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > if (fsp) { > status = vfs_stat_fsp(fsp); > if (!NT_STATUS_IS_OK(status)) { >- TALLOC_FREE(frame); >- return status; >+ goto fail; > } > psbuf = &fsp->fsp_name->st; > } else { >@@ -748,72 +737,74 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > name, > &sbuf); > if (ret == -1) { >- TALLOC_FREE(frame); >- return map_nt_error_from_unix(errno); >+ status = map_nt_error_from_unix(errno); >+ goto fail; > } > } > is_directory = S_ISDIR(psbuf->st_ex_mode); > > if (ignore_file_system_acl) { >- TALLOC_FREE(psd_fs); >+ TALLOC_FREE(psd); > status = make_default_filesystem_acl(mem_ctx, > name, > psbuf, >- &psd_blob); >+ &psd); > if (!NT_STATUS_IS_OK(status)) { >- TALLOC_FREE(frame); >- return status; >+ goto fail; > } > } else { > if (is_directory && >- !sd_has_inheritable_components(psd_blob, >+ !sd_has_inheritable_components(psd, > true)) { > status = add_directory_inheritable_components( > handle, > name, > psbuf, >- psd_blob); >+ psd); > if (!NT_STATUS_IS_OK(status)) { >- TALLOC_FREE(frame); >- return status; >+ goto fail; > } > } > /* The underlying POSIX module always sets > the ~SEC_DESC_DACL_PROTECTED bit, as ACLs > can't be inherited in this way under POSIX. > Remove it for Windows-style ACLs. */ >- psd_blob->type &= ~SEC_DESC_DACL_PROTECTED; >+ psd->type &= ~SEC_DESC_DACL_PROTECTED; > } > } > > if (!(security_info & SECINFO_OWNER)) { >- psd_blob->owner_sid = NULL; >+ psd->owner_sid = NULL; > } > if (!(security_info & SECINFO_GROUP)) { >- psd_blob->group_sid = NULL; >+ psd->group_sid = NULL; > } > if (!(security_info & SECINFO_DACL)) { >- psd_blob->type &= ~SEC_DESC_DACL_PRESENT; >- psd_blob->dacl = NULL; >+ psd->type &= ~SEC_DESC_DACL_PRESENT; >+ psd->dacl = NULL; > } > if (!(security_info & SECINFO_SACL)) { >- psd_blob->type &= ~SEC_DESC_SACL_PRESENT; >- psd_blob->sacl = NULL; >+ psd->type &= ~SEC_DESC_SACL_PRESENT; >+ psd->sacl = NULL; > } > >- TALLOC_FREE(blob.data); >- > if (DEBUGLEVEL >= 10) { > DEBUG(10,("get_nt_acl_internal: returning acl for %s is:\n", > name)); >- NDR_PRINT_DEBUG(security_descriptor, psd_blob); >+ NDR_PRINT_DEBUG(security_descriptor, psd); > } > >- /* The VFS API is that the ACL is expected to be on mem_ctx */ >- *ppdesc = talloc_move(mem_ctx, &psd_blob); >+ *ppdesc = psd; > >- TALLOC_FREE(frame); > return NT_STATUS_OK; >+ >+fail: >+ TALLOC_FREE(psd); >+ TALLOC_FREE(psd_blob); >+ TALLOC_FREE(psd_fs); >+ TALLOC_FREE(sys_acl_blob_description); >+ TALLOC_FREE(sys_acl_blob.data); >+ return status; > } > > /********************************************************************* >-- >2.7.4 > > >From 58d231b54cd00670943d214cebf6fe85088c3cb4 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Tue, 23 Aug 2016 22:32:57 +0200 >Subject: [PATCH 06/13] vfs_acl_common: move the ACL blob validation to a > helper function > >No change in behaviour. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(backported from commit 0de5a128cee90694979d074c2590ddbca0071e82) >--- > source3/modules/vfs_acl_common.c | 210 +++++++++++++++++++++++---------------- > 1 file changed, 122 insertions(+), 88 deletions(-) > >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index 112874f..4d357e7 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -467,20 +467,32 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, > 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 >- filesystem sd. >-*******************************************************************/ >- >-static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, >- files_struct *fsp, >- const char *name, >- uint32_t security_info, >- TALLOC_CTX *mem_ctx, >- struct security_descriptor **ppdesc) >+/** >+ * Validate an ACL blob >+ * >+ * This validates an ACL blob against the underlying filesystem ACL. If this >+ * function returns NT_STATUS_OK ppsd can be >+ * >+ * 1. the ACL from the blob (psd_from_fs=false), or >+ * 2. the ACL from the fs (psd_from_fs=true), or >+ * 3. NULL (!) >+ * >+ * If the return value is anything else then NT_STATUS_OK, ppsd is set to NULL >+ * and psd_from_fs set to false. >+ * >+ * Returning the underlying filesystem ACL in case no. 2 is really just an >+ * optimisation, because some validations have to fetch the filesytem ACL as >+ * part of the validation, so we already have it available and callers might >+ * need it as well. >+ **/ >+static NTSTATUS validate_nt_acl_blob(TALLOC_CTX *mem_ctx, >+ vfs_handle_struct *handle, >+ files_struct *fsp, >+ const char *name, >+ const DATA_BLOB *blob, >+ struct security_descriptor **ppsd, >+ bool *psd_is_from_fs) > { >- DATA_BLOB blob = data_blob_null; > NTSTATUS status; > uint16_t hash_type = XATTR_SD_HASH_TYPE_NONE; > uint16_t xattr_version = 0; >@@ -491,38 +503,29 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > struct security_descriptor *psd = NULL; > struct security_descriptor *psd_blob = NULL; > struct security_descriptor *psd_fs = NULL; >+ char *sys_acl_blob_description = NULL; >+ DATA_BLOB sys_acl_blob = { 0 }; > bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn), > ACL_MODULE_NAME, > "ignore system acls", > false); >- char *sys_acl_blob_description = NULL; >- DATA_BLOB sys_acl_blob = { 0 }; >- bool psd_is_from_fs = false; > >- if (fsp && name == NULL) { >- name = fsp->fsp_name->base_name; >- } >+ *ppsd = NULL; >+ *psd_is_from_fs = false; > >- DEBUG(10, ("get_nt_acl_internal: name=%s\n", name)); >+ status = parse_acl_blob(blob, >+ mem_ctx, >+ &psd_blob, >+ &hash_type, >+ &xattr_version, >+ &hash[0], >+ &sys_acl_hash[0]); > >- status = get_acl_blob(mem_ctx, handle, fsp, name, &blob); > if (!NT_STATUS_IS_OK(status)) { >- DEBUG(10, ("get_nt_acl_internal: get_acl_blob returned %s\n", >- nt_errstr(status))); >- goto out; >- } else { >- status = parse_acl_blob(&blob, mem_ctx, &psd_blob, >- &hash_type, &xattr_version, &hash[0], &sys_acl_hash[0]); >- if (!NT_STATUS_IS_OK(status)) { >- DEBUG(10, ("parse_acl_blob returned %s\n", >- nt_errstr(status))); >- TALLOC_FREE(blob.data); >- goto out; >- } >+ DBG_DEBUG("parse_acl_blob returned %s\n", nt_errstr(status)); >+ goto fail; > } > >- TALLOC_FREE(blob.data); >- > /* determine which type of xattr we got */ > switch (xattr_version) { > case 1: >@@ -531,35 +534,29 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > * require confirmation of the hash. In particular, > * the NTVFS file server uses version 1, but > * 'samba-tool ntacl' can set these as well */ >- psd = psd_blob; >- psd_blob = NULL; >- goto out; >+ *ppsd = psd_blob; >+ return NT_STATUS_OK; > case 3: > case 4: > if (ignore_file_system_acl) { >- psd = psd_blob; >- psd_blob = NULL; >- goto out; >+ *ppsd = psd_blob; >+ return NT_STATUS_OK; > } > > break; > default: >- DEBUG(10, ("get_nt_acl_internal: ACL blob revision " >- "mismatch (%u) for file %s\n", >- (unsigned int)hash_type, >- name)); >+ DBG_DEBUG("ACL blob revision mismatch (%u) for file %s\n", >+ (unsigned int)hash_type, name); > TALLOC_FREE(psd_blob); >- goto out; >+ return NT_STATUS_OK; > } > > /* determine which type of xattr we got */ > if (hash_type != XATTR_SD_HASH_TYPE_SHA256) { >- DEBUG(10, ("get_nt_acl_internal: ACL blob hash type " >- "(%u) unexpected for file %s\n", >- (unsigned int)hash_type, >- name)); >+ DBG_DEBUG("ACL blob hash type (%u) unexpected for file %s\n", >+ (unsigned int)hash_type, name); > TALLOC_FREE(psd_blob); >- goto out; >+ return NT_STATUS_OK; > } > > /* determine which type of xattr we got */ >@@ -597,12 +594,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > if (memcmp(&sys_acl_hash[0], &sys_acl_hash_tmp[0], > XATTR_SD_HASH_SIZE) == 0) { > /* Hash matches, return blob sd. */ >- DEBUG(10, ("get_nt_acl_internal: blob hash " >- "matches for file %s\n", >- name)); >- psd = psd_blob; >- psd_blob = NULL; >- goto out; >+ DBG_DEBUG("blob hash matches for file %s\n", >+ name); >+ *ppsd = psd_blob; >+ return NT_STATUS_OK; > } > } > >@@ -626,51 +621,96 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > } > > if (!NT_STATUS_IS_OK(status)) { >- DEBUG(10, ("get_nt_acl_internal: get_next_acl for file %s " >- "returned %s\n", >- name, >- nt_errstr(status))); >+ DBG_DEBUG("get_next_acl for file %s returned %s\n", >+ name, nt_errstr(status)); > goto fail; > } > > status = hash_sd_sha256(psd_fs, hash_tmp); > if (!NT_STATUS_IS_OK(status)) { > TALLOC_FREE(psd_blob); >- psd = psd_fs; >- psd_fs = NULL; >- psd_is_from_fs = true; >- goto out; >+ *ppsd = psd_fs; >+ *psd_is_from_fs = true; >+ return NT_STATUS_OK; > } > > if (memcmp(&hash[0], &hash_tmp[0], XATTR_SD_HASH_SIZE) == 0) { > /* Hash matches, return blob sd. */ >- DEBUG(10, ("get_nt_acl_internal: blob hash " >- "matches for file %s\n", >- name)); >- psd = psd_blob; >- psd_blob = NULL; >- goto out; >+ DBG_DEBUG("blob hash matches for file %s\n", name); >+ *ppsd = psd_blob; >+ return NT_STATUS_OK; > } > > /* Hash doesn't match, return underlying sd. */ >- DEBUG(10, ("get_nt_acl_internal: blob hash " >- "does not match for file %s - returning " >- "file system SD mapping.\n", >- name )); >+ DBG_DEBUG("blob hash does not match for file %s - returning " >+ "file system SD mapping.\n", name); > > if (DEBUGLEVEL >= 10) { >- DEBUG(10,("get_nt_acl_internal: acl for blob hash for %s is:\n", >- name)); >+ DBG_DEBUG("acl for blob hash for %s is:\n", name); > NDR_PRINT_DEBUG(security_descriptor, psd_fs); > } > > TALLOC_FREE(psd_blob); >- psd = psd_fs; >- psd_fs = NULL; >- psd_is_from_fs = true; >+ *ppsd = psd_fs; >+ *psd_is_from_fs = true; >+ } >+ >+ return NT_STATUS_OK; >+ >+fail: >+ TALLOC_FREE(psd); >+ TALLOC_FREE(psd_blob); >+ TALLOC_FREE(psd_fs); >+ TALLOC_FREE(sys_acl_blob_description); >+ TALLOC_FREE(sys_acl_blob.data); >+ return status; >+} >+ >+/******************************************************************* >+ Pull a DATA_BLOB from an xattr given a pathname. >+ If the hash doesn't match, or doesn't exist - return the underlying >+ filesystem sd. >+*******************************************************************/ >+ >+static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, >+ files_struct *fsp, >+ const char *name, >+ uint32_t security_info, >+ TALLOC_CTX *mem_ctx, >+ struct security_descriptor **ppdesc) >+{ >+ DATA_BLOB blob = data_blob_null; >+ NTSTATUS status; >+ struct security_descriptor *psd = NULL; >+ bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn), >+ ACL_MODULE_NAME, >+ "ignore system acls", >+ false); >+ bool psd_is_from_fs = false; >+ >+ if (fsp && name == NULL) { >+ name = fsp->fsp_name->base_name; >+ } >+ >+ DEBUG(10, ("get_nt_acl_internal: name=%s\n", name)); >+ >+ status = get_acl_blob(mem_ctx, handle, fsp, name, &blob); >+ if (NT_STATUS_IS_OK(status)) { >+ status = validate_nt_acl_blob(mem_ctx, >+ handle, >+ fsp, >+ name, >+ &blob, >+ &psd, >+ &psd_is_from_fs); >+ TALLOC_FREE(blob.data); >+ if (!NT_STATUS_IS_OK(status)) { >+ DBG_DEBUG("ACL validation for [%s] failed\n", >+ name); >+ goto fail; >+ } > } > >-out: > if (psd == NULL) { > /* Get the full underlying sd, as we failed to get the > * blob for the hash, or the revision/hash type wasn't >@@ -691,9 +731,7 @@ out: > > if (!NT_STATUS_IS_OK(status)) { > DEBUG(10, ("get_nt_acl_internal: get_next_acl for file %s " >- "returned %s\n", >- name, >- nt_errstr(status))); >+ "returned %s\n", name, nt_errstr(status))); > goto fail; > } > >@@ -800,10 +838,6 @@ out: > > fail: > TALLOC_FREE(psd); >- TALLOC_FREE(psd_blob); >- TALLOC_FREE(psd_fs); >- TALLOC_FREE(sys_acl_blob_description); >- TALLOC_FREE(sys_acl_blob.data); > return status; > } > >-- >2.7.4 > > >From ae2b1fd9c61c7665e66d0d0c09104c725340f0d2 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 24 Aug 2016 10:01:17 +0200 >Subject: [PATCH 07/13] vfs_acl_tdb|xattr: use a config handle > >Better for performance and a subsequent commit will add one more option >where this will pay off. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(backported from commit 61c3d2124fb1a180fae4c8c0b5ab5b32bd56c8ad) >--- > source3/modules/vfs_acl_common.c | 50 ++++++++++++++++++++++++++++++++-------- > source3/modules/vfs_acl_tdb.c | 7 ++++++ > source3/modules/vfs_acl_xattr.c | 7 ++++++ > 3 files changed, 54 insertions(+), 10 deletions(-) > >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index 4d357e7..4227c0f 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -46,6 +46,34 @@ static NTSTATUS store_acl_blob_fsp(vfs_handle_struct *handle, > SECINFO_DACL | \ > SECINFO_SACL) > >+struct acl_common_config { >+ bool ignore_system_acls; >+}; >+ >+static bool init_acl_common_config(vfs_handle_struct *handle) >+{ >+ struct acl_common_config *config = NULL; >+ >+ config = talloc_zero(handle->conn, struct acl_common_config); >+ if (config == NULL) { >+ DBG_ERR("talloc_zero() failed\n"); >+ errno = ENOMEM; >+ return false; >+ } >+ >+ config->ignore_system_acls = lp_parm_bool(SNUM(handle->conn), >+ ACL_MODULE_NAME, >+ "ignore system acls", >+ false); >+ >+ SMB_VFS_HANDLE_SET_DATA(handle, config, NULL, >+ struct acl_common_config, >+ return false); >+ >+ return true; >+} >+ >+ > /******************************************************************* > Hash a security descriptor. > *******************************************************************/ >@@ -505,14 +533,15 @@ static NTSTATUS validate_nt_acl_blob(TALLOC_CTX *mem_ctx, > struct security_descriptor *psd_fs = NULL; > char *sys_acl_blob_description = NULL; > DATA_BLOB sys_acl_blob = { 0 }; >- bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn), >- ACL_MODULE_NAME, >- "ignore system acls", >- false); >+ struct acl_common_config *config = NULL; > > *ppsd = NULL; > *psd_is_from_fs = false; > >+ SMB_VFS_HANDLE_GET_DATA(handle, config, >+ struct acl_common_config, >+ return NT_STATUS_UNSUCCESSFUL); >+ > status = parse_acl_blob(blob, > mem_ctx, > &psd_blob, >@@ -538,7 +567,7 @@ static NTSTATUS validate_nt_acl_blob(TALLOC_CTX *mem_ctx, > return NT_STATUS_OK; > case 3: > case 4: >- if (ignore_file_system_acl) { >+ if (config->ignore_system_acls) { > *ppsd = psd_blob; > return NT_STATUS_OK; > } >@@ -682,11 +711,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > DATA_BLOB blob = data_blob_null; > NTSTATUS status; > struct security_descriptor *psd = NULL; >- bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn), >- ACL_MODULE_NAME, >- "ignore system acls", >- false); > bool psd_is_from_fs = false; >+ struct acl_common_config *config = NULL; >+ >+ SMB_VFS_HANDLE_GET_DATA(handle, config, >+ struct acl_common_config, >+ return NT_STATUS_UNSUCCESSFUL); > > if (fsp && name == NULL) { > name = fsp->fsp_name->base_name; >@@ -781,7 +811,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > } > is_directory = S_ISDIR(psbuf->st_ex_mode); > >- if (ignore_file_system_acl) { >+ if (config->ignore_system_acls) { > TALLOC_FREE(psd); > status = make_default_filesystem_acl(mem_ctx, > name, >diff --git a/source3/modules/vfs_acl_tdb.c b/source3/modules/vfs_acl_tdb.c >index 1537ba3..d2417d7 100644 >--- a/source3/modules/vfs_acl_tdb.c >+++ b/source3/modules/vfs_acl_tdb.c >@@ -305,6 +305,7 @@ static int connect_acl_tdb(struct vfs_handle_struct *handle, > const char *user) > { > int ret = SMB_VFS_NEXT_CONNECT(handle, service, user); >+ bool ok; > > if (ret < 0) { > return ret; >@@ -315,6 +316,12 @@ static int connect_acl_tdb(struct vfs_handle_struct *handle, > return -1; > } > >+ ok = init_acl_common_config(handle); >+ if (!ok) { >+ DBG_ERR("init_acl_common_config failed\n"); >+ return -1; >+ } >+ > /* Ensure we have the parameters correct if we're > * using this module. */ > DEBUG(2,("connect_acl_tdb: setting 'inherit acls = true' " >diff --git a/source3/modules/vfs_acl_xattr.c b/source3/modules/vfs_acl_xattr.c >index c1b0a60..ae032c8 100644 >--- a/source3/modules/vfs_acl_xattr.c >+++ b/source3/modules/vfs_acl_xattr.c >@@ -180,11 +180,18 @@ static int connect_acl_xattr(struct vfs_handle_struct *handle, > const char *user) > { > int ret = SMB_VFS_NEXT_CONNECT(handle, service, user); >+ bool ok; > > if (ret < 0) { > return ret; > } > >+ ok = init_acl_common_config(handle); >+ if (!ok) { >+ DBG_ERR("init_acl_common_config failed\n"); >+ return -1; >+ } >+ > /* Ensure we have the parameters correct if we're > * using this module. */ > DEBUG(2,("connect_acl_xattr: setting 'inherit acls = true' " >-- >2.7.4 > > >From dc6533b8b5f0783cca43844fed5803b391787581 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 24 Aug 2016 10:30:15 +0200 >Subject: [PATCH 08/13] vfs_acl_common: move stat stuff to a helper function > >Will be reused in the next commit when moving the >make_default_filesystem_acl() stuff to a different place. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(backported from commit 10959698e20de381beec7ab532c8bdc32fa6401c) >--- > source3/modules/vfs_acl_common.c | 79 ++++++++++++++++++++++++---------------- > 1 file changed, 48 insertions(+), 31 deletions(-) > >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index 4227c0f..dfe6d23 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -695,6 +695,48 @@ fail: > return status; > } > >+static NTSTATUS stat_fsp_or_name(vfs_handle_struct *handle, >+ files_struct *fsp, >+ const char *name, >+ SMB_STRUCT_STAT *sbuf, >+ SMB_STRUCT_STAT **psbuf) >+{ >+ NTSTATUS status; >+ int ret; >+ >+ if (fsp) { >+ status = vfs_stat_fsp(fsp); >+ if (!NT_STATUS_IS_OK(status)) { >+ return status; >+ } >+ *psbuf = &fsp->fsp_name->st; >+ } else { >+ /* >+ * https://bugzilla.samba.org/show_bug.cgi?id=11249 >+ * >+ * We are currently guaranteed that 'name' here is a >+ * smb_fname->base_name, which *cannot* contain a stream name >+ * (':'). vfs_stat_smb_fname() splits a name into a base name + >+ * stream name, which when we get here we know we've already >+ * done. So we have to call the stat or lstat VFS calls >+ * directly here. Else, a base_name that contains a ':' (from a >+ * demangled name) will get split again. >+ * >+ * FIXME. >+ * This uglyness will go away once smb_fname is fully plumbed >+ * through the VFS. >+ */ >+ ret = vfs_stat_smb_basename(handle->conn, >+ name, >+ sbuf); >+ if (ret == -1) { >+ return map_nt_error_from_unix(errno); >+ } >+ } >+ >+ 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 >@@ -777,38 +819,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > * filesystem. If it's a directory, and has no > * inheritable ACE entries we have to fake them. > */ >- if (fsp) { >- status = vfs_stat_fsp(fsp); >- if (!NT_STATUS_IS_OK(status)) { >- goto fail; >- } >- psbuf = &fsp->fsp_name->st; >- } else { >- /* >- * https://bugzilla.samba.org/show_bug.cgi?id=11249 >- * >- * We are currently guaranteed that 'name' here is >- * a smb_fname->base_name, which *cannot* contain >- * a stream name (':'). vfs_stat_smb_fname() splits >- * a name into a base name + stream name, which >- * when we get here we know we've already done. >- * So we have to call the stat or lstat VFS >- * calls directly here. Else, a base_name that >- * contains a ':' (from a demangled name) will >- * get split again. >- * >- * FIXME. >- * This uglyness will go away once smb_fname >- * is fully plumbed through the VFS. >- */ >- int ret = vfs_stat_smb_basename(handle->conn, >- name, >- &sbuf); >- if (ret == -1) { >- status = map_nt_error_from_unix(errno); >- goto fail; >- } >+ >+ status = stat_fsp_or_name(handle, fsp, name, >+ &sbuf, &psbuf); >+ if (!NT_STATUS_IS_OK(status)) { >+ goto fail; > } >+ > is_directory = S_ISDIR(psbuf->st_ex_mode); > > if (config->ignore_system_acls) { >-- >2.7.4 > > >From 4fcf30dc1a42a0edec1afda3ce25c9c551beb9c8 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 24 Aug 2016 10:43:47 +0200 >Subject: [PATCH 09/13] vfs_acl_common: check for ignore_system_acls before > fetching filesystem ACL > >If ignore_system_acls is set and we're synthesizing a default ACL, we >were fetching the filesystem ACL just to free it again. This change >avoids this. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(backported from commit f46179ef7310959af095b0ea6234df7523d15457) >--- > source3/modules/vfs_acl_common.c | 96 ++++++++++++++++++++++------------------ > 1 file changed, 54 insertions(+), 42 deletions(-) > >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index dfe6d23..15002ec 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -787,33 +787,56 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > /* Get the full underlying sd, as we failed to get the > * blob for the hash, or the revision/hash type wasn't > * known */ >- if (fsp) { >- status = SMB_VFS_NEXT_FGET_NT_ACL(handle, >- fsp, >- security_info, >- mem_ctx, >- &psd); >+ >+ if (config->ignore_system_acls) { >+ SMB_STRUCT_STAT sbuf; >+ SMB_STRUCT_STAT *psbuf = &sbuf; >+ >+ status = stat_fsp_or_name(handle, fsp, name, >+ &sbuf, &psbuf); >+ if (!NT_STATUS_IS_OK(status)) { >+ goto fail; >+ } >+ >+ status = make_default_filesystem_acl( >+ mem_ctx, >+ name, >+ psbuf, >+ &psd); >+ if (!NT_STATUS_IS_OK(status)) { >+ goto fail; >+ } > } else { >- status = SMB_VFS_NEXT_GET_NT_ACL(handle, >- name, >- security_info, >- mem_ctx, >- &psd); >- } >+ if (fsp) { >+ status = SMB_VFS_NEXT_FGET_NT_ACL(handle, >+ fsp, >+ security_info, >+ mem_ctx, >+ &psd); >+ } else { >+ status = SMB_VFS_NEXT_GET_NT_ACL(handle, >+ name, >+ security_info, >+ mem_ctx, >+ &psd); >+ } > >- if (!NT_STATUS_IS_OK(status)) { >- DEBUG(10, ("get_nt_acl_internal: get_next_acl for file %s " >- "returned %s\n", name, nt_errstr(status))); >- goto fail; >- } >+ if (!NT_STATUS_IS_OK(status)) { >+ DBG_DEBUG("get_next_acl for file %s " >+ "returned %s\n", name, >+ nt_errstr(status)); >+ goto fail; >+ } > >- psd_is_from_fs = true; >+ psd_is_from_fs = true; >+ } > } > > if (psd_is_from_fs) { > SMB_STRUCT_STAT sbuf; > SMB_STRUCT_STAT *psbuf = &sbuf; > bool is_directory = false; >+ > /* > * We're returning the underlying ACL from the > * filesystem. If it's a directory, and has no >@@ -828,34 +851,23 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > > is_directory = S_ISDIR(psbuf->st_ex_mode); > >- if (config->ignore_system_acls) { >- TALLOC_FREE(psd); >- status = make_default_filesystem_acl(mem_ctx, >- name, >- psbuf, >- &psd); >+ if (is_directory && !sd_has_inheritable_components(psd, true)) { >+ status = add_directory_inheritable_components( >+ handle, >+ name, >+ psbuf, >+ psd); > if (!NT_STATUS_IS_OK(status)) { > goto fail; > } >- } else { >- if (is_directory && >- !sd_has_inheritable_components(psd, >- true)) { >- status = add_directory_inheritable_components( >- handle, >- name, >- psbuf, >- psd); >- if (!NT_STATUS_IS_OK(status)) { >- goto fail; >- } >- } >- /* The underlying POSIX module always sets >- the ~SEC_DESC_DACL_PROTECTED bit, as ACLs >- can't be inherited in this way under POSIX. >- Remove it for Windows-style ACLs. */ >- psd->type &= ~SEC_DESC_DACL_PROTECTED; > } >+ >+ /* >+ * The underlying POSIX module always sets the >+ * ~SEC_DESC_DACL_PROTECTED bit, as ACLs can't be inherited in >+ * this way under POSIX. Remove it for Windows-style ACLs. >+ */ >+ psd->type &= ~SEC_DESC_DACL_PROTECTED; > } > > if (!(security_info & SECINFO_OWNER)) { >-- >2.7.4 > > >From 3219e2cc1a15e3539717bdd674952ebe970bd85f Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 24 Aug 2016 20:31:00 +0200 >Subject: [PATCH 10/13] vfs_acl_xattr|tdb: add option to control default ACL > style > >Existing behaviour is "posix" style. Next commit will (re)add the >"windows" style. This commit doesn't change behaviour in any way. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(backported from commit 26a9867ae1a9c69659252ce03c280c7c18a6c58f) >--- > docs-xml/manpages/vfs_acl_tdb.8.xml | 25 ++++++++++++++++++ > docs-xml/manpages/vfs_acl_xattr.8.xml | 25 ++++++++++++++++++ > source3/modules/vfs_acl_common.c | 48 ++++++++++++++++++++++++++++++----- > 3 files changed, 92 insertions(+), 6 deletions(-) > >diff --git a/docs-xml/manpages/vfs_acl_tdb.8.xml b/docs-xml/manpages/vfs_acl_tdb.8.xml >index becbc55..9ef3962 100644 >--- a/docs-xml/manpages/vfs_acl_tdb.8.xml >+++ b/docs-xml/manpages/vfs_acl_tdb.8.xml >@@ -63,6 +63,31 @@ > </para> > </listitem> > </varlistentry> >+ >+ <varlistentry> >+ <term>acl_tdb:default acl style = [posix|windows]</term> >+ <listitem> >+ <para> >+ This parameter determines the type of ACL that is synthesized in >+ case a file or directory lacks an >+ <emphasis>security.NTACL</emphasis> xattr. >+ </para> >+ <para> >+ When set to <emphasis>posix</emphasis>, an ACL will be >+ synthesized based on the POSIX mode permissions for user, group >+ and others, with an additional ACE for <emphasis>NT >+ Authority\SYSTEM</emphasis> will full rights. >+ </para> >+ <para> >+ When set to <emphasis>windows</emphasis>, an ACL is synthesized >+ the same way Windows does it, only including permissions for the >+ owner and <emphasis>NT Authority\SYSTEM</emphasis>. >+ </para> >+ <para> >+ The default for this option is <emphasis>posix</emphasis>. >+ </para> >+ </listitem> >+ </varlistentry> > </variablelist> > > </refsect1> >diff --git a/docs-xml/manpages/vfs_acl_xattr.8.xml b/docs-xml/manpages/vfs_acl_xattr.8.xml >index 82a919a..17cb3d3 100644 >--- a/docs-xml/manpages/vfs_acl_xattr.8.xml >+++ b/docs-xml/manpages/vfs_acl_xattr.8.xml >@@ -67,6 +67,31 @@ > </para> > </listitem> > </varlistentry> >+ >+ <varlistentry> >+ <term>acl_xattr:default acl style = [posix|windows]</term> >+ <listitem> >+ <para> >+ This parameter determines the type of ACL that is synthesized in >+ case a file or directory lacks an >+ <emphasis>security.NTACL</emphasis> xattr. >+ </para> >+ <para> >+ When set to <emphasis>posix</emphasis>, an ACL will be >+ synthesized based on the POSIX mode permissions for user, group >+ and others, with an additional ACE for <emphasis>NT >+ Authority\SYSTEM</emphasis> will full rights. >+ </para> >+ <para> >+ When set to <emphasis>windows</emphasis>, an ACL is synthesized >+ the same way Windows does it, only including permissions for the >+ owner and <emphasis>NT Authority\SYSTEM</emphasis>. >+ </para> >+ <para> >+ The default for this option is <emphasis>posix</emphasis>. >+ </para> >+ </listitem> >+ </varlistentry> > </variablelist> > > </refsect1> >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index 15002ec..e8eaa4f 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -46,8 +46,16 @@ static NTSTATUS store_acl_blob_fsp(vfs_handle_struct *handle, > SECINFO_DACL | \ > SECINFO_SACL) > >+enum default_acl_style {DEFAULT_ACL_POSIX, DEFAULT_ACL_WINDOWS}; >+ >+static const struct enum_list default_acl_style[] = { >+ {DEFAULT_ACL_POSIX, "posix"}, >+ {DEFAULT_ACL_WINDOWS, "windows"} >+}; >+ > struct acl_common_config { > bool ignore_system_acls; >+ enum default_acl_style default_acl_style; > }; > > static bool init_acl_common_config(vfs_handle_struct *handle) >@@ -65,6 +73,11 @@ static bool init_acl_common_config(vfs_handle_struct *handle) > ACL_MODULE_NAME, > "ignore system acls", > false); >+ config->default_acl_style = lp_parm_enum(SNUM(handle->conn), >+ ACL_MODULE_NAME, >+ "default acl style", >+ default_acl_style, >+ DEFAULT_ACL_POSIX); > > SMB_VFS_HANDLE_SET_DATA(handle, config, NULL, > struct acl_common_config, >@@ -387,10 +400,10 @@ 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) >+static NTSTATUS make_default_acl_posix(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; >@@ -400,8 +413,7 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, > struct security_acl *new_dacl = NULL; > int idx = 0; > >- DEBUG(10,("make_default_filesystem_acl: file %s mode = 0%o\n", >- name, (int)mode )); >+ DBG_DEBUG("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); >@@ -495,6 +507,29 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, > return NT_STATUS_OK; > } > >+static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, >+ struct acl_common_config *config, >+ const char *name, >+ SMB_STRUCT_STAT *psbuf, >+ struct security_descriptor **ppdesc) >+{ >+ NTSTATUS status; >+ >+ switch (config->default_acl_style) { >+ >+ case DEFAULT_ACL_POSIX: >+ status = make_default_acl_posix(ctx, name, psbuf, ppdesc); >+ break; >+ >+ default: >+ DBG_ERR("unknown acl style %d", config->default_acl_style); >+ status = NT_STATUS_INTERNAL_ERROR; >+ break; >+ } >+ >+ return status; >+} >+ > /** > * Validate an ACL blob > * >@@ -800,6 +835,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > > status = make_default_filesystem_acl( > mem_ctx, >+ config, > name, > psbuf, > &psd); >-- >2.7.4 > > >From ea10a40651e7b5eb42466be74fa2c8ef3212790b Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 25 Aug 2016 07:45:34 +0200 >Subject: [PATCH 11/13] vfs_acl_common: Windows style default ACL > >Reintroduce Windows style default ACL, but this time as an optional >feature, not changing default behaviour. > >Original bugreport that got reverted because it changed the default >behaviour: https://bugzilla.samba.org/show_bug.cgi?id=12028 > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit 0730cb7e1ce33dbc5fc48a7363204c1220400c68) >--- > source3/modules/vfs_acl_common.c | 76 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 76 insertions(+) > >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index e8eaa4f..44fef12 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -507,6 +507,78 @@ static NTSTATUS make_default_acl_posix(TALLOC_CTX *ctx, > return NT_STATUS_OK; > } > >+static NTSTATUS make_default_acl_windows(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; >+ >+ DBG_DEBUG("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 2 ACEs: >+ * - Owner >+ * - 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++; >+ >+ 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; >+} >+ > static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, > struct acl_common_config *config, > const char *name, >@@ -521,6 +593,10 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, > status = make_default_acl_posix(ctx, name, psbuf, ppdesc); > break; > >+ case DEFAULT_ACL_WINDOWS: >+ status = make_default_acl_windows(ctx, name, psbuf, ppdesc); >+ break; >+ > default: > DBG_ERR("unknown acl style %d", config->default_acl_style); > status = NT_STATUS_INTERNAL_ERROR; >-- >2.7.4 > > >From 681e8e0dc3410d79477d75324dec3d856d097728 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 25 Aug 2016 16:30:24 +0200 >Subject: [PATCH 12/13] s4/torture: tests for vfs_acl_xattr default ACL styles > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(backported from commit 946b93d0e3f6f23fa2325d7aaba4dc6f4cc17cb6) >--- > selftest/target/Samba3.pm | 9 ++ > source3/selftest/tests.py | 4 +- > source4/torture/vfs/acl_xattr.c | 314 ++++++++++++++++++++++++++++++++++++++++ > source4/torture/vfs/vfs.c | 1 + > source4/torture/wscript_build | 2 +- > 5 files changed, 328 insertions(+), 2 deletions(-) > create mode 100644 source4/torture/vfs/acl_xattr.c > >diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm >index 319d00b..06d547c 100755 >--- a/selftest/target/Samba3.pm >+++ b/selftest/target/Samba3.pm >@@ -1574,6 +1574,15 @@ sub provision($$$$$$$$) > vfs objects = shadow_copy2 > shadow:mountpoint = $shadow_mntdir > wide links = yes >+ >+[acl_xattr_ign_sysacl_posix] >+ copy = tmp >+ acl_xattr:ignore system acls = yes >+ acl_xattr:default acl style = posix >+[acl_xattr_ign_sysacl_windows] >+ copy = tmp >+ acl_xattr:ignore system acls = yes >+ acl_xattr:default acl style = windows > "; > close(CONF); > >diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py >index 850ee69..983c89f 100755 >--- a/source3/selftest/tests.py >+++ b/source3/selftest/tests.py >@@ -308,7 +308,7 @@ nbt = ["nbt.dgram" ] > > libsmbclient = ["libsmbclient"] > >-vfs = ["vfs.fruit"] >+vfs = ["vfs.fruit", "vfs.acl_xattr"] > > tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idmap + vfs > >@@ -404,6 +404,8 @@ for t in tests: > plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD --signing=required') > elif t == "smb2.dosmode": > plansmbtorture4testsuite(t, "simpleserver", '//$SERVER/dosmode -U$USERNAME%$PASSWORD') >+ elif t == "vfs.acl_xattr": >+ plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') > else: > plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') > plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD') >diff --git a/source4/torture/vfs/acl_xattr.c b/source4/torture/vfs/acl_xattr.c >new file mode 100644 >index 0000000..7fd10d0 >--- /dev/null >+++ b/source4/torture/vfs/acl_xattr.c >@@ -0,0 +1,314 @@ >+/* >+ Unix SMB/CIFS implementation. >+ >+ Copyright (C) Ralph Boehme 2016 >+ >+ This program is free software; you can redistribute it and/or modify >+ it under the terms of the GNU General Public License as published by >+ the Free Software Foundation; either version 3 of the License, or >+ (at your option) any later version. >+ >+ This program is distributed in the hope that it will be useful, >+ but WITHOUT ANY WARRANTY; without even the implied warranty of >+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >+ GNU General Public License for more details. >+ >+ You should have received a copy of the GNU General Public License >+ along with this program. If not, see <http://www.gnu.org/licenses/>. >+*/ >+ >+#include "includes.h" >+#include "lib/cmdline/popt_common.h" >+#include "libcli/smb2/smb2.h" >+#include "libcli/smb2/smb2_calls.h" >+#include "libcli/smb/smbXcli_base.h" >+#include "torture/torture.h" >+#include "torture/vfs/proto.h" >+#include "libcli/resolve/resolve.h" >+#include "torture/util.h" >+#include "torture/smb2/proto.h" >+#include "libcli/security/security.h" >+#include "librpc/gen_ndr/ndr_security.h" >+#include "lib/param/param.h" >+ >+#define BASEDIR "smb2-testsd" >+ >+#define CHECK_SECURITY_DESCRIPTOR(_sd1, _sd2) do { \ >+ if (!security_descriptor_equal(_sd1, _sd2)) { \ >+ torture_warning(tctx, "security descriptors don't match!\n"); \ >+ torture_warning(tctx, "got:\n"); \ >+ NDR_PRINT_DEBUG(security_descriptor, _sd1); \ >+ torture_warning(tctx, "expected:\n"); \ >+ NDR_PRINT_DEBUG(security_descriptor, _sd2); \ >+ torture_result(tctx, TORTURE_FAIL, \ >+ "%s: security descriptors don't match!\n", \ >+ __location__); \ >+ ret = false; \ >+ } \ >+} while (0) >+ >+/** >+ * SMB2 connect with explicit share >+ **/ >+static bool torture_smb2_con_share(struct torture_context *tctx, >+ const char *share, >+ struct smb2_tree **tree) >+{ >+ struct smbcli_options options; >+ NTSTATUS status; >+ const char *host = torture_setting_string(tctx, "host", NULL); >+ struct cli_credentials *credentials = cmdline_credentials; >+ >+ lpcfg_smbcli_options(tctx->lp_ctx, &options); >+ >+ status = smb2_connect_ext(tctx, >+ host, >+ lpcfg_smb_ports(tctx->lp_ctx), >+ share, >+ lpcfg_resolve_context(tctx->lp_ctx), >+ credentials, >+ 0, >+ tree, >+ tctx->ev, >+ &options, >+ lpcfg_socket_options(tctx->lp_ctx), >+ lpcfg_gensec_settings(tctx, tctx->lp_ctx) >+ ); >+ if (!NT_STATUS_IS_OK(status)) { >+ printf("Failed to connect to SMB2 share \\\\%s\\%s - %s\n", >+ host, share, nt_errstr(status)); >+ return false; >+ } >+ return true; >+} >+ >+static bool test_default_acl_posix(struct torture_context *tctx, >+ struct smb2_tree *tree_unused) >+{ >+ struct smb2_tree *tree = NULL; >+ NTSTATUS status; >+ bool ok; >+ bool ret = true; >+ const char *dname = BASEDIR "\\testdir"; >+ const char *fname = BASEDIR "\\testdir\\testfile"; >+ struct smb2_handle fhandle, dhandle; >+ union smb_fileinfo q; >+ union smb_setfileinfo set; >+ struct security_descriptor *sd = NULL; >+ struct security_descriptor *exp_sd = NULL; >+ char *owner_sid = NULL; >+ char *group_sid = NULL; >+ >+ ok = torture_smb2_con_share(tctx, "acl_xattr_ign_sysacl_posix", &tree); >+ torture_assert_goto(tctx, ok == true, ret, done, >+ "Unable to connect to 'acl_xattr_ign_sysacl_posix'\n"); >+ >+ ok = smb2_util_setup_dir(tctx, tree, BASEDIR); >+ torture_assert_goto(tctx, ok == true, ret, done, "Unable to setup testdir\n"); >+ >+ ZERO_STRUCT(dhandle); >+ status = torture_smb2_testdir(tree, dname, &dhandle); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "torture_smb2_testdir\n"); >+ >+ torture_comment(tctx, "Get the original sd\n"); >+ >+ ZERO_STRUCT(q); >+ q.query_secdesc.level = RAW_FILEINFO_SEC_DESC; >+ q.query_secdesc.in.file.handle = dhandle; >+ q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER | SECINFO_GROUP; >+ status = smb2_getinfo_file(tree, tctx, &q); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_getinfo_file\n"); >+ >+ sd = q.query_secdesc.out.sd; >+ owner_sid = dom_sid_string(tctx, sd->owner_sid); >+ group_sid = dom_sid_string(tctx, sd->group_sid); >+ torture_comment(tctx, "owner [%s] group [%s]\n", owner_sid, group_sid); >+ >+ torture_comment(tctx, "Set ACL with no inheritable ACE\n"); >+ >+ sd = security_descriptor_dacl_create(tctx, >+ 0, NULL, NULL, >+ owner_sid, >+ SEC_ACE_TYPE_ACCESS_ALLOWED, >+ SEC_RIGHTS_DIR_ALL, >+ 0, >+ NULL); >+ >+ ZERO_STRUCT(set); >+ set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC; >+ set.set_secdesc.in.file.handle = dhandle; >+ set.set_secdesc.in.secinfo_flags = SECINFO_DACL; >+ set.set_secdesc.in.sd = sd; >+ status = smb2_setinfo_file(tree, &set); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_setinfo_file\n"); >+ >+ TALLOC_FREE(sd); >+ smb2_util_close(tree, dhandle); >+ >+ torture_comment(tctx, "Create file\n"); >+ >+ ZERO_STRUCT(fhandle); >+ status = torture_smb2_testfile(tree, fname, &fhandle); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_create_complex_file\n"); >+ >+ torture_comment(tctx, "Query file SD\n"); >+ >+ ZERO_STRUCT(q); >+ q.query_secdesc.level = RAW_FILEINFO_SEC_DESC; >+ q.query_secdesc.in.file.handle = fhandle; >+ q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER | SECINFO_GROUP; >+ status = smb2_getinfo_file(tree, tctx, &q); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_getinfo_file\n"); >+ sd = q.query_secdesc.out.sd; >+ >+ smb2_util_close(tree, fhandle); >+ ZERO_STRUCT(fhandle); >+ >+ torture_comment(tctx, "Checking actual file SD against expected SD\n"); >+ >+ exp_sd = security_descriptor_dacl_create( >+ tctx, 0, owner_sid, group_sid, >+ owner_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0, >+ group_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE, 0, >+ SID_WORLD, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE, 0, >+ SID_NT_SYSTEM, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0, >+ NULL); >+ >+ CHECK_SECURITY_DESCRIPTOR(sd, exp_sd); >+ >+done: >+ if (!smb2_util_handle_empty(fhandle)) { >+ smb2_util_close(tree, fhandle); >+ } >+ if (!smb2_util_handle_empty(dhandle)) { >+ smb2_util_close(tree, dhandle); >+ } >+ if (tree != NULL) { >+ smb2_deltree(tree, BASEDIR); >+ smb2_tdis(tree); >+ } >+ >+ return ret; >+} >+ >+static bool test_default_acl_win(struct torture_context *tctx, >+ struct smb2_tree *tree_unused) >+{ >+ struct smb2_tree *tree = NULL; >+ NTSTATUS status; >+ bool ok; >+ bool ret = true; >+ const char *dname = BASEDIR "\\testdir"; >+ const char *fname = BASEDIR "\\testdir\\testfile"; >+ struct smb2_handle fhandle, dhandle; >+ union smb_fileinfo q; >+ union smb_setfileinfo set; >+ struct security_descriptor *sd = NULL; >+ struct security_descriptor *exp_sd = NULL; >+ char *owner_sid = NULL; >+ char *group_sid = NULL; >+ >+ ok = torture_smb2_con_share(tctx, "acl_xattr_ign_sysacl_windows", &tree); >+ torture_assert_goto(tctx, ok == true, ret, done, >+ "Unable to connect to 'acl_xattr_ign_sysacl_windows'\n"); >+ >+ ok = smb2_util_setup_dir(tctx, tree, BASEDIR); >+ torture_assert_goto(tctx, ok == true, ret, done, "Unable to setup testdir\n"); >+ >+ ZERO_STRUCT(dhandle); >+ status = torture_smb2_testdir(tree, dname, &dhandle); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "torture_smb2_testdir\n"); >+ >+ torture_comment(tctx, "Get the original sd\n"); >+ >+ ZERO_STRUCT(q); >+ q.query_secdesc.level = RAW_FILEINFO_SEC_DESC; >+ q.query_secdesc.in.file.handle = dhandle; >+ q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER | SECINFO_GROUP; >+ status = smb2_getinfo_file(tree, tctx, &q); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_getinfo_file\n"); >+ >+ sd = q.query_secdesc.out.sd; >+ owner_sid = dom_sid_string(tctx, sd->owner_sid); >+ group_sid = dom_sid_string(tctx, sd->group_sid); >+ torture_comment(tctx, "owner [%s] group [%s]\n", owner_sid, group_sid); >+ >+ torture_comment(tctx, "Set ACL with no inheritable ACE\n"); >+ >+ sd = security_descriptor_dacl_create(tctx, >+ 0, NULL, NULL, >+ owner_sid, >+ SEC_ACE_TYPE_ACCESS_ALLOWED, >+ SEC_RIGHTS_DIR_ALL, >+ 0, >+ NULL); >+ >+ ZERO_STRUCT(set); >+ set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC; >+ set.set_secdesc.in.file.handle = dhandle; >+ set.set_secdesc.in.secinfo_flags = SECINFO_DACL; >+ set.set_secdesc.in.sd = sd; >+ status = smb2_setinfo_file(tree, &set); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_setinfo_file\n"); >+ >+ TALLOC_FREE(sd); >+ smb2_util_close(tree, dhandle); >+ >+ torture_comment(tctx, "Create file\n"); >+ >+ ZERO_STRUCT(fhandle); >+ status = torture_smb2_testfile(tree, fname, &fhandle); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_create_complex_file\n"); >+ >+ torture_comment(tctx, "Query file SD\n"); >+ >+ ZERO_STRUCT(q); >+ q.query_secdesc.level = RAW_FILEINFO_SEC_DESC; >+ q.query_secdesc.in.file.handle = fhandle; >+ q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER | SECINFO_GROUP; >+ status = smb2_getinfo_file(tree, tctx, &q); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_getinfo_file\n"); >+ sd = q.query_secdesc.out.sd; >+ >+ smb2_util_close(tree, fhandle); >+ ZERO_STRUCT(fhandle); >+ >+ torture_comment(tctx, "Checking actual file SD against expected SD\n"); >+ >+ exp_sd = security_descriptor_dacl_create( >+ tctx, 0, owner_sid, group_sid, >+ owner_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0, >+ SID_NT_SYSTEM, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0, >+ NULL); >+ >+ CHECK_SECURITY_DESCRIPTOR(sd, exp_sd); >+ >+done: >+ if (!smb2_util_handle_empty(fhandle)) { >+ smb2_util_close(tree, fhandle); >+ } >+ if (!smb2_util_handle_empty(dhandle)) { >+ smb2_util_close(tree, dhandle); >+ } >+ if (tree != NULL) { >+ smb2_deltree(tree, BASEDIR); >+ smb2_tdis(tree); >+ } >+ >+ return ret; >+} >+ >+/* >+ basic testing of vfs_acl_xattr >+*/ >+struct torture_suite *torture_acl_xattr(void) >+{ >+ struct torture_suite *suite = torture_suite_create(talloc_autofree_context(), "acl_xattr"); >+ >+ torture_suite_add_1smb2_test(suite, "default-acl-style-posix", test_default_acl_posix); >+ torture_suite_add_1smb2_test(suite, "default-acl-style-windows", test_default_acl_win); >+ >+ suite->description = talloc_strdup(suite, "vfs_acl_xattr tests"); >+ >+ return suite; >+} >diff --git a/source4/torture/vfs/vfs.c b/source4/torture/vfs/vfs.c >index 9b82387..4830461 100644 >--- a/source4/torture/vfs/vfs.c >+++ b/source4/torture/vfs/vfs.c >@@ -107,6 +107,7 @@ NTSTATUS torture_vfs_init(void) > suite->description = talloc_strdup(suite, "VFS modules tests"); > > torture_suite_add_suite(suite, torture_vfs_fruit()); >+ torture_suite_add_suite(suite, torture_acl_xattr()); > > torture_register_suite(suite); > >diff --git a/source4/torture/wscript_build b/source4/torture/wscript_build >index 1b443b7..272227a 100755 >--- a/source4/torture/wscript_build >+++ b/source4/torture/wscript_build >@@ -244,7 +244,7 @@ bld.SAMBA_MODULE('TORTURE_NTP', > ) > > bld.SAMBA_MODULE('TORTURE_VFS', >- source='vfs/vfs.c vfs/fruit.c', >+ source='vfs/vfs.c vfs/fruit.c vfs/acl_xattr.c', > subsystem='smbtorture', > deps='LIBCLI_SMB POPT_CREDENTIALS TORTURE_UTIL smbclient-raw TORTURE_RAW', > internal_module=True, >-- >2.7.4 > > >From 3778433c637d834dd79c64e31caf6eb3886ffcea Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Sat, 27 Aug 2016 10:11:14 +0200 >Subject: [PATCH 13/13] vfs_acl_common: use DBG_LEVEL and remove function > prefixes in DEBUG statements > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(backported from commit 11dddd59aa01195152199443bc26e3141f162c8f) >--- > source3/modules/vfs_acl_common.c | 72 +++++++++++++++++----------------------- > 1 file changed, 30 insertions(+), 42 deletions(-) > >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index 44fef12..9675fca 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -144,8 +144,8 @@ static NTSTATUS parse_acl_blob(const DATA_BLOB *pblob, > (ndr_pull_flags_fn_t)ndr_pull_xattr_NTACL); > > if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { >- DEBUG(5, ("parse_acl_blob: ndr_pull_xattr_NTACL failed: %s\n", >- ndr_errstr(ndr_err))); >+ DBG_INFO("ndr_pull_xattr_NTACL failed: %s\n", >+ ndr_errstr(ndr_err)); > TALLOC_FREE(frame); > return ndr_map_error2ntstatus(ndr_err); > } >@@ -241,8 +241,8 @@ static NTSTATUS create_acl_blob(const struct security_descriptor *psd, > (ndr_push_flags_fn_t)ndr_push_xattr_NTACL); > > if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { >- DEBUG(5, ("create_acl_blob: ndr_push_xattr_NTACL failed: %s\n", >- ndr_errstr(ndr_err))); >+ DBG_INFO("ndr_push_xattr_NTACL failed: %s\n", >+ ndr_errstr(ndr_err)); > return ndr_map_error2ntstatus(ndr_err); > } > >@@ -287,8 +287,8 @@ static NTSTATUS create_sys_acl_blob(const struct security_descriptor *psd, > (ndr_push_flags_fn_t)ndr_push_xattr_NTACL); > > if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { >- DEBUG(5, ("create_acl_blob: ndr_push_xattr_NTACL failed: %s\n", >- ndr_errstr(ndr_err))); >+ DBG_INFO("ndr_push_xattr_NTACL failed: %s\n", >+ ndr_errstr(ndr_err)); > return ndr_map_error2ntstatus(ndr_err); > } > >@@ -345,10 +345,7 @@ static NTSTATUS add_directory_inheritable_components(vfs_handle_struct *handle, > > mode = dir_mode | file_mode; > >- DEBUG(10, ("add_directory_inheritable_components: directory %s, " >- "mode = 0%o\n", >- name, >- (unsigned int)mode )); >+ DBG_DEBUG("directory %s, mode = 0%o\n", name, (unsigned int)mode); > > if (num_aces) { > memcpy(new_ace_list, psd->dacl->aces, >@@ -875,7 +872,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > name = fsp->fsp_name->base_name; > } > >- DEBUG(10, ("get_nt_acl_internal: name=%s\n", name)); >+ DBG_DEBUG("name=%s\n", name); > > status = get_acl_blob(mem_ctx, handle, fsp, name, &blob); > if (NT_STATUS_IS_OK(status)) { >@@ -998,8 +995,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > } > > if (DEBUGLEVEL >= 10) { >- DEBUG(10,("get_nt_acl_internal: returning acl for %s is:\n", >- name)); >+ DBG_DEBUG("returning acl for %s is:\n", name); > NDR_PRINT_DEBUG(security_descriptor, psd); > } > >@@ -1062,9 +1058,8 @@ static NTSTATUS set_underlying_acl(vfs_handle_struct *handle, files_struct *fsp, > return NT_STATUS_ACCESS_DENIED; > } > >- DEBUG(10, ("fset_nt_acl_common: overriding chown on file %s " >- "for sid %s\n", >- fsp_str_dbg(fsp), sid_string_tos(psd->owner_sid))); >+ DBG_DEBUG("overriding chown on file %s for sid %s\n", >+ fsp_str_dbg(fsp), sid_string_tos(psd->owner_sid)); > > /* Ok, we failed to chown and we have > SEC_STD_WRITE_OWNER access - override. */ >@@ -1087,27 +1082,25 @@ static NTSTATUS store_v3_blob(vfs_handle_struct *handle, files_struct *fsp, > DATA_BLOB blob; > > if (DEBUGLEVEL >= 10) { >- DEBUG(10, ("fset_nt_acl_xattr: storing xattr sd for file %s\n", >- fsp_str_dbg(fsp))); >+ DBG_DEBUG("storing xattr sd for file %s\n", >+ fsp_str_dbg(fsp)); > NDR_PRINT_DEBUG( > security_descriptor, > discard_const_p(struct security_descriptor, psd)); > > if (pdesc_next != NULL) { >- DEBUG(10, ("fset_nt_acl_xattr: storing has in xattr sd " >- "based on \n")); >+ DBG_DEBUG("storing xattr sd based on \n"); > NDR_PRINT_DEBUG( > security_descriptor, > discard_const_p(struct security_descriptor, > pdesc_next)); > } else { >- DEBUG(10, >- ("fset_nt_acl_xattr: ignoring underlying sd\n")); >+ DBG_DEBUG("ignoring underlying sd\n"); > } > } > status = create_acl_blob(psd, &blob, XATTR_SD_HASH_TYPE_SHA256, hash); > if (!NT_STATUS_IS_OK(status)) { >- DEBUG(10, ("fset_nt_acl_xattr: create_acl_blob failed\n")); >+ DBG_DEBUG("create_acl_blob failed\n"); > return status; > } > >@@ -1136,8 +1129,7 @@ static NTSTATUS fset_nt_acl_common(vfs_handle_struct *handle, files_struct *fsp, > SNUM(handle->conn), ACL_MODULE_NAME, "ignore system acls", false); > > if (DEBUGLEVEL >= 10) { >- DEBUG(10,("fset_nt_acl_xattr: incoming sd for file %s\n", >- fsp_str_dbg(fsp))); >+ DBG_DEBUG("incoming sd for file %s\n", fsp_str_dbg(fsp)); > NDR_PRINT_DEBUG(security_descriptor, > discard_const_p(struct security_descriptor, orig_psd)); > } >@@ -1255,12 +1247,12 @@ static NTSTATUS fset_nt_acl_common(vfs_handle_struct *handle, files_struct *fsp, > } > > if (DEBUGLEVEL >= 10) { >- DEBUG(10,("fset_nt_acl_xattr: storing xattr sd for file %s based on system ACL\n", >- fsp_str_dbg(fsp))); >+ DBG_DEBUG("storing xattr sd for file %s based on system ACL\n", >+ fsp_str_dbg(fsp)); > NDR_PRINT_DEBUG(security_descriptor, > discard_const_p(struct security_descriptor, psd)); > >- DEBUG(10,("fset_nt_acl_xattr: storing hash in xattr sd based on system ACL and:\n")); >+ DBG_DEBUG("storing hash in xattr sd based on system ACL and:\n"); > NDR_PRINT_DEBUG(security_descriptor, > discard_const_p(struct security_descriptor, pdesc_next)); > } >@@ -1272,7 +1264,7 @@ static NTSTATUS fset_nt_acl_common(vfs_handle_struct *handle, files_struct *fsp, > status = create_sys_acl_blob(psd, &blob, XATTR_SD_HASH_TYPE_SHA256, hash, > sys_acl_description, sys_acl_hash); > if (!NT_STATUS_IS_OK(status)) { >- DEBUG(10, ("fset_nt_acl_xattr: create_sys_acl_blob failed\n")); >+ DBG_DEBUG("create_sys_acl_blob failed\n"); > TALLOC_FREE(frame); > return status; > } >@@ -1309,9 +1301,8 @@ static int acl_common_remove_object(vfs_handle_struct *handle, > goto out; > } > >- DEBUG(10,("acl_common_remove_object: removing %s %s/%s\n", >- is_directory ? "directory" : "file", >- parent_dir, final_component )); >+ DBG_DEBUG("removing %s %s/%s\n", is_directory ? "directory" : "file", >+ parent_dir, final_component); > > /* cd into the parent dir to pin it. */ > ret = vfs_ChDir(conn, parent_dir); >@@ -1344,10 +1335,9 @@ static int acl_common_remove_object(vfs_handle_struct *handle, > } > > if (!fsp) { >- DEBUG(10,("acl_common_remove_object: %s %s/%s " >- "not an open file\n", >- is_directory ? "directory" : "file", >- parent_dir, final_component )); >+ DBG_DEBUG("%s %s/%s not an open file\n", >+ is_directory ? "directory" : "file", >+ parent_dir, final_component); > saved_errno = EACCES; > goto out; > } >@@ -1395,9 +1385,7 @@ static int rmdir_acl_common(struct vfs_handle_struct *handle, > true); > } > >- DEBUG(10,("rmdir_acl_common: unlink of %s failed %s\n", >- path, >- strerror(errno) )); >+ DBG_DEBUG("unlink of %s failed %s\n", path, strerror(errno)); > return -1; > } > >@@ -1424,9 +1412,9 @@ static int unlink_acl_common(struct vfs_handle_struct *handle, > false); > } > >- DEBUG(10,("unlink_acl_common: unlink of %s failed %s\n", >- smb_fname->base_name, >- strerror(errno) )); >+ DBG_DEBUG("unlink of %s failed %s\n", >+ smb_fname->base_name, >+ strerror(errno)); > return -1; > } > >-- >2.7.4 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=12448">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
jra
:
review+
Actions:
View
Attachments on
bug 12177
:
12401
|
12445
|
12447
| 12448 |
12466