2025-09-02, 13:38:10 +1000, Wilfred Mallawa wrote: > From: Wilfred Mallawa <wilfred.mallawa@xxxxxxx> > > During a handshake, an endpoint may specify a maximum record size limit. > Currently, the kernel defaults to TLS_MAX_PAYLOAD_SIZE (16KB) for the > maximum record size. Meaning that, the outgoing records from the kernel > can exceed a lower size negotiated during the handshake. In such a case, > the TLS endpoint must send a fatal "record_overflow" alert [1], and > thus the record is discarded. > > Upcoming Western Digital NVMe-TCP hardware controllers implement TLS > support. For these devices, supporting TLS record size negotiation is > necessary because the maximum TLS record size supported by the controller > is less than the default 16KB currently used by the kernel. > > This patch adds support for retrieving the negotiated record size limit > during a handshake, and enforcing it at the TLS layer such that outgoing > records are no larger than the size negotiated. This patch depends on > the respective userspace support in tlshd and GnuTLS [2]. > > [1] https://www.rfc-editor.org/rfc/rfc8449 > [2] https://gitlab.com/gnutls/gnutls/-/merge_requests/2005 > > Signed-off-by: Wilfred Mallawa <wilfred.mallawa@xxxxxxx> > --- > Documentation/networking/tls.rst | 7 ++++++ > include/net/tls.h | 1 + > include/uapi/linux/tls.h | 2 ++ > net/tls/tls_main.c | 39 ++++++++++++++++++++++++++++++-- > net/tls/tls_sw.c | 4 ++++ > 5 files changed, 51 insertions(+), 2 deletions(-) A selftest would be nice (tools/testing/selftests/net/tls.c), but I'm not sure what we could do on the "RX" side to check that we are respecting the size restriction. Use a basic TCP socket and try to parse (and then discard without decrypting) records manually out of the stream and see if we got the length we wanted? > diff --git a/include/net/tls.h b/include/net/tls.h > index 857340338b69..c9a3759f27ca 100644 > --- a/include/net/tls.h > +++ b/include/net/tls.h > @@ -226,6 +226,7 @@ struct tls_context { > u8 rx_conf:3; > u8 zerocopy_sendfile:1; > u8 rx_no_pad:1; > + u16 record_size_limit; Maybe "tx_record_size_limit", since it's not intended for RX? I don't know if the kernel will ever have a need to enforce the RX record size, but it would maybe avoid future head-scratching "why is this not used on the RX path?" > diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c > index a3ccb3135e51..1098c01f2749 100644 > --- a/net/tls/tls_main.c > +++ b/net/tls/tls_main.c > @@ -812,6 +812,31 @@ static int do_tls_setsockopt_no_pad(struct sock *sk, sockptr_t optval, > return rc; > } > > +static int do_tls_setsockopt_record_size(struct sock *sk, sockptr_t optval, > + unsigned int optlen) > +{ > + struct tls_context *ctx = tls_get_ctx(sk); > + u16 value; > + > + if (sockptr_is_null(optval) || optlen != sizeof(value)) > + return -EINVAL; > + > + if (copy_from_sockptr(&value, optval, sizeof(value))) > + return -EFAULT; > + > + if (ctx->prot_info.version == TLS_1_2_VERSION && > + value > TLS_MAX_PAYLOAD_SIZE) > + return -EINVAL; > + > + if (ctx->prot_info.version == TLS_1_3_VERSION && > + value > TLS_MAX_PAYLOAD_SIZE + 1) > + return -EINVAL; > + > + ctx->record_size_limit = value; > + > + return 0; > +} > + > static int do_tls_setsockopt(struct sock *sk, int optname, sockptr_t optval, > unsigned int optlen) > { > @@ -833,6 +858,9 @@ static int do_tls_setsockopt(struct sock *sk, int optname, sockptr_t optval, > case TLS_RX_EXPECT_NO_PAD: > rc = do_tls_setsockopt_no_pad(sk, optval, optlen); > break; > + case TLS_TX_RECORD_SIZE_LIM: > + rc = do_tls_setsockopt_record_size(sk, optval, optlen); > + break; Adding the corresponding changes to do_tls_getsockopt would also be good. > diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c > index bac65d0d4e3e..9f9359f591d3 100644 > --- a/net/tls/tls_sw.c > +++ b/net/tls/tls_sw.c > @@ -1033,6 +1033,7 @@ static int tls_sw_sendmsg_locked(struct sock *sk, struct msghdr *msg, > unsigned char record_type = TLS_RECORD_TYPE_DATA; > bool is_kvec = iov_iter_is_kvec(&msg->msg_iter); > bool eor = !(msg->msg_flags & MSG_MORE); > + u16 record_size_limit; > size_t try_to_copy; > ssize_t copied = 0; > struct sk_msg *msg_pl, *msg_en; > @@ -1058,6 +1059,9 @@ static int tls_sw_sendmsg_locked(struct sock *sk, struct msghdr *msg, > } > } > > + record_size_limit = tls_ctx->record_size_limit ? > + tls_ctx->record_size_limit : TLS_MAX_PAYLOAD_SIZE; As Simon said (good catch Simon :)), this isn't used anywhere. Are you sure this patch works? The previous version had a hunk in tls_sw_sendmsg_locked that looks like what I would expect. And the the offloaded TX path (in net/tls/tls_device.c) would also need similar changes. I'm wondering if it's better to add this conditional, or just initialize record_size_limit to TLS_MAX_PAYLOAD_SIZE as we set up the tls_context. Then we only have to replace TLS_MAX_PAYLOAD_SIZE with tls_ctx->record_size_limit in a few places? -- Sabrina
