Bug 12466 - se_access_check() incorrectly processes owner rights (S-1-3-4) DENY ace entries.
Summary: se_access_check() incorrectly processes owner rights (S-1-3-4) DENY ace entries.
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-08 19:31 UTC by Jeremy Allison
Modified: 2016-12-20 08:56 UTC (History)
2 users (show)

See Also:

Attachments
git-am fix for master (7.45 KB, patch)
2016-12-08 19:55 UTC, Jeremy Allison 		no flags Details
git-am fix for master. (7.83 KB, patch)
2016-12-09 00:11 UTC, Jeremy Allison 		no flags Details
Ralph's correct fix ! (13.34 KB, patch)
2016-12-09 17:14 UTC, Jeremy Allison 		no flags Details
git-am fix for 4.5.next (13.63 KB, patch)
2016-12-12 19:47 UTC, Jeremy Allison 		slow: review+
Details
git-am fix for 4.4.next (2.15 KB, patch)
2016-12-12 19:48 UTC, Jeremy Allison 		slow: review+
Details
Show Obsolete (2) 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 2016-12-08 19:31:08 UTC 
When processing DENY ACE entries for owner rights SIDs (S-1-3-4) the code OR's in the deny access mask bits without taking into account if they were being requested in the requested access mask.

E.g. The current logic has:

An ACL containining:

[0] SID: S-1-3-4
    TYPE: DENY
    MASK: DENY_WRITE
[0] SID: S-1-3-4
    TYPE: ALLOW
    MASK: ALLOW_ALL

prohibits an open request by the owner for READ_FILE - even though this is explicitly allowed.

Fix and regression test (that passes against Win2K12 to follow).

Bug and idea for a fix reported to samba-technical list by Shilpa K <shilpa.krishnareddy@gmail.com>
Comment 1 Jeremy Allison 2016-12-08 19:55:25 UTC 
Created attachment 12752 [details]
git-am fix for master
Comment 2 Jeremy Allison 2016-12-09 00:11:34 UTC 
Created attachment 12755 [details]
git-am fix for master.

Slightly improved commit message and regression test.
Comment 3 Jeremy Allison 2016-12-09 17:14:20 UTC 
Created attachment 12758 [details]
Ralph's correct fix !
Comment 4 Jeremy Allison 2016-12-12 19:47:06 UTC 
Created attachment 12763 [details]
git-am fix for 4.5.next

Contains cherry-pick info from master.
Comment 5 Jeremy Allison 2016-12-12 19:48:03 UTC 
Created attachment 12764 [details]
git-am fix for 4.4.next

Cherry-pick of fix from master. Doesn't include back-port of regression test (not needed as already tested in master/4.5.x patch).
Comment 6 Ralph Böhme 2016-12-12 20:59:55 UTC 
Reassigning to Karolin for inclusion in 4.4 and 4.5
Comment 7 Karolin Seeger 2016-12-14 11:12:13 UTC 
(In reply to Ralph Böhme from comment #6)
Pushed to autobuild-v4-{5,4}-test.
Comment 8 Karolin Seeger 2016-12-20 08:56:08 UTC 
(In reply to Karolin Seeger from comment #7)
Pushed to both branches.
Closing out bug report.

Thanks!