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

Jeremy Allison jra at samba.org
Fri Dec 9 16:55:47 UTC 2016


On Fri, Dec 09, 2016 at 08:56:20AM +0100, Ralph Böhme wrote:
> 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.

Good catch ! I didn't think of that case..



More information about the samba-technical mailing list