From f859a5df5eabfe0b98a470d9e4e1de578ad54a04 Mon Sep 17 00:00:00 2001 From: Ian Lewis Date: Thu, 29 Jun 2023 07:00:58 +0000 Subject: [PATCH 1/6] refactor: set builder id constants Signed-off-by: Ian Lewis --- verifiers/internal/gha/npm.go | 7 +---- .../gha/slsaprovenance/common/builders.go | 26 +++++++++++++++++++ .../gha/slsaprovenance/common/common.go | 8 ++++-- verifiers/internal/gha/verifier.go | 11 ++++---- 4 files changed, 39 insertions(+), 13 deletions(-) create mode 100644 verifiers/internal/gha/slsaprovenance/common/builders.go diff --git a/verifiers/internal/gha/npm.go b/verifiers/internal/gha/npm.go index 5c8e3e599..8eb87ac03 100644 --- a/verifiers/internal/gha/npm.go +++ b/verifiers/internal/gha/npm.go @@ -25,13 +25,8 @@ type hosted string const ( hostedSelf hosted = "self-hosted" hostedGitHub hosted = "github-hosted" -) -const ( - publishAttestationV01 = "https://github.com/npm/attestation/tree/main/specs/publish/" - builderLegacyGitHubRunnerID = "https://github.com/actions/runner" - builderGitHubHostedRunnerID = builderLegacyGitHubRunnerID + "/" + string(hostedGitHub) - builderSelfHostedRunnerID = builderLegacyGitHubRunnerID + "/" + string(hostedSelf) + publishAttestationV01 = "https://github.com/npm/attestation/tree/main/specs/publish/" ) var errrorInvalidAttestations = errors.New("invalid npm attestations") diff --git a/verifiers/internal/gha/slsaprovenance/common/builders.go b/verifiers/internal/gha/slsaprovenance/common/builders.go new file mode 100644 index 000000000..0d3bfb43b --- /dev/null +++ b/verifiers/internal/gha/slsaprovenance/common/builders.go @@ -0,0 +1,26 @@ +package common + +var trustedBuilderRepository = "https://github.com/slsa-framework/slsa-github-generator" + +var ( + // GenericGeneratorBuilderID is the builder ID for the Generic Generator. + GenericGeneratorBuilderID = trustedBuilderRepository + "/.github/workflows/generator_generic_slsa3.yml" + // ContainerGeneratorBuilderID is the builder ID for the Container Generator. + ContainerGeneratorBuilderID = trustedBuilderRepository + "/.github/workflows/generator_container_slsa3.yml" + // GoBuilderID is the SLSA builder ID for the Go Builder. + GoBuilderID = trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml" + // ContainerBasedBuilderID is the SLSA builder ID for the Container-Based Builder. + ContainerBasedBuilderID = trustedBuilderRepository + "/.github/workflows/builder_container-based_slsa3.yml" + + // NpmCLILegacyBuilderID is the legacy builder ID for the npm CLI. + NpmCLILegacyBuilderID = "https://github.com/actions/runner" + // NpmCLIHostedBuilderID is the builder ID for the npm CLI on Hosted GitHub Actions. + NpmCLIHostedBuilderID = NpmCLILegacyBuilderID + "/github-hosted" + // NpmCLISelfHostedBuilderID is the builder ID for the npm CLI on Self-hosted GitHub Actions. + NpmCLISelfHostedBuilderID = NpmCLILegacyBuilderID + "/self-hosted" + + // GenericDelegatorBuilderID is the SLSA builder ID for the BYOB Generic Low-Permissions Delegated Builder. + GenericDelegatorBuilderID = trustedBuilderRepository + "/.github/workflows/delegator_generic_slsa3.yml" + // GenericLowPermsDelegatorBuilderID is the SLSA builder ID for the BYOB Generic Low-Permissions Delegated Builder. + GenericLowPermsDelegatorBuilderID = trustedBuilderRepository + "/.github/workflows/delegator_lowperms-generic_slsa3.yml" +) diff --git a/verifiers/internal/gha/slsaprovenance/common/common.go b/verifiers/internal/gha/slsaprovenance/common/common.go index e2ebf67d9..3e049a48f 100644 --- a/verifiers/internal/gha/slsaprovenance/common/common.go +++ b/verifiers/internal/gha/slsaprovenance/common/common.go @@ -4,12 +4,16 @@ import ( "fmt" "strings" + slsa1 "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/v1" serrors "github.com/slsa-framework/slsa-verifier/v2/errors" ) const ( // ProvenanceV02Type is the SLSA v0.2 predicate type. ProvenanceV02Type = "https://slsa.dev/provenance/v0.2" + + // ProvenanceV1Type is the SLSA v1.0 predicate type. + ProvenanceV1Type = slsa1.PredicateSLSAProvenance ) // GetWorkflowInputs gets the workflow inputs from the GitHub environment map @@ -176,7 +180,7 @@ func GetTag(environment map[string]any, upperEnv bool) (string, error) { } return GetAsString(environment, refKey) default: - return "", fmt.Errorf("%w: %s %s", serrors.ErrorInvalidDssePayload, + return "", fmt.Errorf("%w: %s %q", serrors.ErrorInvalidDssePayload, "unknown ref type", refType) } } @@ -203,7 +207,7 @@ func GetBranch(environment map[string]any, upperEnv bool) (string, error) { case "tag": return getBranchForTag(environment, upperEnv) default: - return "", fmt.Errorf("%w: %s %s", serrors.ErrorInvalidDssePayload, + return "", fmt.Errorf("%w: %s %q", serrors.ErrorInvalidDssePayload, "unknown ref type", refType) } } diff --git a/verifiers/internal/gha/verifier.go b/verifiers/internal/gha/verifier.go index 3898a3709..bff67243d 100644 --- a/verifiers/internal/gha/verifier.go +++ b/verifiers/internal/gha/verifier.go @@ -16,6 +16,7 @@ import ( serrors "github.com/slsa-framework/slsa-verifier/v2/errors" "github.com/slsa-framework/slsa-verifier/v2/options" "github.com/slsa-framework/slsa-verifier/v2/register" + "github.com/slsa-framework/slsa-verifier/v2/verifiers/internal/gha/slsaprovenance/common" "github.com/slsa-framework/slsa-verifier/v2/verifiers/utils" "github.com/slsa-framework/slsa-verifier/v2/verifiers/utils/container" ) @@ -165,20 +166,20 @@ func verifyNpmEnvAndCert(env *dsse.Envelope, // Verify that the value provided is consistent with certificate information. if workflowInfo.SubjectHosted == nil { - return nil, fmt.Errorf("%w: hosted status unknonwn", serrors.ErrorNotSupported) + return nil, fmt.Errorf("%w: hosted status unknown", serrors.ErrorNotSupported) } switch *builderOpts.ExpectedID { - case builderLegacyGitHubRunnerID, builderGitHubHostedRunnerID: + case common.NpmCLILegacyBuilderID, common.NpmCLIHostedBuilderID: if *workflowInfo.SubjectHosted != HostedGitHub { return nil, fmt.Errorf("%w: re-usable workflow is self-hosted", serrors.ErrorMismatchBuilderID) } - case builderSelfHostedRunnerID: + case common.NpmCLISelfHostedBuilderID: if *workflowInfo.SubjectHosted != HostedSelf { return nil, fmt.Errorf("%w: re-usable workflow is GitHub-hosted", serrors.ErrorMismatchBuilderID) } default: - return nil, fmt.Errorf("%w: builder %v. Expected one of %v, %v", serrors.ErrorNotSupported, *builderOpts.ExpectedID, - builderSelfHostedRunnerID, builderGitHubHostedRunnerID) + return nil, fmt.Errorf("%w: builder %q. Expected one of %q, %q", serrors.ErrorNotSupported, *builderOpts.ExpectedID, + common.NpmCLISelfHostedBuilderID, common.NpmCLIHostedBuilderID) } trustedBuilderID, err = utils.TrustedBuilderIDNew(*builderOpts.ExpectedID, false) From 4d45f6e8adbede9a352627c0fb10fbcbe9cf682e Mon Sep 17 00:00:00 2001 From: Ian Lewis Date: Thu, 29 Jun 2023 08:28:15 +0000 Subject: [PATCH 2/6] refactor: use full builder ID throughout Signed-off-by: Ian Lewis --- verifiers/internal/gha/builder.go | 91 +++-- verifiers/internal/gha/builder_test.go | 314 +++++++++--------- .../internal/gha/provenance_forgeable.go | 4 +- .../internal/gha/provenance_forgeable_test.go | 39 +-- verifiers/internal/gha/verifier.go | 6 +- 5 files changed, 244 insertions(+), 210 deletions(-) diff --git a/verifiers/internal/gha/builder.go b/verifiers/internal/gha/builder.go index d65d67b2d..4d31f8ff7 100644 --- a/verifiers/internal/gha/builder.go +++ b/verifiers/internal/gha/builder.go @@ -4,11 +4,13 @@ import ( "crypto/x509" "encoding/asn1" "fmt" + "net/url" "strings" fulcio "github.com/sigstore/fulcio/pkg/certificate" serrors "github.com/slsa-framework/slsa-verifier/v2/errors" "github.com/slsa-framework/slsa-verifier/v2/options" + "github.com/slsa-framework/slsa-verifier/v2/verifiers/internal/gha/slsaprovenance/common" "github.com/slsa-framework/slsa-verifier/v2/verifiers/utils" ) @@ -24,23 +26,18 @@ var ( ) var defaultArtifactTrustedReusableWorkflows = map[string]bool{ - trustedBuilderRepository + "/.github/workflows/generator_generic_slsa3.yml": true, - trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml": true, - trustedBuilderRepository + "/.github/workflows/builder_container-based_slsa3.yml": true, + common.GenericGeneratorBuilderID: true, + common.GoBuilderID: true, + common.ContainerBasedBuilderID: true, } var defaultContainerTrustedReusableWorkflows = map[string]bool{ - trustedBuilderRepository + "/.github/workflows/generator_container_slsa3.yml": true, + common.ContainerGeneratorBuilderID: true, } -var ( - delegatorGenericReusableWorkflow = trustedBuilderRepository + "/.github/workflows/delegator_generic_slsa3.yml" - delegatorLowPermsGenericReusableWorkflow = trustedBuilderRepository + "/.github/workflows/delegator_lowperms-generic_slsa3.yml" -) - var defaultBYOBReusableWorkflows = map[string]bool{ - delegatorGenericReusableWorkflow: true, - delegatorLowPermsGenericReusableWorkflow: true, + common.GenericDelegatorBuilderID: true, + common.GenericLowPermsDelegatorBuilderID: true, } // VerifyCertficateSourceRepository verifies the source repository. @@ -69,26 +66,30 @@ func VerifyBuilderIdentity(id *WorkflowIdentity, // Issuer verification. // NOTE: this is necessary before we do any further verification. if id.Issuer != certOidcIssuer { - return nil, false, fmt.Errorf("%w: %s", serrors.ErrorInvalidOIDCIssuer, id.Issuer) + return nil, false, fmt.Errorf("%w: %q", serrors.ErrorInvalidOIDCIssuer, id.Issuer) } - // cert URI path is /org/repo/path/to/workflow@ref - workflowPath := strings.SplitN(id.SubjectWorkflowRef, "@", 2) - if len(workflowPath) < 2 { - return nil, false, fmt.Errorf("%w: workflow uri: %s", serrors.ErrorMalformedURI, id.SubjectWorkflowRef) + // cert URI is https://github.com/org/repo/path/to/workflow@ref + // Remove '@' from Path + workflowID := id.SubjectWorkflowName() + workflowTag := id.SubjectWorkflowRef() + + fmt.Println("workflowID", workflowID) + fmt.Println("workflowTag", workflowTag) + + if workflowID == "" || workflowTag == "" { + return nil, false, fmt.Errorf("%w: workflow uri: %q", serrors.ErrorMalformedURI, id.SubjectWorkflow.String()) } // Verify trusted workflow. - reusableWorkflowPath := strings.Trim(workflowPath[0], "/") - reusableWorkflowTag := strings.Trim(workflowPath[1], "/") - builderID, byob, err := verifyTrustedBuilderID(reusableWorkflowPath, reusableWorkflowTag, + builderID, byob, err := verifyTrustedBuilderID(workflowID, workflowTag, builderOpts.ExpectedID, defaultBuilders) if err != nil { return nil, byob, err } // Verify the ref is a full semantic version tag. - if err := verifyTrustedBuilderRef(id, reusableWorkflowTag); err != nil { + if err := verifyTrustedBuilderRef(id, workflowTag); err != nil { return nil, byob, err } @@ -97,26 +98,25 @@ func VerifyBuilderIdentity(id *WorkflowIdentity, // Verifies the builder ID at path against an expected builderID. // If an expected builderID is not provided, uses the defaultBuilders. -func verifyTrustedBuilderID(certPath, certTag string, expectedBuilderID *string, defaultTrustedBuilders map[string]bool) (*utils.TrustedBuilderID, bool, error) { +func verifyTrustedBuilderID(certBuilderID, certTag string, expectedBuilderID *string, defaultTrustedBuilders map[string]bool) (*utils.TrustedBuilderID, bool, error) { var trustedBuilderID *utils.TrustedBuilderID var err error - certBuilderName := httpsGithubCom + 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 expectedBuilderID == nil || *expectedBuilderID == "" { - if _, ok := defaultTrustedBuilders[certPath]; !ok { - return nil, false, fmt.Errorf("%w: %s with builderID provided: %t", serrors.ErrorUntrustedReusableWorkflow, certPath, expectedBuilderID != nil) + if _, ok := defaultTrustedBuilders[certBuilderID]; !ok { + return nil, false, fmt.Errorf("%w: %s with builderID provided: %t", serrors.ErrorUntrustedReusableWorkflow, certBuilderID, expectedBuilderID != nil) } // Construct the builderID using the certificate's builder's name and tag. - trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderName+"@"+certTag, true) + trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderID+"@"+certTag, true) if err != nil { return nil, false, err } } else { // Verify the builderID. // We only accept IDs on github.com. - trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderName+"@"+certTag, true) + trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderID+"@"+certTag, true) if err != nil { return nil, false, err } @@ -204,6 +204,7 @@ const ( HostedGitHub ) +// WorkflowIdentity is a identity captured from a Fulcio certificate. // See https://github.com/sigstore/fulcio/blob/main/docs/oid-info.md. type WorkflowIdentity struct { // The source repository @@ -218,7 +219,7 @@ type WorkflowIdentity struct { SourceOwnerID *string // Workflow path OIDC subject - ref of reuseable workflow or trigger workflow. - SubjectWorkflowRef string + SubjectWorkflow *url.URL // Subject commit sha1. SubjectSha1 *string // Hosted status of the subject. @@ -235,6 +236,32 @@ type WorkflowIdentity struct { Issuer string } +// SubjectWorkflowName returns the subject workflow without the git ref. +func (id *WorkflowIdentity) SubjectWorkflowName() string { + withoutRef, err := url.Parse(id.SubjectWorkflow.String()) + if err != nil { + // This should never happen. + panic(err) + } + withoutRef.Path = id.SubjectWorkflowPath() + return withoutRef.String() +} + +// SubjectWorkflowPath returns the subject workflow without the server url. +func (id *WorkflowIdentity) SubjectWorkflowPath() string { + parts := strings.SplitN(id.SubjectWorkflow.Path, "@", 2) + return parts[0] +} + +// SubjectWorkflowRef returns the ref for the subject workflow. +func (id *WorkflowIdentity) SubjectWorkflowRef() string { + parts := strings.SplitN(id.SubjectWorkflow.Path, "@", 2) + if len(parts) < 2 { + return "" + } + return parts[1] +} + func getHosted(cert *x509.Certificate) (*Hosted, error) { runnerEnv, err := getExtension(cert, fulcio.OIDRunnerEnvironment, true) if err != nil { @@ -423,9 +450,7 @@ func GetWorkflowInfoFromCertificate(cert *x509.Certificate) (*WorkflowIdentity, if !strings.HasPrefix(cert.URIs[0].Path, "/") { return nil, fmt.Errorf("%w: %s", serrors.ErrorInvalidFormat, cert.URIs[0].Path) } - // Remove the starting '/'. - // NOTE: The Path has the following structure: repo/name/path/to/workflow.yml@ref. - subjectWorkflowRef := cert.URIs[0].Path[1:] + subjectWorkflow := cert.URIs[0] var pSubjectSha1, pSourceID, pSourceRef, pSourceOwnerID, pBuildConfigPath, pRunID *string if subjectSha1 != "" { @@ -451,9 +476,9 @@ func GetWorkflowInfoFromCertificate(cert *x509.Certificate) (*WorkflowIdentity, // Issuer. Issuer: issuer, // Subject - SubjectWorkflowRef: subjectWorkflowRef, - SubjectSha1: pSubjectSha1, - SubjectHosted: subjectHosted, + SubjectWorkflow: subjectWorkflow, + SubjectSha1: pSubjectSha1, + SubjectHosted: subjectHosted, // Source. SourceRepository: sourceRepository, SourceSha1: sourceSha1, diff --git a/verifiers/internal/gha/builder_test.go b/verifiers/internal/gha/builder_test.go index d3996bc0d..0ba9590cb 100644 --- a/verifiers/internal/gha/builder_test.go +++ b/verifiers/internal/gha/builder_test.go @@ -13,9 +13,18 @@ import ( fulcio "github.com/sigstore/fulcio/pkg/certificate" serrors "github.com/slsa-framework/slsa-verifier/v2/errors" "github.com/slsa-framework/slsa-verifier/v2/options" + "github.com/slsa-framework/slsa-verifier/v2/verifiers/internal/gha/slsaprovenance/common" "github.com/slsa-framework/slsa-verifier/v2/verifiers/utils" ) +// Must checks the error and panics if not nil. +func Must[T any](val T, err error) T { + if err != nil { + panic(err) + } + return val +} + func Test_VerifyBuilderIdentity(t *testing.T) { t.Parallel() tests := []struct { @@ -30,11 +39,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "invalid job workflow ref", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: "random/workflow/ref", - BuildTrigger: "workflow_dispatch", - Issuer: "https://token.actions.githubusercontent.com", + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse("https://github.com/random/workflow/ref")), + BuildTrigger: "workflow_dispatch", + Issuer: "https://token.actions.githubusercontent.com", }, defaults: defaultArtifactTrustedReusableWorkflows, err: serrors.ErrorMalformedURI, @@ -42,11 +51,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "untrusted job workflow ref", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: "/malicious/slsa-go/.github/workflows/builder.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: "https://token.actions.githubusercontent.com", + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse("https://github.com/malicious/slsa-go/.github/workflows/builder.yml@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: "https://token.actions.githubusercontent.com", }, defaults: defaultArtifactTrustedReusableWorkflows, err: serrors.ErrorUntrustedReusableWorkflow, @@ -54,11 +63,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "untrusted job workflow ref for general repos", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/heads/main", - BuildTrigger: "workflow_dispatch", - Issuer: "https://token.actions.githubusercontent.com", + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/heads/main")), + BuildTrigger: "workflow_dispatch", + Issuer: "https://token.actions.githubusercontent.com", }, defaults: defaultArtifactTrustedReusableWorkflows, err: serrors.ErrorInvalidRef, @@ -66,11 +75,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "untrusted cert issuer for general repos", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: "https://bad.issuer.com", + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: "https://bad.issuer.com", }, defaults: defaultArtifactTrustedReusableWorkflows, err: serrors.ErrorInvalidOIDCIssuer, @@ -78,11 +87,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid trusted builder without tag", workflow: &WorkflowIdentity{ - SourceRepository: trustedBuilderRepository, - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: "https://token.actions.githubusercontent.com", + SourceRepository: trustedBuilderRepository, + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: "https://token.actions.githubusercontent.com", }, defaults: defaultArtifactTrustedReusableWorkflows, builderID: "https://github.com/" + trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml", @@ -90,11 +99,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid generic delegator builder without tag", workflow: &WorkflowIdentity{ - SourceRepository: trustedBuilderRepository, - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/delegator_generic_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: "https://token.actions.githubusercontent.com", + SourceRepository: trustedBuilderRepository, + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GenericDelegatorBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: "https://token.actions.githubusercontent.com", }, defaults: defaultBYOBReusableWorkflows, builderID: "https://github.com/" + trustedBuilderRepository + "/.github/workflows/delegator_generic_slsa3.yml", @@ -103,11 +112,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid low-perms delegator builder with short tag", workflow: &WorkflowIdentity{ - SourceRepository: trustedBuilderRepository, - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/delegator_lowperms-generic_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: "https://token.actions.githubusercontent.com", + SourceRepository: trustedBuilderRepository, + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GenericLowPermsDelegatorBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: "https://token.actions.githubusercontent.com", }, defaults: defaultBYOBReusableWorkflows, builderID: "https://github.com/" + trustedBuilderRepository + "/.github/workflows/delegator_lowperms-generic_slsa3.yml@v1.2.3", @@ -116,11 +125,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid main ref for e2e test", workflow: &WorkflowIdentity{ - SourceRepository: e2eTestRepository, - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: e2eTestRepository, + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, defaults: defaultArtifactTrustedReusableWorkflows, builderID: "https://github.com/" + trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml", @@ -128,11 +137,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid main ref for e2e test - match builderID", workflow: &WorkflowIdentity{ - SourceRepository: e2eTestRepository, - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: e2eTestRepository, + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, buildOpts: &options.BuilderOpts{ ExpectedID: asStringPointer("https://github.com/" + trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml"), @@ -143,11 +152,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid main ref for e2e test - mismatch builderID", workflow: &WorkflowIdentity{ - SourceRepository: e2eTestRepository, - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: e2eTestRepository, + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, buildOpts: &options.BuilderOpts{ ExpectedID: asStringPointer("some-other-builderID"), @@ -158,11 +167,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid workflow identity - match builderID", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, buildOpts: &options.BuilderOpts{ ExpectedID: asStringPointer("https://github.com/" + trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml"), @@ -173,11 +182,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid workflow identity - mismatch builderID", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, buildOpts: &options.BuilderOpts{ ExpectedID: asStringPointer("some-other-builderID"), @@ -188,11 +197,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "invalid workflow identity with prerelease", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3-alpha", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3-alpha")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, err: serrors.ErrorInvalidRef, defaults: defaultArtifactTrustedReusableWorkflows, @@ -201,11 +210,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "invalid workflow identity with build", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3+123", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3+123")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, defaults: defaultArtifactTrustedReusableWorkflows, err: serrors.ErrorInvalidRef, @@ -213,11 +222,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "invalid workflow identity with metadata", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3-alpha+123", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3-alpha+123")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, defaults: defaultArtifactTrustedReusableWorkflows, err: serrors.ErrorInvalidRef, @@ -225,11 +234,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid workflow identity with fully qualified source", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, defaults: defaultArtifactTrustedReusableWorkflows, builderID: "https://github.com/" + trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml", @@ -237,11 +246,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid workflow identity with fully qualified source - no default", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, buildOpts: &options.BuilderOpts{ ExpectedID: asStringPointer("https://github.com/" + trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml"), @@ -251,11 +260,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid workflow identity with fully qualified source - match builderID", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, buildOpts: &options.BuilderOpts{ ExpectedID: asStringPointer("https://github.com/" + trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml"), @@ -266,11 +275,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid workflow identity with fully qualified source - mismatch builderID", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, buildOpts: &options.BuilderOpts{ ExpectedID: asStringPointer("some-other-builderID"), @@ -281,11 +290,11 @@ func Test_VerifyBuilderIdentity(t *testing.T) { { name: "valid workflow identity with fully qualified source - mismatch defaults", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, defaults: defaultContainerTrustedReusableWorkflows, err: serrors.ErrorUntrustedReusableWorkflow, @@ -307,8 +316,8 @@ func Test_VerifyBuilderIdentity(t *testing.T) { t.Errorf(cmp.Diff(byob, tt.byob)) } - if !errCmp(err, tt.err) { - t.Errorf(cmp.Diff(err, tt.err, cmpopts.EquateErrors())) + if diff := cmp.Diff(tt.err, err, cmpopts.EquateErrors()); diff != "" { + t.Errorf("unexpected error (-want +got):\n%s", diff) } if err != nil { return @@ -377,22 +386,22 @@ func Test_VerifyCertficateSourceRepository(t *testing.T) { { name: "repo match", workflow: &WorkflowIdentity{ - SourceRepository: "asraa/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: "asraa/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, source: "github.com/asraa/slsa-on-github-test", }, { name: "unexpected source for e2e test", workflow: &WorkflowIdentity{ - SourceRepository: e2eTestRepository, - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: e2eTestRepository, + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, source: "malicious/source", err: serrors.ErrorMismatchSource, @@ -400,10 +409,10 @@ func Test_VerifyCertficateSourceRepository(t *testing.T) { { name: "valid main ref for builder", workflow: &WorkflowIdentity{ - SourceRepository: trustedBuilderRepository, - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: trustedBuilderRepository, + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, source: "malicious/source", err: serrors.ErrorMismatchSource, @@ -411,11 +420,11 @@ func Test_VerifyCertficateSourceRepository(t *testing.T) { { name: "unexpected source", workflow: &WorkflowIdentity{ - SourceRepository: "malicious/slsa-on-github-test", - SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", - SubjectWorkflowRef: trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.2.3", - BuildTrigger: "workflow_dispatch", - Issuer: certOidcIssuer, + SourceRepository: "malicious/slsa-on-github-test", + SourceSha1: "0dfcd24824432c4ce587f79c918eef8fc2c44d7b", + SubjectWorkflow: Must(url.Parse(common.GoBuilderID + "@refs/tags/v1.2.3")), + BuildTrigger: "workflow_dispatch", + Issuer: certOidcIssuer, }, source: "asraa/slsa-on-github-test", err: serrors.ErrorMismatchSource, @@ -583,11 +592,8 @@ func Test_verifyTrustedBuilderID(t *testing.T) { if byob != tt.byob { t.Errorf(cmp.Diff(byob, tt.byob)) } - if !errCmp(err, tt.err) { - t.Errorf(cmp.Diff(err, tt.err, cmpopts.EquateErrors())) - } - if err != nil { - return + if diff := cmp.Diff(tt.err, err, cmpopts.EquateErrors()); diff != "" { + t.Fatalf("unexpected error (-want +got):\n%s", diff) } expectedID := "https://github.com/" + tt.path + "@" + tt.tag if err := id.MatchesLoose(expectedID, true); err != nil { @@ -899,11 +905,11 @@ func Test_GetWorkflowInfoFromCertificate(t *testing.T) { }, }, workflow: WorkflowIdentity{ - Issuer: issuer, - SubjectWorkflowRef: repo + "/" + buildConfigPath, - SourceRepository: repo, - SourceSha1: digest, - BuildTrigger: trigger, + Issuer: issuer, + SubjectWorkflow: Must(url.Parse(httpsGithubCom + repo + "/" + buildConfigPath)), + SourceRepository: repo, + SourceSha1: digest, + BuildTrigger: trigger, }, }, { @@ -1016,18 +1022,18 @@ func Test_GetWorkflowInfoFromCertificate(t *testing.T) { }, }, workflow: WorkflowIdentity{ - Issuer: issuer, - SubjectSha1: &subjectSha1, - SubjectHosted: &hosted, - SubjectWorkflowRef: repo + "/" + buildConfigPath, - SourceRepository: repo, - SourceSha1: digest, - SourceRef: &ref, - SourceID: &sourceID, - SourceOwnerID: &sourceOwnerID, - BuildTrigger: trigger, - BuildConfigPath: &buildConfigPath, - RunID: &invocationID, + Issuer: issuer, + SubjectSha1: &subjectSha1, + SubjectHosted: &hosted, + SubjectWorkflow: Must(url.Parse(httpsGithubCom + repo + "/" + buildConfigPath)), + SourceRepository: repo, + SourceSha1: digest, + SourceRef: &ref, + SourceID: &sourceID, + SourceOwnerID: &sourceOwnerID, + BuildTrigger: trigger, + BuildConfigPath: &buildConfigPath, + RunID: &invocationID, }, }, { @@ -1161,17 +1167,17 @@ func Test_GetWorkflowInfoFromCertificate(t *testing.T) { }, }, workflow: WorkflowIdentity{ - Issuer: issuer, - SubjectWorkflowRef: repo + "/" + buildConfigPath, - SourceRepository: repo, - SourceSha1: digest, - BuildTrigger: trigger, - SubjectHosted: &hosted, - SourceRef: &ref, - SourceID: &sourceID, - SourceOwnerID: &sourceOwnerID, - BuildConfigPath: &buildConfigPath, - RunID: &invocationID, + Issuer: issuer, + SubjectWorkflow: Must(url.Parse(httpsGithubCom + repo + "/" + buildConfigPath)), + SourceRepository: repo, + SourceSha1: digest, + BuildTrigger: trigger, + SubjectHosted: &hosted, + SourceRef: &ref, + SourceID: &sourceID, + SourceOwnerID: &sourceOwnerID, + BuildConfigPath: &buildConfigPath, + RunID: &invocationID, }, }, { diff --git a/verifiers/internal/gha/provenance_forgeable.go b/verifiers/internal/gha/provenance_forgeable.go index d673e9faf..f90eb1dbd 100644 --- a/verifiers/internal/gha/provenance_forgeable.go +++ b/verifiers/internal/gha/provenance_forgeable.go @@ -310,7 +310,9 @@ func verifySystemParameters(prov iface.Provenance, workflow *WorkflowIdentity) e return err } // 7. GITHUB_WORKFLOW_REF - if err := verifySystemParameter(sysParams, "GITHUB_WORKFLOW_REF", &workflow.SubjectWorkflowRef); err != nil { + // NOTE: GITHUB_WORKFLOW_REF does not include the server url or leading '/' + workflowPath := strings.TrimLeft(workflow.SubjectWorkflow.Path, "/") + if err := verifySystemParameter(sysParams, "GITHUB_WORKFLOW_REF", &workflowPath); err != nil { return err } // 8. GITHUB_WORKFLOW_SHA diff --git a/verifiers/internal/gha/provenance_forgeable_test.go b/verifiers/internal/gha/provenance_forgeable_test.go index 94abb0df9..c18fce55b 100644 --- a/verifiers/internal/gha/provenance_forgeable_test.go +++ b/verifiers/internal/gha/provenance_forgeable_test.go @@ -1,6 +1,7 @@ package gha import ( + "net/url" "testing" "time" @@ -566,15 +567,15 @@ func Test_verifyMetadata(t *testing.T) { func Test_verifySystemParameters(t *testing.T) { t.Parallel() expectedWorkflow := WorkflowIdentity{ - BuildTrigger: "workflow_dispatch", - SubjectWorkflowRef: "laurentsimon/provenance-npm-test/.github/workflows/release.yml@refs/heads/main", - SubjectSha1: asStringPointer("b38894f2dda4355ea5606fccb166e61565e12a14"), - SourceRepository: "laurentsimon/provenance-npm-test", - SourceRef: asStringPointer("refs/heads/main"), - SourceID: asStringPointer("602223945"), - SourceOwnerID: asStringPointer("64505099"), - SourceSha1: "b38894f2dda4355ea5606fccb166e61565e12a14", - RunID: asStringPointer("4757060009/attempt/1"), + BuildTrigger: "workflow_dispatch", + SubjectWorkflow: Must(url.Parse(httpsGithubCom + "laurentsimon/provenance-npm-test/.github/workflows/release.yml@refs/heads/main")), + SubjectSha1: asStringPointer("b38894f2dda4355ea5606fccb166e61565e12a14"), + SourceRepository: "laurentsimon/provenance-npm-test", + SourceRef: asStringPointer("refs/heads/main"), + SourceID: asStringPointer("602223945"), + SourceOwnerID: asStringPointer("64505099"), + SourceSha1: "b38894f2dda4355ea5606fccb166e61565e12a14", + RunID: asStringPointer("4757060009/attempt/1"), } tests := []struct { name string @@ -949,16 +950,16 @@ func Test_verifySystemParameters(t *testing.T) { func Test_verifyProvenanceMatchesCertificate(t *testing.T) { t.Parallel() expectedWorkflow := WorkflowIdentity{ - BuildTrigger: "workflow_dispatch", - BuildConfigPath: asStringPointer("release/workflow/path"), - SubjectWorkflowRef: "repo/name/release/workflow/path@subject-ref", - SubjectSha1: asStringPointer("subject-sha"), - SourceRepository: "repo/name", - SourceRef: asStringPointer("source-ref"), - SourceID: asStringPointer("source-id"), - SourceOwnerID: asStringPointer("source-owner-id"), - SourceSha1: "source-sha", - RunID: asStringPointer("run-id/attempt/run-attempt"), + BuildTrigger: "workflow_dispatch", + BuildConfigPath: asStringPointer("release/workflow/path"), + SubjectWorkflow: Must(url.Parse(httpsGithubCom + "repo/name/release/workflow/path@subject-ref")), + SubjectSha1: asStringPointer("subject-sha"), + SourceRepository: "repo/name", + SourceRef: asStringPointer("source-ref"), + SourceID: asStringPointer("source-id"), + SourceOwnerID: asStringPointer("source-owner-id"), + SourceSha1: "source-sha", + RunID: asStringPointer("run-id/attempt/run-attempt"), } tests := []struct { name string diff --git a/verifiers/internal/gha/verifier.go b/verifiers/internal/gha/verifier.go index bff67243d..139fd3698 100644 --- a/verifiers/internal/gha/verifier.go +++ b/verifiers/internal/gha/verifier.go @@ -83,8 +83,8 @@ func verifyEnvAndCert(env *dsse.Envelope, return nil, nil, err } - fmt.Fprintf(os.Stderr, "Verified build using builder https://github.com%s at commit %s\n", - workflowInfo.SubjectWorkflowRef, + fmt.Fprintf(os.Stderr, "Verified build using builder %q at commit %s\n", + workflowInfo.SubjectWorkflow.String(), workflowInfo.SourceSha1) // Return verified provenance. r, err := base64.StdEncoding.DecodeString(env.Payload) @@ -112,7 +112,7 @@ func verifyNpmEnvAndCert(env *dsse.Envelope, // We verify against the delegator re-usable workflow, not the user-provided // builder. This is because the signing identity for delegator-based builders // is *always* the delegator workflow. - expectedDelegatorWorkflow := httpsGithubCom + delegatorLowPermsGenericReusableWorkflow + expectedDelegatorWorkflow := httpsGithubCom + common.GenericLowPermsDelegatorBuilderID delegatorBuilderOpts := options.BuilderOpts{ ExpectedID: &expectedDelegatorWorkflow, } From bc620da88b070f73af5378bf1312bef2bd4ec285 Mon Sep 17 00:00:00 2001 From: Ian Lewis Date: Thu, 29 Jun 2023 08:31:14 +0000 Subject: [PATCH 3/6] Remove debug code Signed-off-by: Ian Lewis --- verifiers/internal/gha/builder.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/verifiers/internal/gha/builder.go b/verifiers/internal/gha/builder.go index 4d31f8ff7..4465dfd91 100644 --- a/verifiers/internal/gha/builder.go +++ b/verifiers/internal/gha/builder.go @@ -74,9 +74,6 @@ func VerifyBuilderIdentity(id *WorkflowIdentity, workflowID := id.SubjectWorkflowName() workflowTag := id.SubjectWorkflowRef() - fmt.Println("workflowID", workflowID) - fmt.Println("workflowTag", workflowTag) - if workflowID == "" || workflowTag == "" { return nil, false, fmt.Errorf("%w: workflow uri: %q", serrors.ErrorMalformedURI, id.SubjectWorkflow.String()) } From 5e542085c0f626aa50ccca83667c928c7c6dc052 Mon Sep 17 00:00:00 2001 From: Ian Lewis Date: Thu, 29 Jun 2023 09:02:44 +0000 Subject: [PATCH 4/6] Fix tests Signed-off-by: Ian Lewis --- verifiers/internal/gha/builder.go | 2 +- verifiers/internal/gha/builder_test.go | 32 ++++++++++++++++---------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/verifiers/internal/gha/builder.go b/verifiers/internal/gha/builder.go index 4465dfd91..b03b8a6e8 100644 --- a/verifiers/internal/gha/builder.go +++ b/verifiers/internal/gha/builder.go @@ -141,7 +141,7 @@ func verifyTrustedBuilderID(certBuilderID, certTag string, expectedBuilderID *st func isTrustedDelegatorBuilder(certBuilder *utils.TrustedBuilderID, trustedBuilders map[string]bool) bool { for byobBuilder := range defaultBYOBReusableWorkflows { // Check that the certificate builder is a BYOB workflow. - if err := certBuilder.MatchesLoose(httpsGithubCom+byobBuilder, true); err == nil { + if err := certBuilder.MatchesLoose(byobBuilder, true); err == nil { // We found a delegator workflow that matches the certificate identity. // Check that the BYOB builder is trusted by the caller. if _, ok := trustedBuilders[byobBuilder]; !ok { diff --git a/verifiers/internal/gha/builder_test.go b/verifiers/internal/gha/builder_test.go index 0ba9590cb..d3a2a45c4 100644 --- a/verifiers/internal/gha/builder_test.go +++ b/verifiers/internal/gha/builder_test.go @@ -313,7 +313,7 @@ func Test_VerifyBuilderIdentity(t *testing.T) { } id, byob, err := VerifyBuilderIdentity(tt.workflow, opts, tt.defaults) if byob != tt.byob { - t.Errorf(cmp.Diff(byob, tt.byob)) + t.Errorf("unexpected byob value:\n%s", cmp.Diff(tt.byob, byob)) } if diff := cmp.Diff(tt.err, err, cmpopts.EquateErrors()); diff != "" { @@ -340,16 +340,16 @@ func Test_isTrustedDelegatorBuilder(t *testing.T) { }{ { name: "match byob", - certBuilderID: "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/delegator_lowperms-generic_slsa3.yml@refs/tags/v1.6.0", + certBuilderID: common.GenericLowPermsDelegatorBuilderID + "@refs/tags/v1.6.0", trustedBuilderIDs: map[string]bool{ - "slsa-framework/slsa-github-generator/.github/workflows/delegator_lowperms-generic_slsa3.yml": true, - "slsa-framework/slsa-github-generator/.github/workflows/some_delegator.yml": true, + common.GenericLowPermsDelegatorBuilderID: true, + "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/some_delegator.yml": true, }, result: true, }, { name: "match byob but not caller trusted", - certBuilderID: "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/delegator_lowperms-generic_slsa3.yml@refs/tags/v1.6.0", + certBuilderID: common.GenericLowPermsDelegatorBuilderID + "@refs/tags/v1.6.0", trustedBuilderIDs: map[string]bool{ "slsa-framework/slsa-github-generator/.github/workflows/some_other_delegator.yml": true, "slsa-framework/slsa-github-generator/.github/workflows/some_delegator.yml": true, @@ -588,16 +588,18 @@ func Test_verifyTrustedBuilderID(t *testing.T) { tt := tt // Re-initializing variable so it is not changed while executing the closure below t.Run(tt.name, func(t *testing.T) { t.Parallel() - id, byob, err := verifyTrustedBuilderID(tt.path, tt.tag, tt.id, tt.defaults) + id, byob, err := verifyTrustedBuilderID(httpsGithubCom+tt.path, tt.tag, tt.id, tt.defaults) if byob != tt.byob { t.Errorf(cmp.Diff(byob, tt.byob)) } if diff := cmp.Diff(tt.err, err, cmpopts.EquateErrors()); diff != "" { t.Fatalf("unexpected error (-want +got):\n%s", diff) } - expectedID := "https://github.com/" + tt.path + "@" + tt.tag - if err := id.MatchesLoose(expectedID, true); err != nil { - t.Errorf("matches failed:%v", err) + if tt.err == nil { + expectedID := httpsGithubCom + tt.path + "@" + tt.tag + if err := id.MatchesLoose(expectedID, true); err != nil { + t.Errorf("matches failed:%v", err) + } } }) } @@ -878,7 +880,9 @@ func Test_GetWorkflowInfoFromCertificate(t *testing.T) { cert: x509.Certificate{ URIs: []*url.URL{ { - Path: "/" + repo + "/" + buildConfigPath, + Scheme: "https", + Host: "github.com", + Path: "/" + repo + "/" + buildConfigPath, }, }, Extensions: []pkix.Extension{ @@ -945,7 +949,9 @@ func Test_GetWorkflowInfoFromCertificate(t *testing.T) { cert: x509.Certificate{ URIs: []*url.URL{ { - Path: "/" + repo + "/" + buildConfigPath, + Scheme: "https", + Host: "github.com", + Path: "/" + repo + "/" + buildConfigPath, }, }, Extensions: []pkix.Extension{ @@ -1115,7 +1121,9 @@ func Test_GetWorkflowInfoFromCertificate(t *testing.T) { cert: x509.Certificate{ URIs: []*url.URL{ { - Path: "/" + repo + "/" + buildConfigPath, + Scheme: "https", + Host: "github.com", + Path: "/" + repo + "/" + buildConfigPath, }, }, Extensions: []pkix.Extension{ From b6698ff170d4d3333db4287cb7c9d0573e6140e2 Mon Sep 17 00:00:00 2001 From: Ian Lewis Date: Mon, 10 Jul 2023 05:38:58 +0000 Subject: [PATCH 5/6] Copy net.URL Signed-off-by: Ian Lewis --- verifiers/internal/gha/builder.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/verifiers/internal/gha/builder.go b/verifiers/internal/gha/builder.go index b03b8a6e8..8f00a252d 100644 --- a/verifiers/internal/gha/builder.go +++ b/verifiers/internal/gha/builder.go @@ -235,11 +235,9 @@ type WorkflowIdentity struct { // SubjectWorkflowName returns the subject workflow without the git ref. func (id *WorkflowIdentity) SubjectWorkflowName() string { - withoutRef, err := url.Parse(id.SubjectWorkflow.String()) - if err != nil { - // This should never happen. - panic(err) - } + // NOTE: You should be able to copy a net.URL struct safely. + // See: https://github.com/golang/go/issues/38351 + withoutRef := *id.SubjectWorkflow withoutRef.Path = id.SubjectWorkflowPath() return withoutRef.String() } From 035d270d8f1ff198729b6b1e7b59bd635fe37223 Mon Sep 17 00:00:00 2001 From: Ian Lewis Date: Mon, 10 Jul 2023 06:13:30 +0000 Subject: [PATCH 6/6] Fix workflow name/path/ref Signed-off-by: Ian Lewis --- verifiers/internal/gha/builder.go | 13 +++--- verifiers/internal/gha/builder_test.go | 57 ++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/verifiers/internal/gha/builder.go b/verifiers/internal/gha/builder.go index 8f00a252d..51531afb1 100644 --- a/verifiers/internal/gha/builder.go +++ b/verifiers/internal/gha/builder.go @@ -244,17 +244,20 @@ func (id *WorkflowIdentity) SubjectWorkflowName() string { // SubjectWorkflowPath returns the subject workflow without the server url. func (id *WorkflowIdentity) SubjectWorkflowPath() string { - parts := strings.SplitN(id.SubjectWorkflow.Path, "@", 2) - return parts[0] + i := strings.LastIndex(id.SubjectWorkflow.Path, "@") + if i == -1 { + return id.SubjectWorkflow.Path + } + return id.SubjectWorkflow.Path[:i] } // SubjectWorkflowRef returns the ref for the subject workflow. func (id *WorkflowIdentity) SubjectWorkflowRef() string { - parts := strings.SplitN(id.SubjectWorkflow.Path, "@", 2) - if len(parts) < 2 { + i := strings.LastIndex(id.SubjectWorkflow.Path, "@") + if i == -1 { return "" } - return parts[1] + return id.SubjectWorkflow.Path[i+1:] } func getHosted(cert *x509.Certificate) (*Hosted, error) { diff --git a/verifiers/internal/gha/builder_test.go b/verifiers/internal/gha/builder_test.go index d3a2a45c4..e710fec14 100644 --- a/verifiers/internal/gha/builder_test.go +++ b/verifiers/internal/gha/builder_test.go @@ -1260,3 +1260,60 @@ func Test_GetWorkflowInfoFromCertificate(t *testing.T) { }) } } + +func TestWorkflowIdentity(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + workflow WorkflowIdentity + workflowName string + workflowPath string + workflowRef string + }{ + { + name: "no ref", + workflow: WorkflowIdentity{ + SubjectWorkflow: Must(url.Parse("https://github.com/random/workflow/ref")), + }, + workflowName: "https://github.com/random/workflow/ref", + workflowPath: "/random/workflow/ref", + workflowRef: "", + }, + { + name: "with ref", + workflow: WorkflowIdentity{ + SubjectWorkflow: Must(url.Parse("https://github.com/random/workflow/ref@refs/heads/foo")), + }, + workflowName: "https://github.com/random/workflow/ref", + workflowPath: "/random/workflow/ref", + workflowRef: "refs/heads/foo", + }, + { + name: "multiple (at) symbols", + workflow: WorkflowIdentity{ + SubjectWorkflow: Must(url.Parse("https://github.com/random/work@flow/ref@refs/heads/foo")), + }, + workflowName: "https://github.com/random/work@flow/ref", + workflowPath: "/random/work@flow/ref", + workflowRef: "refs/heads/foo", + }, + } + + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + if got, want := tt.workflow.SubjectWorkflowName(), tt.workflowName; got != want { + t.Errorf("unexpected subject workflow name, got %q, want %q", got, want) + } + + if got, want := tt.workflow.SubjectWorkflowPath(), tt.workflowPath; got != want { + t.Errorf("unexpected subject workflow path, got %q, want %q", got, want) + } + + if got, want := tt.workflow.SubjectWorkflowRef(), tt.workflowRef; got != want { + t.Errorf("unexpected subject workflow ref, got %q, want %q", got, want) + } + }) + } +}