[PATCH] Fix se_access_check() to correctly processes owner rights (S-1-3-4) DENY ace entries.

[Ralph Böhme]

On Thu, Dec 08, 2016 at 04:10:56PM -0800, Jeremy Allison wrote:
> On Thu, Dec 08, 2016 at 11:47:33AM -0800, Jeremy Allison wrote:
> > Follows from the samba-technical conversation I was
> > having with Shilpa K <shilpa.krishnareddy at gmail.com>.
> > 
> > When processing DENY ACE entries for owner rights SIDs (S-1-3-4)
> > the current 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
> > [1] 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) included.
> > 
> > Please review and push if happy,
> 
> Slightly updated version with fixed commit message in
> patch #1, and updated test in patch #2 (now checks
> that asking for WRITE_DATA is correctly rejected
> as well as checking READ_DATA succeeds).
> 
> Please review and push if happy,

I think I found that this doesn't cover all cases, as it doesn't take
into account rights that where allowed in a previous ACE (given a
non-canonical ACL).

Not sure if this is a valid ACL, but with this one your patch will get
an access denied when read+write data would be requested:

[0] SID: User SID 1-5-21-something
    TYPE: ALLOW
    MASK: READ_DATA

[1] SID: S-1-3-4
    TYPE: DENY
    MASK: READ_DATA

[0] SID: User SID 1-5-21-something
    TYPE: ALLOW
    MASK: WRITE_DATA

I'll post an alternative patch that should cover all cases later on.

Cheerio!
-slow