From f8dbfed4abbdeb48fe5adea6ab82cb8565eff64d Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 11 Dec 2018 17:17:46 +0100 Subject: [PATCH 01/40] vfs: Use dom_sid_str_buf Signed-off-by: Volker Lendecke Reviewed-by: Gary Lockyer (cherry picked from commit 59f29acb2cd947d2f594a5af3d73d0cbe8298d92) --- source3/modules/nfs4_acls.c | 14 ++++++++++---- source3/modules/vfs_afsacl.c | 6 ++++-- source3/modules/vfs_default.c | 6 ++++-- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index 19f0fefdb98..7776caa16d2 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -317,6 +317,7 @@ static bool smbacl4_nfs42win(TALLOC_CTX *mem_ctx, for (aceint = acl->first; aceint != NULL; aceint = aceint->next) { uint32_t mask; struct dom_sid sid; + struct dom_sid_buf buf; SMB_ACE4PROP_T *ace = &aceint->prop; uint32_t win_ace_flags; @@ -349,7 +350,7 @@ static bool smbacl4_nfs42win(TALLOC_CTX *mem_ctx, } } DEBUG(10, ("mapped %d to %s\n", ace->who.id, - sid_string_dbg(&sid))); + dom_sid_str_buf(&sid, &buf))); if (!is_directory && params->map_full_control) { /* @@ -655,7 +656,10 @@ static bool smbacl4_fill_ace4( SMB_ACE4PROP_T *ace_v4 /* output */ ) { - DEBUG(10, ("got ace for %s\n", sid_string_dbg(&ace_nt->trustee))); + struct dom_sid_buf buf; + + DEBUG(10, ("got ace for %s\n", + dom_sid_str_buf(&ace_nt->trustee, &buf))); ZERO_STRUCTP(ace_v4); @@ -738,7 +742,7 @@ static bool smbacl4_fill_ace4( DEBUG(1, ("nfs4_acls.c: file [%s]: could not " "convert %s to uid or gid\n", filename->base_name, - sid_string_dbg(&ace_nt->trustee))); + dom_sid_str_buf(&ace_nt->trustee, &buf))); return false; } } @@ -882,9 +886,11 @@ static struct SMB4ACL_T *smbacl4_win2nfs4( if (!smbacl4_fill_ace4(fsp->fsp_name, pparams, ownerUID, ownerGID, dacl->aces + i, &ace_v4)) { + struct dom_sid_buf buf; DEBUG(3, ("Could not fill ace for file %s, SID %s\n", filename, - sid_string_dbg(&((dacl->aces+i)->trustee)))); + dom_sid_str_buf(&((dacl->aces+i)->trustee), + &buf))); continue; } diff --git a/source3/modules/vfs_afsacl.c b/source3/modules/vfs_afsacl.c index 54c9344a637..a69a99e8d95 100644 --- a/source3/modules/vfs_afsacl.c +++ b/source3/modules/vfs_afsacl.c @@ -761,8 +761,9 @@ static bool nt_to_afs_acl(const char *filename, } if (!mappable_sid(&ace->trustee)) { + struct dom_sid_buf buf; DEBUG(10, ("Ignoring unmappable SID %s\n", - sid_string_dbg(&ace->trustee))); + dom_sid_str_buf(&ace->trustee, &buf))); continue; } @@ -791,8 +792,9 @@ static bool nt_to_afs_acl(const char *filename, if (!lookup_sid(talloc_tos(), &ace->trustee, &dom_name, &name, &name_type)) { + struct dom_sid_buf buf; DEBUG(1, ("AFSACL: Could not lookup SID %s on file %s\n", - sid_string_dbg(&ace->trustee), + dom_sid_str_buf(&ace->trustee, &buf), filename)); continue; } diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 7dfa335190c..8aaa991781e 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1324,6 +1324,7 @@ static NTSTATUS vfswrap_fsctl(struct vfs_handle_struct *handle, * but I have to check that --metze */ struct dom_sid sid; + struct dom_sid_buf buf; uid_t uid; size_t sid_len; @@ -1343,11 +1344,12 @@ static NTSTATUS vfswrap_fsctl(struct vfs_handle_struct *handle, if (!sid_parse(_in_data + 4, sid_len, &sid)) { return NT_STATUS_INVALID_PARAMETER; } - DEBUGADD(10, ("for SID: %s\n", sid_string_dbg(&sid))); + DEBUGADD(10, ("for SID: %s\n", + dom_sid_str_buf(&sid, &buf))); if (!sid_to_uid(&sid, &uid)) { DEBUG(0,("sid_to_uid: failed, sid[%s] sid_len[%lu]\n", - sid_string_dbg(&sid), + dom_sid_str_buf(&sid, &buf), (unsigned long)sid_len)); uid = (-1); } -- 2.17.0 From 0dd35b58a640a7c84b3c2ba080bf585d4ca2bd5c Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Fri, 7 Jun 2019 12:55:32 -0700 Subject: [PATCH 02/40] Revert "nfs4acl: Fix owner mapping with ID_TYPE_BOTH" This reverts commit 5d4f7bfda579cecb123cfb1d7130688f1d1c98b7. That patch broke the case with ID_TYPE_BOTH where a file is owned by a group (e.g. using autorid and having a file owned by BUILTIN\Administrators). In this case, the ACE entry for the group gets mapped a to a user ACL entry and the group no longer has access (as in the user's token the group is not mapped to a uid). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 42bd3a72a2525aa8a918f4bf7067b30ce8e0e197) --- source3/modules/nfs4_acls.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index 7776caa16d2..6db5a6db6d9 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -723,14 +723,7 @@ static bool smbacl4_fill_ace4( uid_t uid; gid_t gid; - /* - * ID_TYPE_BOTH returns both uid and gid. Explicitly - * check for ownerUID to allow the mapping of the - * owner to a special entry in this idmap config. - */ - if (sid_to_uid(&ace_nt->trustee, &uid) && uid == ownerUID) { - ace_v4->who.uid = uid; - } else if (sid_to_gid(&ace_nt->trustee, &gid)) { + if (sid_to_gid(&ace_nt->trustee, &gid)) { ace_v4->aceFlags |= SMB_ACE4_IDENTIFIER_GROUP; ace_v4->who.gid = gid; } else if (sid_to_uid(&ace_nt->trustee, &uid)) { -- 2.17.0 From 3f5a6810aa835696e6210bd498db8017ad6c8d0d Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 11 Jun 2019 16:15:10 -0700 Subject: [PATCH 03/40] nfs4_acls: Remove fsp from smbacl4_win2nfs4 Only the information whether the ACL is for a file or a directory is required. Replacing the fsp with a flag is clearer and allows for unit testing of the mapping functions. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit a06486bb110d04a90b66a0bca4b1b600ef3c0ebf) --- source3/modules/nfs4_acls.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index 6db5a6db6d9..5543b3a7f58 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -648,7 +648,7 @@ static SMB_ACE4PROP_T *smbacl4_find_equal_special( static bool smbacl4_fill_ace4( - const struct smb_filename *filename, + bool is_directory, const struct smbacl4_vfs_params *params, uid_t ownerUID, gid_t ownerGID, @@ -670,8 +670,7 @@ static bool smbacl4_fill_ace4( ace_nt->flags); /* remove inheritance flags on files */ - if (VALID_STAT(filename->st) && - !S_ISDIR(filename->st.st_ex_mode)) { + if (!is_directory) { DEBUG(10, ("Removing inheritance flags from a file\n")); ace_v4->aceFlags &= ~(SMB_ACE4_FILE_INHERIT_ACE| SMB_ACE4_DIRECTORY_INHERIT_ACE| @@ -732,9 +731,8 @@ static bool smbacl4_fill_ace4( &global_sid_Unix_NFS) == 0) { return false; } else { - DEBUG(1, ("nfs4_acls.c: file [%s]: could not " + DEBUG(1, ("nfs4_acls.c: could not " "convert %s to uid or gid\n", - filename->base_name, dom_sid_str_buf(&ace_nt->trustee, &buf))); return false; } @@ -855,7 +853,7 @@ static int smbacl4_substitute_simple( static struct SMB4ACL_T *smbacl4_win2nfs4( TALLOC_CTX *mem_ctx, - const files_struct *fsp, + bool is_directory, const struct security_acl *dacl, const struct smbacl4_vfs_params *pparams, uid_t ownerUID, @@ -864,7 +862,6 @@ static struct SMB4ACL_T *smbacl4_win2nfs4( { struct SMB4ACL_T *theacl; uint32_t i; - const char *filename = fsp->fsp_name->base_name; DEBUG(10, ("smbacl4_win2nfs4 invoked\n")); @@ -876,12 +873,11 @@ static struct SMB4ACL_T *smbacl4_win2nfs4( SMB_ACE4PROP_T ace_v4; bool addNewACE = true; - if (!smbacl4_fill_ace4(fsp->fsp_name, pparams, + if (!smbacl4_fill_ace4(is_directory, pparams, ownerUID, ownerGID, dacl->aces + i, &ace_v4)) { struct dom_sid_buf buf; - DEBUG(3, ("Could not fill ace for file %s, SID %s\n", - filename, + DEBUG(3, ("Could not fill ace for file, SID %s\n", dom_sid_str_buf(&((dacl->aces+i)->trustee), &buf))); continue; @@ -916,7 +912,7 @@ NTSTATUS smb_set_nt_acl_nfs4(vfs_handle_struct *handle, files_struct *fsp, { struct smbacl4_vfs_params params; struct SMB4ACL_T *theacl = NULL; - bool result; + bool result, is_directory; SMB_STRUCT_STAT sbuf; bool set_acl_as_root = false; @@ -951,6 +947,8 @@ NTSTATUS smb_set_nt_acl_nfs4(vfs_handle_struct *handle, files_struct *fsp, return map_nt_error_from_unix(errno); } + is_directory = S_ISDIR(sbuf.st_ex_mode); + if (pparams->do_chown) { /* chown logic is a copy/paste from posix_acl.c:set_nt_acl */ NTSTATUS status = unpack_nt_owners(fsp->conn, &newUID, &newGID, @@ -998,7 +996,7 @@ NTSTATUS smb_set_nt_acl_nfs4(vfs_handle_struct *handle, files_struct *fsp, return NT_STATUS_OK; } - theacl = smbacl4_win2nfs4(frame, fsp, psd->dacl, pparams, + theacl = smbacl4_win2nfs4(frame, is_directory, psd->dacl, pparams, sbuf.st_ex_uid, sbuf.st_ex_gid); if (!theacl) { TALLOC_FREE(frame); -- 2.17.0 From 5c2e61eb2663b139de90a52d7a878d10faf710a3 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:22:13 -0700 Subject: [PATCH 04/40] selftest: Start implementing unit test for nfs4_acls Existing smbtorture tests set and query ACLs through SMB, only working with the DACLs in the Security Descriptors, but never check the NFSv4 ACL representation. This patch introduces a unit test to verify the mapping between between Security Descriptors and NFSv4 ACLs. As the mapping code queries id mappings, the id mapping cache is first primed with the mappings used by the tests and those mappings are removed again during teardown. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 8fb906a1860452a320c79ac87917a97303729c19) --- source3/modules/test_nfs4_acls.c | 136 +++++++++++++++++++++++++++++++ source3/modules/wscript_build | 5 ++ source3/selftest/tests.py | 4 + 3 files changed, 145 insertions(+) create mode 100644 source3/modules/test_nfs4_acls.c diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c new file mode 100644 index 00000000000..557f27c7428 --- /dev/null +++ b/source3/modules/test_nfs4_acls.c @@ -0,0 +1,136 @@ +/* + * Unix SMB/CIFS implementation. + * + * Unit test for NFS4 ACL handling + * + * Copyright (C) Christof Schmitt 2019 + * + * 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 . + */ + +#include "nfs4_acls.c" +#include "librpc/gen_ndr/idmap.h" +#include "idmap_cache.h" +#include + +struct test_sids { + const char *sid_str; + struct unixid unix_id; +} test_sids[] = { + { "S-1-5-2-123-456-789-100", { 1000, ID_TYPE_UID }}, + { "S-1-5-2-123-456-789-101", { 1001, ID_TYPE_GID }}, + { "S-1-5-2-123-456-789-102", { 1002, ID_TYPE_BOTH }}, + { SID_CREATOR_OWNER, { 1003, ID_TYPE_UID }}, + { SID_CREATOR_GROUP, { 1004, ID_TYPE_GID }}, + { "S-1-5-2-123-456-789-103", { 1000, ID_TYPE_GID }}, + { "S-1-5-2-123-456-789-104", { 1005, ID_TYPE_BOTH }}, + { "S-1-5-2-123-456-789-105", { 1006, ID_TYPE_BOTH }}, + { "S-1-5-2-123-456-789-106", { 1007, ID_TYPE_BOTH }}, +}; + +static int group_setup(void **state) +{ + struct dom_sid *sids = NULL; + int i; + + sids = talloc_array(NULL, struct dom_sid, ARRAY_SIZE(test_sids)); + assert_non_null(sids); + + for (i = 0; i < ARRAY_SIZE(test_sids); i++) { + assert_true(dom_sid_parse(test_sids[i].sid_str, &sids[i])); + idmap_cache_set_sid2unixid(&sids[i], &test_sids[i].unix_id); + } + + *state = sids; + + return 0; + +} + +static int group_teardown(void **state) +{ + struct dom_sid *sids = *state; + int i; + + for (i = 0; i < ARRAY_SIZE(test_sids); i++) { + assert_true(idmap_cache_del_sid(&sids[i])); + } + + TALLOC_FREE(sids); + *state = NULL; + + return 0; +} + +/* + * Run this as first test to verify that the id mappings used by other + * tests are available in the cache. + */ +static void test_cached_id_mappings(void **state) +{ + struct dom_sid *sids = *state; + int i; + + for (i = 0; i < ARRAY_SIZE(test_sids); i++) { + struct dom_sid *sid = &sids[i]; + struct unixid *unix_id = &test_sids[i].unix_id; + uid_t uid; + gid_t gid; + + switch(unix_id->type) { + case ID_TYPE_UID: + assert_true(sid_to_uid(sid, &uid)); + assert_int_equal(uid, unix_id->id); + assert_false(sid_to_gid(sid, &gid)); + break; + case ID_TYPE_GID: + assert_false(sid_to_uid(sid, &uid)); + assert_true(sid_to_gid(sid, &gid)); + assert_int_equal(gid, unix_id->id); + break; + case ID_TYPE_BOTH: + assert_true(sid_to_uid(sid, &uid)); + assert_int_equal(uid, unix_id->id); + assert_true(sid_to_gid(sid, &gid)); + assert_int_equal(gid, unix_id->id); + break; + default: + fail_msg("Unknown id type %d\n", unix_id->type); + break; + } + } +} + +int main(int argc, char **argv) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_cached_id_mappings), + }; + + cmocka_set_message_output(CM_OUTPUT_SUBUNIT); + + if (argc != 2) { + print_error("Usage: %s smb.conf\n", argv[0]); + exit(1); + } + + /* + * Initialize enough of the Samba internals to have the + * mappings tests work. + */ + talloc_stackframe(); + lp_load_global(argv[1]); + + return cmocka_run_group_tests(tests, group_setup, group_teardown); +} diff --git a/source3/modules/wscript_build b/source3/modules/wscript_build index 238b55549a8..fa5e1425665 100644 --- a/source3/modules/wscript_build +++ b/source3/modules/wscript_build @@ -4,6 +4,11 @@ bld.SAMBA3_SUBSYSTEM('NFS4_ACLS', source='nfs4_acls.c', deps='samba-util tdb') +bld.SAMBA3_BINARY('test_nfs4_acls', + source='test_nfs4_acls.c', + deps='smbd_base cmocka', + install=False) + bld.SAMBA3_SUBSYSTEM('vfs_acl_common', source='vfs_acl_common.c') diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 64546900d83..a473309676b 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -367,6 +367,10 @@ if with_pthreadpool and have_ldwrap: plantestsuite("samba3.pthreadpool_cmocka", "none", [os.path.join(bindir(), "pthreadpooltest_cmocka")]) +plantestsuite("samba3.test_nfs4_acl", "none", + [os.path.join(bindir(), "test_nfs4_acls"), + "$SMB_CONF_PATH"]) + plantestsuite("samba3.async_req", "nt4_dc", [os.path.join(samba3srcdir, "script/tests/test_async_req.sh")]) -- 2.17.0 From e0f9274ce4580dd4aeda0c7a25306893586f6d38 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:23:40 -0700 Subject: [PATCH 05/40] test_nfs4_acls: Add tests for mapping of empty ACLs This is a fairly simple test that ensures the mapping of empty ACLs (without any ACL entries) is always done the same way. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 00f494b25f4e1d1aecf6191523e30f20a90b1e4f) --- source3/modules/test_nfs4_acls.c | 53 ++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index 557f27c7428..18322afb4a0 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -112,10 +112,63 @@ static void test_cached_id_mappings(void **state) } } +static void test_empty_nfs4_to_dacl(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + struct SMB4ACL_T *nfs4_acl; + struct security_ace *dacl_aces; + int good_aces; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + + nfs4_acl = smb_create_smb4acl(frame); + assert_non_null(nfs4_acl); + + assert_true(smbacl4_nfs42win(frame, ¶ms, nfs4_acl, + &sids[0], &sids[1], false, + &dacl_aces, &good_aces)); + + assert_int_equal(good_aces, 0); + assert_null(dacl_aces); + + TALLOC_FREE(frame); +} + +static void test_empty_dacl_to_nfs4(void **state) +{ + TALLOC_CTX *frame = talloc_stackframe(); + struct SMB4ACL_T *nfs4_acl; + struct security_acl *dacl; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + + dacl = make_sec_acl(frame, SECURITY_ACL_REVISION_ADS, 0, NULL); + assert_non_null(dacl); + + nfs4_acl = smbacl4_win2nfs4(frame, false, dacl, ¶ms, 1001, 1002); + + assert_non_null(nfs4_acl); + assert_int_equal(smbacl4_get_controlflags(nfs4_acl), + SEC_DESC_SELF_RELATIVE); + assert_int_equal(smb_get_naces(nfs4_acl), 0); + assert_null(smb_first_ace4(nfs4_acl)); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { cmocka_unit_test(test_cached_id_mappings), + cmocka_unit_test(test_empty_nfs4_to_dacl), + cmocka_unit_test(test_empty_dacl_to_nfs4), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From ca939e77c1accbba44b0e347e9a6cd91874640a5 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:25:33 -0700 Subject: [PATCH 06/40] test_nfs4_acls: Add tests for mapping of ACL types Add testcases for mapping the type field (ALLOW or DENY) between NFSv4 ACLs and security descriptors. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit dd5934797526ebb4c6f3027a809401dad3abf701) --- source3/modules/test_nfs4_acls.c | 107 +++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index 18322afb4a0..b29714d23e3 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -163,12 +163,119 @@ static void test_empty_dacl_to_nfs4(void **state) assert_null(smb_first_ace4(nfs4_acl)); } +struct ace_dacl_type_mapping { + uint32_t nfs4_type; + enum security_ace_type dacl_type; +} ace_dacl_type_mapping[] = { + { SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, SEC_ACE_TYPE_ACCESS_ALLOWED }, + { SMB_ACE4_ACCESS_DENIED_ACE_TYPE, SEC_ACE_TYPE_ACCESS_DENIED }, +}; + +static void test_acl_type_nfs4_to_dacl(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + int i; + + for (i = 0; i < ARRAY_SIZE(ace_dacl_type_mapping); i++) { + struct SMB4ACL_T *nfs4_acl; + SMB_ACE4PROP_T nfs4_ace; + struct security_ace *dacl_aces; + int good_aces; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + + nfs4_acl = smb_create_smb4acl(frame); + assert_non_null(nfs4_acl); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = 0, + .who.uid = 1000, + .aceType = ace_dacl_type_mapping[i].nfs4_type, + .aceFlags = 0, + .aceMask = SMB_ACE4_READ_DATA, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + assert_true(smbacl4_nfs42win(frame, ¶ms, nfs4_acl, + &sids[2], &sids[3], false, + &dacl_aces, &good_aces)); + + assert_int_equal(good_aces, 1); + assert_non_null(dacl_aces); + + assert_int_equal(dacl_aces[0].type, + ace_dacl_type_mapping[i].dacl_type); + assert_int_equal(dacl_aces[0].flags, 0); + assert_int_equal(dacl_aces[0].access_mask, SEC_FILE_READ_DATA); + assert_true(dom_sid_equal(&dacl_aces[0].trustee, &sids[0])); + } + + TALLOC_FREE(frame); +} + +static void test_acl_type_dacl_to_nfs4(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + int i; + + for (i = 0; i < ARRAY_SIZE(ace_dacl_type_mapping); i++) { + struct SMB4ACL_T *nfs4_acl; + struct SMB4ACE_T *nfs4_ace_container; + SMB_ACE4PROP_T *nfs4_ace; + struct security_ace dacl_aces[1]; + struct security_acl *dacl; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + + init_sec_ace(&dacl_aces[0], &sids[0], + ace_dacl_type_mapping[i].dacl_type, + SEC_FILE_READ_DATA, 0); + dacl = make_sec_acl(frame, SECURITY_ACL_REVISION_ADS, + ARRAY_SIZE(dacl_aces), dacl_aces); + assert_non_null(dacl); + + nfs4_acl = smbacl4_win2nfs4(frame, false, dacl, ¶ms, + 101, 102); + + assert_non_null(nfs4_acl); + assert_int_equal(smbacl4_get_controlflags(nfs4_acl), + SEC_DESC_SELF_RELATIVE); + assert_int_equal(smb_get_naces(nfs4_acl), 1); + + nfs4_ace_container = smb_first_ace4(nfs4_acl); + assert_non_null(nfs4_ace_container); + assert_null(smb_next_ace4(nfs4_ace_container)); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->who.uid, 1000); + assert_int_equal(nfs4_ace->aceFlags, 0); + assert_int_equal(nfs4_ace->aceType, + ace_dacl_type_mapping[i].nfs4_type); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + } + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { cmocka_unit_test(test_cached_id_mappings), cmocka_unit_test(test_empty_nfs4_to_dacl), cmocka_unit_test(test_empty_dacl_to_nfs4), + cmocka_unit_test(test_acl_type_nfs4_to_dacl), + cmocka_unit_test(test_acl_type_dacl_to_nfs4), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From 023cbb1f1c35afe790767ba841e0a52dec132c8c Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:28:31 -0700 Subject: [PATCH 07/40] test_nfs4_acls: Add test for flags mapping from NFS4 ACL to DACL Add testcase for the mapping of inheritance flags when mapping from a NFSv4 ACL to a DACL in the security descriptor. The mapping is different between files and directories, as some inheritance flags should never be present for files. Some defined flags like SUCCESSFUL_ACCESS are also not mapped at this point, also verify this behavior. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 16eb61a900c6749c2554d635ce2dd903f5de1704) --- source3/modules/test_nfs4_acls.c | 87 ++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index b29714d23e3..47ae14d0e65 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -268,6 +268,92 @@ static void test_acl_type_dacl_to_nfs4(void **state) TALLOC_FREE(frame); } +struct ace_flag_mapping_nfs4_to_dacl { + bool is_directory; + uint32_t nfs4_flag; + uint32_t dacl_flag; +} ace_flags_nfs4_to_dacl[] = { + { true, SMB_ACE4_FILE_INHERIT_ACE, + SEC_ACE_FLAG_OBJECT_INHERIT }, + { false, SMB_ACE4_FILE_INHERIT_ACE, + 0 }, + { true, SMB_ACE4_DIRECTORY_INHERIT_ACE, + SEC_ACE_FLAG_CONTAINER_INHERIT }, + { false, SMB_ACE4_DIRECTORY_INHERIT_ACE, + 0 }, + { true, SMB_ACE4_NO_PROPAGATE_INHERIT_ACE, + SEC_ACE_FLAG_NO_PROPAGATE_INHERIT }, + { false, SMB_ACE4_NO_PROPAGATE_INHERIT_ACE, + SEC_ACE_FLAG_NO_PROPAGATE_INHERIT }, + { true, SMB_ACE4_INHERIT_ONLY_ACE, + SEC_ACE_FLAG_INHERIT_ONLY }, + { false, SMB_ACE4_INHERIT_ONLY_ACE, + SEC_ACE_FLAG_INHERIT_ONLY }, + { true, SMB_ACE4_SUCCESSFUL_ACCESS_ACE_FLAG, + 0 }, + { false, SMB_ACE4_SUCCESSFUL_ACCESS_ACE_FLAG, + 0 }, + { true, SMB_ACE4_FAILED_ACCESS_ACE_FLAG, + 0 }, + { false, SMB_ACE4_FAILED_ACCESS_ACE_FLAG, + 0 }, + { true, SMB_ACE4_INHERITED_ACE, + SEC_ACE_FLAG_INHERITED_ACE }, + { false, SMB_ACE4_INHERITED_ACE, + SEC_ACE_FLAG_INHERITED_ACE }, +}; + +static void test_ace_flags_nfs4_to_dacl(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + SMB_ACE4PROP_T nfs4_ace; + int i; + + for (i = 0; i < ARRAY_SIZE(ace_flags_nfs4_to_dacl); i++) { + struct SMB4ACL_T *nfs4_acl; + bool is_directory; + struct security_ace *dacl_aces; + int good_aces; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + + nfs4_acl = smb_create_smb4acl(frame); + assert_non_null(nfs4_acl); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = 0, + .who.uid = 1000, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = ace_flags_nfs4_to_dacl[i].nfs4_flag, + .aceMask = SMB_ACE4_READ_DATA, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + is_directory = ace_flags_nfs4_to_dacl[i].is_directory; + + assert_true(smbacl4_nfs42win(frame, ¶ms, nfs4_acl, + &sids[2], &sids[3], is_directory, + &dacl_aces, &good_aces)); + + assert_int_equal(good_aces, 1); + assert_non_null(dacl_aces); + + assert_int_equal(dacl_aces[0].type, + SEC_ACE_TYPE_ACCESS_ALLOWED); + assert_int_equal(dacl_aces[0].flags, + ace_flags_nfs4_to_dacl[i].dacl_flag); + assert_int_equal(dacl_aces[0].access_mask, SEC_FILE_READ_DATA); + assert_true(dom_sid_equal(&dacl_aces[0].trustee, &sids[0])); + } + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -276,6 +362,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_empty_dacl_to_nfs4), cmocka_unit_test(test_acl_type_nfs4_to_dacl), cmocka_unit_test(test_acl_type_dacl_to_nfs4), + cmocka_unit_test(test_ace_flags_nfs4_to_dacl), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From 55307e7b938a298f14a11f7ed9e3f4d24632f961 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:30:12 -0700 Subject: [PATCH 08/40] test_nfs4_acls: Add test for flags mapping from DACL to NFS4 ACL Add testcase for the mapping of inheritance flags from the DACL in the security descriptor to the NFSv4 ACL. The mapping is different for files and directories as some inheritance flags should not be present for files. Also other flags are not mapped at all, verify this behavior. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit bccd2612761e26ee2514935d56927b2c0c000859) --- source3/modules/test_nfs4_acls.c | 87 ++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index 47ae14d0e65..a0e7db41b70 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -354,6 +354,92 @@ static void test_ace_flags_nfs4_to_dacl(void **state) TALLOC_FREE(frame); } +struct ace_flag_mapping_dacl_to_nfs4 { + bool is_directory; + uint32_t dacl_flag; + uint32_t nfs4_flag; +} ace_flags_dacl_to_nfs4[] = { + { true, SEC_ACE_FLAG_OBJECT_INHERIT, + SMB_ACE4_FILE_INHERIT_ACE }, + { false, SEC_ACE_FLAG_OBJECT_INHERIT, + 0 }, + { true, SEC_ACE_FLAG_CONTAINER_INHERIT, + SMB_ACE4_DIRECTORY_INHERIT_ACE }, + { false, SEC_ACE_FLAG_CONTAINER_INHERIT, + 0 }, + { true, SEC_ACE_FLAG_NO_PROPAGATE_INHERIT, + SMB_ACE4_NO_PROPAGATE_INHERIT_ACE }, + { false, SEC_ACE_FLAG_NO_PROPAGATE_INHERIT, + 0 }, + { true, SEC_ACE_FLAG_INHERIT_ONLY, + SMB_ACE4_INHERIT_ONLY_ACE }, + { false, SEC_ACE_FLAG_INHERIT_ONLY, + 0 }, + { true, SEC_ACE_FLAG_INHERITED_ACE, + SMB_ACE4_INHERITED_ACE }, + { false, SEC_ACE_FLAG_INHERITED_ACE, + SMB_ACE4_INHERITED_ACE }, + { true, SEC_ACE_FLAG_SUCCESSFUL_ACCESS, + 0 }, + { false, SEC_ACE_FLAG_SUCCESSFUL_ACCESS, + 0 }, + { true, SEC_ACE_FLAG_FAILED_ACCESS, + 0 }, + { false, SEC_ACE_FLAG_FAILED_ACCESS, + 0 }, +}; + +static void test_ace_flags_dacl_to_nfs4(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + int i; + + for (i = 0; i < ARRAY_SIZE(ace_flags_dacl_to_nfs4); i++) { + struct SMB4ACL_T *nfs4_acl; + struct SMB4ACE_T *nfs4_ace_container; + SMB_ACE4PROP_T *nfs4_ace; + bool is_directory; + struct security_ace dacl_aces[1]; + struct security_acl *dacl; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + + init_sec_ace(&dacl_aces[0], &sids[0], + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + ace_flags_dacl_to_nfs4[i].dacl_flag); + dacl = make_sec_acl(frame, SECURITY_ACL_REVISION_ADS, + ARRAY_SIZE(dacl_aces), dacl_aces); + assert_non_null(dacl); + + is_directory = ace_flags_dacl_to_nfs4[i].is_directory; + nfs4_acl = smbacl4_win2nfs4(frame, is_directory, dacl, ¶ms, + 101, 102); + + assert_non_null(nfs4_acl); + assert_int_equal(smbacl4_get_controlflags(nfs4_acl), + SEC_DESC_SELF_RELATIVE); + assert_int_equal(smb_get_naces(nfs4_acl), 1); + + nfs4_ace_container = smb_first_ace4(nfs4_acl); + assert_non_null(nfs4_ace_container); + assert_null(smb_next_ace4(nfs4_ace_container)); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->who.uid, 1000); + assert_int_equal(nfs4_ace->aceFlags, + ace_flags_dacl_to_nfs4[i].nfs4_flag); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + } + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -363,6 +449,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_acl_type_nfs4_to_dacl), cmocka_unit_test(test_acl_type_dacl_to_nfs4), cmocka_unit_test(test_ace_flags_nfs4_to_dacl), + cmocka_unit_test(test_ace_flags_dacl_to_nfs4), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From 0beae5fe9ffff10f9fe9bd833680283268b4d0a3 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:33:29 -0700 Subject: [PATCH 09/40] test_nfs4_acls: Add test for mapping permissions from NFS4 ACL to DACL Add testcase for mapping permissions from the NFSv4 ACL to DACL in the security descriptor. The mapping is simple as each permission bit exists on both sides. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 1767027b44a9e4ebd865022e3f8abb0c72bf15c6) --- source3/modules/test_nfs4_acls.c | 77 ++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index a0e7db41b70..42a69453f5a 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -440,6 +440,82 @@ static void test_ace_flags_dacl_to_nfs4(void **state) TALLOC_FREE(frame); } +struct ace_perm_mapping { + uint32_t nfs4_perm; + uint32_t dacl_perm; +} perm_table_nfs4_to_dacl[] = { + { SMB_ACE4_READ_DATA, SEC_FILE_READ_DATA }, + { SMB_ACE4_LIST_DIRECTORY, SEC_DIR_LIST }, + { SMB_ACE4_WRITE_DATA, SEC_FILE_WRITE_DATA }, + { SMB_ACE4_ADD_FILE, SEC_DIR_ADD_FILE }, + { SMB_ACE4_APPEND_DATA, SEC_FILE_APPEND_DATA }, + { SMB_ACE4_ADD_SUBDIRECTORY, SEC_DIR_ADD_SUBDIR, }, + { SMB_ACE4_READ_NAMED_ATTRS, SEC_FILE_READ_EA }, + { SMB_ACE4_READ_NAMED_ATTRS, SEC_DIR_READ_EA }, + { SMB_ACE4_WRITE_NAMED_ATTRS, SEC_FILE_WRITE_EA }, + { SMB_ACE4_WRITE_NAMED_ATTRS, SEC_DIR_WRITE_EA }, + { SMB_ACE4_EXECUTE, SEC_FILE_EXECUTE }, + { SMB_ACE4_EXECUTE, SEC_DIR_TRAVERSE }, + { SMB_ACE4_DELETE_CHILD, SEC_DIR_DELETE_CHILD }, + { SMB_ACE4_READ_ATTRIBUTES, SEC_FILE_READ_ATTRIBUTE }, + { SMB_ACE4_READ_ATTRIBUTES, SEC_DIR_READ_ATTRIBUTE }, + { SMB_ACE4_WRITE_ATTRIBUTES, SEC_FILE_WRITE_ATTRIBUTE }, + { SMB_ACE4_WRITE_ATTRIBUTES, SEC_DIR_WRITE_ATTRIBUTE }, + { SMB_ACE4_DELETE, SEC_STD_DELETE }, + { SMB_ACE4_READ_ACL, SEC_STD_READ_CONTROL }, + { SMB_ACE4_WRITE_ACL, SEC_STD_WRITE_DAC, }, + { SMB_ACE4_WRITE_OWNER, SEC_STD_WRITE_OWNER }, + { SMB_ACE4_SYNCHRONIZE, SEC_STD_SYNCHRONIZE }, +}; + +static void test_nfs4_permissions_to_dacl(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + int i; + + for (i = 0; i < ARRAY_SIZE(perm_table_nfs4_to_dacl); i++) { + struct SMB4ACL_T *nfs4_acl; + SMB_ACE4PROP_T nfs4_ace; + struct security_ace *dacl_aces; + int good_aces; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + + nfs4_acl = smb_create_smb4acl(frame); + assert_non_null(nfs4_acl); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = 0, + .who.uid = 1000, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = 0, + .aceMask = perm_table_nfs4_to_dacl[i].nfs4_perm, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + assert_true(smbacl4_nfs42win(frame, ¶ms, nfs4_acl, + &sids[0], &sids[1], false, + &dacl_aces, &good_aces)); + + assert_int_equal(good_aces, 1); + assert_non_null(dacl_aces); + + assert_int_equal(dacl_aces[0].type, + SEC_ACE_TYPE_ACCESS_ALLOWED); + assert_int_equal(dacl_aces[0].flags, 0); + assert_int_equal(dacl_aces[0].access_mask, + perm_table_nfs4_to_dacl[i].dacl_perm); + assert_true(dom_sid_equal(&dacl_aces[0].trustee, &sids[0])); + } + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -450,6 +526,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_acl_type_dacl_to_nfs4), cmocka_unit_test(test_ace_flags_nfs4_to_dacl), cmocka_unit_test(test_ace_flags_dacl_to_nfs4), + cmocka_unit_test(test_nfs4_permissions_to_dacl), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From 374c3b99bdf43bdc1fdd840c9da40569f8c566b8 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:35:34 -0700 Subject: [PATCH 10/40] test_nfs4_acls: Add test for mapping permissions from DACL to NFS4 ACL Add testcase for mapping the permission flags from the DACL in the Security Descriptor to a NFSv4 ACL. The mapping is straight-forward as the same permission bits exist for Security Descriptors and NFSv4 ACLs. In addition, the code also maps from the generic DACL permissions to a set of NFSv4 permissions, also verify this mapping. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit e4840e680744bd860beedeb5123704c3c0d6a4d7) --- source3/modules/test_nfs4_acls.c | 106 +++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index 42a69453f5a..d77eceb1b88 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -516,6 +516,111 @@ static void test_nfs4_permissions_to_dacl(void **state) TALLOC_FREE(frame); } +struct ace_perm_mapping_dacl_to_nfs4 { + uint32_t dacl_perm; + uint32_t nfs4_perm; +} perm_table_dacl_to_nfs4[] = { + { SEC_FILE_READ_DATA, SMB_ACE4_READ_DATA, }, + { SEC_DIR_LIST, SMB_ACE4_LIST_DIRECTORY, }, + { SEC_FILE_WRITE_DATA, SMB_ACE4_WRITE_DATA, }, + { SEC_DIR_ADD_FILE, SMB_ACE4_ADD_FILE, }, + { SEC_FILE_APPEND_DATA, SMB_ACE4_APPEND_DATA, }, + { SEC_DIR_ADD_SUBDIR, SMB_ACE4_ADD_SUBDIRECTORY, }, + { SEC_FILE_READ_EA, SMB_ACE4_READ_NAMED_ATTRS, }, + { SEC_DIR_READ_EA, SMB_ACE4_READ_NAMED_ATTRS, }, + { SEC_FILE_WRITE_EA, SMB_ACE4_WRITE_NAMED_ATTRS, }, + { SEC_DIR_WRITE_EA, SMB_ACE4_WRITE_NAMED_ATTRS, }, + { SEC_FILE_EXECUTE, SMB_ACE4_EXECUTE, }, + { SEC_DIR_TRAVERSE, SMB_ACE4_EXECUTE, }, + { SEC_DIR_DELETE_CHILD, SMB_ACE4_DELETE_CHILD, }, + { SEC_FILE_READ_ATTRIBUTE, SMB_ACE4_READ_ATTRIBUTES, }, + { SEC_DIR_READ_ATTRIBUTE, SMB_ACE4_READ_ATTRIBUTES, }, + { SEC_FILE_WRITE_ATTRIBUTE, SMB_ACE4_WRITE_ATTRIBUTES, }, + { SEC_DIR_WRITE_ATTRIBUTE, SMB_ACE4_WRITE_ATTRIBUTES, }, + { SEC_STD_DELETE, SMB_ACE4_DELETE, }, + { SEC_STD_READ_CONTROL, SMB_ACE4_READ_ACL, }, + { SEC_STD_WRITE_DAC, SMB_ACE4_WRITE_ACL, }, + { SEC_STD_WRITE_OWNER, SMB_ACE4_WRITE_OWNER, }, + { SEC_STD_SYNCHRONIZE, SMB_ACE4_SYNCHRONIZE, }, + { SEC_GENERIC_READ, SMB_ACE4_READ_ACL| + SMB_ACE4_READ_DATA| + SMB_ACE4_READ_ATTRIBUTES| + SMB_ACE4_READ_NAMED_ATTRS| + SMB_ACE4_SYNCHRONIZE }, + { SEC_GENERIC_WRITE, SMB_ACE4_WRITE_ACL| + SMB_ACE4_WRITE_DATA| + SMB_ACE4_WRITE_ATTRIBUTES| + SMB_ACE4_WRITE_NAMED_ATTRS| + SMB_ACE4_SYNCHRONIZE }, + { SEC_GENERIC_EXECUTE, SMB_ACE4_READ_ACL| + SMB_ACE4_READ_ATTRIBUTES| + SMB_ACE4_EXECUTE| + SMB_ACE4_SYNCHRONIZE }, + { SEC_GENERIC_ALL, SMB_ACE4_DELETE| + SMB_ACE4_READ_ACL| + SMB_ACE4_WRITE_ACL| + SMB_ACE4_WRITE_OWNER| + SMB_ACE4_SYNCHRONIZE| + SMB_ACE4_WRITE_ATTRIBUTES| + SMB_ACE4_READ_ATTRIBUTES| + SMB_ACE4_EXECUTE| + SMB_ACE4_READ_NAMED_ATTRS| + SMB_ACE4_WRITE_NAMED_ATTRS| + SMB_ACE4_WRITE_DATA| + SMB_ACE4_APPEND_DATA| + SMB_ACE4_READ_DATA| + SMB_ACE4_DELETE_CHILD }, +}; + +static void test_dacl_permissions_to_nfs4(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + int i; + + for (i = 0; i < ARRAY_SIZE(perm_table_nfs4_to_dacl); i++) { + struct SMB4ACL_T *nfs4_acl; + struct SMB4ACE_T *nfs4_ace_container; + SMB_ACE4PROP_T *nfs4_ace; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + struct security_ace dacl_aces[1]; + struct security_acl *dacl; + + init_sec_ace(&dacl_aces[0], &sids[0], + SEC_ACE_TYPE_ACCESS_ALLOWED, + perm_table_dacl_to_nfs4[i].dacl_perm, 0); + dacl = make_sec_acl(frame, SECURITY_ACL_REVISION_ADS, + ARRAY_SIZE(dacl_aces), dacl_aces); + assert_non_null(dacl); + + nfs4_acl = smbacl4_win2nfs4(frame, false, dacl, ¶ms, + 101, 102); + + assert_non_null(nfs4_acl); + assert_int_equal(smbacl4_get_controlflags(nfs4_acl), + SEC_DESC_SELF_RELATIVE); + assert_int_equal(smb_get_naces(nfs4_acl), 1); + + nfs4_ace_container = smb_first_ace4(nfs4_acl); + assert_non_null(nfs4_ace_container); + assert_null(smb_next_ace4(nfs4_ace_container)); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->who.uid, 1000); + assert_int_equal(nfs4_ace->aceFlags, 0); + assert_int_equal(nfs4_ace->aceMask, + perm_table_dacl_to_nfs4[i].nfs4_perm); + } + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -527,6 +632,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_ace_flags_nfs4_to_dacl), cmocka_unit_test(test_ace_flags_dacl_to_nfs4), cmocka_unit_test(test_nfs4_permissions_to_dacl), + cmocka_unit_test(test_dacl_permissions_to_nfs4), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From 11675a050e5d6d9fb288a691b8bf680a5ef19b78 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:46:23 -0700 Subject: [PATCH 11/40] test_nfs4_acls: Add test for mapping of special NFS4 ACL entries to DACL entries In addition to entries for users and groups, NFSv4 ACLs have the concept of entries for "special" entries. Only the "owner", "group" and "everyone" entries are currently used in the ACL mapping. Add a testcase that verifies the mapping from NFSv4 "special" entries to the DACL in the security descriptor. Verify that only "owner", "group" and "everyone" are mapped and all other "special" entries are ignored. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit f86148948c7f89307a34e31f6ddede6923149d34) --- source3/modules/test_nfs4_acls.c | 139 +++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index d77eceb1b88..5b5b37adc82 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -621,6 +621,144 @@ static void test_dacl_permissions_to_nfs4(void **state) TALLOC_FREE(frame); } +/* + * Create NFS4 ACL with all possible "special" entries. Verify that + * the ones that should be mapped to a DACL are mapped and the other + * ones are ignored. + */ +static void test_special_nfs4_to_dacl(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + struct SMB4ACL_T *nfs4_acl; + SMB_ACE4PROP_T nfs4_ace; + struct security_ace *dacl_aces; + int good_aces; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + + nfs4_acl = smb_create_smb4acl(frame); + assert_non_null(nfs4_acl); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id = SMB_ACE4_WHO_OWNER, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = 0, + .aceMask = SMB_ACE4_READ_DATA, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id = SMB_ACE4_WHO_GROUP, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = 0, + .aceMask = SMB_ACE4_WRITE_DATA, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id = SMB_ACE4_WHO_EVERYONE, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = 0, + .aceMask = SMB_ACE4_APPEND_DATA, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id = SMB_ACE4_WHO_INTERACTIVE, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = 0, + .aceMask = SMB_ACE4_READ_NAMED_ATTRS, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id = SMB_ACE4_WHO_NETWORK, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = 0, + .aceMask = SMB_ACE4_WRITE_NAMED_ATTRS, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id = SMB_ACE4_WHO_DIALUP, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = 0, + .aceMask = SMB_ACE4_EXECUTE, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id = SMB_ACE4_WHO_BATCH, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = 0, + .aceMask = SMB_ACE4_READ_ATTRIBUTES, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id = SMB_ACE4_WHO_ANONYMOUS, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = 0, + .aceMask = SMB_ACE4_WRITE_ATTRIBUTES, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id = SMB_ACE4_WHO_AUTHENTICATED, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = 0, + .aceMask = SMB_ACE4_READ_ACL, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id = SMB_ACE4_WHO_SERVICE, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = 0, + .aceMask = SMB_ACE4_WRITE_ACL, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + assert_true(smbacl4_nfs42win(frame, ¶ms, nfs4_acl, + &sids[0], &sids[1], false, + &dacl_aces, &good_aces)); + + assert_int_equal(good_aces, 3); + assert_non_null(dacl_aces); + + assert_int_equal(dacl_aces[0].type, SEC_ACE_TYPE_ACCESS_ALLOWED); + assert_int_equal(dacl_aces[0].flags, 0); + assert_int_equal(dacl_aces[0].access_mask, SEC_FILE_READ_DATA); + assert_true(dom_sid_equal(&dacl_aces[0].trustee, &sids[0])); + + assert_int_equal(dacl_aces[1].type, SEC_ACE_TYPE_ACCESS_ALLOWED); + assert_int_equal(dacl_aces[1].flags, 0); + assert_int_equal(dacl_aces[1].access_mask, SEC_FILE_WRITE_DATA); + assert_true(dom_sid_equal(&dacl_aces[1].trustee, &sids[1])); + + assert_int_equal(dacl_aces[2].type, SEC_ACE_TYPE_ACCESS_ALLOWED); + assert_int_equal(dacl_aces[2].flags, 0); + assert_int_equal(dacl_aces[2].access_mask, SEC_FILE_APPEND_DATA); + assert_true(dom_sid_equal(&dacl_aces[2].trustee, &global_sid_World)); + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -633,6 +771,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_ace_flags_dacl_to_nfs4), cmocka_unit_test(test_nfs4_permissions_to_dacl), cmocka_unit_test(test_dacl_permissions_to_nfs4), + cmocka_unit_test(test_special_nfs4_to_dacl), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From 4545c9559fa57dde9abe1a5c102e0140931586bf Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:53:15 -0700 Subject: [PATCH 12/40] test_nfs4_acls: Add test for mapping from DACL to special NFS4 ACL entries Add testcase for mapping from entries in the DACL security descriptor to "special" entries in the NFSv4 ACL. Verify that the WORLD well-known SID maps to "everyone" in the NFSv4 ACL. Verify that the "Unix NFS" SID is ignored, as there is no meaningful mapping for this entry. Verify that SID entries matching the owner or group are mapped to "special owner" or "special group", but only if no inheritance flags are used. "special owner" and "special group" with inheritance flags have the meaning of CREATOR OWNER and CREATOR GROUP and will be tested in another testcase. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 1f1fa5bde2c76636c1beec39c21067b252ea10be) --- source3/modules/test_nfs4_acls.c | 108 +++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index 5b5b37adc82..46119f83dc4 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -759,6 +759,113 @@ static void test_special_nfs4_to_dacl(void **state) TALLOC_FREE(frame); } +static void test_dacl_to_special_nfs4(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + struct SMB4ACL_T *nfs4_acl; + struct SMB4ACE_T *nfs4_ace_container; + SMB_ACE4PROP_T *nfs4_ace; + struct security_ace dacl_aces[6]; + struct security_acl *dacl; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_dontcare, + .map_full_control = true, + }; + + /* + * global_Sid_World is mapped to EVERYONE. + */ + init_sec_ace(&dacl_aces[0], &global_sid_World, + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_WRITE_DATA, 0); + /* + * global_sid_Unix_NFS is ignored. + */ + init_sec_ace(&dacl_aces[1], &global_sid_Unix_NFS, + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, 0); + /* + * Anything that maps to owner or owning group with inheritance flags + * is NOT mapped to special owner or special group. + */ + init_sec_ace(&dacl_aces[2], &sids[0], + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT); + init_sec_ace(&dacl_aces[3], &sids[0], + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_INHERIT_ONLY); + init_sec_ace(&dacl_aces[4], &sids[1], + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT); + init_sec_ace(&dacl_aces[5], &sids[1], + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_INHERIT_ONLY); + dacl = make_sec_acl(frame, SECURITY_ACL_REVISION_ADS, + ARRAY_SIZE(dacl_aces), dacl_aces); + assert_non_null(dacl); + + nfs4_acl = smbacl4_win2nfs4(frame, true, dacl, ¶ms, 1000, 1001); + + assert_non_null(nfs4_acl); + assert_int_equal(smbacl4_get_controlflags(nfs4_acl), + SEC_DESC_SELF_RELATIVE); + assert_int_equal(smb_get_naces(nfs4_acl), 5); + + nfs4_ace_container = smb_first_ace4(nfs4_acl); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, SMB_ACE4_ID_SPECIAL); + assert_int_equal(nfs4_ace->who.special_id, SMB_ACE4_WHO_EVERYONE); + assert_int_equal(nfs4_ace->aceFlags, 0); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_WRITE_DATA); + + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->who.uid, 1000); + assert_int_equal(nfs4_ace->aceFlags, SMB_ACE4_FILE_INHERIT_ACE); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->who.uid, 1000); + assert_int_equal(nfs4_ace->aceFlags, SMB_ACE4_DIRECTORY_INHERIT_ACE| + SMB_ACE4_INHERIT_ONLY_ACE); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->aceFlags, SMB_ACE4_IDENTIFIER_GROUP| + SMB_ACE4_FILE_INHERIT_ACE); + assert_int_equal(nfs4_ace->who.gid, 1001); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->aceFlags, SMB_ACE4_IDENTIFIER_GROUP| + SMB_ACE4_DIRECTORY_INHERIT_ACE| + SMB_ACE4_INHERIT_ONLY_ACE); + assert_int_equal(nfs4_ace->who.gid, 1001); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + assert_null(smb_next_ace4(nfs4_ace_container)); + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -772,6 +879,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_nfs4_permissions_to_dacl), cmocka_unit_test(test_dacl_permissions_to_nfs4), cmocka_unit_test(test_special_nfs4_to_dacl), + cmocka_unit_test(test_dacl_to_special_nfs4), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From 342f548f42eea89a01a82125e3568c240997ea1b Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:55:59 -0700 Subject: [PATCH 13/40] test_nfs4_acls: Add test for mapping CREATOR entries to NFS4 ACL entries Add testcase for mapping DACL entries CREATOR OWNER and CREATOR GROUP with inheritance flag in the security descriptor to NFSv4 "special owner" and "special group" entries. This is the correct mapping for these entries as inheriting "special owner" and "special group" grants permissions to the actual owner and owning group of the new file or directory, similar to what CREATOR entries do. The other side is that CREATOR entries without any inheritance flags do not make sense, so these are not mapped to NFSv4 ACL entries. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit bfcc19b705f83bdd5cf665fd4daf43e7eae997a9) --- source3/modules/test_nfs4_acls.c | 108 +++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index 46119f83dc4..dcdcb89823f 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -866,6 +866,113 @@ static void test_dacl_to_special_nfs4(void **state) TALLOC_FREE(frame); } +struct creator_ace_flags { + uint32_t dacl_flags; + uint32_t nfs4_flags; +} creator_ace_flags[] = { + { 0, 0 }, + + { SEC_ACE_FLAG_INHERIT_ONLY, 0 }, + + { SEC_ACE_FLAG_CONTAINER_INHERIT, SMB_ACE4_DIRECTORY_INHERIT_ACE| + SMB_ACE4_INHERIT_ONLY_ACE }, + + { SEC_ACE_FLAG_CONTAINER_INHERIT| + SEC_ACE_FLAG_INHERIT_ONLY, SMB_ACE4_DIRECTORY_INHERIT_ACE| + SMB_ACE4_INHERIT_ONLY_ACE }, + + { SEC_ACE_FLAG_OBJECT_INHERIT, SMB_ACE4_FILE_INHERIT_ACE| + SMB_ACE4_INHERIT_ONLY_ACE }, + { SEC_ACE_FLAG_OBJECT_INHERIT| + SEC_ACE_FLAG_INHERIT_ONLY, SMB_ACE4_FILE_INHERIT_ACE| + SMB_ACE4_INHERIT_ONLY_ACE }, + + { SEC_ACE_FLAG_CONTAINER_INHERIT| + SEC_ACE_FLAG_OBJECT_INHERIT, SMB_ACE4_DIRECTORY_INHERIT_ACE| + SMB_ACE4_FILE_INHERIT_ACE| + SMB_ACE4_INHERIT_ONLY_ACE }, + + { SEC_ACE_FLAG_CONTAINER_INHERIT| + SEC_ACE_FLAG_OBJECT_INHERIT| + SEC_ACE_FLAG_INHERIT_ONLY, SMB_ACE4_DIRECTORY_INHERIT_ACE| + SMB_ACE4_FILE_INHERIT_ACE| + SMB_ACE4_INHERIT_ONLY_ACE }, +}; + +static void test_dacl_creator_to_nfs4(void **state) +{ + TALLOC_CTX *frame = talloc_stackframe(); + int i; + + for (i = 0; i < ARRAY_SIZE(creator_ace_flags); i++) { + struct SMB4ACL_T *nfs4_acl; + struct SMB4ACE_T *nfs4_ace_container; + SMB_ACE4PROP_T *nfs4_ace; + struct security_ace dacl_aces[2]; + struct security_acl *dacl; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + + init_sec_ace(&dacl_aces[0], &global_sid_Creator_Owner, + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + creator_ace_flags[i].dacl_flags); + init_sec_ace(&dacl_aces[1], &global_sid_Creator_Group, + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + creator_ace_flags[i].dacl_flags); + dacl = make_sec_acl(frame, SECURITY_ACL_REVISION_ADS, + ARRAY_SIZE(dacl_aces), dacl_aces); + assert_non_null(dacl); + + nfs4_acl = smbacl4_win2nfs4(frame, true, dacl, ¶ms, + 101, 102); + + assert_non_null(nfs4_acl); + assert_int_equal(smbacl4_get_controlflags(nfs4_acl), + SEC_DESC_SELF_RELATIVE); + + if (creator_ace_flags[i].nfs4_flags == 0) { + /* + * CREATOR OWNER and CREATOR GROUP not mapped + * in thise case. + */ + assert_null(smb_first_ace4(nfs4_acl)); + } else { + assert_int_equal(smb_get_naces(nfs4_acl), 2); + + nfs4_ace_container = smb_first_ace4(nfs4_acl); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace); + assert_int_equal(nfs4_ace->flags, SMB_ACE4_ID_SPECIAL); + assert_int_equal(nfs4_ace->who.special_id, + SMB_ACE4_WHO_OWNER); + assert_int_equal(nfs4_ace->aceFlags, + creator_ace_flags[i].nfs4_flags); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + assert_null(smb_next_ace4(nfs4_ace_container)); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace); + assert_int_equal(nfs4_ace->flags, SMB_ACE4_ID_SPECIAL); + assert_int_equal(nfs4_ace->who.special_id, + SMB_ACE4_WHO_GROUP); + assert_int_equal(nfs4_ace->aceFlags, + creator_ace_flags[i].nfs4_flags); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + } + } + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -880,6 +987,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_dacl_permissions_to_nfs4), cmocka_unit_test(test_special_nfs4_to_dacl), cmocka_unit_test(test_dacl_to_special_nfs4), + cmocka_unit_test(test_dacl_creator_to_nfs4), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From ee484d504740ec1ed08724b2ce38c0c9091f965d Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:57:45 -0700 Subject: [PATCH 14/40] test_nfs4_acls: Add test for mapping from NFS4 to DACL CREATOR entries Add testcase for mapping from NFSv4 ACL entries for "special owner" and "special group" to DACL entries in the security descriptor. Each NFSv4 entry here with INHERIT_ONLY maps directly to a CREATOR OWNER or CREATOR GROUP entry in the DACL. Entries without INHERIT_ONLY map to the CREATOR entry and an additional explicit entry granting permission on the current object. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 3c9cda0f6d80258ef0c2a80d6e24dfb650fea1b1) --- source3/modules/test_nfs4_acls.c | 122 +++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index dcdcb89823f..e4e5f1f8b6e 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -973,6 +973,127 @@ static void test_dacl_creator_to_nfs4(void **state) TALLOC_FREE(frame); } +struct creator_owner_nfs4_to_dacl { + uint32_t special_id; + uint32_t nfs4_ace_flags; + uint32_t dacl_ace_flags; +} creator_owner_nfs4_to_dacl[] = { + { SMB_ACE4_WHO_OWNER, + SMB_ACE4_FILE_INHERIT_ACE, + SEC_ACE_FLAG_OBJECT_INHERIT|SEC_ACE_FLAG_INHERIT_ONLY }, + { SMB_ACE4_WHO_OWNER, + SMB_ACE4_DIRECTORY_INHERIT_ACE, + SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_INHERIT_ONLY }, + { SMB_ACE4_WHO_OWNER, + SMB_ACE4_FILE_INHERIT_ACE|SMB_ACE4_DIRECTORY_INHERIT_ACE, + SEC_ACE_FLAG_OBJECT_INHERIT|SEC_ACE_FLAG_CONTAINER_INHERIT| + SEC_ACE_FLAG_INHERIT_ONLY }, + { SMB_ACE4_WHO_GROUP, + SMB_ACE4_FILE_INHERIT_ACE, + SEC_ACE_FLAG_OBJECT_INHERIT|SEC_ACE_FLAG_INHERIT_ONLY }, + { SMB_ACE4_WHO_GROUP, + SMB_ACE4_DIRECTORY_INHERIT_ACE, + SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_INHERIT_ONLY }, + { SMB_ACE4_WHO_GROUP, + SMB_ACE4_FILE_INHERIT_ACE|SMB_ACE4_DIRECTORY_INHERIT_ACE, + SEC_ACE_FLAG_OBJECT_INHERIT|SEC_ACE_FLAG_CONTAINER_INHERIT| + SEC_ACE_FLAG_INHERIT_ONLY }, +}; + +static void test_nfs4_to_dacl_creator(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + int i; + + for (i = 0; i < ARRAY_SIZE(creator_owner_nfs4_to_dacl); i++) { + struct SMB4ACL_T *nfs4_acl; + SMB_ACE4PROP_T nfs4_ace; + struct security_ace *dacl_aces, *creator_dacl_ace; + int good_aces; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + + nfs4_acl = smb_create_smb4acl(frame); + assert_non_null(nfs4_acl); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id + = creator_owner_nfs4_to_dacl[i].special_id, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags + = creator_owner_nfs4_to_dacl[i].nfs4_ace_flags, + .aceMask = SMB_ACE4_READ_DATA, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + assert_true(smbacl4_nfs42win(frame, ¶ms, nfs4_acl, + &sids[0], &sids[1], true, + &dacl_aces, &good_aces)); + assert_non_null(dacl_aces); + + if (creator_owner_nfs4_to_dacl[i].nfs4_ace_flags & + SMB_ACE4_INHERIT_ONLY_ACE) { + /* + * Only one ACE entry for the CREATOR ACE entry. + */ + assert_int_equal(good_aces, 1); + creator_dacl_ace = &dacl_aces[0]; + } else { + /* + * This creates an additional ACE entry for + * the permissions on the current object. + */ + assert_int_equal(good_aces, 2); + + assert_int_equal(dacl_aces[0].type, + SEC_ACE_TYPE_ACCESS_ALLOWED); + assert_int_equal(dacl_aces[0].flags, 0); + assert_int_equal(dacl_aces[0].access_mask, + SEC_FILE_READ_DATA); + + if (creator_owner_nfs4_to_dacl[i].special_id == + SMB_ACE4_WHO_OWNER) { + assert_true(dom_sid_equal(&dacl_aces[0].trustee, + &sids[0])); + } + + if (creator_owner_nfs4_to_dacl[i].special_id == + SMB_ACE4_WHO_GROUP) { + assert_true(dom_sid_equal(&dacl_aces[0].trustee, + &sids[1])); + } + + creator_dacl_ace = &dacl_aces[1]; + } + + assert_int_equal(creator_dacl_ace->type, + SEC_ACE_TYPE_ACCESS_ALLOWED); + assert_int_equal(creator_dacl_ace->flags, + creator_owner_nfs4_to_dacl[i].dacl_ace_flags); + assert_int_equal(creator_dacl_ace->access_mask, + SEC_FILE_READ_DATA); + if (creator_owner_nfs4_to_dacl[i].special_id == + SMB_ACE4_WHO_OWNER) { + assert_true(dom_sid_equal(&creator_dacl_ace->trustee, + &global_sid_Creator_Owner)); + } + + if (creator_owner_nfs4_to_dacl[i].special_id == + SMB_ACE4_WHO_GROUP) { + assert_true(dom_sid_equal(&creator_dacl_ace->trustee, + &global_sid_Creator_Group)); + } + } + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -988,6 +1109,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_special_nfs4_to_dacl), cmocka_unit_test(test_dacl_to_special_nfs4), cmocka_unit_test(test_dacl_creator_to_nfs4), + cmocka_unit_test(test_nfs4_to_dacl_creator), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From 6904d7f5421b0df3e2f15cc8f0747cb8dee3805e Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 12:02:58 -0700 Subject: [PATCH 15/40] test_nfs4_acls: Add test for 'map full control' option "map full control" when enabled adds the DELETE_CHILD permission, when all other permissions are present. This allows Windows clients to display the "FULL CONTROL" permissions. Add a testcase that verifies this mapping when mapping from NFSv4 ACL to the DACL in the security descriptor. Also verify that switching the option off disables this behavior. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 30677df4dac4ebfcf4e3198db33f14be37948197) --- source3/modules/test_nfs4_acls.c | 82 ++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index e4e5f1f8b6e..733217b1f2e 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -1094,6 +1094,87 @@ static void test_nfs4_to_dacl_creator(void **state) TALLOC_FREE(frame); } +struct nfs4_to_dacl_map_full_control{ + bool is_dir; + bool config; + bool delete_child_added; +} nfs4_to_dacl_full_control[] = { + { true, true, false }, + { true, false, false }, + { false, true, true }, + { false, false, false }, +}; + +static void test_full_control_nfs4_to_dacl(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + int i; + + for (i = 0; i < ARRAY_SIZE(nfs4_to_dacl_full_control); i++) { + struct SMB4ACL_T *nfs4_acl; + SMB_ACE4PROP_T nfs4_ace; + struct security_ace *dacl_aces; + int good_aces; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = nfs4_to_dacl_full_control[i].config, + }; + const uint32_t nfs4_ace_mask_except_deletes = + SMB_ACE4_READ_DATA|SMB_ACE4_WRITE_DATA| + SMB_ACE4_APPEND_DATA|SMB_ACE4_READ_NAMED_ATTRS| + SMB_ACE4_WRITE_NAMED_ATTRS|SMB_ACE4_EXECUTE| + SMB_ACE4_READ_ATTRIBUTES|SMB_ACE4_WRITE_ATTRIBUTES| + SMB_ACE4_READ_ACL|SMB_ACE4_WRITE_ACL| + SMB_ACE4_WRITE_OWNER|SMB_ACE4_SYNCHRONIZE; + const uint32_t dacl_ace_mask_except_deletes = + SEC_FILE_READ_DATA|SEC_FILE_WRITE_DATA| + SEC_FILE_APPEND_DATA|SEC_FILE_READ_EA| + SEC_FILE_WRITE_EA|SEC_FILE_EXECUTE| + SEC_FILE_READ_ATTRIBUTE|SEC_FILE_WRITE_ATTRIBUTE| + SEC_STD_READ_CONTROL|SEC_STD_WRITE_DAC| + SEC_STD_WRITE_OWNER|SEC_STD_SYNCHRONIZE; + + nfs4_acl = smb_create_smb4acl(frame); + assert_non_null(nfs4_acl); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = 0, + .who.uid = 1000, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = 0, + .aceMask = nfs4_ace_mask_except_deletes, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + assert_true( + smbacl4_nfs42win(frame, ¶ms, nfs4_acl, + &sids[0], &sids[1], + nfs4_to_dacl_full_control[i].is_dir, + &dacl_aces, &good_aces)); + + assert_int_equal(good_aces, 1); + assert_non_null(dacl_aces); + + assert_int_equal(dacl_aces[0].type, + SEC_ACE_TYPE_ACCESS_ALLOWED); + assert_int_equal(dacl_aces[0].flags, 0); + assert_true(dom_sid_equal(&dacl_aces[0].trustee, &sids[0])); + if (nfs4_to_dacl_full_control[i].delete_child_added) { + assert_int_equal(dacl_aces[0].access_mask, + dacl_ace_mask_except_deletes| + SEC_DIR_DELETE_CHILD); + } else { + assert_int_equal(dacl_aces[0].access_mask, + dacl_ace_mask_except_deletes); + } + } + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -1110,6 +1191,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_dacl_to_special_nfs4), cmocka_unit_test(test_dacl_creator_to_nfs4), cmocka_unit_test(test_nfs4_to_dacl_creator), + cmocka_unit_test(test_full_control_nfs4_to_dacl), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From 8ff74acedd6d0aa7f3e6e60c12f536701425983d Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 12:07:36 -0700 Subject: [PATCH 16/40] test_nfs4_acls: Add test for acedup settings The NFSv4 ACL mapping code has a setting nfs4:acedup. Depending on the setting, when mapping from DACLs to NFSv4 ACLs, duplicate ACL entries are either merged, ignored or rejected. Add a testcase that has duplicate ACL entries and verify the expected behavior for all possible settings of the nfs4:acedup option. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 9671bf2b9f055012057620207624aa2f4ea6833e) --- source3/modules/test_nfs4_acls.c | 124 +++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index 733217b1f2e..c4f3d8052e4 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -1175,6 +1175,129 @@ static void test_full_control_nfs4_to_dacl(void **state) TALLOC_FREE(frame); } +struct acedup_settings { + enum smbacl4_acedup_enum setting; +} acedup_settings[] = { + { e_dontcare }, + { e_reject }, + { e_ignore }, + { e_merge }, +}; + +static void test_dacl_to_nfs4_acedup_settings(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + int i; + + for (i = 0; i < ARRAY_SIZE(acedup_settings); i++) { + struct SMB4ACL_T *nfs4_acl; + struct SMB4ACE_T *nfs4_ace_container; + SMB_ACE4PROP_T *nfs4_ace; + struct security_ace dacl_aces[2]; + struct security_acl *dacl; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = acedup_settings[i].setting, + .map_full_control = true, + }; + + init_sec_ace(&dacl_aces[0], &sids[0], + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT); + init_sec_ace(&dacl_aces[1], &sids[0], + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_WRITE_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT); + dacl = make_sec_acl(frame, SECURITY_ACL_REVISION_ADS, + ARRAY_SIZE(dacl_aces), dacl_aces); + assert_non_null(dacl); + + nfs4_acl = smbacl4_win2nfs4(frame, true, dacl, ¶ms, + 101, 102); + + switch(params.acedup) { + case e_dontcare: + assert_non_null(nfs4_acl); + assert_int_equal(smbacl4_get_controlflags(nfs4_acl), + SEC_DESC_SELF_RELATIVE); + assert_int_equal(smb_get_naces(nfs4_acl), 2); + + nfs4_ace_container = smb_first_ace4(nfs4_acl); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->who.uid, 1000); + assert_int_equal(nfs4_ace->aceFlags, + SMB_ACE4_FILE_INHERIT_ACE); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + assert_null(smb_next_ace4(nfs4_ace_container)); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->who.uid, 1000); + assert_int_equal(nfs4_ace->aceFlags, + SMB_ACE4_FILE_INHERIT_ACE); + assert_int_equal(nfs4_ace->aceMask, + SMB_ACE4_WRITE_DATA); + break; + + case e_reject: + assert_null(nfs4_acl); + assert_int_equal(errno, EINVAL); + break; + + case e_ignore: + assert_non_null(nfs4_acl); + assert_int_equal(smbacl4_get_controlflags(nfs4_acl), + SEC_DESC_SELF_RELATIVE); + assert_int_equal(smb_get_naces(nfs4_acl), 1); + + nfs4_ace_container = smb_first_ace4(nfs4_acl); + assert_non_null(nfs4_ace_container); + assert_null(smb_next_ace4(nfs4_ace_container)); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->who.uid, 1000); + assert_int_equal(nfs4_ace->aceFlags, + SMB_ACE4_FILE_INHERIT_ACE); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + break; + + case e_merge: + assert_non_null(nfs4_acl); + assert_int_equal(smbacl4_get_controlflags(nfs4_acl), + SEC_DESC_SELF_RELATIVE); + assert_int_equal(smb_get_naces(nfs4_acl), 1); + + nfs4_ace_container = smb_first_ace4(nfs4_acl); + assert_non_null(nfs4_ace_container); + assert_null(smb_next_ace4(nfs4_ace_container)); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->who.uid, 1000); + assert_int_equal(nfs4_ace->aceFlags, + SMB_ACE4_FILE_INHERIT_ACE); + assert_int_equal(nfs4_ace->aceMask, + SMB_ACE4_READ_DATA| + SMB_ACE4_WRITE_DATA); + break; + + default: + fail_msg("Unexpected value for acedup: %d\n", + params.acedup); + }; + } + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -1192,6 +1315,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_dacl_creator_to_nfs4), cmocka_unit_test(test_nfs4_to_dacl_creator), cmocka_unit_test(test_full_control_nfs4_to_dacl), + cmocka_unit_test(test_dacl_to_nfs4_acedup_settings), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From a8701c2c3418654b9a9d2557abeba91b4d71185f Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 12:09:04 -0700 Subject: [PATCH 17/40] test_nfs4_acls: Add test for matching DACL entries for acedup The NFSv4 mapping code has a config option nfs4:acedup for the mapping path from DACLs to NFSv4 ACLs. Part of this codepath is detecting duplicate ACL entries. Add a testcase with different ACL entries and verify that only exactly matching entries are detected as duplicates and treated accordingly. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit f55cdf42a14f314102f2e13cb06d4db48c08ad4b) --- source3/modules/test_nfs4_acls.c | 122 +++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index c4f3d8052e4..80078311ce8 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -1298,6 +1298,127 @@ static void test_dacl_to_nfs4_acedup_settings(void **state) TALLOC_FREE(frame); } +struct acedup_match { + int sid_idx1; + enum security_ace_type type1; + uint32_t ace_mask1; + uint8_t flag1; + int sid_idx2; + enum security_ace_type type2; + uint32_t ace_mask2; + uint8_t flag2; + bool match; +} acedup_match[] = { + { 0, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT, + 0, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT, + true }, + { 0, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT, + 1, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT, + false }, + { 0, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT, + 0, SEC_ACE_TYPE_ACCESS_DENIED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT, + false }, + { 0, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT, + 0, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_WRITE_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT, + true }, + { 0, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT, + 0, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_CONTAINER_INHERIT, + false }, + { 0, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT, + 5, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT, + false }, +}; + +static void test_dacl_to_nfs4_acedup_match(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + int i; + + for (i = 0; i < ARRAY_SIZE(acedup_match); i++) { + struct SMB4ACL_T *nfs4_acl; + struct SMB4ACE_T *nfs4_ace_container; + SMB_ACE4PROP_T *nfs4_ace; + struct security_ace dacl_aces[2]; + struct security_acl *dacl; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_ignore, + .map_full_control = true, + }; + + init_sec_ace(&dacl_aces[0], + &sids[acedup_match[i].sid_idx1], + acedup_match[i].type1, + acedup_match[i].ace_mask1, + acedup_match[i].flag1); + init_sec_ace(&dacl_aces[1], + &sids[acedup_match[i].sid_idx2], + acedup_match[i].type2, + acedup_match[i].ace_mask2, + acedup_match[i].flag2); + dacl = make_sec_acl(frame, SECURITY_ACL_REVISION_ADS, + ARRAY_SIZE(dacl_aces), dacl_aces); + assert_non_null(dacl); + + nfs4_acl = smbacl4_win2nfs4(frame, true, dacl, ¶ms, + 101, 102); + assert_non_null(nfs4_acl); + assert_int_equal(smbacl4_get_controlflags(nfs4_acl), + SEC_DESC_SELF_RELATIVE); + + if (acedup_match[i].match) { + assert_int_equal(smb_get_naces(nfs4_acl), 1); + + nfs4_ace_container = smb_first_ace4(nfs4_acl); + assert_non_null(nfs4_ace_container); + assert_null(smb_next_ace4(nfs4_ace_container)); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->who.uid, 1000); + assert_int_equal(nfs4_ace->aceFlags, + SMB_ACE4_FILE_INHERIT_ACE); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + } else { + assert_int_equal(smb_get_naces(nfs4_acl), 2); + + nfs4_ace_container = smb_first_ace4(nfs4_acl); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->who.uid, 1000); + assert_int_equal(nfs4_ace->aceFlags, + SMB_ACE4_FILE_INHERIT_ACE); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + assert_null(smb_next_ace4(nfs4_ace_container)); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + } + } + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -1316,6 +1437,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_nfs4_to_dacl_creator), cmocka_unit_test(test_full_control_nfs4_to_dacl), cmocka_unit_test(test_dacl_to_nfs4_acedup_settings), + cmocka_unit_test(test_dacl_to_nfs4_acedup_match), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From 0c7023e5cb00ac615dc574f563a6f2914d178514 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 12:16:08 -0700 Subject: [PATCH 18/40] test_nfs4_acls: Add test for mapping from DACL to NFS4 ACL with config special The mapping code between NFSv4 ACLs and security descriptors still has the deprecated config setting "nfs4:mode = special". This should not be used as it has security problems: All entries matching owner or group are mapped to "special owner" or "special group", which can change its meaning when being inherited to a new file or directory with different owner and owning group. This mode should eventually be removed, but as long as it still exists add testcases to verify the expected behavior. This patch adds the testcase for "nfs4:mode = special" when mapping from the DACL in the security descriptor to the NFSv4 ACL. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 7ae06d96eb59722154d30e21949f9dba4f2f0bc6) --- source3/modules/test_nfs4_acls.c | 119 +++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index 80078311ce8..eda2fe56d32 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -1419,6 +1419,124 @@ static void test_dacl_to_nfs4_acedup_match(void **state) TALLOC_FREE(frame); } +static void test_dacl_to_nfs4_config_special(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + struct SMB4ACL_T *nfs4_acl; + struct SMB4ACE_T *nfs4_ace_container; + SMB_ACE4PROP_T *nfs4_ace; + struct security_ace dacl_aces[6]; + struct security_acl *dacl; + struct smbacl4_vfs_params params = { + .mode = e_special, + .do_chown = true, + .acedup = e_dontcare, + .map_full_control = true, + }; + + /* + * global_sid_Creator_Owner or global_sid_Special_Group is NOT mapped + * to SMB_ACE4_ID_SPECIAL. + */ + init_sec_ace(&dacl_aces[0], &global_sid_Creator_Owner, + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT); + init_sec_ace(&dacl_aces[1], &global_sid_Creator_Group, + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_WRITE_DATA, + SEC_ACE_FLAG_CONTAINER_INHERIT); + /* + * Anything that maps to owner or owning group with inheritance flags + * IS mapped to special owner or special group. + */ + init_sec_ace(&dacl_aces[2], &sids[0], + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT); + init_sec_ace(&dacl_aces[3], &sids[0], + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_INHERIT_ONLY); + init_sec_ace(&dacl_aces[4], &sids[1], + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_OBJECT_INHERIT); + init_sec_ace(&dacl_aces[5], &sids[1], + SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_FILE_READ_DATA, + SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_INHERIT_ONLY); + dacl = make_sec_acl(frame, SECURITY_ACL_REVISION_ADS, + ARRAY_SIZE(dacl_aces), dacl_aces); + assert_non_null(dacl); + + nfs4_acl = smbacl4_win2nfs4(frame, true, dacl, ¶ms, 1000, 1001); + + assert_non_null(nfs4_acl); + assert_int_equal(smbacl4_get_controlflags(nfs4_acl), + SEC_DESC_SELF_RELATIVE); + assert_int_equal(smb_get_naces(nfs4_acl), 6); + + nfs4_ace_container = smb_first_ace4(nfs4_acl); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->aceFlags, SMB_ACE4_FILE_INHERIT_ACE); + assert_int_equal(nfs4_ace->who.uid, 1003); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, 0); + assert_int_equal(nfs4_ace->aceFlags, + SMB_ACE4_IDENTIFIER_GROUP| + SMB_ACE4_DIRECTORY_INHERIT_ACE); + assert_int_equal(nfs4_ace->who.gid, 1004); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_WRITE_DATA); + + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, SMB_ACE4_ID_SPECIAL); + assert_int_equal(nfs4_ace->who.special_id, SMB_ACE4_WHO_OWNER); + assert_int_equal(nfs4_ace->aceFlags, SMB_ACE4_FILE_INHERIT_ACE); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, SMB_ACE4_ID_SPECIAL); + assert_int_equal(nfs4_ace->aceFlags, SMB_ACE4_DIRECTORY_INHERIT_ACE| + SMB_ACE4_INHERIT_ONLY_ACE); + assert_int_equal(nfs4_ace->who.special_id, SMB_ACE4_WHO_OWNER); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, SMB_ACE4_ID_SPECIAL); + assert_int_equal(nfs4_ace->aceFlags, SMB_ACE4_IDENTIFIER_GROUP| + SMB_ACE4_FILE_INHERIT_ACE); + assert_int_equal(nfs4_ace->who.special_id, SMB_ACE4_WHO_GROUP); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, SMB_ACE4_ID_SPECIAL); + assert_int_equal(nfs4_ace->aceFlags, SMB_ACE4_IDENTIFIER_GROUP| + SMB_ACE4_DIRECTORY_INHERIT_ACE| + SMB_ACE4_INHERIT_ONLY_ACE); + assert_int_equal(nfs4_ace->who.special_id, SMB_ACE4_WHO_GROUP); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + assert_null(smb_next_ace4(nfs4_ace_container)); + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -1438,6 +1556,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_full_control_nfs4_to_dacl), cmocka_unit_test(test_dacl_to_nfs4_acedup_settings), cmocka_unit_test(test_dacl_to_nfs4_acedup_match), + cmocka_unit_test(test_dacl_to_nfs4_config_special), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From 4471d5783a9532faf52fc09f665561910799d01a Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 12:23:02 -0700 Subject: [PATCH 19/40] test_nfs4_acls: Add test for mapping from NFS4 to DACL in config mode special The mapping code between NFSv4 ACLs and security descriptors still has the deprecated config setting "nfs4:mode = special". This should not be used as it has security problems: All entries matching owner or group are mapped to "special owner" or "special group", which can change its meaning when being inherited to a new file or directory with different owner and owning group. This mode should eventually be removed, but as long as it still exists add testcases to verify the expected behavior. This patch adds the testcase for "nfs4:mode = special" when mapping from the NFS4 ACL to the DACL in the security descriptor. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 829c5ea99685c0629fd67ed0528897534ff35b36) --- source3/modules/test_nfs4_acls.c | 63 ++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index eda2fe56d32..341bf179ea9 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -1537,6 +1537,68 @@ static void test_dacl_to_nfs4_config_special(void **state) TALLOC_FREE(frame); } +static void test_nfs4_to_dacl_config_special(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + struct SMB4ACL_T *nfs4_acl; + SMB_ACE4PROP_T nfs4_ace; + struct security_ace *dacl_aces; + int good_aces; + struct smbacl4_vfs_params params = { + .mode = e_special, + .do_chown = true, + .acedup = e_dontcare, + .map_full_control = true, + }; + + nfs4_acl = smb_create_smb4acl(frame); + assert_non_null(nfs4_acl); + + /* + * In config mode special, this is not mapped to Creator Owner + */ + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id = SMB_ACE4_WHO_OWNER, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = SMB_ACE4_FILE_INHERIT_ACE, + .aceMask = SMB_ACE4_READ_DATA, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + /* + * In config mode special, this is not mapped to Creator Group + */ + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = SMB_ACE4_ID_SPECIAL, + .who.special_id = SMB_ACE4_WHO_GROUP, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = SMB_ACE4_DIRECTORY_INHERIT_ACE, + .aceMask = SMB_ACE4_WRITE_DATA, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + assert_true(smbacl4_nfs42win(frame, ¶ms, nfs4_acl, + &sids[0], &sids[1], true, + &dacl_aces, &good_aces)); + + assert_int_equal(good_aces, 2); + assert_non_null(dacl_aces); + + assert_int_equal(dacl_aces[0].type, SEC_ACE_TYPE_ACCESS_ALLOWED); + assert_int_equal(dacl_aces[0].flags, SEC_ACE_FLAG_OBJECT_INHERIT); + assert_int_equal(dacl_aces[0].access_mask, SEC_FILE_READ_DATA); + assert_true(dom_sid_equal(&dacl_aces[0].trustee, &sids[0])); + + assert_int_equal(dacl_aces[1].type, SEC_ACE_TYPE_ACCESS_ALLOWED); + assert_int_equal(dacl_aces[1].flags, SEC_ACE_FLAG_CONTAINER_INHERIT); + assert_int_equal(dacl_aces[1].access_mask, SEC_FILE_WRITE_DATA); + assert_true(dom_sid_equal(&dacl_aces[1].trustee, &sids[1])); + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -1557,6 +1619,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_dacl_to_nfs4_acedup_settings), cmocka_unit_test(test_dacl_to_nfs4_acedup_match), cmocka_unit_test(test_dacl_to_nfs4_config_special), + cmocka_unit_test(test_nfs4_to_dacl_config_special), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From c6877f99e49303e0338a332d2c7d19d0915ab5fb Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 12:50:42 -0700 Subject: [PATCH 20/40] test_nfs4_acls: Add test for mapping from NFS4 ACL to DACL with IDMAP_TYPE_BOTH When id mappings use IDMAP_TYPE_BOTH, the NFSv4 ACL mapping code is not aware whether a particular entry is for a user or a group. The underlying assumption then is that is should not matter, as both the ACL mapping maps everything to NFSv4 ACL group entries and the user's token will contain gid entries for the groups. Add a testcase to verify that when mapping from NFSv4 ACL entries to DACLs with IDMAP_TYPE_BOTH, all entries are mapped as expected. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 86480410aec1d2331c65826a13f909492165a291) --- source3/modules/test_nfs4_acls.c | 67 ++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index 341bf179ea9..964af4ff057 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -1599,6 +1599,72 @@ static void test_nfs4_to_dacl_config_special(void **state) TALLOC_FREE(frame); } +struct nfs_to_dacl_idmap_both { + uint32_t nfs4_flags; + uint32_t nfs4_id; + struct dom_sid *sid; +}; + +static void test_nfs4_to_dacl_idmap_type_both(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + int i; + struct nfs_to_dacl_idmap_both nfs_to_dacl_idmap_both[] = { + { 0, 1002, &sids[2] }, + { SMB_ACE4_IDENTIFIER_GROUP, 1002, &sids[2] }, + { 0, 1005, &sids[6] }, + { SMB_ACE4_IDENTIFIER_GROUP, 1005, &sids[6] }, + }; + + for (i = 0; i < ARRAY_SIZE(nfs_to_dacl_idmap_both); i++) { + struct SMB4ACL_T *nfs4_acl; + struct security_ace *dacl_aces; + SMB_ACE4PROP_T nfs4_ace; + int good_aces; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + + nfs4_acl = smb_create_smb4acl(frame); + assert_non_null(nfs4_acl); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = 0, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = nfs_to_dacl_idmap_both[i].nfs4_flags, + .aceMask = SMB_ACE4_READ_DATA, + }; + + if (nfs_to_dacl_idmap_both[i].nfs4_flags & + SMB_ACE4_IDENTIFIER_GROUP) { + nfs4_ace.who.gid = nfs_to_dacl_idmap_both[i].nfs4_id; + } else { + nfs4_ace.who.uid = nfs_to_dacl_idmap_both[i].nfs4_id; + } + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + assert_true(smbacl4_nfs42win(frame, ¶ms, nfs4_acl, + &sids[2], &sids[2], + false, &dacl_aces, &good_aces)); + + assert_int_equal(good_aces, 1); + assert_non_null(dacl_aces); + + assert_int_equal(dacl_aces[0].type, + SEC_ACE_TYPE_ACCESS_ALLOWED); + assert_int_equal(dacl_aces[0].flags, 0); + assert_int_equal(dacl_aces[0].access_mask, SEC_FILE_READ_DATA); + assert_true(dom_sid_equal(&dacl_aces[0].trustee, + nfs_to_dacl_idmap_both[i].sid)); + } + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -1620,6 +1686,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_dacl_to_nfs4_acedup_match), cmocka_unit_test(test_dacl_to_nfs4_config_special), cmocka_unit_test(test_nfs4_to_dacl_config_special), + cmocka_unit_test(test_nfs4_to_dacl_idmap_type_both), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From 511cbd2a6aa13a30b557f9b80183b6c1fe5d8856 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 13:04:44 -0700 Subject: [PATCH 21/40] test_nfs4_acls: Add test for mapping from DACL to NFS4 ACL with IDMAP_TYPE_BOTH When id mappings use IDMAP_TYPE_BOTH, the NFSv4 ACL mapping code is not aware whether a particular entry is for a user or a group. The underlying assumption then is that is should not matter, as both the ACL mapping maps everything to NFSv4 ACL group entries and the user's token will contain gid entries for the groups. Add a testcase to verify that when mapping from DACLS to NFSv4 ACL entries with IDMAP_TYPE_BOTH, all entries are mapped as expected. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 38331b00521ef764893a74add01758f14567d901) --- source3/modules/test_nfs4_acls.c | 85 ++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index 964af4ff057..d7152a0737a 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -1665,6 +1665,90 @@ static void test_nfs4_to_dacl_idmap_type_both(void **state) TALLOC_FREE(frame); } +struct dacl_to_nfs4_idmap_both { + struct dom_sid *sid; + uint32_t dacl_flags; + uint32_t nfs4_flags; + uint32_t nfs4_ace_flags; + uint32_t nfs4_id; +}; + +/* + * IDMAP_TYPE_BOTH always creates group entries. + */ +static void test_dacl_to_nfs4_idmap_type_both(void **state) +{ + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + int i; + + struct dacl_to_nfs4_idmap_both dacl_to_nfs4_idmap_both[] = { + { &sids[2], 0, + SMB_ACE4_ID_SPECIAL, SMB_ACE4_IDENTIFIER_GROUP, SMB_ACE4_WHO_GROUP }, + { &sids[2], SEC_ACE_FLAG_OBJECT_INHERIT, + 0, SMB_ACE4_IDENTIFIER_GROUP|SMB_ACE4_FILE_INHERIT_ACE, 1002 }, + { &sids[6], 0, + 0, SMB_ACE4_IDENTIFIER_GROUP, 1005 }, + { &sids[6], SEC_ACE_FLAG_OBJECT_INHERIT, + 0, SMB_ACE4_IDENTIFIER_GROUP|SMB_ACE4_FILE_INHERIT_ACE, 1005 }, + }; + + for (i = 0; i < ARRAY_SIZE(dacl_to_nfs4_idmap_both); i++) { + struct SMB4ACL_T *nfs4_acl; + struct SMB4ACE_T *nfs4_ace_container; + SMB_ACE4PROP_T *nfs4_ace; + struct security_ace dacl_aces[1]; + struct security_acl *dacl; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_merge, + .map_full_control = true, + }; + + init_sec_ace(&dacl_aces[0], dacl_to_nfs4_idmap_both[i].sid, + SEC_ACE_TYPE_ACCESS_ALLOWED, + SEC_FILE_READ_DATA, + dacl_to_nfs4_idmap_both[i].dacl_flags); + dacl = make_sec_acl(frame, SECURITY_ACL_REVISION_ADS, + ARRAY_SIZE(dacl_aces), dacl_aces); + assert_non_null(dacl); + + nfs4_acl = smbacl4_win2nfs4(frame, true, dacl, ¶ms, + 1002, 1002); + + assert_non_null(nfs4_acl); + assert_int_equal(smbacl4_get_controlflags(nfs4_acl), + SEC_DESC_SELF_RELATIVE); + assert_int_equal(smb_get_naces(nfs4_acl), 1); + + nfs4_ace_container = smb_first_ace4(nfs4_acl); + assert_non_null(nfs4_ace_container); + assert_null(smb_next_ace4(nfs4_ace_container)); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, + dacl_to_nfs4_idmap_both[i].nfs4_flags); + assert_int_equal(nfs4_ace->aceFlags, + dacl_to_nfs4_idmap_both[i].nfs4_ace_flags); + if (nfs4_ace->flags & SMB_ACE4_ID_SPECIAL) { + assert_int_equal(nfs4_ace->who.special_id, + dacl_to_nfs4_idmap_both[i].nfs4_id); + } else if (nfs4_ace->aceFlags & SMB_ACE4_IDENTIFIER_GROUP) { + assert_int_equal(nfs4_ace->who.gid, + dacl_to_nfs4_idmap_both[i].nfs4_id); + } else { + assert_int_equal(nfs4_ace->who.uid, + dacl_to_nfs4_idmap_both[i].nfs4_id); + } + assert_int_equal(nfs4_ace->aceType, + SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + } + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -1687,6 +1771,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_dacl_to_nfs4_config_special), cmocka_unit_test(test_nfs4_to_dacl_config_special), cmocka_unit_test(test_nfs4_to_dacl_idmap_type_both), + cmocka_unit_test(test_dacl_to_nfs4_idmap_type_both), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From d21769f437c92d3ca97aee244d3ca38e8130f4ef Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 26 Jun 2019 13:24:16 -0700 Subject: [PATCH 22/40] nfs4_acls: Use sids_to_unixids to lookup uid or gid This is the newer API to lookup id mappings and will make it easier to add to the IDMAP_TYPE_BOTH case. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit d9a2ff559e1ad953141b1118a9e370496f1f61fa) --- source3/modules/nfs4_acls.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index 5543b3a7f58..4069c9310ed 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -21,6 +21,7 @@ #include "smbd/smbd.h" #include "nfs4_acls.h" #include "librpc/gen_ndr/ndr_security.h" +#include "librpc/gen_ndr/idmap.h" #include "../libcli/security/dom_sid.h" #include "../libcli/security/security.h" #include "dbwrap/dbwrap.h" @@ -719,14 +720,21 @@ static bool smbacl4_fill_ace4( return false; } } else { - uid_t uid; - gid_t gid; + struct unixid unixid; + bool ok; - if (sid_to_gid(&ace_nt->trustee, &gid)) { + ok = sids_to_unixids(&ace_nt->trustee, 1, &unixid); + if (!ok) { + DBG_WARNING("Could not convert %s to uid or gid.\n", + dom_sid_str_buf(&ace_nt->trustee, &buf)); + return false; + } + + if (unixid.type == ID_TYPE_GID || unixid.type == ID_TYPE_BOTH) { ace_v4->aceFlags |= SMB_ACE4_IDENTIFIER_GROUP; - ace_v4->who.gid = gid; - } else if (sid_to_uid(&ace_nt->trustee, &uid)) { - ace_v4->who.uid = uid; + ace_v4->who.gid = unixid.id; + } else if (unixid.type == ID_TYPE_UID) { + ace_v4->who.uid = unixid.id; } else if (dom_sid_compare_domain(&ace_nt->trustee, &global_sid_Unix_NFS) == 0) { return false; -- 2.17.0 From 5a5e5a2bb080fe9b4e4884c66dc344e0b3de77c5 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Mon, 15 Jul 2019 13:15:32 -0700 Subject: [PATCH 23/40] nfs4_acls: Use switch/case for checking idmap type BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit f198a0867e71f248d4887ab0b6f2832123b16d11) --- source3/modules/nfs4_acls.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index 4069c9310ed..f8861e9058b 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -730,18 +730,27 @@ static bool smbacl4_fill_ace4( return false; } - if (unixid.type == ID_TYPE_GID || unixid.type == ID_TYPE_BOTH) { + if (dom_sid_compare_domain(&ace_nt->trustee, + &global_sid_Unix_NFS) == 0) { + return false; + } + + switch (unixid.type) { + case ID_TYPE_BOTH: ace_v4->aceFlags |= SMB_ACE4_IDENTIFIER_GROUP; ace_v4->who.gid = unixid.id; - } else if (unixid.type == ID_TYPE_UID) { + break; + case ID_TYPE_GID: + ace_v4->aceFlags |= SMB_ACE4_IDENTIFIER_GROUP; + ace_v4->who.gid = unixid.id; + break; + case ID_TYPE_UID: ace_v4->who.uid = unixid.id; - } else if (dom_sid_compare_domain(&ace_nt->trustee, - &global_sid_Unix_NFS) == 0) { - return false; - } else { - DEBUG(1, ("nfs4_acls.c: could not " - "convert %s to uid or gid\n", - dom_sid_str_buf(&ace_nt->trustee, &buf))); + break; + case ID_TYPE_NOT_SPECIFIED: + default: + DBG_WARNING("Could not convert %s to uid or gid.\n", + dom_sid_str_buf(&ace_nt->trustee, &buf)); return false; } } -- 2.17.0 From 57276ca6390b3253045990c43a465d9e404079a2 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 25 Jun 2019 15:21:06 -0700 Subject: [PATCH 24/40] nfs4_acls: Use correct type when checking ownerGID uid and gid are members of the same union so this makes no difference, but for type correctness and readability use the gid to check for ownerGID. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 3b3d722ce579c19c7b08d06a3adea275537545dc) --- source3/modules/nfs4_acls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index f8861e9058b..b2ba4d1d701 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -856,7 +856,7 @@ static int smbacl4_substitute_simple( if (!(ace->flags & SMB_ACE4_ID_SPECIAL) && ace->aceFlags & SMB_ACE4_IDENTIFIER_GROUP && - ace->who.uid == ownerGID && + ace->who.gid == ownerGID && !(ace->aceFlags & SMB_ACE4_INHERIT_ONLY_ACE) && !(ace->aceFlags & SMB_ACE4_FILE_INHERIT_ACE) && !(ace->aceFlags & SMB_ACE4_DIRECTORY_INHERIT_ACE)) { -- 2.17.0 From ab4e90ac14f21e7744949681b58e8cede33a7bac Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 26 Jun 2019 13:20:17 -0700 Subject: [PATCH 25/40] nfs4_acls: Add helper function for checking INHERIT flags. This avoids some code duplication. Do not make this static, as it will be used in a later patch. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmit Reviewed-by: Ralph Boehme (cherry picked from commit 336e8668c1cc3682cb3c198eb6dc49baf522a79a) --- source3/modules/nfs4_acls.c | 15 +++++++++------ source3/modules/nfs4_acls.h | 2 ++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index b2ba4d1d701..bab73a5cb58 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -255,6 +255,13 @@ bool smbacl4_set_controlflags(struct SMB4ACL_T *acl, uint16_t controlflags) return true; } +bool nfs_ace_is_inherit(SMB_ACE4PROP_T *ace) +{ + return ace->aceFlags & (SMB_ACE4_INHERIT_ONLY_ACE| + SMB_ACE4_FILE_INHERIT_ACE| + SMB_ACE4_DIRECTORY_INHERIT_ACE); +} + static int smbacl4_GetFileOwner(struct connection_struct *conn, const struct smb_filename *smb_fname, SMB_STRUCT_STAT *psbuf) @@ -846,9 +853,7 @@ static int smbacl4_substitute_simple( if (!(ace->flags & SMB_ACE4_ID_SPECIAL) && !(ace->aceFlags & SMB_ACE4_IDENTIFIER_GROUP) && ace->who.uid == ownerUID && - !(ace->aceFlags & SMB_ACE4_INHERIT_ONLY_ACE) && - !(ace->aceFlags & SMB_ACE4_FILE_INHERIT_ACE) && - !(ace->aceFlags & SMB_ACE4_DIRECTORY_INHERIT_ACE)) { + !nfs_ace_is_inherit(ace)) { ace->flags |= SMB_ACE4_ID_SPECIAL; ace->who.special_id = SMB_ACE4_WHO_OWNER; DEBUG(10,("replaced with special owner ace\n")); @@ -857,9 +862,7 @@ static int smbacl4_substitute_simple( if (!(ace->flags & SMB_ACE4_ID_SPECIAL) && ace->aceFlags & SMB_ACE4_IDENTIFIER_GROUP && ace->who.gid == ownerGID && - !(ace->aceFlags & SMB_ACE4_INHERIT_ONLY_ACE) && - !(ace->aceFlags & SMB_ACE4_FILE_INHERIT_ACE) && - !(ace->aceFlags & SMB_ACE4_DIRECTORY_INHERIT_ACE)) { + !nfs_ace_is_inherit(ace)) { ace->flags |= SMB_ACE4_ID_SPECIAL; ace->who.special_id = SMB_ACE4_WHO_GROUP; DEBUG(10,("replaced with special group ace\n")); diff --git a/source3/modules/nfs4_acls.h b/source3/modules/nfs4_acls.h index a73b3154f0f..d0cf2d0f1fb 100644 --- a/source3/modules/nfs4_acls.h +++ b/source3/modules/nfs4_acls.h @@ -143,6 +143,8 @@ uint16_t smbacl4_get_controlflags(struct SMB4ACL_T *theacl); bool smbacl4_set_controlflags(struct SMB4ACL_T *theacl, uint16_t controlflags); +bool nfs_ace_is_inherit(SMB_ACE4PROP_T *ace); + NTSTATUS smb_fget_nt_acl_nfs4(files_struct *fsp, const struct smbacl4_vfs_params *pparams, uint32_t security_info, -- 2.17.0 From 99a1f0577101e618366bb79516d6e26d3e5fdaa9 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 13:20:44 -0700 Subject: [PATCH 26/40] nfs4_acls: Add missing braces in smbacl4_win2nfs4 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit ba73d2363d93a376ba4947963c9de45a7e683f02) --- source3/modules/nfs4_acls.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index bab73a5cb58..11cb80e9300 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -905,12 +905,14 @@ static struct SMB4ACL_T *smbacl4_win2nfs4( if (pparams->acedup!=e_dontcare) { if (smbacl4_MergeIgnoreReject(pparams->acedup, theacl, - &ace_v4, &addNewACE, i)) + &ace_v4, &addNewACE, i)) { return NULL; + } } - if (addNewACE) + if (addNewACE) { smb_add_ace4(theacl, &ace_v4); + } } if (pparams->mode==e_simple) { -- 2.17.0 From 32d5c7b5bac3668d5df624fe501aedf14bf33ed0 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Mon, 15 Jul 2019 14:43:01 -0700 Subject: [PATCH 27/40] nfs4_acls: Remove i argument from smbacl4_MergeIgnoreReject This is only used for logging of a rejected ACL, but does not provide additional useful information. Remove it to simplify the function a bit. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 44790721e4f2c6ee6f46de7ac88123ce1a9f6e39) --- source3/modules/nfs4_acls.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index 11cb80e9300..2317e0bc8b1 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -765,13 +765,10 @@ static bool smbacl4_fill_ace4( return true; /* OK */ } -static int smbacl4_MergeIgnoreReject( - enum smbacl4_acedup_enum acedup, - struct SMB4ACL_T *theacl, /* may modify it */ - SMB_ACE4PROP_T *ace, /* the "new" ACE */ - bool *paddNewACE, - int i -) +static int smbacl4_MergeIgnoreReject(enum smbacl4_acedup_enum acedup, + struct SMB4ACL_T *theacl, + SMB_ACE4PROP_T *ace, + bool *paddNewACE) { int result = 0; SMB_ACE4PROP_T *ace4found = smbacl4_find_equal_special(theacl, ace); @@ -788,7 +785,7 @@ static int smbacl4_MergeIgnoreReject( *paddNewACE = false; break; case e_reject: /* do an error */ - DEBUG(8, ("ACL rejected by duplicate nt ace#%d\n", i)); + DBG_INFO("ACL rejected by duplicate nt ace.\n"); errno = EINVAL; /* SHOULD be set on any _real_ error */ result = -1; break; @@ -905,7 +902,7 @@ static struct SMB4ACL_T *smbacl4_win2nfs4( if (pparams->acedup!=e_dontcare) { if (smbacl4_MergeIgnoreReject(pparams->acedup, theacl, - &ace_v4, &addNewACE, i)) { + &ace_v4, &addNewACE)) { return NULL; } } -- 2.17.0 From 312b4e4a645b2f861efe269404841b6725137993 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 16 Jul 2019 15:20:25 -0700 Subject: [PATCH 28/40] nfs4_acls: Move smbacl4_MergeIgnoreReject function This static function will be called earlier in later patches. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 3499d97463110f042415d917160bc2743805a544) --- source3/modules/nfs4_acls.c | 61 ++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index 2317e0bc8b1..cb407c6e032 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -654,6 +654,36 @@ static SMB_ACE4PROP_T *smbacl4_find_equal_special( return NULL; } +static int smbacl4_MergeIgnoreReject(enum smbacl4_acedup_enum acedup, + struct SMB4ACL_T *theacl, + SMB_ACE4PROP_T *ace, + bool *paddNewACE) +{ + int result = 0; + SMB_ACE4PROP_T *ace4found = smbacl4_find_equal_special(theacl, ace); + if (ace4found) + { + switch(acedup) + { + case e_merge: /* "merge" flags */ + *paddNewACE = false; + ace4found->aceFlags |= ace->aceFlags; + ace4found->aceMask |= ace->aceMask; + break; + case e_ignore: /* leave out this record */ + *paddNewACE = false; + break; + case e_reject: /* do an error */ + DBG_INFO("ACL rejected by duplicate nt ace.\n"); + errno = EINVAL; /* SHOULD be set on any _real_ error */ + result = -1; + break; + default: + break; + } + } + return result; +} static bool smbacl4_fill_ace4( bool is_directory, @@ -765,37 +795,6 @@ static bool smbacl4_fill_ace4( return true; /* OK */ } -static int smbacl4_MergeIgnoreReject(enum smbacl4_acedup_enum acedup, - struct SMB4ACL_T *theacl, - SMB_ACE4PROP_T *ace, - bool *paddNewACE) -{ - int result = 0; - SMB_ACE4PROP_T *ace4found = smbacl4_find_equal_special(theacl, ace); - if (ace4found) - { - switch(acedup) - { - case e_merge: /* "merge" flags */ - *paddNewACE = false; - ace4found->aceFlags |= ace->aceFlags; - ace4found->aceMask |= ace->aceMask; - break; - case e_ignore: /* leave out this record */ - *paddNewACE = false; - break; - case e_reject: /* do an error */ - DBG_INFO("ACL rejected by duplicate nt ace.\n"); - errno = EINVAL; /* SHOULD be set on any _real_ error */ - result = -1; - break; - default: - break; - } - } - return result; -} - static int smbacl4_substitute_special( struct SMB4ACL_T *acl, uid_t ownerUID, -- 2.17.0 From 8349de978c34621ad5da5976217047ba637f7588 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 16 Jul 2019 15:30:36 -0700 Subject: [PATCH 29/40] nfs4_acls: Move adding of NFS4 ACE to ACL to smbacl4_fill_ace4 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit abb58b17599bd3f9a06037e208dcc5033c7fdd8b) --- source3/modules/nfs4_acls.c | 68 +++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 29 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index cb407c6e032..bab4dd0fd64 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -685,16 +685,41 @@ static int smbacl4_MergeIgnoreReject(enum smbacl4_acedup_enum acedup, return result; } -static bool smbacl4_fill_ace4( +static int nfs4_acl_add_ace(enum smbacl4_acedup_enum acedup, + struct SMB4ACL_T *nfs4_acl, + SMB_ACE4PROP_T *nfs4_ace) +{ + bool add_ace = true; + + if (acedup != e_dontcare) { + int ret; + + ret = smbacl4_MergeIgnoreReject(acedup, nfs4_acl, + nfs4_ace, &add_ace); + if (ret == -1) { + return -1; + } + } + + if (add_ace) { + smb_add_ace4(nfs4_acl, nfs4_ace); + } + + return 0; +} + +static int smbacl4_fill_ace4( bool is_directory, const struct smbacl4_vfs_params *params, uid_t ownerUID, gid_t ownerGID, const struct security_ace *ace_nt, /* input */ - SMB_ACE4PROP_T *ace_v4 /* output */ + struct SMB4ACL_T *nfs4_acl ) { struct dom_sid_buf buf; + SMB_ACE4PROP_T nfs4_ace = { 0 }; + SMB_ACE4PROP_T *ace_v4 = &nfs4_ace; DEBUG(10, ("got ace for %s\n", dom_sid_str_buf(&ace_nt->trustee, &buf))); @@ -742,7 +767,7 @@ static bool smbacl4_fill_ace4( ace_v4->aceFlags |= SMB_ACE4_INHERIT_ONLY_ACE; if (!(ace_v4->aceFlags & SMB_ACE4_DIRECTORY_INHERIT_ACE) && !(ace_v4->aceFlags & SMB_ACE4_FILE_INHERIT_ACE)) { - return false; + return 0; } } else if (params->mode!=e_special && dom_sid_equal(&ace_nt->trustee, @@ -754,7 +779,7 @@ static bool smbacl4_fill_ace4( ace_v4->aceFlags |= SMB_ACE4_INHERIT_ONLY_ACE; if (!(ace_v4->aceFlags & SMB_ACE4_DIRECTORY_INHERIT_ACE) && !(ace_v4->aceFlags & SMB_ACE4_FILE_INHERIT_ACE)) { - return false; + return 0; } } else { struct unixid unixid; @@ -764,12 +789,12 @@ static bool smbacl4_fill_ace4( if (!ok) { DBG_WARNING("Could not convert %s to uid or gid.\n", dom_sid_str_buf(&ace_nt->trustee, &buf)); - return false; + return 0; } if (dom_sid_compare_domain(&ace_nt->trustee, &global_sid_Unix_NFS) == 0) { - return false; + return 0; } switch (unixid.type) { @@ -788,11 +813,11 @@ static bool smbacl4_fill_ace4( default: DBG_WARNING("Could not convert %s to uid or gid.\n", dom_sid_str_buf(&ace_nt->trustee, &buf)); - return false; + return 0; } } - return true; /* OK */ + return nfs4_acl_add_ace(params->acedup, nfs4_acl, &nfs4_ace); } static int smbacl4_substitute_special( @@ -886,28 +911,13 @@ static struct SMB4ACL_T *smbacl4_win2nfs4( return NULL; for(i=0; inum_aces; i++) { - SMB_ACE4PROP_T ace_v4; - bool addNewACE = true; - - if (!smbacl4_fill_ace4(is_directory, pparams, - ownerUID, ownerGID, - dacl->aces + i, &ace_v4)) { - struct dom_sid_buf buf; - DEBUG(3, ("Could not fill ace for file, SID %s\n", - dom_sid_str_buf(&((dacl->aces+i)->trustee), - &buf))); - continue; - } - - if (pparams->acedup!=e_dontcare) { - if (smbacl4_MergeIgnoreReject(pparams->acedup, theacl, - &ace_v4, &addNewACE)) { - return NULL; - } - } + int ret; - if (addNewACE) { - smb_add_ace4(theacl, &ace_v4); + ret = smbacl4_fill_ace4(is_directory, pparams, + ownerUID, ownerGID, + dacl->aces + i, theacl); + if (ret == -1) { + return NULL; } } -- 2.17.0 From 5095132e10278995c38e1dd760d35e3bcac6f96d Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 16 Jul 2019 15:50:36 -0700 Subject: [PATCH 30/40] nfs4_acls: Remove redundant logging from smbacl4_fill_ace4 Logging flags in case they do not match seems unnecessary. Other log messages should show the flags as well. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 7ab0003ffc098247c3ee3962d7061f2af5a2d00e) --- source3/modules/nfs4_acls.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index bab4dd0fd64..25bcc770095 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -746,14 +746,6 @@ static int smbacl4_fill_ace4( se_map_generic(&ace_v4->aceMask, &file_generic_mapping); - if (ace_v4->aceFlags!=ace_nt->flags) - DEBUG(9, ("ace_v4->aceFlags(0x%x)!=ace_nt->flags(0x%x)\n", - ace_v4->aceFlags, ace_nt->flags)); - - if (ace_v4->aceMask!=ace_nt->access_mask) - DEBUG(9, ("ace_v4->aceMask(0x%x)!=ace_nt->access_mask(0x%x)\n", - ace_v4->aceMask, ace_nt->access_mask)); - if (dom_sid_equal(&ace_nt->trustee, &global_sid_World)) { ace_v4->who.special_id = SMB_ACE4_WHO_EVERYONE; ace_v4->flags |= SMB_ACE4_ID_SPECIAL; -- 2.17.0 From 8379bff33369cdbc662fde83a3888db48c266cc5 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 16 Jul 2019 15:56:12 -0700 Subject: [PATCH 31/40] nfs4_acls: Remove redundant pointer variable The previous patch introduced a pointer to a local variable to reduce the amount of lines changed. Remove that pointer and adjust all usage accordingly. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit aa4644193635d846c2e08e8c1e7b512e8009c2ef) --- source3/modules/nfs4_acls.c | 56 +++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index 25bcc770095..d169377295a 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -719,58 +719,54 @@ static int smbacl4_fill_ace4( { struct dom_sid_buf buf; SMB_ACE4PROP_T nfs4_ace = { 0 }; - SMB_ACE4PROP_T *ace_v4 = &nfs4_ace; DEBUG(10, ("got ace for %s\n", dom_sid_str_buf(&ace_nt->trustee, &buf))); - ZERO_STRUCTP(ace_v4); - /* only ACCESS|DENY supported right now */ - ace_v4->aceType = ace_nt->type; + nfs4_ace.aceType = ace_nt->type; - ace_v4->aceFlags = map_windows_ace_flags_to_nfs4_ace_flags( - ace_nt->flags); + nfs4_ace.aceFlags = + map_windows_ace_flags_to_nfs4_ace_flags(ace_nt->flags); /* remove inheritance flags on files */ if (!is_directory) { DEBUG(10, ("Removing inheritance flags from a file\n")); - ace_v4->aceFlags &= ~(SMB_ACE4_FILE_INHERIT_ACE| - SMB_ACE4_DIRECTORY_INHERIT_ACE| - SMB_ACE4_NO_PROPAGATE_INHERIT_ACE| - SMB_ACE4_INHERIT_ONLY_ACE); + nfs4_ace.aceFlags &= ~(SMB_ACE4_FILE_INHERIT_ACE| + SMB_ACE4_DIRECTORY_INHERIT_ACE| + SMB_ACE4_NO_PROPAGATE_INHERIT_ACE| + SMB_ACE4_INHERIT_ONLY_ACE); } - ace_v4->aceMask = ace_nt->access_mask & - (SEC_STD_ALL | SEC_FILE_ALL); + nfs4_ace.aceMask = ace_nt->access_mask & (SEC_STD_ALL | SEC_FILE_ALL); - se_map_generic(&ace_v4->aceMask, &file_generic_mapping); + se_map_generic(&nfs4_ace.aceMask, &file_generic_mapping); if (dom_sid_equal(&ace_nt->trustee, &global_sid_World)) { - ace_v4->who.special_id = SMB_ACE4_WHO_EVERYONE; - ace_v4->flags |= SMB_ACE4_ID_SPECIAL; + nfs4_ace.who.special_id = SMB_ACE4_WHO_EVERYONE; + nfs4_ace.flags |= SMB_ACE4_ID_SPECIAL; } else if (params->mode!=e_special && dom_sid_equal(&ace_nt->trustee, &global_sid_Creator_Owner)) { DEBUG(10, ("Map creator owner\n")); - ace_v4->who.special_id = SMB_ACE4_WHO_OWNER; - ace_v4->flags |= SMB_ACE4_ID_SPECIAL; + nfs4_ace.who.special_id = SMB_ACE4_WHO_OWNER; + nfs4_ace.flags |= SMB_ACE4_ID_SPECIAL; /* A non inheriting creator owner entry has no effect. */ - ace_v4->aceFlags |= SMB_ACE4_INHERIT_ONLY_ACE; - if (!(ace_v4->aceFlags & SMB_ACE4_DIRECTORY_INHERIT_ACE) - && !(ace_v4->aceFlags & SMB_ACE4_FILE_INHERIT_ACE)) { + nfs4_ace.aceFlags |= SMB_ACE4_INHERIT_ONLY_ACE; + if (!(nfs4_ace.aceFlags & SMB_ACE4_DIRECTORY_INHERIT_ACE) + && !(nfs4_ace.aceFlags & SMB_ACE4_FILE_INHERIT_ACE)) { return 0; } } else if (params->mode!=e_special && dom_sid_equal(&ace_nt->trustee, &global_sid_Creator_Group)) { DEBUG(10, ("Map creator owner group\n")); - ace_v4->who.special_id = SMB_ACE4_WHO_GROUP; - ace_v4->flags |= SMB_ACE4_ID_SPECIAL; + nfs4_ace.who.special_id = SMB_ACE4_WHO_GROUP; + nfs4_ace.flags |= SMB_ACE4_ID_SPECIAL; /* A non inheriting creator group entry has no effect. */ - ace_v4->aceFlags |= SMB_ACE4_INHERIT_ONLY_ACE; - if (!(ace_v4->aceFlags & SMB_ACE4_DIRECTORY_INHERIT_ACE) - && !(ace_v4->aceFlags & SMB_ACE4_FILE_INHERIT_ACE)) { + nfs4_ace.aceFlags |= SMB_ACE4_INHERIT_ONLY_ACE; + if (!(nfs4_ace.aceFlags & SMB_ACE4_DIRECTORY_INHERIT_ACE) + && !(nfs4_ace.aceFlags & SMB_ACE4_FILE_INHERIT_ACE)) { return 0; } } else { @@ -791,15 +787,15 @@ static int smbacl4_fill_ace4( switch (unixid.type) { case ID_TYPE_BOTH: - ace_v4->aceFlags |= SMB_ACE4_IDENTIFIER_GROUP; - ace_v4->who.gid = unixid.id; + nfs4_ace.aceFlags |= SMB_ACE4_IDENTIFIER_GROUP; + nfs4_ace.who.gid = unixid.id; break; case ID_TYPE_GID: - ace_v4->aceFlags |= SMB_ACE4_IDENTIFIER_GROUP; - ace_v4->who.gid = unixid.id; + nfs4_ace.aceFlags |= SMB_ACE4_IDENTIFIER_GROUP; + nfs4_ace.who.gid = unixid.id; break; case ID_TYPE_UID: - ace_v4->who.uid = unixid.id; + nfs4_ace.who.uid = unixid.id; break; case ID_TYPE_NOT_SPECIFIED: default: -- 2.17.0 From ccd365b2e6f126533f2dbdb0765aad501816fbac Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 17 Jul 2019 10:49:47 -0700 Subject: [PATCH 32/40] nfs4_acls: Add additional owner entry when mapping to NFS4 ACL with IDMAP_TYPE_BOTH With IDMAP_TYPE_BOTH, all entries have to be mapped to group entries. In order to have the file system reflect the owner permissions in the POSIX modebits, create a second entry for the user. This will be mapped to the "special owner" entry. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit b796119e2df38d1935064556934dd10da6f3d339) --- source3/modules/nfs4_acls.c | 37 +++++++++++++++++++++++++++++- source3/modules/test_nfs4_acls.c | 39 +++++++++++++++++++++++++++----- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index d169377295a..70d725eb937 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -719,6 +719,9 @@ static int smbacl4_fill_ace4( { struct dom_sid_buf buf; SMB_ACE4PROP_T nfs4_ace = { 0 }; + SMB_ACE4PROP_T nfs4_ace_2 = { 0 }; + bool add_ace2 = false; + int ret; DEBUG(10, ("got ace for %s\n", dom_sid_str_buf(&ace_nt->trustee, &buf))); @@ -789,6 +792,29 @@ static int smbacl4_fill_ace4( case ID_TYPE_BOTH: nfs4_ace.aceFlags |= SMB_ACE4_IDENTIFIER_GROUP; nfs4_ace.who.gid = unixid.id; + + if (ownerUID == unixid.id && + !nfs_ace_is_inherit(&nfs4_ace)) + { + /* + * IDMAP_TYPE_BOTH for owner. Add + * additional user entry, which can be + * mapped to special:owner to reflect + * the permissions in the modebits. + * + * This only applies to non-inheriting + * entries as only these are replaced + * with SPECIAL_OWNER in nfs4:mode=simple. + */ + nfs4_ace_2 = (SMB_ACE4PROP_T) { + .who.uid = unixid.id, + .aceFlags = (nfs4_ace.aceFlags & + ~SMB_ACE4_IDENTIFIER_GROUP), + .aceMask = nfs4_ace.aceMask, + .aceType = nfs4_ace.aceType, + }; + add_ace2 = true; + } break; case ID_TYPE_GID: nfs4_ace.aceFlags |= SMB_ACE4_IDENTIFIER_GROUP; @@ -805,7 +831,16 @@ static int smbacl4_fill_ace4( } } - return nfs4_acl_add_ace(params->acedup, nfs4_acl, &nfs4_ace); + ret = nfs4_acl_add_ace(params->acedup, nfs4_acl, &nfs4_ace); + if (ret != 0) { + return -1; + } + + if (!add_ace2) { + return 0; + } + + return nfs4_acl_add_ace(params->acedup, nfs4_acl, &nfs4_ace_2); } static int smbacl4_substitute_special( diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index d7152a0737a..170a397579a 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -1671,6 +1671,7 @@ struct dacl_to_nfs4_idmap_both { uint32_t nfs4_flags; uint32_t nfs4_ace_flags; uint32_t nfs4_id; + int num_nfs4_aces; }; /* @@ -1684,13 +1685,17 @@ static void test_dacl_to_nfs4_idmap_type_both(void **state) struct dacl_to_nfs4_idmap_both dacl_to_nfs4_idmap_both[] = { { &sids[2], 0, - SMB_ACE4_ID_SPECIAL, SMB_ACE4_IDENTIFIER_GROUP, SMB_ACE4_WHO_GROUP }, + SMB_ACE4_ID_SPECIAL, SMB_ACE4_IDENTIFIER_GROUP, SMB_ACE4_WHO_GROUP, + 2 }, { &sids[2], SEC_ACE_FLAG_OBJECT_INHERIT, - 0, SMB_ACE4_IDENTIFIER_GROUP|SMB_ACE4_FILE_INHERIT_ACE, 1002 }, + 0, SMB_ACE4_IDENTIFIER_GROUP|SMB_ACE4_FILE_INHERIT_ACE, 1002, + 1 }, { &sids[6], 0, - 0, SMB_ACE4_IDENTIFIER_GROUP, 1005 }, + 0, SMB_ACE4_IDENTIFIER_GROUP, 1005, + 1 }, { &sids[6], SEC_ACE_FLAG_OBJECT_INHERIT, - 0, SMB_ACE4_IDENTIFIER_GROUP|SMB_ACE4_FILE_INHERIT_ACE, 1005 }, + 0, SMB_ACE4_IDENTIFIER_GROUP|SMB_ACE4_FILE_INHERIT_ACE, 1005, + 1 }, }; for (i = 0; i < ARRAY_SIZE(dacl_to_nfs4_idmap_both); i++) { @@ -1720,11 +1725,11 @@ static void test_dacl_to_nfs4_idmap_type_both(void **state) assert_non_null(nfs4_acl); assert_int_equal(smbacl4_get_controlflags(nfs4_acl), SEC_DESC_SELF_RELATIVE); - assert_int_equal(smb_get_naces(nfs4_acl), 1); + assert_int_equal(smb_get_naces(nfs4_acl), + dacl_to_nfs4_idmap_both[i].num_nfs4_aces); nfs4_ace_container = smb_first_ace4(nfs4_acl); assert_non_null(nfs4_ace_container); - assert_null(smb_next_ace4(nfs4_ace_container)); nfs4_ace = smb_get_ace4(nfs4_ace_container); assert_int_equal(nfs4_ace->flags, @@ -1744,6 +1749,28 @@ static void test_dacl_to_nfs4_idmap_type_both(void **state) assert_int_equal(nfs4_ace->aceType, SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE); assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + + if (dacl_to_nfs4_idmap_both[i].num_nfs4_aces == 2) { + nfs4_ace_container = smb_next_ace4(nfs4_ace_container); + assert_non_null(nfs4_ace_container); + + nfs4_ace = smb_get_ace4(nfs4_ace_container); + assert_int_equal(nfs4_ace->flags, + dacl_to_nfs4_idmap_both[i].nfs4_flags); + assert_int_equal(nfs4_ace->aceFlags, + dacl_to_nfs4_idmap_both[i].nfs4_ace_flags & + ~SMB_ACE4_IDENTIFIER_GROUP); + if (nfs4_ace->flags & SMB_ACE4_ID_SPECIAL) { + assert_int_equal(nfs4_ace->who.special_id, + SMB_ACE4_WHO_OWNER); + } else { + assert_int_equal(nfs4_ace->who.uid, + dacl_to_nfs4_idmap_both[i].nfs4_id); + } + assert_int_equal(nfs4_ace->aceType, + SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE); + assert_int_equal(nfs4_ace->aceMask, SMB_ACE4_READ_DATA); + } } TALLOC_FREE(frame); -- 2.17.0 From 163da7a264495fbe68f8c6174973f7f10848e8bf Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Thu, 18 Jul 2019 11:49:29 -0700 Subject: [PATCH 33/40] nfs4_acls: Rename smbacl4_fill_ace4 function As this function now maps the ACE and also adds it to the NFSv4 ACE, change the name to better describe its behavior. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 169812943de23cf2752289c63331d786b0b063bd) --- source3/modules/nfs4_acls.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index 70d725eb937..663fcba67aa 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -708,14 +708,12 @@ static int nfs4_acl_add_ace(enum smbacl4_acedup_enum acedup, return 0; } -static int smbacl4_fill_ace4( - bool is_directory, - const struct smbacl4_vfs_params *params, - uid_t ownerUID, - gid_t ownerGID, - const struct security_ace *ace_nt, /* input */ - struct SMB4ACL_T *nfs4_acl -) +static int nfs4_acl_add_sec_ace(bool is_directory, + const struct smbacl4_vfs_params *params, + uid_t ownerUID, + gid_t ownerGID, + const struct security_ace *ace_nt, + struct SMB4ACL_T *nfs4_acl) { struct dom_sid_buf buf; SMB_ACE4PROP_T nfs4_ace = { 0 }; @@ -936,9 +934,9 @@ static struct SMB4ACL_T *smbacl4_win2nfs4( for(i=0; inum_aces; i++) { int ret; - ret = smbacl4_fill_ace4(is_directory, pparams, - ownerUID, ownerGID, - dacl->aces + i, theacl); + ret = nfs4_acl_add_sec_ace(is_directory, pparams, + ownerUID, ownerGID, + dacl->aces + i, theacl); if (ret == -1) { return NULL; } -- 2.17.0 From ac863f670869b72d267ce0c8c5c2f39b68e7159f Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 15:08:11 -0700 Subject: [PATCH 34/40] nfs4_acls: Remove duplicate entries when mapping from NFS4 ACL to DACL The previous patch added an additional entry for IDMAP_TYPE_BOTH. When mapping back to a DACL, there should be no additional entry. Add a loop that will check and remove entries that are exact duplicates. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 9c88602128592ddad537bf70cbe3c51f0b2cebe5) --- source3/modules/nfs4_acls.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index 663fcba67aa..74b66a2c392 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -297,6 +297,35 @@ static int smbacl4_fGetFileOwner(files_struct *fsp, SMB_STRUCT_STAT *psbuf) return 0; } +static void check_for_duplicate_sec_ace(struct security_ace *nt_ace_list, + int *good_aces) +{ + struct security_ace *last = NULL; + int i; + + if (*good_aces < 2) { + return; + } + + last = &nt_ace_list[(*good_aces) - 1]; + + for (i = 0; i < (*good_aces) - 1; i++) { + struct security_ace *cur = &nt_ace_list[i]; + + if (cur->type == last->type && + cur->flags == last->flags && + cur->access_mask == last->access_mask && + dom_sid_equal(&cur->trustee, &last->trustee)) + { + struct dom_sid_buf sid_buf; + + DBG_INFO("Removing duplicate entry for SID %s.\n", + dom_sid_str_buf(&last->trustee, &sid_buf)); + (*good_aces)--; + } + } +} + static bool smbacl4_nfs42win(TALLOC_CTX *mem_ctx, const struct smbacl4_vfs_params *params, struct SMB4ACL_T *acl, /* in */ @@ -438,6 +467,8 @@ static bool smbacl4_nfs42win(TALLOC_CTX *mem_ctx, ace->aceType, mask, win_ace_flags); } + + check_for_duplicate_sec_ace(nt_ace_list, &good_aces); } nt_ace_list = talloc_realloc(mem_ctx, nt_ace_list, struct security_ace, -- 2.17.0 From 5361098b1d9ad5c72b64cbc134dbadd82a775691 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 10 Jul 2019 13:14:32 -0700 Subject: [PATCH 35/40] nfs4_acls: Add test for merging duplicates when mapping from NFS4 ACL to DACL The previous patch introduced merging of duplicates on the mapping path from NFS4 ACL entries to DACL entries. Add a testcase to verify the expected behavior of this codepath. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 1a137a2f20c2f159c5feaef230a2b85bb9fb23b5) --- source3/modules/test_nfs4_acls.c | 79 ++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/source3/modules/test_nfs4_acls.c b/source3/modules/test_nfs4_acls.c index 170a397579a..0b23bd1d02e 100644 --- a/source3/modules/test_nfs4_acls.c +++ b/source3/modules/test_nfs4_acls.c @@ -1776,6 +1776,84 @@ static void test_dacl_to_nfs4_idmap_type_both(void **state) TALLOC_FREE(frame); } +static void test_nfs4_to_dacl_remove_duplicate(void **state) +{ + + struct dom_sid *sids = *state; + TALLOC_CTX *frame = talloc_stackframe(); + struct SMB4ACL_T *nfs4_acl; + SMB_ACE4PROP_T nfs4_ace; + struct security_ace *dacl_aces; + int good_aces; + struct smbacl4_vfs_params params = { + .mode = e_simple, + .do_chown = true, + .acedup = e_dontcare, + .map_full_control = true, + }; + + nfs4_acl = smb_create_smb4acl(frame); + assert_non_null(nfs4_acl); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = 0, + .who.uid = 1002, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = SMB_ACE4_INHERITED_ACE, + .aceMask = SMB_ACE4_WRITE_DATA, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = 0, + .who.gid = 1002, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = SMB_ACE4_IDENTIFIER_GROUP| + SMB_ACE4_INHERITED_ACE, + .aceMask = SMB_ACE4_WRITE_DATA, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = 0, + .who.gid = 1002, + .aceType = SMB_ACE4_ACCESS_DENIED_ACE_TYPE, + .aceFlags = SMB_ACE4_IDENTIFIER_GROUP| + SMB_ACE4_INHERITED_ACE, + .aceMask = SMB_ACE4_WRITE_DATA, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + nfs4_ace = (SMB_ACE4PROP_T) { + .flags = 0, + .who.gid = 1002, + .aceType = SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE, + .aceFlags = SMB_ACE4_IDENTIFIER_GROUP| + SMB_ACE4_INHERITED_ACE, + .aceMask = SMB_ACE4_WRITE_DATA, + }; + assert_non_null(smb_add_ace4(nfs4_acl, &nfs4_ace)); + + assert_true(smbacl4_nfs42win(frame, ¶ms, nfs4_acl, + &sids[0], &sids[1], true, + &dacl_aces, &good_aces)); + + assert_int_equal(good_aces, 2); + assert_non_null(dacl_aces); + + assert_int_equal(dacl_aces[0].type, SEC_ACE_TYPE_ACCESS_ALLOWED); + assert_int_equal(dacl_aces[0].flags, SEC_ACE_FLAG_INHERITED_ACE); + assert_int_equal(dacl_aces[0].access_mask, SEC_FILE_WRITE_DATA); + assert_true(dom_sid_equal(&dacl_aces[0].trustee, &sids[2])); + + assert_int_equal(dacl_aces[1].type, SEC_ACE_TYPE_ACCESS_DENIED); + assert_int_equal(dacl_aces[1].flags, SEC_ACE_FLAG_INHERITED_ACE); + assert_int_equal(dacl_aces[1].access_mask, SEC_FILE_WRITE_DATA); + assert_true(dom_sid_equal(&dacl_aces[1].trustee, &sids[2])); + + TALLOC_FREE(frame); +} + int main(int argc, char **argv) { const struct CMUnitTest tests[] = { @@ -1799,6 +1877,7 @@ int main(int argc, char **argv) cmocka_unit_test(test_nfs4_to_dacl_config_special), cmocka_unit_test(test_nfs4_to_dacl_idmap_type_both), cmocka_unit_test(test_dacl_to_nfs4_idmap_type_both), + cmocka_unit_test(test_nfs4_to_dacl_remove_duplicate), }; cmocka_set_message_output(CM_OUTPUT_SUBUNIT); -- 2.17.0 From e739fe233a080ce965e6f95c5b9e5990aeb85512 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 17 Jul 2019 15:29:06 -0700 Subject: [PATCH 36/40] nfs4_acls: Use correct owner information for ACL after owner change After a chown, the cached stat data is obviously no longer valid. The code in smb_set_nt_acl_nfs4 checked the file correctly, but did only use a local buffer for the stat data. So later checks of the stat buffer under the fsp->fsp_name->st would still see the old information. Fix this by removing the local stat buffer and always update the one under fsp->fsp_name->st. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 86f7af84f04b06ed96b30f936ace92aa0937be06) --- source3/modules/nfs4_acls.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c index 74b66a2c392..eb76696948b 100644 --- a/source3/modules/nfs4_acls.c +++ b/source3/modules/nfs4_acls.c @@ -994,11 +994,11 @@ NTSTATUS smb_set_nt_acl_nfs4(vfs_handle_struct *handle, files_struct *fsp, struct SMB4ACL_T *theacl = NULL; bool result, is_directory; - SMB_STRUCT_STAT sbuf; bool set_acl_as_root = false; uid_t newUID = (uid_t)-1; gid_t newGID = (gid_t)-1; int saved_errno; + NTSTATUS status; TALLOC_CTX *frame = talloc_stackframe(); DEBUG(10, ("smb_set_nt_acl_nfs4 invoked for %s\n", fsp_str_dbg(fsp))); @@ -1022,25 +1022,29 @@ NTSTATUS smb_set_nt_acl_nfs4(vfs_handle_struct *handle, files_struct *fsp, pparams = ¶ms; } - if (smbacl4_fGetFileOwner(fsp, &sbuf)) { + status = vfs_stat_fsp(fsp); + if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(frame); - return map_nt_error_from_unix(errno); + return status; } - is_directory = S_ISDIR(sbuf.st_ex_mode); + is_directory = S_ISDIR(fsp->fsp_name->st.st_ex_mode); if (pparams->do_chown) { /* chown logic is a copy/paste from posix_acl.c:set_nt_acl */ - NTSTATUS status = unpack_nt_owners(fsp->conn, &newUID, &newGID, - security_info_sent, psd); + + uid_t old_uid = fsp->fsp_name->st.st_ex_uid; + uid_t old_gid = fsp->fsp_name->st.st_ex_uid; + status = unpack_nt_owners(fsp->conn, &newUID, &newGID, + security_info_sent, psd); if (!NT_STATUS_IS_OK(status)) { DEBUG(8, ("unpack_nt_owners failed")); TALLOC_FREE(frame); return status; } - if (((newUID != (uid_t)-1) && (sbuf.st_ex_uid != newUID)) || - ((newGID != (gid_t)-1) && (sbuf.st_ex_gid != newGID))) { - + if (((newUID != (uid_t)-1) && (old_uid != newUID)) || + ((newGID != (gid_t)-1) && (old_gid != newGID))) + { status = try_chown(fsp, newUID, newGID); if (!NT_STATUS_IS_OK(status)) { DEBUG(3,("chown %s, %u, %u failed. Error = " @@ -1055,11 +1059,14 @@ NTSTATUS smb_set_nt_acl_nfs4(vfs_handle_struct *handle, files_struct *fsp, DEBUG(10,("chown %s, %u, %u succeeded.\n", fsp_str_dbg(fsp), (unsigned int)newUID, (unsigned int)newGID)); - if (smbacl4_GetFileOwner(fsp->conn, - fsp->fsp_name, - &sbuf)){ + + /* + * Owner change, need to update stat info. + */ + status = vfs_stat_fsp(fsp); + if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(frame); - return map_nt_error_from_unix(errno); + return status; } /* If we successfully chowned, we know we must @@ -1077,7 +1084,8 @@ NTSTATUS smb_set_nt_acl_nfs4(vfs_handle_struct *handle, files_struct *fsp, } theacl = smbacl4_win2nfs4(frame, is_directory, psd->dacl, pparams, - sbuf.st_ex_uid, sbuf.st_ex_gid); + fsp->fsp_name->st.st_ex_uid, + fsp->fsp_name->st.st_ex_gid); if (!theacl) { TALLOC_FREE(frame); return map_nt_error_from_unix(errno); -- 2.17.0 From 081f7aaccbe2503fd879e1b102f9d0c4ed1463af Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 9 Jul 2019 12:04:35 -0700 Subject: [PATCH 37/40] vfs_gpfs: Remove merge_writeappend parameter All supported GPFS versions now support setting WRITE and APPEND in the ACLs independently. Remove this now unused parameter to simplify the code. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 0aca678fcf1788a76cf0ff11399211c795aa7d2f) --- source3/modules/vfs_gpfs.c | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c index d733df0aa5c..63aeeddf797 100644 --- a/source3/modules/vfs_gpfs.c +++ b/source3/modules/vfs_gpfs.c @@ -709,29 +709,6 @@ static struct gpfs_acl *vfs_gpfs_smbacl2gpfsacl(TALLOC_CTX *mem_ctx, gace->aceType = aceprop->aceType; gace->aceFlags = aceprop->aceFlags; gace->aceMask = aceprop->aceMask; - - /* - * GPFS can't distinguish between WRITE and APPEND on - * files, so one being set without the other is an - * error. Sorry for the many ()'s :-) - */ - - if (!fsp->is_directory - && - ((((gace->aceMask & ACE4_MASK_WRITE) == 0) - && ((gace->aceMask & ACE4_MASK_APPEND) != 0)) - || - (((gace->aceMask & ACE4_MASK_WRITE) != 0) - && ((gace->aceMask & ACE4_MASK_APPEND) == 0))) - && - lp_parm_bool(fsp->conn->params->service, "gpfs", - "merge_writeappend", True)) { - DEBUG(2, ("vfs_gpfs.c: file [%s]: ACE contains " - "WRITE^APPEND, setting WRITE|APPEND\n", - fsp_str_dbg(fsp))); - gace->aceMask |= ACE4_MASK_WRITE|ACE4_MASK_APPEND; - } - gace->aceIFlags = (aceprop->flags&SMB_ACE4_ID_SPECIAL) ? ACE4_IFLAG_SPECIAL_ID : 0; if (aceprop->flags&SMB_ACE4_ID_SPECIAL) -- 2.17.0 From bc9aa66978adb15cf0acbc9fb27b7fc4fbf6ed6e Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 10 Jul 2019 11:06:19 -0700 Subject: [PATCH 38/40] docs: Remove gpfs:merge_writeappend from vfs_gpfs manpage BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit 8bd79ecc37376dbaa35606f9c2777653eb3d55e3) --- docs-xml/manpages/vfs_gpfs.8.xml | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/docs-xml/manpages/vfs_gpfs.8.xml b/docs-xml/manpages/vfs_gpfs.8.xml index 428f48a6bf0..f854d8900b2 100644 --- a/docs-xml/manpages/vfs_gpfs.8.xml +++ b/docs-xml/manpages/vfs_gpfs.8.xml @@ -204,26 +204,6 @@ - gpfs:merge_writeappend = [ yes | no ] - - - GPFS ACLs doesn't know about the 'APPEND' right. - This option lets Samba map the 'APPEND' right to 'WRITE'. - - - - - yes(default) - map 'APPEND' to 'WRITE'. - - - no - do not map 'APPEND' to 'WRITE'. - - - - - - - gpfs:acl = [ yes | no ] -- 2.17.0 From dbf371e3f4d0a9af9cfc4421da0be5faa55c6cea Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 9 Jul 2019 13:08:35 -0700 Subject: [PATCH 39/40] vfs_gpfs: Move mapping from generic NFSv ACL to GPFS ACL to separate function This is not functional change. It cleans up the code a bit and makes expanding this codepath in a later patch easier. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit fbf3a090a9ec94262b2924461cc1d6336af9919c) --- source3/modules/vfs_gpfs.c | 69 ++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c index 63aeeddf797..fc7d73508d8 100644 --- a/source3/modules/vfs_gpfs.c +++ b/source3/modules/vfs_gpfs.c @@ -673,6 +673,43 @@ static NTSTATUS gpfsacl_get_nt_acl(vfs_handle_struct *handle, return map_nt_error_from_unix(errno); } +static bool vfs_gpfs_nfs4_ace_to_gpfs_ace(SMB_ACE4PROP_T *nfs4_ace, + struct gpfs_ace_v4 *gace) +{ + gace->aceType = nfs4_ace->aceType; + gace->aceFlags = nfs4_ace->aceFlags; + gace->aceMask = nfs4_ace->aceMask; + + if (nfs4_ace->flags & SMB_ACE4_ID_SPECIAL) { + switch(nfs4_ace->who.special_id) { + case SMB_ACE4_WHO_EVERYONE: + gace->aceIFlags = ACE4_IFLAG_SPECIAL_ID; + gace->aceWho = ACE4_SPECIAL_EVERYONE; + break; + case SMB_ACE4_WHO_OWNER: + gace->aceIFlags = ACE4_IFLAG_SPECIAL_ID; + gace->aceWho = ACE4_SPECIAL_OWNER; + break; + case SMB_ACE4_WHO_GROUP: + gace->aceIFlags = ACE4_IFLAG_SPECIAL_ID; + gace->aceWho = ACE4_SPECIAL_GROUP; + break; + default: + DBG_WARNING("Unsupported special_id %d\n", + nfs4_ace->who.special_id); + return false; + } + + return true; + } + + gace->aceIFlags = 0; + gace->aceWho = (nfs4_ace->aceFlags & SMB_ACE4_IDENTIFIER_GROUP) ? + nfs4_ace->who.gid : nfs4_ace->who.uid; + + return true; +} + static struct gpfs_acl *vfs_gpfs_smbacl2gpfsacl(TALLOC_CTX *mem_ctx, files_struct *fsp, struct SMB4ACL_T *smbacl, @@ -705,35 +742,11 @@ static struct gpfs_acl *vfs_gpfs_smbacl2gpfsacl(TALLOC_CTX *mem_ctx, for (smbace=smb_first_ace4(smbacl); smbace!=NULL; smbace = smb_next_ace4(smbace)) { struct gpfs_ace_v4 *gace = gpfs_ace_ptr(gacl, gacl->acl_nace); SMB_ACE4PROP_T *aceprop = smb_get_ace4(smbace); + bool add_ace; - gace->aceType = aceprop->aceType; - gace->aceFlags = aceprop->aceFlags; - gace->aceMask = aceprop->aceMask; - gace->aceIFlags = (aceprop->flags&SMB_ACE4_ID_SPECIAL) ? ACE4_IFLAG_SPECIAL_ID : 0; - - if (aceprop->flags&SMB_ACE4_ID_SPECIAL) - { - switch(aceprop->who.special_id) - { - case SMB_ACE4_WHO_EVERYONE: - gace->aceWho = ACE4_SPECIAL_EVERYONE; - break; - case SMB_ACE4_WHO_OWNER: - gace->aceWho = ACE4_SPECIAL_OWNER; - break; - case SMB_ACE4_WHO_GROUP: - gace->aceWho = ACE4_SPECIAL_GROUP; - break; - default: - DEBUG(8, ("unsupported special_id %d\n", aceprop->who.special_id)); - continue; /* don't add it !!! */ - } - } else { - /* just only for the type safety... */ - if (aceprop->aceFlags&SMB_ACE4_IDENTIFIER_GROUP) - gace->aceWho = aceprop->who.gid; - else - gace->aceWho = aceprop->who.uid; + add_ace = vfs_gpfs_nfs4_ace_to_gpfs_ace(aceprop, gace); + if (!add_ace) { + continue; } gacl->acl_nace++; -- 2.17.0 From f397da61dc8ac19548c6030435b42dd44415c6b9 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 9 Jul 2019 13:39:55 -0700 Subject: [PATCH 40/40] vfs_gpfs: Implement special case for denying owner access to ACL In GPFS, it is not possible to deny ACL or attribute access through a SPECIAL_OWNER entry. The best that can be done is mapping this to a named user entry, as this one can at least be stored in an ACL. The same cannot be done for inheriting SPECIAL_OWNER entries, as these represent CREATOR OWNER entries, and the limitation of not being able to deny owner access to ACL or attributes remains. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14032 Signed-off-by: Christof Schmitt Reviewed-by: Ralph Boehme (cherry picked from commit c1770ed96fd3137f45d584ba9328333d5505e3af) --- source3/modules/vfs_gpfs.c | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c index fc7d73508d8..38f324cb785 100644 --- a/source3/modules/vfs_gpfs.c +++ b/source3/modules/vfs_gpfs.c @@ -674,7 +674,8 @@ static NTSTATUS gpfsacl_get_nt_acl(vfs_handle_struct *handle, } static bool vfs_gpfs_nfs4_ace_to_gpfs_ace(SMB_ACE4PROP_T *nfs4_ace, - struct gpfs_ace_v4 *gace) + struct gpfs_ace_v4 *gace, + uid_t owner_uid) { gace->aceType = nfs4_ace->aceType; gace->aceFlags = nfs4_ace->aceFlags; @@ -687,8 +688,35 @@ static bool vfs_gpfs_nfs4_ace_to_gpfs_ace(SMB_ACE4PROP_T *nfs4_ace, gace->aceWho = ACE4_SPECIAL_EVERYONE; break; case SMB_ACE4_WHO_OWNER: - gace->aceIFlags = ACE4_IFLAG_SPECIAL_ID; - gace->aceWho = ACE4_SPECIAL_OWNER; + /* + * With GPFS it is not possible to deny ACL or + * attribute access to the owner. Setting an + * ACL with such an entry is not possible. + * Denying ACL or attribute access for the + * owner through a named ACL entry can be + * stored in an ACL, it is just not effective. + * + * Map this case to a named entry to allow at + * least setting this ACL, which will be + * enforced by the smbd permission check. Do + * not do this for an inheriting OWNER entry, + * as this represents a CREATOR OWNER ACE. The + * remaining limitation is that CREATOR OWNER + * cannot deny ACL or attribute access. + */ + if (!nfs_ace_is_inherit(nfs4_ace) && + nfs4_ace->aceType == + SMB_ACE4_ACCESS_DENIED_ACE_TYPE && + nfs4_ace->aceMask & (SMB_ACE4_READ_ATTRIBUTES| + SMB_ACE4_WRITE_ATTRIBUTES| + SMB_ACE4_READ_ACL| + SMB_ACE4_WRITE_ACL)) { + gace->aceIFlags = 0; + gace->aceWho = owner_uid; + } else { + gace->aceIFlags = ACE4_IFLAG_SPECIAL_ID; + gace->aceWho = ACE4_SPECIAL_OWNER; + } break; case SMB_ACE4_WHO_GROUP: gace->aceIFlags = ACE4_IFLAG_SPECIAL_ID; @@ -744,7 +772,8 @@ static struct gpfs_acl *vfs_gpfs_smbacl2gpfsacl(TALLOC_CTX *mem_ctx, SMB_ACE4PROP_T *aceprop = smb_get_ace4(smbace); bool add_ace; - add_ace = vfs_gpfs_nfs4_ace_to_gpfs_ace(aceprop, gace); + add_ace = vfs_gpfs_nfs4_ace_to_gpfs_ace(aceprop, gace, + fsp->fsp_name->st.st_ex_uid); if (!add_ace) { continue; } -- 2.17.0