FIXUP: Use the digest when pulling the container

dangerzone/container_utils.py

@@ -228,13 +228,14 @@ def get_image_id_by_digest(digest: str) -> str:
     return process.stdout.decode().strip().split("\n")[0]
 
 
-def container_pull(image: str) -> bool:
+def container_pull(image: str, manifest_digest: str):
     """Pull a container image from a registry."""
     runtime = Runtime()
-    cmd = [str(runtime.path), "pull", f"{image}"]
+    cmd = [str(runtime.path), "pull", f"{image}@sha256:{manifest_digest}"]
     process = subprocess.Popen(cmd, stdout=subprocess.PIPE)
     process.communicate()
-    return process.returncode == 0
+    if process.returncode != 0:
+        raise errors.ContainerPullException(f"Could not pull the container image: {e}")
 
 
 def get_local_image_digest(image: str) -> str:

dangerzone/errors.py

@@ -122,25 +122,37 @@ def handle_document_errors(func: F) -> F:
 #### Container-related errors
 
 
-class ImageNotPresentException(Exception):
+class ContainerException(Exception):
     pass
 
 
-class ImageInstallationException(Exception):
+class ImageNotPresentException(ContainerException):
     pass
 
 
-class NoContainerTechException(Exception):
+class MultipleImagesFoundException(ContainerException):
+    pass
+
+
+class ImageInstallationException(ContainerException):
+    pass
+
+
+class NoContainerTechException(ContainerException):
     def __init__(self, container_tech: str) -> None:
         super().__init__(f"{container_tech} is not installed")
 
 
-class NotAvailableContainerTechException(Exception):
+class NotAvailableContainerTechException(ContainerException):
     def __init__(self, container_tech: str, error: str) -> None:
         self.error = error
         self.container_tech = container_tech
         super().__init__(f"{container_tech} is not available")
 
 
-class UnsupportedContainerRuntime(Exception):
+class UnsupportedContainerRuntime(ContainerException):
+    pass
+
+
+class ContainerPullException(ContainerException):
     pass

dangerzone/updater/cli.py

@@ -38,6 +38,9 @@ def upgrade(image: str, pubkey: str) -> None:
     except errors.ImageAlreadyUpToDate as e:
         click.echo(f"✅ {e}")
         raise click.Abort()
+    except Exception as e:
+        click.echo(f"❌ {e}")
+        raise click.Abort()
 
 
 @main.command()

dangerzone/updater/signatures.py

@@ -455,7 +455,7 @@ def prepare_airgapped_archive(image_name: str, destination: str) -> None:
             archive.add(tmpdir, arcname=".")
 
 
-def upgrade_container_image(image: str, manifest_digest: str, pubkey: str) -> bool:
+def upgrade_container_image(image: str, manifest_digest: str, pubkey: str) -> str:
     """Verify and upgrade the image to the latest, if signed."""
     update_available, _ = is_update_available(image)
     if not update_available:
@@ -464,20 +464,17 @@ def upgrade_container_image(image: str, manifest_digest: str, pubkey: str) -> bo
     signatures = get_remote_signatures(image, manifest_digest)
     verify_signatures(signatures, manifest_digest, pubkey)
 
-    # Ensure that we only upgrade if the log index is higher than the last known one
+    # Only upgrade if the log index is higher than the last known one
     incoming_log_index = get_log_index_from_signatures(signatures)
     last_log_index = get_last_log_index()
 
     if incoming_log_index < last_log_index:
         raise errors.InvalidLogIndex(
-            "The log index is not higher than the last known one"
+            "Trying to upgrade to an image with a lower log index"
         )
 
-    # let's upgrade the image
-    # XXX Use the image digest here to avoid race conditions
-    upgraded = runtime.container_pull(image)
+    runtime.container_pull(image, manifest_digest)
 
-    # At this point, the signatures are verified
-    # We store the signatures just now to avoid storing unverified signatures
+    # Store the signatures just now to avoid storing them unverified
     store_signatures(signatures, manifest_digest, pubkey)
-    return upgraded
+    return manifest_digest

tests/conftest.py

@@ -13,12 +13,6 @@ from dangerzone.gui import Application
 sys.dangerzone_dev = True  # type: ignore[attr-defined]
 
 
-ASSETS_PATH = Path(__file__).parent / "assets"
-TEST_PUBKEY_PATH = ASSETS_PATH / "test.pub.key"
-INVALID_SIGNATURES_PATH = ASSETS_PATH / "signatures" / "invalid"
-VALID_SIGNATURES_PATH = ASSETS_PATH / "signatures" / "valid"
-TEMPERED_SIGNATURES_PATH = ASSETS_PATH / "signatures" / "tempered"
-
 
 # Use this fixture to make `pytest-qt` invoke our custom QApplication.
 # See https://pytest-qt.readthedocs.io/en/latest/qapplication.html#testing-custom-qapplications
@@ -140,10 +134,6 @@ for_each_doc = pytest.mark.parametrize(
 )
 
 
-@pytest.fixture
-def signature():
-    return {}
-
 
 # External Docs - base64 docs encoded for externally sourced documents
 # XXX to reduce the chance of accidentally opening them