From 43d4fd97ca4a1b50e96dac3f59c49a8617036a8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Powa=C5=82owski?= Date: Tue, 14 Dec 2021 15:56:33 +0100 Subject: [PATCH 1/2] [Auth] Fixed macOS extension keychain access by adding recommended kSecUseDataProtectionKeychain key --- FirebaseAuth/CHANGELOG.md | 1 + .../Sources/SystemService/FIRAuthStoredUserManager.m | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/FirebaseAuth/CHANGELOG.md b/FirebaseAuth/CHANGELOG.md index 0b379a6e9b7..4e60b5766b1 100644 --- a/FirebaseAuth/CHANGELOG.md +++ b/FirebaseAuth/CHANGELOG.md @@ -1,6 +1,7 @@ # Unreleased - [changed] Added a `X-Firebase-GMPID` header to network requests. (#9046) - [fixed] Added multi-tenancy support to generic OAuth providers. (#7990) +- [fixed] macOS Extension access to Shared Keychain by adding kSecUseDataProtectionKeychain recommended key (#6876) # 8.9.0 - [changed] Improved error logging. (#8704) diff --git a/FirebaseAuth/Sources/SystemService/FIRAuthStoredUserManager.m b/FirebaseAuth/Sources/SystemService/FIRAuthStoredUserManager.m index deae373fb4f..ca8d00486eb 100644 --- a/FirebaseAuth/Sources/SystemService/FIRAuthStoredUserManager.m +++ b/FirebaseAuth/Sources/SystemService/FIRAuthStoredUserManager.m @@ -81,6 +81,9 @@ - (FIRUser *)getStoredUserForAccessGroup:(NSString *)accessGroup query[(__bridge id)kSecAttrAccessGroup] = accessGroup; query[(__bridge id)kSecAttrService] = projectIdentifier; query[(__bridge id)kSecAttrAccount] = kSharedKeychainAccountValue; + if (@available(iOS 13.0, macOS 10.15, macCatalyst 13.0, tvOS 13.0, watchOS 6.0, *)) { + query[(__bridge id)kSecUseDataProtectionKeychain] = (__bridge id)kCFBooleanTrue; + } if (shareAuthStateAcrossDevices) { query[(__bridge id)kSecAttrSynchronizable] = (__bridge id)kCFBooleanTrue; } @@ -125,6 +128,9 @@ - (BOOL)setStoredUser:(FIRUser *)user query[(__bridge id)kSecAttrAccessGroup] = accessGroup; query[(__bridge id)kSecAttrService] = projectIdentifier; query[(__bridge id)kSecAttrAccount] = kSharedKeychainAccountValue; + if (@available(iOS 13.0, macOS 10.15, macCatalyst 13.0, tvOS 13.0, watchOS 6.0, *)) { + query[(__bridge id)kSecUseDataProtectionKeychain] = (__bridge id)kCFBooleanTrue; + } if (shareAuthStateAcrossDevices) { query[(__bridge id)kSecAttrSynchronizable] = (__bridge id)kCFBooleanTrue; } @@ -168,6 +174,9 @@ - (BOOL)removeStoredUserForAccessGroup:(NSString *)accessGroup query[(__bridge id)kSecAttrAccessGroup] = accessGroup; query[(__bridge id)kSecAttrService] = projectIdentifier; query[(__bridge id)kSecAttrAccount] = kSharedKeychainAccountValue; + if (@available(iOS 13.0, macOS 10.15, macCatalyst 13.0, tvOS 13.0, watchOS 6.0, *)) { + query[(__bridge id)kSecUseDataProtectionKeychain] = (__bridge id)kCFBooleanTrue; + } return [self.keychainServices removeItemWithQuery:query error:outError]; } From 810d1570368e6019d54463216a7ca3e5f7bfb984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Powa=C5=82owski?= Date: Mon, 20 Dec 2021 12:11:40 +0100 Subject: [PATCH 2/2] Refactored FIRAuthStoredUserManager to extract keychain query building to separated function. --- .../SystemService/FIRAuthStoredUserManager.m | 62 +++++++++---------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/FirebaseAuth/Sources/SystemService/FIRAuthStoredUserManager.m b/FirebaseAuth/Sources/SystemService/FIRAuthStoredUserManager.m index ca8d00486eb..0fa71f17313 100644 --- a/FirebaseAuth/Sources/SystemService/FIRAuthStoredUserManager.m +++ b/FirebaseAuth/Sources/SystemService/FIRAuthStoredUserManager.m @@ -75,19 +75,9 @@ - (FIRUser *)getStoredUserForAccessGroup:(NSString *)accessGroup shareAuthStateAcrossDevices:(BOOL)shareAuthStateAcrossDevices projectIdentifier:(NSString *)projectIdentifier error:(NSError *_Nullable *_Nullable)outError { - NSMutableDictionary *query = [[NSMutableDictionary alloc] init]; - query[(__bridge id)kSecClass] = (__bridge id)kSecClassGenericPassword; - - query[(__bridge id)kSecAttrAccessGroup] = accessGroup; - query[(__bridge id)kSecAttrService] = projectIdentifier; - query[(__bridge id)kSecAttrAccount] = kSharedKeychainAccountValue; - if (@available(iOS 13.0, macOS 10.15, macCatalyst 13.0, tvOS 13.0, watchOS 6.0, *)) { - query[(__bridge id)kSecUseDataProtectionKeychain] = (__bridge id)kCFBooleanTrue; - } - if (shareAuthStateAcrossDevices) { - query[(__bridge id)kSecAttrSynchronizable] = (__bridge id)kCFBooleanTrue; - } - + NSMutableDictionary *query = [self keychainQueryForAccessGroup:accessGroup + shareAuthStateAcrossDevices:shareAuthStateAcrossDevices + projectIdentifier:projectIdentifier]; NSData *data = [self.keychainServices getItemWithQuery:query error:outError]; // If there's an outError parameter and it's populated, or there's no data, return. if ((outError && *outError) || !data) { @@ -116,25 +106,15 @@ - (BOOL)setStoredUser:(FIRUser *)user shareAuthStateAcrossDevices:(BOOL)shareAuthStateAcrossDevices projectIdentifier:(NSString *)projectIdentifier error:(NSError *_Nullable *_Nullable)outError { - NSMutableDictionary *query = [[NSMutableDictionary alloc] init]; - query[(__bridge id)kSecClass] = (__bridge id)kSecClassGenericPassword; + NSMutableDictionary *query = [self keychainQueryForAccessGroup:accessGroup + shareAuthStateAcrossDevices:shareAuthStateAcrossDevices + projectIdentifier:projectIdentifier]; if (shareAuthStateAcrossDevices) { query[(__bridge id)kSecAttrAccessible] = (__bridge id)kSecAttrAccessibleAfterFirstUnlock; } else { query[(__bridge id)kSecAttrAccessible] = (__bridge id)kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly; } - - query[(__bridge id)kSecAttrAccessGroup] = accessGroup; - query[(__bridge id)kSecAttrService] = projectIdentifier; - query[(__bridge id)kSecAttrAccount] = kSharedKeychainAccountValue; - if (@available(iOS 13.0, macOS 10.15, macCatalyst 13.0, tvOS 13.0, watchOS 6.0, *)) { - query[(__bridge id)kSecUseDataProtectionKeychain] = (__bridge id)kCFBooleanTrue; - } - if (shareAuthStateAcrossDevices) { - query[(__bridge id)kSecAttrSynchronizable] = (__bridge id)kCFBooleanTrue; - } - #if TARGET_OS_WATCH NSKeyedArchiver *archiver = [[NSKeyedArchiver alloc] initRequiringSecureCoding:false]; #else @@ -159,26 +139,44 @@ - (BOOL)removeStoredUserForAccessGroup:(NSString *)accessGroup shareAuthStateAcrossDevices:(BOOL)shareAuthStateAcrossDevices projectIdentifier:(NSString *)projectIdentifier error:(NSError *_Nullable *_Nullable)outError { - NSMutableDictionary *query = [[NSMutableDictionary alloc] init]; - query[(__bridge id)kSecClass] = (__bridge id)kSecClassGenericPassword; + NSMutableDictionary *query = [self keychainQueryForAccessGroup:accessGroup + shareAuthStateAcrossDevices:shareAuthStateAcrossDevices + projectIdentifier:projectIdentifier]; if (shareAuthStateAcrossDevices) { query[(__bridge id)kSecAttrAccessible] = (__bridge id)kSecAttrAccessibleAfterFirstUnlock; } else { query[(__bridge id)kSecAttrAccessible] = (__bridge id)kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly; } - if (shareAuthStateAcrossDevices) { - query[(__bridge id)kSecAttrSynchronizable] = (__bridge id)kCFBooleanTrue; - } + return [self.keychainServices removeItemWithQuery:query error:outError]; +} + +#pragma mark - Internal Methods +- (NSMutableDictionary *)keychainQueryForAccessGroup:(NSString *)accessGroup + shareAuthStateAcrossDevices:(BOOL)shareAuthStateAcrossDevices + projectIdentifier:(NSString *)projectIdentifier { + NSMutableDictionary *query = [[NSMutableDictionary alloc] init]; + query[(__bridge id)kSecClass] = (__bridge id)kSecClassGenericPassword; query[(__bridge id)kSecAttrAccessGroup] = accessGroup; query[(__bridge id)kSecAttrService] = projectIdentifier; query[(__bridge id)kSecAttrAccount] = kSharedKeychainAccountValue; + if (@available(iOS 13.0, macOS 10.15, macCatalyst 13.0, tvOS 13.0, watchOS 6.0, *)) { + /* + "The data protection key affects operations only in macOS. + Other platforms automatically behave as if the key is set to true, + and ignore the key in the query dictionary. You can safely use the key on all platforms." + [kSecUseDataProtectionKeychain](https://developer.apple.com/documentation/security/ksecusedataprotectionkeychain) + */ query[(__bridge id)kSecUseDataProtectionKeychain] = (__bridge id)kCFBooleanTrue; } - return [self.keychainServices removeItemWithQuery:query error:outError]; + if (shareAuthStateAcrossDevices) { + query[(__bridge id)kSecAttrSynchronizable] = (__bridge id)kCFBooleanTrue; + } + + return query; } @end