Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

network: catch SIGPIPEs via SO_NOSIGPIPE on Darwin #12039

Merged
merged 12 commits into from
Jul 17, 2020
8 changes: 8 additions & 0 deletions source/common/network/socket_option_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,14 @@ std::unique_ptr<Socket::Options> SocketOptionFactory::buildSocketMarkOptions(uin
return options;
}

std::unique_ptr<Socket::Options> SocketOptionFactory::buildSocketNoSigpipeOptions() {
// Provide additional handling for `SIGPIPE` at the socket layer by converting it to `EPIPE`.
std::unique_ptr<Socket::Options> options = std::make_unique<Socket::Options>();
options->push_back(std::make_shared<Network::SocketOptionImpl>(
envoy::config::core::v3::SocketOption::STATE_PREBIND, ENVOY_SOCKET_SO_NOSIGPIPE, 1));
return options;
}

std::unique_ptr<Socket::Options> SocketOptionFactory::buildLiteralOptions(
const Protobuf::RepeatedPtrField<envoy::config::core::v3::SocketOption>& socket_options) {
auto options = std::make_unique<Socket::Options>();
Expand Down
1 change: 1 addition & 0 deletions source/common/network/socket_option_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class SocketOptionFactory : Logger::Loggable<Logger::Id::connection> {
static std::unique_ptr<Socket::Options> buildIpFreebindOptions();
static std::unique_ptr<Socket::Options> buildIpTransparentOptions();
static std::unique_ptr<Socket::Options> buildSocketMarkOptions(uint32_t mark);
static std::unique_ptr<Socket::Options> buildSocketNoSigpipeOptions();
static std::unique_ptr<Socket::Options> buildTcpFastOpenOptions(uint32_t queue_length);
static std::unique_ptr<Socket::Options> buildLiteralOptions(
const Protobuf::RepeatedPtrField<envoy::config::core::v3::SocketOption>& socket_options);
Expand Down
6 changes: 6 additions & 0 deletions source/common/network/socket_option_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ namespace Network {
#define ENVOY_SOCKET_SO_MARK Network::SocketOptionName()
#endif

#ifdef SO_NOSIGPIPE
#define ENVOY_SOCKET_SO_NOSIGPIPE ENVOY_MAKE_SOCKET_OPTION_NAME(SOL_SOCKET, SO_NOSIGPIPE)
#else
#define ENVOY_SOCKET_SO_NOSIGPIPE Network::SocketOptionName()
#endif

#ifdef SO_REUSEPORT
#define ENVOY_SOCKET_SO_REUSEPORT ENVOY_MAKE_SOCKET_OPTION_NAME(SOL_SOCKET, SO_REUSEPORT)
#else
Expand Down
1 change: 1 addition & 0 deletions source/common/upstream/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,7 @@ envoy_cc_library(
"//source/common/network:address_lib",
"//source/common/network:resolver_lib",
"//source/common/network:socket_option_factory_lib",
"//source/common/network:socket_option_lib",
"//source/common/network:utility_lib",
"//source/common/protobuf",
"//source/common/protobuf:utility_lib",
Expand Down
7 changes: 7 additions & 0 deletions source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "common/network/address_impl.h"
#include "common/network/resolver_impl.h"
#include "common/network/socket_option_factory.h"
#include "common/network/socket_option_impl.h"
#include "common/protobuf/protobuf.h"
#include "common/protobuf/utility.h"
#include "common/router/config_utility.h"
Expand Down Expand Up @@ -102,6 +103,12 @@ parseClusterSocketOptions(const envoy::config::cluster::v3::Cluster& config,
const envoy::config::core::v3::BindConfig bind_config) {
Network::ConnectionSocket::OptionsSharedPtr cluster_options =
std::make_shared<Network::ConnectionSocket::Options>();
// The process-wide `signal()` handling may fail to handle SIGPIPE if overridden
// in the process (i.e., on a mobile client). Some OSes support handling it at the socket layer:
if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) {
Network::Socket::appendOptions(cluster_options,
Network::SocketOptionFactory::buildSocketNoSigpipeOptions());
}
// Cluster IP_FREEBIND settings, when set, will override the cluster manager wide settings.
if ((bind_config.freebind().value() && !config.upstream_bind_config().has_freebind()) ||
config.upstream_bind_config().freebind().value()) {
Expand Down
1 change: 1 addition & 0 deletions source/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ envoy_cc_library(
"//source/common/config:utility_lib",
"//source/common/network:resolver_lib",
"//source/common/network:socket_option_factory_lib",
"//source/common/network:socket_option_lib",
"//source/common/network:utility_lib",
"//source/common/protobuf:utility_lib",
"@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
Expand Down
6 changes: 6 additions & 0 deletions source/server/listener_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "common/network/connection_balancer_impl.h"
#include "common/network/resolver_impl.h"
#include "common/network/socket_option_factory.h"
#include "common/network/socket_option_impl.h"
#include "common/network/utility.h"
#include "common/protobuf/utility.h"
#include "common/runtime/runtime_features.h"
Expand Down Expand Up @@ -371,6 +372,11 @@ void ListenerImpl::buildUdpListenerFactory(Network::Socket::Type socket_type,
}

void ListenerImpl::buildListenSocketOptions(Network::Socket::Type socket_type) {
// The process-wide `signal()` handling may fail to handle SIGPIPE if overridden
// in the process (i.e., on a mobile client). Some OSes support handling it at the socket layer:
if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) {
addListenSocketOptions(Network::SocketOptionFactory::buildSocketNoSigpipeOptions());
}
if (PROTOBUF_GET_WRAPPED_OR_DEFAULT(config_, transparent, false)) {
addListenSocketOptions(Network::SocketOptionFactory::buildIpTransparentOptions());
}
Expand Down
82 changes: 69 additions & 13 deletions test/common/upstream/cluster_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class ClusterManagerImplTest : public testing::Test {
address:
socket_address:
address: 127.0.0.1
port_value: 11002
port_value: 11002
)EOF";
const std::string merge_window_enabled = R"EOF(
common_lb_config:
Expand Down Expand Up @@ -3089,6 +3089,8 @@ TEST_F(ClusterManagerInitHelperTest, RemoveClusterWithinInitLoop) {
init_helper_.startInitializingSecondaryClusters();
}

using NameVals = std::vector<std::pair<Network::SocketOptionName, int>>;

// Validate that when options are set in the ClusterManager and/or Cluster, we see the socket option
// propagated to setsockopt(). This is as close to an end-to-end test as we have for this feature,
// due to the complexity of creating an integration test involving the network stack. We only test
Expand All @@ -3101,8 +3103,7 @@ class SockoptsTest : public ClusterManagerImplTest {
void TearDown() override { factory_.tls_.shutdownThread(); }

// TODO(tschroed): Extend this to support socket state as well.
void expectSetsockopts(const std::vector<std::pair<Network::SocketOptionName, int>>& names_vals) {

void expectSetsockopts(const NameVals& names_vals) {
NiceMock<Api::MockOsSysCalls> os_sys_calls;
TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls(&os_sys_calls);
NiceMock<Network::MockConnectionSocket> socket;
Expand Down Expand Up @@ -3145,8 +3146,15 @@ class SockoptsTest : public ClusterManagerImplTest {
}

void expectSetsockoptFreebind() {
std::vector<std::pair<Network::SocketOptionName, int>> names_vals{
{ENVOY_SOCKET_IP_FREEBIND, 1}};
NameVals names_vals{{ENVOY_SOCKET_IP_FREEBIND, 1}};
if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) {
names_vals.emplace_back(std::make_pair(ENVOY_SOCKET_SO_NOSIGPIPE, 1));
}
expectSetsockopts(names_vals);
}

void expectOnlyNoSigpipeOptions() {
NameVals names_vals{{std::make_pair(ENVOY_SOCKET_SO_NOSIGPIPE, 1)}};
expectSetsockopts(names_vals);
}

Expand Down Expand Up @@ -3186,7 +3194,11 @@ TEST_F(SockoptsTest, SockoptsUnset) {
port_value: 11001
)EOF";
initialize(yaml);
expectNoSocketOptions();
if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) {
expectOnlyNoSigpipeOptions();
} else {
expectNoSocketOptions();
}
}

TEST_F(SockoptsTest, FreebindClusterOnly) {
Expand Down Expand Up @@ -3289,8 +3301,11 @@ TEST_F(SockoptsTest, SockoptsClusterOnly) {

)EOF";
initialize(yaml);
std::vector<std::pair<Network::SocketOptionName, int>> names_vals{
{ENVOY_MAKE_SOCKET_OPTION_NAME(1, 2), 3}, {ENVOY_MAKE_SOCKET_OPTION_NAME(4, 5), 6}};
NameVals names_vals{{ENVOY_MAKE_SOCKET_OPTION_NAME(1, 2), 3},
{ENVOY_MAKE_SOCKET_OPTION_NAME(4, 5), 6}};
if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) {
names_vals.emplace_back(std::make_pair(ENVOY_SOCKET_SO_NOSIGPIPE, 1));
}
expectSetsockopts(names_vals);
}

Expand Down Expand Up @@ -3318,8 +3333,11 @@ TEST_F(SockoptsTest, SockoptsClusterManagerOnly) {
{ level: 4, name: 5, int_value: 6, state: STATE_PREBIND }]
)EOF";
initialize(yaml);
std::vector<std::pair<Network::SocketOptionName, int>> names_vals{
{ENVOY_MAKE_SOCKET_OPTION_NAME(1, 2), 3}, {ENVOY_MAKE_SOCKET_OPTION_NAME(4, 5), 6}};
NameVals names_vals{{ENVOY_MAKE_SOCKET_OPTION_NAME(1, 2), 3},
{ENVOY_MAKE_SOCKET_OPTION_NAME(4, 5), 6}};
if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) {
names_vals.emplace_back(std::make_pair(ENVOY_SOCKET_SO_NOSIGPIPE, 1));
}
expectSetsockopts(names_vals);
}

Expand Down Expand Up @@ -3349,8 +3367,11 @@ TEST_F(SockoptsTest, SockoptsClusterOverride) {
socket_options: [{ level: 7, name: 8, int_value: 9, state: STATE_PREBIND }]
)EOF";
initialize(yaml);
std::vector<std::pair<Network::SocketOptionName, int>> names_vals{
{ENVOY_MAKE_SOCKET_OPTION_NAME(1, 2), 3}, {ENVOY_MAKE_SOCKET_OPTION_NAME(4, 5), 6}};
NameVals names_vals{{ENVOY_MAKE_SOCKET_OPTION_NAME(1, 2), 3},
{ENVOY_MAKE_SOCKET_OPTION_NAME(4, 5), 6}};
if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) {
names_vals.emplace_back(std::make_pair(ENVOY_SOCKET_SO_NOSIGPIPE, 1));
}
expectSetsockopts(names_vals);
}

Expand Down Expand Up @@ -3399,6 +3420,14 @@ class TcpKeepaliveTest : public ClusterManagerImplTest {
options, socket, envoy::config::core::v3::SocketOption::STATE_PREBIND)));
return connection_;
}));
if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) {
EXPECT_CALL(socket, setSocketOption(ENVOY_SOCKET_SO_NOSIGPIPE.level(),
ENVOY_SOCKET_SO_NOSIGPIPE.option(), _, sizeof(int)))
.WillOnce(Invoke([](int, int, const void* optval, socklen_t) -> Api::SysCallIntResult {
EXPECT_EQ(1, *static_cast<const int*>(optval));
return {0, 0};
}));
}
EXPECT_CALL(socket, setSocketOption(ENVOY_SOCKET_SO_KEEPALIVE.level(),
ENVOY_SOCKET_SO_KEEPALIVE.option(), _, sizeof(int)))
.WillOnce(Invoke([](int, int, const void* optval, socklen_t) -> Api::SysCallIntResult {
Expand Down Expand Up @@ -3436,6 +3465,29 @@ class TcpKeepaliveTest : public ClusterManagerImplTest {
EXPECT_EQ(connection_, conn_data.connection_.get());
}

void expectOnlyNoSigpipeOptions() {
NiceMock<Network::MockConnectionSocket> socket;
EXPECT_CALL(factory_.tls_.dispatcher_, createClientConnection_(_, _, _, _))
.WillOnce(Invoke([this, &socket](Network::Address::InstanceConstSharedPtr,
Network::Address::InstanceConstSharedPtr,
Network::TransportSocketPtr&,
const Network::ConnectionSocket::OptionsSharedPtr& options)
-> Network::ClientConnection* {
EXPECT_NE(nullptr, options.get());
EXPECT_TRUE((Network::Socket::applyOptions(
options, socket, envoy::config::core::v3::SocketOption::STATE_PREBIND)));
return connection_;
}));
EXPECT_CALL(socket, setSocketOption(ENVOY_SOCKET_SO_NOSIGPIPE.level(),
ENVOY_SOCKET_SO_NOSIGPIPE.option(), _, sizeof(int)))
.WillOnce(Invoke([](int, int, const void* optval, socklen_t) -> Api::SysCallIntResult {
EXPECT_EQ(1, *static_cast<const int*>(optval));
return {0, 0};
}));
auto conn_data = cluster_manager_->tcpConnForCluster("TcpKeepaliveCluster", nullptr);
EXPECT_EQ(connection_, conn_data.connection_.get());
}

void expectNoSocketOptions() {
EXPECT_CALL(factory_.tls_.dispatcher_, createClientConnection_(_, _, _, _))
.WillOnce(
Expand Down Expand Up @@ -3472,7 +3524,11 @@ TEST_F(TcpKeepaliveTest, TcpKeepaliveUnset) {
port_value: 11001
)EOF";
initialize(yaml);
expectNoSocketOptions();
if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) {
expectOnlyNoSigpipeOptions();
} else {
expectNoSocketOptions();
}
}

TEST_F(TcpKeepaliveTest, TcpKeepaliveCluster) {
Expand Down