Closed etan-status closed 1 year ago
Splitting this into multiple PRs
one thing we could do is create a nim-randoms library that provides utilities on top of RNG:s - the most common one is to reduce random bytes to an integer range but there are others as well (for example different distributions etc, like C++ does it). We ended up deciding that bearssl is not the right place for these utilities but we don't really have a good place to put them so at some point, it probably will make sense to write a new library - until then, copy-pasting is not a bad second option (it's trivial enough for most of our use cases)
Yes, that's the logical next step. Just because it is a small library doesn't mean it can't exist.
copy-pasting is not a bad second option (it's trivial enough for most of our use cases)
We have quite a jungle already.
nim-websock
has a copy that has dead code (the x
var iable is shadowed by another x
variable), and doesn't use ref
but instead var
. Notably, some other nim-eth
functions that also use var
then do weird unsafeAddr
to get ref
once more, e.g., when referring to it in a callback. Not too clean. Maybe also a Nim issue though, they lack annotation for non-escaping closures (or, vice versa, for escaping closures, as that's quite rare normally), where taking addr
of var
should be a non-issue.nim-websock
also has a typealias
for Rng
. But nimcrypto
defines Rng
differently so is a bad name even though separate modules, as it may lead to unnecessary conflictnim-libp2p
has a copy of newRng
, and also put that as part of their client facing API, so it can't be renamed easily as it affects all clients of nim-libp2p
. Notably, they also copy paste the nim-bearssl
implementation, but with var
instead of let
and otherwise same.nim-eth
split it up between random2
and keys
, but overall seems to be the most "canonical" version. They don't have a typealias to plug in different random generators though, like nim-websock
offers.newRng
callers seem to check quite randomly for nil
. The ones that check for nil
actually just toss a Defect
if its nil
. Some others happily use it and run into segfaults I guess? The nil
check should probably be part of newRng
. Furthermore, on platforms such as macOS, each individual RNG call could fail, I guess if those are used (e.g., SecRandomCopyBytes
because you can't open random system files on iOS) it makes sense to also handle this inside the small rand
utility.I think nim-randoms
is the way to go, for both the newRng
function, and the tiny helpers. When sysrand
becomes viable, simply change the typealias
to be an empty object
instead and it will propagate through all Status libs. It may still be nice to have the concept of a RNG context for environments that actually need to do things, e.g., embedded, that may have to turn on some hardware.
a RNG context for environments
yes, like any other code, relying on globals makes the code less flexible for a number of reasons - I don't see sysrand becoming a viable option any time soon also because it complicates security analysis in that each sysrand must be evaluated separately on different os's and hardware. bearssl (or any other cryptoprng) isolates us from that complexity (keeping in mind that most of the cryptography we use completely breaks down if the randomness source is broken, ie all the way from leaking private key details to allowing invalid signatures to pass etc etc)
nim-randoms
: https://github.com/status-im/nim-randoms/pull/2
Update
rand
with the implementation fromnim-eth
(no unusedvar x
) and change use to haveref
explicitly for theRng
context, similar to how it is done in the other Status libraries. Further align name toSecureRngContext
(https://github.com/status-im/nim-eth/pull/617)