win: Remove use of rpcrt4 and advapi32 from some util code

ConvertStringSecurityDescriptorToSecurityDescriptor() is used when
creating the initial connection pipe. Because this is done from inside
DllMain(), we cannot use advapi32 (where this function is). Instead,
save the binary representation of the self-relative SECURITY_DESCRIPTOR.
It is conceivable that this could change, but unlikely as this is the
same blob that would be stored on a file in NTFS.

Another potential approach would be to not make the pipe available to
all integrity levels here, and instead modify the Chromium sandbox code
to allow a specific pipe name prefix that would have to correspond with
the pipe name that Crashpad creates.

Similarly, UuidCreate() (used when initializing the database) is in a
DLL that can't be loaded early, so use the Linux/Android implementation
on Windows too.

R=mark@chromium.org
BUG=chromium:655788,chromium:656800

Change-Id: I434f8e96fc275fc30d0a31208b025bfc08595ff9
Reviewed-on: https://chromium-review.googlesource.com/417223
Reviewed-by: Mark Mentovai <mark@chromium.org>

util/misc/uuid.cc

@@ -95,17 +95,11 @@ bool UUID::InitializeWithNew() {
   uuid_generate(uuid);
   InitializeFromBytes(uuid);
   return true;
-#elif defined(OS_WIN)
+#elif defined(OS_WIN) || defined(OS_LINUX) || defined(OS_ANDROID)
-  ::UUID system_uuid;
-  if (UuidCreate(&system_uuid) != RPC_S_OK) {
-    LOG(ERROR) << "UuidCreate";
-    return false;
-  }
-  InitializeFromSystemUUID(&system_uuid);
-  return true;
-#elif defined(OS_LINUX) || defined(OS_ANDROID)
   // Linux does not provide a UUID generator in a widely-available system
   // library. uuid_generate() from libuuid is not available everywhere.
+  // On Windows, do not use UuidCreate() to avoid a dependency on rpcrt4, so
+  // that this function is usable early in DllMain().
   base::RandBytes(this, sizeof(*this));
 
   // Set six bits per RFC 4122 §4.4 to identify this as a pseudo-random UUID.

util/util.gyp

@@ -263,8 +263,6 @@
         ['OS=="win"', {
           'link_settings': {
             'libraries': [
-              '-ladvapi32.lib',
-              '-lrpcrt4.lib',
               '-lwinhttp.lib',
             ],
           },

util/util_test.gyp

@@ -91,6 +91,7 @@
         'win/handle_test.cc',
         'win/initial_client_data_test.cc',
         'win/process_info_test.cc',
+        'win/registration_protocol_win_test.cc',
         'win/scoped_process_suspend_test.cc',
         'win/time_test.cc',
       ],
@@ -108,6 +109,7 @@
           ],
           'link_settings': {
             'libraries': [
+              '-ladvapi32.lib',
               '-limagehlp.lib',
               '-lrpcrt4.lib',
             ],

util/win/registration_protocol_win.cc

@@ -15,12 +15,11 @@
 #include "util/win/registration_protocol_win.h"
 
 #include <windows.h>
-#include <sddl.h>
 
 #include "base/logging.h"
+#include "base/macros.h"
 #include "util/win/exception_handler_server.h"
 #include "util/win/scoped_handle.h"
-#include "util/win/scoped_local_alloc.h"
 
 namespace crashpad {
 
@@ -97,7 +96,6 @@ HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name,
                                bool first_instance) {
   SECURITY_ATTRIBUTES security_attributes;
   SECURITY_ATTRIBUTES* security_attributes_pointer = nullptr;
-  ScopedLocalAlloc scoped_sec_desc;
 
   if (first_instance) {
     // Pre-Vista does not have integrity levels.
@@ -105,21 +103,10 @@ HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name,
     const DWORD major_version = LOBYTE(LOWORD(version));
     const bool is_vista_or_later = major_version >= 6;
     if (is_vista_or_later) {
-      // 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;
-      PCHECK(ConvertStringSecurityDescriptorToSecurityDescriptor(
-          kSddl, SDDL_REVISION_1, &sec_desc, nullptr))
-          << "ConvertStringSecurityDescriptorToSecurityDescriptor";
-
-      // Take ownership of the allocated SECURITY_DESCRIPTOR.
-      scoped_sec_desc.reset(sec_desc);
-
       memset(&security_attributes, 0, sizeof(security_attributes));
       security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES);
-      security_attributes.lpSecurityDescriptor = sec_desc;
+      security_attributes.lpSecurityDescriptor =
+          const_cast<void*>(GetSecurityDescriptorForNamedPipeInstance(nullptr));
       security_attributes.bInheritHandle = TRUE;
       security_attributes_pointer = &security_attributes;
     }
@@ -136,4 +123,86 @@ HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name,
       security_attributes_pointer);
 }
 
+const void* GetSecurityDescriptorForNamedPipeInstance(size_t* size) {
+  // Mandatory Label, no ACE flags, no ObjectType, integrity level untrusted is
+  // "S:(ML;;;;;S-1-16-0)". Typically
+  // ConvertStringSecurityDescriptorToSecurityDescriptor() would be used to
+  // convert from a string representation. However, that function cannot be used
+  // because it is in advapi32.dll and CreateNamedPipeInstance() is called from
+  // within DllMain() where the loader lock is held. advapi32.dll is delay
+  // loaded in chrome_elf.dll because it must avoid loading user32.dll. If an
+  // advapi32.dll function were used, it would cause a load of the DLL, which
+  // would in turn cause deadlock.
+
+#pragma pack(push, 1)
+  static const struct SecurityDescriptorBlob {
+    // See https://msdn.microsoft.com/en-us/library/cc230366.aspx.
+    SECURITY_DESCRIPTOR_RELATIVE sd_rel;
+    struct {
+      ACL acl;
+      struct {
+        // This is equivalent to SYSTEM_MANDATORY_LABEL_ACE, but there's no
+        // DWORD offset to the SID, instead it's inline.
+        ACE_HEADER header;
+        ACCESS_MASK mask;
+        SID sid;
+      } ace[1];
+    } sacl;
+  } kSecDescBlob = {
+      // sd_rel.
+      {
+          SECURITY_DESCRIPTOR_REVISION1,  // Revision.
+          0x00,  // Sbz1.
+          SE_SELF_RELATIVE | SE_SACL_PRESENT,  // Control.
+          0,  // OffsetOwner.
+          0,  // OffsetGroup.
+          offsetof(SecurityDescriptorBlob, sacl),  // OffsetSacl.
+          0,  // OffsetDacl.
+      },
+
+      // sacl.
+      {
+          // acl.
+          {
+              ACL_REVISION,  // AclRevision.
+              0,  // Sbz1.
+              sizeof(SecurityDescriptorBlob::sacl),  // AclSize.
+              arraysize(SecurityDescriptorBlob::sacl.ace),  // AceCount.
+              0,  // Sbz2.
+          },
+
+          // ace[0].
+          {
+              {
+                  // header.
+                  {
+                      SYSTEM_MANDATORY_LABEL_ACE_TYPE,  // AceType.
+                      0,  // AceFlags.
+                      sizeof(SecurityDescriptorBlob::sacl.ace[0]),  // AceSize.
+                  },
+
+                  // mask.
+                  0,
+
+                  // sid.
+                  {
+                      SID_REVISION,  // Revision.
+                      // SubAuthorityCount.
+                      arraysize(
+                          SecurityDescriptorBlob::sacl.ace[0].sid.SubAuthority),
+                      // IdentifierAuthority.
+                      SECURITY_MANDATORY_LABEL_AUTHORITY,
+                      {SECURITY_MANDATORY_UNTRUSTED_RID},  // SubAuthority.
+                  },
+              },
+          },
+      },
+  };
+#pragma pack(pop)
+
+  if (size)
+    *size = sizeof(kSecDescBlob);
+  return reinterpret_cast<const void*>(&kSecDescBlob);
+}
+
 }  // namespace crashpad

util/win/registration_protocol_win.h

@@ -145,6 +145,18 @@ bool SendToCrashHandlerServer(const base::string16& pipe_name,
 HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name,
                                bool first_instance);
 
+//! \brief Returns the SECURITY_DESCRIPTOR blob that will be used for creating
+//!     the connection pipe in CreateNamedPipeInstance().
+//!
+//! This function is exposed for only for testing.
+//!
+//! \param[out] size The size of the returned blob. May be `nullptr` if not
+//!     required.
+//!
+//! \return A pointer to a self-relative `SECURITY_DESCRIPTOR`. Ownership is not
+//!     transferred to the caller.
+const void* GetSecurityDescriptorForNamedPipeInstance(size_t* size);
+
 }  // namespace crashpad
 
 #endif  // CRASHPAD_UTIL_WIN_REGISTRATION_PROTOCOL_WIN_H_

util/win/registration_protocol_win_test.cc

@@ -0,0 +1,53 @@
+// Copyright 2016 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "util/win/registration_protocol_win.h"
+
+#include <windows.h>
+#include <sddl.h>
+#include <string.h>
+
+#include "gtest/gtest.h"
+#include "test/errors.h"
+#include "util/win/scoped_local_alloc.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+TEST(SecurityDescriptor, MatchesAdvapi32) {
+  // This security descriptor is built manually in the connection code to avoid
+  // calling the advapi32 functions. Verify that it returns the same thing as
+  // ConvertStringSecurityDescriptorToSecurityDescriptor() would.
+
+  // 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;
+  ULONG sec_desc_len;
+  ASSERT_TRUE(ConvertStringSecurityDescriptorToSecurityDescriptor(
+      kSddl, SDDL_REVISION_1, &sec_desc, &sec_desc_len))
+      << ErrorMessage("ConvertStringSecurityDescriptorToSecurityDescriptor");
+  ScopedLocalAlloc sec_desc_owner(sec_desc);
+
+  size_t created_len;
+  const void* const created =
+      GetSecurityDescriptorForNamedPipeInstance(&created_len);
+  ASSERT_EQ(sec_desc_len, created_len);
+  EXPECT_EQ(0, memcmp(sec_desc, created, sec_desc_len));
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace crashpad