fix(apple): handle errSecInteractionNotAllowed and change keychain protection class (#158)

* fix(apple): handle errSecInteractionNotAllowed (-25308) in keychain load

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.

* fix(apple): use AfterFirstUnlockThisDeviceOnly for keychain protection 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

---------

Co-authored-by: Jay Gowdy <jay@gowdy.me>

Author: jgowdy-godaddy

crates/enclaveapp-apple/src/ffi.rs

@@ -100,9 +100,13 @@ extern "C" {
     //   0   SE_OK
     //   4   SE_ERR_BUFFER_TOO_SMALL
     //   9   SE_ERR_KEYCHAIN_STORE
-    //   10  SE_ERR_KEYCHAIN_LOAD
+    //   10  SE_ERR_KEYCHAIN_LOAD (generic load failure)
     //   11  SE_ERR_KEYCHAIN_DELETE
     //   12  SE_ERR_KEYCHAIN_NOT_FOUND
+    //   13  SE_ERR_KEYCHAIN_AUTH_DENIED (errSecAuthFailed -25293)
+    //   14  SE_ERR_KEYCHAIN_INTERACTION_REQUIRED (errSecInteractionNotAllowed -25308, has CG session)
+    //   15  SE_ERR_KEYCHAIN_NO_WINDOW_SERVER (errSecInteractionNotAllowed -25308, no CG session)
+    //   16  SE_ERR_USER_CANCEL (errSecUserCanceled -128)
     // `access_group` (UTF-8 pointer) + `access_group_len`: when
     // non-null with len > 0, the bridge routes SecItemAdd through the
     // Data Protection keychain with `kSecAttrAccessGroup` set —

crates/enclaveapp-apple/src/keychain.rs

@@ -786,6 +786,7 @@ pub fn load_handle_with_context(
         config.wrapping_key_cache_ttl,
         config.keychain_access_group.as_deref(),
         lacontext_token,
+        Some(config.wrapping_key_user_presence),
     )? {
         Some(plaintext) => Ok(Zeroizing::new(plaintext)),
         None => Err(Error::KeyOperation {

crates/enclaveapp-apple/src/keychain_wrap.rs

@@ -297,6 +297,12 @@ fn cache_evict(app_name: &str, label: &str) {
     crate::lacontext::evict(app_name, label);
 }
 
+/// Evict the cached wrapping key and LAContext for `label`, forcing the
+/// next operation to reload from the keychain with a fresh LAContext.
+pub fn cache_evict_for(app_name: &str, label: &str) {
+    cache_evict(app_name, label);
+}
+
 /// Store a wrapping key in the login keychain. Replaces any existing
 /// entry for the same service+account pair.
 ///
@@ -456,14 +462,14 @@ pub(crate) fn keychain_load(
         }),
         14 => Err(Error::KeychainInteractionRequired {
             // SE_ERR_KEYCHAIN_INTERACTION_REQUIRED: macOS returned
-            // errSecInteractionRequired (-25308) and the process has a CG
+            // errSecInteractionNotAllowed (-25308) and the process has a CG
             // session — screen is locked or biometric was cancelled.
             // Transient: retry after the user unlocks the screen.
             label: label.to_string(),
         }),
         15 => Err(Error::KeychainNoWindowServer {
             // SE_ERR_KEYCHAIN_NO_WINDOW_SERVER: macOS returned
-            // errSecInteractionRequired (-25308) and CGSessionCopyCurrentDictionary()
+            // errSecInteractionNotAllowed (-25308) and CGSessionCopyCurrentDictionary()
             // returned nil — the process has no window server connection.
             // Touch ID UI cannot be displayed. Recovery: restart the agent
             // via launchd so it inherits the user's GUI session.
@@ -542,6 +548,12 @@ pub fn generate_and_wrap(
 /// Returns `Ok(None)` if no wrapping-key entry exists — the caller can
 /// distinguish that from a decrypt/IO error.
 ///
+/// When `use_user_presence` is `Some(bool)` and the wrapping key was
+/// freshly loaded from the keychain (not served from the in-process
+/// cache), the item is re-stored to migrate it to the current data
+/// protection class (`AfterFirstUnlockThisDeviceOnly`). This is a
+/// one-time transparent upgrade for items created before the fix.
+///
 /// Raw wrapping-key bytes are fetched, used, and dropped inside this
 /// function — they do not escape `keychain_wrap`.
 pub fn decrypt_with_cached_key(
@@ -551,9 +563,24 @@ pub fn decrypt_with_cached_key(
     cache_ttl: Duration,
     access_group: Option<&str>,
     lacontext_token: u64,
+    use_user_presence: Option<bool>,
 ) -> Result<Option<Vec<u8>>> {
+    let was_cached = cache_lookup(app_name, label, cache_ttl).is_some();
     match keychain_load(app_name, label, cache_ttl, access_group, lacontext_token)? {
         Some(wrapping_key) => {
+            if !was_cached {
+                if let Some(up) = use_user_presence {
+                    if let Err(e) = keychain_store(app_name, label, &wrapping_key, up, access_group)
+                    {
+                        tracing::warn!(
+                            label = label,
+                            error = %e,
+                            "protection-class migration re-store failed; \
+                             item retains old protection class until next successful load"
+                        );
+                    }
+                }
+            }
             let plaintext = decrypt_blob(&wrapping_key, blob)?;
             Ok(Some(plaintext))
         }

crates/enclaveapp-apple/src/sign.rs

@@ -226,4 +226,8 @@ impl EnclaveSigner for SecureEnclaveSigner {
             }
         }
     }
+
+    fn evict_wrapping_key_cache(&self, label: &str) {
+        crate::keychain_wrap::cache_evict_for(&self.config.app_name, label);
+    }
 }

crates/enclaveapp-apple/swift/bridge.swift

@@ -49,16 +49,16 @@ let SE_ERR_KEYCHAIN_NOT_FOUND: Int32 = 12
 /// ("open Keychain Access and change Deny → Always Allow") rather than
 /// the generic "keychain load error".
 let SE_ERR_KEYCHAIN_AUTH_DENIED: Int32 = 13
-/// Returned when `SecItemCopyMatching` returns `errSecInteractionRequired`
-/// (-25308) — the item requires user presence but no authenticated LAContext
-/// was provided.  This happens when `lacontext_token=0` is passed and the
-/// keychain item was stored with `.userPresence` access control.  The screen
-/// may be locked or biometric auth was cancelled; the caller should retry
-/// after the user unlocks the screen.
+/// Returned when `SecItemCopyMatching` returns `errSecInteractionNotAllowed`
+/// (-25308) or `errSecInteractionRequired` (-25315) — the item requires user
+/// presence but no authenticated LAContext was provided.  This happens when
+/// `lacontext_token=0` is passed and the keychain item was stored with
+/// `.userPresence` access control.  The screen may be locked or biometric auth
+/// was cancelled; the caller should retry after the user unlocks the screen.
 let SE_ERR_KEYCHAIN_INTERACTION_REQUIRED: Int32 = 14
-/// Returned when `SecItemCopyMatching` returns `errSecInteractionRequired`
-/// and `CGSessionCopyCurrentDictionary()` returns nil — meaning the process
-/// has no window server session at all.  Touch ID requires a window server
+/// Returned when `SecItemCopyMatching` returns `errSecInteractionNotAllowed`
+/// or `errSecInteractionRequired` and `CGSessionCopyCurrentDictionary()`
+/// returns nil — meaning the process has no window server session at all.  Touch ID requires a window server
 /// connection to display its prompt; background processes started outside of
 /// launchd (e.g. `sshenc-agent &` in a shell) never get one.  Recovery:
 /// restart the agent via launchd so it inherits the user's GUI session.
@@ -71,6 +71,11 @@ let SE_ERR_KEYCHAIN_NO_WINDOW_SERVER: Int32 = 15
 let SE_ERR_USER_CANCEL: Int32 = 16
 
 // MARK: - Thread-local last error detail
+//
+// Captures the underlying error message (e.g. CryptoKit exception,
+// SecAccessControlCreateWithFlags failure) so the Rust side can include
+// it in diagnostics. Set by any function that returns a non-zero code;
+// cleared on success. Read via enclaveapp_se_last_error().
 
 private var _lastError: String = ""
 private let _lastErrorLock = NSLock()
@@ -273,7 +278,7 @@ func makeAccessControl(_ authPolicy: Int32) -> SecAccessControl? {
     }
     var error: Unmanaged<CFError>?
     let ac = SecAccessControlCreateWithFlags(
-        nil, kSecAttrAccessibleWhenUnlockedThisDeviceOnly, flags, &error
+        nil, kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly, flags, &error
     )
     if ac == nil {
         let desc = error.map { $0.takeRetainedValue().localizedDescription } ?? "unknown"
@@ -722,9 +727,11 @@ public func enclaveapp_se_decrypt(
 // errSecMissingEntitlement (-34018). The legacy keychain accepts
 // unsigned callers.
 //
-// Items are created with `kSecAttrAccessibleWhenUnlockedThisDeviceOnly`:
-// the device must be unlocked to read them, and they do NOT sync via
-// iCloud / migrate across devices.
+// Items are created with `kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly`:
+// available from first unlock until reboot, and they do NOT sync via
+// iCloud / migrate across devices. The AfterFirstUnlock class keeps the
+// class key in memory across sleep/wake cycles so background agents can
+// access wrapping keys without requiring the screen to be unlocked.
 
 /// Open the invoking user's login keychain by explicit path.
 ///
@@ -960,10 +967,17 @@ public func enclaveapp_keychain_store(
         // Bind access to user presence (Touch ID or device passcode)
         // via LocalAuthentication. `kSecAttrAccessControl` implies
         // accessibility, so `kSecAttrAccessible` must NOT also be set.
+        //
+        // AfterFirstUnlockThisDeviceOnly: the class key stays in memory
+        // from first unlock until reboot, so the wrapping key survives
+        // sleep/wake cycles. The .userPresence flag still requires
+        // Touch ID per-access. WhenUnlockedThisDeviceOnly would purge
+        // the class key on lock/sleep, making the wrapping key
+        // inaccessible to a background agent after sleep/wake.
         var acError: Unmanaged<CFError>?
         guard let ac = SecAccessControlCreateWithFlags(
             nil,
-            kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
+            kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly,
             .userPresence,
             &acError
         ) else {
@@ -972,7 +986,7 @@ public func enclaveapp_keychain_store(
         }
         addQuery[kSecAttrAccessControl as String] = ac
     } else {
-        addQuery[kSecAttrAccessible as String] = kSecAttrAccessibleWhenUnlockedThisDeviceOnly
+        addQuery[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
     }
     if let kc = kc { addQuery[kSecUseKeychain as String] = kc }
     var status = SecItemAdd(addQuery as CFDictionary, nil)
@@ -1008,7 +1022,7 @@ public func enclaveapp_keychain_store(
             // Userpresence on legacy keychain is errSecParam → flatten now
             addQuery.removeValue(forKey: kSecAttrAccessControl as String)
             addQuery[kSecAttrAccessible as String] =
-                kSecAttrAccessibleWhenUnlockedThisDeviceOnly
+                kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
         }
         // The DP-keychain initial path runs with `useExplicit = false`,
         // so we never opened a SecKeychain handle. If the default
@@ -1059,7 +1073,7 @@ public func enclaveapp_keychain_store(
         ).utf8))
         addQuery.removeValue(forKey: kSecAttrAccessControl as String)
         addQuery[kSecAttrAccessible as String] =
-            kSecAttrAccessibleWhenUnlockedThisDeviceOnly
+            kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
         status = SecItemAdd(addQuery as CFDictionary, nil)
         keychainTrace(
             "op=store_add_fallback service=\(serviceStr) account=\(accountStr) status=\(status)"
@@ -1142,7 +1156,7 @@ public func enclaveapp_keychain_load(
         if status == errSecAuthFailed {
             return SE_ERR_KEYCHAIN_AUTH_DENIED
         }
-        if status == errSecInteractionRequired {
+        if status == errSecInteractionNotAllowed || status == errSecInteractionRequired {
             // Distinguish "no window server" (agent started outside launchd)
             // from "screen locked" (agent is fine, user needs to unlock).
             // CGSessionCopyCurrentDictionary() returns nil when the process

crates/enclaveapp-core/src/traits.rs

@@ -101,6 +101,10 @@ pub trait EnclaveSigner: EnclaveKeyManager {
         let _ = (mode, cache_ttl_secs, reason);
         self.sign(label, data)
     }
+
+    /// Evict cached wrapping key and LAContext for `label`, forcing the next
+    /// operation to reload from the keychain with fresh authentication.
+    fn evict_wrapping_key_cache(&self, _label: &str) {}
 }
 
 /// ECIES encryption operations. Used by awsenc and sso-jwt for credential caching.