Add RNG for Fortanix SGX #883
Conversation
|
This PR could be greatly simplified by just using the implementation from See the implementation: https://github.com/rust-random/getrandom/blob/master/src/rdrand.rs This implementation also fixes two issues present in this PR:
|
|
@josephlr Given the desire for a small TCB inside an SGX enclave, I'm in favor of reducing the number of crates required. Given that Rust already has a stable |
Currently, ring doesn't provide a random number generator for SGX at all. This is probably by accident, but is, in fact, the correct behavior. Due to distrust of the OS, SGX enclave applications need a source of randomness that is not forgable by the OS. This pretty much leaves only the RDRAND instruction. Fortunately, Rust provides an intrinsic for this instruction.
|
@josephlr The latest push uses the stable |
Oh ya, I should have clarified what I meant. Just use the |
|
It looks like the only test failure was a spurious one. Regarding the test coverage decrease, I'm not sure how to test this. @lkatalin Do you know how to write tests that run inside SGX? |
|
@npmccallum I could certainly give it a try. |
| // Section 5.2.1 | ||
| for _ in 0..10 { | ||
| let mut r = 0; | ||
| if unsafe { _rdrand64_step(&mut r) } == 1 { |
There was a problem hiding this comment.
Explain why this is safe (i.e. that SGX always supports RDRAND).
| } | ||
|
|
||
| pub fn fill(mut dest: &mut [u8]) -> Result<(), error::Unspecified> { | ||
| while dest.len() > 0 { |
There was a problem hiding this comment.
This can me made more clear (and more efficient by eliding memcpy calls) by using chunks_exact_mut, like the getrandom implementation does.
See the comparison here: https://rust.godbolt.org/z/gwprxd
| use core::arch::x86_64::_rdrand64_step; | ||
| use crate::error; | ||
|
|
||
| #[inline] |
There was a problem hiding this comment.
#[inline] does nothing on a private function, the attribute is only useful for pub functions/methods.
| use crate::error; | ||
|
|
||
| #[inline] | ||
| fn rdrand() -> Result<u64, error::Unspecified> { |
There was a problem hiding this comment.
If you end up having multiple call-sites for this function (i.e. if you make the chunks_exact_mut changes), it might be worth having this function return a [u8; 8] instead of a u64.
If you just want to check that the RDRAND implementation is good, you could test this on a normal x86_64 machine (without SGX) and just do the CPUID check. |
Do not do this, because the license is not compatible. I will probably implement this myself so I don't have to worry about the license implications. Have somebody with spending authority email me: [email protected] if/when you want to see progress on this. |
|
@briansmith I did not use |
This is incorrect, |
|
There's already a PR for this. |
#738 I'm assuming |
I have a small number of days per month I work on stuff that isn't paid for. This has resulted in a backlog, unfortunately. Three are a variety of factors I use to prioritize the backlog. The #1 thing somebody can do to get items on the backlog prioritized higher is sponsor this project financially. That's not the only criteria, though. |
Currently, ring doesn't provide a random number generator for SGX at all.
This is probably by accident, but is, in fact, the correct behavior.
Due to distrust of the OS, SGX enclave applications need a source of
randomness that is not forgable by the OS. This pretty much leaves only
the RDRAND instruction. Fortunately, LLVM provides an intrinsic for this
instruction.
Compiling to the Fortanix SGX target currently requires Rust nightly.
The implementation in this patch takes advantage of this to call the
LLVM intrinsic directly; a Rust feature that will never be stabilized.
At some point in the future, the Fortanix SGX target may be stabilized
and then we will need a way to invoke RDRAND wihout nightly Rust.