Add mode to PipeInstance (envoyproxy#8423)

athampy · mattklein123 · commit 750431ad659c · 2019-12-16T17:07:15.000-08:00
Signed-off-by: Akhil Thampy &lt;akhilthampy@yahoo.com&gt;
diff --git a/api/envoy/api/v2/core/address.proto b/api/envoy/api/v2/core/address.proto
@@ -20,6 +20,9 @@ message Pipe {
   // Paths starting with '@' will result in an error in environments other than
   // Linux.
   string path = 1 [(validate.rules).string = {min_bytes: 1}];
+
+  // The mode for the Pipe. Not applicable for abstract sockets.
+  uint32 mode = 2 [(validate.rules).uint32 = {lte: 511}];
 }
 
 // [#next-free-field: 7]
diff --git a/api/envoy/api/v3alpha/core/address.proto b/api/envoy/api/v3alpha/core/address.proto
@@ -24,6 +24,9 @@ message Pipe {
   // Paths starting with '@' will result in an error in environments other than
   // Linux.
   string path = 1 [(validate.rules).string = {min_bytes: 1}];
+
+  // The mode for the Pipe. Not applicable for abstract sockets.
+  uint32 mode = 2 [(validate.rules).uint32 = {lte: 511}];
 }
 
 // [#next-free-field: 7]
diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -6,6 +6,7 @@ Version history
 * access log: added FILTER_STATE :ref:`access log formatters <config_access_log_format>` and gRPC access logger.
 * access log: added a :ref:`typed JSON logging mode <config_access_log_format_dictionaries>` to output access logs in JSON format with non-string values
 * api: remove all support for v1
+* api: added ability to specify `mode` for :ref:`Pipe <envoy_api_field_core.Pipe.mode>`.
 * buffer: remove old implementation
 * build: official released binary is now built against libc++.
 * cluster: added :ref: `aggregate cluster <arch_overview_aggregate_cluster>` that allows load balancing between clusters.
diff --git a/include/envoy/api/os_sys_calls.h b/include/envoy/api/os_sys_calls.h
@@ -21,6 +21,11 @@ class OsSysCalls {
    */
   virtual SysCallIntResult bind(int sockfd, const sockaddr* addr, socklen_t addrlen) PURE;
 
+  /**
+   * @see chmod (man 2 chmod)
+   */
+  virtual SysCallIntResult chmod(const std::string& path, mode_t mode) PURE;
+
   /**
    * @see ioctl (man 2 ioctl)
    */
diff --git a/include/envoy/common/platform.h b/include/envoy/common/platform.h
@@ -48,6 +48,8 @@ typedef unsigned int sa_family_t;
 #include <sys/socket.h>
 #include <sys/uio.h> // for iovec
 #include <sys/un.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 #include <unistd.h>
 
 #if defined(__linux__)
diff --git a/source/common/api/os_sys_calls_impl.cc b/source/common/api/os_sys_calls_impl.cc
@@ -5,6 +5,7 @@
 #include <unistd.h>
 
 #include <cerrno>
+#include <string>
 
 namespace Envoy {
 namespace Api {
@@ -14,6 +15,11 @@ SysCallIntResult OsSysCallsImpl::bind(int sockfd, const sockaddr* addr, socklen_
   return {rc, errno};
 }
 
+SysCallIntResult OsSysCallsImpl::chmod(const std::string& path, mode_t mode) {
+  const int rc = ::chmod(path.c_str(), mode);
+  return {rc, errno};
+}
+
 SysCallIntResult OsSysCallsImpl::ioctl(int sockfd, unsigned long int request, void* argp) {
   const int rc = ::ioctl(sockfd, request, argp);
   return {rc, errno};
diff --git a/source/common/api/os_sys_calls_impl.h b/source/common/api/os_sys_calls_impl.h
@@ -1,5 +1,7 @@
 #pragma once
 
+#include <string>
+
 #include "envoy/api/os_sys_calls.h"
 
 #include "common/singleton/threadsafe_singleton.h"
@@ -11,6 +13,7 @@ class OsSysCallsImpl : public OsSysCalls {
 public:
   // Api::OsSysCalls
   SysCallIntResult bind(int sockfd, const sockaddr* addr, socklen_t addrlen) override;
+  SysCallIntResult chmod(const std::string& path, mode_t mode) override;
   SysCallIntResult ioctl(int sockfd, unsigned long int request, void* argp) override;
   SysCallSizeResult writev(int fd, const iovec* iovec, int num_iovec) override;
   SysCallSizeResult readv(int fd, const iovec* iovec, int num_iovec) override;
diff --git a/source/common/network/address_impl.cc b/source/common/network/address_impl.cc
@@ -332,7 +332,7 @@ IoHandlePtr Ipv6Instance::socket(SocketType type) const {
   return io_handle;
 }
 
-PipeInstance::PipeInstance(const sockaddr_un* address, socklen_t ss_len)
+PipeInstance::PipeInstance(const sockaddr_un* address, socklen_t ss_len, mode_t mode)
     : InstanceBase(Type::Pipe) {
   if (address->sun_path[0] == '\0') {
 #if !defined(__linux__)
@@ -345,15 +345,19 @@ PipeInstance::PipeInstance(const sockaddr_un* address, socklen_t ss_len)
   }
   address_ = *address;
   if (abstract_namespace_) {
+    if (mode != 0) {
+      throw EnvoyException("Cannot set mode for Abstract AF_UNIX sockets");
+    }
     // Replace all null characters with '@' in friendly_name_.
     friendly_name_ =
         friendlyNameFromAbstractPath(absl::string_view(address_.sun_path, address_length_));
   } else {
     friendly_name_ = address->sun_path;
   }
+  this->mode = mode;
 }
 
-PipeInstance::PipeInstance(const std::string& pipe_path) : InstanceBase(Type::Pipe) {
+PipeInstance::PipeInstance(const std::string& pipe_path, mode_t mode) : InstanceBase(Type::Pipe) {
   if (pipe_path.size() >= sizeof(address_.sun_path)) {
     throw EnvoyException(
         fmt::format("Path \"{}\" exceeds maximum UNIX domain socket path size of {}.", pipe_path,
@@ -370,6 +374,9 @@ PipeInstance::PipeInstance(const std::string& pipe_path) : InstanceBase(Type::Pi
 #if !defined(__linux__)
     throw EnvoyException("Abstract AF_UNIX sockets are only supported on linux.");
 #endif
+    if (mode != 0) {
+      throw EnvoyException("Cannot set mode for Abstract AF_UNIX sockets");
+    }
     abstract_namespace_ = true;
     address_length_ = pipe_path.size();
     memcpy(&address_.sun_path[0], pipe_path.data(), pipe_path.size());
@@ -385,6 +392,7 @@ PipeInstance::PipeInstance(const std::string& pipe_path) : InstanceBase(Type::Pi
     StringUtil::strlcpy(&address_.sun_path[0], pipe_path.c_str(), sizeof(address_.sun_path));
     friendly_name_ = address_.sun_path;
   }
+  this->mode = mode;
 }
 
 bool PipeInstance::operator==(const Instance& rhs) const { return asString() == rhs.asString(); }
@@ -397,7 +405,14 @@ Api::SysCallIntResult PipeInstance::bind(int fd) const {
     unlink(address_.sun_path);
   }
   auto& os_syscalls = Api::OsSysCallsSingleton::get();
-  return os_syscalls.bind(fd, sockAddr(), sockAddrLen());
+  auto bind_result = os_syscalls.bind(fd, sockAddr(), sockAddrLen());
+  if (mode != 0 && !abstract_namespace_ && bind_result.rc_ == 0) {
+    auto set_permissions = os_syscalls.chmod(address_.sun_path, mode);
+    if (set_permissions.rc_ != 0) {
+      throw EnvoyException(fmt::format("Failed to create socket with mode {}", mode));
+    }
+  }
+  return bind_result;
 }
 
 Api::SysCallIntResult PipeInstance::connect(int fd) const {
diff --git a/source/common/network/address_impl.h b/source/common/network/address_impl.h
@@ -228,12 +228,12 @@ class PipeInstance : public InstanceBase {
   /**
    * Construct from an existing unix address.
    */
-  explicit PipeInstance(const sockaddr_un* address, socklen_t ss_len);
+  explicit PipeInstance(const sockaddr_un* address, socklen_t ss_len, mode_t mode = 0);
 
   /**
    * Construct from a string pipe path.
    */
-  explicit PipeInstance(const std::string& pipe_path);
+  explicit PipeInstance(const std::string& pipe_path, mode_t mode = 0);
 
   // Network::Address::Instance
   bool operator==(const Instance& rhs) const override;
@@ -256,6 +256,7 @@ class PipeInstance : public InstanceBase {
   // For abstract namespaces.
   bool abstract_namespace_{false};
   uint32_t address_length_{0};
+  mode_t mode{0};
 };
 
 } // namespace Address
diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc
@@ -455,7 +455,8 @@ Utility::protobufAddressToAddress(const envoy::api::v2::core::Address& proto_add
                                          proto_address.socket_address().port_value(),
                                          !proto_address.socket_address().ipv4_compat());
   case envoy::api::v2::core::Address::kPipe:
-    return std::make_shared<Address::PipeInstance>(proto_address.pipe().path());
+    return std::make_shared<Address::PipeInstance>(proto_address.pipe().path(),
+                                                   proto_address.pipe().mode());
   default:
     NOT_REACHED_GCOVR_EXCL_LINE;
   }
diff --git a/test/common/network/BUILD b/test/common/network/BUILD
@@ -35,8 +35,10 @@ envoy_cc_test(
     deps = [
         "//source/common/network:address_lib",
         "//source/common/network:utility_lib",
+        "//test/mocks/api:api_mocks",
         "//test/test_common:environment_lib",
         "//test/test_common:network_utility_lib",
+        "//test/test_common:threadsafe_singleton_injector_lib",
         "//test/test_common:utility_lib",
     ],
 )
diff --git a/test/common/network/address_impl_test.cc b/test/common/network/address_impl_test.cc
@@ -5,17 +5,24 @@
 #include "envoy/common/exception.h"
 #include "envoy/common/platform.h"
 
+#include "common/api/os_sys_calls_impl.h"
 #include "common/common/fmt.h"
 #include "common/common/utility.h"
 #include "common/network/address_impl.h"
 #include "common/network/utility.h"
 
+#include "test/mocks/api/mocks.h"
 #include "test/test_common/environment.h"
 #include "test/test_common/network_utility.h"
+#include "test/test_common/threadsafe_singleton_injector.h"
 #include "test/test_common/utility.h"
 
 #include "gtest/gtest.h"
 
+using testing::_;
+using testing::NiceMock;
+using testing::Return;
+
 namespace Envoy {
 namespace Network {
 namespace Address {
@@ -312,6 +319,63 @@ TEST(PipeInstanceTest, Basic) {
   EXPECT_EQ(nullptr, address.ip());
 }
 
+TEST(PipeInstanceTest, BasicPermission) {
+  std::string path = TestEnvironment::unixDomainSocketPath("foo.sock");
+
+  const mode_t mode = 0777;
+  PipeInstance address(path, mode);
+
+  IoHandlePtr io_handle = address.socket(SocketType::Stream);
+  ASSERT_GE(io_handle->fd(), 0) << address.asString();
+
+  Api::SysCallIntResult result = address.bind(io_handle->fd());
+  ASSERT_EQ(result.rc_, 0) << address.asString() << "\nerror: " << strerror(result.errno_)
+                           << "\terrno: " << result.errno_;
+
+  Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get();
+  struct stat stat_buf;
+  result = os_sys_calls.stat(path.c_str(), &stat_buf);
+  EXPECT_EQ(result.rc_, 0);
+  // Get file permissions bits
+  ASSERT_EQ(stat_buf.st_mode & 07777, mode)
+      << path << std::oct << "\t" << (stat_buf.st_mode & 07777) << std::dec << "\t"
+      << (stat_buf.st_mode) << strerror(result.errno_);
+}
+
+TEST(PipeInstanceTest, PermissionFail) {
+  NiceMock<Api::MockOsSysCalls> os_sys_calls;
+  TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls(&os_sys_calls);
+  std::string path = TestEnvironment::unixDomainSocketPath("foo.sock");
+
+  const mode_t mode = 0777;
+  PipeInstance address(path, mode);
+
+  IoHandlePtr io_handle = address.socket(SocketType::Stream);
+  ASSERT_GE(io_handle->fd(), 0) << address.asString();
+  EXPECT_CALL(os_sys_calls, bind(_, _, _)).WillOnce(Return(Api::SysCallIntResult{0, 0}));
+  EXPECT_CALL(os_sys_calls, chmod(_, _)).WillOnce(Return(Api::SysCallIntResult{-1, 0}));
+  EXPECT_THROW_WITH_REGEX(address.bind(io_handle->fd()), EnvoyException,
+                          "Failed to create socket with mode");
+}
+
+TEST(PipeInstanceTest, AbstractNamespacePermission) {
+#if defined(__linux__)
+  std::string path = "@/foo";
+  const mode_t mode = 0777;
+  EXPECT_THROW_WITH_REGEX(PipeInstance address(path, mode), EnvoyException,
+                          "Cannot set mode for Abstract AF_UNIX sockets");
+
+  sockaddr_un sun;
+  sun.sun_family = AF_UNIX;
+  StringUtil::strlcpy(&sun.sun_path[1], path.data(), path.size());
+  sun.sun_path[0] = '\0';
+  socklen_t ss_len = offsetof(struct sockaddr_un, sun_path) + 1 + strlen(sun.sun_path);
+
+  EXPECT_THROW_WITH_REGEX(PipeInstance address(&sun, ss_len, mode), EnvoyException,
+                          "Cannot set mode for Abstract AF_UNIX sockets");
+#endif
+}
+
 TEST(PipeInstanceTest, AbstractNamespace) {
 #if defined(__linux__)
   PipeInstance address("@/foo");
@@ -499,7 +563,6 @@ struct TestCase test_cases[] = {
 INSTANTIATE_TEST_SUITE_P(AddressCrossProduct, MixedAddressTest,
                          ::testing::Combine(::testing::ValuesIn(test_cases),
                                             ::testing::ValuesIn(test_cases)));
-
 } // namespace Address
 } // namespace Network
 } // namespace Envoy
diff --git a/test/mocks/api/mocks.h b/test/mocks/api/mocks.h
@@ -70,6 +70,7 @@ class MockOsSysCalls : public OsSysCallsImpl {
   MOCK_METHOD6(mmap, SysCallPtrResult(void* addr, size_t length, int prot, int flags, int fd,
                                       off_t offset));
   MOCK_METHOD2(stat, SysCallIntResult(const char* name, struct stat* stat));
+  MOCK_METHOD2(chmod, SysCallIntResult(const std::string& name, mode_t mode));
   MOCK_METHOD5(setsockopt_,
                int(int sockfd, int level, int optname, const void* optval, socklen_t optlen));
   MOCK_METHOD5(getsockopt_,
diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt
@@ -22,6 +22,7 @@ CDS
 CEL
 CHACHA
 CHLO
+CHMOD
 CHLOS
 CIDR
 CLA

Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,9 @@ message Pipe {`
`20`	`20`	`// Paths starting with '@' will result in an error in environments other than`
`21`	`21`	`// Linux.`
`22`	`22`	`string path = 1 [(validate.rules).string = {min_bytes: 1}];`
	`23`	`+`
	`24`	`+ // The mode for the Pipe. Not applicable for abstract sockets.`
	`25`	`+ uint32 mode = 2 [(validate.rules).uint32 = {lte: 511}];`
`23`	`26`	`}`
`24`	`27`
`25`	`28`	`// [#next-free-field: 7]`
Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,9 @@ message Pipe {`
`24`	`24`	`// Paths starting with '@' will result in an error in environments other than`
`25`	`25`	`// Linux.`
`26`	`26`	`string path = 1 [(validate.rules).string = {min_bytes: 1}];`
	`27`	`+`
	`28`	`+ // The mode for the Pipe. Not applicable for abstract sockets.`
	`29`	`+ uint32 mode = 2 [(validate.rules).uint32 = {lte: 511}];`
`27`	`30`	`}`
`28`	`31`
`29`	`32`	`// [#next-free-field: 7]`