From ffc021d3db251e63e5ab2316718b92e20326497d Mon Sep 17 00:00:00 2001 From: danzh Date: Wed, 10 Jan 2024 00:09:47 -0500 Subject: [PATCH] quic: fix socket UAF during port migration (#31702) Commit Message: the probing socket is released when port migration fails. If this happens in response to an incoming packet during an I/O event, the follow socket read could cause use-after-free. [2024-01-08 16:30:53.386][12][critical][backtrace] [./source/server/backtrace.h:104] Caught Segmentation fault, suspect faulting address 0x0 [2024-01-08 16:30:53.387][12][critical][backtrace] [./source/server/backtrace.h:91] Backtrace (use tools/stack_decode.py to get line numbers): [2024-01-08 16:30:53.387][12][critical][backtrace] [./source/server/backtrace.h:92] Envoy version: 0/1.29.0-dev/test/DEBUG/BoringSSL [2024-01-08 16:30:53.413][12][critical][backtrace] [./source/server/backtrace.h:96] #0: Envoy::SignalAction::sigHandler() [0x55bb876d499e] [2024-01-08 16:30:53.413][12][critical][backtrace] [./source/server/backtrace.h:98] #1: [0x7f55fbf92510] [2024-01-08 16:30:53.440][12][critical][backtrace] [./source/server/backtrace.h:96] #2: Envoy::Network::Utility::readPacketsFromSocket() [0x55bb875de0ef] [2024-01-08 16:30:53.466][12][critical][backtrace] [./source/server/backtrace.h:96] #3: Envoy::Quic::EnvoyQuicClientConnection::onFileEvent() [0x55bb8663e1eb] [2024-01-08 16:30:53.492][12][critical][backtrace] [./source/server/backtrace.h:96] #4: Envoy::Quic::EnvoyQuicClientConnection::setUpConnectionSocket()::$_0::operator()() [0x55bb8663f192] [2024-01-08 16:30:53.518][12][critical][backtrace] [./source/server/backtrace.h:96] #5: std::__invoke_impl<>() [0x55bb8663f151] [2024-01-08 16:30:53.544][12][critical][backtrace] [./source/server/backtrace.h:96] #6: std::__invoke_r<>() [0x55bb8663f0e2] [2024-01-08 16:30:53.569][12][critical][backtrace] [./source/server/backtrace.h:96] #7: std::_Function_handler<>::_M_invoke() [0x55bb8663efc2] [2024-01-08 16:30:53.595][12][critical][backtrace] [./source/server/backtrace.h:96] #8: std::function<>::operator()() [0x55bb85cb8f44] [2024-01-08 16:30:53.621][12][critical][backtrace] [./source/server/backtrace.h:96] #9: Envoy::Event::DispatcherImpl::createFileEvent()::$_5::operator()() [0x55bb8722560f] [2024-01-08 16:30:53.648][12][critical][backtrace] [./source/server/backtrace.h:96] #10: std::__invoke_impl<>() [0x55bb872255c1] [2024-01-08 16:30:53.674][12][critical][backtrace] [./source/server/backtrace.h:96] #11: std::__invoke_r<>() [0x55bb87225562] [2024-01-08 16:30:53.700][12][critical][backtrace] [./source/server/backtrace.h:96] #12: std::_Function_handler<>::_M_invoke() [0x55bb872253e2] [2024-01-08 16:30:53.700][12][critical][backtrace] [./source/server/backtrace.h:96] #13: std::function<>::operator()() [0x55bb85cb8f44] [2024-01-08 16:30:53.726][12][critical][backtrace] [./source/server/backtrace.h:96] #14: Envoy::Event::FileEventImpl::mergeInjectedEventsAndRunCb() [0x55bb872358ec] [2024-01-08 16:30:53.752][12][critical][backtrace] [./source/server/backtrace.h:96] #15: Envoy::Event::FileEventImpl::assignEvents()::$_1::operator()() [0x55bb87235ed1] [2024-01-08 16:30:53.778][12][critical][backtrace] [./source/server/backtrace.h:96] #16: Envoy::Event::FileEventImpl::assignEvents()::$_1::__invoke() [0x55bb87235949] [2024-01-08 16:30:53.804][12][critical][backtrace] [./source/server/backtrace.h:96] #17: event_persist_closure [0x55bb87fab72b] [2024-01-08 16:30:53.830][12][critical][backtrace] [./source/server/backtrace.h:96] #18: event_process_active_single_queue [0x55bb87faada2] [2024-01-08 16:30:53.856][12][critical][backtrace] [./source/server/backtrace.h:96] #19: event_process_active [0x55bb87fa56c8] [2024-01-08 16:30:53.882][12][critical][backtrace] [./source/server/backtrace.h:96] #20: event_base_loop [0x55bb87fa45cc] [2024-01-08 16:30:53.908][12][critical][backtrace] [./source/server/backtrace.h:96] #21: Envoy::Event::LibeventScheduler::run() [0x55bb8760a59f] Risk Level: low Testing: new unit test Docs Changes: N/A Release Notes: Yes Platform Specific Features: N/A Signed-off-by: Dan Zhang Co-authored-by: Dan Zhang --- changelogs/current.yaml | 3 + .../quic/envoy_quic_client_connection.cc | 16 ++++ .../quic/envoy_quic_client_connection.h | 46 +++++------ .../quic/envoy_quic_client_session_test.cc | 76 ++++++++++++++++--- test/common/quic/test_utils.h | 1 + 5 files changed, 110 insertions(+), 32 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index f2a6f89ae884..25f23c5dda3c 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -113,6 +113,9 @@ bug_fixes: change: | Fixed a bug in QUIC and HCM interaction which could cause ``use-after-free`` during asynchronous certificates retrieval. The fix is guarded by runtime ``envoy.reloadable_features.quic_fix_filter_manager_uaf``. +- area: quic + change: | + Fixed a bug in QUIC upstream port migration which could cause use-after-free upon STATELESS_RESET packets. - area: redis change: | Fixed a bug causing crash if incoming redis key does not match against a ``prefix_route`` and ``catch_all_route`` is not defined. diff --git a/source/common/quic/envoy_quic_client_connection.cc b/source/common/quic/envoy_quic_client_connection.cc index 09a8745572bd..0616b93808a4 100644 --- a/source/common/quic/envoy_quic_client_connection.cc +++ b/source/common/quic/envoy_quic_client_connection.cc @@ -11,6 +11,18 @@ namespace Envoy { namespace Quic { +// Used to defer deleting connection socket to avoid deleting IoHandle in a read loop. +class DeferredDeletableSocket : public Event::DeferredDeletable { +public: + explicit DeferredDeletableSocket(std::unique_ptr socket) + : socket_(std::move(socket)) {} + + void deleteIsPending() override { socket_->close(); } + +private: + std::unique_ptr socket_; +}; + EnvoyQuicClientConnection::EnvoyQuicClientConnection( const quic::QuicConnectionId& server_connection_id, Network::Address::InstanceConstSharedPtr& initial_peer_address, @@ -188,6 +200,10 @@ void EnvoyQuicClientConnection::onPathValidationFailure( // Note that the probing socket and probing writer will be deleted once context goes out of // scope. OnPathValidationFailureAtClient(/*is_multi_port=*/false, *context); + std::unique_ptr probing_socket = + static_cast(context.get())->releaseSocket(); + // Extend the socket life time till the end of the current event loop. + dispatcher_.deferredDelete(std::make_unique(std::move(probing_socket))); } void EnvoyQuicClientConnection::onFileEvent(uint32_t events, diff --git a/source/common/quic/envoy_quic_client_connection.h b/source/common/quic/envoy_quic_client_connection.h index 2ddd3e523e76..d8b917e0f4dd 100644 --- a/source/common/quic/envoy_quic_client_connection.h +++ b/source/common/quic/envoy_quic_client_connection.h @@ -27,6 +27,29 @@ class EnvoyQuicClientConnection : public quic::QuicConnection, public QuicNetworkConnection, public Network::UdpPacketProcessor { public: + // Holds all components needed for a QUIC connection probing/migration. + class EnvoyQuicPathValidationContext : public quic::QuicPathValidationContext { + public: + EnvoyQuicPathValidationContext(const quic::QuicSocketAddress& self_address, + const quic::QuicSocketAddress& peer_address, + std::unique_ptr writer, + std::unique_ptr probing_socket); + + ~EnvoyQuicPathValidationContext() override; + + quic::QuicPacketWriter* WriterToUse() override; + + EnvoyQuicPacketWriter* releaseWriter(); + + Network::ConnectionSocket& probingSocket(); + + std::unique_ptr releaseSocket(); + + private: + std::unique_ptr writer_; + Network::ConnectionSocketPtr socket_; + }; + // A connection socket will be created with given |local_addr|. If binding // port not provided in |local_addr|, pick up a random port. EnvoyQuicClientConnection(const quic::QuicConnectionId& server_connection_id, @@ -91,29 +114,6 @@ class EnvoyQuicClientConnection : public quic::QuicConnection, probeAndMigrateToServerPreferredAddress(const quic::QuicSocketAddress& server_preferred_address); private: - // Holds all components needed for a QUIC connection probing/migration. - class EnvoyQuicPathValidationContext : public quic::QuicPathValidationContext { - public: - EnvoyQuicPathValidationContext(const quic::QuicSocketAddress& self_address, - const quic::QuicSocketAddress& peer_address, - std::unique_ptr writer, - std::unique_ptr probing_socket); - - ~EnvoyQuicPathValidationContext() override; - - quic::QuicPacketWriter* WriterToUse() override; - - EnvoyQuicPacketWriter* releaseWriter(); - - Network::ConnectionSocket& probingSocket(); - - std::unique_ptr releaseSocket(); - - private: - std::unique_ptr writer_; - Network::ConnectionSocketPtr socket_; - }; - // Receives notifications from the Quiche layer on path validation results. class EnvoyPathValidationResultDelegate : public quic::QuicPathValidator::ResultDelegate { public: diff --git a/test/common/quic/envoy_quic_client_session_test.cc b/test/common/quic/envoy_quic_client_session_test.cc index 500423532dfd..153316969de0 100644 --- a/test/common/quic/envoy_quic_client_session_test.cc +++ b/test/common/quic/envoy_quic_client_session_test.cc @@ -47,6 +47,8 @@ class TestEnvoyQuicClientConnection : public EnvoyQuicClientConnection { generator) { SetEncrypter(quic::ENCRYPTION_FORWARD_SECURE, std::make_unique(quic::ENCRYPTION_FORWARD_SECURE)); + InstallDecrypter(quic::ENCRYPTION_FORWARD_SECURE, + std::make_unique()); SetDefaultEncryptionLevel(quic::ENCRYPTION_FORWARD_SECURE); } @@ -64,10 +66,11 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParamallocateDispatcher("test_thread")), connection_helper_(*dispatcher_), alarm_factory_(*dispatcher_, *connection_helper_.GetClock()), quic_version_({GetParam()}), - peer_addr_(Network::Utility::getAddressWithPort(*Network::Utility::getIpv6LoopbackAddress(), - 12345)), + peer_addr_( + Network::Utility::getAddressWithPort(*Network::Utility::getIpv6LoopbackAddress(), 0)), self_addr_(Network::Utility::getAddressWithPort(*Network::Utility::getIpv6LoopbackAddress(), 54321)), + peer_socket_(createConnectionSocket(self_addr_, peer_addr_, nullptr)), quic_connection_(new TestEnvoyQuicClientConnection( quic::test::TestConnectionId(), connection_helper_, alarm_factory_, writer_, quic_version_, *dispatcher_, createConnectionSocket(peer_addr_, self_addr_, nullptr), @@ -83,12 +86,15 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParammutable_num_timeouts_to_trigger_port_migration() + ->set_value(1); + http_connection_ = std::make_unique( + envoy_quic_session_, http_connection_callbacks_, stats_, http3_options_, 64 * 1024, 100); EXPECT_EQ(time_system_.systemTime(), envoy_quic_session_.streamInfo().startTime()); EXPECT_EQ(EMPTY_STRING, envoy_quic_session_.nextProtocol()); - EXPECT_EQ(Http::Protocol::Http3, http_connection_.protocol()); + EXPECT_EQ(Http::Protocol::Http3, http_connection_->protocol()); time_system_.advanceTimeWait(std::chrono::milliseconds(1)); ON_CALL(writer_, WritePacket(_, _, _, _, _, _)) @@ -98,6 +104,9 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParamclose(); } EnvoyQuicClientStream& sendGetRequest(Http::ResponseDecoder& response_decoder, Http::StreamCallbacks& stream_callbacks) { auto& stream = - dynamic_cast(http_connection_.newStream(response_decoder)); + dynamic_cast(http_connection_->newStream(response_decoder)); stream.getStream().addCallbacks(stream_callbacks); std::string host("www.abc.com"); @@ -136,8 +146,12 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam writer_; + // Initialized with port 0 and modified during peer_socket_ creation. Network::Address::InstanceConstSharedPtr peer_addr_; Network::Address::InstanceConstSharedPtr self_addr_; + // Used in some tests to trigger a read event on the connection to test its full interaction with + // socket read utility functions. + Network::ConnectionSocketPtr peer_socket_; quic::DeterministicConnectionIdGenerator connection_id_generator_{ quic::kQuicDefaultConnectionIdLength}; TestEnvoyQuicClientConnection* quic_connection_; @@ -156,13 +170,13 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam write_current_; Http::Http3::CodecStats stats_; envoy::config::core::v3::Http3ProtocolOptions http3_options_; - QuicHttpClientConnectionImpl http_connection_; + std::unique_ptr http_connection_; }; INSTANTIATE_TEST_SUITE_P(EnvoyQuicClientSessionTests, EnvoyQuicClientSessionTest, testing::ValuesIn(quic::CurrentSupportedHttp3Versions())); -TEST_P(EnvoyQuicClientSessionTest, ShutdownNoOp) { http_connection_.shutdownNotice(); } +TEST_P(EnvoyQuicClientSessionTest, ShutdownNoOp) { http_connection_->shutdownNotice(); } TEST_P(EnvoyQuicClientSessionTest, NewStream) { Http::MockResponseDecoder response_decoder; @@ -412,5 +426,49 @@ TEST_P(EnvoyQuicClientSessionTest, VerifyContextAbortOnFlushWriteBuffer) { "unexpectedly reached"); } +// Tests that receiving a STATELESS_RESET packet on the probing socket doesn't cause crash. +TEST_P(EnvoyQuicClientSessionTest, StatelessResetOnProbingSocket) { + quic::QuicNewConnectionIdFrame frame; + frame.connection_id = quic::test::TestConnectionId(1234); + ASSERT_NE(frame.connection_id, quic_connection_->connection_id()); + frame.stateless_reset_token = quic::QuicUtils::GenerateStatelessResetToken(frame.connection_id); + frame.retire_prior_to = 0u; + frame.sequence_number = 1u; + quic_connection_->OnNewConnectionIdFrame(frame); + quic_connection_->SetSelfAddress(envoyIpAddressToQuicSocketAddress(self_addr_->ip())); + + // Trigger port migration. + quic_connection_->OnPathDegradingDetected(); + EXPECT_TRUE(envoy_quic_session_.HasPendingPathValidation()); + auto* path_validation_context = + dynamic_cast( + quic_connection_->GetPathValidationContext()); + Network::ConnectionSocket& probing_socket = path_validation_context->probingSocket(); + const Network::Address::InstanceConstSharedPtr& new_self_address = + probing_socket.connectionInfoProvider().localAddress(); + EXPECT_NE(new_self_address->asString(), self_addr_->asString()); + + // Send a STATELESS_RESET packet to the probing socket. + std::unique_ptr stateless_reset_packet = + quic::QuicFramer::BuildIetfStatelessResetPacket( + frame.connection_id, /*received_packet_length*/ 1200, + quic::QuicUtils::GenerateStatelessResetToken(quic::test::TestConnectionId())); + Buffer::RawSlice slice; + slice.mem_ = const_cast(stateless_reset_packet->data()); + slice.len_ = stateless_reset_packet->length(); + peer_socket_->ioHandle().sendmsg(&slice, 1, 0, peer_addr_->ip(), *new_self_address); + + // Receiving the STATELESS_RESET on the probing socket shouldn't close the connection but should + // fail the probing. + EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::RemoteClose)) + .Times(0); + while (envoy_quic_session_.HasPendingPathValidation()) { + // Running event loop to receive the STATELESS_RESET and following socket reads shouldn't cause + // crash. + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + } + EXPECT_EQ(self_addr_->asString(), quic_connection_->self_address().ToString()); +} + } // namespace Quic } // namespace Envoy diff --git a/test/common/quic/test_utils.h b/test/common/quic/test_utils.h index 63a378bc3496..7a760158e411 100644 --- a/test/common/quic/test_utils.h +++ b/test/common/quic/test_utils.h @@ -162,6 +162,7 @@ class TestQuicCryptoClientStream : public quic::QuicCryptoClientStream { proof_handler, has_application_state) {} bool encryption_established() const override { return true; } + quic::HandshakeState GetHandshakeState() const override { return quic::HANDSHAKE_CONFIRMED; } }; class TestQuicCryptoClientStreamFactory : public EnvoyQuicCryptoClientStreamFactoryInterface {