updated ksmbd (cifsd)

[Stefan Metzmacher]

Am 15.12.20 um 03:28 schrieb Namjae Jeon via samba-technical:
>  
>> I just looked briefly, but I'm wondering about a few things:
>>
>> 1. The xattr's to store additional meta data are not compatible with
>>    Samba's way of storing things:
>>     https://git.samba.org/?p=samba.git;a=blob;f=librpc/idl/xattr.idl
>>
>>    In order to make it possible to use the same filesystem with both servers
>>    it would be great if the well established way used in Samba would be used
>>    as well.
> Yes, Maybe... I will check it after sending ksmbd to linux-next.
>  
>> 2. Why does smb2_set_info_sec() have fp->saccess |= FILE_SHARE_DELETE_LE; ?
> Because of smb2.acls.GENERIC failure.
> 
> TESTING FILE GENERIC BITS
> get the original sd
> Testing generic bits 0x00000000
> time: 2020-12-15 00:00:37.940992
> failure: GENERIC [
> (../../source4/torture/smb2/acls.c:439) Incorrect status NT_STATUS_SHARING_VIOLATION - should be NT_STATUS_OK
> 
> I really don't understand this test. This testcase expect that FILE_DELETE is set in response if desired access
> of smb2 open is FILE_MAXIMAL_ACCESS.
> I don't know why smb2 open should be allowed although FILE_SHARE_DELETE is not set in previous open in this test.
> Can you give me a hint ?

As far as I can see the test assumes the user has SeRestorePrivilege, with that
SEC_FLAG_MAXIMUM_ALLOWED will add FILE_DELETE,
see https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fsa/8ada5fbe-db4e-49fd-aef6-20d54b748e40

>> 3. Why does ksmbd_override_fsids() only reset cred->fs[g|u]id, but group_info
>>    is kept unchanged, I guess at least the groups array should be set to be empty.
> Yes, We need to handle the groups list. Will fix it.
> 
>> 4. What is work->saved_cred_level used for in ksmbd_override_fsids()?
>>    It seems to be unused and adds a lot of complexity.
> ksmbd_override_fsids could be called recursively.
> work->saved_cred_level prevents ksmbd from overriding fs[g|u]id again.

But that will always be on the same session/share combination?

>> 5. Documentation/filesystems/cifsd.rst and fs/cifsd/Kconfig still references https://github.com/cifsd-team/cifsd-tools
>>   instead of https://github.com/cifsd-team/ksmbd-tools
> Okay. Will update it.

Thanks!

>> 6. Why is SMB_SERVER_CHECK_CAP_NET_ADMIN an compile time option and why is it off by default?
>>    I think the behavior should be enforced without a switch.
> I can make it default yes. Can you explain more why it should be enforced ?

Why should an unprivileged user ever be able to start the server?
Wouldn't that be a massive security problem as that user would provide
the share definitions and users and controls what ksmbd_override_fsids() will use?

metze

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