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

2026-08-06, 20:44:08 -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 async decrypt reaches EBADMSG through the
> recorded-failure check alone.
> 
> bad_auth builds the same corrupted record, so its construction moves
> into a helper the two tests share.
> 
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>  tools/testing/selftests/net/tls.c | 75 +++++++++++++++++++++++++++++++++------
>  1 file changed, 65 insertions(+), 10 deletions(-)

Reviewed-by: Sabrina Dubroca <[email protected]>


(just one nit:)

> +/* 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 understand what the function does, but it took me a while to parse
this comment. Now I see it applies more to the
"MSG_WAITALL/len/MSG_WAITALL" construct than to the overall function
(the function's name gives a good enough description of what it does).

> +static void tls_send_bad_auth(struct __test_metadata *_metadata,
> +			      int fd, int cfd, int fd2)
> +{
> +	char buf[128];
> +	int len;
> +
> +	memrnd(buf, sizeof(buf) / 2);
> +	ASSERT_EQ(send(fd, buf, sizeof(buf) / 2, 0), sizeof(buf) / 2);
> +
> +	ASSERT_EQ(recv(cfd, buf, TLS_HDR_LEN, MSG_WAITALL), TLS_HDR_LEN);
> +
> +	len = ((unsigned char)buf[3] << 8) | (unsigned char)buf[4];
> +	ASSERT_GT(len, 0);
> +	ASSERT_LE(len, (int)sizeof(buf) - TLS_HDR_LEN);
> +
> +	ASSERT_EQ(recv(cfd, buf + TLS_HDR_LEN, len, MSG_WAITALL), len);
> +
> +	buf[TLS_HDR_LEN + len - 1]++;
> +
> +	ASSERT_EQ(send(fd2, buf, TLS_HDR_LEN + len, 0), TLS_HDR_LEN + len);
> +}

-- 
Sabrina