Theseus icon indicating copy to clipboard operation
Theseus copied to clipboard

Add `random` crate

Open tsoutsman opened this issue 1 year ago • 2 comments

The crate mimics the RngCore interface. It was named random to not conflict with rand's symbols.

I've also replaced locally defined RNGs in the ixgbe and ota_update_client crates, that used the ... HPET timer count as a seed.

A further development could be to expose an add_entropy function. I'm not sure it's a major concren because a CSPRNG seeded with 32 bytes of entropy should be plenty secure. However, if RDSEED or RDRAND don't exist, it is absolutely necessary. Adding entropy would require locking the CSPRNG, generating a random seed, combining it with the entropy, and reseeding the CSPRNG. This would somehow need to be arbitrated to ensure the the CSPRNG isn't being constantly reseeded, impacting performance.

Our QEMU configuration currently doesn't support RDSEED, so the backup RDRAND is used.

Signed-off-by: Klim Tsoutsman [email protected]

tsoutsman avatar Jul 31 '22 05:07 tsoutsman

This looks great! Have you thought about implementing the RngCore trait? Would that be a good thing? https://docs.rs/rand_core/latest/rand_core/trait.RngCore.html

NathanRoyer avatar Aug 05 '22 15:08 NathanRoyer

We only expose one source of randomness, so implementing the trait seems unnecessary. The functions would have to be converted to methods on a ZST, and the caller would have to import the RngCore trait. It also means we expose rand in the public API, which is notoriously unstable.

tsoutsman avatar Aug 06 '22 14:08 tsoutsman

So, to summarize the key points of our discussion:

  • Document why we have a single CSPRNG, and why we're using a CS one instead of non-CS
  • Describe how other crates would want to use this (use the CSPRNG to seed a non-CS PRNG)
  • Undo the changes to other crates that were using various random sources (we can address than in a new PR)
  • Add explicit initialization, perhaps from the captain, but another place could work too (device_manager?)

As part of a future PR we could:

  • Implement rand::RngCore for our CSPRNG such that other crates could use it to seed another instance of anything that implements rand::SeedableRng
  • Change crates across Theseus to use this new random crate, perhaps adding convenience interfaces to it that create new non-CS PRNG instances on demand for each caller

kevinaboos avatar Aug 19 '22 22:08 kevinaboos

I've changed the PR to simply add a new unused rand interface addressing the points of our discussion. I've added explicit initialization to captain but I'm not sure if we need it because the CSPRNG would just be initialised the first time we access it.

Running tsc_seed on my computer yields the following seed:

[
    1,
    110,
    179,
    224,
    12,
    55,
    96,
    138,
    175,
    208,
    241,
    18,
    51,
    84,
    117,
    148,
    179,
    212,
    243,
    18,
    49,
    80,
    111,
    142,
    175,
    206,
    237,
    29,
    60,
    93,
    126,
    157,
]

The bytes are very clearly increasing in a loop. I tried using just the last bit (calling tsc_ticks() 256 times) but this resulted in (I think) an even worse seed:

[
    177,
    169,
    181,
    85,
    170,
    170,
    85,
    90,
    170,
    85,
    74,
    170,
    85,
    170,
    165,
    85,
    170,
    85,
    74,
    170,
    85,
    42,
    169,
    85,
    42,
    173,
    85,
    170,
    213,
    82,
    170,
    85,
]

tsoutsman avatar Aug 20 '22 00:08 tsoutsman

infallible init

That's because tsc is incorrectly infallible; it doesn't check whether the counter exists. But that's a separate issue.

tsoutsman avatar Aug 22 '22 23:08 tsoutsman

That's because tsc is incorrectly infallible; it doesn't check whether the counter exists. But that's a separate issue.

Hmm, i see. While that technically might be a concern for ancient CPUs, I doubt it's of practical concern to us since we don't even support anything prior to 64-bit x86. AFAICT TSC has existed since Pentium was first introduced, so I think we'd be okay.

kevinaboos avatar Aug 23 '22 00:08 kevinaboos