-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get rid of the getrandom
and OsRng
requirement
#86
base: master
Are you sure you want to change the base?
Get rid of the getrandom
and OsRng
requirement
#86
Conversation
I don't think I like this approach. It feels really hacky to me. What exact problem are you trying to solve? Building/testing on |
@newpavlov Agreed. I rethought about it and changed it like that without using OnceCell |
This reverts commit 8b5bfa2.
I had a more fundamental question: why exactly do you want to make dependency on If you want a way to change entropy source (e.g. for deterministic testing), then I think its better to introduce |
For one, it broke no_std as the automatic target selection of getrandom looks very arcane (https://docs.rs/getrandom/latest/getrandom/), that means whenever you have a new triplet you are not likely to port getrandom on day one, and second it assumes the existence of an operating system, at least within the definition of But I do think your approach would also be alright, let me try that out as well. |
Do you perhaps mean the As for the
It's no surprise that you need additional configuration on
No, it does not. |
I would still argue that we should unify the RNG to use Rustls provider supplied one, rather than assuming I'm talking about things like this: Line 17 in 992778d
|
If you have ideas how we could make it easier to configure
I don't see any major issues with this code. It may make more sense to use |
This PR solves the
getrandom
dependency problem, due toOsRng
being added to the library code itself. SinceOsRng
assumed to be running under an operating system, this makes it impossible to run in freestanding environment such as bare-metal kernel and isolated raw WebAssembly.It has now been removed and the dependency on RNG source in the provider itself has been inversed, meaning that the user has to supply a RNG provider from the start in the initial provider initialization call, and during execution, we take get take back the RNG implementation in rustls global code.
Assuming the code that needs randomness in this library would only be called by the library itself (e.g. RustCrypto Provider -> ECDHE key gen), we can safely assume there has to be a crypto provider and it has to be our own library.
no_std
users can still enablegetrandom
but they have to make the implementation manually. This can simply be done by adding thegetrandom
crate in their end, and then set the default RNG usingOsRng
themselves, rather than pushing the responsibility to us to always assumeOsRng
to be available.Now, it is really debatable whether
CryptoRng
is also needed or not forRNG
, as it is intended for embedded system wheregetrandom
is unlikely to be ported to the platform, so the user might just want to use a good initial entropy (like from CPU jitter or other micro-movements) then shove it to a PRNG that is fast and generates random numbers for the rest of the lifetime. This is not cryptographically safe at all, but so far this is a necessary evil if you want it to run inno_std
environment where the existence of a good CSPRNG implementation is not guaranteed.Thus,
impl CryptoRng for CryptoProviderRng
can be considered a phony implementation out of necessity for the code to compile. It is now user-controlled and does not guarantee the output to be of cryptographic quality.