diff --git a/cli/slsa-verifier/main_test.go b/cli/slsa-verifier/main_test.go index 7c994f4..dff2fb5 100644 --- a/cli/slsa-verifier/main_test.go +++ b/cli/slsa-verifier/main_test.go @@ -68,8 +68,10 @@ func getBuildersAndVersions(t *testing.T, return res } -func Test_runVerifyArtifactPath(t *testing.T) { +func Test_runVerifyGHAArtifactPath(t *testing.T) { t.Parallel() + goBuilder := "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/builder_go_slsa3.yml" + genericBuilder := "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml" tests := []struct { name string artifact string @@ -77,7 +79,7 @@ func Test_runVerifyArtifactPath(t *testing.T) { pbranch *string ptag *string pversiontag *string - pbuilderID *string + pBuilderID *string outBuilderID string inputs map[string]string err error @@ -100,7 +102,7 @@ func Test_runVerifyArtifactPath(t *testing.T) { name: "valid main branch default - invalid builderID", artifact: "binary-linux-amd64-workflow_dispatch", source: "github.com/slsa-framework/example-package", - pbuilderID: pString("https://github.com/slsa-framework/slsa-github-generator/.github/workflows/not-trusted.yml"), + pBuilderID: pString("https://github.com/slsa-framework/slsa-github-generator/.github/workflows/not-trusted.yml"), err: serrors.ErrorUntrustedReusableWorkflow, }, { @@ -380,7 +382,7 @@ func Test_runVerifyArtifactPath(t *testing.T) { source: "github.com/slsa-framework/example-package", minversion: "v1.2.0", builders: []string{"gha_generic"}, - pbuilderID: pString("https://github.com/slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml"), + pBuilderID: pString("https://github.com/slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml"), outBuilderID: "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml", }, // Special case of the e2e test repository building builder from head. @@ -390,6 +392,7 @@ func Test_runVerifyArtifactPath(t *testing.T) { source: "github.com/slsa-framework/example-package", pbranch: pString("main"), noversion: true, + pBuilderID: pString("https://github.com/slsa-framework/slsa-github-generator/.github/workflows/builder_go_slsa3.yml"), outBuilderID: "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/builder_go_slsa3.yml", }, // Malicious builders and workflows. @@ -469,11 +472,12 @@ func Test_runVerifyArtifactPath(t *testing.T) { }, // Regression test of sharded UUID. { - name: "regression: sharded uuids", - artifact: "binary-linux-amd64-sharded", - source: "github.com/slsa-framework/slsa-verifier", - pbranch: pString("release/v1.0"), - noversion: true, + name: "regression: sharded uuids", + artifact: "binary-linux-amd64-sharded", + source: "github.com/slsa-framework/slsa-verifier", + pbranch: pString("release/v1.0"), + pBuilderID: pString("https://github.com/slsa-framework/slsa-github-generator/.github/workflows/builder_go_slsa3.yml"), + noversion: true, }, } for _, tt := range tests { @@ -491,34 +495,87 @@ func Test_runVerifyArtifactPath(t *testing.T) { artifactPath := filepath.Clean(filepath.Join(TEST_DIR, v, tt.artifact)) provenancePath := fmt.Sprintf("%s.intoto.jsonl", artifactPath) - cmd := verify.VerifyArtifactCommand{ - ProvenancePath: provenancePath, - SourceURI: tt.source, - SourceBranch: tt.pbranch, - BuilderID: tt.pbuilderID, - SourceTag: tt.ptag, - SourceVersionTag: tt.pversiontag, - BuildWorkflowInputs: tt.inputs, + // TODO(#258): invalid builder ref. + sv := path.Base(v) + // For each test, we run 4 sub-tests: + // 1. With the the full builderID including the semver in short form. + // 2. With the the full builderID including the semver in long form. + // 3. With only the name of the builder. + // 4. With no builder ID. + var builder string + // Select the right builder based on directory structure. + parts := strings.Split(v, "/") + name := parts[0] + version := "" + if len(parts) > 1 { + version = parts[1] + } + switch { + case strings.HasSuffix(name, "_go"): + builder = goBuilder + case strings.HasSuffix(name, "_generic"): + builder = genericBuilder + default: + builder = genericBuilder } - outBuilderID, err := cmd.Exec(context.Background(), []string{artifactPath}) - - if !errCmp(err, tt.err) { - t.Errorf(cmp.Diff(err, tt.err, cmpopts.EquateErrors())) + // Default builders to test. + builderIDs := []*string{ + pString(builder), + nil, } - if err != nil { - return + // We only add the tags to tests for versions >= 1, + // because we generated them with a builder at `@main` + // before GA. Add the tests for tag verification. + if version != "" && semver.Compare(version, "v1.0.0") > 0 { + builderIDs = append(builderIDs, []*string{ + pString(builder + "@" + sv), + pString(builder + "@refs/tags/" + sv), + }...) } - // Validate against test's expected builderID, if provided. - if tt.outBuilderID != "" { - if err := outBuilderID.Matches(tt.outBuilderID); err != nil { - t.Errorf(fmt.Sprintf("matches failed: %v", err)) + // If builder ID is set, use it. + if tt.pBuilderID != nil { + builderIDs = []*string{tt.pBuilderID} + } + + for _, bid := range builderIDs { + cmd := verify.VerifyArtifactCommand{ + ProvenancePath: provenancePath, + SourceURI: tt.source, + SourceBranch: tt.pbranch, + BuilderID: bid, + SourceTag: tt.ptag, + SourceVersionTag: tt.pversiontag, + BuildWorkflowInputs: tt.inputs, + } + + outBuilderID, err := cmd.Exec(context.Background(), []string{artifactPath}) + if !errCmp(err, tt.err) { + t.Errorf(cmp.Diff(err, tt.err, cmpopts.EquateErrors())) + } + + if err != nil { + return + } + + // Validate against test's expected builderID, if provided. + if tt.outBuilderID != "" { + if err := outBuilderID.Matches(tt.outBuilderID, false); err != nil { + t.Errorf(fmt.Sprintf("matches failed (1): %v", err)) + } + } + + if bid == nil { + return + } + + // Validate against builderID we generated automatically. + if err := outBuilderID.Matches(*bid, false); err != nil { + t.Errorf(fmt.Sprintf("matches failed (2): %v", err)) } } - - // TODO: verify using Matches(). } }) } @@ -571,6 +628,7 @@ func Test_runVerifyGHAArtifactImage(t *testing.T) { return strings.TrimPrefix(h.String(), "sha256:"), nil } + builder := "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/generator_container_slsa3.yml" tests := []struct { name string artifact string @@ -578,7 +636,7 @@ func Test_runVerifyGHAArtifactImage(t *testing.T) { pbranch *string ptag *string pversiontag *string - pbuilderID *string + pBuilderID *string outBuilderID string err error // noversion is a special case where we are not testing all builder versions @@ -587,65 +645,64 @@ func Test_runVerifyGHAArtifactImage(t *testing.T) { // When true, this does not iterate over all builder versions. noversion bool }{ - { - name: "valid main branch default", - artifact: "container_workflow_dispatch", - source: "github.com/slsa-framework/example-package", - }, - { - name: "valid main branch default - invalid builderID", - artifact: "container_workflow_dispatch", - source: "github.com/slsa-framework/example-package", - pbuilderID: pString("https://github.com/slsa-framework/slsa-github-generator/.github/workflows/not-trusted.yml"), - err: serrors.ErrorUntrustedReusableWorkflow, - }, + // { + // name: "valid main branch default", + // artifact: "container_workflow_dispatch", + // source: "github.com/slsa-framework/example-package", + // }, + // { + // name: "valid main branch default - invalid builderID", + // artifact: "container_workflow_dispatch", + // source: "github.com/slsa-framework/example-package", + // pBuilderID: pString("https://github.com/slsa-framework/slsa-github-generator/.github/workflows/not-trusted.yml"), + // err: serrors.ErrorUntrustedReusableWorkflow, + // }, + // { + // name: "valid main branch set", + // artifact: "container_workflow_dispatch", + // source: "github.com/slsa-framework/example-package", + // pbranch: pString("main"), + // }, - { - name: "valid main branch set", - artifact: "container_workflow_dispatch", - source: "github.com/slsa-framework/example-package", - pbranch: pString("main"), - }, - - { - name: "wrong branch master", - artifact: "container_workflow_dispatch", - source: "github.com/slsa-framework/example-package", - pbranch: pString("master"), - err: serrors.ErrorMismatchBranch, - }, - { - name: "wrong source append A", - artifact: "container_workflow_dispatch", - source: "github.com/slsa-framework/example-packageA", - err: serrors.ErrorMismatchSource, - }, - { - name: "wrong source prepend A", - artifact: "container_workflow_dispatch", - source: "Agithub.com/slsa-framework/example-package", - err: serrors.ErrorMismatchSource, - }, - { - name: "wrong source middle A", - artifact: "container_workflow_dispatch", - source: "github.com/Aslsa-framework/example-package", - err: serrors.ErrorMismatchSource, - }, - { - name: "tag no match empty tag workflow_dispatch", - artifact: "container_workflow_dispatch", - source: "github.com/slsa-framework/example-package", - ptag: pString("v1.2.3"), - err: serrors.ErrorMismatchTag, - }, - { - name: "versioned tag no match empty tag workflow_dispatch", - artifact: "container_workflow_dispatch", - source: "github.com/slsa-framework/example-package", - pversiontag: pString("v1"), - err: serrors.ErrorInvalidSemver, - }, + // { + // name: "wrong branch master", + // artifact: "container_workflow_dispatch", + // source: "github.com/slsa-framework/example-package", + // pbranch: pString("master"), + // err: serrors.ErrorMismatchBranch, + // }, + // { + // name: "wrong source append A", + // artifact: "container_workflow_dispatch", + // source: "github.com/slsa-framework/example-packageA", + // err: serrors.ErrorMismatchSource, + // }, + // { + // name: "wrong source prepend A", + // artifact: "container_workflow_dispatch", + // source: "Agithub.com/slsa-framework/example-package", + // err: serrors.ErrorMismatchSource, + // }, + // { + // name: "wrong source middle A", + // artifact: "container_workflow_dispatch", + // source: "github.com/Aslsa-framework/example-package", + // err: serrors.ErrorMismatchSource, + // }, + // { + // name: "tag no match empty tag workflow_dispatch", + // artifact: "container_workflow_dispatch", + // source: "github.com/slsa-framework/example-package", + // ptag: pString("v1.2.3"), + // err: serrors.ErrorMismatchTag, + // }, + // { + // name: "versioned tag no match empty tag workflow_dispatch", + // artifact: "container_workflow_dispatch", + // source: "github.com/slsa-framework/example-package", + // pversiontag: pString("v1"), + // err: serrors.ErrorInvalidSemver, + // }, } for _, tt := range tests { tt := tt // Re-initializing variable so it is not changed while executing the closure below @@ -659,34 +716,60 @@ func Test_runVerifyGHAArtifactImage(t *testing.T) { for _, v := range checkVersions { image := filepath.Clean(filepath.Join(TEST_DIR, v, tt.artifact)) - - cmd := verify.VerifyImageCommand{ - SourceURI: tt.source, - SourceBranch: tt.pbranch, - BuilderID: tt.pbuilderID, - SourceTag: tt.ptag, - SourceVersionTag: tt.pversiontag, - DigestFn: localDigestComputeFn, + // TODO(#258): test for tagged builder. + sv := path.Base(v) + // For each test, we run 2 sub-tests: + // 1. With the the full builderID including the semver in short form. + // 2. With the the full builderID including the semver in long form. + // 3. With only the name of the builder. + // 4. With no builder ID. + builderIDs := []*string{ + pString(builder + "@" + sv), + pString(builder + "@refs/tags/" + sv), + pString(builder), + nil, } - outBuilderID, err := cmd.Exec(context.Background(), []string{image}) - - if !errCmp(err, tt.err) { - t.Errorf(cmp.Diff(err, tt.err, cmpopts.EquateErrors())) + // If builder ID is set, use it. + if tt.pBuilderID != nil { + builderIDs = []*string{tt.pBuilderID} } - if err != nil { - return - } + for _, bid := range builderIDs { + cmd := verify.VerifyImageCommand{ + SourceURI: tt.source, + SourceBranch: tt.pbranch, + BuilderID: bid, + SourceTag: tt.ptag, + SourceVersionTag: tt.pversiontag, + DigestFn: localDigestComputeFn, + } - // Validate against test's expected builderID, if provided. - if tt.outBuilderID != "" { - if err := outBuilderID.Matches(tt.outBuilderID); err != nil { + outBuilderID, err := cmd.Exec(context.Background(), []string{image}) + if !errCmp(err, tt.err) { + t.Errorf(cmp.Diff(err, tt.err, cmpopts.EquateErrors())) + } + + if err != nil { + return + } + + // Validate against test's expected builderID, if provided. + if tt.outBuilderID != "" { + if err := outBuilderID.Matches(tt.outBuilderID, false); err != nil { + t.Errorf(fmt.Sprintf("matches failed: %v", err)) + } + } + + if bid == nil { + return + } + // Validate against builderID we generated automatically. + if err := outBuilderID.Matches(*bid, false); err != nil { t.Errorf(fmt.Sprintf("matches failed: %v", err)) } } - // TODO: verify using Matches(). } }) } @@ -900,9 +983,6 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { // If builder ID is set, use it. if tt.pBuilderID != nil { - // if !tt.noversion { - // panic("builderID set but not noversion option") - // } builderIDs = []string{*tt.pBuilderID} } @@ -950,13 +1030,13 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { // Validate against test's expected builderID, if provided. if tt.outBuilderID != "" { - if err := outBuilderID.Matches(tt.outBuilderID); err != nil { + if err := outBuilderID.Matches(tt.outBuilderID, false); err != nil { t.Errorf(fmt.Sprintf("matches failed: %v", err)) } } // Validate against builderID we generated automatically. - if err := outBuilderID.Matches(bid); err != nil { + if err := outBuilderID.Matches(bid, false); err != nil { t.Errorf(fmt.Sprintf("matches failed: %v", err)) } } diff --git a/cli/slsa-verifier/verify/verify_artifact.go b/cli/slsa-verifier/verify/verify_artifact.go index 16ad8bd..2866455 100644 --- a/cli/slsa-verifier/verify/verify_artifact.go +++ b/cli/slsa-verifier/verify/verify_artifact.go @@ -39,7 +39,7 @@ type VerifyArtifactCommand struct { PrintProvenance bool } -func (c *VerifyArtifactCommand) Exec(ctx context.Context, artifacts []string) (*utils.BuilderID, error) { +func (c *VerifyArtifactCommand) Exec(ctx context.Context, artifacts []string) (*utils.TrustedBuilderID, error) { artifactHash, err := getArtifactHash(artifacts[0]) if err != nil { return nil, err diff --git a/cli/slsa-verifier/verify/verify_image.go b/cli/slsa-verifier/verify/verify_image.go index 489b2c7..53de39c 100644 --- a/cli/slsa-verifier/verify/verify_image.go +++ b/cli/slsa-verifier/verify/verify_image.go @@ -41,7 +41,7 @@ type VerifyImageCommand struct { DigestFn ComputeDigestFn } -func (c *VerifyImageCommand) Exec(ctx context.Context, artifacts []string) (*utils.BuilderID, error) { +func (c *VerifyImageCommand) Exec(ctx context.Context, artifacts []string) (*utils.TrustedBuilderID, error) { artifactImage := artifacts[0] // Retrieve the image digest. if c.DigestFn == nil { diff --git a/errors/errors.go b/errors/errors.go index 359f51a..72ea094 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -21,6 +21,7 @@ var ( ErrorUntrustedReusableWorkflow = errors.New("untrusted reusable workflow") ErrorNoValidRekorEntries = errors.New("could not find a matching valid signature entry") ErrorVerifierNotSupported = errors.New("no verifier support the builder") + ErrorInvalidOIDCIssuer = errors.New("invalid OIDC issuer") ErrorNotSupported = errors.New("not supported") ErrorInvalidFormat = errors.New("invalid format") ErrorInvalidPEM = errors.New("invalid PEM") diff --git a/register/register.go b/register/register.go index a7fd2f6..71d8a93 100644 --- a/register/register.go +++ b/register/register.go @@ -20,14 +20,14 @@ type SLSAVerifier interface { provenance []byte, artifactHash string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, - ) ([]byte, *utils.BuilderID, error) + ) ([]byte, *utils.TrustedBuilderID, error) // VerifyImage verifies a provenance for a supplied OCI image. VerifyImage(ctx context.Context, provenance []byte, artifactImage string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, - ) ([]byte, *utils.BuilderID, error) + ) ([]byte, *utils.TrustedBuilderID, error) } func RegisterVerifier(name string, verifier SLSAVerifier) { diff --git a/verifiers/internal/gcb/provenance.go b/verifiers/internal/gcb/provenance.go index b6d0d54..180a73d 100644 --- a/verifiers/internal/gcb/provenance.go +++ b/verifiers/internal/gcb/provenance.go @@ -222,7 +222,7 @@ func isValidBuilderID(id string) error { return serrors.ErrorInvalidBuilderID } -func validateRecipeType(builderID utils.BuilderID, recipeType string) error { +func validateRecipeType(builderID utils.TrustedBuilderID, recipeType string) error { var err error v := builderID.Version() switch v { @@ -262,7 +262,7 @@ func validateRecipeType(builderID utils.BuilderID, recipeType string) error { // - in the recipe type // - the recipe argument type // - the predicate builder ID. -func (self *Provenance) VerifyBuilder(builderOpts *options.BuilderOpts) (*utils.BuilderID, error) { +func (self *Provenance) VerifyBuilder(builderOpts *options.BuilderOpts) (*utils.TrustedBuilderID, error) { if err := self.isVerified(); err != nil { return nil, err } @@ -275,14 +275,14 @@ func (self *Provenance) VerifyBuilder(builderOpts *options.BuilderOpts) (*utils. return nil, err } - provBuilderID, err := utils.BuilderIDNew(predicateBuilderID) + provBuilderID, err := utils.TrustedBuilderIDNew(predicateBuilderID) if err != nil { return nil, err } // Validate with user-provided value. if builderOpts != nil && builderOpts.ExpectedID != nil { - if err := provBuilderID.Matches(*builderOpts.ExpectedID); err != nil { + if err := provBuilderID.Matches(*builderOpts.ExpectedID, false); err != nil { return nil, err } } @@ -346,7 +346,7 @@ func (self *Provenance) VerifySubjectDigest(expectedHash string) error { } // Verify source URI in provenance statement. -func (self *Provenance) VerifySourceURI(expectedSourceURI string, builderID utils.BuilderID) error { +func (self *Provenance) VerifySourceURI(expectedSourceURI string, builderID utils.TrustedBuilderID) error { if err := self.isVerified(); err != nil { return err } diff --git a/verifiers/internal/gcb/provenance_test.go b/verifiers/internal/gcb/provenance_test.go index 993766d..0d5ab91 100644 --- a/verifiers/internal/gcb/provenance_test.go +++ b/verifiers/internal/gcb/provenance_test.go @@ -227,7 +227,7 @@ func Test_VerifyBuilder(t *testing.T) { panic("outBuilderID is nil") } - if err := outBuilderID.Matches(tt.builderID); err != nil { + if err := outBuilderID.Matches(tt.builderID, false); err != nil { t.Errorf(fmt.Sprintf("matches failed: %v", err)) } }) @@ -290,7 +290,7 @@ func Test_validateRecipeType(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - builderID, err := utils.BuilderIDNew(tt.builderID) + builderID, err := utils.TrustedBuilderIDNew(tt.builderID) if err != nil { panic(fmt.Errorf("BuilderIDNew: %w", err)) } @@ -416,7 +416,7 @@ func Test_VerifySourceURI(t *testing.T) { panic(fmt.Errorf("ProvenanceFromBytes: %w", err)) } - builderID, err := utils.BuilderIDNew(tt.builderID) + builderID, err := utils.TrustedBuilderIDNew(tt.builderID) if err != nil { panic(fmt.Errorf("BuilderIDNew: %w", err)) } diff --git a/verifiers/internal/gcb/verifier.go b/verifiers/internal/gcb/verifier.go index fbe727d..522e8af 100644 --- a/verifiers/internal/gcb/verifier.go +++ b/verifiers/internal/gcb/verifier.go @@ -35,7 +35,7 @@ func (v *GCBVerifier) VerifyArtifact(ctx context.Context, provenance []byte, artifactHash string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, -) ([]byte, *utils.BuilderID, error) { +) ([]byte, *utils.TrustedBuilderID, error) { return nil, nil, serrors.ErrorNotSupported } @@ -44,7 +44,7 @@ func (v *GCBVerifier) VerifyImage(ctx context.Context, provenance []byte, artifactImage string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, -) ([]byte, *utils.BuilderID, error) { +) ([]byte, *utils.TrustedBuilderID, error) { prov, err := ProvenanceFromBytes(provenance) if err != nil { return nil, nil, err diff --git a/verifiers/internal/gha/builder.go b/verifiers/internal/gha/builder.go index 05223fb..1f49936 100644 --- a/verifiers/internal/gha/builder.go +++ b/verifiers/internal/gha/builder.go @@ -10,6 +10,7 @@ import ( serrors "github.com/slsa-framework/slsa-verifier/errors" "github.com/slsa-framework/slsa-verifier/options" + "github.com/slsa-framework/slsa-verifier/verifiers/utils" ) var ( @@ -33,29 +34,30 @@ var defaultContainerTrustedReusableWorkflows = map[string]bool{ func VerifyWorkflowIdentity(id *WorkflowIdentity, builderOpts *options.BuilderOpts, source string, defaultBuilders map[string]bool, -) (string, error) { +) (*utils.TrustedBuilderID, error) { // cert URI path is /org/repo/path/to/workflow@ref workflowPath := strings.SplitN(id.JobWobWorkflowRef, "@", 2) if len(workflowPath) < 2 { - return "", fmt.Errorf("%w: workflow uri: %s", serrors.ErrorMalformedURI, id.JobWobWorkflowRef) + return nil, fmt.Errorf("%w: workflow uri: %s", serrors.ErrorMalformedURI, id.JobWobWorkflowRef) } - // Trusted workflow verification by name. + // Verify trusted workflow. reusableWorkflowPath := strings.Trim(workflowPath[0], "/") - builderID, err := verifyTrustedBuilderID(reusableWorkflowPath, + reusableWorkflowTag := strings.Trim(workflowPath[1], "/") + builderID, err := verifyTrustedBuilderID(reusableWorkflowPath, reusableWorkflowTag, builderOpts.ExpectedID, defaultBuilders) if err != nil { - return "", err + return nil, err } - // Verify the ref. - if err := verifyTrustedBuilderRef(id, strings.Trim(workflowPath[1], "/")); err != nil { - return "", err + // Verify the ref is a full semantic version tag. + if err := verifyTrustedBuilderRef(id, reusableWorkflowTag); err != nil { + return nil, err } // Issuer verification. if !strings.EqualFold(id.Issuer, certOidcIssuer) { - return "", fmt.Errorf("untrusted token issuer: %s", id.Issuer) + return nil, fmt.Errorf("%w: %s", serrors.ErrorInvalidOIDCIssuer, id.Issuer) } // The caller repository in the x509 extension is not fully qualified. It only contains @@ -63,33 +65,50 @@ func VerifyWorkflowIdentity(id *WorkflowIdentity, expectedSource := strings.TrimPrefix(source, "git+https://") expectedSource = strings.TrimPrefix(expectedSource, "github.com/") if !strings.EqualFold(id.CallerRepository, expectedSource) { - return "", fmt.Errorf("%w: expected source '%s', got '%s'", serrors.ErrorMismatchSource, + return nil, fmt.Errorf("%w: expected source '%s', got '%s'", serrors.ErrorMismatchSource, expectedSource, id.CallerRepository) } // Return the builder and its tag. + // Note: the tag has the format `refs/tags/v1.2.3`. return builderID, nil } // Verifies the builder ID at path against an expected builderID. // If an expected builderID is not provided, uses the defaultBuilders. -func verifyTrustedBuilderID(path string, builderID *string, defaultBuilders map[string]bool) (string, error) { +func verifyTrustedBuilderID(certPath, certTag string, expectedBuilderID *string, defaultBuilders map[string]bool) (*utils.TrustedBuilderID, error) { + var trustedBuilderID *utils.TrustedBuilderID + var err error + certBuilderName := "https://github.com/" + certPath + // WARNING: we don't validate the tag here, because we need to allow + // refs/heads/main for e2e tests. See verifyTrustedBuilderRef(). // No builder ID provided by user: use the default trusted workflows. - if builderID == nil || *builderID == "" { - if _, ok := defaultBuilders[path]; !ok { - return "", fmt.Errorf("%w: %s got %t", serrors.ErrorUntrustedReusableWorkflow, path, builderID == nil) + if expectedBuilderID == nil || *expectedBuilderID == "" { + if _, ok := defaultBuilders[certPath]; !ok { + return nil, fmt.Errorf("%w: %s got %t", serrors.ErrorUntrustedReusableWorkflow, certPath, expectedBuilderID == nil) + } + // Construct the builderID using the certificate's builder's name and tag. + trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderName + "@" + certTag) + if err != nil { + return nil, err } } else { // Verify the builderID. // We only accept IDs on github.com. - url := "https://github.com/" + path - if url != *builderID { - return "", fmt.Errorf("%w: expected buildID '%s', got '%s'", serrors.ErrorUntrustedReusableWorkflow, - *builderID, url) + trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderName + "@" + certTag) + if err != nil { + return nil, err + } + + // BuilderID provided by user should match the certificate. + // Note: the certificate builderID has the form `name@refs/tags/v1.2.3`, + // so we pass `allowRef = true`. + if err := trustedBuilderID.Matches(*expectedBuilderID, true); err != nil { + return nil, fmt.Errorf("%w: %v", serrors.ErrorUntrustedReusableWorkflow, err) } } - return "https://github.com/" + path, nil + return trustedBuilderID, nil } // Only allow `@refs/heads/main` for the builder and the e2e tests that need to work at HEAD. @@ -102,12 +121,13 @@ func verifyTrustedBuilderRef(id *WorkflowIdentity, ref string) error { return nil } - if !strings.HasPrefix(ref, "refs/tags/") { - return fmt.Errorf("%w: %s: not of the form 'refs/tags/name'", serrors.ErrorInvalidRef, ref) + // Extract the pin. + pin, err := utils.TagFromGitHubRef(ref) + if err != nil { + return err } // Valid semver of the form vX.Y.Z with no metadata. - pin := strings.TrimPrefix(ref, "refs/tags/") if !(semver.IsValid(pin) && len(strings.Split(pin, ".")) == 3 && semver.Prerelease(pin) == "" && diff --git a/verifiers/internal/gha/builder_test.go b/verifiers/internal/gha/builder_test.go index 1073d30..90940d5 100644 --- a/verifiers/internal/gha/builder_test.go +++ b/verifiers/internal/gha/builder_test.go @@ -39,7 +39,7 @@ func Test_VerifyWorkflowIdentity(t *testing.T) { workflow: &WorkflowIdentity{ CallerRepository: "asraa/slsa-on-github-test", CallerHash: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - JobWobWorkflowRef: "/malicious/slsa-go/.github/workflows/builder.yml@refs/heads/main", + JobWobWorkflowRef: "/malicious/slsa-go/.github/workflows/builder.yml@refs/tags/v1.2.3", Trigger: "workflow_dispatch", Issuer: "https://token.actions.githubusercontent.com", }, @@ -61,11 +61,24 @@ func Test_VerifyWorkflowIdentity(t *testing.T) { err: serrors.ErrorInvalidRef, }, { - name: "valid main ref for trusted builder", + name: "untrusted cert issuer for general repos", + workflow: &WorkflowIdentity{ + CallerRepository: "asraa/slsa-on-github-test", + CallerHash: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", + Trigger: "workflow_dispatch", + Issuer: "https://bad.issuer.com", + }, + source: "asraa/slsa-on-github-test", + defaults: defaultArtifactTrustedReusableWorkflows, + err: serrors.ErrorInvalidOIDCIssuer, + }, + { + name: "valid trusted builder without tag", workflow: &WorkflowIdentity{ CallerRepository: trustedBuilderRepository, CallerHash: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/heads/main", + JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", Trigger: "workflow_dispatch", Issuer: "https://token.actions.githubusercontent.com", }, @@ -78,7 +91,7 @@ func Test_VerifyWorkflowIdentity(t *testing.T) { workflow: &WorkflowIdentity{ CallerRepository: e2eTestRepository, CallerHash: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/heads/main", + JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", Trigger: "workflow_dispatch", Issuer: certOidcIssuer, }, @@ -91,7 +104,7 @@ func Test_VerifyWorkflowIdentity(t *testing.T) { workflow: &WorkflowIdentity{ CallerRepository: e2eTestRepository, CallerHash: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/heads/main", + JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", Trigger: "workflow_dispatch", Issuer: certOidcIssuer, }, @@ -107,7 +120,7 @@ func Test_VerifyWorkflowIdentity(t *testing.T) { workflow: &WorkflowIdentity{ CallerRepository: e2eTestRepository, CallerHash: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/heads/main", + JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", Trigger: "workflow_dispatch", Issuer: certOidcIssuer, }, @@ -123,7 +136,7 @@ func Test_VerifyWorkflowIdentity(t *testing.T) { workflow: &WorkflowIdentity{ CallerRepository: e2eTestRepository, CallerHash: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/heads/main", + JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", Trigger: "workflow_dispatch", Issuer: certOidcIssuer, }, @@ -136,7 +149,7 @@ func Test_VerifyWorkflowIdentity(t *testing.T) { name: "valid main ref for builder", workflow: &WorkflowIdentity{ CallerRepository: trustedBuilderRepository, - JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/heads/main", + JobWobWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", Trigger: "workflow_dispatch", Issuer: certOidcIssuer, }, @@ -332,8 +345,9 @@ func Test_VerifyWorkflowIdentity(t *testing.T) { if err != nil { return } - if id != tt.builderID { - t.Errorf(cmp.Diff(id, tt.builderID)) + + if err := id.Matches(tt.builderID, true); err != nil { + t.Errorf("matches failed:%v", err) } }) } @@ -349,46 +363,111 @@ func Test_verifyTrustedBuilderID(t *testing.T) { name string id *string path string + tag string defaults map[string]bool expected error }{ { - name: "default trusted", + name: "default trusted short tag", path: trustedBuilderRepository + "/.github/workflows/generator_generic_slsa3.yml", + tag: "v1.2.3", defaults: defaultArtifactTrustedReusableWorkflows, }, { - name: "default mismatch against container defaults", + name: "default trusted long tag", path: trustedBuilderRepository + "/.github/workflows/generator_generic_slsa3.yml", + tag: "refs/tags/v1.2.3", + defaults: defaultArtifactTrustedReusableWorkflows, + }, + { + name: "default mismatch against container defaults long tag", + path: trustedBuilderRepository + "/.github/workflows/generator_generic_slsa3.yml", + tag: "refs/tags/v1.2.3", defaults: defaultContainerTrustedReusableWorkflows, expected: serrors.ErrorUntrustedReusableWorkflow, }, { - name: "valid ID for GitHub builder", + name: "valid ID for GitHub builder short tag", path: "some/repo/someBuilderID", - id: asStringPointer("https://github.com/some/repo/someBuilderID"), + tag: "v1.2.3", + id: asStringPointer("https://github.com/some/repo/someBuilderID@v1.2.3"), }, { - name: "non GitHub builder ID", + name: "valid ID for GitHub builder long tag", + path: "some/repo/someBuilderID", + tag: "refs/tags/v1.2.3", + id: asStringPointer("https://github.com/some/repo/someBuilderID@refs/tags/v1.2.3"), + }, + { + name: "valid short ID for GitHub builder long tag", + path: "some/repo/someBuilderID", + tag: "refs/tags/v1.2.3", + id: asStringPointer("https://github.com/some/repo/someBuilderID@v1.2.3"), + }, + { + name: "valid long ID for GitHub builder short tag", path: "some/repo/someBuilderID", + tag: "v1.2.3", + id: asStringPointer("https://github.com/some/repo/someBuilderID@refs/tags/v1.2.3"), + expected: serrors.ErrorUntrustedReusableWorkflow, + }, + { + name: "valid ID for GitHub builder long tag", + path: "some/repo/someBuilderID", + tag: "refs/tags/v1.2.3", + id: asStringPointer("https://github.com/some/repo/someBuilderID@refs/tags/v1.2.3"), + }, + { + name: "valid ID for GitHub builder short tag", + path: "some/repo/someBuilderID", + tag: "v1.2.3", + id: asStringPointer("https://github.com/some/repo/someBuilderID@v1.2.3"), + }, + { + name: "valid short ID for GitHub builder long tag", + path: "some/repo/someBuilderID", + tag: "refs/tags/v1.2.3", + id: asStringPointer("https://github.com/some/repo/someBuilderID@v1.2.3"), + }, + { + name: "valid long ID for GitHub builder short tag", + path: "some/repo/someBuilderID", + tag: "v1.2.3", + id: asStringPointer("https://github.com/some/repo/someBuilderID@refs/tags/v1.2.3"), + expected: serrors.ErrorUntrustedReusableWorkflow, + }, + { + name: "non GitHub builder ID long builder tag", + path: "some/repo/someBuilderID", + tag: "refs/tags/v1.2.3", id: asStringPointer("https://not-github.com/some/repo/someBuilderID"), expected: serrors.ErrorUntrustedReusableWorkflow, }, { - name: "mismatch org GitHub", + name: "mismatch org GitHub short builder tag", path: "some/repo/someBuilderID", + tag: "v1.2.3", id: asStringPointer("https://github.com/other/repo/someBuilderID"), expected: serrors.ErrorUntrustedReusableWorkflow, }, { - name: "mismatch name GitHub", + name: "mismatch org GitHub long builder tag", path: "some/repo/someBuilderID", + tag: "refs/tags/v1.2.3", + id: asStringPointer("https://github.com/other/repo/someBuilderID"), + expected: serrors.ErrorUntrustedReusableWorkflow, + }, + { + name: "mismatch name GitHub long builder tag", + path: "some/repo/someBuilderID", + tag: "refs/tags/v1.2.3", id: asStringPointer("https://github.com/some/other/someBuilderID"), expected: serrors.ErrorUntrustedReusableWorkflow, }, { - name: "mismatch id GitHub", + name: "mismatch id GitHub long builder tag", path: "some/repo/someBuilderID", + tag: "refs/tags/v1.2.3", id: asStringPointer("https://github.com/some/repo/ID"), expected: serrors.ErrorUntrustedReusableWorkflow, }, @@ -398,16 +477,16 @@ func Test_verifyTrustedBuilderID(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - id, err := verifyTrustedBuilderID(tt.path, tt.id, tt.defaults) + id, err := verifyTrustedBuilderID(tt.path, tt.tag, tt.id, tt.defaults) if !errCmp(err, tt.expected) { t.Errorf(cmp.Diff(err, tt.expected, cmpopts.EquateErrors())) } if err != nil { return } - expectedID := "https://github.com/" + tt.path - if id != expectedID { - t.Errorf(cmp.Diff(id, expectedID)) + expectedID := "https://github.com/" + tt.path + "@" + tt.tag + if err := id.Matches(expectedID, true); err != nil { + t.Errorf("matches failed:%v", err) } }) } diff --git a/verifiers/internal/gha/provenance.go b/verifiers/internal/gha/provenance.go index 1ab45a5..ab26ec8 100644 --- a/verifiers/internal/gha/provenance.go +++ b/verifiers/internal/gha/provenance.go @@ -42,22 +42,14 @@ func provenanceFromEnv(env *dsselib.Envelope) (prov *intoto.ProvenanceStatement, } // Verify Builder ID in provenance statement. -func verifyBuilderID(prov *intoto.ProvenanceStatement, builderID string) error { - // Check that the BuilderID is well-formed. - provid, err := sourceFromURI(prov.Predicate.Builder.ID, false) - if err != nil { - return err - } - // Note: builderID does not contain the tag. - // TODO(#189): support cases where user wants to match on the full builderID, including the tag. - bid, err := sourceFromURI(builderID, true) - if err != nil { - return err - } - if provid != bid { +// This function does an exact comparison, and expects certBuilderID to be the full +// `name@refs/tags/`. +func verifyBuilderIDExactMatch(prov *intoto.ProvenanceStatement, certBuilderID string) error { + if certBuilderID != prov.Predicate.Builder.ID { return fmt.Errorf("%w: expected '%s' in builder.id, got '%s'", serrors.ErrorMismatchBuilderID, - bid, provid) + certBuilderID, prov.Predicate.Builder.ID) } + return nil } @@ -193,7 +185,9 @@ func VerifyProvenance(env *dsselib.Envelope, provenanceOpts *options.ProvenanceO } // Verify Builder ID. - if err := verifyBuilderID(prov, provenanceOpts.ExpectedBuilderID); err != nil { + // Note: `provenanceOpts.ExpectedBuilderID` is not provided by the user, + // but taken from the certificate. It always is of the form `name@refs/tags/`. + if err := verifyBuilderIDExactMatch(prov, provenanceOpts.ExpectedBuilderID); err != nil { return err } diff --git a/verifiers/internal/gha/provenance_test.go b/verifiers/internal/gha/provenance_test.go index a0aeed0..aebc542 100644 --- a/verifiers/internal/gha/provenance_test.go +++ b/verifiers/internal/gha/provenance_test.go @@ -354,7 +354,7 @@ func Test_verifySourceURI(t *testing.T) { } } -func Test_verifyBuilderID(t *testing.T) { +func Test_verifyBuilderIDExactMatch(t *testing.T) { t.Parallel() tests := []struct { name string @@ -363,7 +363,7 @@ func Test_verifyBuilderID(t *testing.T) { expected error }{ { - name: "id has no @", + name: "match no version", prov: &intoto.ProvenanceStatement{ Predicate: slsa.ProvenancePredicate{ Builder: slsa.ProvenanceBuilder{ @@ -371,22 +371,10 @@ func Test_verifyBuilderID(t *testing.T) { }, }, }, - id: "some/builderID", - expected: serrors.ErrorMalformedURI, - }, - { - name: "same builderID", - prov: &intoto.ProvenanceStatement{ - Predicate: slsa.ProvenancePredicate{ - Builder: slsa.ProvenanceBuilder{ - ID: "some/builderID@v1.2.3", - }, - }, - }, id: "some/builderID", }, { - name: "same builderID full match", + name: "match with tag", prov: &intoto.ProvenanceStatement{ Predicate: slsa.ProvenancePredicate{ Builder: slsa.ProvenanceBuilder{ @@ -405,11 +393,12 @@ func Test_verifyBuilderID(t *testing.T) { }, }, }, - id: "some/builderID@v1.2.4", + id: "some/builderID@v1.2.4", + expected: serrors.ErrorMismatchBuilderID, // TODO(#189): this should fail. }, { - name: "mismatch builderID", + name: "mismatch builderID same version", prov: &intoto.ProvenanceStatement{ Predicate: slsa.ProvenancePredicate{ Builder: slsa.ProvenanceBuilder{ @@ -417,14 +406,26 @@ func Test_verifyBuilderID(t *testing.T) { }, }, }, + id: "some/builderID@v1.2.3", + expected: serrors.ErrorMismatchBuilderID, + }, + { + name: "empty prov builderID", + prov: &intoto.ProvenanceStatement{}, id: "some/builderID", expected: serrors.ErrorMismatchBuilderID, }, { - name: "empty builderID", - prov: &intoto.ProvenanceStatement{}, - id: "some/builderID", - expected: serrors.ErrorMalformedURI, + name: "empty expected builderID", + prov: &intoto.ProvenanceStatement{ + Predicate: slsa.ProvenancePredicate{ + Builder: slsa.ProvenanceBuilder{ + ID: "tome/builderID@v1.2.3", + }, + }, + }, + id: "", + expected: serrors.ErrorMismatchBuilderID, }, } for _, tt := range tests { @@ -432,7 +433,7 @@ func Test_verifyBuilderID(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := verifyBuilderID(tt.prov, tt.id) + err := verifyBuilderIDExactMatch(tt.prov, tt.id) if !errCmp(err, tt.expected) { t.Errorf(cmp.Diff(err, tt.expected)) } diff --git a/verifiers/internal/gha/verifier.go b/verifiers/internal/gha/verifier.go index 4fc87a3..3fb0c67 100644 --- a/verifiers/internal/gha/verifier.go +++ b/verifiers/internal/gha/verifier.go @@ -13,6 +13,7 @@ import ( "github.com/sigstore/cosign/cmd/cosign/cli/rekor" "github.com/sigstore/cosign/pkg/cosign" + serrors "github.com/slsa-framework/slsa-verifier/errors" "github.com/slsa-framework/slsa-verifier/options" "github.com/slsa-framework/slsa-verifier/register" "github.com/slsa-framework/slsa-verifier/verifiers/utils" @@ -44,7 +45,7 @@ func verifyEnvAndCert(env *dsse.Envelope, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, defaultBuilders map[string]bool, -) ([]byte, *utils.BuilderID, error) { +) ([]byte, *utils.TrustedBuilderID, error) { /* Verify properties of the signing identity. */ // Get the workflow info given the certificate information. workflowInfo, err := GetWorkflowInfoFromCertificate(cert) @@ -61,7 +62,7 @@ func verifyEnvAndCert(env *dsse.Envelope, // Verify properties of the SLSA provenance. // Unpack and verify info in the provenance, including the Subject Digest. - provenanceOpts.ExpectedBuilderID = builderID + provenanceOpts.ExpectedBuilderID = builderID.String() if err := VerifyProvenance(env, provenanceOpts); err != nil { return nil, nil, err } @@ -75,13 +76,7 @@ func verifyEnvAndCert(env *dsse.Envelope, return nil, nil, err } - // Temporary code. - // TODO: remove `SetName` and `SetVersion` function once GHA is supported, - // and use: bid, err := utils.BuilderIDNew(builderID) - bid := &utils.BuilderID{} - bid.SetName(builderID) - bid.SetVersion(strings.Split(workflowInfo.JobWobWorkflowRef, "@")[1]) - return r, bid, nil + return r, builderID, nil } // VerifyArtifact verifies provenance for an artifact. @@ -89,7 +84,7 @@ func (v *GHAVerifier) VerifyArtifact(ctx context.Context, provenance []byte, artifactHash string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, -) ([]byte, *utils.BuilderID, error) { +) ([]byte, *utils.TrustedBuilderID, error) { rClient, err := rekor.NewClient(defaultRekorAddr) if err != nil { return nil, nil, err @@ -111,7 +106,7 @@ func (v *GHAVerifier) VerifyImage(ctx context.Context, provenance []byte, artifactImage string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, -) ([]byte, *utils.BuilderID, error) { +) ([]byte, *utils.TrustedBuilderID, error) { /* Retrieve any valid signed attestations that chain up to Fulcio root CA. */ roots, err := fulcio.GetRoots() if err != nil { @@ -128,8 +123,8 @@ func (v *GHAVerifier) VerifyImage(ctx context.Context, } /* Now verify properties of the attestations */ - var verifyErr error - var builderID *utils.BuilderID + var errs []error + var builderID *utils.TrustedBuilderID var verifiedProvenance []byte for _, att := range atts { pyld, err := att.Payload() @@ -147,13 +142,22 @@ func (v *GHAVerifier) VerifyImage(ctx context.Context, fmt.Fprintf(os.Stderr, "unexpected error getting certificate from OCI registry %s", err) continue } - verifiedProvenance, builderID, verifyErr = verifyEnvAndCert(env, + verifiedProvenance, builderID, err = verifyEnvAndCert(env, cert, provenanceOpts, builderOpts, defaultContainerTrustedReusableWorkflows) - if verifyErr == nil { + if err == nil { return verifiedProvenance, builderID, nil } + errs = append(errs, err) } - return nil, nil, fmt.Errorf("no valid attestations found on OCI registry: %w", verifyErr) + // Return the first error. + if len(errs) > 0 { + var s string + if len(errs) > 1 { + s = fmt.Sprintf(": %v", errs[1:]) + } + return nil, nil, fmt.Errorf("%w%s", errs[0], s) + } + return nil, nil, fmt.Errorf("%w", serrors.ErrorNoValidSignature) } diff --git a/verifiers/utils/builder.go b/verifiers/utils/builder.go index 182cb57..990284b 100644 --- a/verifiers/utils/builder.go +++ b/verifiers/utils/builder.go @@ -7,18 +7,18 @@ import ( serrors "github.com/slsa-framework/slsa-verifier/errors" ) -type BuilderID struct { +type TrustedBuilderID struct { name, version string } -// BuilderIDNew creates a new BuilderID structure. -func BuilderIDNew(builderID string) (*BuilderID, error) { +// TrustedBuilderIDNew creates a new BuilderID structure. +func TrustedBuilderIDNew(builderID string) (*TrustedBuilderID, error) { name, version, err := ParseBuilderID(builderID, true) if err != nil { return nil, err } - return &BuilderID{ + return &TrustedBuilderID{ name: name, version: version, }, nil @@ -27,7 +27,11 @@ func BuilderIDNew(builderID string) (*BuilderID, error) { // Matches matches the builderID string against the reference builderID. // If the builderID contains a semver, the full builderID must match. // Otherwise, only the name needs to match. -func (b *BuilderID) Matches(builderID string) error { +// `allowRef: true` indicates that the matching need not be an eaxct +// match. In this case, if the BuilderID version is a GitHub ref +// `refs/tags/name`, we will consider it equal to user-provided +// builderID `name`. +func (b *TrustedBuilderID) Matches(builderID string, allowRef bool) error { name, version, err := ParseBuilderID(builderID, false) if err != nil { return err @@ -39,6 +43,11 @@ func (b *BuilderID) Matches(builderID string) error { } if version != "" && version != b.version { + // If allowRef is true, try the long version `refs/tags/` match. + if allowRef && + "refs/tags/"+version == b.version { + return nil + } return fmt.Errorf("%w: expected version '%s', got '%s'", serrors.ErrorMismatchBuilderID, version, b.version) } @@ -46,27 +55,18 @@ func (b *BuilderID) Matches(builderID string) error { return nil } -func (b *BuilderID) Name() string { +func (b *TrustedBuilderID) Name() string { return b.name } -func (b *BuilderID) Version() string { +func (b *TrustedBuilderID) Version() string { return b.version } -func (b *BuilderID) String() string { +func (b *TrustedBuilderID) String() string { return fmt.Sprintf("%s@%s", b.name, b.version) } -// TODO: remove this function once GHA is supported. -func (b *BuilderID) SetName(name string) { - b.name = name -} - -func (b *BuilderID) SetVersion(version string) { - b.version = version -} - func ParseBuilderID(id string, needVersion bool) (string, string, error) { parts := strings.Split(id, "@") if len(parts) == 2 { @@ -84,3 +84,17 @@ func ParseBuilderID(id string, needVersion bool) (string, string, error) { return "", "", fmt.Errorf("%w: builderID: '%s'", serrors.ErrorInvalidFormat, id) } + +func ValidateGitHubTagRef(tag string) error { + if !strings.HasPrefix(tag, "refs/tags/") { + return fmt.Errorf("%w: %s: not of the form 'refs/tags/name'", serrors.ErrorInvalidRef, tag) + } + return nil +} + +func TagFromGitHubRef(ref string) (string, error) { + if err := ValidateGitHubTagRef(ref); err != nil { + return "", err + } + return strings.TrimPrefix(ref, "refs/tags/"), nil +} diff --git a/verifiers/utils/builder_test.go b/verifiers/utils/builder_test.go index 23f1d49..0ce6c9e 100644 --- a/verifiers/utils/builder_test.go +++ b/verifiers/utils/builder_test.go @@ -94,32 +94,32 @@ func Test_ParseBuilderID(t *testing.T) { func Test_BuilderIDNew(t *testing.T) { t.Parallel() tests := []struct { - name string - builderID string - builderName string - builderVersion string - err error + name string + trustedBuilderID string + builderName string + builderVersion string + err error }{ { - name: "valid", - builderID: "some/name@v1.2.3", - builderName: "some/name", - builderVersion: "v1.2.3", + name: "valid", + trustedBuilderID: "some/name@v1.2.3", + builderName: "some/name", + builderVersion: "v1.2.3", }, { - name: "empty version", - builderID: "some/name@", - err: serrors.ErrorInvalidFormat, + name: "empty version", + trustedBuilderID: "some/name@", + err: serrors.ErrorInvalidFormat, }, { - name: "too many '@' - need version", - builderID: "some/name@vla@blo", - err: serrors.ErrorInvalidFormat, + name: "too many '@' - need version", + trustedBuilderID: "some/name@vla@blo", + err: serrors.ErrorInvalidFormat, }, { - name: "too many '@' - no need version", - builderID: "some/name@vla@blo", - err: serrors.ErrorInvalidFormat, + name: "too many '@' - no need version", + trustedBuilderID: "some/name@vla@blo", + err: serrors.ErrorInvalidFormat, }, } for _, tt := range tests { @@ -127,7 +127,7 @@ func Test_BuilderIDNew(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - builderID, err := BuilderIDNew(tt.builderID) + trustedBuilderID, err := TrustedBuilderIDNew(tt.trustedBuilderID) if !cmp.Equal(err, tt.err, cmpopts.EquateErrors()) { t.Errorf(cmp.Diff(err, tt.err)) } @@ -136,9 +136,9 @@ func Test_BuilderIDNew(t *testing.T) { return } - name := builderID.Name() - version := builderID.Version() - full := builderID.String() + name := trustedBuilderID.Name() + version := trustedBuilderID.Version() + full := trustedBuilderID.String() if name != tt.builderName { t.Errorf(cmp.Diff(tt.builderName, name)) @@ -146,8 +146,8 @@ func Test_BuilderIDNew(t *testing.T) { if version != tt.builderVersion { t.Errorf(cmp.Diff(tt.builderVersion, version)) } - if full != tt.builderID { - t.Errorf(cmp.Diff(tt.builderID, full)) + if full != tt.trustedBuilderID { + t.Errorf(cmp.Diff(tt.trustedBuilderID, full)) } }) } @@ -156,50 +156,130 @@ func Test_BuilderIDNew(t *testing.T) { func Test_Matches(t *testing.T) { t.Parallel() tests := []struct { - name string - builderID string - match string - err error + name string + trustedBuilderID string + allowRef bool + match string + err error }{ { - name: "match full", - builderID: "some/name@v1.2.3", - match: "some/name@v1.2.3", + name: "match full", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name@v1.2.3", }, { - name: "match name", - builderID: "some/name@v1.2.3", - match: "some/name", + name: "match name", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name", }, { - name: "mismatch name", - builderID: "some/name@v1.2.3", - match: "some/name2", - err: serrors.ErrorMismatchBuilderID, + name: "mismatch name", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name2", + err: serrors.ErrorMismatchBuilderID, }, { - name: "mismatch version", - builderID: "some/name@v1.2.3", - match: "some/name@v1.2.4", - err: serrors.ErrorMismatchBuilderID, + name: "mismatch version", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name@v1.2.4", + err: serrors.ErrorMismatchBuilderID, }, { - name: "invalid empty version", - builderID: "some/name@v1.2.3", - match: "some/name@", - err: serrors.ErrorInvalidFormat, + name: "invalid empty version", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name@", + err: serrors.ErrorInvalidFormat, }, { - name: "too many '@' - need version", - builderID: "some/name@v1.2.3", - match: "some/name@vla@blo", - err: serrors.ErrorInvalidFormat, + name: "too many '@' - need version", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name@vla@blo", + err: serrors.ErrorInvalidFormat, }, { - name: "too many '@' - no need version", - builderID: "some/name@v1.2.3", - match: "some/name@vla@blo", - err: serrors.ErrorInvalidFormat, + name: "too many '@' - no need version", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name@vla@blo", + err: serrors.ErrorInvalidFormat, + }, + // Same as above with `allowRef: true`. + { + name: "match full", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name@v1.2.3", + allowRef: true, + }, + { + name: "match name", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name", + allowRef: true, + }, + { + name: "mismatch name", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name2", + allowRef: true, + err: serrors.ErrorMismatchBuilderID, + }, + { + name: "mismatch version", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name@v1.2.4", + allowRef: true, + err: serrors.ErrorMismatchBuilderID, + }, + { + name: "invalid empty version", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name@", + allowRef: true, + err: serrors.ErrorInvalidFormat, + }, + { + name: "too many '@' - need version", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name@vla@blo", + allowRef: true, + err: serrors.ErrorInvalidFormat, + }, + { + name: "too many '@' - no need version", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name@vla@blo", + allowRef: true, + err: serrors.ErrorInvalidFormat, + }, + // Mismatch of tag length. + { + name: "match long tag match short", + trustedBuilderID: "some/name@refs/tags/v1.2.3", + match: "some/name@v1.2.3", + allowRef: true, + }, + { + name: "long tag match short no ref", + trustedBuilderID: "some/name@refs/tags/v1.2.3", + match: "some/name@v1.2.3", + err: serrors.ErrorMismatchBuilderID, + }, + { + name: "match long tags", + trustedBuilderID: "some/name@refs/tags/v1.2.3", + match: "some/name@refs/tags/v1.2.3", + allowRef: true, + }, + { + name: "mismatch tag length", + trustedBuilderID: "some/name@refs/tags/v1.2.3", + match: "some/name@v1.2.3", + err: serrors.ErrorMismatchBuilderID, + }, + { + name: "mismatch tag length inversed", + trustedBuilderID: "some/name@v1.2.3", + match: "some/name@refs/tags/v1.2.3", + err: serrors.ErrorMismatchBuilderID, }, } for _, tt := range tests { @@ -207,12 +287,12 @@ func Test_Matches(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - builderID, err := BuilderIDNew(tt.builderID) + trustedBuilderID, err := TrustedBuilderIDNew(tt.trustedBuilderID) if err != nil { panic(fmt.Errorf("BuilderIDNew: %w", err)) } - err = builderID.Matches(tt.match) + err = trustedBuilderID.Matches(tt.match, tt.allowRef) if !cmp.Equal(err, tt.err, cmpopts.EquateErrors()) { t.Errorf(cmp.Diff(err, tt.err)) } diff --git a/verifiers/verifier.go b/verifiers/verifier.go index 88a8b32..e7e5204 100644 --- a/verifiers/verifier.go +++ b/verifiers/verifier.go @@ -39,7 +39,7 @@ func VerifyImage(ctx context.Context, artifactImage string, provenance []byte, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, -) ([]byte, *utils.BuilderID, error) { +) ([]byte, *utils.TrustedBuilderID, error) { verifier, err := getVerifier(builderOpts) if err != nil { return nil, nil, err @@ -52,7 +52,7 @@ func VerifyArtifact(ctx context.Context, provenance []byte, artifactHash string, provenanceOpts *options.ProvenanceOpts, builderOpts *options.BuilderOpts, -) ([]byte, *utils.BuilderID, error) { +) ([]byte, *utils.TrustedBuilderID, error) { verifier, err := getVerifier(builderOpts) if err != nil { return nil, nil, err