i Revert 213862 "net: make QUIC ProofVerifier more generic."

> net: make QUIC ProofVerifier more generic.
> 
> The QUIC ProofVerifier code is currently rather Chromium specific: it's using
> weak pointers, base::Bind etc. This change wraps abstractions around things so
> that the code is more portable again.
> 
> It also solves an issue where, when a QUIC connection is canceled while a
> verification is running, the verification can write into free memory.
> 
> I went back and forth on this change a bit. It effectively reinvents weak
> pointers and callbacks in order not to use the Chromium versions of these
> things. This is unfortunate but it is desirable to minimise the amount of skew
> between different copies of the QUIC code. In the end, the duplicate didn't
> seem so bad to me.
> 
> Weak pointers are replaced with an explicit callback interface for the
> ProofVerifier. The QUIC client stream implements a Cancel method so that it can
> cope with being deleted while a proof verification is still running.
> 
> The need to store a CertVerifyDetails is taken care of with an abstract class
> for wrapping "implementation specific" results from a verification. There is
> still Chromium-specific code that casts it to a Chromium object, but that's
> unavoidable somewhere. (Although it's not clear that the QUIC client stream is
> the best place for it.)
> 
> BUG=none
> 
> Review URL: https://chromiumcodereview.appspot.com/20047002

TBR=agl@chromium.org

Review URL: https://codereview.chromium.org/20822005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@214035 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 194 insertions, 342 deletions

diff --git a/net/net.gyp b/net/net.gyp
index 2a9d78a..657a772 100644
--- a/net/net.gyp
+++ b/net/net.gyp
@@ -762,7 +762,6 @@
         'quic/crypto/proof_source.h',
         'quic/crypto/proof_source_chromium.cc',
         'quic/crypto/proof_source_chromium.h',
-        'quic/crypto/proof_verifier.cc',
         'quic/crypto/proof_verifier_chromium.cc',
         'quic/crypto/proof_verifier_chromium.h',
         'quic/crypto/quic_decrypter.cc',
diff --git a/net/quic/crypto/crypto_handshake.cc b/net/quic/crypto/crypto_handshake.cc
index ad4366d..00cc342 100644
--- a/net/quic/crypto/crypto_handshake.cc
+++ b/net/quic/crypto/crypto_handshake.cc
@@ -500,9 +500,9 @@ uint64 QuicCryptoClientConfig::CachedState::generation_counter() const {
   return generation_counter_;
 }
 
-const ProofVerifyDetails*
-QuicCryptoClientConfig::CachedState::proof_verify_details() const {
-  return proof_verify_details_.get();
+const CertVerifyResult*
+QuicCryptoClientConfig::CachedState::cert_verify_result() const {
+  return &cert_verify_result_;
 }
 
 void QuicCryptoClientConfig::CachedState::set_source_address_token(
@@ -510,9 +510,9 @@ void QuicCryptoClientConfig::CachedState::set_source_address_token(
   source_address_token_ = token.as_string();
 }
 
-void QuicCryptoClientConfig::CachedState::SetProofVerifyDetails(
-    ProofVerifyDetails* details) {
-  proof_verify_details_.reset(details);
+void QuicCryptoClientConfig::CachedState::SetCertVerifyResult(
+    const CertVerifyResult& cert_verify_result) {
+  cert_verify_result_.CopyFrom(cert_verify_result);
 }
 
 void QuicCryptoClientConfig::SetDefaults() {
diff --git a/net/quic/crypto/crypto_handshake.h b/net/quic/crypto/crypto_handshake.h
index fdc92a0..d2f11f8 100644
--- a/net/quic/crypto/crypto_handshake.h
+++ b/net/quic/crypto/crypto_handshake.h
@@ -15,7 +15,6 @@
 #include "net/cert/cert_verify_result.h"
 #include "net/cert/x509_certificate.h"
 #include "net/quic/crypto/crypto_protocol.h"
-#include "net/quic/crypto/proof_verifier.h"
 #include "net/quic/quic_protocol.h"
 
 namespace net {
@@ -282,12 +281,10 @@ class NET_EXPORT_PRIVATE QuicCryptoClientConfig : public QuicCryptoConfig {
     const std::string& signature() const;
     bool proof_valid() const;
     uint64 generation_counter() const;
-    const ProofVerifyDetails* proof_verify_details() const;
+    const CertVerifyResult* cert_verify_result() const;
 
     void set_source_address_token(base::StringPiece token);
-
-    // SetProofVerifyDetails takes ownership of |details|.
-    void SetProofVerifyDetails(ProofVerifyDetails* details);
+    void SetCertVerifyResult(const CertVerifyResult& cert_verify_result);
 
    private:
     std::string server_config_id_;      // An opaque id from the server.
@@ -304,7 +301,10 @@ class NET_EXPORT_PRIVATE QuicCryptoClientConfig : public QuicCryptoConfig {
     // server_config_valid_ to false.
     uint64 generation_counter_;
 
-    scoped_ptr<ProofVerifyDetails> proof_verify_details_;
+    // The result of certificate verification.
+    // TODO(rtenneti): should we change CertVerifyResult to be
+    // RefCountedThreadSafe object to avoid copying.
+    CertVerifyResult cert_verify_result_;
 
     // scfg contains the cached, parsed value of |server_config|.
     mutable scoped_ptr<CryptoHandshakeMessage> scfg_;
diff --git a/net/quic/crypto/proof_test.cc b/net/quic/crypto/proof_test.cc
index 8facb82..3258c12 100644
--- a/net/quic/crypto/proof_test.cc
+++ b/net/quic/crypto/proof_test.cc
@@ -89,68 +89,6 @@ TEST(Proof, Verify) {
 #endif  // 0
 }
 
-// TestProofVerifierCallback is a simple callback for a ProofVerifier that
-// signals a TestCompletionCallback when called and stores the results from the
-// ProofVerifier in pointers passed to the constructor.
-class TestProofVerifierCallback : public ProofVerifierCallback {
- public:
-  TestProofVerifierCallback(TestCompletionCallback* comp_callback,
-                            bool* ok,
-                            std::string* error_details)
-      : comp_callback_(comp_callback),
-        ok_(ok),
-        error_details_(error_details) {}
-
-  virtual void Run(bool ok,
-                   const std::string& error_details,
-                   scoped_ptr<ProofVerifyDetails>* details) OVERRIDE {
-    *ok_ = ok;
-    *error_details_ = error_details;
-
-    comp_callback_->callback().Run(0);
-  }
-
- private:
-  TestCompletionCallback* const comp_callback_;
-  bool* const ok_;
-  std::string* const error_details_;
-};
-
-// RunVerification runs |verifier->VerifyProof| and asserts that the result
-// matches |expected_ok|.
-static void RunVerification(ProofVerifier* verifier,
-                            const std::string& hostname,
-                            const std::string& server_config,
-                            const vector<std::string>& certs,
-                            const std::string& proof,
-                            bool expected_ok) {
-  scoped_ptr<ProofVerifyDetails> details;
-  TestCompletionCallback comp_callback;
-  bool ok;
-  std::string error_details;
-  TestProofVerifierCallback* callback =
-      new TestProofVerifierCallback(&comp_callback, &ok, &error_details);
-
-  ProofVerifier::Status status = verifier->VerifyProof(
-      hostname, server_config, certs, proof, &error_details, &details,
-      callback);
-
-  switch (status) {
-    case ProofVerifier::FAILURE:
-      ASSERT_FALSE(expected_ok);
-      ASSERT_NE("", error_details);
-      return;
-    case ProofVerifier::SUCCESS:
-      ASSERT_TRUE(expected_ok);
-      ASSERT_EQ("", error_details);
-      return;
-    case ProofVerifier::PENDING:
-      comp_callback.WaitForResult();
-      ASSERT_EQ(expected_ok, ok);
-      break;
-  }
-}
-
 static string PEMCertFileToDER(const string& file_name) {
   base::FilePath certs_dir = GetTestCertsDirectory();
   scoped_refptr<X509Certificate> cert =
@@ -289,26 +227,48 @@ TEST(Proof, VerifyRSAKnownAnswerTest) {
 
   for (size_t i = 0; i < signatures.size(); i++) {
     const string& signature = signatures[i];
-
-    RunVerification(
-        verifier.get(), hostname, server_config, certs, signature, true);
-    RunVerification(
-        verifier.get(), "foo.com", server_config, certs, signature, false);
-    RunVerification(
-        verifier.get(), hostname, server_config.substr(1, string::npos),
-        certs, signature, false);
+    int rv;
+    TestCompletionCallback callback;
+    rv = verifier->VerifyProof(hostname, server_config, certs, signature,
+                               &error_details, &cert_verify_result,
+                               callback.callback());
+    rv = callback.GetResult(rv);
+    ASSERT_EQ(OK, rv);
+    ASSERT_EQ("", error_details);
+    ASSERT_FALSE(IsCertStatusError(cert_verify_result.cert_status));
+
+    rv = verifier->VerifyProof("foo.com", server_config, certs, signature,
+                               &error_details, &cert_verify_result,
+                               callback.callback());
+    rv = callback.GetResult(rv);
+    ASSERT_EQ(ERR_FAILED, rv);
+    ASSERT_NE("", error_details);
+
+    rv = verifier->VerifyProof(hostname, server_config.substr(1, string::npos),
+                               certs, signature, &error_details,
+                               &cert_verify_result, callback.callback());
+    rv = callback.GetResult(rv);
+    ASSERT_EQ(ERR_FAILED, rv);
+    ASSERT_NE("", error_details);
 
     const string corrupt_signature = "1" + signature;
-    RunVerification(
-        verifier.get(), hostname, server_config, certs, corrupt_signature,
-        false);
+    rv = verifier->VerifyProof(hostname, server_config, certs,
+                               corrupt_signature, &error_details,
+                               &cert_verify_result, callback.callback());
+    rv = callback.GetResult(rv);
+    ASSERT_EQ(ERR_FAILED, rv);
+    ASSERT_NE("", error_details);
 
     vector<string> wrong_certs;
     for (size_t i = 1; i < certs.size(); i++) {
       wrong_certs.push_back(certs[i]);
     }
-    RunVerification(verifier.get(), hostname, server_config, wrong_certs,
-                    signature, false);
+    rv = verifier->VerifyProof("foo.com", server_config, wrong_certs, signature,
+                               &error_details, &cert_verify_result,
+                               callback.callback());
+    rv = callback.GetResult(rv);
+    ASSERT_EQ(ERR_FAILED, rv);
+    ASSERT_NE("", error_details);
   }
 }
 
@@ -380,35 +340,60 @@ TEST(Proof, MAYBE_VerifyECDSAKnownAnswerTest) {
 
   for (size_t i = 0; i < signatures.size(); i++) {
     const string& signature = signatures[i];
-
-    RunVerification(
-        verifier.get(), hostname, server_config, certs, signature, true);
-    RunVerification(
-        verifier.get(), "foo.com", server_config, certs, signature, false);
-    RunVerification(
-        verifier.get(), hostname, server_config.substr(1, string::npos),
-        certs, signature, false);
+    int rv;
+    TestCompletionCallback callback;
+    rv = verifier->VerifyProof(hostname, server_config, certs, signature,
+                               &error_details, &cert_verify_result,
+                               callback.callback());
+    rv = callback.GetResult(rv);
+    ASSERT_EQ(OK, rv);
+    ASSERT_EQ("", error_details);
+    ASSERT_FALSE(IsCertStatusError(cert_verify_result.cert_status));
+
+    rv = verifier->VerifyProof("foo.com", server_config, certs, signature,
+                               &error_details, &cert_verify_result,
+                               callback.callback());
+    rv = callback.GetResult(rv);
+    ASSERT_EQ(ERR_FAILED, rv);
+    ASSERT_NE("", error_details);
+
+    rv = verifier->VerifyProof(hostname, server_config.substr(1, string::npos),
+                               certs, signature, &error_details,
+                               &cert_verify_result, callback.callback());
+    rv = callback.GetResult(rv);
+    ASSERT_EQ(ERR_FAILED, rv);
+    ASSERT_NE("", error_details);
 
     // An ECDSA signature is DER-encoded. Corrupt the last byte so that the
     // signature can still be DER-decoded correctly.
     string corrupt_signature = signature;
     corrupt_signature[corrupt_signature.size() - 1] += 1;
-    RunVerification(
-        verifier.get(), hostname, server_config, certs, corrupt_signature,
-        false);
+    rv = verifier->VerifyProof(hostname, server_config, certs,
+                               corrupt_signature, &error_details,
+                               &cert_verify_result, callback.callback());
+    rv = callback.GetResult(rv);
+    ASSERT_EQ(ERR_FAILED, rv);
+    ASSERT_NE("", error_details);
 
     // Prepending a "1" makes the DER invalid.
     const string bad_der_signature1 = "1" + signature;
-    RunVerification(
-        verifier.get(), hostname, server_config, certs, bad_der_signature1,
-        false);
+    rv = verifier->VerifyProof(hostname, server_config, certs,
+                               bad_der_signature1, &error_details,
+                               &cert_verify_result, callback.callback());
+    rv = callback.GetResult(rv);
+    ASSERT_EQ(ERR_FAILED, rv);
+    ASSERT_NE("", error_details);
 
     vector<string> wrong_certs;
     for (size_t i = 1; i < certs.size(); i++) {
       wrong_certs.push_back(certs[i]);
     }
-    RunVerification(
-        verifier.get(), hostname, server_config, wrong_certs, signature, false);
+    rv = verifier->VerifyProof("foo.com", server_config, wrong_certs, signature,
+                               &error_details, &cert_verify_result,
+                               callback.callback());
+    rv = callback.GetResult(rv);
+    ASSERT_EQ(ERR_FAILED, rv);
+    ASSERT_NE("", error_details);
   }
 }
 
diff --git a/net/quic/crypto/proof_verifier.cc b/net/quic/crypto/proof_verifier.cc
deleted file mode 100644
index 7bccba2..0000000
--- a/net/quic/crypto/proof_verifier.cc
+++ /dev/null
@@ -1,15 +0,0 @@
-// Copyright 2013 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "net/quic/crypto/proof_verifier.h"
-
-namespace net {
-
-ProofVerifyDetails::~ProofVerifyDetails() {}
-
-ProofVerifierCallback::~ProofVerifierCallback() {}
-
-ProofVerifier::~ProofVerifier() {}
-
-}  // namespace net
diff --git a/net/quic/crypto/proof_verifier.h b/net/quic/crypto/proof_verifier.h
index f469c55..eb96898 100644
--- a/net/quic/crypto/proof_verifier.h
+++ b/net/quic/crypto/proof_verifier.h
@@ -15,68 +15,32 @@ namespace net {
 
 class CertVerifyResult;
 
-// ProofVerifyDetails is an abstract class that acts as a container for any
-// implementation specific details that a ProofVerifier wishes to return. These
-// details are saved in the CachedInfo for the origin in question.
-class ProofVerifyDetails {
- public:
-  virtual ~ProofVerifyDetails();
-};
-
-// ProofVerifierCallback provides a generic mechanism for a ProofVerifier to
-// call back after an asynchronous verification.
-class NET_EXPORT_PRIVATE ProofVerifierCallback {
- public:
-  virtual ~ProofVerifierCallback();
-
-  // Run is called on the original thread to mark the completion of an
-  // asynchonous verification. If |ok| is true then the certificate is valid
-  // and |*error_details| is unused. Otherwise, |*error_details| contains a
-  // description of the error. |details| contains implementation-specific
-  // details of the verification. |Run| may take ownership of |details| by
-  // calling |release| on it.
-  virtual void Run(bool ok,
-                   const std::string& error_details,
-                   scoped_ptr<ProofVerifyDetails>* details) = 0;
-};
-
 // A ProofVerifier checks the signature on a server config, and the certificate
 // chain that backs the public key.
 class NET_EXPORT_PRIVATE ProofVerifier {
  public:
-  // Status enumerates the possible results of verifying a proof.
-  enum Status {
-    SUCCESS = 0,
-    FAILURE = 1,
-    // PENDING results from a verification which will occur asynchonously. When
-    // the verification is complete, |callback|'s |Run| method will be called.
-    PENDING = 2,
-  };
-
-  virtual ~ProofVerifier();
+  virtual ~ProofVerifier() {}
 
   // VerifyProof checks that |signature| is a valid signature of
   // |server_config| by the public key in the leaf certificate of |certs|, and
-  // that |certs| is a valid chain for |hostname|. On success, it returns
-  // SUCCESS. On failure, it returns ERROR and sets |*error_details| to a
-  // description of the problem. In either case it may set |*details|, which the
-  // caller takes ownership of.
-  //
-  // This function may also return PENDING, in which case the ProofVerifier
-  // will call back, on the original thread, via |callback| when complete.
-  //
-  // This function takes ownership of |callback|. It will be deleted even if
-  // the call returns immediately.
+  // that |certs| is a valid chain for |hostname|. On success, it returns OK.
+  // On failure, it returns ERR_FAILED and sets |*error_details| to a
+  // description of the problem. This function may also return ERR_IO_PENDING,
+  // in which case the |callback| will be run on the calling thread with the
+  // final OK/ERR_FAILED result when the proof is verified.
   //
   // The signature uses SHA-256 as the hash function and PSS padding in the
   // case of RSA.
-  virtual Status VerifyProof(const std::string& hostname,
-                             const std::string& server_config,
-                             const std::vector<std::string>& certs,
-                             const std::string& signature,
-                             std::string* error_details,
-                             scoped_ptr<ProofVerifyDetails>* details,
-                             ProofVerifierCallback* callback) = 0;
+  //
+  // Note: this is just for testing. The CN of the certificate is ignored and
+  // wildcards in the SANs are not supported.
+  virtual int VerifyProof(const std::string& hostname,
+                          const std::string& server_config,
+                          const std::vector<std::string>& certs,
+                          const std::string& signature,
+                          std::string* error_details,
+                          CertVerifyResult* cert_verify_result,
+                          const CompletionCallback& callback) = 0;
 };
 
 }  // namespace net
diff --git a/net/quic/crypto/proof_verifier_chromium.cc b/net/quic/crypto/proof_verifier_chromium.cc
index 4d70e6c..6f54788 100644
--- a/net/quic/crypto/proof_verifier_chromium.cc
+++ b/net/quic/crypto/proof_verifier_chromium.cc
@@ -33,6 +33,8 @@ namespace net {
 ProofVerifierChromium::ProofVerifierChromium(CertVerifier* cert_verifier,
                                              const BoundNetLog& net_log)
   : cert_verifier_(cert_verifier),
+    cert_verify_result_(NULL),
+    error_details_(NULL),
     next_state_(STATE_NONE),
     net_log_(net_log) {
 }
@@ -41,36 +43,29 @@ ProofVerifierChromium::~ProofVerifierChromium() {
   verifier_.reset();
 }
 
-ProofVerifierChromium::Status ProofVerifierChromium::VerifyProof(
-    const string& hostname,
-    const string& server_config,
-    const vector<string>& certs,
-    const string& signature,
-    std::string* error_details,
-    scoped_ptr<ProofVerifyDetails>* details,
-    ProofVerifierCallback* callback) {
+int ProofVerifierChromium::VerifyProof(const string& hostname,
+                                       const string& server_config,
+                                       const vector<string>& certs,
+                                       const string& signature,
+                                       std::string* error_details,
+                                       CertVerifyResult* cert_verify_result,
+                                       const CompletionCallback& callback) {
   DCHECK(error_details);
-  DCHECK(details);
-  DCHECK(callback);
-
-  callback_.reset(callback);
+  DCHECK(cert_verify_result);
   error_details->clear();
+  cert_verify_result->Reset();
 
   DCHECK_EQ(STATE_NONE, next_state_);
   if (STATE_NONE != next_state_) {
     *error_details = "Certificate is already set and VerifyProof has begun";
     DLOG(WARNING) << *error_details;
-    return FAILURE;
+    return ERR_FAILED;
   }
 
-  verify_details_.reset(new ProofVerifyDetailsChromium);
-
   if (certs.empty()) {
     *error_details = "Failed to create certificate chain. Certs are empty.";
     DLOG(WARNING) << *error_details;
-    verify_details_->cert_verify_result.cert_status = CERT_STATUS_INVALID;
-    details->reset(verify_details_.release());
-    return FAILURE;
+    return ERR_FAILED;
   }
 
   // Convert certs to X509Certificate.
@@ -82,9 +77,8 @@ ProofVerifierChromium::Status ProofVerifierChromium::VerifyProof(
   if (!cert_.get()) {
     *error_details = "Failed to create certificate chain";
     DLOG(WARNING) << *error_details;
-    verify_details_->cert_verify_result.cert_status = CERT_STATUS_INVALID;
-    details->reset(verify_details_.release());
-    return FAILURE;
+    cert_verify_result->cert_status = CERT_STATUS_INVALID;
+    return ERR_FAILED;
   }
 
   // We call VerifySignature first to avoid copying of server_config and
@@ -92,23 +86,16 @@ ProofVerifierChromium::Status ProofVerifierChromium::VerifyProof(
   if (!VerifySignature(server_config, signature, certs[0])) {
     *error_details = "Failed to verify signature of server config";
     DLOG(WARNING) << *error_details;
-    verify_details_->cert_verify_result.cert_status = CERT_STATUS_INVALID;
-    details->reset(verify_details_.release());
-    return FAILURE;
+    return ERR_FAILED;
   }
 
   hostname_ = hostname;
+  callback_ = callback;
+  error_details_ = error_details;
+  cert_verify_result_ = cert_verify_result;
 
   next_state_ = STATE_VERIFY_CERT;
-  switch (DoLoop(OK)) {
-    case OK:
-      return SUCCESS;
-    case ERR_IO_PENDING:
-      return PENDING;
-    default:
-      *error_details = error_details_;
-      return FAILURE;
-  }
+  return DoLoop(OK);
 }
 
 int ProofVerifierChromium::DoLoop(int last_result) {
@@ -137,9 +124,7 @@ int ProofVerifierChromium::DoLoop(int last_result) {
 void ProofVerifierChromium::OnIOComplete(int result) {
   int rv = DoLoop(result);
   if (rv != ERR_IO_PENDING) {
-    scoped_ptr<ProofVerifyDetails> scoped_details(verify_details_.release());
-    callback_->Run(rv == OK, error_details_, &scoped_details);
-    callback_.reset();
+    base::ResetAndReturn(&callback_).Run(rv);
   }
 }
 
@@ -153,7 +138,7 @@ int ProofVerifierChromium::DoVerifyCert(int result) {
       hostname_,
       flags,
       SSLConfigService::GetCRLSet().get(),
-      &verify_details_->cert_verify_result,
+      cert_verify_result_,
       base::Bind(&ProofVerifierChromium::OnIOComplete,
                  base::Unretained(this)),
       net_log_);
@@ -163,9 +148,9 @@ int ProofVerifierChromium::DoVerifyCertComplete(int result) {
   verifier_.reset();
 
   if (result <= ERR_FAILED) {
-    error_details_ = StringPrintf("Failed to verify certificate chain: %s",
-                                  ErrorToString(result));
-    DLOG(WARNING) << error_details_;
+    *error_details_ = StringPrintf("Failed to verify certificate chain: %s",
+                                   ErrorToString(result));
+    DLOG(WARNING) << *error_details_;
     result = ERR_FAILED;
   }
 
diff --git a/net/quic/crypto/proof_verifier_chromium.h b/net/quic/crypto/proof_verifier_chromium.h
index 4969cc8..134f62b 100644
--- a/net/quic/crypto/proof_verifier_chromium.h
+++ b/net/quic/crypto/proof_verifier_chromium.h
@@ -23,13 +23,6 @@ namespace net {
 class CertVerifier;
 class SingleRequestCertVerifier;
 
-// ProofVerifyDetailsChromium is the implementation-specific information that a
-// ProofVerifierChromium returns about a certificate verification.
-struct ProofVerifyDetailsChromium : public ProofVerifyDetails {
- public:
-  CertVerifyResult cert_verify_result;
-};
-
 // ProofVerifierChromium implements the QUIC ProofVerifier interface.
 // TODO(rtenneti): Add support for multiple requests for one ProofVerifier.
 class NET_EXPORT_PRIVATE ProofVerifierChromium : public ProofVerifier {
@@ -39,13 +32,13 @@ class NET_EXPORT_PRIVATE ProofVerifierChromium : public ProofVerifier {
   virtual ~ProofVerifierChromium();
 
   // ProofVerifier interface
-  virtual Status VerifyProof(const std::string& hostname,
-                             const std::string& server_config,
-                             const std::vector<std::string>& certs,
-                             const std::string& signature,
-                             std::string* error_details,
-                             scoped_ptr<ProofVerifyDetails>* details,
-                             ProofVerifierCallback* callback) OVERRIDE;
+  virtual int VerifyProof(const std::string& hostname,
+                          const std::string& server_config,
+                          const std::vector<std::string>& certs,
+                          const std::string& signature,
+                          std::string* error_details,
+                          CertVerifyResult* cert_verify_result,
+                          const CompletionCallback& callback) OVERRIDE;
 
  private:
   enum State {
@@ -70,9 +63,11 @@ class NET_EXPORT_PRIVATE ProofVerifierChromium : public ProofVerifier {
   // |hostname| specifies the hostname for which |certs| is a valid chain.
   std::string hostname_;
 
-  scoped_ptr<ProofVerifierCallback> callback_;
-  scoped_ptr<ProofVerifyDetailsChromium> verify_details_;
-  std::string error_details_;
+  CompletionCallback callback_;
+
+  // The result of certificate verification.
+  CertVerifyResult* cert_verify_result_;
+  std::string* error_details_;
 
   // X509Certificate from a chain of DER encoded certificates.
   scoped_refptr<X509Certificate> cert_;
diff --git a/net/quic/quic_crypto_client_stream.cc b/net/quic/quic_crypto_client_stream.cc
index 69c63a5..78b39e7 100644
--- a/net/quic/quic_crypto_client_stream.cc
+++ b/net/quic/quic_crypto_client_stream.cc
@@ -10,7 +10,6 @@
 #include "net/quic/crypto/crypto_utils.h"
 #include "net/quic/crypto/null_encrypter.h"
 #include "net/quic/crypto/proof_verifier.h"
-#include "net/quic/crypto/proof_verifier_chromium.h"
 #include "net/quic/quic_protocol.h"
 #include "net/quic/quic_session.h"
 #include "net/ssl/ssl_connection_status_flags.h"
@@ -18,63 +17,30 @@
 
 namespace net {
 
-QuicCryptoClientStream::ProofVerifierCallbackImpl::ProofVerifierCallbackImpl(
-    QuicCryptoClientStream* stream)
-    : stream_(stream) {}
-
-QuicCryptoClientStream::ProofVerifierCallbackImpl::
-    ~ProofVerifierCallbackImpl() {}
-
-void QuicCryptoClientStream::ProofVerifierCallbackImpl::Run(
-    bool ok,
-    const string& error_details,
-    scoped_ptr<ProofVerifyDetails>* details) {
-  if (stream_ == NULL) {
-    return;
-  }
-
-  stream_->verify_ok_ = ok;
-  stream_->verify_error_details_ = error_details;
-  stream_->verify_details_.reset(details->release());
-  stream_->proof_verify_callback_ = NULL;
-  stream_->DoHandshakeLoop(NULL);
-
-  // The ProofVerifier owns this object and will delete it when this method
-  // returns.
-}
-
-void QuicCryptoClientStream::ProofVerifierCallbackImpl::Cancel() {
-  stream_ = NULL;
-}
-
-
 QuicCryptoClientStream::QuicCryptoClientStream(
     const string& server_hostname,
     QuicSession* session,
     QuicCryptoClientConfig* crypto_config)
     : QuicCryptoStream(session),
+      weak_factory_(this),
       next_state_(STATE_IDLE),
       num_client_hellos_(0),
       crypto_config_(crypto_config),
       server_hostname_(server_hostname),
-      generation_counter_(0),
-      proof_verify_callback_(NULL) {
+      generation_counter_(0) {
 }
 
 QuicCryptoClientStream::~QuicCryptoClientStream() {
-  if (proof_verify_callback_) {
-    proof_verify_callback_->Cancel();
-  }
 }
 
 void QuicCryptoClientStream::OnHandshakeMessage(
     const CryptoHandshakeMessage& message) {
-  DoHandshakeLoop(&message);
+  DoHandshakeLoop(&message, OK);
 }
 
 bool QuicCryptoClientStream::CryptoConnect() {
   next_state_ = STATE_SEND_CHLO;
-  DoHandshakeLoop(NULL);
+  DoHandshakeLoop(NULL, OK);
   return true;
 }
 
@@ -91,8 +57,7 @@ bool QuicCryptoClientStream::GetSSLInfo(SSLInfo* ssl_info) {
     return false;
   }
   const CertVerifyResult* cert_verify_result =
-      &(reinterpret_cast<const ProofVerifyDetailsChromium*>(
-            cached->proof_verify_details()))->cert_verify_result;
+      cached->cert_verify_result();
 
   ssl_info->cert_status = cert_verify_result->cert_status;
   ssl_info->cert = cert_verify_result->verified_cert;
@@ -130,7 +95,8 @@ bool QuicCryptoClientStream::GetSSLInfo(SSLInfo* ssl_info) {
 static const int kMaxClientHellos = 3;
 
 void QuicCryptoClientStream::DoHandshakeLoop(
-    const CryptoHandshakeMessage* in) {
+    const CryptoHandshakeMessage* in,
+    int result) {
   CryptoHandshakeMessage out;
   QuicErrorCode error;
   string error_details;
@@ -146,6 +112,7 @@ void QuicCryptoClientStream::DoHandshakeLoop(
     next_state_ = STATE_IDLE;
     switch (state) {
       case STATE_SEND_CHLO: {
+        DCHECK_EQ(OK, result);
         // Send the client hello in plaintext.
         session()->connection()->SetDefaultEncryptionLevel(ENCRYPTION_NONE);
         if (num_client_hellos_ > kMaxClientHellos) {
@@ -204,6 +171,7 @@ void QuicCryptoClientStream::DoHandshakeLoop(
         return;
       }
       case STATE_RECV_REJ:
+        DCHECK_EQ(OK, result);
         // We sent a dummy CHLO because we didn't have enough information to
         // perform a handshake, or we sent a full hello that the server
         // rejected. Here we hope to have a REJ that contains the information
@@ -237,40 +205,25 @@ void QuicCryptoClientStream::DoHandshakeLoop(
         DCHECK(verifier);
         next_state_ = STATE_VERIFY_PROOF_COMPLETE;
         generation_counter_ = cached->generation_counter();
-
-        ProofVerifierCallbackImpl* proof_verify_callback =
-            new ProofVerifierCallbackImpl(this);
-
-        verify_ok_ = false;
-
-        ProofVerifier::Status status = verifier->VerifyProof(
+        result = verifier->VerifyProof(
             server_hostname_,
             cached->server_config(),
             cached->certs(),
             cached->signature(),
-            &error_details,
-            &verify_details_,
-            proof_verify_callback);
-
-        switch (status) {
-          case ProofVerifier::PENDING:
-            proof_verify_callback_ = proof_verify_callback;
-            DVLOG(1) << "Doing VerifyProof";
-            return;
-          case ProofVerifier::FAILURE:
-            CloseConnectionWithDetails(
-                QUIC_PROOF_INVALID, "Proof invalid: " + error_details);
-            return;
-          case ProofVerifier::SUCCESS:
-            verify_ok_ = true;
-            break;
+            &error_details_,
+            &cert_verify_result_,
+            base::Bind(&QuicCryptoClientStream::OnVerifyProofComplete,
+                       weak_factory_.GetWeakPtr()));
+        if (result == ERR_IO_PENDING) {
+          DVLOG(1) << "Doing VerifyProof";
+          return;
         }
         break;
       }
       case STATE_VERIFY_PROOF_COMPLETE:
-        if (!verify_ok_) {
+        if (result != OK) {
             CloseConnectionWithDetails(
-                QUIC_PROOF_INVALID, "Proof invalid: " + verify_error_details_);
+                QUIC_PROOF_INVALID, "Proof invalid: " + error_details_);
             return;
         }
         // Check if generation_counter has changed between STATE_VERIFY_PROOF
@@ -279,7 +232,7 @@ void QuicCryptoClientStream::DoHandshakeLoop(
           next_state_ = STATE_VERIFY_PROOF;
         } else {
           cached->SetProofValid();
-          cached->SetProofVerifyDetails(verify_details_.release());
+          cached->SetCertVerifyResult(cert_verify_result_);
           next_state_ = STATE_SEND_CHLO;
         }
         break;
@@ -352,4 +305,10 @@ void QuicCryptoClientStream::DoHandshakeLoop(
   }
 }
 
+void QuicCryptoClientStream::OnVerifyProofComplete(int result) {
+  DCHECK_EQ(STATE_VERIFY_PROOF_COMPLETE, next_state_);
+  DVLOG(1) << "VerifyProof completed: " << result;
+  DoHandshakeLoop(NULL, result);
+}
+
 }  // namespace net
diff --git a/net/quic/quic_crypto_client_stream.h b/net/quic/quic_crypto_client_stream.h
index 50cfbb8..4686fed 100644
--- a/net/quic/quic_crypto_client_stream.h
+++ b/net/quic/quic_crypto_client_stream.h
@@ -10,13 +10,11 @@
 #include "net/cert/cert_verify_result.h"
 #include "net/cert/x509_certificate.h"
 #include "net/quic/crypto/crypto_handshake.h"
-#include "net/quic/crypto/proof_verifier.h"
 #include "net/quic/quic_config.h"
 #include "net/quic/quic_crypto_stream.h"
 
 namespace net {
 
-class ProofVerifyDetails;
 class QuicSession;
 class SSLInfo;
 
@@ -49,29 +47,7 @@ class NET_EXPORT_PRIVATE QuicCryptoClientStream : public QuicCryptoStream {
   bool GetSSLInfo(SSLInfo* ssl_info);
 
  private:
-  // ProofVerifierCallbackImpl is passed as the callback method to VerifyProof.
-  // The ProofVerifier calls this class with the result of proof verification
-  // when verification is performed asynchronously.
-  class ProofVerifierCallbackImpl : public ProofVerifierCallback {
-   public:
-    explicit ProofVerifierCallbackImpl(QuicCryptoClientStream* stream);
-    virtual ~ProofVerifierCallbackImpl();
-
-    // ProofVerifierCallback interface.
-    virtual void Run(bool ok,
-                     const string& error_details,
-                     scoped_ptr<ProofVerifyDetails>* details) OVERRIDE;
-
-    // Cancel causes any future callbacks to be ignored. It must be called on
-    // the same thread as the callback will be made on.
-    void Cancel();
-
-   private:
-    QuicCryptoClientStream* stream_;
-  };
-
   friend class test::CryptoTestUtils;
-  friend class ProofVerifierCallbackImpl;
 
   enum State {
     STATE_IDLE,
@@ -83,8 +59,17 @@ class NET_EXPORT_PRIVATE QuicCryptoClientStream : public QuicCryptoStream {
   };
 
   // DoHandshakeLoop performs a step of the handshake state machine. Note that
-  // |in| may be NULL if the call did not result from a received message
-  void DoHandshakeLoop(const CryptoHandshakeMessage* in);
+  // |in| is NULL for the first call. OnVerifyProofComplete passes the |result|
+  // it has received from VerifyProof call (from all other places |result| is
+  // set to OK).
+  void DoHandshakeLoop(const CryptoHandshakeMessage* in, int result);
+
+  // OnVerifyProofComplete is passed as the callback method to VerifyProof.
+  // ProofVerifier calls this method with the result of proof verification when
+  // verification is performed asynchronously.
+  void OnVerifyProofComplete(int result);
+
+  base::WeakPtrFactory<QuicCryptoClientStream> weak_factory_;
 
   State next_state_;
   // num_client_hellos_ contains the number of client hello messages that this
@@ -101,15 +86,13 @@ class NET_EXPORT_PRIVATE QuicCryptoClientStream : public QuicCryptoStream {
   // Generation counter from QuicCryptoClientConfig's CachedState.
   uint64 generation_counter_;
 
-  // proof_verify_callback_ contains the callback object that we passed to an
-  // asynchronous proof verification. The ProofVerifier owns this object.
-  ProofVerifierCallbackImpl* proof_verify_callback_;
+  // The result of certificate verification.
+  // TODO(rtenneti): should we change CertVerifyResult to be
+  // RefCountedThreadSafe object to avoid copying.
+  CertVerifyResult cert_verify_result_;
 
-  // These members are used to store the result of an asynchronous proof
-  // verification.
-  bool verify_ok_;
-  string verify_error_details_;
-  scoped_ptr<ProofVerifyDetails> verify_details_;
+  // Error details for ProofVerifier's VerifyProof call.
+  std::string error_details_;
 
   DISALLOW_COPY_AND_ASSIGN(QuicCryptoClientStream);
 };
diff --git a/net/tools/quic/test_tools/quic_test_client.cc b/net/tools/quic/test_tools/quic_test_client.cc
index eb26078..ef9a164 100644
--- a/net/tools/quic/test_tools/quic_test_client.cc
+++ b/net/tools/quic/test_tools/quic_test_client.cc
@@ -24,19 +24,16 @@ namespace {
 class RecordingProofVerifier : public net::ProofVerifier {
  public:
   // ProofVerifier interface.
-  virtual net::ProofVerifier::Status VerifyProof(
-      const string& hostname,
-      const string& server_config,
-      const vector<string>& certs,
-      const string& signature,
-      string* error_details,
-      scoped_ptr<net::ProofVerifyDetails>* details,
-      net::ProofVerifierCallback* callback) OVERRIDE {
-    delete callback;
-
+  virtual int VerifyProof(const string& hostname,
+                          const string& server_config,
+                          const vector<string>& certs,
+                          const string& signature,
+                          string* error_details,
+                          net::CertVerifyResult* cert_verify_result,
+                          const net::CompletionCallback& callback) OVERRIDE {
     common_name_.clear();
     if (certs.empty()) {
-      return FAILURE;
+      return net::ERR_FAILED;
     }
 
     // Convert certs to X509Certificate.
@@ -47,11 +44,11 @@ class RecordingProofVerifier : public net::ProofVerifier {
     scoped_refptr<net::X509Certificate> cert =
         net::X509Certificate::CreateFromDERCertChain(cert_pieces);
     if (!cert.get()) {
-      return FAILURE;
+      return net::ERR_FAILED;
     }
 
     common_name_ = cert->subject().GetDisplayName();
-    return SUCCESS;
+    return net::OK;
   }
 
   const string& common_name() const { return common_name_; }