Various cleanups in crypto code discovered while syncing recent changes.

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

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

6 files changed, 92 insertions, 40 deletions

diff --git a/net/quic/crypto/proof_verifier.h b/net/quic/crypto/proof_verifier.h
index 0cbdb6a..779d63f 100644
--- a/net/quic/crypto/proof_verifier.h
+++ b/net/quic/crypto/proof_verifier.h
@@ -8,7 +8,7 @@
 #include <string>
 #include <vector>
 
-#include "net/base/completion_callback.h"
+#include "base/memory/scoped_ptr.h"
 #include "net/base/net_export.h"
 
 namespace net {
diff --git a/net/quic/crypto/proof_verifier_chromium.cc b/net/quic/crypto/proof_verifier_chromium.cc
index fdb6a0d..cbb4436 100644
--- a/net/quic/crypto/proof_verifier_chromium.cc
+++ b/net/quic/crypto/proof_verifier_chromium.cc
@@ -110,7 +110,6 @@ ProofVerifierChromium::Status ProofVerifierChromium::Job::VerifyProof(
   DCHECK(verify_details);
   DCHECK(callback);
 
-  callback_.reset(callback);
   error_details->clear();
 
   if (STATE_NONE != next_state_) {
@@ -161,6 +160,7 @@ ProofVerifierChromium::Status ProofVerifierChromium::Job::VerifyProof(
       verify_details->reset(verify_details_.release());
       return SUCCESS;
     case ERR_IO_PENDING:
+      callback_.reset(callback);
       return PENDING;
     default:
       *error_details = error_details_;
diff --git a/net/quic/crypto/quic_crypto_client_config.cc b/net/quic/crypto/quic_crypto_client_config.cc
index e58e4d1..a6b0e39 100644
--- a/net/quic/crypto/quic_crypto_client_config.cc
+++ b/net/quic/crypto/quic_crypto_client_config.cc
@@ -711,7 +711,7 @@ void QuicCryptoClientConfig::PopulateFromCanonicalConfig(
     const QuicServerId& server_id,
     CachedState* server_state) {
   DCHECK(server_state->IsEmpty());
-  unsigned i = 0;
+  size_t i = 0;
   for (; i < canoncial_suffixes_.size(); ++i) {
     if (EndsWith(server_id.host(), canoncial_suffixes_[i], false)) {
       break;
diff --git a/net/quic/crypto/quic_crypto_client_config_test.cc b/net/quic/crypto/quic_crypto_client_config_test.cc
index a085d16..cb2e48f 100644
--- a/net/quic/crypto/quic_crypto_client_config_test.cc
+++ b/net/quic/crypto/quic_crypto_client_config_test.cc
@@ -66,35 +66,13 @@ TEST(QuicCryptoClientConfigTest, InchoateChlo) {
   EXPECT_EQ(QuicVersionToQuicTag(QuicVersionMax()), cver);
 }
 
-TEST(QuicCryptoClientConfigTest, FillClientHello) {
-  QuicCryptoClientConfig::CachedState state;
+TEST(QuicCryptoClientConfigTest, PreferAesGcm) {
   QuicCryptoClientConfig config;
-  QuicCryptoNegotiatedParameters params;
-  QuicConnectionId kConnectionId = 1234;
-  uint32 kInitialFlowControlWindow = 5678;
-  string error_details;
-  MockRandom rand;
-  CryptoHandshakeMessage chlo;
-  QuicServerId server_id("www.google.com", 80, false, PRIVACY_MODE_DISABLED);
-  config.FillClientHello(server_id,
-                         kConnectionId,
-                         QuicVersionMax(),
-                         kInitialFlowControlWindow,
-                         &state,
-                         QuicWallTime::Zero(),
-                         &rand,
-                         &params,
-                         &chlo,
-                         &error_details);
-
-  // Verify that certain QuicTags have been set correctly in the CHLO.
-  QuicTag cver;
-  EXPECT_EQ(QUIC_NO_ERROR, chlo.GetUint32(kVER, &cver));
-  EXPECT_EQ(QuicVersionToQuicTag(QuicVersionMax()), cver);
-
-  QuicTag ifcw;
-  EXPECT_EQ(QUIC_NO_ERROR, chlo.GetUint32(kIFCW, &ifcw));
-  EXPECT_EQ(kInitialFlowControlWindow, ifcw);
+  config.SetDefaults();
+  if (config.aead.size() > 1)
+    EXPECT_NE(kAESG, config.aead[0]);
+  config.PreferAesGcm();
+  EXPECT_EQ(kAESG, config.aead[0]);
 }
 
 TEST(QuicCryptoClientConfigTest, InchoateChloSecure) {
@@ -126,6 +104,37 @@ TEST(QuicCryptoClientConfigTest, InchoateChloSecureNoEcdsa) {
   EXPECT_EQ(kX59R, pdmd);
 }
 
+TEST(QuicCryptoClientConfigTest, FillClientHello) {
+  QuicCryptoClientConfig::CachedState state;
+  QuicCryptoClientConfig config;
+  QuicCryptoNegotiatedParameters params;
+  QuicConnectionId kConnectionId = 1234;
+  uint32 kInitialFlowControlWindow = 5678;
+  string error_details;
+  MockRandom rand;
+  CryptoHandshakeMessage chlo;
+  QuicServerId server_id("www.google.com", 80, false, PRIVACY_MODE_DISABLED);
+  config.FillClientHello(server_id,
+                         kConnectionId,
+                         QuicVersionMax(),
+                         kInitialFlowControlWindow,
+                         &state,
+                         QuicWallTime::Zero(),
+                         &rand,
+                         &params,
+                         &chlo,
+                         &error_details);
+
+  // Verify that certain QuicTags have been set correctly in the CHLO.
+  QuicTag cver;
+  EXPECT_EQ(QUIC_NO_ERROR, chlo.GetUint32(kVER, &cver));
+  EXPECT_EQ(QuicVersionToQuicTag(QuicVersionMax()), cver);
+
+  QuicTag ifcw;
+  EXPECT_EQ(QUIC_NO_ERROR, chlo.GetUint32(kIFCW, &ifcw));
+  EXPECT_EQ(kInitialFlowControlWindow, ifcw);
+}
+
 TEST(QuicCryptoClientConfigTest, ProcessServerDowngradeAttack) {
   QuicVersionVector supported_versions = QuicSupportedVersions();
   if (supported_versions.size() == 1) {
@@ -173,5 +182,49 @@ TEST(QuicCryptoClientConfigTest, InitializeFrom) {
   EXPECT_EQ(1u, other->generation_counter());
 }
 
+TEST(QuicCryptoClientConfigTest, Canonical) {
+  QuicCryptoClientConfig config;
+  config.AddCanonicalSuffix(".google.com");
+  QuicServerId canonical_id1("www.google.com", 80, false,
+                              PRIVACY_MODE_DISABLED);
+  QuicServerId canonical_id2("mail.google.com", 80, false,
+                              PRIVACY_MODE_DISABLED);
+  QuicCryptoClientConfig::CachedState* state =
+      config.LookupOrCreate(canonical_id1);
+  // TODO(rch): Populate other fields of |state|.
+  state->set_source_address_token("TOKEN");
+  state->SetProofValid();
+
+  QuicCryptoClientConfig::CachedState* other =
+      config.LookupOrCreate(canonical_id2);
+
+  EXPECT_TRUE(state->IsEmpty());
+  EXPECT_EQ(state->server_config(), other->server_config());
+  EXPECT_EQ(state->source_address_token(), other->source_address_token());
+  EXPECT_EQ(state->certs(), other->certs());
+  EXPECT_EQ(1u, other->generation_counter());
+
+  QuicServerId different_id("mail.google.org", 80, false,
+                             PRIVACY_MODE_DISABLED);
+  EXPECT_TRUE(config.LookupOrCreate(different_id)->IsEmpty());
+}
+
+TEST(QuicCryptoClientConfigTest, CanonicalNotUsedIfNotValid) {
+  QuicCryptoClientConfig config;
+  config.AddCanonicalSuffix(".google.com");
+  QuicServerId canonical_id1("www.google.com", 80, false,
+                              PRIVACY_MODE_DISABLED);
+  QuicServerId canonical_id2("mail.google.com", 80, false,
+                              PRIVACY_MODE_DISABLED);
+  QuicCryptoClientConfig::CachedState* state =
+      config.LookupOrCreate(canonical_id1);
+  // TODO(rch): Populate other fields of |state|.
+  state->set_source_address_token("TOKEN");
+
+  // Do not set the proof as valid, and check that it is not used
+  // as a canonical entry.
+  EXPECT_TRUE(config.LookupOrCreate(canonical_id2)->IsEmpty());
+}
+
 }  // namespace test
 }  // namespace net
diff --git a/net/quic/quic_crypto_client_stream.cc b/net/quic/quic_crypto_client_stream.cc
index 0ae7247..856696d 100644
--- a/net/quic/quic_crypto_client_stream.cc
+++ b/net/quic/quic_crypto_client_stream.cc
@@ -10,6 +10,7 @@
 #include "net/quic/crypto/proof_verifier.h"
 #include "net/quic/quic_client_session_base.h"
 #include "net/quic/quic_protocol.h"
+#include "net/quic/quic_session.h"
 
 namespace net {
 
@@ -127,9 +128,7 @@ void QuicCryptoClientStream::DoHandshakeLoop(
           crypto_config_->FillInchoateClientHello(
               server_id_,
               session()->connection()->supported_versions().front(),
-              cached,
-              &crypto_negotiated_params_,
-              &out);
+              cached, &crypto_negotiated_params_, &out);
           // Pad the inchoate client hello to fill up a packet.
           const size_t kFramingOverhead = 50;  // A rough estimate.
           const size_t max_packet_size =
@@ -253,8 +252,10 @@ void QuicCryptoClientStream::DoHandshakeLoop(
             DVLOG(1) << "Doing VerifyProof";
             return;
           case ProofVerifier::FAILURE:
+            delete proof_verify_callback;
             break;
           case ProofVerifier::SUCCESS:
+            delete proof_verify_callback;
             verify_ok_ = true;
             break;
         }
diff --git a/net/quic/quic_crypto_server_stream_test.cc b/net/quic/quic_crypto_server_stream_test.cc
index bbc0525..3d1127b 100644
--- a/net/quic/quic_crypto_server_stream_test.cc
+++ b/net/quic/quic_crypto_server_stream_test.cc
@@ -156,8 +156,7 @@ TEST_P(QuicCryptoServerStreamTest, ZeroRTT) {
   CHECK(client->CryptoConnect());
   CHECK_EQ(1u, client_conn->packets_.size());
 
-  scoped_ptr<TestClientSession> server_session(
-      new TestClientSession(server_conn, config_));
+  scoped_ptr<TestSession> server_session(new TestSession(server_conn, config_));
   scoped_ptr<QuicCryptoServerStream> server(
       new QuicCryptoServerStream(crypto_config_, server_session.get()));
   server_session->SetCryptoStream(server.get());
@@ -180,9 +179,9 @@ TEST_P(QuicCryptoServerStreamTest, ZeroRTT) {
   // strike-register from rejecting the repeated nonce.
   reinterpret_cast<MockRandom*>(client_conn->random_generator())->ChangeValue();
   client_session.reset(new TestClientSession(client_conn, client_config));
-  server_session.reset(new TestClientSession(server_conn, config_));
-  client.reset(new QuicCryptoClientStream(
-      server_id, client_session.get(), NULL, &client_crypto_config));
+  server_session.reset(new TestSession(server_conn, config_));
+  client.reset(new QuicCryptoClientStream(server_id, client_session.get(),
+                                          NULL, &client_crypto_config));
   client_session->SetCryptoStream(client.get());
 
   server.reset(new QuicCryptoServerStream(crypto_config_,
@@ -255,7 +254,6 @@ TEST_P(QuicCryptoServerStreamTest, WithoutCertificates) {
 
 TEST_P(QuicCryptoServerStreamTest, ChannelID) {
   client_options_.channel_id_enabled = true;
-  // TODO(rtenneti): Enable testing of ProofVerifier.
   // CompleteCryptoHandshake verifies
   // stream_.crypto_negotiated_params().channel_id is correct.
   EXPECT_EQ(2, CompleteCryptoHandshake());