From 5ac1288277577a1b644fe9836a3db520c7b1fd39 Mon Sep 17 00:00:00 2001 From: Alex Pyrgiotis Date: Mon, 2 Dec 2024 18:12:16 +0200 Subject: [PATCH 1/6] Move container-specific method from base class Move the `is_runtime_available()` method from the base `IsolationProvider` class, and into the `Dummy` provider class. This method was originally defined in the base class, in order to be mocked in our tests for the `Dummy` provider. There's no reason for the `Qubes` class to have it though, so we can just move it to the `Dummy` provider. --- dangerzone/gui/main_window.py | 1 + dangerzone/isolation_provider/base.py | 4 ---- dangerzone/isolation_provider/dummy.py | 4 ++++ tests/gui/test_main_window.py | 7 ++++--- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/dangerzone/gui/main_window.py b/dangerzone/gui/main_window.py index e292ff0..788ad6a 100644 --- a/dangerzone/gui/main_window.py +++ b/dangerzone/gui/main_window.py @@ -500,6 +500,7 @@ class WaitingWidgetContainer(WaitingWidget): error: Optional[str] = None try: + assert isinstance(self.dangerzone.isolation_provider, (Dummy, Container)) self.dangerzone.isolation_provider.is_runtime_available() except NoContainerTechException as e: log.error(str(e)) diff --git a/dangerzone/isolation_provider/base.py b/dangerzone/isolation_provider/base.py index 6a55a20..9404cee 100644 --- a/dangerzone/isolation_provider/base.py +++ b/dangerzone/isolation_provider/base.py @@ -93,10 +93,6 @@ class IsolationProvider(ABC): else: self.proc_stderr = subprocess.DEVNULL - @staticmethod - def is_runtime_available() -> bool: - return True - @abstractmethod def install(self) -> bool: pass diff --git a/dangerzone/isolation_provider/dummy.py b/dangerzone/isolation_provider/dummy.py index 9ebc345..b8e3b87 100644 --- a/dangerzone/isolation_provider/dummy.py +++ b/dangerzone/isolation_provider/dummy.py @@ -39,6 +39,10 @@ class Dummy(IsolationProvider): def install(self) -> bool: return True + @staticmethod + def is_runtime_available() -> bool: + return True + def start_doc_to_pixels_proc(self, document: Document) -> subprocess.Popen: cmd = [ sys.executable, diff --git a/tests/gui/test_main_window.py b/tests/gui/test_main_window.py index ff45075..7e96d22 100644 --- a/tests/gui/test_main_window.py +++ b/tests/gui/test_main_window.py @@ -30,6 +30,7 @@ from dangerzone.isolation_provider.container import ( NoContainerTechException, NotAvailableContainerTechException, ) +from dangerzone.isolation_provider.dummy import Dummy from .test_updater import assert_report_equal, default_updater_settings @@ -510,9 +511,9 @@ def test_not_available_container_tech_exception( ) -> None: # Setup mock_app = mocker.MagicMock() - dummy = mocker.MagicMock() - - dummy.is_runtime_available.side_effect = NotAvailableContainerTechException( + dummy = Dummy() + fn = mocker.patch.object(dummy, "is_runtime_available") + fn.side_effect = NotAvailableContainerTechException( "podman", "podman image ls logs" ) From 587cb6d40e541bcf18c24f025f2a729fa8332320 Mon Sep 17 00:00:00 2001 From: Alex Pyrgiotis Date: Mon, 2 Dec 2024 16:12:56 +0200 Subject: [PATCH 2/6] container: Manipulate Dangerzone image tags Add the following methods that allow the `Container` isolation provider to work with tags for the Dangerzone image: * `list_image_tag()` * `delete_image_tag()` * `add_image_tag()` --- dangerzone/isolation_provider/container.py | 70 +++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/dangerzone/isolation_provider/container.py b/dangerzone/isolation_provider/container.py index 94f894d..3f855e0 100644 --- a/dangerzone/isolation_provider/container.py +++ b/dangerzone/isolation_provider/container.py @@ -1,11 +1,12 @@ import gzip +import json import logging import os import platform import shlex import shutil import subprocess -from typing import List, Tuple +from typing import Dict, List, Tuple from ..document import Document from ..util import get_resource_path, get_subprocess_startupinfo @@ -154,6 +155,73 @@ class Container(IsolationProvider): return security_args + @staticmethod + def list_image_tags() -> Dict[str, str]: + """Get the tags of all loaded Dangerzone images. + + This method returns a mapping of image tags to image IDs, for all Dangerzone + images. This can be useful when we want to find which are the local image tags, + and which image ID does the "latest" tag point to. + """ + images = json.loads( + subprocess.check_output( + [ + Container.get_runtime(), + "image", + "list", + "--format", + "json", + Container.CONTAINER_NAME, + ], + text=True, + startupinfo=get_subprocess_startupinfo(), + ) + ) + + # Grab every image name and associate it with an image ID. + tags = {} + for image in images: + for name in image["Names"]: + tag = name.split(":")[1] + tags[tag] = image["Id"] + + return tags + + @staticmethod + def delete_image_tag(tag: str) -> None: + """Delete a Dangerzone image tag.""" + name = Container.CONTAINER_NAME + ":" + tag + log.warning(f"Deleting old container image: {name}") + try: + subprocess.check_output( + [Container.get_runtime(), "rmi", "--force", name], + startupinfo=get_subprocess_startupinfo(), + ) + except Exception as e: + log.warning( + f"Couldn't delete old container image '{name}', so leaving it there." + f" Original error: {e}" + ) + + @staticmethod + def add_image_tag(cur_tag: str, new_tag: str) -> None: + """Add a tag to an existing Dangerzone image.""" + cur_image_name = Container.CONTAINER_NAME + ":" + cur_tag + new_image_name = Container.CONTAINER_NAME + ":" + new_tag + subprocess.check_output( + [ + Container.get_runtime(), + "tag", + cur_image_name, + new_image_name, + ], + startupinfo=get_subprocess_startupinfo(), + ) + + log.info( + f"Successfully tagged container image '{cur_image_name}' as '{new_image_name}'" + ) + @staticmethod def install() -> bool: """ From eb560b8e7f859ed372749f2a2bac47bca87d1d72 Mon Sep 17 00:00:00 2001 From: Alex Pyrgiotis Date: Mon, 2 Dec 2024 16:30:43 +0200 Subject: [PATCH 3/6] container: Factor out loading an image tarball --- dangerzone/isolation_provider/container.py | 26 +++++++++++++++------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/dangerzone/isolation_provider/container.py b/dangerzone/isolation_provider/container.py index 3f855e0..0954a53 100644 --- a/dangerzone/isolation_provider/container.py +++ b/dangerzone/isolation_provider/container.py @@ -223,16 +223,14 @@ class Container(IsolationProvider): ) @staticmethod - def install() -> bool: - """ - Make sure the podman container is installed. Linux only. - """ - if Container.is_container_installed(): - return True + def get_expected_tag() -> str: + """Get the tag of the Dangerzone image tarball from the image-id.txt file.""" + with open(get_resource_path("image-id.txt")) as f: + return f.read.strip() - # Load the container into podman + @staticmethod + def load_image_tarball() -> None: log.info("Installing Dangerzone container image...") - p = subprocess.Popen( [Container.get_runtime(), "load"], stdin=subprocess.PIPE, @@ -259,6 +257,18 @@ class Container(IsolationProvider): f"Could not install container image: {error}" ) + log.info("Successfully installed container image from") + + @staticmethod + def install() -> bool: + """ + Make sure the podman container is installed. Linux only. + """ + if Container.is_container_installed(): + return True + + Container.load_image_tarball() + if not Container.is_container_installed(raise_on_error=True): return False From 8793871ffb1f4a3619f3d788390b8f52d9f13b90 Mon Sep 17 00:00:00 2001 From: Alex Pyrgiotis Date: Mon, 2 Dec 2024 16:51:26 +0200 Subject: [PATCH 4/6] Build and tag Dangerzone images Build Dangerzone images and tag them with a unique ID that stems from the Git reop. Note that using tags as image IDs instead of regular image IDs breaks the current Dangerzone expectations, but this will be addressed in subsequent commits. --- .github/workflows/build.yml | 2 ++ .github/workflows/ci.yml | 4 ++- .github/workflows/scan.yml | 2 ++ install/common/build-image.py | 52 ++++++++++++++++++++++------------- 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 92440e7..1a270a8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -74,6 +74,8 @@ jobs: runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Get current date id: date diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5c72dd1..e694f31 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,6 +48,8 @@ jobs: runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Get current date id: date @@ -245,7 +247,7 @@ jobs: install-deb: name: "install-deb (${{ matrix.distro }} ${{ matrix.version }})" runs-on: ubuntu-latest - needs: + needs: - build-deb strategy: matrix: diff --git a/.github/workflows/scan.yml b/.github/workflows/scan.yml index d9f397b..3080476 100644 --- a/.github/workflows/scan.yml +++ b/.github/workflows/scan.yml @@ -14,6 +14,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Install container build dependencies run: sudo apt install pipx && pipx install poetry - name: Build container image diff --git a/install/common/build-image.py b/install/common/build-image.py index 9f2dcc8..921a520 100644 --- a/install/common/build-image.py +++ b/install/common/build-image.py @@ -2,12 +2,13 @@ import argparse import gzip import os import platform +import secrets import subprocess import sys from pathlib import Path BUILD_CONTEXT = "dangerzone/" -TAG = "dangerzone.rocks/dangerzone:latest" +IMAGE_NAME = "dangerzone.rocks/dangerzone" REQUIREMENTS_TXT = "container-pip-requirements.txt" if platform.system() in ["Darwin", "Windows"]: CONTAINER_RUNTIME = "docker" @@ -44,8 +45,31 @@ def main(): ) args = parser.parse_args() + tarball_path = Path("share") / "container.tar.gz" + image_id_path = Path("share") / "image-id.txt" + print(f"Building for architecture '{ARCH}'") + # Designate a unique tag for this image, depending on the Git commit it was created + # from: + # 1. If created from a Git tag (e.g., 0.8.0), the image tag will be `0.8.0`. + # 2. If created from a commit, it will be something like `0.8.0-31-g6bdaa7a`. + # 3. If the contents of the Git repo are dirty, we will append a unique identifier + # for this run, something like `0.8.0-31-g6bdaa7a-fdcb` or `0.8.0-fdcb`. + dirty_ident = secrets.token_hex(2) + tag = ( + subprocess.check_output( + ["git", "describe", "--first-parent", f"--dirty=-{dirty_ident}"], + ) + .decode() + .strip()[1:] # remove the "v" prefix of the tag. + ) + image_name_tagged = IMAGE_NAME + ":" + tag + + print(f"Will tag the container image as '{image_name_tagged}'") + with open(image_id_path, "w") as f: + f.write(tag) + print("Exporting container pip dependencies") with ContainerPipDependencies(): if not args.use_cache: @@ -59,8 +83,11 @@ def main(): check=True, ) + # Build the container image, and tag it with two tags; the one we calculated + # above, and the "latest" tag. print("Building container image") cache_args = [] if args.use_cache else ["--no-cache"] + image_name_latest = IMAGE_NAME + ":latest" subprocess.run( [ args.runtime, @@ -74,7 +101,9 @@ def main(): "-f", "Dockerfile", "--tag", - TAG, + image_name_latest, + "--tag", + image_name_tagged, ], check=True, ) @@ -85,7 +114,7 @@ def main(): [ CONTAINER_RUNTIME, "save", - TAG, + image_name_tagged, ], stdout=subprocess.PIPE, ) @@ -93,7 +122,7 @@ def main(): print("Compressing container image") chunk_size = 4 << 20 with gzip.open( - "share/container.tar.gz", + tarball_path, "wb", compresslevel=args.compress_level, ) as gzip_f: @@ -105,21 +134,6 @@ def main(): break cmd.wait(5) - print("Looking up the image id") - image_id = subprocess.check_output( - [ - args.runtime, - "image", - "list", - "--format", - "{{.ID}}", - TAG, - ], - text=True, - ) - with open("share/image-id.txt", "w") as f: - f.write(image_id) - class ContainerPipDependencies: """Generates PIP dependencies within container""" From ab6d9ba1fd9e2dd94dbac5e730d1f7e6b2116546 Mon Sep 17 00:00:00 2001 From: Alex Pyrgiotis Date: Mon, 2 Dec 2024 17:05:38 +0200 Subject: [PATCH 5/6] container: Revamp container image installation Revamp the container image installation process in a way that does not involve using image IDs. We don't want to rely on image IDs anymore, since they are brittle (see https://github.com/freedomofpress/dangerzone/issues/933). Instead, we use image tags, as provided in the `image-id.txt` file. This allows us to check fast if an image is up to date, and we no longer need to maintain multiple image IDs from various container runtimes. Refs #933 Refs #988 Fixes #1020 --- dangerzone/isolation_provider/container.py | 98 +++++++++------------- tests/isolation_provider/test_container.py | 6 +- 2 files changed, 43 insertions(+), 61 deletions(-) diff --git a/dangerzone/isolation_provider/container.py b/dangerzone/isolation_provider/container.py index 0954a53..36376de 100644 --- a/dangerzone/isolation_provider/container.py +++ b/dangerzone/isolation_provider/container.py @@ -226,7 +226,7 @@ class Container(IsolationProvider): def get_expected_tag() -> str: """Get the tag of the Dangerzone image tarball from the image-id.txt file.""" with open(get_resource_path("image-id.txt")) as f: - return f.read.strip() + return f.read().strip() @staticmethod def load_image_tarball() -> None: @@ -261,18 +261,50 @@ class Container(IsolationProvider): @staticmethod def install() -> bool: + """Install the container image tarball, or verify that it's already installed. + + Perform the following actions: + 1. Get the tags of any locally available images that match Dangerzone's image + name. + 2. Get the expected image tag from the image-id.txt file. + - If this tag is present in the local images, and that image is also tagged + as "latest", then we can return. + - Else, prune the older container images and continue. + 3. Load the image tarball and make sure it matches the expected tag. + 4. Tag that image as "latest", and mark the installation as finished. """ - Make sure the podman container is installed. Linux only. - """ - if Container.is_container_installed(): + old_tags = Container.list_image_tags() + expected_tag = Container.get_expected_tag() + + if expected_tag not in old_tags: + # Prune older container images. + log.info( + f"Could not find a Dangerzone container image with tag '{expected_tag}'" + ) + for tag in old_tags.keys(): + Container.delete_image_tag(tag) + elif old_tags[expected_tag] != old_tags.get("latest"): + log.info(f"The expected tag '{expected_tag}' is not the latest one") + Container.add_image_tag(expected_tag, "latest") + return True + else: return True + # Load the image tarball into the container runtime. Container.load_image_tarball() - if not Container.is_container_installed(raise_on_error=True): - return False + # Check that the container image has the expected image tag. + # See https://github.com/freedomofpress/dangerzone/issues/988 for an example + # where this was not the case. + new_tags = Container.list_image_tags() + if expected_tag not in new_tags: + raise ImageNotPresentException( + f"Could not find expected tag '{expected_tag}' after loading the" + " container image tarball" + ) - log.info("Container image installed") + # Mark the expected tag as "latest". + Container.add_image_tag(expected_tag, "latest") return True @staticmethod @@ -291,58 +323,6 @@ class Container(IsolationProvider): raise NotAvailableContainerTechException(runtime_name, stderr.decode()) return True - @staticmethod - def is_container_installed(raise_on_error: bool = False) -> bool: - """ - See if the container is installed. - """ - # Get the image id - with open(get_resource_path("image-id.txt")) as f: - expected_image_ids = f.read().strip().split() - - # See if this image is already installed - installed = False - found_image_id = subprocess.check_output( - [ - Container.get_runtime(), - "image", - "list", - "--format", - "{{.ID}}", - Container.CONTAINER_NAME, - ], - text=True, - startupinfo=get_subprocess_startupinfo(), - ) - found_image_id = found_image_id.strip() - - if found_image_id in expected_image_ids: - installed = True - elif found_image_id == "": - if raise_on_error: - raise ImageNotPresentException( - "Image is not listed after installation. Bailing out." - ) - else: - msg = ( - f"{Container.CONTAINER_NAME} images found, but IDs do not match." - f" Found: {found_image_id}, Expected: {','.join(expected_image_ids)}" - ) - if raise_on_error: - raise ImageNotPresentException(msg) - log.info(msg) - log.info("Deleting old dangerzone container image") - - try: - subprocess.check_output( - [Container.get_runtime(), "rmi", "--force", found_image_id], - startupinfo=get_subprocess_startupinfo(), - ) - except Exception: - log.warning("Couldn't delete old container image, so leaving it there") - - return installed - def doc_to_pixels_container_name(self, document: Document) -> str: """Unique container name for the doc-to-pixels phase.""" return f"dangerzone-doc-to-pixels-{document.id}" diff --git a/tests/isolation_provider/test_container.py b/tests/isolation_provider/test_container.py index 3fb3243..a1a844d 100644 --- a/tests/isolation_provider/test_container.py +++ b/tests/isolation_provider/test_container.py @@ -69,10 +69,11 @@ class TestContainer(IsolationProviderTest): "image", "list", "--format", - "{{.ID}}", + "json", "dangerzone.rocks/dangerzone", ], occurrences=2, + stdout="{}", ) # Make podman load fail @@ -102,10 +103,11 @@ class TestContainer(IsolationProviderTest): "image", "list", "--format", - "{{.ID}}", + "json", "dangerzone.rocks/dangerzone", ], occurrences=2, + stdout="{}", ) # Patch gzip.open and podman load so that it works From 57973b8a2eb17c773318e2aed88a0077765fbd7a Mon Sep 17 00:00:00 2001 From: Alex Pyrgiotis Date: Mon, 2 Dec 2024 18:38:33 +0200 Subject: [PATCH 6/6] Update our release instructions --- RELEASE.md | 11 +++++------ dev_scripts/qa.py | 6 ++++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 4d47d46..0deaab0 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -141,9 +141,10 @@ Close the Dangerzone application and get the container image for that version. For example: ``` -$ docker images dangerzone.rocks/dangerzone:latest +$ docker images dangerzone.rocks/dangerzone REPOSITORY TAG IMAGE ID CREATED SIZE dangerzone.rocks/dangerzone latest +dangerzone.rocks/dangerzone ``` Then run the version under QA and ensure that the settings remain changed. @@ -152,9 +153,10 @@ Afterwards check that new docker image was installed by running the same command and seeing the following differences: ``` -$ docker images dangerzone.rocks/dangerzone:latest +$ docker images dangerzone.rocks/dangerzone REPOSITORY TAG IMAGE ID CREATED SIZE dangerzone.rocks/dangerzone latest +dangerzone.rocks/dangerzone ``` #### 4. Dangerzone successfully installs the container image @@ -280,7 +282,7 @@ Once we are confident that the release will be out shortly, and doesn't need any ``` Then copy the `share/container.tar.gz` to the assets folder on `dangerzone-$VERSION-arm64.tar.gz`, along with the `share/image-id.txt` file. - [ ] Run `poetry run ./install/macos/build-app.py`; this will make `dist/Dangerzone.app` -- [ ] Make sure that the build application works with the containerd graph +- [ ] Make sure that the built application works with the containerd graph driver (see [#933](https://github.com/freedomofpress/dangerzone/issues/933)) - [ ] Run `poetry run ./install/macos/build-app.py --only-codesign`; this will make `dist/Dangerzone.dmg` * You need to run this command as the account that has access to the code signing certificate @@ -326,9 +328,6 @@ The Windows release is performed in a Windows 11 virtual machine as opposed to a - [ ] Copy the container image into the VM > [!IMPORTANT] > Instead of running `python .\install\windows\build-image.py` in the VM, run the build image script on the host (making sure to build for `linux/amd64`). Copy `share/container.tar.gz` and `share/image-id.txt` from the host into the `share` folder in the VM. - > Also, don't forget to add the supplementary image ID (see - > [#933](https://github.com/freedomofpress/dangerzone/issues/933)) in - > `share/image-id.txt`) - [ ] Run `poetry run .\install\windows\build-app.bat` - [ ] When you're done you will have `dist\Dangerzone.msi` diff --git a/dev_scripts/qa.py b/dev_scripts/qa.py index 8c99a76..54bc676 100755 --- a/dev_scripts/qa.py +++ b/dev_scripts/qa.py @@ -108,9 +108,10 @@ Close the Dangerzone application and get the container image for that version. For example: ``` -$ docker images dangerzone.rocks/dangerzone:latest +$ docker images dangerzone.rocks/dangerzone REPOSITORY TAG IMAGE ID CREATED SIZE dangerzone.rocks/dangerzone latest +dangerzone.rocks/dangerzone ``` Then run the version under QA and ensure that the settings remain changed. @@ -119,9 +120,10 @@ Afterwards check that new docker image was installed by running the same command and seeing the following differences: ``` -$ docker images dangerzone.rocks/dangerzone:latest +$ docker images dangerzone.rocks/dangerzone REPOSITORY TAG IMAGE ID CREATED SIZE dangerzone.rocks/dangerzone latest +dangerzone.rocks/dangerzone ``` #### 4. Dangerzone successfully installs the container image