Make named pipe security configurable by security descriptor string
Client: cpp
Patch: Nathan Breakwell
This closes #2083
diff --git a/lib/cpp/src/thrift/transport/TPipeServer.cpp b/lib/cpp/src/thrift/transport/TPipeServer.cpp
index 0855e2c..e1cee80 100644
--- a/lib/cpp/src/thrift/transport/TPipeServer.cpp
+++ b/lib/cpp/src/thrift/transport/TPipeServer.cpp
@@ -28,6 +28,7 @@
#include <thrift/windows/OverlappedSubmissionThread.h>
#include <AccCtrl.h>
#include <Aclapi.h>
+#include <sddl.h>
#endif //_WIN32
namespace apache {
@@ -95,9 +96,15 @@
class TNamedPipeServer : public TPipeServerImpl {
public:
- TNamedPipeServer(const std::string& pipename, uint32_t bufsize, uint32_t maxconnections)
- : stopping_(false), pipename_(pipename), bufsize_(bufsize), maxconns_(maxconnections)
- {
+ TNamedPipeServer(const std::string& pipename,
+ uint32_t bufsize,
+ uint32_t maxconnections,
+ const std::string& securityDescriptor)
+ : stopping_(false),
+ pipename_(pipename),
+ bufsize_(bufsize),
+ maxconns_(maxconnections),
+ securityDescriptor_(securityDescriptor) {
connectOverlap_.action = TOverlappedWorkItem::CONNECT;
cancelOverlap_.action = TOverlappedWorkItem::CANCELIO;
TAutoCrit lock(pipe_protect_);
@@ -134,6 +141,7 @@
bool stopping_;
std::string pipename_;
+ std::string securityDescriptor_;
uint32_t bufsize_;
uint32_t maxconns_;
TManualResetEvent listen_event_;
@@ -155,17 +163,30 @@
: bufsize_(bufsize), isAnonymous_(false) {
setMaxConnections(TPIPE_SERVER_MAX_CONNS_DEFAULT);
setPipename(pipename);
+ setSecurityDescriptor(DEFAULT_PIPE_SECURITY);
}
TPipeServer::TPipeServer(const std::string& pipename, uint32_t bufsize, uint32_t maxconnections)
: bufsize_(bufsize), isAnonymous_(false) {
setMaxConnections(maxconnections);
setPipename(pipename);
+ setSecurityDescriptor(DEFAULT_PIPE_SECURITY);
+}
+
+TPipeServer::TPipeServer(const std::string& pipename,
+ uint32_t bufsize,
+ uint32_t maxconnections,
+ const std::string& securityDescriptor)
+ : bufsize_(bufsize), isAnonymous_(false) {
+ setMaxConnections(maxconnections);
+ setPipename(pipename);
+ setSecurityDescriptor(securityDescriptor);
}
TPipeServer::TPipeServer(const std::string& pipename) : bufsize_(1024), isAnonymous_(false) {
setMaxConnections(TPIPE_SERVER_MAX_CONNS_DEFAULT);
setPipename(pipename);
+ setSecurityDescriptor(DEFAULT_PIPE_SECURITY);
}
TPipeServer::TPipeServer(int bufsize) : bufsize_(bufsize), isAnonymous_(true) {
@@ -191,7 +212,7 @@
void TPipeServer::listen() {
if (isAnonymous_)
return;
- impl_.reset(new TNamedPipeServer(pipename_, bufsize_, maxconns_));
+ impl_.reset(new TNamedPipeServer(pipename_, bufsize_, maxconns_, securityDescriptor_));
}
shared_ptr<TTransport> TPipeServer::acceptImpl() {
@@ -327,73 +348,47 @@
impl_.reset();
}
-bool TNamedPipeServer::createNamedPipe(const TAutoCrit & /*lockProof*/) {
+bool TNamedPipeServer::createNamedPipe(const TAutoCrit& /*lockProof*/) {
- // Windows - set security to allow non-elevated apps
- // to access pipes created by elevated apps.
- SID_IDENTIFIER_AUTHORITY SIDAuthWorld = SECURITY_WORLD_SID_AUTHORITY;
- PSID everyone_sid = NULL;
- AllocateAndInitializeSid(
- &SIDAuthWorld, 1, SECURITY_WORLD_RID, 0, 0, 0, 0, 0, 0, 0, &everyone_sid);
+ PSECURITY_DESCRIPTOR psd = NULL;
+ ULONG size = 0;
- EXPLICIT_ACCESS ea;
- ZeroMemory(&ea, sizeof(EXPLICIT_ACCESS));
- ea.grfAccessPermissions = SPECIFIC_RIGHTS_ALL | STANDARD_RIGHTS_ALL;
- ea.grfAccessMode = SET_ACCESS;
- ea.grfInheritance = NO_INHERITANCE;
- ea.Trustee.TrusteeForm = TRUSTEE_IS_SID;
- ea.Trustee.TrusteeType = TRUSTEE_IS_WELL_KNOWN_GROUP;
- ea.Trustee.ptstrName = static_cast<LPTSTR>(everyone_sid);
-
- PACL acl = NULL;
- SetEntriesInAcl(1, &ea, NULL, &acl);
-
- PSECURITY_DESCRIPTOR sd = (PSECURITY_DESCRIPTOR)LocalAlloc(LPTR, SECURITY_DESCRIPTOR_MIN_LENGTH);
- if (!InitializeSecurityDescriptor(sd, SECURITY_DESCRIPTOR_REVISION)) {
- auto lastError = GetLastError();
- LocalFree(sd);
- LocalFree(acl);
- GlobalOutput.perror("TPipeServer::InitializeSecurityDescriptor() GLE=", lastError);
- throw TTransportException(TTransportException::NOT_OPEN, "InitializeSecurityDescriptor() failed",
- lastError);
- }
- if (!SetSecurityDescriptorDacl(sd, TRUE, acl, FALSE)) {
- auto lastError = GetLastError();
- LocalFree(sd);
- LocalFree(acl);
- GlobalOutput.perror("TPipeServer::SetSecurityDescriptorDacl() GLE=", lastError);
- throw TTransportException(TTransportException::NOT_OPEN,
- "SetSecurityDescriptorDacl() failed", lastError);
+ if (!ConvertStringSecurityDescriptorToSecurityDescriptorA(securityDescriptor_.c_str(),
+ SDDL_REVISION_1, &psd, &size)) {
+ DWORD lastError = GetLastError();
+ GlobalOutput.perror("TPipeServer::ConvertStringSecurityDescriptorToSecurityDescriptorA() GLE=",
+ lastError);
+ throw TTransportException(
+ TTransportException::NOT_OPEN,
+ "TPipeServer::ConvertStringSecurityDescriptorToSecurityDescriptorA() failed", lastError);
}
SECURITY_ATTRIBUTES sa;
sa.nLength = sizeof(SECURITY_ATTRIBUTES);
- sa.lpSecurityDescriptor = sd;
+ sa.lpSecurityDescriptor = psd;
sa.bInheritHandle = FALSE;
// Create an instance of the named pipe
- TAutoHandle hPipe(CreateNamedPipeA(pipename_.c_str(), // pipe name
- PIPE_ACCESS_DUPLEX | // read/write access
- FILE_FLAG_OVERLAPPED, // async mode
- PIPE_TYPE_BYTE | // byte type pipe
- PIPE_READMODE_BYTE, // byte read mode
- maxconns_, // max. instances
- bufsize_, // output buffer size
- bufsize_, // input buffer size
- 0, // client time-out
- &sa)); // security attributes
+ TAutoHandle hPipe(CreateNamedPipeA(pipename_.c_str(), // pipe name
+ PIPE_ACCESS_DUPLEX | // read/write access
+ FILE_FLAG_OVERLAPPED, // async mode
+ PIPE_TYPE_BYTE | // byte type pipe
+ PIPE_READMODE_BYTE, // byte read mode
+ maxconns_, // max. instances
+ bufsize_, // output buffer size
+ bufsize_, // input buffer size
+ 0, // client time-out
+ &sa)); // security attributes
auto lastError = GetLastError();
- LocalFree(sd);
- LocalFree(acl);
- FreeSid(everyone_sid);
+ if (psd)
+ LocalFree(psd);
if (hPipe.h == INVALID_HANDLE_VALUE) {
Pipe_.reset();
GlobalOutput.perror("TPipeServer::TCreateNamedPipe() GLE=", lastError);
- throw TTransportException(TTransportException::NOT_OPEN,
- "TCreateNamedPipe() failed",
- lastError);
+ throw TTransportException(TTransportException::NOT_OPEN, "TCreateNamedPipe() failed",
+ lastError);
}
Pipe_.reset(hPipe.release());
@@ -404,13 +399,12 @@
SECURITY_ATTRIBUTES sa;
SECURITY_DESCRIPTOR sd; // security information for pipes
- if (!InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION))
- {
- GlobalOutput.perror("TPipeServer InitializeSecurityDescriptor (anon) failed, GLE=", GetLastError());
+ if (!InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION)) {
+ GlobalOutput.perror("TPipeServer InitializeSecurityDescriptor (anon) failed, GLE=",
+ GetLastError());
return false;
}
- if (!SetSecurityDescriptorDacl(&sd, true, NULL, false))
- {
+ if (!SetSecurityDescriptorDacl(&sd, true, NULL, false)) {
GlobalOutput.perror("TPipeServer SetSecurityDescriptorDacl (anon) failed, GLE=",
GetLastError());
return false;
@@ -482,6 +476,10 @@
isAnonymous_ = anon;
}
+void TPipeServer::setSecurityDescriptor(const std::string& securityDescriptor) {
+ securityDescriptor_ = securityDescriptor;
+}
+
void TPipeServer::setMaxConnections(uint32_t maxconnections) {
if (maxconnections == 0)
maxconns_ = 1;
diff --git a/lib/cpp/src/thrift/transport/TPipeServer.h b/lib/cpp/src/thrift/transport/TPipeServer.h
index d4255cb..67c5d51 100644
--- a/lib/cpp/src/thrift/transport/TPipeServer.h
+++ b/lib/cpp/src/thrift/transport/TPipeServer.h
@@ -31,6 +31,11 @@
#define TPIPE_SERVER_MAX_CONNS_DEFAULT PIPE_UNLIMITED_INSTANCES
+// Windows - set security to allow non-elevated apps
+// to access pipes created by elevated apps.
+// Full access to everyone
+const std::string DEFAULT_PIPE_SECURITY{"D:(A;;FA;;;WD)"};
+
namespace apache {
namespace thrift {
namespace transport {
@@ -51,6 +56,10 @@
// Named Pipe -
TPipeServer(const std::string& pipename, uint32_t bufsize);
TPipeServer(const std::string& pipename, uint32_t bufsize, uint32_t maxconnections);
+ TPipeServer(const std::string& pipename,
+ uint32_t bufsize,
+ uint32_t maxconnections,
+ const std::string& securityDescriptor);
TPipeServer(const std::string& pipename);
// Anonymous pipe -
TPipeServer(int bufsize);
@@ -78,6 +87,7 @@
bool getAnonymous();
void setAnonymous(bool anon);
void setMaxConnections(uint32_t maxconnections);
+ void setSecurityDescriptor(const std::string& securityDescriptor);
// this function is intended to be used in generic / template situations,
// so its name needs to be the same as TPipe's
@@ -90,6 +100,7 @@
std::shared_ptr<TPipeServerImpl> impl_;
std::string pipename_;
+ std::string securityDescriptor_;
uint32_t bufsize_;
uint32_t maxconns_;
bool isAnonymous_;