Fix SSL_BUILD_CHAIN_FLAG_IGNORE_ERROR behavior

aws · Jun 4, 2024 · 263b7cf · 263b7cf
1 parent e44fc2c
commit 263b7cf
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 24 deletions.
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
@@ -5217,12 +5217,14 @@ TEST(SSLTest, BuildCertChain) {
 
   // Verification will fail because there is no valid root cert available.
   EXPECT_FALSE(SSL_CTX_build_cert_chain(ctx.get(), 0));
+  ERR_clear_error();
 
   // Should return 2 when |SSL_BUILD_CHAIN_FLAG_IGNORE_ERROR| is set.
   EXPECT_EQ(
       SSL_CTX_build_cert_chain(ctx.get(), SSL_BUILD_CHAIN_FLAG_IGNORE_ERROR),
       2);
   EXPECT_TRUE(ExpectSingleError(ERR_LIB_SSL, SSL_R_CERTIFICATE_VERIFY_FAILED));
+  ERR_clear_error();
 
   // Should return 2, but with no error on the stack when
   // |SSL_BUILD_CHAIN_FLAG_IGNORE_ERROR| and |SSL_BUILD_CHAIN_FLAG_CLEAR_ERROR|

diff --git a/ssl/ssl_x509.cc b/ssl/ssl_x509.cc
@@ -535,24 +535,24 @@ static void ssl_crypto_x509_ssl_ctx_free(SSL_CTX *ctx) {
 }
 
 const SSL_X509_METHOD ssl_crypto_x509_method = {
-  ssl_crypto_x509_check_client_CA_list,
-  ssl_crypto_x509_cert_clear,
-  ssl_crypto_x509_cert_free,
-  ssl_crypto_x509_cert_dup,
-  ssl_crypto_x509_cert_flush_cached_chain,
-  ssl_crypto_x509_cert_flush_leaf,
-  ssl_crypto_x509_session_cache_objects,
-  ssl_crypto_x509_session_dup,
-  ssl_crypto_x509_session_clear,
-  ssl_crypto_x509_session_verify_cert_chain,
-  ssl_crypto_x509_hs_flush_cached_ca_names,
-  ssl_crypto_x509_ssl_new,
-  ssl_crypto_x509_ssl_config_free,
-  ssl_crypto_x509_ssl_flush_cached_client_CA,
-  ssl_crypto_x509_ssl_auto_chain_if_needed,
-  ssl_crypto_x509_ssl_ctx_new,
-  ssl_crypto_x509_ssl_ctx_free,
-  ssl_crypto_x509_ssl_ctx_flush_cached_client_CA,
+    ssl_crypto_x509_check_client_CA_list,
+    ssl_crypto_x509_cert_clear,
+    ssl_crypto_x509_cert_free,
+    ssl_crypto_x509_cert_dup,
+    ssl_crypto_x509_cert_flush_cached_chain,
+    ssl_crypto_x509_cert_flush_leaf,
+    ssl_crypto_x509_session_cache_objects,
+    ssl_crypto_x509_session_dup,
+    ssl_crypto_x509_session_clear,
+    ssl_crypto_x509_session_verify_cert_chain,
+    ssl_crypto_x509_hs_flush_cached_ca_names,
+    ssl_crypto_x509_ssl_new,
+    ssl_crypto_x509_ssl_config_free,
+    ssl_crypto_x509_ssl_flush_cached_client_CA,
+    ssl_crypto_x509_ssl_auto_chain_if_needed,
+    ssl_crypto_x509_ssl_ctx_new,
+    ssl_crypto_x509_ssl_ctx_free,
+    ssl_crypto_x509_ssl_ctx_flush_cached_client_CA,
 };
 
 BSSL_NAMESPACE_END
@@ -1061,12 +1061,13 @@ static int ssl_build_cert_chain(CERT *cert, X509_STORE *cert_store, int flags) {
 
   bool ignore_error = false;
   if (X509_verify_cert(store_ctx.get()) <= 0) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_VERIFY_FAILED);
+    ERR_add_error_data(2, "Verify error:",
+                       X509_verify_cert_error_string(
+                           X509_STORE_CTX_get_error(store_ctx.get())));
+
     // Fail if |SSL_BUILD_CHAIN_FLAG_IGNORE_ERROR| is not set.
-    if(!is_flag_set(flags, SSL_BUILD_CHAIN_FLAG_IGNORE_ERROR)) {
-      OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_VERIFY_FAILED);
-      ERR_add_error_data(2, "Verify error:",
-                         X509_verify_cert_error_string(
-                             X509_STORE_CTX_get_error(store_ctx.get())));
+    if (!is_flag_set(flags, SSL_BUILD_CHAIN_FLAG_IGNORE_ERROR)) {
       return 0;
     }
 
@@ -1098,7 +1099,7 @@ static int ssl_build_cert_chain(CERT *cert, X509_STORE *cert_store, int flags) {
   // Anything that has passed successfully up to here is valid.
   // 2 is used to indicate a verification error has happened, but was ignored
   // because |SSL_BUILD_CHAIN_FLAG_IGNORE_ERROR| was set.
-  if(ignore_error) {
+  if (ignore_error) {
     return 2;
   }
   return 1;