Uncovering Inconsistent Keychain Behavior

A few months ago, a developer sent the following error stack trace to Square from their iOS app:

2017-08-07 21:41:37.458 [**App name redacted**][5316:2314238] *** 
Terminating app due to uncaught exception 'NSInvalidArgumentException', 
reason: '-[__NSCFData isEqualToString:]: unrecognized selector sent to instance 0x6080000b97b0'

*** First throw call stack:
(
0   CoreFoundation                      0x0000000119ce4d4b __exceptionPreprocess + 171
1   libobjc.A.dylib                     0x000000011a7c121e objc_exception_throw + 48
2   CoreFoundation                      0x0000000119d54f04 -[NSObject(NSObject) doesNotRecognizeSelector:] + 132
3   CoreFoundation                      0x0000000119c6a005 ___forwarding___ + 1013
4   CoreFoundation                      0x0000000119c69b88 _CF_forwarding_prep_0 + 120
5   Valet                               0x000000012ad30860 -[VALValet migrateObjectsMatchingQuery:removeOnCompletion:] + 2759
… < truncated > … 
)

This led to a patch of Valet, Square’s popular open source library for managing iOS and macOS keychains. Along the way, we also uncovered a subtle but interesting inconsistency in keychain behavior on iOS and macOS.

This post will cover the following:

• A detailed look at the error;

• An overview of the iOS keychain and Valet;

• Debugging and patching Valet; and

• How we stumbled upon the differing keychain behavior in iOS and macOS, and the factors that contribute to it.

Error Details

NSInvalidArgumentException’, reason: ‘-[__NSCFData isEqualToString:]: unrecognized selector sent to instance 0x6080000b97b0

The error message above indicates that isEqualToString (an NSString method) is being called on an __NSCFData instance (a private subclass of NSData that does not implement isEqualToString).

Looking further down the stack trace, the first call that is not part of Apple’s CoreFoundation framework is:

-[VALValet migrateObjectsMatchingQuery:removeOnCompletion:]

VALValet is a class in Valet, and the migrateObjectsMatchingQuery method calls isEqualToString once here:

// Sanity check that we are capable of migrating the data.
NSMutableSet *const keysToMigrate = [NSMutableSet new];

for (NSDictionary *const keychainEntry in queryResultWithData) {
    NSString *const key = keychainEntry[(__bridge id)kSecAttrAccount];
    
    if ([key isEqualToString:VALCanAccessKeychainCanaryKey]) /** ERROR OCCURS HERE **/ {
    
        // We don't care about this key. Move along.
        continue;
    }
    ...

Immediately before the code calls** isEqualToString** and the crash occurs, the kSecAttrAccount value retrieved from the keychain entry is stored as an NSString:

NSString *const key = keychainEntry[(__bridge id)kSecAttrAccount];

The exception, however, indicates that key is actually an NSData object.

Why does Valet assume that the kSecAttrAccount value is a String?

Before answering this, having answers to these prerequisite questions is helpful:

• What is the iOS keychain?

• What is Valet?

• What does Valet’s migrateObjectsMatchingQuery method do?

What is the iOS Keychain?

If you’re already familiar with the iOS keychain, feel free to skip this section.

One way of persisting data between app launches is via UserDefaults, which is a key-value store with a simple API:

let myUsername = "foo"

// Store value in UserDefaults
UserDefaults.standard.set("bar", forKey: myKey)

// Retrieve data
UserDefaults.standard.string(forKey: myKey)

The iOS keychain is another persistence option, allowing apps to store sensitive data, such as passwords, in an encrypted SQLite database file on the device. Keychain entries can be retrieved and stored through the use of predefined attribute fields (e.g., kSecAttrAccount, kSecClass, kSecAttrService).

What’s the difference between the iOS keychain and UserDefaults?

The iOS keychain is secure storage (and has a hard-to-use API), while UserDefaults is entirely insecure (but has an easy-to-use API).

UserDefaults stores data in **plain text **in a .plist (property list) file within the application’s sandbox. Utilities like iBrowse and iExplorer can easily inspect files on a device, making UserDefaults only suited for storing non-sensitive data like user preferences and settings.

The following illustrates the minimum amount of code necessary to create and fetch a username/password entry in the the iOS keychain:

/*
 Adding an entry to the keychain when a matching entry already exists will throw an error.
 
 To properly add an entry to the keychain:
 
 1. Check for any matching keychain entries.
 2. If a matching entry exists, update.
 3. If no matching entries are found, insert a new entry.
 */

/** Retrieve password from keychain **/
let username = "my-username"
let newPassword = "my-new-password"
let appIdentifier = Bundle.main.bundleIdentifier

// The kSecClassGenericPassword keychain type uses (kSecAttrAccount,kSecAttrService) 
// as a composite primary key.

let fetchQuery: [String: Any] = [
    kSecClass as String: kSecClassGenericPassword,
    kSecAttrAccount as String: username,
    kSecAttrService as String: appIdentifier as Any,
    // Necessary to return any values associated with keychain attributes
    kSecMatchLimit as String: kSecMatchLimitOne,
    kSecReturnAttributes as String: true,
    kSecReturnData as String: true,
]

var item: CFTypeRef?
let fetchStatus = SecItemCopyMatching(fetchQuery as CFDictionary, &item) // Fetch keychain entry

// Keychain kSecValueData values must be Data objects
guard let encodedNewPassword = newPassword.data(using: .utf8) else { 
    fatalError("Unable to encode password") 
}

switch fetchStatus {
case errSecSuccess:
    // Read fetched keychain entry data
    if let item = item as? [String: Any] {
        guard let fetchedEncodedPasswordData = item[kSecValueData as String] as? Data,
            let existingPassword = String(data: fetchedEncodedPasswordData, encoding: .utf8) else {
                fatalError("Unable to read data.")
        }
        puts("Existing password: \(existingPassword)")
    }
    
    // Update existing keychain entry with new password
    let updateQuery: [String: Any] = [
        kSecClass as String: kSecClassGenericPassword,
        kSecAttrService as String: appIdentifier as Any,
        kSecAttrAccount as String: username,
    ]
    
    let attributesToUpdate: [String: Any] = [
        kSecValueData as String: encodedNewPassword
    ]
    
    let updateStatus = 
        SecItemUpdate(updateQuery as CFDictionary, attributesToUpdate as CFDictionary)
    
    guard updateStatus == errSecSuccess else { fatalError("Update error") }
    
case errSecItemNotFound:
    let insertQuery: [String: Any] = [
        kSecClass as String: kSecClassGenericPassword,
        kSecAttrService as String: appIdentifier as Any,
        kSecAttrAccount as String: username,
        kSecValueData as String: encodedNewPassword
    ]
    
    // Create new keychain entry
    let addStatus = SecItemAdd(insertQuery as CFDictionary, nil)
    
    guard addStatus == errSecSuccess else { fatalError("Insert error") }
    
default:
    fatalError("Fetch Error")
}

Not only is keychain usage verbose, but every function that interacts with the keychain (e.g., SecItem*) returns a bevy of error codes (see OSStatus in SecBase.h) that need to be checked and handled.

What is Valet?

Dealing with the keychain is complex, prone to error, and usually includes a large amount of boilerplate code. Unfortunately, it’s also unavoidable for any app that needs to store account information or sensitive data.

To make keychain management easier, Valet was developed by Square as an open source library for both iOS and macOS that hides the complexities of the keychain under a simple key-value interface.

Storing and retrieving a username/password in the keychain via Valet is similar to UserDefaults:

let myUsername = "my-username"; let myPassword = "my-password"

let valet = VALValet(
    identifier: Bundle.main.bundleIdentifier!, 
    accessibility: .whenUnlocked
)

// Store username/password
valet?.setString(myPassword, forKey: myUsername)

// Retrieve password by username
valet?.string(forKey: myUsername)

Now that we have more context, let’s return to our investigation of the error received by the developer.

What does Valet’s migrateObjectsMatchingQuery Method do?

As mentioned previously, the exception:

NSInvalidArgumentException [__NSCFData isEqualToString:]: unrecognized selector

occurs in VALValet’s migrateObjectsMatchingQuery method. What does this method do?

Typical usage of the **migrateObjectsMatchingQuery **method looks like this:

let valet = VALValet(
    identifier: Bundle.main.bundleIdentifier!, 
    accessibility: .whenUnlocked
)

let query: [String: Any] = [
    kSecAttrService as String: Bundle.main.bundleIdentifier as Any,
    kSecClass as String: kSecClassGenericPassword as Any
]

valet?.migrateObjects(
    matchingQuery: query, 
    removeOnCompletion: true
)

migrateObjectsMatchingQuery:removeOnCompletion: allows an application to shift management of existing keychain entries to Valet. If the application was utilizing the keychain to persist data prior to using Valet, this method restructures existing keychain entries to a format that allows these entries to be updated or fetched via Valet’s simpler API.

let query: [String: Any] = [
    **kSecAttrService** as String: Bundle.main.bundleIdentifier as Any,
    **kSecClass** as String: kSecClassGenericPassword as Any
]

The query parameter is used to select the keychain entries to be migrated to Valet, and uses the standard keychain attribute field names as filter criteria.

Migration allows apps to transition to Valet without forcing users to re-enter account passwords or other information stored in the keychain.

Taking a Closer Look at Where the Error Occurs

Revisiting the code block in migrateObjectsMatchingQuery where the exception is thrown:

...
// Sanity check that we are capable of migrating the data.
NSMutableSet *const keysToMigrate = [NSMutableSet new];

for (NSDictionary *const keychainEntry in queryResultWithData) {
    NSString *const key = keychainEntry[(__bridge id)kSecAttrAccount];
    if ([key isEqualToString:VALCanAccessKeychainCanaryKey]) /** ERROR OCCURS HERE **/ {
        // We don't care about this key. Move along.
        continue;
    }
    NSData *const data = keychainEntry[(__bridge id)kSecValueData];
    
    // Check keychain entry to ensure it meets the required criteria
    ...
}

// Perform migration of all matching keychain entries to Valet
...

Prior to migration, this section of code is responsible for sanity checking the existing keychain entries that are candidates for migration. For example, to be eligible, existing entries must have kSecAttrAccount and kSecValueData values.

A line-by-line explanation:

for (NSDictionary *const keychainEntry in **queryResultWithData**)

queryResultWithData is an Array containing data for the entries returned after searching the keychain using the query supplied to the migrateObjectsMatchingQuery method.

**NSString** *const **key** = keychainEntry[(__bridge id)kSecAttrAccount];

Each entry’s kSecAttrAccount value is stored in the key variable. Note the assumption here that the value is a String. The compiler allows this due to the fact that NSDictionary values are id pointers, which allow implicit casts.

if ([**key isEqualToString:VALCanAccessKeychainCanaryKey]**) { 
    // We don't care about this key. Move along.
    continue;
}

Before each entry is sanity checked, key is compared against VALCanAccessKeychainCanaryKey and all checks are skipped if a match is found.

VALCanAccessKeychainCanaryKey is a string constant stored in the kSecAttrAccount field of an entry by Valet when performing exploratory (canary) keychain entry inserts. These inserts verify that the proper keychain accessibility was specified when initializing a Valet instance.

If the query results contain a canary entry, we want to skip any sanity checks for that entry. Canary entries are a special case that should not cause any migration sanity checks to fail.

Everything works fine when key (the value in an entry’s kSecAttrAccount field) is an NSString. The exception, however, proves that it’s possible for key to be an** NSData** object.

The problem is the assumption Valet makes that an entry’s kSecAttrAccount value will be an **NSString **object:

**NSString** *const **key** = keychainEntry[(__bridge id)kSecAttrAccount];

Why does Valet assume that the kSecAttrAccount value is an NSString?

Apple’s documentation for the keychain value associated with the SecAttrAccount attribute provides the following description:

A key whose value is a string indicating the item’s [keychain entry’s] account name.

From this, it seems reasonable to assume that kSecAttrAccount values will always be strings.

Also, from this example of Valet usage:

let myUsername = "my-username"; let myPassword = "my-password"

let valet = VALValet(
    identifier: Bundle.main.bundleIdentifier!, 
    accessibility: .whenUnlocked
)

// Store username/password
valet?.setString(myPassword, forKey: myUsername)

// Retrieve password by username
valet?.string(forKey: myUsername)

Valet only accepts String arguments for keys (myUsername in the example above), which are then stored as kSecAttrAccount values in the keychain.

Any entries created by Valet abide by Apple’s type restriction on kSecAttrAccount values. However, for existing keychain entries, Valet makes the flawed assumption that either:

• All developers comply with these keychain type restrictions; or

• The framework or operating system enforces type restrictions on keychain data.

Reproducing the Error

The exception received by the developer, however, indicates that it is possible to store an NSData object in a keychain entry’s **kSecAttrAccount **field, which contradicts Apple’s documentation.

The following code snippet proves that this is indeed the case, and the same exception is thrown when attempting to migrate keychain entries.

/** Create a Data object **/
let dataBlob = "foo".data(using: .utf8)

let existingEntryData: [String: Any] = [
    kSecAttrService as String!: Bundle.main.bundleIdentifier as Any,
    // Use Data object for kSecAttrAccount, which expects a String
    kSecAttrAccount as String!: dataBlob as Any, 
    kSecClass as String!: kSecClassGenericPassword as Any,
    kSecValueData as String!: dataBlob as Any
]

// Insert the non-conforming data into the keychain
let status = SecItemAdd(existingEntryData as CFDictionary, nil)
print(String(describing: status)) // prints “0” indicating a successful insert

let valet = VALValet(
    identifier: Bundle.main.bundleIdentifier!, accessibility: .whenUnlocked
)

let query: [String: Any] = [
    kSecAttrService as String!: Bundle.main.bundleIdentifier as Any,
    kSecClass as String!: kSecClassGenericPassword as Any
]

/**
  Attempt to migrate the existing non-conforming entry so Valet can manage it.
  
  #### THIS THROWS THE EXPECTED EXCEPTION!!! ####
  */
valet?.migrateObjects(matchingQuery: query, removeOnCompletion: true)

Fix: Adding a Type Check

Since it can no longer be assumed that kSecAttrAccount is always a string, we need to add a type check prior to the string comparison to avoid sending the isEqualToString message to a non-NSString object.

if (**[key isKindOfClass:[NSString class]] && 
    **[key isEqualToString:VALCanAccessKeychainCanaryKey]) {

    // We don’t care about this key. Move along.
    continue;
}

Since Valet always uses NSString values in the kSecAttrAccount field for entries it creates, a Valet canary entry’s key will always be an NSString. Therefore, adding this type check is a valid requirement.

An Unintentional Find: Inconsistent behavior in macOS and iOS Keychains

Next, we added the test below to verify our change (Valet is written in Objective C):

- (void)test_migrateObjectsMatchingQueryRemoveOnCompletion_withNSDataAsAccount_doesNotRaiseException;
{
    NSString *identifier = @"my_identifier";
    NSData *dataBlob = [@"foo" dataUsingEncoding:NSUTF8StringEncoding];

    // kSecAttrAccount entry is expected to be a CFString, but a CFDataRef can also be stored as a value.
    NSDictionary *keychainData = @{ 
        (__bridge id)kSecAttrService : identifier, 
        (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword, 
        (__bridge id)kSecAttrAccount : dataBlob, // NSData object instead of NSString
        (__bridge id)kSecValueData : dataBlob
    };

    OSStatus status = SecItemAdd((__bridge CFDictionaryRef)keychainData, NULL);
    XCTAssertEqual(status, errSecSuccess); // Insert Succeeded

    NSDictionary *query = @{
        (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword, 
        (__bridge id)kSecAttrService : identifier 
    };

    NSError *error = [self.valet migrateObjectsMatchingQuery:query removeOnCompletion:NO];
    XCTAssertNil(error); // Passes for iOS, fails for macOS
}

NSString *identifier = @"my_identifier";
NSData ***dataBlob** = [@"foo" dataUsingEncoding:NSUTF8StringEncoding];

NSDictionary ***keychainData** = @{ 
    (__bridge id)kSecAttrService : identifier, 
    (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword, 
**    (__bridge id)kSecAttrAccount : dataBlob,
**    (__bridge id)kSecValueData : dataBlob
};

The test deliberately stores dataBlob, which is an NSData object in the kSecAttrAccount field of keychainData, which is inserted in to the keychain. As expected, the fix prevents an exception from being thrown.

Interestingly, the last assertion passes on an iOS target but fails in a macOS **environment **(Valet can manage iOS and macOS keychains):

NSError *error = 
    [self.valet migrateObjectsMatchingQuery:query removeOnCompletion:NO];
    
**XCTAssertNil(error); // Passes for iOS, fails for macOS**

Running the test on a macOS target resulted in a VALMigrationErrorKeyInQueryResultInvalid error, indicating that the keychain entry’s kSecAttrAccount value was nil. Recall that any keys that are candidates for migration must have a value in the kSecAttrAccount field.

Inserting an entry containing a kSecAttrAccount Data object on macOS results in the kSecAttrAccount value not being persisted if it is not the correct type. iOS, on the other hand, persists the kSecAttrAccount value regardless of whether it is the correct type or not.

Inspecting the keychain entry after inserting keychainData in both operating systems confirms that the kSecAttrAccount data value is stored successfully on iOS but not on macOS:

**macOS:**

(lldb) po keychainEntry
{

    /** MISSING ACCT VALUE !!! **/
    cdat = "2017-08-12 11:37:39 +0000";
    class = genp; // kSecClass
    labl = "Keychain_With_Account_Name_As_NSData";
    mdat = "2017-08-12 11:37:39 +0000";
    svce = "my_identifier"; // kSecAttrService
    "v_Data" = <666f6f>; // kSecValueData
    "v_PersistentRef" = <...>;
}

iOS:

(lldb) po keychainEntry
{
    /** PERSISTED kSecAttrAccount VALUE !!! **/
    acct = <666f6f>;
    agrp = "<*>.com.squareup.Valet-iOS-Test-Host-App";
    cdat = "2017-08-12 11:35:45 +0000";
    mdat = "2017-08-12 11:35:45 +0000";
    musr = <>;
    pdmn = ak;
    svce = "my_identifier"; // kSecAttrService
    sync = 0;
    tomb = 0;
    "v_Data" = <666f6f>; // kSecValueData
    "v_PersistentRef" = <67656e70 00000000 00000325>;
}

Why does iOS persist the kSecAttrAccount value while macOS does not?

A developer at Apple posted this reply to a thread inquiring about this very issue:

There are two implementations of the SecItem API: the iOS implementation, which is also used for iCloud Keychain on OS X the OS X implementation, which is a compatibility shim that bridges over to the traditional keychain Note Both are available in Darwin. Search the Security project for SecItemUpdate_ios and SecItemUpdate_osx to see how this works under the covers. The iOS implementation is based on SQLite. If you’re familiar with SQLite you’ll know that it is, at its core, untyped, and thus the iOS implementation has to do its own type conversion. This is why that implementation is somewhat forgiving on the types front. However… IMPORTANT I strongly recommend that you use the types specified in the header. Other types do work but that’s an accident of the implementation rather than a designed in feature. Moreover, as there are two implementations, it’s not always the case that these accidents line up. I realise that this rule is broken by various bits of Apple sample code.

In short, these two factors contribute to this difference in behavior:

• iOS and macOS do not use the same data store for their keychains.

• The keychain implementation is different for both operating systems.

iOS and macOS Use Different Keychain Data Stores

SQLite, which is the data store backing the iOS keychain, uses **type affinity **rather than rigid typing, meaning a column’s datatype in SQLite is more a recommendation rather than a requirement.

From the SQLite documentation:

Any column can still store any type of data. It is just that some columns, given the choice, will prefer to use one storage class over another…A column with TEXT affinity stores all data using storage classes NULL, TEXT or BLOB.

This explains the observed behavior in iOS, which allowed an NSData object to be stored in the kSecAttrAccount column, even though the recommended data type is a string.

Differing Keychain Implementation on iOS and macOS

iOS and macOS both use the SecItemUpdate function to update keychain entries. Its implementation can be found in SecItem.cpp, which is open sourced as part of Apple’s Security framework:

OSStatus
SecItemUpdate(CFDictionaryRef query, CFDictionaryRef attributesToUpdate)
{
    ...
  OSStatus status_osx = errSecItemNotFound, status_ios = errSecItemNotFound;
  bool can_target_ios, can_target_osx;
  OSStatus status = SecItemCategorizeQuery(query, can_target_ios, can_target_osx);
    ...

  if (can_target_ios) {
          ...
          status_ios = SecItemUpdate_ios(attrs_ios, attributesToUpdate);
        ...
  }

  if (can_target_osx) {
         ...
         if (SecItemHasSynchronizableUpdate(false, attributesToUpdate))
            status_osx = SecItemChangeSynchronizability(attrs_osx, attributesToUpdate, true);
         else
            status_osx = SecItemUpdate_osx(attrs_osx, attributesToUpdate);
         ...
  }
    ...
}

It’s apparent from the can_target_(ios|osx) boolean values and SecItemUpdate_(ios|osx) functions that the codepaths are different for each operating system.

For interested readers, SecItemUpdate_ios is aliased via a preprocessor macro to the SecItemUpdate function in SecItem.c. SecItemUpdate_osx is defined in SecItem.cpp. The entire Security framework can be downloaded here.

Summary of Keychain Findings

To summarize what we’ve learned about keychains as a result of this investigation:

• Both iOS and macOS do not return an error if a Data object is stored in a keychain entry’s kSecAttrAccount field.

• iOS persists the kSecAttrAccount value, regardless of type, while macOS does not.

• It’s clear that macOS keychains are persisted in a type safe database, or type checks have been added to the macOS codepath that do not exist on iOS.

Fixing the Test

Returning back to the failing Valet test, in order to account for the differing keychain behavior across platforms, the following:

**XCTAssertNil(error); // Passes for iOS, fails for macOS**

needs to be replaced with:

# if **TARGET_OS_IPHONE**
    XCTAssertNil(error);

# elif TARGET_OS_MAC

/**
  iOS allows kSecAttrAccount NSData entries, 
  while OSX sets the value to nil for any non-string entry.
  */

    XCTAssertEqual(
        error.code, 
        VALMigrationErrorKeyInQueryResultInvalid
    );

# else
    [NSException raise:@"UnsupportedOperatingSystem" format:@"Only OSX and iOS are supported"];

# endif

With this change the test now passes, and a link to the final Valet fix can be found here on Github.

Communicating Findings to the Developer

After completing our investigation, we relayed to the developer that the crash was due to an NSData object being stored against a keychain entry’s kSecAttrAccount attribute.

They responded that their app had always stored kSecAttrAccount string values, but were using a third-party library that was also storing credentials containing kSecAttrAccount values in the keychain. It was highly probable that this library was inserting the offending keychain entry. The library, however, was critical to the app’s functionality and could not be removed.

It’s common to use the app’s main bundle identifier as the kSecAttrService attribute value, which acts as a namespace within the keychain. The developer’s code and the library were most likely storing entries under the same namespace.

The code used in the developer’s app to migrate their keychain entries to Valet:

let valet = VALValet(
    identifier: Bundle.main.bundleIdentifier!, 
    accessibility: .whenUnlocked
)

let query: [String: Any] = [
    kSecAttrService as String!: Bundle.main.bundleIdentifier as Any,   
    kSecClass as String!: kSecClassGenericPassword as Any
]

valet?.migrateObjects(
    matchingQuery: query, 
    removeOnCompletion: true
)

The query above returns all generic passwords stored under the app’s bundle identifier, and in this case, it included an entry stored by a third party library with a kSecAttrService Data object.

Updating the developer’s app to a version of Valet containing the fix resolved the issue for them.

I’d like to thank Eric Muller and Dan Federman, the creators of Valet, for helping to debug and fix this issue.