feat: support builderID matching with or without semver for GCB

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 {
@@ -858,7 +859,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 +872,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 +893,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))
 				}
 
 			}

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)

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
 	}

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))
 			}
 		})
 	}

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,9 @@ 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@")
+	// Note: `ParseBuilderID` never fails with `needVersion=false`.
+	name, _, _ := utils.ParseBuilderID(builderID, false)
+	return name == "https://cloudbuild.googleapis.com/GoogleHostedWorker"
 }
 
 // VerifyArtifact verifies provenance for an artifact.

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"

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)
+}

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))
+			}
+		})
+	}
+}