Wait for CertificateStatus message to verify certificate.

Applications may require the stapled OCSP response in order to verify
the certificate within the verification callback.

Change-Id: I8002e527f90c3ce7b6a66e3203c0a68371aac5ec
Reviewed-on: https://boringssl-review.googlesource.com/5730
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>

include/openssl/ssl3.h

@@ -563,6 +563,7 @@ typedef struct ssl3_state_st {
 /* extra state */
 #define SSL3_ST_CW_FLUSH (0x100 | SSL_ST_CONNECT)
 #define SSL3_ST_FALSE_START (0x101 | SSL_ST_CONNECT)
+#define SSL3_ST_VERIFY_SERVER_CERT (0x102 | SSL_ST_CONNECT)
 /* write to server */
 #define SSL3_ST_CW_CLNT_HELLO_A (0x110 | SSL_ST_CONNECT)
 #define SSL3_ST_CW_CLNT_HELLO_B (0x111 | SSL_ST_CONNECT)

ssl/d1_clnt.c

@@ -260,7 +260,7 @@ int dtls1_connect(SSL *s) {
           if (s->s3->tmp.certificate_status_expected) {
             s->state = SSL3_ST_CR_CERT_STATUS_A;
           } else {
-            s->state = SSL3_ST_CR_KEY_EXCH_A;
+            s->state = SSL3_ST_VERIFY_SERVER_CERT;
           }
         } else {
           skip = 1;
@@ -269,6 +269,16 @@ int dtls1_connect(SSL *s) {
         s->init_num = 0;
         break;
 
+      case SSL3_ST_VERIFY_SERVER_CERT:
+        ret = ssl3_verify_server_cert(s);
+        if (ret <= 0) {
+          goto end;
+        }
+
+        s->state = SSL3_ST_CR_KEY_EXCH_A;
+        s->init_num = 0;
+        break;
+
       case SSL3_ST_CR_KEY_EXCH_A:
       case SSL3_ST_CR_KEY_EXCH_B:
         ret = ssl3_get_server_key_exchange(s);
@@ -418,7 +428,7 @@ int dtls1_connect(SSL *s) {
         if (ret <= 0) {
           goto end;
         }
-        s->state = SSL3_ST_CR_KEY_EXCH_A;
+        s->state = SSL3_ST_VERIFY_SERVER_CERT;
         s->init_num = 0;
         break;
 

ssl/internal.h

@@ -1119,6 +1119,7 @@ int ssl3_get_server_key_exchange(SSL *s);
 int ssl3_get_server_certificate(SSL *s);
 int ssl3_send_next_proto(SSL *s);
 int ssl3_send_channel_id(SSL *s);
+int ssl3_verify_server_cert(SSL *s);
 
 /* some server-only functions */
 int ssl3_get_initial_bytes(SSL *s);

ssl/s3_clnt.c

@@ -276,7 +276,7 @@ int ssl3_connect(SSL *s) {
           if (s->s3->tmp.certificate_status_expected) {
             s->state = SSL3_ST_CR_CERT_STATUS_A;
           } else {
-            s->state = SSL3_ST_CR_KEY_EXCH_A;
+            s->state = SSL3_ST_VERIFY_SERVER_CERT;
           }
         } else {
           skip = 1;
@@ -285,6 +285,16 @@ int ssl3_connect(SSL *s) {
         s->init_num = 0;
         break;
 
+      case SSL3_ST_VERIFY_SERVER_CERT:
+        ret = ssl3_verify_server_cert(s);
+        if (ret <= 0) {
+          goto end;
+        }
+
+        s->state = SSL3_ST_CR_KEY_EXCH_A;
+        s->init_num = 0;
+        break;
+
       case SSL3_ST_CR_KEY_EXCH_A:
       case SSL3_ST_CR_KEY_EXCH_B:
         ret = ssl3_get_server_key_exchange(s);
@@ -468,7 +478,7 @@ int ssl3_connect(SSL *s) {
         if (ret <= 0) {
           goto end;
         }
-        s->state = SSL3_ST_CR_KEY_EXCH_A;
+        s->state = SSL3_ST_VERIFY_SERVER_CERT;
         s->init_num = 0;
         break;
 
@@ -946,7 +956,7 @@ err:
 }
 
 int ssl3_get_server_certificate(SSL *s) {
-  int al, i, ok, ret = -1;
+  int al, ok, ret = -1;
   unsigned long n;
   X509 *x = NULL;
   STACK_OF(X509) *sk = NULL;
@@ -1004,14 +1014,6 @@ int ssl3_get_server_certificate(SSL *s) {
     x = NULL;
   }
 
-  i = ssl_verify_cert_chain(s, sk);
-  if (s->verify_mode != SSL_VERIFY_NONE && i <= 0) {
-    al = ssl_verify_alarm_type(s->verify_result);
-    OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_VERIFY_FAILED);
-    goto f_err;
-  }
-  ERR_clear_error(); /* but we keep s->verify_result */
-
   X509 *leaf = sk_X509_value(sk, 0);
   if (!ssl3_check_certificate_for_cipher(leaf, s->s3->tmp.new_cipher)) {
     al = SSL_AD_ILLEGAL_PARAMETER;
@@ -2197,3 +2199,17 @@ int ssl_do_client_cert_cb(SSL *s, X509 **px509, EVP_PKEY **ppkey) {
   }
   return i;
 }
+
+int ssl3_verify_server_cert(SSL *s) {
+  int ret = ssl_verify_cert_chain(s, s->session->cert_chain);
+  if (s->verify_mode != SSL_VERIFY_NONE && ret <= 0) {
+    int al = ssl_verify_alarm_type(s->verify_result);
+    ssl3_send_alert(s, SSL3_AL_FATAL, al);
+    OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_VERIFY_FAILED);
+  } else {
+    ret = 1;
+    ERR_clear_error(); /* but we keep s->verify_result */
+  }
+
+  return ret;
+}

ssl/test/bssl_shim.cc

@@ -301,10 +301,29 @@ static int SelectCertificateCallback(const struct ssl_early_callback_ctx *ctx) {
   return 1;
 }
 
-static int SkipVerify(int preverify_ok, X509_STORE_CTX *store_ctx) {
+static int VerifySucceed(X509_STORE_CTX *store_ctx, void *arg) {
+  SSL* ssl = (SSL*)X509_STORE_CTX_get_ex_data(store_ctx,
+      SSL_get_ex_data_X509_STORE_CTX_idx());
+  const TestConfig *config = GetConfigPtr(ssl);
+
+  if (!config->expected_ocsp_response.empty()) {
+    const uint8_t *data;
+    size_t len;
+    SSL_get0_ocsp_response(ssl, &data, &len);
+    if (len == 0) {
+      fprintf(stderr, "OCSP response not available in verify callback\n");
+      return 0;
+    }
+  }
+
   return 1;
 }
 
+static int VerifyFail(X509_STORE_CTX *store_ctx, void *arg) {
+  store_ctx->error = X509_V_ERR_APPLICATION_VERIFICATION;
+  return 0;
+}
+
 static int NextProtosAdvertisedCallback(SSL *ssl, const uint8_t **out,
                                         unsigned int *out_len, void *arg) {
   const TestConfig *config = GetConfigPtr(ssl);
@@ -675,6 +694,12 @@ static ScopedSSL_CTX SetupCtx(const TestConfig *config) {
     return nullptr;
   }
 
+  if (config->verify_fail) {
+    SSL_CTX_set_cert_verify_callback(ssl_ctx.get(), VerifyFail, NULL);
+  } else {
+    SSL_CTX_set_cert_verify_callback(ssl_ctx.get(), VerifySucceed, NULL);
+  }
+
   return ssl_ctx;
 }
 
@@ -908,6 +933,17 @@ static bool CheckHandshakeProperties(SSL *ssl, bool is_resume) {
     }
   }
 
+  if (config->expect_verify_result) {
+    int expected_verify_result = config->verify_fail ?
+      X509_V_ERR_APPLICATION_VERIFICATION :
+      X509_V_OK;
+
+    if (SSL_get_verify_result(ssl) != expected_verify_result) {
+      fprintf(stderr, "Wrong certificate verification result\n");
+      return false;
+    }
+  }
+
   if (!config->is_server) {
     /* Clients should expect a peer certificate chain iff this was not a PSK
      * cipher suite. */
@@ -955,7 +991,10 @@ static bool DoExchange(ScopedSSL_SESSION *out_session, SSL_CTX *ssl_ctx,
   }
   if (config->require_any_client_certificate) {
     SSL_set_verify(ssl.get(), SSL_VERIFY_PEER|SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
-                   SkipVerify);
+                   NULL);
+  }
+  if (config->verify_peer) {
+    SSL_set_verify(ssl.get(), SSL_VERIFY_PEER, NULL);
   }
   if (config->false_start) {
     SSL_set_mode(ssl.get(), SSL_MODE_ENABLE_FALSE_START);

ssl/test/runner/runner.go

@@ -2624,6 +2624,7 @@ func addStateMachineCoverageTests(async, splitHandshake bool, protocol protocol)
 			"-enable-ocsp-stapling",
 			"-expect-ocsp-response",
 			base64.StdEncoding.EncodeToString(testOCSPResponse),
+			"-verify-peer",
 		},
 	})
 
@@ -2637,6 +2638,34 @@ func addStateMachineCoverageTests(async, splitHandshake bool, protocol protocol)
 		},
 	})
 
+	tests = append(tests, testCase{
+		testType: clientTest,
+		name:     "CertificateVerificationSucceed",
+		flags: []string{
+			"-verify-peer",
+		},
+	})
+
+	tests = append(tests, testCase{
+		testType: clientTest,
+		name:     "CertificateVerificationFail",
+		flags: []string{
+			"-verify-fail",
+			"-verify-peer",
+		},
+		shouldFail:    true,
+		expectedError: ":CERTIFICATE_VERIFY_FAILED:",
+	})
+
+	tests = append(tests, testCase{
+		testType: clientTest,
+		name:     "CertificateVerificationSoftFail",
+		flags: []string{
+			"-verify-fail",
+			"-expect-verify-result",
+		},
+	})
+
 	if protocol == tls {
 		tests = append(tests, testCase{
 			name:        "Renegotiate-Client",

ssl/test/test_config.cc

@@ -93,6 +93,9 @@ const Flag<bool> kBoolFlags[] = {
   { "-check-close-notify", &TestConfig::check_close_notify },
   { "-shim-shuts-down", &TestConfig::shim_shuts_down },
   { "-microsoft-big-sslv3-buffer", &TestConfig::microsoft_big_sslv3_buffer },
+  { "-verify-fail", &TestConfig::verify_fail },
+  { "-verify-peer", &TestConfig::verify_peer },
+  { "-expect-verify-result", &TestConfig::expect_verify_result }
 };
 
 const Flag<std::string> kStringFlags[] = {

ssl/test/test_config.h

@@ -92,6 +92,9 @@ struct TestConfig {
   bool check_close_notify = false;
   bool shim_shuts_down = false;
   bool microsoft_big_sslv3_buffer = false;
+  bool verify_fail = false;
+  bool verify_peer = false;
+  bool expect_verify_result = false;
 };
 
 bool ParseConfig(int argc, char **argv, TestConfig *out_config);