From 6d8ad901fb5966f86330a5bc030d7c5147f88753 Mon Sep 17 00:00:00 2001 From: Manu <3916435+m3nu@users.noreply.github.com> Date: Mon, 1 Mar 2021 15:25:31 +0800 Subject: [PATCH] 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 | 2 +- src/vorta/application.py | 6 ++- src/vorta/borg/borg_thread.py | 26 +++++------ src/vorta/keyring/abc.py | 75 +++++++++++++++--------------- src/vorta/keyring/darwin.py | 13 ++++++ src/vorta/keyring/db.py | 7 ++- src/vorta/keyring/kwallet.py | 10 ++++ src/vorta/keyring/secretstorage.py | 10 +++- src/vorta/models.py | 6 +++ src/vorta/utils.py | 12 ----- src/vorta/views/repo_add_dialog.py | 12 +++-- tests/conftest.py | 25 +++++----- 12 files changed, 117 insertions(+), 87 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b4adf1db..67d4fd72 100644 --- a/.github/workflows/test.yml +++ b/.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 diff --git a/src/vorta/application.py b/src/vorta/application.py index 04f9cbe4..34228030 100644 --- a/src/vorta/application.py +++ b/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): diff --git a/src/vorta/borg/borg_thread.py b/src/vorta/borg/borg_thread.py index 3cb9f924..6810e058 100644 --- a/src/vorta/borg/borg_thread.py +++ b/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(): - mutex.unlock() - return False - else: - return True + return mutex.locked() @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() - mutex.unlock() - self.terminate() + os.killpg(os.getpgid(self.process.pid), signal.SIGTERM) + self.quit() + self.wait() + if mutex.locked(): + mutex.release() def process_result(self, result): pass diff --git a/src/vorta/keyring/abc.py b/src/vorta/keyring/abc.py index 4ee1528b..7e545e3d 100644 --- a/src/vorta/keyring/abc.py +++ b/src/vorta/keyring/abc.py @@ -1,49 +1,39 @@ -""" -Set the most appropriate Keyring backend for the current system. -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 +import importlib +from vorta.i18n import trans_late 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. - Once it finds a secure keyring, it wil always use that keyring + Choose available Keyring. First assign a score and then try to initialize it. """ - if cls._keyring is None or not cls._keyring.is_primary: - if sys.platform == 'darwin': # Use Keychain on macOS - from .darwin import VortaDarwinKeyring - cls._keyring = VortaDarwinKeyring() - else: - # Try to use KWallet (KDE) - from .kwallet import VortaKWallet5Keyring, KWalletNotAvailableException - 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 + available_keyrings = [] + for _module, _class in cls.all_keyrings: + try: + keyring = getattr(importlib.import_module(_module, package='vorta.keyring'), _class) + available_keyrings.append((keyring, keyring.get_priority())) + except Exception: + continue - # secretstorage has two different libraries based on version - if parse_version(secretstorage.__version__) >= parse_version("3.0.0"): - from jeepney.wrappers import DBusErrorResponse as DBusException - else: - from dbus.exceptions import DBusException + for keyring, _ in sorted(available_keyrings, key=lambda k: k[1], reverse=True): + try: + return keyring() + except Exception: + continue - try: - cls._keyring = VortaSecretStorageKeyring() - except (secretstorage.exceptions.SecretStorageException, DBusException): - # Save passwords in DB, if all else fails. - from .db import VortaDBKeyring - cls._keyring = VortaDBKeyring() - return cls._keyring + def get_backend_warning(self): + if self.is_system: + return trans_late('utils', 'Storing password in your password manager.') + else: + return trans_late('utils', 'Saving password with Vorta settings.') 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): diff --git a/src/vorta/keyring/darwin.py b/src/vorta/keyring/darwin.py index d5ccac7f..36a14f2e 100644 --- a/src/vorta/keyring/darwin.py +++ b/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() + diff --git a/src/vorta/keyring/db.py b/src/vorta/keyring/db.py index fc8b0af7..b32502f3 100644 --- a/src/vorta/keyring/db.py +++ b/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 diff --git a/src/vorta/keyring/kwallet.py b/src/vorta/keyring/kwallet.py index 60048ae6..80701b21 100644 --- a/src/vorta/keyring/kwallet.py +++ b/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 diff --git a/src/vorta/keyring/secretstorage.py b/src/vorta/keyring/secretstorage.py index 5761fd8f..27aabf61 100644 --- a/src/vorta/keyring/secretstorage.py +++ b/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 diff --git a/src/vorta/models.py b/src/vorta/models.py index 9ad1db73..12d7bb2d 100644 --- a/src/vorta/models.py +++ b/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): diff --git a/src/vorta/utils.py b/src/vorta/utils.py index 33b7754b..8c3c820f 100644 --- a/src/vorta/utils.py +++ b/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 "" diff --git a/src/vorta/views/repo_add_dialog.py b/src/vorta/views/repo_add_dialog.py index 4c97acaa..8da48350 100644 --- a/src/vorta/views/repo_add_dialog.py +++ b/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): - self.passwordLabel.setText(translate('utils', display_password_backend(self.encryptionComboBox.currentData()))) + def display_backend_warning(self): + '''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(): diff --git a/tests/conftest.py b/tests/conftest.py index afe950e8..43b50505 100644 --- a/tests/conftest.py +++ b/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 - -@pytest.fixture(scope='function', autouse=True) -def cleanup(request, qapp, qtbot): - """ - 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) + yield + qapp.backup_cancelled_event.emit() + qtbot.waitUntil(lambda: not vorta.borg.borg_thread.BorgThread.is_running()) + mock_db.close() @pytest.fixture