jrendel / SwiftKeychainWrapper

A simple wrapper for the iOS Keychain to allow you to use it in a similar fashion to User Defaults. Written in Swift.
MIT License
1.59k stars 340 forks source link

Storage improvement for unchanged keys #142

Open JoaoPinho opened 5 years ago

JoaoPinho commented 5 years ago

Hi,

I faced an issue while using Realm SDK that can be found here Realm file decryption failed.

After some resource i understood that the problem was how i managed to store the Realm encryption key in keychain.

This was how i check if there is some encryption key in keychain and if none, i create a new one like this:

static private func encryptionKey() throws -> Data {

        if let storedEncryptionKey = KeychainWrapper.standard.data(forKey: .realmEncryptionKey) {
            return storedEncryptionKey
        }

        Log.debug?.message("Generating random Realm encryption key.")

        let newEncryptionKey = Data.withRandomBytes(count: 64)

        guard KeychainWrapper.standard.set(newEncryptionKey, forKey: .realmEncryptionKey, withAccessibility: KeychainItemAccessibility.whenUnlockedThisDeviceOnly) else {
            throw LocalStorageError.realmInstantationFailed
        }

        return newEncryptionKey
}

So, when i tried to retrieve the encryption key from keychain using KeychainWrapper, this tries to get value from keychain an if no error retrieves the Data value, but with some error, this gives me nil. So i assume that on nil value means that i don't have the encryption key and i have to create a new one (With this i can't decrypt anymore my Realm DB since i can't know the right encryption key anymore).

KeychainWrapper retrieve data value func:

open func data(forKey key: String, withAccessibility accessibility: KeychainItemAccessibility? = nil) -> Data? {
        var keychainQueryDictionary = setupKeychainQueryDictionary(forKey: key, withAccessibility: accessibility)

        // Limit search results to one
        keychainQueryDictionary[SecMatchLimit] = kSecMatchLimitOne

        // Specify we want Data/CFData returned
        keychainQueryDictionary[SecReturnData] = kCFBooleanTrue

        // Search
        var result: AnyObject?
        let status = SecItemCopyMatching(keychainQueryDictionary as CFDictionary, &result)

        return status == noErr ? result as? Data : nil
}

In my opinion, when retrieving values from Keychain using KeychainWrapper, you should give the chance to know if there is no value or other error as occurred.

I had developed my own keychain access for this case:

static func getKeychainData(forKey key: SharedConstants.UniqueKey) throws -> Data {

        guard let encodedIdentifier = key.rawValue.data(using: String.Encoding.utf8) else {
            throw LocalStorageError.couldntGetEncryptionKey
        }

        let keychainQueryDictionary: [String: Any] = [
            kSecClass as String: kSecClassGenericPassword,
            kSecAttrService as String: Bundle.main.bundleIdentifier ?? "SwiftKeychainWrapper",
            kSecAttrGeneric as String: encodedIdentifier,
            kSecAttrAccount as String: encodedIdentifier,
            kSecMatchLimit as String: kSecMatchLimitOne,
            kSecReturnData as String: kCFBooleanTrue
        ]

        var result: AnyObject?
        let status = SecItemCopyMatching(keychainQueryDictionary as CFDictionary, &result)

        switch status {
        case errSecSuccess:

            guard let result = result as? Data else {
                throw LocalStorageError.couldntGetEncryptionKey
            }

            return result

        case errSecItemNotFound:
            throw LocalStorageError.keychainItemNotFound

        default:
            throw LocalStorageError.couldntGetEncryptionKey
        }

}

This was my workaround to avoid replacing a value that i just want to store one time only, could be an idea for you too, since keychain gives us errSecItemNotFound.

I found that your set function is automatically an insert or update, without giving us the option to choose that:

@discardableResult open func set(_ value: Data, forKey key: String, withAccessibility accessibility: KeychainItemAccessibility? = nil) -> Bool {
        var keychainQueryDictionary: [String:Any] = setupKeychainQueryDictionary(forKey: key, withAccessibility: accessibility)

        keychainQueryDictionary[SecValueData] = value

        if let accessibility = accessibility {
            keychainQueryDictionary[SecAttrAccessible] = accessibility.keychainAttrValue
        } else {
            // Assign default protection - Protect the keychain entry so it's only valid when the device is unlocked
            keychainQueryDictionary[SecAttrAccessible] = KeychainItemAccessibility.whenUnlocked.keychainAttrValue
        }

        let status: OSStatus = SecItemAdd(keychainQueryDictionary as CFDictionary, nil)

        if status == errSecSuccess {
            return true
        } else if status == errSecDuplicateItem {
            return update(value, forKey: key, withAccessibility: accessibility)
        } else {
            return false
        }
}

So i had to develop my own set keychain in order to don't force update:

static func setKeychainData(with value: Data, forKey key: SharedConstants.UniqueKey) -> Bool {

        guard let encodedIdentifier = key.rawValue.data(using: String.Encoding.utf8) else {
            return false
        }

        let keychainQueryDictionary: [String: Any] = [
            kSecClass as String: kSecClassGenericPassword,
            kSecAttrService as String: Bundle.main.bundleIdentifier ?? "SwiftKeychainWrapper",
            kSecAttrGeneric as String: encodedIdentifier,
            kSecAttrAccount as String: encodedIdentifier,
            kSecValueData as String: value,
            kSecAttrAccessible as String: kSecAttrAccessibleWhenUnlockedThisDeviceOnly
        ]

        let status: OSStatus = SecItemAdd(keychainQueryDictionary as CFDictionary, nil)

        return status == errSecSuccess

}

Ideally this should have an option to force update, but in my case i just needed this. This is how i check if there is some encryption key in keychain and if not, how i create a new one:

static private func encryptionKey() throws -> Data {

        do {

            let storedEncryptionKey = try getKeychainData(forKey: .realmEncryptionKey)
            return storedEncryptionKey

        } catch LocalStorageError.keychainItemNotFound {

            Log.debug?.message("Generating random Realm encryption key.")

            let newEncryptionKey = Data.withRandomBytes(count: 64)

            guard setKeychainData(with: newEncryptionKey, forKey: .realmEncryptionKey) else {
                throw LocalStorageError.realmInstantationFailed
            }

            return newEncryptionKey
        }
}

It's hard to reproduce the error that i had, but i found a way, since my keychain accessibility was whenUnlockedThisDeviceOnly for security reasons.

And now, i replaced my encryption key on keychain and the user will not be able to open my Realm DB anymore. And in production, i notice that some users had this problem.

I think that giving us more flexibility to manage those kind of things will improve our experience.

Thank you.