OpenSSL/NSS implementation of ProofVerfifier.

net/quic/quic_crypto_client_stream.cc

Index: net/quic/quic_crypto_client_stream.cc
diff --git a/net/quic/quic_crypto_client_stream.cc b/net/quic/quic_crypto_client_stream.cc
index 674a441e7971a0b62e9c6ee49486139a489cfcbf..e9647f1ee7879406e88509043b840565d1145309 100644
--- a/net/quic/quic_crypto_client_stream.cc
+++ b/net/quic/quic_crypto_client_stream.cc
@@ -4,6 +4,8 @@
 
 #include "net/quic/quic_crypto_client_stream.h"
 
+#include "net/base/completion_callback.h"
+#include "net/base/net_errors.h"
 #include "net/quic/crypto/crypto_protocol.h"
 #include "net/quic/crypto/crypto_utils.h"
 #include "net/quic/crypto/null_encrypter.h"
@@ -139,23 +141,32 @@ void QuicCryptoClientStream::DoHandshakeLoop(
           return;
         }
         if (!cached->proof_valid()) {
-          const ProofVerifier* verifier = crypto_config_->proof_verifier();
+          ProofVerifier* verifier = crypto_config_->proof_verifier();
           if (!verifier) {
             // If no verifier is set then we don't check the certificates.
             cached->SetProofValid();
           } else if (!cached->signature().empty()) {
             // TODO(rtenneti): In Chromium, we will need to make VerifyProof()
             // asynchronous.

[wtc, 2013/06/24 22:36:56]

Delete this TODO comment because it's done.

[ramant (doing other things), 2013/06/28 19:16:56]

Done.

-            if (!verifier->VerifyProof(server_hostname_,
-                                       cached->server_config(),
-                                       cached->certs(),
-                                       cached->signature(),
-                                       &error_details)) {
-              CloseConnectionWithDetails(QUIC_PROOF_INVALID,
-                                         "Proof invalid: " + error_details);
-              return;
+            int rv = verifier->VerifyProof(
+                server_hostname_,
+                cached->server_config(),
+                cached->certs(),
+                cached->signature(),
+                base::Bind(&QuicCryptoClientStream::VerifyProofCompleted,
+                           base::Unretained(this)),
+                &error_details);

[wtc, 2013/06/24 22:36:56]

Ideally we should set next_state_ to a state that handles
the completion of verifier->VerifyProof(). That is the
convention of the state machines in net.

[ramant (doing other things), 2013/06/28 19:16:56]

Done.

+            if (rv == ERR_IO_PENDING) {
+              DVLOG(1) << "Doing VerifyProof";
+              break;
+            } else {

[wtc, 2013/06/24 22:36:56]

Nit: omit the "else" after a break statement.

[ramant (doing other things), 2013/06/28 19:16:56]

Done.

+              if (rv != OK) {
+                CloseConnectionWithDetails(QUIC_PROOF_INVALID,
+                                           "Proof invalid: " + error_details);
+                return;
+              }
+              cached->SetProofValid();
             }
-            cached->SetProofValid();
           }
         }
         // Send the subsequent client hello in plaintext.
@@ -163,6 +174,12 @@ void QuicCryptoClientStream::DoHandshakeLoop(
             ENCRYPTION_NONE);
         next_state_ = STATE_SEND_CHLO;

[wtc, 2013/06/24 22:36:56]

It is possible to use additional states to avoid duplicating this code
(lines 172-175) in two states (this state and the
STATE_PROOF_VERIFICATION_COMPLETED
below). The tricky part is how to deal with the cached->SetProofValid() call.

[ramant (doing other things), 2013/06/28 19:16:56]

Done.

         break;
+      case STATE_PROOF_VERIFICATION_COMPLETED:
+        cached->SetProofValid();
+        // Send the subsequent client hello in plaintext.
+        session()->connection()->SetDefaultEncryptionLevel(ENCRYPTION_NONE);
+        next_state_ = STATE_SEND_CHLO;
+        break;
       case STATE_RECV_SHLO: {
         // We sent a CHLO that we expected to be accepted and now we're hoping
         // for a SHLO from the server to confirm that.
@@ -232,4 +249,13 @@ void QuicCryptoClientStream::DoHandshakeLoop(
   }
 }
 
+void QuicCryptoClientStream::VerifyProofCompleted(int result) {
+  if (result != OK) {
+    CloseConnectionWithDetails(QUIC_PROOF_INVALID, "Proof invalid:");

[wtc, 2013/06/24 22:36:56]

The error_details string is incomplete: "Proof invalid:". It is missing
the actual error details.

I think this code (lines 253-256) should be moved to the code that handles
the STATE_PROOF_VERIFICATION_COMPLETED. This requires passing |result| to
DoHandshakeLoop(). Is it easy to do that?

[ramant (doing other things), 2013/06/28 19:16:56]

Done.

+    return;
+  }
+  next_state_ = STATE_PROOF_VERIFICATION_COMPLETED;
+  DoHandshakeLoop(NULL);
+}
+
 }  // namespace net