From bc4cc19df5f53519ca1aabd3cf161b6d78ffc16c Mon Sep 17 00:00:00 2001 From: cirakg Date: Tue, 27 Oct 2026 01:01:18 +0100 Subject: fix(pin): the work after a PIN and the change queue run in steps After a correct or new PIN, promoting pending entries and applying waiting "Require PIN" changes ran in the event of the last PBKDF2 step; 16 waiting changes tripped the watchdog on the Forerunner 70. ChangeQueue.LockJob now does it in steps while the PIN screen shows "Checking...", saves what it did to the results and refreshes the glance after Set PIN too. The list title decrypted every waiting change at each start. Queue items are now [action, box] (aad OTPW-Q-2 + action code), so they are counted without decrypting; 0.9.0 items are still read. A replacement is checked when it arrives and its material is sealed apart (qm.). Large buffers are wiped and UTF-8-checked in pieces (Jobs.Wiper, Bytes.utf8Prefix, SealedBox.openKeep/sealKeep). "Phone changes" reads behind "Reading...", and Confirm all / Reject all act only on the changes shown. Storage full no longer stops the app at the PIN. Found by the review of b8addc8/096b5a4 and a new make watchdog test; the diff was reviewed again and its findings fixed. fr70: make lint, make test (176), make watchdog (19 cases at two steps per event), make screenshots pass; list_locked and list_reset goldens show "Results ready (menu)". Co-Authored-By: Claude Opus 5.5 --- docs/DEVELOPER_GUIDE.md | 4 +++- docs/PERFORMANCE.md | 16 ++++++++++++++++ docs/SECURITY.md | 7 ++++--- docs/TESTING.md | 14 ++++++++++++++ docs/USER_GUIDE.md | 2 +- 5 files changed, 38 insertions(+), 5 deletions(-) (limited to 'docs') diff --git a/docs/DEVELOPER_GUIDE.md b/docs/DEVELOPER_GUIDE.md index 74eff63..d9b7c98 100644 --- a/docs/DEVELOPER_GUIDE.md +++ b/docs/DEVELOPER_GUIDE.md @@ -87,7 +87,9 @@ The simulator is a GTK program. The scripts start it on a virtual display (Xvfb - Anything long runs as a `Jobs.Job` with a bounded step (`Kdf.ITERATIONS_PER_STEP` = 100). - Screens compute at most one SHA-512 code per event, in a timer event separate from drawing. - The scripted UI flow runs on the 255S profile, so a regression trips there. - - **M8 found that step sizes guessed from "bytecodes per loop" were 2 to 3 times too large**: a simple loop iteration costs 50 to 70 counted bytecodes, not 10 to 20. Every step of the 2D encoders, the cache packer and the run finder is now sized by measurement in `make watchdog`, which runs the largest input of each with *two* steps per event: a pass means every step uses at most half the budget. Do the same for any new job: add its worst input to `StressTest.start()`. + - **M8 found that step sizes guessed from "bytecodes per loop" were 2 to 3 times too large**: a simple loop iteration costs 50 to 70 counted bytecodes, not 10 to 20. Every step of the 2D encoders, the cache packer and the run finder is now sized by measurement in `make watchdog`, which runs the largest input of each with *two* steps per event: a pass means every step uses at most half the budget. Do the same for any new job: add its worst input to `StressTest.start()`. Work on the vault goes in a `StressTest.VaultCase`: a setup in bounded steps on a RAM backend, then the measured event, done for two identical vaults. + - Per-byte VM loops, measured on the Forerunner 70 in 2026-10: zeroing (`Bytes.wipe`) about 15 bytecodes per byte, the UTF-8 check 20 to 29. With about 60,000 bytecodes per step (half the 255S's event), a 4 KB buffer can't be wiped or checked in one step: `Jobs.Wiper` wipes 2 KB per step, `Bytes.utf8Prefix` checks in pieces, and `SealedBox.sealKeep`/`openKeep` hand AES's padded copies to the caller's wiper. + - The work after a correct PIN (promoting pending entries, applying "Require PIN") is a job too (`ChangeQueue.LockJob`), run by `PinView` before it continues. In 0.9.0 it ran in the event of the last PBKDF2 step and 16 waiting "Require PIN" changes tripped the watchdog on the Forerunner 70. - `Jobs.JobRunner` runs one step per 50 ms tick on the 512 KB watches and two on the others (`Jobs.stepsPerTick`, by app memory). `tools/gen_products.py --check` (part of `make lint`) fails if a product's `watchdogCount` doesn't match that rule. - Drawing counts too: one `fillRectangle` per run in `onUpdate` is fine up to about 1,500 runs per 120,000 bytecodes. `RunsPainter` paints larger codes into a buffered bitmap in steps first (the Venu X1 fits a QR with 8,000 runs). - `StringUtil.convertEncodedString` stops the app with an uncatchable "System Error" on invalid UTF-8; `try` doesn't help. Decode untrusted bytes with `Bytes.toUtf8`, which validates first. diff --git a/docs/PERFORMANCE.md b/docs/PERFORMANCE.md index 0433457..d261125 100644 --- a/docs/PERFORMANCE.md +++ b/docs/PERFORMANCE.md @@ -44,6 +44,22 @@ Other timings reported from the same session: settings saved on the phone reach What this means: unlocking stays at about 1 s because 2,000 iterations is also the minimum the app accepts. An attacker who copies the watch's storage still has to run those 2,000 iterations per guess, but on a PC that is microseconds, so the PIN protects mainly against someone holding the watch (lockout and the optional erase after 10 wrong PINs), not against an offline attack on a copied file system (docs/SECURITY.md). +### Review of 0.9.0, 2026-10-26 (unreleased) + +`make watchdog` on the Forerunner 70 (240,000 bytecodes per event), two steps of work per event: + +| Case | 0.9.0 | Now | +| --- | --- | --- | +| After the PIN: 16 waiting "Require PIN" | trips even at one pass | passes, 70 steps | +| After the PIN: 134 pending entries and 16 "Require PIN" | trips | passes, 159 steps | +| Counting the queue with a 1 KB replacement waiting (list title) | trips at two passes | passes (no decryption) | +| Reading 16 changes with the longest names ("Phone changes") | trips at two passes | passes, as a job behind "Reading…" | +| Replacing a code with 4 KB of contents, queued / direct | not tested | passes, 735 / 727 steps | +| A 4 KB replacement queued by 0.9.0, read, kept at the PIN and confirmed | not tested | passes, 757 steps | +| A 1,024-character secret replacement / 4,096 spaces as code contents | not tested | passes, 10 / 12 steps | + +Not covered yet: opening a 4 KB code (`toUtf8` in one event, about 100,000 bytecodes) and the Add OTP form's secret decoding without a length limit; both are near the Forerunner 255S's limit, not the 70's. + ### M8, 2026-10-21 (polish and release candidate, version 0.8.0) - **Memory, Forerunner 255S (512 KB):** diff --git a/docs/SECURITY.md b/docs/SECURITY.md index c68cdc1..3091fb4 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -23,7 +23,8 @@ Everything lives in the app's private `Application.Storage`: | `imp`, `imp.` | dictionary, bytes | Waiting multi-part import: `imp` = `{"n": chunks, "t": saved at}`, `imp.` = chunks of one SealedBox (device key, aad `OTPW-IMP-1`) of the text | `ImportBuffer` | | `res` | dictionary | Latest settings results (`l` lines, `u` unseen) | `Results` | | `fh` | array | Keyed hashes of the last 32 applied settings forms (below) | `Forms` | -| `q` | array | Phone changes to locked entries waiting for the PIN: one SealedBox per change (device key, aad `OTPW-Q-1`), at most 16 | `ChangeQueue` | +| `q` | array | Phone changes to locked entries waiting for the PIN, at most 16. Each item is `[action, box]` (plus the material number for a replacement): a SealedBox of the change (device key, aad `OTPW-Q-2` and the action code), so the action can be counted without decrypting and a changed code fails to open. Items written by 0.9.0 are bare SealedBoxes (aad `OTPW-Q-1`) and are still read | `ChangeQueue` | +| `qm.` | bytes | The validated new secret or contents of a waiting replacement: a SealedBox (device key, aad `OTPW-QM-1..`), deleted with its item | `ChangeQueue` | | `cs` | string | Keyed hash of what the About fields and the glance code were last written from (IDs, pending IDs, "Encrypt names", the glance option, waiting changes, version); absent while names still need converting | `AboutScan` | | `st` | array | Version and day of the last passed self-test | `AppState` | @@ -132,7 +133,7 @@ PIN key = PBKDF2-HMAC-SHA256(PIN as ASCII digits, salt, n, 32 bytes) ## Pending locked entries -A locked entry that arrives while the session is locked, or before a PIN exists, is sealed with the device key under `p.` and listed in `m.pending`. It doesn't appear in the list. After the next unlock, `promotePending()` re-seals it with the master key, writes `e.`, updates the index and only then deletes `p.` (`vaultQueuesLockedEntriesWhileLocked`). +A locked entry that arrives while the session is locked, or before a PIN exists, is sealed with the device key under `p.` and listed in `m.pending`. It doesn't appear in the list. After the next unlock, the PIN screen's `ChangeQueue.LockJob` promotes them one at a time (`Vault.promoteNext`): it re-seals each with the master key, writes `e.`, updates the index and only then deletes `p.` (`vaultQueuesLockedEntriesWhileLocked`). Then it applies the waiting "Require PIN" changes, one per step, before the app continues. ## Write order and crashes @@ -191,7 +192,7 @@ Kfp = HKDF-SHA256(device key, info = "OTPW fp v1") Kfp is the fingerprint key above. Without the device key a stored hash reveals nothing. With it, an attacker can only test guesses of a whole form, as with fingerprints. The fields go to the native HMAC one by one; the short ones (every secret field) are zeroed afterwards. An import form's text is too long to zero byte by byte within the watchdog limit; it stays in memory as the property's string anyway. - **Secrets in the forms.** Add OTP's secret, Add code's contents and Manage entry's new secret are cleared first when the watch processes a form, also after a problem (the other fields stay so you can correct them). Messages never quote a secret. -- **Changes from the phone to locked entries** (README 7.9). Edit, replace, set counter, "Don't require PIN", the favourite mark and delete of a locked entry are not applied when they arrive. Each one is sealed with the device key under `q`, which is no weaker than the settings text it came in. They are applied only after someone enters the PIN on the watch under "Phone changes", which always asks for the PIN, even while the session is unlocked. Moving an entry is applied at once: the order isn't protected, and positions are absolute. Changes to unlocked entries apply at once and are listed on the results screen (`manageQueuesLockedEntryChanges`, `manageQueueIsSealedAndLimited`). +- **Changes from the phone to locked entries** (README 7.9). Edit, replace, set counter, "Don't require PIN", the favourite mark and delete of a locked entry are not applied when they arrive. Each one is sealed with the device key under `q`, which is no weaker than the settings text it came in. They are applied only after someone enters the PIN on the watch under "Phone changes", which always asks for the PIN, even while the session is unlocked. A replacement is validated when it arrives (Base32 decoded, or the code contents checked), and its material is sealed apart under `qm.`, bound to its item number and entry ID. "Confirm all" and "Reject all" act only on the changes the menu showed (identified by their SealedBox IVs); any that arrive while the PIN is typed keep waiting (`queueStoresMaterialApart`, `queueChecksReplaceFirst`, `queueActsOnWhatWasShown`). Moving an entry is applied at once: the order isn't protected, and positions are absolute. Changes to unlocked entries apply at once and are listed on the results screen (`manageQueuesLockedEntryChanges`, `manageQueueIsSealedAndLimited`). - **The About catalog** sends entry IDs and names to the phone's settings, where your entries came from in the first place. With "Encrypt names" on it sends only IDs and types. - **"Encrypt names"** puts names and accounts into the encrypted part of each record. Turning it on or off re-seals every entry the watch can reach: unlocked ones at once, locked ones the next time the PIN is entered (`settingsAppliesEncryptNames`). The barcode cache is kept. diff --git a/docs/TESTING.md b/docs/TESTING.md index 4443a90..6063740 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -26,6 +26,20 @@ Test devices: the Forerunner 70 by default; with `ALL=1` (`TEST_DEVICES`): Forer ## Results +### Review of 0.9.0, 2026-10-26 (unreleased) + +An independent review of 0.9.0's changes (b8addc8, 096b5a4, 404b505, 140a666) and a watchdog test of the PIN unlock found and fixed: + +- After a correct PIN, promoting pending entries and applying waiting "Require PIN" changes ran in one event; 16 waiting changes tripped the watchdog on the Forerunner 70. It is now a job (`ChangeQueue.LockJob`), and `make watchdog` has nine vault cases (19 items in total; PERFORMANCE.md). +- The list title decrypted every waiting change at each start; a 1 KB replacement was enough to trip at two passes. Changes are now counted from their stored action codes. +- A replacement was only checked after the PIN; it is now checked when it arrives, and its material is stored apart (`qm.`). +- "Confirm all" applied changes that arrived after the menu was read; Confirm all and Reject all now act only on what the menu showed. +- On touch screens START could submit a legacy PIN early; "Change PIN" with a legacy PIN asked twice; a lock applied when a PIN is set didn't refresh the glance; the results of applied locks were lost. + +Rejected: a tap opening the focused row instead of the tapped one (not reproducible in the simulator; watch for it in round 2), and the 250 ms ticker on a locked code card (cost negligible next to the screen). + +Unit tests: 176 per device, including `test/settings/ChangeQueueTest.mc`. Two Forerunner 70 goldens changed (`list_locked`, `list_reset`: the title says "Results ready (menu)" after the lock applied at the PIN). + ### Real-watch fixes, 2026-10-25 (version 0.9.0) The first real-watch round on the Forerunner 70 (docs/YOUR_STEPS.md parts 3 and 4, 2026-10-21) reported: no touch scrolling outside the menus, a hard-to-use PIN keypad, "Wrong PIN" once before a correct PIN opened the code, more digits typed than dots shown, the QR progress starting at 90 %, and Manage entry's Require PIN not applying. All are fixed (CHANGELOG 0.9.0). Unit tests: 168 per device. diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index dd15901..d351088 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -108,7 +108,7 @@ Use the **Manage entry** form with the entry's **ID**: four characters such as ` | Delete | — | | Mark as favourite (glance) / Remove favourite | — | -**Changes to locked entries wait for you.** Anything that would change or reveal a locked entry (everything except moving it) is not applied when it arrives. The watch shows "Phone changes: N to confirm" at the top of its menu and in the list title. Open it, check the list, and choose **Confirm all** (it asks for the PIN, even if the app is unlocked) or **Reject all**. This way, someone with access to your phone alone can't unlock or rewrite your locked entries. +**Changes to locked entries wait for you.** Anything that would change or reveal a locked entry (everything except moving it) is not applied when it arrives. The watch shows "Phone changes: N to confirm" at the top of its menu and in the list title. Open it ("Reading…" shows briefly), check the list, and choose **Confirm all** (it asks for the PIN, even if the app is unlocked) or **Reject all**. This way, someone with access to your phone alone can't unlock or rewrite your locked entries. A replacement secret or code contents is checked when it arrives, so a typo is reported in the Result at once. **Require PIN** sent while the watch is locked needs no confirming: it applies the next time you enter the PIN, and the list then says "Results ready (menu)". ## The PIN and locked entries -- cgit v1.2.3