Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ jobs:
fi
echo "checks=$checks" >> "$GITHUB_OUTPUT"
# Pure shell, seconds, no Nix evaluation: an entropy-source defect is reported
# immediately and stays reportable even when the flake or the VM matrix is broken
# for unrelated reasons. See scripts/check-rng-hygiene.sh for the rules and the
# COLDCARD firmware disclosure that motivated them.
rng-hygiene:
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- uses: actions/checkout@v6
- name: Check for silent RNG fallbacks
run: ./scripts/check-rng-hygiene.sh
Comment on lines +58 to +61

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Disable credential persistence for this checkout.

The job only reads tracked files. It never pushes. actions/checkout writes the job token into .git/config by default, which leaves it readable by any later step or tool in the job.

Also confirm that rng-hygiene is a required status check in branch protection. Otherwise a failing guard does not block a merge.

🔒 Proposed fix
     steps:
-      - uses: actions/checkout@v6
+      - uses: actions/checkout@v6
+        with:
+          persist-credentials: false
       - name: Check for silent RNG fallbacks
         run: ./scripts/check-rng-hygiene.sh
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
steps:
- uses: actions/checkout@v6
- name: Check for silent RNG fallbacks
run: ./scripts/check-rng-hygiene.sh
steps:
- uses: actions/checkout@v6
with:
persist-credentials: false
- name: Check for silent RNG fallbacks
run: ./scripts/check-rng-hygiene.sh
🧰 Tools
🪛 zizmor (1.28.0)

[warning] 59-59: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 58 - 61, Update the actions/checkout
step in the CI workflow to disable credential persistence, while keeping the
existing rng-hygiene step unchanged. Also configure branch protection so the
rng-hygiene status check is required for merges.

Source: Linters/SAST tools


# Build the expensive shared packages once and PUSH them to the `privkey` Cachix binary cache, so
# the per-check jobs substitute keep-web/keep-cli instead of cold-rebuilding them. keep-cli
# (tpm-attestation + bindgen) is a ~10min Rust build that oprf-unlock depends on; keep-web is what
Expand Down
8 changes: 8 additions & 0 deletions docs/SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ This chapter sets out what Keep Node protects, what it does not, and the assumpt
guarantees rest on. It is deliberately conservative: a security appliance is only as honest
as its threat model.

## Randomness

A degraded RNG path must never succeed quietly. The only key material the *appliance runtime* generates itself is the LUKS bootstrap passphrase for the vault data volume, drawn during first-boot provisioning in `nixos/frost-gate.nix`; everything else (FROST shares, mesh identities, Vaultwarden's own keys) is generated by the keep binaries or by Vaultwarden, which have their own guarantees. Two other generators exist and are deliberately out of that scope: the opt-in `keepnode-debug` profile mints a self-signed TLS keypair at first boot, and `flake.nix` mints test fixtures into the Nix store, consumed only by `tests/`.

That draw reads `/dev/random`, not `/dev/urandom`. On Linux 5.6 and later `/dev/random` blocks only until the kernel CRNG is initialised and never afterwards, which is exactly the guarantee wanted on the first boot of a freshly imaged appliance: there is no stored seed yet, and `/dev/urandom` would return output from an uninitialised CRNG with nothing but a kernel log line to say so. The read length is checked as well, because `head -c 32 /dev/random | base64` exits 0 with a short string if the read is truncated, which would format the volume under a truncated passphrase and carry on.

`scripts/check-rng-hygiene.sh` checks both in CI, along with a ban on bash `$RANDOM` outside explicitly marked retry jitter. It also pins the draw positively (the read must still come from `/dev/random`), because a rule that only bans `/dev/urandom` leaves swapping the whole draw for a derived value green. Note that `main` is not a protected branch today, so the job is advisory rather than gating. Know what it does not cover: it is a grep, so it checks the source of a value, not what the value becomes; it cannot see into the keep binaries the appliance runs, into nixpkgs, or into systemd's own seeding; and "reads `/dev/random`" means the read waited for the kernel to declare the CRNG ready, not that a given piece of hardware fed it good entropy. The rule exists because this class of bug is silent by construction, as in the COLDCARD firmware disclosure the script header cites.

## Current status

Keep Node is an early scaffold, so this chapter describes the **target** design alongside
Expand Down
35 changes: 34 additions & 1 deletion nixos/frost-gate.nix
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ let
# Do not rely on the ~24s discovery time for this: a fast failure (holder online but declining)
# can cycle much quicker. (2) A power blip reboots a whole fleet at once, so the +RANDOM jitter
# spreads retries instead of hammering one relay/holder set in lockstep.
# rng-hygiene: ok - retry jitter to de-synchronise a fleet reboot, not key material
sleep $(( 10 + RANDOM % 5 ))
done
cryptsetup open --key-file "$keyf" --keyfile-size 32 "$dev" "$mapper"
Expand Down Expand Up @@ -303,7 +304,26 @@ let
echo "frost-gate: WARNING could not wipe $dev during rollback; a signature persists. The next boot re-provisions it (no completion marker)." >&2
}
trap cleanup ERR
pass="$(head -c 32 /dev/urandom | base64)"
# A stop-timeout SIGTERM (or INT) does not fire the ERR trap, so roll back
# explicitly too: without this, a timeout mid-provision leaves a labelled,
# TPM2-enrolled device with no filesystem, which the next boot has to
# recognise as unprovisioned rather than as healthy. The oprf path already
# traps TERM/INT for the same reason.
trap 'cleanup; exit 1' TERM INT
# /dev/random, not /dev/urandom. On Linux >= 5.6 /dev/random blocks only
# until the CRNG is initialised and never afterwards, which is exactly the
# guarantee wanted here: this runs on the first boot of a freshly imaged
# appliance, before any seed file exists, and its output becomes the LUKS
# passphrase for the vault data volume. /dev/urandom never blocks and will
# return output from an uninitialised CRNG if read early enough -- quietly,
# with only a kernel log line. The blocking read costs nothing once the
# CRNG is up, which by this point in boot it almost always is; "almost
# always" is not the standard this key should be held to.
pass="$(head -c 32 /dev/random | base64)"
# base64 of exactly 32 bytes is 44 characters. `set -o pipefail` already
# catches a failing read, but a short one would not raise: it would
# luksFormat the volume under a truncated passphrase and carry on.
[ "''${#pass}" -eq 44 ] || { echo "frost-gate: FATAL short entropy read; refusing to format $dev" >&2; exit 1; }
echo -n "$pass" | cryptsetup luksFormat -q --label "$label" --iter-time 1000 "$dev" -
${lib.optionalString (cfg.recoveryKeyFile != null) ''
# Opt-in recovery keyslot (keepNode.frostGate.recoveryKeyFile). Enroll a high-entropy
Expand Down Expand Up @@ -928,6 +948,19 @@ in
RemainAfterExit = true;
}
// hardening
// lib.optionalAttrs (cfg.mode == "tpm") {
# Bound first-boot provisioning explicitly. The default this would
# otherwise inherit is 90s, and the VM tests run with 300s
# (nixpkgs test-instrumentation), so CI cannot observe the real budget.
# The window has to cover: the /dev/random read (instant once the CRNG
# is up, which on x86_64 with RDRAND is within the first milliseconds,
# but unbounded on a board booted with random.trust_cpu=0 and no other
# early entropy), luksFormat --iter-time 1000, systemd-cryptenroll's
# argon2id benchmark, and mkfs.ext4. Failing this unit leaves the vault
# down and needs a manual restart, so the budget is generous rather than
# tight: a slow first boot is a much better outcome than a false timeout.
TimeoutStartSec = 600;
}
// lib.optionalAttrs (cfg.mode == "oprf") {
# systemd TPM-decrypts these into $CREDENTIALS_DIRECTORY (ramfs) at unit start. If a PCR
# changed, decryption fails, the unit fails, and the volume stays locked (fail-closed).
Expand Down
217 changes: 217 additions & 0 deletions scripts/check-rng-hygiene.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
#!/usr/bin/env bash
# Fail if the appliance can generate key material from an entropy source that
# might not be ready.
#
# Motivated by the COLDCARD firmware disclosure (Block, 2026-07):
# https://engineering.block.xyz/blog/predictable-rng-fallback-and-32-bit-reseed-in-coldcard-firmware
# A guard that checked only whether a macro was *defined* -- not whether it was
# *enabled* -- silently bound wallet seed generation to a non-cryptographic
# fallback PRNG. Nothing crashed, nothing logged, and the firmware shipped that
# way for years. The lesson is not "use a CSPRNG"; every codebase already
# intends to. The lesson is that a degraded RNG path must not be able to succeed
# quietly, and that only a mechanical check keeps it that way.
#
# The shape that bug takes in an appliance image:
#
# 1. /dev/urandom never blocks. Read before the kernel CRNG is initialised it
# returns output anyway, with nothing but a kernel log line to say so. This
# box generates the LUKS passphrase for the vault volume on the FIRST boot
# of a freshly imaged disk, before any seed file exists, which is precisely
# the window where that matters. /dev/random on Linux >= 5.6 blocks only
# until the CRNG is up and never afterwards, so it is strictly better here
# at no cost. Rule 1 requires it.
# 2. $RANDOM is bash's seeded PRNG. Fine for retry jitter, never for a key.
# Rule 2 requires an explicit marker.
#
# Deliberate non-crypto randomness is allowed with an inline opt-out on the same
# line or in the comment block directly above:
#
# sleep $(( 10 + RANDOM % 5 )) # rng-hygiene: ok - retry jitter
#
# This guard fails CLOSED. A scanner error, a run outside a git work tree, or an
# empty file list is reported as a failure rather than silently passing: a guard
# that prints "OK" when it scanned nothing is worse than no guard, because it
# gets trusted.
#
# Scope: TRACKED Nix, shell, and Python under nixos/ and the repo root. tests/
# is excluded -- test fixtures are deterministic on purpose, and a VM test that
# reads /dev/urandom is not provisioning a real box.
#
# What this does NOT cover: it is a grep, so it catches the source, not what the
# value becomes. It cannot see into the keep binaries the appliance runs (those
# have their own guard in the keep repo), into nixpkgs, or into systemd's own
# seeding. And "reads /dev/random" is not proof the CRNG had real entropy behind
# it on a given piece of hardware, only that the read waited for the kernel to
# say it was ready.
#
# Portable to BSD awk. Run from anywhere; exits non-zero with the offending lines.

set -uo pipefail
cd "$(dirname "$0")/.." || exit 1

status=0
fail() { printf '\n\033[31mFAIL\033[0m %s\n' "$1"; status=1; }

OPT_OUT='rng-hygiene: ok'
# This script is a .sh file and its rules name the very tokens they ban, so it
# would report itself. Excluded by path rather than by opt-out markers, which
# would blunt the markers' signal. Caught only after committing: run untracked,
# `git ls-files` did not list it and it passed locally while failing in CI.
SELF='scripts/check-rng-hygiene.sh'

git rev-parse --is-inside-work-tree >/dev/null 2>&1 || {
printf '\n\033[31mFAIL\033[0m not inside a git work tree; this guard scans tracked files only\n'
exit 1
}

list_sources() {
git ls-files '*.nix' '*.sh' '*.py' 'justfile' \
| grep -vE '^tests/' \
| grep -vxF "$SELF"
}

SOURCES=$(list_sources)
if [ -z "$SOURCES" ]; then
printf '\n\033[31mFAIL\033[0m no sources found to scan; the file list is broken\n'
exit 1
fi

# Emit "file:line:code" for live code lines, dropping `#` comments (Nix, shell
# and Python all use them) and honouring the opt-out marker. String literals are
# left intact: the values this guard cares about -- a device path like
# "/dev/urandom" -- live inside them.
preprocess() {
local f rc out
rc=0
for f in $SOURCES; do
out=$(awk -v fname="$f" -v optout="$OPT_OUT" '
function trim(s) { sub(/^[ \t]*/, "", s); sub(/[ \t]*$/, "", s); return s }

# Split a line into code and comment, honouring quoting. A naive
# index($0, "#") splits `''${#pass}` and, worse, treats a `#` inside a
# string as a comment start -- which let `printf "#x"; pass=/dev/urandom`
# hide the read, and let an opt-out marker inside a string literal exempt
# a line with no comment a reviewer could see.
function split_line(s, i, c, n, q, out) {
out = ""; cmt = ""; q = ""
n = length(s)
for (i = 1; i <= n; i++) {
c = substr(s, i, 1)
if (q != "") {
if (c == "\\") { out = out c substr(s, i + 1, 1); i++; continue }
if (c == q) q = ""
out = out c
continue
}
if (c == "\"" || c == "'"'"'") { q = c; out = out c; continue }
if (c == "#") { cmt = substr(s, i); return out }
out = out c
}
return out
}

BEGIN { blockopt = 0; prose = 0 }
{
raw = $0
code = trim(split_line(raw))

# Nix option documentation is prose, not code. Without this, a
# description mentioning /dev/urandom is a finding, and the only way to
# silence it is a marker that renders as literal text in the rendered
# option docs.
if (prose) {
if (raw ~ /(\x27\x27;|\x27\x27[ \t]*$)/) prose = 0
next
}
if (code ~ /(description|example|longDescription)[ \t]*=[ \t]*\x27\x27/ && code !~ /\x27\x27.*\x27\x27[ \t]*;/) {
prose = 1
next
}

# A blank line ends the comment block, so a marker cannot carry across
# unrelated lines to exempt something far below it.
if (code == "" && cmt == "") { blockopt = 0; next }
if (code == "") { blockopt = (index(cmt, optout) ? 1 : blockopt); next }
Comment on lines +90 to +134

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Reproduce the ''${ misparse in isolation with the file's own split_line logic.
set -euo pipefail
awk '
  function split_line(s,   i, c, n, q, out) {
    out = ""; cmt = ""; q = ""
    n = length(s)
    for (i = 1; i <= n; i++) {
      c = substr(s, i, 1)
      if (q != "") {
        if (c == "\\") { out = out c substr(s, i + 1, 1); i++; continue }
        if (c == q) q = ""
        out = out c
        continue
      }
      if (c == "\"" || c == "'"'"'") { q = c; out = out c; continue }
      if (c == "#") { cmt = substr(s, i); return out }
      out = out c
    }
    return out
  }
  { print "code=[" split_line($0) "] cmt=[" cmt "]" }
' <<< $'n=\x27\x27${`#pass`}; leaked=/dev/urandom'

Repository: privkeyio/keep-node

Length of output: 205


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== File context =="
wc -l scripts/check-rng-hygiene.sh
sed -n '1,240p' scripts/check-rng-hygiene.sh | cat -n

echo "== References to files =="
rg -n "nixos/frost-gate\\.nix|/dev/(u)?random|\\$RANDOM|split_line|BLOCKOPT|SELF|list_sources" scripts/check-rng-hygiene.sh

echo "== Focused Nix path if present =="
if [ -f nixos/frost-gate.nix ]; then
  rg -n -C 4 "''\\$\\{`#pass`\\}|/dev/(u)?random|base64|LUKS" nixos/frost-gate.nix
fi

echo "== POSIX awk availability =="
command -v awk nawk gawk mawk 2>/dev/null || true

Repository: privkeyio/keep-node

Length of output: 11372


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== search relevant paths =="
rg -E 'nixos/frost-gate\.nix|/dev/(u)?random|split_line|SELF|list_sources|RANDOM' scripts/check-rng-hygiene.sh

echo "== nixos/frost-gate.nix excerpt if present =="
if [ -f nixos/frost-gate.nix ]; then
  wc -l nixos/frost-gate.nix
  rg -n -C 6 "''\\$\\{`#pass`\\}|head -c 32 /dev/random|-eq 44|base64" nixos/frost-gate.nix || true
else
  echo "nixos/frost-gate.nix not present"
fi

echo "== awk output for escaped-outer-quote ''${ and escaped outer comment =="
awk '
  function split_line(s,   i, c, n, q, out) {
    out = ""; cmt = ""; q = ""
    n = length(s)
    for (i = 1; i <= n; i++) {
      c = substr(s, i, 1)
      if (q != "") {
        if (c == "\\") { out = out c substr(s, i + 1, 1)";i++"; continue }
        if (c == q) q = ""
        out = out c
        continue
      }
      if (c == "\"" || c == "'"'"'") { q = c; out = out c; continue }
      if (c == "#") { cmt = substr(s, i); return out }
      out = out c
    }
    return out
  }
  { print "raw=[" NR "] code=[" split_line($0) "] cmt=[" cmt "]" }
' <<'EOF'
n=''${`#pass`}; leaked=/dev/urandom
printf "`#not-a-comment`"; read /dev/random
EOF

Repository: privkeyio/keep-node

Length of output: 323


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== search relevant paths =="
grep -nE 'nixos/frost-gate\.nix|/dev/(u)?random|split_line|SELF|list_sources|RANDOM' scripts/check-rng-hygiene.sh

echo "== nixos/frost-gate.nix excerpt if present =="
if [ -f nixos/frost-gate.nix ]; then
  wc -l nixos/frost-gate.nix
  grep -n -C 6 "''\${`#pass`}\|head -c 32 /dev/random|-eq 44|base64" nixos/frost-gate.nix || true
else
  echo "nixos/frost-gate.nix not present"
fi

echo "== awk output for escaped-outer-quote ''${ and escaped outer comment =="
awk '
  function split_line(s,   i, c, n, q, out) {
    out = ""; cmt = ""; q = ""
    n = length(s)
    for (i = 1; i <= n; i++) {
      c = substr(s, i, 1)
      if (q != "") {
        if (c == "\\") { out = out c substr(s, i + 1, 1)";i++"; continue }
        if (c == q) q = ""
        out = out c
        continue
      }
      if (c == "\"" || c == "'"'"'") { q = c; out = out c; continue }
      if (c == "#") { cmt = substr(s, i); return out }
      out = out c
    }
    return out
  }
  { print "raw=[" NR "] code=[" split_line($0) "] cmt=[" cmt "]" }
' <<'EOF'
n=''${`#pass`}; leaked=/dev/urandom
printf "`#not-a-comment`"; read /dev/random
EOF

Repository: privkeyio/keep-node

Length of output: 3366


Don’t treat Nix ''${ as two independent quote operators.

split_line() toggles a quote state on each ', so an unwrapped Nix escape like ''${#pass} closes the quote before the #. Content after that # is classified as a comment and is not sent to scan(), so a banned pattern on the same line is bypassed. Keep '' as literal/token data in the Nix preprocessor and preserve the outer-scope backslash skip for escaped #/quotes too.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/check-rng-hygiene.sh` around lines 90 - 134, Update split_line() so
Nix’s ``''`` sequence is consumed as literal/token data rather than toggling q
twice, ensuring a # within ``''${...}`` remains code and reaches scan().
Preserve the existing backslash escape handling while recognizing this sequence,
including escaped # and quote characters outside quoted strings.

if (index(cmt, optout) || blockopt) { blockopt = 0; next }
blockopt = 0
printf "%s:%d:%s\n", fname, FNR, code
}
' "$f") || rc=2
[ -n "$out" ] && printf '%s\n' "$out"
done
return "$rc"
}

CODE=$(preprocess) || {
printf '\n\033[31mFAIL\033[0m the scanner itself failed; refusing to report a clean tree\n'
exit 1
}
if [ -z "$CODE" ]; then
printf '\n\033[31mFAIL\033[0m preprocessor produced no code lines; the guard scanned nothing\n'
exit 1
fi

# Called from the CALLER, never from inside the command substitution: an `exit`
# in a substitution terminates only the subshell, so `x=$(scan ...) ` would have
# swallowed the failure and reported a clean tree. That is the fail-open shape
# this guard exists to reject.
scanner_died() {
printf '\n\033[31mFAIL\033[0m the scanner itself failed; refusing to report a clean tree\n' >&2
exit 1
}

scan() { # $1 = ERE
printf '%s\n' "$CODE" | awk -v pat="$1" '{
line = $0; sub(/^[^:]*:[0-9]+:/, "", line)
if (line ~ pat) print $0
}'
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

report() { # $1 = findings, $2 = headline, $3.. = hints
[ -z "$1" ] && return 0
fail "$2"
printf '%s\n' "$1" | sed 's/^/ /'
shift 2
for hint in "$@"; do echo " → $hint"; done
}

# ------------------------------------------ 1. no non-blocking entropy read ----
urandom_bad=$(scan '/dev/urandom') || scanner_died
report "$urandom_bad" "reads /dev/urandom, which never waits for the kernel CRNG:" \
'use /dev/random. On Linux >= 5.6 it blocks only until the CRNG is initialised' \
'and never afterwards, which is the guarantee a first-boot appliance needs.' \
"Mark a deliberate non-key-material read: # $OPT_OUT - <reason>"

# ------------------------------------------------- 2. no bash seeded PRNG ----
bashrandom_bad=$(scan '(^|[^a-zA-Z0-9_])RANDOM([^a-zA-Z0-9_]|$)') || scanner_died
report "$bashrandom_bad" "uses bash \$RANDOM, a seeded PRNG:" \
'fine for retry jitter, never for key material.' \
"Mark the jitter case: # $OPT_OUT - <reason>"

# ------------------ 3. the bootstrap passphrase draw keeps both its guarantees --
# The repo-specific invariant, and the one rule that has to check for something
# being PRESENT rather than absent. Rule 1 only bans /dev/urandom; on its own,
# replacing the whole draw with `date +%s | sha256sum` leaves CI green.
#
# Pinned structurally, not by error message: an earlier version grepped for the
# text "refusing to format", so deleting the check while leaving the words in a
# comment passed.
if [ ! -f nixos/frost-gate.nix ]; then
fail "nixos/frost-gate.nix not found; rule 3 cannot run (was the module moved?)"
else
if ! grep -qE 'head -c 32 /dev/random' nixos/frost-gate.nix; then
fail "the LUKS bootstrap passphrase no longer reads 32 bytes from /dev/random:"
echo " → this is the only key material this repo mints itself. It must come from"
echo " the source that waits for the kernel CRNG, not from a derived value."
fi
if ! grep -qE '\-eq 44' nixos/frost-gate.nix; then
fail "nixos/frost-gate.nix no longer length-checks the LUKS bootstrap passphrase:"
echo " → a short entropy read must abort provisioning, not format the volume"
echo " under a truncated passphrase. base64 of 32 bytes is exactly 44 chars."
fi
fi

if [ "$status" -eq 0 ]; then
echo "RNG hygiene: OK (no /dev/urandom, no bash \$RANDOM outside marked jitter, bootstrap passphrase drawn from /dev/random and length-checked)"
fi
exit "$status"
Loading