Check if sys/random.h is required for getentropy. #10301

This should check and include sys/random.h if required for osx as mentioned here.

utACK (can someone on MacOSX 10.12+ test this please?)

I tested this on MacOSX 10.12.4 myself, should we have a runtime /dev/random fallback for getentropy like we do for the linux SYS_getrandom?

I tested this on MacOSX 10.12.4 myself, should we have a runtime /dev/random fallback for getentropy like we do for the linux SYS_getrandom?

As I understand it, MacOSX guarantees that anything compiled with the MacOSX 10.12 SDK, which is necessary to get autotools to detect this in the first place doesn't work on earlier versions of the OS. In which case this would be unnecessary.

Feel free to correct me though @jonasschnelli @theuni.

@laanwj macOS uses weak imports for new symbols. They're fine to use as long as they're checked at runtime. This is how we're able to build with 10.11 (soon 10.12), but remain back-compat to 10.8. See an example of runtime handling at MacNotificationHandler::hasUserNotificationCenterSupport.

getentropy does not exist in the 10.11 sdk. So I'm assuming that it is a weak import introduced in 10.12. Meaning that this would (I believe) blow up at runtime on <= 10.11 when built with 10.12.

@theuni I'm unsure whether your comment above means that this PR is fine or the opposite.

@sipa Sorry for being unclear. Cautiously, the opposite. I believe that this PR would cause runtime crashes when building with SDK >= 10.12, and running on < 10.12. We definitely need someone to test that scenario before merge, as 10.12 will likely be the SDK we use for 0.15.

@theuni so this means we need to do a runtime check and fallback like we do for SYS_getrandom?

Something like this maybe?

if (getentropy != NULL) {
    if (getentropy(ent32, NUM_OS_RANDOM_BYTES) != 0) {
        RandFailure();
    }
} else {
    GetDevURandom(ent32);
}

@jameshilliard Yes. According to the apple docs it would look something like:

#if defined(HAVE_GETENTROPY_RAND)
#if defined(MAC_OSX)
if (getentropy != NULL) {
    getentropy(foo, bar);
} else {
// fallback
}
#else
getentropy(foo, bar);
#endif
#endif

I've added a runtime fallback but I don't have an OSX system with anything older than 10.12 to test against.

Apparently the Solaris version of getentropy is not safe to use directly for cryptographic functions unlike OpenBSD and OSX and getrandom should be used there instead.

On Solaris the output of getentropy(2) is entropy and should not be used where randomness is needed, in particular it must not be used where an IV or nonce is needed when calling a cryptographic operation. It is intended only for seeding a user space RBG (Random Bit Generator) system. More specifically the data returned by getentropy(2) has not had the required FIPS 140-2 processing for the DRBG applied to it.

It is intended only for seeding a user space RBG (Random Bit Generator) system.

Great, that's what we're doing.

I'll give it a test (build with 10.12 and run it own 10.8). Will report soon.

And I agree with @theuni (Using getentropy – which way introduced in 10.12 would crash in < 10.12). Here's the 10.12 manpage for genentropy for the records:

GETENTROPY(2)               BSD System Calls Manual              GETENTROPY(2)

NAME
     getentropy -- get entropy

SYNOPSIS
     #include <sys/random.h>

     int
     getentropy(void *buf, size_t buflen);

DESCRIPTION
     getentropy() fills a buffer with random data, which can be used as input
     for process-context pseudorandom generators like arc4random(3).

     The maximum buffer size permitted is 256 bytes.  If buflen exceeds this,
     an error of EIO will be indicated.

     getentropy() should be used as a replacement for random(4) when random
     data derived directly from the kernel random byte generator is required.
     Unlike the random(4) pseudo-devices, it is not vulnerable to file
     descriptor exhaustion attacks and is available when sandboxed or in a
     chroot, making it more reliable for security-critical applications.

     However, it should be noted that getentropy() is primarily intended for
     use in the construction and seeding of userspace PRNGs like arc4random(3)
     or CC_crypto(3).  Clients who simply require random data should use
     arc4random(3), CCRandomGenerateBytes() from CC_crypto(3), or
     SecRandomCopyBytes() from the Security framework instead of getentropy()
     or random(4)

RETURN VALUES
     Upon successful completion, the value 0 is returned; otherwise the
     value -1 is returned and the global variable errno is set to indicate the
     error.

ERRORS
     getentropy() will succeed unless:

     [EINVAL]           The buf parameter points to an invalid address.

     [EIO]              Too many bytes requested, or some other fatal error
                        occurred.

SEE ALSO
     arc4random(3) CC_crypto(3) random(4)

HISTORY
     The getentropy() function appeared in OSX 10.12

BSD                             October 2 2015                             BSD

Apparently the Solaris version of getentropy is not safe to use directly for cryptographic functions unlike OpenBSD and OSX and getrandom should be used there instead.

Most of the OSes warn against using kernel entropy directly for cryptographic primitives. For example OpenBSD:

     getentropy() is not intended for regular code; please use the
     arc4random(3) family of functions instead.

The foremost reason for this is because of the performance characteristics of calling the kernel for every 256 bytes of random data that is needed, the functionality is not set up for high-bandwidth randomness.

Solaris' reason is more interesting [

More specifically the data returned by getentropy(2) has not had the required FIPS 140-2 processing for the DRBG applied to it.

] This seems to imply that the function returns lower-quality instead of higher-quality entropy?!

In any case we're fine - we always combine the output with that of the OpenSSL random number generator, the OS entropy is added as an extra safety measure.

This has a lot of discussion but not really (ut)ACKs. Is the current approach ok, anything left to do here?

This is no longer relevant after #10855 (only use getentropy on openbsd)

@laanwj It seems #10855 would not result in getentropy being included on systems like OSX, wouldn't this still be needed?

That's intentional, getentropy is only to be used on OpenBSD as there it's a system call. On OSX and Linux it's emulation of something at most so there it's not used.

Edit: hrmph, on so OSX 10.12 it's a system call too, so in principle it could be used, but you'd have to ask @theuni.

on so OSX 10.12 it's a system call too, so in principle it could be used, but you'd have to ask @theuni.

Yeah, that was the main purpose of this PR.

Reopening then, I had understood from the title that it just adds a check, which would no longer be relevant when the function is only used on obsd. Needs rebase.

This seems to imply that the function returns lower-quality instead of higher-quality entropy?!

Nah, FIPS 140-2 processing means standard mandated low qualityness. :P Basically they do post processing to make sure they don't return all zeros or whatnot, which means (negligibly) lower entropy. The original motivation is to make certain kinds of hardware RNG failures fail secure instead of fail to making your keys all zeros; but then people go cargo-cult applying these tests on far end of cryptographic whitening, where they make no sense.

In our case we use it to feed our own entropy pool, so that processing isn't useful for us (to the extent that it's useful anywhere).

I've modified this to check for OSX explicitly. Should I try and make this more universal or is that good enough for now?

I think this is ok as-is. It'd be nice to add a little refactor on top (later) to:

• early-return on success, and have a single fall-back for /dev/urandom
• Move detection out to a separate function so that we can log what's in use.

utACK ee2d10ad0c0e04d0b9da4535a6fff265ac2501e5. I'm a little paranoid that an overly clever linker might try to optimize this check out of lto builds, but the crash would be obvious in that case.