Skip to content

Commit

Permalink
quiche: check quic transport socket configuration is correct (#15000)
Browse files Browse the repository at this point in the history
Made listener manager to depends on extensions/quic_listeners/quiche:quic_transport_socket_factory_lib. The library itself doesn't depends on QUICHE, so quic listener can still be compiled out.

This is to prevent the case where a problematic config gets loaded which uses a legal transport socket config but not for quic transport socket and a new quic connection fails to get cert chain after the quic listener starts listening.

Risk Level: low
Testing: added unit test

#2557

Signed-off-by: Dan Zhang <[email protected]>
  • Loading branch information
danzh2010 authored Feb 22, 2021
1 parent 7b42ebd commit e29e3c0
Show file tree
Hide file tree
Showing 14 changed files with 114 additions and 18 deletions.
1 change: 1 addition & 0 deletions api/envoy/config/listener/v3/quic_config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ option java_multiple_files = true;
option (udpa.annotations.file_status).package_version_status = ACTIVE;

// [#protodoc-title: QUIC listener Config]
// [#extension: envoy.listener.quic]

// Configuration specific to the QUIC protocol.
// Next id: 5
Expand Down
1 change: 1 addition & 0 deletions api/envoy/config/listener/v4alpha/quic_config.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ REPOSITORY_LOCATIONS_SPEC = dict(
sha256 = "52680dea984dbe899c27176155578b97276e1f1516b7c3a63fb16ba593061859",
urls = ["https://storage.googleapis.com/quiche-envoy-integration/{version}.tar.gz"],
use_category = ["dataplane_ext"],
extensions = ["envoy.transport_sockets.quic"],
extensions = ["envoy.transport_sockets.quic", "envoy.listener.quic"],
release_date = "2020-11-10",
cpe = "N/A",
),
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion source/extensions/extensions_build_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,12 @@ EXTENSIONS = {
"envoy.tracers.xray": "//source/extensions/tracers/xray:config",
"envoy.tracers.skywalking": "//source/extensions/tracers/skywalking:config",

#
# Listener
#

"envoy.listener.quic": "//source/extensions/quic_listeners/quiche:quic_factory_lib",

#
# Transport sockets
#
Expand All @@ -181,7 +187,7 @@ EXTENSIONS = {
"envoy.transport_sockets.upstream_proxy_protocol": "//source/extensions/transport_sockets/proxy_protocol:upstream_config",
"envoy.transport_sockets.raw_buffer": "//source/extensions/transport_sockets/raw_buffer:config",
"envoy.transport_sockets.tap": "//source/extensions/transport_sockets/tap:config",
"envoy.transport_sockets.quic": "//source/extensions/quic_listeners/quiche:quic_factory_lib",
"envoy.transport_sockets.quic": "//source/extensions/quic_listeners/quiche:quic_transport_socket_factory_lib",
"envoy.transport_sockets.starttls": "//source/extensions/transport_sockets/starttls:config",

#
Expand Down
12 changes: 10 additions & 2 deletions source/extensions/quic_listeners/quiche/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -351,10 +351,19 @@ envoy_cc_library(
],
)

envoy_cc_library(
envoy_cc_extension(
name = "quic_transport_socket_factory_lib",
srcs = ["quic_transport_socket_factory.cc"],
hdrs = ["quic_transport_socket_factory.h"],
category = (
"envoy.transport_sockets.downstream",
"envoy.transport_sockets.upstream",
),
# Needed to verify that a quic specific configuration is used for quic transport socket.
extra_visibility = [
"//source/server:__subpackages__",
],
security_posture = "unknown",
tags = ["nofips"],
deps = [
"//include/envoy/network:transport_socket_interface",
Expand All @@ -377,7 +386,6 @@ envoy_cc_extension(
),
security_posture = "unknown",
tags = ["nofips"],

# QUICHE can't build against FIPS BoringSSL until the FIPS build
# is on a new enough version to have QUIC support. Remove it from
# the build until then. Re-enable as part of #7433.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,15 @@ EnvoyQuicProofSource::getTlsCertConfigAndFilterChain(const quic::QuicSocketAddre
}
const Network::TransportSocketFactory& transport_socket_factory =
filter_chain->transportSocketFactory();

std::vector<std::reference_wrapper<const Envoy::Ssl::TlsCertificateConfig>> tls_cert_configs =
dynamic_cast<const QuicServerTransportSocketFactory&>(transport_socket_factory)
.serverContextConfig()
.tlsCertificates();

if (tls_cert_configs.empty()) {
return {absl::nullopt, absl::nullopt};
}
// Only return the first TLS cert config.
// TODO(danzh) Choose based on supported cipher suites in TLS1.3 CHLO and prefer EC
// certs if supported.
Expand Down
1 change: 1 addition & 0 deletions source/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ envoy_cc_library(
"//source/common/stream_info:stream_info_lib",
"//source/extensions/filters/listener:well_known_names",
"//source/extensions/filters/network/http_connection_manager:config",
"//source/extensions/quic_listeners/quiche:quic_transport_socket_factory_lib",
"//source/extensions/transport_sockets:well_known_names",
"//source/extensions/upstreams/http/generic:config",
"@envoy_api//envoy/admin/v3:pkg_cc_proto",
Expand Down
22 changes: 13 additions & 9 deletions source/server/listener_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "server/well_known_names.h"

#include "extensions/filters/listener/well_known_names.h"
#include "extensions/quic_listeners/quiche/quic_transport_socket_factory.h"
#include "extensions/transport_sockets/well_known_names.h"

namespace Envoy {
Expand Down Expand Up @@ -977,15 +978,8 @@ void ListenerManagerImpl::endListenerUpdate(FailureStates&& failure_states) {
ListenerFilterChainFactoryBuilder::ListenerFilterChainFactoryBuilder(
ListenerImpl& listener,
Server::Configuration::TransportSocketFactoryContextImpl& factory_context)
: ListenerFilterChainFactoryBuilder(listener.validation_visitor_, listener.parent_.factory_,
factory_context) {}

ListenerFilterChainFactoryBuilder::ListenerFilterChainFactoryBuilder(
ProtobufMessage::ValidationVisitor& validator,
ListenerComponentFactory& listener_component_factory,
Server::Configuration::TransportSocketFactoryContextImpl& factory_context)
: validator_(validator), listener_component_factory_(listener_component_factory),
factory_context_(factory_context) {}
: listener_(listener), validator_(listener.validation_visitor_),
listener_component_factory_(listener.parent_.factory_), factory_context_(factory_context) {}

Network::DrainableFilterChainSharedPtr ListenerFilterChainFactoryBuilder::buildFilterChain(
const envoy::config::listener::v3::FilterChain& filter_chain,
Expand Down Expand Up @@ -1014,6 +1008,16 @@ Network::DrainableFilterChainSharedPtr ListenerFilterChainFactoryBuilder::buildF

auto& config_factory = Config::Utility::getAndCheckFactory<
Server::Configuration::DownstreamTransportSocketConfigFactory>(transport_socket);
// The only connection oriented UDP transport protocol right now is QUIC.
const bool is_quic = listener_.udpListenerFactory() != nullptr &&
!listener_.udpListenerFactory()->isTransportConnectionless();
if (is_quic &&
dynamic_cast<Quic::QuicServerTransportSocketConfigFactory*>(&config_factory) == nullptr) {
throw EnvoyException(fmt::format("error building filter chain for quic listener: wrong "
"transport socket config specified for quic transport socket: "
"{}. \nUse QuicDownstreamTransport instead.",
transport_socket.DebugString()));
}
ProtobufTypes::MessagePtr message =
Config::Utility::translateToFactoryConfig(transport_socket, validator_, config_factory);

Expand Down
6 changes: 1 addition & 5 deletions source/server/listener_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -320,11 +320,6 @@ class ListenerFilterChainFactoryBuilder : public FilterChainFactoryBuilder {
ListenerFilterChainFactoryBuilder(
ListenerImpl& listener, Configuration::TransportSocketFactoryContextImpl& factory_context);

ListenerFilterChainFactoryBuilder(
ProtobufMessage::ValidationVisitor& validator,
ListenerComponentFactory& listener_component_factory,
Server::Configuration::TransportSocketFactoryContextImpl& factory_context);

Network::DrainableFilterChainSharedPtr
buildFilterChain(const envoy::config::listener::v3::FilterChain& filter_chain,
FilterChainFactoryContextCreator& context_creator) const override;
Expand All @@ -334,6 +329,7 @@ class ListenerFilterChainFactoryBuilder : public FilterChainFactoryBuilder {
const envoy::config::listener::v3::FilterChain& filter_chain,
Configuration::FilterChainFactoryContextPtr&& filter_chain_factory_context) const;

ListenerImpl& listener_;
ProtobufMessage::ValidationVisitor& validator_;
ListenerComponentFactory& listener_component_factory_;
Configuration::TransportSocketFactoryContextImpl& factory_context_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,35 @@ TEST_F(EnvoyQuicProofSourceTest, GetProofFailNoFilterChain) {
EXPECT_TRUE(called);
}

TEST_F(EnvoyQuicProofSourceTest, GetProofFailNoCertConfig) {
bool called = false;
auto callback = std::make_unique<TestGetProofCallback>(called, false, server_config_, version_,
chlo_hash_, filter_chain_);
EXPECT_CALL(listen_socket_, ioHandle()).Times(1u);
EXPECT_CALL(filter_chain_manager_, findFilterChain(_))
.WillRepeatedly(Invoke([&](const Network::ConnectionSocket& connection_socket) {
EXPECT_EQ(*quicAddressToEnvoyAddressInstance(server_address_),
*connection_socket.addressProvider().localAddress());
EXPECT_EQ(*quicAddressToEnvoyAddressInstance(client_address_),
*connection_socket.addressProvider().remoteAddress());
EXPECT_EQ(Extensions::TransportSockets::TransportProtocolNames::get().Quic,
connection_socket.detectedTransportProtocol());
EXPECT_EQ("h2", connection_socket.requestedApplicationProtocols()[0]);
return &filter_chain_;
}));
EXPECT_CALL(filter_chain_, transportSocketFactory())
.WillRepeatedly(ReturnRef(transport_socket_factory_));

EXPECT_CALL(dynamic_cast<const Ssl::MockServerContextConfig&>(
transport_socket_factory_.serverContextConfig()),
tlsCertificates())
.WillRepeatedly(
Return(std::vector<std::reference_wrapper<const Envoy::Ssl::TlsCertificateConfig>>{}));
proof_source_.GetProof(server_address_, client_address_, hostname_, server_config_, version_,
chlo_hash_, std::move(callback));
EXPECT_TRUE(called);
}

TEST_F(EnvoyQuicProofSourceTest, GetProofFailInvalidCert) {
std::string invalid_cert{R"(-----BEGIN CERTIFICATE-----
invalid certificate
Expand Down
1 change: 1 addition & 0 deletions test/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ envoy_cc_test(
":utility_lib",
"//source/extensions/quic_listeners/quiche:quic_factory_lib",
"//source/extensions/transport_sockets/raw_buffer:config",
"//source/extensions/transport_sockets/tls:config",
"//test/test_common:threadsafe_singleton_injector_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
"@envoy_api//envoy/config/listener/v3:pkg_cc_proto",
Expand Down
43 changes: 43 additions & 0 deletions test/server/listener_manager_impl_quic_only_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,49 @@ reuse_port: true
EXPECT_TRUE(quic_socket_factory.serverContextConfig().isReady());
}

TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithWrongTransportSocket) {
const std::string yaml = TestEnvironment::substitute(R"EOF(
address:
socket_address:
address: 127.0.0.1
protocol: UDP
port_value: 1234
filter_chains:
- filter_chain_match:
transport_protocol: "quic"
filters: []
transport_socket:
name: envoy.transport_sockets.quic
typed_config:
"@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.DownstreamTlsContext
common_tls_context:
tls_certificates:
- certificate_chain:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_cert.pem"
private_key:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_key.pem"
validation_context:
trusted_ca:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem"
match_subject_alt_names:
- exact: localhost
- exact: 127.0.0.1
reuse_port: true
udp_listener_config:
udp_listener_name: "quiche_quic_listener"
udp_writer_config:
name: "udp_gso_batch_writer"
typed_config:
"@type": type.googleapis.com/envoy.config.listener.v3.UdpGsoBatchWriterOptions
)EOF",
Network::Address::IpVersion::v4);

envoy::config::listener::v3::Listener listener_proto = parseListenerFromV3Yaml(yaml);

EXPECT_THROW_WITH_REGEX(manager_->addOrUpdateListener(listener_proto, "", true), EnvoyException,
"wrong transport socket config specified for quic transport socket");
}

} // namespace
} // namespace Server
} // namespace Envoy

0 comments on commit e29e3c0

Please sign in to comment.