netdev - Re: [PATCH v3 1/2] tls: TLS 1.3 hardware offload support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAKaoeS3Kps61Wbk+EtEpbijemj=_vpHK0w35BBGfMuDCHnXuFg@mail.gmail.com>
Date: Wed, 21 Jan 2026 14:17:49 -0800
From: Rishikesh Jethwani <rjethwani@...estorage.com>
To: Jakub Kicinski <kuba@...nel.org>
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

 in
> I don't see you handling re-keying, which is supported in SW.

Added hardware offload key update support in v4


> > 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.

Removed Broadcom bnxt_en out-of-tree driver mention in v4

> > --- 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.
>
Removed record_type check from tls_device_record_close() in v4

> > +
> > +             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.
>

Refactored in v4

> > +             }
> > +             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.

Yes, currently the following 3 drivers support tls offload and all of
them reject  TLS 1.3 sessions.
- drivers/net/ethernet/chelsio/inline_crypto/chtls
- drivers/net/ethernet/fungible/funeth/
- drivers/net/ethernet/mellanox/mlx5/core/en_accel/