fix(apple): handle errSecInteractionNotAllowed and change keychain protection class#158
Merged
Merged
Conversation
The Swift bridge checked only errSecInteractionRequired (-25315, CSSM layer) but SecItemCopyMatching returns errSecInteractionNotAllowed (-25308) after sleep/wake. The mismatch caused all post-sleep keychain errors to fall through to the generic SE_ERR_KEYCHAIN_LOAD (code 10), bypassing the dedicated recovery paths for codes 14/15. Add errSecInteractionNotAllowed to the status check so the correct error code propagates to Rust. Also expose cache_evict_for() and add evict_wrapping_key_cache() to EnclaveSigner so callers can force a fresh keychain load with a new LAContext on transient failures.
…n class WhenUnlockedThisDeviceOnly purges the keybag class key from memory on device lock/sleep, making wrapping keys inaccessible to background agents after sleep/wake. AfterFirstUnlockThisDeviceOnly keeps the class key in memory from first unlock until reboot, which is the correct behavior for an SSH agent that must sign in the background. Changes: - keychain_store: all protection class references changed from WhenUnlockedThisDeviceOnly to AfterFirstUnlockThisDeviceOnly (userPresence path, non-userPresence path, and both fallback paths) - makeAccessControl: same protection class change for SE key generation with auth_policy, plus proper error capture via CFError - decrypt_with_cached_key: on non-cached keychain load, re-stores the wrapping key to transparently migrate existing items to the new protection class - Swift bridge: add last-error detail mechanism (setLastError / enclaveapp_se_last_error FFI) so Rust callers get CryptoKit and SecAccessControl error descriptions instead of bare error codes - ffi.rs: declare enclaveapp_se_last_error extern - keychain.rs: read and surface bridge error details in GenerateFailed
jgowdy-godaddy
changed the title
jgowdy-godaddy
force-pushed
the
fix/errSecInteractionNotAllowed
branch
from
146f163 to
55c4ec6
Compare
4 tasks
jgowdy-godaddy
added a commit
that referenced
this pull request
May 21, 2026
…rol (#160) PR #158 blanket-replaced all protection classes to AfterFirstUnlockThisDeviceOnly, including makeAccessControl() which sets the Secure Enclave key's access control at generation time. This broke CryptoKit's touchIDAuthenticationAllowableReuseDuration — every sign required Touch ID instead of caching biometric auth. Since the SE key's access control is immutable after creation, fixing this requires regenerating all affected keys. The keychain wrapping key (keychain_store) correctly uses AfterFirstUnlockThisDeviceOnly for sleep/wake survival. The SE key access control must use WhenUnlockedThisDeviceOnly for biometric caching. These are separate decisions with different requirements. Also adds AGENTS.md with protection class safety rules and updates CLAUDE.md to document the split, so this class of mistake is prevented in future changes. Co-authored-by: Jay Gowdy <jay@gowdy.me>
This was referenced May 21, 2026
jgowdy-godaddy
added a commit
that referenced
this pull request
May 21, 2026
…egressions (#163) Add 7 tests for the wrapping-key cache layer: 5 pure-Rust unit tests (run in CI, no keychain needed): - cache_insert_then_lookup_returns_key - cache_evict_removes_entry - cache_lookup_with_zero_ttl_always_misses - cache_entries_are_isolated_by_label / by_app - keychain_store_evicts_cache - migration_restore_must_not_evict_cache (key regression guard) 2 keychain integration tests (#[ignore], run locally): - decrypt_with_cached_key_preserves_cache_after_migration - keychain_store_evicts_but_keychain_store_ffi_does_not The migration_restore_must_not_evict_cache test directly guards the invariant broken in PR #158 — if anyone changes the migration path in decrypt_with_cached_key to call keychain_store instead of keychain_store_ffi, this test fails with an explicit message explaining the biometric caching consequences.
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
2 participants
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.
Summary
errSecInteractionRequired(-25315, CSSM layer) butSecItemCopyMatchingreturnserrSecInteractionNotAllowed(-25308) after sleep/wake. The mismatch caused all post-sleep keychain errors to fall through toSE_ERR_KEYCHAIN_LOAD(code 10), bypassing the dedicated recovery paths for codes 14/15.kSecAttrAccessibleWhenUnlockedThisDeviceOnlytokSecAttrAccessibleAfterFirstUnlockThisDeviceOnly. The old class purges keybag class keys on device lock/sleep, making wrapping keys inaccessible to background agents. The new class keeps the class key in memory from first unlock until reboot — correct behavior for an SSH agent.setLastError/enclaveapp_se_last_errorFFI so Rust callers get CryptoKit and SecAccessControl error descriptions instead of bare error codes.cache_evict_for()and addedevict_wrapping_key_cache()toEnclaveSignerso callers can force a fresh keychain load with a new LAContext on transient failures.Test plan
cargo test --workspacepassescargo clippy --workspace --all-targets -- -D warningsclean