Use RtlGenRandom() directly instead of going through rand_s().

base/rand_util_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/rand_util.h"
 
 #include <stdlib.h>
+#include <windows.h>
 
 #include "base/basictypes.h"
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 
 namespace {
+typedef BOOLEAN (WINAPI *RtlGenRandomPtr)(PVOID, ULONG);
 
-uint32 RandUint32() {
-  uint32 number;
-  CHECK_EQ(rand_s(&number), 0);
-  return number;
-}
+class WinRandom {
+ public:
+  WinRandom()
+      : rtl_gen_random_(reinterpret_cast<RtlGenRandomPtr>(
+            ::GetProcAddress(::GetModuleHandle(L"advapi32.dll"),

[bradn, 2014/01/22 22:31:29]

Any accomodation need to be made for failure here?
Or is it fair to assume all that talk of it being altered or unavailable in
subsequent versions is moot because srand_s uses it internally?

[wtc, 2014/01/22 22:42:04]

RtlGenRandom is already mentioned in some official Microsoft blog posts, so it
cannot be altered or removed without causing big problems.

[jschuh, 2014/01/22 23:04:24]

One thing I forgot. Contrary to popular belief, this is officially supported,
does have a proper import library, and doesn't need an explicit dynamic import.
See the note at this link regarding NTSecAPI.h:
http://msdn.microsoft.com/en-us/library/windows/desktop/aa387694(v=vs.85).aspx

[DaleCurtis, 2014/01/22 23:50:51]

This compiles and works using the workaround mentioned there.

+                             "SystemFunction036"))) {}
+  ~WinRandom() {}
+
+  void RtlGenRandom(void* output, uint32 output_length) const {
+    CHECK_EQ(rtl_gen_random_(output, output_length), TRUE);
+  }
+
+ private:
+  const RtlGenRandomPtr rtl_gen_random_;
+};
+
+base::LazyInstance<WinRandom>::Leaky g_win_random = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
 namespace base {
 
 // NOTE: This function must be cryptographically secure. http://crbug.com/140076
 uint64 RandUint64() {
-  uint32 first_half = RandUint32();
-  uint32 second_half = RandUint32();
-  return (static_cast<uint64>(first_half) << 32) + second_half;
+  uint64 number;

[Ryan Sleevi, 2014/01/22 22:12:26]

Free fixup while you're here -
http://src.chromium.org/viewvc/chrome/trunk/src/base/basictypes.h?revision=24...
(would have been nicer if we didn't split this out :P)

Not needed in this CL though, just FYI.

[DaleCurtis, 2014/01/22 22:27:53]

I've left this one alone for now since it requires changing the headers and all
implementations. I'll put together a separate cleanup CL to get them all.

+  g_win_random.Pointer()->RtlGenRandom(&number, sizeof(number));
+  return number;
 }
 
 void RandBytes(void* output, size_t output_length) {
-  uint64 random_int;
-  const size_t random_int_size = sizeof(random_int);
-  for (size_t i = 0; i < output_length; i += random_int_size) {
-    random_int = base::RandUint64();
-    size_t copy_count = std::min(output_length - i, random_int_size);
-    memcpy(((uint8*)output) + i, &random_int, copy_count);
+  char* output_ptr = static_cast<char*>(output);
+  const WinRandom* win_random_ptr = g_win_random.Pointer();
+  while (output_length > 0) {
+    const uint32 output_bytes_this_pass =

[Ryan Sleevi, 2014/01/22 22:12:26]

use uint32_t and use stdint.h -
https://code.google.com/p/chromium/codesearch#chromium/src/base/basictypes.h&...

[DaleCurtis, 2014/01/22 22:27:53]

Thanks for the pointer. I didn't realize this was the new hotness. Done.

+        std::min(output_length, static_cast<size_t>(kuint32max));

[Ryan Sleevi, 2014/01/22 22:12:26]

note: Don't you need a cast here, since you're providing two size_t types to
std::min(), it returns a size_t (the C++03 25.3.7 definition only takes one type
- T - not a return type)

const uint32_t output_bytes_this_pass = static_cast<uint32_t>(std::min(...))

[DaleCurtis, 2014/01/22 22:27:53]

Done.

[DaleCurtis, 2014/01/22 23:50:51]

Sadly this doesn't seem to work with MSVC, it refused to compile indicating that
the type was ambiguously "size_t" or "unsigned long".

[jschuh, 2014/01/23 17:24:13]

I think the explicit cast is actually correct, as MSVC has ramped up its
warnings for 64-bit unsafe coercions.

+    win_random_ptr->RtlGenRandom(output_ptr, output_bytes_this_pass);
+    output_length -= output_bytes_this_pass;
+    output_ptr += output_bytes_this_pass;
   }
 }
 
 }  // namespace base