Re: ksmbd: AES-GMAC signing support?

[Enzo Matsumiya <ematsumiya@suse.de>]

Hi Michail,

On 10/23, Michail Andreas Michalopoulos wrote:
>Hi Namjae,
>
>ksmbd currently negotiates HMAC-SHA256 and AES-CMAC for SMB 3.1.1
>signing, but not AES-GMAC (SIGNING_ALG_AES_GMAC is defined in
>fs/smb/common/smb2pdu.h but unused), which is what Windows 11 and
>Server 2022 clients prefer when it is offered.
>
>Would you be interested in patches adding it? Roughly:
>
> - accept AES-GMAC in the SMB2_SIGNING_CAPABILITIES negotiate context,
> - sign and verify with gcm(aes) over the message, using the nonce
>   from MS-SMB2 3.1.4.1 (MessageId plus the request/response and
>   CANCEL bits), and
> - keep HMAC-SHA256/AES-CMAC behaviour unchanged.
>
>I would test it with smbtorture's signing tests and the ksmbd CI
>tests, and check the existing signing paths against an older Windows
>server I have access to.
>
>I have exams until mid-November, so I would start on it after that,
>and as it is a bigger change it may take a few weeks. If someone
>needs it sooner, I'm happy to leave it to them.
>
>Before I start: is anyone already working on this, or is there
>anything you would like done differently?

I did this 4 years ago for cifs:

https://github.com/smfrench/smb3-kernel/commits/aes-gmac-v3/

I never sent the patches because my tests [0] showed that AES-CMAC was
slightly faster for cifs case.

After talking to Steve about it, our conclusion back then was that such
non-negligible overhead wasn't really worth the code change [1].

Feel free to use that code as a reference for your implementation.

I could certainly have gotten something wrong, but I didn't (don't)
really have the time to work on it again.


Cheers,

Enzo


[0] my tests were done with a user POV in mind, so everything was
measured in workload time rather than us/ms tracing.  Tested only
on common desktop HW too.

[1] AES-CMAC uses the shash crypto API, but AES-GMAC uses AEAD
encryption API (to only sign a payload you use an empty plaintext
buffer, and set the full payload to the associated data buffer).

This makes it require non-trivial changes other than simply loading
another shash alg.