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-07-14 08:15 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
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

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
Comment 15 Samba QA Contact 2021-07-14 08:15:22 UTC 
This bug was referenced in samba v4-13-stable (Release samba-4.13.10):

abcddbae481034e35da7062e46ac86bc1c0b37d1