fix(dpop): cache key pair to drop per-request keychain lookup and lock - #4178
Merged
Merged
Conversation
keyPair(forScope:) did a Keychain lookup (SecItemCopyMatching) and took a global exclusive barrier lock on every DPoP proof build, serializing all DPoP-bound requests behind uncached securityd round-trips. Add a process-local key-pair cache keyed by scope: warm hits are served via a non-barrier concurrent read (no Keychain I/O, no lock); the barrier is taken only on the cold load/generate path (double-checked to guarantee a single mint) and on delete, which evicts the cached entry. Key material and wire behavior are unchanged: proofs and headers are byte-identical; signing still runs per request against the Keychain-backed key. iOS counterpart to Android forcedotcom#3045.
sfdctaka
marked this pull request as ready for review
September 22, 2026 21:02
|
||||||||||||||||||
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #4178 +/- ##
==========================================
- Coverage 71.73% 67.81% -3.92%
==========================================
Files 254 254
Lines 23027 23040 +13
==========================================
- Hits 16518 15625 -893
- Misses 6509 7415 +906
🚀 New features to boost your workflow:
|
wmathurin
reviewed
Sep 22, 2026
wmathurin
reviewed
Sep 22, 2026
Contributor
|
One PR-description correction: “proofs and headers are byte-identical” is not accurate because DPoP proofs intentionally vary through |
|
||||||||||||||
…plicit Address review feedback: - hasKeyPair doc now states key material is reported from the in-process cache OR the Keychain, and notes the out-of-band-deletion window. - Rework the hasKeyPair test into a genuine cache-first regression test: remove the persisted key directly (bypassing delete, so the cache is not evicted), assert presence still holds via the cache, then clear the cache and assert the Keychain fallback reports absence.
wmathurin
approved these changes
Sep 22, 2026
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
DPoPKeyStore.keyPair(forScope:)performed a Keychain lookup (SecItemCopyMatching) and acquired a global exclusive barrier lock on every DPoP proof build (resource + token endpoint). Under DPoP, that serialized all DPoP-bound requests behind uncachedsecuritydround-trips.This adds a process-local key-pair cache keyed by credential scope:
.barrieris taken only to load-or-generate the key pair once, double-checked so concurrent cold callers converge on a single mint.hasKeyPair(forScope:)short-circuits on a cache hit.Key material, proof format, claims, and header-attachment behavior are unchanged; signing still occurs independently for every request (the proof's
jti/iatand its ECDSA signature vary per request by design). No public API change; no feature flag (straight revert to roll back).iOS counterpart to Android #3045.
Verification
SFSDKDPoPTests: warm-hit object identity, concurrent convergence (viaconcurrentPerform, single-mint), delete eviction, in-memory-clear reloads the same persistent key, and ahasKeyPaircache-first regression test (persisted key removed out of band, cache still reports present until cleared).SalesforceSDKCoreTestsfull scheme passing (iPhone 16, iOS 18.6).docs/auth/token-lifecycle.mdupdated to document the cache and eviction-on-delete.