Skip to content

linux: TLS PSK derivation fixes#1040

Merged
igaw merged 1 commit intolinux-nvme:masterfrom
cleech:tls-hkdf-expand-label
Jul 24, 2025
Merged

linux: TLS PSK derivation fixes#1040
igaw merged 1 commit intolinux-nvme:masterfrom
cleech:tls-hkdf-expand-label

Conversation

@cleech
Copy link
Contributor

@cleech cleech commented Jul 24, 2025

I was attempting to debug connecting the Linux driver / libnvme /
ktls-utils host stack to the SPDK nvmf_tgt over TLS, and ran into some
issues.

The TLS connection fails to complete a handshake because the TLS PSKs
are different. The NVMe/TCP specified key derivation steps from the
configured interchange format, to a retained PSK and finally the TLS
PSK, is implemented incompatibly in libnvme and SPDK. After some
investigation, I believe the SPDK implementation to be correct and am
providing a libnvme patch to match it. With libnvme modified, I see the
TLS handshake complete in tlshd.

There are issues with the Retained and TLS PSK derivations due to the
implementation not adhering to the RFC 8446 definition of the
HKDF-Expand-Label function.

  1) The 16-bit HkdfLabel.length value must be converted to network byte
     order.

  2) The variable length HkdfLabel.label and HkdfLabel.context vectors
     must be prefixed with a length byte.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Link: https://lore.kernel.org/r/20250721021718.1159879-2-cleech@redhat.com
Signed-off-by: Chris Leech <cleech@redhat.com>
@cleech cleech force-pushed the tls-hkdf-expand-label branch from 8f91307 to f3949db Compare July 24, 2025 17:20
@igaw igaw merged commit 3435511 into linux-nvme:master Jul 24, 2025
12 checks passed
@igaw
Copy link
Collaborator

igaw commented Jul 24, 2025

Thanks!

@hreinecke
Copy link
Collaborator

Bzzt. That patch is wrong. Setting the hkdf 'info' string in derived_retained_key() is now completely garbled.

@igaw
Copy link
Collaborator

igaw commented Jul 25, 2025

Alright, I'll revert it in this case.

edit: I am reverting it because I wanted to do a release today and this would not be so great to have it in the release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants