[PATCH] Second part of bugfix for bug #13319 (round-tripping ACE's through vfs_fruit).
Jeremy Allison
jra at samba.org
Thu Mar 15 22:28:03 UTC 2018
On Thu, Mar 15, 2018 at 06:15:04PM -0400, jim via samba-technical wrote:
> Should vfs_fruit.c debug message use DEBUG_WARNING instead of DEBUG(1, per
> replaced code?
Yes, that's correct, I missed that. Updated patch attached.
Jeremy.
-------------- next part --------------
From b92c33e8004e8fb220cbf4c3f5d724252f600edb Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 15 Mar 2018 09:52:30 -0700
Subject: [PATCH 1/4] s3: smbd: vfs_fruit: Add remove_virtual_nfs_aces() a
generic NFS ACE remover.
Not yet used, will be used to tidyup existing code.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13319
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/modules/vfs_fruit.c | 43 +++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 29372e90174..67af69843ed 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -2954,6 +2954,49 @@ static NTSTATUS readdir_attr_macmeta(struct vfs_handle_struct *handle,
return status;
}
+static NTSTATUS remove_virtual_nfs_aces(struct security_descriptor *psd)
+{
+ NTSTATUS status;
+ uint32_t i;
+
+ if (psd->dacl == NULL) {
+ return NT_STATUS_OK;
+ }
+
+ for (i = 0; i < psd->dacl->num_aces; i++) {
+ /* MS NFS style mode/uid/gid */
+ if (!dom_sid_compare_domain(
+ &global_sid_Unix_NFS,
+ &psd->dacl->aces[i].trustee) == 0) {
+ /* Normal ACE entry. */
+ continue;
+ }
+
+ /*
+ * security_descriptor_dacl_del()
+ * *must* return NT_STATUS_OK as we know
+ * we have something to remove.
+ */
+
+ status = security_descriptor_dacl_del(psd,
+ &psd->dacl->aces[i].trustee);
+ if (!NT_STATUS_IS_OK(status)) {
+ DBG_WARNING("failed to remove MS NFS style ACE: %s\n",
+ nt_errstr(status));
+ return status;
+ }
+
+ /*
+ * security_descriptor_dacl_del() may delete more
+ * then one entry subsequent to this one if the
+ * SID matches, but we only need to ensure that
+ * we stay looking at the same element in the array.
+ */
+ i--;
+ }
+ return NT_STATUS_OK;
+}
+
/* Search MS NFS style ACE with UNIX mode */
static NTSTATUS check_ms_nfs(vfs_handle_struct *handle,
files_struct *fsp,
--
2.16.2.804.g6dcf76e118-goog
From 969a53ae0f846deeb12553080ed6616314dc0f8a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 15 Mar 2018 09:54:41 -0700
Subject: [PATCH 2/4] s3: smbd: vfs_fruit: Replace code in check_ms_nfs() with
remove_virtual_nfs_aces().
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13319
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/modules/vfs_fruit.c | 38 +------------------------------------
1 file changed, 1 insertion(+), 37 deletions(-)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 67af69843ed..38f421c337d 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -3006,9 +3006,6 @@ static NTSTATUS check_ms_nfs(vfs_handle_struct *handle,
{
uint32_t i;
struct fruit_config_data *config = NULL;
- struct dom_sid sid;
- NTSTATUS status = NT_STATUS_OK;
- bool remove_ok = false;
*pdo_chmod = false;
@@ -3042,40 +3039,7 @@ static NTSTATUS check_ms_nfs(vfs_handle_struct *handle,
* fruit_fget_nt_acl().
*/
- /* MS NFS style mode */
- sid_compose(&sid, &global_sid_Unix_NFS_Mode,
- fsp->fsp_name->st.st_ex_mode);
- status = security_descriptor_dacl_del(psd, &sid);
- remove_ok = (NT_STATUS_IS_OK(status) ||
- NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND));
- if (!remove_ok) {
- DBG_WARNING("failed to remove MS NFS_mode style ACE\n");
- return status;
- }
-
- /* MS NFS style uid */
- sid_compose(&sid, &global_sid_Unix_NFS_Users,
- fsp->fsp_name->st.st_ex_uid);
- status = security_descriptor_dacl_del(psd, &sid);
- remove_ok = (NT_STATUS_IS_OK(status) ||
- NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND));
- if (!remove_ok) {
- DBG_WARNING("failed to remove MS NFS_users style ACE\n");
- return status;
- }
-
- /* MS NFS style gid */
- sid_compose(&sid, &global_sid_Unix_NFS_Groups,
- fsp->fsp_name->st.st_ex_gid);
- status = security_descriptor_dacl_del(psd, &sid);
- remove_ok = (NT_STATUS_IS_OK(status) ||
- NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND));
- if (!remove_ok) {
- DBG_WARNING("failed to remove MS NFS_groups style ACE\n");
- return status;
- }
-
- return NT_STATUS_OK;
+ return remove_virtual_nfs_aces(psd);
}
/****************************************************************************
--
2.16.2.804.g6dcf76e118-goog
From 32d685339be9e559e5ee881d5b0477074d187b15 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 15 Mar 2018 09:57:09 -0700
Subject: [PATCH 3/4] s3: smbd: vfs_fruit: Replace code in fruit_fget_nt_acl()
with remove_virtual_nfs_aces().
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13319
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/modules/vfs_fruit.c | 35 +++++++----------------------------
1 file changed, 7 insertions(+), 28 deletions(-)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 38f421c337d..19b78edb949 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -5735,7 +5735,6 @@ static NTSTATUS fruit_fget_nt_acl(vfs_handle_struct *handle,
struct security_ace ace;
struct dom_sid sid;
struct fruit_config_data *config;
- bool remove_ok = false;
SMB_VFS_HANDLE_GET_DATA(handle, config,
struct fruit_config_data,
@@ -5757,18 +5756,16 @@ static NTSTATUS fruit_fget_nt_acl(vfs_handle_struct *handle,
return NT_STATUS_OK;
}
+ /* First remove any existing ACE's with NFS style mode/uid/gid SIDs. */
+ status = remove_virtual_nfs_aces(*ppdesc);
+ if (!NT_STATUS_IS_OK(status)) {
+ DBG_WARNING("failed to remove MS NFS style ACEs\n");
+ return status;
+ }
+
/* MS NFS style mode */
sid_compose(&sid, &global_sid_Unix_NFS_Mode, fsp->fsp_name->st.st_ex_mode);
init_sec_ace(&ace, &sid, SEC_ACE_TYPE_ACCESS_DENIED, 0, 0);
-
- /* First remove any existing ACE's with this SID. */
- status = security_descriptor_dacl_del(*ppdesc, &sid);
- remove_ok = (NT_STATUS_IS_OK(status) ||
- NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND));
- if (!remove_ok) {
- DBG_WARNING("failed to remove MS NFS_mode style ACE\n");
- return status;
- }
status = security_descriptor_dacl_add(*ppdesc, &ace);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(1,("failed to add MS NFS style ACE\n"));
@@ -5778,15 +5775,6 @@ static NTSTATUS fruit_fget_nt_acl(vfs_handle_struct *handle,
/* MS NFS style uid */
sid_compose(&sid, &global_sid_Unix_NFS_Users, fsp->fsp_name->st.st_ex_uid);
init_sec_ace(&ace, &sid, SEC_ACE_TYPE_ACCESS_DENIED, 0, 0);
-
- /* First remove any existing ACE's with this SID. */
- status = security_descriptor_dacl_del(*ppdesc, &sid);
- remove_ok = (NT_STATUS_IS_OK(status) ||
- NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND));
- if (!remove_ok) {
- DBG_WARNING("failed to remove MS NFS_users style ACE\n");
- return status;
- }
status = security_descriptor_dacl_add(*ppdesc, &ace);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(1,("failed to add MS NFS style ACE\n"));
@@ -5796,15 +5784,6 @@ static NTSTATUS fruit_fget_nt_acl(vfs_handle_struct *handle,
/* MS NFS style gid */
sid_compose(&sid, &global_sid_Unix_NFS_Groups, fsp->fsp_name->st.st_ex_gid);
init_sec_ace(&ace, &sid, SEC_ACE_TYPE_ACCESS_DENIED, 0, 0);
-
- /* First remove any existing ACE's with this SID. */
- status = security_descriptor_dacl_del(*ppdesc, &sid);
- remove_ok = (NT_STATUS_IS_OK(status) ||
- NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND));
- if (!remove_ok) {
- DBG_WARNING("failed to remove MS NFS_groups style ACE\n");
- return status;
- }
status = security_descriptor_dacl_add(*ppdesc, &ace);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(1,("failed to add MS NFS style ACE\n"));
--
2.16.2.804.g6dcf76e118-goog
From 56a5d8318d144f94837d584897dbcec61fe0cf68 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 15 Mar 2018 14:45:06 -0700
Subject: [PATCH 4/4] s4: vfs: fruit tests: Add regression test for dealing
with NFS ACE entries.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13319
Signed-off-by: Jeremy Allison <jra at samba.org>
---
selftest/target/Samba3.pm | 10 ++
source3/selftest/tests.py | 4 +-
source4/torture/vfs/fruit.c | 184 ++++++++++++++++++++++++++++++++++++
source4/torture/vfs/vfs.c | 1 +
4 files changed, 198 insertions(+), 1 deletion(-)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index e6c95fa991a..2e3a027d970 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1926,6 +1926,16 @@ sub provision($$$$$$$$$)
fruit:time machine = yes
fruit:time machine max size = 32K
+[vfs_fruit_nfs_aces]
+ path = $shrdir
+ vfs objects = catia fruit streams_xattr acl_xattr xattr_tdb
+ fruit:resource = file
+ fruit:metadata = netatalk
+ fruit:locking = netatalk
+ fruit:encoding = native
+ fruit:veto_appledouble = no
+ fruit:nfs_aces = yes
+
[badname-tmp]
path = $badnames_shrdir
guest ok = yes
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 5232aecb609..5f3d35cf1ad 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -398,7 +398,7 @@ nbt = ["nbt.dgram" ]
libsmbclient = ["libsmbclient"]
-vfs = ["vfs.fruit", "vfs.acl_xattr", "vfs.fruit_netatalk", "vfs.fruit_file_id", "vfs.fruit_timemachine"]
+vfs = ["vfs.fruit", "vfs.acl_xattr", "vfs.fruit_netatalk", "vfs.fruit_file_id", "vfs.fruit_timemachine", "vfs.fruit_nfs_aces"]
tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idmap + vfs
@@ -507,6 +507,8 @@ for t in tests:
plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit_timemachine -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share')
elif t == "vfs.fruit_file_id":
plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit -U$USERNAME%$PASSWORD')
+ elif t == "vfs.fruit_nfs_aces":
+ plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit_nfs_aces -U$USERNAME%$PASSWORD')
elif t == "rpc.schannel_anon_setpw":
plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$%', description="anonymous password set")
plansmbtorture4testsuite(t, "nt4_dc_schannel", '//$SERVER_IP/tmp -U$%', description="anonymous password set (schannel enforced server-side)")
diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
index d071cf6f9af..0127bcb6536 100644
--- a/source4/torture/vfs/fruit.c
+++ b/source4/torture/vfs/fruit.c
@@ -36,6 +36,10 @@
#include "torture/smb2/proto.h"
#include "torture/vfs/proto.h"
#include "librpc/gen_ndr/ndr_ioctl.h"
+#include "libcli/security/dom_sid.h"
+#include "../librpc/gen_ndr/ndr_security.h"
+#include "libcli/security/secace.h"
+#include "libcli/security/security_descriptor.h"
#define BASEDIR "vfs_fruit_dir"
#define FNAME_CC_SRC "testfsctl.dat"
@@ -4664,3 +4668,183 @@ struct torture_suite *torture_vfs_fruit_timemachine(TALLOC_CTX *ctx)
return suite;
}
+
+/*
+ * Ensure this security descriptor has exactly one mode, uid
+ * and gid.
+ */
+
+static NTSTATUS check_nfs_sd(const struct security_descriptor *psd)
+{
+ uint32_t i;
+ bool got_one_mode = false;
+ bool got_one_uid = false;
+ bool got_one_gid = false;
+
+ if (psd->dacl == NULL) {
+ return NT_STATUS_INVALID_SECURITY_DESCR;
+ }
+
+ for (i = 0; i < psd->dacl->num_aces; i++) {
+ if (dom_sid_compare_domain(&global_sid_Unix_NFS_Mode,
+ &psd->dacl->aces[i].trustee) == 0) {
+ if (got_one_mode == true) {
+ /* Can't have more than one. */
+ return NT_STATUS_INVALID_SECURITY_DESCR;
+ }
+ got_one_mode = true;
+ }
+ }
+ for (i = 0; i < psd->dacl->num_aces; i++) {
+ if (dom_sid_compare_domain(&global_sid_Unix_NFS_Users,
+ &psd->dacl->aces[i].trustee) == 0) {
+ if (got_one_uid == true) {
+ /* Can't have more than one. */
+ return NT_STATUS_INVALID_SECURITY_DESCR;
+ }
+ got_one_uid = true;
+ }
+ }
+ for (i = 0; i < psd->dacl->num_aces; i++) {
+ if (dom_sid_compare_domain(&global_sid_Unix_NFS_Groups,
+ &psd->dacl->aces[i].trustee) == 0) {
+ if (got_one_gid == true) {
+ /* Can't have more than one. */
+ return NT_STATUS_INVALID_SECURITY_DESCR;
+ }
+ got_one_gid = true;
+ }
+ }
+ /* Must have at least one of each. */
+ if (got_one_mode == false ||
+ got_one_uid == false ||
+ got_one_gid == false) {
+ return NT_STATUS_INVALID_SECURITY_DESCR;
+ }
+ return NT_STATUS_OK;
+}
+
+static bool test_nfs_aces(struct torture_context *tctx,
+ struct smb2_tree *tree)
+{
+ TALLOC_CTX *mem_ctx = talloc_new(tctx);
+ struct security_ace ace;
+ struct dom_sid sid;
+ const char *fname = BASEDIR "\\nfs_aces.txt";
+ struct smb2_handle h = {{0}};
+ union smb_fileinfo finfo2;
+ union smb_setfileinfo set;
+ struct security_descriptor *psd = NULL;
+ NTSTATUS status;
+ bool ret = true;
+
+ ret = enable_aapl(tctx, tree);
+ torture_assert(tctx, ret == true, "enable_aapl failed");
+
+ /* clean slate ...*/
+ smb2_util_unlink(tree, fname);
+ smb2_deltree(tree, fname);
+ smb2_deltree(tree, BASEDIR);
+
+ status = torture_smb2_testdir(tree, BASEDIR, &h);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ smb2_util_close(tree, h);
+
+ /* Create a test file. */
+ status = torture_smb2_testfile_access(tree,
+ fname,
+ &h,
+ SEC_STD_READ_CONTROL |
+ SEC_STD_WRITE_DAC |
+ SEC_RIGHTS_FILE_ALL);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ /* Get the ACL. */
+ finfo2.query_secdesc.in.secinfo_flags =
+ SECINFO_OWNER |
+ SECINFO_GROUP |
+ SECINFO_DACL;
+ finfo2.generic.level = RAW_FILEINFO_SEC_DESC;
+ finfo2.generic.in.file.handle = h;
+ status = smb2_getinfo_file(tree, tctx, &finfo2);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ psd = finfo2.query_secdesc.out.sd;
+
+ /* Ensure we have only single mode/uid/gid NFS entries. */
+ status = check_nfs_sd(psd);
+ if (!NT_STATUS_IS_OK(status)) {
+ NDR_PRINT_DEBUG(
+ security_descriptor,
+ discard_const_p(struct security_descriptor, psd));
+ }
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ /* Add a couple of extra NFS uids and gids. */
+ sid_compose(&sid, &global_sid_Unix_NFS_Users, 27);
+ init_sec_ace(&ace, &sid, SEC_ACE_TYPE_ACCESS_DENIED, 0, 0);
+ status = security_descriptor_dacl_add(psd, &ace);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ status = security_descriptor_dacl_add(psd, &ace);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ sid_compose(&sid, &global_sid_Unix_NFS_Groups, 300);
+ init_sec_ace(&ace, &sid, SEC_ACE_TYPE_ACCESS_DENIED, 0, 0);
+ status = security_descriptor_dacl_add(psd, &ace);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ status = security_descriptor_dacl_add(psd, &ace);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ /* Now set on the file handle. */
+ set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
+ set.set_secdesc.in.file.handle = h;
+ set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
+ set.set_secdesc.in.sd = psd;
+ status = smb2_setinfo_file(tree, &set);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ /* Get the ACL again. */
+ finfo2.query_secdesc.in.secinfo_flags =
+ SECINFO_OWNER |
+ SECINFO_GROUP |
+ SECINFO_DACL;
+ finfo2.generic.level = RAW_FILEINFO_SEC_DESC;
+ finfo2.generic.in.file.handle = h;
+ status = smb2_getinfo_file(tree, tctx, &finfo2);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ psd = finfo2.query_secdesc.out.sd;
+
+ /* Ensure we have only single mode/uid/gid NFS entries. */
+ status = check_nfs_sd(psd);
+ if (!NT_STATUS_IS_OK(status)) {
+ NDR_PRINT_DEBUG(
+ security_descriptor,
+ discard_const_p(struct security_descriptor, psd));
+ }
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+done:
+ if (!smb2_util_handle_empty(h)) {
+ smb2_util_close(tree, h);
+ }
+ smb2_util_unlink(tree, fname);
+ smb2_deltree(tree, fname);
+ smb2_deltree(tree, BASEDIR);
+ talloc_free(mem_ctx);
+ return ret;
+}
+
+struct torture_suite *torture_vfs_fruit_nfs_aces(TALLOC_CTX *ctx)
+{
+ struct torture_suite *suite = torture_suite_create(
+ ctx, "fruit_nfs_aces");
+
+ suite->description = talloc_strdup(
+ suite, "vfs_fruit tests for NFS ACE entries");
+
+ torture_suite_add_1smb2_test(suite, "nfs_aces",
+ test_nfs_aces);
+
+ return suite;
+}
diff --git a/source4/torture/vfs/vfs.c b/source4/torture/vfs/vfs.c
index 64cb0e3d6c4..585c0ed34c5 100644
--- a/source4/torture/vfs/vfs.c
+++ b/source4/torture/vfs/vfs.c
@@ -113,6 +113,7 @@ NTSTATUS torture_vfs_init(TALLOC_CTX *ctx)
torture_suite_add_suite(suite, torture_acl_xattr(suite));
torture_suite_add_suite(suite, torture_vfs_fruit_file_id(suite));
torture_suite_add_suite(suite, torture_vfs_fruit_timemachine(suite));
+ torture_suite_add_suite(suite, torture_vfs_fruit_nfs_aces(suite));
torture_register_suite(ctx, suite);
--
2.16.2.804.g6dcf76e118-goog
More information about the samba-technical
mailing list