feat: fixes #724: add input for --provenance-repository while image verification #736

saisatishkarra · 2024-01-15T16:04:58Z

@laurentsimon Added a new image verification cmd input --provenance-repository
This replicates the feature of the COSIGN_REPOSITORY environment variable when provenance is stored in a different repository/registry

Order of precedence:

If input --provenance-repository is set, leverages the non-empty input value
If the env variable COSIGN_REPOSITORY is set, it is NOT consumed

README edit : https://github.com/slsa-framework/slsa-verifier/pull/736/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R280

…ation Signed-off-by: saisatishkarra <saisatish.karra@konghq.com>

saisatishkarra · 2024-01-15T17:29:40Z

Refer comment and description before merging for the UX

ianlewis · 2024-01-16T05:25:43Z

cli/slsa-verifier/verify/options.go

+	cmd.Flags().Var(&o.BuildWorkflowInputs, "build-workflow-input",
+		"[optional] a workflow input provided by a user at trigger time in the format 'key=value'. (Only for 'workflow_dispatch' events on GitHub Actions).")
+
+	cmd.Flags().StringVar(&o.BuilderID, "builder-id", "", "[optional] the unique builder ID who created the provenance")
+
+	/* Source options */
+	cmd.Flags().StringVar(&o.SourceURI, "source-uri", "",
+		"expected source repository that should have produced the binary, e.g. github.com/some/repo")
+
+	cmd.Flags().StringVar(&o.SourceBranch, "source-branch", "", "[optional] expected branch the binary was compiled from")
+
+	cmd.Flags().StringVar(&o.SourceTag, "source-tag", "", "[optional] expected tag the binary was compiled from")
+
+	cmd.Flags().StringVar(&o.SourceVersionTag, "source-versioned-tag", "",
+		"[optional] expected version the binary was compiled from. Uses semantic version to match the tag")
+
+	/* Other options */
+	cmd.Flags().StringVar(&o.ProvenancePath, "provenance-path", "",
+		"path to a provenance file")
+
+	cmd.Flags().StringVar(&o.ProvenanceRepository, "provenance-repository", "",
+		"image repository for provenance with format: <registry>/<repository>. When set, overrides COSIGN_REPOSITORY environment variable")
+
+	cmd.Flags().BoolVar(&o.PrintProvenance, "print-provenance", false,
+		"[optional] print the verified provenance to stdout")
+
+	cmd.MarkFlagRequired("source-uri")
+	cmd.MarkFlagsMutuallyExclusive("source-versioned-tag", "source-tag")


Could you just call o.VerifyImage.AddFlags(cmd)?

Suggested change

cmd.Flags().Var(&o.BuildWorkflowInputs, "build-workflow-input",

"[optional] a workflow input provided by a user at trigger time in the format 'key=value'. (Only for 'workflow_dispatch' events on GitHub Actions).")

cmd.Flags().StringVar(&o.BuilderID, "builder-id", "", "[optional] the unique builder ID who created the provenance")

/* Source options */

cmd.Flags().StringVar(&o.SourceURI, "source-uri", "",

"expected source repository that should have produced the binary, e.g. github.com/some/repo")

cmd.Flags().StringVar(&o.SourceBranch, "source-branch", "", "[optional] expected branch the binary was compiled from")

cmd.Flags().StringVar(&o.SourceTag, "source-tag", "", "[optional] expected tag the binary was compiled from")

cmd.Flags().StringVar(&o.SourceVersionTag, "source-versioned-tag", "",

"[optional] expected version the binary was compiled from. Uses semantic version to match the tag")

/* Other options */

cmd.Flags().StringVar(&o.ProvenancePath, "provenance-path", "",

"path to a provenance file")

cmd.Flags().StringVar(&o.ProvenanceRepository, "provenance-repository", "",

"image repository for provenance with format: <registry>/<repository>. When set, overrides COSIGN_REPOSITORY environment variable")

cmd.Flags().BoolVar(&o.PrintProvenance, "print-provenance", false,

"[optional] print the verified provenance to stdout")

cmd.MarkFlagRequired("source-uri")

cmd.MarkFlagsMutuallyExclusive("source-versioned-tag", "source-tag")

o.VerifyImage.AddFlags(cmd)

cmd.Flags().StringVar(&o.ProvenanceRepository, "provenance-repository", "",

"image repository for provenance with format: <registry>/<repository>. When set, overrides COSIGN_REPOSITORY environment variable")

ianlewis · 2024-01-16T05:35:14Z

verifiers/verifier.go

@@ -37,14 +37,15 @@ func getVerifier(builderOpts *options.BuilderOpts) (register.SLSAVerifier, error

 func VerifyImage(ctx context.Context, artifactImage string,


Some folks use slsa-verifier's Go API. Could we maybe add a new method VerifyImageProvenanceRepo with the new signature rather than modify VerifyImage in order to maintain API backwards compatibility?

Or maybe it makes sense to add another options.ImageOptions (with just ProvenanceRepository on it for now) and add a VerifyImageWithOptions function instead in order to protect against future API changes? WDUT?

/cc @laurentsimon

Signed-off-by: saisatishkarra <saisatish.karra@konghq.com>

saisatishkarra · 2024-01-19T18:32:47Z

@ianlewis / @laurentsimon added a VerifyImageProvenanceRepo function for verifier interface
GHA implementation of the new function overrides the cosign opts with provided input and backwards compatible for verifyImage()

GCB implementation of the new function VerifyImageProvenanceRepo calls back the verifyImage() internally and ignores the passed provenanceRepository argument.

LMK what you folks think!!

laurentsimon · 2024-01-19T19:06:42Z

verifiers/internal/gcb/verifier.go

@@ -126,3 +125,12 @@ func (v *GCBVerifier) VerifyImage(ctx context.Context,
 	}
 	return content, builderID, nil
 }
+
+// VerifyImageProvenanceRepo verifies provenance for an OCI image.
+func (v *GCBVerifier) VerifyImageProvenanceRepo(ctx context.Context,


Let's re-use the VerifyImage(). Adding an additional parameter will be a breaking change, and that's why you're using a different unction, correct? Let' s keep it simple and just add a parameter to the options.ProvenanceOpts structure. This should make your changes simpler too.

I know retrofitting the provenance repo name in ProvenanceOpts is not as clean as having a dedicated imageOption, but that's a problem with the original API design. We're going to fix this eventually and redesign it with variadic options. @ramonpetgrave64

Sure. I was also leaning toward adding ProvenanceOpts struct but it didn't seem to fit from underlying verification and expected params POV. Since refactoring this later is on the table, i will dump in there and submit new changes!! Thx @laurentsimon

@laurentsimon Added provenanceRepository to provenanceOpts struct and reused verifyImage

Signed-off-by: saisatishkarra <saisatish.karra@konghq.com>

laurentsimon

Thanks.

cli/slsa-verifier/verify/options.go

laurentsimon · 2024-01-22T16:58:42Z

verifiers/internal/gha/verifier.go

+			return nil, nil, err
+		}
+	} else {
+		// If user input --provenance-repository is empty, look for COSIGN_REPOSITORY environment


let's remove support for COSIGN_REPOSITORY.

+1. I was about to suggest this as well since we'll probably want to move to using sigstore-go and not rely on cosign packages. The --provenance-repository option is a better way to implement what we want.

laurentsimon · 2024-01-22T17:46:42Z

cli/slsa-verifier/verify/verify_image.go

@@ -70,7 +72,11 @@ func (c *VerifyImageCommand) Exec(ctx context.Context, artifacts []string) (*uti
 		}
 	}

-	verifiedProvenance, outBuilderID, err := verifiers.VerifyImage(ctx, artifacts[0], provenance, provenanceOpts, builderOpts)
+	var verifiedProvenance []byte


nit: we can remove defining these two variables, correct?

verifiers/internal/gha/verifier.go

Signed-off-by: saisatishkarra <saisatish.karra@konghq.com>

saisatishkarra · 2024-01-22T17:57:02Z

@laurentsimon can you review these changes again pls?

laurentsimon

LGTM, thank you!

laurentsimon · 2024-01-22T18:00:30Z

Let's update the example-package workfow next?

I also created slsa-framework/slsa-github-generator#3095 which should be a simple change

Thanks again for the hard work.

@laurentsimon

… while image verification (slsa-framework#736) @laurentsimon Added a new image verification cmd input `--provenance-repository` This replicates the feature of the `COSIGN_REPOSITORY` environment variable when provenance is stored in a different repository/registry Order of precedence: - If input `--provenance-repository` is set, leverages the non-empty input value - If the env variable `COSIGN_REPOSITORY` is set, it is NOT consumed README edit : https://github.com/slsa-framework/slsa-verifier/pull/736/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R280 --------- Signed-off-by: saisatishkarra <saisatish.karra@konghq.com> Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com> Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

saisatishkarra force-pushed the feat/provenance-repository branch from 107f4e9 to 04e5f06 Compare January 15, 2024 16:54

saisatishkarra marked this pull request as ready for review January 15, 2024 16:55

saisatishkarra requested review from ianlewis, laurentsimon and kpk47 as code owners January 15, 2024 16:55

saisatishkarra mentioned this pull request Jan 15, 2024

Add attestation store option #724

Closed

(feat/cli): add input for --provenance-repository while image verific…

2c971c1

…ation Signed-off-by: saisatishkarra <saisatish.karra@konghq.com>

saisatishkarra force-pushed the feat/provenance-repository branch from 04e5f06 to 2c971c1 Compare January 15, 2024 17:02

saisatishkarra changed the title ~~add input for --provenance-repository while image verification~~ (feat/cli): add input for --provenance-repository while image verification Jan 15, 2024

saisatishkarra changed the title ~~(feat/cli): add input for --provenance-repository while image verification~~ (feat): add input for --provenance-repository while image verification Jan 15, 2024

saisatishkarra changed the title ~~(feat): add input for --provenance-repository while image verification~~ feat: fixes #724: add input for --provenance-repository while image verification Jan 15, 2024

ianlewis reviewed Jan 16, 2024

View reviewed changes

add VerifyImageProvenanceRepo for backward api compatibility

c475cc5

Signed-off-by: saisatishkarra <saisatish.karra@konghq.com>

laurentsimon reviewed Jan 19, 2024

View reviewed changes

Avoid breaking change for provenanceRepository cli option

36bf836

Signed-off-by: saisatishkarra <saisatish.karra@konghq.com>

saisatishkarra force-pushed the feat/provenance-repository branch from 2a5a666 to 36bf836 Compare January 22, 2024 16:39

laurentsimon reviewed Jan 22, 2024

View reviewed changes

saisatishkarra force-pushed the feat/provenance-repository branch from bbb67f9 to 2adb4a7 Compare January 22, 2024 17:50

deprecate COSIGN_REPOSITORY in favor of provenanceRepository

93ddb5d

Signed-off-by: saisatishkarra <saisatish.karra@konghq.com>

saisatishkarra force-pushed the feat/provenance-repository branch from 2adb4a7 to 93ddb5d Compare January 22, 2024 17:55

saisatishkarra requested a review from laurentsimon January 22, 2024 17:57

laurentsimon approved these changes Jan 22, 2024

View reviewed changes

Merge branch 'main' into feat/provenance-repository

cf39061

laurentsimon enabled auto-merge (squash) January 22, 2024 18:01

laurentsimon merged commit 9b2467f into slsa-framework:main Jan 22, 2024
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: fixes #724: add input for --provenance-repository while image verification #736

feat: fixes #724: add input for --provenance-repository while image verification #736

saisatishkarra commented Jan 15, 2024 •

edited

Loading

saisatishkarra commented Jan 15, 2024

ianlewis Jan 16, 2024 •

edited

Loading

ianlewis Jan 16, 2024

saisatishkarra commented Jan 19, 2024

laurentsimon Jan 19, 2024 •

edited

Loading

saisatishkarra Jan 22, 2024

saisatishkarra Jan 22, 2024

laurentsimon left a comment

laurentsimon Jan 22, 2024

ianlewis Jan 23, 2024 •

edited

Loading

laurentsimon Jan 22, 2024

saisatishkarra Jan 22, 2024

saisatishkarra commented Jan 22, 2024

laurentsimon left a comment

laurentsimon commented Jan 22, 2024 •

edited

Loading

		@@ -37,14 +37,15 @@ func getVerifier(builderOpts *options.BuilderOpts) (register.SLSAVerifier, error

		func VerifyImage(ctx context.Context, artifactImage string,

feat: fixes #724: add input for --provenance-repository while image verification #736

feat: fixes #724: add input for --provenance-repository while image verification #736

Conversation

saisatishkarra commented Jan 15, 2024 • edited Loading

saisatishkarra commented Jan 15, 2024

ianlewis Jan 16, 2024 • edited Loading

Choose a reason for hiding this comment

ianlewis Jan 16, 2024

Choose a reason for hiding this comment

saisatishkarra commented Jan 19, 2024

laurentsimon Jan 19, 2024 • edited Loading

Choose a reason for hiding this comment

saisatishkarra Jan 22, 2024

Choose a reason for hiding this comment

saisatishkarra Jan 22, 2024

Choose a reason for hiding this comment

laurentsimon left a comment

Choose a reason for hiding this comment

laurentsimon Jan 22, 2024

Choose a reason for hiding this comment

ianlewis Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

laurentsimon Jan 22, 2024

Choose a reason for hiding this comment

saisatishkarra Jan 22, 2024

Choose a reason for hiding this comment

saisatishkarra commented Jan 22, 2024

laurentsimon left a comment

Choose a reason for hiding this comment

laurentsimon commented Jan 22, 2024 • edited Loading

saisatishkarra commented Jan 15, 2024 •

edited

Loading

ianlewis Jan 16, 2024 •

edited

Loading

laurentsimon Jan 19, 2024 •

edited

Loading

ianlewis Jan 23, 2024 •

edited

Loading

laurentsimon commented Jan 22, 2024 •

edited

Loading