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

Allow kSecAttrService to be a customer-friendly string on Mac #140

Closed
abey opened this issue May 31, 2018 · 16 comments
Closed

Allow kSecAttrService to be a customer-friendly string on Mac #140

abey opened this issue May 31, 2018 · 16 comments

Comments

@abey
Copy link

abey commented May 31, 2018

I'd been working on a project with uses valet and I was curious to know if I could change the name of the location to something more simple.
For example, I would like it to display something like "Storage Space" instead of "VAL_VALValet_initWi...."

40080620-5d5bb8f2-5840-11e8-9c6a-3b92af8a1e82

@dfed
Copy link
Collaborator

dfed commented May 31, 2018

Oh, snap. No, we don't have a way to change the name of the keychain, since that's supposed to be an implementation detail. Honestly, we hadn't thought of this case. I assume the above dialog happens with the SecureEnclaveValet (or SinglePromptSecureEnclaveValet)?

We can take it as a feature request to allow folks to customize their kSecAttrService (where the above string comes from) on these Valets. We'd probably limit said customizability to the Mac code though. Thoughts @EricMuller22?

For now, classifying this as an enhancement request.

@dfed dfed changed the title How to change keychain location's name? Allow kSecAttrService to be a customer-friendly string on Mac May 31, 2018
@gregcotten
Copy link

I second this!

@gregcotten
Copy link

gregcotten commented Jul 10, 2018

I get this on macOS when I try to set a key/value pair with good old-fashioned Valet Looks like normal Valet just prompts "(App Name) wants to access the "login" keychain." which is perfectly acceptable.

@dfed
Copy link
Collaborator

dfed commented Jul 11, 2018

@abey & @gregcotten, do you know under what situations macOS prompts the user with the keychain identifier vs not? That info would be really helpful for helping us scope the work we have to do for this issue.

@gregcotten
Copy link

gregcotten commented Jul 11, 2018 via email

@antons
Copy link

antons commented Jul 30, 2018

As far as I know, this can happen when the code signing identity of the app launched app doesn’t match a know signature, for example when a user tries a Developer ID signed trial, then buys the app from the Mac App Store.

@dfed
Copy link
Collaborator

dfed commented Jul 30, 2018

Thank you for the explanation @antons!

@abey and @gregcotten, does the above jive with your experience?

I’m tempted to say that if the kSecAttrService is exposed in the access dialog only if the codesigning is wrong, this issue isn’t worth addressing.

@antons
Copy link

antons commented Jul 31, 2018

@dfed I wouldn’t say that the code signing is “wrong”. Developer ID and MAS are both valid ways to sign a public release. The real world example with a trial was meant to underline that I’d like to see the issue fixed as well. It’s not great when implementation details leak into UI like this.

@dfed
Copy link
Collaborator

dfed commented Jul 31, 2018

What led to the “codesigning is wrong” statement was the phrase “doesn’t match a known signature”. That sounds like the configuration is incorrect, no?

Ensuring that the kSecAttrService is unique per Valet configuration is one of the ways that we ensure that migration between Valets always works 100% of the time. The sandboxing it gives us is part of what makes Valet bulletproof. If there is a way for Mac devs to avoid getting this kind of implementation-detail-revealing dialog, I’d prefer to recommend that as a solution vs weakening the API.

Please note I have only released first-party macOS apps before... so how 3rd-party Mac app signing works is pretty new to me. Let’s back up a bit: can one of ya’ll explain what kind of scenarios can lead to this kind of dialog, and if these scenarios can be fixed by changing codesigning prior to ship?

@antons
Copy link

antons commented Jul 31, 2018

@dfed Ah, for someone unfamiliar with direct distribution Developer ID might sound like something that would be used on development devices, but it’s not. :-)

Apps distributed on MAS are signed by Apple using their certificate, Apps distributed directly are signed using a Developer ID. Code signatures are different, but both are valid and correct.

We distribute trials and beta versions signed with Developer ID. As far as I can see, the keychain password prompt appears when an app with a new unknown signature tries to access an existing keychain item. This means that every trial user will see the password prompt on the first launch of MAS version.

Apps cannot be signed using MAS certificate for direct distribution. I doubt that code signing changes can fix this problem.

@dfed
Copy link
Collaborator

dfed commented Jul 31, 2018

Thank you for the detailed explanation! That makes sense. So it is common practice in Mac apps to make the kSecAttrService human-readable?

Sounds like we need to add this API for our Mac folks. We’ll need a giant Here Be Dragons comment block with why you shouldn’t use it, but we’ll make it 🙂

@antons
Copy link

antons commented Jul 31, 2018

@dfed I haven’t used keychain without Valet for a few years, so I don’t know. :-)

It’d be great if there was a migration API to transfer existing items to the new identifier.

@dfed
Copy link
Collaborator

dfed commented Jan 17, 2020

This has been merged into develop--4.0, which we'll eventually merge into master. This issue is now resolved.

@dfed dfed closed this as completed Jan 17, 2020
@gregcotten
Copy link

Great work! 🙌

@dfed
Copy link
Collaborator

dfed commented Jan 17, 2020

@antons you should be able to utilize the existing migration API, since you'll be creating a Valet with the new explicit identifier, and can always migrate between Valets. Just remember to make your explicit identifier globally unique!

If you ever change anything about the configuration in a Valet with an explicitly set identifier, remember to change the Valet's identifier at the same time.

@antons
Copy link

antons commented Jan 18, 2020

@dfed This is great, thank you!

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

No branches or pull requests

4 participants