SecureDefaults icon indicating copy to clipboard operation
SecureDefaults copied to clipboard

Question: how should I use the `keychainAccessible` attribute?

Open cwhenderson20 opened this issue 2 years ago • 1 comments

Description

First of all, thanks for this package! Please forgive me if this question seems uninformed, as I'm somewhat new to iOS development.

I'm wondering how best to use the keychainAccessible attribute. I see that it can be set after initialization, which I'm glad for, since I'd like to set it to kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly rather than the default of kSecAttrAccessibleAlways. However, if I change this value after the initial instantiation, I'm met with various crashes such as this one if I set the accessibility before checking if a key is created:

image image

Or this one if I set the accessibility after checking:

image image

As far as I understand, a keychain item's accessibility cannot be modified in place, so I'm not terribly surprised this doesn't Just Work™, but is there a way to guard against these crashes? As some additional context, I'm wrapping SecureDefaults to be used in a React Native package, and I'm wondering if there is a way to bubble the [expected?] error to the user so that they can deal with it rather than having a hard requirement that the kSecAttrAccessible value never be changed for a given suite. Or maybe this will have to be code I'd write outside of SecureDefaults before it is instantiated?

Requirements

  • [x] I've read and agree to the Code of Conduct.
  • [x] I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • [x] I've searched for any related issues and avoided creating a duplicate issue.

Reproducible in:

SecureDefaults version: 1.1.0 iOS version: 16.1

Steps to reproduce:

  1. Instantiate a SecureDefaults shared instance using a suiteName and some value for keychainAccessible
  2. Set values, etc.
  3. Dynamically change the keychainAccessible value
  4. Attempt to access a value stored in SecureDefaults

Expected result:

Not exactly sure, honestly!

Actual result:

Crash!

cwhenderson20 avatar Feb 22 '23 17:02 cwhenderson20

Hi @cwhenderson20, actually, it's quite a tricky question. I'm not even sure if it's a good idea to set anything else other than kSecAttrAccessibleAlways to that property 🙂. I remember that it hasn't been possible to modify any key in Keychain since it was created. So to update a key, it must be removed first (e.g., here or here). The solution the is to clear old AES & AESIV keys every time you need to change the Keychain accessibility level. However, it means that all data will be lost when the accessibility level is changed. Does that sound good to you @cwhenderson20, I mean the data loss?

So, I think we can consider the following approach as a fix :

public var password: String? {
    didSet {
         let AESKey = suitename != nil ? "\(Keys.AESKey)-\(suitename!)" : Keys.AESKey
         let AESIVKey = suitename != nil ? "\(Keys.AESIV)-\(suitename!)" : Keys.AESIV
         let clearKeychain = { (AESIVKey: String, AESKey: String, accessible: String) in
             KeychainHelper.remove(forKey: AESIVKey, accessible: accessible)
             KeychainHelper.remove(forKey: AESKey, accessible: accessible)
         }
         clearKeychain(AESIVKey, AESKey, keychainAccessible)
         clearKeychain(AESIVKey, AESKey, kSecAttrAccessibleWhenUnlocked as String)
         clearKeychain(AESIVKey, AESKey, kSecAttrAccessibleAfterFirstUnlock as String)
         clearKeychain(AESIVKey, AESKey, kSecAttrAccessibleAlways as String)
         clearKeychain(AESIVKey, AESKey, kSecAttrAccessibleWhenPasscodeSetThisDeviceOnly as String)
         clearKeychain(AESIVKey, AESKey, kSecAttrAccessibleWhenUnlockedThisDeviceOnly as String)
         clearKeychain(AESIVKey, AESKey, kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly as String)
         clearKeychain(AESIVKey, AESKey, kSecAttrAccessibleAlwaysThisDeviceOnly as String)
    }
}

Both ways (before & after defaults.isKeyCreated) seem to work fine then:

let defaults = SecureDefaults()
if !defaults.isKeyCreated {
    defaults.password = UUID().uuidString
}
defaults.keychainAccessible = kSecAttrAccessibleAlwaysThisDeviceOnly as String

and

let defaults = SecureDefaults()
if !defaults.isKeyCreated {
    defaults.password = UUID().uuidString
}
defaults.keychainAccessible = kSecAttrAccessibleAlwaysThisDeviceOnly as String

I made a PR with those changes #16. Could you please test it and let me know if it works for you and answers your question?

vpeschenkov avatar Feb 23 '23 14:02 vpeschenkov