Skip to content

Fix device wipe on final PIN attempt - #380

Open
schnuartz-ai wants to merge 8 commits into
cryptoadvance:masterfrom
Schnuartz:fix/pin-attempts-off-by-one-test
Open

schnuartz-ai wants to merge 8 commits into
cryptoadvance:masterfrom
Schnuartz:fix/pin-attempts-off-by-one-test

Conversation

@schnuartz-ai

Copy link
Copy Markdown
Contributor

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

  • Nine wrong PINs leave one attempt.
  • The correct PIN on the tenth attempt unlocks the device.
  • A wrong PIN on the tenth attempt wipes the device.

Testing

  • python -m py_compile src/keystore/flash.py
  • Logic simulation confirms the old and new tenth-attempt behavior.
  • Hardware testing on a Specter DIY device is required before merging.

…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.
@netlify

netlify Bot commented Aug 19, 2026

Copy link
Copy Markdown

Deploy Preview for specter-diy-docs ready!

Name Link
🔨 Latest commit f4f77ee
🔍 Latest deploy log https://app.netlify.com/projects/specter-diy-docs/deploys/6a8df469e720d30008054412
😎 Deploy Preview https://deploy-preview-380--specter-diy-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@Schnuartz

Copy link
Copy Markdown
Contributor

I tested that on Real Hardware.

The bug is real. In both cases I entered the wrong PIN nine times in a row, followed by one correct entry.

After entering it correctly:

In the current firmware, the device was wiped. In the fix I did, everything stayed the same, as it should.
IMG_20260819_140336

after that, I inserted the wrong PIN code ten times in a row, and this time, after the tenth time, everything got deleted as intended.
Timelock receovery (3)

@Schnuartz Schnuartz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checked it on Hardware and had a look at the very tiny code. Approved.

@Schnuartz
Schnuartz requested review from k9ert and miketlk August 19, 2026 12:33
@Schnuartz

Copy link
Copy Markdown
Contributor

@al-munazzim can you review

@al-munazzim al-munazzim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.md to 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.

Comment thread src/keystore/flash.py Outdated
Comment on lines +127 to +132
# 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!")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working prio1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Device wipes before checking the final PIN attempt

5 participants