[PATCH] Dynamic share permission change detection.

[boyang]

Volker Lendecke wrote:
> On Mon, Nov 30, 2009 at 07:56:46PM +0800, boyang wrote:
>   
>> --- a/source3/smbd/open.c
>> +++ b/source3/smbd/open.c
>> @@ -1470,9 +1470,26 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
>>  	uint32 open_access_mask = access_mask;
>>  	NTSTATUS status;
>>  	char *parent_dir;
>> +	uint16 session_tag;
>>  
>>  	ZERO_STRUCT(id);
>>  
>> +	/* 
>> +	 * Check share access control in each open, what is 
>> +	 * windows does. We must not use vuid cache to perform 
>> +	 * checks here. And we will update vuid cache here.
>> +	 */
>> +	session_tag = (lp_security() == SEC_SHARE) ?
>> +			UID_FIELD_INVALID : req->vuid;
>> +	
>> +	become_root();
>> +	if (!check_share_perm_without_cache(conn, session_tag)) {
>> +		DEBUG(4, ("open_file_ntcreate: cannot access share!\n"));
>> +		unbecome_root();
>> +		return NT_STATUS_ACCESS_DENIED;
>> +	}
>> +	unbecome_root();
>>     
>
> Quick comment: We can't do a become_root() on every open.
> This is too expensive. We need to attach something like a
> security descriptor to the connection_struct to do the
> se_access_check in the open call.
>   
The problem here is that security descriptor alone cannot decide whether
the user has right to access or not..
lp_valid_users() and lp_xxx_users() affects access control.

User shares have no such list, so I think a security descriptor can work
for user shares. But for normal shares, we can not ignore lp_xx_users()
lists, unless we don't want it work. :-)

Should we ignore normal share's lp_xx_users() list at present?
>   
>> --- a/source3/smbd/process.c
>> +++ b/source3/smbd/process.c
>> @@ -1340,6 +1340,24 @@ static connection_struct *switch_message(uint8 type, struct smb_request *req, in
>>  			return NULL;
>>  		}
>>  
>> +		/* All NEED_WRITE and CAN_IPC flags must also have AS_USER. */
>> +
>> +		/* 
>> +		 * Does it need write permission? Do a share permission check
>> +		 * without cache to update connection struct
>> +		 */
>> +		if (flags & NEED_WRITE) {
>> +			if (!check_share_perm_without_cache(conn, session_tag)) {
>> +				DEBUG(4, ("Error: cannot access share!\n"));
>> +				reply_nterror(req, NT_STATUS_ACCESS_DENIED);
>> +				return conn;
>> +			}
>> +			if (!CAN_WRITE(conn)) {
>> +				reply_nterror(req, NT_STATUS_MEDIA_WRITE_PROTECTED);
>> +				return conn;
>> +			}
>> +		}
>> +
>>     
>
> Same here. We can't do this in the central event loop.
>   
Do you mean we cannot become_root()? or we cannot do checks here?
The commands with NEED_WRITE flag have to e aware of share permission,
which is the case in windows. I think you mean we cannot become_root() here?
> Volker
>   


-- 
Bo Yang, Software Engineer, Suse Labs
GPG-key-ID   538C4C1A
Samba Team   boyang at samba.org    http://www.samba.org/
SUSE Linux   boyang at suse.de      http://www.novell.com/