win: Lower integrity level of connection pipe

util/win/exception_handler_server.cc

Index: util/win/exception_handler_server.cc
diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc
index ce5687d74b7ba6f853708f68acf5e4f25390f5b0..4806113deb5bb8dc1c887c943828eb0369c6fb3c 100644
--- a/util/win/exception_handler_server.cc
+++ b/util/win/exception_handler_server.cc
@@ -14,6 +14,8 @@
 
 #include "util/win/exception_handler_server.h"
 
+#include <aclapi.h>
+#include <sddl.h>
 #include <string.h>
 
 #include "base/logging.h"
@@ -30,6 +32,7 @@
 #include "util/win/get_function.h"
 #include "util/win/handle.h"
 #include "util/win/registration_protocol_win.h"
+#include "util/win/scoped_local_free.h"
 #include "util/win/xp_compat.h"
 
 namespace crashpad {
@@ -45,18 +48,74 @@ const size_t kPipeInstances = 2;
 // If first_instance is true, the named pipe instance will be created with
 // FILE_FLAG_FIRST_PIPE_INSTANCE. This ensures that the the pipe name is not
 // already in use when created.
+//
+// The integrity level of the pipe is lowered so that it can be connected to by
+// low integrity processes.

[jschuh, 2015/11/05 20:05:13]

Should be "connected to by processes at any integrity level".

[scottmg, 2015/11/05 20:15:30]

Done.

 HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name,
                                bool first_instance) {
-  return CreateNamedPipe(pipe_name.c_str(),
-                         PIPE_ACCESS_DUPLEX |
-                             (first_instance ? FILE_FLAG_FIRST_PIPE_INSTANCE
-                                             : 0),
-                         PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
-                         kPipeInstances,
-                         512,
-                         512,
-                         0,
-                         nullptr);
+  ScopedFileHandle pipe(CreateNamedPipe(
+      pipe_name.c_str(),
+      PIPE_ACCESS_DUPLEX | (first_instance ? FILE_FLAG_FIRST_PIPE_INSTANCE : 0),
+      PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
+      kPipeInstances,
+      512,
+      512,
+      0,
+      nullptr));
+  if (!pipe.is_valid()) {
+    PLOG(ERROR) << "CreateNamedPipe";
+    return INVALID_HANDLE_VALUE;
+  }
+
+  // We only need to set the integrity level on the first instance of the pipe.

[Mark Mentovai, 2015/11/05 20:08:01]

Shouldn’t we still do this if we run crashpad_handler in persistent --pipe-name
mode without having its client start it?

[scottmg, 2015/11/05 20:15:30]

Yes, we should, but don't we still in ExceptionHandlerServer::Run, regardless of
whether we created the pipe name or someone else did? (I'm probably
misunderstanding what you're saying.)

[Mark Mentovai, 2015/11/05 21:01:13]

scottmg wrote:
first_instance only controls whether FILE_FLAG_FIRST_PIPE_INSTANCE is set. It’s
set when called by CreatePipe() to establish a pipe name before Run(), but it’s
not set if SetPipeName() establishes the name but no pipe. Run() doesn’t set it.

You can change this up appropriately, but as written now, none of this new stuff
will happen in persistent mode with a well-known --pipe-name.

+  if (!first_instance)
+    return pipe.release();
+
+  // Lower the integrity of the pipe so that it can be connected to from low
+  // integrity processes (on Vista and later).
+  const DWORD version = GetVersion();
+  const DWORD major_version = LOBYTE(LOWORD(version));
+  const bool is_pre_vista = major_version < 6;
+  if (is_pre_vista)
+    return pipe.release();
+
+  // Mandatory Label, no ACE flags, no ObjectType, integrity level untrusted.
+  const wchar_t kSddl[] = L"S:(ML;;;;;S-1-16-0)";
+
+  PSECURITY_DESCRIPTOR sec_desc = nullptr;

[Mark Mentovai, 2015/11/05 20:08:01]

SECURITY_DESCRIPTOR*, ACL*.

[scottmg, 2015/11/05 20:15:30]

Done.

+
+  PACL sacl = nullptr;
+  if (!ConvertStringSecurityDescriptorToSecurityDescriptor(
+          kSddl, SDDL_REVISION, &sec_desc, nullptr)) {
+    PLOG(ERROR) << "ConvertStringSecurityDescriptorToSecurityDescriptorW";

[Mark Mentovai, 2015/11/05 20:08:01]

No W on the end.

[scottmg, 2015/11/05 20:15:30]

Done.

+    return INVALID_HANDLE_VALUE;
+  }
+
+  // Take ownership of the allocated SECURITY_DESCRIPTOR.
+  ScopedLocalFree scoped_sec_desc(sec_desc);
+
+  BOOL sacl_present = FALSE;
+  BOOL sacl_defaulted = FALSE;
+  if (!GetSecurityDescriptorSacl(
+          sec_desc, &sacl_present, &sacl, &sacl_defaulted)) {
+    PLOG(ERROR) << "GetSecurityDescriptorSacl";
+    return INVALID_HANDLE_VALUE;
+  }
+
+  DWORD error = SetSecurityInfo(pipe.get(),
+                                SE_KERNEL_OBJECT,
+                                LABEL_SECURITY_INFORMATION,
+                                nullptr,
+                                nullptr,
+                                nullptr,
+                                sacl);
+  if (error != ERROR_SUCCESS) {
+    LOG(ERROR) << "SetSecurityInfo: "
+               << logging::SystemErrorCodeToString(error);
+    return INVALID_HANDLE_VALUE;
+  }
+
+  return pipe.release();
 }
 
 decltype(GetNamedPipeClientProcessId)* GetNamedPipeClientProcessIdFunction() {