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

When adding a repo, setting password fails without feedback when password manager is locked #606

Closed
samuel-w opened this issue Sep 1, 2020 · 14 comments · Fixed by #607 or #779
Closed
Labels
type:bug Something doesn't work as intended

Comments

@samuel-w
Copy link
Contributor

samuel-w commented Sep 1, 2020

Describe the bug
When adding a repo, setting password fails without feedback when password manager is locked

To Reproduce
Steps to reproduce the behavior:

  1. Open the initialize/add repository with kwallet/secretstorage unlocked
  2. Lock kwallet/secretstorage.
  3. Enter a password and repo that uses encryption
  4. Click add

Set password invisibly fails for KWallet, or does nothing while throwing exception in log for secretstorage.
Not sure about macOS keychain.

Suggested fix: Make set_password return whether or not the password was set correctly with get, and either:

  • Fallback to VortaDB (not advised due to security concerns)
  • Tell user to unlock password manager ("Please unlock your password manager")

Should check if exception is thrown, otherwise generic fix method is to check with get_password after set_password
return bool(self.get_password(sevice, repo_url))

Desktop (please complete the following information):

  • OS: Ubuntu 20.04/Kubuntu 20.04

Additional context
If appropriate include logs. Can be found in Main Window > Misc Tab > Log.

Log for secretstorage:

2020-08-31 20:53:34,066 - vorta.borg.borg_thread - DEBUG - Using VortaSecretStorageKeyring keyring to store passwords.
2020-08-31 20:53:34,066 - asyncio - DEBUG - Using selector: EpollSelector
Traceback (most recent call last):
  File "/home/user/.local/lib/python3.8/site-packages/vorta/views/repo_add_dialog.py", line 64, in run
    params = BorgInitThread.prepare(self.values)
  File "/home/user/.local/lib/python3.8/site-packages/vorta/borg/init.py", line 20, in prepare
    ret = super().prepare(profile)
  File "/home/user/.local/lib/python3.8/site-packages/vorta/borg/borg_thread.py", line 121, in prepare
    ret['password'] = keyring.get_password('vorta-repo', profile.repo.url)
  File "/home/user/.local/lib/python3.8/site-packages/vorta/keyring/secretstorage.py", line 30, in get_password
    collection = secretstorage.get_default_collection(self.connection)
  File "/usr/lib/python3/dist-packages/secretstorage/collection.py", line 155, in get_default_collection
    return Collection(bus)
  File "/usr/lib/python3/dist-packages/secretstorage/collection.py", line 42, in __init__
    self.collection_props_iface.Get(COLLECTION_IFACE, 'Label',
  File "/usr/lib/python3/dist-packages/secretstorage/util.py", line 31, in function_out
    return function_in(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/dbus/proxies.py", line 141, in __call__
    return self._connection.call_blocking(self._named_service,
  File "/usr/lib/python3/dist-packages/dbus/connection.py", line 652, in call_blocking
    reply_message = self.send_message_with_reply_and_block(
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.UnknownObject: No such object path '/org/freedesktop/secrets/aliases/default'
@Hofer-Julian
Copy link
Collaborator

Nice catch!

@samuel-w samuel-w added the type:bug Something doesn't work as intended label Sep 4, 2020
@as3ii
Copy link

as3ii commented Nov 12, 2020

I don't know if it is really related, but I'm having a similar issue when importing an encrypted repository.

2020-11-12 17:11:43,057 - vorta.borg.borg_thread - DEBUG - Using VortaSecretStorageKeyring keyring to store passwords.
2020-11-12 17:11:43,057 - asyncio - DEBUG - Using selector: EpollSelector
2020-11-12 17:11:43,079 - root - DEBUG - Found 0 passwords matching repo URL.
2020-11-12 17:11:43,080 - vorta.borg.borg_thread - DEBUG - Password not found in primary keyring. Falling back to VortaDBKeyring.
2020-11-12 17:11:43,097 - vorta.borg.borg_thread - INFO - Running command /usr/bin/borg info --info --json --log-json /mnt/archive/backup
2020-11-12 17:11:44,029 - asyncio - DEBUG - Using selector: EpollSelector
Traceback (most recent call last):
  File "/usr/lib/python3.8/site-packages/vorta/borg/borg_thread.py", line 238, in run
    self.process_result(result)
  File "/usr/lib/python3.8/site-packages/vorta/borg/info.py", line 63, in process_result
    keyring.set_password("vorta-repo", new_repo.url, result['params']['password'])
  File "/usr/lib/python3.8/site-packages/vorta/keyring/secretstorage.py", line 26, in set_password
    collection.create_item(repo_url, attributes, password, replace=True)
  File "/usr/lib/python3.8/site-packages/secretstorage/collection.py", line 115, in create_item
    self.ensure_not_locked()
  File "/usr/lib/python3.8/site-packages/secretstorage/collection.py", line 55, in ensure_not_locked
    raise LockedException('Collection is locked!')
secretstorage.exceptions.LockedException: Collection is locked!
[1]    62193 abort (core dumped)  vorta

@samuel-w
Copy link
Contributor Author

Yeah, its the same bug.

@m3nu m3nu closed this as completed in #607 Dec 16, 2020
@luminoso
Copy link

luminoso commented Feb 1, 2021

I think that this PR is blocking me from using Vorta.
I'm on a kde/plasma 5.20.5 session and it was working fine on vorta 0.71.0.

Log shows the following:

QSettings::value: Empty key passed
QSettings::value: Empty key passed
2021-02-01 09:44:39,520 - vorta.i18n - DEBUG - Loading translation succeeded for ['en', 'en-US', 'en-Latn-US'].
2021-02-01 09:44:39,527 - apscheduler.scheduler - INFO - Scheduler started
2021-02-01 09:44:39,815 - root - INFO - Using NetworkManagerMonitor NetworkStatusMonitor implementation.
2021-02-01 09:44:40,010 - vorta.borg.borg_thread - INFO - Running command /usr/bin/borg --version
2021-02-01 09:44:48,206 - asyncio - DEBUG - Using selector: EpollSelector
2021-02-01 09:44:48,208 - vorta.borg.borg_thread - DEBUG - Using VortaSecretStorageKeyring keyring to store passwords.
2021-02-01 09:44:48,213 - vorta.notifications - DEBUG - notification not suppressed

And GUI says: Please unlock your password manager
I've checked and Kwallet is unlocked

@m3nu
Copy link
Contributor

m3nu commented Feb 1, 2021

Can't reproduce this on macOS, but will reopen it for more discussion. Are those still the right steps?

Steps to reproduce the behavior:

Open the initialize/add repository with kwallet/secretstorage unlocked
Lock kwallet/secretstorage.
Enter a password and repo that uses encryption
Click add

@m3nu m3nu reopened this Feb 1, 2021
@m3nu
Copy link
Contributor

m3nu commented Feb 1, 2021

The only interesting thing in the logs is Using VortaSecretStorageKeyring keyring to store passwords.. Maybe an issue there?

Relevant code for this keyring module would be here. Would be simple to test in just iPython or so.

@luminoso
Copy link

luminoso commented Feb 1, 2021

Can't reproduce this on macOS, but will reopen it for more discussion. Are those still the right steps?

Steps to reproduce the behavior:
Open the initialize/add repository with kwallet/secretstorage unlocked
Lock kwallet/secretstorage.
Enter a password and repo that uses encryption
Click add

Open Vorta in a KDE/plasma session. And any of the following:

  1. Try to run a backup job
  2. When adding a new repo
  3. Delete all settings and start from scratch

leads to Please unlock your password manager
Kwallet says: The 'kdewallet' wallet is currently open

It looks like it is related to Kwallet detection and #540 ?

I've tested and if I edit abc.py from

                try:
                    cls._keyring = VortaSecretStorageKeyring()
                except (secretstorage.exceptions.SecretStorageException, DBusException):  # Try to use KWallet (KDE)
                    from .kwallet import VortaKWallet5Keyring, KWalletNotAvailableException
                    try:
                        cls._keyring = VortaKWallet5Keyring()
                    except KWalletNotAvailableException:  # Save passwords in DB, if all else fails.
                        from .db import VortaDBKeyring
                        cls._keyring = VortaDBKeyring()

to

                try:
                    cls._keyring = VortaSecretStorageKeyring()
                    if not cls._keyring.is_unlocked:
                        raise secretstorage.exceptions.SecretStorageException("Secret Storage is locked")
                except (secretstorage.exceptions.SecretStorageException, DBusException):  # Try to use KWallet (KDE)
                    from .kwallet import VortaKWallet5Keyring, KWalletNotAvailableException
                    try:
                        cls._keyring = VortaKWallet5Keyring()
                    except KWalletNotAvailableException:  # Save passwords in DB, if all else fails.
                        from .db import VortaDBKeyring
                        cls._keyring = VortaDBKeyring()

I had to re-add the repo but it works just fine

@graugelb
Copy link

graugelb commented Feb 1, 2021

"..And GUI says: Please unlock your password manager..".
Same for System -->USB/Mint LMDE4 Vorta USB source --> USB target connected.
Hmm.
Do I understand it right, that it is a bug??
Pls. (this is important for me, as if so, I will not proceed, spending time-> lacking of knowledge).
Even w i t h o u t a password it is not possible to proceed!?

@m3nu
Copy link
Contributor

m3nu commented Feb 1, 2021

I don't know much about Linux desktops, but from the patch above it seems our Secret Storage class initializes successfully, even though Secret Storage isn't usable?

If so, VortaSecretStorageKeyring.__init__() should properly make those checks or fail. So this line should go into the VortaSecretStorageKeyring class, so it fails early.

if not cls._keyring.is_unlocked:
   raise secretstorage.exceptions.SecretStorageException("Secret Storage is locked")

@luminoso
Copy link

luminoso commented Feb 1, 2021

I think that it is expected to have the wallet unlocked when your session is running. And you can have multiple wallet back-ends depending on your desktop environment. But someone please correct me.

What my patch does is:

  1. Check if VortaSecretStorageKeyring is able to unlock secret-storage
  2. If it isn't unlocked then we're potentially running plasma session
  3. Then it tries Kwallet, which works (because it is unlocked because I'm in a plasma session)

However I don't know if being locked can be an heuristic for the desktop environment being used.

Maybe @samuel-w can clarify

@samuel-w
Copy link
Contributor Author

samuel-w commented Feb 1, 2021

With the patch, when secret storage is locked it will automatically use kwallet, leading to this issue again #616.

@samuel-w
Copy link
Contributor Author

samuel-w commented Feb 1, 2021

Do you have gnome keyring running?

@samuel-w
Copy link
Contributor Author

samuel-w commented Feb 1, 2021

Actually, by default Ubuntu pulls in the gnome keyring, so it and all derivatives have secretstorage set up by default. I'll change the order so KWallet runs first.

@luminoso
Copy link

luminoso commented Feb 1, 2021

Yes, for some reason gnome-keyring-daemon is running when I start my plasma session. I don't know if the same applies for kwallet in a Gnome session

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something doesn't work as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants