Use hardware timestamps in RNG seeding #10322

sipa commented at 1:22 AM on May 3, 2017: member

Faster, and more entropy.

gmaxwell commented at 1:25 AM on May 3, 2017: contributor

utACK. Consider adding a unit test that calls it twice, with a sleep and makes sure the value changes?

in src/random.cpp:59 in 69e03dd7ca outdated

  53 | +    __asm__ volatile ("rdtsc" : "=A"(r));
  54 | +    return r;
  55 | +#elif defined(__x86_64__) || defined(__amd64__)
  56 | +    uint64_t r1, r2;
  57 | +    __asm__ volatile ("rdtsc" : "=a"(r1), "=d"(r2));
  58 | +    return (r2 << 32) | r1;

dcousens commented at 1:43 AM on May 3, 2017:

Maybe link to https://en.wikipedia.org/wiki/Time_Stamp_Counter for readers that don't know what this could be?

Also, I couldn't find a description on the constraints a or d, I assume they are just writing to the inline variables r1 and r2, but I found documentation on this to be difficult to find.

Also, is volatile necessary? It doesn't hurt I suppose?

dcousens commented at 1:47 AM on May 3, 2017:

If resuming from hibernation, might this counter be, or close to, zero?

sipa commented at 1:59 AM on May 3, 2017:

I added some comments, but I don't think this warrants a full explanation of extended asm syntax in comments.

Also, I couldn't find a description on the constraints a or d, I assume they are just writing to the inline variables r1 and r2, but I found documentation on this to be difficult to find.

See https://gcc.gnu.org/onlinedocs/gcc-4.9.0/gcc/Machine-Constraints.html#Machine-Constraints

"A" is the ax:dx register pair, so on x86 we constrain the r variable to be 64 bits consisting of 32 bits in ax and 32 bits in dx. Since rdtsc always stores its result in ax/dx, we get the correct result in the C++ variable r. "a" and "d" just constrain to the (r)ax and (r)dx registers, which is where the individual parts of the rdtsc result are stored on x86_64.

Also, is volatile necessary? It doesn't hurt I suppose?

It's there to limit the optimizations the compiler can apply to the assembly block. See https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile

sipa commented at 2:01 AM on May 3, 2017:

If resuming from hibernation, might this counter be, or close to, zero?

I don't think that's a concern. It's still a fine entropy source. This thing counts in clock cycles (~nanoseconds and less, usually). If an attacker knows how many nanoseconds ago your last wakeup from hibernation was, you probably have bigger problems.

dcousens commented at 2:28 AM on May 3, 2017:

Thanks for the explanations and links @sipa, I had been searching for that list but couldn't find it.

sipa force-pushed on May 3, 2017

dcousens approved

dcousens commented at 2:27 AM on May 3, 2017: contributor

utACK

laanwj added the label Utils and libraries on May 3, 2017

in src/random.cpp:51 in 88f73de063 outdated

  47 | -#ifdef WIN32
  48 | -    QueryPerformanceCounter((LARGE_INTEGER*)&nCounter);
  49 | +    // Read the hardware time stamp counter when available.
  50 | +    // See https://en.wikipedia.org/wiki/Time_Stamp_Counter for more information.
  51 | +#if defined(_MSC_VER)
  52 | +    return __rdtsc();

TheBlueMatt commented at 11:37 PM on May 3, 2017:

Does MSVC support ARM now?

sipa commented at 12:03 AM on May 4, 2017:

No idea. But their documentation doesn't specify any restrictions on when __rdtsc is available.

TheBlueMatt commented at 2:37 AM on May 4, 2017:

Looks like it may not be https://docs.microsoft.com/en-us/cpp/intrinsics/rdtsc Maybe move this inside the x86 ifdef checks as well?

TheBlueMatt commented at 2:37 AM on May 4, 2017:

(Compare to https://msdn.microsoft.com/en-us/library/wfd9z0bb.aspx

sipa commented at 8:02 AM on May 4, 2017:

MSVC has different macros. I've solved it in a way that seems right, but I can't test on MSVC.

sipa force-pushed on May 4, 2017

sipa commented at 8:03 AM on May 4, 2017: member

Switched to using C++11 std::high_precision_clock instead of gettimeofday (which on Linux and OSX seems to use a nanosecond precision timer, and microsecond precision in MinGW).

TheBlueMatt commented at 2:54 PM on May 4, 2017: member

utACK de485d50f9fc647407acd848856bacf108b87172

theuni commented at 6:50 PM on May 4, 2017: member

Consider adding a unit test that calls it twice, with a sleep and makes sure the value changes?

Better yet, make this a runtime sanity check?

sipa commented at 6:52 PM on May 4, 2017: member

Better yet, make this a runtime sanity check?

That's both more powerful and easy. However, how long do we sleep? Is a 10ms slowdown at startup acceptable?

in src/random.cpp:52 in de485d50f9 outdated

  50 | +    // Read the hardware time stamp counter when available.
  51 | +    // See https://en.wikipedia.org/wiki/Time_Stamp_Counter for more information.
  52 | +#if defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))
  53 | +    return __rdtsc();
  54 | +#elif !defined(_MSC_VER) && defined(__i386__)
  55 | +    uint64_t r;

theuni commented at 6:53 PM on May 4, 2017:

Mind initializing these? IIRC valgrind isn't always able to follow inline asm and reports uninitialized values.

sipa commented at 6:56 PM on May 5, 2017:

Done.

theuni commented at 6:58 PM on May 4, 2017: member

How about comparing values taken from beginning/end of the sanity checks?

dcousens commented at 12:13 AM on May 5, 2017: contributor

Is a 10ms slowdown at startup acceptable?

If it was, you could run this in an alternate thread?

TheBlueMatt commented at 6:07 PM on May 5, 2017: member

Just sleep 1ms, if it doesnt advance (because you only have a millisecond clock on your hardware) you're probably not on a machine that will ever finish chainsync anyway.

sipa commented at 6:32 PM on May 5, 2017: member

How about comparing values taken from beginning/end of the sanity checks?

Just sleep 1ms, if it doesnt advance (because you only have a millisecond clock on your hardware) you're probably not on a machine that will ever finish chainsync anyway.

Done a combination of both.

Use hardware timestamps in RNG seeding f544094d5e

Test that GetPerformanceCounter() increments 33f853d8d8

Use sanity check timestamps as entropy 2c0a6f157d

sipa force-pushed on May 5, 2017

TheBlueMatt commented at 7:34 PM on May 5, 2017: member

utACK 2c0a6f157da3c6bb3b0a1e77f003caf0d9cb9d6c

theuni commented at 7:43 PM on May 8, 2017: member

Thanks. utACK 2c0a6f157da3c6bb3b0a1e77f003caf0d9cb9d6c

jonasschnelli commented at 6:35 AM on May 9, 2017: contributor

utACK 2c0a6f157da3c6bb3b0a1e77f003caf0d9cb9d6c

sipa merged this on May 9, 2017

sipa closed this on May 9, 2017

sipa referenced this in commit bc64b5aa0f on May 9, 2017

sipa deleted the branch on Jun 23, 2017

PastaPastaPasta referenced this in commit 7df883c5b9 on Jun 10, 2019

PastaPastaPasta referenced this in commit 02e2ac70ef on Jun 10, 2019

PastaPastaPasta referenced this in commit 26d49650df on Jun 10, 2019

PastaPastaPasta referenced this in commit 76d5bdaaa0 on Jun 11, 2019

PastaPastaPasta referenced this in commit 0b96205d8d on Jun 11, 2019

PastaPastaPasta referenced this in commit ab9e682c32 on Jun 15, 2019

PastaPastaPasta referenced this in commit ff2e70ce98 on Jun 19, 2019

PastaPastaPasta referenced this in commit f587bf9e13 on Jun 19, 2019

PastaPastaPasta referenced this in commit f19a945df0 on Jun 19, 2019

PastaPastaPasta referenced this in commit bf8cf79b7f on Jun 19, 2019

PastaPastaPasta referenced this in commit cfac3f16a7 on Jun 19, 2019

PastaPastaPasta referenced this in commit b13d92079d on Jun 19, 2019

PastaPastaPasta referenced this in commit bec086380e on Jun 19, 2019

PastaPastaPasta referenced this in commit b0b470df74 on Jun 20, 2019

barrystyle referenced this in commit e3393dae02 on Jan 22, 2020

DrahtBot locked this on Sep 8, 2021