diff --git a/util/misc/uuid.cc b/util/misc/uuid.cc index 4f1fbcbe..d4345a8a 100644 --- a/util/misc/uuid.cc +++ b/util/misc/uuid.cc @@ -95,17 +95,11 @@ bool UUID::InitializeWithNew() { uuid_generate(uuid); InitializeFromBytes(uuid); return true; -#elif defined(OS_WIN) - ::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) +#elif defined(OS_WIN) || 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. diff --git a/util/util.gyp b/util/util.gyp index 418256d3..abf0bfdc 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -263,8 +263,6 @@ ['OS=="win"', { 'link_settings': { 'libraries': [ - '-ladvapi32.lib', - '-lrpcrt4.lib', '-lwinhttp.lib', ], }, diff --git a/util/util_test.gyp b/util/util_test.gyp index b5989de9..e6bf5635 100644 --- a/util/util_test.gyp +++ b/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', ], diff --git a/util/win/registration_protocol_win.cc b/util/win/registration_protocol_win.cc index 17638415..ebadd60f 100644 --- a/util/win/registration_protocol_win.cc +++ b/util/win/registration_protocol_win.cc @@ -15,12 +15,11 @@ #include "util/win/registration_protocol_win.h" #include -#include #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(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(&kSecDescBlob); +} + } // namespace crashpad diff --git a/util/win/registration_protocol_win.h b/util/win/registration_protocol_win.h index 12209835..5f04a466 100644 --- a/util/win/registration_protocol_win.h +++ b/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_ diff --git a/util/win/registration_protocol_win_test.cc b/util/win/registration_protocol_win_test.cc new file mode 100644 index 00000000..60e7c86c --- /dev/null +++ b/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 +#include +#include + +#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