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

Bharath SM <[email protected]> Thu, 16 Apr 2026 15:03:19 +0000
Newsgroups gmane.linux.kernel,gmane.linux.kernel.cifs,gmane.network.samba.internals
Message-ID <CAGypqWzW6YPfB2Z9nARHVj=qv8h6bcxShxxM=T1ceG9654ky8A@mail.gmail.com>
On Thu, Apr 9, 2026 at 9:16 AM <[email protected]> wrote:
>
> From: Piyush Sachdeva <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Piyush Sachdeva <[email protected]>
> ---
>  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.