Prompt to unlock individual item when getting secret from keyring in unix system

AlanD20 commented 4 months ago

Hi, this PR unlocks individual items before getting secret from the keyring for unix systems.

Why?

I'm using KeepassXC on Arch as a secret service. And I was trying to setup gh CLI where it requires git credentials, and I wanted to use libsecret to store the credentials, so I updated my git config:

[credential]
  helper = !/usr/lib/git-core/git-credential-libsecret

In my KeepassXC settings, I have a checkbox checked where, by default it locks all collections and items. A client has to ask for unlock (a prompt) so that I will allow the client to retrieve the secret from the item or the collection.

KeepassXC => Tools > Settings > Secret Service Integration > Confirm when passwords are retrieved by clients

gh CLI is able to use the libsecret with my KeepassXC as a secret service but after debugging and using dbus-monitor and busctl monitor --user to see what the requests are.

I found this error from `dbus-monitor` (Exapndable)

``` method call time=1717061633.905718 sender=:1.554 -> destination=org.freedesktop.secrets serial=2 path=/org/freedesktop/secrets; interface=org.freedesktop.DBus.Properties; member=Get string "org.freedesktop.Secret.Service" string "Collections" method call time=1717061633.905779 sender=:1.463 -> destination=org.freedesktop.DBus serial=3485 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=GetConnectionUnixProcessID string ":1.554" method return time=1717061633.905787 sender=org.freedesktop.DBus -> destination=:1.463 serial=103 reply_serial=3485 uint32 863002 method call time=1717061633.906061 sender=:1.463 -> destination=org.freedesktop.DBus serial=3486 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=GetConnectionUnixProcessID string ":1.554" method return time=1717061633.906069 sender=org.freedesktop.DBus -> destination=:1.463 serial=104 reply_serial=3486 uint32 863002 method call time=1717061633.906348 sender=:1.463 -> destination=org.freedesktop.DBus serial=3487 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=AddMatch string "type='signal',sender='org.freedesktop.DBus',interface='org.freedesktop.DBus',member='NameOwnerChanged',arg0=':1.554',arg2=''" method return time=1717061633.906499 sender=:1.463 -> destination=:1.554 serial=3488 reply_serial=2 variant array [ object path "/org/freedesktop/secrets/collection/db_2Dsecrets" ] method call time=1717061633.906623 sender=:1.554 -> destination=org.freedesktop.secrets serial=3 path=/org/freedesktop/secrets; interface=org.freedesktop.Secret.Service; member=Unlock <<< COLLECTION IS UNLOCKED>>> array [ object path "/org/freedesktop/secrets/aliases/default" ] method return time=1717061633.906641 sender=:1.463 -> destination=:1.554 serial=3489 reply_serial=3 array [ object path "/org/freedesktop/secrets/collection/db_2Dsecrets" ] object path "/" method call time=1717061633.906708 sender=:1.554 -> destination=org.freedesktop.secrets serial=4 path=/org/freedesktop/secrets/aliases/default; interface=org.freedesktop.Secret.Collection; member=SearchItems array [ dict entry( string "username" string "" ) dict entry( string "service" string "gh:github.com" ) ] method return time=1717061633.906890 sender=:1.463 -> destination=:1.554 serial=3490 reply_serial=4 array [ object path "/org/freedesktop/secrets/collection/db_2Dsecrets/5e85bdec38db4f9fa00a4a00004e0977" ] method call time=1717061633.907022 sender=:1.554 -> destination=org.freedesktop.secrets serial=5 path=/org/freedesktop/secrets; interface=org.freedesktop.Secret.Service; member=OpenSession string "plain" variant string "" method return time=1717061633.907143 sender=:1.463 -> destination=:1.554 serial=3491 reply_serial=5 variant string "" object path "/org/freedesktop/secrets/session/42b891c426fa4ddab4ca9d5b98096369" method call time=1717061633.907231 sender=:1.554 -> destination=org.freedesktop.secrets serial=6 path=/org/freedesktop/secrets/collection/db_2Dsecrets/5e85bdec38db4f9fa00a4a00004e0977; interface=org.freedesktop.Secret.Item; member=GetSecret <<>> object path "/org/freedesktop/secrets/session/42b891c426fa4ddab4ca9d5b98096369" error time=1717061633.907283 sender=:1.463 -> destination=:1.554 error_name=org.freedesktop.Secret.Error.IsLocked reply_serial=6 <<>> method call time=1717061633.907338 sender=:1.554 -> destination=org.freedesktop.secrets serial=7 path=/org/freedesktop/secrets/session/42b891c426fa4ddab4ca9d5b98096369; interface=org.freedesktop.Secret.Session; member=Close method return time=1717061633.907424 sender=:1.463 -> destination=:1.554 serial=3493 reply_serial=7 ```

Looking closely at the requests here, it appears that go-keyring is able to unlock the collection which is done at https://github.com/zalando/go-keyring/blob/28657a580d2cfb4b21ff91769ce687ce4a31cb22/keyring_unix.go#L54-L65 but when it tries to get the secret, since I have the KeepassXC option enabled where the items are also locked, go-keyring does not attempt to unlock the item, and tries to get the secret. specifically at these log output in the dbus-monitor:

path=/org/freedesktop/secrets; interface=org.freedesktop.Secret.Service; member=OpenSession
   string "plain"
   variant       string ""
method return time=1717061633.907143 sender=:1.463 -> destination=:1.554 serial=3491 reply_serial=5
   variant       string ""
   object path "/org/freedesktop/secrets/session/42b891c426fa4ddab4ca9d5b98096369"
method call time=1717061633.907231 sender=:1.554 -> destination=org.freedesktop.secrets serial=6 path=/org/freedesktop/secrets/collection/db_2Dsecrets/5e85bdec38db4f9fa00a4a00004e0977; interface=org.freedesktop.Secret.Item; member=GetSecret <<<ATTEMPTING TO GET SECRET>>>
   object path "/org/freedesktop/secrets/session/42b891c426fa4ddab4ca9d5b98096369"
error time=1717061633.907283 sender=:1.463 -> destination=:1.554 error_name=org.freedesktop.Secret.Error.IsLocked reply_serial=6 <<<REJECTS DUE TO ITEM IS LOCKED>>>
method call time=1717061633.907338 sender=:1.554 -> destination=org.freedesktop.secrets serial=7 path=/org/freedesktop/secrets/session/42b891c426fa4ddab4ca9d5b98096369; interface=org.freedesktop.Secret.Session; member=Close

PR Changes

My changes attempt to unlock the item before getting the secret, which I have tried locally, and it works with gh cli as well. (see below to reproduce my local environment and my test steps.)

Looking at the secret-service documentation and specifically the first line says:

Some items and/or collections may be marked as locked by the service. The secrets of locked items cannot be accessed. Additionally, locked items or collections cannot be modified by the client application.

and the fifth line:

A client application should always be ready to unlock the items for the secrets it needs, or objects it must modify. It must not assume that an item is already unlocked for whatever reason.

How is gh cli related to this?

A https://github.com/cli/cli/issues/8691#issuecomment-2088541233 by GitHub CLI team confirms that they are not doing anything and simply using go-keyring library to set and get these secrets from keyrings. I have also confirmed locally that they use go-keyring wrapper here: https://github.com/cli/cli/blob/trunk/internal/keyring/keyring.go

What issue does it resolve?

https://github.com/zalando/go-keyring/issues/88 (my guess here is that the client tries to access a locked item and for some reason it waits.? not sure how this happened but could possibly fix this issue.)
https://github.com/cli/cli/issues/8691
https://github.com/cli/cli/issues/8729

probably other packages that use go-keyring as a dependency. I'm only using gh which uses go-keyring as a primary dependency for getting secrets from keyring.

Test Steps to Reproduce

Setup KeepassXC as a secret service.
Enable Confirm when passwords are retrieved by clients at KeepassXC => Tools > Settings > Secret Service Integration > Confirm when passwords are retrieved by clients
Install gh cli and authenticate using https protocol.

Configure git to use libsecret, for arch users, it already has git-credentials-libsecret at /usr/lib/git-core/git-credential-libsecret.

git config --global credential.helper libsecret
# or absolute path
git config --global credential.helper /usr/lib/git-core/git-credential-libsecret

Run dbus-monitor to see secret service requests.
check gh authentication status: gh auth status. (This will fail and says not authenticated)

Test PR Changes locally

working directory is ~/builds and the following steps will clone 2 projects in the ~/builds directory, and also builds gh cli locally at ~/builds as well.
1. First try to reproduce the error to create the environment. and make sure KeepassXC, git and gh cli are properly setup. Because this will confirm the PR changes resolve the issue.
2. Clone PR changes locally.
3. Clone gh cli repository at https://github.com/cli/cli then cd into your local gh cli repository
4. Modify the go.mod file to point the go-keyring package to the PR changes. My changes are at ../go-keyring relative to the cloned gh cli repository locally.
```
module github.com/cli/cli/v2
```
go 1.22

add this line to point to the local go-keyring changes

replace github.com/zalando/go-keyring => ../go-keyring
```
5. Run `go mod tidy` just in case.
6. Build gh cli locally, [Here is the doc](https://github.com/cli/cli/blob/trunk/docs/source.md), but it should be fairly simple by running `make install prefix=../gh`
7. Run the new gh cli binary with `go-keyring` changes: `cd ~builds/gh/bin && ./gh auth status`
8. A KeepassXC prompt appears that asks for confirmation by user, and after confirming, gh authentication succeeds.
```

Notes

I'm not very familiar with all of these, let me know if I have missed anything.
Feel free to review the test cases that I have modified for the mock provider.

Thanks!

AlanD20 commented 4 months ago

After looking into it more, looks like the mock provider changes will be a breaking change. Not sure what to do here. Probably I should remove the mock changes, or should it unlock whenever it gets a secret without the user explicitly unlocking the item?

mikkeloscar commented 4 months ago

@AlanD20 Thanks for the PR and the detailed description. Please allow a few days for us to review it.

mikkeloscar commented 4 months ago

:+1:

szuecs commented 4 months ago

:+1:

AlanD20 commented 4 months ago

Thanks for the help and review!

Will there be a new release for this? I'm planning to create a new PR in gh cli repository to use this change so that it will get fixed.

mikkeloscar commented 3 months ago

@AlanD20 New release here: https://github.com/zalando/go-keyring/releases/tag/v0.2.5

AlanD20 commented 3 months ago

@mikkeloscar Awesome! Thanks again :)

zalando / go-keyring