Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Independent container updates #1080

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

Conversation

almet
Copy link
Member

@almet almet commented Feb 11, 2025

This pull request introduces independent container updates.

Signatures are meant to be done with an hardware key (yubikey), for which the public key will be packaged alongside the code.

container signatures are stored locally and checked against a known public key just before doing the conversion of the documents. Behind the curtain, it uses cosign so that the signatures are publicly auditable.

It adds a dangerzone-image CLI providing tooling to check for remote new images, verification of the attestations and signatures, also with the ability to create and use archives, for air-gapped environments.

$ dangerzone-image --help

  attest-provenance  Look up the image attestation to see if the image...
  get-manifest       Retrieves a remote manifest for a given image and...
  list-remote-tags   List the tags available for a given image.
  load-archive       Upgrade the local image to the one in the archive.
  prepare-archive    Prepare an archive to upgrade the dangerzone image...
  upgrade            Upgrade the image to the latest signed version.
  verify-local       Verify the local image signature against a public...

It is fixing the following issues:

The last commit here introduces a way to use the code, but is not meant to be shipped "as is". We still need to get the full story for this, especially in terms of UX.

Before being ready to be shipped, we still miss a few key components:

almet and others added 6 commits February 11, 2025 18:13
It contains utilities to interact with OCI registries, like getting the list of
published tags and getting the content of a manifest. It does so
via the use of the Docker Registry API v2 [0].

The script has been added to the `dev_scripts`, and is also installed on
the system under `dangerzone-image`.

[0]  https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-container-registry
Signatures are stored in the OCI Manifest v2 registry [0], and are
expected to follow the Cosign Signature Specification [0]

The following CLI utilities are provided with `dangerzone-image`:

For checking new container images, upgrading them and downloading them:

- `upgrade` allows to upgrade the current installed image to the
  last one available on the OCI registry, downloading and storing the
  signatures in the process.
- `verify-local` allows the verify the currently installed image against
  downloaded signatures and public key.

To prepare and install archives on air-gapped environments:

- `prepare-archive` helps to prepare an archive to install on another
  machine
- `load-archive` helps upgrade the local image to the archive given
  in argument.

Signatures are stored locally using the format provided by `cosign
download signature`, and the Rekor log index is used to ensure the
requested-to-install container image is fresher than the one already
present on the system.

[0] https://github.com/sigstore/cosign/blob/main/specs/SIGNATURE_SPEC.md
The hash list provided on the Github releases page is now bundled in the
`reproduce-image.py` script, and the proper hashes are checked after
download.
A new `dangerzone-image attest-provenance` script is now available,
making it possible to verify the attestations of an image published on
the github container registry.

Container images are now build nightly and uploaded to the container
registry.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads up, I'll make changes in this file, based on the findings in https://github.com/freedomofpress/repro-build. Nothing too radical, the changes in our build-image.py script will be far more reaching. I'll add those as new commits, if you don't mind.



def load_image_tarball() -> None:
def load_image_tarball_from_gzip() -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While working on reproducible builds, I started to realize that our container image becomes marginally smaller if we compress it with gzip. That could be because Buildkit compresses the files in each layer, but I'm not sure yet. Anyway, what I'm getting at is that this code path is a strong candidate for ✂️ .


def container_pull(image: str) -> bool:
"""Pull a container image from a registry."""
cmd = [get_runtime_name(), "pull", f"{image}"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cmd = [get_runtime_name(), "pull", f"{image}"]
cmd = [get_runtime_name(), "pull", image]

Comment on lines +185 to +186
process = subprocess.Popen(cmd, stdout=subprocess.PIPE)
process.communicate()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we want to achieve with .communicate(). Is it to enable some sort of progress report in the future? In any case, I think a progress_fn and iterating the stdout lines will do.

Comment on lines +206 to +209
if not image_digest:
raise errors.ImageNotPresentException(
f"The image {image} does not exist locally"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I get what you're talking about, you're handling here the case where the image exists locally, but Podman/Docker somehow don't return a digest for it. That's a weird one indeed, I've seen it while testing things out, but never could figure out why it happens.

At any rate, I'd change the message here to show that the image exists locally, but the container engine reports no digest for it. Solely to not get confused with the error message below.

Comment on lines +84 to +87
if False: # Comment this for now, just as an exemple of this can be implemented
# # Load the image tarball into the container runtime.
update_available, image_digest = updater.is_update_available(
container_utils.CONTAINER_NAME
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping a note here to revisit this method once we nail the UX.

Comment on lines +6 to +15
# NOTE: You can grab the SLSA attestation for an image/tag pair with the following
# commands:
#
# IMAGE=ghcr.io/apyrgio/dangerzone/dangerzone
# TAG=20250129-0.8.0-149-gbf2f5ac
# DIGEST=$(crane digest ${IMAGE?}:${TAG?})
# ATT_MANIFEST=${IMAGE?}:${DIGEST/:/-}.att
# ATT_BLOB=${IMAGE?}@$(crane manifest ${ATT_MANIFEST?} | jq -r '.layers[0].digest')
# crane blob ${ATT_BLOB?} | jq -r '.payload' | base64 -d | jq
CUE_POLICY = r"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before settling on the final CUE policy, I'd like to take one last look at the SLSA attestations, to see if we can use any of them.

return False


def is_update_available(image: str) -> Tuple[bool, Optional[str]]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few comments on this function:

  1. Ideally, I'd prefer if the log index check happened here, so that this function never returns True if we can't perform the update.
  2. I'm not sure if this function is related with signatures. Should it go to a different module, perhaps the registry.py one?
  3. If the image name already contains a digest, we can use that instead of the network call.

log.debug(" ".join(cmd))
result = subprocess.run(cmd, capture_output=True)
if result.returncode != 0:
# XXX Raise instead?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, we already raise in the above lines, so I wouldn't mix booleans here.

}


def verify_signature(signature: dict, image_digest: str, pubkey: str | Path) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but it's kind of a sensitive function, so I want to be more explicit about what we verify here. I'd call this verify_image_digest, because it verifies that the digest of a container image has been signed by our private key. If you're ok with it, let's update the docstring as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants