[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aL4DkNijXKKx2LVY@krikkit>
Date: Mon, 8 Sep 2025 00:13:36 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Wilfred Mallawa <wilfred.mallawa@....com>
Cc: "corbet@....net" <corbet@....net>,
"dlemoal@...nel.org" <dlemoal@...nel.org>,
Alistair Francis <Alistair.Francis@....com>,
"davem@...emloft.net" <davem@...emloft.net>,
"john.fastabend@...il.com" <john.fastabend@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"kuba@...nel.org" <kuba@...nel.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"horms@...nel.org" <horms@...nel.org>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"pabeni@...hat.com" <pabeni@...hat.com>
Subject: Re: [PATCH v2] net/tls: support maximum record size limit
2025-09-04, 23:31:23 +0000, Wilfred Mallawa wrote:
> On Wed, 2025-09-03 at 10:21 +0200, Sabrina Dubroca wrote:
> > 2025-09-02, 22:50:53 +0000, Wilfred Mallawa wrote:
> > > On Tue, 2025-09-02 at 18:07 +0200, Sabrina Dubroca wrote:
> > > > 2025-09-02, 13:38:10 +1000, Wilfred Mallawa wrote:
> > > > > From: Wilfred Mallawa <wilfred.mallawa@....com>
> > > Hey Sabrina,
> > > > 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?
> > > >
> > > So far I have just been using an NVMe TCP Target with TLS enabled
> > > and
> > > checking that the targets RX record sizes are <= negotiated size in
> > > tls_rx_one_record(). I didn't check for this patch and the bug
> > > below
> > > got through...my bad!
> > >
> > > Is it possible to get the exact record length into the testing
> > > layer?
> >
> > Not really, unless we come up with some mechanism using probes. I
> > wouldn't go that route unless we don't have any other choice.
> >
> > > Wouldn't the socket just return N bytes received which doesn't
> > > necessarily correlate to a record size?
> >
> > Yes. That's why I suggested only using ktls on one side of the test,
> > and parsing the records out of the raw stream of bytes on the RX
> > side.
> >
> Ah okay I see.
> > Actually, control records don't get aggregated on read, so sending a
> > large non-data buffer should result in separate limit-sized reads.
> > But
> > this makes me wonder if this limit is supposed to apply to control
> > records, and how the userspace library/application is supposed to
> > deal
> > with the possible splitting of those records?
> >
> Good point, from the spec, "When the "record_size_limit" extension is
> negotiated, an endpoint MUST NOT generate a protected record with
> plaintext that is larger than the RecordSizeLimit value it receives
> from its peer. Unprotected messages are not subject to this limit." [1]
>
> From what I understand, as long as it in encrypted. It must respect the
> record size limit?
Yes, and the kernel will make sure to split all the data it sends over
records of the maximum acceptable length (currently
TLS_MAX_PAYLOAD_SIZE, with your patch tx_record_size_limit). The
question was more about what happens if userspace does a send(!DATA,
length > tx_record_size_limit). The kernel will happily split that
over N consecutive records of tx_record_size_limit (or fewer) bytes,
and the peer will receive N separate messages. But this could already
happen with a non-DATA record larger than TLS_MAX_PAYLOAD_SIZE, so
it's not really something we need to worry about here. It's a concern
for the userspace library (reconstructing the original message from
consecutive records read separately from the ktls socket). So, my
comment here was pretty much noise, sorry.
> In regards to user-space, do you mean for TX or RX? For TX, there
> shouldn't need to be any changes as record splitting occurs in the
> kernel. For RX, I am not too sure, but this patch shouldn't change
> anything for that case?
Yes, I'm talking about TX here.
--
Sabrina
Powered by blists - more mailing lists