linux-kernel - Re: [PATCH v2] net/tls: support maximum record size limit

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250902114027.GD15473@horms.kernel.org>
Date: Tue, 2 Sep 2025 12:40:27 +0100
From: Simon Horman <horms@...nel.org>
To: Wilfred Mallawa <wilfred.opensource@...il.com>
Cc: "David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Jonathan Corbet <corbet@....net>,
	John Fastabend <john.fastabend@...il.com>, netdev@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	Alistair Francis <alistair.francis@....com>,
	Damien Le'Moal <dlemoal@...nel.org>,
	Wilfred Mallawa <wilfred.mallawa@....com>
Subject: Re: [PATCH v2] net/tls: support maximum record size limit

On Tue, Sep 02, 2025 at 01:38:10PM +1000, Wilfred Mallawa wrote:
> From: Wilfred Mallawa <wilfred.mallawa@....com>
> 
> 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@....com>

Hi Wilfred,

I'll leave review of this approach to others.
But in the meantime I wanted to pass on a minor problem I noticed in the code

> 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;
> +
>  	while (msg_data_left(msg)) {
>  		if (sk->sk_err) {
>  			ret = -sk->sk_err;

record_size_limit is set but otherwise unused.
Did you forget to add something here?

If not, please remove record_size_limit from this function.

-- 
pw-bot: changes-requested