ktls: Disallow transmitting empty frames outside of TLS 1.0/CBC mode

There was nothing preventing one from sending an empty fragment on an
arbitrary KTLS TX-enabled socket, but ktls_frame() asserts that this
could not happen.  Though the transmit path handles this case for TLS
1.0 with AES-CBC, we should be strict and allow empty fragments only in
modes where it is explicitly allowed.

Modify sosend_generic() to reject writes to a KTLS-enabled socket if the
number of data bytes is zero, so that userspace cannot trigger the
aforementioned assertion.

Add regression tests to exercise this case.

Reported by:	syzkaller
Reviewed by:	gallatin, jhb
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D34195

sys/kern/uipc_ktls.c

@@ -1691,19 +1691,18 @@ ktls_frame(struct mbuf *top, struct ktls_session *tls, int *enq_cnt,
 		 * All mbufs in the chain should be TLS records whose
 		 * payload does not exceed the maximum frame length.
 		 *
-		 * Empty TLS records are permitted when using CBC.
+		 * Empty TLS 1.0 records are permitted when using CBC.
 		 */
-		KASSERT(m->m_len <= maxlen &&
+		KASSERT(m->m_len <= maxlen && m->m_len >= 0 &&
-		    (tls->params.cipher_algorithm == CRYPTO_AES_CBC ?
+		    (m->m_len > 0 || ktls_permit_empty_frames(tls)),
-		    m->m_len >= 0 : m->m_len > 0),
+		    ("ktls_frame: m %p len %d", m, m->m_len));
-		    ("ktls_frame: m %p len %d\n", m, m->m_len));
 
 		/*
 		 * TLS frames require unmapped mbufs to store session
 		 * info.
 		 */
 		KASSERT((m->m_flags & M_EXTPG) != 0,
-		    ("ktls_frame: mapped mbuf %p (top = %p)\n", m, top));
+		    ("ktls_frame: mapped mbuf %p (top = %p)", m, top));
 
 		tls_len = m->m_len;
 
@@ -1797,6 +1796,13 @@ ktls_frame(struct mbuf *top, struct ktls_session *tls, int *enq_cnt,
 	}
 }
 
+bool
+ktls_permit_empty_frames(struct ktls_session *tls)
+{
+	return (tls->params.cipher_algorithm == CRYPTO_AES_CBC &&
+	    tls->params.tls_vminor == TLS_MINOR_VER_ZERO);
+}
+
 void
 ktls_check_rx(struct sockbuf *sb)
 {

sys/kern/uipc_socket.c

@@ -1667,6 +1667,11 @@ sosend_generic(struct socket *so, struct sockaddr *addr, struct uio *uio,
 				atomic = 1;
 			}
 		}
+
+		if (resid == 0 && !ktls_permit_empty_frames(tls)) {
+			error = EINVAL;
+			goto release;
+		}
 	}
 #endif
 

sys/sys/ktls.h

@@ -213,6 +213,7 @@ int ktls_enable_tx(struct socket *so, struct tls_enable *en);
 void ktls_destroy(struct ktls_session *tls);
 void ktls_frame(struct mbuf *m, struct ktls_session *tls, int *enqueue_cnt,
     uint8_t record_type);
+bool ktls_permit_empty_frames(struct ktls_session *tls);
 void ktls_seq(struct sockbuf *sb, struct mbuf *m);
 void ktls_enqueue(struct mbuf *m, struct socket *so, int page_count);
 void ktls_enqueue_to_free(struct mbuf *m);

tests/sys/kern/ktls_test.c

@@ -1105,9 +1105,19 @@ test_ktls_transmit_empty_fragment(struct tls_enable *en, uint64_t seqno)
 	fd_set_blocking(sockets[0]);
 	fd_set_blocking(sockets[1]);
 
-	/* A write of zero bytes should send an empty fragment. */
+	/*
+	 * A write of zero bytes should send an empty fragment only for
+	 * TLS 1.0, otherwise an error should be raised.
+	 */
 	rv = write(sockets[1], NULL, 0);
-	ATF_REQUIRE(rv == 0);
+	if (rv == 0) {
+		ATF_REQUIRE(en->cipher_algorithm == CRYPTO_AES_CBC);
+		ATF_REQUIRE(en->tls_vminor == TLS_MINOR_VER_ZERO);
+	} else {
+		ATF_REQUIRE(rv == -1);
+		ATF_REQUIRE(errno == EINVAL);
+		goto out;
+	}
 
 	/*
 	 * First read the header to determine how much additional data
@@ -1129,6 +1139,7 @@ test_ktls_transmit_empty_fragment(struct tls_enable *en, uint64_t seqno)
 	    "read %zd decrypted bytes for an empty fragment", rv);
 	ATF_REQUIRE(record_type == TLS_RLTYPE_APP);
 
+out:
 	free(outbuf);
 
 	ATF_REQUIRE(close(sockets[1]) == 0);
@@ -1369,7 +1380,7 @@ ATF_TC_BODY(ktls_transmit_##cipher_name##_##name, tc)			\
 	ATF_TP_ADD_TC(tp, ktls_transmit_##cipher_name##_##name);
 
 #define GEN_TRANSMIT_EMPTY_FRAGMENT_TEST(cipher_name, cipher_alg,	\
-	    key_size, auth_alg)						\
+	    key_size, auth_alg, minor)					\
 ATF_TC_WITHOUT_HEAD(ktls_transmit_##cipher_name##_empty_fragment);	\
 ATF_TC_BODY(ktls_transmit_##cipher_name##_empty_fragment, tc)		\
 {									\
@@ -1378,14 +1389,14 @@ ATF_TC_BODY(ktls_transmit_##cipher_name##_empty_fragment, tc)		\
 									\
 	ATF_REQUIRE_KTLS();						\
 	seqno = random();						\
-	build_tls_enable(cipher_alg, key_size, auth_alg,		\
+	build_tls_enable(cipher_alg, key_size, auth_alg, minor, seqno,	\
-	    TLS_MINOR_VER_ZERO,	seqno, &en);				\
+	    &en);							\
 	test_ktls_transmit_empty_fragment(&en, seqno);			\
 	free_tls_enable(&en);						\
 }
 
 #define ADD_TRANSMIT_EMPTY_FRAGMENT_TEST(cipher_name, cipher_alg,	\
-	    key_size, auth_alg)						\
+	    key_size, auth_alg, minor)					\
 	ATF_TP_ADD_TC(tp, ktls_transmit_##cipher_name##_empty_fragment);
 
 #define GEN_TRANSMIT_TESTS(cipher_name, cipher_alg, key_size, auth_alg,	\
@@ -1506,7 +1517,9 @@ AES_CBC_TESTS(GEN_TRANSMIT_PADDING_TESTS);
  * Test "empty fragments" which are TLS records with no payload that
  * OpenSSL can send for TLS 1.0 connections.
  */
-TLS_10_TESTS(GEN_TRANSMIT_EMPTY_FRAGMENT_TEST);
+AES_CBC_TESTS(GEN_TRANSMIT_EMPTY_FRAGMENT_TEST);
+AES_GCM_TESTS(GEN_TRANSMIT_EMPTY_FRAGMENT_TEST);
+CHACHA20_TESTS(GEN_TRANSMIT_EMPTY_FRAGMENT_TEST);
 
 static void
 test_ktls_invalid_transmit_cipher_suite(struct tls_enable *en)
@@ -1768,7 +1781,9 @@ ATF_TP_ADD_TCS(tp)
 	AES_GCM_TESTS(ADD_TRANSMIT_TESTS);
 	CHACHA20_TESTS(ADD_TRANSMIT_TESTS);
 	AES_CBC_TESTS(ADD_TRANSMIT_PADDING_TESTS);
-	TLS_10_TESTS(ADD_TRANSMIT_EMPTY_FRAGMENT_TEST);
+	AES_CBC_TESTS(ADD_TRANSMIT_EMPTY_FRAGMENT_TEST);
+	AES_GCM_TESTS(ADD_TRANSMIT_EMPTY_FRAGMENT_TEST);
+	CHACHA20_TESTS(ADD_TRANSMIT_EMPTY_FRAGMENT_TEST);
 	INVALID_CIPHER_SUITES(ADD_INVALID_TRANSMIT_TEST);
 
 	/* Receive tests */