uuid: use best practices for UUID generation #424
Labels
kind: refactor
Change to production code that does not fix a bug or add a feature
Comments
ee7
added
the
kind: refactor
This was referenced Aug 6, 2023
ee7
added a commit
that referenced
this issue
Aug 7, 2023
Replace two Nimble dependencies with our own code. Before this commit, configlet would generate a UUID with the Nimble packages pragmagic/uuids [1] and pragmagic/isaac [2] (itself a port of a C implementation [3]). However, the uuids package doesn't currently work on Windows with Nim 2.0 (due to Nim removing [4] some functions for a deprecated Win32 API). I've created a PR for the package [5], but: - That PR may not be sufficient. - I want to move away from these dependencies anyway (see #424). - Our tests for UUID generation are good enough that we can confidently change the implementation. std/sysrand has been around for 2.5 years, but still warns that it hasn't been formally audited and doesn't guarantee safety for cryptographic purposes [6]. But it's probably good enough for our purposes: there's more use, review, and maintenance of std/sysrand than pragmagic/isaac anyway (whose most recent commit was in 2017, and ISAAC is questionable as a CSPRNG). In the tests, add a test for distinctness, and test more UUIDs. The `configlet uuid` command sets a limit of 1000 UUIDs, so let's assert that nothing strange happens above that number. Pick the number such that the genUuid tests take on the order of 100 ms. We can't make this a much higher number because we compile the tests in debug mode, and `configlet uuid` is intended for generating a small number of UUIDs. But I tested locally that the new implementation could produce 500M distinct valid version 4 UUIDs. As a future optimization we could either read more than 16 bytes each time from the system CSPRNG, or once again use the system CSPRNG to seed a different PRNG. Other benefits of this change: - It makes configlet more portable. Previously, `configlet uuid` would only work on Windows and platforms with `/dev/urandom` [7]. - We remove a gotcha from the nimble file version pinning. - We can rename from `genUUID` to `genUuid` fit our style elsewhere. Closes: #424 [1] https://github.com/pragmagic/uuids/blob/8cb8720b567c/uuids.nim [2] https://github.com/pragmagic/isaac/blob/45a5cbbd54ff/src/isaac.nim [3] https://burtleburtle.net/bob/c/readable.c [4] nim-lang/Nim@612abda4f40b [5] https://www.github.com/pragmagic/uuids/pull/9 [6] https://github.com/nim-lang/Nim/blob/v2.0.0/lib/std/sysrand.nim#L10-L17 [7] https://github.com/pragmagic/uuids/blob/8cb8720b567c/uuids/urandom.nim#L41-L62
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Low priority.
Currently to generate a UUID we use:
pragmagic/uuids: uuids.nimpragmagic/isaac: isaac.nim (a port of https://burtleburtle.net/bob/c/readable.c)This approach:
But ISAAC is outdated, and it "remains to be seen" whether ISAAC is actually "cryptographically secure" (at least according to Thomas Pornin, a notable cryptographer).
See also:
It's not strictly necessary to use a cryptographically secure PRNG if
configlet uuidis only used for populating values in a trackconfig.jsonfile. But I don't want to write/explain warnings everywhere saying:It's better to use the operating system's built-in UUID generation, as seen in:
status-im/nim-eth: eth/keyfile/uuid.nimcheatfate/nimcrypto: sysrand.nimwhich generates the UUIDs as follows:
UuidCreate()fromrpcrt4.dll/proc/sys/kernel/random/uuiduuid_generate_random()uuid_create()To illustrate: on Linux, you can write:
In the future, it might also be possible to use the new Nim stdlib module std/sysrand if it stabilizes.
The text was updated successfully, but these errors were encountered: