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)

(cherry picked from commit ddd99d52d30e2fdae08f9684947cba45ce53898b)

diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 72c00574be686351603d1bce84a0353957bcdd94..df97554886daa0485c7c7cc82be0701e558e3539 100644 (file)
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -275,7 +275,13 @@ int tls_parse_ctos_sig_algs_cert(SSL *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;
     }
@@ -294,7 +300,13 @@ int tls_parse_ctos_sig_algs(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
         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;
     }