| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20260105172745.5cc67e79@kernel.org>
Date: Mon, 5 Jan 2026 17:27:45 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Rishikesh Jethwani <rjethwani@...estorage.com>
Cc: netdev@...r.kernel.org, andrew@...n.ch, saeedm@...dia.com,
tariqt@...dia.com, mbloch@...dia.com, borisp@...dia.com,
john.fastabend@...il.com, sd@...asysnail.net, davem@...emloft.net
Subject: Re: [PATCH v3 1/2] tls: TLS 1.3 hardware offload support
On Fri, 2 Jan 2026 11:47:07 -0700 Rishikesh Jethwani wrote:
> Add TLS 1.3 support to the kernel TLS hardware offload infrastructure,
> enabling hardware acceleration for TLS 1.3 connections on capable NICs.
>
> This patch implements the critical differences between TLS 1.2 and TLS 1.3
> record formats for hardware offload:
>
> TLS 1.2 record structure:
> [Header (5)] + [Explicit IV (8)] + [Ciphertext] + [Tag (16)]
>
> TLS 1.3 record structure:
> [Header (5)] + [Ciphertext + ContentType (1)] + [Tag (16)]
>
> Key changes:
> 1. Content type handling: In TLS 1.3, the content type byte is appended
> to the plaintext before encryption and tag computation. This byte must
> be encrypted along with the ciphertext to compute the correct
> authentication tag. Modified tls_device_record_close() to append
> the content type before the tag for TLS 1.3 records.
>
> 2. Version validation: Both tls_set_device_offload() and
> tls_set_device_offload_rx() now accept TLS_1_3_VERSION in addition
> to TLS_1_2_VERSION.
>
> 3. Pre-populate dummy_page with valid record types for memory
> allocation failure fallback path.
>
> Note: TLS 1.3 protocol parameters (aad_size, tail_size, prepend_size)
> are already handled by init_prot_info() in tls_sw.c.
I don't see you handling re-keying, which is supported in SW.
> Testing:
> Verified on Broadcom BCM957608 (Thor 2) and Mellanox ConnectX-6 Dx
> (Crypto Enabled) using ktls_test. Both TX and RX hardware offload working
> successfully with TLS 1.3 AES-GCM-128 and AES-GCM-256 cipher suites.
The kernel has come a long way in terms of HW testing since TLS was
added. We now require in-tree selftests for new capabilities. Some
relevant information here: https://github.com/linux-netdev/nipa/wiki
> The upstream Broadcom bnxt_en driver does not yet support kTLS offload.
> Testing was performed using the out-of-tree driver version
> bnxt_en-1.10.3-235.1.154.0, which works without modifications.
It's a bit odd to mention that you tested some out of tree code.
Glad it works, but I don't see the relevance upstream. Please drop
the mentions of Broadcom until the driver has TLS offload support added.
> Signed-off-by: Rishikesh Jethwani <rjethwani@...estorage.com>
> ---
> net/tls/tls_device.c | 49 ++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index 82ea407e520a..f57e96862b1c 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c
> @@ -319,6 +319,36 @@ static void tls_device_record_close(struct sock *sk,
> struct tls_prot_info *prot = &ctx->prot_info;
> struct page_frag dummy_tag_frag;
>
> + /* TLS 1.3: append content type byte before tag.
> + * Record structure: [Header (5)] + [Ciphertext + ContentType (1)] + [Tag (16)]
> + * The content type is encrypted with the ciphertext for authentication.
> + */
> + if (prot->version == TLS_1_3_VERSION) {
> + struct page_frag dummy_content_type_frag;
> + struct page_frag *content_type_pfrag = pfrag;
> +
> + /* Validate record type range */
> + if (unlikely(record_type < TLS_RECORD_TYPE_CHANGE_CIPHER_SPEC ||
> + record_type > TLS_RECORD_TYPE_ACK)) {
> + pr_err_once("tls_device: invalid record type %u\n",
> + record_type);
> + return;
This check is really odd. Why is it relevant, and yet not relevant
enough to handle cleanly? On a quick look it appears that the user
can set whatever content type they want.
> +
> + if (unlikely(pfrag->size - pfrag->offset < prot->tail_size) &&
> + !skb_page_frag_refill(prot->tail_size, pfrag, sk->sk_allocation)) {
> + /* Out of memory: use pre-populated dummy_page */
> + dummy_content_type_frag.page = dummy_page;
> + dummy_content_type_frag.offset = record_type;
> + content_type_pfrag = &dummy_content_type_frag;
> + } else {
> + /* Current pfrag has space or allocation succeeded - write content type */
> + *(unsigned char *)(page_address(pfrag->page) + pfrag->offset) =
> + record_type;
wrap at 80chars and please refactor this long line into something more
readable.
> + }
> + tls_append_frag(record, content_type_pfrag, prot->tail_size);
> + }
> +
> /* append tag
> * device will fill in the tag, we just need to append a placeholder
> * use socket memory to improve coalescing (re-using a single buffer
> @@ -335,7 +365,7 @@ static void tls_device_record_close(struct sock *sk,
>
> /* fill prepend */
> tls_fill_prepend(ctx, skb_frag_address(&record->frags[0]),
> - record->len - prot->overhead_size,
> + (record->len - prot->overhead_size) + prot->tail_size,
> record_type);
> }
>
> @@ -1089,7 +1119,8 @@ int tls_set_device_offload(struct sock *sk)
> }
>
> crypto_info = &ctx->crypto_send.info;
> - if (crypto_info->version != TLS_1_2_VERSION) {
> + if (crypto_info->version != TLS_1_2_VERSION &&
> + crypto_info->version != TLS_1_3_VERSION) {
> rc = -EOPNOTSUPP;
> goto release_netdev;
> }
Are all existing drivers rejecting TLS 1.3 sessions?
These days we prefer for drivers to explicitly opt into new features
to avoid surprises.
--
pw-bot: cr
Powered by blists - more mailing lists