Replace KeychainAccess with Valet for Data Protection Keychain usage

[mgcm]

WIP for #49

[scoates]

Genuinely, thank you for working on this.

Honest question: is it worth using a library like Valet here?

The reason I ask is: the native SecItem/kSec API in Foundation is ugly, and I admit I might not be doing it in the best way, but I have a "KeychainHelper" class in a client project that's less than 100 lines long… seems our needs here are also pretty light.

Also admittedly, I have an aversion to unnecessary dependencies, so there's some bias on my side.

Additionally, using upToNextMajorVersion (while much better than pointing at HEAD of a branch, and I know is considered common and good practice) is a much bigger opening to a supply chain attack than pinning a specific commit hash—it's easy for an account compromise to tag a malicious release version, and much harder to fake a specific hash. (Pinning a specific hash requires ongoing maintenance to get updates from upstream, though.) Here's an example of this kind of exploit from a different but similarly-managed (through version numbers vs. hash pinning) system—we'd at least have a chance to catch an exploit before rolling a version in Mactrix, a realtime exploit with GH Actions.

I write this with no illusion of authority here. Just want to have these conversations. (Sincerely hope that's okay; happy to move the conversation elsewhere, too.)

[mgcm]

@scoates fully agree on everything you said - I had this PR as a draft and used Valet just to confirm that using the Data Protection variant of the SecItem API got rid of those nasty keychain password dialogs.

As discussed in the corresponding ticket (#49) with @viktorstrate , going this route (with or without Valet) has implications on distribution and we're still figuring out the best way to move forward on that.

@viktorstrate not sure why you removed this from draft but I would refrain from merging this for now.

[viktorstrate]

As I understand it the entitlement is only required if we need keychain sharing (which we don't). The new API should be useable without this entitlement or any special developer certificate.

From the Apple docs:

Keychain items can be shared only between apps from the same developer. To share keychain items, third-party apps use access groups with a prefix allocated to them through the Apple Developer Program in their application groups. The prefix requirement and application group uniqueness are enforced through code signing, provisioning profiles, and the Apple Developer Program.

I agree that we should not depend on an external library for this.

[scoates]

Great! Thank you both. I'm sorry I missed the discussion. Glad we're on the same page.

[mgcm]

I've updated this MR with a minimal Keychain Wrapper that removes the third party dependencies.

To enable the Data Protection keychain mode, the kSecUseDataProtectionKeychain attribute must be set to true.

This PR keeps it false but if you toggle it, running the app without the keychain-access-groups entitlement will throw an error with the code -34018 -> Internal error when a required entitlement isn't present. see here.

As soon as the entitlement is added and the signing configured, it works.