[PATCH net-next v2 2/2] tls: make the record number wrap abort final

@@ -228,6 +228,25 @@ The number of successful and failed key updates is tracked in the
 ``TlsRxRekeyError`` statistics. The ``TlsRxRekeyReceived`` statistic
 counts KeyUpdate handshake messages that have been received.
 
+Record number exhaustion
+------------------------
+
+A record number must not wrap under the same key. When it would, the
+connection is aborted with ``EBADMSG``. On transmit, the record that
+uses the last 64-bit record number is not sent, and the send that
+completes it doesn't count its data as sent. On receive, that record is
+still delivered, but no record after it is. Either way, no further
+record data is sent, not even the rest of a partially sent record.
+Unlike other socket errors, this one is not cleared when it is reported,
+by ``SO_ERROR`` or otherwise: every later send, and every receive that
+needs another record, fails with ``EBADMSG``, and a new key does not
+change that. ``poll()`` reports ``EPOLLERR``, and ``EPOLLIN`` only while
+data that was already decrypted remains; that data can still be read.
+
+A TLS 1.3 KeyUpdate may be the last record received under a key. In
+that case receive is paused as for any other KeyUpdate, and continues
+once the new key is provided.
+
 Integrating in to userspace TLS library
 ---------------------------------------
 

@@ -251,6 +251,7 @@ struct tls_context {
 
 	bool splicing_pages;
 	bool pending_open_record_frags;
+	int final_err; /* set by tls_err_abort_final(), never cleared */
 
 	struct mutex tx_lock; /* protects partially_sent_* fields and
 			       * per-type TX fields

@@ -142,6 +142,7 @@ void update_sk_prot(struct sock *sk, struct tls_context *ctx);
 
 int wait_on_pending_writer(struct sock *sk, long *timeo);
 void tls_err_abort(struct sock *sk, int err);
+void tls_err_abort_final(struct sock *sk, int err);
 void tls_strp_abort_strp(struct tls_strparser *strp, int err);
 
 int init_prot_info(struct tls_prot_info *prot,

@@ -307,17 +308,30 @@ 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)
+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 wrapped = 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 wrapped;
+}
+
+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_final(sk, -EBADMSG);
+}
+
+static inline int tls_final_err(const struct tls_context *ctx)
+{
+	return READ_ONCE(ctx->final_err);
 }
 
 static inline void

@@ -430,6 +430,7 @@ static int tls_push_data(struct sock *sk,
 	int tls_push_record_flags;
 	struct page_frag *pfrag;
 	size_t orig_size = size;
+	size_t rec_size = size;
 	u32 max_open_record_len;
 	bool more = false;
 	bool done = false;

@@ -444,6 +445,10 @@ static int tls_push_data(struct sock *sk,
 	if ((flags & (MSG_MORE | MSG_EOR)) == (MSG_MORE | MSG_EOR))
 		return -EINVAL;
 
+	rc = tls_final_err(tls_ctx);
+	if (unlikely(rc))
+		return rc;
+
 	if (unlikely(sk->sk_err))
 		return -sk->sk_err;
 

@@ -550,8 +555,13 @@ static int tls_push_data(struct sock *sk,
 					     ctx,
 					     record,
 					     tls_push_record_flags);
-			if (rc < 0)
+			if (rc < 0) {
+				/* the refused record isn't sent, don't count it */
+				if (tls_final_err(tls_ctx))
+					size = rec_size;
 				break;
+			}
+			rec_size = size;
 		}
 	} while (!done);
 

@@ -190,7 +190,7 @@ int tls_push_sg(struct sock *sk,
 		bvec_set_page(&bvec, p, size, offset);
 		iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1, size);
 
-		ret = tcp_sendmsg_locked(sk, &msg, size);
+		ret = tls_final_err(ctx) ?: tcp_sendmsg_locked(sk, &msg, size);
 
 		if (ret != size) {
 			if (ret > 0) {

@@ -414,8 +414,12 @@ static __poll_t tls_sk_poll(struct file *file, struct socket *sock,
 	tls_ctx = tls_get_ctx(sk);
 	ctx = tls_sw_ctx_rx(tls_ctx);
 
+	/* the final abort outlives sk_err, only decrypted data is readable */
+	if (unlikely(tls_final_err(tls_ctx)))
+		mask |= EPOLLERR;
+
 	if ((skb_queue_empty_lockless(&ctx->rx_list) &&
-	     !tls_strp_msg_ready(ctx)) ||
+	     (!tls_strp_msg_ready(ctx) || tls_final_err(tls_ctx))) ||
 	    READ_ONCE(ctx->key_update_pending))
 		mask &= ~(EPOLLIN | EPOLLRDNORM);
 

@@ -78,6 +78,18 @@ noinline void tls_err_abort(struct sock *sk, int err)
 	sk_error_report(sk);
 }
 
+/* Like tls_err_abort(), but the error can't be cleared: SO_ERROR and
+ * sock_error() consume sk_err, final_err keeps failing reads and sends.
+ */
+void tls_err_abort_final(struct sock *sk, int err)
+{
+	struct tls_context *ctx = tls_get_ctx(sk);
+
+	if (!tls_final_err(ctx))
+		WRITE_ONCE(ctx->final_err, err);
+	tls_err_abort(sk, err);
+}
+
 static int __skb_nsg(struct sk_buff *skb, int offset, int len,
                      unsigned int recursion_level)
 {

@@ -458,7 +470,9 @@ int tls_tx_records(struct sock *sk, int flags)
 	}
 
 tx_err:
-	if (rc < 0 && rc != -EAGAIN && rc != -EINTR && rc != -ERESTARTSYS)
+	/* a final abort already reported its error */
+	if (rc < 0 && rc != -EAGAIN && rc != -EINTR && rc != -ERESTARTSYS &&
+	    !tls_final_err(tls_ctx))
 		tls_err_abort(sk, rc);
 
 	return rc;

@@ -628,6 +642,11 @@ static int tls_push_record(struct sock *sk, int flags,
 	int rc;
 	u32 i;
 
+	/* tls_push_sg() would refuse it, don't encrypt it */
+	rc = tls_final_err(tls_ctx);
+	if (unlikely(rc))
+		return rc;
+
 	if (!rec)
 		return 0;
 

@@ -802,6 +821,13 @@ static int tls_sw_sendmsg_locked(struct sock *sk, struct msghdr *msg,
 	}
 
 	while (msg_data_left(msg)) {
+		int err = tls_final_err(tls_ctx);
+
+		if (unlikely(err)) {
+			ret = err;
+			goto send_end;
+		}
+
 		if (sk->sk_err) {
 			ret = -sk->sk_err;
 			goto send_end;

@@ -1106,6 +1132,11 @@ tls_rx_rec_wait(struct sock *sk, bool nonblock, bool released, bool has_copied)
 	if (unlikely(ctx->key_update_pending))
 		return -EKEYEXPIRED;
 
+	/* checked even if a record is ready, unlike sk_err below */
+	ret = tls_final_err(tls_ctx);
+	if (unlikely(ret))
+		return ret;
+
 	timeo = sock_rcvtimeo(sk, nonblock);
 
 	while (!tls_strp_msg_ready(ctx)) {

@@ -1582,9 +1613,14 @@ 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);
 
-	return tls_check_pending_rekey(sk, tls_ctx, darg->skb);
+	err = tls_check_pending_rekey(sk, tls_ctx, darg->skb);
+	/* A KeyUpdate in the last record retires the key as usual */
+	if (__tls_advance_record_sn(prot, &tls_ctx->rx) &&
+	    !tls_sw_ctx_rx(tls_ctx)->key_update_pending)
+		tls_err_abort_final(sk, -EBADMSG);
+
+	return err;
 }
 
 int decrypt_skb(struct sock *sk, struct scatterlist *sgout)

@@ -2182,7 +2218,7 @@ bool tls_sw_sock_is_readable(struct sock *sk)
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
 	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
 
-	return tls_strp_msg_ready(ctx) ||
+	return (tls_strp_msg_ready(ctx) && !tls_final_err(tls_ctx)) ||
 		!skb_queue_empty(&ctx->rx_list);
 }
 

@@ -2421,6 +2421,203 @@ TEST_F(tls, rekey_poll_delay)
 	}
 }
 
+/* Record number exhaustion, TLS 1.3 only. The record number only enters
+ * the nonce there, so a TX key whose IV is XORed with N sends record
+ * number N at its own record number 0, and TX never has to wrap.
+ */
+FIXTURE(tls_recseq)
+{
+	int fd, cfd;
+	bool notls;
+};
+
+FIXTURE_SETUP(tls_recseq)
+{
+	ulp_sock_pair(_metadata, &self->fd, &self->cfd, &self->notls);
+}
+
+FIXTURE_TEARDOWN(tls_recseq)
+{
+	close(self->fd);
+	close(self->cfd);
+}
+
+static void tls_recseq_keys(struct tls_crypto_info_keys *tls,
+			    uint64_t rec_seq, uint64_t iv_xor)
+{
+	int i;
+
+	tls_crypto_info_init(TLS_1_3_VERSION, TLS_CIPHER_AES_GCM_128, tls, 0);
+	for (i = 0; i < 8; i++) {
+		tls->aes128.rec_seq[i] = rec_seq >> (56 - 8 * i);
+		tls->aes128.iv[i] ^= iv_xor >> (56 - 8 * i);
+	}
+}
+
+/* RX starts at the second to last record number, TX sends it and the
+ * last one at its record numbers 0 and 1.
+ */
+static void tls_recseq_set_keys(struct __test_metadata *_metadata,
+				int fd, int cfd)
+{
+	struct tls_crypto_info_keys tls;
+
+	tls_recseq_keys(&tls, 0, UINT64_MAX - 1);
+	ASSERT_EQ(setsockopt(fd, SOL_TLS, TLS_TX, &tls, tls.len), 0);
+	tls_recseq_keys(&tls, UINT64_MAX - 1, 0);
+	ASSERT_EQ(setsockopt(cfd, SOL_TLS, TLS_RX, &tls, tls.len), 0);
+}
+
+/* Silent rekey to the same key, so TX sends the wrapped record number 0 */
+static void tls_recseq_tx_wrapped(struct __test_metadata *_metadata, int fd)
+{
+	struct tls_crypto_info_keys tls;
+
+	tls_recseq_keys(&tls, 0, 0);
+	ASSERT_EQ(setsockopt(fd, SOL_TLS, TLS_TX, &tls, tls.len), 0);
+}
+
+static int tls_so_error(struct __test_metadata *_metadata, int fd)
+{
+	socklen_t len = sizeof(int);
+	int err = -1;
+
+	EXPECT_EQ(getsockopt(fd, SOL_SOCKET, SO_ERROR, &err, &len), 0);
+	return err;
+}
+
+/* All three records are queued, only the first two may be received */
+TEST_F(tls_recseq, rx_wrap)
+{
+	struct pollfd pfd = { .events = POLLIN };
+	char const *test_str = "test_read";
+	int send_len = strlen(test_str) + 1;
+	char buf[3 * 10];
+
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	tls_recseq_set_keys(_metadata, self->fd, self->cfd);
+	EXPECT_EQ(send(self->fd, test_str, send_len, 0), send_len);
+	EXPECT_EQ(send(self->fd, test_str, send_len, 0), send_len);
+	tls_recseq_tx_wrapped(_metadata, self->fd);
+	EXPECT_EQ(send(self->fd, test_str, send_len, 0), send_len);
+
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), 0), 2 * send_len);
+	EXPECT_EQ(memcmp(buf, test_str, send_len), 0);
+	EXPECT_EQ(memcmp(buf + send_len, test_str, send_len), 0);
+
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), 0), -1);
+	EXPECT_EQ(errno, EBADMSG);
+
+	/* reading SO_ERROR clears sk_err, the abort stays */
+	EXPECT_EQ(tls_so_error(_metadata, self->cfd), EBADMSG);
+	EXPECT_EQ(tls_so_error(_metadata, self->cfd), 0);
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT), -1);
+	EXPECT_EQ(errno, EBADMSG);
+
+	/* the parsed third record isn't readable */
+	pfd.fd = self->cfd;
+	EXPECT_EQ(poll(&pfd, 1, 0), 1);
+	EXPECT_EQ(pfd.revents, POLLERR);
+}
+
+TEST_F(tls_recseq, rx_wrap_splice)
+{
+	char const *test_str = "test_read";
+	int send_len = strlen(test_str) + 1;
+	char buf[10];
+	int p[2];
+
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	ASSERT_GE(pipe(p), 0);
+
+	tls_recseq_set_keys(_metadata, self->fd, self->cfd);
+	EXPECT_EQ(send(self->fd, test_str, send_len, 0), send_len);
+	EXPECT_EQ(send(self->fd, test_str, send_len, 0), send_len);
+	tls_recseq_tx_wrapped(_metadata, self->fd);
+	EXPECT_EQ(send(self->fd, test_str, send_len, 0), send_len);
+
+	EXPECT_EQ(splice(self->cfd, NULL, p[1], NULL, send_len, 0), send_len);
+	EXPECT_EQ(splice(self->cfd, NULL, p[1], NULL, send_len, 0), send_len);
+	EXPECT_EQ(read(p[0], buf, send_len), send_len);
+	EXPECT_EQ(memcmp(buf, test_str, send_len), 0);
+	EXPECT_EQ(read(p[0], buf, send_len), send_len);
+	EXPECT_EQ(memcmp(buf, test_str, send_len), 0);
+
+	EXPECT_EQ(tls_so_error(_metadata, self->cfd), EBADMSG);
+	EXPECT_EQ(splice(self->cfd, NULL, p[1], NULL, send_len,
+			 SPLICE_F_NONBLOCK), -1);
+	EXPECT_EQ(errno, EBADMSG);
+
+	close(p[0]);
+	close(p[1]);
+}
+
+/* A KeyUpdate may use the last record number, then a new key takes over */
+TEST_F(tls_recseq, rx_keyupdate)
+{
+	struct tls_crypto_info_keys tls12_1;
+	char const *test_str = "test_read";
+	int send_len = strlen(test_str) + 1;
+	char buf[10];
+
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	tls_crypto_info_init(TLS_1_3_VERSION, TLS_CIPHER_AES_GCM_128,
+			     &tls12_1, 1);
+
+	tls_recseq_set_keys(_metadata, self->fd, self->cfd);
+	EXPECT_EQ(send(self->fd, test_str, send_len, 0), send_len);
+	tls_send_keyupdate(_metadata, self->fd);
+	ASSERT_EQ(setsockopt(self->fd, SOL_TLS, TLS_TX, &tls12_1,
+			     tls12_1.len), 0);
+	EXPECT_EQ(send(self->fd, test_str, send_len, 0), send_len);
+
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), 0), send_len);
+	EXPECT_EQ(memcmp(buf, test_str, send_len), 0);
+	tls_recv_keyupdate(_metadata, self->cfd, 0);
+
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT), -1);
+	EXPECT_EQ(errno, EKEYEXPIRED);
+	EXPECT_EQ(tls_so_error(_metadata, self->cfd), 0);
+
+	ASSERT_EQ(setsockopt(self->cfd, SOL_TLS, TLS_RX, &tls12_1,
+			     tls12_1.len), 0);
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), 0), send_len);
+	EXPECT_EQ(memcmp(buf, test_str, send_len), 0);
+}
+
+/* The record using the last record number is never sent */
+TEST_F(tls_recseq, tx_wrap)
+{
+	struct tls_crypto_info_keys tls;
+	char const *test_str = "test_read";
+	int send_len = strlen(test_str) + 1;
+	char buf[10];
+
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	tls_recseq_keys(&tls, UINT64_MAX, 0);
+	ASSERT_EQ(setsockopt(self->fd, SOL_TLS, TLS_TX, &tls, tls.len), 0);
+	ASSERT_EQ(setsockopt(self->cfd, SOL_TLS, TLS_RX, &tls, tls.len), 0);
+
+	EXPECT_EQ(send(self->fd, test_str, send_len, 0), -1);
+	EXPECT_EQ(errno, EBADMSG);
+
+	/* reading SO_ERROR clears sk_err, the abort stays */
+	EXPECT_EQ(tls_so_error(_metadata, self->fd), EBADMSG);
+	EXPECT_EQ(send(self->fd, test_str, send_len, 0), -1);
+	EXPECT_EQ(errno, EBADMSG);
+
+	EXPECT_EQ(recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT), -1);
+	EXPECT_EQ(errno, EAGAIN);
+}
+
 struct raw_rec {
 	unsigned int plain_len;
 	unsigned char plain_data[100];