From 0a6b33ebedb551dbd6374f86a374290f6e74fb24 Mon Sep 17 00:00:00 2001 From: deeplow Date: Thu, 21 Sep 2023 10:09:52 +0100 Subject: [PATCH] Qubes: detect qube failing to start (missing RAM) In Qubes OS it's often the case that the user doesn't have enough RAM to start the conversion. In this case it raises BrokenPipeException and exits with code 126. It didn't seem possible to distinguish this kind of failure to one where the user has misconfigured qrexec policies. NOTE: this approach is not ideal UX-wise. After the first doc failing the next one will also try and fail. Upon first failure we should inform the user that they need to close some programs or qubes. --- dangerzone/conversion/errors.py | 8 ++++ dangerzone/isolation_provider/qubes.py | 54 ++++++++++++++------------ tests/isolation_provider/test_qubes.py | 32 +++++++++++++++ 3 files changed, 70 insertions(+), 24 deletions(-) diff --git a/dangerzone/conversion/errors.py b/dangerzone/conversion/errors.py index a432e41..971f2da 100644 --- a/dangerzone/conversion/errors.py +++ b/dangerzone/conversion/errors.py @@ -24,6 +24,14 @@ class ConversionException(Exception): return subclasses +class QubesNotEnoughRAMError(ConversionException): + error_code = 126 # No ERROR_SHIFT since this is a qrexec error + error_message = ( + "Your system does not have enough RAM available to start the conversion. " + "Please close some qubes or programs and try again." + ) + + class DocFormatUnsupported(ConversionException): error_code = ERROR_SHIFT + 10 error_message = "The document format is not supported" diff --git a/dangerzone/isolation_provider/qubes.py b/dangerzone/isolation_provider/qubes.py index a762842..8c6429d 100644 --- a/dangerzone/isolation_provider/qubes.py +++ b/dangerzone/isolation_provider/qubes.py @@ -88,33 +88,13 @@ class Qubes(IsolationProvider): percentage = 0.0 with open(document.input_filename, "rb") as f: - # TODO handle lack of memory to start qube - if getattr(sys, "dangerzone_dev", False): - # Use dz.ConvertDev RPC call instead, if we are in development mode. - # Basically, the change is that we also transfer the necessary Python - # code as a zipfile, before sending the doc that the user requested. - self.proc = subprocess.Popen( - ["/usr/bin/qrexec-client-vm", "@dispvm:dz-dvm", "dz.ConvertDev"], - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - ) - assert self.proc is not None + self.proc = self.qrexec_subprocess() + try: assert self.proc.stdin is not None - - # Send the dangerzone module first. - self.teleport_dz_module(self.proc.stdin) - - # Finally, send the document, as in the normal case. self.proc.stdin.write(f.read()) self.proc.stdin.close() - else: - self.proc = subprocess.Popen( - ["/usr/bin/qrexec-client-vm", "@dispvm:dz-dvm", "dz.Convert"], - stdin=f, - stdout=subprocess.PIPE, - stderr=subprocess.DEVNULL, - ) + except BrokenPipeError as e: + raise errors.InterruptedConversion() # Get file size (in MiB) size = os.path.getsize(document.input_filename) / 1024**2 @@ -207,6 +187,32 @@ class Qubes(IsolationProvider): def get_max_parallel_conversions(self) -> int: return 1 + def qrexec_subprocess(self) -> subprocess.Popen: + dev_mode = getattr(sys, "dangerzone_dev", False) == True + if dev_mode: + # Use dz.ConvertDev RPC call instead, if we are in development mode. + # Basically, the change is that we also transfer the necessary Python + # code as a zipfile, before sending the doc that the user requested. + qrexec_policy = "dz.ConvertDev" + stderr = subprocess.PIPE + else: + qrexec_policy = "dz.Convert" + stderr = subprocess.DEVNULL + + p = subprocess.Popen( + ["/usr/bin/qrexec-client-vm", "@dispvm:dz-dvm", qrexec_policy], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=stderr, + ) + + if dev_mode: + assert p.stdin is not None + # Send the dangerzone module first. + self.teleport_dz_module(p.stdin) + + return p + def teleport_dz_module(self, wpipe: IO[bytes]) -> None: """Send the dangerzone module to another qube, as a zipfile.""" # Grab the absolute file path of the dangerzone module. diff --git a/tests/isolation_provider/test_qubes.py b/tests/isolation_provider/test_qubes.py index 45ac4d2..f8f6172 100644 --- a/tests/isolation_provider/test_qubes.py +++ b/tests/isolation_provider/test_qubes.py @@ -1,4 +1,9 @@ +import signal +import subprocess +import time + import pytest +from pytest import MonkeyPatch from pytest_mock import MockerFixture from dangerzone.conversion import errors @@ -54,3 +59,30 @@ class TestQubes(IsolationProviderTest): with pytest.raises(errors.MaxPageHeightException): success = provider._convert(Document(sample_bad_height), ocr_lang=None) assert not success + + def test_out_of_ram( + self, + provider: Qubes, + mocker: MockerFixture, + monkeypatch: MonkeyPatch, + sample_doc: str, + ) -> None: + provider.progress_callback = mocker.MagicMock() + + def qrexec_subprocess() -> subprocess.Popen: + p = subprocess.Popen( + # XXX error 126 simulates a qrexec-policy failure. Source: + # https://github.com/QubesOS/qubes-core-qrexec/blob/fdcbfd7/daemon/qrexec-daemon.c#L1022 + ["exit 126"], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + shell=True, + ) + return p + + monkeypatch.setattr(provider, "qrexec_subprocess", qrexec_subprocess) + + with pytest.raises(errors.QubesNotEnoughRAMError) as e: + doc = Document(sample_doc) + provider._convert(doc, ocr_lang=None)