Get random data from OS APIs #13

wilzbach · 2016-12-28T08:51:04Z

So I managed to find the issue in the Windows API. It was due to wrong API headers in druntime - for 64-bit pointers need to have 8 bytes.

Let me summarize:

Current Behavior

Windows -> CryptGenRandom
Linux
- >= 3.17 -> getrandom system call, intro article
- otherwise: /dev/(u)?random (see e.g. /dev/random vs. /dev/urandom)
MacOS/OpenBSD/Solaris: /dev/(u)?random (see e.g. /dev/random vs. /dev/urandom)

Todo

decide on an API
write more documentation
add support for D attributes
add tests

Future work

OpenBSD -> getentropy (Not implemented as I don't have a OpenBSD atm - anyone?)

References

C++ API: http://en.cppreference.com/w/cpp/numeric/random/random_device
LLVM stdcxx: https://github.com/llvm-mirror/libcxx/blob/master/src/random.cpp
G++: https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/src/c%2B%2B11/random.cc
Python urandom: https://github.com/python/cpython/blob/master/Python/random.c
Vibe.d cryptorand: https://github.com/rejectedsoftware/vibe.d/blob/master/crypto/vibe/crypto/cryptorand.d
pycrypto: https://github.com/dlitz/pycrypto/blob/master/src/winrand.c

wilzbach · 2016-12-28T09:01:21Z

source/mir/random/engine/test.d

+            mov RDX, arg2[RBP];
+            syscall;
+            mov ret, RAX;
+        }


@9il Is dmd guaranteed to execute the asm block in one go?

What do you mean?

I got the inspiration to use this syscall style from here: https://github.com/kubo39/syscall-d/blob/master/source/syscalld/arch/syscall_x86_64.d
Hence also the synchronized statements, but afaict we don't need it - e.g.:
https://wiki.dlang.org/LDC_inline_assembly_expressions#X86-64

wilzbach · 2017-01-04T15:03:41Z

I accidentally came by two other implementation in the D ecosystem:

https://github.com/LightBender/SecureD/blob/master/source/random.d
https://github.com/etcimon/botan/blob/master/source/botan/libstate/libstate.d#L190

(the latter is interesting as it uses quite a bunch of entropy sources)

9il · 2017-01-04T15:06:21Z

I accidentally came by two other implementation in the D ecosystem:

Awesome!

9il · 2017-01-04T15:11:50Z

But do not spent much time on deprecated targets ;)

9il · 2017-01-07T16:31:01Z

@wilzbach do you need help with this PR?

wilzbach · 2017-01-07T17:18:29Z

@wilzbach do you need help with this PR?

Oh sorry for letting this get stalled.

As discussed this exposes two different functions with C-style like interface, but with D-guarantees in terms of @safe nothrow @nogc:

getRandomBlocking
getRandomNonBlocking

The difference between these two functions is that the Blocking one will loop until the buffer is filled (i.e. until enough entropy is generated), while the NonBlocking one will yield an error code.
If initialization needs to be done, it's done on the first call.

@9il please let me know if there's sth. missing for this PR.

About Blocking behavior:

tl;dr: only useful on Linux.

Linux docs state:

The /dev/random device is a legacy interface which dates back to a
time where the cryptographic primitives used in the implementation of
/dev/urandom were not widely trusted. It will return random bytes
only within the estimated number of bits of fresh noise in the
entropy pool, blocking if necessary. /dev/random is suitable for
applications that need high quality randomness, and can afford
indeterminate delays.

When the entropy pool is empty, reads from /dev/random will block
until additional environmental noise is gathered.

And for the newer getRandom:

GRND_NONBLOCK
By default, when reading from the random source, getrandom()
blocks if no random bytes are available, and when reading from
the urandom source, it blocks if the entropy pool has not yet
been initialized. If the GRND_NONBLOCK flag is set, then
getrandom() does not block in these cases, but instead
immediately returns -1 with errno set to EAGAIN.

Windows doesn't support blocking behavior (the documentation doesn't even state how CryptGenRandom behaves if the entropy pool is empty)
macOS doesn't support blocking either (according to the docs it isn't necessary to block as a special SecurityServer takes care of ensuring that there's always enough entropy)

9il

Each public function should be extern(C) and starts with mir_random_. mir_random_genRandomBlocking. An alias can be added: genRandomBlocking = mir_random_genRandomBlocking. Please replace get* with gen*.

9il · 2017-01-07T18:05:58Z

source/mir/random/engine/package.d

-    k ^= k >> 33;
-    return cast(size_t)k;
+    size_t seed;
+    //getRandomBlocking(&seed, seed.sizeof);


Lets preserve old version if getRandomBlocking returns -1

and uncomment this line)

9il · 2017-01-07T18:07:36Z

source/mir/random/engine/package.d

+    import core.sys.posix.sys.utsname : utsname;
+
+    // druntime isn't properly annotated
+    extern(C) int uname(utsname* __name) @nogc nothrow;


9il · 2017-01-07T18:08:17Z

source/mir/random/engine/package.d

+        return false;
+    }
+
+    extern(C) int syscall(size_t ident, size_t n, size_t arg1, size_t arg2) @nogc nothrow;


9il · 2017-01-07T18:10:19Z

source/mir/random/engine/package.d

+        {
+            auto res = syscall(GETRANDOM, cast(size_t) ptr + genBytes, len - genBytes, 0);
+            if (res >= 0)
+                genBytes -= res;


bug.
Just move ptr and reduce length, instead of genBytes variable

9il · 2017-01-07T18:11:01Z

source/mir/random/engine/package.d

+    import core.stdc.stdio : fclose, feof, ferror, fopen, fread;
+    alias IOType = typeof(fopen("a", "b"));
+    static private IOType fdRandom;
+    static private IOType fdURandom;


__gshared

9il · 2017-01-07T18:13:00Z

source/mir/random/engine/package.d

+    static private IOType fdRandom;
+    static private IOType fdURandom;
+
+    shared static ~this()


extern(C)

9il · 2017-01-07T18:13:22Z

source/mir/random/engine/package.d

+    extern(Windows) BOOL CryptGenRandom(HCRYPTPROV, DWORD, PBYTE) @nogc @safe nothrow;
+    extern(Windows) BOOL CryptAcquireContextA(HCRYPTPROV*, LPCSTR, LPCSTR, DWORD, DWORD) @nogc nothrow;
+    extern(Windows) BOOL CryptAcquireContextW(HCRYPTPROV*, LPCWSTR, LPCWSTR, DWORD, DWORD) @nogc nothrow;
+    extern(Windows) BOOL CryptReleaseContext(HCRYPTPROV, ULONG_PTR) @nogc nothrow;


9il · 2017-01-07T18:14:03Z

source/mir/random/engine/package.d

+        return 0;
+    }
+
+    shared static ~this()


9il · 2017-01-07T18:14:18Z

source/mir/random/engine/package.d

+    extern(Windows) BOOL CryptAcquireContextW(HCRYPTPROV*, LPCWSTR, LPCWSTR, DWORD, DWORD) @nogc nothrow;
+    extern(Windows) BOOL CryptReleaseContext(HCRYPTPROV, ULONG_PTR) @nogc nothrow;
+
+    private static ULONG_PTR hProvider;


__gshared

wilzbach · 2017-01-08T05:55:55Z

Each public function should be extern(C) and starts with mir_random_. mir_random_genRandomBlocking

Ok - questions:

It's currently in the mir.random.engine - so shouldn't it be mir_random_engine_genRandomBlocking?
This will look ugly on the documentation pages. Any trick for this? Can't we use extern(C) on an alias?
What about the module deconstructor? Should we expose sth. like mir_random_engine_dtor?

__gshared

I assume I can't use any locking in betterC?
There are four __gshared variables then. In case of a race:

hasGetRandom: Linux kernel version will be calculated twice (will always yield the same) -> not a problem
hProvider: we could acquire two Windows Crypto contexts (leak)
fdRandom / fdURandom: we could open the file twice and leak a file handle

We could provide a special init method which inits the handles and ensures that no leaks can occur. On the D side it could automatically be called with the shared module constructor

wilzbach · 2017-01-08T06:01:47Z

source/mir/random/engine/package.d

+            if (initGetRandom != 0)
+                return -1;
+
+        while(!CryptGenRandom(hProvider, len, cast(PBYTE) ptr)) {}


For Windows all error codes of CryptGenRandom seem unrecoverable, so the loop here doesn't seem very useful. I think we should directly yield the error here.

9il · 2017-01-08T08:37:37Z

It's currently in the mir.random.engine - so shouldn't it be mir_random_engine_genRandomBlocking?

no

This will look ugly on the documentation pages. Any trick for this? Can't we use extern(C) on an alias?

This should be a part of documentation.

What about the module deconstructor? Should we expose sth. like mir_random_engine_dtor?

Yes, but extern(C) should be added to the D's module destructor.

9il · 2017-01-08T08:40:00Z

We could provide a special init method which inits the handles and ensures that no leaks can occur. On the D side it could automatically be called with the shared module constructor

mir_random_engine_ctor and the extern(C) D's module constructor

9il · 2017-02-03T03:49:56Z

Hey @wilzbach , could you please finalise this PR?

wilzbach · 2017-03-02T22:59:32Z

Hey @wilzbach , could you please finalise this PR?

Sorry that I left this pending for such a long time.

mir_random_engine_ctor and the extern(C) D's module constructor

Added :)

9il · 2017-03-07T11:38:08Z

@wilzbach please convert all private functions to templates and add version(LDC) pragma(inline, true); for them. This will make object files clearer.

@wilzbach

Based on code originally written by @wilzbach libmir/mir-random#13

@wilzbach

Based on code originally written by @wilzbach libmir/mir-random#13

@wilzbach

Based on code originally written by @wilzbach libmir/mir-random#13

@wilzbach

Based on code originally written by @wilzbach libmir/mir-random#13

wilzbach mentioned this pull request Dec 28, 2016

[WIP][DO NOT MERGE] Test OS random APIs #12

Closed

wilzbach force-pushed the test-random-os-apis branch 2 times, most recently from 88aac41 to 655496f Compare December 28, 2016 08:58

wilzbach commented Dec 28, 2016

View reviewed changes

wilzbach force-pushed the test-random-os-apis branch 14 times, most recently from fc776c3 to 0c14f5a Compare December 29, 2016 07:43

add getRandom to query OS system APIs for random data

8c12586

wilzbach force-pushed the test-random-os-apis branch from 0c14f5a to 8c12586 Compare December 29, 2016 14:26

wilzbach mentioned this pull request Dec 29, 2016

Add attributes to core.thread dlang/druntime#1726

Merged

9 tasks

getRandomNonBlocking: add more comments

cb955a8

wilzbach force-pushed the test-random-os-apis branch from be9bb7c to cb955a8 Compare January 7, 2017 17:04

9il reviewed Jan 7, 2017

View reviewed changes

address @9il's review

f31951f

wilzbach added 2 commits January 8, 2017 06:40

cleanup imports of time/thread fallback

76fd960

Fix typo in import cleanup

8b5b012

wilzbach commented Jan 8, 2017

View reviewed changes

wilzbach added 2 commits January 8, 2017 07:03

reenable unittest

e0cdf2d

genRandomImplFileNonBlocking: fix check for possible errors

12fd5d0

Add mir_random_engine_ctor - an extern(C) way to init the engine module

01597d6

wilzbach force-pushed the test-random-os-apis branch from 2c4208c to 01597d6 Compare March 3, 2017 02:31

wilzbach mentioned this pull request Mar 3, 2017

RFC: Improve std.random.unpredictableSeed dlang/phobos#5230

Closed

9il added 2 commits September 26, 2017 10:56

Merge branch 'master' into test-random-os-apis

9a3bf25

Update dub.json

6a014fc

9il merged commit 65a07f9 into libmir:master Sep 26, 2017

n8sh added a commit to n8sh/vibe.d that referenced this pull request Mar 1, 2018

Use getrandom syscall on linux if available

d2b7e3a

Based on code originally written by @wilzbach libmir/mir-random#13

n8sh mentioned this pull request Mar 1, 2018

Use getrandom syscall on linux if available vibe-d/vibe.d#2093

Merged

n8sh added a commit to n8sh/vibe.d that referenced this pull request Mar 1, 2018

Use getrandom syscall on linux if available

cb20f10

Based on code originally written by @wilzbach libmir/mir-random#13

n8sh added a commit to n8sh/vibe.d that referenced this pull request Mar 1, 2018

Use getrandom syscall on linux if available

00d62da

Based on code originally written by @wilzbach libmir/mir-random#13

n8sh added a commit to n8sh/vibe.d that referenced this pull request Mar 1, 2018

Use getrandom syscall on linux if available

b142e55

Based on code originally written by @wilzbach libmir/mir-random#13

wilzbach mentioned this pull request Mar 22, 2018

Partially Fix Issue 18596: use arc4random when available for unpredictableSeed dlang/phobos#6267

Merged

n8sh mentioned this pull request Mar 30, 2018

Fix Issue 18595 & Issue 18596: add unpredictableSeed!UIntType, support non-arc4random entropy sources, & replace MindstdRand0 with SplitMix dlang/phobos#6388

Closed

wilzbach deleted the test-random-os-apis branch April 6, 2018 17:42

wilzbach mentioned this pull request Sep 6, 2020

WIP: OS Random number generator interface dlang/phobos#7619

Closed

Get random data from OS APIs #13

Get random data from OS APIs #13

Uh oh!

Conversation

wilzbach commented Dec 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current Behavior

Todo

Future work

References

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Jan 4, 2017

Uh oh!

9il commented Jan 4, 2017

Uh oh!

9il commented Jan 4, 2017

Uh oh!

9il commented Jan 7, 2017

Uh oh!

wilzbach commented Jan 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

About Blocking behavior:

Uh oh!

9il left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Jan 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

9il commented Jan 8, 2017

Uh oh!

9il commented Jan 8, 2017

Uh oh!

9il commented Feb 3, 2017

Uh oh!

wilzbach commented Mar 2, 2017

Uh oh!

9il commented Mar 7, 2017

Uh oh!

Uh oh!

wilzbach commented Dec 28, 2016 •

edited

Loading

wilzbach commented Jan 7, 2017 •

edited

Loading