diff options
author | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-02 01:17:35 +0000 |
---|---|---|
committer | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-02 01:17:35 +0000 |
commit | ef65c323668aed3b9fb880441084dd2807fa5d67 (patch) | |
tree | 832a662877821a8c6df7bb77ea4a519a495eb421 /net | |
parent | c4b6c85962e5c1dca60c2fc08c75abd52dbd6f01 (diff) | |
download | chromium_src-ef65c323668aed3b9fb880441084dd2807fa5d67.zip chromium_src-ef65c323668aed3b9fb880441084dd2807fa5d67.tar.gz chromium_src-ef65c323668aed3b9fb880441084dd2807fa5d67.tar.bz2 |
Revert of Various cleanups in crypto code discovered while syncing recent changes. (https://codereview.chromium.org/217653003/)
Reason for revert:
Seems to be related to a tree-closing failure.
http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%2BLSan%20Tests%20%281%29/builds/984
See bug for more details.
BUG=358919
Original issue's description:
> Various cleanups in crypto code discovered while syncing recent changes.
>
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260979
TBR=rtenneti@chromium.org,rch@chromium.org
NOTREECHECKS=true
NOTRY=true
Review URL: https://codereview.chromium.org/212223009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@261017 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/quic/crypto/proof_verifier.h | 2 | ||||
-rw-r--r-- | net/quic/crypto/proof_verifier_chromium.cc | 2 | ||||
-rw-r--r-- | net/quic/crypto/quic_crypto_client_config.cc | 2 | ||||
-rw-r--r-- | net/quic/crypto/quic_crypto_client_config_test.cc | 109 | ||||
-rw-r--r-- | net/quic/quic_crypto_client_stream.cc | 7 | ||||
-rw-r--r-- | net/quic/quic_crypto_server_stream_test.cc | 10 |
6 files changed, 40 insertions, 92 deletions
diff --git a/net/quic/crypto/proof_verifier.h b/net/quic/crypto/proof_verifier.h index 779d63f..0cbdb6a 100644 --- a/net/quic/crypto/proof_verifier.h +++ b/net/quic/crypto/proof_verifier.h @@ -8,7 +8,7 @@ #include <string> #include <vector> -#include "base/memory/scoped_ptr.h" +#include "net/base/completion_callback.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 cbb4436..fdb6a0d 100644 --- a/net/quic/crypto/proof_verifier_chromium.cc +++ b/net/quic/crypto/proof_verifier_chromium.cc @@ -110,6 +110,7 @@ ProofVerifierChromium::Status ProofVerifierChromium::Job::VerifyProof( DCHECK(verify_details); DCHECK(callback); + callback_.reset(callback); error_details->clear(); if (STATE_NONE != next_state_) { @@ -160,7 +161,6 @@ 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 a6b0e39..e58e4d1 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()); - size_t i = 0; + unsigned 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 cb2e48f..a085d16 100644 --- a/net/quic/crypto/quic_crypto_client_config_test.cc +++ b/net/quic/crypto/quic_crypto_client_config_test.cc @@ -66,13 +66,35 @@ TEST(QuicCryptoClientConfigTest, InchoateChlo) { EXPECT_EQ(QuicVersionToQuicTag(QuicVersionMax()), cver); } -TEST(QuicCryptoClientConfigTest, PreferAesGcm) { +TEST(QuicCryptoClientConfigTest, FillClientHello) { + QuicCryptoClientConfig::CachedState state; QuicCryptoClientConfig config; - config.SetDefaults(); - if (config.aead.size() > 1) - EXPECT_NE(kAESG, config.aead[0]); - config.PreferAesGcm(); - EXPECT_EQ(kAESG, config.aead[0]); + 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, + ¶ms, + &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, InchoateChloSecure) { @@ -104,37 +126,6 @@ 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, - ¶ms, - &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) { @@ -182,49 +173,5 @@ 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 856696d..0ae7247 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/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 { @@ -128,7 +127,9 @@ 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 = @@ -252,10 +253,8 @@ 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 3d1127b..bbc0525 100644 --- a/net/quic/quic_crypto_server_stream_test.cc +++ b/net/quic/quic_crypto_server_stream_test.cc @@ -156,7 +156,8 @@ TEST_P(QuicCryptoServerStreamTest, ZeroRTT) { CHECK(client->CryptoConnect()); CHECK_EQ(1u, client_conn->packets_.size()); - scoped_ptr<TestSession> server_session(new TestSession(server_conn, config_)); + scoped_ptr<TestClientSession> server_session( + new TestClientSession(server_conn, config_)); scoped_ptr<QuicCryptoServerStream> server( new QuicCryptoServerStream(crypto_config_, server_session.get())); server_session->SetCryptoStream(server.get()); @@ -179,9 +180,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 TestSession(server_conn, config_)); - client.reset(new QuicCryptoClientStream(server_id, client_session.get(), - NULL, &client_crypto_config)); + server_session.reset(new TestClientSession(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_, @@ -254,6 +255,7 @@ 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()); |