From 53a75a2be798600f20d72bd3301bff035113dc5b Mon Sep 17 00:00:00 2001
From: ee7 <45465154+ee7@users.noreply.github.com>
Date: Mon, 7 Aug 2023 15:44:18 +0200
Subject: [PATCH] nimble, uuid: generate UUIDs via std/sysrand, not
 pragmagic/uuids (#777)

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] https://github.com/nim-lang/Nim/commit/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
---
 configlet.nimble          |  9 ---------
 src/create/approaches.nim |  6 +++---
 src/create/articles.nim   |  6 +++---
 src/uuid/uuid.nim         | 29 ++++++++++++++++++++++++++---
 tests/test_uuid.nim       | 22 ++++++++++++++--------
 5 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/configlet.nimble b/configlet.nimble
index b6a906bf..7f11c537 100644
--- a/configlet.nimble
+++ b/configlet.nimble
@@ -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"
diff --git a/src/create/approaches.nim b/src/create/approaches.nim
index 987ccc91..23186a13 100644
--- a/src/create/approaches.nim
+++ b/src/create/approaches.nim
@@ -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)
@@ -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: "",
diff --git a/src/create/articles.nim b/src/create/articles.nim
index 08280d90..1e170816 100644
--- a/src/create/articles.nim
+++ b/src/create/articles.nim
@@ -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)
@@ -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: "",
diff --git a/src/uuid/uuid.nim b/src/uuid/uuid.nim
index e9ba33b9..ab2b40de 100644
--- a/src/uuid/uuid.nim
+++ b/src/uuid/uuid.nim
@@ -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.
@@ -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
 
diff --git a/tests/test_uuid.nim b/tests/test_uuid.nim
index 4f257b5f..abfaa767 100644
--- a/tests/test_uuid.nim
+++ b/tests/test_uuid.nim
@@ -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.}