Skip to content

Commit 602f3e7

Browse files
authored
[Auth] Handle corrupt keychain value resulting from incomplete v11 port (#13643)
1 parent d39335c commit 602f3e7

File tree

1 file changed

+81
-1
lines changed

1 file changed

+81
-1
lines changed

FirebaseAuth/Sources/Swift/SystemService/AuthStoredUserManager.swift

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,87 @@ class AuthStoredUserManager {
119119
archiver.encode(user, forKey: Self.storedUserCoderKey)
120120
archiver.finishEncoding()
121121

122-
try keychainServices.setItem(archiver.encodedData, withQuery: query)
122+
// In Firebase 10, the below query contained the `kSecAttrSynchronizable`
123+
// key set to `true` when `shareAuthStateAcrossDevices == true`. This
124+
// allows a user entry to be shared across devices via the iCloud keychain.
125+
// For the purpose of this discussion, such a user entry will be referred
126+
// to as a "iCloud entry". Conversely, a "non-iCloud entry" will refer to a
127+
// user entry stored when `shareAuthStateAcrossDevices == false`. Keep in
128+
// mind that this class exclusively manages user entries stored in
129+
// device-specific keychain access groups, so both iCloud and non-iCloud
130+
// entries are implicitly available at the device level to apps that
131+
// have access rights to the specific keychain access group used.
132+
//
133+
// The iCloud/non-iCloud distinction is important because entries stored
134+
// with `kSecAttrSynchronizable == true` can only be retrieved when the
135+
// search query includes `kSecAttrSynchronizable == true`. Likewise,
136+
// entries stored without the `kSecAttrSynchronizable` key (or
137+
// `kSecAttrSynchronizable == false`) can only be retrieved when
138+
// the search query omits `kSecAttrSynchronizable` or sets it to `false`.
139+
//
140+
// So for each access group, the SDK manages up to two buckets in the
141+
// keychain, one for iCloud entries and one for non-iCloud entries.
142+
//
143+
// From Firebase 11.0.0 up to but not including 11.3.0, the
144+
// `kSecAttrSynchronizable` key was *not* included in the query when
145+
// `shareAuthStateAcrossDevices == true`. This had the effect of the iCloud
146+
// bucket being inaccessible, and iCloud and non-iCloud entries attempting
147+
// to be written to the same bucket. This was problematic because the
148+
// two types of entries use another flag, the `kSecAttrAccessible` flag,
149+
// with different values. If two queries are identical apart from different
150+
// values for their `kSecAttrAccessible` key, whichever query written to
151+
// the keychain first won't be accessible for reading or updating via the
152+
// other query (resulting in a OSStatus of -25300 indicating the queried
153+
// item cannot be found). And worse, attempting to write the other query to
154+
// the keychain won't work because the write will conflict with the
155+
// previously written query (resulting in a OSStatus of -25299 indicating a
156+
// duplicate item already exists in the keychain). This formed the basis
157+
// for the issues this bug caused.
158+
//
159+
// The missing key was added back in 11.3, but adding back the key
160+
// introduced a new issue. If the buggy version succeeded at writing an
161+
// iCloud entry to the non-iCloud bucket (e.g. keychain was empty before
162+
// iCloud entry was written), then all future non-iCloud writes would fail
163+
// due to the mismatching `kSecAttrAccessible` flag and throw an
164+
// unrecoverable error. To address this the below error handling is used to
165+
// detect such cases, remove the "corrupt" iCloud entry stored by the buggy
166+
// version in the non-iCloud bucket, and retry writing the current
167+
// non-iCloud entry again.
168+
do {
169+
try keychainServices.setItem(archiver.encodedData, withQuery: query)
170+
} catch let error as NSError {
171+
guard shareAuthStateAcrossDevices == false,
172+
error.localizedFailureReason == "SecItemAdd (-25299)" else {
173+
// The error is not related to the 11.0 - 11.2 issue described above,
174+
// and should be rethrown.
175+
throw error
176+
}
177+
// We are trying to write a non-iCloud entry but a corrupt iCloud entry
178+
// is likely preventing it from happening.
179+
//
180+
// The corrupt query was supposed to contain the following keys:
181+
// {
182+
// kSecAttrSynchronizable: true,
183+
// kSecAttrAccessible: kSecAttrAccessibleAfterFirstUnlock
184+
// }
185+
// Instead, it contained:
186+
// {
187+
// kSecAttrAccessible: kSecAttrAccessibleAfterFirstUnlock
188+
// }
189+
//
190+
// Excluding `kSecAttrSynchronizable` treats the query as if it's false
191+
// and the entry won't be shared in iCloud across devices. It is instead
192+
// written to the non-iCloud bucket. This query is corrupting the
193+
// non-iCloud bucket because its `kSecAttrAccessible` value is not
194+
// compatible with the value used for non-iCloud entries. To delete it,
195+
// a compatible query is formed by swapping the accessibility flag
196+
// out for `kSecAttrAccessibleAfterFirstUnlock`. This frees up the bucket
197+
// so the non-iCloud entry can attempt to be written again.
198+
let corruptQuery = query
199+
.merging([kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlock]) { $1 }
200+
try keychainServices.removeItem(query: corruptQuery)
201+
try keychainServices.setItem(archiver.encodedData, withQuery: query)
202+
}
123203
}
124204

125205
/// Remove the user that stored locally.

0 commit comments

Comments
 (0)