diff mbox series

[3/4] tls: Implement RFC 5746 Secure Renegotiation

Message ID 20221109174746.569046-3-andrew.zaborowski@intel.com (mailing list archive)
State Accepted, archived
Headers show
Series [1/4] tls: Allow ServerHello extensions when resuming session | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success

Commit Message

Andrew Zaborowski Nov. 9, 2022, 5:47 p.m. UTC
Implement the renegotiation_info extension to prevent some types of
attacks using renegotiation (describen in the RFC) and make the l_tls
server compatible with more clients.

Move the tls_verify_data_length declaration to tls.h for use in
tls-extensions.c.

Due to an assertion in tls_save_verify_data(), tls_send_finished() now
returns a bool so its callers are also changed to check the return value.
---
 ell/tls-extensions.c | 133 +++++++++++++++++++++++++++++++++++++++++++
 ell/tls-private.h    |   9 +++
 ell/tls.c            |  79 +++++++++++++++++++++++--
 3 files changed, 216 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/ell/tls-extensions.c b/ell/tls-extensions.c
index d03c331..bc8fc3d 100644
--- a/ell/tls-extensions.c
+++ b/ell/tls-extensions.c
@@ -850,6 +850,130 @@  static bool tls_signature_algorithms_client_absent(struct l_tls *tls)
 	return true;
 }
 
+/* RFC 5746, Section 3.2 */
+static ssize_t tls_renegotiation_info_client_write(struct l_tls *tls,
+						uint8_t *buf, size_t len)
+{
+	/*
+	 * Section 4.2 implies we should send the client_verify_data on
+	 * renegotiation even if .secure_renegotiation is false, if we want
+	 * to go through with the renegotiation in the first place.
+	 */
+	if (tls->ready) {
+		size_t vdl = tls_verify_data_length(tls, 1);
+
+		if (len < vdl + 1)
+			return -ENOMEM;
+
+		buf[0] = vdl;
+		memcpy(buf + 1, tls->renegotiation_info.client_verify_data,
+			vdl);
+		return 1 + vdl;
+	} else {
+		if (len < 1)
+			return -ENOMEM;
+
+		buf[0] = 0x00;	/* Empty "renegotiated_connection" */
+		return 1;
+	}
+}
+
+static ssize_t tls_renegotiation_info_server_write(struct l_tls *tls,
+						uint8_t *buf, size_t len)
+{
+	if (tls->ready) {
+		size_t rx_vdl = tls_verify_data_length(tls, 0);
+		size_t tx_vdl = tls_verify_data_length(tls, 1);
+
+		if (len < rx_vdl + tx_vdl + 1)
+			return -ENOMEM;
+
+		buf[0] = rx_vdl + tx_vdl;
+		memcpy(buf + 1,
+			tls->renegotiation_info.client_verify_data, rx_vdl);
+		memcpy(buf + 1 + rx_vdl,
+			tls->renegotiation_info.server_verify_data, tx_vdl);
+		return 1 + rx_vdl + tx_vdl;
+	} else {
+		if (len < 1)
+			return -ENOMEM;
+
+		buf[0] = 0x00;	/* Empty "renegotiated_connection" */
+		return 1;
+	}
+}
+
+static bool tls_renegotiation_info_client_handle(struct l_tls *tls,
+						const uint8_t *buf, size_t len)
+{
+	if (tls->ready) {
+		size_t vdl = tls_verify_data_length(tls, 0);
+
+		return len >= 1 + vdl &&
+			tls->renegotiation_info.secure_renegotiation &&
+			!memcmp(tls->renegotiation_info.client_verify_data,
+				buf + 1, vdl);
+	}
+
+	/*
+	 * RFC 5746 Section 3.6: "The server MUST then verify that the length
+	 * of the "renegotiated_connection" field is zero, ..."
+	 */
+	if (len < 1 || buf[0] != 0x00)
+		return false;
+
+	tls->renegotiation_info.secure_renegotiation = true;
+	return true;
+}
+
+static bool tls_renegotiation_info_server_handle(struct l_tls *tls,
+						const uint8_t *buf, size_t len)
+{
+	if (tls->ready) {
+		size_t rx_vdl = tls_verify_data_length(tls, 0);
+		size_t tx_vdl = tls_verify_data_length(tls, 1);
+
+		return len >= 1 + rx_vdl + tx_vdl &&
+			tls->renegotiation_info.secure_renegotiation &&
+			!memcmp(tls->renegotiation_info.client_verify_data,
+				buf + 1, tx_vdl) &&
+			!memcmp(tls->renegotiation_info.server_verify_data,
+				buf + 1 + tx_vdl, rx_vdl);
+	}
+
+	/*
+	 * RFC 5746 Section 3.4: "The client MUST then verify that the length
+	 * of the "renegotiated_connection" field is zero, ..."
+	 */
+	if (len < 1 || buf[0] != 0x00)
+		return false;
+
+	tls->renegotiation_info.secure_renegotiation = true;
+	return true;
+}
+
+static bool tls_renegotiation_info_absent(struct l_tls *tls)
+{
+	/*
+	 * RFC 5746 Section 4.2: "It is possible that un-upgraded servers
+	 * will request that the client renegotiate.  It is RECOMMENDED
+	 * that clients refuse this renegotiation request." and Section 4.4:
+	 * "It is RECOMMENDED that servers not permit legacy renegotiation."
+	 *
+	 * This may need to be made configurable, for now follow the
+	 * recommendation and don't renegotiate.
+	 */
+	if (tls->ready)
+		return false;
+
+	/*
+	 * The normal policy otherwise is that the extension must be
+	 * present in renegotation if the previous Client or Server Hello
+	 * did include this extension, or the SCSV in the Client Hello case.
+	 */
+	return !tls->ready || !tls->renegotiation_info.secure_renegotiation;
+}
+
 const struct tls_hello_extension tls_extensions[] = {
 	{
 		"Supported Groups", "elliptic_curves", 10,
@@ -873,6 +997,15 @@  const struct tls_hello_extension tls_extensions[] = {
 		tls_signature_algorithms_client_absent,
 		NULL, NULL, NULL,
 	},
+	{
+		"Secure Renegotiation", "renegotiation_info", 0xff01,
+		tls_renegotiation_info_client_write,
+		tls_renegotiation_info_client_handle,
+		tls_renegotiation_info_absent,
+		tls_renegotiation_info_server_write,
+		tls_renegotiation_info_server_handle,
+		tls_renegotiation_info_absent,
+	},
 	{}
 };
 
diff --git a/ell/tls-private.h b/ell/tls-private.h
index a3a9393..6a9bd29 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -267,6 +267,13 @@  struct l_tls {
 	uint8_t session_compression_method_id;
 	char *session_peer_identity;
 
+	struct {
+		bool secure_renegotiation;
+		/* Max .verify_data_length over supported cipher suites */
+		uint8_t client_verify_data[12];
+		uint8_t server_verify_data[12];
+	} renegotiation_info;
+
 	/* SecurityParameters current and pending */
 
 	struct {
@@ -341,6 +348,8 @@  void tls_generate_master_secret(struct l_tls *tls,
 				const uint8_t *pre_master_secret,
 				int pre_master_secret_len);
 
+size_t tls_verify_data_length(struct l_tls *tls, unsigned int index);
+
 const struct tls_named_group *tls_find_group(uint16_t id);
 const struct tls_named_group *tls_find_ff_group(const uint8_t *prime,
 						size_t prime_len,
diff --git a/ell/tls.c b/ell/tls.c
index 14c8550..2a8d509 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -1234,6 +1234,7 @@  void tls_disconnect(struct l_tls *tls, enum l_tls_alert_desc desc,
 
 	tls->negotiated_version = 0;
 	tls->ready = false;
+	tls->renegotiation_info.secure_renegotiation = false;
 
 	if (forget_session) {
 		tls_forget_cached_session(tls, NULL, tls->session_id,
@@ -1749,7 +1750,7 @@  static void tls_send_change_cipher_spec(struct l_tls *tls)
 	tls_tx_record(tls, TLS_CT_CHANGE_CIPHER_SPEC, &buf, 1);
 }
 
-static size_t tls_verify_data_length(struct l_tls *tls, unsigned int index)
+size_t tls_verify_data_length(struct l_tls *tls, unsigned int index)
 {
 	/*
 	 * RFC 5246, Section 7.4.9:
@@ -1762,7 +1763,34 @@  static size_t tls_verify_data_length(struct l_tls *tls, unsigned int index)
 	return maxsize(tls->cipher_suite[index]->verify_data_length, 12);
 }
 
-static void tls_send_finished(struct l_tls *tls)
+static bool tls_save_verify_data(struct l_tls *tls, bool txrx,
+					const uint8_t *vd, size_t vdl)
+{
+	uint8_t *buf;
+
+	if (tls->server == txrx) {
+		if (vdl > sizeof(tls->renegotiation_info.server_verify_data))
+			goto error;
+
+		buf = tls->renegotiation_info.server_verify_data;
+	} else {
+		if (vdl > sizeof(tls->renegotiation_info.client_verify_data))
+			goto error;
+
+		buf = tls->renegotiation_info.client_verify_data;
+	}
+
+	memcpy(buf, vd, vdl);
+	return true;
+
+error:
+	TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+			"tls->renegotiation_info.*verify too small for %s, "
+			"report an ell bug", tls->cipher_suite[txrx]->name);
+	return false;
+}
+
+static bool tls_send_finished(struct l_tls *tls)
 {
 	uint8_t buf[512];
 	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
@@ -1785,9 +1813,14 @@  static void tls_send_finished(struct l_tls *tls)
 				"client finished",
 				seed, seed_len,
 				ptr, vdl);
+
+	if (!tls_save_verify_data(tls, 1, ptr, vdl))
+		return false;
+
 	ptr += vdl;
 
 	tls_tx_handshake(tls, TLS_FINISHED, buf, ptr - buf);
+	return true;
 }
 
 static bool tls_verify_finished(struct l_tls *tls, const uint8_t *received,
@@ -1830,6 +1863,9 @@  static bool tls_verify_finished(struct l_tls *tls, const uint8_t *received,
 		return false;
 	}
 
+	if (!tls_save_verify_data(tls, 0, received, vdl))
+		return false;
+
 	return true;
 }
 
@@ -1992,6 +2028,10 @@  static void tls_server_resume_error(struct l_tls *tls)
 	l_free(l_steal_ptr(tls->session_peer_identity));
 }
 
+/* RFC 5746 */
+static const uint8_t tls_empty_renegotiation_info_scsv[2] = { 0x00, 0xff };
+static const uint16_t tls_renegotiation_info_id = 0xff01;
+
 static void tls_handle_client_hello(struct l_tls *tls,
 					const uint8_t *buf, size_t len)
 {
@@ -2100,6 +2140,30 @@  static void tls_handle_client_hello(struct l_tls *tls,
 		goto cleanup;
 	}
 
+	if (!tls->renegotiation_info.secure_renegotiation || tls->ready) {
+		for (i = 0; i < cipher_suites_size; i += 2)
+			if (l_get_be16(cipher_suites + i) == l_get_be16(
+					tls_empty_renegotiation_info_scsv))
+				break;
+
+		if (i < cipher_suites_size) {
+			if (unlikely(tls->ready)) {
+				TLS_DISCONNECT(TLS_ALERT_ILLEGAL_PARAM, 0,
+						"Empty renegotiation_info in "
+						"renegotiation Client Hello");
+				goto cleanup;
+			}
+
+			/*
+			 * RFC 5746 Section 3.6, act as if we had received an
+			 * empty renegotiation_info extension.
+			 */
+			tls->renegotiation_info.secure_renegotiation = true;
+			l_queue_push_tail(extensions_offered, L_UINT_TO_PTR(
+						tls_renegotiation_info_id));
+		}
+	}
+
 	/* Select a cipher suite according to client's preference list */
 	while (cipher_suites_size) {
 		struct tls_cipher_suite *suite =
@@ -2243,7 +2307,9 @@  static void tls_handle_client_hello(struct l_tls *tls,
 			return;
 		}
 
-		tls_send_finished(tls);
+		if (!tls_send_finished(tls))
+			return;
+
 		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_CHANGE_CIPHER_SPEC);
 		return;
 	}
@@ -2721,7 +2787,8 @@  static void tls_handle_server_hello_done(struct l_tls *tls,
 		return;
 	}
 
-	tls_send_finished(tls);
+	if (!tls_send_finished(tls))
+		return;
 
 	TLS_SET_STATE(TLS_HANDSHAKE_WAIT_CHANGE_CIPHER_SPEC);
 }
@@ -3222,7 +3289,9 @@  static void tls_handle_handshake(struct l_tls *tls, int type,
 						error);
 				break;
 			}
-			tls_send_finished(tls);
+
+			if (!tls_send_finished(tls))
+				break;
 		}
 
 		/*