Verify SignedExchange cert in parallel with signature verification

Signature verification of signed exchange takes some time (>10ms on
Nexus 5X), so total latency of SignedExchange loading can be reduced by
starting certificate verification before doing signature verification.

Unit tests are changed so that mock verifiers don't complete Verify()
synchronously.

Change-Id: Idc8e222c810e3a65db9072b21ed852daf8bcd316
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2309517Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794373}

content/browser/web_package/signed_exchange_handler.cc

@@ -84,12 +84,23 @@ using VerifyCallback = base::OnceCallback<void(int32_t,
                                                const net::CertVerifyResult&,
                                                const net::ct::CTVerifyResult&)>;
 
-void VerifyCert(const scoped_refptr<net::X509Certificate>& certificate,
+void VerifyCert(const SignedExchangeCertificateChain& unverified_cert_chain,
                 const GURL& url,
-                const std::string& ocsp_result,
-                const std::string& sct_list,
                 int frame_tree_node_id,
                 VerifyCallback callback) {
+  // https://wicg.github.io/webpackage/draft-yasskin-http-origin-signed-responses.html#cross-origin-trust
+  // Step 6.4 Validate that valid SCTs from trusted logs are available from any
+  // of:
+  // - The SignedCertificateTimestampList in main-certificate’s sct property
+  //   (Section 3.3),
+  const std::string& sct_list_from_cert_cbor = unverified_cert_chain.sct();
+  // - An OCSP extension in the OCSP response in main-certificate’s ocsp
+  //   property, or
+  const std::string& stapled_ocsp_response = unverified_cert_chain.ocsp();
+  // - An X.509 extension in the certificate in main-certificate's cert property
+  const scoped_refptr<net::X509Certificate>& certificate =
+      unverified_cert_chain.cert();
+
   VerifyCallback wrapped_callback = mojo::WrapCallbackWithDefaultInvokeIfNotRun(
       std::move(callback), net::ERR_FAILED, net::CertVerifyResult(),
       net::ct::CTVerifyResult());
@@ -108,7 +119,8 @@ void VerifyCert(const scoped_refptr<net::X509Certificate>& certificate,
   }
 
   network_context->VerifyCertForSignedExchange(
-      certificate, url, ocsp_result, sct_list, std::move(wrapped_callback));
+      certificate, url, stapled_ocsp_response, sct_list_from_cert_cbor,
+      std::move(wrapped_callback));
 }
 
 std::string OCSPErrorToString(const net::OCSPVerifyResult& ocsp_result) {
@@ -488,6 +500,13 @@ void SignedExchangeHandler::OnCertReceived(
                              cert_fetch_duration);
   unverified_cert_chain_ = std::move(cert_chain);
 
+  // Start cert verification here so that it runs in parallel with signature
+  // verification.
+  VerifyCert(*unverified_cert_chain_, envelope_->request_url().url,
+             frame_tree_node_id_,
+             base::BindOnce(&SignedExchangeHandler::OnVerifyCert,
+                            weak_factory_.GetWeakPtr()));
+
   DCHECK(version_.has_value());
   const SignedExchangeSignatureVerifier::Result verify_result =
       SignedExchangeSignatureVerifier::Verify(
@@ -509,24 +528,7 @@ void SignedExchangeHandler::OnCertReceived(
         net::ERR_INVALID_SIGNED_EXCHANGE);
     return;
   }
-
-  auto certificate = unverified_cert_chain_->cert();
-  auto url = envelope_->request_url().url;
-
-  // https://wicg.github.io/webpackage/draft-yasskin-http-origin-signed-responses.html#cross-origin-trust
-  // Step 6.4 Validate that valid SCTs from trusted logs are available from any
-  // of:
-  // - The SignedCertificateTimestampList in main-certificate’s sct property
-  //   (Section 3.3),
-  const std::string& sct_list_from_cert_cbor = unverified_cert_chain_->sct();
-  // - An OCSP extension in the OCSP response in main-certificate’s ocsp
-  //   property, or
-  const std::string& stapled_ocsp_response = unverified_cert_chain_->ocsp();
-
-  VerifyCert(certificate, url, stapled_ocsp_response, sct_list_from_cert_cbor,
-             frame_tree_node_id_,
-             base::BindOnce(&SignedExchangeHandler::OnVerifyCert,
-                            weak_factory_.GetWeakPtr()));
+  state_ = State::kSignatureVerified;
 }
 
 // https://wicg.github.io/webpackage/draft-yasskin-http-origin-signed-responses.html#cross-origin-cert-req
@@ -601,6 +603,15 @@ void SignedExchangeHandler::OnVerifyCert(
     const net::ct::CTVerifyResult& ct_result) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("loading"),
                "SignedExchangeHandler::OnCertVerifyComplete");
+
+  if (state_ == State::kHeadersCallbackCalled) {
+    // This means signature verification has failed.
+    // NOTE: This code may not be reached because |this_| may have been deleted
+    // by |headers_callback_|.
+    return;
+  }
+  CHECK_EQ(state_, State::kSignatureVerified);
+
   // net::Error codes are negative, so we put - in front of it.
   base::UmaHistogramSparse(kHistogramCertVerificationResult, -error_code);
   UMA_HISTOGRAM_ENUMERATION(kHistogramCTVerificationResult,

content/browser/web_package/signed_exchange_handler.h

@@ -122,6 +122,7 @@ class CONTENT_EXPORT SignedExchangeHandler {
     kReadingPrologueFallbackUrlAndAfter,
     kReadingHeaders,
     kFetchingCertificate,
+    kSignatureVerified,
     kHeadersCallbackCalled,
   };
 

content/browser/web_package/signed_exchange_handler_unittest.cc

@@ -137,7 +137,10 @@ class GMockCertVerifier : public net::CertVerifier {
              std::unique_ptr<net::CertVerifier::Request>* out_req,
              const net::NetLogWithSource& net_log) override {
     verify_result->Reset();
-    return VerifyImpl(params, verify_result, out_req, net_log);
+    int result = VerifyImpl(params, verify_result, out_req, net_log);
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(std::move(callback), result));
+    return net::ERR_IO_PENDING;
   }
 
   MOCK_METHOD4(VerifyImpl,
@@ -265,6 +268,7 @@ class SignedExchangeHandlerTest
         LoadCertificate(cert_file);
     result.verified_cert = original_cert;
     auto mock_cert_verifier = std::make_unique<net::MockCertVerifier>();
+    mock_cert_verifier->set_async(true);
     mock_cert_verifier->AddResultForCertAndHost(
         original_cert, "test.example.org", result, net::OK);
     SetCertVerifier(std::move(mock_cert_verifier));
@@ -693,6 +697,7 @@ TEST_P(SignedExchangeHandlerTest, CertSha256Mismatch) {
   // Set the default result of MockCertVerifier to OK, to check that the
   // verification of SignedExchange must fail even if the certificate is valid.
   auto mock_cert_verifier = std::make_unique<net::MockCertVerifier>();
+  mock_cert_verifier->set_async(true);
   mock_cert_verifier->set_default_result(net::OK);
   SetCertVerifier(std::move(mock_cert_verifier));