Skip to content

update keychain access level #4172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 25, 2019
Merged

update keychain access level #4172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
161de19
update keychain access level
charlotteliang Oct 25, 2019
c826a84
update comments
charlotteliang Oct 25, 2019
67d8331
fix test and style
charlotteliang Oct 25, 2019
afbd3d6
fix format
charlotteliang Oct 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
194 changes: 92 additions & 102 deletions Example/InstanceID/Tests/FIRInstanceIDAuthKeyChainTest.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ - (void)testKeyChainNoCorruptionWithUniqueAccount {
__weak FIRInstanceIDAuthKeychain *weakKeychain = keychain;
[keychain setData:tokenInfoData1
forService:service
accessibility:NULL
account:account1
handler:^(NSError *error) {
XCTAssertNil(error);
Expand All @@ -90,49 +89,47 @@ - (void)testKeyChainNoCorruptionWithUniqueAccount {
scope:kScope
token:kToken2];
[weakKeychain
setData:tokenInfoData2
forService:service
accessibility:NULL
account:account2
handler:^(NSError *error) {
XCTAssertNil(error);
// Now query the token and compare, they should not corrupt
// each other.
NSData *data1 = [weakKeychain dataForService:service account:account1];
setData:tokenInfoData2
forService:service
account:account2
handler:^(NSError *error) {
XCTAssertNil(error);
// Now query the token and compare, they should not corrupt
// each other.
NSData *data1 = [weakKeychain dataForService:service account:account1];
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
FIRInstanceIDTokenInfo *tokenInfo1 =
[NSKeyedUnarchiver unarchiveObjectWithData:data1];
XCTAssertEqualObjects(kToken1, tokenInfo1.token);
FIRInstanceIDTokenInfo *tokenInfo1 =
[NSKeyedUnarchiver unarchiveObjectWithData:data1];
XCTAssertEqualObjects(kToken1, tokenInfo1.token);

NSData *data2 = [weakKeychain dataForService:service account:account2];
FIRInstanceIDTokenInfo *tokenInfo2 =
[NSKeyedUnarchiver unarchiveObjectWithData:data2];
NSData *data2 = [weakKeychain dataForService:service account:account2];
FIRInstanceIDTokenInfo *tokenInfo2 =
[NSKeyedUnarchiver unarchiveObjectWithData:data2];
#pragma clang diagnostic pop
XCTAssertEqualObjects(kToken2, tokenInfo2.token);
// Also check the cache data.
XCTAssertEqual(weakKeychain.cachedKeychainData.count, 1);
XCTAssertEqual(weakKeychain.cachedKeychainData[service].count, 2);
XCTAssertEqualObjects(
weakKeychain.cachedKeychainData[service][account1].firstObject,
tokenInfoData1);
XCTAssertEqualObjects(
weakKeychain.cachedKeychainData[service][account2].firstObject,
tokenInfoData2);

// Check wildcard query
NSArray *results = [weakKeychain itemsMatchingService:service
account:@"*"];
XCTAssertEqual(results.count, 2);

// Clean up keychain at the end
[weakKeychain removeItemsMatchingService:service
account:@"*"
handler:^(NSError *_Nonnull error) {
XCTAssertNil(error);
[noCurruptionExpectation fulfill];
}];
}];
XCTAssertEqualObjects(kToken2, tokenInfo2.token);
// Also check the cache data.
XCTAssertEqual(weakKeychain.cachedKeychainData.count, 1);
XCTAssertEqual(weakKeychain.cachedKeychainData[service].count, 2);
XCTAssertEqualObjects(
weakKeychain.cachedKeychainData[service][account1].firstObject,
tokenInfoData1);
XCTAssertEqualObjects(
weakKeychain.cachedKeychainData[service][account2].firstObject,
tokenInfoData2);

// Check wildcard query
NSArray *results = [weakKeychain itemsMatchingService:service account:@"*"];
XCTAssertEqual(results.count, 2);

// Clean up keychain at the end
[weakKeychain removeItemsMatchingService:service
account:@"*"
handler:^(NSError *_Nonnull error) {
XCTAssertNil(error);
[noCurruptionExpectation fulfill];
}];
}];
}];
[self waitForExpectationsWithTimeout:1.0 handler:NULL];
#endif
Expand All @@ -151,67 +148,64 @@ - (void)testKeyChainNoCorruptionWithUniqueService {
FIRInstanceIDAuthKeychain *keychain =
[[FIRInstanceIDAuthKeychain alloc] initWithIdentifier:kFIRInstanceIDTestKeychainId];
__weak FIRInstanceIDAuthKeychain *weakKeychain = keychain;
[keychain setData:tokenData
forService:service1
accessibility:NULL
account:account
handler:^(NSError *error) {
XCTAssertNil(error);
// Store a checkin info using the same keychain account, but different service.
NSString *service2 = @"com.google.iid.checkin";
FIRInstanceIDCheckinPreferences *preferences =
[[FIRInstanceIDCheckinPreferences alloc] initWithDeviceID:kAuthID
secretToken:kSecret];
NSString *checkinKeychainContent = [preferences checkinKeychainContent];
NSData *checkinData = [checkinKeychainContent dataUsingEncoding:NSUTF8StringEncoding];

[weakKeychain
setData:checkinData
forService:service2
accessibility:NULL
account:account
handler:^(NSError *error) {
XCTAssertNil(error);
// Now query the token and compare, they should not corrupt
// each other.
NSData *data1 = [weakKeychain dataForService:service1 account:account];
[keychain
setData:tokenData
forService:service1
account:account
handler:^(NSError *error) {
XCTAssertNil(error);
// Store a checkin info using the same keychain account, but different service.
NSString *service2 = @"com.google.iid.checkin";
FIRInstanceIDCheckinPreferences *preferences =
[[FIRInstanceIDCheckinPreferences alloc] initWithDeviceID:kAuthID
secretToken:kSecret];
NSString *checkinKeychainContent = [preferences checkinKeychainContent];
NSData *checkinData = [checkinKeychainContent dataUsingEncoding:NSUTF8StringEncoding];

[weakKeychain
setData:checkinData
forService:service2
account:account
handler:^(NSError *error) {
XCTAssertNil(error);
// Now query the token and compare, they should not corrupt
// each other.
NSData *data1 = [weakKeychain dataForService:service1 account:account];
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
FIRInstanceIDTokenInfo *tokenInfo1 =
[NSKeyedUnarchiver unarchiveObjectWithData:data1];
FIRInstanceIDTokenInfo *tokenInfo1 =
[NSKeyedUnarchiver unarchiveObjectWithData:data1];
#pragma clang diagnostic pop
XCTAssertEqualObjects(kToken1, tokenInfo1.token);

NSData *data2 = [weakKeychain dataForService:service2 account:account];
NSString *checkinKeychainContent =
[[NSString alloc] initWithData:data2 encoding:NSUTF8StringEncoding];
FIRInstanceIDCheckinPreferences *checkinPreferences =
[FIRInstanceIDCheckinPreferences
preferencesFromKeychainContents:checkinKeychainContent];
XCTAssertEqualObjects(checkinPreferences.secretToken, kSecret);
XCTAssertEqualObjects(checkinPreferences.deviceID, kAuthID);

NSArray *results = [weakKeychain itemsMatchingService:@"*"
account:account];
XCTAssertEqual(results.count, 2);
// Also check the cache data.
XCTAssertEqual(weakKeychain.cachedKeychainData.count, 2);
XCTAssertEqualObjects(
weakKeychain.cachedKeychainData[service1][account].firstObject,
tokenData);
XCTAssertEqualObjects(
weakKeychain.cachedKeychainData[service2][account].firstObject,
checkinData);

// Clean up keychain at the end
[weakKeychain removeItemsMatchingService:@"*"
account:@"*"
handler:^(NSError *_Nonnull error) {
XCTAssertNil(error);
[noCurruptionExpectation fulfill];
}];
}];
}];
XCTAssertEqualObjects(kToken1, tokenInfo1.token);

NSData *data2 = [weakKeychain dataForService:service2 account:account];
NSString *checkinKeychainContent =
[[NSString alloc] initWithData:data2 encoding:NSUTF8StringEncoding];
FIRInstanceIDCheckinPreferences *checkinPreferences =
[FIRInstanceIDCheckinPreferences
preferencesFromKeychainContents:checkinKeychainContent];
XCTAssertEqualObjects(checkinPreferences.secretToken, kSecret);
XCTAssertEqualObjects(checkinPreferences.deviceID, kAuthID);

NSArray *results = [weakKeychain itemsMatchingService:@"*" account:account];
XCTAssertEqual(results.count, 2);
// Also check the cache data.
XCTAssertEqual(weakKeychain.cachedKeychainData.count, 2);
XCTAssertEqualObjects(
weakKeychain.cachedKeychainData[service1][account].firstObject, tokenData);
XCTAssertEqualObjects(
weakKeychain.cachedKeychainData[service2][account].firstObject,
checkinData);

// Clean up keychain at the end
[weakKeychain removeItemsMatchingService:@"*"
account:@"*"
handler:^(NSError *_Nonnull error) {
XCTAssertNil(error);
[noCurruptionExpectation fulfill];
}];
}];
}];
[self waitForExpectationsWithTimeout:1.0 handler:NULL];
#endif
}
Expand All @@ -237,7 +231,6 @@ - (void)testQueryCachedKeychainItems {
__weak id weakKeychainMock = keychainMock;
[keychain setData:tokenData
forService:service
accessibility:NULL
account:account
handler:^(NSError *error) {
XCTAssertNil(error);
Expand Down Expand Up @@ -299,7 +292,6 @@ - (void)testCachedKeychainOverwrite {
__weak FIRInstanceIDAuthKeychain *weakKeychain = keychain;
[keychain setData:tokenData
forService:service
accessibility:NULL
account:account
handler:^(NSError *error) {
XCTAssertNil(error);
Expand Down Expand Up @@ -344,7 +336,6 @@ - (void)testSetKeychainItemShouldDeleteOldEntry {
__weak FIRInstanceIDAuthKeychain *weakKeychain = keychain;
[keychain setData:tokenData
forService:service
accessibility:NULL
account:account
handler:^(NSError *error) {
XCTAssertNil(error);
Expand Down Expand Up @@ -374,7 +365,6 @@ - (void)testInvalidQuery {
NSData *data = [[NSData alloc] init];
[keychain setData:data
forService:@"*"
accessibility:NULL
account:@"*"
handler:^(NSError *error) {
XCTAssertNotNil(error);
Expand Down
1 change: 0 additions & 1 deletion Example/InstanceID/Tests/FIRInstanceIDCheckinStoreTest.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,6 @@ - (void)testCheckinMigrationMovesToNewLocationInKeychain {
NSData *data = [checkinKeychainContent dataUsingEncoding:NSUTF8StringEncoding];
[fakeKeychain setData:data
forService:kFIRInstanceIDLegacyCheckinKeychainService
accessibility:nil
account:kFIRInstanceIDLegacyCheckinKeychainAccount
handler:^(NSError *error) {
XCTAssertNil(error);
Expand Down
7 changes: 3 additions & 4 deletions Example/InstanceID/Tests/FIRInstanceIDFakeKeychain.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,9 @@ - (void)removeItemsMatchingService:(NSString *)service
}

- (void)setData:(NSData *)data
forService:(NSString *)service
accessibility:(nullable CFTypeRef)accessibility
account:(NSString *)account
handler:(void (^)(NSError *error))handler {
forService:(NSString *)service
account:(NSString *)account
handler:(void (^)(NSError *error))handler {
if (self.cannotWriteToKeychain) {
if (handler) {
handler([NSError errorWithDomain:kFakeKeychainErrorDomain code:1001 userInfo:nil]);
Expand Down
13 changes: 5 additions & 8 deletions Firebase/InstanceID/FIRInstanceIDAuthKeyChain.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,25 +73,22 @@ NS_ASSUME_NONNULL_BEGIN
handler:(nullable void (^)(NSError *error))handler;

/**
* Set the data for a given service and account with a specific accessibility. If
* accessibility is NULL we use `kSecAttrAccessibleAlwaysThisDeviceOnly` which
* Set the data for a given service and account.
* We use `kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly` which
* prevents backup and restore to iCloud, and works for app extension that can
* execute right after a device is restarted (and not unlocked).
*
* @param data The data to save.
* @param service The `kSecAttrService` used to save the password.
* @param accessibility The `kSecAttrAccessibility` used to save the password. If NULL
* set this to `kSecAttrAccessibleAlwaysThisDeviceOnly`.
* @param account The `kSecAttrAccount` used to save the password.
* @param handler The callback handler which is invoked when the add operation is complete,
* with an error if there is any.
*
*/
- (void)setData:(NSData *)data
forService:(NSString *)service
accessibility:(nullable CFTypeRef)accessibility
account:(NSString *)account
handler:(nullable void (^)(NSError *))handler;
forService:(NSString *)service
account:(NSString *)account
handler:(nullable void (^)(NSError *))handler;

@end

Expand Down
17 changes: 5 additions & 12 deletions Firebase/InstanceID/FIRInstanceIDAuthKeyChain.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,9 @@ - (void)removeItemsMatchingService:(NSString *)service
}

- (void)setData:(NSData *)data
forService:(NSString *)service
accessibility:(CFTypeRef)accessibility
account:(NSString *)account
handler:(void (^)(NSError *))handler {
forService:(NSString *)service
account:(NSString *)account
handler:(void (^)(NSError *))handler {
if ([service isEqualToString:kFIRInstanceIDKeychainWildcardIdentifier] ||
[account isEqualToString:kFIRInstanceIDKeychainWildcardIdentifier]) {
if (handler) {
Expand All @@ -194,14 +193,8 @@ - (void)setData:(NSData *)data
[self keychainQueryForService:service account:account];
keychainQuery[(__bridge id)kSecValueData] = data;

if (accessibility != NULL) {
keychainQuery[(__bridge id)kSecAttrAccessible] =
(__bridge id)accessibility;
} else {
// Defaults to No backup
keychainQuery[(__bridge id)kSecAttrAccessible] =
(__bridge id)kSecAttrAccessibleAlwaysThisDeviceOnly;
}
keychainQuery[(__bridge id)kSecAttrAccessible] =
(__bridge id)kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly;
[[FIRInstanceIDKeychain sharedInstance]
addItemWithQuery:keychainQuery
handler:handler];
Expand Down
2 changes: 0 additions & 2 deletions Firebase/InstanceID/FIRInstanceIDCheckinStore.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ - (void)saveCheckinPreferences:(FIRInstanceIDCheckinPreferences *)preferences
NSData *data = [checkinKeychainContent dataUsingEncoding:NSUTF8StringEncoding];
[self.keychain setData:data
forService:kFIRInstanceIDCheckinKeychainService
accessibility:nil
account:self.bundleIdentifierForKeychainAccount
handler:^(NSError *error) {
if (error) {
Expand Down Expand Up @@ -225,7 +224,6 @@ - (void)migrateCheckinItemIfNeeded {
// Save to new location
[self.keychain setData:dataInOldLocation
forService:kFIRInstanceIDCheckinKeychainService
accessibility:NULL
account:self.bundleIdentifierForKeychainAccount
handler:nil];
// Remove from old location
Expand Down
2 changes: 1 addition & 1 deletion Firebase/InstanceID/FIRInstanceIDKeyPairStore.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ - (void)updateKeyRef:(SecKeyRef)keyRef
(__bridge id)kSecAttrApplicationTag : updatedTagData,
(__bridge id)kSecClass : (__bridge id)kSecClassKey,
(__bridge id)kSecValueRef : (__bridge id)keyRef,
(__bridge id)kSecAttrAccessible : (__bridge id)kSecAttrAccessibleAlwaysThisDeviceOnly,
(__bridge id)kSecAttrAccessible : (__bridge id)kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly,
};
[[FIRInstanceIDKeychain sharedInstance] addItemWithQuery:addQuery
handler:^(NSError *addError) {
Expand Down
Loading