netdev - Re: [PATCH net-next v8 1/2] net/tls: support setting the maximum payload size

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aP9YMiW9V7Dkhu_1@krikkit>
Date: Mon, 27 Oct 2025 12:32:02 +0100
From: Sabrina Dubroca <sd@...asysnail.net>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Wilfred Mallawa <wilfred.opensource@...il.com>, netdev@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-kselftest@...r.kernel.org,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Jonathan Corbet <corbet@....net>, Simon Horman <horms@...nel.org>,
	John Fastabend <john.fastabend@...il.com>,
	Shuah Khan <shuah@...nel.org>
Subject: Re: [PATCH net-next v8 1/2] net/tls: support setting the maximum
 payload size

2025-10-24, 16:33:36 -0700, Jakub Kicinski wrote:
> On Fri, 24 Oct 2025 12:11:11 +1000 Wilfred Mallawa wrote:
> > In the previous record_size_limit approach for TLS 1.3, we need to
> > account for the ContentType byte. Which complicates get/setsockopt()
> > and tls_get_info(), where in setsockopt() for TLS 1.3 we need to
> > subtract 1 to the user provided value and in getsockopt() we need add 1
> > to keep the symmetry between the two (similarly in tls_get_info()). The
> > underlying assumption was that userspace passes up directly what the
> > endpoint specified as the record_size_limit.
> > 
> > With this approach we don't need to worry about it and we can pass the
> > responsibility to user-space as documented, which I think makes the
> > kernel code simpler.
> 
> But we haven't managed to avoid that completely:
> 
> +	if (value < TLS_MIN_RECORD_SIZE_LIM - (tls_13 ? 1 : 0) ||

We could, by taking a smaller minimum payload size than what the RFC
says (anything that allows us to make progress, maybe 8B?). ie, I
don't think we have to be as strict as rfc8449 (leave the userspace
library in charge of rejecting bogus values during negotiation of this
extension).

> I understand the motivation, the kernel code is indeed simpler.

Also more consistent: the kernel syscalls work with record payload (at
the send()/recv() level). The rest is hidden. Userspace could try an
approximation by sending max_payload-sized chunks with MSG_EOR.

> Last night I read the RFC and then this patch, and it took me like
> 10min to get all of it straight in my head.

I don't find this stuff very clear either tbh, but maybe that's a
problem in the RFC itself.

>  Maybe I was tried but
> I feel like the user space developers will judge us harshly for 
> the current uAPI.

But userspace libraries have to do the same computations on their side
if they want to implement this RFC. They have to figure out what the
max payload size is as they're building the record, they can't just
chop off a bit at the end after filling it.

Quick grepping through gnutls got me to this:
https://gitlab.com/gnutls/gnutls/-/blob/eb3c9febfa9969792b8ac0ca56ee9fbd9b0bd7ee/lib/ext/record_size_limit.c#L104-106

So I have a slight preference for not being tied to a (kind of
confusing) RFC.

-- 
Sabrina