Re: [PATCH] smb: client: use FullSessionKey for AES-256 encryption key derivation

[Piyush Sachdeva <s.piyush1024@gmail.com>]

Bharath SM <bharathsm.hsk@gmail.com> writes:

> On Thu, Apr 9, 2026 at 9:16 AM <s.piyush1024@gmail.com> wrote:
>>
>> From: Piyush Sachdeva <psachdeva@microsoft.com>
>>
>> When Kerberos authentication is used with AES-256 encryption (AES-256-CCM
>> or AES-256-GCM), the SMB3 encryption and decryption keys must be derived
>> using the full session key (Session.FullSessionKey) rather than just the
>> first 16 bytes (Session.SessionKey).
>>
>> Per MS-SMB2 section 3.2.5.3.1, when Connection.Dialect is "3.1.1" and
>> Connection.CipherId is AES-256-CCM or AES-256-GCM, Session.FullSessionKey
>> must be set to the full cryptographic key from the GSS authentication
>> context. The encryption and decryption key derivation (SMBC2SCipherKey,
>> SMBS2CCipherKey) must use this FullSessionKey as the KDF input. The
>> signing key derivation continues to use Session.SessionKey (first 16
>> bytes) in all cases.
>>
>> Previously, generate_key() hardcoded SMB2_NTLMV2_SESSKEY_SIZE (16) as the
>> HMAC-SHA256 key input length for all derivations. When Kerberos with
>> AES-256 provides a 32-byte session key, the KDF for encryption/decryption
>> was using only the first 16 bytes, producing keys that did not match the
>> server's, causing mount failures with sec=krb5 and require_gcm_256=1.
>>
>> Add a `full_key_size` parameter to generate_key() and pass the appropriate
>> size from generate_smb3signingkey():
>>  - Signing: always SMB2_NTLMV2_SESSKEY_SIZE (16 bytes)
>>  - Encryption/Decryption: ses->auth_key.len when AES-256, otherwise 16
>>
>> Also fix cifs_dump_full_key() to report the actual session key length for
>> AES-256 instead of hardcoded CIFS_SESS_KEY_SIZE, so that userspace tools
>> like Wireshark receive the correct key for decryption.
>>
>> Signed-off-by: Piyush Sachdeva <psachdeva@microsoft.com>
>> Signed-off-by: Piyush Sachdeva <s.piyush1024@gmail.com>
>> ---
>>  fs/smb/client/ioctl.c         |  2 +-
>>  fs/smb/client/smb2transport.c | 32 +++++++++++++++++++++++++-------
>>  2 files changed, 26 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/smb/client/ioctl.c b/fs/smb/client/ioctl.c
>> index 9afab3237e54..17408bb8ab65 100644
>> --- a/fs/smb/client/ioctl.c
>> +++ b/fs/smb/client/ioctl.c
>> @@ -296,7 +296,7 @@ static int cifs_dump_full_key(struct cifs_tcon *tcon, struct smb3_full_key_debug
>>                 break;
>>         case SMB2_ENCRYPTION_AES256_CCM:
>>         case SMB2_ENCRYPTION_AES256_GCM:
>> -               out.session_key_length = CIFS_SESS_KEY_SIZE;
>> +               out.session_key_length = ses->auth_key.len;
>>                 out.server_in_key_length = out.server_out_key_length = SMB3_GCM256_CRYPTKEY_SIZE;
>>                 break;
>>         default:
>> diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c
>> index 81be2b226e26..57e515774b97 100644
>> --- a/fs/smb/client/smb2transport.c
>> +++ b/fs/smb/client/smb2transport.c
>> @@ -259,7 +259,8 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
>>  }
>>
>>  static int generate_key(struct cifs_ses *ses, struct kvec label,
>> -                       struct kvec context, __u8 *key, unsigned int key_size)
>> +                       struct kvec context, __u8 *key, unsigned int key_size,
>> +                       unsigned int full_key_size)
>>  {
>>         unsigned char zero = 0x0;
>>         __u8 i[4] = {0, 0, 0, 1};
>> @@ -280,7 +281,7 @@ static int generate_key(struct cifs_ses *ses, struct kvec label,
>>         }
>>
>>         hmac_sha256_init_usingrawkey(&hmac_ctx, ses->auth_key.response,
>> -                                    SMB2_NTLMV2_SESSKEY_SIZE);
>> +                                    full_key_size);
>>         hmac_sha256_update(&hmac_ctx, i, 4);
>>         hmac_sha256_update(&hmac_ctx, label.iov_base, label.iov_len);
>>         hmac_sha256_update(&hmac_ctx, &zero, 1);
>> @@ -315,6 +316,7 @@ generate_smb3signingkey(struct cifs_ses *ses,
>>                         const struct derivation_triplet *ptriplet)
>>  {
>>         int rc;
>> +       unsigned int full_key_size;
>>         bool is_binding = false;
>>         int chan_index = 0;
>>
>> @@ -344,18 +346,32 @@ generate_smb3signingkey(struct cifs_ses *ses,
>>          * master connection signing key stored in the session
>>          */
>>
>> +       /*
>> +        * Per MS-SMB2 3.2.5.3.1, signing key always uses Session.SessionKey
>> +        * (first 16 bytes). Encryption/decryption keys use
>> +        * Session.FullSessionKey when dialect is 3.1.1 and cipher is
>> +        * AES-256-CCM or AES-256-GCM, otherwise Session.SessionKey.
>> +        */
> If this change is specific to 3.1.1 should we check for version as
> this looks like a common function for SMB 3.?
>
>>         if (is_binding) {
>>                 rc = generate_key(ses, ptriplet->signing.label,
>>                                   ptriplet->signing.context,
>>                                   ses->chans[chan_index].signkey,
>> -                                 SMB3_SIGN_KEY_SIZE);
>> +                                 SMB3_SIGN_KEY_SIZE,
>> +                                 SMB2_NTLMV2_SESSKEY_SIZE);
>>                 if (rc)
>>                         return rc;
>>         } else {
>> +               if (server->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
>> +                   server->cipher_type == SMB2_ENCRYPTION_AES256_GCM)
>> +                       full_key_size = ses->auth_key.len;
> Should we validate the length passed by user space and make sure it is
> within limits.?
>
>> +               else
>> +                       full_key_size = SMB2_NTLMV2_SESSKEY_SIZE;
> Should we move the above assignment down ? As this change is
> needed only for encryption and decryption and not for signing.
>
>>                 rc = generate_key(ses, ptriplet->signing.label,
>>                                   ptriplet->signing.context,
>>                                   ses->smb3signingkey,
>> -                                 SMB3_SIGN_KEY_SIZE);
>> +                                 SMB3_SIGN_KEY_SIZE,
>> +                                 SMB2_NTLMV2_SESSKEY_SIZE);
>>                 if (rc)
>>                         return rc;
>>
>> @@ -368,13 +384,15 @@ generate_smb3signingkey(struct cifs_ses *ses,
>>                 rc = generate_key(ses, ptriplet->encryption.label,
>>                                   ptriplet->encryption.context,
>>                                   ses->smb3encryptionkey,
>> -                                 SMB3_ENC_DEC_KEY_SIZE);
>> +                                 SMB3_ENC_DEC_KEY_SIZE,
>> +                                 full_key_size);
>>                 if (rc)
>>                         return rc;
>>                 rc = generate_key(ses, ptriplet->decryption.label,
>>                                   ptriplet->decryption.context,
>>                                   ses->smb3decryptionkey,
>> -                                 SMB3_ENC_DEC_KEY_SIZE);
>> +                                 SMB3_ENC_DEC_KEY_SIZE,
>> +                                 full_key_size);
>>                 if (rc)
>>                         return rc;
>>         }
>> @@ -389,7 +407,7 @@ generate_smb3signingkey(struct cifs_ses *ses,
>>                         &ses->Suid);
>>         cifs_dbg(VFS, "Cipher type   %d\n", server->cipher_type);
>>         cifs_dbg(VFS, "Session Key   %*ph\n",
>> -                SMB2_NTLMV2_SESSKEY_SIZE, ses->auth_key.response);
>> +                ses->auth_key.len, ses->auth_key.response);
>>         cifs_dbg(VFS, "Signing Key   %*ph\n",
>>                  SMB3_SIGN_KEY_SIZE, ses->smb3signingkey);
>>         if ((server->cipher_type == SMB2_ENCRYPTION_AES256_CCM) ||
>> --
>
> Other than the above comments, Patch looks good to me.

Hey Bharath,
Thanks for the comments. I will prepare a V2 with the recommended
changes, and post the updated patch.

--
Best,
Piyush