Skip to content

Commit

Permalink
quic: fix socket UAF during port migration (envoyproxy#31702)
Browse files Browse the repository at this point in the history 
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 <danzh@google.com>
Co-authored-by: Dan Zhang <danzh@google.com>
  • Loading branch information
@danzh2010 @danzh1989
danzh2010 and danzh1989 authored Jan 10, 2024
1 parent 24ffda3 commit ffc021d
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 32 deletions.
3 changes: 3 additions & 0 deletions changelogs/current.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
16 changes: 16 additions & 0 deletions source/common/quic/envoy_quic_client_connection.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Network::ConnectionSocket> socket)
: socket_(std::move(socket)) {}

void deleteIsPending() override { socket_->close(); }

private:
std::unique_ptr<Network::ConnectionSocket> socket_;
};

EnvoyQuicClientConnection::EnvoyQuicClientConnection(
const quic::QuicConnectionId& server_connection_id,
Network::Address::InstanceConstSharedPtr& initial_peer_address,
Expand Down Expand Up @@ -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<Network::ConnectionSocket> probing_socket =
static_cast<EnvoyQuicPathValidationContext*>(context.get())->releaseSocket();
// Extend the socket life time till the end of the current event loop.
dispatcher_.deferredDelete(std::make_unique<DeferredDeletableSocket>(std::move(probing_socket)));
}

void EnvoyQuicClientConnection::onFileEvent(uint32_t events,
Expand Down
46 changes: 23 additions & 23 deletions source/common/quic/envoy_quic_client_connection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<EnvoyQuicPacketWriter> writer,
std::unique_ptr<Network::ConnectionSocket> probing_socket);

~EnvoyQuicPathValidationContext() override;

quic::QuicPacketWriter* WriterToUse() override;

EnvoyQuicPacketWriter* releaseWriter();

Network::ConnectionSocket& probingSocket();

std::unique_ptr<Network::ConnectionSocket> releaseSocket();

private:
std::unique_ptr<EnvoyQuicPacketWriter> 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,
Expand Down Expand Up @@ -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<EnvoyQuicPacketWriter> writer,
std::unique_ptr<Network::ConnectionSocket> probing_socket);

~EnvoyQuicPathValidationContext() override;

quic::QuicPacketWriter* WriterToUse() override;

EnvoyQuicPacketWriter* releaseWriter();

Network::ConnectionSocket& probingSocket();

std::unique_ptr<Network::ConnectionSocket> releaseSocket();

private:
std::unique_ptr<EnvoyQuicPacketWriter> writer_;
Network::ConnectionSocketPtr socket_;
};

// Receives notifications from the Quiche layer on path validation results.
class EnvoyPathValidationResultDelegate : public quic::QuicPathValidator::ResultDelegate {
public:
Expand Down
76 changes: 67 additions & 9 deletions test/common/quic/envoy_quic_client_session_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class TestEnvoyQuicClientConnection : public EnvoyQuicClientConnection {
generator) {
SetEncrypter(quic::ENCRYPTION_FORWARD_SECURE,
std::make_unique<quic::test::TaggingEncrypter>(quic::ENCRYPTION_FORWARD_SECURE));
InstallDecrypter(quic::ENCRYPTION_FORWARD_SECURE,
std::make_unique<quic::test::TaggingDecrypter>());
SetDefaultEncryptionLevel(quic::ENCRYPTION_FORWARD_SECURE);
}

Expand All @@ -64,10 +66,11 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam<quic::ParsedQui
: api_(Api::createApiForTest(time_system_)),
dispatcher_(api_->allocateDispatcher("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),
Expand All @@ -83,12 +86,15 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam<quic::ParsedQui
/*send_buffer_limit*/ 1024 * 1024, crypto_stream_factory_, quic_stat_names_, {},
*store_.rootScope(), transport_socket_options_, {}),
stats_({ALL_HTTP3_CODEC_STATS(POOL_COUNTER_PREFIX(store_, "http3."),
POOL_GAUGE_PREFIX(store_, "http3."))}),
http_connection_(envoy_quic_session_, http_connection_callbacks_, stats_, http3_options_,
64 * 1024, 100) {
POOL_GAUGE_PREFIX(store_, "http3."))}) {
http3_options_.mutable_quic_protocol_options()
->mutable_num_timeouts_to_trigger_port_migration()
->set_value(1);
http_connection_ = std::make_unique<QuicHttpClientConnectionImpl>(
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(_, _, _, _, _, _))
Expand All @@ -98,6 +104,9 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam<quic::ParsedQui
void SetUp() override {
envoy_quic_session_.Initialize();
setQuicConfigWithDefaultValues(envoy_quic_session_.config());
quic::test::QuicConfigPeer::SetReceivedStatelessResetToken(
envoy_quic_session_.config(),
quic::QuicUtils::GenerateStatelessResetToken(quic::test::TestConnectionId()));
envoy_quic_session_.OnConfigNegotiated();
envoy_quic_session_.addConnectionCallbacks(network_connection_callbacks_);
envoy_quic_session_.setConnectionStats(
Expand All @@ -112,12 +121,13 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam<quic::ParsedQui
EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::LocalClose));
envoy_quic_session_.close(Network::ConnectionCloseType::NoFlush);
}
peer_socket_->close();
}

EnvoyQuicClientStream& sendGetRequest(Http::ResponseDecoder& response_decoder,
Http::StreamCallbacks& stream_callbacks) {
auto& stream =
dynamic_cast<EnvoyQuicClientStream&>(http_connection_.newStream(response_decoder));
dynamic_cast<EnvoyQuicClientStream&>(http_connection_->newStream(response_decoder));
stream.getStream().addCallbacks(stream_callbacks);

std::string host("www.abc.com");
Expand All @@ -136,8 +146,12 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam<quic::ParsedQui
EnvoyQuicAlarmFactory alarm_factory_;
quic::ParsedQuicVersionVector quic_version_;
testing::NiceMock<quic::test::MockPacketWriter> 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_;
Expand All @@ -156,13 +170,13 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam<quic::ParsedQui
testing::StrictMock<Stats::MockGauge> write_current_;
Http::Http3::CodecStats stats_;
envoy::config::core::v3::Http3ProtocolOptions http3_options_;
QuicHttpClientConnectionImpl http_connection_;
std::unique_ptr<QuicHttpClientConnectionImpl> 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;
Expand Down Expand Up @@ -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<EnvoyQuicClientConnection::EnvoyQuicPathValidationContext*>(
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<quic::QuicEncryptedPacket> 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<char*>(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
1 change: 1 addition & 0 deletions test/common/quic/test_utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit ffc021d

Please sign in to comment.