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/src/lib/libssl/s3_lib.c b/src/lib/libssl/s3_lib.c
index e2fef72588..c2cf922973 100644
--- a/src/lib/libssl/s3_lib.c
+++ b/src/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/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index bfc3c1ad9b..bf1f846d13 100644
--- a/src/lib/libssl/ssl_locl.h
+++ b/src/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/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c
index cf54fc4d2c..f6943c83ae 100644
--- a/src/lib/libssl/ssl_tlsext.c
+++ b/src/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/src/lib/libssl/tls13_internal.h b/src/lib/libssl/tls13_internal.h
index 96ed981959..a18184f505 100644
--- a/src/lib/libssl/tls13_internal.h
+++ b/src/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/src/lib/libssl/tls13_lib.c b/src/lib/libssl/tls13_lib.c
index 174da2f9c3..b5939aecab 100644
--- a/src/lib/libssl/tls13_lib.c
+++ b/src/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/src/lib/libssl/tls13_server.c b/src/lib/libssl/tls13_server.c
index edc87fcdcb..ccbb46652b 100644
--- a/src/lib/libssl/tls13_server.c
+++ b/src/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/src/regress/lib/libssl/tlsfuzzer/tlsfuzzer.py b/src/regress/lib/libssl/tlsfuzzer/tlsfuzzer.py
index c1e89bd43b..f81bf558f4 100644
--- a/src/regress/lib/libssl/tlsfuzzer/tlsfuzzer.py
+++ b/src/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: