Pin the KDF parameter chains for every production deriver - #572
Merged
Merged
Conversation
Backup.Secure.v1 had no drift protection because a pinned key vector costs minutes of KDF. uniqueAlgorithmIdentifier already encodes the whole chain, so pin the exact identifier strings, the persisted signature raw values, and the signature lookup table instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bradleymackey
added a commit
that referenced
this pull request
Sep 15, 2026
## Changes (test-only) — final PR of the pre-release audit series (#561–#572) Snapshot coverage for security-relevant screens rebuilt in #551–#560 that had no view tests: - **`BackupKeyDecryptorViewSnapshotTests`** — initial state (light/dark × 3 type sizes, in a NavigationStack so the Cancel toolbar renders) plus a deterministic decrypt-failure state: the failure is produced by actually running `attemptDecryption()` with the fast testing deriver and an erroring decoder mock, not by faking view state. - **`BackupImportFlowViewSnapshotTests`** — all three `BackupImportContext` variants (empty vault / merge / override), light/dark. - **`VaultDetailEncryptionEditViewSnapshotTests`** — encryption-disabled (full grid) and encryption-enabled variants. - `SettingsDangerView` was covered in #565; `AutoBackupSettingsView` is deliberately not given a standalone suite — it is already snapshotted transitively through `BackupCreateView` (`BackupViewSnapshotTests`), and its enabled/error states are only reachable through an async `.task` handoff that would flake under synchronous snapshot rendering. Driving those states needs a small initial-state injection refactor — left as follow-up. Release gate: full `CI_iOS` scheme (all 13 test targets, `iOSAllTests` plan including the TSAN configuration) run locally on iPhone 18 Pro Max / iOS 27.0.⚠️ Automatic CI is still disabled (#548), so this is local verification only. --- ## Release findings — report-only (no code in this series) The pre-release audit surfaced the following items that need **design decisions**, not patches. Recorded here so they are not lost: **MANIFESTO C7 gaps (protective defaults):** - Clipboard paste TTL defaults to never-expire (`PasteTTL.default = nil`) — copied OTPs/passwords sit on the pasteboard indefinitely unless the user opts in to a TTL. - No screenshot / app-switcher privacy protection anywhere (no `privacySensitive()`, no capture detection, no cover view). - Danger Zone full wipe has no confirmation dialog — one tap + biometric. **Design-level:** - Backups export killphrase/search-passphrase salts+digests; anyone holding the backup password can enumerate which items are duress-protected (C5 tension). - The killphrase/search-passphrase HMAC keys are device-local and not exported, so a restore onto a new device silently disarms every killphrase and permanently hides `.onlyPassphrase` items (rows exist, digests unverifiable). - No app-level lock / auto-lock; background purge clears only the backup password from memory. - Killphrase-triggered auto-backup + widget reload is an out-of-band success signal for a hidden item's deletion (C2 tension). - `payloadHash` and `lastBackupHash` live in plaintext UserDefaults — mutation-time evidence (C6 tension). - `deleteVault()` does not refresh the auto-backup hash, so the newest auto-backup still describes the wiped vault (recovery safety net vs C6 — decide). - `DerivedEncryptionKey.debugDescription` prints raw key material as hex; keychain replace (remove→store) is non-atomic; killphrase/passphrase edit fields are plain `TextField` not `SecureField`; the `vault://` HOTP-increment deep link is unauthenticated; `Data.random` relies on `SystemRandomNumberGenerator` (CSPRNG on Apple platforms, but unannotated as the app's sole randomness source); no `protectedDataWillBecomeUnavailable` handling. **Hygiene (non-blocking):** CI triggers commented out; CHANGELOG ~9 versions stale vs MARKETING_VERSION 2.0; hardcoded strings in rebuilt screens bypass the string catalogs (app is currently English-only, so cosmetic); stale scheme/test-plan references (`CI_iOS` scheme, orphan `VaultUITests` scheme) and a stale snapshot directory; `VaultBackup.xcstrings` not declared as a target resource; the keygen speedtest CLI prints a derived key in hex; feed search reload has no debounce/cancellation; `ForEach` identity built from `Hasher().finalize()`; reorder persist failures are swallowed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Backup.Secure.v1— the deriver that protects stolen backups (PBKDF2 5,452,351 iterations → HKDF-SHA3/512 → scrypt N=2^18) — had no drift protection: only fast derivers have pinned key vectors, because a full vector test of the secure chain costs minutes of KDF per run and would rot skipped. Any accidental edit to the secure parameters would silently break decryption of every existing backup.Changes (test-only)
New
VaultKeyDeriverParameterPinTests, exploiting the fact thatuniqueAlgorithmIdentifieralready encodes the complete chain — algorithm order, key length, iterations, variants, cost factors — including nesting viaCOMBINATION<...|...>:backupSecureV1_pinsExactKDFChain/backupFastV1/itemSecureV1/itemFastV1— each pins the exact identifier string. Any parameter drift fails on every CI run at zero KDF cost. A comment records the rule: parameter changes are a new keygen version (new signature), never an edit to v1.signatureIDs_areStable— pins the persisted signature raw values (stored in backups and the keychain for decrypt-time lookup).lookup_returnsDeriverMatchingEverySignature— the signature → deriver table stays consistent across all cases.Together with the existing fast pinned vectors in
VaultKeyDeriverTests(which prove the shared composition machinery produces stable output), this covers secure-parameter drift without minutes of KDF.Local verification: VaultKeygenTests scheme green on iPhone 18 Pro Max / iOS 27.0.
🤖 Generated with Claude Code