[PATCH net] tls: refuse RX records after record-number exhaustion

[PATCH net] tls: refuse RX records after record-number exhaustion

[Rishikesh Jethwani]

A receive socket records sequence-number exhaustion only in sk_err.
After processing record UINT64_MAX, it advances the counter to zero and
reports EBADMSG, but SO_ERROR or sock_error() can clear that error. A
parser-ready record also bypasses the socket-error check in the record
wait. The receiver can then accept a record authenticated for sequence
zero under the same exhausted key.

Getting there needs an initial record number installed close to the
limit, since 2^64 records cannot be exchanged in practice, and a peer
that sends the wrapped record, which the Linux TX path refuses to do.

Mark the RX key exhausted when its record number wraps, and refuse to
wait for or decrypt another record under it, or to replace the key.
Plaintext that is already decrypted can still be read. RFC 8446 lets a
sender retire a key with a KeyUpdate as its last record, so in that
case leave sk_err alone, report the pending key update as usual and
let the new TLS_RX key clear the mark.

Add fixed AES-GCM wire vectors and TLS 1.2/1.3 selftests for queued
wrapped records, SO_ERROR clearing, splice, peek, partial reads, TLS
1.3 key replacement and a KeyUpdate in the last record. Every case
first verifies the penultimate record succeeds. The vectors avoid a
userspace crypto dependency and the TX path's refusal to generate the
maximum-number record.

Fixes: c46234ebb4d1 ("tls: RX path for ktls")
Assisted-by: LLM
Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
 Documentation/networking/tls.rst  |  17 ++
 include/net/tls.h                 |   2 +
 net/tls/tls.h                     |  19 ++-
 net/tls/tls_main.c                |   9 ++
 net/tls/tls_sw.c                  |  19 ++-
 tools/testing/selftests/net/tls.c | 258 +++++++++++++++++++++++++++++-
 6 files changed, 315 insertions(+), 9 deletions(-)

diff --git a/Documentation/networking/tls.rst b/Documentation/networking/tls.rst
index 980c442d7161..4b3c7a5a0780 100644
--- a/Documentation/networking/tls.rst
+++ b/Documentation/networking/tls.rst
@@ -202,6 +202,23 @@ received without a cmsg buffer set.
 
 recv will never return data from mixed types of TLS records.
 
+Receive record-number exhaustion
+--------------------------------
+
+The receive record number must not wrap under the same key. Once the
+record with the last 64-bit record number has been received, no further
+record is decrypted under that key: a receive that needs another record
+fails with ``EBADMSG``, even after ``SO_ERROR`` has been read. Plaintext
+that was already decrypted, such as the rest of a partially read or
+peeked final record, can still be read. This applies to both software
+and hardware receive offload.
+
+Unless the final record is a TLS 1.3 KeyUpdate, the condition is
+terminal: the socket error is also set to ``EBADMSG``, and a ``TLS_RX``
+key replacement fails with ``EBADMSG``. A KeyUpdate in the final record
+retires the key the usual way: receives fail with ``EKEYEXPIRED`` until a
+new ``TLS_RX`` key is installed, after which receiving continues.
+
 TLS 1.3 Key Updates
 -------------------
 
diff --git a/include/net/tls.h b/include/net/tls.h
index e57bef58851e..73f2d0f24e54 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -139,6 +139,8 @@ struct tls_sw_context_rx {
 	u8 zc_capable:1;
 	u8 reader_contended:1;
 	bool key_update_pending;
+	/* record numbers of this key are used up; cleared by a KeyUpdate */
+	bool rec_seq_exhausted;
 
 	struct tls_strparser strp;
 
diff --git a/net/tls/tls.h b/net/tls/tls.h
index 60a37bdaaa25..3b2af529d977 100644
--- a/net/tls/tls.h
+++ b/net/tls/tls.h
@@ -307,17 +307,26 @@ static inline void tls_bigint_subtract(unsigned char *seq, int  n)
 	*p = cpu_to_be64(rcd_sn - n);
 }
 
-static inline void
-tls_advance_record_sn(struct sock *sk, struct tls_prot_info *prot,
-		      struct cipher_context *ctx)
+/* Returns true if the record number wrapped */
+static inline bool
+__tls_advance_record_sn(struct tls_prot_info *prot, struct cipher_context *ctx)
 {
-	if (tls_bigint_increment(ctx->rec_seq, prot->rec_seq_size))
-		tls_err_abort(sk, -EBADMSG);
+	bool overflow = tls_bigint_increment(ctx->rec_seq, prot->rec_seq_size);
 
 	if (prot->version != TLS_1_3_VERSION &&
 	    prot->cipher_type != TLS_CIPHER_CHACHA20_POLY1305)
 		tls_bigint_increment(ctx->iv + prot->salt_size,
 				     prot->iv_size);
+
+	return overflow;
+}
+
+static inline void
+tls_advance_record_sn(struct sock *sk, struct tls_prot_info *prot,
+		      struct cipher_context *ctx)
+{
+	if (__tls_advance_record_sn(prot, ctx))
+		tls_err_abort(sk, -EBADMSG);
 }
 
 static inline void
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index fbb274287aa5..63bef0153182 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -730,6 +730,15 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
 			conf = TLS_SW;
 		}
 	} else {
+		struct tls_sw_context_rx *rx_ctx = tls_sw_ctx_rx(ctx);
+
+		/* Only a KeyUpdate may follow the last record number */
+		if (update && rx_ctx->rec_seq_exhausted &&
+		    !rx_ctx->key_update_pending) {
+			rc = -EBADMSG;
+			goto err_crypto_info;
+		}
+
 		rc = tls_set_device_offload_rx(sk, ctx);
 		conf = TLS_HW;
 		if (!rc) {
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index d1ad31986cf2..cd1590e1f20f 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1104,6 +1104,10 @@ tls_rx_rec_wait(struct sock *sk, bool nonblock, bool released, bool has_copied)
 	if (unlikely(ctx->key_update_pending))
 		return -EKEYEXPIRED;
 
+	/* no record can follow the last one under this key */
+	if (unlikely(ctx->rec_seq_exhausted))
+		return -EBADMSG;
+
 	timeo = sock_rcvtimeo(sk, nonblock);
 
 	while (!tls_strp_msg_ready(ctx)) {
@@ -1566,6 +1570,7 @@ static int tls_rx_one_record(struct sock *sk, struct msghdr *msg,
 {
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
 	struct tls_prot_info *prot = &tls_ctx->prot_info;
+	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
 	struct strp_msg *rxm;
 	int err;
 
@@ -1580,9 +1585,18 @@ static int tls_rx_one_record(struct sock *sk, struct msghdr *msg,
 	rxm = strp_msg(darg->skb);
 	rxm->offset += prot->prepend_size;
 	rxm->full_len -= prot->overhead_size;
-	tls_advance_record_sn(sk, prot, &tls_ctx->rx);
+	err = tls_check_pending_rekey(sk, tls_ctx, darg->skb);
+	if (__tls_advance_record_sn(prot, &tls_ctx->rx)) {
+		/* sk_err alone can't stop the next record, SO_ERROR clears
+		 * it. A KeyUpdate in the last record properly retires the
+		 * key, so don't treat that one as an error.
+		 */
+		ctx->rec_seq_exhausted = true;
+		if (!ctx->key_update_pending)
+			tls_err_abort(sk, -EBADMSG);
+	}
 
-	return tls_check_pending_rekey(sk, tls_ctx, darg->skb);
+	return err;
 }
 
 int decrypt_skb(struct sock *sk, struct scatterlist *sgout)
@@ -2518,6 +2532,7 @@ static void tls_finish_key_update(struct sock *sk, struct tls_context *tls_ctx)
 	struct tls_sw_context_rx *ctx = tls_ctx->priv_ctx_rx;
 
 	WRITE_ONCE(ctx->key_update_pending, false);
+	ctx->rec_seq_exhausted = false;
 	/* wake-up pre-existing poll() */
 	ctx->saved_data_ready(sk);
 }
diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index 9d3cd4fff062..f69c3a736b94 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -26,6 +26,7 @@
 #define TLS_PAYLOAD_MAX_LEN 16384
 #define TLS_HDR_LEN 5
 #define SOL_TLS 282
+#define TLS_RECORD_TYPE_HANDSHAKE      0x16
 
 static int fips_enabled;
 
@@ -300,6 +301,261 @@ TEST_F(tls_basic, recseq_wrap)
 	EXPECT_EQ(errno, EBADMSG);
 }
 
+/* Fixed AES-GCM records avoid a dependency on a userspace crypto library.
+ * Each authenticates "test", with zero key and static IV, at sequence
+ * numbers UINT64_MAX - 1, UINT64_MAX and zero. TLS 1.2 uses the record
+ * number as its explicit IV; TLS 1.3 appends inner content type 23.
+ * The last record must never be accepted after counter exhaustion.
+ * tls_rx_recseq_ku_record is a TLS 1.3 KeyUpdate at UINT64_MAX.
+ */
+static const char tls_rx_recseq_ku_record[] =
+	"\x17\x03\x03\x00\x16\x49\xd1\xc7\x79\x45\x74\xd7"
+	"\x12\xe8\xbf\xe4\xaa\x70\x17\x58\x60\x66\xa0\xf9"
+	"\x5b\xac\x6a";
+
+FIXTURE(tls_rx_recseq)
+{
+	int fd, cfd;
+	bool notls;
+};
+
+FIXTURE_VARIANT(tls_rx_recseq)
+{
+	uint16_t tls_version;
+	size_t record_len;
+	const char *records[3];
+};
+
+FIXTURE_VARIANT_ADD(tls_rx_recseq, 12_aes_gcm)
+{
+	.tls_version = TLS_1_2_VERSION,
+	.record_len = 33,
+	.records = {
+		"\x17\x03\x03\x00\x1c\xff\xff\xff\xff\xff\xff\xff"
+		"\xfe\x62\x3c\xf3\xee\x4d\x18\x45\xf7\x62\x62\x2c"
+		"\x01\x9a\xfc\x47\x3d\x82\xb0\x9f\x7f",
+		"\x17\x03\x03\x00\x1c\xff\xff\xff\xff\xff\xff\xff"
+		"\xff\x25\xb4\xb4\x0c\xf7\x4c\xef\x86\xa2\xe1\x2d"
+		"\xa6\x31\xb5\x64\xf6\x21\x81\x7f\xad",
+		"\x17\x03\x03\x00\x1c\x00\x00\x00\x00\x00\x00\x00"
+		"\x00\x77\xed\xa9\xba\xbb\x29\xb7\x1a\x44\x35\x46"
+		"\x0f\x55\x94\x07\xdd\xc3\xe4\x91\x80",
+	},
+};
+
+FIXTURE_VARIANT_ADD(tls_rx_recseq, 13_aes_gcm)
+{
+	.tls_version = TLS_1_3_VERSION,
+	.record_len = 26,
+	.records = {
+		"\x17\x03\x03\x00\x15\x62\x3c\xf3\xee\xd7\x87\xd1"
+		"\x21\x46\x96\xc2\xfc\x59\x1c\x4c\x15\xec\x57\x6d"
+		"\xc8\x0a",
+		"\x17\x03\x03\x00\x15\x25\xb4\xb4\x0c\x52\x2e\x4d"
+		"\xcd\xf4\xec\xc2\x7e\xb8\x85\xed\xb7\x79\xf8\x39"
+		"\x68\xd6",
+		"\x17\x03\x03\x00\x15\x77\xed\xa9\xba\x77\x59\x33"
+		"\x88\x89\x13\x8d\x3c\xb4\x2e\xe0\x64\x72\xa5\x17"
+		"\x71\x45",
+	},
+};
+
+FIXTURE_SETUP(tls_rx_recseq)
+{
+	struct timeval timeout = { .tv_sec = 2 };
+	struct tls_crypto_info_keys key;
+	socklen_t len;
+	char buf[4];
+	int err;
+
+	ulp_sock_pair(_metadata, &self->fd, &self->cfd, &self->notls);
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	ASSERT_EQ(setsockopt(self->cfd, SOL_SOCKET, SO_RCVTIMEO,
+			     &timeout, sizeof(timeout)), 0);
+	tls_crypto_info_init(variant->tls_version, TLS_CIPHER_AES_GCM_128,
+			     &key, 0);
+	memset(key.aes128.rec_seq, 0xff, sizeof(key.aes128.rec_seq));
+	key.aes128.rec_seq[sizeof(key.aes128.rec_seq) - 1]--;
+	ASSERT_EQ(setsockopt(self->cfd, SOL_TLS, TLS_RX, &key, key.len), 0);
+
+	/* The penultimate record must still advance normally. */
+	ASSERT_EQ(send(self->fd, variant->records[0], variant->record_len, 0),
+		  variant->record_len);
+	ASSERT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_WAITALL), sizeof(buf));
+	ASSERT_EQ(memcmp(buf, "test", sizeof(buf)), 0);
+	len = sizeof(err);
+	ASSERT_EQ(getsockopt(self->cfd, SOL_SOCKET, SO_ERROR, &err, &len), 0);
+	ASSERT_EQ(err, 0);
+};
+
+FIXTURE_TEARDOWN(tls_rx_recseq)
+{
+	close(self->fd);
+	close(self->cfd);
+};
+
+static void tls_clear_rx_error(struct __test_metadata *_metadata, int fd)
+{
+	socklen_t len;
+	int err;
+
+	len = sizeof(err);
+	ASSERT_EQ(getsockopt(fd, SOL_SOCKET, SO_ERROR, &err, &len), 0);
+	ASSERT_EQ(err, EBADMSG);
+	len = sizeof(err);
+	ASSERT_EQ(getsockopt(fd, SOL_SOCKET, SO_ERROR, &err, &len), 0);
+	ASSERT_EQ(err, 0);
+}
+
+TEST_F(tls_rx_recseq, queued)
+{
+	char records[80], buf[8];
+
+	memcpy(records, variant->records[1], variant->record_len);
+	memcpy(records + variant->record_len, variant->records[2],
+	       variant->record_len);
+	ASSERT_EQ(send(self->fd, records, 2 * variant->record_len, 0),
+		  2 * variant->record_len);
+
+	/* Both records are queued, but only the final valid one can pass. */
+	ASSERT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_WAITALL), 4);
+	EXPECT_EQ(memcmp(buf, "test", 4), 0);
+	tls_clear_rx_error(_metadata, self->cfd);
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT), -1);
+	EXPECT_EQ(errno, EBADMSG);
+}
+
+TEST_F(tls_rx_recseq, so_error)
+{
+	char buf[4];
+
+	ASSERT_EQ(send(self->fd, variant->records[1], variant->record_len, 0),
+		  variant->record_len);
+	ASSERT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_WAITALL), sizeof(buf));
+	tls_clear_rx_error(_metadata, self->cfd);
+
+	ASSERT_EQ(send(self->fd, variant->records[2], variant->record_len, 0),
+		  variant->record_len);
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), 0), -1);
+	EXPECT_EQ(errno, EBADMSG);
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT), -1);
+	EXPECT_EQ(errno, EBADMSG);
+}
+
+TEST_F(tls_rx_recseq, splice)
+{
+	char buf[4];
+	int p[2];
+
+	ASSERT_EQ(send(self->fd, variant->records[1], variant->record_len, 0),
+		  variant->record_len);
+	ASSERT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_WAITALL), sizeof(buf));
+	tls_clear_rx_error(_metadata, self->cfd);
+
+	ASSERT_EQ(send(self->fd, variant->records[2], variant->record_len, 0),
+		  variant->record_len);
+	ASSERT_EQ(pipe(p), 0);
+	EXPECT_EQ(splice(self->cfd, NULL, p[1], NULL, sizeof(buf), 0), -1);
+	EXPECT_EQ(errno, EBADMSG);
+	close(p[0]);
+	close(p[1]);
+}
+
+TEST_F(tls_rx_recseq, peek)
+{
+	char buf[4];
+
+	ASSERT_EQ(send(self->fd, variant->records[1], variant->record_len, 0),
+		  variant->record_len);
+	ASSERT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_PEEK), sizeof(buf));
+	tls_clear_rx_error(_metadata, self->cfd);
+
+	/* Plaintext decrypted before exhaustion can still be read. */
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT), sizeof(buf));
+	EXPECT_EQ(memcmp(buf, "test", sizeof(buf)), 0);
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT), -1);
+	EXPECT_EQ(errno, EBADMSG);
+}
+
+TEST_F(tls_rx_recseq, partial_read)
+{
+	char buf[4];
+
+	ASSERT_EQ(send(self->fd, variant->records[1], variant->record_len, 0),
+		  variant->record_len);
+	ASSERT_EQ(recv(self->cfd, buf, 1, 0), 1);
+	tls_clear_rx_error(_metadata, self->cfd);
+
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT), 3);
+	EXPECT_EQ(memcmp(buf, "est", 3), 0);
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT), -1);
+	EXPECT_EQ(errno, EBADMSG);
+}
+
+/* Only a KeyUpdate may introduce a key after the last record number. */
+TEST_F(tls_rx_recseq, rekey)
+{
+	struct tls_crypto_info_keys key;
+	struct tls12_crypto_info_aes_gcm_128 before, after;
+	socklen_t len;
+	char buf[4];
+
+	if (variant->tls_version != TLS_1_3_VERSION)
+		SKIP(return, "rekey requires TLS 1.3");
+
+	ASSERT_EQ(send(self->fd, variant->records[1], variant->record_len, 0),
+		  variant->record_len);
+	ASSERT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_WAITALL), sizeof(buf));
+	tls_clear_rx_error(_metadata, self->cfd);
+
+	len = sizeof(before);
+	ASSERT_EQ(getsockopt(self->cfd, SOL_TLS, TLS_RX, &before, &len), 0);
+	tls_crypto_info_init(variant->tls_version, TLS_CIPHER_AES_GCM_128,
+			     &key, 1);
+	EXPECT_EQ(setsockopt(self->cfd, SOL_TLS, TLS_RX, &key, key.len), -1);
+	EXPECT_EQ(errno, EBADMSG);
+	len = sizeof(after);
+	ASSERT_EQ(getsockopt(self->cfd, SOL_TLS, TLS_RX, &after, &len), 0);
+	EXPECT_EQ(memcmp(&before, &after, sizeof(before)), 0);
+}
+
+/* A KeyUpdate in the last record retires the key without an error. */
+TEST_F(tls_rx_recseq, keyupdate)
+{
+	size_t len = sizeof(tls_rx_recseq_ku_record) - 1;
+	struct tls_crypto_info_keys key;
+	socklen_t optlen;
+	char buf[16];
+	int err;
+
+	if (variant->tls_version != TLS_1_3_VERSION)
+		SKIP(return, "KeyUpdate requires TLS 1.3");
+
+	ASSERT_EQ(send(self->fd, tls_rx_recseq_ku_record, len, 0), len);
+	EXPECT_EQ(tls_recv_cmsg(_metadata, self->cfd, TLS_RECORD_TYPE_HANDSHAKE,
+				buf, sizeof(buf), 0), 5);
+	EXPECT_EQ(memcmp(buf, "\x18\x00\x00\x01\x00", 5), 0);
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT), -1);
+	EXPECT_EQ(errno, EKEYEXPIRED);
+	optlen = sizeof(err);
+	ASSERT_EQ(getsockopt(self->cfd, SOL_SOCKET, SO_ERROR, &err, &optlen), 0);
+	EXPECT_EQ(err, 0);
+
+	/* The new key resets the record number, so receiving continues. */
+	tls_crypto_info_init(variant->tls_version, TLS_CIPHER_AES_GCM_128,
+			     &key, 1);
+	ASSERT_EQ(setsockopt(self->cfd, SOL_TLS, TLS_RX, &key, key.len), 0);
+	ASSERT_EQ(setsockopt(self->fd, SOL_TLS, TLS_TX, &key, key.len), 0);
+	ASSERT_EQ(send(self->fd, "test", 4, 0), 4);
+	ASSERT_EQ(recv(self->cfd, buf, 4, MSG_WAITALL), 4);
+	EXPECT_EQ(memcmp(buf, "test", 4), 0);
+	optlen = sizeof(err);
+	ASSERT_EQ(getsockopt(self->cfd, SOL_SOCKET, SO_ERROR, &err, &optlen), 0);
+	EXPECT_EQ(err, 0);
+}
+
 FIXTURE(tls)
 {
 	int fd, cfd;
@@ -1839,8 +2095,6 @@ TEST_F(tls, recv_efault)
 		EXPECT_EQ(memcmp(rec2, recv_mem + 9, ret - 9), 0);
 }
 
-#define TLS_RECORD_TYPE_HANDSHAKE      0x16
-
 TEST_F(tls_basic, recvmsg_nopad_retry_iov)
 {
 	char payload[32];

base-commit: 8e2fbe0e75d38690d551456658194c632933cb57
-- 
2.50.1

Re: [PATCH net] tls: refuse RX records after record-number exhaustion

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] tls: refuse RX records after record-number exhaustion

[Sabrina Dubroca]

2026-10-12, 18:27:01 -0600, Rishikesh Jethwani wrote:
> A receive socket records sequence-number exhaustion only in sk_err.
> After processing record UINT64_MAX, it advances the counter to zero and
> reports EBADMSG, but SO_ERROR or sock_error() can clear that error.

I vaguely remember a discussion around making some fatal errors more
permanent (ie not use sk_err).

> parser-ready record also bypasses the socket-error check in the record
> wait. The receiver can then accept a record authenticated for sequence
> zero under the same exhausted key.

I guess, but what's the problem with wrapping as long as we don't
reuse the same record number?

> Getting there needs an initial record number installed close to the
> limit, since 2^64 records cannot be exchanged in practice, and a peer
> that sends the wrapped record, which the Linux TX path refuses to do.
> 
> Mark the RX key exhausted when its record number wraps, and refuse to
> wait for or decrypt another record under it, or to replace the key.
> Plaintext that is already decrypted can still be read. RFC 8446 lets a
> sender retire a key with a KeyUpdate as its last record, so in that
> case leave sk_err alone, report the pending key update as usual and
> let the new TLS_RX key clear the mark.
> 
> Add fixed AES-GCM wire vectors and TLS 1.2/1.3 selftests for queued
> wrapped records, SO_ERROR clearing, splice, peek, partial reads, TLS
> 1.3 key replacement and a KeyUpdate in the last record. Every case
> first verifies the penultimate record succeeds. The vectors avoid a
> userspace crypto dependency and the TX path's refusal to generate the
> maximum-number record.

For 1.3, just do a "silent rekey" (change the key but don't send a
keyupdate) on the TX side to record number 0 with the same key? And it
should be enough to test this with just 1.3.

> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index fbb274287aa5..63bef0153182 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -730,6 +730,15 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
>  			conf = TLS_SW;
>  		}
>  	} else {
> +		struct tls_sw_context_rx *rx_ctx = tls_sw_ctx_rx(ctx);
> +
> +		/* Only a KeyUpdate may follow the last record number */
> +		if (update && rx_ctx->rec_seq_exhausted &&
> +		    !rx_ctx->key_update_pending) {

I'm really confused by this.

> +			rc = -EBADMSG;
> +			goto err_crypto_info;
> +		}
> +

-- 
Sabrina

Re: [PATCH net] tls: refuse RX records after record-number exhaustion

[Rishikesh Jethwani]

On Tue, Oct 13, 2026 at 2:38 AM Sabrina Dubroca <sd@queasysnail.net> wrote:
>
> 2026-10-12, 18:27:01 -0600, Rishikesh Jethwani wrote:
> > A receive socket records sequence-number exhaustion only in sk_err.
> > After processing record UINT64_MAX, it advances the counter to zero and
> > reports EBADMSG, but SO_ERROR or sock_error() can clear that error.
>
> I vaguely remember a discussion around making some fatal errors more
> permanent (ie not use sk_err).

Do you have a pointer to that thread? If there's a preferred way to make
these errors sticky, I'd rather use it than add a one-off flag.

> > parser-ready record also bypasses the socket-error check in the record
> > wait. The receiver can then accept a record authenticated for sequence
> > zero under the same exhausted key.
>
> I guess, but what's the problem with wrapping as long as we don't
> reuse the same record number?

You're right, in practice there isn't one. The only way to get there is
for userspace to install a rec_seq close to the limit, so sequence 0 was
never used under that key and nothing can be replayed. Starting there
already breaks RFC 8446 5.3, which requires the first record under a
key to use sequence number 0.

What's left is consistency. Today the wrap gives at most one EBADMSG,
and none if the next record has already been parsed. After that recv
continues at record 0, which is neither rekeying nor terminating. TX
behaves the same way: the UINT64_MAX record is queued, and it goes out
once SO_ERROR is cleared.

So I'll drop the security framing and the Fixes tag. I can either drop
the patch, or send a minimal version for net/net-next that makes the wrap
error stick on both RX and TX, without touching the rekey path. Which
would you prefer?

> > Getting there needs an initial record number installed close to the
> > limit, since 2^64 records cannot be exchanged in practice, and a peer
> > that sends the wrapped record, which the Linux TX path refuses to do.
> >
> > Mark the RX key exhausted when its record number wraps, and refuse to
> > wait for or decrypt another record under it, or to replace the key.
> > Plaintext that is already decrypted can still be read. RFC 8446 lets a
> > sender retire a key with a KeyUpdate as its last record, so in that
> > case leave sk_err alone, report the pending key update as usual and
> > let the new TLS_RX key clear the mark.
> >
> > Add fixed AES-GCM wire vectors and TLS 1.2/1.3 selftests for queued
> > wrapped records, SO_ERROR clearing, splice, peek, partial reads, TLS
> > 1.3 key replacement and a KeyUpdate in the last record. Every case
> > first verifies the penultimate record succeeds. The vectors avoid a
> > userspace crypto dependency and the TX path's refusal to generate the
> > maximum-number record.
>
> For 1.3, just do a "silent rekey" (change the key but don't send a
> keyupdate) on the TX side to record number 0 with the same key? And it
> should be enough to test this with just 1.3.

Agreed, that gets rid of the fixed vectors. Since 1.3 only uses the
record number in the nonce, TX can also produce the UINT64_MAX record
cleanly, using rec_seq 0 and the IV XORed with UINT64_MAX. If the patch
stays, the test will be 1.3-only and built that way.

> > diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> > index fbb274287aa5..63bef0153182 100644
> > --- a/net/tls/tls_main.c
> > +++ b/net/tls/tls_main.c
> > @@ -730,6 +730,15 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
> >                       conf = TLS_SW;
> >               }
> >       } else {
> > +             struct tls_sw_context_rx *rx_ctx = tls_sw_ctx_rx(ctx);
> > +
> > +             /* Only a KeyUpdate may follow the last record number */
> > +             if (update && rx_ctx->rec_seq_exhausted &&
> > +                 !rx_ctx->key_update_pending) {
>
> I'm really confused by this.

It refused a TLS_RX rekey after a
a wrap that didn't include a KeyUpdate. A new key is harmless, as you
say, so I'll remove it.

> > +                     rc = -EBADMSG;
> > +                     goto err_crypto_info;
> > +             }
> > +
>
> --
> Sabrina

Re: [PATCH net] tls: refuse RX records after record-number exhaustion

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 1 · High: 0 · Medium: 2 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Critical] tls: AES-GCM IV reuse on TX sequence number exhaustion via
  SO_ERROR bypass
- [Medium] The new KeyUpdate exemption in tls_rx_one_record() decides
  whether the final record 'retires the key' by checking only…
- [Medium] The new rule that blocks replacing an exhausted key ('Only a
  KeyUpdate may follow the last record number') depends on…
- [Low] The new section in Documentation/networking/tls.rst says that
  after receive record-number exhaustion (unless the final record is a…

--

tls: refuse RX records after record-number exhaustion

This patch adds a per-key rec_seq_exhausted flag. The flag is set when
the RX record number wraps. Once it is set, tls_rx_rec_wait() refuses to
wait for or decrypt more records, and do_tls_setsockopt_conf() refuses
to replace the key, unless the final record was a KeyUpdate. It also
documents the behaviour and adds AES-GCM wire-vector selftests for TLS
1.2 and 1.3.

> diff --git a/Documentation/networking/tls.rst b/Documentation/networking/tls.rst
> index 980c442d7161a..4b3c7a5a07806 100644
> --- a/Documentation/networking/tls.rst
> +++ b/Documentation/networking/tls.rst

[ ... ]

> +Unless the final record is a TLS 1.3 KeyUpdate, the condition is
> +terminal: the socket error is also set to ``EBADMSG``, and a ``TLS_RX``
> +key replacement fails with ``EBADMSG``. A KeyUpdate in the final record
> +retires the key the usual way: receives fail with ``EKEYEXPIRED`` until a
> +new ``TLS_RX`` key is installed, after which receiving continues.

[Severity: Low]
Is the TLS_RX replacement errno accurate for TLS 1.2 sockets?

On TLS 1.2, do_tls_setsockopt_conf() returns -EBUSY for any second
TLS_RX setsockopt. That happens before the new exhaustion check runs:

net/tls/tls_main.c:do_tls_setsockopt_conf() {
    ...
	if (TLS_CRYPTO_INFO_READY(crypto_info)) {
		if (crypto_info->version != TLS_1_3_VERSION) {
			...
			return -EBUSY;
		}

		update = true;
    ...
}

The new EBADMSG check needs update to be true, and that only happens for
TLS 1.3. The rekey selftest skips the TLS 1.2 variant for this reason.

Could the documentation limit the EBADMSG statement to TLS 1.3, or say
that TLS 1.2 returns EBUSY?

> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index fbb274287aa5f..63bef0153182b 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -730,6 +730,15 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
>  			conf = TLS_SW;
>  		}
>  	} else {
> +		struct tls_sw_context_rx *rx_ctx = tls_sw_ctx_rx(ctx);
> +
> +		/* Only a KeyUpdate may follow the last record number */
> +		if (update && rx_ctx->rec_seq_exhausted &&
> +		    !rx_ctx->key_update_pending) {
> +			rc = -EBADMSG;
> +			goto err_crypto_info;
> +		}

[Severity: Medium]
Can this check reject a valid KeyUpdate?

key_update_pending is set only when the first handshake byte of the
record is TLS_HANDSHAKE_KEYUPDATE:

net/tls/tls_sw.c:tls_check_pending_rekey() {
    ...
	err = skb_copy_bits(skb, rxm->offset, &hs_type, 1);
    ...
	if (hs_type == TLS_HANDSHAKE_KEYUPDATE) {
    ...
}

RFC 8446 section 5.1 allows several handshake messages in one record, as
long as the messages before a key change end at a record boundary. So a
KeyUpdate can be the last message in a record that starts with something
else, for example a NewSessionTicket.

Say the record at UINT64_MAX is such a coalesced record:

- key_update_pending stays false.
- tls_rx_one_record() sets rec_seq_exhausted and calls
  tls_err_abort(sk, -EBADMSG).
- Userspace parses the KeyUpdate from the handshake record and calls
  setsockopt(TLS_RX).
- This check returns -EBADMSG, so the connection cannot continue.

Before this patch, userspace could still install the new key after
reading SO_ERROR.

The commit message says "RFC 8446 lets a sender retire a key with a
KeyUpdate as its last record". Should the coalesced case be handled
too?

> +
>  		rc = tls_set_device_offload_rx(sk, ctx);
>  		conf = TLS_HW;
>  		if (!rc) {
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index d1ad31986cf2c..cd1590e1f20fa 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c

[ ... ]

> @@ -1580,9 +1585,18 @@ static int tls_rx_one_record(struct sock *sk, struct msghdr *msg,
>  	rxm = strp_msg(darg->skb);
>  	rxm->offset += prot->prepend_size;
>  	rxm->full_len -= prot->overhead_size;
> -	tls_advance_record_sn(sk, prot, &tls_ctx->rx);
> +	err = tls_check_pending_rekey(sk, tls_ctx, darg->skb);
> +	if (__tls_advance_record_sn(prot, &tls_ctx->rx)) {
> +		/* sk_err alone can't stop the next record, SO_ERROR clears
> +		 * it. A KeyUpdate in the last record properly retires the
> +		 * key, so don't treat that one as an error.
> +		 */
> +		ctx->rec_seq_exhausted = true;
> +		if (!ctx->key_update_pending)
> +			tls_err_abort(sk, -EBADMSG);

[Severity: Medium]
Should this exemption also check prot->version == TLS_1_3_VERSION?

tls_check_pending_rekey() sets key_update_pending for any handshake
record whose first plaintext byte is TLS_HANDSHAKE_KEYUPDATE. It does not
check the protocol version:

net/tls/tls_sw.c:tls_check_pending_rekey() {
    ...
	if (hs_type == TLS_HANDSHAKE_KEYUPDATE) {
		struct tls_sw_context_rx *rx_ctx = ctx->priv_ctx_rx;

		WRITE_ONCE(rx_ctx->key_update_pending, true);
    ...
}

Say a TLS 1.2 socket receives, at UINT64_MAX, a handshake record that
starts with 0x18:

- rec_seq_exhausted is set, but tls_err_abort() is skipped.
- sk_err stays zero, so poll() does not report EPOLLERR and SO_ERROR
  returns 0.
- tls_rx_rec_wait() checks key_update_pending before rec_seq_exhausted,
  so every later recv returns EKEYEXPIRED.
- setsockopt(TLS_RX) returns EBUSY, because TLS 1.2 cannot rekey.

Such a record already left a TLS 1.2 socket stuck in EKEYEXPIRED before
this patch. However, the wrap still set sk_err to EBADMSG then.

The new documentation says "Unless the final record is a TLS 1.3
KeyUpdate, the condition is terminal: the socket error is also set to
EBADMSG". Is the TLS 1.2 behaviour here meant to differ from that?

> +	}
>  
> -	return tls_check_pending_rekey(sk, tls_ctx, darg->skb);
> +	return err;
>  }

[ ... ]

> diff --git a/net/tls/tls.h b/net/tls/tls.h
> --- a/net/tls/tls.h
> +++ b/net/tls/tls.h
> @@ -307,17 +307,26 @@ static inline void tls_bigint_subtract(unsigned char *seq, int  n)
>  	*p = cpu_to_be64(rcd_sn - n);
>  }
>  
> -static inline void
> -tls_advance_record_sn(struct sock *sk, struct tls_prot_info *prot,
> -		      struct cipher_context *ctx)
> +/* Returns true if the record number wrapped */
> +static inline bool
> +__tls_advance_record_sn(struct tls_prot_info *prot, struct cipher_context *ctx)
>  {
> -	if (tls_bigint_increment(ctx->rec_seq, prot->rec_seq_size))
> -		tls_err_abort(sk, -EBADMSG);
> +	bool overflow = tls_bigint_increment(ctx->rec_seq, prot->rec_seq_size);
>  
>  	if (prot->version != TLS_1_3_VERSION &&
>  	    prot->cipher_type != TLS_CIPHER_CHACHA20_POLY1305)
>  		tls_bigint_increment(ctx->iv + prot->salt_size,
>  				     prot->iv_size);
> +
> +	return overflow;
> +}
> +
> +static inline void
> +tls_advance_record_sn(struct sock *sk, struct tls_prot_info *prot,
> +		      struct cipher_context *ctx)
> +{
> +	if (__tls_advance_record_sn(prot, ctx))
> +		tls_err_abort(sk, -EBADMSG);
>  }
>  
>  static inline void

[Severity: Critical]
Does the TX side keep the same SO_ERROR bypass that this patch closes
on RX?

The TX callers still go through tls_advance_record_sn(), which only
records the wrap in sk_err. tls_do_encryption() calls it for
tls_ctx->tx, and the device path in tls_push_record() in
net/tls/tls_device.c does the same. The send loop only checks sk_err:

net/tls/tls_sw.c:tls_sw_sendmsg_locked() {
    ...
	while (msg_data_left(msg)) {
		if (sk->sk_err) {
			ret = -sk->sk_err;
			goto send_end;
		}
    ...
}

Say the TX key was installed close to the limit:

- tls_do_encryption() encrypts the record at UINT64_MAX, and the
  advance wraps tls_ctx->tx.rec_seq to zero. For TLS 1.2 AES-GCM the
  explicit IV in tls_ctx->tx.iv wraps with it.
- tls_err_abort() sets sk_err to EBADMSG and send() fails.
- The application reads SO_ERROR, which clears sk_err.
- The next sendmsg() passes the check above and encrypts new records
  from sequence zero under the same key.

For AES-GCM that reuses nonces under one key, which breaks both
confidentiality and integrity for those records. A Linux receiver with
this patch would reject them, but other peers need not.

The commit message says the Linux TX path refuses to send the wrapped
record, but that refusal also rests only on sk_err. Should TX get a
persistent exhaustion mark too, checked wherever records are pushed in
both the software and device offload paths? If that is better as a
separate patch, could the commit message at least avoid implying the
TX side is already safe?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013002701.1957686-1-rjethwani%40purestorage.com