Commit 404ec514 authored by sammc's avatar sammc Committed by Commit bot

Allow custom security descriptors when creating named pipes on Windows.

Remoting requires control over the security descriptor used for its
named pipes. This adds this as an option when creating server handles
from named pipes on Windows.

BUG=604282

Review-Url: https://codereview.chromium.org/2444793002
Cr-Commit-Position: refs/heads/master@{#427266}
parent fea82c13
...@@ -16,7 +16,7 @@ UnixDomainSocketAcceptor::UnixDomainSocketAcceptor(const base::FilePath& path, ...@@ -16,7 +16,7 @@ UnixDomainSocketAcceptor::UnixDomainSocketAcceptor(const base::FilePath& path,
Delegate* delegate) Delegate* delegate)
: named_pipe_(path.value()), : named_pipe_(path.value()),
delegate_(delegate), delegate_(delegate),
listen_handle_(mojo::edk::CreateServerHandle(named_pipe_, false)) { listen_handle_(mojo::edk::CreateServerHandle(named_pipe_)) {
DCHECK(delegate_); DCHECK(delegate_);
} }
......
...@@ -464,7 +464,7 @@ base::Process CloudPrintProxyPolicyStartupTest::Launch( ...@@ -464,7 +464,7 @@ base::Process CloudPrintProxyPolicyStartupTest::Launch(
base::RandInt(0, std::numeric_limits<int>::max()))); base::RandInt(0, std::numeric_limits<int>::max())));
startup_channel_ = IPC::ChannelProxy::Create( startup_channel_ = IPC::ChannelProxy::Create(
mojo::edk::ConnectToPeerProcess( mojo::edk::ConnectToPeerProcess(
mojo::edk::CreateServerHandle(startup_channel_handle_, false)) mojo::edk::CreateServerHandle(startup_channel_handle_))
.release(), .release(),
IPC::Channel::MODE_SERVER, this, IOTaskRunner()); IPC::Channel::MODE_SERVER, this, IOTaskRunner());
......
...@@ -131,7 +131,7 @@ mojo::edk::ScopedPlatformHandle CreateServerHandle( ...@@ -131,7 +131,7 @@ mojo::edk::ScopedPlatformHandle CreateServerHandle(
return mojo::edk::ScopedPlatformHandle(platform_handle); return mojo::edk::ScopedPlatformHandle(platform_handle);
#else #else
return mojo::edk::CreateServerHandle( return mojo::edk::CreateServerHandle(
mojo::edk::NamedPlatformHandle(channel_handle.name), false); mojo::edk::NamedPlatformHandle(channel_handle.name));
#endif #endif
} }
#endif #endif
...@@ -334,7 +334,9 @@ mojo::ScopedMessagePipeHandle ServiceProcess::CreateChannelMessagePipe() { ...@@ -334,7 +334,9 @@ mojo::ScopedMessagePipeHandle ServiceProcess::CreateChannelMessagePipe() {
#if defined(OS_POSIX) #if defined(OS_POSIX)
channel_handle = mojo::edk::DuplicatePlatformHandle(server_handle_.get()); channel_handle = mojo::edk::DuplicatePlatformHandle(server_handle_.get());
#elif defined(OS_WIN) #elif defined(OS_WIN)
channel_handle = mojo::edk::CreateServerHandle(server_handle_, false); mojo::edk::CreateServerHandleOptions options;
options.enforce_uniqueness = false;
channel_handle = mojo::edk::CreateServerHandle(server_handle_, options);
#endif #endif
CHECK(channel_handle.is_valid()); CHECK(channel_handle.is_valid());
......
...@@ -28,7 +28,16 @@ namespace edk { ...@@ -28,7 +28,16 @@ namespace edk {
// resolve it into a client handle. // resolve it into a client handle.
class MOJO_SYSTEM_IMPL_EXPORT NamedPlatformChannelPair { class MOJO_SYSTEM_IMPL_EXPORT NamedPlatformChannelPair {
public: public:
NamedPlatformChannelPair(); struct Options {
#if defined(OS_WIN)
// If non-empty, a security descriptor to use when creating the pipe. If
// empty, a default security descriptor will be used. See
// kDefaultSecurityDescriptor in named_platform_handle_utils_win.cc.
base::string16 security_descriptor;
#endif
};
NamedPlatformChannelPair(const Options& options = {});
~NamedPlatformChannelPair(); ~NamedPlatformChannelPair();
// Note: It is NOT acceptable to use this handle as a generic pipe channel. It // Note: It is NOT acceptable to use this handle as a generic pipe channel. It
......
...@@ -35,9 +35,13 @@ std::wstring GeneratePipeName() { ...@@ -35,9 +35,13 @@ std::wstring GeneratePipeName() {
} // namespace } // namespace
NamedPlatformChannelPair::NamedPlatformChannelPair() NamedPlatformChannelPair::NamedPlatformChannelPair(
const NamedPlatformChannelPair::Options& options)
: pipe_handle_(GeneratePipeName()) { : pipe_handle_(GeneratePipeName()) {
server_handle_ = CreateServerHandle(pipe_handle_, true); CreateServerHandleOptions server_handle_options;
server_handle_options.security_descriptor = options.security_descriptor;
server_handle_options.enforce_uniqueness = true;
server_handle_ = CreateServerHandle(pipe_handle_, server_handle_options);
PCHECK(server_handle_.is_valid()); PCHECK(server_handle_.is_valid());
} }
......
...@@ -9,21 +9,37 @@ ...@@ -9,21 +9,37 @@
#include "mojo/edk/embedder/scoped_platform_handle.h" #include "mojo/edk/embedder/scoped_platform_handle.h"
#include "mojo/edk/system/system_impl_export.h" #include "mojo/edk/system/system_impl_export.h"
#if defined(OS_WIN)
#include "base/strings/string16.h"
#endif
namespace mojo { namespace mojo {
namespace edk { namespace edk {
struct NamedPlatformHandle; struct NamedPlatformHandle;
struct CreateServerHandleOptions {
#if defined(OS_WIN)
// If true, creating a server handle will fail if another pipe with the same
// name exists.
bool enforce_uniqueness = true;
// If non-empty, a security descriptor to use when creating the pipe. If
// empty, a default security descriptor will be used. See
// kDefaultSecurityDescriptor in named_platform_handle_utils_win.cc.
base::string16 security_descriptor;
#endif
};
// Creates a client platform handle from |handle|. This may block until |handle| // Creates a client platform handle from |handle|. This may block until |handle|
// is ready to receive connections. // is ready to receive connections.
MOJO_SYSTEM_IMPL_EXPORT ScopedPlatformHandle MOJO_SYSTEM_IMPL_EXPORT ScopedPlatformHandle
CreateClientHandle(const NamedPlatformHandle& handle); CreateClientHandle(const NamedPlatformHandle& handle);
// Creates a server platform handle from |handle|. On Windows, if // Creates a server platform handle from |handle|.
// |enforce_uniqueness| is true, this will fail if another pipe with the same
// name exists. On other platforms, |enforce_uniqueness| must be false.
MOJO_SYSTEM_IMPL_EXPORT ScopedPlatformHandle MOJO_SYSTEM_IMPL_EXPORT ScopedPlatformHandle
CreateServerHandle(const NamedPlatformHandle& handle, bool enforce_uniqueness); CreateServerHandle(const NamedPlatformHandle& handle,
const CreateServerHandleOptions& options = {});
} // namespace edk } // namespace edk
} // namespace mojo } // namespace mojo
......
...@@ -99,9 +99,9 @@ ScopedPlatformHandle CreateClientHandle( ...@@ -99,9 +99,9 @@ ScopedPlatformHandle CreateClientHandle(
return handle; return handle;
} }
ScopedPlatformHandle CreateServerHandle(const NamedPlatformHandle& named_handle, ScopedPlatformHandle CreateServerHandle(
bool enforce_uniqueness) { const NamedPlatformHandle& named_handle,
CHECK(!enforce_uniqueness); const CreateServerHandleOptions& options) {
if (!named_handle.is_valid()) if (!named_handle.is_valid())
return ScopedPlatformHandle(); return ScopedPlatformHandle();
......
...@@ -15,6 +15,18 @@ ...@@ -15,6 +15,18 @@
namespace mojo { namespace mojo {
namespace edk { namespace edk {
namespace {
// A DACL to grant:
// GA = Generic All
// access to:
// SY = LOCAL_SYSTEM
// BA = BUILTIN_ADMINISTRATORS
// OW = OWNER_RIGHTS
constexpr base::char16 kDefaultSecurityDescriptor[] =
L"D:(A;;GA;;;SY)(A;;GA;;;BA)(A;;GA;;;OW)";
} // namespace
ScopedPlatformHandle CreateClientHandle( ScopedPlatformHandle CreateClientHandle(
const NamedPlatformHandle& named_handle) { const NamedPlatformHandle& named_handle) {
...@@ -41,27 +53,23 @@ ScopedPlatformHandle CreateClientHandle( ...@@ -41,27 +53,23 @@ ScopedPlatformHandle CreateClientHandle(
return handle; return handle;
} }
ScopedPlatformHandle CreateServerHandle(const NamedPlatformHandle& named_handle, ScopedPlatformHandle CreateServerHandle(
bool enforce_uniqueness) { const NamedPlatformHandle& named_handle,
const CreateServerHandleOptions& options) {
if (!named_handle.is_valid()) if (!named_handle.is_valid())
return ScopedPlatformHandle(); return ScopedPlatformHandle();
PSECURITY_DESCRIPTOR security_desc = nullptr; PSECURITY_DESCRIPTOR security_desc = nullptr;
ULONG security_desc_len = 0; ULONG security_desc_len = 0;
// Create a DACL to grant:
// GA = Generic All
// access to:
// SY = LOCAL_SYSTEM
// BA = BUILTIN_ADMINISTRATORS
// OW = OWNER_RIGHTS
PCHECK(ConvertStringSecurityDescriptorToSecurityDescriptor( PCHECK(ConvertStringSecurityDescriptorToSecurityDescriptor(
L"D:(A;;GA;;;SY)(A;;GA;;;BA)(A;;GA;;;OW)", SDDL_REVISION_1, options.security_descriptor.empty() ? kDefaultSecurityDescriptor
&security_desc, &security_desc_len)); : options.security_descriptor.c_str(),
SDDL_REVISION_1, &security_desc, &security_desc_len));
std::unique_ptr<void, decltype(::LocalFree)*> p(security_desc, ::LocalFree); std::unique_ptr<void, decltype(::LocalFree)*> p(security_desc, ::LocalFree);
SECURITY_ATTRIBUTES security_attributes = {sizeof(SECURITY_ATTRIBUTES), SECURITY_ATTRIBUTES security_attributes = {sizeof(SECURITY_ATTRIBUTES),
security_desc, FALSE}; security_desc, FALSE};
const DWORD kOpenMode = enforce_uniqueness const DWORD kOpenMode = options.enforce_uniqueness
? PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED | ? PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED |
FILE_FLAG_FIRST_PIPE_INSTANCE FILE_FLAG_FIRST_PIPE_INSTANCE
: PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED; : PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED;
...@@ -69,9 +77,9 @@ ScopedPlatformHandle CreateServerHandle(const NamedPlatformHandle& named_handle, ...@@ -69,9 +77,9 @@ ScopedPlatformHandle CreateServerHandle(const NamedPlatformHandle& named_handle,
PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_REJECT_REMOTE_CLIENTS; PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_REJECT_REMOTE_CLIENTS;
PlatformHandle handle( PlatformHandle handle(
CreateNamedPipeW(named_handle.pipe_name().c_str(), kOpenMode, kPipeMode, CreateNamedPipeW(named_handle.pipe_name().c_str(), kOpenMode, kPipeMode,
enforce_uniqueness ? 1 : 255, // Max instances. options.enforce_uniqueness ? 1 : 255, // Max instances.
4096, // Out buffer size. 4096, // Out buffer size.
4096, // In buffer size. 4096, // In buffer size.
5000, // Timeout in milliseconds. 5000, // Timeout in milliseconds.
&security_attributes)); &security_attributes));
handle.needs_connection = true; handle.needs_connection = true;
......
...@@ -148,7 +148,7 @@ ScopedMessagePipeHandle MultiprocessTestHelper::StartChildWithExtraSwitch( ...@@ -148,7 +148,7 @@ ScopedMessagePipeHandle MultiprocessTestHelper::StartChildWithExtraSwitch(
} else if (launch_type == LaunchType::PEER) { } else if (launch_type == LaunchType::PEER) {
pipe = ConnectToPeerProcess(channel.PassServerHandle()); pipe = ConnectToPeerProcess(channel.PassServerHandle());
} else if (launch_type == LaunchType::NAMED_PEER) { } else if (launch_type == LaunchType::NAMED_PEER) {
pipe = ConnectToPeerProcess(CreateServerHandle(named_pipe, false)); pipe = ConnectToPeerProcess(CreateServerHandle(named_pipe));
} }
test_child_ = test_child_ =
...@@ -160,9 +160,8 @@ ScopedMessagePipeHandle MultiprocessTestHelper::StartChildWithExtraSwitch( ...@@ -160,9 +160,8 @@ ScopedMessagePipeHandle MultiprocessTestHelper::StartChildWithExtraSwitch(
ChildProcessLaunched(test_child_.Handle(), channel.PassServerHandle(), ChildProcessLaunched(test_child_.Handle(), channel.PassServerHandle(),
child_token, process_error_callback_); child_token, process_error_callback_);
} else if (launch_type == LaunchType::NAMED_CHILD) { } else if (launch_type == LaunchType::NAMED_CHILD) {
ChildProcessLaunched(test_child_.Handle(), ChildProcessLaunched(test_child_.Handle(), CreateServerHandle(named_pipe),
CreateServerHandle(named_pipe, false), child_token, child_token, process_error_callback_);
process_error_callback_);
} }
CHECK(test_child_.IsValid()); CHECK(test_child_.IsValid());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment