From: Matt Caswell <matt@openssl.org>
Date: Thu, 29 May 2025 11:31:33 +0000 (+0100)
Subject: Implement explicit storing of the server_finished_hash
X-Git-Tag: openssl-3.5.1~62
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=2d21ae48688d2a57c5f3d81521293c3326f7c12f;p=thirdparty%2Fopenssl.git

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 ba66405d2e9..597b4d4b639 100644
--- 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 dffff631a55..cb1e4f8c4b8 100644
--- 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 46f6cd8f116..43986121efd 100644
--- 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 d89a42720c7..f4af3ad3c89 100644
--- 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