Skip to content

Commit de5236f

Browse files
authoredMar 13, 2025
Refactor FXIOS-11415 logins keychain calls (#24845)
* Replaced logins MZKeychainWrapper calls with RustKeychain calls * Added RustKeychain logins tests, removed unnecessary RustLogins variable
1 parent a117c8a commit de5236f

File tree

12 files changed

+515
-76
lines changed

12 files changed

+515
-76
lines changed
 

‎firefox-ios/Client/Application/AppDelegate.swift

+8-1
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,17 @@ class AppDelegate: UIResponder, UIApplicationDelegate, FeatureFlaggable {
2222
.value()
2323
.creditCardAutofillStatus
2424

25+
private let rustKeychainEnabled = FxNimbus.shared
26+
.features
27+
.rustKeychainRefactor
28+
.value()
29+
.rustKeychainEnabled
30+
2531
lazy var profile: Profile = BrowserProfile(
2632
localName: "profile",
2733
fxaCommandsDelegate: UIApplication.shared.fxaCommandsDelegate,
28-
creditCardAutofillEnabled: creditCardAutofillStatus
34+
creditCardAutofillEnabled: creditCardAutofillStatus,
35+
rustKeychainEnabled: rustKeychainEnabled
2936
)
3037

3138
lazy var themeManager: ThemeManager = DefaultThemeManager(sharedContainerIdentifier: AppInfo.sharedContainerIdentifier)

‎firefox-ios/Client/FeatureFlags/NimbusFlaggableFeature.swift

+4-1
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ enum NimbusFeatureFlagID: String, CaseIterable {
5858
case toolbarNavigationHint
5959
case tosFeature
6060
case trackingProtectionRefactor
61+
case useRustKeychain
6162
case zoomFeature
6263

6364
// Add flags here if you want to toggle them in the `FeatureFlagsDebugViewController`. Add in alphabetical order.
@@ -80,7 +81,8 @@ enum NimbusFeatureFlagID: String, CaseIterable {
8081
.pdfRefactor,
8182
.downloadLiveActivities,
8283
.unifiedAds,
83-
.unifiedSearch:
84+
.unifiedSearch,
85+
.useRustKeychain:
8486
return rawValue + PrefsKeys.FeatureFlags.DebugSuffixKey
8587
default:
8688
return nil
@@ -158,6 +160,7 @@ struct NimbusFlaggableFeature: HasNimbusSearchBar {
158160
.toolbarNavigationHint,
159161
.tosFeature,
160162
.trackingProtectionRefactor,
163+
.useRustKeychain,
161164
.zoomFeature:
162165
return nil
163166
}

‎firefox-ios/Client/Frontend/Settings/Main/Debug/FeatureFlags/FeatureFlagsDebugViewController.swift

+7
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,13 @@ final class FeatureFlagsDebugViewController: SettingsTableViewController, Featur
143143
) { [weak self] _ in
144144
self?.reloadView()
145145
},
146+
FeatureFlagsBoolSetting(
147+
with: .useRustKeychain,
148+
titleText: format(string: "Enable Rust Keychain"),
149+
statusText: format(string: "Toggle to enable Rust Components rust keychain")
150+
) { [weak self] _ in
151+
self?.reloadView()
152+
},
146153
FeatureFlagsBoolSetting(
147154
with: .sentFromFirefox,
148155
titleText: format(string: "Enable Sent from Firefox"),

‎firefox-ios/Client/Nimbus/NimbusFeatureFlagLayer.swift

+7
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,9 @@ final class NimbusFeatureFlagLayer {
145145
case .trackingProtectionRefactor:
146146
return checkTrackingProtectionRefactor(from: nimbus)
147147

148+
case .useRustKeychain:
149+
return checkUseRustKeychainFeature(from: nimbus)
150+
148151
case .zoomFeature:
149152
return checkZoomFeature(from: nimbus)
150153
}
@@ -443,4 +446,8 @@ final class NimbusFeatureFlagLayer {
443446
private func checkNICErrorPageFeature(from nimbus: FxNimbus) -> Bool {
444447
return nimbus.features.nativeErrorPageFeature.value().noInternetConnectionError
445448
}
449+
450+
private func checkUseRustKeychainFeature(from nimbus: FxNimbus) -> Bool {
451+
return nimbus.features.rustKeychainRefactor.value().rustKeychainEnabled
452+
}
446453
}

‎firefox-ios/Providers/Profile.swift

+44-19
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,10 @@ open class BrowserProfile: Profile {
230230
fatalError("Could not create directory at root path: \(error)")
231231
}
232232
}()
233+
private var rustKeychainEnabled = false
233234
fileprivate let name: String
234-
fileprivate let keychain: MZKeychainWrapper
235+
fileprivate let keychain: RustKeychain
236+
fileprivate let legacyKeychain: MZKeychainWrapper
235237
var isShutdown = false
236238

237239
internal let files: FileAccessor
@@ -257,14 +259,17 @@ open class BrowserProfile: Profile {
257259
init(localName: String,
258260
fxaCommandsDelegate: FxACommandsDelegate? = nil,
259261
creditCardAutofillEnabled: Bool = false,
262+
rustKeychainEnabled: Bool = false,
260263
clear: Bool = false,
261264
logger: Logger = DefaultLogger.shared) {
262265
logger.log("Initing profile \(localName) on thread \(Thread.current).",
263266
level: .debug,
264267
category: .setup)
265268
self.name = localName
266269
self.files = ProfileFileAccessor(localName: localName)
267-
self.keychain = MZKeychainWrapper.sharedClientAppContainerKeychain
270+
self.rustKeychainEnabled = rustKeychainEnabled
271+
self.keychain = KeychainManager.shared
272+
self.legacyKeychain = KeychainManager.legacyShared
268273
self.logger = logger
269274
self.fxaCommandsDelegate = fxaCommandsDelegate
270275

@@ -301,7 +306,11 @@ open class BrowserProfile: Profile {
301306
logger.log("New profile. Removing old Keychain/Prefs data.",
302307
level: .info,
303308
category: .setup)
304-
MZKeychainWrapper.wipeKeychain()
309+
if rustKeychainEnabled {
310+
RustKeychain.wipeKeychain()
311+
} else {
312+
MZKeychainWrapper.wipeKeychain()
313+
}
305314
prefs.clearAll()
306315
}
307316

@@ -311,7 +320,8 @@ open class BrowserProfile: Profile {
311320
// Initiating the sync manager has to happen prior to the databases being opened,
312321
// because opening them can trigger events to which the SyncManager listens.
313322
self.syncManager = RustSyncManager(profile: self,
314-
creditCardAutofillEnabled: creditCardAutofillEnabled)
323+
creditCardAutofillEnabled: creditCardAutofillEnabled,
324+
rustKeychainEnabled: rustKeychainEnabled)
315325

316326
let notificationCenter = NotificationCenter.default
317327

@@ -676,7 +686,7 @@ open class BrowserProfile: Profile {
676686
fileURLWithPath: directory,
677687
isDirectory: true
678688
).appendingPathComponent("loginsPerField.db").path
679-
return RustLogins(databasePath: databasePath)
689+
return RustLogins(databasePath: databasePath, rustKeychainEnabled: self.rustKeychainEnabled)
680690
}()
681691

682692
lazy var remoteSettingsService: RemoteSettingsService? = {
@@ -755,24 +765,39 @@ open class BrowserProfile: Profile {
755765
prefs.removeObjectForKey(PrefsKeys.KeyLastRemoteTabSyncTime)
756766

757767
// Save the keys that will be restored
758-
let rustAutofillKey = RustAutofillEncryptionKeys()
759-
let creditCardKey = keychain.string(forKey: rustAutofillKey.ccKeychainKey)
760768
let rustLoginsKeys = RustLoginEncryptionKeys()
761-
let perFieldKey = keychain.string(forKey: rustLoginsKeys.loginPerFieldKeychainKey)
762-
// Remove all items, removal is not key-by-key specific (due to the risk of failing to delete something),
763-
// simply restore what is needed.
764-
keychain.removeAllKeys()
765-
766-
if let perFieldKey = perFieldKey {
767-
keychain.set(
768-
perFieldKey,
769-
forKey: rustLoginsKeys.loginPerFieldKeychainKey,
770-
withAccessibility: .afterFirstUnlock
771-
)
769+
let rustAutofillKey = RustAutofillEncryptionKeys()
770+
var loginsKey: String?
771+
let creditCardKey = legacyKeychain.string(forKey: rustAutofillKey.ccKeychainKey)
772+
773+
if rustKeychainEnabled {
774+
(loginsKey, _) = keychain.getLoginsKeyData()
775+
776+
// Remove all items, removal is not key-by-key specific (due to the risk of failing to delete something),
777+
// simply restore what is needed.
778+
keychain.removeAllKeys()
779+
780+
if let loginsKey = loginsKey {
781+
keychain.setLoginsKey(loginsKey)
782+
}
783+
} else {
784+
loginsKey = legacyKeychain.string(forKey: rustLoginsKeys.loginPerFieldKeychainKey)
785+
786+
// Remove all items, removal is not key-by-key specific (due to the risk of failing to delete something),
787+
// simply restore what is needed.
788+
legacyKeychain.removeAllKeys()
789+
790+
if let loginsKey = loginsKey {
791+
legacyKeychain.set(loginsKey,
792+
forKey: rustLoginsKeys.loginPerFieldKeychainKey,
793+
withAccessibility: .afterFirstUnlock)
794+
}
772795
}
773796

774797
if let creditCardKey = creditCardKey {
775-
keychain.set(creditCardKey, forKey: rustAutofillKey.ccKeychainKey, withAccessibility: .afterFirstUnlock)
798+
legacyKeychain.set(creditCardKey,
799+
forKey: rustAutofillKey.ccKeychainKey,
800+
withAccessibility: .afterFirstUnlock)
776801
}
777802

778803
// Tell any observers that our account has changed.

‎firefox-ios/Providers/RustSyncManager.swift

+12-3
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public class RustSyncManager: NSObject, SyncManager {
3636
private let fxaDeclinedEngines = "fxa.cwts.declinedSyncEngines"
3737
private var notificationCenter: NotificationProtocol
3838
var creditCardAutofillEnabled = false
39+
var rustKeychainEnabled = false
3940

4041
let fifteenMinutesInterval = TimeInterval(60 * 15)
4142

@@ -68,6 +69,7 @@ public class RustSyncManager: NSObject, SyncManager {
6869

6970
init(profile: BrowserProfile,
7071
creditCardAutofillEnabled: Bool = false,
72+
rustKeychainEnabled: Bool = false,
7173
logger: Logger = DefaultLogger.shared,
7274
notificationCenter: NotificationProtocol = NotificationCenter.default) {
7375
self.profile = profile
@@ -77,6 +79,7 @@ public class RustSyncManager: NSObject, SyncManager {
7779

7880
super.init()
7981
self.creditCardAutofillEnabled = creditCardAutofillEnabled
82+
self.rustKeychainEnabled = rustKeychainEnabled
8083
}
8184

8285
@objc
@@ -273,9 +276,15 @@ public class RustSyncManager: NSObject, SyncManager {
273276
.prefsForSync
274277
.branch("scratchpad")
275278
.stringForKey("keyLabel") {
276-
MZKeychainWrapper
277-
.sharedClientAppContainerKeychain
278-
.removeObject(forKey: keyLabel)
279+
if self.rustKeychainEnabled {
280+
RustKeychain
281+
.sharedClientAppContainerKeychain
282+
.removeObject(key: keyLabel)
283+
} else {
284+
MZKeychainWrapper
285+
.sharedClientAppContainerKeychain
286+
.removeObject(forKey: keyLabel)
287+
}
279288
}
280289
self.prefsForSync.clearAll()
281290
}

‎firefox-ios/Storage/MockRustKeychain.swift

+37-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// License, v. 2.0. If a copy of the MPL was not distributed with this
33
// file, You can obtain one at http://mozilla.org/MPL/2.0/
44

5+
import func MozillaAppServices.createCanary
6+
import func MozillaAppServices.createKey
7+
58
class MockRustKeychain: RustKeychain {
69
static let shared = MockRustKeychain()
710

@@ -11,16 +14,45 @@ class MockRustKeychain: RustKeychain {
1114
super.init(serviceName: "Test")
1215
}
1316

14-
override func getBaseKeychainQuery(key: String) -> [String: Any] {
15-
return [:]
17+
override public func removeObject(key: String) {
18+
_ = storage.removeValue(forKey: key)
19+
}
20+
21+
override public func removeAllKeys() {
22+
storage.removeAll()
23+
}
24+
25+
override public func setLoginsKey(_ value: String) {
26+
storage[loginsKeyIdentifier] = value
27+
}
28+
29+
override public func getLoginsKeyData() -> (String?, String?) {
30+
return (storage[loginsKeyIdentifier], storage[loginsCanaryKeyIdentifier])
31+
}
32+
33+
override public class func wipeKeychain() {}
34+
35+
override func createLoginsKeyData() throws -> String {
36+
guard let keyValue = try? createKey(),
37+
let canaryValue = try? createCanary(text: loginsCanaryPhrase, encryptionKey: keyValue) else {
38+
throw LoginEncryptionKeyError.noKeyCreated
39+
}
40+
41+
storage[loginsCanaryKeyIdentifier] = canaryValue
42+
storage[loginsKeyIdentifier] = keyValue
43+
return keyValue
1644
}
1745

1846
override func queryKeychainForKey(key: String) -> Result<String?, Error> {
1947
return .success(storage[key])
2048
}
2149

22-
override func addOrUpdateKeychainKey(_ value: String, key: String) -> OSStatus {
23-
storage[key] = value
24-
return errSecSuccess
50+
override func createAndStoreKey(canaryPhrase: String, canaryIdentifier: String, keyIdentifier: String) throws -> String {
51+
let keyValue = try createKey()
52+
let canaryValue = try createCanary(text: canaryPhrase, encryptionKey: keyValue)
53+
54+
storage[canaryIdentifier] = canaryValue
55+
storage[keyIdentifier] = keyValue
56+
return keyValue
2557
}
2658
}

0 commit comments

Comments
 (0)
Failed to load comments.