Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(679)

Unified Diff: net/quic/core/quic_crypto_server_stream_test.cc

Issue 2797523002: Prevent multiple simultaneous calls to GetProof (Closed)
Patch Set: Change to use MockClock Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/quic/core/quic_crypto_server_stream.cc ('k') | net/quic/core/quic_flags_list.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/quic/core/quic_crypto_server_stream_test.cc
diff --git a/net/quic/core/quic_crypto_server_stream_test.cc b/net/quic/core/quic_crypto_server_stream_test.cc
index d7330b0806cdb21c17140cd6e038805d73eb3b54..563612a5828bee56ccc6ad57615c23da279ba8ef 100644
--- a/net/quic/core/quic_crypto_server_stream_test.cc
+++ b/net/quic/core/quic_crypto_server_stream_test.cc
@@ -26,6 +26,7 @@
#include "net/quic/platform/api/quic_socket_address.h"
#include "net/quic/test_tools/crypto_test_utils.h"
#include "net/quic/test_tools/failing_proof_source.h"
+#include "net/quic/test_tools/fake_proof_source.h"
#include "net/quic/test_tools/quic_crypto_server_config_peer.h"
#include "net/quic/test_tools/quic_test_utils.h"
#include "testing/gmock/include/gmock/gmock.h"
@@ -505,6 +506,58 @@ TEST_P(QuicCryptoServerStreamTestWithFailingProofSource, Test) {
EXPECT_FALSE(server_stream()->handshake_confirmed());
}
+class QuicCryptoServerStreamTestWithFakeProofSource
+ : public QuicCryptoServerStreamTest {
+ public:
+ QuicCryptoServerStreamTestWithFakeProofSource()
+ : QuicCryptoServerStreamTest(
+ std::unique_ptr<FakeProofSource>(new FakeProofSource)),
+ crypto_config_peer_(&server_crypto_config_) {}
+
+ FakeProofSource* GetFakeProofSource() const {
+ return static_cast<FakeProofSource*>(crypto_config_peer_.GetProofSource());
+ }
+
+ protected:
+ QuicCryptoServerConfigPeer crypto_config_peer_;
+};
+
+INSTANTIATE_TEST_CASE_P(YetMoreTests,
+ QuicCryptoServerStreamTestWithFakeProofSource,
+ testing::Bool());
+
+// Regression test for b/35422225, in which multiple CHLOs arriving on the same
+// connection in close succession could cause a crash, especially when the use
+// of Mentat signing meant that it took a while for each CHLO to be processed.
+TEST_P(QuicCryptoServerStreamTestWithFakeProofSource, MultipleChlo) {
+ Initialize();
+ GetFakeProofSource()->Activate();
+ base::SetFlag(&FLAGS_quic_reloadable_flag_fix_quic_callback_crash, true);
+ EXPECT_CALL(*server_session_->helper(), CanAcceptClientHello(_, _, _))
+ .WillOnce(testing::Return(true));
+
+ // Create a minimal CHLO
+ MockClock clock;
+ QuicVersion version = AllSupportedVersions().front();
+ CryptoHandshakeMessage chlo = crypto_test_utils::GenerateDefaultInchoateCHLO(
+ &clock, version, &server_crypto_config_);
+
+ // Send in the CHLO, and check that a callback is now pending in the
+ // ProofSource.
+ server_stream()->OnHandshakeMessage(chlo);
+ EXPECT_EQ(GetFakeProofSource()->NumPendingCallbacks(), 1);
+
+ // Send in a second CHLO while processing of the first is still pending.
+ // Verify that the server closes the connection rather than crashing. Note
+ // that the crash is a use-after-free, so it may only show up consistently in
+ // ASAN tests.
+ EXPECT_CALL(
+ *server_connection_,
+ CloseConnection(QUIC_CRYPTO_MESSAGE_WHILE_VALIDATING_CLIENT_HELLO,
+ "Unexpected handshake message while processing CHLO", _));
+ server_stream()->OnHandshakeMessage(chlo);
+}
+
} // namespace
} // namespace test
} // namespace net
« no previous file with comments | « net/quic/core/quic_crypto_server_stream.cc ('k') | net/quic/core/quic_flags_list.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698