[App Check] Migrate AppAttestKeyIDStorage from UserDefaults to Keychain

FirebaseAppCheck/CHANGELOG.md

@@ -1,6 +1,7 @@
 # 10.17.0
 - [fixed] Replaced semantic imports (`@import FirebaseAppCheckInterop`) with umbrella header imports
-  (`#import <FirebaseAppCheckInterop/FirebaseAppCheckInterop.h>`) for ObjC++ compatibility (#11916).
+  (`#import <FirebaseAppCheckInterop/FirebaseAppCheckInterop.h>`) for ObjC++ compatibility. (#11916)
+- [fixed] Prevented App Attest key migration during setup or restoration of a device from a backup. (#11962)
 
 # 10.9.0
 - [feature] Added `limitedUseToken(completion:)` for obtaining limited-use tokens for

FirebaseAppCheck/Sources/AppAttestProvider/FIRAppAttestProvider.m

@@ -140,7 +140,9 @@ - (nullable instancetype)initWithApp:(FIRApp *)app {
       sessionWithConfiguration:[NSURLSessionConfiguration ephemeralSessionConfiguration]];
 
   FIRAppAttestKeyIDStorage *keyIDStorage =
-      [[FIRAppAttestKeyIDStorage alloc] initWithAppName:app.name appID:app.options.googleAppID];
+      [[FIRAppAttestKeyIDStorage alloc] initWithAppName:app.name
+                                                  appID:app.options.googleAppID
+                                            accessGroup:app.options.appGroupID];
 
   FIRAppCheckAPIService *APIService =
       [[FIRAppCheckAPIService alloc] initWithURLSession:URLSession

FirebaseAppCheck/Sources/AppAttestProvider/Storage/FIRAppAttestKeyIDStorage.h

@@ -17,6 +17,7 @@
 #import <Foundation/Foundation.h>
 
 @class FBLPromise<ValueType>;
+@class GULKeychainStorage;
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -53,7 +54,14 @@ NS_ASSUME_NONNULL_BEGIN
  *  @param appID A Firebase App identifier (`FirebaseOptions.googleAppID`). The app ID will be used
  * as a part of the key to store the token for the storage instance.
  */
-- (instancetype)initWithAppName:(NSString *)appName appID:(NSString *)appID;
+- (instancetype)initWithAppName:(NSString *)appName
+                          appID:(NSString *)appID
+                    accessGroup:(nullable NSString *)accessGroup;
+
+- (instancetype)initWithAppName:(NSString *)appName
+                          appID:(NSString *)appID
+                keychainStorage:(GULKeychainStorage *)keychainStorage
+                    accessGroup:(nullable NSString *)accessGroup NS_DESIGNATED_INITIALIZER;
 
 @end
 

FirebaseAppCheck/Sources/AppAttestProvider/Storage/FIRAppAttestKeyIDStorage.m

@@ -22,64 +22,91 @@
 #import "FBLPromises.h"
 #endif
 
+#import <GoogleUtilities/GULKeychainStorage.h>
+
 #import "FirebaseAppCheck/Sources/Core/Errors/FIRAppCheckErrorUtil.h"
 
-/// The `NSUserDefaults` suite name for the storage location of the app attest key ID.
-static NSString *const kKeyIDStorageDefaultsSuiteName = @"com.firebase.FIRAppAttestKeyIDStorage";
+static NSString *const kKeychainService = @"com.firebase.app_check.app_attest_key_id_storage";
 
 @interface FIRAppAttestKeyIDStorage ()
 
 @property(nonatomic, readonly) NSString *appName;
 @property(nonatomic, readonly) NSString *appID;
-
-/// The app attest key ID is stored using `NSUserDefaults` .
-@property(nonatomic, readonly) NSUserDefaults *userDefaults;
+@property(nonatomic, readonly) GULKeychainStorage *keychainStorage;
+@property(nonatomic, readonly, nullable) NSString *accessGroup;
 
 @end
 
 @implementation FIRAppAttestKeyIDStorage
 
-- (instancetype)initWithAppName:(NSString *)appName appID:(NSString *)appID {
+- (instancetype)initWithAppName:(NSString *)appName
+                          appID:(NSString *)appID
+                keychainStorage:(GULKeychainStorage *)keychainStorage
+                    accessGroup:(nullable NSString *)accessGroup {
   self = [super init];
   if (self) {
     _appName = [appName copy];
     _appID = [appID copy];
-    _userDefaults = [[NSUserDefaults alloc] initWithSuiteName:kKeyIDStorageDefaultsSuiteName];
+    _keychainStorage = keychainStorage;
+    _accessGroup = [accessGroup copy];
   }
   return self;
 }
 
-- (nonnull FBLPromise<NSString *> *)setAppAttestKeyID:(nullable NSString *)keyID {
-  [self storeAppAttestKeyID:keyID];
-  return [FBLPromise resolvedWith:keyID];
+- (instancetype)initWithAppName:(NSString *)appName
+                          appID:(NSString *)appID
+                    accessGroup:(nullable NSString *)accessGroup {
+  GULKeychainStorage *keychainStorage =
+      [[GULKeychainStorage alloc] initWithService:kKeychainService];
+  return [self initWithAppName:appName
+                         appID:appID
+               keychainStorage:keychainStorage
+                   accessGroup:accessGroup];
 }
 
-- (nonnull FBLPromise<NSString *> *)getAppAttestKeyID {
-  NSString *appAttestKeyID = [self appAttestKeyIDFromStorage];
-  if (appAttestKeyID) {
-    return [FBLPromise resolvedWith:appAttestKeyID];
+- (nonnull FBLPromise<NSString *> *)setAppAttestKeyID:(nullable NSString *)keyID {
+  if (keyID) {
+    return [self storeAppAttestKeyID:keyID].recover(^NSError *(NSError *error) {
+      return [FIRAppCheckErrorUtil keychainErrorWithError:error];
+    });
   } else {
-    NSError *error = [FIRAppCheckErrorUtil appAttestKeyIDNotFound];
-    FBLPromise *rejectedPromise = [FBLPromise pendingPromise];
-    [rejectedPromise reject:error];
-    return rejectedPromise;
+    return [self.keychainStorage removeObjectForKey:[self keyIDStorageKey]
+                                        accessGroup:self.accessGroup]
+        .then(^id _Nullable(NSNull *_Nullable value) {
+          return nil;
+        })
+        .recover(^NSError *(NSError *error) {
+          return [FIRAppCheckErrorUtil keychainErrorWithError:error];
+        });
   }
 }
 
-#pragma mark - Helpers
-
-- (void)storeAppAttestKeyID:(nullable NSString *)keyID {
-  if (keyID) {
-    [self.userDefaults setObject:keyID forKey:[self keyIDStorageKey]];
-  } else {
-    [self.userDefaults removeObjectForKey:[self keyIDStorageKey]];
-  }
+- (nonnull FBLPromise<NSString *> *)getAppAttestKeyID {
+  return [self.keychainStorage getObjectForKey:[self keyIDStorageKey]
+                                   objectClass:[NSString class]
+                                   accessGroup:self.accessGroup]
+      .then(^NSString *(id<NSSecureCoding> storedKeyID) {
+        NSString *keyID = (NSString *)storedKeyID;
+        if ([keyID isKindOfClass:[NSString class]]) {
+          return keyID;
+        } else {
+          return nil;
+        }
+      })
+      .recover(^NSError *(NSError *error) {
+        return [FIRAppCheckErrorUtil appAttestKeyIDNotFound];
+      });
 }
 
-- (nullable NSString *)appAttestKeyIDFromStorage {
-  NSString *appAttestKeyID = nil;
-  appAttestKeyID = [self.userDefaults objectForKey:[self keyIDStorageKey]];
-  return appAttestKeyID;
+#pragma mark - Helpers
+
+- (FBLPromise<NSString *> *)storeAppAttestKeyID:(nullable NSString *)keyID {
+  return [self.keychainStorage setObject:keyID
+                                  forKey:[self keyIDStorageKey]
+                             accessGroup:self.accessGroup]
+      .then(^id _Nullable(NSNull *_Nullable value) {
+        return keyID;
+      });
 }
 
 - (NSString *)keyIDStorageKey {

FirebaseAppCheck/Tests/Unit/AppAttestProvider/Storage/FIRAppAttestKeyIDStorageTests.m

@@ -14,8 +14,23 @@
  * limitations under the License.
  */
 
+#import <TargetConditionals.h>
+
+// Tests that use the Keychain require a host app and Swift Package Manager
+// does not support adding a host app to test targets.
+#if !SWIFT_PACKAGE
+
+// Skip keychain tests on Catalyst and macOS. Tests are skipped because they
+// involve interactions with the keychain that require a provisioning profile.
+// See go/firebase-macos-keychain-popups for more details.
+#if !TARGET_OS_MACCATALYST && !TARGET_OS_OSX
+
 #import <XCTest/XCTest.h>
 
+#import <OCMock/OCMock.h>
+
+#import <GoogleUtilities/GULKeychainStorage.h>
+
 #import "FBLPromise+Testing.h"
 
 #import "FirebaseAppCheck/Sources/AppAttestProvider/Storage/FIRAppAttestKeyIDStorage.h"
@@ -35,7 +50,9 @@ - (void)setUp {
 
   self.appName = @"FIRAppAttestKeyIDStorageTestsApp";
   self.appID = @"app_id";
-  self.storage = [[FIRAppAttestKeyIDStorage alloc] initWithAppName:self.appName appID:self.appID];
+  self.storage = [[FIRAppAttestKeyIDStorage alloc] initWithAppName:self.appName
+                                                             appID:self.appID
+                                                       accessGroup:nil];
 }
 
 - (void)tearDown {
@@ -48,7 +65,9 @@ - (void)tearDown {
 }
 
 - (void)testInitWithApp {
-  XCTAssertNotNil([[FIRAppAttestKeyIDStorage alloc] initWithAppName:self.appName appID:self.appID]);
+  XCTAssertNotNil([[FIRAppAttestKeyIDStorage alloc] initWithAppName:self.appName
+                                                              appID:self.appID
+                                                        accessGroup:nil]);
 }
 
 - (void)testSetAndGetAppAttestKeyID {
@@ -73,10 +92,29 @@ - (void)testRemoveAppAttestKeyID {
 }
 
 - (void)testGetAppAttestKeyID_WhenAppAttestKeyIDNotFoundError {
-  __auto_type getPromise = [self.storage getAppAttestKeyID];
+  // 1. Set up storage mock.
+  id mockKeychainStorage = OCMClassMock([GULKeychainStorage class]);
+  FIRAppAttestKeyIDStorage *keyIDStorage =
+      [[FIRAppAttestKeyIDStorage alloc] initWithAppName:self.appName
+                                                  appID:self.appID
+                                        keychainStorage:mockKeychainStorage
+                                            accessGroup:nil];
+
+  // 2. Create and expect keychain error.
+  NSError *gulsKeychainError = [NSError errorWithDomain:@"com.guls.keychain" code:-1 userInfo:nil];
+  OCMExpect([mockKeychainStorage getObjectForKey:[OCMArg any]
+                                     objectClass:[OCMArg any]
+                                     accessGroup:[OCMArg any]])
+      .andReturn([FBLPromise resolvedWith:gulsKeychainError]);
+
+  // 3. Get key ID and verify results.
+  __auto_type getPromise = [keyIDStorage getAppAttestKeyID];
   XCTAssert(FBLWaitForPromisesWithTimeout(0.5));
   XCTAssertNotNil(getPromise.error);
   XCTAssertEqualObjects(getPromise.error, [FIRAppCheckErrorUtil appAttestKeyIDNotFound]);
+
+  // 4. Verify storage mock.
+  OCMVerifyAll(mockKeychainStorage);
 }
 
 - (void)testSetGetAppAttestKeyIDPerApp {
@@ -105,9 +143,11 @@ - (void)assertIndependentSetGetForStoragesWithAppName1:(NSString *)appName1
                                                 appID2:(NSString *)appID2 {
   // Create two storages.
   FIRAppAttestKeyIDStorage *storage1 = [[FIRAppAttestKeyIDStorage alloc] initWithAppName:appName1
-                                                                                   appID:appID1];
+                                                                                   appID:appID1
+                                                                             accessGroup:nil];
   FIRAppAttestKeyIDStorage *storage2 = [[FIRAppAttestKeyIDStorage alloc] initWithAppName:appName2
-                                                                                   appID:appID2];
+                                                                                   appID:appID2
+                                                                             accessGroup:nil];
   // 1. Independently set app attest key IDs for the two storages.
   NSString *appAttestKeyID1 = @"app_attest_key_ID1";
   FBLPromise *setPromise1 = [storage1 setAppAttestKeyID:appAttestKeyID1];
@@ -143,3 +183,7 @@ - (void)assertIndependentSetGetForStoragesWithAppName1:(NSString *)appName1
 }
 
 @end
+
+#endif  // !TARGET_OS_MACCATALYST && !TARGET_OS_OSX
+
+#endif  // !SWIFT_PACKAGE