From: Chris Leech <cleech@redhat.com>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Hannes Reinecke <hare@suse.de>,
linux-nvme@lists.infradead.org,
Chaitanya Kulkarni <kch@nvidia.com>,
Sagi Grimberg <sagi@grimberg.me>, Christoph Hellwig <hch@lst.de>,
linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
Ard Biesheuvel <ardb@kernel.org>,
"Jason A . Donenfeld" <Jason@zx2c4.com>,
Herbert Xu <herbert@gondor.apana.org.au>
Subject: Re: [PATCH 04/21] nvme-auth: common: add KUnit tests for TLS key derivation
Date: Mon, 2 Mar 2026 17:11:36 -0800 [thread overview]
Message-ID: <20260302-crewless-multiple-5bd14f68a73a@redhat.com> (raw)
In-Reply-To: <20260303002649.GE20209@quark>
Hi Eric, I'm reviewing your patches but just wanted to say thank you for
the details on this comment and respond to them quickly.
On Mon, Mar 02, 2026 at 04:26:49PM -0800, Eric Biggers wrote:
> On Mon, Mar 02, 2026 at 11:04:43AM +0100, Hannes Reinecke wrote:
> > Which discrepancies do you see between the specified algorithm
> > and the implementation?
>
> I'm looking at the latest NVM Express Base Specification, v2.3.
>
> First, there's the following:
>
> The host computes KS as the hash of the ephemeral DH key resulting
> from the combination of the random value y selected by the host with
> the DH exponential (i.e., gx mod p) received from the controller
> (i.e., KS = H((gx mod p)y mod p) = H(gxy mod p)).
>
> The actual code skips that step when deriving the PSK, and just
> considers the DH value directly to be "KS" and uses it directly as an
> HMAC key. That is something that should never be done. DH values are
> not uniformly distributed and must not be used directly as keys.
We might have an issue with the secure concatantion generated PSK here,
and a shortage of independant implementations to catch this in testing.
I'll take a closer look at it, but at a glance I think I agree.
> Second, the only mention of HKDF is in section 8.3.5.6.2. Assuming that
> corresponds to what was attempted to be implemented in
> nvme_auth_derive_tls_psk(), it does not match because (at least) the
> specified label does not match the one used in the code.
The AVE stuff in NVMe 8.3.5.6 is _not_ what nvme_auth_derive_tls_psk() is
doing. Most of the TLS handling is specific to TCP as a fabric
transport, and is in the seperate "NVM Express NVMe over TCP Transport
Specification". In this case, section 3.6.1.3 "TLS PSK and PSK Identity
Derivation".
I'm fairly certain that's sorted now, after some confusion caused by the NVMe
specs calling for HKDF-Expand-Label vs. HKDF-Expand.
- Chris
> Those are just a couple things I noticed in a very quick glance.
>
> (There's also the key reuse bug I pointed out before. But it sounds
> like that's a bug in the spec, not the code.)
>
> - Eric
>
next prev parent reply other threads:[~2026-03-03 1:11 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-02 7:59 [PATCH 00/21] nvme-auth: use crypto library for HMAC and hashing Eric Biggers
2026-03-02 7:59 ` [PATCH 01/21] nvme-auth: add NVME_AUTH_MAX_DIGEST_SIZE constant Eric Biggers
2026-03-02 9:44 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 02/21] nvme-auth: common: constify static data Eric Biggers
2026-03-02 9:45 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 03/21] nvme-auth: use proper argument types Eric Biggers
2026-03-02 9:45 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 04/21] nvme-auth: common: add KUnit tests for TLS key derivation Eric Biggers
2026-03-02 10:04 ` Hannes Reinecke
2026-03-03 0:26 ` Eric Biggers
2026-03-03 1:11 ` Chris Leech [this message]
2026-03-03 22:47 ` Chris Leech
2026-03-04 0:30 ` Eric Biggers
2026-03-02 7:59 ` [PATCH 05/21] nvme-auth: rename nvme_auth_generate_key() to nvme_auth_parse_key() Eric Biggers
2026-03-02 10:05 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 06/21] nvme-auth: common: explicitly verify psk_len == hash_len Eric Biggers
2026-03-02 10:05 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 07/21] nvme-auth: common: add HMAC helper functions Eric Biggers
2026-03-02 10:07 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 08/21] nvme-auth: common: use crypto library in nvme_auth_transform_key() Eric Biggers
2026-03-02 10:09 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 09/21] nvme-auth: common: use crypto library in nvme_auth_augmented_challenge() Eric Biggers
2026-03-02 10:10 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 10/21] nvme-auth: common: use crypto library in nvme_auth_generate_psk() Eric Biggers
2026-03-03 7:37 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 11/21] nvme-auth: common: use crypto library in nvme_auth_generate_digest() Eric Biggers
2026-03-03 7:38 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 12/21] nvme-auth: common: use crypto library in nvme_auth_derive_tls_psk() Eric Biggers
2026-03-03 7:40 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 13/21] nvme-auth: host: use crypto library in nvme_auth_dhchap_setup_host_response() Eric Biggers
2026-03-03 7:40 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 14/21] nvme-auth: host: use crypto library in nvme_auth_dhchap_setup_ctrl_response() Eric Biggers
2026-03-03 7:41 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 15/21] nvme-auth: host: remove allocation of crypto_shash Eric Biggers
2026-03-03 7:42 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 16/21] nvme-auth: target: remove obsolete crypto_has_shash() checks Eric Biggers
2026-03-03 7:43 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 17/21] nvme-auth: target: use crypto library in nvmet_auth_host_hash() Eric Biggers
2026-03-03 7:43 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 18/21] nvme-auth: target: use crypto library in nvmet_auth_ctrl_hash() Eric Biggers
2026-03-03 7:44 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 19/21] nvme-auth: common: remove nvme_auth_digest_name() Eric Biggers
2026-03-03 7:45 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 20/21] nvme-auth: common: remove selections of no-longer used crypto modules Eric Biggers
2026-03-03 7:45 ` Hannes Reinecke
2026-03-02 7:59 ` [PATCH 21/21] crypto: remove HKDF library Eric Biggers
2026-03-03 7:46 ` Hannes Reinecke
2026-03-02 15:06 ` [PATCH 00/21] nvme-auth: use crypto library for HMAC and hashing Ard Biesheuvel
2026-03-03 4:04 ` Chris Leech
2026-03-04 13:23 ` Christoph Hellwig
2026-03-05 19:31 ` Eric Biggers
2026-03-05 19:35 ` Keith Busch
2026-03-25 20:20 ` Eric Biggers
2026-03-25 21:09 ` Keith Busch
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260302-crewless-multiple-5bd14f68a73a@redhat.com \
--to=cleech@redhat.com \
--cc=Jason@zx2c4.com \
--cc=ardb@kernel.org \
--cc=ebiggers@kernel.org \
--cc=hare@suse.de \
--cc=hch@lst.de \
--cc=herbert@gondor.apana.org.au \
--cc=kch@nvidia.com \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=sagi@grimberg.me \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox