feat: Verification for when sha1 is specified in BYOB TRW

errors/errors.go

@@ -9,6 +9,7 @@ var (
 	ErrorMismatchPackageName       = errors.New("package name does not match provenance")
 	ErrorMismatchBuilderID         = errors.New("builderID does not match provenance")
 	ErrorInvalidBuilderID          = errors.New("builderID is invalid")
+	ErrorInvalidBuildType          = errors.New("buildType is invalid")
 	ErrorMismatchSource            = errors.New("source used to generate the binary does not match provenance")
 	ErrorMismatchWorkflowInputs    = errors.New("workflow input does not match")
 	ErrorMalformedURI              = errors.New("URI is malformed")

verifiers/internal/gha/provenance.go

@@ -76,100 +76,84 @@ func verifyBuilderIDLooseMatch(prov iface.Provenance, expectedBuilderID string)
 	return nil
 }
 
-func asURI(s string) string {
-	source := s
-	if !strings.HasPrefix(source, "https://") &&
-		!strings.HasPrefix(source, "git+") {
-		source = "git+https://" + source
-	}
-	if !strings.HasPrefix(source, "git+") {
-		source = "git+" + source
-	}
-
-	return source
-}
-
 // Verify source URI in provenance statement.
-func verifySourceURI(prov iface.Provenance, expectedSourceURI string, allowNoMaterialRef bool) error {
-	source := asURI(expectedSourceURI)
+func verifySourceURI(prov iface.Provenance, expectedSourceURI string) error {
+	source := utils.NormalizeGitURI(expectedSourceURI)
 
 	// We expect github.com URIs only.
 	if !strings.HasPrefix(source, "git+https://github.com/") {
-		return fmt.Errorf("%w: expected source github.com repository '%s'", serrors.ErrorMalformedURI,
+		return fmt.Errorf("%w: expected source github.com repository %q", serrors.ErrorMalformedURI,
 			source)
 	}
 
 	// Verify source in the trigger
-	fullConfigURI, err := prov.TriggerURI()
+	fullTriggerURI, err := prov.TriggerURI()
 	if err != nil {
 		return err
 	}
 
-	configURI, err := sourceFromURI(fullConfigURI, false)
+	triggerURI, triggerRef, err := utils.ParseGitURIAndRef(fullTriggerURI)
 	if err != nil {
 		return err
 	}
-	if configURI != source {
-		return fmt.Errorf("%w: expected source '%s' in configSource.uri, got '%s'", serrors.ErrorMismatchSource,
-			source, fullConfigURI)
+	if triggerURI != source {
+		return fmt.Errorf("%w: expected trigger %q to match source-uri %q", serrors.ErrorMismatchSource,
+			source, fullTriggerURI)
+	}
+	// We expect the trigger URI to always have a ref.
+	if triggerRef == "" {
+		return fmt.Errorf("%w: missing ref: %q", serrors.ErrorMalformedURI, fullTriggerURI)
 	}
 
 	// Verify source from material section.
-	materialSourceURI, err := prov.SourceURI()
+	fullSourceURI, err := prov.SourceURI()
 	if err != nil {
 		return err
 	}
 
-	materialURI, err := sourceFromURI(materialSourceURI, allowNoMaterialRef)
+	sourceURI, sourceRef, err := utils.ParseGitURIAndRef(fullSourceURI)
 	if err != nil {
 		return err
 	}
-	if materialURI != source {
-		return fmt.Errorf("%w: expected source '%s' in material section, got '%s'", serrors.ErrorMismatchSource,
-			source, materialSourceURI)
+	if sourceURI != source {
+		return fmt.Errorf("%w: expected source %q to match source-uri %q", serrors.ErrorMismatchSource,
+			fullSourceURI, source)
 	}
 
-	// Last, verify that both fields match.
-	// We use the full URI to match on the tag as well.
-	if allowNoMaterialRef && len(strings.Split(materialSourceURI, "@")) == 1 {
+	buildType, err := prov.BuildType()
+	if err != nil {
+		return fmt.Errorf("checking buildType: %v", err)
+	}
+	if sourceRef == "" {
 		// NOTE: this is an exception for npm packages built before GA,
 		// see https://github.com/slsa-framework/slsa-verifier/issues/492.
 		// We don't need to compare the ref since materialSourceURI does not contain it.
-		return nil
+		if buildType == common.NpmCLIBuildTypeV1 {
+			return nil
+		}
+		// NOTE: BYOB builders can build from a different ref than the triggering ref.
+		// This most often happens when a TRW makes a commit as part of the release process.
+		// NOTE: Currently only building from a different git sha is supported
+		// which means the sourceRef is empty. Building from an arbitrary ref
+		// is currently not supported.
+		if buildType == common.BYOBBuildTypeV0 {
+			return nil
+		}
+		return fmt.Errorf("%w: missing ref: %q", serrors.ErrorMalformedURI, fullSourceURI)
 	}
-	if fullConfigURI != materialSourceURI {
-		return fmt.Errorf("%w: material and config URIs do not match: '%s' != '%s'",
+
+	// Last, verify that both fields match. We expect that the trigger URI and
+	// the source URI match but the ref used to trigger the build and source ref
+	// could be different.
+	if fullTriggerURI != fullSourceURI {
+		return fmt.Errorf("%w: source and trigger URIs do not match: %q != %q",
 			serrors.ErrorInvalidDssePayload,
-			fullConfigURI, materialSourceURI)
+			fullTriggerURI, fullSourceURI)
 	}
 
 	return nil
 }
 
-// sourceFromURI retrieves the source repository given a repository URI with ref.
-//
-// NOTE: `allowNoRef` is to allow for verification of npm packages
-// generated before GA. Their provenance did not have a ref,
-// see https://github.com/slsa-framework/slsa-verifier/issues/492.
-// `allowNoRef` should be set to `false` for all other cases.
-func sourceFromURI(uri string, allowNoRef bool) (string, error) {
-	if uri == "" {
-		return "", fmt.Errorf("%w: empty uri", serrors.ErrorMalformedURI)
-	}
-
-	r := strings.Split(uri, "@")
-	if len(r) < 2 && !allowNoRef {
-		return "", fmt.Errorf("%w: %s", serrors.ErrorMalformedURI,
-			uri)
-	}
-	if len(r) < 1 {
-		return "", fmt.Errorf("%w: %s", serrors.ErrorMalformedURI,
-			uri)
-	}
-
-	return r[0], nil
-}
-
 // Verify Subject Digest from the provenance statement.
 func verifyDigest(prov iface.Provenance, expectedHash string) error {
 	subjects, err := prov.Subjects()
@@ -270,7 +254,7 @@ func VerifyNpmPackageProvenance(env *dsselib.Envelope, workflow *WorkflowIdentit
 	}
 
 	// Also, the GitHub context is not recorded for the default builder.
-	if err := VerifyProvenanceCommonOptions(prov, provenanceOpts, true); err != nil {
+	if err := VerifyProvenanceCommonOptions(prov, provenanceOpts); err != nil {
 		return err
 	}
 
@@ -320,15 +304,13 @@ func VerifyProvenance(env *dsselib.Envelope, provenanceOpts *options.ProvenanceO
 		}
 	}
 
-	return VerifyProvenanceCommonOptions(prov, provenanceOpts, false)
+	return VerifyProvenanceCommonOptions(prov, provenanceOpts)
 }
 
 // VerifyProvenanceCommonOptions verifies the given provenance.
-func VerifyProvenanceCommonOptions(prov iface.Provenance, provenanceOpts *options.ProvenanceOpts,
-	allowNoMaterialRef bool,
-) error {
+func VerifyProvenanceCommonOptions(prov iface.Provenance, provenanceOpts *options.ProvenanceOpts) error {
 	// Verify source.
-	if err := verifySourceURI(prov, provenanceOpts.ExpectedSourceURI, allowNoMaterialRef); err != nil {
+	if err := verifySourceURI(prov, provenanceOpts.ExpectedSourceURI); err != nil {
 		return err
 	}
 

verifiers/internal/gha/provenance_test.go

@@ -7,17 +7,19 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	intoto "github.com/in-toto/in-toto-golang/in_toto"
-	"github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/common"
+	slsacommon "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/common"
 	slsa02 "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/v0.2"
 	slsa1 "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/v1"
 
 	serrors "github.com/slsa-framework/slsa-verifier/v2/errors"
 	"github.com/slsa-framework/slsa-verifier/v2/verifiers/internal/gha/slsaprovenance"
+	"github.com/slsa-framework/slsa-verifier/v2/verifiers/internal/gha/slsaprovenance/common"
 	"github.com/slsa-framework/slsa-verifier/v2/verifiers/internal/gha/slsaprovenance/iface"
 )
 
 type testProvenance struct {
 	builderID         string
+	buildType         string
 	sourceURI         string
 	triggerURI        string
 	subjects          []intoto.Subject
@@ -33,6 +35,7 @@ type testProvenance struct {
 }
 
 func (p *testProvenance) BuilderID() (string, error)           { return p.builderID, nil }
+func (p *testProvenance) BuildType() (string, error)           { return p.buildType, nil }
 func (p *testProvenance) SourceURI() (string, error)           { return p.sourceURI, nil }
 func (p *testProvenance) TriggerURI() (string, error)          { return p.triggerURI, nil }
 func (p *testProvenance) Subjects() ([]intoto.Subject, error)  { return p.subjects, nil }
@@ -121,7 +124,7 @@ func Test_VerifyDigest(t *testing.T) {
 			prov: &testProvenance{
 				subjects: []intoto.Subject{
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha1": "4506290e2e8feb1f34b27a044f7cc863c830ef6b",
 						},
 					},
@@ -136,7 +139,7 @@ func Test_VerifyDigest(t *testing.T) {
 			prov: &testProvenance{
 				subjects: []intoto.Subject{
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha1": "4506290e2e8feb1f34b27a044f7cc863c830ef6b",
 						},
 					},
@@ -157,7 +160,7 @@ func Test_VerifyDigest(t *testing.T) {
 			prov: &testProvenance{
 				subjects: []intoto.Subject{
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha256": "0ae7e4fa71686538440012ee36a2634dbaa19df2dd16a466f52411fb348bbc4e",
 						},
 					},
@@ -171,7 +174,7 @@ func Test_VerifyDigest(t *testing.T) {
 			prov: &testProvenance{
 				subjects: []intoto.Subject{
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha256": "0ae7e4fa71686538440012ee36a2634dbaa19df2dd16a466f52411fb348bbc4e",
 						},
 					},
@@ -184,17 +187,17 @@ func Test_VerifyDigest(t *testing.T) {
 			prov: &testProvenance{
 				subjects: []intoto.Subject{
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha256": "03e7e4fa71686538440012ee36a2634dbaa19df2dd16a466f52411fb348bbc4e",
 						},
 					},
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha1": "4506290e2e8feb1f34b27a044f7cc863c830ef6b",
 						},
 					},
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha256": "0ae7e4fa71686538440012ee36a2634dbaa19df2dd16a466f52411fb348bbc4e",
 						},
 					},
@@ -207,17 +210,17 @@ func Test_VerifyDigest(t *testing.T) {
 			prov: &testProvenance{
 				subjects: []intoto.Subject{
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha256": "03e7e4fa71686538440012ee36a2634dbaa19df2dd16a466f52411fb348bbc4e",
 						},
 					},
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha256": "0ae7e4fa71686538440012ee36a2634dbaa19df2dd16a466f52411fb348bbc4e",
 						},
 					},
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha1": "4506290e2e8feb1f34b27a044f7cc863c830ef6b",
 						},
 					},
@@ -230,17 +233,17 @@ func Test_VerifyDigest(t *testing.T) {
 			prov: &testProvenance{
 				subjects: []intoto.Subject{
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha256": "03e7e4fa71686538440012ee36a2634dbaa19df2dd16a466f52411fb348bbc4e",
 						},
 					},
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha256": "0ae7e4fa71686538440012ee36a2634dbaa19df2dd16a466f52411fb348bbc4e",
 						},
 					},
 					{
-						Digest: common.DigestSet{
+						Digest: slsacommon.DigestSet{
 							"sha1": "4506290e2e8feb1f34b27a044f7cc863c830ef6b",
 						},
 					},
@@ -266,6 +269,7 @@ func Test_verifySourceURI(t *testing.T) {
 	t.Parallel()
 	tests := []struct {
 		name               string
+		provBuildType      string
 		provMaterialsURI   string
 		provTriggerURI     string
 		expectedSourceURI  string
@@ -322,14 +326,37 @@ func Test_verifySourceURI(t *testing.T) {
 			expectedSourceURI: "https://github.com/some/repo",
 		},
 		{
-			name:               "match source no git no material ref",
-			provTriggerURI:     "git+https://github.com/some/[email protected]",
-			provMaterialsURI:   "git+https://github.com/some/repo",
-			allowNoMaterialRef: true,
-			expectedSourceURI:  "https://github.com/some/repo",
+			name:              "match source no git no material ref (npm)",
+			provBuildType:     common.NpmCLIBuildTypeV1,
+			provTriggerURI:    "git+https://github.com/some/[email protected]",
+			provMaterialsURI:  "git+https://github.com/some/repo",
+			expectedSourceURI: "https://github.com/some/repo",
+		},
+		{
+			name:              "mismatch source material ref (npm)",
+			provBuildType:     common.NpmCLIBuildTypeV1,
+			provTriggerURI:    "git+https://github.com/some/[email protected]",
+			provMaterialsURI:  "git+https://github.com/some/[email protected]",
+			expectedSourceURI: "https://github.com/some/repo",
+			err:               serrors.ErrorInvalidDssePayload,
+		},
+		{
+			name:              "match source no git no material ref (byob)",
+			provBuildType:     common.BYOBBuildTypeV0,
+			provTriggerURI:    "git+https://github.com/some/[email protected]",
+			provMaterialsURI:  "git+https://github.com/some/repo",
+			expectedSourceURI: "https://github.com/some/repo",
+		},
+		{
+			name:              "mismatch source material ref (byob)",
+			provBuildType:     common.BYOBBuildTypeV0,
+			provTriggerURI:    "git+https://github.com/some/[email protected]",
+			provMaterialsURI:  "git+https://github.com/some/[email protected]",
+			expectedSourceURI: "https://github.com/some/repo",
+			err:               serrors.ErrorInvalidDssePayload,
 		},
 		{
-			name:              "match source no git no material ref ref not allowed",
+			name:              "match source no git no material ref",
 			provTriggerURI:    "git+https://github.com/some/[email protected]",
 			provMaterialsURI:  "git+https://github.com/some/repo",
 			expectedSourceURI: "https://github.com/some/repo",
@@ -397,11 +424,12 @@ func Test_verifySourceURI(t *testing.T) {
 			t.Parallel()
 
 			prov02 := &testProvenance{
+				buildType:  tt.provBuildType,
 				sourceURI:  tt.provMaterialsURI,
 				triggerURI: tt.provTriggerURI,
 			}
 
-			err := verifySourceURI(prov02, tt.expectedSourceURI, tt.allowNoMaterialRef)
+			err := verifySourceURI(prov02, tt.expectedSourceURI)
 			if !errCmp(err, tt.err) {
 				t.Errorf(cmp.Diff(err, tt.err))
 			}