statem: always save sigalgs during PHA

We use the same extension-parsing function on server and client
for convenience, but while the server might worry about tracking
what was previously received and not overwriting it, on the client
receiving a request for post-handshake authentication, we always
want to use the values from the current extension (and should
always have a new session object that we are free to mutate).

It is somewhat unclear whether the server also needs the check
for a resumed connection; it appears to have been added back in
2015 in commit 062178678f5374b09f00d70796f6e692e8775aca as part
of a broad pass to handle extensions on resumption, but without
specific documentation of each extension's handling.

Fixes: #10370
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24651)

diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index dd771207f698899daab216fbdea889d68afe9b17..1a09913ad63f6e2dff4e21b0cd1d6450115b60bf 100644 (file)
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -289,7 +289,13 @@ int tls_parse_ctos_sig_algs_cert(SSL_CONNECTION *s, PACKET *pkt,
         return 0;
     }
 
-    if (!s->hit && !tls1_save_sigalgs(s, &supported_sig_algs, 1)) {
+    /*
+     * We use this routine on both clients and servers, and when clients
+     * get asked for PHA we need to always save the sigalgs regardless
+     * of whether it was a resumption or not.
+     */
+    if ((!s->server || (s->server && !s->hit))
+            && !tls1_save_sigalgs(s, &supported_sig_algs, 1)) {
         SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
         return 0;
     }
@@ -308,7 +314,13 @@ int tls_parse_ctos_sig_algs(SSL_CONNECTION *s, PACKET *pkt,
         return 0;
     }
 
-    if (!s->hit && !tls1_save_sigalgs(s, &supported_sig_algs, 0)) {
+    /*
+     * We use this routine on both clients and servers, and when clients
+     * get asked for PHA we need to always save the sigalgs regardless
+     * of whether it was a resumption or not.
+     */
+    if ((!s->server || (s->server && !s->hit))
+            && !tls1_save_sigalgs(s, &supported_sig_algs, 0)) {
         SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
         return 0;
     }