evgenyneu / keychain-swift

Helper functions for saving text in Keychain securely for iOS, OS X, tvOS and watchOS.
MIT License
2.82k stars 345 forks source link

Delete removes items on all keychains #91

Open igorkulman opened 5 years ago

igorkulman commented 5 years ago

I have a "normal" keychain and I need data from it migrated to a shared keychain. The shared keychain is already set up and working properly, I just need to move some old data to it because of a new feature.

I use code like this to do the migration

keychain = KeychainSwift()
keychain.accessGroup = shareAppInfo.sharedKeyChainName

let oldKeyChain = KeychainSwift()

let keys = [..]

for key in keys {
    if let value = oldKeyChain.get(key) {
        keychain.set(value, forKey: key, withAccess: .accessibleAfterFirstUnlockThisDeviceOnly)
        oldKeyChain.delete(key)
    }
}

The problem is that keychain.set saves the value to the shared keychain but oldKeyChain.delete deletes it not only from the old keychain but also from the shared one.

This is visible adding some print statements

for key in keys {
    if let value = oldKeyChain.get(key) {
        keychain.set(value, forKey: key, withAccess: .accessibleAfterFirstUnlockThisDeviceOnly)
        print(keychain.get(key))
        oldKeyChain.delete(key)
        print(keychain.get(key))
    }
}

Results in

Optional("v20tZbtx2Rh69AsarZAORKJU+MLWky3wMxdXETzcoKXCYXIahJIEWjXbLdeB6378JTTdC3Zw90OR80uMzK65vO2WNqcTpBaA+Sglgaj7uLa+wUU/vI5ebOt5WQ1N22FsMlc12C/JYshoNSyyeXoZO0GduGFkNoOzSnlQtSxL0d4=")
nil
Optional("d0xVq9GyxTD9+sUO7UTRtLbpUwghoo0pGaW41dhh4CD0A3aAQHUDvKvjhui0lfWVamoTuCFMCMX9ImaD54O/nrsJcU4/XGd8xe24r9mvEiqdIanRfc5B5CqAqtiR5O9r")
nil
evgenyneu commented 5 years ago

This is very interesting, @igorkulman, thanks for reporting!

It looks like after the key is assigned to an access group it will then be treated as a shared key, even if you don't specify the access group. Consequently, it seems to be impossible to move a key to another app. We can only copy a key, but not move it.

Let me know if you find a workaround.

igorkulman commented 5 years ago

In my specific case, the workaround is quite simple

for key in keys {
    if let value = oldKeyChain.get(key) {
        oldKeyChain.delete(key)
        keychain.set(value, forKey: key, withAccess: .accessibleAfterFirstUnlockThisDeviceOnly)
    }
}

I have the value in a local variable so I can delete it first from the old keychain and then write it to the new one.