Keyring debugging and error handling, support `darwin/arm64` #147

simpsora · 2023-06-28T00:44:56Z

To help track down some of the Keyring issues we're seeing, here are a few improvements:

Enable debug mode in Keyring when verbose is enabled
In add, catch an error opening the keyring (same as in exec)
Also in add, sorta catch an error when creating creds (see the commit comment in 988642d)
Add darwin/arm64 as a build target
Add $GOARCH to the paths in the build script (otherwise the two darwin builds will step on each other)

This will log details about the available backends, the chosen backend, and other Keyring things.

Same as done in `exec.go`

Currently an error when setting the item misleadingly results in the success message. There's nothing in the Keyring docs about this (`Set` doesn't return an error), but I've found that `nil` is returned when the call is successful, and an error is returned otherwise. Unsure if this is good Golang practice or not.

simpsora · 2023-06-28T00:50:10Z

@jacobbednarz when running this locally, I am currently getting the macOS Keychain backend:

$ go run . add -v test
Email address: a@a
Authentication value (API key or API token):
DEBU[0006] API token detected
DEBU[0006] session-duration was not set, not using short lived tokens
DEBU[0006] new profile: {Email:a@a AuthType:api_token SessionDuration: Policies:[]}
2023/06/28 12:45:54 [keyring] Considering backends: [keychain pass file]
2023/06/28 12:45:54 [keyring] Checking keychain status
2023/06/28 12:45:54 [keyring] Keychain status returned nil, keychain exists
2023/06/28 12:45:54 [keyring] Keychain item trusts keyring
2023/06/28 12:45:54 [keyring] Adding service="cf-vault", label="", account="test-api_token", trusted=true to osx keychain "cf-vault.keychain"

Success! Credentials have been set and are now ready for use!

I don't know why this is different from the RC binary you shipped me yesterday. Looking at Keyring docs on this, it uses an ordered list that should pick the OS-native backend first. If it doesn't, there should be a reason emitted via the debug logging added in this PR.

Would you mind building a new RC and sending it across? Keen to see if it works with the macOS keychain, or if there are any useful error messages.

jacobbednarz · 2023-06-28T01:15:33Z

thanks for this, hopefully i'll have a look later today. in the meantime, you can build the releases using script/build <version> (which is all i was doing on a beefy machine).

i'll send one over too in case it is an amd/arm thing as well.

simpsora · 2023-06-28T02:06:51Z

Interesting!

I built it on an Intel Mac and an M1 Mac, and then ran it on the M1, and the behavior is different!

Built on an Intel mac:

Build env:

$ sw_vers; arch
ProductName:		macOS
ProductVersion:		13.4.1
BuildVersion:		22F82
i386

Resulting binary:

$ ./cf-vault  version
cf-vault 0.0.999+988642d (go1.20.4,gc-amd64)

Adding a profile:

$ ./cf-vault add -v foo
Email address: a@a
Authentication value (API key or API token):
DEBU[0002] API token detected
DEBU[0002] session-duration was not set, not using short lived tokens
DEBU[0002] new profile: {Email:a@a AuthType:api_token SessionDuration: Policies:[]}
2023/06/28 13:48:18 [keyring] Considering backends: [keychain pass file]
2023/06/28 13:48:18 [keyring] Checking keychain status
2023/06/28 13:48:18 [keyring] Keychain status returned nil, keychain exists
2023/06/28 13:48:18 [keyring] Keychain item trusts keyring
2023/06/28 13:48:18 [keyring] Adding service="cf-vault", label="", account="foo-api_token", trusted=true to osx keychain "cf-vault.keychain"
(macOS keychain prompt appears)

Uses the macOS keychain, as expected 👍

Built on an M1 mac:

Build env:

$ sw_vers; arch
ProductName:            macOS
ProductVersion:         13.4.1
BuildVersion:           22F82
arm64

Resulting binary:

$ ./cf-vault version
cf-vault 0.998+988642d (go1.20.5,gc-amd64)

Adding a profile:

$ ./cf-vault add -v foo
Email address: a@a
Authentication value (API key or API token):
DEBU[0006] API token detected
DEBU[0006] session-duration was not set, not using short lived tokens
DEBU[0006] new profile: {Email:a@a AuthType:api_token SessionDuration: Policies:[]}
2023/06/28 13:59:32 [keyring] Considering backends: [pass file]
2023/06/28 13:59:32 [keyring] Failed backend pass: The pass program is not available
2023/06/28 13:59:32 [keyring] Expanded file dir to /Users/rosssimpson/.cf-vault/keys/
Enter passphrase to unlock /Users/rosssimpson/.cf-vault/keys/:

No macOS keychain backend available 👎

One obvious other difference is the Golang version. I updated the Intel machine to go1.20.5 and it behaves the same (still pops the macOS prompt). Ruled that out.

So perhaps this has to do with the architecture of the build machine? Would need to dig more into Keyring to better understand it, I guess.

jacobbednarz · 2023-06-28T02:46:47Z

the machine i've been building on the release assets on is the same as your intel tests above.

$ sw_vers; arch
ProductName:		macOS
ProductVersion:		13.4.1
BuildVersion:		22F82
i386

the issue is already mentioned upstream at 99designs/keyring#126 but i suspect it is wrongly label for OS version but is more about the arch being used.

simpsora · 2023-06-28T21:57:21Z

@jacobbednarz I have narrowed this down a bit. I was experimenting with what's in script/build and found the following:

Building the darwin/amd64 target creates a binary that doesn't support the macOS Keychain backend
Building darwin/arm64 instead creates a binary that does support the macOS Keychain backend

It appears that on an M1 Mac, the amd64 binary is run by the Rosetta emulation thingo, and in this scenario Keyring doesn't see the Keychain backend. I'm still digging, but one of the fixes may be to just add darwin/arm64 as a build target.

Also adds `$GOARCH` to the the build paths, since there are two darwin targets now (otherwise one would overwrite the other).

simpsora · 2023-06-28T22:53:07Z

I added some debugging to Keyring's backend consideration code, and all I can get out of the amd64 build is:

2023/06/29 10:40:50 Backend keychain is NOT supported (q=<nil>, ok=false)

I tried using Delve to dig in and see what's up, but it doesn't work with "translated" processes (Rosetta, I'm guessing).

$ dlv attach 10412 ./cf-vault
Trying to attach to a translated process with the native debugserver, exiting...
could not attach to pid 10412: stub exited while waiting for connection: exit status 1

There appear to be other ways to debug this, but I'm far enough down the rabbit hole already. I think adding a build target of darwin/arm64 should be good enough, and have added support for that in 314aa8f.

jacobbednarz · 2023-06-29T01:31:17Z

i'm happy with that as it would also explain why we are seeing different outcomes here (i'm plain ol' intel, no apple silicone here).

jacobbednarz · 2023-06-29T03:13:12Z

i've cut 0.0.15 with these changes; do you want to give those a run and make sure they work for ARM after being cross compiled from an intel machine? the updates are automatically pushed into the homebrew tap if that is easier for you too.

building on intel for intel use is working here.

simpsora · 2023-06-29T03:29:58Z

@jacobbednarz well I've got no idea what's going on now. This release is behaving in the opposite manner than the last one:

amd64:

$ ./cf-vault version ; echo; ./cf-vault add -v test-amd64
cf-vault 0.0.15+2ff0299 (go1.20.1,gc-amd64)

Email address: a
Authentication value (API key or API token):
DEBU[0006] API token detected
DEBU[0006] session-duration was not set, not using short lived tokens
DEBU[0006] new profile: {Email:a AuthType:api_token SessionDuration: Policies:[]}
2023/06/29 15:23:43 [keyring] Considering backends: [keychain pass file]
2023/06/29 15:23:43 [keyring] Checking keychain status
2023/06/29 15:23:43 [keyring] Keychain status returned nil, keychain exists
2023/06/29 15:23:43 [keyring] Keychain item trusts keyring
2023/06/29 15:23:43 [keyring] Adding service="cf-vault", label="", account="test-amd64-api_token", trusted=true to osx keychain "cf-vault.keychain"
(keychain prompt appears)

arm64:

$ ./cf-vault version ; echo; ./cf-vault add -v test-arm64
cf-vault 0.0.15+2ff0299 (go1.20.1,gc-arm64)

Email address: a
Authentication value (API key or API token):
DEBU[0007] API token detected
DEBU[0007] session-duration was not set, not using short lived tokens
DEBU[0007] new profile: {Email:a AuthType:api_token SessionDuration: Policies:[]}
2023/06/29 15:28:25 [keyring] Considering backends: [pass file]
2023/06/29 15:28:25 [keyring] Failed backend pass: The pass program is not available
2023/06/29 15:28:25 [keyring] Expanded file dir to /<snip>.cf-vault/keys/
Enter passphrase to unlock /<snip>/.cf-vault/keys/:

jacobbednarz · 2023-06-29T04:12:13Z

is this from the release binaries? or from source on your local machine? if it is the first, maybe it is more that intel can't cross compile the keychain properly for ARM machines? do you get the same outcome if you build them on an ARM machine for use on ARM?

simpsora · 2023-06-29T22:47:02Z

I've got no idea what's causing the different behavior here. Might be something weird on my machine, might be something else. I'm not sure if there's anything else to do here.

Build → Runtime ↓	Github release arm64	Github release amd64	M1 local build arm64	M1 local build amd64	Intel local build arm64	Intel local build amd64
M1	❌	✅	✅	❌	❌	✅
Intel	N/A	✅	N/A	❌	N/A	✅

✅ = macOS Keychain backend works
❌ = macOS Keychain backend does not work

simpson-ross added 3 commits June 28, 2023 12:21

Enable Keyring debugging in verbose mode

bb1802d

This will log details about the available backends, the chosen backend, and other Keyring things.

Exit with error is the keyring can't be opened

fceac03

Same as done in `exec.go`

jacobbednarz mentioned this pull request Jun 28, 2023

macos Ventura 13.2.1 reports only 'pass' and 'file' as available backends, no 'keychain'. 99designs/keyring#126

Open

Adds darwin/arm64 as a build target

314aa8f

Also adds `$GOARCH` to the the build paths, since there are two darwin targets now (otherwise one would overwrite the other).

simpsora changed the title ~~Keyring debugging and error handling~~ Keyring debugging and error handling, support darwin/arm64 Jun 28, 2023

jacobbednarz merged commit 2ff0299 into jacobbednarz:master Jun 29, 2023
6 checks passed

simpsora deleted the keyring_debugging branch June 29, 2023 01:32

simpson-ross mentioned this pull request Jun 30, 2023

Existing credential secrets can't be found on MacOS after upgrading to 0.0.12+ #96

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keyring debugging and error handling, support `darwin/arm64` #147

Keyring debugging and error handling, support `darwin/arm64` #147

simpsora commented Jun 28, 2023 •

edited

simpsora commented Jun 28, 2023

jacobbednarz commented Jun 28, 2023

simpsora commented Jun 28, 2023

jacobbednarz commented Jun 28, 2023

simpsora commented Jun 28, 2023 •

edited

simpsora commented Jun 28, 2023 •

edited

jacobbednarz commented Jun 29, 2023

jacobbednarz commented Jun 29, 2023

simpsora commented Jun 29, 2023

jacobbednarz commented Jun 29, 2023

simpsora commented Jun 29, 2023 •

edited

Keyring debugging and error handling, support darwin/arm64 #147

Keyring debugging and error handling, support darwin/arm64 #147

Conversation

simpsora commented Jun 28, 2023 • edited

simpsora commented Jun 28, 2023

jacobbednarz commented Jun 28, 2023

simpsora commented Jun 28, 2023

Built on an Intel mac:

Built on an M1 mac:

jacobbednarz commented Jun 28, 2023

simpsora commented Jun 28, 2023 • edited

simpsora commented Jun 28, 2023 • edited

jacobbednarz commented Jun 29, 2023

jacobbednarz commented Jun 29, 2023

simpsora commented Jun 29, 2023

jacobbednarz commented Jun 29, 2023

simpsora commented Jun 29, 2023 • edited

Keyring debugging and error handling, support `darwin/arm64` #147

Keyring debugging and error handling, support `darwin/arm64` #147

simpsora commented Jun 28, 2023 •

edited

simpsora commented Jun 28, 2023 •

edited

simpsora commented Jun 28, 2023 •

edited

simpsora commented Jun 29, 2023 •

edited