[RFC] Keychain-backed settings

[paulhdk]

Hi @maxgoedjen !

As promised in #524 , here's my current progress in the form of an RFC pull request. I was hoping to get some feedback and make sure that I'm running in the right direction 🙂

This pull request adds:

• An API for storing to and reading from the keychain via simple get and set functions.
• Replace the use of UserDefaults in JustUpdatedChecker.swift.
• A settings screen which can be accessed via File -> Settings... or CMD + ,.
• The ability to disable the SSH comment in the new settings screen by selecting "None".

Here's how the Settings looks:

Concrete questions I have:

• What styles should I add for the SSH comments?
• How should the different SSH comment styles be presented? I guess it would make sense to add some kind of preview.
• What else belongs in the settings screen?
• Did I understand the keychain suggestion correctly? Should it be used somewhere, or is the API enough for now?
• Should the settings have an "About" or "Acknowledgements" tab?
• Re: how UserDefaults are replaced inJustUpdatedChecker.swift, is this how it's supposed to work? There is a slight reinterpretation of the keychain functionality happening here since the keychain thinks that we're storing a password, which strictly speaking isn't the case 🙂

[maxgoedjen]

Answering your questions from PR:

• What styles should I add for the SSH comments? I put a few in comments, but I think a few variations of user@host, [email protected], secretive@host, etc probably make sense. If we can figure out a way to make it an actual format string with a few components the user can rearrange, I'd love that, but may be a little out of scope.

• How should the different SSH comment styles be presented? I guess it would make sense to add some kind of preview. A preview is a great idea. Having a CopyableView below the editor with a dummy secret (or one from their actual store) and what the comment would look like would be cool.

• What else belongs in the settings screen? Mainly we just want the comment style there for now. If/when I can figure out a CLI interface, enabling it will also happen there.

• Did I understand the keychain suggestion correctly? Should it be used somewhere, or is the API enough for now? Kinda – commented in the code, but I think it's basically just backwards now. Comment style goes in keychain, update checker sticks with user defaults.

• Should the settings have an "About" or "Acknowledgements" tab? macOS convention is to have that be a different thing, it's already there in Secretive->About Secretive, so let's keep that there for now.

• Re: how UserDefaults are replaced inJustUpdatedChecker.swift, is this how it's supposed to work? There is a slight reinterpretation of the keychain functionality happening here since the keychain thinks that we're storing a password, which strictly speaking isn't the case 🙂 Commented on that there, but basically we don't want to be using this for updated checker. That being said, GenericPassword is a verrrrrry broad keychain category, it's okay to store whatever in there. Doesn't have to be a literal password.

[paulhdk]

Thanks so much for the detailed comments, @maxgoedjen !

I'll try to address them ASAP, but it might end up being next week since I have to prioritise some other work in the next few days.

[ChristopherA]

We too have been puzzling on a number of best practices for SSH keys. In particular, we believe key separation not only for each computer, but separation of keys "key usage/proof purpose" is critical, and thus you should NOT use the same key for auth & signing. This doesn't seem like something Secretive can support right now.

My document is still an early draft, but will give you some of our thinking on naming conventions for ssh key files.

https://gist.github.com/ChristopherA/3d6a2f39c4b623a1a287b3fb7e0aa05b

Regarding key comments, GitHub doesn't seem to preserve them when uploaded, however, I do try to make the "title" field in GitHub match the SSH comment. You can see my own GitHub signing keys at https://api.github.com/users/ChristopherA/ssh_signing_keys along with their titles/comments. They are not completely consistent yet as I'm still working on our conventions to support best practices there.

(If you are interested, this will give you a clue where we hope to go with ssh signatures in the more long-term https://gist.github.com/ChristopherA/dcf963c3849bc0d67b35c215efd15f86 & https://github.com/BlockchainCommons/ssh-envelope-python )

SSH signing and improving digital attribution and provenance is an important project for us this quarter, so welcome any advice.

[maxgoedjen]

you should NOT use the same key for auth & signing. This doesn't seem like something Secretive can support right now.

That's (assuming I'm understanding you correctly) definitely not the case. I personally have a different key for SSH access and signing git commits (which you can see in my profile). Is there something about this flow that doesn't work for you? Essentially I just have one key that I tell git to use for signing commits, and another one for SSH access.

[ChristopherA]

That's (assuming I'm understanding you correctly) definitely not the case. I personally have a different key for SSH access and signing git commits (which you can see in my profile). Is there something about this flow that doesn't work for you? Essentially I just have one key that I tell git to use for signing commits, and another one for SSH access.

Yes, that is what is required. I think the docs for configuring this are not clear. When searching for an answer, I saw somewhere (in an issues or PR?) that you needed to put the same public key in both the auth and signing.

[ChristopherA]

I found the errant doc:

https://github.com/maxgoedjen/secretive/pull/439/files

2. Add it to your GitHub account as an Auth and a Signing key, i.e. add it twice.

[maxgoedjen]

Yeah that's not merged ;) – there's no requirement on Secretive's side for that to be the case at all, it's totally agnostic of key usage. Some people may find that to be the most convenient setup, but there's no requirement to that effect.

[ChristopherA]

Thanks. We are still experimenting with various proof-of-concepts to support best practices, but when we are satisfied, I'll consider a improved PR for docs here.

[paulhdk]

@maxgoedjen sorry for letting this PR sit so long. Aiming to finally address all of your feedback within the week!