network: catch SIGPIPEs via SO_NOSIGPIPE on Darwin (envoyproxy#12039)

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Co-authored-by: Jose Nino <jnino@lyft.com>
Signed-off-by: scheler <santosh.cheler@appdynamics.com>

source/common/network/socket_option_factory.cc

@@ -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>();

source/common/network/socket_option_factory.h

@@ -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);

source/common/network/socket_option_impl.h

@@ -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

source/common/upstream/BUILD

@@ -440,6 +440,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",

source/common/upstream/upstream_impl.cc

@@ -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"
@@ -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()) {

source/server/BUILD

@@ -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",

source/server/listener_impl.cc

@@ -17,6 +17,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"
@@ -372,6 +373,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());
   }

test/common/upstream/cluster_manager_impl_test.cc

@@ -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:
@@ -3125,6 +3125,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
@@ -3137,8 +3139,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;
@@ -3181,8 +3182,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);
   }
 
@@ -3222,7 +3230,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) {
@@ -3325,8 +3337,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);
 }
 
@@ -3354,8 +3369,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);
 }
 
@@ -3385,8 +3403,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);
 }
 
@@ -3435,6 +3456,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 {
@@ -3472,6 +3501,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(
@@ -3508,7 +3560,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) {