From 147ccbf183a0e21380ed3662bef1a92a56124fbe Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Sat, 28 Sep 2019 12:57:15 -0700 Subject: [PATCH 01/23] syscall: add chmod #5808 Signed-off-by: Akhil Thampy --- include/envoy/api/os_sys_calls.h | 5 +++++ source/common/api/os_sys_calls_impl.cc | 6 ++++++ source/common/api/os_sys_calls_impl.h | 3 +++ 3 files changed, 14 insertions(+) diff --git a/include/envoy/api/os_sys_calls.h b/include/envoy/api/os_sys_calls.h index e2213a0db0a1..0e1d63b200e0 100644 --- a/include/envoy/api/os_sys_calls.h +++ b/include/envoy/api/os_sys_calls.h @@ -28,6 +28,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, int mode) PURE; + /** * @see ioctl (man 2 ioctl) */ diff --git a/source/common/api/os_sys_calls_impl.cc b/source/common/api/os_sys_calls_impl.cc index 75f3f373da07..603440440cde 100644 --- a/source/common/api/os_sys_calls_impl.cc +++ b/source/common/api/os_sys_calls_impl.cc @@ -5,6 +5,7 @@ #include #include +#include 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, int 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 index fa4ae8920401..b635d80b5810 100644 --- 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 + #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, int 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; From e8245dbec77b6d58f2b13b4f80ab5646a08d8c07 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Sat, 28 Sep 2019 10:47:22 -0700 Subject: [PATCH 02/23] api: add mode to Pipe #5808 Signed-off-by: Akhil Thampy --- api/envoy/api/v2/core/address.proto | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/envoy/api/v2/core/address.proto b/api/envoy/api/v2/core/address.proto index 89fd0adb1eb6..5c84602b6f36 100644 --- a/api/envoy/api/v2/core/address.proto +++ b/api/envoy/api/v2/core/address.proto @@ -20,6 +20,8 @@ message Pipe { // Paths starting with '@' will result in an error in environments other than // Linux. string path = 1 [(validate.rules).string = {min_bytes: 1}]; + // [#not-implemented-hide:] + uint32 mode = 2 [(validate.rules).uint32 = {lte: 777}]; } message SocketAddress { From 0715211d57f8bd5dc0aba132a390700708db39a5 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Sat, 28 Sep 2019 13:08:43 -0700 Subject: [PATCH 03/23] address: add mode to PipeInstance #5808 Signed-off-by: Akhil Thampy --- source/common/network/address_impl.cc | 16 +++++++++++++--- source/common/network/address_impl.h | 6 ++++-- source/common/network/utility.cc | 3 ++- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/source/common/network/address_impl.cc b/source/common/network/address_impl.cc index ab5244844c0d..9bb115e76d52 100644 --- a/source/common/network/address_impl.cc +++ b/source/common/network/address_impl.cc @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -329,7 +330,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,9 +346,10 @@ PipeInstance::PipeInstance(const sockaddr_un* address, socklen_t ss_len) abstract_namespace_ ? fmt::format("@{}", absl::string_view(address_.sun_path + 1, address_length_ - 1)) : 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, @@ -365,6 +367,7 @@ PipeInstance::PipeInstance(const std::string& pipe_path) : InstanceBase(Type::Pi address_length_ = strlen(address_.sun_path); address_.sun_path[0] = '\0'; } + this->mode = mode; } bool PipeInstance::operator==(const Instance& rhs) const { return asString() == rhs.asString(); } @@ -377,7 +380,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 and !abstract_namespace_ and 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 index 63e0566ffaaa..da6c141759a1 100644 --- a/source/common/network/address_impl.h +++ b/source/common/network/address_impl.h @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -230,12 +231,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; @@ -258,6 +259,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 index 667d0ccc997f..1ed242affd72 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -469,7 +469,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(proto_address.pipe().path()); + return std::make_shared(proto_address.pipe().path(), + proto_address.pipe().mode()); default: NOT_REACHED_GCOVR_EXCL_LINE; } From ec7ea45b955db1e28e1b16bcecf096c6723c70a8 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Sat, 28 Sep 2019 14:12:12 -0700 Subject: [PATCH 04/23] test: add tests for setting mode for PipeInstance #5808 Signed-off-by: Akhil Thampy --- test/common/network/address_impl_test.cc | 26 +++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/test/common/network/address_impl_test.cc b/test/common/network/address_impl_test.cc index fe4b75217a3d..09c6613852df 100644 --- a/test/common/network/address_impl_test.cc +++ b/test/common/network/address_impl_test.cc @@ -2,6 +2,7 @@ #include #include #include +#include #include #include @@ -11,6 +12,7 @@ #include "envoy/common/exception.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" @@ -318,6 +320,29 @@ TEST(PipeInstanceTest, Basic) { EXPECT_EQ(nullptr, address.ip()); } +TEST(PipeInstanceTest, BasicPermission) { + std::string path = TestEnvironment::unixDomainSocketPath("foo.sock"); + + const mode_t mode = 777; + 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, AbstractNamespace) { #if defined(__linux__) PipeInstance address("@/foo"); @@ -482,7 +507,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 From 39208083199943f47f9fd19836a51b6e049e0550 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Sun, 29 Sep 2019 22:36:30 -0700 Subject: [PATCH 05/23] fix format Signed-off-by: Akhil Thampy --- api/envoy/api/v2/core/address.proto | 1 + 1 file changed, 1 insertion(+) diff --git a/api/envoy/api/v2/core/address.proto b/api/envoy/api/v2/core/address.proto index 5c84602b6f36..c34d2742e27f 100644 --- a/api/envoy/api/v2/core/address.proto +++ b/api/envoy/api/v2/core/address.proto @@ -20,6 +20,7 @@ message Pipe { // Paths starting with '@' will result in an error in environments other than // Linux. string path = 1 [(validate.rules).string = {min_bytes: 1}]; + // [#not-implemented-hide:] uint32 mode = 2 [(validate.rules).uint32 = {lte: 777}]; } From 5c30cf22515864c4b199b1c7c7288d53496a3443 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Sun, 29 Sep 2019 23:14:13 -0700 Subject: [PATCH 06/23] proto sync Signed-off-by: Akhil Thampy --- api/envoy/api/v3alpha/core/address.proto | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/envoy/api/v3alpha/core/address.proto b/api/envoy/api/v3alpha/core/address.proto index 8f72dab8788b..5b962da6f683 100644 --- a/api/envoy/api/v3alpha/core/address.proto +++ b/api/envoy/api/v3alpha/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}]; + + // [#not-implemented-hide:] + uint32 mode = 2 [(validate.rules).uint32 = {lte: 777}]; } message SocketAddress { From 318defbfcec94605fa31320fb51f148abb65c7fe Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Sun, 29 Sep 2019 23:33:57 -0700 Subject: [PATCH 07/23] tools: add chmod to dictionary Signed-off-by: Akhil Thampy --- tools/spelling_dictionary.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index ab5842d2a194..5d8b1dd21b10 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -22,6 +22,7 @@ CEL CDS CHACHA CHLO +CHMOD CIDR CLA CLI From 72dbba49a7867dd3e43ff99d04663406726573f1 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Tue, 1 Oct 2019 22:14:24 -0700 Subject: [PATCH 08/23] add leading zero for mode_t values Signed-off-by: Akhil Thampy --- api/envoy/api/v2/core/address.proto | 2 +- api/envoy/api/v3alpha/core/address.proto | 2 +- test/common/network/address_impl_test.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/envoy/api/v2/core/address.proto b/api/envoy/api/v2/core/address.proto index c34d2742e27f..c2ce02f56ffe 100644 --- a/api/envoy/api/v2/core/address.proto +++ b/api/envoy/api/v2/core/address.proto @@ -22,7 +22,7 @@ message Pipe { string path = 1 [(validate.rules).string = {min_bytes: 1}]; // [#not-implemented-hide:] - uint32 mode = 2 [(validate.rules).uint32 = {lte: 777}]; + uint32 mode = 2 [(validate.rules).uint32 = {lte: 0777}]; } message SocketAddress { diff --git a/api/envoy/api/v3alpha/core/address.proto b/api/envoy/api/v3alpha/core/address.proto index 5b962da6f683..ebf06ae6d2d6 100644 --- a/api/envoy/api/v3alpha/core/address.proto +++ b/api/envoy/api/v3alpha/core/address.proto @@ -22,7 +22,7 @@ message Pipe { string path = 1 [(validate.rules).string = {min_bytes: 1}]; // [#not-implemented-hide:] - uint32 mode = 2 [(validate.rules).uint32 = {lte: 777}]; + uint32 mode = 2 [(validate.rules).uint32 = {lte: 0777}]; } message SocketAddress { diff --git a/test/common/network/address_impl_test.cc b/test/common/network/address_impl_test.cc index 09c6613852df..408ba7c168e9 100644 --- a/test/common/network/address_impl_test.cc +++ b/test/common/network/address_impl_test.cc @@ -323,7 +323,7 @@ TEST(PipeInstanceTest, Basic) { TEST(PipeInstanceTest, BasicPermission) { std::string path = TestEnvironment::unixDomainSocketPath("foo.sock"); - const mode_t mode = 777; + const mode_t mode = 0777; PipeInstance address(path, mode); IoHandlePtr io_handle = address.socket(SocketType::Stream); From 970c88c5c1a543fd04258912d61a9e2a54d5533c Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Thu, 3 Oct 2019 22:45:10 -0700 Subject: [PATCH 09/23] api: remove not-implemented-hide Signed-off-by: Akhil Thampy --- api/envoy/api/v2/core/address.proto | 1 - api/envoy/api/v3alpha/core/address.proto | 1 - 2 files changed, 2 deletions(-) diff --git a/api/envoy/api/v2/core/address.proto b/api/envoy/api/v2/core/address.proto index c2ce02f56ffe..d53f06a14966 100644 --- a/api/envoy/api/v2/core/address.proto +++ b/api/envoy/api/v2/core/address.proto @@ -21,7 +21,6 @@ message Pipe { // Linux. string path = 1 [(validate.rules).string = {min_bytes: 1}]; - // [#not-implemented-hide:] uint32 mode = 2 [(validate.rules).uint32 = {lte: 0777}]; } diff --git a/api/envoy/api/v3alpha/core/address.proto b/api/envoy/api/v3alpha/core/address.proto index ebf06ae6d2d6..8c9cb2e56be6 100644 --- a/api/envoy/api/v3alpha/core/address.proto +++ b/api/envoy/api/v3alpha/core/address.proto @@ -21,7 +21,6 @@ message Pipe { // Linux. string path = 1 [(validate.rules).string = {min_bytes: 1}]; - // [#not-implemented-hide:] uint32 mode = 2 [(validate.rules).uint32 = {lte: 0777}]; } From 23d9f96e16647cdb416624921c81ab446533cf44 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Thu, 3 Oct 2019 22:59:37 -0700 Subject: [PATCH 10/23] api: update validate rules for mode Signed-off-by: Akhil Thampy --- api/envoy/api/v2/core/address.proto | 2 +- api/envoy/api/v3alpha/core/address.proto | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/envoy/api/v2/core/address.proto b/api/envoy/api/v2/core/address.proto index d53f06a14966..6466e7d4b507 100644 --- a/api/envoy/api/v2/core/address.proto +++ b/api/envoy/api/v2/core/address.proto @@ -21,7 +21,7 @@ message Pipe { // Linux. string path = 1 [(validate.rules).string = {min_bytes: 1}]; - uint32 mode = 2 [(validate.rules).uint32 = {lte: 0777}]; + uint32 mode = 2 [(validate.rules).uint32 = {lte: 511}]; } message SocketAddress { diff --git a/api/envoy/api/v3alpha/core/address.proto b/api/envoy/api/v3alpha/core/address.proto index 8c9cb2e56be6..c94c00d07554 100644 --- a/api/envoy/api/v3alpha/core/address.proto +++ b/api/envoy/api/v3alpha/core/address.proto @@ -21,7 +21,7 @@ message Pipe { // Linux. string path = 1 [(validate.rules).string = {min_bytes: 1}]; - uint32 mode = 2 [(validate.rules).uint32 = {lte: 0777}]; + uint32 mode = 2 [(validate.rules).uint32 = {lte: 511}]; } message SocketAddress { From 4a2c94d8347e372a6eff74866c6b13de7b19d938 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Sat, 16 Nov 2019 13:14:12 -0800 Subject: [PATCH 11/23] add docs; use '&&' instead of 'and' for MSVC compatibility Signed-off-by: Akhil Thampy --- api/envoy/api/v2/core/address.proto | 1 + source/common/network/address_impl.cc | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/api/envoy/api/v2/core/address.proto b/api/envoy/api/v2/core/address.proto index 8592b243f424..09c5aaec7571 100644 --- a/api/envoy/api/v2/core/address.proto +++ b/api/envoy/api/v2/core/address.proto @@ -21,6 +21,7 @@ message Pipe { // 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}]; } diff --git a/source/common/network/address_impl.cc b/source/common/network/address_impl.cc index ebd3d5999c12..5ea1f0e3b8c6 100644 --- a/source/common/network/address_impl.cc +++ b/source/common/network/address_impl.cc @@ -406,7 +406,7 @@ Api::SysCallIntResult PipeInstance::bind(int fd) const { } auto& os_syscalls = Api::OsSysCallsSingleton::get(); auto bind_result = os_syscalls.bind(fd, sockAddr(), sockAddrLen()); - if (mode != 0 and !abstract_namespace_ and bind_result.rc_ == 0) { + 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)); From fc7182a8644551df66992267da051e889280dc2c Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Mon, 18 Nov 2019 15:01:55 -0800 Subject: [PATCH 12/23] fix format Signed-off-by: Akhil Thampy --- api/envoy/api/v3alpha/core/address.proto | 1 + 1 file changed, 1 insertion(+) diff --git a/api/envoy/api/v3alpha/core/address.proto b/api/envoy/api/v3alpha/core/address.proto index 4388a52fe1fc..206aa5978f82 100644 --- a/api/envoy/api/v3alpha/core/address.proto +++ b/api/envoy/api/v3alpha/core/address.proto @@ -21,6 +21,7 @@ message Pipe { // 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}]; } From ba59ed5dcb6580878c25c93fa6041162b4e57797 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Tue, 19 Nov 2019 10:06:41 -0800 Subject: [PATCH 13/23] network: throw exception if mode is set for abstract sockets; remove unnecessary include; syscall: use mode_t for chmod Signed-off-by: Akhil Thampy --- include/envoy/api/os_sys_calls.h | 2 +- source/common/api/os_sys_calls_impl.cc | 2 +- source/common/api/os_sys_calls_impl.h | 2 +- source/common/network/address_impl.cc | 6 ++++++ source/common/network/address_impl.h | 1 - 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/include/envoy/api/os_sys_calls.h b/include/envoy/api/os_sys_calls.h index 940524fdc115..6291ae181695 100644 --- a/include/envoy/api/os_sys_calls.h +++ b/include/envoy/api/os_sys_calls.h @@ -24,7 +24,7 @@ class OsSysCalls { /** * @see chmod (man 2 chmod) */ - virtual SysCallIntResult chmod(const std::string& path, int mode) PURE; + virtual SysCallIntResult chmod(const std::string& path, mode_t mode) PURE; /** * @see ioctl (man 2 ioctl) diff --git a/source/common/api/os_sys_calls_impl.cc b/source/common/api/os_sys_calls_impl.cc index d1a34a7a026c..5bf163d05b66 100644 --- a/source/common/api/os_sys_calls_impl.cc +++ b/source/common/api/os_sys_calls_impl.cc @@ -15,7 +15,7 @@ SysCallIntResult OsSysCallsImpl::bind(int sockfd, const sockaddr* addr, socklen_ return {rc, errno}; } -SysCallIntResult OsSysCallsImpl::chmod(const std::string& path, int mode) { +SysCallIntResult OsSysCallsImpl::chmod(const std::string& path, mode_t mode) { const int rc = ::chmod(path.c_str(), mode); return {rc, errno}; } diff --git a/source/common/api/os_sys_calls_impl.h b/source/common/api/os_sys_calls_impl.h index 46b3be5bde58..69c20e84c8d5 100644 --- a/source/common/api/os_sys_calls_impl.h +++ b/source/common/api/os_sys_calls_impl.h @@ -13,7 +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, int mode) 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 index 5ea1f0e3b8c6..8b9b0f7a28b3 100644 --- a/source/common/network/address_impl.cc +++ b/source/common/network/address_impl.cc @@ -351,6 +351,9 @@ PipeInstance::PipeInstance(const sockaddr_un* address, socklen_t ss_len, mode_t } 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_)); @@ -377,6 +380,9 @@ PipeInstance::PipeInstance(const std::string& pipe_path, mode_t mode) : Instance #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()); diff --git a/source/common/network/address_impl.h b/source/common/network/address_impl.h index efabe1307650..5b7ab5e72de3 100644 --- a/source/common/network/address_impl.h +++ b/source/common/network/address_impl.h @@ -1,6 +1,5 @@ #pragma once -#include #include #include #include From 2f1862ca36aa20cc261c0daeabcdb518995a3a23 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Mon, 25 Nov 2019 05:01:48 -0800 Subject: [PATCH 14/23] fix format Signed-off-by: Akhil Thampy --- source/common/network/address_impl.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/common/network/address_impl.cc b/source/common/network/address_impl.cc index 8b9b0f7a28b3..f886d4fc61bf 100644 --- a/source/common/network/address_impl.cc +++ b/source/common/network/address_impl.cc @@ -351,8 +351,8 @@ PipeInstance::PipeInstance(const sockaddr_un* address, socklen_t ss_len, mode_t } address_ = *address; if (abstract_namespace_) { - if(mode != 0){ - throw EnvoyException("Cannot set mode for Abstract AF_UNIX sockets"); + if (mode != 0) { + throw EnvoyException("Cannot set mode for Abstract AF_UNIX sockets"); } // Replace all null characters with '@' in friendly_name_. friendly_name_ = @@ -380,8 +380,8 @@ PipeInstance::PipeInstance(const std::string& pipe_path, mode_t mode) : Instance #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"); + if (mode != 0) { + throw EnvoyException("Cannot set mode for Abstract AF_UNIX sockets"); } abstract_namespace_ = true; address_length_ = pipe_path.size(); From 1ad0a2ce1062fef447f92f7ab2e305cc301a7813 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Tue, 26 Nov 2019 10:39:14 -0800 Subject: [PATCH 15/23] add test for abstract PipeInstance with mode Signed-off-by: Akhil Thampy --- test/common/network/address_impl_test.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/common/network/address_impl_test.cc b/test/common/network/address_impl_test.cc index b42497655444..bc00e736a49a 100644 --- a/test/common/network/address_impl_test.cc +++ b/test/common/network/address_impl_test.cc @@ -344,6 +344,14 @@ TEST(PipeInstanceTest, BasicPermission) { << (stat_buf.st_mode) << strerror(result.errno_); } +TEST(PipeInstanceTest, AbstractNamespacePermission) { +#if defined(__linux__) + const mode_t mode = 0777; + EXPECT_THROW_WITH_REGEX(PipeInstance address("@/foo", mode), EnvoyException, + "Cannot set mode for Abstract AF_UNIX sockets"); +#endif +} + TEST(PipeInstanceTest, AbstractNamespace) { #if defined(__linux__) PipeInstance address("@/foo"); From aa6f82beb1894dc07aafa143a12edd85969c2514 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Tue, 26 Nov 2019 13:55:05 -0800 Subject: [PATCH 16/23] remove unnecessary headers Signed-off-by: Akhil Thampy --- source/common/network/address_impl.cc | 1 - test/common/network/address_impl_test.cc | 1 - 2 files changed, 2 deletions(-) diff --git a/source/common/network/address_impl.cc b/source/common/network/address_impl.cc index f886d4fc61bf..70b786ef080b 100644 --- a/source/common/network/address_impl.cc +++ b/source/common/network/address_impl.cc @@ -1,6 +1,5 @@ #include "common/network/address_impl.h" -#include #include #include #include diff --git a/test/common/network/address_impl_test.cc b/test/common/network/address_impl_test.cc index bc00e736a49a..0414d4921128 100644 --- a/test/common/network/address_impl_test.cc +++ b/test/common/network/address_impl_test.cc @@ -1,4 +1,3 @@ -#include #include #include #include From 6a735c3a18d3636db0e637660fb8a5e5e06b5b71 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Wed, 27 Nov 2019 14:02:29 -0800 Subject: [PATCH 17/23] move platform specific includes to platform.h Signed-off-by: Akhil Thampy --- include/envoy/common/platform.h | 2 ++ source/common/network/address_impl.cc | 5 ----- source/common/network/address_impl.h | 2 -- test/common/network/address_impl_test.cc | 7 ------- 4 files changed, 2 insertions(+), 14 deletions(-) diff --git a/include/envoy/common/platform.h b/include/envoy/common/platform.h index 7656d3feb161..cfb8c3bda3c6 100644 --- a/include/envoy/common/platform.h +++ b/include/envoy/common/platform.h @@ -48,6 +48,8 @@ typedef unsigned int sa_family_t; #include #include // for iovec #include +#include +#include #include #if defined(__linux__) diff --git a/source/common/network/address_impl.cc b/source/common/network/address_impl.cc index 70b786ef080b..70ea9faf66b0 100644 --- a/source/common/network/address_impl.cc +++ b/source/common/network/address_impl.cc @@ -1,10 +1,5 @@ #include "common/network/address_impl.h" -#include -#include -#include -#include - #include #include #include diff --git a/source/common/network/address_impl.h b/source/common/network/address_impl.h index 5b7ab5e72de3..f14440b1d502 100644 --- a/source/common/network/address_impl.h +++ b/source/common/network/address_impl.h @@ -1,7 +1,5 @@ #pragma once -#include -#include #include #include diff --git a/test/common/network/address_impl_test.cc b/test/common/network/address_impl_test.cc index 0414d4921128..5d11eafe3651 100644 --- a/test/common/network/address_impl_test.cc +++ b/test/common/network/address_impl_test.cc @@ -1,10 +1,3 @@ -#include -#include -#include -#include -#include -#include - #include #include #include From fd10e2e637d99b9cc1b34c51f3c25b30ddc0b430 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Tue, 10 Dec 2019 22:57:53 -0800 Subject: [PATCH 18/23] docs: add release notes for Pipe::mode Signed-off-by: Akhil Thampy --- docs/root/intro/version_history.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 3ecee6399aca..849ccaaef0ff 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -23,6 +23,7 @@ Version history * thrift_proxy: added support for cluster header based routing. * tls: remove TLS 1.0 and 1.1 from client defaults * router: exposed DOWNSTREAM_REMOTE_ADDRESS as custom HTTP request/response headers. +* api: added ability to specify `mode` for :ref:`Pipe `. 1.12.0 (October 31, 2019) ========================= From 342c394125878a4e713dfb34024d8d192cd4ad20 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Wed, 11 Dec 2019 10:30:33 -0800 Subject: [PATCH 19/23] test: add test for Pipe chmod fail; create mock for chmod syscall Signed-off-by: Akhil Thampy --- docs/root/intro/version_history.rst | 2 +- test/common/network/BUILD | 2 ++ test/common/network/address_impl_test.cc | 22 ++++++++++++++++++++++ test/mocks/api/mocks.h | 1 + 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 1e804c25c1a6..a4688b23a2fe 100644 --- 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 ` and gRPC access logger. * access log: added a :ref:`typed JSON logging mode ` 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 `. * buffer: remove old implementation * build: official released binary is now built against libc++. * cluster: added :ref: `aggregate cluster ` that allows load balancing between clusters. @@ -33,7 +34,6 @@ Version history * thrift_proxy: added stats to the router filter. * tls: remove TLS 1.0 and 1.1 from client defaults * router: exposed DOWNSTREAM_REMOTE_ADDRESS as custom HTTP request/response headers. -* api: added ability to specify `mode` for :ref:`Pipe `. * tracing: added the ability to set custom tags on both the :ref:`HTTP connection manager` and the :ref:`HTTP route `. * tracing: added upstream_address tag. * udp: added initial support for :ref:`UDP proxy ` diff --git a/test/common/network/BUILD b/test/common/network/BUILD index ac397390c95e..f8362d067ffc 100644 --- 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 index 5d11eafe3651..f28b00c4edcd 100644 --- a/test/common/network/address_impl_test.cc +++ b/test/common/network/address_impl_test.cc @@ -11,12 +11,18 @@ #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 { @@ -336,6 +342,22 @@ TEST(PipeInstanceTest, BasicPermission) { << (stat_buf.st_mode) << strerror(result.errno_); } +TEST(PipeInstanceTest, PermissionFail) { + NiceMock os_sys_calls; + TestThreadsafeSingletonInjector 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__) const mode_t mode = 0777; diff --git a/test/mocks/api/mocks.h b/test/mocks/api/mocks.h index ef1e9b128dbd..8e0df1924b30 100644 --- 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_, From 8a32d7849f93fa5995ac51cb847de42af3a4a25c Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Thu, 12 Dec 2019 20:47:30 -0800 Subject: [PATCH 20/23] docs: revert Signed-off-by: Akhil Thampy --- docs/root/intro/version_history.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index a4688b23a2fe..e4ef72578198 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -33,7 +33,6 @@ Version history * thrift_proxy: added support for cluster header based routing. * thrift_proxy: added stats to the router filter. * tls: remove TLS 1.0 and 1.1 from client defaults -* router: exposed DOWNSTREAM_REMOTE_ADDRESS as custom HTTP request/response headers. * tracing: added the ability to set custom tags on both the :ref:`HTTP connection manager` and the :ref:`HTTP route `. * tracing: added upstream_address tag. * udp: added initial support for :ref:`UDP proxy ` From ff16f6dba5bfab780c16a9bdbd068c3b35523e47 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Fri, 13 Dec 2019 05:29:46 -0800 Subject: [PATCH 21/23] Kick CI Signed-off-by: Akhil Thampy From 0b72b99adabcf02604309a929d52d9c199a70079 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Sat, 14 Dec 2019 18:31:49 -0800 Subject: [PATCH 22/23] fix coverage Signed-off-by: Akhil Thampy --- test/common/network/address_impl_test.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/common/network/address_impl_test.cc b/test/common/network/address_impl_test.cc index f28b00c4edcd..95d80fae2b22 100644 --- a/test/common/network/address_impl_test.cc +++ b/test/common/network/address_impl_test.cc @@ -363,6 +363,15 @@ TEST(PipeInstanceTest, AbstractNamespacePermission) { const mode_t mode = 0777; EXPECT_THROW_WITH_REGEX(PipeInstance address("@/foo", mode), EnvoyException, "Cannot set mode for Abstract AF_UNIX sockets"); + + sockaddr_un sun; + sun.sun_family = AF_UNIX; + StringUtil::strlcpy(&sun.sun_path[1], "@/foo", sizeof sun.sun_path); + 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 } From 818e6b159126395d628bd5a86a7b43fd0e782287 Mon Sep 17 00:00:00 2001 From: Akhil Thampy Date: Mon, 16 Dec 2019 19:05:34 +0000 Subject: [PATCH 23/23] fix asan off by one Signed-off-by: Akhil Thampy --- test/common/network/address_impl_test.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/common/network/address_impl_test.cc b/test/common/network/address_impl_test.cc index 95d80fae2b22..88258a1a1cce 100644 --- a/test/common/network/address_impl_test.cc +++ b/test/common/network/address_impl_test.cc @@ -360,13 +360,14 @@ TEST(PipeInstanceTest, PermissionFail) { TEST(PipeInstanceTest, AbstractNamespacePermission) { #if defined(__linux__) + std::string path = "@/foo"; const mode_t mode = 0777; - EXPECT_THROW_WITH_REGEX(PipeInstance address("@/foo", mode), EnvoyException, + 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], "@/foo", sizeof sun.sun_path); + 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);