Add Cargo feature for CPU randomness

[josephlr]

Closes #84

cpu Cargo feature

Many CPU architectures support instructions to get randomness:

• x86/x86_64 have the well-known RDRAND/RDSEED
• ARMv8.5-RNG recently added RNDR (LLVM patch, Linux patch)
• PowerPC (version 3.0B of the ISA, i.e. POWER9 and later) has DARN (Deliver A Random Number)
• S390 supports PRNO (perform PseudoRandom Number Operation) Linux code

This PR adds a cpu Cargo feature, which calls those instructions if there is not a supported implementation. Right now we only support x86_64. This cpu feature is on by default (as there's no real reason to disable it, other than wanting to make sure all buildable targets have a concrete implementations). This feature takes lower priority than custom implementations. This is necessary to have specific RNG implementations on unsupported platforms.

We can now drop explicit support for targets where we used RDRAND simply because we didn't know what else to use. This includes:

• x86_64-unknown-hermit
• x86_64-unknown-l4re-uclibc
• x86_64-unknown-uefi

As the cpu feature is on by default, these targets will keep working by default, but they can now be overridden by custom implementations.

We continue to explicitly support all SGX targets. This means that custom implementations cannot, override the RDRAND implementation. Intel makes it very clear you have to use RDRAND to get randomness on SGX. The other targets don't have this mandate. For example, UEFI could get it's randomness form either RDRAND or EFI_RNG_PROTOCOL.

Testing improvements

I also took some time to fix the test structure. Now, there is a test_common module, which various test files import. This still allows for us to write tests once, but removes the need for hacky Cargo features. I then cleaned up some of the testing in .travis.yml. I also removed duplicate tests for minimum dependency versions.

We also now test the CPU-based RNG implementation on all x86 targets. This makes it possible to directly test on Linux, instead of needing a custom target.

[dhardy]

Good rationale for using a separate cpurand-getrandom crate, though maybe it should be called getrandom-cpu?

The code looks reasonable. The tests — I think you removed min-dependency-version tests?

[newpavlov]

On x86, s390, some ARM chips, and some PowerPC chips, CPUs support instructions to get randomness

AFAIK ARM does not have a dedicated CPU instruction for retrieving random numbers and instead dedicated peripherals are used, so you can't rely on target triplet here. I also couldn't find PowerPC instruction (although it was a shallow search). I think it may be worth to introduce getrandom-rdrand crate instead and work on other targets in separate crates.

[josephlr]

Good rationale for using a separate cpurand-getrandom crate, though maybe it should be called getrandom-cpu?

@dhardy, good idea. I switched the crates to be named getrandom-* instead of *-getrandom. This changes the name scheme we talked about in #109 (comment), but that seems fine. Having getrandom, getrandom-cpu, getrandom-stdweb, etc... makes it easier to see that these crates go together.

The code looks reasonable. The tests — I think you removed min-dependency-version tests?

Fixed. I also cleaned up the Travis scripts in general, let me know what you think.

AFAIK ARM does not have a dedicated CPU instruction for retrieving random numbers and instead dedicated peripherals are used, so you can't rely on target triplet here. I also couldn't find PowerPC instruction (although it was a shallow search). I think it may be worth to introduce getrandom-rdrand crate instead and work on other targets in separate crates.

@newpavlov I updated the OP to document the instructions on ARM, Power, and s390. At lot of these are very new, but were essentially introduced to be the counterparts of Intel's RDRAND. Particularly in the case of ARM and Power, these CPU instructions were introduced to avoid needing separate drivers for each board (i.e. the current state of affairs on ARM/Power).

Given these instructions, do you think it makes sense to have a getrandom-cpu and just add arch support as needed?

[newpavlov]

@josephlr
Thank you for the links! IIUC we will not be able to properly use it until rust-lang/rfcs#2725 gets accepted and implemented, correct? (addition of respective intrinsics goes without saying)

So we have two use-cases (see my other message). Ideally we should separate "use a custom CPU-based getrandom" and "give me a CPU-based HWRNG without overwriting getrandom". The second use-case is already covered by the rdrand crate (modulo potential ARM/PowerPC support). We may introduce a competitor crate for rdrand with a wider range of supported platforms (let's call it rand-cpu for now), then I think getrandom-cpu should focus solely on the first use-case and rand-cpu on the second. To reduce duplication getrandom-cpu can use rand-cpu under the hood. We may even use rand-cpu as a target-specific dependency of getrandom for SGX targets.

[dhardy]

"give me a CPU-based HWRNG without overwriting getrandom"

I'm not sure that we should care about this use-case? Is there a significant set of users? As you say, the rdrand crate already does provide this.

It also directly exposes a function getrandom_cpu::cpurand ...

Exposing this directly for testing / niche purposes seems acceptable to me (even though it is of limited utility). It is a small API extension with no extra code, and I don't think there's any reason we'd need to make a breaking change here (without also breaking the main getrandom function).

[newpavlov]

I'm not sure that we should care about this use-case?

We may support wider range of targets with such crate, plus it would allow us to fix bug faster (e.g. rdrand still does not contain a workaround for the AMD bug). As for potential users, I am not sure.

Exposing this directly for testing / niche purposes seems acceptable to me (even though it is of limited utility).

For testing it should be enough to use module-level tests, no need for exposing this function publicly. I am not sure why you brought breaking changes into this discussion. The approach, which I've drafted, will not introduce any breaking changes.

As for keeping RDRAND code for SGX targets, I think it may be worth to remove it from getrandom and tell users to use getrandom-cpu or other custom option instead. Since IIUC "the official" way for reading randomness on those targets is to use sgx_read_rand.

[dhardy]

From the OP:

We can now drop support for targets where we used RDRAND simply because we didn't know what else to use.

It may still be desirable to make these targets work by default. Note that libs should definitely not be importing a crate like getrandom-cpu since it precludes the use of a more specific implementation, such as a hypothetical getrandom-sgx crate. This means that there is no (good) way for a lib to include rand and have it "just work" on these targets.

[josephlr]

@newpavlov @dhardy I implemented my plan from #84 (comment), I rewrote the commit history, and the tests are now passing.

PTAL, each of the 5 commits is mostly self contained (be sure to keep the history when merging).

The only remaining question is: What do we do for SGX targets? Right now, they are marked as explicitly supported, as RDRAND is how Intel implements sgx_read_rand (the supported way to get randomness on SGX w/ C/C++). We could just lump SGX in with the other x86_64 freestanding targets, but that doesn't seem quite right to me. Thoughts?

[dhardy]

LGTM

[newpavlov]

What do we do for SGX targets?

I guess we can leave it as-is, or if the cpu feature will be removed (see my comment below), then it can be lumped together with other x86-64 targets without issues.

UPD: Essentially we have to decide if our support of SGX via RDRAND is on the same level as for targets with OS or not, i.e. should we give users an ability to overwrite it or not. I am fine with both options.

[josephlr]

What do we do for SGX targets?

I guess we can leave it as-is, or if the cpu feature will be removed (see my comment below), then it can be lumped together with other x86-64 targets without issues.

UPD: Essentially we have to decide if our support of SGX via RDRAND is on the same level as for targets with OS or not, i.e. should we give users an ability to overwrite it or not. I am fine with both options.

If we have the CPU feature off by default, we should still have the SGX targets build by default, as the fortanix sgx taget is Tier 2 with full 'std' support. So I think having SGX be "fully suported" is fine.