Skip to content

Commit 93d3f8c

Browse files
laurentsimonIan Lewis
and
Ian Lewis
authored
fix: Verify the TRW tag is a semver tag (#619)
* update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * Update verifiers/utils/builder.go Co-authored-by: Ian Lewis <[email protected]> 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 de79463 commit 93d3f8c

File tree

4 files changed

+143
-32
lines changed

4 files changed

+143
-32
lines changed

verifiers/internal/gha/builder.go

+2-32
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import (
66
"fmt"
77
"strings"
88

9-
"golang.org/x/mod/semver"
10-
119
fulcio "github.com/sigstore/fulcio/pkg/certificate"
1210
serrors "github.com/slsa-framework/slsa-verifier/v2/errors"
1311
"github.com/slsa-framework/slsa-verifier/v2/options"
@@ -170,38 +168,10 @@ func verifyTrustedBuilderRef(id *WorkflowIdentity, ref string) error {
170168
return nil
171169
}
172170

173-
// Extract the tag.
174-
pin, err := utils.TagFromGitHubRef(ref)
175-
if err != nil {
176-
return err
177-
}
178-
179-
// Tags on trusted repositories should be a valid semver with version
180-
// core including all three parts and no build identifier.
181-
versionCore := strings.Split(pin, "-")[0]
182-
if !semver.IsValid(pin) ||
183-
len(strings.Split(versionCore, ".")) != 3 ||
184-
semver.Build(pin) != "" {
185-
return fmt.Errorf("%w: %s: version tag not valid", serrors.ErrorInvalidRef, pin)
186-
}
187-
188-
return nil
171+
return utils.IsValidBuilderTag(ref, true)
189172
}
190173

191-
// Extract the pin.
192-
pin, err := utils.TagFromGitHubRef(ref)
193-
if err != nil {
194-
return err
195-
}
196-
197-
// Valid semver of the form vX.Y.Z with no metadata.
198-
if !(semver.IsValid(pin) &&
199-
len(strings.Split(pin, ".")) == 3 &&
200-
semver.Prerelease(pin) == "" &&
201-
semver.Build(pin) == "") {
202-
return fmt.Errorf("%w: %s: not of the form vX.Y.Z", serrors.ErrorInvalidRef, pin)
203-
}
204-
return nil
174+
return utils.IsValidBuilderTag(ref, false)
205175
}
206176

207177
func getExtension(cert *x509.Certificate, oid asn1.ObjectIdentifier, encoded bool) (string, error) {

verifiers/internal/gha/provenance.go

+16
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,19 @@ func VerifyNpmPackageProvenance(env *dsselib.Envelope, workflow *WorkflowIdentit
277277
return nil
278278
}
279279

280+
func isValidDelegatorBuilderID(prov slsaprovenance.Provenance) error {
281+
// Verify the TRW was referenced at a proper tag by the user.
282+
id, err := prov.BuilderID()
283+
if err != nil {
284+
return err
285+
}
286+
parts := strings.Split(id, "@")
287+
if len(parts) != 2 {
288+
return fmt.Errorf("%w: %s", serrors.ErrorInvalidBuilderID, id)
289+
}
290+
return utils.IsValidBuilderTag(parts[1], false)
291+
}
292+
280293
func VerifyProvenance(env *dsselib.Envelope, provenanceOpts *options.ProvenanceOpts, byob bool,
281294
) error {
282295
prov, err := slsaprovenance.ProvenanceFromEnvelope(env)
@@ -286,6 +299,9 @@ func VerifyProvenance(env *dsselib.Envelope, provenanceOpts *options.ProvenanceO
286299

287300
// Verify Builder ID.
288301
if byob {
302+
if err := isValidDelegatorBuilderID(prov); err != nil {
303+
return err
304+
}
289305
// Note: `provenanceOpts.ExpectedBuilderID` is provided by the user.
290306
if err := verifyBuilderIDLooseMatch(prov, provenanceOpts.ExpectedBuilderID); err != nil {
291307
return err

verifiers/internal/gha/provenance_test.go

+95
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,101 @@ func Test_verifySourceURI(t *testing.T) {
382382
}
383383
}
384384

385+
func Test_isValidDelegatorBuilderID(t *testing.T) {
386+
t.Parallel()
387+
tests := []struct {
388+
name string
389+
prov *intoto.ProvenanceStatement
390+
err error
391+
}{
392+
{
393+
name: "no @",
394+
prov: &intoto.ProvenanceStatement{
395+
Predicate: slsa02.ProvenancePredicate{
396+
Builder: slsacommon.ProvenanceBuilder{
397+
ID: "some/builderID",
398+
},
399+
},
400+
},
401+
err: serrors.ErrorInvalidBuilderID,
402+
},
403+
{
404+
name: "invalid ref",
405+
prov: &intoto.ProvenanceStatement{
406+
Predicate: slsa02.ProvenancePredicate{
407+
Builder: slsacommon.ProvenanceBuilder{
408+
ID: "some/[email protected]",
409+
},
410+
},
411+
},
412+
err: serrors.ErrorInvalidRef,
413+
},
414+
{
415+
name: "invalid ref not tag",
416+
prov: &intoto.ProvenanceStatement{
417+
Predicate: slsa02.ProvenancePredicate{
418+
Builder: slsacommon.ProvenanceBuilder{
419+
ID: "some/builderID@refs/head/v1.2.3",
420+
},
421+
},
422+
},
423+
err: serrors.ErrorInvalidRef,
424+
},
425+
{
426+
name: "invalid ref not full semver",
427+
prov: &intoto.ProvenanceStatement{
428+
Predicate: slsa02.ProvenancePredicate{
429+
Builder: slsacommon.ProvenanceBuilder{
430+
ID: "some/builderID@refs/heads/v1.2",
431+
},
432+
},
433+
},
434+
err: serrors.ErrorInvalidRef,
435+
},
436+
{
437+
name: "valid builder",
438+
prov: &intoto.ProvenanceStatement{
439+
Predicate: slsa02.ProvenancePredicate{
440+
Builder: slsacommon.ProvenanceBuilder{
441+
ID: "some/builderID@refs/tags/v1.2.3",
442+
},
443+
},
444+
},
445+
},
446+
}
447+
for _, tt := range tests {
448+
tt := tt // Re-initializing variable so it is not changed while executing the closure below
449+
t.Run(tt.name, func(t *testing.T) {
450+
t.Parallel()
451+
452+
prov := &v02.ProvenanceV02{
453+
ProvenanceStatement: tt.prov,
454+
}
455+
456+
err := isValidDelegatorBuilderID(prov)
457+
if !errCmp(err, tt.err) {
458+
t.Errorf(cmp.Diff(err, tt.err))
459+
}
460+
461+
// Update to v1 SLSA provenance.
462+
prov1 := &v1.ProvenanceV1{
463+
Predicate: slsa1.ProvenancePredicate{
464+
RunDetails: slsa1.ProvenanceRunDetails{
465+
Builder: slsa1.Builder{
466+
ID: tt.prov.Predicate.Builder.ID,
467+
},
468+
},
469+
},
470+
}
471+
472+
err = isValidDelegatorBuilderID(prov1)
473+
if !errCmp(err, tt.err) {
474+
t.Errorf(cmp.Diff(err, tt.err))
475+
}
476+
})
477+
}
478+
}
479+
385480
func Test_verifyBuilderIDExactMatch(t *testing.T) {
386481
t.Parallel()
387482
tests := []struct {

verifiers/utils/builder.go

+30
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"fmt"
55
"strings"
66

7+
"golang.org/x/mod/semver"
8+
79
serrors "github.com/slsa-framework/slsa-verifier/v2/errors"
810
)
911

@@ -131,3 +133,31 @@ func TagFromGitHubRef(ref string) (string, error) {
131133
}
132134
return strings.TrimPrefix(ref, "refs/tags/"), nil
133135
}
136+
137+
func IsValidBuilderTag(ref string, testing bool) error {
138+
// Extract the pin.
139+
pin, err := TagFromGitHubRef(ref)
140+
if err != nil {
141+
return err
142+
}
143+
144+
if testing {
145+
// Tags on trusted repositories should be a valid semver with version
146+
// core including all three parts and no build identifier.
147+
versionCore := strings.Split(pin, "-")[0]
148+
if !semver.IsValid(pin) ||
149+
len(strings.Split(versionCore, ".")) != 3 ||
150+
semver.Build(pin) != "" {
151+
return fmt.Errorf("%w: %s: version tag not valid", serrors.ErrorInvalidRef, pin)
152+
}
153+
}
154+
155+
// Valid semver of the form vX.Y.Z with no metadata.
156+
if !semver.IsValid(pin) ||
157+
len(strings.Split(pin, ".")) != 3 ||
158+
semver.Prerelease(pin) != "" ||
159+
semver.Build(pin) != "" {
160+
return fmt.Errorf("%w: %s: not of the form vX.Y.Z", serrors.ErrorInvalidRef, pin)
161+
}
162+
return nil
163+
}

0 commit comments

Comments
 (0)