Fix TlsHandshakerTest.HandshakeWithAsyncProofVerifier

Bug: b/69008819
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I2d7c9c77f19964227ba46c4e2c54b01c8124f346
Reviewed-on: https://chromium-review.googlesource.com/764795
Commit-Queue: Nick Harper <nharper@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515902}

net/quic/core/tls_client_handshaker.cc

@@ -213,10 +213,9 @@ enum ssl_verify_result_t TlsClientHandshaker::VerifyCert(uint8_t* out_alert) {
   ProofVerifierCallbackImpl* proof_verify_callback =
       new ProofVerifierCallbackImpl(this);
 
-  string error_details;
   QuicAsyncStatus verify_result = proof_verifier_->VerifyCertChain(
-      server_id_.host(), certs, verify_context_.get(), &error_details,
-      &verify_details_,
+      server_id_.host(), certs, verify_context_.get(),
+      &cert_verify_error_details_, &verify_details_,
       std::unique_ptr<ProofVerifierCallback>(proof_verify_callback));
   switch (verify_result) {
     case QUIC_SUCCESS:
@@ -227,7 +226,8 @@ enum ssl_verify_result_t TlsClientHandshaker::VerifyCert(uint8_t* out_alert) {
       return ssl_verify_retry;
     case QUIC_FAILURE:
     default:
-      QUIC_LOG(INFO) << "Cert chain verification failed: " << error_details;
+      QUIC_LOG(INFO) << "Cert chain verification failed: "
+                     << cert_verify_error_details_;
       return ssl_verify_invalid;
   }
 }

net/quic/core/tls_client_handshaker.h

@@ -98,6 +98,7 @@ class QUIC_EXPORT_PRIVATE TlsClientHandshaker
   ProofVerifierCallbackImpl* proof_verify_callback_ = nullptr;
   std::unique_ptr<ProofVerifyDetails> verify_details_;
   enum ssl_verify_result_t verify_result_ = ssl_verify_retry;
+  std::string cert_verify_error_details_;
 
   bool encryption_established_ = false;
   bool handshake_confirmed_ = false;

net/quic/core/tls_handshaker_test.cc

@@ -70,6 +70,17 @@ class FakeProofVerifier : public ProofVerifier {
   }
 
  private:
+  // Implementation of ProofVerifierCallback that fails if the callback is ever
+  // run.
+  class FailingProofVerifierCallback : public ProofVerifierCallback {
+   public:
+    void Run(bool ok,
+             const std::string& error_details,
+             std::unique_ptr<ProofVerifyDetails>* details) override {
+      FAIL();
+    }
+  };
+
   class VerifyChainPendingOp {
    public:
     VerifyChainPendingOp(const string& hostname,
@@ -89,10 +100,12 @@ class FakeProofVerifier : public ProofVerifier {
 
     void Run() {
       // FakeProofVerifier depends on crypto_test_utils::ProofVerifierForTesting
-      // running synchronously, and passes a null callback.
+      // running synchronously. It passes a FailingProofVerifierCallback and
+      // runs the original callback after asserting that the verification ran
+      // synchronously.
       QuicAsyncStatus status = delegate_->VerifyCertChain(
           hostname_, certs_, context_, error_details_, details_,
-          std::unique_ptr<ProofVerifierCallback>());
+          QuicMakeUnique<FailingProofVerifierCallback>());
       ASSERT_NE(status, QUIC_PENDING);
       callback_->Run(status == QUIC_SUCCESS, *error_details_, details_);
     }
@@ -320,7 +333,7 @@ TEST_F(TlsHandshakerTest, CancelPendingProofSource) {
   proof_source->InvokePendingCallback(0);
 }
 
-TEST_F(TlsHandshakerTest, DISABLED_HandshakeWithAsyncProofVerifier) {
+TEST_F(TlsHandshakerTest, HandshakeWithAsyncProofVerifier) {
   EXPECT_CALL(*client_conn_, CloseConnection(_, _, _)).Times(0);
   EXPECT_CALL(*server_conn_, CloseConnection(_, _, _)).Times(0);
   // Enable FakeProofVerifier to capture call to VerifyCertChain and run it