Fix device wipe on final PIN attempt - #380
schnuartz-ai wants to merge 8 commits into
Conversation
…ity-model.md - Add root SECURITY.md: vulnerability reporting via association@specter.solutions, GPG keys of release signers for encrypted reports, scope and disclosure process - Rename docs/security.md to docs/security-model.md (it describes the security model) - Rewrite security-model.md: threat model, firmware verification incl. bootloader details and RDP, PIN brute-force protection, storage modes (temporary seed, smartcard, reckless), entropy, communication channels, known limitations - Update README.md and docs/README.md links accordingly
Future Claude Code commits in this repo use the Schnuartz AI name/email and drop the Co-Authored-By/Claude-Session trailer, matching the convention already used on feature branches.
✅ Deploy Preview for specter-diy-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Schnuartz
left a comment
There was a problem hiding this comment.
Checked it on Hardware and had a look at the very tiny code. Approved.
|
@al-munazzim can you review |
al-munazzim
left a comment
There was a problem hiding this comment.
Reviewed the final PIN-attempt change and ran local checks on the PR branch: python3 -m py_compile src/keystore/flash.py and python3 test/run_native_tests.py both completed successfully (native suite: 5/5). The counter is still decremented/persisted before verification, preserving the power-cut rewind protection, but wiping is now gated on an actually-wrong final PIN. Combined with the reported hardware test for correct-vs-wrong tenth attempt, this looks good to me.
There was a problem hiding this comment.
Pull request overview
This PR fixes an off-by-one issue in FlashKeyStore._unlock() where the device could wipe before verifying the final PIN attempt, and updates the security model documentation to reflect the corrected behavior.
Changes:
- Adjust
FlashKeyStore._unlock()so the device wipes only when the final attempt is actually wrong (while still persisting the decremented counter before verification). - Update
docs/security-model.mdto describe the corrected 10th-attempt behavior and the power-cut rewind rationale.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/keystore/flash.py | Moves the “wipe on exhausted attempts” logic to occur only after a failed PIN verification. |
| docs/security-model.md | Updates brute-force protection documentation to match the corrected final-attempt behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # wipe only if the LAST attempt was just used up - | ||
| # the counter is decreased before verification to | ||
| # protect against power-cut rewind attacks | ||
| if self._pin_attempts_left <= 0: | ||
| self.wipe(self.path) | ||
| raise CriticalErrorWipeImmediately("No more PIN attempts!\nWipe!") |


Summary
Fix the internal-storage PIN attempt off-by-one behavior. Fixes Schnuartz#7.
FlashKeyStore._unlock()previously wiped the device as soon as the counter reached zero, before checking the entered PIN. After nine wrong entries, the UI reported one attempt remaining, but the tenth entry wiped the device even when the PIN was correct.This change keeps decrementing and persisting the counter before verification to preserve the power-cut rewind protection discussed in #229. It verifies the PIN first, and wipes only when the final attempt was actually wrong.
Expected behavior
Testing
python -m py_compile src/keystore/flash.py