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

Added item class feature #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Oleygen
Copy link

@Oleygen Oleygen commented Jan 26, 2019

Adding possibility to set a class for items to set/get/delete/clear. Backwards compatibility reached by using default parameter (genericPassword).
Please check is this pull request is valuable for your library, and notify me if it needs any improvement or I've missed some caveats.
Please note: There is missing objective-c compatibility changes, and mb some other files should be modified (you can help if specify what may I missed)

@evgenyneu
Copy link
Owner

Thanks for the update @Oleygen. What motivated you to add this feature?

@Oleygen
Copy link
Author

Oleygen commented Jan 27, 2019

I'm using your lib in my app. I need to store RSA private key on a device, and I've noticed that all items are stored as password, which is not correct for my case

@evgenyneu
Copy link
Owner

Did you need to store your key as kSecClassCertificate?

@Oleygen
Copy link
Author

Oleygen commented Jan 27, 2019

Yes, exactly

@evgenyneu
Copy link
Owner

Is saving text as kSecClassCertificate different than saving it as kSecClassGenericPassword?

@Oleygen
Copy link
Author

Oleygen commented Jan 30, 2019

  1. It differes in a way it displays in a keychain
  2. Apple documentation notes that items stores on disk in a different way, based on its class (see https://developer.apple.com/documentation/security/keychain_services/keychain_items/item_class_keys_and_values)

@evgenyneu
Copy link
Owner

It differes in a way it displays in a keychain

In what "way", sorry? And why does it matter?

Apple documentation notes that items stores on disk in a different way, based on its class (see https://developer.apple.com/documentation/security/keychain_services/keychain_items/item_class_keys_and_values)

The documentation says

The item's class dictates which attributes apply and enables the system to decide whether or not the data should be encrypted on disk. For example, passwords require encryption, but certificates don't because they are not secret.

So I guess, the text stored as kSecClassCertificate won't be encrypted in the keychain. Why is it important to store things without encryption?

@Oleygen
Copy link
Author

Oleygen commented Feb 1, 2019

here is keychain UI block: https://imgur.com/a/BNjyRNa as you can see, password, certs and keys are separated.
To summarize our conversation: yes, item class is mostly about semantic

How do you think do you need this changes for your repo? Cause I'd like to be fully correct in my apps.
Also I'd like to add enum swift wrapper on top of OSStatus error, will add pull request later

@evgenyneu
Copy link
Owner

How do you think do you need this changes for your repo?

No, sorry. While specifying keychain classes will be useful to some users of the library, I am not sure this is something that majority of users of this library care about. I don't think it is worth increasing the complexity of the library to introduce this feature. The only point of this library is to save text to keychain without caring about details. There are many alternative full-featured Keychain libraries that people can choose if they need extra features.

Also I'd like to add enum swift wrapper on top of OSStatus error, will add pull request later

Cool, could you explain what is this change about and your motivation before implementing it?

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

Successfully merging this pull request may close these issues.

2 participants