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

Keychain Services support #70

Closed
tarcieri opened this issue Nov 1, 2018 · 10 comments · Fixed by #155
Closed

Keychain Services support #70

tarcieri opened this issue Nov 1, 2018 · 10 comments · Fixed by #155

Comments

@tarcieri
Copy link

tarcieri commented Nov 1, 2018

FYI, I've started writing a crate which provides a somewhat idiomatic Keychain Services binding:

https://github.com/iqlusioninc/keychain-services-rs

I'd be interested in trying to contribute its functionality upstream to this crate, or otherwise finding a way to share code (e.g. SecKey).

See also: iqlusioninc/keychain-services.rs#3

@kornelski
Copy link
Owner

I've just released a version with some keychain support.

As for integration, as a first step you could try using security-framework-sys for the low-level bindings.

@tarcieri
Copy link
Author

@kornelski I don't have time to work on this myself, but I'd be very interested in mentoring anyone who wants to port the code over.

The types in the keychain-services and security-framework crates are based on similar rust-objc abstractions so I imagine it'd largely be copy paste, just keeping an eye out for what security-framework types already exist, and otherwise it should be a pretty much additive contribution for all of the Keychain Services-related bits which are presently missing.

Perhaps you could flag this issue with "Help Wanted"/"Good First Issue" or whatever labels are appropriate?

@brotskydotcom
Copy link
Contributor

Hi @tarcieri and @kornelski, I am one of the maintainers of the keyring crate and I recently noticed the lack of support in this framework for adding and removing keyring items on iOS. I would be willing to take on adding this extension if I could get some mentoring. Are either of you willing to think about helping me at this time?

@tarcieri
Copy link
Author

tarcieri commented Dec 3, 2021

I can certainly help review attempts to merge keychain-services into security-framework, although I have no iOS experience and it was only tested on macOS.

@brotskydotcom
Copy link
Contributor

Thanks, @tarcieri! I've taken a look at the keychain-services code and it's definitely macOS-specific. The keychain stuff on iOS is much simpler and works pretty much the same way the key and other storage works, but with slightly different attributes. So I think I'm going to have to approach this by extending the existing security-framework code to support a couple more system calls than it does currently.

@kornelski, is there any specific reason the framework currently supports only the SecItemCopyMatching call and not the SecItemAdd, SecItemUpdate, and SecItemDelete calls? Is that the result, for example, of a design decision, or problems trying to implement support for them? If there is no specific reason, I'm happy to try and extend the current item query support to include support for add queries, and to extend the supported attribute types to include generic and internet credentials.

@kornelski
Copy link
Owner

@brotskydotcom There's no grand reason behind it. It's fine to change the implementation.

@brotskydotcom
Copy link
Contributor

@kornelski I'm making good progress, and have the start of something working that's a consistent overlay on what's there. A few questions:

  • Now that I'm re-compiling the framework, I notice that it generates warnings in a few places (unread structure members, for example). I'm assuming I should leave those on, but would you rather I silenced any of them?
  • As part of my work, I'm adding an iOS shim/testing crate. I'm assuming I should match its earliest version to the 2018 being used by all the other crates in the project, yes?
  • Also as part of my work, I'm adding an actual iOS app in Swift that uses the shim crate for testing/illustrative purposes. The app is at crate-level in the folder structure, but of course it's not a crate. Do you want me to include that app in my PR?

@kornelski
Copy link
Owner

I've fixed a couple of warnings. Generally there shouldn't be any, but sometimes unused fields or imports creep in with particular combinations of features. Except systest — it does some deprecated things that have no proper alternative yet.

@kornelski
Copy link
Owner

kornelski commented Dec 10, 2021

Yes, please use 2018 edition. I don't think 2021 adds enough to warrant a big jump in minimum supported rust version yet.

Sure, a Swift app would be nice. But please have tests for it, so that it doesn't rot.

@brotskydotcom
Copy link
Contributor

Understood and will definitely have tests. Thanks for the quick answers.

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

Successfully merging a pull request may close this issue.

3 participants