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@xxxxxxx> > > > 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
