From 8f7a5b3adb942e42dfbe51462b802c85c76d0cf1 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Fri, 7 Jun 2019 12:55:32 -0700 Subject: [PATCH 01/39] 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 957ee481429bf3a3ffc9a66090cb47b555602823 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 11 Jun 2019 16:15:10 -0700 Subject: [PATCH 02/39] 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 146a054e77ecdcf94c1a806c2663dca61dc2a93b Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:22:13 -0700 Subject: [PATCH 03/39] 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 4a51002182f..4d61dc6b9b2 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 91b870430b9..082d9d0d305 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -415,6 +415,10 @@ if with_pthreadpool: "script/tests/test_libwbclient_threads.sh"), "$DOMAIN", "$DC_USERNAME"]) +plantestsuite("samba3.test_nfs4_acl", "none", + [os.path.join(bindir(), "test_nfs4_acls"), + "$SMB_CONF_PATH"]) + plantestsuite( "samba3.resolvconf", "none", [os.path.join(samba3srcdir, "script/tests/test_resolvconf.sh")]) -- 2.17.0 From f4f30f6f4d278b8b309d03cdf87b16cd5565600f Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:23:40 -0700 Subject: [PATCH 04/39] 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 251d22055a397e6372c64c7a302525302b0fa966 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:25:33 -0700 Subject: [PATCH 05/39] 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 b2f605bb97e8acac411f83049b03531d1e751dc0 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:28:31 -0700 Subject: [PATCH 06/39] 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 3bf6c7a2de5ee965ed8dc91cbd5cfab92afde725 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:30:12 -0700 Subject: [PATCH 07/39] 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 3b067b3dbb4a5c11a7542f00fd0de6f986c7fb30 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:33:29 -0700 Subject: [PATCH 08/39] 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 75a0b714b96f96561e76696a8b433c2e5fb7325f Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:35:34 -0700 Subject: [PATCH 09/39] 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 f6691c449b3faf08e5ebc0b6c6e613234964be3a Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:46:23 -0700 Subject: [PATCH 10/39] 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 7cb01df896df39752a5dedfee75c296828d29a9c Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:53:15 -0700 Subject: [PATCH 11/39] 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 a80aa79938a68929da5130c8df2b34ee9d988721 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:55:59 -0700 Subject: [PATCH 12/39] 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 4b860c824833f24697ed2eec28bd9079b38ceca8 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 11:57:45 -0700 Subject: [PATCH 13/39] 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 5e876c5b7d7803d7fd47481073ddf7ab041afbb5 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 12:02:58 -0700 Subject: [PATCH 14/39] 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 fd0793c2f9b17118f00f5b0b06a98f46f91cce07 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 12:07:36 -0700 Subject: [PATCH 15/39] 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 69642f4099649ef18781ae35056bd6cf00e04d25 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 12:09:04 -0700 Subject: [PATCH 16/39] 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 915f06ba783723bfe0a32abf554d86987f36041d Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 12:16:08 -0700 Subject: [PATCH 17/39] 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 c57c988db9e0ea6542c58cc11b8dc5e2f955787a Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 12:23:02 -0700 Subject: [PATCH 18/39] 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 e073c5608c8cc84a97ac82415d8db450ccb1a2a6 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 12:50:42 -0700 Subject: [PATCH 19/39] 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 91a108923ef693568c2754f758bd246477893f99 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 13:04:44 -0700 Subject: [PATCH 20/39] 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 8371c19d3926a0a6e6426ddf238eead0331994e1 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 26 Jun 2019 13:24:16 -0700 Subject: [PATCH 21/39] 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 fd2db2bf8322a26e015e3004325f9dc3995014d4 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Mon, 15 Jul 2019 13:15:32 -0700 Subject: [PATCH 22/39] 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 71c553ff88a80e593846f81bb55f3ed0ada61d2b Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 25 Jun 2019 15:21:06 -0700 Subject: [PATCH 23/39] 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 68e9768830575955113922f2462904a462650afd Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 26 Jun 2019 13:20:17 -0700 Subject: [PATCH 24/39] 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 941269a4b76c455921cb37d05349d5a9d59d00d9 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 13:20:44 -0700 Subject: [PATCH 25/39] 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 9e446d134e6c0a7d6016f77702ca3994d8462932 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Mon, 15 Jul 2019 14:43:01 -0700 Subject: [PATCH 26/39] 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 60209020566635323898bd3050d7d89fa9600ecf Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 16 Jul 2019 15:20:25 -0700 Subject: [PATCH 27/39] 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 e9ffa8c259aeb21af0c45288718475419a2801c5 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 16 Jul 2019 15:30:36 -0700 Subject: [PATCH 28/39] 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 e38a3e7f358523e1f7c9bb815984ea23d9d134df Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 16 Jul 2019 15:50:36 -0700 Subject: [PATCH 29/39] 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 0b3efed61e06bc1b8e3695696686dd3b5a2c2821 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 16 Jul 2019 15:56:12 -0700 Subject: [PATCH 30/39] 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 a17861a920a22fcfad76f1f766adf650c6ac90b7 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 17 Jul 2019 10:49:47 -0700 Subject: [PATCH 31/39] 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 ae8fbc44dd58f8769326641dba0d7773a449f2f8 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Thu, 18 Jul 2019 11:49:29 -0700 Subject: [PATCH 32/39] 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 4b461b2a3d38d3c8492df076cc7a22eaadde2840 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 2 Jul 2019 15:08:11 -0700 Subject: [PATCH 33/39] 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 cbf675b6316c5196092754e68e8628f1ee182a0d Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 10 Jul 2019 13:14:32 -0700 Subject: [PATCH 34/39] 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 f9d3726934860d08daf7ee71e9ddd8abac35a54d Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 17 Jul 2019 15:29:06 -0700 Subject: [PATCH 35/39] 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 cd6544904e50c8dd88aba9c9f87b76288722e773 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 9 Jul 2019 12:04:35 -0700 Subject: [PATCH 36/39] 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 3a75efdf5e6..0beb3ced573 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 0f5b2e595e94b9b772afcb7c762a99c312f30346 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Wed, 10 Jul 2019 11:06:19 -0700 Subject: [PATCH 37/39] 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 15e7bcf9d77..2a9af57d661 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 822721f23af33999f9059e6f6f28a2635a8bf946 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 9 Jul 2019 13:08:35 -0700 Subject: [PATCH 38/39] 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 0beb3ced573..0d5e21a7727 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 bfc47e9b13bb13f268863e41eaf10cf65b8de692 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 9 Jul 2019 13:39:55 -0700 Subject: [PATCH 39/39] 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 0d5e21a7727..06bebc0c167 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