Retracted: Conflicts between access_check in [MS_DTYP].pdf and se_access_check in Samba

[Richard Sharpe]

On Thu, Mar 7, 2013 at 10:13 AM, Richard Sharpe
<realrichardsharpe at gmail.com> wrote:
> On Wed, Mar 6, 2013 at 8:54 PM, Richard Sharpe
> <realrichardsharpe at gmail.com> wrote:
>> Hi folks,
>>
>> I just checked section 2.5.4.1 of [MS_DTYP].pdf and it seems that
>> there are some differences that might be important.
>>
>> This is what access_check is described as doing. It takes a Security
>> Descriptor, an Auth Context and an Access Mask. (It takes a couple
>> more parameters but that is not important here.)
>>
>> Firstly, check to see if SECURITY_ACCESS is required and if so, if the
>> Auth Context contains SeSecurityPrivilege, remove that bit from Access
>> Mask.
>>
>> Then, check if WRITE_DAC is required and if so, if the Auth Context
>> contains SeTakeOwnershipPrivilege, remove that bit from the Access
>> Mask.
>>
>> (Actually, these are slightly confusing in the document, but I think
>> this is the intent.)
>>
>> Then, check if Auth Context contains the Owner Sid from the Security
>> Descriptor, and if so, remove WRITE_DAC and READ_CONTROL from the
>> Access Mask. (Note, this has to change a bit because of OWNER_RIGHTS)
>>
>> Then, For Each ACE in DACL DO
>>
>>     if ACE.Flag does not contain INHERIT ONLY then
>>         case ACE.Type:
>>         ALLOWED: If ACE.Trustee is in the Auth Context, remove those
>> bits in ACE.AccessMask that are also in Access Mask
>>
>>         DENIED: If ACE.Trustee is in the Auth Context and any bits in
>> ACE.AccessMask remain in the Access Mask, return STATUS_DENIED. (This
>> is important. See below.)
>>
>>         end case
>>
>>     fi
>> done
>>
>> The fact that access checking stops on hitting the first DENIED bit is
>> why Microsoft says that Denied Entries should appear before Allowed
>> Entries and is different than the way Samba does things.
>>
>> In particular, if an ACL is ordered in the canonical manner as prescribed by MS:
>>
>>     Explicit Deny ACEs
>>     Explicit Allow ACEs
>>     Inherited Deny ACEs
>>     Inherited Allow ACEs
>>
>> Then there are cases where se_access_check in Samba will deny access
>> when Windows would no.
>>
>> For example, if there was an ACL containing:
>>
>>     Allow: R,W:Explicit
>>     Deny:W,WDAC:Inherited
>>
>> Windows would allow an open that only asked for R,W access while Samba
>> would Deny it I believe (because we add back the denied bits in
>> se_access_check:
>>
>>         /* Explicitly denied bits always override */
>>         bits_remaining |= explicitly_denied_bits;
>> )
>>
>> There is discussion on the web that the MS approach allows true
>> discretionary control in that the owner of a file or folder can
>> override inherited DENY bits if they desire.
>>
>> I will try to work up some examples to check if this is correct.
>
> OK, I have verified that Windows does it the way that section 2.5.4.1
> of [MS-DTYP].pdf describes.
>
> This means that there will be cases where Samba will DENY access when
> Windows would ALLOW access.
>
> This is not a security violation, I believe.
>
> I will try to work up a patch to fix this issue. I will likely create
> a bug in bugzilla first.

OK, after creating a test for this case, and running it against Samba
3.6.12 and finding that we do not fail the test, I went back and
looked more carefully at the code.

While we do things in a different way to what Windows does, I was
mistaken about Samba being in error. The code is complex and I missed
one aspect of it.

-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)