Skip to content

Commit

Permalink
quiche: change crypto stream factory interfaces (#17046)
Browse files Browse the repository at this point in the history
Commit Message: pass dispatcher and transport socket factory into createEnvoyQuicCryptoServerStream(). These two parameters are not used in the default extension, but can be used for other extensions if needed.

Risk Level: low
Testing: existing test

Signed-off-by: Dan Zhang <danzh@google.com>
  • Loading branch information
danzh2010 authored Jun 22, 2021
1 parent c7cfbf1 commit 59c2c1a
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 23 deletions.
13 changes: 8 additions & 5 deletions source/common/quic/envoy_quic_crypto_stream_factory.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#pragma once

#include "envoy/common/optref.h"
#include "envoy/config/typed_config.h"
#include "envoy/network/transport_socket.h"

#if defined(__GNUC__)
#pragma GCC diagnostic push
Expand All @@ -26,11 +28,12 @@ class EnvoyQuicCryptoServerStreamFactoryInterface : public Config::TypedFactory
std::string category() const override { return "envoy.quic.server.crypto_stream"; }

// Return an Envoy specific quic crypto server stream object.
virtual std::unique_ptr<quic::QuicCryptoServerStreamBase>
createEnvoyQuicCryptoServerStream(const quic::QuicCryptoServerConfig* crypto_config,
quic::QuicCompressedCertsCache* compressed_certs_cache,
quic::QuicSession* session,
quic::QuicCryptoServerStreamBase::Helper* helper) PURE;
virtual std::unique_ptr<quic::QuicCryptoServerStreamBase> createEnvoyQuicCryptoServerStream(
const quic::QuicCryptoServerConfig* crypto_config,
quic::QuicCompressedCertsCache* compressed_certs_cache, quic::QuicSession* session,
quic::QuicCryptoServerStreamBase::Helper* helper,
OptRef<const Network::TransportSocketFactory> transport_socket_factory,
Event::Dispatcher& dispatcher) PURE;
};

class EnvoyQuicCryptoClientStreamFactoryInterface {
Expand Down
6 changes: 5 additions & 1 deletion source/common/quic/envoy_quic_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#include <openssl/crypto.h>

#include "envoy/common/optref.h"

#include "source/common/common/safe_memcpy.h"
#include "source/common/http/utility.h"
#include "source/common/quic/envoy_quic_server_connection.h"
Expand Down Expand Up @@ -71,7 +73,9 @@ std::unique_ptr<quic::QuicSession> EnvoyQuicDispatcher::CreateQuicSession(
quic_config, quic::ParsedQuicVersionVector{version}, std::move(quic_connection), this,
session_helper(), crypto_config(), compressed_certs_cache(), dispatcher_,
listener_config_.perConnectionBufferLimitBytes(), quic_stat_names_,
listener_config_.listenerScope(), crypto_server_stream_factory_);
listener_config_.listenerScope(), crypto_server_stream_factory_,
makeOptRefFromPtr(filter_chain == nullptr ? nullptr
: &filter_chain->transportSocketFactory()));
if (filter_chain != nullptr) {
const bool has_filter_initialized =
listener_config_.filterChainFactory().createNetworkFilterChain(
Expand Down
10 changes: 6 additions & 4 deletions source/common/quic/envoy_quic_server_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ EnvoyQuicServerSession::EnvoyQuicServerSession(
quic::QuicCryptoServerStream::Helper* helper, const quic::QuicCryptoServerConfig* crypto_config,
quic::QuicCompressedCertsCache* compressed_certs_cache, Event::Dispatcher& dispatcher,
uint32_t send_buffer_limit, QuicStatNames& quic_stat_names, Stats::Scope& listener_scope,
EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory)
EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory,
OptRef<const Network::TransportSocketFactory> transport_socket_factory)
: quic::QuicServerSessionBase(config, supported_versions, connection.get(), visitor, helper,
crypto_config, compressed_certs_cache),
QuicFilterManagerConnectionImpl(*connection, connection->connection_id(), dispatcher,
send_buffer_limit),
quic_connection_(std::move(connection)), quic_stat_names_(quic_stat_names),
listener_scope_(listener_scope), crypto_server_stream_factory_(crypto_server_stream_factory) {
}
listener_scope_(listener_scope), crypto_server_stream_factory_(crypto_server_stream_factory),
transport_socket_factory_(transport_socket_factory) {}

EnvoyQuicServerSession::~EnvoyQuicServerSession() {
ASSERT(!quic_connection_->connected());
Expand All @@ -38,7 +39,8 @@ EnvoyQuicServerSession::CreateQuicCryptoServerStream(
const quic::QuicCryptoServerConfig* crypto_config,
quic::QuicCompressedCertsCache* compressed_certs_cache) {
return crypto_server_stream_factory_.createEnvoyQuicCryptoServerStream(
crypto_config, compressed_certs_cache, this, stream_helper());
crypto_config, compressed_certs_cache, this, stream_helper(), transport_socket_factory_,
dispatcher());
}

quic::QuicSpdyStream* EnvoyQuicServerSession::CreateIncomingStream(quic::QuicStreamId id) {
Expand Down
4 changes: 3 additions & 1 deletion source/common/quic/envoy_quic_server_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ class EnvoyQuicServerSession : public quic::QuicServerSessionBase,
quic::QuicCompressedCertsCache* compressed_certs_cache,
Event::Dispatcher& dispatcher, uint32_t send_buffer_limit,
QuicStatNames& quic_stat_names, Stats::Scope& listener_scope,
EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory);
EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory,
OptRef<const Network::TransportSocketFactory> transport_socket_factory);

~EnvoyQuicServerSession() override;

Expand Down Expand Up @@ -118,6 +119,7 @@ class EnvoyQuicServerSession : public quic::QuicServerSessionBase,
Stats::Scope& listener_scope_;

EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory_;
OptRef<const Network::TransportSocketFactory> transport_socket_factory_;
};

} // namespace Quic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ std::unique_ptr<quic::QuicCryptoServerStreamBase>
EnvoyQuicCryptoServerStreamFactoryImpl::createEnvoyQuicCryptoServerStream(
const quic::QuicCryptoServerConfig* crypto_config,
quic::QuicCompressedCertsCache* compressed_certs_cache, quic::QuicSession* session,
quic::QuicCryptoServerStreamBase::Helper* helper) {
quic::QuicCryptoServerStreamBase::Helper* helper,
// Though this extension doesn't use the two parameters below, they might be used by
// downstreams. Do not remove them.
OptRef<const Network::TransportSocketFactory> /*transport_socket_factory*/,
Envoy::Event::Dispatcher& /*dispatcher*/) {
return quic::CreateCryptoServerStream(crypto_config, compressed_certs_cache, session, helper);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ class EnvoyQuicCryptoServerStreamFactoryImpl : public EnvoyQuicCryptoServerStrea
return std::make_unique<envoy::extensions::quic::crypto_stream::v3::CryptoServerStreamConfig>();
}
std::string name() const override { return "envoy.quic.crypto_stream.server.quiche"; }
std::unique_ptr<quic::QuicCryptoServerStreamBase>
createEnvoyQuicCryptoServerStream(const quic::QuicCryptoServerConfig* crypto_config,
quic::QuicCompressedCertsCache* compressed_certs_cache,
quic::QuicSession* session,
quic::QuicCryptoServerStreamBase::Helper* helper) override;
std::unique_ptr<quic::QuicCryptoServerStreamBase> createEnvoyQuicCryptoServerStream(
const quic::QuicCryptoServerConfig* crypto_config,
quic::QuicCompressedCertsCache* compressed_certs_cache, quic::QuicSession* session,
quic::QuicCryptoServerStreamBase::Helper* helper,
OptRef<const Network::TransportSocketFactory> transport_socket_factory,
Envoy::Event::Dispatcher& dispatcher) override;
};

DECLARE_FACTORY(EnvoyQuicCryptoServerStreamFactoryImpl);
Expand Down
4 changes: 4 additions & 0 deletions test/common/quic/active_quic_listener_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,9 @@ class ActiveQuicListenerTest : public QuicMultiVersionTest {
read_filters_.push_back(std::move(read_filter));
// A Sequence must be used to allow multiple EXPECT_CALL().WillOnce()
// calls for the same object.
EXPECT_CALL(*filter_chain_, transportSocketFactory())
.InSequence(seq)
.WillOnce(ReturnRef(transport_socket_factory_));
EXPECT_CALL(*filter_chain_, networkFilterFactories())
.InSequence(seq)
.WillOnce(ReturnRef(filter_factories_.back()));
Expand Down Expand Up @@ -321,6 +324,7 @@ class ActiveQuicListenerTest : public QuicMultiVersionTest {
// of elements are saved in expectations before new elements are added.
std::list<std::vector<Network::FilterFactoryCb>> filter_factories_;
const Network::MockFilterChain* filter_chain_;
Network::MockTransportSocketFactory transport_socket_factory_;
quic::ParsedQuicVersion quic_version_;
uint32_t connection_window_size_{1024u};
uint32_t stream_window_size_{1024u};
Expand Down
6 changes: 6 additions & 0 deletions test/common/quic/envoy_quic_dispatcher_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,9 @@ class EnvoyQuicDispatcherTest : public QuicMultiVersionTest,
EXPECT_EQ("test.example.org", socket.requestedServerName());
return &proof_source_->filterChain();
}));
Network::MockTransportSocketFactory transport_socket_factory;
EXPECT_CALL(proof_source_->filterChain(), transportSocketFactory())
.WillOnce(ReturnRef(transport_socket_factory));
EXPECT_CALL(proof_source_->filterChain(), networkFilterFactories())
.WillOnce(ReturnRef(filter_factory));
EXPECT_CALL(listener_config_, filterChainFactory());
Expand Down Expand Up @@ -290,6 +293,9 @@ TEST_P(EnvoyQuicDispatcherTest, CloseConnectionDuringFilterInstallation) {
EXPECT_CALL(listener_config_, filterChainManager()).WillOnce(ReturnRef(filter_chain_manager));
EXPECT_CALL(filter_chain_manager, findFilterChain(_))
.WillOnce(Return(&proof_source_->filterChain()));
Network::MockTransportSocketFactory transport_socket_factory;
EXPECT_CALL(proof_source_->filterChain(), transportSocketFactory())
.WillOnce(ReturnRef(transport_socket_factory));
EXPECT_CALL(proof_source_->filterChain(), networkFilterFactories())
.WillOnce(ReturnRef(filter_factory));
EXPECT_CALL(listener_config_, filterChainFactory());
Expand Down
14 changes: 8 additions & 6 deletions test/common/quic/envoy_quic_server_session_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,12 @@ class EnvoyQuicTestCryptoServerStreamFactory : public EnvoyQuicCryptoServerStrea
ProtobufTypes::MessagePtr createEmptyConfigProto() override { return nullptr; }
std::string name() const override { return "quic.test_crypto_server_stream"; }

std::unique_ptr<quic::QuicCryptoServerStreamBase>
createEnvoyQuicCryptoServerStream(const quic::QuicCryptoServerConfig* crypto_config,
quic::QuicCompressedCertsCache* compressed_certs_cache,
quic::QuicSession* session,
quic::QuicCryptoServerStreamBase::Helper* helper) override {
std::unique_ptr<quic::QuicCryptoServerStreamBase> createEnvoyQuicCryptoServerStream(
const quic::QuicCryptoServerConfig* crypto_config,
quic::QuicCompressedCertsCache* compressed_certs_cache, quic::QuicSession* session,
quic::QuicCryptoServerStreamBase::Helper* helper,
OptRef<const Network::TransportSocketFactory> /*transport_socket_factory*/,
Event::Dispatcher& /*dispatcher*/) override {
switch (session->connection()->version().handshake_protocol) {
case quic::PROTOCOL_QUIC_CRYPTO:
return std::make_unique<TestQuicCryptoServerStream>(crypto_config, compressed_certs_cache,
Expand Down Expand Up @@ -166,7 +167,8 @@ class EnvoyQuicServerSessionTest : public testing::TestWithParam<bool> {
&compressed_certs_cache_, *dispatcher_,
/*send_buffer_limit*/ quic::kDefaultFlowControlSendWindow * 1.5,
quic_stat_names_, listener_config_.listenerScope(),
crypto_stream_factory_),
crypto_stream_factory_,
makeOptRefFromPtr<const Network::TransportSocketFactory>(nullptr)),
stats_({ALL_HTTP3_CODEC_STATS(
POOL_COUNTER_PREFIX(listener_config_.listenerScope(), "http3."),
POOL_GAUGE_PREFIX(listener_config_.listenerScope(), "http3."))}) {
Expand Down

0 comments on commit 59c2c1a

Please sign in to comment.