feat: support oci image verification #147

asraa · 2022-07-13T20:41:43Z

Signed-off-by: Asra Ali [email protected]

Support OCI image verification in the GHA verifier. OCI images can be verified using

slsa-verifier verify-image ${IMAGE} --source-uri ${SOURCE_URI}

Added initial testing, for valid signatures, invalid builder IDs and invalid source repositories.

Signed-off-by: Asra Ali <[email protected]>

naveensrinivasan

What is the process to verify the attached binaries?

The goal is to make sure the e2e tests are reliable.

asraa · 2022-07-19T13:59:28Z

Yes @naveensrinivasan I'm still trying to figure out a testing strategy

What is the process to verify the attached binaries?

I don't understand the question. Binaries are already e2e tested.

The goal is to make sure the e2e tests are reliable.

Yeah. Ideally I'd be able to attach manifests for OCI images and use those to verify. I will probably need to implement a fake registry and a fake signed image for cosign. do you have other ideas?

naveensrinivasan · 2022-07-19T14:16:27Z

Binaries are already e2e tested.

How were these specific binaries already e2e tested? I am trying to understand, so pardon my ignorance.

naveensrinivasan · 2022-07-19T14:17:56Z

Binaries are already e2e tested.

How were these specific binaries already e2e tested? I am trying to understand, so pardon my ignorance.

🤦 Now, I see them as renamed and not new binaries. Thanks, it makes sense. I thought these were new binaries.

ianlewis · 2022-08-02T09:27:28Z

main.go

@@ -82,7 +92,13 @@ func main() {
 		"print the verified provenance to std out")
 	flag.Parse()

-	if provenancePath == "" || artifactPath == "" || source == "" {
+	if (provenancePath == "" || artifactPath == "") && ociImageRef == "" {


I feel like we should have different commands for containers vs. artifacts

The combinations of how flags are used isn't really very user friendly and I feel like it will just get worse over time.

...not that I expect that to be implemented in this PR. Just a general comment.

For v2, I'll try to write a one-pager on the SLSA verifier API. There are a few issues on this (#180) and I am worried without writing it up first I'll get it wrong.

Just to be clear, I was simply thinking something like slsa-verifier verify-artifact ... and slsa-verifier verify-image ... something like that. Right now it's down to the combination of what flags users use. Commands allow us to have different flags for different CLI commands.

Ah, yes I agree too we should have sub-commands separating the two.

main.go

Signed-off-by: Asra Ali <[email protected]>

asraa · 2022-08-02T20:16:18Z

Currently working with example-test packages

$ go run . -oci-image ghcr.io/slsa-framework/example-package.e2e.container.schedule.main.default.slsa3@sha256:93e556b16d8be1283f3825aa0ee595250649f56e70e18a3089d92ba29cd81e85 -source github.com/slsa-framework/example-package
Verified build using builder https://github.com/slsa-framework/slsa-github-generator/.github/workflows/generator_container_slsa3.yml@refs/heads/main at commit 8837317a8c8021fa59bb05260f363c7deb4ca63d
PASSED: Verified SLSA provenance

For some reason, crane manifest'ing the image doesn't show me where the cosign attesation layer is. Trying to retrieve some testadata manifests.

asraa · 2022-08-04T15:57:58Z

Got tests in. Waiting for the queue of PRs #187

and then I'll implement the VerifyImage on the builder interface.

Signed-off-by: Asra Ali <[email protected]>

asraa · 2022-08-09T19:59:10Z

@ianlewis @laurentsimon This is ready now.

I had to make a few changes for testing -- remote and local images behave slightly different. See the container package.

I will make an issue to add more testing: malicious tests, tag verification. Just wanted to start the functionality.

laurentsimon

LGTM overall. Excited to see this one land!!

laurentsimon · 2022-08-09T21:56:30Z

cli/slsa-verifier/main.go

 )

 func main() {
 	flag.StringVar(&builderID, "builder-id", "", "EXPERIMENTAL: the unique builder ID who created the provenance")
 	flag.StringVar(&provenancePath, "provenance", "", "path to a provenance file")
 	flag.StringVar(&artifactPath, "artifact-path", "", "path to an artifact to verify")
+	flag.StringVar(&artifactReference, "artifact-reference", "", "reference to an OCI image to verify")


so are we giving up on the idea of verify-artifact and verify-container subcommand? What was decided in the end?

I think we should, should I do that in this PR though? If it's pretty easy to review I can split the commands

up to you, I don't mind either way

cli/slsa-verifier/main.go

laurentsimon · 2022-08-09T21:59:13Z

cli/slsa-verifier/main.go

 )

 func main() {
 	flag.StringVar(&builderID, "builder-id", "", "EXPERIMENTAL: the unique builder ID who created the provenance")
 	flag.StringVar(&provenancePath, "provenance", "", "path to a provenance file")
 	flag.StringVar(&artifactPath, "artifact-path", "", "path to an artifact to verify")
+	flag.StringVar(&artifactReference, "artifact-reference", "", "reference to an OCI image to verify")


I'm curious: is "reference" is standard term in container world?

yes, it means repository:tag. technically it also supports the actual digest, so maybe I should say -artifact-image (and image reference resolution will be implicit)?

I see. If this is a common term, then it's fine. artifact-image would also work for me.
/cc @ianlewis any thoughts?

Changed to artifact-image for now. Leaving unresolved for feedback from Ian

cli/slsa-verifier/main.go

cli/slsa-verifier/main_test.go

verifiers/internal/gha/verifier.go

container/container.go

verifiers/internal/gha/builder.go

verifiers/internal/gha/verifier.go

Signed-off-by: Asra Ali <[email protected]>

cli/slsa-verifier/main.go

laurentsimon

Thanks. Left a few comments.

verifiers/internal/gha/verifier.go

laurentsimon · 2022-08-16T18:30:31Z

verifiers/container/container.go

+	crname "github.com/google/go-containerregistry/pkg/name"
+)
+
+var GetImageDigest = func(image string) (string, error) {


Note: would this break if we verify in parallel different images that require changing this global variable?
I don't think we're going to need that any time soon.. unless the unit tests have to.

I wonder whether we this should be a func GetImageDigest() and have an interface that this function implement in a structure. This way anyone who needs to update the function can create their structure and set the function to whatever they want. By default, the function would be the one declared in this file.

Hmmm The purpose of this global is just for testing -- only testing will use the modified vars (and always will)

I wonder whether we this should be a func GetImageDigest() and have an interface that this function implement in a structure.

Actually, that can work. We can do a file-based impl and a registry-based implementation of the container. Then we can instantiate based on the params (was image str a local file or a container reference)

It's fine to do that in a follow-up PR. Up to you.

Signed-off-by: Asra Ali <[email protected]>

asraa added 2 commits July 13, 2022 15:41

feat: support oci image verification

f6c26dd

Signed-off-by: Asra Ali <[email protected]>

add testing folder

d733abe

Signed-off-by: Asra Ali <[email protected]>

naveensrinivasan reviewed Jul 18, 2022

View reviewed changes

ianlewis reviewed Aug 2, 2022

View reviewed changes

asraa added 2 commits August 2, 2022 14:43

Merge remote-tracking branch 'upstream/main' into verify-oci

3315e22

update name and make fix

837175b

Signed-off-by: Asra Ali <[email protected]>

asraa mentioned this pull request Aug 3, 2022

Support for more SLSA default fields #180

Open

Merge branch 'main' into verify-oci

3e42dd8

asraa mentioned this pull request Aug 4, 2022

feat: Create interface for verifiers #187

Merged

asraa added 3 commits August 4, 2022 10:58

add tests

7a779f6

Signed-off-by: Asra Ali <[email protected]>

Merge remote-tracking branch 'upstream/main' into verify-oci

e27766e

Add initial testing

f329e07

Signed-off-by: Asra Ali <[email protected]>

asraa marked this pull request as ready for review August 9, 2022 19:58

asraa mentioned this pull request Aug 9, 2022

test: Add more testing for OCI images, and add documentation for test generation #201

Open

laurentsimon reviewed Aug 9, 2022

View reviewed changes

laurentsimon reviewed Aug 10, 2022

View reviewed changes

container/container.go Outdated Show resolved Hide resolved

laurentsimon reviewed Aug 10, 2022

View reviewed changes

verifiers/internal/gha/builder.go Outdated Show resolved Hide resolved

laurentsimon reviewed Aug 10, 2022

View reviewed changes

verifiers/internal/gha/verifier.go Show resolved Hide resolved

laurentsimon reviewed Aug 10, 2022

View reviewed changes

verifiers/internal/gha/verifier.go Outdated Show resolved Hide resolved

asraa added 2 commits August 10, 2022 10:24

Merge remote-tracking branch 'upstream/main' into verify-oci

8e3d3d0

updated comments

13894e3

Signed-off-by: Asra Ali <[email protected]>

asraa force-pushed the verify-oci branch from a73a8cc to 13894e3 Compare August 10, 2022 21:35

This was referenced Aug 10, 2022

Create subcommands for artifact and image verification #205

Closed

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

Open

laurentsimon reviewed Aug 15, 2022

View reviewed changes

cli/slsa-verifier/main.go Show resolved Hide resolved

laurentsimon reviewed Aug 15, 2022

View reviewed changes

cli/slsa-verifier/main.go Outdated Show resolved Hide resolved

laurentsimon reviewed Aug 15, 2022

View reviewed changes

cli/slsa-verifier/main.go Show resolved Hide resolved

laurentsimon approved these changes Aug 15, 2022

View reviewed changes

verifiers/internal/gha/verifier.go Show resolved Hide resolved

Merge branch 'main' into verify-oci

a605e2e

laurentsimon reviewed Aug 16, 2022

View reviewed changes

update

aad1aa5

Signed-off-by: Asra Ali <[email protected]>

asraa mentioned this pull request Aug 16, 2022

Container: Refactor file/OCI based implementations for verifying and computing digest #220

Open

fix digest calculation

6b34dda

Signed-off-by: Asra Ali <[email protected]>

laurentsimon mentioned this pull request Aug 16, 2022

feat: Support for GCB verification #202

Merged

2 tasks

asraa merged commit 7b4b9cd into slsa-framework:main Aug 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support oci image verification #147

feat: support oci image verification #147

asraa commented Jul 13, 2022 •

edited

Loading

naveensrinivasan left a comment

asraa commented Jul 19, 2022

naveensrinivasan commented Jul 19, 2022

naveensrinivasan commented Jul 19, 2022

ianlewis Aug 2, 2022

ianlewis Aug 2, 2022

asraa Aug 2, 2022

ianlewis Aug 3, 2022

asraa Aug 3, 2022

laurentsimon Aug 4, 2022

asraa commented Aug 2, 2022

asraa commented Aug 4, 2022

asraa commented Aug 9, 2022

laurentsimon left a comment •

edited

Loading

laurentsimon Aug 9, 2022

asraa Aug 10, 2022

laurentsimon Aug 10, 2022

laurentsimon Aug 9, 2022

asraa Aug 10, 2022

laurentsimon Aug 10, 2022 •

edited

Loading

asraa Aug 10, 2022

laurentsimon left a comment

laurentsimon Aug 16, 2022 •

edited

Loading

asraa Aug 16, 2022

laurentsimon Aug 16, 2022

feat: support oci image verification #147

feat: support oci image verification #147

Conversation

asraa commented Jul 13, 2022 • edited Loading

naveensrinivasan left a comment

Choose a reason for hiding this comment

asraa commented Jul 19, 2022

naveensrinivasan commented Jul 19, 2022

naveensrinivasan commented Jul 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asraa commented Aug 2, 2022

asraa commented Aug 4, 2022

asraa commented Aug 9, 2022

laurentsimon left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laurentsimon Aug 10, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laurentsimon left a comment

Choose a reason for hiding this comment

laurentsimon Aug 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asraa commented Jul 13, 2022 •

edited

Loading

laurentsimon left a comment •

edited

Loading

laurentsimon Aug 10, 2022 •

edited

Loading

laurentsimon Aug 16, 2022 •

edited

Loading