CTAP2 ClientPIN secrets bypass the checked RNG and retain the ECDH private key

[BitHighlander]

Affected candidate

Draft PR #621 at 79c2bc853eaba0277d06afcf7a13df3c25eaec13 (firmware 7.16.0).

Finding

The new CTAP2 ClientPIN implementation creates authentication secrets with unchecked RNG calls, then retains the ECDH private key after a successful exchange.

Evidence in lib/firmware/ctap2.c:

• generate_key_agreement() draws the P-256 private key with plain random_buffer(), ignores ecdsa_get_public_key65()'s failure result, and sets key_agreement.valid = true unconditionally.
• setPIN and changePIN draw storage.pin_salt with plain random_buffer().
• getPINToken draws the 32-byte pin_token with plain random_buffer().
• Successful set/change/token operations only assign key_agreement.valid = false; they do not wipe the static private/public key.
• The setPIN invalid-MAC path can derive shared_secret and return through the compound condition without wiping it.

include/keepkey/rand/rng_health.h explicitly states that plain random_buffer() is unchecked and that adding a new key-material draw does not inherit the boot-lifetime health gate. These 7.16 call sites are absent from its complete covered list.

Impact

High / release blocker. A source that the firmware health machinery would reject can still generate the ClientPIN ECDH private key and pin_token. A predictable token permits pinUvAuthParam forgery without the PIN; a predictable ECDH scalar exposes PIN-protocol traffic. An invalid/out-of-range P-256 draw is advertised as a valid all-zero public key because the conversion result is ignored. Successful exchanges also leave the private scalar resident until another key agreement or reset overwrites it.

This is independent of #613: fixing the hardware-error latch does not protect call sites that bypass random_buffer_checked().

Required remediation

• Route the key-agreement scalar, PIN token, and security-relevant salt draws through random_buffer_checked() and fail the command closed on a bad verdict.
• Rejection-sample a valid P-256 scalar and check ecdsa_get_public_key65() before setting valid or encoding a response.
• Use cleanup blocks that wipe shared_secret, plaintext PIN material, verifiers, and authenticated-message buffers on every exit after population.
• Wipe the complete key-agreement object after successful consumption; if retry reuse is required, channel-bind and expire it explicitly.
• Add injected-RNG regressions for a latched failure, all-zero output, out-of-range scalar, and predictable/stuck output; no PIN token or usable public key may be returned.

[BitHighlander]

Code remediation is in #621 at e7937a0e.

The ClientPIN ECDH scalar, PIN salts, and PIN token now use random_buffer_checked(). P-256 scalars are rejection-sampled and validated before advertisement. The key agreement is one-shot, channel-bound, expires after 30 seconds, and is completely wiped on first attempted consumption; token invalidation and populated secret buffers are wiped through the relevant exits.

New regressions cover checked-entropy failure, zero/all-FF out-of-range scalars, a valid boundary scalar, and first-use consumption/wiping. Full and Bitcoin-only emulator matrices pass.

Leaving open until #621 CI and independent review complete.