0

Call ProcessPrng not RtlGenRandom/SystemFunction036

Calling RtlGenRandom causes advapi & cryptbase to load, ultimately
causing a handle to \Device\KsecDD to be opened in the renderer
that we do not need.

This CL replaces Chrome's calls to RtlGenRandom / SystemFunction036
with calls to bcryptprimitives!ProcessPrng or base:: functions.
In Windows 10 and onwards RtlGenRandom is simply a direct call to
ProcessPrng so this does not change any behavior. See doc[1] for
details.

Before this CL these functions got system randomness from calls to RtlGenRandom. This function is undocumented and unsupported,
but has always been available by linking to SystemFunction036 in
advadpi32.dll. In Windows 10 and later, this export simply forwards
to cryptbase.dll!SystemFunction036 which calls ProcessPrng()
directly.

cryptbase!SystemFunction036 decompiled:

```
BOOLEAN SystemFunction036(PVOID RandomBuffer,ULONG RandomBufferLength)
{
  BOOL retval;
  retval = ProcessPrng(RandomBuffer,RandomBufferLength);
  return retval != 0;
}
```

In addition:

 - ProcessPrng takes a size_t length argument, making
   it easier to call in Chrome's wrappers.
 - ProcessPrng involves loading fewer DLLs.
 - Removing all calls to RtlGenRandom prevents cargo-culting it back
   in where it might be used in the renderer.

Note: boringssl still uses RtlGenRandom. If UseBoringSSLForRandBytes
is enabled base/rand_util.h functions use RtlGenRandom().
boringsll will be switched to ProcessPrng in a following CL.

[1] https://docs.google.com/document/d/13n1t5ak0yofzcadQCF7Ew5TewSUkNfQ3n-IYodjeRYc/edit

Bug: 74242
Change-Id: Ibd6e9ec72b03c6556e4a9283e4b5008209c045be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4606732
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1160694}
This commit is contained in:
Alex Gough
2023-06-21 16:27:10 +00:00
committed by Chromium LUCI CQ
parent c26cd08d7f
commit c53f0e2e84
7 changed files with 50 additions and 84 deletions

@ -8,31 +8,36 @@
#include <stdint.h>
#include <windows.h>
// #define needed to link in RtlGenRandom(), a.k.a. SystemFunction036. See the
// "Community Additions" comment on MSDN here:
// http://msdn.microsoft.com/en-us/library/windows/desktop/aa387694.aspx
#define SystemFunction036 NTAPI SystemFunction036
#include <NTSecAPI.h>
#undef SystemFunction036
#include <algorithm>
#include <limits>
#include "base/allocator/partition_allocator/partition_alloc_check.h"
// Prototype for ProcessPrng.
// See: https://learn.microsoft.com/en-us/windows/win32/seccng/processprng
extern "C" {
BOOL WINAPI ProcessPrng(PBYTE pbData, SIZE_T cbData);
}
namespace partition_alloc::internal::base {
void RandBytes(void* output, size_t output_length) {
char* output_ptr = static_cast<char*>(output);
while (output_length > 0) {
const ULONG output_bytes_this_pass = static_cast<ULONG>(std::min(
output_length, static_cast<size_t>(std::numeric_limits<ULONG>::max())));
const bool success =
RtlGenRandom(output_ptr, output_bytes_this_pass) != FALSE;
PA_CHECK(success);
output_length -= output_bytes_this_pass;
output_ptr += output_bytes_this_pass;
// Import bcryptprimitives directly rather than cryptbase to avoid opening a
// handle to \\Device\KsecDD in the renderer.
// Note: we cannot use a magic static here as PA runs too early in process
// startup, but this should be safe as the process will be single-threaded
// when this first runs.
static decltype(&ProcessPrng) process_prng_fn = nullptr;
if (!process_prng_fn) {
HMODULE hmod = LoadLibraryW(L"bcryptprimitives.dll");
PA_CHECK(hmod);
process_prng_fn = reinterpret_cast<decltype(&ProcessPrng)>(
GetProcAddress(hmod, "ProcessPrng"));
PA_CHECK(process_prng_fn);
}
BOOL success = process_prng_fn(static_cast<BYTE*>(output), output_length);
// ProcessPrng is documented to always return TRUE.
PA_CHECK(success);
}
} // namespace partition_alloc::internal::base

@ -9,13 +9,6 @@
#include <stddef.h>
#include <stdint.h>
// #define needed to link in RtlGenRandom(), a.k.a. SystemFunction036. See the
// "Community Additions" comment on MSDN here:
// http://msdn.microsoft.com/en-us/library/windows/desktop/aa387694.aspx
#define SystemFunction036 NTAPI SystemFunction036
#include <NTSecAPI.h>
#undef SystemFunction036
#include <algorithm>
#include <atomic>
#include <limits>
@ -25,6 +18,12 @@
#include "third_party/boringssl/src/include/openssl/crypto.h"
#include "third_party/boringssl/src/include/openssl/rand.h"
// Prototype for ProcessPrng.
// See: https://learn.microsoft.com/en-us/windows/win32/seccng/processprng
extern "C" {
BOOL WINAPI ProcessPrng(PBYTE pbData, SIZE_T cbData);
}
namespace base {
namespace internal {
@ -54,6 +53,18 @@ bool UseBoringSSLForRandBytes() {
namespace {
// Import bcryptprimitives!ProcessPrng rather than cryptbase!RtlGenRandom to
// avoid opening a handle to \\Device\KsecDD in the renderer.
decltype(&ProcessPrng) GetProcessPrng() {
HMODULE hmod = LoadLibraryW(L"bcryptprimitives.dll");
CHECK(hmod);
decltype(&ProcessPrng) process_prng_fn =
reinterpret_cast<decltype(&ProcessPrng)>(
GetProcAddress(hmod, "ProcessPrng"));
CHECK(process_prng_fn);
return process_prng_fn;
}
void RandBytes(void* output, size_t output_length, bool avoid_allocation) {
if (!avoid_allocation && internal::UseBoringSSLForRandBytes()) {
// Ensure BoringSSL is initialized so it can use things like RDRAND.
@ -63,16 +74,10 @@ void RandBytes(void* output, size_t output_length, bool avoid_allocation) {
return;
}
char* output_ptr = static_cast<char*>(output);
while (output_length > 0) {
const ULONG output_bytes_this_pass = static_cast<ULONG>(std::min(
output_length, static_cast<size_t>(std::numeric_limits<ULONG>::max())));
const bool success =
RtlGenRandom(output_ptr, output_bytes_this_pass) != FALSE;
CHECK(success);
output_length -= output_bytes_this_pass;
output_ptr += output_bytes_this_pass;
}
static decltype(&ProcessPrng) process_prng_fn = GetProcessPrng();
BOOL success = process_prng_fn(static_cast<BYTE*>(output), output_length);
// ProcessPrng is documented to always return TRUE.
CHECK(success);
}
} // namespace

@ -99,8 +99,6 @@ static_library("sandbox") {
"src/sandbox_policy_base.h",
"src/sandbox_policy_diagnostic.cc",
"src/sandbox_policy_diagnostic.h",
"src/sandbox_rand.cc",
"src/sandbox_rand.h",
"src/security_capabilities.cc",
"src/security_capabilities.h",
"src/service_resolver.cc",

@ -16,13 +16,13 @@
#include "base/bits.h"
#include "base/check_op.h"
#include "base/notreached.h"
#include "base/rand_util.h"
#include "base/scoped_native_library.h"
#include "base/win/pe_image.h"
#include "sandbox/win/src/interception_internal.h"
#include "sandbox/win/src/interceptors.h"
#include "sandbox/win/src/internal_types.h"
#include "sandbox/win/src/sandbox.h"
#include "sandbox/win/src/sandbox_rand.h"
#include "sandbox/win/src/service_resolver.h"
#include "sandbox/win/src/target_interceptions.h"
#include "sandbox/win/src/target_process.h"
@ -50,12 +50,8 @@ namespace internal {
// Find a random offset within 64k and aligned to ceil(log2(size)).
size_t GetGranularAlignedRandomOffset(size_t size) {
CHECK_LE(size, kAllocGranularity);
unsigned int offset;
do {
GetRandom(&offset);
offset &= (kAllocGranularity - 1);
} while (offset > (kAllocGranularity - size));
unsigned int offset = static_cast<unsigned int>(
base::RandInt(0, static_cast<int>(kAllocGranularity - size)));
// Find an alignment between 64 and the page size (4096).
size_t align_size = kPageSize;

@ -15,6 +15,7 @@
#include "base/check_op.h"
#include "base/files/file_path.h"
#include "base/notreached.h"
#include "base/rand_util.h"
#include "base/scoped_native_library.h"
#include "base/win/access_token.h"
#include "base/win/windows_version.h"
@ -22,7 +23,6 @@
#include "sandbox/win/src/interception.h"
#include "sandbox/win/src/nt_internals.h"
#include "sandbox/win/src/restricted_token_utils.h"
#include "sandbox/win/src/sandbox_rand.h"
#include "sandbox/win/src/win_utils.h"
// These are missing in 10.0.19551.0 but are in 10.0.19041.0 and 10.0.20226.0.
@ -547,12 +547,11 @@ bool ApplyProcessMitigationsToSuspendedProcess(HANDLE process,
// This is a hack to fake a weak bottom-up ASLR on 32-bit Windows.
#if !defined(_WIN64)
if (flags & MITIGATION_BOTTOM_UP_ASLR) {
unsigned int limit;
GetRandom(&limit);
char* ptr = 0;
const size_t kMask64k = 0xFFFF;
// Random range (512k-16.5mb) in 64k steps.
const char* end = ptr + ((((limit % 16384) + 512) * 1024) & ~kMask64k);
// Random range (512k-16.5mb) in 64k steps. 16895=512+16384-1.
unsigned int limit = static_cast<unsigned int>(base::RandInt(512, 16895));
const char* end = ptr + ((limit * 1024) & ~kMask64k);
while (ptr < end) {
MEMORY_BASIC_INFORMATION memory_info;
if (!::VirtualQueryEx(process, ptr, &memory_info, sizeof(memory_info)))

@ -1,22 +0,0 @@
// Copyright 2015 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "sandbox/win/src/sandbox_rand.h"
#include <windows.h>
// #define needed to link in RtlGenRandom(), a.k.a. SystemFunction036. See the
// "Community Additions" comment on MSDN here:
// http://msdn.microsoft.com/en-us/library/windows/desktop/aa387694.aspx
#define SystemFunction036 NTAPI SystemFunction036
#include <NTSecAPI.h>
#undef SystemFunction036
namespace sandbox {
bool GetRandom(unsigned int* random_value) {
return RtlGenRandom(random_value, sizeof(unsigned int)) != false;
}
} // namespace sandbox

@ -1,15 +0,0 @@
// Copyright 2015 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef SANDBOX_WIN_SRC_SANDBOX_RAND_H_
#define SANDBOX_WIN_SRC_SANDBOX_RAND_H_
namespace sandbox {
// Generate a random value in |random_value|. Returns true on success.
bool GetRandom(unsigned int* random_value);
} // namespace sandbox
#endif // SANDBOX_WIN_SRC_SANDBOX_RAND_H_