feat(keychain): opt-in Touch ID / Face ID via SecAccessControl#549
Closed
ndeloof wants to merge 1 commit into
Closed
feat(keychain): opt-in Touch ID / Face ID via SecAccessControl#549ndeloof wants to merge 1 commit into
ndeloof wants to merge 1 commit into
Conversation
Add a WithBiometricAuth(prompt) DarwinOptions that wraps stored secrets in a kSecAccessControl object requiring Touch ID, Face ID, or the device passcode before the keychain releases the secret. The optional prompt string customises the system dialog via kSecUseOperationPrompt at query time. The default remains kSecAttrAccessibleAfterFirstUnlock (password popup on first access of the session), so existing callers see no behavioural change. Items already in the keychain when the option is enabled keep their original ACL; only secrets written after construction pick up the biometric policy. Implementation notes: - accesscontrol_darwin.go in internal/go-keychain exposes the cgo bindings for SecAccessControlCreateWithFlags + the flag constants (UserPresence, BiometryAny, BiometryCurrentSet, DevicePasscode, Or/And combiners). - AccessControl implements the existing Convertable interface so the ConvertMapToCFDictionary release path correctly frees the CFTypeRef after each dictionary build, avoiding leaks across multi-call store operations (Save -> Delete -> Save). - SetAccessControl deliberately removes kSecAttrAccessible: the protection class is encoded inside the SecAccessControl object and supplying both yields errSecParam. - The default flag combination is AccessControlUserPresence, which accepts any enrolled biometric and falls back to the device passcode. This is friendlier than BiometryCurrentSet (which invalidates the item when a fingerprint or face enrolment changes). Non-darwin platforms keep their existing behaviour: the option is declared cross-platform but only the darwin keychain store implements the new setBiometricAuth method on darwinOptions, so the option falls through errSkipOptions on Linux/Windows. Tests: - AccessControl.Convert() validates flag combinations and the unsupported-protection failure path without touching the system keychain. - SetAccessControl removes kSecAttrAccessible from the Item's attr map. - Store construction wires the flag and prompt through New(); the default-off invariant is covered explicitly. - A full Save/Get round-trip is intentionally NOT exercised in the test suite because the macOS authentication dialog would block CI.
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
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
Add an opt-in
WithBiometricAuth(prompt)DarwinOptionsso callers can require Touch ID / Face ID (with the device passcode as a fallback) instead of the keychain password popup when releasing a stored secret.The default behaviour is unchanged: keychain items keep
kSecAttrAccessibleAfterFirstUnlockand prompt for the user's macOS password on first access of the session. The new option is fully opt-in.Why
Downstream callers (e.g. docker/sandboxes) want to give end-users a friendlier auth prompt when reading credentials from the keychain. Touch ID / Face ID with passcode fallback is the natural macOS pattern, but the current
store/keychainAPI exposes only thekSecAttrAccessible*constants — no path to setkSecAttrAccessControlor wire a custom operation prompt.Implementation
internal/go-keychain/accesscontrol_darwin.go— cgo bindings forSecAccessControlCreateWithFlags, thekSecAccessControl*flag constants (UserPresence,BiometryAny,BiometryCurrentSet,DevicePasscode,Or/Andcombiners), andkSecAttrAccessControl/kSecUseOperationPromptattribute keys. The newAccessControlvalue implements the existingConvertableinterface soConvertMapToCFDictionaryreleases the freshly-createdCFTypeRefonce the dictionary is destroyed — important becauseItemis reused across multi-call store operations (Save→Delete→Save).SetAccessControlintentionally dropskSecAttrAccessiblefrom the item's attribute map: the protection class is encoded inside theSecAccessControlobject, and supplying both yieldserrSecParam.store/keychain/keychain.go— exposesWithBiometricAuth(prompt string) DarwinOptions. Non-darwin platforms keep their existing behaviour: only the darwin store implements the newsetBiometricAuthmethod on thedarwinOptionsinterface, so the option falls througherrSkipOptionson Linux/Windows.store/keychain/keychain_darwin.go—newKeychainItemsetskSecUseOperationPrompton every query when biometric auth is enabled;applyBiometricACLswapsAccessible→AccessControlonSave. The default flag combination isAccessControlUserPresence, which accepts any enrolled biometric and falls back to the device passcode — friendlier thanBiometryCurrentSet, which would invalidate the item whenever a fingerprint or face enrolment changes on the device.Migration
Items already in the keychain keep their original ACL. Only secrets written after the store is constructed with
WithBiometricAuthpick up the biometric policy. Downstream callers that want every existing secret to be protected need a one-off delete + re-save migration; this is documented in the option's godoc.Test plan
AccessControl.Convert()covered for multiple valid flag combinations + the unsupported-protection failure path, without touching the system keychain.SetAccessControlremoveskSecAttrAccessiblefrom theItem.attrmap.New(...)plumbs the flag and prompt through; default-off invariant is covered explicitly.TestMacosKeychain,TestUpsert,TestConvertAttributesstill pass (Save/Get round-trips on the defaultAccessiblepath remain unchanged).go.modreplacedirective.Downstream integration
I'm pulling this branch into
docker/sandboxesviago.mod replaceand gating it behind a newplatform.keychainBiometricAuthsetting (off by default; opt-in viasbx settings set).