Re: [PATCH net 9/9] selftests: tls: cover splice after a failed decrypt

[Sabrina Dubroca <sd@queasysnail.net>]

2026-07-26, 20:33:37 -0400, Chuck Lever wrote:
> Nothing in this file splices a socket whose last decrypt failed, so
> the check that fails tls_sw_splice_read() on a broken connection can
> be removed without a test noticing. Such a splice hands the
> application plaintext that recvmsg() and read_sock() already refuse
> to return.
> 
> Extend the bad_auth pattern: corrupt an authenticated record, confirm
> recvmsg() reports EBADMSG, then splice the same socket and require
> EBADMSG again. A synchronous decrypt fails again on the still-queued
> record, so only an asynchronous decrypt, which needs TLS 1.2 and an
> AEAD advertising CRYPTO_ALG_ASYNC, reaches EBADMSG solely through the
> recorded-failure check.

async crypto is going away soon, I don't think it's worth mentioning
it. (but first I'm finishing the surgery to get rid of skmsg)

> bad_auth builds the same corrupted record, so its construction moves
> into a helper the two tests share.
> 
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
>  tools/testing/selftests/net/tls.c | 77 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 67 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
> index 7c178541edf4..8b68dbfcc592 100644
> --- a/tools/testing/selftests/net/tls.c
> +++ b/tools/testing/selftests/net/tls.c
> @@ -24,6 +24,7 @@
>  #include "kselftest_harness.h"
>  
>  #define TLS_PAYLOAD_MAX_LEN 16384
> +#define TLS_HDR_LEN 5
>  #define SOL_TLS 282
>  
>  static int fips_enabled;
> @@ -2883,28 +2884,85 @@ TEST_F(tls_err, bad_rec)
>  	EXPECT_EQ(errno, EAGAIN);
>  }
>  
> +/* Encrypt a record on the TX-only socket pair, corrupt its last
> + * byte, and hand the result to the socket under test. cfd carries a
> + * byte stream, so one recv() can return part of a record: take the
> + * fragment length from the record header and wait for the remainder.
> + */

I'm not a fan of adding a 5-line comment for every 10-line function.

[...]
> +TEST_F(tls_err, bad_auth_splice)
> +{
> +	char buf[128];
> +	ssize_t ret;
> +	int p[2];
> +
> +	if (self->notls)
> +		SKIP(return, "no TLS support");
> +
> +	tls_send_bad_auth(_metadata, self->fd, self->cfd, self->fd2);
> +
> +	EXPECT_EQ(recv(self->cfd2, buf, sizeof(buf), 0), -1);
> +	EXPECT_EQ(errno, EBADMSG);
> +
> +	ASSERT_GE(pipe(p), 0);
> +
> +	ret = splice(self->cfd2, NULL, p[1], NULL, sizeof(buf),
> +		     SPLICE_F_NONBLOCK);
> +	EXPECT_EQ(ret, -1);
> +	if (ret == -1)

nit: The rest of the tls tests just check errno when an error was
expected. If we didn't get -1, the errno check will probably fail too,
but at this point the test has already failed so IMO it's not worth
adding extra conditionals.

> +		EXPECT_EQ(errno, EBADMSG);
> +
> +	close(p[0]);
> +	close(p[1]);
> +}

-- 
Sabrina