Re: Add support for GCM256 encryption

[Stefan Metzmacher <metze@samba.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3952 bytes --]

Am 16.10.20 um 06:45 schrieb Steve French:
> Redid patch 5 (includes Aurelien's suggestion, fixes a typo and fixes
> a problem with vers=3.0 mounts) - attached.
> 
> On Thu, Oct 15, 2020 at 12:41 PM Steve French <smfrench@gmail.com> wrote:
>>
>> found another typo in patch 5 ccm instead of gcm - fixing it now
>>
>> On Thu, Oct 15, 2020 at 11:33 AM Steve French <smfrench@gmail.com> wrote:
>>>
>>> Good point.  Updated patches attached.  Also added a one line comment
>>> to smb2pdu.h mentioning why we don't request AES_256_CCM
>>>
>>>
>>> On Thu, Oct 15, 2020 at 3:49 AM Aurélien Aptel <aaptel@suse.com> wrote:
>>>>
>>>> Hi Steve,
>>>>
>>>> Patch 2:
>>>>
>>>>> From 3897b440fd14dfc7b2ad2b0a922302ea7705b5d9 Mon Sep 17 00:00:00 2001
>>>>> From: Steve French <stfrench@microsoft.com>
>>>>> Date: Wed, 14 Oct 2020 20:24:09 -0500
>>>>> Subject: [PATCH 2/5] smb3.1.1: add new module load parm enable_gcm_256
>>>>> --- a/fs/cifs/smb2pdu.h
>>>>> +++ b/fs/cifs/smb2pdu.h
>>>>> @@ -361,8 +361,9 @@ struct smb2_encryption_neg_context {
>>>>>       __le16  ContextType; /* 2 */
>>>>>       __le16  DataLength;
>>>>>       __le32  Reserved;
>>>>> -     __le16  CipherCount; /* AES-128-GCM and AES-128-CCM */
>>>>> -     __le16  Ciphers[2];
>>>>> +     /* CipherCount usally 2, but can be 3 when AES256-GCM enabled */
>>>>> +     __le16  CipherCount; /* AES128-GCM and AES128-CCM by defalt */
>>>>
>>>> Typo defalt => default
>>>>
>>>>> +     __le16  Ciphers[3];
>>>>>  } __packed;
>>>>>
>>>>>  /* See MS-SMB2 2.2.3.1.3 */
>>>>> --
>>>>> 2.25.1
>>>>>
>>>>
>>>> Patch 5:
>>>>
>>>>> From 314d7476e404c37acb77c3f9ecc142122e7afbfd Mon Sep 17 00:00:00 2001
>>>>> From: Steve French <stfrench@microsoft.com>
>>>>> Date: Fri, 11 Sep 2020 16:47:09 -0500
>>>>> Subject: [PATCH 5/5] smb3.1.1: set gcm256 when requested
>>>>>
>>>>> update code to set 32 byte key length and to set gcm256 when requested
>>>>> on mount.
>>>>>
>>>>> Signed-off-by: Steve French <stfrench@microsoft.com>
>>>>> ---
>>>>>  fs/cifs/smb2glob.h      |  1 +
>>>>>  fs/cifs/smb2ops.c       | 20 ++++++++++++--------
>>>>>  fs/cifs/smb2transport.c | 16 ++++++++--------
>>>>>  3 files changed, 21 insertions(+), 16 deletions(-)
>>>>>
>>>>> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
>>>>> index dd1edabec328..d8e74954d101 100644
>>>>> --- a/fs/cifs/smb2ops.c
>>>>> +++ b/fs/cifs/smb2ops.c
>>>>> @@ -3954,7 +3954,12 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
>>>>>
>>>>>       tfm = enc ? server->secmech.ccmaesencrypt :
>>>>>                                               server->secmech.ccmaesdecrypt;
>>>>> -     rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
>>>>> +
>>>>> +     if (require_gcm_256)
>>>>> +             rc = crypto_aead_setkey(tfm, key, SMB3_GCM256_CRYPTKEY_SIZE);
>>>>
>>>> Shouldn't the check be on server->cipher_type?
>>>>
>>>>> +     else
>>>>> +             rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
>>>>> +

You still just use u8 key[SMB3_SIGN_KEY_SIZE];

Shouldn't smb2_get_enc_key() get the buffer size and return the size actually used?

I also don't see where you setup the 32 byte encryption/decryption keys from
the authentication session key?

[MS-SMB2] 3.3.5.5.3 Handling GSS-API Authentication point 11.)
specifies that the full authentication session key should be used as key derivation key
for AES256 (NTLMSSP always returns 16 bytes, kerberos can return 16 or 32 bytes).
3.3.1.8 Per Session says the resulting keys should be 256-bit (32 bytes) for AES-256.

I don't see any of this in your patchset.

Did you actually tested this successful against a Windows Server?

Can you use 'git format-patch --stdout > patches.txt' and attach patches.txt
as inline text attachment? Or use git send-email ...
Individual randomly sorted non text/plain attachments are very hard to comment on
(at least for me).

metze


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]