share-level permissions and SYSTEM_SECURITY to files and dirs failing even though token has SeSecurityPrivilege

[Richard Sharpe]

On Wed, May 16, 2012 at 12:53 AM, Stefan (metze) Metzmacher
<metze at samba.org> wrote:
> Hi,
>
>>>> in smbd_check_access_rights we first check if the share permissions
>>>> allow the mode of access. This fails of the caller asked for
>>>> SYSTEM_SECURITY and no further check is made.
>>>>
>>>> However, if the logged in user's token contains SeSecurityPrivilege
>>>> and rejected_share_access was only SYSTEM_SECURITY, then I believe
>>>> that the requested operation should be allowed to continue to further
>>>> checks.
>>>>
>>>> What say ye?
>>>
>>> Errrr. Yeah. Sounds about right to me :-).
>>
>> OK, but in thinking about this some more, I think the correct fix is a
>> slight restructuring of the current code.
>>
>> We currently have share_rejected_access, which is checked first up,
>> and we error out immediately if any bits are rejected at that point.
>>
>> Rather than duplicate the privilege checks that are in
>> se_access_check, I would rather use those share_rejected_bits to feed
>> into rejected_mask and have se_access_check pull those in to see if
>> privileges override them ...
>>
>> I guess I should look at how Windows handles this. The algorithm is
>> available, at least.
>>
>> The issue came up because xcopy /X with a SACL does really weird
>> things if it cannot get access to the parent directory with
>> SYSTEM_SECURITY but can get access to the newly created file with that
>> access mode. (It stores the SACL but creates an empty DACL :-)
>
> I think we should also need to change the way we do our 'vuid' cache
> based on the connection_struct.
>
> We currently call SMB_VFS_CONNECT/SMB_VFS_DISCONNECT only
> for the session that creates the tcon.
> And we also do the share security_descriptor checking also only at
> tcon time.
>
> Which means that others sessions will use the wrong 'share_access' masks.
>
> I think we should create a connection_struct for each vuid/tid combination
> and call SMB_VFS_CONNECT/SMB_VFS_DISCONNECT when ever we
> create a new connection_struct for a new vuid/tid combination.

Hmmm, that is something I had not noticed, but seem right now you mention it.

One other thing is that when you look at Share Permissions, you only
get a choice between Full Access, Read Access (I think) and None.
Today we use an Access Mask and compute MAXIMUM ALLOWED from the SD on
the share (but only for the first connecting user, as you say.)

Seems like I have two or three problems to solve then.

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