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

Image signature not getting verified for container-native builds signed with Cosign #4272

Open
ahgencer opened this issue Jan 20, 2023 · 25 comments · May be fixed by containers/image#2235
Open
Assignees
Labels
container-native jira for syncing to jira priority/high triaged This issue was triaged
Milestone

Comments

@ahgencer
Copy link

ahgencer commented Jan 20, 2023

I built my own container image based off of @cgwalters's Fedora Silverblue image from this repo, pushed it onto a container registry, and signed it using Cosign. Now, I'm trying to rebase onto it using ostree-image-signed, but rpm-ostree does not seem to be checking for a good signature.

I set up containers-policy.json(5) and containers-registries.d(5) along with the public key of the image (see below). Interestingly, everything works as expected when I try to pull the image with podman pull (i.e., bad signatures are getting rejected). However, rpm-ostree does not seem to care about bad signatures, or even that one exists at all.

I'm not quite sure if I'm doing something wrong here, or if this is a bug. The documentation on this seems to be quite sparse still (understandably).

Host system details

# rpm-ostree status
State: idle
Deployments:
● fedora:fedora/37/x86_64/silverblue
                  Version: 37.20230120.0 (2023-01-20T00:46:10Z)
                   Commit: 99a409a04f7249f49224be6b07f45c082ed82ffa83d75c2eaddc00d66a999a4c
             GPGSignature: Valid signature by ACB5EE4E831C74BB7C168D27F55AD3FB5323552A

I've tested this both in a virtual machine and on real hardware.

Expected vs actual behavior

I'll be using my pre-made image, ghcr.io/ahgencer/silverblue:pr-3, as an example.

With good signature

Expected and actual:

# rpm-ostree rebase ostree-image-signed:docker://ghcr.io/ahgencer/silverblue:pr-3
Success!

So far, so good.

With bad / no signature

Expected:

# rpm-ostree rebase ostree-image-signed:docker://ghcr.io/ahgencer/silverblue:pr-3
Error: Image signature verification failed

Actual:

# rpm-ostree rebase ostree-image-signed:docker://ghcr.io/ahgencer/silverblue:pr-3
Success!

Steps to reproduce it

Starting from a fresh, up-to-date Fedora Silverblue image, add / modify the following files:

/etc/containers/policy.json:

{
    "default": [
        {
            "type": "reject"
        }
    ],
    "transports": {
        "docker": {
            "registry.access.redhat.com": [
                {
                    "type": "signedBy",
                    "keyType": "GPGKeys",
                    "keyPath": "/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release"
                }
            ],
            "registry.redhat.io": [
                {
                    "type": "signedBy",
                    "keyType": "GPGKeys",
                    "keyPath": "/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release"
                }
            ],
            "ghcr.io/ahgencer/silverblue": [
                {
                    "type": "sigstoreSigned",
                    "keyPath": "/etc/pki/containers/ocitree.pub",
                    "signedIdentity": {
                        "type": "matchRepository"
                    }
                }
            ]
        },
        "docker-daemon": {
            "": [
                {
                    "type": "insecureAcceptAnything"
                }
            ]
        }
    }
}

/etc/containers/registries.d/ocitree.yaml:

docker:
  ghcr.io/ahgencer/silverblue:
    use-sigstore-attachments: true

To test with a known-good signature, the public key in /etc/pki/containers/ocitree.pub should be:

-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEnJEh0T2c+4saH2BDmWhq/XuWhqeG
akLGeWgCtA609gRKYSyblP0nmtO/LqSZt3BsTmYAfxHnwTenJXeFdC8a+w==
-----END PUBLIC KEY-----

If we instead use a non-matching public key, the rebase should fail, because effectively it looks like the image was signed using a different private key. Here is one example of a public key that should fail the verification:

-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEpmrxU4aOD0242K6bFc1uLEZG1WAw
Xu3KSgffAkq2NOQlPe6aWF3RnCSHfE7g2Oi7WQT22KrSaEFWAwtBvZdTOA==
-----END PUBLIC KEY-----

If it helps, the repository where I'm building my image can be found in the pr-cosign branch here.

Would you like to work on the issue?

I'm not familiar enough with rpm-ostree (or any other project that might be affected by this) to work on this issue myself.

@cgwalters
Copy link
Member

Thanks for filing this. This is an important issue to me and I want to be sure this works - we want an e2e test for this. I haven't yet context switched to do that. It is certainly possible there are bugs here.

This is also related to containers/skopeo#1829

@ahgencer
Copy link
Author

Hi, has there been any progress on this?

@RishabhSaini
Copy link
Contributor

Upon initial review, changes might need to be made to containers/container-image-proxy-rs and skopeo/proxy.go to be able to correctly verify the signed images by sigstore. Right now podman pull with the following configurations in /etc/containers/registries.d/ and /etc/containers/policy.json does identify incorrect signature from sigstore and will give an error. Thus those changes made to containers/image may need to be used in `containers/containers-image-proxy-rs'

@RishabhSaini RishabhSaini self-assigned this Jun 27, 2023
@RishabhSaini
Copy link
Contributor

Deeper inspection revealed that, rpm-ostree rebase only handles authfiles needed to access private registries. It does not actually do any policy verification of the image or the author afterwards. (except checking if /etc/containers/policy.json has insecureAcceptAnything)

  • What podman pull does that rpm-ostree rebase does not? :
    Policy verification from container/image/signature

  • How does current rpm-ostree rebase work?
    It pulls the container-image from registry using ostree-ext APIs.
    If it instead used the Image() function from containers/image meant for this purpose and also used by podman pull and skopeo copy, it would by default do this verification.
    However currently the logic of pulling an ostree-based container image and unencapsulating it into an ostree repository are intertwined in this function ImageImporter::Import()

  • What are possible steps forward?

    1. Separate the logic of pull and unencapsulate of a container image. Then we can simply call skopeo copy and get pull and verify in one step. Then unencaspulate in the next step.
    2. Add the logic of policy_verification in containers/containers-image-proxy-rs. Basically having a rust binding for the go function. Then invoke the policy verification while doing ImageImporter::Prepare()

@cgwalters any preferences in the solution forward?

@cgwalters
Copy link
Member

Separate the logic of pull and unencapsulate of a container image. Then we can simply call skopeo copy and get pull and verify in one step. Then unencaspulate in the next step.

In that model we can't reuse already fetched layers, forcing every update to be an entire re-pull; it's a nonstarter.

Add the logic of policy_verification in containers/containers-image-proxy-rs. Basically having a rust binding for the go function. Then invoke the policy verification while doing ImageImporter::Prepare()

This SGTM!

@cgwalters
Copy link
Member

Did you see the discussion in containers/skopeo#1829 btw? That's very related to the latter path.

@RishabhSaini
Copy link
Contributor

Did you see the discussion in containers/skopeo#1829 btw? That's very related to the latter path.

yes also looking over containers/skopeo#1844. Insightful

@jbtrystram
Copy link
Collaborator

I stumbled on this today, and had a look at a few PRs. I am not sure what's is blocking this. Is there a way I can help ? I'm happy to jump in and learn the codebase to contribute the missing bits if I can get a little guidance :)

@RishabhSaini
Copy link
Contributor

Fixed by containers/skopeo#2029
A bump to container-image-proxy-rs should introduce the changes in rpm-ostree

@cgwalters cgwalters reopened this Jul 6, 2023
@cgwalters
Copy link
Member

Let's keep this open until we have a bit more verification of the fix.

A bump to container-image-proxy-rs should introduce the changes in rpm-ostree

I think actually it's just bumping the skopeo package right?

@RishabhSaini RishabhSaini added the jira for syncing to jira label Jul 11, 2023
@castrojo
Copy link

Hopefully this will help with verification!

https://universal-blue.org/blog/2023/07/19/sigstore-signed-images-are-now-here/

Thanks everyone for working on this!

@jmpolom
Copy link

jmpolom commented Aug 23, 2023

This does not appear to work for containers signed via the "keyless" method in github action workflows. A github workflow to sign a container using the github actions OIDC token as detailed in the cosign-installer examples results in a container signed with a certificate that doesn't have an email address as the SAN. The SAN is instead a URI to the github workflow yaml file.

See this example workflow with cosign sign steps:

name: Build and push ostree containers
on:
  workflow_dispatch:
  push:
    branches:
      - 'main'
      - 'test'
      - 'stage'
      - 'next'
jobs:
  build-push-base-image:
    name: Build and push baseline workstation image
    runs-on: ubuntu-latest
    timeout-minutes: 60
    permissions:
      contents: read
      id-token: write
      packages: write
    steps:
      - name: checkout repo
        uses: actions/checkout@v3
      - name: cosign-installer
        uses: sigstore/cosign-installer@v3.1.1
      - name: build base image
        id: build-base-image
        uses: redhat-actions/buildah-build@v2
        with:
          image: fedora-silverblue-ws
          tags: 38-${{ github.ref_name }} ${{ github.sha }}
          containerfiles: ./Containerfile.workstation
      - name: push base image
        id: push-base-image
        uses: redhat-actions/push-to-registry@v2
        with:
          image: ${{ steps.build-base-image.outputs.image }}
          tags: ${{ steps.build-base-image.outputs.tags }}
          registry: ghcr.io/${{ github.repository_owner }}
          username: ${{ github.repository_owner }}
          password: ${{ secrets.GITHUB_TOKEN }}
      - name: cosign login (rh push-to-registry does not login)
        run: cosign login ghcr.io -u ${{ github.repository_owner }} -p ${{ secrets.GITHUB_TOKEN }}
      - name: sign base container image with sigstore fulcio and github oidc
        run: cosign sign --yes ghcr.io/${{ github.repository_owner }}/${{ steps.build-base-image.outputs.image }}@${{ steps.push-base-image.outputs.digest }}

Now, extract the certificate from the signature in the registry:

$ skopeo inspect docker://ghcr.io/jmpolom/fedora-silverblue-ws@sha256:48bb3ee03f4687f86ac0c4d095e096dfa547003321f2519f2d547a2a7dd81ab0 | jq -r .LayersData[0].Annotations.\"dev.sigstore.cosign/certificate\" > cert

And dump the text of the cert:

$ openssl x509 -text -noout -in cert
Certificate:
    (...)
        X509v3 extensions:
            (...)
            X509v3 Subject Alternative Name: critical
                URI:https://github.com/jmpolom/fedora-ostree-ws-container/.github/workflows/build.yml@refs/heads/main

The SAN is a URI and not an email address but containers-policy.json only allows a subject email for the certificate identity. Sigstore seems to be moving away from only allowing email addresses as identities. There are some relevant discussions in issues within their projects. I do not believe this is a sigstore/cosign/fulcio issue whatsoever.

The resulting signed container is perfectly verifiable using cosign verify but podman nor rpm-ostree seem capable of verifying it given what can be configured with container policies. The only reference for subject identity in the containers-policy.json man page is subjectEmail and we do not have a certificate with an email address in it here.

Here is my entire policy.json file:

{
  "default": [
    {
      "type": "reject"
    }
  ],
  "transports": {
    "docker": {
      "registry.access.redhat.com": [
        {
          "type": "signedBy",
          "keyType": "GPGKeys",
          "keyPath": "/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release"
        }
      ],
      "registry.redhat.io": [
        {
          "type": "signedBy",
          "keyType": "GPGKeys",
          "keyPath": "/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release"
        }
      ],
      "ghcr.io/jmpolom": [
        {
          "type": "sigstoreSigned",
          "fulcio": {
            "caPath": "/etc/pki/containers/fulcio.sigstore.dev.pub",
            "oidcIssuer": "https://token.actions.githubusercontent.com",
            "subjectEmail": "jon@subly.me"
          },
          "rekorPublicKeyPath": "/etc/pki/containers/rekor.sigstore.dev.pub",
          "signedIdentity": {
            "type": "matchRepository"
          }
        }
      ],
      "": [
        {
          "type": "insecureAcceptAnything"
        }
      ]
    },
    "docker-daemon": {
      "": [
        {
          "type": "insecureAcceptAnything"
        }
      ]
    }
  }
}

For kicks I did try putting the URI from the certificate SAN into the subjectEmail field in policy.json but of course it did not work. Is there a secret configuration key to use here for non-Email subject identities? I assume this simply isn't supported yet within skopeo/podman/rpm-ostree.

@jmpolom
Copy link

jmpolom commented Sep 19, 2023

@cgwalters have you had a chance to review this yet? Presumably what I'm attempting to do is beyond the capabilities of rpm-ostree/skopeo at this time.

@jmpolom
Copy link

jmpolom commented Oct 30, 2023

Bump on this again for @cgwalters and @RishabhSaini

My conclusion at this point is the sigstore/cosign verification support available in rpm-ostree, podman and skopeo cannot support the keyless cosign signature workflow that works within CI workflows. I think the way containers policy.json allows specifying the certificate subject requires generalization to permit specifying a URI in addition to email.

Is there any chance that this change might get picked up in rpm-ostree? It is nice to see support for cosign signed containers however the support appears incomplete.

@cgwalters
Copy link
Member

Hi, sorry for the delayed reply. We are going to look at doing some work on this issue soon I hope; pulling it more closely into the priority list.

@cgwalters cgwalters added this to the 2023-q4 milestone Oct 30, 2023
@sallyom
Copy link

sallyom commented Oct 30, 2023

It is possible to verify keyless signatures with podman - let me try locally to see if I can find the problem with rpm-ostree - and if there is an issue let's get it fixed because it should be working as expected.

@jmpolom
Copy link

jmpolom commented Oct 31, 2023

It is possible to verify keyless signatures with podman

So I don't disagree that podman can verify keyless signatures in the general sense. The problem is keyless signatures made in CI via github actions (in this specific case, I have not tried gitlab CI). Could you share if you're referring to verifying keyless signatures with podman created in a CI job? The certificate SAN is different and none of the documentation I have reviewed for containers policy.json indicates how I can specify a certificate identity other than an email address (Fulcio certificates for CI jobs do not have an email address identity).

@sallyom
Copy link

sallyom commented Oct 31, 2023

Could you share if you're referring to verifying keyless signatures with podman created in a CI job?

I see now - I read too quickly - if this is a limitation it should not be, I'm going to dig in to fix this (but first I'll be sure it's not just a docs limitation)

@jmpolom
Copy link

jmpolom commented Nov 1, 2023

Is rpm-ostree using the signature verification logic in containers/image and containers/skopeo? I do see some skopeo proxy errors when signature validation fails. I very quickly looked through the skopeo and image repos and only found references to subjectEmail for Fulcio certs.

If you need any package builds tested let me know, I would be able to test.

@lukewarmtemp
Copy link
Contributor

@jmpolom I used a github workflow to sign a container using the github actions OIDC token, just as you have done in a previous message of yours. However, my cert still uses an email address as the SAN and not a URI, so I was not able to reproduce the container image.

containers/skopeo#2182 should fix your problem since I've been able to configure my policy.json and rebase to your signed container image. If you would like to test it, you will just need to compile the skopeo binary from source based on my fork and replace it with the one found at /usr/bin/skopeo on your system.

@jmpolom
Copy link

jmpolom commented Dec 19, 2023

I used a github workflow to sign a container using the github actions OIDC token, just as you have done in a previous message of yours. However, my cert still uses an email address as the SAN and not a URI, so I was not able to reproduce the container image.

Could you link me to the exact github workflow you used? I would like to review it. I was trying to see if there was a way to switch to an email address when performing the signature in a workflow using the OIDC token but could not find anything. Perhaps it has been added or there has been an upstream change I'm not aware of.

PR looks reasonable though. Figured it would be fairly straightforward to support the URI based SAN. This definitely should be supported functionality.

Question before I can test: on an ostree based system how do I replace something in /usr/bin as that area is immutable by design? I have had bad results attempting to do "live" filesystem edits with rpm-ostree. Is the correct way rpm-ostree override replace?

@lukewarmtemp
Copy link
Contributor

Could you link me to the exact github workflow you used?

https://github.com/lukewarmtemp/custom-container-images/blob/main/.github/workflows/docker-image.yml

how do I replace something in /usr/bin as that area is immutable by design?

I just use the following to remove immutability and replace the bin in /usr/bin

$ sudo ostree admin unlock

@jmpolom
Copy link

jmpolom commented Jan 2, 2024

Ah, I think I see why you did not end up with a container signed with a URI as the SAN. Are you defining a github actions secret called GH_PAT?

I believe you are inadvertently using the "workaround" method for the issue described here. It's not really a workaround because you need to enter the one time code from the workflow in a browser manually so your builds are not really automatic at that point.

What you want to use is the GITHUB_TOKEN secret for the workflow job. This is also somewhat indirectly mentioned in the cosign-installer actions docs in the example where they do a docker login. The github actions workflow that I am using uses this ephemeral workflow token and is what results in the URI in the certificate SAN.

I will try to test this later this week. You may want to revise your test workflow to use the workflow token instead of a user defined access token.

@frzifus
Copy link

frzifus commented Apr 29, 2024

I will try to test this later this week. You may want to revise your test workflow to use the workflow token instead of a user defined access token.

Hey @jmpolom @sallyom before trying this again I may ask for your results? 😅

@jmpolom
Copy link

jmpolom commented Apr 29, 2024

Hey @jmpolom @sallyom before trying this again I may ask for your results? 😅

This occurred so far in the past I no longer recall what I was going to test. I don't think I tested anything specific because I probably determined there was in fact nothing to test. I have a container image regularly being signed and I'll probably be building a system this week with it I'll try to pull with signature verification enabled.

The PR by @lukewarmtemp did not appear to add the necessary functionality (logic seemed backwards) to resolve this issue. The PR is is also seemingly stalled on ideological grounds by the maintainer @mtrmac not wanting to add support for pulling the identity from the certificate SAN. See the PR for the details. Maybe someone else asking for this to "just work" (R) as it should might get things moving in the right direction.

This remains an open issue and this functionality does not work as needed, as of today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
container-native jira for syncing to jira priority/high triaged This issue was triaged
Projects
None yet
9 participants