This reverts commit 73b0f8b7d4.
Unfortunately, disabling DirectFS causes a problem in Linux systems that
enable Yama mode 2. Turns out that Tails is such a system, so we have to
revert this change, if we want to support it.
Refs #982
Do not close stderr as part of the Qubes termination logic, since we
need to read the debug logs. This shouldn't affect typical termination
scenarios, since we expect our disposable qube to be either busy reading
from stdin, or writing to stdout. If this is not the case, then
forcefully killing the `qrexec-client-vm` process should unblock the
qube.
Make the Dummy isolation provider follow the rest of the isolation
providers and perform the second part of the conversion on the host. The
first part of the conversion is just a dummy script that reads a file
from stdin and prints pixels to stdout.
Extend the base isolation provider to immediately convert each page to
a PDF, and optionally use OCR. In contract with the way we did things
previously, there are no more two separate stages (document to pixels,
pixels to PDF). We now handle each page individually, for two main
reasons:
1. We don't want to buffer pixel data, either on disk or in memory,
since they take a lot of space, and can potentially leave traces.
2. We can perform these operations in parallel, saving time. This is
more evident when OCR is not used, where the time to convert a page
to pixels, and then back to a PDF are comparable.
Add a new way to detect where the Tesseract data are stored in a user's
system. On Linux, the Tesseract data should be installed via the package
manager. On macOS and Windows, they should be bundled with the
Dangerzone application.
There is also the exception of running Dangerzone locally, where even
on Linux, we should get the Tesseract data from the Dangerzone share/
folder.
Make the dummy provider behave a bit more like the other providers, with
a proper function and termination logic. This will be helpful soon in
the tests.
Instead of killing just the invoked Podman/Docker/qrexec process, kill
the whole process group, to make sure that other components that have
been spawned die as well. In the case of Podman, conmon is one of the
processes that lingers, so that's one way to kill it.
Start the conversion process in a new session, so that we can later on
kill the process group, without killing the controlling script (i.e.,
the Dangezone UI). This should not affect the conversion process in any
other way.
As per Etienne Perot's comment on #908:
> Then it seems to me like it would be easy to simply apply this seccomp
profile under all container runtimes (since there's no reason why the
same image and the same command-line would call different syscalls under
different container runtimes).
Docker Desktop 4.30.0 uses the containerd image store by default, which
generates different IDs for the images, and as a result breaks the logic
we are using when verifying the images IDs are present.
Now, multiple IDs can be stored in the `image-id.txt` file.
Fixes#933
Use "podman" when on Linux, and "docker" otherwise.
This commit also adds a text widget to the interface, showing the actual
content fo the error that happened, to help debug further if needed.
Fixes#212
DirectFS is enabled by default in gVisor to improve I/O performance,
but comes at the cost of enabling the `openat(2)` syscall (with severe
restrictions, but still). As Dangerzone is not performance-sensitive,
and that it is desirable to guarantee for the document conversion
process to not open any files (to mimic some of what SELinux provides),
might as well disable it by default.
See #226.
PyMUPDF logs to stdout by default, which is problematic because we use
the stdout of the conversion process to read the pixel stream of a
document.
Make PyMuPDF always log to stderr, by setting the following environment
variables: PYMUPDF_MESSAGE and PYMUPDF_LOG.
Fixes#877
Set the `container_engine_t` SELinux on the **outer** Podman container,
so that gVisor does not break on systems where SELinux is enforcing.
This label is provided for container engines running within a container,
which fits our `runsc` within `crun` situation.
We have considered using the more permissive `label=disable` option, to
disable SELinux labels altogether, but we want to take advantage of as
many SELinux protections as we can, even for the **outer** container.
Cherry-picked from e1e63d14f8Fixes#880
Set the `container_engine_t` SELinux on the **outer** Podman container,
so that gVisor does not break on systems where SELinux is enforcing.
This label is provided for container engines running within a container,
which fits our `runsc` within `crun` situation.
We have considered using the more permissive `label=disable` option, to
disable SELinux labels altogether, but we want to take advantage of as
many SELinux protections as we can, even for the **outer** container.
Fixes#880
We have encountered several conversions where the `docker kill` command
hangs. Handle this case by specifying a timeout to this command. If the
timeout expires, log a warning and proceed with the rest of the
termination logic (i.e., kill the conversion process).
Fixes#854
We are aware that some Docker Desktop releases before 25.0.0 ship with a
seccomp policy which disables the `ptrace(2)` system call. In such
cases, we opt to use our own seccomp policy which allows this system
call. This seccomp policy is the default one in the latest releases of
Podman, and we use it in Linux distributions where Podman version is <
4.0.
Fixes#846
This wraps the existing container image inside a gVisor-based sandbox.
gVisor is an open-source OCI-compliant container runtime.
It is a userspace reimplementation of the Linux kernel in a
memory-safe language.
It works by creating a sandboxed environment in which regular Linux
applications run, but their system calls are intercepted by gVisor.
gVisor then redirects these system calls and reinterprets them in
its own kernel. This means the host Linux kernel is isolated
from the sandboxed application, thereby providing protection against
Linux container escape attacks.
It also uses `seccomp-bpf` to provide a secondary layer of defense
against container escapes. Even if its userspace kernel gets
compromised, attackers would have to additionally have a Linux
container escape vector, and that exploit would have to fit within
the restricted `seccomp-bpf` rules that gVisor adds on itself.
Fixes#126Fixes#224Fixes#225Fixes#228
Get the (major, minor) parts of the Docker/Podman version, to check if
some specific features can be used, or if we need a fallback. These
features are related with the upcoming gVisor integration, and will be
added in subsequent commits.
A few minor changes about when to use `==` and when to use `is`.
Basically, this uses `is` for booleans, and `==` for other values.
With a few other changes about coding style which was enforced by
`ruff`.
This commit removes code that's not being used, it can be exceptions
with the `as e` where the exception itself is not used, the same with
`with` statements, and some other parts where there were duplicated
code.
> f-strings are a convenient way to format strings, but they are not
> necessary if there are no placeholder expressions to format. In this
> case, a regular string should be used instead, as an f-string without
> placeholders can be confusing for readers, who may expect such a
> placeholder to be present.
>
> — [ruff docs](https://docs.astral.sh/ruff/rules/f-string-missing-placeholders/)
When building the Dangerzone RPMs, we were seeing the following shebang
warnings:
+ /usr/lib/rpm/redhat/brp-mangle-shebangs
mangling shebang in /usr/lib/python3.12/site-packages/dangerzone/conversion/doc_to_pixels.py from /usr/bin/env python3 to #!/usr/bin/python3
mangling shebang in /usr/lib/python3.12/site-packages/dangerzone/conversion/common.py from /usr/bin/env python3 to #!/usr/bin/python3
mangling shebang in /usr/lib/python3.12/site-packages/dangerzone/conversion/pixels_to_pdf.py from /usr/bin/env python3 to #!/usr/bin/python3
mangling shebang in /etc/qubes-rpc/dz.ConvertDev from /usr/bin/env python3 to #!/usr/bin/python3
mangling shebang in /etc/qubes-rpc/dz.Convert from /bin/sh to #!/usr/bin/sh
These warnings are benign in nature, but coupled with #727, they could
lead to incorrect file permissions.
Remove shebangs from the following files, since they are not executed
directly, but are imported instead:
dangerzone/conversion/common.py
dangerzone/conversion/doc_to_pixels.py
dangerzone/conversion/pixels_to_pdf.py
Also, accept the suggestions by Fedora (/bin/sh -> /usr/bin/sh,
/usr/bin/env python3 -> /usr/bin/python3) for the following files:
qubes/dz.Convert
qubes/dz.ConvertDev
Refs #727
Gracefully terminate certain conversion processes that may get stuck
when writing lots of data to stdout. Also, handle a race condition when
a conversion process terminates slightly after the associated container.
Fixes#791
We have recently [1] changed the name of the Dangerzone application to
capital-case "Dangerzone", but this breaks our PDF viewer detection
logic. Adjust our check to exclude Dangerzone from the list.
Fixes#790
[1]: See commit 3d426ed36b
In d632908a44 we improved our
`replace_control_chars()` function, by replacing every control or
invalid Unicode character with a placeholder one. This change, however,
made our debug logs harder to read, since newlines were not preserved.
There are indeed various cases in which replacing newlines is wise
(e.g., in filenames), so we should keep this behavior by default.
However, specifically for reading debug logs, we add an option to keep
newlines to improve readability, at no expense to security.
The `exit()` [1] function is not necessarily present in every Python
environment, as it's added by the `site` module. Also, this function is
"[...] useful for the interactive interpreter shell and should not be
used in programs"
For this reason, we replace all such occurrences with `sys.exit()` [2],
which is the canonical function to exit Python programs.
[1]: https://docs.python.org/3/library/constants.html#exit
[2]: https://docs.python.org/3/library/sys.html#sys.exit
On Windows, if we don't use the `startupinfo=` argument of
subprocess.Popen, then a terminal window will flash while running the
command.
Use `startupinfo=` when killing a container, as we do for every other
command.
On Windows, if we somehow attempt to archive the same document twice
(e.g, because it got archived once, and then we copy it back), we will
get an error, because Windows does not overwrite the target path, if it
already exists.
Fix this issue by always removing the previously archived version, when
performing the next archival action, and update our tests.
Currently, the app ID of the Dangerzone GUI application when running
under Wayland is `python3`, which is not very useful if one wants to
automate some action related to the Dangerzone application window (e.g.
to always start Dangerzone window in floating mode under Sway WM).
Setting the desktop filename property also sets the app ID of the
application under Wayland. According to Qt documentation[1], the
property value should be the name of the application's .desktop file but
without the extension.
Qt documentation also states:
> This property gives a precise indication of what desktop entry
> represents the application and it is needed by the windowing system to
> retrieve such information without resorting to imprecise heuristics.
Therefore I also think that setting this property is needed to display
the correct application name and icon (taken from the .desktop entry)
when running under certain windowing systems (like Wayland)
(see also #402).
Note that this property is not enough, as we've encountered systems
where setting just the desktop file name does not alter the detected
application name by the window manager. For this reason, we also use
set the application name [2] to `dangerzone`, to remove any ambiguity.
[1]: https://doc.qt.io/qt-6/qguiapplication.html#desktopFileName-prop
[2]: https://doc.qt.io/qt-6/qcoreapplication.html#applicationName-propFixes#402
On Unix systems a filename can be a sequence of bytes that is not valid
UTF-8. Python uses[1] surrogate escapes to allow to decode such
filenames to Unicode (bytes that cannot be decoded are replaced by a
surrogate; upon encoding the surrogate is converted to the original
byte).
From `click` docs[2]:
> Invalid bytes or surrogate escapes will raise an error when written
> to a stream with `errors="strict"`. This will typically happen with
> `stdout` when the locale is something like `en_GB.UTF-8`.
To fix that, we use `utils.replace_control_chars()` before printing the
filenames to `stdout` so that surrogate escapes are replaced by �.
Fixes#768
The `util.replace_control_chars()` function was overly strict, and
would replace every non-ASCII character with "_". This included both
control characters, as well as normal characters in a non-English
alphabet.
Relax these restrictions by checking each character and deciding if it's
a Unicode control character, using the `unicodedata` Python package.
With this change, emojis and non-English letters are now allowed.
Previously, we always assumed that the spawned process would quit
within 3 seconds. This was an arbitrary call, and did not work in
practice.
We can improve our standing here by doing the following:
1. Make `Popen.wait()` calls take a generous amount of time (since they
are usually on the sad path), and handle any timeout errors that they
throw. This way, a slow conversion process cleanup does not take too
much of our users time, nor is it reported as an error.
2. Always make sure that once the conversion of doc to pixels is over,
the corresponding process will finish within a reasonable amount of
time as well.
Fixes#749
Get the exit code of the spawned process for the doc-to-pixels phase,
without timing out. More specifically, if the spawned process has not
finished within a generous amount of time (hardcode to 15 seconds),
return UnexpectedConversionError, with a custom message.
This way, the happy path is not affected, and we still make our best to
learn the underlying cause of the I/O error.
Extend the IsolationProvider class with a
`terminate_doc_to_pixels_proc()` method, which must be implemented by
the Qubes/Container providers and gracefully terminate a process started
for the doc to pixels phase.
Refs #563
Set a unique name for spawned containers, based on the ID of the
provided document. This ID is not globally unique, as it has few bits of
entropy. However, since we only want to avoid collisions within a
single Dangerzone invocation, and since we can't support multiple
containers running in parallel, this ID will suffice.
Pass the Document instance that will be converted to the
`IsolationProvider.start_doc_to_pixels_proc()` method. Concrete classes
can then associate this name with the started process, so that they can
later on kill it.
Settings().set() would fail if we were trying to set a setting that did
not exist before. The reason is because before setting it would try to
get the previous value, but though direct key access, which would lead
to an exception.
PyMuPDF has some hardcoded log messages that print to stdout [1]. We don't
have a way to silence them, because they don't use the Python logging
infrastructure.
What we can do here is silence a particular call that's been creating
debug messages. For a long term solution, we have sent a PR to the
PyMuPDF team, and we will follow up there [2].
Fixes#700
[1]: https://github.com/freedomofpress/dangerzone/issues/700
[2]: https://github.com/pymupdf/PyMuPDF/pull/3137
For a while now, we didn't get logs for the second-stage conversion
when using containers. Extend the code to log any captured output from
the second stage conversion, only if we run Dangerzone via our dev
entrypoint.
Note that the Qubes isolation provider was always logging output from
the second stage of the conversion.
On Qubes the conversion in dev mode would fail when converting from a
Fedora 38 development qube via a Fedora 39 disposable qube. The reason
was that dz.ConvertDev was receiving `.pyc` files, which were compiled
for python 3.11 but running on python 3.12.
Unfortunately PyZipFile objects cannot send source python files, even
though the documentation is a little bit unclear on this [1].
Fixes#723
[1]: https://docs.python.org/3/library/zipfile.html#pyzipfile-objects
Provide a fix for an OCR bug that affected Fedora 38 templates of Qubes
OS. In that specific configuration, the PyMuPDF version accepts the
Tesseract data directory only from the `TESSDATA_PREFIX` environment
variable. Our mistake was that we were setting this environment variable
in a dev script, instead of setting it for all configurations.
In this commit, we set an attribute in the fitz.fitz module, so that
both dev scripts and end-user installations can work. This is hacky, but
it targets an old PyMuPDF release after all, so we don't expect things
to break in the long run.
Fixes#737
Accept `.svg` and `.bmp` files when browsing via the Dangerzone GUI.
Support for these extensions has already been added in the converter
code that runs in the sandbox (cd99122385)
but they were erroneously left out from the filter in the Dangerzone
main window.
Do not throw exceptions for unknown error codes. If
`get_proc_exception()` gets called from within an exception context and
raises an exception itself, then this exception will not get caught, and
it will get lost.
Prefer instead to return an exception class that we have for this
purpose, and show to the user the unknown error code of the converesion
process.
When we get an early EOF from the converter process, we should
immediately get the exit code of that process, to find out the actual
underlying error. Currently, the exception we raise masks the underlying
error.
Raise a ConverterProcException, that in turns makes our error handling
code read the exit code of the spawned process, and converts it to a
helpful error message.
Fixes#714
50% would show twice in the conversion progress due to an overlap in
conversion progress values. The doc_to_pixels would be from 0-50% and
the pixels_to_pdf from 50%-100%.
This commit makes the first part go from 0 to 49% instead.
Fixes#715
The container image does not need the TESSDATA_PREFIX env variable since
its PyMuPDF version is new enough to support `tessdata` as an argument
when calling the PyMuPDF tesseract method.
Since the progress information is now inferred on host based on the
number of pages obtained, progress-tracking variables should be removed
from the server.
Remove timeouts due to several reasons:
1. Lost purpose: after implementing the containers page streaming the
only subprocess we have left is LibreOffice. So don't have such a
big risk of commands hanging (the original reason for timeouts).
2. Little benefit: predicting execution time is generically unsolvable
computer science problem. Ultimately we were guessing an arbitrary
time based on the number of pages and the document size. As a guess
we made it pretty lax (30s per page or MB). A document hanging for
this long will probably lead to user frustration in any case and the
user may be compelled to abort the conversion.
3. Technical Challenges with non-blocking timeout: there have been
several technical challenges in keeping timeouts that we've made effort
to accommodate. A significant one was having to do non-blocking read to
ensure we could timeout when reading conversion stream (and then used
here)
Fixes#687
This reverts commit fea193e935.
This is part of the purge of timeout-related code since we no longer
need it [1]. Non-blocking reads were introduced in the reverted commit
in order to be able to cut a stream mid-way due to a timeout. This is
no longer needed now that we're getting rid of timeouts.
[1]: https://github.com/freedomofpress/dangerzone/issues/687
If we increased the number of parallel conversions, we'd run into an
issue where the streams were getting mixed together. This was because
the Converter.proc was a single attribute. This breaks it down into a
local variable such that this mixup doesn't happen.
Conversions methods had changed and that was part of the reason why
the tests were failing. Furthermore, due to the `provider.proc`, which
stores the associated qrexec / container process, "server" exceptions
raise a IterruptedConversion error (now ConverterProcException), which
then requires interpretation of the process exit code to obtain the
"real" exception.
Now that only the second container can send JSON-encoded progress
information, we can the untrusted JSON parsing. The parse_progress was
also renamed to `parse_progress_trusted` to ensure future developers
don't mistake this as a safe method.
The old methods for sending untrusted JSON were repurposed to send the
progress instead to stderr for troubleshooting in development mode.
Fixes#456
If one converted more than one document, since the state of
IsolationProvider.percentage would be stored in the IsolationProvider
instance, it would get reused for the second document. The fix is to
keep it as a local variable, but we can explore having progress stored
on the document itself, for example. Or having one IsolationProvider per
conversion.
Merge Qubes and Containers isolation providers core code into the class
parent IsolationProviders abstract class.
This is done by streaming pages in containers for exclusively in first
conversion process. The commit is rather large due to the multiple
interdependencies of the code, making it difficult to split into various
commits.
The main conversion method (_convert) now in the superclass simply calls
two methods:
- doc_to_pixels()
- pixels_to_pdf()
Critically, doc_to_pixels is implemented in the superclass, diverging
only in a specialized method called "start_doc_to_pixels_proc()". This
method obtains the process responsible that communicates with the
isolation provider (container / disp VM) via `podman/docker` and qrexec
on Containers and Qubes respectively.
Known regressions:
- progress reports stopped working on containers
Fixes#443
Some tests [1] lead to the conclusion that ocr_compression does the same
to the file (performance and size-wise) to the file as deflating images
when saving the file. However, both methods active do add a bit of extra
time. For this reason we're disabling the image deflation (default
option).
[1]: https://github.com/freedomofpress/dangerzone/pull/622#discussion_r1434042296
Qubes does on-host pixels-to-pdf whereas the containers version doesn't.
This leads to an issue where on the containers version it tries to load
fitz, which isn't installed there, just because it's trying to check if
it should run the Qubes version.
The error it was showing was something like this:
ImportError while loading conftest '/home/user/dangerzone/tests/conftest.py'.
tests/__init__.py:8: in <module>
from dangerzone.document import SAFE_EXTENSION
dangerzone/__init__.py:16: in <module>
from .gui import gui_main as main
dangerzone/gui/__init__.py:28: in <module>
from ..isolation_provider.qubes import Qubes, is_qubes_native_conversion
dangerzone/isolation_provider/qubes.py:15: in <module>
from ..conversion.pixels_to_pdf import PixelsToPDF
dangerzone/conversion/pixels_to_pdf.py:16: in <module>
import fitz
E ModuleNotFoundError: No module named 'fitz'
For context see discussion in [1].
[1]: https://github.com/freedomofpress/dangerzone/pull/622#issuecomment-1839164885
The original document was larger in dimensions than the original one due
to a mismatch in DPI settings. When converting documents to pixels we
were setting the DPI to 150 pixels per inch. Then when converting back
into a PDF we were using 70 DPI. This difference would result in an
overall larger document in dimensions (though not necessarily in file
size).
Fixes#626
