Use arc4random for entropy on supported systems #9677

This commit is half the fix for issue #9676 .

The purpose is to use arc4random() for entropy instead of "/dev/urandom", on supported systems, so that BitcoinD hence will function in environments where there is no "/dev/urandom", which can be the case for various reasons (nodev filesystem, chroot, etc.).

** To function, this commit must be complemented with autodetection of arc4random() support and accordingly setting the arc4random_supported define when found.

Someone else make such a commit please. Thanks!

I dont think we should be using an RC4-based stream cipher for our crypto random stuff. Possibly in conjunction with /dev/urandom and other sources, but not by itself. Given this is only used in conjunction with OpenSSL's rand, its likely fine, but I super dont trust OpenSSL's rand either, so I'd say NACK.

Please do not submit incomplete patches, certainly not one-liners. No one is just suddenly going to do your work for you, that's not how open source works, sorry.

@TheBlueMatt The arc4random functioms are the way to obtain kernel randomness in some (all?) BSDs, even though they've stopped using the RC4 method internally.

OP, you'll need to make your patch work though.

@sipa ahh, that was not at all clear to me from man pages I saw, thanks.

Hi, sorry for the incomplete patch, will fix that.

arc4random() is the primary randomness interface on OpenBSD and all programs use it for everything.

I submitted this so that BitcoinD will function even if there's no "/dev/urandom" device. BitcoinD's current behavior is to terminate if that device not exists, which is a disaster as there are situations when it's totally untrivial or/and unsuitable to create such a device. (Also, why depend on the device's existence when it's technically unnecessary in the first place.)

Perhaps the very simplest approach would be to apply the arc4random() interface specifically to OpenBSD. Anyhow i'll intend to come up with a complete patch and refer to it here. Be back in a bit.

Also, why depend on the device's existence when it's technically unnecessary in the first place

We're relying on the availability of strong randomness, and won't run without. I'm perfectly fine with using a specific kernel interface instead of /dev/random if that's available.

Great. Before crafting the patch: Do you guys prefer that arc4random is used on all platforms where it's supported, or specifically on OpenBSD?

As soon as that has been clarified, we just need to ensure that a properly scoped define is in the code, and then commit the code patch I suggested (but depending on the proper define), that's all.

One approach here would be to use arc4random() on any arc4random-supported platform, as failover when /dev/urandom not is existent. That would make the risk profile in BitcoinD's entropy sampling unnecessarily complex though.

arc4random seems to not be solid since long on other platforms than OpenBSD, so that would be FreeBSD and Mac OS X.

For that reason, for the time being, and for the sake of code conciseness, adding an OpenBSD-specific clause where on OpenBSD specifically arc4random() is used only, may be preferable.

Looking through the code, I do not see any OpenBSD-specific DEFINE - there is "LEVELDB_TARGET_FLAGS" but that one should be used by LevelDB code only, I guess.

Then there is "TARGET_OS" which only has the options "linux", "windows" and "darwin" currently.

If I read configure.ac right, there are TARGET_WINDOWS and TARGET_DARWIN defines (and no TARGET_LINUX), but, they are not use anywhere in the code.

An OpenBSD-specific define could be introduced in the OpenBSD detection code in configure.ac, that is here: https://github.com/bitcoin/bitcoin/blob/be31a2b3635b893930c731a2929bb593dae9847e/configure.ac#L380 .

From my place with limited insight, I would guess just adding a TARGET_OPENBSD define there in configure.ac would be sensible, and then use that one in src/random.cpp GetOSRand().

Thoughts?

@theuni could you comment on the build system question ^^

@mlmikael Generally we don't run platform-specific tests, rather we run feature-tests. That way if another platform just happens to implement the thing, it will just work without us having to keep up with a list.

See the check for MSG_NOSIGNAL in configure.ac for example.

You'll want to do something like:

AC_MSG_CHECKING(for arc4random)
AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <stdlib.h>]],
 [[ uint32_t f = arc4random(); ]])],
 [ AC_MSG_RESULT(yes); AC_DEFINE(HAVE_ARC4RANDOM, 1,[Define this symbol if you have arc4random]) ],
 [ AC_MSG_RESULT(no)]
)

Then in the code, you can use #ifdef HAVE_ARC4RANDOM.

All that ignores the "should we" part. I'm not sure if we actually want to use it unconditionally if it's found, in this case, we might actually want it whitelisted to certain platforms.

@theuni , noted and understood the parts about how to bring in the HAVE_ARC4RANDOM define.

The finishing part "All that ignores the "should we" part. I'm not sure if we actually want to use it unconditionally if it's found, in this case, we might actually want it whitelisted to certain platforms.", do you mean that we also should make a "TARGET_OPENBSD" DEFINE or alike as a way to whitelist OpenBSD for arc4random use?

When I looked around among platforms with arc4random support, I ran into OpenBSD, FreeBSD and Mac OS X, and it seems that it's only well developed and "a big deal" on OpenBSD, whereas FreeBSD and Mac OS X ran with an obsolete algorithm at least until recently, not sure.

Depending on how good entropy GetOSRand() needs, the arc4random() could either be used exclusively on all platforms where it's implemented, or, it could be used as a failover for when opening /dev/urandom fails.

Thoughts?

@theuni im not sure there's anyway to do a proper feature test here, arc4random is no longer arc4 on openbsd and other systems which implement it; however it was at one point so the feature test would have to be for a secure arc4random implementation and not just arc4random

arc4random is no longer arc4 on openbsd and other systems which implement it

So we only want "non-arc4 that's called arc4random". That's horribly confusing, and sounds like a really dangerous situation for something this critical.

Depending on how good entropy GetOSRand() needs, the arc4random() could either be used exclusively on all platforms where it's implemented, or, it could be used as a failover for when opening /dev/urandom fails.

It's used for the wallet so entropy is extremely important. Erroring out is 100x preferable to producing reproducible or insecure keys.

(on the flip side, if the wallet is disabled, there's nothing that secure randomness is very important for in bitcoind)

I've read about this a bit deeper: arc4random as a kernel-supported cryptographic randomness source is indeed OpenBSD specific. Internally it uses the getentropy(2) system call (which we could also use directly. It is also available on solaris ).

On other OSes with the arc4random function in libc (such as FreeBSD) it's really arc 4 random, and we really don't want to use it.

So all in all, I'd prefer making this strictly OpenBSD specific and not have it depend on availability of the function.

I guess it would make sense to make arc4random use OpenBSD-specific. What do you think?

In that case, we need some define for #ifdef-ing the OpenBSD target. @theuni and others, how do you suggest we do this, introduce a TARGET_OPENBSD define?? (e.g. here https://github.com/bitcoin/bitcoin/blob/be31a2b3635b893930c731a2929bb593dae9847e/configure.ac#L382 )

https://download.libsodium.org/doc/generating_random_data/ shows how LibSodium does it:

"On Windows systems, the RtlGenRandom() function is used On OpenBSD and Bitrig, the arc4random() function is used On recent Linux kernels, the getrandom system call is used (since Sodium 1.0.3) On other Unices, the /dev/urandom device is used"

Perhaps this behavior can be borrowed.

@mlmikael Rather than worrying about the platform in the code, we can just do the feature-check for known-supported platforms. So you could insert the check i pasted above, but inside the "*openbsd*)" case that you linked to.

Or if it's really guaranteed to be there, we could just skip the check and unconditionally AC_DEFINE(HAVE_ARC4RANDOM) there.

That way we still only have to check for #ifdef HAVE_ARC4RANDOM in code, but it's already been filtered so that it only includes the platforms that we want (only openbsd in this case). This lets us reason about what options are available, rather than what platform we're building for.

@theuni , arc4random is guaranteed to be there and provide excellent entropy on OpenBSD, see https://www.openbsd.org/papers/hackfest2014-arc4random/index.html . OpenBSD reads arc4random "A-Replacement Call For(4) /dev/Random".

Maybe we should call it "HAVE_HIGHENTROPY_ARC4RANDOM" or "HAVE_GOOD_ARC4RANDOM" or perhaps simply "TARGET_OPENBSD"?

And define that under "openbsd)". (If any very low entropy implementation would be fine, we could just check for arc4random support on any platform and use it if available, though I guess the above makes more sense. The above is also in line with what LibSodium does.)

Please let me know and i'll craft a pull request accordingly.

Edit: misunderstood @theuni, he got this correct.

(@laanwj @theuni , Bitrig is an OpenBSD derivate so I think Bitrig trigs the "openbsd)" section in configure.ac too. So, any OpenBSD-specific treatment we add will supposedly automatically also cover Bitrig.)

@theuni and everyone, bump! -

So: How do we add a DEFINE that's unique to OpenBSD (and hence by implication to Bitrig)?

That's what we need to do to be able to add an #elif-something for the arc4random usecase.

https://github.com/bitcoin/bitcoin/blob/be31a2b3635b893930c731a2929bb593dae9847e/configure.ac#L382 this is a good place to add that define. So we'll add an AC_DEFINE(something) line there.

Should we call it "TARGET_OPENBSD", does that make sense, or should we give it some other name? "USE_ARC4RANDOM"?? Please advise @theuni , @TheBlueMatt , @sipa and others, thanks!

Will follow up to your response, with a mature pull request.

We actually discussed this last IRC meeting. The preference would be to bypass arc4random (which as discussed above is a risk on some platforms), and to use the syscalls directly when available to get randomness directly from the kernel. GetOSRandom could use:

• getrandom(buf, buflen, 0) on Linux
• getentropy(buf, buflen) on OpenBSD
• sysctl(KERN_ARND) on FreeBSD and NetBSD

Other BSDs may have one of these too. Unlike for arc4random we can simply detect availability of these in the build system without worrying about the specific platform.

Neat!

So checks for getrandom() and getentropy() will be added, e.g. under the define names HAVE_GETRANDOM and HAVE_GETENTROPY .

Who will implement this? This is beyond my automake/configure.ac skills, maybe @theuni can do it.

I'll can have a try at this. I have all those OSes available anyway.

@mlmikael see #9821