New SMB2/3 features and SMB_VFS_* and connection_struct...

[Stefan (metze) Metzmacher]

Hi Richard,

>> Currently we're trying to prepare the new structures and infrastructure
>> for master, while doing so we noticed a few problems and some undefined
>> behaviors related to the usage of VFS modules.
>>
>> 1. What SMB_VFS modules assume
>>  1.1 I guess most of the vfs modules assume that the SMB_VFS_CONNECT and
>>    SMB_VFS_DISCONNECT runs as the same user (they run as root,
>>    but connection_struct->session_info is for the same smb session,
>>    which means %U and others are the same).
>>  1.2 I guess most vfs modules assume that SMB_VFS_CREATE_FILE and
>> SMB_VFS_CLOSE
>>    (all other handle based operations) on the same fsp are called as the
>>    same user (after a change_to_user()).
>>  1.3 I guess vfs modules assume that the connection_struct they're
>> operating
>>    on doesn't change during an async request. The module would always
>>    get the same information out of connection_struct->session_info
>>  1.4 I guess vfs modules in general expect just one user session per
>>    connection_struct, which means fsp->access_mask is correctly
>>    checked against the connection_struct->share_access via
>>    smbd_calculate_access_mask()
>>
>> 2. What administrators assume
>>  2.1 I guess admins assume that "root preexec"/"preexec" and
>>      "root postexec"/"postexec" run as the same user (at least the same %U)
>>
>> It seems that none of the above is true, for smb1 if the client reuses
>> a tree connect for a different session, as the 'connection_struct' is reused
>> for all session/tcon combinations using the 'vuid cache', which just alters
>> 'connection_struct->read_only' and 'connection_struct->session_info'.
>>
>> Note: SMB_VFS_CLOSE() can run as any user (including root) depending
>> on the client behavior.
>>
>> In order to provide a more predicable behavior (which will also allow to add
>> more generic async tevent_req based functions), I'd suggest that
>> we have one 'connection_struct' per session/tcon combination
>> and also remove per request data (connection_struct->case_sensitive).
>>
>> We should also impersonate more correctly, so that all operations
>> on a file handle run as the same user, including TCP disconnects.
>> To do this sanely we need to have an tevent_context wrapper,
>> which impersonates before calling any event handler.
>> See
>> https://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-impersonate
> 
> Are you proposing that the smbd/thread change to the logged in user
> before any VFS function is called?

It's like that in most cases already, but not *all* cases.
* during a TCP disconnect it SMB_VFS_CLOSE gets called as root.
* during a TDIS SMB_VFS_CLOSE gets called as the session doing the
   disconnect, which might not be the same, as the one who opened the file.

I propose to always do the correct impersonation before calling into the
SMB_VFS
layer, even for events triggered by the SMB_VFS modules.

> Or, are you proposing that all VFS functions are called as root and
> that they change to the logged in user when and as needed. Ie, that
> the guarantee is that they are always called as root?

No, that might be something for the long term future...

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120529/d87ff498/attachment.pgp>