From fd1b4786e5d1e5f8a458f9767c1d5f1d89bfbb46 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 18:50:16 +0000 Subject: [PATCH 01/20] update --- cli/slsa-verifier/main_test.go | 71 ++++++++++++++------ cli/slsa-verifier/verify/verify_image.go | 2 +- verifiers/internal/gcb/provenance.go | 40 +++++++---- verifiers/internal/gcb/provenance_test.go | 52 ++++++++++++-- verifiers/internal/gcb/verifier.go | 5 +- verifiers/internal/gha/verifier.go | 2 +- verifiers/{ => utils}/container/container.go | 0 verifiers/{ => utils}/container/cosign.go | 0 8 files changed, 129 insertions(+), 43 deletions(-) rename verifiers/{ => utils}/container/container.go (100%) rename verifiers/{ => utils}/container/cosign.go (100%) diff --git a/cli/slsa-verifier/main_test.go b/cli/slsa-verifier/main_test.go index 073578d54..c432ff1b1 100644 --- a/cli/slsa-verifier/main_test.go +++ b/cli/slsa-verifier/main_test.go @@ -21,7 +21,8 @@ import ( "github.com/slsa-framework/slsa-verifier/cli/slsa-verifier/verify" serrors "github.com/slsa-framework/slsa-verifier/errors" - "github.com/slsa-framework/slsa-verifier/verifiers/container" + "github.com/slsa-framework/slsa-verifier/verifiers/utils" + "github.com/slsa-framework/slsa-verifier/verifiers/utils/container" ) func errCmp(e1, e2 error) bool { @@ -845,6 +846,7 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { pBuilderID: pString(builder + "@v0.2"), err: serrors.ErrorMutableImage, }, + // TODO: add wrong builder ID for all versions. } for _, tt := range tests { tt := tt // Re-initializing variable so it is not changed while executing the closure below @@ -858,7 +860,10 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { for _, v := range checkVersions { semver := path.Base(v) - builderID := pString(builder + "@" + semver) + // For each test, we run 2 sub-tests: + // 1. With the the full builderID including the semver + // 2. WIth only the name of the builder. + builderIDs := []string{builder + "@" + semver, builder} provenance := filepath.Clean(filepath.Join(TEST_DIR, v, tt.provenance)) image := tt.artifact var fn verify.ComputeDigestFn @@ -868,7 +873,7 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { if !tt.noversion { panic("builderID set but not noversion option") } - builderID = tt.pBuilderID + builderIDs = []string{*tt.pBuilderID} } // Select the right image according to the builder version we are testing. @@ -889,28 +894,52 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { fn = localDigestComputeFn } - cmd := verify.VerifyImageCommand{ - SourceURI: tt.source, - SourceBranch: nil, - BuilderID: builderID, - SourceTag: nil, - SourceVersionTag: nil, - DigestFn: fn, - ProvenancePath: &provenance, - } + // We run the test for each builderID, in order to test + // a builderID provided by name and one containing both the name + // and semver. + for _, bid := range builderIDs { + cmd := verify.VerifyImageCommand{ + SourceURI: tt.source, + SourceBranch: nil, + BuilderID: &bid, + SourceTag: nil, + SourceVersionTag: nil, + DigestFn: fn, + ProvenancePath: &provenance, + } - outBuilderID, err := cmd.Exec(context.Background(), []string{image}) + outBuilderID, err := cmd.Exec(context.Background(), []string{image}) - if !errCmp(err, tt.err) { - t.Errorf(cmp.Diff(err, tt.err, cmpopts.EquateErrors())) - } + if !errCmp(err, tt.err) { + t.Errorf(cmp.Diff(err, tt.err, cmpopts.EquateErrors())) + } - if err != nil { - return - } + if err != nil { + return + } + + // Validate against test's expected builderID, if provided. + if tt.outBuilderID != "" && outBuilderID != tt.outBuilderID { + t.Errorf(cmp.Diff(outBuilderID, tt.outBuilderID)) + } + + // Validate against builderID we generated automatically. + expectedName, expectedVersion, err := utils.ParseBuilderID(bid, false) + if err != nil { + panic(fmt.Errorf("ParseBuilderID: %w: %s", err, bid)) + } + builderName, builderVersion, err := utils.ParseBuilderID(outBuilderID, true) + if err != nil { + panic(fmt.Errorf("ParseBuilderID: %w: %s", err, outBuilderID)) + } + + if expectedName != builderName { + t.Errorf(cmp.Diff(expectedName, builderName)) + } + if expectedVersion != "" && expectedVersion != builderVersion { + t.Errorf(cmp.Diff(expectedVersion, builderVersion)) + } - if tt.outBuilderID != "" && outBuilderID != tt.outBuilderID { - t.Errorf(cmp.Diff(outBuilderID, tt.outBuilderID)) } } diff --git a/cli/slsa-verifier/verify/verify_image.go b/cli/slsa-verifier/verify/verify_image.go index 064a4df2b..499fa518f 100644 --- a/cli/slsa-verifier/verify/verify_image.go +++ b/cli/slsa-verifier/verify/verify_image.go @@ -21,7 +21,7 @@ import ( "github.com/slsa-framework/slsa-verifier/options" "github.com/slsa-framework/slsa-verifier/verifiers" - "github.com/slsa-framework/slsa-verifier/verifiers/container" + "github.com/slsa-framework/slsa-verifier/verifiers/utils/container" ) type ComputeDigestFn func(string) (string, error) diff --git a/verifiers/internal/gcb/provenance.go b/verifiers/internal/gcb/provenance.go index 19dba9ad1..cd6539f3d 100644 --- a/verifiers/internal/gcb/provenance.go +++ b/verifiers/internal/gcb/provenance.go @@ -19,6 +19,7 @@ import ( serrors "github.com/slsa-framework/slsa-verifier/errors" "github.com/slsa-framework/slsa-verifier/options" "github.com/slsa-framework/slsa-verifier/verifiers/internal/gcb/keys" + "github.com/slsa-framework/slsa-verifier/verifiers/utils" ) var GCBBuilderIDs = []string{ @@ -221,16 +222,8 @@ func isValidBuilderID(id string) error { return serrors.ErrorInvalidBuilderID } -func getBuilderVersion(builderID string) (string, error) { - parts := strings.Split(builderID, "@") - if len(parts) != 2 { - return "", fmt.Errorf("%w: '%s'", serrors.ErrorInvalidFormat, parts) - } - return parts[1], nil -} - func validateRecipeType(builderID, recipeType string) error { - v, err := getBuilderVersion(builderID) + _, v, err := utils.ParseBuilderID(builderID, true) if err != nil { return err } @@ -270,7 +263,10 @@ func validateRecipeType(builderID, recipeType string) error { // VerifyBuilder verifies the builder in the DSSE payload: // - in the recipe type // - the recipe argument type -// - the predicate builder ID +// - the predicate builder ID. +// +// The builder ID may be of the form `name@vx.y.z` or `name`. When the version is omitted, we accept any +// version. When a version is provided, it must be an exact match. func (self *Provenance) VerifyBuilder(builderOpts *options.BuilderOpts) (string, error) { if err := self.isVerified(); err != nil { return "", err @@ -286,9 +282,25 @@ func (self *Provenance) VerifyBuilder(builderOpts *options.BuilderOpts) (string, // Validate with user-provided value. if builderOpts != nil && builderOpts.ExpectedID != nil { - if *builderOpts.ExpectedID != predicateBuilderID { - return "", fmt.Errorf("%w: expected '%s', got '%s'", serrors.ErrorMismatchBuilderID, - *builderOpts.ExpectedID, predicateBuilderID) + expectedName, expectedVersion, err := utils.ParseBuilderID(*builderOpts.ExpectedID, false) + if err != nil { + return "", err + } + builderName, builderVersion, err := utils.ParseBuilderID(predicateBuilderID, true) + if err != nil { + return "", err + } + + // The builder name must always match. + if expectedName != builderName { + return "", fmt.Errorf("%w: expected name '%s', got '%s'", serrors.ErrorMismatchBuilderID, + builderName, builderName) + } + + // The builder version must match if the user explicitely provided it. + if expectedVersion != "" && expectedVersion != builderVersion { + return "", fmt.Errorf("%w: expected version '%s', got '%s'", serrors.ErrorMismatchBuilderID, + expectedVersion, builderVersion) } } @@ -366,7 +378,7 @@ func (self *Provenance) VerifySourceURI(expectedSourceURI, builderID string) err expectedSourceURI = "https://" + expectedSourceURI } - v, err := getBuilderVersion(builderID) + _, v, err := utils.ParseBuilderID(builderID, true) if err != nil { return err } diff --git a/verifiers/internal/gcb/provenance_test.go b/verifiers/internal/gcb/provenance_test.go index a6095249c..dae2321bc 100644 --- a/verifiers/internal/gcb/provenance_test.go +++ b/verifiers/internal/gcb/provenance_test.go @@ -7,13 +7,12 @@ import ( "strings" "testing" - //"time" - "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" serrors "github.com/slsa-framework/slsa-verifier/errors" "github.com/slsa-framework/slsa-verifier/options" + "github.com/slsa-framework/slsa-verifier/verifiers/utils" ) // This function sets the statement of the proveannce, as if @@ -158,6 +157,39 @@ func Test_VerifyBuilder(t *testing.T) { builderID: "https://cloudbuild.googleapis.com/GoogleHostedWorker@v0.3", expected: serrors.ErrorInvalidRecipe, }, + { + name: "v0.2 valid builder - name only", + path: "./testdata/gcloud-container-github.json", + builderID: "https://cloudbuild.googleapis.com/GoogleHostedWorker", + }, + { + name: "v0.2 mismatch builder - name only", + path: "./testdata/gcloud-container-github.json", + builderID: "https://cloudbuild.googleapis.com/GoogleHostedWorke", + expected: serrors.ErrorMismatchBuilderID, + }, + { + name: "v0.3 valid builder CloudBuildSteps - name only", + path: "./testdata/gcloud-container-invalid-recipetypestepsv03.json", + builderID: "https://cloudbuild.googleapis.com/GoogleHostedWorker", + }, + { + name: "v0.3 valid builder CloudBuildYaml - name only", + path: "./testdata/gcloud-container-invalid-recipetypecloudv03.json", + builderID: "https://cloudbuild.googleapis.com/GoogleHostedWorker", + }, + { + name: "v0.3 mismatch builder CloudBuildSteps - name only", + path: "./testdata/gcloud-container-invalid-recipetypestepsv03.json", + builderID: "https://cloudbuild.googleapis.com/GoogleHostedWorke", + expected: serrors.ErrorMismatchBuilderID, + }, + { + name: "v0.3 mismatch CloudBuildYaml - name only", + path: "./testdata/gcloud-container-invalid-recipetypecloudv03.json", + builderID: "https://cloudbuild.googleapis.com/GoogleHostedWorke", + expected: serrors.ErrorMismatchBuilderID, + }, } for _, tt := range tests { tt := tt // Re-initializing variable so it is not changed while executing the closure below @@ -191,8 +223,20 @@ func Test_VerifyBuilder(t *testing.T) { return } - if outBuilderID != tt.builderID { - t.Errorf(cmp.Diff(outBuilderID, tt.builderID)) + expectedName, expectedVersion, err := utils.ParseBuilderID(tt.builderID, false) + if err != nil { + panic(fmt.Errorf("ParseBuilderID: %w: %s", err, tt.builderID)) + } + builderName, builderVersion, err := utils.ParseBuilderID(outBuilderID, true) + if err != nil { + panic(fmt.Errorf("ParseBuilderID: %w: %s", err, outBuilderID)) + } + + if expectedName != builderName { + t.Errorf(cmp.Diff(expectedName, builderName)) + } + if expectedVersion != "" && expectedVersion != builderVersion { + t.Errorf(cmp.Diff(expectedVersion, builderVersion)) } }) } diff --git a/verifiers/internal/gcb/verifier.go b/verifiers/internal/gcb/verifier.go index 083236287..3ac392e4e 100644 --- a/verifiers/internal/gcb/verifier.go +++ b/verifiers/internal/gcb/verifier.go @@ -2,12 +2,12 @@ package gcb import ( "context" - "strings" serrors "github.com/slsa-framework/slsa-verifier/errors" "github.com/slsa-framework/slsa-verifier/options" register "github.com/slsa-framework/slsa-verifier/register" _ "github.com/slsa-framework/slsa-verifier/verifiers/internal/gcb/keys" + "github.com/slsa-framework/slsa-verifier/verifiers/utils" ) const VerifierName = "GCB" @@ -27,7 +27,8 @@ func GCBVerifierNew() *GCBVerifier { // generated by the builderID. func (v *GCBVerifier) IsAuthoritativeFor(builderID string) bool { // This verifier only supports the GCB builders. - return strings.HasPrefix(builderID, "https://cloudbuild.googleapis.com/GoogleHostedWorker@") + name, _, _ := utils.ParseBuilderID(builderID, false) + return name == "https://cloudbuild.googleapis.com/GoogleHostedWorker" } // VerifyArtifact verifies provenance for an artifact. diff --git a/verifiers/internal/gha/verifier.go b/verifiers/internal/gha/verifier.go index e3a7896f8..51dd59b4c 100644 --- a/verifiers/internal/gha/verifier.go +++ b/verifiers/internal/gha/verifier.go @@ -15,7 +15,7 @@ import ( "github.com/slsa-framework/slsa-verifier/options" "github.com/slsa-framework/slsa-verifier/register" - "github.com/slsa-framework/slsa-verifier/verifiers/container" + "github.com/slsa-framework/slsa-verifier/verifiers/utils/container" ) const VerifierName = "GHA" diff --git a/verifiers/container/container.go b/verifiers/utils/container/container.go similarity index 100% rename from verifiers/container/container.go rename to verifiers/utils/container/container.go diff --git a/verifiers/container/cosign.go b/verifiers/utils/container/cosign.go similarity index 100% rename from verifiers/container/cosign.go rename to verifiers/utils/container/cosign.go From 8a9838c39a6d6e96b8133fccdea9c6615bb13b14 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 18:52:18 +0000 Subject: [PATCH 02/20] update --- cli/slsa-verifier/main_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/slsa-verifier/main_test.go b/cli/slsa-verifier/main_test.go index c432ff1b1..5b9447ef0 100644 --- a/cli/slsa-verifier/main_test.go +++ b/cli/slsa-verifier/main_test.go @@ -861,8 +861,8 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { for _, v := range checkVersions { semver := path.Base(v) // For each test, we run 2 sub-tests: - // 1. With the the full builderID including the semver - // 2. WIth only the name of the builder. + // 1. With the the full builderID including the semver. + // 2. With only the name of the builder. builderIDs := []string{builder + "@" + semver, builder} provenance := filepath.Clean(filepath.Join(TEST_DIR, v, tt.provenance)) image := tt.artifact From fe8c9f8632f015e36091e4c9baf22e61a5174a84 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 18:53:17 +0000 Subject: [PATCH 03/20] update --- verifiers/internal/gcb/verifier.go | 1 + 1 file changed, 1 insertion(+) diff --git a/verifiers/internal/gcb/verifier.go b/verifiers/internal/gcb/verifier.go index 3ac392e4e..99585215b 100644 --- a/verifiers/internal/gcb/verifier.go +++ b/verifiers/internal/gcb/verifier.go @@ -27,6 +27,7 @@ func GCBVerifierNew() *GCBVerifier { // generated by the builderID. func (v *GCBVerifier) IsAuthoritativeFor(builderID string) bool { // This verifier only supports the GCB builders. + // Note: `ParseBuilderID` never fails with `needVersion=false`. name, _, _ := utils.ParseBuilderID(builderID, false) return name == "https://cloudbuild.googleapis.com/GoogleHostedWorker" } From 846dcaf313ece2c035e30373b96f90d6e94a5a31 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 18:54:08 +0000 Subject: [PATCH 04/20] update --- cli/slsa-verifier/main_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/slsa-verifier/main_test.go b/cli/slsa-verifier/main_test.go index 5b9447ef0..0828a7670 100644 --- a/cli/slsa-verifier/main_test.go +++ b/cli/slsa-verifier/main_test.go @@ -846,7 +846,6 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { pBuilderID: pString(builder + "@v0.2"), err: serrors.ErrorMutableImage, }, - // TODO: add wrong builder ID for all versions. } for _, tt := range tests { tt := tt // Re-initializing variable so it is not changed while executing the closure below From 39f370fee2e592d6ac0936c3b876c875fcaad767 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 18:58:03 +0000 Subject: [PATCH 05/20] update --- verifiers/utils/builder.go | 22 +++++++++++ verifiers/utils/builder_test.go | 70 +++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 verifiers/utils/builder.go create mode 100644 verifiers/utils/builder_test.go diff --git a/verifiers/utils/builder.go b/verifiers/utils/builder.go new file mode 100644 index 000000000..7aee6f68e --- /dev/null +++ b/verifiers/utils/builder.go @@ -0,0 +1,22 @@ +package utils + +import ( + "fmt" + "strings" + + serrors "github.com/slsa-framework/slsa-verifier/errors" +) + +func ParseBuilderID(id string, needVersion bool) (string, string, error) { + parts := strings.Split(id, "@") + if len(parts) == 2 { + return parts[0], parts[1], nil + } + + if len(parts) == 1 && !needVersion { + return parts[0], "", nil + } + + return "", "", fmt.Errorf("%w: builderID: '%s'", + serrors.ErrorInvalidFormat, id) +} diff --git a/verifiers/utils/builder_test.go b/verifiers/utils/builder_test.go new file mode 100644 index 000000000..67a4700a1 --- /dev/null +++ b/verifiers/utils/builder_test.go @@ -0,0 +1,70 @@ +package utils + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + + serrors "github.com/slsa-framework/slsa-verifier/errors" +) + +func Test_ParseBuilderID(t *testing.T) { + t.Parallel() + tests := []struct { + name string + builderID string + needVersion bool + builderName string + builderVersion string + err error + }{ + { + name: "valid builder with version / need version", + builderID: "some/name@v1.2.3", + needVersion: true, + builderName: "some/name", + builderVersion: "v1.2.3", + }, + { + name: "valid builder with version / no need version", + builderID: "some/name@v1.2.3", + builderName: "some/name", + builderVersion: "v1.2.3", + }, + { + name: "valid builder without version / no need version", + builderID: "some/name", + builderName: "some/name", + }, + { + name: "no version ID need version", + builderID: "some/name", + needVersion: true, + err: serrors.ErrorInvalidFormat, + }, + } + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + name, version, err := ParseBuilderID(tt.builderID, tt.needVersion) + if !cmp.Equal(err, tt.err, cmpopts.EquateErrors()) { + t.Errorf(cmp.Diff(err, tt.err)) + } + + if err != nil { + return + } + + if name != tt.builderName { + t.Errorf(cmp.Diff(name, tt.builderName)) + } + + if version != tt.builderVersion { + t.Errorf(cmp.Diff(version, tt.builderVersion)) + } + }) + } +} From 614b945ca1183fac2a043b683cfb5ec6ddfce9a8 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 18:59:59 +0000 Subject: [PATCH 06/20] update --- verifiers/utils/builder_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/verifiers/utils/builder_test.go b/verifiers/utils/builder_test.go index 67a4700a1..6f92f2656 100644 --- a/verifiers/utils/builder_test.go +++ b/verifiers/utils/builder_test.go @@ -39,10 +39,20 @@ func Test_ParseBuilderID(t *testing.T) { }, { name: "no version ID need version", - builderID: "some/name", needVersion: true, err: serrors.ErrorInvalidFormat, }, + { + name: "too many '@' - need version", + builderID: "some/name@vla@blo", + needVersion: true, + err: serrors.ErrorInvalidFormat, + }, + { + name: "too many '@' - no need version", + builderID: "some/name@vla@blo", + err: serrors.ErrorInvalidFormat, + }, } for _, tt := range tests { tt := tt // Re-initializing variable so it is not changed while executing the closure below From 6d2ce3cb73780591755c0c7b0c2a50983a2c28da Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 19:00:52 +0000 Subject: [PATCH 07/20] update --- verifiers/utils/builder_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/verifiers/utils/builder_test.go b/verifiers/utils/builder_test.go index 6f92f2656..aae518b9b 100644 --- a/verifiers/utils/builder_test.go +++ b/verifiers/utils/builder_test.go @@ -20,25 +20,25 @@ func Test_ParseBuilderID(t *testing.T) { err error }{ { - name: "valid builder with version / need version", + name: "valid builder with version - need version", builderID: "some/name@v1.2.3", needVersion: true, builderName: "some/name", builderVersion: "v1.2.3", }, { - name: "valid builder with version / no need version", + name: "valid builder with version - no need version", builderID: "some/name@v1.2.3", builderName: "some/name", builderVersion: "v1.2.3", }, { - name: "valid builder without version / no need version", + name: "valid builder without version - no need version", builderID: "some/name", builderName: "some/name", }, { - name: "no version ID need version", + name: "no version ID - need version", needVersion: true, err: serrors.ErrorInvalidFormat, }, From 97e6a8fbd2621a4f4bed447d11a02ceedee2f9eb Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 20:23:59 +0000 Subject: [PATCH 08/20] update --- cli/slsa-verifier/main_test.go | 36 +++++--------- cli/slsa-verifier/verify/verify_artifact.go | 9 ++-- cli/slsa-verifier/verify/verify_image.go | 11 +++-- register/register.go | 5 +- verifiers/internal/gcb/provenance.go | 54 +++++++-------------- verifiers/internal/gcb/provenance_test.go | 16 ++---- verifiers/internal/gcb/verifier.go | 34 ++++++------- verifiers/internal/gha/verifier.go | 40 +++++++++------ verifiers/utils/builder.go | 49 +++++++++++++++++++ verifiers/verifier.go | 9 ++-- 10 files changed, 146 insertions(+), 117 deletions(-) diff --git a/cli/slsa-verifier/main_test.go b/cli/slsa-verifier/main_test.go index 0828a7670..a79cc1ce9 100644 --- a/cli/slsa-verifier/main_test.go +++ b/cli/slsa-verifier/main_test.go @@ -21,7 +21,6 @@ import ( "github.com/slsa-framework/slsa-verifier/cli/slsa-verifier/verify" serrors "github.com/slsa-framework/slsa-verifier/errors" - "github.com/slsa-framework/slsa-verifier/verifiers/utils" "github.com/slsa-framework/slsa-verifier/verifiers/utils/container" ) @@ -512,10 +511,12 @@ func Test_runVerifyArtifactPath(t *testing.T) { return } - if tt.outBuilderID != "" && outBuilderID != tt.outBuilderID { - t.Errorf(cmp.Diff(outBuilderID, tt.outBuilderID)) + // Validate against test's expected builderID, if provided. + if tt.outBuilderID != "" && tt.outBuilderID != outBuilderID.String() { + t.Errorf(cmp.Diff(tt.outBuilderID, outBuilderID.String())) } + // TODO: verify using Matches(). } }) } @@ -676,9 +677,12 @@ func Test_runVerifyGHAArtifactImage(t *testing.T) { return } - if tt.outBuilderID != "" && outBuilderID != tt.outBuilderID { - t.Errorf(cmp.Diff(outBuilderID, tt.outBuilderID)) + // Validate against test's expected builderID, if provided. + if tt.outBuilderID != "" && tt.outBuilderID != outBuilderID.String() { + t.Errorf(cmp.Diff(tt.outBuilderID, outBuilderID.String())) } + + // TODO: verify using Matches(). } }) } @@ -918,29 +922,15 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { } // Validate against test's expected builderID, if provided. - if tt.outBuilderID != "" && outBuilderID != tt.outBuilderID { - t.Errorf(cmp.Diff(outBuilderID, tt.outBuilderID)) + if tt.outBuilderID != "" && tt.outBuilderID != outBuilderID.String() { + t.Errorf(cmp.Diff(tt.outBuilderID, outBuilderID.String())) } // Validate against builderID we generated automatically. - expectedName, expectedVersion, err := utils.ParseBuilderID(bid, false) - if err != nil { - panic(fmt.Errorf("ParseBuilderID: %w: %s", err, bid)) + if err := outBuilderID.Matches(bid); err != nil { + t.Errorf(fmt.Sprintf("matches failed: %v", err)) } - builderName, builderVersion, err := utils.ParseBuilderID(outBuilderID, true) - if err != nil { - panic(fmt.Errorf("ParseBuilderID: %w: %s", err, outBuilderID)) - } - - if expectedName != builderName { - t.Errorf(cmp.Diff(expectedName, builderName)) - } - if expectedVersion != "" && expectedVersion != builderVersion { - t.Errorf(cmp.Diff(expectedVersion, builderVersion)) - } - } - } }) } diff --git a/cli/slsa-verifier/verify/verify_artifact.go b/cli/slsa-verifier/verify/verify_artifact.go index 6c40e58bd..16ad8bd2d 100644 --- a/cli/slsa-verifier/verify/verify_artifact.go +++ b/cli/slsa-verifier/verify/verify_artifact.go @@ -24,6 +24,7 @@ import ( "github.com/slsa-framework/slsa-verifier/options" "github.com/slsa-framework/slsa-verifier/verifiers" + "github.com/slsa-framework/slsa-verifier/verifiers/utils" ) // Note: nil branch, tag, version-tag and builder-id means we ignore them during verification. @@ -38,10 +39,10 @@ type VerifyArtifactCommand struct { PrintProvenance bool } -func (c *VerifyArtifactCommand) Exec(ctx context.Context, artifacts []string) (string, error) { +func (c *VerifyArtifactCommand) Exec(ctx context.Context, artifacts []string) (*utils.BuilderID, error) { artifactHash, err := getArtifactHash(artifacts[0]) if err != nil { - return "", err + return nil, err } provenanceOpts := &options.ProvenanceOpts{ @@ -59,12 +60,12 @@ func (c *VerifyArtifactCommand) Exec(ctx context.Context, artifacts []string) (s provenance, err := os.ReadFile(c.ProvenancePath) if err != nil { - return "", err + return nil, err } verifiedProvenance, outBuilderID, err := verifiers.VerifyArtifact(ctx, provenance, artifactHash, provenanceOpts, builderOpts) if err != nil { - return "", err + return nil, err } if c.PrintProvenance { diff --git a/cli/slsa-verifier/verify/verify_image.go b/cli/slsa-verifier/verify/verify_image.go index 499fa518f..489b2c792 100644 --- a/cli/slsa-verifier/verify/verify_image.go +++ b/cli/slsa-verifier/verify/verify_image.go @@ -21,6 +21,7 @@ import ( "github.com/slsa-framework/slsa-verifier/options" "github.com/slsa-framework/slsa-verifier/verifiers" + "github.com/slsa-framework/slsa-verifier/verifiers/utils" "github.com/slsa-framework/slsa-verifier/verifiers/utils/container" ) @@ -40,7 +41,7 @@ type VerifyImageCommand struct { DigestFn ComputeDigestFn } -func (c *VerifyImageCommand) Exec(ctx context.Context, artifacts []string) (string, error) { +func (c *VerifyImageCommand) Exec(ctx context.Context, artifacts []string) (*utils.BuilderID, error) { artifactImage := artifacts[0] // Retrieve the image digest. if c.DigestFn == nil { @@ -48,12 +49,12 @@ func (c *VerifyImageCommand) Exec(ctx context.Context, artifacts []string) (stri } digest, err := c.DigestFn(artifactImage) if err != nil { - return "", err + return nil, err } // Verify that the reference is immutable. if err := container.ValidateArtifactReference(artifactImage, digest); err != nil { - return "", err + return nil, err } provenanceOpts := &options.ProvenanceOpts{ @@ -73,13 +74,13 @@ func (c *VerifyImageCommand) Exec(ctx context.Context, artifacts []string) (stri if c.ProvenancePath != nil { provenance, err = os.ReadFile(*c.ProvenancePath) if err != nil { - return "", err + return nil, err } } verifiedProvenance, outBuilderID, err := verifiers.VerifyImage(ctx, artifacts[0], provenance, provenanceOpts, builderOpts) if err != nil { - return "", err + return nil, err } if c.PrintProvenance { diff --git a/register/register.go b/register/register.go index 32622cbb1..ca2fee770 100644 --- a/register/register.go +++ b/register/register.go @@ -4,6 +4,7 @@ import ( "context" "github.com/slsa-framework/slsa-verifier/options" + "github.com/slsa-framework/slsa-verifier/verifiers/utils" ) var SLSAVerifiers = make(map[string]SLSAVerifier) @@ -19,14 +20,14 @@ type SLSAVerifier interface { provenance []byte, artifactHash string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, - ) ([]byte, string, error) + ) ([]byte, *utils.BuilderID, error) // VerifyImage verifies a provenance for a supplied OCI image. VerifyImage(ctx context.Context, provenance []byte, artifactImage string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, - ) ([]byte, string, error) + ) ([]byte, *utils.BuilderID, error) } func RegisterVerifier(name string, verifier SLSAVerifier) { diff --git a/verifiers/internal/gcb/provenance.go b/verifiers/internal/gcb/provenance.go index cd6539f3d..021b63170 100644 --- a/verifiers/internal/gcb/provenance.go +++ b/verifiers/internal/gcb/provenance.go @@ -264,12 +264,9 @@ func validateRecipeType(builderID, recipeType string) error { // - in the recipe type // - the recipe argument type // - the predicate builder ID. -// -// The builder ID may be of the form `name@vx.y.z` or `name`. When the version is omitted, we accept any -// version. When a version is provided, it must be an exact match. -func (self *Provenance) VerifyBuilder(builderOpts *options.BuilderOpts) (string, error) { +func (self *Provenance) VerifyBuilder(builderOpts *options.BuilderOpts) (*utils.BuilderID, error) { if err := self.isVerified(); err != nil { - return "", err + return nil, err } statement := self.verifiedIntotoStatement @@ -277,55 +274,43 @@ func (self *Provenance) VerifyBuilder(builderOpts *options.BuilderOpts) (string, // Sanity check the builderID. if err := isValidBuilderID(predicateBuilderID); err != nil { - return "", err + return nil, err + } + + provBuilderID, err := utils.BuilderIDNew(predicateBuilderID) + if err != nil { + return nil, err } // Validate with user-provided value. if builderOpts != nil && builderOpts.ExpectedID != nil { - expectedName, expectedVersion, err := utils.ParseBuilderID(*builderOpts.ExpectedID, false) - if err != nil { - return "", err - } - builderName, builderVersion, err := utils.ParseBuilderID(predicateBuilderID, true) - if err != nil { - return "", err - } - - // The builder name must always match. - if expectedName != builderName { - return "", fmt.Errorf("%w: expected name '%s', got '%s'", serrors.ErrorMismatchBuilderID, - builderName, builderName) - } - - // The builder version must match if the user explicitely provided it. - if expectedVersion != "" && expectedVersion != builderVersion { - return "", fmt.Errorf("%w: expected version '%s', got '%s'", serrors.ErrorMismatchBuilderID, - expectedVersion, builderVersion) + if err := provBuilderID.Matches(*builderOpts.ExpectedID); err != nil { + return nil, err } } // Valiate the recipe type. if err := validateRecipeType(predicateBuilderID, statement.Predicate.Recipe.Type); err != nil { - return "", err + return nil, err } // Validate the recipe argument type. expectedType := "type.googleapis.com/google.devtools.cloudbuild.v1.Build" args, ok := statement.Predicate.Recipe.Arguments.(map[string]interface{}) if !ok { - return "", fmt.Errorf("%w: recipe arguments is not a map", serrors.ErrorInvalidDssePayload) + return nil, fmt.Errorf("%w: recipe arguments is not a map", serrors.ErrorInvalidDssePayload) } ts, err := getAsString(args, "@type") if err != nil { - return "", err + return nil, err } if ts != expectedType { - return "", fmt.Errorf("%w: expected '%s', got '%s'", serrors.ErrorMismatchBuilderID, + return nil, fmt.Errorf("%w: expected '%s', got '%s'", serrors.ErrorMismatchBuilderID, expectedType, ts) } - return predicateBuilderID, nil + return provBuilderID, nil } func getAsString(m map[string]interface{}, key string) (string, error) { @@ -363,7 +348,7 @@ func (self *Provenance) VerifySubjectDigest(expectedHash string) error { } // Verify source URI in provenance statement. -func (self *Provenance) VerifySourceURI(expectedSourceURI, builderID string) error { +func (self *Provenance) VerifySourceURI(expectedSourceURI string, builderID utils.BuilderID) error { if err := self.isVerified(); err != nil { return err } @@ -378,11 +363,8 @@ func (self *Provenance) VerifySourceURI(expectedSourceURI, builderID string) err expectedSourceURI = "https://" + expectedSourceURI } - _, v, err := utils.ParseBuilderID(builderID, true) - if err != nil { - return err - } - + var err error + v := builderID.Version() switch v { case "v0.2": // In v0.2, it uses format diff --git a/verifiers/internal/gcb/provenance_test.go b/verifiers/internal/gcb/provenance_test.go index dae2321bc..cb429f759 100644 --- a/verifiers/internal/gcb/provenance_test.go +++ b/verifiers/internal/gcb/provenance_test.go @@ -223,20 +223,12 @@ func Test_VerifyBuilder(t *testing.T) { return } - expectedName, expectedVersion, err := utils.ParseBuilderID(tt.builderID, false) - if err != nil { - panic(fmt.Errorf("ParseBuilderID: %w: %s", err, tt.builderID)) - } - builderName, builderVersion, err := utils.ParseBuilderID(outBuilderID, true) - if err != nil { - panic(fmt.Errorf("ParseBuilderID: %w: %s", err, outBuilderID)) + if outBuilderID == nil { + panic("outBuilderID is nil") } - if expectedName != builderName { - t.Errorf(cmp.Diff(expectedName, builderName)) - } - if expectedVersion != "" && expectedVersion != builderVersion { - t.Errorf(cmp.Diff(expectedVersion, builderVersion)) + if err := outBuilderID.Matches(tt.builderID); err != nil { + t.Errorf(fmt.Errorf("matches failed: %w", err)) } }) } diff --git a/verifiers/internal/gcb/verifier.go b/verifiers/internal/gcb/verifier.go index 99585215b..1a70f5494 100644 --- a/verifiers/internal/gcb/verifier.go +++ b/verifiers/internal/gcb/verifier.go @@ -37,8 +37,8 @@ func (v *GCBVerifier) VerifyArtifact(ctx context.Context, provenance []byte, artifactHash string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, -) ([]byte, string, error) { - return nil, "todo", serrors.ErrorNotSupported +) ([]byte, *utils.BuilderID, error) { + return nil, nil, serrors.ErrorNotSupported } // VerifyImage verifies provenance for an OCI image. @@ -46,81 +46,81 @@ func (v *GCBVerifier) VerifyImage(ctx context.Context, provenance []byte, artifactImage string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, -) ([]byte, string, error) { +) ([]byte, *utils.BuilderID, error) { prov, err := ProvenanceFromBytes(provenance) if err != nil { - return nil, "", err + return nil, nil, err } // Verify signature on the intoto attestation. if err = prov.VerifySignature(); err != nil { - return nil, "", err + return nil, nil, err } // Verify intoto header. if err = prov.VerifyIntotoHeaders(); err != nil { - return nil, "", err + return nil, nil, err } // Verify the builder. builderID, err := prov.VerifyBuilder(builderOpts) if err != nil { - return nil, "", err + return nil, nil, err } // Verify subject digest. if err = prov.VerifySubjectDigest(provenanceOpts.ExpectedDigest); err != nil { - return nil, "", err + return nil, nil, err } // Verify source. - if err = prov.VerifySourceURI(provenanceOpts.ExpectedSourceURI, builderID); err != nil { - return nil, "", err + if err = prov.VerifySourceURI(provenanceOpts.ExpectedSourceURI, *builderID); err != nil { + return nil, nil, err } // Verify metadata. // This is metadata that GCB appends to the DSSE content. if err = prov.VerifyMetadata(provenanceOpts); err != nil { - return nil, "", err + return nil, nil, err } // Verify the summary. // This is an additional structure that GCB prepends to the provenance. if err = prov.VerifySummary(provenanceOpts); err != nil { - return nil, "", err + return nil, nil, err } // Verify the text provenance. // This is an additional structure that GCB prepends to the provenance, // intended for humans. It reflect the DSSE payload. if err = prov.VerifyTextProvenance(); err != nil { - return nil, "", err + return nil, nil, err } // Verify branch. if provenanceOpts.ExpectedBranch != nil { if err = prov.VerifyBranch(*provenanceOpts.ExpectedBranch); err != nil { - return nil, "", err + return nil, nil, err } } // Verify the tag. if provenanceOpts.ExpectedTag != nil { if err := prov.VerifyTag(*provenanceOpts.ExpectedTag); err != nil { - return nil, "", err + return nil, nil, err } } // Verify the versioned tag. if provenanceOpts.ExpectedVersionedTag != nil { if err := prov.VerifyVersionedTag(*provenanceOpts.ExpectedVersionedTag); err != nil { - return nil, "", err + return nil, nil, err } } content, err := prov.GetVerifiedIntotoStatement() if err != nil { - return nil, "", err + return nil, nil, err } return content, builderID, nil } diff --git a/verifiers/internal/gha/verifier.go b/verifiers/internal/gha/verifier.go index 51dd59b4c..c16ca3e2a 100644 --- a/verifiers/internal/gha/verifier.go +++ b/verifiers/internal/gha/verifier.go @@ -15,6 +15,7 @@ import ( "github.com/slsa-framework/slsa-verifier/options" "github.com/slsa-framework/slsa-verifier/register" + "github.com/slsa-framework/slsa-verifier/verifiers/utils" "github.com/slsa-framework/slsa-verifier/verifiers/utils/container" ) @@ -43,26 +44,26 @@ func verifyEnvAndCert(env *dsse.Envelope, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, defaultBuilders map[string]bool, -) ([]byte, string, error) { +) ([]byte, *utils.BuilderID, error) { /* Verify properties of the signing identity. */ // Get the workflow info given the certificate information. workflowInfo, err := GetWorkflowInfoFromCertificate(cert) if err != nil { - return nil, "", err + return nil, nil, err } // Verify the workflow identity. builderID, err := VerifyWorkflowIdentity(workflowInfo, builderOpts, provenanceOpts.ExpectedSourceURI, defaultBuilders) if err != nil { - return nil, "", err + return nil, nil, err } // Verify properties of the SLSA provenance. // Unpack and verify info in the provenance, including the Subject Digest. provenanceOpts.ExpectedBuilderID = builderID if err := VerifyProvenance(env, provenanceOpts); err != nil { - return nil, "", err + return nil, nil, err } fmt.Fprintf(os.Stderr, "Verified build using builder https://github.com%s at commit %s\n", @@ -70,7 +71,13 @@ func verifyEnvAndCert(env *dsse.Envelope, workflowInfo.CallerHash) // Return verified provenance. r, err := base64.StdEncoding.DecodeString(env.Payload) - return r, builderID, err + if err != nil { + return nil, nil, err + } + + // Temporary code. + bid, _ := utils.BuilderIDNew(builderID) + return r, bid, nil } // VerifyArtifact verifies provenance for an artifact. @@ -78,21 +85,26 @@ func (v *GHAVerifier) VerifyArtifact(ctx context.Context, provenance []byte, artifactHash string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, -) ([]byte, string, error) { +) ([]byte, *utils.BuilderID, error) { rClient, err := rekor.NewClient(defaultRekorAddr) if err != nil { - return nil, "", err + return nil, nil, err } /* Verify signature on the intoto attestation. */ env, cert, err := VerifyProvenanceSignature(ctx, rClient, provenance, artifactHash) if err != nil { - return nil, "", err + return nil, nil, err } - return verifyEnvAndCert(env, cert, + content, builderID, err := verifyEnvAndCert(env, cert, provenanceOpts, builderOpts, defaultArtifactTrustedReusableWorkflows) + if err != nil { + return nil, nil, err + } + + return content, builderID, nil } // VerifyImage verifies provenance for an OCI image. @@ -100,11 +112,11 @@ func (v *GHAVerifier) VerifyImage(ctx context.Context, provenance []byte, artifactImage string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, -) ([]byte, string, error) { +) ([]byte, *utils.BuilderID, error) { /* Retrieve any valid signed attestations that chain up to Fulcio root CA. */ roots, err := fulcio.GetRoots() if err != nil { - return nil, "", err + return nil, nil, err } opts := &cosign.CheckOpts{ RootCerts: roots, @@ -113,12 +125,12 @@ func (v *GHAVerifier) VerifyImage(ctx context.Context, atts, _, err := container.RunCosignImageVerification(ctx, artifactImage, opts) if err != nil { - return nil, "", err + return nil, nil, err } /* Now verify properties of the attestations */ var verifyErr error - var builderID string + var builderID *utils.BuilderID var verifiedProvenance []byte for _, att := range atts { pyld, err := att.Payload() @@ -144,5 +156,5 @@ func (v *GHAVerifier) VerifyImage(ctx context.Context, } } - return nil, "", fmt.Errorf("no valid attestations found on OCI registry: %w", verifyErr) + return nil, nil, fmt.Errorf("no valid attestations found on OCI registry: %w", verifyErr) } diff --git a/verifiers/utils/builder.go b/verifiers/utils/builder.go index 7aee6f68e..2ec5857b2 100644 --- a/verifiers/utils/builder.go +++ b/verifiers/utils/builder.go @@ -7,6 +7,55 @@ import ( serrors "github.com/slsa-framework/slsa-verifier/errors" ) +type BuilderID struct { + name, version string +} + +// BuilderIDNew creates a new BuilderID structure. +func BuilderIDNew(builderID string) (*BuilderID, error) { + name, version, err := ParseBuilderID(builderID, true) + if err != nil { + return nil, err + } + return &BuilderID{ + name: name, + version: version, + }, nil +} + +// Matches matches the builderID string against the reference builderID. +// If the builderID contains a semver, the full builderID must match. +// Otherwise, only the name needs to match. +func (b *BuilderID) Matches(builderID string) error { + name, version, err := ParseBuilderID(builderID, false) + if err != nil { + return err + } + if name != b.name { + return fmt.Errorf("%w: expected name '%s', got '%s'", serrors.ErrorMismatchBuilderID, + name, b.name) + } + + if version != "" && version != b.version { + fmt.Errorf("%w: expected version '%s', got '%s'", serrors.ErrorMismatchBuilderID, + version, b.version) + } + + return nil +} + +func (b *BuilderID) Name() string { + return b.name +} + +func (b *BuilderID) Version() string { + return b.version +} + +func (b *BuilderID) String() string { + return fmt.Sprintf("%s@%s", b.name, b.version) +} + func ParseBuilderID(id string, needVersion bool) (string, string, error) { parts := strings.Split(id, "@") if len(parts) == 2 { diff --git a/verifiers/verifier.go b/verifiers/verifier.go index 77706793b..8278bf50f 100644 --- a/verifiers/verifier.go +++ b/verifiers/verifier.go @@ -9,6 +9,7 @@ import ( "github.com/slsa-framework/slsa-verifier/register" _ "github.com/slsa-framework/slsa-verifier/verifiers/internal/gcb" "github.com/slsa-framework/slsa-verifier/verifiers/internal/gha" + "github.com/slsa-framework/slsa-verifier/verifiers/utils" ) func getVerifier(builderOpts *options.BuilderOpts) (register.SLSAVerifier, error) { @@ -34,10 +35,10 @@ func VerifyImage(ctx context.Context, artifactImage string, provenance []byte, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, -) ([]byte, string, error) { +) ([]byte, *utils.BuilderID, error) { verifier, err := getVerifier(builderOpts) if err != nil { - return nil, "", err + return nil, nil, err } return verifier.VerifyImage(ctx, provenance, artifactImage, provenanceOpts, builderOpts) @@ -47,10 +48,10 @@ func VerifyArtifact(ctx context.Context, provenance []byte, artifactHash string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, -) ([]byte, string, error) { +) ([]byte, *utils.BuilderID, error) { verifier, err := getVerifier(builderOpts) if err != nil { - return nil, "", err + return nil, nil, err } return verifier.VerifyArtifact(ctx, provenance, artifactHash, From 58eb872148a794362e6ead902164b05d09f34780 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 20:27:37 +0000 Subject: [PATCH 09/20] update --- register/register.go | 2 +- verifiers/internal/gcb/verifier.go | 6 ++---- verifiers/verifier.go | 6 +++++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/register/register.go b/register/register.go index ca2fee770..a7fd2f630 100644 --- a/register/register.go +++ b/register/register.go @@ -13,7 +13,7 @@ type SLSAVerifier interface { // IsAuthoritativeFor checks whether a verifier can // verify provenance for a given builder identified by its // `BuilderID`. - IsAuthoritativeFor(builderID string) bool + IsAuthoritativeFor(builderIDName string) bool // VerifyArtifact verifies a provenance for a supplied artifact. VerifyArtifact(ctx context.Context, diff --git a/verifiers/internal/gcb/verifier.go b/verifiers/internal/gcb/verifier.go index 1a70f5494..fbe727d0c 100644 --- a/verifiers/internal/gcb/verifier.go +++ b/verifiers/internal/gcb/verifier.go @@ -25,11 +25,9 @@ func GCBVerifierNew() *GCBVerifier { // IsAuthoritativeFor returns true of the verifier can verify provenance // generated by the builderID. -func (v *GCBVerifier) IsAuthoritativeFor(builderID string) bool { +func (v *GCBVerifier) IsAuthoritativeFor(builderIDName string) bool { // This verifier only supports the GCB builders. - // Note: `ParseBuilderID` never fails with `needVersion=false`. - name, _, _ := utils.ParseBuilderID(builderID, false) - return name == "https://cloudbuild.googleapis.com/GoogleHostedWorker" + return builderIDName == "https://cloudbuild.googleapis.com/GoogleHostedWorker" } // VerifyArtifact verifies provenance for an artifact. diff --git a/verifiers/verifier.go b/verifiers/verifier.go index 8278bf50f..88a8b327b 100644 --- a/verifiers/verifier.go +++ b/verifiers/verifier.go @@ -19,8 +19,12 @@ func getVerifier(builderOpts *options.BuilderOpts) (register.SLSAVerifier, error // If user provids a builderID, find the right verifier based on its ID. if builderOpts.ExpectedID != nil && *builderOpts.ExpectedID != "" { + name, _, err := utils.ParseBuilderID(*builderOpts.ExpectedID, false) + if err != nil { + return nil, err + } for _, v := range register.SLSAVerifiers { - if v.IsAuthoritativeFor(*builderOpts.ExpectedID) { + if v.IsAuthoritativeFor(name) { return v, nil } } From a4df6cdfd764ebc135594921477faff2836694af Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 20:32:29 +0000 Subject: [PATCH 10/20] update --- verifiers/internal/gcb/provenance.go | 14 ++++++-------- verifiers/internal/gcb/provenance_test.go | 6 +++++- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/verifiers/internal/gcb/provenance.go b/verifiers/internal/gcb/provenance.go index 021b63170..b6d0d542a 100644 --- a/verifiers/internal/gcb/provenance.go +++ b/verifiers/internal/gcb/provenance.go @@ -222,20 +222,18 @@ func isValidBuilderID(id string) error { return serrors.ErrorInvalidBuilderID } -func validateRecipeType(builderID, recipeType string) error { - _, v, err := utils.ParseBuilderID(builderID, true) - if err != nil { - return err - } +func validateRecipeType(builderID utils.BuilderID, recipeType string) error { + var err error + v := builderID.Version() switch v { case "v0.2": // In this version, the recipe type should be the same as // the builder ID. - if builderID == recipeType { + if builderID.String() == recipeType { return nil } err = fmt.Errorf("%w: expected '%s', got '%s'", - serrors.ErrorInvalidRecipe, builderID, recipeType) + serrors.ErrorInvalidRecipe, builderID.String(), recipeType) case "v0.3": // In this version, two recipe types are allowed, depending how the @@ -290,7 +288,7 @@ func (self *Provenance) VerifyBuilder(builderOpts *options.BuilderOpts) (*utils. } // Valiate the recipe type. - if err := validateRecipeType(predicateBuilderID, statement.Predicate.Recipe.Type); err != nil { + if err := validateRecipeType(*provBuilderID, statement.Predicate.Recipe.Type); err != nil { return nil, err } diff --git a/verifiers/internal/gcb/provenance_test.go b/verifiers/internal/gcb/provenance_test.go index cb429f759..de39aa648 100644 --- a/verifiers/internal/gcb/provenance_test.go +++ b/verifiers/internal/gcb/provenance_test.go @@ -290,7 +290,11 @@ func Test_validateRecipeType(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := validateRecipeType(tt.builderID, tt.recipeType) + builderID, err := utils.BuilderIDNew(tt.builderID) + if err != nil { + panic(fmt.Sprintf("BuilderIDNew failed: %v", err)) + } + err := validateRecipeType(*builderID, tt.recipeType) if !cmp.Equal(err, tt.expected, cmpopts.EquateErrors()) { t.Errorf(cmp.Diff(err, tt.expected, cmpopts.EquateErrors())) } From f8ae2963dce2fb0e3763cf98259d0f6af0531b45 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 22:52:03 +0000 Subject: [PATCH 11/20] update --- cli/slsa-verifier/main_test.go | 35 ++++++- experimental/rest/service.go | 2 +- verifiers/internal/gcb/provenance_test.go | 2 +- verifiers/utils/builder.go | 8 +- verifiers/utils/builder_test.go | 119 ++++++++++++++++++++++ 5 files changed, 158 insertions(+), 8 deletions(-) diff --git a/cli/slsa-verifier/main_test.go b/cli/slsa-verifier/main_test.go index a79cc1ce9..5ed75536f 100644 --- a/cli/slsa-verifier/main_test.go +++ b/cli/slsa-verifier/main_test.go @@ -734,6 +734,29 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { provenance: "gcloud-container-github.json", source: "github.com/laurentsimon/gcb-tests", }, + { + name: "mismatch input builder version", + artifact: "gcloud-container-github", + provenance: "gcloud-container-github.json", + source: "github.com/laurentsimon/gcb-tests", + pBuilderID: pString(builder + "@v0.4"), + err: serrors.ErrorMismatchBuilderID, + }, + { + name: "unsupported builder", + artifact: "gcloud-container-github", + provenance: "gcloud-container-github.json", + source: "github.com/laurentsimon/gcb-tests", + pBuilderID: pString(builder + "a"), + err: serrors.ErrorVerifierNotSupported, + }, + { + name: "match output builder name", + artifact: "gcloud-container-github", + provenance: "gcloud-container-github.json", + source: "github.com/laurentsimon/gcb-tests", + outBuilderID: builder, + }, { name: "invalid repo name", artifact: "gcloud-container-github", @@ -873,9 +896,9 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { // If builder ID is set, use it. if tt.pBuilderID != nil { - if !tt.noversion { - panic("builderID set but not noversion option") - } + // if !tt.noversion { + // panic("builderID set but not noversion option") + // } builderIDs = []string{*tt.pBuilderID} } @@ -922,8 +945,10 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { } // Validate against test's expected builderID, if provided. - if tt.outBuilderID != "" && tt.outBuilderID != outBuilderID.String() { - t.Errorf(cmp.Diff(tt.outBuilderID, outBuilderID.String())) + if tt.outBuilderID != "" { + if err := outBuilderID.Matches(tt.outBuilderID); err != nil { + t.Errorf(fmt.Sprintf("matches failed: %v", err)) + } } // Validate against builderID we generated automatically. diff --git a/experimental/rest/service.go b/experimental/rest/service.go index b3f58719c..cfe2df229 100644 --- a/experimental/rest/service.go +++ b/experimental/rest/service.go @@ -139,7 +139,7 @@ func verifyHandlerV1(r *http.Request) *v1Result { results = results.withIntotoStatement(p) } - return results.withBuilderID(builderID).withValidation(validationSuccess) + return results.withBuilderID(builderID.String()).withValidation(validationSuccess) } func queryFromString(content []byte) (*v1Query, error) { diff --git a/verifiers/internal/gcb/provenance_test.go b/verifiers/internal/gcb/provenance_test.go index de39aa648..5719a4c7e 100644 --- a/verifiers/internal/gcb/provenance_test.go +++ b/verifiers/internal/gcb/provenance_test.go @@ -294,7 +294,7 @@ func Test_validateRecipeType(t *testing.T) { if err != nil { panic(fmt.Sprintf("BuilderIDNew failed: %v", err)) } - err := validateRecipeType(*builderID, tt.recipeType) + err = validateRecipeType(*builderID, tt.recipeType) if !cmp.Equal(err, tt.expected, cmpopts.EquateErrors()) { t.Errorf(cmp.Diff(err, tt.expected, cmpopts.EquateErrors())) } diff --git a/verifiers/utils/builder.go b/verifiers/utils/builder.go index 2ec5857b2..dcd6f876e 100644 --- a/verifiers/utils/builder.go +++ b/verifiers/utils/builder.go @@ -17,6 +17,7 @@ func BuilderIDNew(builderID string) (*BuilderID, error) { if err != nil { return nil, err } + return &BuilderID{ name: name, version: version, @@ -31,13 +32,14 @@ func (b *BuilderID) Matches(builderID string) error { if err != nil { return err } + if name != b.name { return fmt.Errorf("%w: expected name '%s', got '%s'", serrors.ErrorMismatchBuilderID, name, b.name) } if version != "" && version != b.version { - fmt.Errorf("%w: expected version '%s', got '%s'", serrors.ErrorMismatchBuilderID, + return fmt.Errorf("%w: expected version '%s', got '%s'", serrors.ErrorMismatchBuilderID, version, b.version) } @@ -59,6 +61,10 @@ func (b *BuilderID) String() string { func ParseBuilderID(id string, needVersion bool) (string, string, error) { parts := strings.Split(id, "@") if len(parts) == 2 { + if parts[1] == "" && needVersion { + return "", "", fmt.Errorf("%w: builderID: '%s'", + serrors.ErrorInvalidFormat, id) + } return parts[0], parts[1], nil } diff --git a/verifiers/utils/builder_test.go b/verifiers/utils/builder_test.go index aae518b9b..f2e70a2f4 100644 --- a/verifiers/utils/builder_test.go +++ b/verifiers/utils/builder_test.go @@ -1,6 +1,7 @@ package utils import ( + "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -78,3 +79,121 @@ func Test_ParseBuilderID(t *testing.T) { }) } } + +func Test_BuilderIDNew(t *testing.T) { + t.Parallel() + tests := []struct { + name string + builderID string + builderName string + builderVersion string + err error + }{ + { + name: "valid", + builderID: "some/name@v1.2.3", + builderName: "some/name", + builderVersion: "v1.2.3", + }, + { + name: "too many '@' - need version", + builderID: "some/name@vla@blo", + err: serrors.ErrorInvalidFormat, + }, + { + name: "too many '@' - no need version", + builderID: "some/name@vla@blo", + err: serrors.ErrorInvalidFormat, + }, + } + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + builderID, err := BuilderIDNew(tt.builderID) + if !cmp.Equal(err, tt.err, cmpopts.EquateErrors()) { + t.Errorf(cmp.Diff(err, tt.err)) + } + + if err != nil { + return + } + + name := builderID.Name() + version := builderID.Version() + full := builderID.String() + + if name != tt.builderName { + t.Errorf(cmp.Diff(tt.builderName, name)) + } + if version != tt.builderVersion { + t.Errorf(cmp.Diff(tt.builderVersion, version)) + } + if full != tt.builderID { + t.Errorf(cmp.Diff(tt.builderID, full)) + } + }) + } +} + +func Test_Matches(t *testing.T) { + t.Parallel() + tests := []struct { + name string + builderID string + match string + err error + }{ + { + name: "match full", + builderID: "some/name@v1.2.3", + match: "some/name@v1.2.3", + }, + { + name: "match name", + builderID: "some/name@v1.2.3", + match: "some/name", + }, + { + name: "mismatch name", + builderID: "some/name@v1.2.3", + match: "some/name2", + err: serrors.ErrorMismatchBuilderID, + }, + { + name: "mismatch version", + builderID: "some/name@v1.2.3", + match: "some/name@v1.2.4", + err: serrors.ErrorMismatchBuilderID, + }, + { + name: "too many '@' - need version", + builderID: "some/name@v1.2.3", + match: "some/name@vla@blo", + err: serrors.ErrorInvalidFormat, + }, + { + name: "too many '@' - no need version", + builderID: "some/name@v1.2.3", + match: "some/name@vla@blo", + err: serrors.ErrorInvalidFormat, + }, + } + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + builderID, err := BuilderIDNew(tt.builderID) + if err != nil { + panic(fmt.Sprintf("BuilderIDNew: %v", err)) + } + + err = builderID.Matches(tt.match) + if !cmp.Equal(err, tt.err, cmpopts.EquateErrors()) { + t.Errorf(cmp.Diff(err, tt.err)) + } + }) + } +} From b75db5cb11f19eef67c5793d5c4409b69bb9c371 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 22:58:11 +0000 Subject: [PATCH 12/20] update --- verifiers/internal/gha/verifier.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/verifiers/internal/gha/verifier.go b/verifiers/internal/gha/verifier.go index c16ca3e2a..7e866ee86 100644 --- a/verifiers/internal/gha/verifier.go +++ b/verifiers/internal/gha/verifier.go @@ -76,7 +76,11 @@ func verifyEnvAndCert(env *dsse.Envelope, } // Temporary code. - bid, _ := utils.BuilderIDNew(builderID) + bid, err := utils.BuilderIDNew(builderID) + if err != nil { + return r, nil, err + } + return r, bid, nil } @@ -97,14 +101,9 @@ func (v *GHAVerifier) VerifyArtifact(ctx context.Context, return nil, nil, err } - content, builderID, err := verifyEnvAndCert(env, cert, + return verifyEnvAndCert(env, cert, provenanceOpts, builderOpts, defaultArtifactTrustedReusableWorkflows) - if err != nil { - return nil, nil, err - } - - return content, builderID, nil } // VerifyImage verifies provenance for an OCI image. From ef2447717b8ab7e14b37f9fb2419f039aa7cfd98 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 23:05:03 +0000 Subject: [PATCH 13/20] update --- verifiers/internal/gcb/provenance_test.go | 10 +++++++--- verifiers/utils/builder_test.go | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/verifiers/internal/gcb/provenance_test.go b/verifiers/internal/gcb/provenance_test.go index 5719a4c7e..993766d53 100644 --- a/verifiers/internal/gcb/provenance_test.go +++ b/verifiers/internal/gcb/provenance_test.go @@ -228,7 +228,7 @@ func Test_VerifyBuilder(t *testing.T) { } if err := outBuilderID.Matches(tt.builderID); err != nil { - t.Errorf(fmt.Errorf("matches failed: %w", err)) + t.Errorf(fmt.Sprintf("matches failed: %v", err)) } }) } @@ -292,7 +292,7 @@ func Test_validateRecipeType(t *testing.T) { builderID, err := utils.BuilderIDNew(tt.builderID) if err != nil { - panic(fmt.Sprintf("BuilderIDNew failed: %v", err)) + panic(fmt.Errorf("BuilderIDNew: %w", err)) } err = validateRecipeType(*builderID, tt.recipeType) if !cmp.Equal(err, tt.expected, cmpopts.EquateErrors()) { @@ -416,7 +416,11 @@ func Test_VerifySourceURI(t *testing.T) { panic(fmt.Errorf("ProvenanceFromBytes: %w", err)) } - err = prov.VerifySourceURI(tt.source, tt.builderID) + builderID, err := utils.BuilderIDNew(tt.builderID) + if err != nil { + panic(fmt.Errorf("BuilderIDNew: %w", err)) + } + err = prov.VerifySourceURI(tt.source, *builderID) if !cmp.Equal(err, tt.expected, cmpopts.EquateErrors()) { t.Errorf(cmp.Diff(err, tt.expected, cmpopts.EquateErrors())) } diff --git a/verifiers/utils/builder_test.go b/verifiers/utils/builder_test.go index f2e70a2f4..a309bafa3 100644 --- a/verifiers/utils/builder_test.go +++ b/verifiers/utils/builder_test.go @@ -187,7 +187,7 @@ func Test_Matches(t *testing.T) { builderID, err := BuilderIDNew(tt.builderID) if err != nil { - panic(fmt.Sprintf("BuilderIDNew: %v", err)) + panic(fmt.Errorf("BuilderIDNew: %w", err)) } err = builderID.Matches(tt.match) From b72d511cb73b50a337015f120e5303692523fc31 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 23:09:26 +0000 Subject: [PATCH 14/20] update --- verifiers/utils/builder.go | 2 +- verifiers/utils/builder_test.go | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/verifiers/utils/builder.go b/verifiers/utils/builder.go index dcd6f876e..bc100eb79 100644 --- a/verifiers/utils/builder.go +++ b/verifiers/utils/builder.go @@ -61,7 +61,7 @@ func (b *BuilderID) String() string { func ParseBuilderID(id string, needVersion bool) (string, string, error) { parts := strings.Split(id, "@") if len(parts) == 2 { - if parts[1] == "" && needVersion { + if parts[1] == "" { return "", "", fmt.Errorf("%w: builderID: '%s'", serrors.ErrorInvalidFormat, id) } diff --git a/verifiers/utils/builder_test.go b/verifiers/utils/builder_test.go index a309bafa3..23f1d4932 100644 --- a/verifiers/utils/builder_test.go +++ b/verifiers/utils/builder_test.go @@ -54,6 +54,17 @@ func Test_ParseBuilderID(t *testing.T) { builderID: "some/name@vla@blo", err: serrors.ErrorInvalidFormat, }, + { + name: "empty version - need version", + builderID: "some/name@", + needVersion: true, + err: serrors.ErrorInvalidFormat, + }, + { + name: "empty version - no need version", + builderID: "some/name@", + err: serrors.ErrorInvalidFormat, + }, } for _, tt := range tests { tt := tt // Re-initializing variable so it is not changed while executing the closure below @@ -95,6 +106,11 @@ func Test_BuilderIDNew(t *testing.T) { builderName: "some/name", builderVersion: "v1.2.3", }, + { + name: "empty version", + builderID: "some/name@", + err: serrors.ErrorInvalidFormat, + }, { name: "too many '@' - need version", builderID: "some/name@vla@blo", @@ -167,6 +183,12 @@ func Test_Matches(t *testing.T) { match: "some/name@v1.2.4", err: serrors.ErrorMismatchBuilderID, }, + { + name: "invalid empty version", + builderID: "some/name@v1.2.3", + match: "some/name@", + err: serrors.ErrorInvalidFormat, + }, { name: "too many '@' - need version", builderID: "some/name@v1.2.3", From 34261ed8e4b33b2cbe50d83b693ca8877e5e4a8e Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 23:14:22 +0000 Subject: [PATCH 15/20] update --- verifiers/internal/gha/verifier.go | 9 ++++----- verifiers/utils/builder.go | 3 ++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/verifiers/internal/gha/verifier.go b/verifiers/internal/gha/verifier.go index 7e866ee86..689de10fd 100644 --- a/verifiers/internal/gha/verifier.go +++ b/verifiers/internal/gha/verifier.go @@ -76,11 +76,10 @@ func verifyEnvAndCert(env *dsse.Envelope, } // Temporary code. - bid, err := utils.BuilderIDNew(builderID) - if err != nil { - return r, nil, err - } - + // TODO: make the variable `Name` private once GHA support is added, + // and check the error. + bid, _ := utils.BuilderIDNew(builderID) + bid.Name = builderID return r, bid, nil } diff --git a/verifiers/utils/builder.go b/verifiers/utils/builder.go index bc100eb79..c5b78f50e 100644 --- a/verifiers/utils/builder.go +++ b/verifiers/utils/builder.go @@ -8,7 +8,8 @@ import ( ) type BuilderID struct { - name, version string + // TODO: make the variable private once GHA support is added. + Name, version string } // BuilderIDNew creates a new BuilderID structure. From 87b42fc0177bfb3c83bd505eca45f6dfc1ec16c3 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 23:20:18 +0000 Subject: [PATCH 16/20] update --- verifiers/internal/gha/verifier.go | 6 +++--- verifiers/utils/builder.go | 8 ++++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/verifiers/internal/gha/verifier.go b/verifiers/internal/gha/verifier.go index 689de10fd..5eaf0c8f2 100644 --- a/verifiers/internal/gha/verifier.go +++ b/verifiers/internal/gha/verifier.go @@ -76,10 +76,10 @@ func verifyEnvAndCert(env *dsse.Envelope, } // Temporary code. - // TODO: make the variable `Name` private once GHA support is added, - // and check the error. + // TODO: remove `SetName` function once GHA is supported, + // and handle the error. bid, _ := utils.BuilderIDNew(builderID) - bid.Name = builderID + bid.SetName(builderID) return r, bid, nil } diff --git a/verifiers/utils/builder.go b/verifiers/utils/builder.go index c5b78f50e..c50f9fd62 100644 --- a/verifiers/utils/builder.go +++ b/verifiers/utils/builder.go @@ -8,8 +8,7 @@ import ( ) type BuilderID struct { - // TODO: make the variable private once GHA support is added. - Name, version string + name, version string } // BuilderIDNew creates a new BuilderID structure. @@ -59,6 +58,11 @@ func (b *BuilderID) String() string { return fmt.Sprintf("%s@%s", b.name, b.version) } +// TODO: remove this function once GHA is supported. +func (b *BuilderID) SetName(name string) { + b.name = name +} + func ParseBuilderID(id string, needVersion bool) (string, string, error) { parts := strings.Split(id, "@") if len(parts) == 2 { From f9f41426455ee985944c1da8181d6bd350af3f86 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 23:30:29 +0000 Subject: [PATCH 17/20] update --- verifiers/internal/gha/verifier.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/verifiers/internal/gha/verifier.go b/verifiers/internal/gha/verifier.go index 5eaf0c8f2..414357ff1 100644 --- a/verifiers/internal/gha/verifier.go +++ b/verifiers/internal/gha/verifier.go @@ -77,8 +77,8 @@ func verifyEnvAndCert(env *dsse.Envelope, // Temporary code. // TODO: remove `SetName` function once GHA is supported, - // and handle the error. - bid, _ := utils.BuilderIDNew(builderID) + // and use: bid, err := utils.BuilderIDNew(builderID) + bid := &utils.BuilderID{} bid.SetName(builderID) return r, bid, nil } From 94fede089579ad0d486fec3931055f60a9c71b73 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 23:40:03 +0000 Subject: [PATCH 18/20] update --- verifiers/internal/gha/verifier.go | 3 ++- verifiers/utils/builder.go | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/verifiers/internal/gha/verifier.go b/verifiers/internal/gha/verifier.go index 414357ff1..a955334c0 100644 --- a/verifiers/internal/gha/verifier.go +++ b/verifiers/internal/gha/verifier.go @@ -76,10 +76,11 @@ func verifyEnvAndCert(env *dsse.Envelope, } // Temporary code. - // TODO: remove `SetName` function once GHA is supported, + // TODO: remove `SetName` and `SetVersion` function once GHA is supported, // and use: bid, err := utils.BuilderIDNew(builderID) bid := &utils.BuilderID{} bid.SetName(builderID) + bid.SetVersion(strings.Split(workflowInfo.JobWobWorkflowRef)[1]) return r, bid, nil } diff --git a/verifiers/utils/builder.go b/verifiers/utils/builder.go index c50f9fd62..182cb575b 100644 --- a/verifiers/utils/builder.go +++ b/verifiers/utils/builder.go @@ -63,6 +63,10 @@ func (b *BuilderID) SetName(name string) { b.name = name } +func (b *BuilderID) SetVersion(version string) { + b.version = version +} + func ParseBuilderID(id string, needVersion bool) (string, string, error) { parts := strings.Split(id, "@") if len(parts) == 2 { From 5462df9476803cefd0b160c7eaba06b8997ef34f Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 12 Sep 2022 23:49:33 +0000 Subject: [PATCH 19/20] update --- verifiers/internal/gha/verifier.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/verifiers/internal/gha/verifier.go b/verifiers/internal/gha/verifier.go index a955334c0..4fc87a358 100644 --- a/verifiers/internal/gha/verifier.go +++ b/verifiers/internal/gha/verifier.go @@ -80,7 +80,7 @@ func verifyEnvAndCert(env *dsse.Envelope, // and use: bid, err := utils.BuilderIDNew(builderID) bid := &utils.BuilderID{} bid.SetName(builderID) - bid.SetVersion(strings.Split(workflowInfo.JobWobWorkflowRef)[1]) + bid.SetVersion(strings.Split(workflowInfo.JobWobWorkflowRef, "@")[1]) return r, bid, nil } From 7e07bbd9611488c9da4097f64b8ab9edab9f4f3a Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 13 Sep 2022 00:02:20 +0000 Subject: [PATCH 20/20] update --- cli/slsa-verifier/main_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/cli/slsa-verifier/main_test.go b/cli/slsa-verifier/main_test.go index 5ed75536f..7c994f43b 100644 --- a/cli/slsa-verifier/main_test.go +++ b/cli/slsa-verifier/main_test.go @@ -512,8 +512,10 @@ func Test_runVerifyArtifactPath(t *testing.T) { } // Validate against test's expected builderID, if provided. - if tt.outBuilderID != "" && tt.outBuilderID != outBuilderID.String() { - t.Errorf(cmp.Diff(tt.outBuilderID, outBuilderID.String())) + if tt.outBuilderID != "" { + if err := outBuilderID.Matches(tt.outBuilderID); err != nil { + t.Errorf(fmt.Sprintf("matches failed: %v", err)) + } } // TODO: verify using Matches(). @@ -678,8 +680,10 @@ func Test_runVerifyGHAArtifactImage(t *testing.T) { } // Validate against test's expected builderID, if provided. - if tt.outBuilderID != "" && tt.outBuilderID != outBuilderID.String() { - t.Errorf(cmp.Diff(tt.outBuilderID, outBuilderID.String())) + if tt.outBuilderID != "" { + if err := outBuilderID.Matches(tt.outBuilderID); err != nil { + t.Errorf(fmt.Sprintf("matches failed: %v", err)) + } } // TODO: verify using Matches().