Bug 9674 - Samba denies owner Read Control when there is a DENY entry while W2K08 does not
Samba denies owner Read Control when there is a DENY entry while W2K08 does not
Status: RESOLVED FIXED
Product: Samba 4.0
Classification: Unclassified
Component: File services
unspecified
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-22 20:35 UTC by Richard Sharpe
Modified: 2013-02-27 08:52 UTC (History)
1 user (show)

See Also:


Attachments
Proposed patch (for master). (783 bytes, patch)
2013-02-22 23:58 UTC, Jeremy Allison
rsharpe: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Sharpe 2013-02-22 20:35:55 UTC
Test:

Create a file.
Add an ACE for DOMAIN Users: Deny ALL

Try to open the file for read: Denied. This is correct and the same as what W2K08 does.

Try to open the file with READ CONTROL: Denied. This is not correct. W2K08 allows the owner READ CONTROL in the presence of DENY entries for the same.

Thew problem is that in libcli/security/access_check.c we have this:

        /* the owner always gets owner rights as defined above */
        if (security_token_has_sid(token, sd->owner_sid)) {
                if (owner_rights_default)
                        /*
                         * Just remove them, no need to check if they are
                         * there.
                         */
                        bits_remaining &= ~(SEC_STD_WRITE_DAC |
                                            SEC_STD_READ_CONTROL);

                else {
                        bits_remaining &= ~owner_rights_allowed;
                        bits_remaining |= (owner_rights_denied &
access_desired);
                }
        }

        /* Explicitly denied bits always override */
        bits_remaining |= (explicitly_denied_bits & access_desired);

However, the manipulation of explicitly denied bits should happen before we do the OWNER RIGHTS check.
Comment 1 Jeremy Allison 2013-02-22 23:58:37 UTC
Created attachment 8579 [details]
Proposed patch (for master).

Richard, can you confirm this is the patch you need ?

Cheers,

Jeremy.
Comment 2 Richard Sharpe 2013-02-23 02:41:07 UTC
Comment on attachment 8579 [details]
Proposed patch (for master).

Yes, this is exactly what I tested today.
Comment 3 Jeremy Allison 2013-02-25 19:02:18 UTC
From master:

git cherry-pick 3e5acc155bb7be5c531a4a35b16e040f71f628ac

applies cleanly to 4.0.x.

Jeremy.
Comment 4 Jeremy Allison 2013-02-25 23:11:46 UTC
I don't think this applies to 3.6.x, only 4.0.x. In that case, re-assigning to Karolin for inclusion in 4.0.next.
Jeremy.
Comment 5 Karolin Seeger 2013-02-26 07:51:25 UTC
Pushed to autobuild-v4-0-test.
Comment 6 Karolin Seeger 2013-02-27 08:52:58 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!