Revert "Delay setting the sig algs until after the cert_cb has been called"
authorBenjamin Kaduk <bkaduk@akamai.com>
Thu, 13 Jun 2019 19:04:52 +0000 (12:04 -0700)
committerBenjamin Kaduk <kaduk@mit.edu>
Wed, 26 Jun 2019 17:20:55 +0000 (12:20 -0500)
This reverts commit 524006dd1b80c1a86a20119ad988666a80d8d8f5.

While this change did prevent the sigalgs from getting inadvertently
clobbered by SSL_set_SSL_CTX(), it also caused the sigalgs to not be
set when the cert_cb runs.  This, in turn, caused significant breakage,
such as SSL_check_chain() failing to find any valid chain.  An alternate
approach to fixing the issue from #7244 will follow.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9157)

ssl/statem/statem_srvr.c

index 79c2aa0..acd3e27 100644 (file)
@@ -2065,6 +2065,10 @@ static int tls_early_post_process_client_hello(SSL *s)
 #else
         s->session->compress_meth = (comp == NULL) ? 0 : comp->id;
 #endif
+        if (!tls1_set_server_sigalgs(s)) {
+            /* SSLfatal() already called */
+            goto err;
+        }
     }
 
     sk_SSL_CIPHER_free(ciphers);
@@ -2232,25 +2236,19 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
     if (wst == WORK_MORE_B) {
         if (!s->hit || SSL_IS_TLS13(s)) {
             /* Let cert callback update server certificates if required */
-            if (!s->hit) {
-                if (s->cert->cert_cb != NULL) {
-                    int rv = s->cert->cert_cb(s, s->cert->cert_cb_arg);
-                    if (rv == 0) {
-                        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
-                                 SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,
-                                 SSL_R_CERT_CB_ERROR);
-                        goto err;
-                    }
-                    if (rv < 0) {
-                        s->rwstate = SSL_X509_LOOKUP;
-                        return WORK_MORE_B;
-                    }
-                    s->rwstate = SSL_NOTHING;
-                }
-                if (!tls1_set_server_sigalgs(s)) {
-                    /* SSLfatal already called */
+            if (!s->hit && s->cert->cert_cb != NULL) {
+                int rv = s->cert->cert_cb(s, s->cert->cert_cb_arg);
+                if (rv == 0) {
+                    SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                             SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,
+                             SSL_R_CERT_CB_ERROR);
                     goto err;
                 }
+                if (rv < 0) {
+                    s->rwstate = SSL_X509_LOOKUP;
+                    return WORK_MORE_B;
+                }
+                s->rwstate = SSL_NOTHING;
             }
 
             /* In TLSv1.3 we selected the ciphersuite before resumption */