Use ERR marks also when verifying server X.509 certs

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 8716ed669f36846aa5373c038de10c015017c2a6..df2eed7594c17e59093fdddb4443dafe35d0d155 100644 (file)
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -2082,10 +2082,7 @@ WORK_STATE tls_post_process_server_certificate(SSL_CONNECTION *s,
 
     if (s->rwstate == SSL_RETRY_VERIFY)
         s->rwstate = SSL_NOTHING;
-    i = ssl_verify_cert_chain(s, s->session->peer_chain);
-    if (i > 0 && s->rwstate == SSL_RETRY_VERIFY) {
-        return WORK_MORE_A;
-    }
+
     /*
      * The documented interface is that SSL_VERIFY_PEER should be set in order
      * for client side verification of the server certificate to take place.
@@ -2100,12 +2097,17 @@ WORK_STATE tls_post_process_server_certificate(SSL_CONNECTION *s,
      * (less clean) historic behaviour of performing validation if any flag is
      * set. The *documented* interface remains the same.
      */
-    if (s->verify_mode != SSL_VERIFY_NONE && i <= 0) {
+    ERR_set_mark();
+    i = ssl_verify_cert_chain(s, s->session->peer_chain);
+    if (i <= 0 && s->verify_mode != SSL_VERIFY_NONE) {
+        ERR_clear_last_mark();
         SSLfatal(s, ssl_x509err2alert(s->verify_result),
                  SSL_R_CERTIFICATE_VERIFY_FAILED);
         return WORK_ERROR;
     }
-    ERR_clear_error();          /* but we keep s->verify_result */
+    ERR_pop_to_mark();      /* but we keep s->verify_result */
+    if (i > 0 && s->rwstate == SSL_RETRY_VERIFY)
+        return WORK_MORE_A;
 
     /*
      * Inconsistency alert: cert_chain does include the peer's certificate,

diff --git a/test/rpktest.c b/test/rpktest.c
index 0be8461f77b53e2ba8f9f7b42ab9736f8c509bb1..624d366508f146f75ac2d26e58f392d9e07173c9 100644 (file)
--- a/test/rpktest.c
+++ b/test/rpktest.c
@@ -490,24 +490,22 @@ static int test_rpk(int idx)
     }
 
     /* Make sure client gets RPK or certificate as configured */
-    if (expected == 1) {
-        if (idx_server_server_rpk && idx_client_server_rpk) {
-            if (!TEST_long_eq(SSL_get_verify_result(clientssl), client_verify_result))
-                goto end;
-            if (!TEST_ptr(SSL_get0_peer_rpk(clientssl)))
-                goto end;
-            if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(serverssl), TLSEXT_cert_type_rpk))
-                goto end;
-            if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(clientssl), TLSEXT_cert_type_rpk))
-                goto end;
-        } else {
-            if (!TEST_ptr(SSL_get0_peer_certificate(clientssl)))
-                goto end;
-            if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(serverssl), TLSEXT_cert_type_x509))
-                goto end;
-            if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(clientssl), TLSEXT_cert_type_x509))
-                goto end;
-        }
+    if (idx_server_server_rpk && idx_client_server_rpk) {
+        if (!TEST_long_eq(SSL_get_verify_result(clientssl), client_verify_result))
+            goto end;
+        if (!TEST_ptr(SSL_get0_peer_rpk(clientssl)))
+            goto end;
+        if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(serverssl), TLSEXT_cert_type_rpk))
+            goto end;
+        if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(clientssl), TLSEXT_cert_type_rpk))
+            goto end;
+    } else {
+        if (!TEST_ptr(SSL_get0_peer_certificate(clientssl)))
+            goto end;
+        if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(serverssl), TLSEXT_cert_type_x509))
+            goto end;
+        if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(clientssl), TLSEXT_cert_type_x509))
+            goto end;
     }
 
     if (idx == 9) {
@@ -534,8 +532,7 @@ static int test_rpk(int idx)
             if (!TEST_int_eq(SSL_get_negotiated_client_cert_type(clientssl), TLSEXT_cert_type_rpk))
                 goto end;
         } else {
-            /* only if connection is expected to succeed */
-            if (expected == 1 && !TEST_ptr(SSL_get0_peer_certificate(serverssl)))
+            if (!TEST_ptr(SSL_get0_peer_certificate(serverssl)))
                 goto end;
             if (!TEST_int_eq(SSL_get_negotiated_client_cert_type(serverssl), TLSEXT_cert_type_x509))
                 goto end;
@@ -625,7 +622,7 @@ static int test_rpk(int idx)
         }
 
         ret = create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE);
-        if (!TEST_int_eq(expected, ret))
+        if (!TEST_true(ret))
             goto end;
         verify = SSL_get_verify_result(clientssl);
         if (!TEST_int_eq(client_expected, verify))