Skip to content

fix(yubihsm): wipe the key on every keys import exit - #14

Merged
aeddi merged 13 commits into
mainfrom
yubihsm-import-wipes-on-every-exit
Nov 4, 2026
Merged

aeddi merged 13 commits into
mainfrom
yubihsm-import-wipes-on-every-exit

Conversation

@aeddi

@aeddi aeddi commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

tmkms yubihsm keys import ended the process with process::exit from 15 places after
reading the key file, and process::exit runs no destructors. So a failed import left the
file's contents, and any key already decoded, in memory unwiped. Now:

  • Every failure comes back to run(), which reports it and exits only after the import has
    dropped the file buffer and any decoded key, so whatever wipes itself on drop is wiped, as on
    success. Exit codes and messages are unchanged.
  • The import connects to the HSM before it reads the key file, because a failed first
    connection still ends the process where it happens. With an unreachable HSM, its error now
    wins over a bad key file, key type or missing key ID, and a yubihsm-server CLI password
    prompt comes before any of those errors.
  • A Base64 key that fails to decode partway is wiped too. The decoder writes every group before
    it reports an invalid one, into a buffer that used to be freed unwiped. The KMS's own startup
    load of Base64 keys shares the fix.
  • FORK.md row 14 points to a note naming the key copies tmkms cannot wipe: inside
    iq-cometbft, for every priv_validator_key.json, and inside yubihsm, for every key
    keys import stores.

Changes

  • Pin what every keys import outcome prints and returns, before the refactor — aa29139
  • Return every failure to run(), the only place that exits — 2b5cabe
  • Connect to the HSM before reading the key — 8ade48f
  • Hand the import the client run() connected with, so the HSM's own failures can be tested, and share the Ed25519 import steps — a5bfa81, 36717d7
  • Wipe a Base64 key that fails to decode partway, in keys import and in key load — 03c0aa1
  • Test the keys import path too, which makes commands::yubihsm::keys, its import module and the import method public — f623cbf
  • Share the stderr check between the keys import CLI tests — 6f5a017
  • Reword FORK.md row 14 and name the unwiped dependency copies — 4fa75cb, 8a18913
  • Update the verification counts in FORK.md — 4925164, f3924bc, 1d85340

Testing

  • A CLI test runs 15 keys import cases against MockHsm and pins each exit code and every
    stderr line. It passed before the refactor and passes after.
  • Unit tests show that each failure after the key file is read comes back to run(),
    including the HSM's own refusals (a MockHsm client with the wrong password). A helper that
    exits in place kills the test process.
  • A test binary whose allocator scans freed memory finds no decoded key bytes after a key fails
    to decode, through key load or through keys import. Before the fix it found them.
  • With a config that has no YubiHSM provider, a missing key file reports the HSM setup error,
    so the import connects first.
  • The tests can't reach a successful wrapped import, nor the HSM then failing to return the
    imported key's public key: the CLI's MockHsm has no wrap key, and an in-process test can't
    print the success message. Those paths change only mechanically.
  • Rebased on test: keep integration test output pristine #13: 70 unit + 31 integration tests pass on stable and 1.85, printing nothing
    but their results.

@aeddi
aeddi merged commit 457f787 into main Nov 4, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant