Bug 14708 - POSIX default ACL not mapped into returned Windows ACL for directory handles.
Summary: POSIX default ACL not mapped into returned Windows ACL for directory handles.
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.13.0
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-17 22:20 UTC by Jeremy Allison
Modified: 2021-06-01 07:26 UTC (History)
2 users (show)

See Also:


Attachments
git-am fix for master. (1.59 KB, patch)
2021-05-17 22:43 UTC, Jeremy Allison
slow: review+
Details
git-am fix for master. (12.52 KB, patch)
2021-05-18 21:32 UTC, Jeremy Allison
no flags Details
git-am fix for 4.14.next, 4.13.next. (1.84 KB, patch)
2021-05-19 17:21 UTC, Jeremy Allison
npower: review+
jra: review? (slow)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2021-05-17 22:20:08 UTC
Looks like:

vfswrap_fget_nt_acl() has been broken for POSIX ACLs on
directories since 4.13. In 4.12 we have:

        /* can it happen that fsp_name == NULL ? */
        if (fsp->is_directory ||  fsp->fh->fd == -1) {
                status = posix_get_nt_acl(fsp->conn, fsp->fsp_name,
                                          security_info, mem_ctx, ppdesc);
                TALLOC_FREE(frame);
                return status;
        }

and posix_get_nt_acl() will add in the default POSIX ACL
for directories.

But in 4.13 and onwards we have:

        /* Get the ACL from the fd. */
        posix_acl = SMB_VFS_SYS_ACL_GET_FD(fsp, frame);

which never returns the default POSIX ACL for directories.

Seems like no one noticed. Have a patch, need a bugnumber.
Comment 1 Jeremy Allison 2021-05-17 22:43:08 UTC
Created attachment 16608 [details]
git-am fix for master.
Comment 2 Jeremy Allison 2021-05-18 01:31:03 UTC
Hmmm. fake_acls module might enable me to test this..
Comment 3 Ralph Böhme 2021-05-18 12:45:34 UTC
Comment on attachment 16608 [details]
git-am fix for master.

Lgtm, feel free to push.
Comment 5 Jeremy Allison 2021-05-18 21:32:54 UTC
Created attachment 16613 [details]
git-am fix for master.

Hi Ralph, turns out it wasn't too hard to add a regression test, so I did. Core of the patch hasn't changed, just an additional torture test before, and removal of the knownfail.d/ in the patch.

MR: is https://gitlab.com/samba-team/samba/-/merge_requests/1967
Comment 6 Samba QA Contact 2021-05-19 09:23:05 UTC
This bug was referenced in samba master:

544289b54bbf85098f4cc354f655290600c7f5ba
b7f62e13933da14c381f70cd46ad13849b108e68
Comment 7 Jeremy Allison 2021-05-19 17:21:25 UTC
Created attachment 16619 [details]
git-am fix for 4.14.next, 4.13.next.

This is only the cherry-pick of the code fix from master, backporting the test is too messy. So long as we have the test in master we should be good.
Comment 8 Jeremy Allison 2021-05-19 21:00:28 UTC
Re-assigning to Karolin for inclusion in 4.14.next, 4.13.next.
Comment 9 Jeremy Allison 2021-05-19 21:00:28 UTC
Re-assigning to Karolin for inclusion in 4.14.next, 4.13.next.
Comment 10 Karolin Seeger 2021-05-21 07:01:44 UTC
(In reply to Jeremy Allison from comment #9)
Pushed to autobuild-v4-{14,13}-test.
Comment 11 Samba QA Contact 2021-05-21 08:00:06 UTC
This bug was referenced in samba v4-14-test:

42726c3f665516a22006e2c6af8367ab377e15c4
Comment 12 Samba QA Contact 2021-05-21 08:51:03 UTC
This bug was referenced in samba v4-13-test:

abcddbae481034e35da7062e46ac86bc1c0b37d1
Comment 13 Karolin Seeger 2021-05-25 08:11:26 UTC
Pushed to both branches.
Closing out bug report.

Thanks!
Comment 14 Samba QA Contact 2021-06-01 07:26:24 UTC
This bug was referenced in samba v4-14-stable (Release samba-4.14.5):

42726c3f665516a22006e2c6af8367ab377e15c4