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

SecretServiceDBusHelper.Get() try to access locked secrets #355

Closed
histausse opened this issue Mar 4, 2023 · 8 comments
Closed

SecretServiceDBusHelper.Get() try to access locked secrets #355

histausse opened this issue Mar 4, 2023 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@histausse
Copy link

I am using keepassxc as a password manager but the bridge fails to access its secret using secret service.

After investigation it appears that SecretServiceDBusHelper.Get() try to access the secret without unlocking thems first.

Expected Behavior

The bridge should be able to acces its secret stored by keepassxc by unlocking them first.

Current Behavior

The bridge does not unlock all its secrets, resulting in an error when trying to access them and ultimatly leading to a "Vault is insecure" error message:

$ ./bridge --cli
INFO[0000] Migrating keychain helper
WARN[Mar  4 21:35:23.837] Captured message                              message="Vault is insecure" reportID=09f90a0d2ee2461083f5c77dde3987f7
WARN[Mar  4 21:35:23.847] The vault key could not be retrieved; the vault will not be encrypted

Possible Solution

One way to solve this is to explicitly unlocking the items before quering them:

diff --git a/pkg/keychain/helper_dbus_linux.go b/pkg/keychain/helper_dbus_linux.go
index b759c7f4..376794bc 100644
--- a/pkg/keychain/helper_dbus_linux.go
+++ b/pkg/keychain/helper_dbus_linux.go
@@ -175,6 +175,11 @@ func (s *SecretServiceDBusHelper) Get(serverURL string) (string, string, error)
                return "", "", err
        }

+       err = service.Unlock(items)
+       if err != nil {
+               return "", "", err
+       }
+
        var secretPlaintext []byte
        err = handleTimeout(func() error {
                var err error

Steps to Reproduce

Running the bridge with keepassxc's secret service integration with the option "confirm when password are retrieved by clients"

I already had the bridge more or less working with keepassxc before it stopped working completly with the last update, so some other steps may be required to repoduce.

Version Information

The example and potential fix are for the version v3.0.19 of the bridge (commit c6f1f15)

@gudvinr
Copy link

gudvinr commented Mar 5, 2023

I am having this issue with kwallet secret service too. Which basically means you need to sign in manually every time you start PC.

@LBeernaertProton
Copy link
Collaborator

Hey @histausse can you please provide your KeepassXC version so we can verify this behavior.

@LBeernaertProton LBeernaertProton added the question Further information is requested label Mar 13, 2023
@histausse
Copy link
Author

I'm using the one available in the archlinux repository:
KeePassXC - Version 2.7.4
Revision: 63b2394

@LBeernaertProton
Copy link
Collaborator

LBeernaertProton commented Mar 15, 2023

Hey @histausse I used the AppImage version of KeePassXC 2.7.4 on Kubuntu 22.04, which also doesn't have a default secret service setup. I am able to get KeepPassXC to prompt me for a password without any changes. But it seems that it does not send a signal that the prompt has been closed.

WARN[0030] Failed to add test credentials to keychain    error="failed to prompt: prompt timed out" helper="*keychain.SecretServiceDBusHelper"
INFO[0030] Migrating keychain helper                    
WARN[Mar 15 12:59:27.762] Captured message                              message="Vault is insecure" reportID=31caa39bc9ca4c5cb16a6362c5b2e83d
WARN[Mar 15 12:59:27.772] The vault key could not be retrieved; the vault will not be encrypted

If I use the version available via the Kubuntu repos (2.6.6) I get the following output:

WARN[0000] Failed to add test credentials to keychain    error="failed to unlock items: org.freedesktop.Secret.Error.NoSuchObject" helper="*keychain.SecretServiceDBusHelper"
INFO[0000] Migrating keychain helper                    
WARN[Mar 15 13:04:00.595] Captured message                              message="Vault is insecure" reportID=c841faeb00314615b4e6d17a98f3e068
WARN[Mar 15 13:04:00.604] The vault key could not be retrieved; the vault will not be encrypted 

Is this similar to what you see?

@histausse
Copy link
Author

Not really, but it's simmilar, probably because the secret are already stored on keepasswc. On my hand I have this error:

INFO[0000] Migrating keychain helper
WARN[Mar 15 13:31:26.242] Captured message                              message="Vault is insecure" reportID=b7590f45d9e64242b151e1c6a376ee8b
WARN[Mar 15 13:31:26.248] The vault key could not be retrieved; the vault will not be encrypted

Keepassxc prompt for unlocking access ... / protonmail/bridge-v3/users/bridge-vault-key but either the bridge does not wait for the secret to be unlocked or keepassxc does not response (--cli display the promt before I have a chance to unlock the secret).

On my patched version, after the message

INFO[0000] Migrating keychain helper

the bridge ask to unlock the secret ... / protonmail/bridge-v3/users/bridge-vault-key two times and wait for the secret to be unlocked (--cli only display the prompt after the secret is unlock or after a timeout of ~ 30s). I guess the first time is the original request from the bridge that is ignored, and the second one is the one I added myself.

I'll try to see if I can reproduce on a clean VM when I'll have some time.

@LBeernaertProton
Copy link
Collaborator

I have managed to reproduce the issue. I didn't have the db setup to allow access through the secret service.

@LBeernaertProton LBeernaertProton added bug Something isn't working and removed question Further information is requested labels Mar 15, 2023
@LBeernaertProton LBeernaertProton self-assigned this Mar 15, 2023
@LBeernaertProton
Copy link
Collaborator

This issue may also be affecting #359

@LBeernaertProton
Copy link
Collaborator

LBeernaertProton commented Mar 16, 2023

Closing this issue as it is a duplicate of #359

@histausse The fix for #359 will also solve the issue you are having with KeepassXC. Everything works as expected on v2.7.4 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants