Clean up and harden TLSv1.2 master key derivation.

The master key and its length are only stored in one location, so it makes
no sense to handle these outside of the derivation function (the current
'out' argument is unused). This simplifies the various call sites.

If derivation fails for some reason, fail hard rather than continuing on
and hoping that something deals with this correctly later.

ok inoguchi@ tb@

5 files changed, 51 insertions, 54 deletions

diff --git a/src/lib/libssl/ssl_clnt.c b/src/lib/libssl/ssl_clnt.c
index c129bb6d66..a38d1f1ed4 100644
--- a/src/lib/libssl/ssl_clnt.c
+++ b/src/lib/libssl/ssl_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.93 2021/04/25 13:15:22 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.94 2021/04/30 19:26:44 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -2001,9 +2001,8 @@ ssl3_send_client_kex_rsa(SSL *s, SESS_CERT *sess_cert, CBB *cbb)
         if (!CBB_flush(cbb))
                 goto err;
 
-        s->session->master_key_length =
-            tls1_generate_master_secret(s,
-                s->session->master_key, pms, sizeof(pms));
+        if (!tls12_derive_master_secret(s, pms, sizeof(pms)))
+                goto err;
 
         ret = 1;
 
@@ -2055,10 +2054,8 @@ ssl3_send_client_kex_dhe(SSL *s, SESS_CERT *sess_cert, CBB *cbb)
                 goto err;
         }
 
-        /* Generate master key from the result. */
-        s->session->master_key_length =
-            tls1_generate_master_secret(s,
-                s->session->master_key, key, key_len);
+        if (!tls12_derive_master_secret(s, key, key_len))
+                goto err;
 
         if (!CBB_add_u16_length_prefixed(cbb, &dh_Yc))
                 goto err;
@@ -2104,8 +2101,8 @@ ssl3_send_client_kex_ecdhe_ecp(SSL *s, SESS_CERT *sc, CBB *cbb)
 
         if (!ssl_kex_derive_ecdhe_ecp(ecdh, sc->peer_ecdh_tmp, &key, &key_len))
                 goto err;
-        s->session->master_key_length = tls1_generate_master_secret(s,
-                s->session->master_key, key, key_len);
+        if (!tls12_derive_master_secret(s, key, key_len))
+                goto err;
 
         ret = 1;
 
@@ -2142,10 +2139,8 @@ ssl3_send_client_kex_ecdhe_ecx(SSL *s, SESS_CERT *sc, CBB *cbb)
         if (!CBB_flush(cbb))
                 goto err;
 
-        /* Generate master key from the result. */
-        s->session->master_key_length =
-            tls1_generate_master_secret(s,
-                s->session->master_key, shared_key, X25519_KEY_LENGTH);
+        if (!tls12_derive_master_secret(s, shared_key, X25519_KEY_LENGTH))
+                goto err;
 
         ret = 1;
 
@@ -2276,9 +2271,9 @@ ssl3_send_client_kex_gost(SSL *s, SESS_CERT *sess_cert, CBB *cbb)
                 s->s3->flags |= TLS1_FLAGS_SKIP_CERT_VERIFY;
         }
         EVP_PKEY_CTX_free(pkey_ctx);
-        s->session->master_key_length =
-            tls1_generate_master_secret(s,
-                s->session->master_key, premaster_secret, 32);
+
+        if (!tls12_derive_master_secret(s, premaster_secret, 32))
+                goto err;
 
         ret = 1;
 
diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index a9cab69ee0..c55dada70f 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.338 2021/04/25 13:15:22 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.339 2021/04/30 19:26:44 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1379,8 +1379,6 @@ int tls1_PRF(SSL *s, const unsigned char *secret, size_t secret_len,
 void tls1_cleanup_key_block(SSL *s);
 int tls1_change_cipher_state(SSL *s, int which);
 int tls1_setup_key_block(SSL *s);
-int tls1_generate_master_secret(SSL *s, unsigned char *out,
-    unsigned char *p, int len);
 int tls1_export_keying_material(SSL *s, unsigned char *out, size_t olen,
     const char *label, size_t llen, const unsigned char *p, size_t plen,
     int use_context);
@@ -1389,6 +1387,8 @@ int ssl_ok(SSL *s);
 
 int tls12_derive_finished(SSL *s);
 int tls12_derive_peer_finished(SSL *s);
+int tls12_derive_master_secret(SSL *s, uint8_t *premaster_secret,
+    size_t premaster_secret_len);
 
 int ssl_using_ecc_cipher(SSL *s);
 int ssl_check_srvr_ecc_cert_and_alg(X509 *x, SSL *s);
diff --git a/src/lib/libssl/ssl_srvr.c b/src/lib/libssl/ssl_srvr.c
index 2c15081f45..32ffa88f15 100644
--- a/src/lib/libssl/ssl_srvr.c
+++ b/src/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.104 2021/04/25 13:15:22 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.105 2021/04/30 19:26:45 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1797,9 +1797,8 @@ ssl3_get_client_kex_rsa(SSL *s, CBS *cbs)
                 p = fakekey;
         }
 
-        s->session->master_key_length =
-            tls1_generate_master_secret(s,
-                s->session->master_key, p, SSL_MAX_MASTER_KEY_LENGTH);
+        if (!tls12_derive_master_secret(s, p, SSL_MAX_MASTER_KEY_LENGTH))
+                goto err;
 
         freezero(pms, pms_len);
 
@@ -1867,8 +1866,8 @@ ssl3_get_client_kex_dhe(SSL *s, CBS *cbs)
                 goto fatal_err;
         }
 
-        s->session->master_key_length = tls1_generate_master_secret(s,
-            s->session->master_key, key, key_len);
+        if (!tls12_derive_master_secret(s, key, key_len))
+                goto err;
 
         DH_free(S3I(s)->tmp.dh);
         S3I(s)->tmp.dh = NULL;
@@ -1928,8 +1927,8 @@ ssl3_get_client_kex_ecdhe_ecp(SSL *s, CBS *cbs)
         /* Derive the shared secret and compute master secret. */
         if (!ssl_kex_derive_ecdhe_ecp(ecdh, ecdh_peer, &key, &key_len))
                 goto err;
-        s->session->master_key_length = tls1_generate_master_secret(s,
-            s->session->master_key, key, key_len);
+        if (!tls12_derive_master_secret(s, key, key_len))
+                goto err;
 
         EC_KEY_free(S3I(s)->tmp.ecdh);
         S3I(s)->tmp.ecdh = NULL;
@@ -1966,9 +1965,8 @@ ssl3_get_client_kex_ecdhe_ecx(SSL *s, CBS *cbs)
         freezero(S3I(s)->tmp.x25519, X25519_KEY_LENGTH);
         S3I(s)->tmp.x25519 = NULL;
 
-        s->session->master_key_length =
-            tls1_generate_master_secret(
-                s, s->session->master_key, shared_key, X25519_KEY_LENGTH);
+        if (!tls12_derive_master_secret(s, shared_key, X25519_KEY_LENGTH))
+                goto err;
 
         ret = 1;
 
@@ -2033,10 +2031,8 @@ ssl3_get_client_kex_gost(SSL *s, CBS *cbs)
                 goto gerr;
         }
 
-        /* Generate master secret */
-        s->session->master_key_length =
-            tls1_generate_master_secret(
-                s, s->session->master_key, premaster_secret, 32);
+        if (!tls12_derive_master_secret(s, premaster_secret, 32))
+                goto err;
 
         /* Check if pubkey from client certificate was used */
         if (EVP_PKEY_CTX_ctrl(pkey_ctx, -1, -1,
diff --git a/src/lib/libssl/t1_enc.c b/src/lib/libssl/t1_enc.c
index 3f93bcecf5..642c210900 100644
--- a/src/lib/libssl/t1_enc.c
+++ b/src/lib/libssl/t1_enc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_enc.c,v 1.139 2021/04/25 13:15:22 jsing Exp $ */
+/* $OpenBSD: t1_enc.c,v 1.140 2021/04/30 19:26:45 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -466,23 +466,6 @@ tls1_setup_key_block(SSL *s)
 }
 
 int
-tls1_generate_master_secret(SSL *s, unsigned char *out, unsigned char *p,
-    int len)
-{
-        if (len < 0)
-                return 0;
-
-        if (!tls1_PRF(s, p, len,
-            TLS_MD_MASTER_SECRET_CONST, TLS_MD_MASTER_SECRET_CONST_SIZE,
-            s->s3->client_random, SSL3_RANDOM_SIZE, NULL, 0,
-            s->s3->server_random, SSL3_RANDOM_SIZE, NULL, 0,
-            s->session->master_key, SSL_MAX_MASTER_KEY_LENGTH))
-                return 0;
-
-        return (SSL_MAX_MASTER_KEY_LENGTH);
-}
-
-int
 tls1_export_keying_material(SSL *s, unsigned char *out, size_t olen,
     const char *label, size_t llen, const unsigned char *context,
     size_t contextlen, int use_context)
diff --git a/src/lib/libssl/tls12_lib.c b/src/lib/libssl/tls12_lib.c
index 520f41678d..e7171ba833 100644
--- a/src/lib/libssl/tls12_lib.c
+++ b/src/lib/libssl/tls12_lib.c
@@ -1,4 +1,4 @@
-/*      $OpenBSD: tls12_lib.c,v 1.1 2021/04/25 13:15:23 jsing Exp $ */
+/*      $OpenBSD: tls12_lib.c,v 1.2 2021/04/30 19:26:45 jsing Exp $ */
 /*
  * Copyright (c) 2021 Joel Sing <jsing@openbsd.org>
  *
@@ -90,3 +90,26 @@ tls12_derive_peer_finished(SSL *s)
                     &S3I(s)->hs.peer_finished_len);
         }
 }
+
+int
+tls12_derive_master_secret(SSL *s, uint8_t *premaster_secret,
+    size_t premaster_secret_len)
+{
+        s->session->master_key_length = 0;
+
+        if (premaster_secret_len == 0)
+                return 0;
+
+        CTASSERT(sizeof(s->session->master_key) == SSL_MAX_MASTER_KEY_LENGTH);
+
+        if (!tls1_PRF(s, premaster_secret, premaster_secret_len,
+            TLS_MD_MASTER_SECRET_CONST, TLS_MD_MASTER_SECRET_CONST_SIZE,
+            s->s3->client_random, SSL3_RANDOM_SIZE, NULL, 0,
+            s->s3->server_random, SSL3_RANDOM_SIZE, NULL, 0,
+            s->session->master_key, sizeof(s->session->master_key)))
+                return 0;
+
+        s->session->master_key_length = SSL_MAX_MASTER_KEY_LENGTH;
+
+        return 1;
+}