Skip to content

Commit ae38103

Browse files
laurentsimonIan Lewis
andauthored
feat: verify sourceURI for npm packages (#521)
* update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * Update verifiers/internal/gha/provenance.go Co-authored-by: Ian Lewis <[email protected]> Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> --------- Signed-off-by: laurentsimon <[email protected]> Signed-off-by: laurentsimon <[email protected]> Co-authored-by: Ian Lewis <[email protected]>
1 parent 5a77b25 commit ae38103

File tree

3 files changed

+71
-20
lines changed

3 files changed

+71
-20
lines changed

verifiers/internal/gcb/provenance.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,9 @@ func (p *Provenance) VerifySourceURI(expectedSourceURI string, builderID utils.T
349349
return fmt.Errorf("%w: no materials", serrors.ErrorInvalidDssePayload)
350350
}
351351
uri := materials[0].URI
352+
// NOTE: the material URI did not contain 'git+' for GCB versions <= v0.3.
353+
// A change occurred sometimes in v0.3 witout version bump.
354+
// Versions >= 0.3 contain the prefix (https://github.com/slsa-framework/slsa-verifier/pull/519).
352355
uri = strings.TrimPrefix(uri, "git+")
353356

354357
// It is possible that GCS builds at level 2 use GCS sources, prefixed by gs://.

verifiers/internal/gha/provenance.go

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func asURI(s string) string {
7171
}
7272

7373
// Verify source URI in provenance statement.
74-
func verifySourceURI(prov slsaprovenance.Provenance, expectedSourceURI string, verifyMaterials bool) error {
74+
func verifySourceURI(prov slsaprovenance.Provenance, expectedSourceURI string, allowNoMaterialRef bool) error {
7575
source := asURI(expectedSourceURI)
7676

7777
// We expect github.com URIs only.
@@ -85,7 +85,7 @@ func verifySourceURI(prov slsaprovenance.Provenance, expectedSourceURI string, v
8585
if err != nil {
8686
return err
8787
}
88-
configURI, err := sourceFromURI(fullConfigURI)
88+
configURI, err := sourceFromURI(fullConfigURI, false)
8989
if err != nil {
9090
return err
9191
}
@@ -95,15 +95,11 @@ func verifySourceURI(prov slsaprovenance.Provenance, expectedSourceURI string, v
9595
}
9696

9797
// Verify source from material section.
98-
// TODO(#492): disable this option.
99-
if !verifyMaterials {
100-
return nil
101-
}
10298
materialSourceURI, err := prov.SourceURI()
10399
if err != nil {
104100
return err
105101
}
106-
materialURI, err := sourceFromURI(materialSourceURI)
102+
materialURI, err := sourceFromURI(materialSourceURI, allowNoMaterialRef)
107103
if err != nil {
108104
return err
109105
}
@@ -114,6 +110,12 @@ func verifySourceURI(prov slsaprovenance.Provenance, expectedSourceURI string, v
114110

115111
// Last, verify that both fields match.
116112
// We use the full URI to match on the tag as well.
113+
if allowNoMaterialRef && len(strings.Split(materialSourceURI, "@")) == 1 {
114+
// NOTE: this is an exception for npm packages built before GA,
115+
// see https://github.com/slsa-framework/slsa-verifier/issues/492.
116+
// We don't need to compare the ref since materialSourceURI does not contain it.
117+
return nil
118+
}
117119
if fullConfigURI != materialSourceURI {
118120
return fmt.Errorf("%w: material and config URIs do not match: '%s' != '%s'",
119121
serrors.ErrorInvalidDssePayload,
@@ -123,13 +125,19 @@ func verifySourceURI(prov slsaprovenance.Provenance, expectedSourceURI string, v
123125
return nil
124126
}
125127

126-
func sourceFromURI(uri string) (string, error) {
128+
// sourceFromURI retrieves the source repository given a repository URI with ref.
129+
//
130+
// NOTE: `allowNoRef` is to allow for verification of npm packages
131+
// generated before GA. Their provenance did not have a ref,
132+
// see https://github.com/slsa-framework/slsa-verifier/issues/492.
133+
// `allowNoRef` should be set to `false` for all other cases.
134+
func sourceFromURI(uri string, allowNoRef bool) (string, error) {
127135
if uri == "" {
128136
return "", fmt.Errorf("%w: empty uri", serrors.ErrorMalformedURI)
129137
}
130138

131-
r := strings.SplitN(uri, "@", 2)
132-
if len(r) < 2 {
139+
r := strings.Split(uri, "@")
140+
if len(r) < 2 && !allowNoRef {
133141
return "", fmt.Errorf("%w: %s", serrors.ErrorMalformedURI,
134142
uri)
135143
}
@@ -217,7 +225,7 @@ func VerifyNpmPackageProvenance(env *dsselib.Envelope, provenanceOpts *options.P
217225
}
218226
// NOTE: for the non trusted builders, the information may be forgeable.
219227
// Also, the GitHub context is not recorded for the default builder.
220-
return VerifyProvenanceCommonOptions(prov, provenanceOpts, false)
228+
return VerifyProvenanceCommonOptions(prov, provenanceOpts, true)
221229
}
222230

223231
func VerifyProvenance(env *dsselib.Envelope, provenanceOpts *options.ProvenanceOpts,
@@ -234,14 +242,14 @@ func VerifyProvenance(env *dsselib.Envelope, provenanceOpts *options.ProvenanceO
234242
return err
235243
}
236244

237-
return VerifyProvenanceCommonOptions(prov, provenanceOpts, true)
245+
return VerifyProvenanceCommonOptions(prov, provenanceOpts, false)
238246
}
239247

240248
func VerifyProvenanceCommonOptions(prov slsaprovenance.Provenance, provenanceOpts *options.ProvenanceOpts,
241-
verifyMaterials bool,
249+
allowNoMaterialRef bool,
242250
) error {
243251
// Verify source.
244-
if err := verifySourceURI(prov, provenanceOpts.ExpectedSourceURI, verifyMaterials); err != nil {
252+
if err := verifySourceURI(prov, provenanceOpts.ExpectedSourceURI, allowNoMaterialRef); err != nil {
245253
return err
246254
}
247255

verifiers/internal/gha/provenance_test.go

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,11 @@ func Test_VerifyDigest(t *testing.T) {
160160
func Test_verifySourceURI(t *testing.T) {
161161
t.Parallel()
162162
tests := []struct {
163-
name string
164-
prov *intoto.ProvenanceStatement
165-
sourceURI string
166-
expected error
163+
name string
164+
prov *intoto.ProvenanceStatement
165+
sourceURI string
166+
allowNoMaterialRef bool
167+
expected error
167168
// v1 provenance does not include materials
168169
skipv1 bool
169170
}{
@@ -288,6 +289,45 @@ func Test_verifySourceURI(t *testing.T) {
288289
},
289290
sourceURI: "https://github.com/some/repo",
290291
},
292+
{
293+
name: "match source no git no material ref",
294+
prov: &intoto.ProvenanceStatement{
295+
Predicate: slsa02.ProvenancePredicate{
296+
Invocation: slsa02.ProvenanceInvocation{
297+
ConfigSource: slsa02.ConfigSource{
298+
URI: "git+https://github.com/some/[email protected]",
299+
},
300+
},
301+
Materials: []slsacommon.ProvenanceMaterial{
302+
{
303+
URI: "git+https://github.com/some/repo",
304+
},
305+
},
306+
},
307+
},
308+
allowNoMaterialRef: true,
309+
sourceURI: "https://github.com/some/repo",
310+
},
311+
{
312+
name: "match source no git no material ref ref not allowed",
313+
prov: &intoto.ProvenanceStatement{
314+
Predicate: slsa02.ProvenancePredicate{
315+
Invocation: slsa02.ProvenanceInvocation{
316+
ConfigSource: slsa02.ConfigSource{
317+
URI: "git+https://github.com/some/[email protected]",
318+
},
319+
},
320+
Materials: []slsacommon.ProvenanceMaterial{
321+
{
322+
URI: "git+https://github.com/some/repo",
323+
},
324+
},
325+
},
326+
},
327+
sourceURI: "https://github.com/some/repo",
328+
expected: serrors.ErrorMalformedURI,
329+
skipv1: true,
330+
},
291331
{
292332
name: "match source no git+https",
293333
prov: &intoto.ProvenanceStatement{
@@ -412,7 +452,7 @@ func Test_verifySourceURI(t *testing.T) {
412452
ProvenanceStatement: tt.prov,
413453
}
414454

415-
err := verifySourceURI(prov02, tt.sourceURI, true)
455+
err := verifySourceURI(prov02, tt.sourceURI, tt.allowNoMaterialRef)
416456
if !errCmp(err, tt.expected) {
417457
t.Errorf(cmp.Diff(err, tt.expected))
418458
}
@@ -433,7 +473,7 @@ func Test_verifySourceURI(t *testing.T) {
433473
},
434474
},
435475
}
436-
err = verifySourceURI(prov1, tt.sourceURI, true)
476+
err = verifySourceURI(prov1, tt.sourceURI, tt.allowNoMaterialRef)
437477
if !errCmp(err, tt.expected) {
438478
t.Errorf(cmp.Diff(err, tt.expected))
439479
}

0 commit comments

Comments
 (0)