Clean up derivation of finished/peer finished.

Make this process more readable by having specific client/server functions,
calling the correct one based on s->server. This allows to remove various
SSL_ST_ACCEPT/SSL_ST_CONNECT checks, along with duplicate code.

ok inoguchi@ tb@

8 files changed, 147 insertions, 116 deletions

diff --git a/src/lib/libssl/Makefile b/src/lib/libssl/Makefile
index e4ad5f36f7..d6730a5e04 100644
--- a/src/lib/libssl/Makefile
+++ b/src/lib/libssl/Makefile
@@ -1,4 +1,4 @@
-# $OpenBSD: Makefile,v 1.68 2020/10/03 18:01:55 jsing Exp $
+# $OpenBSD: Makefile,v 1.69 2021/04/25 13:15:22 jsing Exp $
 
 .include <bsd.own.mk>
 .ifndef NOMAN
@@ -67,6 +67,7 @@ SRCS= \
         ssl_versions.c \
         t1_enc.c \
         t1_lib.c \
+        tls12_lib.c \
         tls12_record_layer.c \
         tls13_buffer.c \
         tls13_client.c \
diff --git a/src/lib/libssl/ssl_both.c b/src/lib/libssl/ssl_both.c
index ad9b0ee257..fe04f809b0 100644
--- a/src/lib/libssl/ssl_both.c
+++ b/src/lib/libssl/ssl_both.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_both.c,v 1.28 2021/04/19 16:51:56 jsing Exp $ */
+/* $OpenBSD: ssl_both.c,v 1.29 2021/04/25 13:15:22 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -164,42 +164,39 @@ ssl3_do_write(SSL *s, int type)
 }
 
 int
-ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen)
+ssl3_send_finished(SSL *s, int state_a, int state_b)
 {
         CBB cbb, finished;
-        int md_len;
 
         memset(&cbb, 0, sizeof(cbb));
 
-        if (S3I(s)->hs.state == a) {
-                md_len = TLS1_FINISH_MAC_LENGTH;
-                OPENSSL_assert(md_len <= EVP_MAX_MD_SIZE);
-
-                if (tls1_final_finish_mac(s, sender, slen,
-                    S3I(s)->hs.finished) != md_len)
-                        return (0);
-                S3I(s)->hs.finished_len = md_len;
+        if (S3I(s)->hs.state == state_a) {
+                if (!tls12_derive_finished(s))
+                        goto err;
 
                 /* Copy finished so we can use it for renegotiation checks. */
                 if (!s->server) {
                         memcpy(S3I(s)->previous_client_finished,
-                            S3I(s)->hs.finished, md_len);
-                        S3I(s)->previous_client_finished_len = md_len;
+                            S3I(s)->hs.finished, S3I(s)->hs.finished_len);
+                        S3I(s)->previous_client_finished_len =
+                            S3I(s)->hs.finished_len;
                 } else {
                         memcpy(S3I(s)->previous_server_finished,
-                            S3I(s)->hs.finished, md_len);
-                        S3I(s)->previous_server_finished_len = md_len;
+                            S3I(s)->hs.finished, S3I(s)->hs.finished_len);
+                        S3I(s)->previous_server_finished_len =
+                            S3I(s)->hs.finished_len;
                 }
 
                 if (!ssl3_handshake_msg_start(s, &cbb, &finished,
                     SSL3_MT_FINISHED))
                         goto err;
-                if (!CBB_add_bytes(&finished, S3I(s)->hs.finished, md_len))
+                if (!CBB_add_bytes(&finished, S3I(s)->hs.finished,
+                    S3I(s)->hs.finished_len))
                         goto err;
                 if (!ssl3_handshake_msg_finish(s, &cbb))
                         goto err;
 
-                S3I(s)->hs.state = b;
+                S3I(s)->hs.state = state_b;
         }
 
         return (ssl3_handshake_write(s));
@@ -210,36 +207,6 @@ ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen)
         return (-1);
 }
 
-/*
- * ssl3_take_mac calculates the Finished MAC for the handshakes messages seen
- * so far.
- */
-static void
-ssl3_take_mac(SSL *s)
-{
-        const char *sender;
-        int slen;
-
-        /*
-         * If no new cipher setup return immediately: other functions will
-         * set the appropriate error.
-         */
-        if (S3I(s)->hs.cipher == NULL)
-                return;
-
-        if (S3I(s)->hs.state & SSL_ST_CONNECT) {
-                sender = TLS_MD_SERVER_FINISH_CONST;
-                slen = TLS_MD_SERVER_FINISH_CONST_SIZE;
-        } else {
-                sender = TLS_MD_CLIENT_FINISH_CONST;
-                slen = TLS_MD_CLIENT_FINISH_CONST_SIZE;
-        }
-
-        S3I(s)->hs.peer_finished_len =
-            tls1_final_finish_mac(s, sender, slen,
-                S3I(s)->hs.peer_finished);
-}
-
 int
 ssl3_get_finished(SSL *s, int a, int b)
 {
@@ -544,10 +511,16 @@ ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok)
                 n -= i;
         }
 
-        /* If receiving Finished, record MAC of prior handshake messages for
-         * Finished verification. */
-        if (*s->internal->init_buf->data == SSL3_MT_FINISHED)
-                ssl3_take_mac(s);
+        /*
+         * If receiving Finished, record MAC of prior handshake messages for
+         * Finished verification.
+         */
+        if (*s->internal->init_buf->data == SSL3_MT_FINISHED) {
+                if (S3I(s)->hs.cipher != NULL) {
+                        if (!tls12_derive_peer_finished(s))
+                                goto err;
+                }
+        }
 
         /* Feed this message into MAC computation. */
         if (s->internal->mac_packet) {
@@ -566,7 +539,7 @@ ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok)
 
  fatal_err:
         ssl3_send_alert(s, SSL3_AL_FATAL, al);
-err:
+ err:
         *ok = 0;
         return (-1);
 }
diff --git a/src/lib/libssl/ssl_clnt.c b/src/lib/libssl/ssl_clnt.c
index 7f69b8ba98..c129bb6d66 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.92 2021/04/21 19:27:56 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.93 2021/04/25 13:15:22 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -497,8 +497,7 @@ ssl3_connect(SSL *s)
                         if (SSL_is_dtls(s) && !s->internal->hit)
                                 dtls1_start_timer(s);
                         ret = ssl3_send_finished(s, SSL3_ST_CW_FINISHED_A,
-                            SSL3_ST_CW_FINISHED_B, TLS_MD_CLIENT_FINISH_CONST,
-                            TLS_MD_CLIENT_FINISH_CONST_SIZE);
+                            SSL3_ST_CW_FINISHED_B);
                         if (ret <= 0)
                                 goto end;
                         if (!SSL_is_dtls(s))
diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index 27397308ef..a9cab69ee0 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.337 2021/04/21 19:27:56 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.338 2021/04/25 13:15:22 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1219,7 +1219,7 @@ int ssl3_do_write(SSL *s, int type);
 int ssl3_send_alert(SSL *s, int level, int desc);
 int ssl3_get_req_cert_types(SSL *s, CBB *cbb);
 long ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok);
-int ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen);
+int ssl3_send_finished(SSL *s, int state_a, int state_b);
 int ssl3_num_ciphers(void);
 const SSL_CIPHER *ssl3_get_cipher(unsigned int u);
 const SSL_CIPHER *ssl3_get_cipher_by_id(unsigned int id);
@@ -1371,10 +1371,14 @@ void tls1_transcript_freeze(SSL *s);
 void tls1_transcript_unfreeze(SSL *s);
 int tls1_transcript_record(SSL *s, const unsigned char *buf, size_t len);
 
+int tls1_PRF(SSL *s, const unsigned char *secret, size_t secret_len,
+    const void *seed1, size_t seed1_len, const void *seed2, size_t seed2_len,
+    const void *seed3, size_t seed3_len, const void *seed4, size_t seed4_len,
+    const void *seed5, size_t seed5_len, unsigned char *out, size_t out_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_final_finish_mac(SSL *s, const char *str, int slen, unsigned char *p);
 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,
@@ -1383,6 +1387,9 @@ int tls1_export_keying_material(SSL *s, unsigned char *out, size_t olen,
 int tls1_alert_code(int code);
 int ssl_ok(SSL *s);
 
+int tls12_derive_finished(SSL *s);
+int tls12_derive_peer_finished(SSL *s);
+
 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_pkt.c b/src/lib/libssl/ssl_pkt.c
index a760f90a3a..6e0cfe2102 100644
--- a/src/lib/libssl/ssl_pkt.c
+++ b/src/lib/libssl/ssl_pkt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_pkt.c,v 1.40 2021/03/29 16:46:09 jsing Exp $ */
+/* $OpenBSD: ssl_pkt.c,v 1.41 2021/04/25 13:15:22 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1155,13 +1155,6 @@ int
 ssl3_do_change_cipher_spec(SSL *s)
 {
         int i;
-        const char *sender;
-        int slen;
-
-        if (S3I(s)->hs.state & SSL_ST_ACCEPT)
-                i = SSL3_CHANGE_CIPHER_SERVER_READ;
-        else
-                i = SSL3_CHANGE_CIPHER_CLIENT_READ;
 
         if (S3I(s)->hs.tls12.key_block == NULL) {
                 if (s->session == NULL || s->session->master_key_length == 0) {
@@ -1175,27 +1168,20 @@ ssl3_do_change_cipher_spec(SSL *s)
                         return (0);
         }
 
+        if (S3I(s)->hs.state & SSL_ST_ACCEPT)
+                i = SSL3_CHANGE_CIPHER_SERVER_READ;
+        else
+                i = SSL3_CHANGE_CIPHER_CLIENT_READ;
+
         if (!tls1_change_cipher_state(s, i))
                 return (0);
 
-        /* we have to record the message digest at
-         * this point so we can get it before we read
-         * the finished message */
-        if (S3I(s)->hs.state & SSL_ST_CONNECT) {
-                sender = TLS_MD_SERVER_FINISH_CONST;
-                slen = TLS_MD_SERVER_FINISH_CONST_SIZE;
-        } else {
-                sender = TLS_MD_CLIENT_FINISH_CONST;
-                slen = TLS_MD_CLIENT_FINISH_CONST_SIZE;
-        }
-
-        i = tls1_final_finish_mac(s, sender, slen,
-            S3I(s)->hs.peer_finished);
-        if (i == 0) {
-                SSLerror(s, ERR_R_INTERNAL_ERROR);
-                return 0;
-        }
-        S3I(s)->hs.peer_finished_len = i;
+        /*
+         * We have to record the message digest at this point so we can get it
+         * before we read the finished message.
+         */
+        if (!tls12_derive_peer_finished(s))
+                return (0);
 
         return (1);
 }
diff --git a/src/lib/libssl/ssl_srvr.c b/src/lib/libssl/ssl_srvr.c
index c85a25158f..2c15081f45 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.103 2021/04/21 19:27:56 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.104 2021/04/25 13:15:22 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -666,10 +666,8 @@ ssl3_accept(SSL *s)
 
                 case SSL3_ST_SW_FINISHED_A:
                 case SSL3_ST_SW_FINISHED_B:
-                        ret = ssl3_send_finished(s,
-                            SSL3_ST_SW_FINISHED_A, SSL3_ST_SW_FINISHED_B,
-                            TLS_MD_SERVER_FINISH_CONST,
-                            TLS_MD_SERVER_FINISH_CONST_SIZE);
+                        ret = ssl3_send_finished(s, SSL3_ST_SW_FINISHED_A,
+                            SSL3_ST_SW_FINISHED_B);
                         if (ret <= 0)
                                 goto end;
                         S3I(s)->hs.state = SSL3_ST_SW_FLUSH;
diff --git a/src/lib/libssl/t1_enc.c b/src/lib/libssl/t1_enc.c
index 6b3d40d8ec..3f93bcecf5 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.138 2021/04/19 17:26:39 jsing Exp $ */
+/* $OpenBSD: t1_enc.c,v 1.139 2021/04/25 13:15:22 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -144,11 +144,6 @@
 #include <openssl/hmac.h>
 #include <openssl/md5.h>
 
-int tls1_PRF(SSL *s, const unsigned char *secret, size_t secret_len,
-    const void *seed1, size_t seed1_len, const void *seed2, size_t seed2_len,
-    const void *seed3, size_t seed3_len, const void *seed4, size_t seed4_len,
-    const void *seed5, size_t seed5_len, unsigned char *out, size_t out_len);
-
 void
 tls1_cleanup_key_block(SSL *s)
 {
@@ -471,26 +466,6 @@ tls1_setup_key_block(SSL *s)
 }
 
 int
-tls1_final_finish_mac(SSL *s, const char *str, int str_len, unsigned char *out)
-{
-        unsigned char buf[EVP_MAX_MD_SIZE];
-        size_t hash_len;
-
-        if (str_len < 0)
-                return 0;
-
-        if (!tls1_transcript_hash_value(s, buf, sizeof(buf), &hash_len))
-                return 0;
-
-        if (!tls1_PRF(s, s->session->master_key, s->session->master_key_length,
-            str, str_len, buf, hash_len, NULL, 0, NULL, 0, NULL, 0,
-            out, TLS1_FINISH_MAC_LENGTH))
-                return 0;
-
-        return TLS1_FINISH_MAC_LENGTH;
-}
-
-int
 tls1_generate_master_secret(SSL *s, unsigned char *out, unsigned char *p,
     int len)
 {
diff --git a/src/lib/libssl/tls12_lib.c b/src/lib/libssl/tls12_lib.c
new file mode 100644
index 0000000000..520f41678d
--- /dev/null
+++ b/src/lib/libssl/tls12_lib.c
@@ -0,0 +1,92 @@
+/*      $OpenBSD: tls12_lib.c,v 1.1 2021/04/25 13:15:23 jsing Exp $ */
+/*
+ * Copyright (c) 2021 Joel Sing <jsing@openbsd.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include "ssl_locl.h"
+
+static int
+tls12_finished_verify_data(SSL *s, const char *finished_label,
+    size_t finished_label_len, uint8_t *verify_data, size_t verify_data_len,
+    size_t *out_len)
+{
+        uint8_t transcript_hash[EVP_MAX_MD_SIZE];
+        size_t transcript_hash_len;
+
+        *out_len = 0;
+
+        if (verify_data_len < TLS1_FINISH_MAC_LENGTH)
+                return 0;
+
+        if (!tls1_transcript_hash_value(s, transcript_hash,
+            sizeof(transcript_hash), &transcript_hash_len))
+                return 0;
+
+        if (!tls1_PRF(s, s->session->master_key, s->session->master_key_length,
+            finished_label, finished_label_len, transcript_hash,
+            transcript_hash_len, NULL, 0, NULL, 0, NULL, 0, verify_data,
+            TLS1_FINISH_MAC_LENGTH))
+                return 0;
+
+        *out_len = TLS1_FINISH_MAC_LENGTH;
+
+        return 1;
+}
+
+static int
+tls12_client_finished_verify_data(SSL *s, uint8_t *verify_data,
+    size_t verify_data_len, size_t *out_len)
+{
+        return tls12_finished_verify_data(s, TLS_MD_CLIENT_FINISH_CONST,
+            TLS_MD_CLIENT_FINISH_CONST_SIZE, verify_data, verify_data_len,
+            out_len);
+}
+
+static int
+tls12_server_finished_verify_data(SSL *s, uint8_t *verify_data,
+    size_t verify_data_len, size_t *out_len)
+{
+        return tls12_finished_verify_data(s, TLS_MD_SERVER_FINISH_CONST,
+            TLS_MD_SERVER_FINISH_CONST_SIZE, verify_data, verify_data_len,
+            out_len);
+}
+
+int
+tls12_derive_finished(SSL *s)
+{
+        if (!s->server) {
+                return tls12_client_finished_verify_data(s,
+                    S3I(s)->hs.finished, sizeof(S3I(s)->hs.finished),
+                    &S3I(s)->hs.finished_len);
+        } else {
+                return tls12_server_finished_verify_data(s,
+                    S3I(s)->hs.finished, sizeof(S3I(s)->hs.finished),
+                    &S3I(s)->hs.finished_len);
+        }
+}
+
+int
+tls12_derive_peer_finished(SSL *s)
+{
+        if (s->server) {
+                return tls12_client_finished_verify_data(s,
+                    S3I(s)->hs.peer_finished, sizeof(S3I(s)->hs.peer_finished),
+                    &S3I(s)->hs.peer_finished_len);
+        } else {
+                return tls12_server_finished_verify_data(s,
+                    S3I(s)->hs.peer_finished, sizeof(S3I(s)->hs.peer_finished),
+                    &S3I(s)->hs.peer_finished_len);
+        }
+}