Pass a handshake message content CBS to TLSv1.3 receive handlers.

This avoids every receive handler from having to get the handshake message
content itself. Additionally, pull the trailing data check up so that each
receive handler does not have to implement it. This makes the code more
readable and reduces duplication.

ok beck@ tb@

5 files changed, 70 insertions, 85 deletions

diff --git a/src/lib/libssl/tls13_client.c b/src/lib/libssl/tls13_client.c
index 3648d09b22..4ec5e58f02 100644
--- a/src/lib/libssl/tls13_client.c
+++ b/src/lib/libssl/tls13_client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_client.c,v 1.27 2020/01/22 11:26:47 beck Exp $ */
+/* $OpenBSD: tls13_client.c,v 1.28 2020/01/22 13:10:51 jsing Exp $ */
 /*
  * Copyright (c) 2018, 2019 Joel Sing <jsing@openbsd.org>
  *
@@ -288,17 +288,17 @@ tls13_server_hello_process(struct tls13_ctx *ctx, CBS *cbs)
         if (!CBS_get_u8(cbs, &compression_method))
                 goto err;
 
-        if (tls13_server_hello_is_legacy(cbs))
+        if (tls13_server_hello_is_legacy(cbs)) {
+                if (!CBS_skip(cbs, CBS_len(cbs)))
+                        goto err;
                 return tls13_use_legacy_client(ctx);
+        }
 
         if (!tlsext_client_parse(s, cbs, &alert_desc, SSL_TLSEXT_MSG_SH)) {
                 ctx->alert = alert_desc;
                 goto err;
         }
 
-        if (CBS_len(cbs) != 0)
-                goto err;
-
         /*
          * See if a supported versions extension was returned. If it was then
          * the legacy version must be set to 0x0303 (RFC 8446 section 4.1.3).
@@ -359,7 +359,7 @@ tls13_server_hello_process(struct tls13_ctx *ctx, CBS *cbs)
 }
 
 int
-tls13_server_hello_recv(struct tls13_ctx *ctx)
+tls13_server_hello_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         struct tls13_secrets *secrets;
         struct tls13_secret context;
@@ -368,12 +368,8 @@ tls13_server_hello_recv(struct tls13_ctx *ctx)
         size_t hash_len;
         SSL *s = ctx->ssl;
         int ret = 0;
-        CBS cbs;
-
-        if (!tls13_handshake_msg_content(ctx->hs_msg, &cbs))
-                goto err;
 
-        if (!tls13_server_hello_process(ctx, &cbs))
+        if (!tls13_server_hello_process(ctx, cbs))
                 goto err;
 
         /* See if we switched back to the legacy client method. */
@@ -440,22 +436,15 @@ tls13_server_hello_recv(struct tls13_ctx *ctx)
 }
 
 int
-tls13_server_encrypted_extensions_recv(struct tls13_ctx *ctx)
+tls13_server_encrypted_extensions_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
-        CBS cbs;
         int alert_desc;
 
-        if (!tls13_handshake_msg_content(ctx->hs_msg, &cbs))
-                goto err;
-
-        if (!tlsext_client_parse(ctx->ssl, &cbs, &alert_desc, SSL_TLSEXT_MSG_EE)) {
+        if (!tlsext_client_parse(ctx->ssl, cbs, &alert_desc, SSL_TLSEXT_MSG_EE)) {
                 ctx->alert = alert_desc;
                 goto err;
         }
 
-        if (CBS_len(&cbs) != 0)
-                goto err;
-
         return 1;
 
  err:
@@ -465,7 +454,7 @@ tls13_server_encrypted_extensions_recv(struct tls13_ctx *ctx)
 }
 
 int
-tls13_server_certificate_request_recv(struct tls13_ctx *ctx)
+tls13_server_certificate_request_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         /*
          * Thanks to poor state design in the RFC, this function can be called
@@ -475,7 +464,7 @@ tls13_server_certificate_request_recv(struct tls13_ctx *ctx)
          */
         if (tls13_handshake_msg_type(ctx->hs_msg) == TLS13_MT_CERTIFICATE) {
                 ctx->handshake_stage.hs_type |= WITHOUT_CR;
-                return tls13_server_certificate_recv(ctx);
+                return tls13_server_certificate_recv(ctx, cbs);
         }
 
         /* XXX - unimplemented. */
@@ -484,9 +473,9 @@ tls13_server_certificate_request_recv(struct tls13_ctx *ctx)
 }
 
 int
-tls13_server_certificate_recv(struct tls13_ctx *ctx)
+tls13_server_certificate_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
-        CBS cbs, cert_request_context, cert_list, cert_data, cert_exts;
+        CBS cert_request_context, cert_list, cert_data, cert_exts;
         struct stack_st_X509 *certs = NULL;
         SSL *s = ctx->ssl;
         X509 *cert = NULL;
@@ -498,16 +487,11 @@ tls13_server_certificate_recv(struct tls13_ctx *ctx)
         if ((certs = sk_X509_new_null()) == NULL)
                 goto err;
 
-        if (!tls13_handshake_msg_content(ctx->hs_msg, &cbs))
-                goto err;
-
-        if (!CBS_get_u8_length_prefixed(&cbs, &cert_request_context))
+        if (!CBS_get_u8_length_prefixed(cbs, &cert_request_context))
                 goto err;
         if (CBS_len(&cert_request_context) != 0)
                 goto err;
-        if (!CBS_get_u24_length_prefixed(&cbs, &cert_list))
-                goto err;
-        if (CBS_len(&cbs) != 0)
+        if (!CBS_get_u24_length_prefixed(cbs, &cert_list))
                 goto err;
 
         while (CBS_len(&cert_list) > 0) {
@@ -595,7 +579,7 @@ static uint8_t cert_verify_pad[64] = {
 static uint8_t server_cert_verify_context[] = "TLS 1.3, server CertificateVerify";
 
 int
-tls13_server_certificate_verify_recv(struct tls13_ctx *ctx)
+tls13_server_certificate_verify_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         const struct ssl_sigalg *sigalg;
         uint16_t signature_scheme;
@@ -605,20 +589,15 @@ tls13_server_certificate_verify_recv(struct tls13_ctx *ctx)
         EVP_PKEY_CTX *pctx;
         EVP_PKEY *pkey;
         X509 *cert;
-        CBS cbs, signature;
+        CBS signature;
         CBB cbb;
         int ret = 0;
 
         memset(&cbb, 0, sizeof(cbb));
 
-        if (!tls13_handshake_msg_content(ctx->hs_msg, &cbs))
-                goto err;
-
-        if (!CBS_get_u16(&cbs, &signature_scheme))
-                goto err;
-        if (!CBS_get_u16_length_prefixed(&cbs, &signature))
+        if (!CBS_get_u16(cbs, &signature_scheme))
                 goto err;
-        if (CBS_len(&cbs) != 0)
+        if (!CBS_get_u16_length_prefixed(cbs, &signature))
                 goto err;
 
         if ((sigalg = ssl_sigalg(signature_scheme, tls13_sigalgs,
@@ -680,7 +659,7 @@ tls13_server_certificate_verify_recv(struct tls13_ctx *ctx)
 }
 
 int
-tls13_server_finished_recv(struct tls13_ctx *ctx)
+tls13_server_finished_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         struct tls13_secrets *secrets = ctx->hs->secrets;
         struct tls13_secret context = { .data = "", .len = 0 };
@@ -693,10 +672,6 @@ tls13_server_finished_recv(struct tls13_ctx *ctx)
         HMAC_CTX *hmac_ctx = NULL;
         unsigned int hlen;
         int ret = 0;
-        CBS cbs;
-
-        if (!tls13_handshake_msg_content(ctx->hs_msg, &cbs))
-                goto err;
 
         /*
          * Verify server finished.
@@ -725,11 +700,14 @@ tls13_server_finished_recv(struct tls13_ctx *ctx)
         if (hlen != verify_data_len)
                 goto err;
 
-        if (!CBS_mem_equal(&cbs, verify_data, verify_data_len)) {
+        if (!CBS_mem_equal(cbs, verify_data, verify_data_len)) {
                 ctx->alert = TLS1_AD_DECRYPTION_FAILED;
                 goto err;
         }
 
+        if (!CBS_skip(cbs, verify_data_len))
+                goto err;
+
         /*
          * Derive application traffic keys.
          */
@@ -864,9 +842,6 @@ tls13_client_hello_retry_process(struct tls13_ctx *ctx, CBS *cbs)
                 goto err;
         }
 
-        if (CBS_len(cbs) != 0)
-                goto err;
-
         /* XXX for now, just say no, we will not change our hello */
         ctx->alert = SSL_AD_ILLEGAL_PARAMETER;
  err:
@@ -876,15 +851,11 @@ tls13_client_hello_retry_process(struct tls13_ctx *ctx, CBS *cbs)
 }
 
 int
-tls13_client_hello_retry_recv(struct tls13_ctx *ctx)
+tls13_client_hello_retry_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         int ret = 0;
-        CBS cbs;
-
-        if (!tls13_handshake_msg_content(ctx->hs_msg, &cbs))
-                goto err;
 
-        if (!tls13_client_hello_retry_process(ctx, &cbs)) {
+        if (!tls13_client_hello_retry_process(ctx, cbs)) {
                 if (ctx->alert == SSL_AD_ILLEGAL_PARAMETER)
                         tls13_set_errorx(ctx, TLS13_ERR_HRR_FAILED, 0,
                             "Unsatisfiable hello retry request", NULL);
diff --git a/src/lib/libssl/tls13_handshake.c b/src/lib/libssl/tls13_handshake.c
index ca36f879b4..d4d998248d 100644
--- a/src/lib/libssl/tls13_handshake.c
+++ b/src/lib/libssl/tls13_handshake.c
@@ -1,4 +1,4 @@
-/*      $OpenBSD: tls13_handshake.c,v 1.39 2020/01/22 02:39:45 tb Exp $ */
+/*      $OpenBSD: tls13_handshake.c,v 1.40 2020/01/22 13:10:51 jsing Exp $      */
 /*
  * Copyright (c) 2018-2019 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2019 Joel Sing <jsing@openbsd.org>
@@ -32,7 +32,7 @@ struct tls13_handshake_action {
 
         int (*send)(struct tls13_ctx *ctx);
         int (*sent)(struct tls13_ctx *ctx);
-        int (*recv)(struct tls13_ctx *ctx);
+        int (*recv)(struct tls13_ctx *ctx, CBS *cbs);
 };
 
 enum tls13_message_type tls13_handshake_active_state(struct tls13_ctx *ctx);
@@ -389,11 +389,21 @@ tls13_handshake_recv_action(struct tls13_ctx *ctx,
              action->handshake_type != TLS13_MT_CERTIFICATE_REQUEST))
                 return tls13_send_alert(ctx->rl, SSL_AD_UNEXPECTED_MESSAGE);
 
-        /* XXX provide CBS and check all consumed. */
+        if (!tls13_handshake_msg_content(ctx->hs_msg, &cbs))
+                return TLS13_IO_FAILURE;
+
         ret = TLS13_IO_FAILURE;
-        if (action->recv(ctx))
-                ret = TLS13_IO_SUCCESS;
-        else if (ctx->alert)
+        if (action->recv(ctx, &cbs)) {
+                if (CBS_len(&cbs) != 0) {
+                        tls13_set_errorx(ctx, TLS13_ERR_TRAILING_DATA, 0,
+                            "trailing data in handshake message", NULL);
+                        ctx->alert = SSL_AD_DECODE_ERROR;
+                } else {
+                        ret = TLS13_IO_SUCCESS;
+                }
+        }
+
+        if (ctx->alert)
                 ret = tls13_send_alert(ctx->rl, ctx->alert);
 
         tls13_handshake_msg_free(ctx->hs_msg);
diff --git a/src/lib/libssl/tls13_internal.h b/src/lib/libssl/tls13_internal.h
index 68a129a634..ba34961e33 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.44 2020/01/22 06:23:00 jsing Exp $ */
+/* $OpenBSD: tls13_internal.h,v 1.45 2020/01/22 13:10:51 jsing Exp $ */
 /*
  * Copyright (c) 2018 Bob Beck <beck@openbsd.org>
  * Copyright (c) 2018 Theo Buehler <tb@openbsd.org>
@@ -39,6 +39,7 @@ __BEGIN_HIDDEN_DECLS
 
 #define TLS13_ERR_VERIFY_FAILED 16
 #define TLS13_ERR_HRR_FAILED    17
+#define TLS13_ERR_TRAILING_DATA 18
 
 typedef void (*tls13_alert_cb)(uint8_t _alert_desc, void *_cb_arg);
 typedef ssize_t (*tls13_phh_recv_cb)(void *_cb_arg, CBS *cbs);
@@ -258,33 +259,33 @@ int tls13_handshake_perform(struct tls13_ctx *ctx);
 
 int tls13_client_hello_send(struct tls13_ctx *ctx);
 int tls13_client_hello_sent(struct tls13_ctx *ctx);
-int tls13_client_hello_recv(struct tls13_ctx *ctx);
+int tls13_client_hello_recv(struct tls13_ctx *ctx, CBS *cbs);
 int tls13_client_hello_retry_send(struct tls13_ctx *ctx);
-int tls13_client_hello_retry_recv(struct tls13_ctx *ctx);
+int tls13_client_hello_retry_recv(struct tls13_ctx *ctx, CBS *cbs);
 int tls13_client_end_of_early_data_send(struct tls13_ctx *ctx);
-int tls13_client_end_of_early_data_recv(struct tls13_ctx *ctx);
+int tls13_client_end_of_early_data_recv(struct tls13_ctx *ctx, CBS *cbs);
 int tls13_client_certificate_send(struct tls13_ctx *ctx);
-int tls13_client_certificate_recv(struct tls13_ctx *ctx);
+int tls13_client_certificate_recv(struct tls13_ctx *ctx, CBS *cbs);
 int tls13_client_certificate_verify_send(struct tls13_ctx *ctx);
-int tls13_client_certificate_verify_recv(struct tls13_ctx *ctx);
-int tls13_client_finished_recv(struct tls13_ctx *ctx);
+int tls13_client_certificate_verify_recv(struct tls13_ctx *ctx, CBS *cbs);
+int tls13_client_finished_recv(struct tls13_ctx *ctx, CBS *cbs);
 int tls13_client_finished_send(struct tls13_ctx *ctx);
 int tls13_client_finished_sent(struct tls13_ctx *ctx);
 int tls13_client_key_update_send(struct tls13_ctx *ctx);
-int tls13_client_key_update_recv(struct tls13_ctx *ctx);
-int tls13_server_hello_recv(struct tls13_ctx *ctx);
+int tls13_client_key_update_recv(struct tls13_ctx *ctx, CBS *cbs);
+int tls13_server_hello_recv(struct tls13_ctx *ctx, CBS *cbs);
 int tls13_server_hello_send(struct tls13_ctx *ctx);
-int tls13_server_hello_retry_recv(struct tls13_ctx *ctx);
+int tls13_server_hello_retry_recv(struct tls13_ctx *ctx, CBS *cbs);
 int tls13_server_hello_retry_send(struct tls13_ctx *ctx);
-int tls13_server_encrypted_extensions_recv(struct tls13_ctx *ctx);
+int tls13_server_encrypted_extensions_recv(struct tls13_ctx *ctx, CBS *cbs);
 int tls13_server_encrypted_extensions_send(struct tls13_ctx *ctx);
-int tls13_server_certificate_recv(struct tls13_ctx *ctx);
+int tls13_server_certificate_recv(struct tls13_ctx *ctx, CBS *cbs);
 int tls13_server_certificate_send(struct tls13_ctx *ctx);
-int tls13_server_certificate_request_recv(struct tls13_ctx *ctx);
+int tls13_server_certificate_request_recv(struct tls13_ctx *ctx, CBS *cbs);
 int tls13_server_certificate_request_send(struct tls13_ctx *ctx);
 int tls13_server_certificate_verify_send(struct tls13_ctx *ctx);
-int tls13_server_certificate_verify_recv(struct tls13_ctx *ctx);
-int tls13_server_finished_recv(struct tls13_ctx *ctx);
+int tls13_server_certificate_verify_recv(struct tls13_ctx *ctx, CBS *cbs);
+int tls13_server_finished_recv(struct tls13_ctx *ctx, CBS *cbs);
 int tls13_server_finished_send(struct tls13_ctx *ctx);
 
 void tls13_error_clear(struct tls13_error *error);
diff --git a/src/lib/libssl/tls13_lib.c b/src/lib/libssl/tls13_lib.c
index 73d936ac3f..51a2a383ed 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.20 2020/01/22 06:23:00 jsing Exp $ */
+/*      $OpenBSD: tls13_lib.c,v 1.21 2020/01/22 13:10:51 jsing Exp $ */
 /*
  * Copyright (c) 2018, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2019 Bob Beck <beck@openbsd.org>
@@ -356,6 +356,9 @@ tls13_legacy_error(SSL *ssl)
         case TLS13_ERR_HRR_FAILED:
                 reason = SSL_R_NO_CIPHERS_AVAILABLE;
                 break;
+        case TLS13_ERR_TRAILING_DATA:
+                reason = SSL_R_EXTRA_DATA_IN_MESSAGE;
+                break;
         }
 
         ERR_put_error(ERR_LIB_SSL, (0xfff), reason, ctx->error.file,
diff --git a/src/lib/libssl/tls13_server.c b/src/lib/libssl/tls13_server.c
index fc3e80ad58..90a339dc61 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.5 2020/01/22 05:06:23 tb Exp $ */
+/* $OpenBSD: tls13_server.c,v 1.6 2020/01/22 13:10:51 jsing Exp $ */
 /*
  * Copyright (c) 2019 Joel Sing <jsing@openbsd.org>
  *
@@ -79,7 +79,7 @@ tls13_legacy_accept(SSL *ssl)
 }
 
 int
-tls13_client_hello_recv(struct tls13_ctx *ctx)
+tls13_client_hello_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         tls13_record_layer_allow_ccs(ctx->rl, 1);
 
@@ -93,7 +93,7 @@ tls13_client_hello_retry_send(struct tls13_ctx *ctx)
 }
 
 int
-tls13_server_hello_retry_recv(struct tls13_ctx *ctx)
+tls13_server_hello_retry_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         return 0;
 }
@@ -105,7 +105,7 @@ tls13_client_end_of_early_data_send(struct tls13_ctx *ctx)
 }
 
 int
-tls13_client_end_of_early_data_recv(struct tls13_ctx *ctx)
+tls13_client_end_of_early_data_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         return 0;
 }
@@ -117,7 +117,7 @@ tls13_client_certificate_send(struct tls13_ctx *ctx)
 }
 
 int
-tls13_client_certificate_recv(struct tls13_ctx *ctx)
+tls13_client_certificate_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         return 0;
 }
@@ -129,13 +129,13 @@ tls13_client_certificate_verify_send(struct tls13_ctx *ctx)
 }
 
 int
-tls13_client_certificate_verify_recv(struct tls13_ctx *ctx)
+tls13_client_certificate_verify_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         return 0;
 }
 
 int
-tls13_client_finished_recv(struct tls13_ctx *ctx)
+tls13_client_finished_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         tls13_record_layer_allow_ccs(ctx->rl, 0);
 
@@ -149,7 +149,7 @@ tls13_client_key_update_send(struct tls13_ctx *ctx)
 }
 
 int
-tls13_client_key_update_recv(struct tls13_ctx *ctx)
+tls13_client_key_update_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         return 0;
 }