Skip to content
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.

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

Would there be any benefit in providing a "resetKeychain" API (or similar)? #15

Open
erikackermann opened this issue Feb 26, 2014 · 5 comments

Comments

@erikackermann
Copy link

It would be convenient to be able to do:

[keychain reset];

instead of having to manually delete each key for a given keychain service.
(This probably wouldn't make sense to allow on the default keychain).

@fcy
Copy link
Contributor

fcy commented Apr 29, 2014

This is useful specially for acceptance tests.

@n00neimp0rtant
Copy link

I agree that this would be very useful, but I worry that it would introduce a security vulnerability. FXKeychain does not allow you to retrieve an arbitrary list of all keys within the keychain; you have to request a specific key (I assume this is due to an underlying, intentional limitation by Apple). If this were possible, any third-party libraries linked with the binary could silently poll the keychain for all keys and phone the values home, which would be a huge security issue.

At first it seems like the trade off probably isn't worth it, but when you consider that the keychain isn't wiped even if you delete an app (yes, even on iOS), it starts to feel like this is something that should be addressed.

@nicklockwood
Copy link
Owner

Security isn't an issue on iOS since each has its own sandboxed keychain. On Mac it would be, but ultimately that's up to the developer and Apple to worry about.

My bigger concern is the first point - how to actually retrieve a list of all keys in the chain so I can wipe them?

@n00neimp0rtant
Copy link

Bottom line, it would not be immediately backward-compatible with existing keys, but you could maintain a set of all keys within the keychain itself whenever a value is accessed/mutated. For example, when running [FXKeychain defaultKeychain][@"UserPassword"] = @"password123", it would not only store the value @"password123" in the keychain for the key @"UserPassword", but it would also store the value @"UserPassword" within an NSSet in the keychain that has the key @"__FXKEYCHAIN_INTERNAL_KNOWNKEYS" (or something like that).

Before any read/write from/to the keychain, you would check NSUserDefaults for the Boolean key @"FXKeychainLaunchedBefore"; if it is NO (or doesn't exist), iterate through every key stored in @"__FXKEYCHAIN_INTERNAL_KNOWNKEYS", remove the values for those keys if they exist, remove all of the known keys from the set (this functionality could be added as the [keychain reset] method proposed by OP), then set @"FXKeychainLaunchedBefore" to YES. The keychain persists across app installs indefinitely, but NSUserDefaults do not; when you uninstall, @"FXKeychainLaunchedBefore" gets removed, so on the next launch, when you get a "NO" for that key, it cleans out the keychain. On iOS, users and developers alike are familiar with the concept of "when you uninstall it, all of the data goes with it," but such isn't the case with the keychain. I add code do to this manually in every project I work on that uses FXKeychain (thanks for what you do btw =P), but it would be great if FXKeychain did it automatically.

For the sake of transparency, the security issue is that your entire binary, including any open source projects you may be using, will have access to all of the keys by looking at the value of that constant key. Because of that, you should block any attempts to read/write to that key through FXKeychain itself; not foolproof, but it helps.

If you like this idea but can't find the time to implement it, let me know and I'll see if I can put in a pull request.

@jshier
Copy link

jshier commented Nov 24, 2014

All of that seems unnecessary. The easiest way to reset the keychain for a particular app (on iOS at least) is simply to perform a general query of all keychain item types and then call SecItemDelete() on the result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants