Skip to content

fix(yubihsm): refuse a secp256k1 priv_validator key instead of panicking - #12

Merged
aeddi merged 6 commits into
mainfrom
yubihsm-import-rejects-secp256k1
Nov 4, 2026
Merged

aeddi merged 6 commits into
mainfrom
yubihsm-import-rejects-secp256k1

Conversation

@aeddi

@aeddi aeddi commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

tmkms yubihsm keys import crashed when a priv_validator key file held a secp256k1 key —
with -t json, or with no -t when the file is named priv_validator_key.json, since both
take the same path. The file parses fine, since PrivValidatorKey accepts both key types, and
the command then reached an unreachable!() and exited 101 with "entered unreachable
code". It now exits with an error that names the file and says an Ed25519 consensus key is
required — the same wording the softsign provider uses when it loads such a file as a
consensus key (FORK.md entry 10).

Changes

  • Refuse a secp256k1 key with an error instead of panicking — 2e7d379
  • Test it through the real CLI, with a valid secp256k1 key file as the fixture — 2e7d379, b2d5ca0, fd0b80b
  • Update the verification counts in FORK.md — 72e33ca
  • Build both import tests' arguments in one place — 4537133

Testing

  • Before the fix, the new test failed through the CLI with exit 101 and "entered unreachable
    code". After, it exits 1, and the error names the file and asks for an Ed25519 key.
  • The fixture is a consistent secp256k1 key file: its public key and address are derived from
    the secret. Parsing accepts any key that can be a consensus key, and secp256k1 can, so the
    command reaches the branch under test.
  • The existing Ed25519 import test still passes.
  • 64 unit + 24 integration tests pass; cargo clippy --all-features is clean.

@aeddi
aeddi force-pushed the yubihsm-import-rejects-secp256k1 branch from fd0b80b to 891734d Compare November 4, 2026 11:35
@aeddi
aeddi marked this pull request as ready for review November 4, 2026 11:38
@aeddi
aeddi merged commit 2ea7bfa 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