Allow to fully disable using the system keychain. (#898)

- Add option to avoid using system keychain.
- Prioritize keychains first. Then try to start them.
- Maintenance: Possible fixes for hung tests and segfault on exit on some setups.

.github/workflows/test.yml

@@ -59,7 +59,7 @@ jobs:
     - name: Test with pytest (macOS)
       if: runner.os == 'macOS'
       run: |
-        pytest
+        pytest --cov=vorta
 
     - name: Upload coverage to Codecov
       uses: codecov/codecov-action@v1

src/vorta/application.py

@@ -102,9 +102,11 @@ class VortaApp(QtSingleApplication):
         return False
 
     def quit_app_action(self):
-        del self.main_window
-        self.scheduler.shutdown()
         self.backup_cancelled_event.emit()
+        self.scheduler.shutdown()
+        del self.main_window
+        self.tray.deleteLater()
+        del self.tray
         cleanup_db()
 
     def create_backup_action(self, profile_id=None):

src/vorta/borg/borg_thread.py

@@ -8,6 +8,7 @@ import select
 import time
 import logging
 from collections import namedtuple
+from threading import Lock
 from PyQt5 import QtCore
 from PyQt5.QtWidgets import QApplication
 from subprocess import Popen, PIPE, TimeoutExpired
@@ -18,7 +19,7 @@ from vorta.utils import borg_compat, pretty_bytes
 from vorta.keyring.abc import VortaKeyring
 from vorta.keyring.db import VortaDBKeyring
 
-mutex = QtCore.QMutex()
+mutex = Lock()
 logger = logging.getLogger(__name__)
 
 FakeRepo = namedtuple('Repo', ['url', 'id', 'extra_borg_arguments', 'encryption'])
@@ -93,11 +94,7 @@ class BorgThread(QtCore.QThread, BackupProfileMixin):
 
     @classmethod
     def is_running(cls):
-        if mutex.tryLock():
+        return mutex.locked()
-            mutex.unlock()
-            return False
-        else:
-            return True
 
     @classmethod
     def prepare(cls, profile):
@@ -139,11 +136,12 @@ class BorgThread(QtCore.QThread, BackupProfileMixin):
 
         # Check if keyring is locked
         if profile.repo.encryption != 'none' and not cls.keyring.is_unlocked:
-            ret['message'] = trans_late('messages', 'Please unlock your password manager.')
+            ret['message'] = trans_late('messages',
+                                        'Please unlock your system password manager or disable it under Misc')
             return ret
 
         # Try to fall back to DB Keyring, if we use the system keychain.
-        if ret['password'] is None and cls.keyring.is_primary:
+        if ret['password'] is None and cls.keyring.is_system:
             logger.debug('Password not found in primary keyring. Falling back to VortaDBKeyring.')
             ret['password'] = VortaDBKeyring().get_password('vorta-repo', profile.repo.url)
 
@@ -189,7 +187,7 @@ class BorgThread(QtCore.QThread, BackupProfileMixin):
 
     def run(self):
         self.started_event()
-        mutex.lock()
+        mutex.acquire()
         log_entry = EventLogModel(category='borg-run',
                                   subcommand=self.cmd[1],
                                   profile=self.params.get('profile_name', None)
@@ -274,7 +272,7 @@ class BorgThread(QtCore.QThread, BackupProfileMixin):
 
         self.process_result(result)
         self.finished_event(result)
-        mutex.unlock()
+        mutex.release()
 
     def cancel(self):
         """
@@ -286,9 +284,11 @@ class BorgThread(QtCore.QThread, BackupProfileMixin):
             try:
                 self.process.wait(timeout=3)
             except TimeoutExpired:
-                self.process.terminate()
+                os.killpg(os.getpgid(self.process.pid), signal.SIGTERM)
-            mutex.unlock()
+            self.quit()
-            self.terminate()
+            self.wait()
+            if mutex.locked():
+                mutex.release()
 
     def process_result(self, result):
         pass

src/vorta/keyring/abc.py

@@ -1,49 +1,39 @@
-"""
+import importlib
-Set the most appropriate Keyring backend for the current system.
+from vorta.i18n import trans_late
-For Linux not every system has SecretService available, so it will
-fall back to a simple database keystore if needed.
-"""
-import sys
-from pkg_resources import parse_version
 
 
 class VortaKeyring:
-    _keyring = None
+    all_keyrings = [
+        ('.db', 'VortaDBKeyring'),
+        ('.darwin', 'VortaDarwinKeyring'),
+        ('.kwallet', 'VortaKWallet5Keyring'),
+        ('.secretstorage', 'VortaSecretStorageKeyring')
+    ]
 
     @classmethod
     def get_keyring(cls):
         """
-        Attempts to get secure keyring at runtime if current keyring is insecure.
+        Choose available Keyring. First assign a score and then try to initialize it.
-        Once it finds a secure keyring, it wil always use that keyring
         """
-        if cls._keyring is None or not cls._keyring.is_primary:
+        available_keyrings = []
-            if sys.platform == 'darwin':  # Use Keychain on macOS
+        for _module, _class in cls.all_keyrings:
-                from .darwin import VortaDarwinKeyring
+            try:
-                cls._keyring = VortaDarwinKeyring()
+                keyring = getattr(importlib.import_module(_module, package='vorta.keyring'), _class)
-            else:
+                available_keyrings.append((keyring, keyring.get_priority()))
-                # Try to use KWallet (KDE)
+            except Exception:
-                from .kwallet import VortaKWallet5Keyring, KWalletNotAvailableException
+                continue
-                try:
-                    cls._keyring = VortaKWallet5Keyring()
-                except KWalletNotAvailableException:
-                    # Try to use DBus and Gnome-Keyring (available on Linux and *BSD)
-                    # Put this last as gnome keyring is included by default on many distros
-                    import secretstorage
-                    from .secretstorage import VortaSecretStorageKeyring
 
-                    # secretstorage has two different libraries based on version
+        for keyring, _ in sorted(available_keyrings, key=lambda k: k[1], reverse=True):
-                    if parse_version(secretstorage.__version__) >= parse_version("3.0.0"):
+            try:
-                        from jeepney.wrappers import DBusErrorResponse as DBusException
+                return keyring()
-                    else:
+            except Exception:
-                        from dbus.exceptions import DBusException
+                continue
 
-                    try:
+    def get_backend_warning(self):
-                        cls._keyring = VortaSecretStorageKeyring()
+        if self.is_system:
-                    except (secretstorage.exceptions.SecretStorageException, DBusException):
+            return trans_late('utils', 'Storing password in your password manager.')
-                        # Save passwords in DB, if all else fails.
+        else:
-                        from .db import VortaDBKeyring
+            return trans_late('utils', 'Saving password with Vorta settings.')
-                        cls._keyring = VortaDBKeyring()
-        return cls._keyring
 
     def set_password(self, service, repo_url, password):
         """
@@ -58,12 +48,21 @@ class VortaKeyring:
         raise NotImplementedError
 
     @property
-    def is_primary(self):
+    def is_system(self):
         """
         Return True if the current subclass is the system's primary keychain mechanism,
         rather than a fallback (like our own VortaDBKeyring).
         """
-        return True
+        raise NotImplementedError
+
+    @classmethod
+    def get_priority(cls):
+        """
+        Return priority of this keyring on current system. Higher is more important.
+
+        Shout-out to https://github.com/jaraco/keyring for this idea.
+        """
+        raise NotImplementedError
 
     @property
     def is_unlocked(self):

src/vorta/keyring/darwin.py

@@ -7,6 +7,7 @@ objc modules.
 
 Adapted from https://gist.github.com/apettinen/5dc7bf1f6a07d148b2075725db6b1950
 """
+import sys
 from .abc import VortaKeyring
 
 
@@ -79,8 +80,20 @@ class VortaDarwinKeyring(VortaKeyring):
 
         return keychain_status & kSecUnlockStateStatus
 
+    @classmethod
+    def get_priority(cls):
+        if sys.platform == 'darwin':
+            return 8
+        else:
+            raise RuntimeError('Only available on macOS')
+
+    @property
+    def is_system(self):
+        return True
+
 
 def _resolve_password(password_length, password_buffer):
     from ctypes import c_char
     s = (c_char*password_length).from_address(password_buffer.__pointer__)[:]
     return s.decode()
+

src/vorta/keyring/db.py

@@ -1,5 +1,6 @@
 import peewee
 from .abc import VortaKeyring
+from vorta.models import SettingsModel
 
 
 class VortaDBKeyring(VortaKeyring):
@@ -27,9 +28,13 @@ class VortaDBKeyring(VortaKeyring):
             return None
 
     @property
-    def is_primary(self):
+    def is_system(self):
         return False
 
     @property
     def is_unlocked(self):
         return True
+
+    @classmethod
+    def get_priority(cls):
+        return 1 if SettingsModel.get(key='use_system_keyring').value else 10

src/vorta/keyring/kwallet.py

@@ -1,3 +1,4 @@
+import os
 from PyQt5 import QtDBus
 from PyQt5.QtCore import QVariant
 from vorta.keyring.abc import VortaKeyring
@@ -20,6 +21,7 @@ class VortaKWallet5Keyring(VortaKeyring):
             self.object_path,
             self.interface_name,
             QtDBus.QDBusConnection.sessionBus())
+        self.handle = -1
         if not (self.iface.isValid() and self.get_result("isEnabled") is True):
             raise KWalletNotAvailableException
 
@@ -54,6 +56,14 @@ class VortaKWallet5Keyring(VortaKeyring):
         except ValueError:  # For when kwallet is disabled or dbus otherwise broken
             self.handle = -2
 
+    @classmethod
+    def get_priority(cls):
+        return 6 if "KDE" in os.getenv("XDG_CURRENT_DESKTOP", "") else 4
+
+    @property
+    def is_system(self):
+        return True
+
 
 class KWalletNotAvailableException(Exception):
     pass

src/vorta/keyring/secretstorage.py

@@ -1,6 +1,6 @@
 import asyncio
 import sys
-
+import os
 import secretstorage
 
 from vorta.keyring.abc import VortaKeyring
@@ -52,3 +52,11 @@ class VortaSecretStorageKeyring(VortaKeyring):
         except secretstorage.exceptions.SecretServiceNotAvailableException:
             logger.debug('SecretStorage is closed.')
             return False
+
+    @classmethod
+    def get_priority(cls):
+        return 6 if "GNOME" in os.getenv("XDG_CURRENT_DESKTOP", "") else 5
+
+    @property
+    def is_system(self):
+        return True

src/vorta/models.py

@@ -225,6 +225,11 @@ def get_misc_settings():
             'label': trans_late('settings',
                                 'Get statistics of file/folder when added')
         },
+        {
+            'key': 'use_system_keyring', 'value': True, 'type': 'checkbox',
+            'label': trans_late('settings',
+                                'Store repository passwords in system keychain, if available.')
+        },
         {
             'key': 'override_mount_permissions', 'value': False, 'type': 'checkbox',
             'label': trans_late('settings',
@@ -274,6 +279,7 @@ def get_misc_settings():
 def cleanup_db():
     # Clean up database
     db.execute_sql("VACUUM")
+    db.close()
 
 
 def init_db(con=None):

src/vorta/utils.py

@@ -22,7 +22,6 @@ from PyQt5.QtWidgets import QApplication, QFileDialog, QSystemTrayIcon
 
 from vorta.borg._compatibility import BorgCompatibility
 from vorta.i18n import trans_late
-from vorta.keyring.abc import VortaKeyring
 from vorta.log import logger
 from vorta.network_status.abc import NetworkStatusMonitor
 
@@ -337,14 +336,3 @@ def validate_passwords(first_pass, second_pass):
         return trans_late('utils', "Passwords must be greater than 8 characters long.")
 
     return ""
-
-
-def display_password_backend(encryption):
-    ''' Display password backend message based off current keyring '''
-    # flake8: noqa E501
-    if encryption != 'none':
-        keyring = VortaKeyring.get_keyring()
-        return trans_late('utils', "Storing the password in your password manager.") if keyring.is_primary else trans_late(
-            'utils', 'Saving the password to disk. To store password more securely install a supported secret store such as KeepassXC')
-    else:
-        return ""

src/vorta/views/repo_add_dialog.py

@@ -3,7 +3,7 @@ from PyQt5 import uic, QtCore
 from PyQt5.QtWidgets import QLineEdit, QAction
 
 from vorta.utils import get_private_keys, get_asset, choose_file_dialog, \
-    borg_compat, validate_passwords, display_password_backend
+    borg_compat, validate_passwords
 from vorta.keyring.abc import VortaKeyring
 from vorta.borg.init import BorgInitThread
 from vorta.borg.info_repo import BorgInfoRepoThread
@@ -31,7 +31,7 @@ class AddRepoWindow(AddRepoBase, AddRepoUI):
         self.repoURL.textChanged.connect(self.set_password)
         self.passwordLineEdit.textChanged.connect(self.password_listener)
         self.confirmLineEdit.textChanged.connect(self.password_listener)
-        self.encryptionComboBox.activated.connect(self.display_password_backend)
+        self.encryptionComboBox.activated.connect(self.display_backend_warning)
 
         # Add clickable icon to toggle password visibility to end of box
         self.showHideAction = QAction(self.tr("Show my passwords"), self)
@@ -45,7 +45,7 @@ class AddRepoWindow(AddRepoBase, AddRepoUI):
         self.init_encryption()
         self.init_ssh_key()
         self.set_icons()
-        self.display_password_backend()
+        self.display_backend_warning()
 
     def set_icons(self):
         self.chooseLocalFolderButton.setIcon(get_colored_icon('folder-open'))
@@ -64,8 +64,10 @@ class AddRepoWindow(AddRepoBase, AddRepoUI):
             out['encryption'] = self.encryptionComboBox.currentData()
         return out
 
-    def display_password_backend(self):
+    def display_backend_warning(self):
-        self.passwordLabel.setText(translate('utils', display_password_backend(self.encryptionComboBox.currentData())))
+        '''Display password backend message based off current keyring'''
+        if self.encryptionComboBox.currentData() != 'none':
+            self.passwordLabel.setText(VortaKeyring.get_keyring().get_backend_warning())
 
     def choose_local_backup_folder(self):
         def receive():

tests/conftest.py

@@ -1,7 +1,7 @@
-import pytest
-import peewee
-import sys
 import os
+import peewee
+import pytest
+import sys
 from datetime import datetime as dt
 from unittest.mock import MagicMock
 
@@ -34,11 +34,12 @@ def qapp(tmpdir_factory):
     qapp = VortaApp([])  # Only init QApplication once to avoid segfaults while testing.
 
     yield qapp
+    mock_db.close()
     qapp.quit()
 
 
 @pytest.fixture(scope='function', autouse=True)
-def init_db(qapp, tmpdir_factory):
+def init_db(qapp, qtbot, tmpdir_factory):
     tmp_db = tmpdir_factory.mktemp('Vorta').join('settings.sqlite')
     mock_db = peewee.SqliteDatabase(str(tmp_db), pragmas={'journal_mode': 'wal', })
     vorta.models.init_db(mock_db)
@@ -61,18 +62,14 @@ def init_db(qapp, tmpdir_factory):
     source_dir = SourceFileModel(dir='/tmp/another', repo=new_repo, dir_size=100, dir_files_count=18, path_isdir=True)
     source_dir.save()
 
+    qapp.main_window.deleteLater()
+    del qapp.main_window
     qapp.main_window = MainWindow(qapp)  # Re-open main window to apply mock data in UI
 
-
+    yield
-@pytest.fixture(scope='function', autouse=True)
+    qapp.backup_cancelled_event.emit()
-def cleanup(request, qapp, qtbot):
+    qtbot.waitUntil(lambda: not vorta.borg.borg_thread.BorgThread.is_running())
-    """
+    mock_db.close()
-    Cleanup after each test
-    """
-    def ensure_borg_thread_stopped():
-        qapp.backup_cancelled_event.emit()
-        qtbot.waitUntil(lambda: not vorta.borg.borg_thread.BorgThread.is_running())
-    request.addfinalizer(ensure_borg_thread_stopped)
 
 
 @pytest.fixture