From ae38103ecf90555fdf2af5dc536a106c9250331b Mon Sep 17 00:00:00 2001 From: laurentsimon <64505099+laurentsimon@users.noreply.github.com> Date: Fri, 10 Mar 2023 09:13:29 -0800 Subject: [PATCH] feat: verify sourceURI for npm packages (#521) * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * Update verifiers/internal/gha/provenance.go Co-authored-by: Ian Lewis Signed-off-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com> * update Signed-off-by: laurentsimon --------- Signed-off-by: laurentsimon Signed-off-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com> Co-authored-by: Ian Lewis --- verifiers/internal/gcb/provenance.go | 3 ++ verifiers/internal/gha/provenance.go | 36 ++++++++++------ verifiers/internal/gha/provenance_test.go | 52 ++++++++++++++++++++--- 3 files changed, 71 insertions(+), 20 deletions(-) diff --git a/verifiers/internal/gcb/provenance.go b/verifiers/internal/gcb/provenance.go index a01264d..823524b 100644 --- a/verifiers/internal/gcb/provenance.go +++ b/verifiers/internal/gcb/provenance.go @@ -349,6 +349,9 @@ func (p *Provenance) VerifySourceURI(expectedSourceURI string, builderID utils.T return fmt.Errorf("%w: no materials", serrors.ErrorInvalidDssePayload) } uri := materials[0].URI + // NOTE: the material URI did not contain 'git+' for GCB versions <= v0.3. + // A change occurred sometimes in v0.3 witout version bump. + // Versions >= 0.3 contain the prefix (https://github.com/slsa-framework/slsa-verifier/pull/519). uri = strings.TrimPrefix(uri, "git+") // It is possible that GCS builds at level 2 use GCS sources, prefixed by gs://. diff --git a/verifiers/internal/gha/provenance.go b/verifiers/internal/gha/provenance.go index e4585e9..ebbe05a 100644 --- a/verifiers/internal/gha/provenance.go +++ b/verifiers/internal/gha/provenance.go @@ -71,7 +71,7 @@ func asURI(s string) string { } // Verify source URI in provenance statement. -func verifySourceURI(prov slsaprovenance.Provenance, expectedSourceURI string, verifyMaterials bool) error { +func verifySourceURI(prov slsaprovenance.Provenance, expectedSourceURI string, allowNoMaterialRef bool) error { source := asURI(expectedSourceURI) // We expect github.com URIs only. @@ -85,7 +85,7 @@ func verifySourceURI(prov slsaprovenance.Provenance, expectedSourceURI string, v if err != nil { return err } - configURI, err := sourceFromURI(fullConfigURI) + configURI, err := sourceFromURI(fullConfigURI, false) if err != nil { return err } @@ -95,15 +95,11 @@ func verifySourceURI(prov slsaprovenance.Provenance, expectedSourceURI string, v } // Verify source from material section. - // TODO(#492): disable this option. - if !verifyMaterials { - return nil - } materialSourceURI, err := prov.SourceURI() if err != nil { return err } - materialURI, err := sourceFromURI(materialSourceURI) + materialURI, err := sourceFromURI(materialSourceURI, allowNoMaterialRef) if err != nil { return err } @@ -114,6 +110,12 @@ func verifySourceURI(prov slsaprovenance.Provenance, expectedSourceURI string, v // 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 { + // 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 fullConfigURI != materialSourceURI { return fmt.Errorf("%w: material and config URIs do not match: '%s' != '%s'", serrors.ErrorInvalidDssePayload, @@ -123,13 +125,19 @@ func verifySourceURI(prov slsaprovenance.Provenance, expectedSourceURI string, v return nil } -func sourceFromURI(uri string) (string, error) { +// 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.SplitN(uri, "@", 2) - if len(r) < 2 { + r := strings.Split(uri, "@") + if len(r) < 2 && !allowNoRef { return "", fmt.Errorf("%w: %s", serrors.ErrorMalformedURI, uri) } @@ -217,7 +225,7 @@ func VerifyNpmPackageProvenance(env *dsselib.Envelope, provenanceOpts *options.P } // NOTE: for the non trusted builders, the information may be forgeable. // Also, the GitHub context is not recorded for the default builder. - return VerifyProvenanceCommonOptions(prov, provenanceOpts, false) + return VerifyProvenanceCommonOptions(prov, provenanceOpts, true) } func VerifyProvenance(env *dsselib.Envelope, provenanceOpts *options.ProvenanceOpts, @@ -234,14 +242,14 @@ func VerifyProvenance(env *dsselib.Envelope, provenanceOpts *options.ProvenanceO return err } - return VerifyProvenanceCommonOptions(prov, provenanceOpts, true) + return VerifyProvenanceCommonOptions(prov, provenanceOpts, false) } func VerifyProvenanceCommonOptions(prov slsaprovenance.Provenance, provenanceOpts *options.ProvenanceOpts, - verifyMaterials bool, + allowNoMaterialRef bool, ) error { // Verify source. - if err := verifySourceURI(prov, provenanceOpts.ExpectedSourceURI, verifyMaterials); err != nil { + if err := verifySourceURI(prov, provenanceOpts.ExpectedSourceURI, allowNoMaterialRef); err != nil { return err } diff --git a/verifiers/internal/gha/provenance_test.go b/verifiers/internal/gha/provenance_test.go index 6057faa..a2d73b4 100644 --- a/verifiers/internal/gha/provenance_test.go +++ b/verifiers/internal/gha/provenance_test.go @@ -160,10 +160,11 @@ func Test_VerifyDigest(t *testing.T) { func Test_verifySourceURI(t *testing.T) { t.Parallel() tests := []struct { - name string - prov *intoto.ProvenanceStatement - sourceURI string - expected error + name string + prov *intoto.ProvenanceStatement + sourceURI string + allowNoMaterialRef bool + expected error // v1 provenance does not include materials skipv1 bool }{ @@ -288,6 +289,45 @@ func Test_verifySourceURI(t *testing.T) { }, sourceURI: "https://github.com/some/repo", }, + { + name: "match source no git no material ref", + prov: &intoto.ProvenanceStatement{ + Predicate: slsa02.ProvenancePredicate{ + Invocation: slsa02.ProvenanceInvocation{ + ConfigSource: slsa02.ConfigSource{ + URI: "git+https://github.com/some/repo@v1.2.3", + }, + }, + Materials: []slsacommon.ProvenanceMaterial{ + { + URI: "git+https://github.com/some/repo", + }, + }, + }, + }, + allowNoMaterialRef: true, + sourceURI: "https://github.com/some/repo", + }, + { + name: "match source no git no material ref ref not allowed", + prov: &intoto.ProvenanceStatement{ + Predicate: slsa02.ProvenancePredicate{ + Invocation: slsa02.ProvenanceInvocation{ + ConfigSource: slsa02.ConfigSource{ + URI: "git+https://github.com/some/repo@v1.2.3", + }, + }, + Materials: []slsacommon.ProvenanceMaterial{ + { + URI: "git+https://github.com/some/repo", + }, + }, + }, + }, + sourceURI: "https://github.com/some/repo", + expected: serrors.ErrorMalformedURI, + skipv1: true, + }, { name: "match source no git+https", prov: &intoto.ProvenanceStatement{ @@ -412,7 +452,7 @@ func Test_verifySourceURI(t *testing.T) { ProvenanceStatement: tt.prov, } - err := verifySourceURI(prov02, tt.sourceURI, true) + err := verifySourceURI(prov02, tt.sourceURI, tt.allowNoMaterialRef) if !errCmp(err, tt.expected) { t.Errorf(cmp.Diff(err, tt.expected)) } @@ -433,7 +473,7 @@ func Test_verifySourceURI(t *testing.T) { }, }, } - err = verifySourceURI(prov1, tt.sourceURI, true) + err = verifySourceURI(prov1, tt.sourceURI, tt.allowNoMaterialRef) if !errCmp(err, tt.expected) { t.Errorf(cmp.Diff(err, tt.expected)) }