Skip to content

Commit

Permalink
nimble, uuid: generate UUIDs via std/sysrand, not pragmagic/uuids (#777)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ee7 authored Aug 7, 2023
1 parent 2396697 commit 53a75a2
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 26 deletions.
9 changes: 0 additions & 9 deletions configlet.nimble
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,6 @@ requires "cligen#b962cf8bc0be847cbc1b4f77952775de765e9689" # 1.5.19 (2021-0
requires "jsony#2a2cc4331720b7695c8b66529dbfea6952727e7b" # 1.1.3 (2022-01-03)
requires "parsetoml#6e5e16179fa2db60f2f37d8b1af4128aaa9c8aaf" # 0.7.1 (2023-08-06)
requires "supersnappy#e4df8cb5468dd96fc5a4764028e20c8a3942f16a" # 2.1.3 (2022-06-12)
requires "uuids#8cb8720b567c6bcb261bd1c0f7491bdb5209ad06" # 0.1.11 (2021-01-15)
# To make Nimble use the pinned `isaac` version, we must pin `isaac` after `uuids`
# (which has `isaac` as a dependency).
# Nimble still clones the latest `isaac` tag if there is no tag-versioned one
# on-disk (e.g. at ~/.nimble/pkgs/isaac-0.1.3), and adds it to the path when
# building, but (due to writing it later) the pinned version takes precedence.
# Nimble will support lock files in the future, which should provide more robust
# version pinning.
requires "isaac#45a5cbbd54ff59ba3ed94242620c818b9aad1b5b" # 0.1.3 (2017-11-16)

task test, "Runs the test suite":
exec "nim r ./tests/all_tests.nim"
Expand Down
6 changes: 3 additions & 3 deletions src/create/approaches.nim
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import std/[os, strformat, strutils]
import pkg/[jsony, uuids]
import ".."/[helpers, sync/sync_common, types_track_config, types_approaches_config]
import pkg/jsony
import ".."/[helpers, sync/sync_common, types_track_config, types_approaches_config, uuid/uuid]

func kebabToTitleCase(slug: Slug): string =
result = newStringOfCap(slug.len)
Expand Down Expand Up @@ -44,7 +44,7 @@ proc createApproach*(approachSlug: Slug, exerciseSlug: Slug,

if not approachExists:
config.approaches.add ApproachConfig(
uuid: $genUUID(),
uuid: $genUuid(),
slug: $approachSlug,
title: title,
blurb: "",
Expand Down
6 changes: 3 additions & 3 deletions src/create/articles.nim
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import std/[os, strformat, strutils]
import pkg/[jsony, uuids]
import ".."/[helpers, sync/sync_common, types_track_config, types_articles_config]
import pkg/jsony
import ".."/[helpers, sync/sync_common, types_track_config, types_articles_config, uuid/uuid]

func kebabToTitleCase(slug: Slug): string =
result = newStringOfCap(slug.len)
Expand Down Expand Up @@ -40,7 +40,7 @@ proc createArticle*(articleSlug: Slug, exerciseSlug: Slug,

if not articleExists:
config.articles.add ArticleConfig(
uuid: $genUUID(),
uuid: $genUuid(),
slug: $articleSlug,
title: title,
blurb: "",
Expand Down
29 changes: 26 additions & 3 deletions src/uuid/uuid.nim
Original file line number Diff line number Diff line change
@@ -1,7 +1,30 @@
import std/strformat
import pkg/uuids
import std/[strformat, sysrand]
import ".."/logger

type
Uuid* = array[16, byte]

proc genUuid*: Uuid {.noinit.} =
## Returns a version 4 UUID, using the system CSPRNG as the source of randomness.
if urandom(result):
result[6] = (result[6] and 0x0f) or 0x40 # Set version to 4
result[8] = (result[8] and 0x3f) or 0x80 # Set variant to 1
else:
stderr.writeLine "uuid: error: failed to read from the system CSPRNG"
quit 1

func `$`*(u: Uuid): string =
## Returns the canonical string representation for the given UUID `u`.
result = newString(36)
result[8] = '-'
result[13] = '-'
result[18] = '-'
result[23] = '-'
for i, j in [0, 2, 4, 6, 9, 11, 14, 16, 19, 21, 24, 26, 28, 30, 32, 34]:
const hex = "0123456789abcdef"
result[j + 0] = hex[u[i] shr 4]
result[j + 1] = hex[u[i] and 0x0f]

proc outputUuids(n: Positive) =
## Writes `n` version 4 UUIDs to stdout. Writes only 1000 UUIDs if `n` is
## greater than 1000.
Expand All @@ -12,7 +35,7 @@ proc outputUuids(n: Positive) =
let numUuidsToGenerate = min(n, maxNumUuids)
var s = newStringOfCap(numUuidsToGenerate * 37)
for i in 1 .. numUuidsToGenerate:
s.add $genUUID()
s.add $genUuid()
s.add '\n'
stdout.write s

Expand Down
22 changes: 14 additions & 8 deletions tests/test_uuid.nim
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
import std/unittest
import pkg/uuids
import "."/lint/validators
import std/[sets, strformat, strutils, unittest]
import "."/[lint/validators, uuid/uuid]

proc main =
suite "genUUID: returns a string that isUuidV4 says is valid":
test "1000 UUIDs":
for i in 1 .. 1000:
let uuid = $genUUID()
check isUuidV4(uuid)
suite "genUuid":
const n = 100_000
var uuids = initHashSet[Uuid](n)

test &"can generate {insertSep($n)} valid version 4 UUIDs":
for i in 1 .. n:
let uuid = genUuid()
check isUuidV4($uuid)
uuids.incl uuid

test "those UUIDs are distinct":
check uuids.len == n

main()
{.used.}

0 comments on commit 53a75a2

Please sign in to comment.