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

Container: Verify Subject.Name matches image name? #208

Open
asraa opened this issue Aug 11, 2022 · 9 comments
Open

Container: Verify Subject.Name matches image name? #208

asraa opened this issue Aug 11, 2022 · 9 comments
Labels
area:container Issue with container provenance verification type:feature New feature request

Comments

@asraa
Copy link
Contributor

asraa commented Aug 11, 2022

The container provenance generators and builders populate Subject.Name of the provenance statement with the image reference.

Should we verify that the user supplied container image name matches the Subject.Name?

Some problems:

  • If a user passes the container image pinned by digest rather than tag, then we need to resolve the Subject.Name. In that case, we are verifying Subject.Digest, as we already are.
  • If the user supplies, say, a local image path of a saved container, then there is no way to reconstruct the expected Subject.Name. This is what happens during testing.
  • Maybe we should just verify the repository and image name? Still, we have the local image path problems.

@ianlewis, would love feedback

See this thread for context:
do we want to verify the subject? Our builder uses image:tag as subject, so this should be safe?
/cc @ianlewis

Originally posted by @laurentsimon in #147 (comment)

@laurentsimon
Copy link
Contributor

local image path

Does the local path problem happen with our GH builders?

Could we have the verification be optional?

@asraa
Copy link
Contributor Author

asraa commented Aug 11, 2022

Does the local path problem happen with our GH builders?

It does for testing, since I save the image locally from the GH builders, and commit that to testdata -- which means we've lost the path. In the non-test path of the open PR for the implementation, user's can't supply local paths, so I guess that's not a problem. We are always expected images on a registry.

No need to optimize for cases where someone wants to save first and verify (e.g. auth problems or something) until someone asks for it.

Could we have the verification be optional?

Yeah, I'm thinking we can just print to Stderr in case the matching digest has a mismatched subject name, and see how that fares in e2e tests

@asraa
Copy link
Contributor Author

asraa commented Aug 11, 2022

A quick update: I inspected some provenance generated by the generator, and it does NOT include the tag... so it may look like:
"ghcr.io/slsa-framework/example-package.e2e.container.workflow_dispatch.main.default.slsa3

so I think that resolves the concern around tags being included, because they're not.

We won't get local image validation for tests, but that's OK. Happy to do this in another PR

@laurentsimon
Copy link
Contributor

is it an arbitrary decision that the tag is not included? Or we have good reasons not to :-)?

@ianlewis
Copy link
Member

ianlewis commented Aug 15, 2022

I would have assumed that slsa-verifier would be retrieving the image itself so in that case the name would have to be correct ( or at least instructing docker to do so?) Maybe I'm missing something.

As for the tag, I suppose we could do the kind of semver check we do for the git repo tags, but I'm not sure the image tag is really meaningful for us. It can be changed and isn't really useful from a verification perspective. I also kind of expect that folks would use the sha to reference the image when retrieving it.

@asraa
Copy link
Contributor Author

asraa commented Aug 15, 2022

It can be changed and isn't really useful from a verification perspective. I also kind of expect that folks would use the sha to reference the image when retrieving it.

Yeah, exaclty -- tag is mutable. And the Subject.Digest verification already provides us with the assurance that the provenance matches

I would have assumed that slsa-verifier would be retrieving the image itself so in that case the name would have to be correct ( or at least instructing docker to do so?)

It does! But the provenance is stored in a file discoverable by a known tag name alongside the artifact -- so the check here would be a failsafe: make sure the attached provenance actually corresponds to the artifact.

@ianlewis
Copy link
Member

I would have assumed that slsa-verifier would be retrieving the image itself so in that case the name would have to be correct ( or at least instructing docker to do so?)

It does! But the provenance is stored in a file discoverable by a known tag name alongside the artifact -- so the check here would be a failsafe: make sure the attached provenance actually corresponds to the artifact.

hmm. I wonder what cosign verify-attestation <image>@<sha> does since the tag name isn't given in that case. I assumed we would want to do the same thing. I would think for our purposes the tag doesn't really matter. Just image name and digest.

@asraa
Copy link
Contributor Author

asraa commented Aug 22, 2022

I assumed we would want to do the same thing. I would think for our purposes the tag doesn't really matter. Just image name and digest.

Yeah, cosign won't do any Name validation. I'm actually not sure it does Digest verification. D:

Agree that image name and digest can be verified. I can add the name validation!

@laurentsimon
Copy link
Contributor

Do we want to do this by default or via an option -subject-name? I'm asking to keep consistency across builders, which may help users in the long-run if they use several builders. Wdut?

@ianlewis ianlewis added type:feature New feature request area:container Issue with container provenance verification labels Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:container Issue with container provenance verification type:feature New feature request
Projects
None yet
Development

No branches or pull requests

3 participants