Implement explicit storing of the server_finished_hash

tls13_change_cipher_state was storing the server_finished_hash as a
side effect of its operation. This decision is better made by the state
machine which actually knows what state we are in.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27732)

(cherry picked from commit c7f9c4d7d184cec988251b2a9c697302774fbe77)

diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index ba66405d2e93c8beeb57bd31b5e01464df9e1b56..597b4d4b6395ee646d14b213d8e5ffdd41138fa5 100644 (file)
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -2773,6 +2773,7 @@ __owur int tls13_setup_key_block(SSL_CONNECTION *s);
 __owur size_t tls13_final_finish_mac(SSL_CONNECTION *s, const char *str, size_t slen,
                                      unsigned char *p);
 __owur int tls13_store_handshake_traffic_hash(SSL_CONNECTION *s);
+__owur int tls13_store_server_finished_hash(SSL_CONNECTION *s);
 __owur int tls13_change_cipher_state(SSL_CONNECTION *s, int which);
 __owur int tls13_update_key(SSL_CONNECTION *s, int send);
 __owur int tls13_hkdf_expand(SSL_CONNECTION *s,

diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index dffff631a55bc8f1416005e447be5fbe0ba95fd9..cb1e4f8c4b8eb7e0648c26585363b70a292a1ef3 100644 (file)
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -934,6 +934,10 @@ MSG_PROCESS_RETURN tls_process_finished(SSL_CONNECTION *s, PACKET *pkt)
                 /* SSLfatal() already called */
                 return MSG_PROCESS_ERROR;
             }
+            if (!tls13_store_server_finished_hash(s)) {
+                /* SSLfatal() already called */
+                return MSG_PROCESS_ERROR;
+            }
             if (!ssl->method->ssl3_enc->change_cipher_state(s,
                     SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_CLIENT_READ)) {
                 /* SSLfatal() already called */

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 46f6cd8f11673ba1f71946ed0e41d16047a94c49..43986121efd302581760a6b92ce1b2fd6a63da3d 100644 (file)
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1041,6 +1041,7 @@ WORK_STATE ossl_statem_server_post_work(SSL_CONNECTION *s, WORK_STATE wst)
             if (!ssl->method->ssl3_enc->generate_master_secret(s,
                         s->master_secret, s->handshake_secret, 0,
                         &dummy)
+                || !tls13_store_server_finished_hash(s)
                 || !ssl->method->ssl3_enc->change_cipher_state(s,
                         SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_SERVER_WRITE))
             /* SSLfatal() already called */

diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c
index d89a42720c7127b8d25b16ecc0527805ef9d1fde..f4af3ad3c89073dcb2415db634b6d0ef9f23f02d 100644 (file)
--- a/ssl/tls13_enc.c
+++ b/ssl/tls13_enc.c
@@ -446,13 +446,12 @@ static int derive_secret_key_and_iv(SSL_CONNECTION *s, const EVP_MD *md,
     return 1;
 }
 
-int tls13_store_handshake_traffic_hash(SSL_CONNECTION *s)
+static int tls13_store_hash(SSL_CONNECTION *s, unsigned char *hash, size_t len)
 {
     size_t hashlen;
 
     if (!ssl3_digest_cached_records(s, 1)
-            || !ssl_handshake_hash(s, s->handshake_traffic_hash,
-                                   sizeof(s->handshake_traffic_hash), &hashlen)) {
+            || !ssl_handshake_hash(s, hash, len, &hashlen)) {
         /* SSLfatal() already called */;
         return 0;
     }
@@ -460,6 +459,18 @@ int tls13_store_handshake_traffic_hash(SSL_CONNECTION *s)
     return 1;
 }
 
+int tls13_store_handshake_traffic_hash(SSL_CONNECTION *s)
+{
+    return tls13_store_hash(s, s->handshake_traffic_hash,
+                            sizeof(s->handshake_traffic_hash));
+}
+
+int tls13_store_server_finished_hash(SSL_CONNECTION *s)
+{
+    return tls13_store_hash(s, s->server_finished_hash,
+                            sizeof(s->server_finished_hash));
+}
+
 int tls13_change_cipher_state(SSL_CONNECTION *s, int which)
 {
     /* ASCII: "c e traffic", in hex for EBCDIC compatibility */
@@ -662,13 +673,6 @@ int tls13_change_cipher_state(SSL_CONNECTION *s, int which)
         }
     }
 
-    /*
-     * Save the hash of handshakes up to now for use when we calculate the
-     * client application traffic secret
-     */
-    if (label == server_application_traffic)
-        memcpy(s->server_finished_hash, hashval, hashlen);
-
     if (label == client_application_traffic) {
         /*
          * We also create the resumption master secret, but this time use the