Implement a rolling hash of the ClientHello message, Enforce RFC 8446

section 4.1.2 to ensure subsequent ClientHello messages after a
HelloRetryRequest messages must be unchanged from the initial
ClientHello.

ok tb@ jsing@

7 files changed, 181 insertions, 9 deletions

diff --git a/lib/libssl/s3_lib.c b/lib/libssl/s3_lib.c
index e2fef725889..c2cf9229739 100644
--- a/lib/libssl/s3_lib.c
+++ b/lib/libssl/s3_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: s3_lib.c,v 1.195 2020/06/05 18:14:05 jsing Exp $ */
+/* $OpenBSD: s3_lib.c,v 1.196 2020/06/06 01:40:08 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1568,6 +1568,7 @@ ssl3_free(SSL *s)
 	tls13_key_share_free(S3I(s)->hs_tls13.key_share);
 	tls13_secrets_destroy(S3I(s)->hs_tls13.secrets);
 	freezero(S3I(s)->hs_tls13.cookie, S3I(s)->hs_tls13.cookie_len);
+	tls13_clienthello_hash_clear(&S3I(s)->hs_tls13);
 
 	sk_X509_NAME_pop_free(S3I(s)->tmp.ca_names, X509_NAME_free);
 
@@ -1612,6 +1613,7 @@ ssl3_clear(SSL *s)
 	freezero(S3I(s)->hs_tls13.cookie, S3I(s)->hs_tls13.cookie_len);
 	S3I(s)->hs_tls13.cookie = NULL;
 	S3I(s)->hs_tls13.cookie_len = 0;
+	tls13_clienthello_hash_clear(&S3I(s)->hs_tls13);
 
 	S3I(s)->hs.extensions_seen = 0;
 
diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index bfc3c1ad9b5..bf1f846d136 100644
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.279 2020/05/31 18:03:32 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.280 2020/06/06 01:40:09 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -470,6 +470,12 @@ typedef struct ssl_handshake_tls13_st {
 	/* Legacy session ID. */
 	uint8_t legacy_session_id[SSL_MAX_SSL_SESSION_ID_LENGTH];
 	size_t legacy_session_id_len;
+
+	/* ClientHello hash, used to validate following HelloRetryRequest */
+	EVP_MD_CTX *clienthello_md_ctx;
+	unsigned char *clienthello_hash;
+	unsigned int clienthello_hash_len;
+
 } SSL_HANDSHAKE_TLS13;
 
 typedef struct ssl_ctx_internal_st {
diff --git a/lib/libssl/ssl_tlsext.c b/lib/libssl/ssl_tlsext.c
index cf54fc4d2cb..f6943c83ae5 100644
--- a/lib/libssl/ssl_tlsext.c
+++ b/lib/libssl/ssl_tlsext.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_tlsext.c,v 1.74 2020/05/29 17:39:42 jsing Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.75 2020/06/06 01:40:09 beck Exp $ */
 /*
  * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -2059,6 +2059,33 @@ tlsext_build(SSL *s, CBB *cbb, int is_server, uint16_t msg_type)
 	return 1;
 }
 
+int
+tlsext_clienthello_hash_extension(SSL *s, uint16_t type, CBS *cbs)
+{
+	/*
+	 * RFC 8446 4.1.2. For subsequent CH, early data will be removed,
+	 * cookie may be added, padding may be removed.
+	 */
+	struct tls13_ctx *ctx = s->internal->tls13;
+
+	if (type == TLSEXT_TYPE_early_data || type == TLSEXT_TYPE_cookie ||
+	    type == TLSEXT_TYPE_padding)
+		return 1;
+	if (!tls13_clienthello_hash_update_bytes(ctx, (void *)&type,
+	    sizeof(type)))
+		return 0;
+	/*
+	 * key_share data may be changed, and pre_shared_key data may
+	 * be changed
+	 */
+	if (type == TLSEXT_TYPE_pre_shared_key || type == TLSEXT_TYPE_key_share)
+		return 1;
+	if (!tls13_clienthello_hash_update(ctx, cbs))
+		return 0;
+
+	return 1;
+}
+
 static int
 tlsext_parse(SSL *s, CBS *cbs, int *alert, int is_server, uint16_t msg_type)
 {
@@ -2098,6 +2125,13 @@ tlsext_parse(SSL *s, CBS *cbs, int *alert, int is_server, uint16_t msg_type)
 			    CBS_len(&extension_data),
 			    s->internal->tlsext_debug_arg);
 
+		if (!SSL_IS_DTLS(s) && version >= TLS1_3_VERSION && is_server &&
+		    msg_type == SSL_TLSEXT_MSG_CH) {
+			if (!tlsext_clienthello_hash_extension(s, type,
+			    &extension_data))
+				goto err;
+		}
+
 		/* Unknown extensions are ignored. */
 		if ((tlsext = tls_extension_find(type, &idx)) == NULL)
 			continue;
diff --git a/lib/libssl/tls13_internal.h b/lib/libssl/tls13_internal.h
index 96ed9819594..a18184f505a 100644
--- a/lib/libssl/tls13_internal.h
+++ b/lib/libssl/tls13_internal.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_internal.h,v 1.83 2020/05/29 17:47:30 jsing Exp $ */
+/* $OpenBSD: tls13_internal.h,v 1.84 2020/06/06 01:40:09 beck Exp $ */
 /*
  * Copyright (c) 2018 Bob Beck <beck@openbsd.org>
  * Copyright (c) 2018 Theo Buehler <tb@openbsd.org>
@@ -386,6 +386,13 @@ int tls13_cert_add(struct tls13_ctx *ctx, CBB *cbb, X509 *cert,
     int(*build_extensions)(SSL *s, CBB *cbb, uint16_t msg_type));
 
 int tls13_synthetic_handshake_message(struct tls13_ctx *ctx);
+int tls13_clienthello_hash_init(struct tls13_ctx *ctx);
+void tls13_clienthello_hash_clear(struct ssl_handshake_tls13_st *hs);
+int tls13_clienthello_hash_update_bytes(struct tls13_ctx *ctx, void *data,
+    size_t len);
+int tls13_clienthello_hash_update(struct tls13_ctx *ctx, CBS *cbs);
+int tls13_clienthello_hash_finalize(struct tls13_ctx *ctx);
+int tls13_clienthello_hash_validate(struct tls13_ctx *ctx);
 
 int tls13_error_set(struct tls13_error *error, int code, int subcode,
     const char *file, int line, const char *fmt, ...);
diff --git a/lib/libssl/tls13_lib.c b/lib/libssl/tls13_lib.c
index 174da2f9c3c..b5939aecab0 100644
--- a/lib/libssl/tls13_lib.c
+++ b/lib/libssl/tls13_lib.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tls13_lib.c,v 1.50 2020/05/22 02:37:27 beck Exp $ */
+/*	$OpenBSD: tls13_lib.c,v 1.51 2020/06/06 01:40:09 beck Exp $ */
 /*
  * Copyright (c) 2018, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2019 Bob Beck <beck@openbsd.org>
@@ -486,3 +486,82 @@ tls13_synthetic_handshake_message(struct tls13_ctx *ctx)
 
 	return ret;
 }
+
+int
+tls13_clienthello_hash_init(struct tls13_ctx *ctx)
+{
+	if (ctx->hs->clienthello_md_ctx != NULL)
+		return 0;
+	if ((ctx->hs->clienthello_md_ctx = EVP_MD_CTX_new()) == NULL)
+		return 0;
+	if (!EVP_DigestInit_ex(ctx->hs->clienthello_md_ctx,
+	    EVP_sha256(), NULL))
+		return 0;
+
+	if ((ctx->hs->clienthello_hash == NULL) &&
+	    (ctx->hs->clienthello_hash = calloc(1, EVP_MAX_MD_SIZE)) ==
+	    NULL)
+		return 0;
+
+	return 1;
+}
+
+void
+tls13_clienthello_hash_clear(struct ssl_handshake_tls13_st *hs)
+{
+	EVP_MD_CTX_free(hs->clienthello_md_ctx);
+	hs->clienthello_md_ctx = NULL;
+	freezero(hs->clienthello_hash, EVP_MAX_MD_SIZE);
+	hs->clienthello_hash = NULL;
+}
+
+int
+tls13_clienthello_hash_update_bytes(struct tls13_ctx *ctx, void *data,
+    size_t len)
+{
+	return EVP_DigestUpdate(ctx->hs->clienthello_md_ctx, data, len);
+}
+
+int
+tls13_clienthello_hash_update(struct tls13_ctx *ctx, CBS *cbs)
+{
+	return tls13_clienthello_hash_update_bytes(ctx, (void *)CBS_data(cbs),
+	    CBS_len(cbs));
+}
+
+int
+tls13_clienthello_hash_finalize(struct tls13_ctx *ctx)
+{
+	if (!EVP_DigestFinal_ex(ctx->hs->clienthello_md_ctx,
+	    ctx->hs->clienthello_hash,
+	    &ctx->hs->clienthello_hash_len))
+		return 0;
+	EVP_MD_CTX_free(ctx->hs->clienthello_md_ctx);
+	ctx->hs->clienthello_md_ctx = NULL;
+	return 1;
+}
+
+int
+tls13_clienthello_hash_validate(struct tls13_ctx *ctx)
+{
+	unsigned char new_ch_hash[EVP_MAX_MD_SIZE];
+	unsigned int new_ch_hash_len;
+
+	if (ctx->hs->clienthello_hash == NULL)
+		return 0;
+
+	if (!EVP_DigestFinal_ex(ctx->hs->clienthello_md_ctx,
+	    new_ch_hash, &new_ch_hash_len))
+		return 0;
+	EVP_MD_CTX_free(ctx->hs->clienthello_md_ctx);
+	ctx->hs->clienthello_md_ctx = NULL;
+
+	if (ctx->hs->clienthello_hash_len != new_ch_hash_len)
+		return 0;
+	if (memcmp(ctx->hs->clienthello_hash, new_ch_hash,
+	    new_ch_hash_len) != 0)
+		return 0;
+
+	return 1;
+}
+
diff --git a/lib/libssl/tls13_server.c b/lib/libssl/tls13_server.c
index edc87fcdcbd..ccbb46652bd 100644
--- a/lib/libssl/tls13_server.c
+++ b/lib/libssl/tls13_server.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_server.c,v 1.57 2020/06/04 18:46:21 tb Exp $ */
+/* $OpenBSD: tls13_server.c,v 1.58 2020/06/06 01:40:09 beck Exp $ */
 /*
  * Copyright (c) 2019, 2020 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
@@ -126,11 +126,52 @@ tls13_client_hello_process(struct tls13_ctx *ctx, CBS *cbs)
 		return tls13_use_legacy_server(ctx);
 	}
 
+	/* Add decoded values to the current ClientHello hash */
+	if (!tls13_clienthello_hash_init(ctx)) {
+		ctx->alert = TLS13_ALERT_INTERNAL_ERROR;
+		goto err;
+	}
+	if (!tls13_clienthello_hash_update_bytes(ctx, (void *)&legacy_version,
+	    sizeof(legacy_version))) {
+		ctx->alert = TLS13_ALERT_INTERNAL_ERROR;
+		goto err;
+	}
+	if (!tls13_clienthello_hash_update(ctx, &client_random)) {
+		ctx->alert = TLS13_ALERT_INTERNAL_ERROR;
+		goto err;
+	}
+	if (!tls13_clienthello_hash_update(ctx, &session_id)) {
+		ctx->alert = TLS13_ALERT_INTERNAL_ERROR;
+		goto err;
+	}
+	if (!tls13_clienthello_hash_update(ctx, &cipher_suites)) {
+		ctx->alert = TLS13_ALERT_INTERNAL_ERROR;
+		goto err;
+	}
+	if (!tls13_clienthello_hash_update(ctx, &compression_methods)) {
+		ctx->alert = TLS13_ALERT_INTERNAL_ERROR;
+		goto err;
+	}
+
 	if (!tlsext_server_parse(s, cbs, &alert_desc, SSL_TLSEXT_MSG_CH)) {
 		ctx->alert = alert_desc;
 		goto err;
 	}
 
+	/* Finalize first ClientHello hash, or validate against it */
+	if (!ctx->hs->hrr) {
+		if (!tls13_clienthello_hash_finalize(ctx)) {
+			ctx->alert = TLS13_ALERT_INTERNAL_ERROR;
+			goto err;
+		}
+	} else {
+		if (!tls13_clienthello_hash_validate(ctx)) {
+			ctx->alert = TLS13_ALERT_ILLEGAL_PARAMETER;
+			goto err;
+		}
+		tls13_clienthello_hash_clear(ctx->hs);
+	}
+
 	/*
 	 * If we got this far we have a supported versions extension that offers
 	 * TLS 1.3 or later. This requires the legacy version be set to 0x0303.
@@ -146,8 +187,11 @@ tls13_client_hello_process(struct tls13_ctx *ctx, CBS *cbs)
 		goto err;
 	}
 	if (!CBS_write_bytes(&session_id, ctx->hs->legacy_session_id,
-	    sizeof(ctx->hs->legacy_session_id), &ctx->hs->legacy_session_id_len))
+	    sizeof(ctx->hs->legacy_session_id),
+	    &ctx->hs->legacy_session_id_len)) {
+		ctx->alert = TLS13_ALERT_INTERNAL_ERROR;
 		goto err;
+	}
 
 	/* Parse cipher suites list and select preferred cipher. */
 	if ((ciphers = ssl_bytes_to_cipher_list(s, &cipher_suites)) == NULL) {
diff --git a/regress/lib/libssl/tlsfuzzer/tlsfuzzer.py b/regress/lib/libssl/tlsfuzzer/tlsfuzzer.py
index c1e89bd43ba..f81bf558f4c 100644
--- a/regress/lib/libssl/tlsfuzzer/tlsfuzzer.py
+++ b/regress/lib/libssl/tlsfuzzer/tlsfuzzer.py
@@ -1,4 +1,4 @@
-#   $OpenBSD: tlsfuzzer.py,v 1.6 2020/06/03 04:47:03 tb Exp $
+#   $OpenBSD: tlsfuzzer.py,v 1.7 2020/06/06 01:40:08 beck Exp $
 #
 # Copyright (c) 2020 Theo Buehler <tb@openbsd.org>
 #
@@ -79,6 +79,7 @@ tls13_tests = TestGroup("TLSv1.3 tests", [
     Test("test-tls13-legacy-version.py"),
     Test("test-tls13-nociphers.py"),
     Test("test-tls13-record-padding.py"),
+    Test("test-tls13-shuffled-extentions.py"),
 
     # The skipped tests fail due to a bug in BIO_gets() which masks the retry
     # signalled from an SSL_read() failure. Testing with httpd(8) shows we're
@@ -145,7 +146,6 @@ tls13_failing_tests = TestGroup("failing TLSv1.3 tests", [
 
 tls13_slow_failing_tests = TestGroup("slow, failing TLSv1.3 tests", [
     # Other test failures bugs in keyshare/tlsext negotiation?
-    Test("test-tls13-shuffled-extentions.py"),    # should reject 2nd CH
     Test("test-tls13-unrecognised-groups.py"),    # unexpected closure
 
     # 5 failures: