From cb43fd60a9a516fbf59df69bc1d0e84f9251da79 Mon Sep 17 00:00:00 2001 From: Ali Mirjamali Date: Wed, 30 Oct 2024 22:43:13 +0330 Subject: [PATCH] Ignore invalid GUI related features and options resolves: https://github.com/QubesOS/qubes-issues/issues/7730 --- .gitlab-ci.yml | 10 +- debian/control | 1 + qubesadmin/tests/tools/qvm_start_daemon.py | 36 ++++++- qubesadmin/tools/qvm_start_daemon.py | 104 +++++++++++++++++---- rpm_spec/qubes-core-admin-client.spec.in | 1 + 5 files changed, 128 insertions(+), 24 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 00ade0a7f..5865980e0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -7,7 +7,7 @@ include: project: QubesOS/qubes-continuous-integration checks:pylint: before_script: - - sudo dnf install -y python3-rpm + - sudo dnf install -y python3-rpm python3-xlib - pip3 install --quiet -r ci/requirements.txt script: - export PATH="$HOME/.local/bin:$PATH" @@ -19,15 +19,15 @@ checks:tests: after_script: - ci/codecov-wrapper before_script: - - sudo dnf install -y openssl python3-rpm sequoia-sqv + - sudo dnf install -y openssl python3-rpm sequoia-sqv python3-xlib libnotify + xorg-x11-server-Xvfb - pip3 install --quiet -r ci/requirements.txt - git config --global --add safe.directory "$PWD" script: - python3 setup.py build # simulate "normal" qubes env for most tests - - export DISPLAY=:0 - - ./run-tests - - python3 -m coverage xml + - xvfb-run ./run-tests + - xvfb-run python3 -m coverage xml stage: checks tags: - docker diff --git a/debian/control b/debian/control index 6e3b4d54a..fc9f59da8 100644 --- a/debian/control +++ b/debian/control @@ -28,6 +28,7 @@ Depends: qubes-repo-templates, scrypt, qubes-rpm-oxide (>= 0.2.3), + python3-xlib, ${python:Depends}, ${python3:Depends}, ${misc:Depends}, diff --git a/qubesadmin/tests/tools/qvm_start_daemon.py b/qubesadmin/tests/tools/qvm_start_daemon.py index 7c53c8ed3..e0d51b9d3 100644 --- a/qubesadmin/tests/tools/qvm_start_daemon.py +++ b/qubesadmin/tests/tools/qvm_start_daemon.py @@ -27,7 +27,9 @@ import qubesadmin.tests import qubesadmin.tools.qvm_start_daemon -from qubesadmin.tools.qvm_start_daemon import GUI_DAEMON_OPTIONS +from qubesadmin.tools.qvm_start_daemon import GUI_DAEMON_OPTIONS +from qubesadmin.tools.qvm_start_daemon import validator_key_sequence, \ + validator_trayicon_mode, validator_color import qubesadmin.vm @@ -93,7 +95,7 @@ def setup_common_args(self): ('test-vm', 'admin.vm.property.Get', 'xid', None)] = \ b'0\x00default=99 type=int 99' - for name, _kind in GUI_DAEMON_OPTIONS: + for name, _kind, _validator in GUI_DAEMON_OPTIONS: self.app.expected_calls[ ('test-vm', 'admin.vm.feature.Get', 'gui-' + name.replace('_', '-'), None)] = \ @@ -754,3 +756,33 @@ def test_070_send_monitor_layout_all(self): unittest.mock.call(vm2, monitor_layout)]) mock_get_monior_layout.assert_called_once_with() self.assertAllCalled() + + def test_071_validators(self): + self.assertFalse(validator_key_sequence(123)) + self.assertFalse(validator_key_sequence('Super-X')) + self.assertTrue(validator_key_sequence('Ctrl-shift-F12')) + self.assertFalse(validator_key_sequence('F13')) + self.assertFalse(validator_trayicon_mode(True)) + self.assertTrue(validator_trayicon_mode('bg')) + self.assertTrue(validator_trayicon_mode('tint+border1')) + self.assertFalse(validator_trayicon_mode('shadow')) + self.assertFalse(validator_color(321)) + self.assertTrue(validator_color('0x123456')) + self.assertTrue(validator_color('0XFFF')) + self.assertTrue(validator_color('lime')) + self.assertFalse(validator_color('scarlet')) + self.assertFalse(validator_color('octarine')) + + def test_072_feature_validation(self): + self.setup_common_args() + + self.app.expected_calls[ + ('gui-vm', 'admin.vm.feature.Get', + 'gui-default-override-redirect', None)] = \ + b'0\x00ask' + + _args, config = self.run_common_args() + self.assertEqual(config, '''\ +global: { +} +''') diff --git a/qubesadmin/tools/qvm_start_daemon.py b/qubesadmin/tools/qvm_start_daemon.py index c2c5394af..ae86a5320 100644 --- a/qubesadmin/tools/qvm_start_daemon.py +++ b/qubesadmin/tools/qvm_start_daemon.py @@ -39,22 +39,75 @@ import qubesadmin.vm from qubesadmin.tools import xcffibhelpers +# pylint: disable=wrong-import-position,wrong-import-order +from Xlib import X, XK +from Xlib.display import Display +from Xlib.error import DisplayConnectionError + GUI_DAEMON_PATH = '/usr/bin/qubes-guid' PACAT_DAEMON_PATH = '/usr/bin/pacat-simple-vchan' QUBES_ICON_DIR = '/usr/share/icons/hicolor/128x128/devices' +def validator_key_sequence(sequence: str) -> bool: + """ xside.c key sequence validation is not case sensitive and supports more + choices than Global Config's limited choices, so we replicate it here """ + if not isinstance(sequence, str): + return False + while '-' in sequence: + modifier, sequence = sequence.split('-', 1) + if not modifier.lower() in \ + ['shift', 'ctrl', 'alt', 'mod1', 'mod2', 'mod3', 'mod4']: + return False + try: + display = Display() + result = display.keysym_to_keycode(XK.string_to_keysym(sequence)) + display.close() + return result != X.NoSymbol + except DisplayConnectionError: + return False + +def validator_trayicon_mode(mode: str) -> bool: + """ xside.c tray mode validation is replicated here """ + if not isinstance(mode, str): + return False + if mode in ['bg', 'border1', 'border2', 'tint']: + return True + if mode.startswith('tint'): + if mode[4:] in ['+border1', '+border2', '+saturation50', '+whitehack']: + return True + return False + +def validator_color(color: str) -> bool: + """ xside.c `parse_color` validation code is replicated here """ + if not isinstance(color, str): + return False + if re.match(r"^0[xX](?:[0-9a-fA-F]{3}){1,2}$", color.strip()): + # Technically `parse_color` could parse values such as `0x0` + # Xlib.xobject.colormap conventions & standards are different. + return True + try: + display = Display() + result = display.screen().default_colormap.alloc_named_color(color) + display.close() + if result is not None: + return True + except DisplayConnectionError: + return False + return False + GUI_DAEMON_OPTIONS = [ - ('allow_fullscreen', 'bool'), - ('override_redirect_protection', 'bool'), - ('override_redirect', 'str'), - ('allow_utf8_titles', 'bool'), - ('secure_copy_sequence', 'str'), - ('secure_paste_sequence', 'str'), - ('windows_count_limit', 'int'), - ('trayicon_mode', 'str'), - ('window_background_color', 'str'), - ('startup_timeout', 'int'), - ('max_clipboard_size', 'int'), + ('allow_fullscreen', 'bool', (lambda x: isinstance(x, bool))), + ('override_redirect_protection', 'bool', (lambda x: isinstance(x, bool))), + ('override_redirect', 'str', (lambda x: x in ['allow', 'disable'])), + ('allow_utf8_titles', 'bool', (lambda x: isinstance(x, bool))), + ('secure_copy_sequence', 'str', validator_key_sequence), + ('secure_paste_sequence', 'str', validator_key_sequence), + ('windows_count_limit', 'int', (lambda x: isinstance(x, int) and x > 0)), + ('trayicon_mode', 'str', validator_trayicon_mode), + ('window_background_color', 'str', validator_color), + ('startup_timeout', 'int', (lambda x: isinstance(x, int) and x >= 0)), + ('max_clipboard_size', 'int', \ + (lambda x: isinstance(x, int) and 256 <= x <= 256000)), ] formatter = logging.Formatter( @@ -108,12 +161,12 @@ def retrieve_gui_daemon_options(vm, guivm): options = {} - for name, kind in GUI_DAEMON_OPTIONS: - feature_value = vm.features.get( - 'gui-' + name.replace('_', '-'), None) + for name, kind, validator in GUI_DAEMON_OPTIONS: + feature = 'gui-' + name.replace('_', '-') + feature_value = vm.features.get(feature, None) if feature_value is None: - feature_value = guivm.features.get( - 'gui-default-' + name.replace('_', '-'), None) + feature = 'gui-default-' + name.replace('_', '-') + feature_value = guivm.features.get(feature, None) if feature_value is None: continue @@ -126,6 +179,15 @@ def retrieve_gui_daemon_options(vm, guivm): else: assert False, kind + if not validator(value): + message = f"{vm.name}: Ignoring invalid feature:\n" \ + f"{feature}={feature_value}" + log.error(message) + if not sys.stdout.isatty(): + subprocess.run(['notify-send', '-a', 'Qubes GUI Daemon', \ + '--icon', 'dialog-warning', message], check=False) + continue + options[name] = value return options @@ -141,7 +203,7 @@ def serialize_gui_daemon_options(options): '', 'global: {', ] - for name, kind in GUI_DAEMON_OPTIONS: + for name, kind, validator in GUI_DAEMON_OPTIONS: if name in options: value = options[name] if kind == 'bool': @@ -153,6 +215,14 @@ def serialize_gui_daemon_options(options): else: assert False, kind + if not validator(value): + message = f"Ignoring invalid GUI property:\n{name}={value}" + log.error(message) + if not sys.stdout.isatty(): + subprocess.run(['notify-send', '-a', 'Qubes GUI Daemon', \ + '--icon', 'dialog-warning', message], check=False) + continue + lines.append(' {} = {};'.format(name, serialized)) lines.append('}') lines.append('') diff --git a/rpm_spec/qubes-core-admin-client.spec.in b/rpm_spec/qubes-core-admin-client.spec.in index 117f79984..df3bf39e0 100644 --- a/rpm_spec/qubes-core-admin-client.spec.in +++ b/rpm_spec/qubes-core-admin-client.spec.in @@ -42,6 +42,7 @@ Requires: python%{python3_pkgversion}-qubesdb Requires: python%{python3_pkgversion}-rpm Requires: python%{python3_pkgversion}-tqdm Requires: python%{python3_pkgversion}-pyxdg +Requires: python%{python3_pkgversion}-xlib Conflicts: qubes-core-dom0 < 4.3.0 Conflicts: qubes-manager < 4.0.6