From a3f353d2f7f942e2c83aeae73529dcd054e5a443 Mon Sep 17 00:00:00 2001 From: Miroma <136986257+its-miroma@users.noreply.github.com> Date: Tue, 29 Oct 2024 09:47:45 +0100 Subject: [PATCH 01/33] Set `dnf5` commands as default Fedora 41 is now stable! :tada: Closes #9840 Co-authored-by: Melamit64 <151834564+melamit@users.noreply.github.com> --- docs/install_linux.md | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/docs/install_linux.md b/docs/install_linux.md index 9be6aed9b..5943bba24 100644 --- a/docs/install_linux.md +++ b/docs/install_linux.md @@ -33,39 +33,37 @@ sudo apt install gh > [!NOTE] > If errors regarding GPG signatures occur, see [cli/cli#9569](https://github.com/cli/cli/issues/9569) for steps to fix this. -### Fedora, CentOS, Red Hat Enterprise Linux (dnf) +### Fedora, CentOS, Red Hat Enterprise Linux (dnf5) Install from our package repository for immediate access to latest releases: ```bash -sudo dnf install 'dnf-command(config-manager)' -sudo dnf config-manager --add-repo https://cli.github.com/packages/rpm/gh-cli.repo +sudo dnf install dnf5-plugins +sudo dnf config-manager addrepo --from-repofile=https://cli.github.com/packages/rpm/gh-cli.repo sudo dnf install gh --repo gh-cli ``` -
-Show dnf5 commands +These commands apply for `dnf5`. If you're using `dnf4`, commands will vary slightly. -If you're using `dnf5`, commands will vary slightly: +
+Show dnf4 commands ```bash -sudo dnf5 install dnf5-plugins -sudo dnf5 config-manager addrepo --from-repofile=https://cli.github.com/packages/rpm/gh-cli.repo -sudo dnf5 install gh --repo gh-cli +sudo dnf4 install 'dnf-command(config-manager)' +sudo dnf4 config-manager --add-repo https://cli.github.com/packages/rpm/gh-cli.repo +sudo dnf4 install gh --repo gh-cli ``` - -For more details, check out the [`dnf5 config-manager` documentation](https://dnf5.readthedocs.io/en/latest/dnf5_plugins/config-manager.8.html).
+> [!NOTE] +> If errors regarding GPG signatures occur, see [cli/cli#9569](https://github.com/cli/cli/issues/9569) for steps to fix this. + Alternatively, install from the [community repository](https://packages.fedoraproject.org/pkgs/gh/gh/): ```bash sudo dnf install gh ``` -> [!NOTE] -> If errors regarding GPG signatures occur, see [cli/cli#9569](https://github.com/cli/cli/issues/9569) for steps to fix this. - Upgrade: ```bash From 704de0cf372972167f4b127f2a1a031c4c40e03e Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Tue, 29 Oct 2024 15:33:24 -0600 Subject: [PATCH 02/33] start building a separate policy struct Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index f41b2f66b..60a666ad0 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -8,6 +8,7 @@ import ( "github.com/sigstore/sigstore-go/pkg/fulcio/certificate" "github.com/sigstore/sigstore-go/pkg/verify" + "github.com/cli/cli/v2/pkg/cmd/attestation/api" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" ) @@ -18,6 +19,32 @@ const ( hostRegex = `^[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+.*$` ) +type ExpectedExtensions struct { + RunnerEnvironment string + SAN string + buildSourceRepo string + SignerWorkflow string +} + +type Policy struct { + ExpectedExtensions ExpectedExtensions + ExpectedPredicateType string + ExpectedSigstoreInstance string +} + +func buildPolicy(opts *Options, a artifact.DigestedArtifact) Policy { + return Policy{} +} + +func (p *Policy) Verify(a []*api.Attestation) (bool, string) { + filtered := verification.FilterAttestations(p.ExpectedPredicateType, a) + if len(filtered) == 0 { + return false, fmt.Sprintf("✗ No attestations found with predicate type: %s\n", p.ExpectedPredicateType) + } + + return true, "" +} + func expandToGitHubURL(tenant, ownerOrRepo string) string { if tenant == "" { return fmt.Sprintf("(?i)^https://github.com/%s/", ownerOrRepo) From e5b2b09a6e1c1963f64efcca379963d71bd5de2a Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Tue, 29 Oct 2024 16:41:17 -0600 Subject: [PATCH 03/33] move policy functions into methods Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 100 +++++++++++++-------------- pkg/cmd/attestation/verify/verify.go | 28 ++++++++ 2 files changed, 78 insertions(+), 50 deletions(-) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index 60a666ad0..abebba9e3 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -21,19 +21,58 @@ const ( type ExpectedExtensions struct { RunnerEnvironment string + SANRegex string SAN string - buildSourceRepo string + BuildSourceRepo string SignerWorkflow string } +type SigstoreInstance string + +const ( + PublicGood SigstoreInstance = "public-good" + GitHub SigstoreInstance = "github" + Custom SigstoreInstance = "custom" +) + type Policy struct { ExpectedExtensions ExpectedExtensions ExpectedPredicateType string ExpectedSigstoreInstance string + Artifact artifact.DigestedArtifact } -func buildPolicy(opts *Options, a artifact.DigestedArtifact) Policy { - return Policy{} +func newPolicy(opts *Options, a artifact.DigestedArtifact) (Policy, error) { + p := Policy{} + + if opts.SignerRepo != "" { + signedRepoRegex := expandToGitHubURL(opts.Tenant, opts.SignerRepo) + p.ExpectedExtensions.SANRegex = signedRepoRegex + } else if opts.SignerWorkflow != "" { + validatedWorkflowRegex, err := validateSignerWorkflow(opts) + if err != nil { + return Policy{}, err + } + + p.ExpectedExtensions.SANRegex = validatedWorkflowRegex + } else { + p.ExpectedExtensions.SANRegex = opts.SANRegex + p.ExpectedExtensions.SAN = opts.SAN + } + + if opts.DenySelfHostedRunner { + p.ExpectedExtensions.RunnerEnvironment = GitHubRunner + } else { + p.ExpectedExtensions.RunnerEnvironment = "*" + } + + if opts.Repo != "" { + if opts.Tenant != "" { + p.ExpectedExtensions.BuildSourceRepo = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo) + } + p.ExpectedExtensions.BuildSourceRepo = fmt.Sprintf("https://github.com/%s", opts.Repo) + } + return p, nil } func (p *Policy) Verify(a []*api.Attestation) (bool, string) { @@ -52,26 +91,8 @@ func expandToGitHubURL(tenant, ownerOrRepo string) string { return fmt.Sprintf("(?i)^https://%s.ghe.com/%s/", tenant, ownerOrRepo) } -func buildSANMatcher(opts *Options) (verify.SubjectAlternativeNameMatcher, error) { - if opts.SignerRepo != "" { - signedRepoRegex := expandToGitHubURL(opts.Tenant, opts.SignerRepo) - return verify.NewSANMatcher("", signedRepoRegex) - } else if opts.SignerWorkflow != "" { - validatedWorkflowRegex, err := validateSignerWorkflow(opts) - if err != nil { - return verify.SubjectAlternativeNameMatcher{}, err - } - - return verify.NewSANMatcher("", validatedWorkflowRegex) - } else if opts.SAN != "" || opts.SANRegex != "" { - return verify.NewSANMatcher(opts.SAN, opts.SANRegex) - } - - return verify.SubjectAlternativeNameMatcher{}, nil -} - -func buildCertificateIdentityOption(opts *Options, runnerEnv string) (verify.PolicyOption, error) { - sanMatcher, err := buildSANMatcher(opts) +func (p *Policy) buildCertificateIdentityOption() (verify.PolicyOption, error) { + sanMatcher, err := verify.NewSANMatcher(p.ExpectedExtensions.SAN, p.ExpectedExtensions.SANRegex) if err != nil { return nil, err } @@ -83,7 +104,7 @@ func buildCertificateIdentityOption(opts *Options, runnerEnv string) (verify.Pol } extensions := certificate.Extensions{ - RunnerEnvironment: runnerEnv, + RunnerEnvironment: p.ExpectedExtensions.RunnerEnvironment, } certId, err := verify.NewCertificateIdentity(sanMatcher, issuerMatcher, extensions) @@ -94,34 +115,13 @@ func buildCertificateIdentityOption(opts *Options, runnerEnv string) (verify.Pol return verify.WithCertificateIdentity(certId), nil } -func buildVerifyCertIdOption(opts *Options) (verify.PolicyOption, error) { - if opts.DenySelfHostedRunner { - withGHRunner, err := buildCertificateIdentityOption(opts, GitHubRunner) - if err != nil { - return nil, err - } - - return withGHRunner, nil - } - - // if Extensions.RunnerEnvironment value is set to the empty string - // through the second function argument, - // no certificate matching will happen on the RunnerEnvironment field - withAnyRunner, err := buildCertificateIdentityOption(opts, "") - if err != nil { - return nil, err - } - - return withAnyRunner, nil -} - -func buildVerifyPolicy(opts *Options, a artifact.DigestedArtifact) (verify.PolicyBuilder, error) { - artifactDigestPolicyOption, err := verification.BuildDigestPolicyOption(a) +func (p *Policy) SigstorePolicy() (verify.PolicyBuilder, error) { + artifactDigestPolicyOption, err := verification.BuildDigestPolicyOption(p.Artifact) if err != nil { return verify.PolicyBuilder{}, err } - certIdOption, err := buildVerifyCertIdOption(opts) + certIdOption, err := p.buildCertificateIdentityOption() if err != nil { return verify.PolicyBuilder{}, err } @@ -143,12 +143,12 @@ func validateSignerWorkflow(opts *Options) (string, error) { } if match { - return addSchemeToRegex(opts.SignerWorkflow), nil + return fmt.Sprintf("^https://%s", opts.SignerWorkflow), nil } if opts.Hostname == "" { return "", errors.New("unknown host") } - return addSchemeToRegex(fmt.Sprintf("%s/%s", opts.Hostname, opts.SignerWorkflow)), nil + return fmt.Sprintf("^https://%s/%s/%s", opts.Hostname, opts.SignerWorkflow), nil } diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index d14081dd8..8b58296f3 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -364,3 +364,31 @@ func buildTableVerifyContent(tenant string, results []*verification.AttestationP return content, nil } + +func verifyAll(opts *Options, artifact artifact.DigestedArtifact, attestations []*api.Attestation) error { + policy, err := newPolicy(opts, artifact) + if err != nil { + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy")) + return err + } + + sp, err := policy.SigstorePolicy() + if err != nil { + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy")) + return err + } + + sigstoreRes := opts.SigstoreVerifier.Verify(attestations, sp) + if sigstoreRes.Error != nil { + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Verification failed")) + return sigstoreRes.Error + } + + // Verify extensions + if err := verification.VerifyCertExtensions(sigstoreRes.VerifyResults, opts.Tenant, opts.Owner, opts.Repo, opts.OIDCIssuer); err != nil { + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Verification failed")) + return err + } + + return nil +} From e16b69bd08d6c1b9e4a4c634398d11fbdf960181 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Tue, 29 Oct 2024 17:27:47 -0600 Subject: [PATCH 04/33] cert extension funcs are now policy methods Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 103 +++++++++++++++++++--- pkg/cmd/attestation/verify/policy_test.go | 3 +- pkg/cmd/attestation/verify/verify.go | 36 +++----- 3 files changed, 101 insertions(+), 41 deletions(-) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index abebba9e3..b4672213d 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "regexp" + "strings" "github.com/sigstore/sigstore-go/pkg/fulcio/certificate" "github.com/sigstore/sigstore-go/pkg/verify" @@ -20,11 +21,13 @@ const ( ) type ExpectedExtensions struct { - RunnerEnvironment string - SANRegex string - SAN string - BuildSourceRepo string - SignerWorkflow string + RunnerEnvironment string + SANRegex string + SAN string + BuildSourceRepoURI string + SignerWorkflow string + SourceRepositoryOwnerURI string + SourceRepositoryURI string } type SigstoreInstance string @@ -40,10 +43,13 @@ type Policy struct { ExpectedPredicateType string ExpectedSigstoreInstance string Artifact artifact.DigestedArtifact + OIDCIssuer string } func newPolicy(opts *Options, a artifact.DigestedArtifact) (Policy, error) { - p := Policy{} + p := Policy{ + Artifact: a, + } if opts.SignerRepo != "" { signedRepoRegex := expandToGitHubURL(opts.Tenant, opts.SignerRepo) @@ -68,10 +74,25 @@ func newPolicy(opts *Options, a artifact.DigestedArtifact) (Policy, error) { if opts.Repo != "" { if opts.Tenant != "" { - p.ExpectedExtensions.BuildSourceRepo = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo) + p.ExpectedExtensions.BuildSourceRepoURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo) } - p.ExpectedExtensions.BuildSourceRepo = fmt.Sprintf("https://github.com/%s", opts.Repo) + p.ExpectedExtensions.BuildSourceRepoURI = fmt.Sprintf("https://github.com/%s", opts.Repo) } + + if opts.Tenant != "" { + p.ExpectedExtensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Owner) + } else { + p.ExpectedExtensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) + } + + // if issuer is anything other than the default, use the user-provided value; + // otherwise, select the appropriate default based on the tenant + if opts.Tenant != "" { + p.OIDCIssuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant) + } else { + p.OIDCIssuer = opts.OIDCIssuer + } + return p, nil } @@ -115,6 +136,16 @@ func (p *Policy) buildCertificateIdentityOption() (verify.PolicyOption, error) { return verify.WithCertificateIdentity(certId), nil } +func (p *Policy) VerifyPredicateType(a []*api.Attestation) ([]*api.Attestation, error) { + filteredAttestations := verification.FilterAttestations(p.ExpectedPredicateType, a) + + if len(filteredAttestations) == 0 { + return nil, fmt.Errorf("✗ No attestations found with predicate type: %s\n", p.ExpectedPredicateType) + } + + return filteredAttestations, nil +} + func (p *Policy) SigstorePolicy() (verify.PolicyBuilder, error) { artifactDigestPolicyOption, err := verification.BuildDigestPolicyOption(p.Artifact) if err != nil { @@ -130,10 +161,6 @@ func (p *Policy) SigstorePolicy() (verify.PolicyBuilder, error) { return policy, nil } -func addSchemeToRegex(s string) string { - return fmt.Sprintf("^https://%s", s) -} - func validateSignerWorkflow(opts *Options) (string, error) { // we expect a provided workflow argument be in the format [HOST/]///path/to/workflow.yml // if the provided workflow does not contain a host, set the host @@ -150,5 +177,55 @@ func validateSignerWorkflow(opts *Options) (string, error) { return "", errors.New("unknown host") } - return fmt.Sprintf("^https://%s/%s/%s", opts.Hostname, opts.SignerWorkflow), nil + return fmt.Sprintf("^https://%s/%s", opts.Hostname, opts.SignerWorkflow), nil +} + +func (p *Policy) VerifyCertExtensions(results []*verification.AttestationProcessingResult) error { + if len(results) == 0 { + return errors.New("no attestations proccessing results") + } + + var atLeastOneVerified bool + for _, attestation := range results { + if err := p.verifyCertExtensions(attestation); err != nil { + return err + } + atLeastOneVerified = true + } + + if atLeastOneVerified { + return nil + } else { + return verification.ErrNoAttestationsVerified + } +} + +func (p *Policy) verifyCertExtensions(attestation *verification.AttestationProcessingResult) error { + if p.ExpectedExtensions.SourceRepositoryOwnerURI != "" { + sourceRepositoryOwnerURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI + if !strings.EqualFold(p.ExpectedExtensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { + return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", p.ExpectedExtensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) + } + } + + // if repo is set, check the SourceRepositoryURI field + if p.ExpectedExtensions.SourceRepositoryURI != "" { + sourceRepositoryURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryURI + if !strings.EqualFold(p.ExpectedExtensions.SourceRepositoryURI, sourceRepositoryURI) { + return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", p.ExpectedExtensions.SourceRepositoryURI, sourceRepositoryURI) + } + } + + if p.OIDCIssuer != "" { + certIssuer := attestation.VerificationResult.Signature.Certificate.Extensions.Issuer + if !strings.EqualFold(p.OIDCIssuer, certIssuer) { + if strings.Index(certIssuer, p.OIDCIssuer+"/") == 0 { + return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", p.OIDCIssuer, certIssuer) + } else { + return fmt.Errorf("expected Issuer to be %s, got %s", p.OIDCIssuer, certIssuer) + } + } + } + + return nil } diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go index ae1e52955..70c3079e1 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -26,7 +26,7 @@ func TestBuildPolicy(t *testing.T) { SANRegex: "^https://github.com/sigstore/", } - _, err = buildVerifyPolicy(opts, *artifact) + _, err = newPolicy(opts, *artifact) require.NoError(t, err) } @@ -87,7 +87,6 @@ func TestValidateSignerWorkflow(t *testing.T) { workflowRegex, err := validateSignerWorkflow(opts) require.NoError(t, err) require.Equal(t, tc.expectedWorkflowRegex, workflowRegex) - } } diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 8b58296f3..a873d6c96 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -255,21 +255,9 @@ func runVerify(opts *Options) error { attestations = filteredAttestations } - policy, err := buildVerifyPolicy(opts, *artifact) + sigstoreResults, err := verifyAll(opts, *artifact, attestations) if err != nil { - opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy")) - return err - } - - sigstoreRes := opts.SigstoreVerifier.Verify(attestations, policy) - if sigstoreRes.Error != nil { - opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Verification failed")) - return sigstoreRes.Error - } - - // Verify extensions - if err := verification.VerifyCertExtensions(sigstoreRes.VerifyResults, opts.Tenant, opts.Owner, opts.Repo, opts.OIDCIssuer); err != nil { - opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Verification failed")) + opts.Logger.Println(opts.Logger.ColorScheme.Red(err.Error())) return err } @@ -278,7 +266,7 @@ func runVerify(opts *Options) error { // If an exporter is provided with the --json flag, write the results to the terminal in JSON format if opts.exporter != nil { // print the results to the terminal as an array of JSON objects - if err = opts.exporter.Write(opts.Logger.IO, sigstoreRes.VerifyResults); err != nil { + if err = opts.exporter.Write(opts.Logger.IO, sigstoreResults.VerifyResults); err != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to write JSON output")) return err } @@ -288,7 +276,7 @@ func runVerify(opts *Options) error { opts.Logger.Printf("%s was attested by:\n", artifact.DigestWithAlg()) // Otherwise print the results to the terminal in a table - tableContent, err := buildTableVerifyContent(opts.Tenant, sigstoreRes.VerifyResults) + tableContent, err := buildTableVerifyContent(opts.Tenant, sigstoreResults.VerifyResults) if err != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("failed to parse results")) return err @@ -365,30 +353,26 @@ func buildTableVerifyContent(tenant string, results []*verification.AttestationP return content, nil } -func verifyAll(opts *Options, artifact artifact.DigestedArtifact, attestations []*api.Attestation) error { +func verifyAll(opts *Options, artifact artifact.DigestedArtifact, attestations []*api.Attestation) (*verification.SigstoreResults, error) { policy, err := newPolicy(opts, artifact) if err != nil { - opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy")) - return err + return nil, fmt.Errorf("✗ Failed to build verification policy") } sp, err := policy.SigstorePolicy() if err != nil { - opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy")) - return err + return nil, fmt.Errorf("✗ Failed to build Sigstore verification policy") } sigstoreRes := opts.SigstoreVerifier.Verify(attestations, sp) if sigstoreRes.Error != nil { - opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Verification failed")) - return sigstoreRes.Error + return nil, fmt.Errorf("✗ Sigstore verification failed") } // Verify extensions if err := verification.VerifyCertExtensions(sigstoreRes.VerifyResults, opts.Tenant, opts.Owner, opts.Repo, opts.OIDCIssuer); err != nil { - opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Verification failed")) - return err + return nil, fmt.Errorf("✗ Policy verification failed") } - return nil + return sigstoreRes, nil } From 41c3ba5fa74eedbe27740cd3cc45cfffd77812d0 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Tue, 29 Oct 2024 18:19:19 -0600 Subject: [PATCH 05/33] drop sigstore instance for now Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 63 ++++++++++++---------------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index b4672213d..2278b1cb5 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -20,7 +20,7 @@ const ( hostRegex = `^[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+.*$` ) -type ExpectedExtensions struct { +type Extensions struct { RunnerEnvironment string SANRegex string SAN string @@ -30,20 +30,11 @@ type ExpectedExtensions struct { SourceRepositoryURI string } -type SigstoreInstance string - -const ( - PublicGood SigstoreInstance = "public-good" - GitHub SigstoreInstance = "github" - Custom SigstoreInstance = "custom" -) - type Policy struct { - ExpectedExtensions ExpectedExtensions - ExpectedPredicateType string - ExpectedSigstoreInstance string - Artifact artifact.DigestedArtifact - OIDCIssuer string + Extensions Extensions + PredicateType string + Artifact artifact.DigestedArtifact + OIDCIssuer string } func newPolicy(opts *Options, a artifact.DigestedArtifact) (Policy, error) { @@ -53,36 +44,36 @@ func newPolicy(opts *Options, a artifact.DigestedArtifact) (Policy, error) { if opts.SignerRepo != "" { signedRepoRegex := expandToGitHubURL(opts.Tenant, opts.SignerRepo) - p.ExpectedExtensions.SANRegex = signedRepoRegex + p.Extensions.SANRegex = signedRepoRegex } else if opts.SignerWorkflow != "" { validatedWorkflowRegex, err := validateSignerWorkflow(opts) if err != nil { return Policy{}, err } - p.ExpectedExtensions.SANRegex = validatedWorkflowRegex + p.Extensions.SANRegex = validatedWorkflowRegex } else { - p.ExpectedExtensions.SANRegex = opts.SANRegex - p.ExpectedExtensions.SAN = opts.SAN + p.Extensions.SANRegex = opts.SANRegex + p.Extensions.SAN = opts.SAN } if opts.DenySelfHostedRunner { - p.ExpectedExtensions.RunnerEnvironment = GitHubRunner + p.Extensions.RunnerEnvironment = GitHubRunner } else { - p.ExpectedExtensions.RunnerEnvironment = "*" + p.Extensions.RunnerEnvironment = "*" } if opts.Repo != "" { if opts.Tenant != "" { - p.ExpectedExtensions.BuildSourceRepoURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo) + p.Extensions.BuildSourceRepoURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo) } - p.ExpectedExtensions.BuildSourceRepoURI = fmt.Sprintf("https://github.com/%s", opts.Repo) + p.Extensions.BuildSourceRepoURI = fmt.Sprintf("https://github.com/%s", opts.Repo) } if opts.Tenant != "" { - p.ExpectedExtensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Owner) + p.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Owner) } else { - p.ExpectedExtensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) + p.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) } // if issuer is anything other than the default, use the user-provided value; @@ -97,9 +88,9 @@ func newPolicy(opts *Options, a artifact.DigestedArtifact) (Policy, error) { } func (p *Policy) Verify(a []*api.Attestation) (bool, string) { - filtered := verification.FilterAttestations(p.ExpectedPredicateType, a) + filtered := verification.FilterAttestations(p.PredicateType, a) if len(filtered) == 0 { - return false, fmt.Sprintf("✗ No attestations found with predicate type: %s\n", p.ExpectedPredicateType) + return false, fmt.Sprintf("✗ No attestations found with predicate type: %s\n", p.PredicateType) } return true, "" @@ -113,7 +104,7 @@ func expandToGitHubURL(tenant, ownerOrRepo string) string { } func (p *Policy) buildCertificateIdentityOption() (verify.PolicyOption, error) { - sanMatcher, err := verify.NewSANMatcher(p.ExpectedExtensions.SAN, p.ExpectedExtensions.SANRegex) + sanMatcher, err := verify.NewSANMatcher(p.Extensions.SAN, p.Extensions.SANRegex) if err != nil { return nil, err } @@ -125,7 +116,7 @@ func (p *Policy) buildCertificateIdentityOption() (verify.PolicyOption, error) { } extensions := certificate.Extensions{ - RunnerEnvironment: p.ExpectedExtensions.RunnerEnvironment, + RunnerEnvironment: p.Extensions.RunnerEnvironment, } certId, err := verify.NewCertificateIdentity(sanMatcher, issuerMatcher, extensions) @@ -137,10 +128,10 @@ func (p *Policy) buildCertificateIdentityOption() (verify.PolicyOption, error) { } func (p *Policy) VerifyPredicateType(a []*api.Attestation) ([]*api.Attestation, error) { - filteredAttestations := verification.FilterAttestations(p.ExpectedPredicateType, a) + filteredAttestations := verification.FilterAttestations(p.PredicateType, a) if len(filteredAttestations) == 0 { - return nil, fmt.Errorf("✗ No attestations found with predicate type: %s\n", p.ExpectedPredicateType) + return nil, fmt.Errorf("✗ No attestations found with predicate type: %s\n", p.PredicateType) } return filteredAttestations, nil @@ -201,18 +192,18 @@ func (p *Policy) VerifyCertExtensions(results []*verification.AttestationProcess } func (p *Policy) verifyCertExtensions(attestation *verification.AttestationProcessingResult) error { - if p.ExpectedExtensions.SourceRepositoryOwnerURI != "" { + if p.Extensions.SourceRepositoryOwnerURI != "" { sourceRepositoryOwnerURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI - if !strings.EqualFold(p.ExpectedExtensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { - return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", p.ExpectedExtensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) + if !strings.EqualFold(p.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { + return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", p.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) } } // if repo is set, check the SourceRepositoryURI field - if p.ExpectedExtensions.SourceRepositoryURI != "" { + if p.Extensions.SourceRepositoryURI != "" { sourceRepositoryURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryURI - if !strings.EqualFold(p.ExpectedExtensions.SourceRepositoryURI, sourceRepositoryURI) { - return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", p.ExpectedExtensions.SourceRepositoryURI, sourceRepositoryURI) + if !strings.EqualFold(p.Extensions.SourceRepositoryURI, sourceRepositoryURI) { + return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", p.Extensions.SourceRepositoryURI, sourceRepositoryURI) } } From 3378b546da257a82795c02c7f8493eca7fff7693 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 30 Oct 2024 12:58:40 -0600 Subject: [PATCH 06/33] simplify if else logic Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index 2278b1cb5..d23cf32df 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -212,9 +212,8 @@ func (p *Policy) verifyCertExtensions(attestation *verification.AttestationProce if !strings.EqualFold(p.OIDCIssuer, certIssuer) { if strings.Index(certIssuer, p.OIDCIssuer+"/") == 0 { return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", p.OIDCIssuer, certIssuer) - } else { - return fmt.Errorf("expected Issuer to be %s, got %s", p.OIDCIssuer, certIssuer) } + return fmt.Errorf("expected Issuer to be %s, got %s", p.OIDCIssuer, certIssuer) } } From b44c9d3003f1e8e6869d8aedba6943a4f5ae0b6e Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 30 Oct 2024 15:23:50 -0600 Subject: [PATCH 07/33] undo policy method changes Signed-off-by: Meredith Lancaster --- .../attestation/verification/extensions.go | 56 ++--- pkg/cmd/attestation/verification/policy.go | 17 ++ pkg/cmd/attestation/verify/policy.go | 233 ++++++------------ pkg/cmd/attestation/verify/policy_test.go | 2 +- pkg/cmd/attestation/verify/verify.go | 50 ++-- 5 files changed, 131 insertions(+), 227 deletions(-) diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index 94ba88208..13c83651e 100644 --- a/pkg/cmd/attestation/verification/extensions.go +++ b/pkg/cmd/attestation/verification/extensions.go @@ -11,14 +11,14 @@ var ( GitHubTenantOIDCIssuer = "https://token.actions.%s.ghe.com" ) -func VerifyCertExtensions(results []*AttestationProcessingResult, tenant, owner, repo, issuer string) error { +func VerifyCertExtensions(results []*AttestationProcessingResult, ec EnforcementCriteria) error { if len(results) == 0 { return errors.New("no attestations proccessing results") } var atLeastOneVerified bool for _, attestation := range results { - if err := verifyCertExtensions(attestation, tenant, owner, repo, issuer); err != nil { + if err := verifyCertExtensions(attestation, ec); err != nil { return err } atLeastOneVerified = true @@ -31,51 +31,31 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, tenant, owner, } } -func verifyCertExtensions(attestation *AttestationProcessingResult, tenant, owner, repo, issuer string) error { - var want string - - if tenant == "" { - want = fmt.Sprintf("https://github.com/%s", owner) - } else { - want = fmt.Sprintf("https://%s.ghe.com/%s", tenant, owner) - } - sourceRepositoryOwnerURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI - if !strings.EqualFold(want, sourceRepositoryOwnerURI) { - return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", want, sourceRepositoryOwnerURI) +func verifyCertExtensions(attestation *AttestationProcessingResult, c EnforcementCriteria) error { + if c.Extensions.SourceRepositoryOwnerURI != "" { + sourceRepositoryOwnerURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI + if !strings.EqualFold(c.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { + return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", c.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) + } } // if repo is set, check the SourceRepositoryURI field - if repo != "" { - if tenant == "" { - want = fmt.Sprintf("https://github.com/%s", repo) - } else { - want = fmt.Sprintf("https://%s.ghe.com/%s", tenant, repo) - } - + if c.Extensions.SourceRepositoryURI != "" { sourceRepositoryURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryURI - if !strings.EqualFold(want, sourceRepositoryURI) { - return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", want, sourceRepositoryURI) + if !strings.EqualFold(c.Extensions.SourceRepositoryURI, sourceRepositoryURI) { + return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", c.Extensions.SourceRepositoryURI, sourceRepositoryURI) } } // if issuer is anything other than the default, use the user-provided value; // otherwise, select the appropriate default based on the tenant - if issuer != GitHubOIDCIssuer { - want = issuer - } else { - if tenant != "" { - want = fmt.Sprintf(GitHubTenantOIDCIssuer, tenant) - } else { - want = GitHubOIDCIssuer - } - } - - certIssuer := attestation.VerificationResult.Signature.Certificate.Extensions.Issuer - if !strings.EqualFold(want, certIssuer) { - if strings.Index(certIssuer, want+"/") == 0 { - return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", want, certIssuer) - } else { - return fmt.Errorf("expected Issuer to be %s, got %s", want, certIssuer) + if c.OIDCIssuer != "" { + certIssuer := attestation.VerificationResult.Signature.Certificate.Extensions.Issuer + if !strings.EqualFold(c.OIDCIssuer, certIssuer) { + if strings.Index(certIssuer, c.OIDCIssuer+"/") == 0 { + return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", c.OIDCIssuer, certIssuer) + } + return fmt.Errorf("expected Issuer to be %s, got %s", c.OIDCIssuer, certIssuer) } } diff --git a/pkg/cmd/attestation/verification/policy.go b/pkg/cmd/attestation/verification/policy.go index 91b54c75e..21210e730 100644 --- a/pkg/cmd/attestation/verification/policy.go +++ b/pkg/cmd/attestation/verification/policy.go @@ -18,3 +18,20 @@ func BuildDigestPolicyOption(a artifact.DigestedArtifact) (verify.ArtifactPolicy } return verify.WithArtifactDigest(a.Algorithm(), decoded), nil } + +type Extensions struct { + RunnerEnvironment string + SANRegex string + SAN string + BuildSourceRepoURI string + SignerWorkflow string + SourceRepositoryOwnerURI string + SourceRepositoryURI string +} + +type EnforcementCriteria struct { + Extensions Extensions + PredicateType string + Artifact artifact.DigestedArtifact + OIDCIssuer string +} diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index d23cf32df..e2c357a0f 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -4,12 +4,10 @@ import ( "errors" "fmt" "regexp" - "strings" "github.com/sigstore/sigstore-go/pkg/fulcio/certificate" "github.com/sigstore/sigstore-go/pkg/verify" - "github.com/cli/cli/v2/pkg/cmd/attestation/api" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" ) @@ -20,82 +18,6 @@ const ( hostRegex = `^[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+.*$` ) -type Extensions struct { - RunnerEnvironment string - SANRegex string - SAN string - BuildSourceRepoURI string - SignerWorkflow string - SourceRepositoryOwnerURI string - SourceRepositoryURI string -} - -type Policy struct { - Extensions Extensions - PredicateType string - Artifact artifact.DigestedArtifact - OIDCIssuer string -} - -func newPolicy(opts *Options, a artifact.DigestedArtifact) (Policy, error) { - p := Policy{ - Artifact: a, - } - - if opts.SignerRepo != "" { - signedRepoRegex := expandToGitHubURL(opts.Tenant, opts.SignerRepo) - p.Extensions.SANRegex = signedRepoRegex - } else if opts.SignerWorkflow != "" { - validatedWorkflowRegex, err := validateSignerWorkflow(opts) - if err != nil { - return Policy{}, err - } - - p.Extensions.SANRegex = validatedWorkflowRegex - } else { - p.Extensions.SANRegex = opts.SANRegex - p.Extensions.SAN = opts.SAN - } - - if opts.DenySelfHostedRunner { - p.Extensions.RunnerEnvironment = GitHubRunner - } else { - p.Extensions.RunnerEnvironment = "*" - } - - if opts.Repo != "" { - if opts.Tenant != "" { - p.Extensions.BuildSourceRepoURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo) - } - p.Extensions.BuildSourceRepoURI = fmt.Sprintf("https://github.com/%s", opts.Repo) - } - - if opts.Tenant != "" { - p.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Owner) - } else { - p.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) - } - - // if issuer is anything other than the default, use the user-provided value; - // otherwise, select the appropriate default based on the tenant - if opts.Tenant != "" { - p.OIDCIssuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant) - } else { - p.OIDCIssuer = opts.OIDCIssuer - } - - return p, nil -} - -func (p *Policy) Verify(a []*api.Attestation) (bool, string) { - filtered := verification.FilterAttestations(p.PredicateType, a) - if len(filtered) == 0 { - return false, fmt.Sprintf("✗ No attestations found with predicate type: %s\n", p.PredicateType) - } - - return true, "" -} - func expandToGitHubURL(tenant, ownerOrRepo string) string { if tenant == "" { return fmt.Sprintf("(?i)^https://github.com/%s/", ownerOrRepo) @@ -103,55 +25,6 @@ func expandToGitHubURL(tenant, ownerOrRepo string) string { return fmt.Sprintf("(?i)^https://%s.ghe.com/%s/", tenant, ownerOrRepo) } -func (p *Policy) buildCertificateIdentityOption() (verify.PolicyOption, error) { - sanMatcher, err := verify.NewSANMatcher(p.Extensions.SAN, p.Extensions.SANRegex) - if err != nil { - return nil, err - } - - // Accept any issuer, we will verify the issuer as part of the extension verification - issuerMatcher, err := verify.NewIssuerMatcher("", ".*") - if err != nil { - return nil, err - } - - extensions := certificate.Extensions{ - RunnerEnvironment: p.Extensions.RunnerEnvironment, - } - - certId, err := verify.NewCertificateIdentity(sanMatcher, issuerMatcher, extensions) - if err != nil { - return nil, err - } - - return verify.WithCertificateIdentity(certId), nil -} - -func (p *Policy) VerifyPredicateType(a []*api.Attestation) ([]*api.Attestation, error) { - filteredAttestations := verification.FilterAttestations(p.PredicateType, a) - - if len(filteredAttestations) == 0 { - return nil, fmt.Errorf("✗ No attestations found with predicate type: %s\n", p.PredicateType) - } - - return filteredAttestations, nil -} - -func (p *Policy) SigstorePolicy() (verify.PolicyBuilder, error) { - artifactDigestPolicyOption, err := verification.BuildDigestPolicyOption(p.Artifact) - if err != nil { - return verify.PolicyBuilder{}, err - } - - certIdOption, err := p.buildCertificateIdentityOption() - if err != nil { - return verify.PolicyBuilder{}, err - } - - policy := verify.NewPolicy(artifactDigestPolicyOption, certIdOption) - return policy, nil -} - func validateSignerWorkflow(opts *Options) (string, error) { // we expect a provided workflow argument be in the format [HOST/]///path/to/workflow.yml // if the provided workflow does not contain a host, set the host @@ -171,51 +44,91 @@ func validateSignerWorkflow(opts *Options) (string, error) { return fmt.Sprintf("^https://%s/%s", opts.Hostname, opts.SignerWorkflow), nil } -func (p *Policy) VerifyCertExtensions(results []*verification.AttestationProcessingResult) error { - if len(results) == 0 { - return errors.New("no attestations proccessing results") +func newEnforcementCriteria(opts *Options, a artifact.DigestedArtifact) (verification.EnforcementCriteria, error) { + c := verification.EnforcementCriteria{ + Artifact: a, } - var atLeastOneVerified bool - for _, attestation := range results { - if err := p.verifyCertExtensions(attestation); err != nil { - return err + if opts.SignerRepo != "" { + signedRepoRegex := expandToGitHubURL(opts.Tenant, opts.SignerRepo) + c.Extensions.SANRegex = signedRepoRegex + } else if opts.SignerWorkflow != "" { + validatedWorkflowRegex, err := validateSignerWorkflow(opts) + if err != nil { + return verification.EnforcementCriteria{}, err } - atLeastOneVerified = true - } - if atLeastOneVerified { - return nil + c.Extensions.SANRegex = validatedWorkflowRegex } else { - return verification.ErrNoAttestationsVerified + c.Extensions.SANRegex = opts.SANRegex + c.Extensions.SAN = opts.SAN } + + if opts.DenySelfHostedRunner { + c.Extensions.RunnerEnvironment = GitHubRunner + } else { + c.Extensions.RunnerEnvironment = "*" + } + + if opts.Repo != "" { + if opts.Tenant != "" { + c.Extensions.BuildSourceRepoURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo) + } + c.Extensions.BuildSourceRepoURI = fmt.Sprintf("https://github.com/%s", opts.Repo) + } + + if opts.Tenant != "" { + c.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Owner) + } else { + c.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) + } + + // if issuer is anything other than the default, use the user-provided value; + // otherwise, select the appropriate default based on the tenant + if opts.Tenant != "" { + c.OIDCIssuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant) + } else { + c.OIDCIssuer = opts.OIDCIssuer + } + + return c, nil } -func (p *Policy) verifyCertExtensions(attestation *verification.AttestationProcessingResult) error { - if p.Extensions.SourceRepositoryOwnerURI != "" { - sourceRepositoryOwnerURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI - if !strings.EqualFold(p.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { - return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", p.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) - } +func buildCertificateIdentityOption(c verification.EnforcementCriteria) (verify.PolicyOption, error) { + sanMatcher, err := verify.NewSANMatcher(c.Extensions.SAN, c.Extensions.SANRegex) + if err != nil { + return nil, err } - // if repo is set, check the SourceRepositoryURI field - if p.Extensions.SourceRepositoryURI != "" { - sourceRepositoryURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryURI - if !strings.EqualFold(p.Extensions.SourceRepositoryURI, sourceRepositoryURI) { - return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", p.Extensions.SourceRepositoryURI, sourceRepositoryURI) - } + // Accept any issuer, we will verify the issuer as part of the extension verification + issuerMatcher, err := verify.NewIssuerMatcher("", ".*") + if err != nil { + return nil, err } - if p.OIDCIssuer != "" { - certIssuer := attestation.VerificationResult.Signature.Certificate.Extensions.Issuer - if !strings.EqualFold(p.OIDCIssuer, certIssuer) { - if strings.Index(certIssuer, p.OIDCIssuer+"/") == 0 { - return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", p.OIDCIssuer, certIssuer) - } - return fmt.Errorf("expected Issuer to be %s, got %s", p.OIDCIssuer, certIssuer) - } + extensions := certificate.Extensions{ + RunnerEnvironment: c.Extensions.RunnerEnvironment, } - return nil + certId, err := verify.NewCertificateIdentity(sanMatcher, issuerMatcher, extensions) + if err != nil { + return nil, err + } + + return verify.WithCertificateIdentity(certId), nil +} + +func SigstorePolicy(c verification.EnforcementCriteria) (verify.PolicyBuilder, error) { + artifactDigestPolicyOption, err := verification.BuildDigestPolicyOption(c.Artifact) + if err != nil { + return verify.PolicyBuilder{}, err + } + + certIdOption, err := buildCertificateIdentityOption(c) + if err != nil { + return verify.PolicyBuilder{}, err + } + + policy := verify.NewPolicy(artifactDigestPolicyOption, certIdOption) + return policy, nil } diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go index 70c3079e1..8d6c1dfbc 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -26,7 +26,7 @@ func TestBuildPolicy(t *testing.T) { SANRegex: "^https://github.com/sigstore/", } - _, err = newPolicy(opts, *artifact) + _, err = newEnforcementCriteria(opts, *artifact) require.NoError(t, err) } diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index a873d6c96..4c6af2b70 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -255,9 +255,27 @@ func runVerify(opts *Options) error { attestations = filteredAttestations } - sigstoreResults, err := verifyAll(opts, *artifact, attestations) + ec, err := newEnforcementCriteria(opts, *artifact) if err != nil { - opts.Logger.Println(opts.Logger.ColorScheme.Red(err.Error())) + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy")) + return err + } + + sp, err := SigstorePolicy(ec) + if err != nil { + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build Sigstore verification policy")) + return err + } + + sigstoreRes := opts.SigstoreVerifier.Verify(attestations, sp) + if sigstoreRes.Error != nil { + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Sigstore verification failed")) + return err + } + + // Verify extensions + if err := verification.VerifyCertExtensions(sigstoreRes.VerifyResults, ec); err != nil { + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Policy verification failed")) return err } @@ -266,7 +284,7 @@ func runVerify(opts *Options) error { // If an exporter is provided with the --json flag, write the results to the terminal in JSON format if opts.exporter != nil { // print the results to the terminal as an array of JSON objects - if err = opts.exporter.Write(opts.Logger.IO, sigstoreResults.VerifyResults); err != nil { + if err = opts.exporter.Write(opts.Logger.IO, sigstoreRes.VerifyResults); err != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to write JSON output")) return err } @@ -276,7 +294,7 @@ func runVerify(opts *Options) error { opts.Logger.Printf("%s was attested by:\n", artifact.DigestWithAlg()) // Otherwise print the results to the terminal in a table - tableContent, err := buildTableVerifyContent(opts.Tenant, sigstoreResults.VerifyResults) + tableContent, err := buildTableVerifyContent(opts.Tenant, sigstoreRes.VerifyResults) if err != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("failed to parse results")) return err @@ -352,27 +370,3 @@ func buildTableVerifyContent(tenant string, results []*verification.AttestationP return content, nil } - -func verifyAll(opts *Options, artifact artifact.DigestedArtifact, attestations []*api.Attestation) (*verification.SigstoreResults, error) { - policy, err := newPolicy(opts, artifact) - if err != nil { - return nil, fmt.Errorf("✗ Failed to build verification policy") - } - - sp, err := policy.SigstorePolicy() - if err != nil { - return nil, fmt.Errorf("✗ Failed to build Sigstore verification policy") - } - - sigstoreRes := opts.SigstoreVerifier.Verify(attestations, sp) - if sigstoreRes.Error != nil { - return nil, fmt.Errorf("✗ Sigstore verification failed") - } - - // Verify extensions - if err := verification.VerifyCertExtensions(sigstoreRes.VerifyResults, opts.Tenant, opts.Owner, opts.Repo, opts.OIDCIssuer); err != nil { - return nil, fmt.Errorf("✗ Policy verification failed") - } - - return sigstoreRes, nil -} From 93c78a21346ed579252ade47ba41f3d5bb94f337 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 30 Oct 2024 15:28:34 -0600 Subject: [PATCH 08/33] use sigstore specific err Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/verify.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 4c6af2b70..7aae31e23 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -270,7 +270,7 @@ func runVerify(opts *Options) error { sigstoreRes := opts.SigstoreVerifier.Verify(attestations, sp) if sigstoreRes.Error != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Sigstore verification failed")) - return err + return sigstoreRes.Error } // Verify extensions From 4fa5f0c5eed22e8585c74d24ac66c99985de420e Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 30 Oct 2024 15:44:53 -0600 Subject: [PATCH 09/33] update extensions test Signed-off-by: Meredith Lancaster --- .../verification/extensions_test.go | 92 +++++++++++++++---- 1 file changed, 73 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/attestation/verification/extensions_test.go b/pkg/cmd/attestation/verification/extensions_test.go index 5eb28829d..e1a1555c5 100644 --- a/pkg/cmd/attestation/verification/extensions_test.go +++ b/pkg/cmd/attestation/verification/extensions_test.go @@ -25,33 +25,53 @@ func TestVerifyCertExtensions(t *testing.T) { }, } + c := EnforcementCriteria{ + Extensions: Extensions{ + SourceRepositoryOwnerURI: "https://github.com/owner", + SourceRepositoryURI: "https://github.com/owner/repo", + }, + OIDCIssuer: GitHubOIDCIssuer, + } + t.Run("VerifyCertExtensions with owner and repo", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "owner/repo", GitHubOIDCIssuer) + err := VerifyCertExtensions(results, c) require.NoError(t, err) }) t.Run("VerifyCertExtensions with owner and repo, but wrong tenant", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "owner/repo", GitHubOIDCIssuer) + expectedCriteria := c + expectedCriteria.Extensions.SourceRepositoryOwnerURI = "https://foo.ghe.com/owner" + expectedCriteria.Extensions.SourceRepositoryURI = "https://foo.ghe.com/owner/repo" + err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://foo.ghe.com/owner, got https://github.com/owner") }) t.Run("VerifyCertExtensions with owner", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "", GitHubOIDCIssuer) + expectedCriteria := c + expectedCriteria.Extensions.SourceRepositoryURI = "" + err := VerifyCertExtensions(results, expectedCriteria) require.NoError(t, err) }) t.Run("VerifyCertExtensions with wrong owner", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "wrong", "", GitHubOIDCIssuer) + expectedCriteria := c + expectedCriteria.Extensions.SourceRepositoryOwnerURI = "https://github.com/wrong" + expectedCriteria.Extensions.SourceRepositoryURI = "" + err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://github.com/wrong, got https://github.com/owner") }) t.Run("VerifyCertExtensions with wrong repo", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "wrong", GitHubOIDCIssuer) + expectedCriteria := c + expectedCriteria.Extensions.SourceRepositoryURI = "https://github.com/owner/wrong" + err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected SourceRepositoryURI to be https://github.com/wrong, got https://github.com/owner/repo") }) t.Run("VerifyCertExtensions with wrong issuer", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "", "wrong") + expectedCriteria := c + expectedCriteria.OIDCIssuer = "wrong" + err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected Issuer to be wrong, got https://token.actions.githubusercontent.com") }) } @@ -73,25 +93,35 @@ func TestVerifyCertExtensionsCustomizedIssuer(t *testing.T) { }, } + c := EnforcementCriteria{ + Extensions: Extensions{ + SourceRepositoryOwnerURI: "https://github.com/owner", + SourceRepositoryURI: "https://github.com/owner/repo", + }, + OIDCIssuer: "https://token.actions.githubusercontent.com/foo-bar", + } + t.Run("VerifyCertExtensions with exact issuer match", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "owner/repo", "https://token.actions.githubusercontent.com/foo-bar") + err := VerifyCertExtensions(results, c) require.NoError(t, err) }) t.Run("VerifyCertExtensions with partial issuer match", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "owner/repo", "https://token.actions.githubusercontent.com") + expectedCriteria := c + expectedCriteria.OIDCIssuer = "https://token.actions.githubusercontent.com" + err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected Issuer to be https://token.actions.githubusercontent.com, got https://token.actions.githubusercontent.com/foo-bar -- if you have a custom OIDC issuer") }) t.Run("VerifyCertExtensions with wrong issuer", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "", "wrong") + expectedCriteria := c + expectedCriteria.OIDCIssuer = "wrong" + err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected Issuer to be wrong, got https://token.actions.githubusercontent.com/foo-bar") }) } func TestVerifyTenancyCertExtensions(t *testing.T) { - defaultIssuer := GitHubOIDCIssuer - results := []*AttestationProcessingResult{ { VerificationResult: &verify.VerificationResult{ @@ -108,43 +138,67 @@ func TestVerifyTenancyCertExtensions(t *testing.T) { }, } + c := EnforcementCriteria{ + Extensions: Extensions{ + SourceRepositoryOwnerURI: "https://foo.ghe.com/owner", + SourceRepositoryURI: "https://foo.ghe.com/owner/repo", + }, + OIDCIssuer: GitHubOIDCIssuer, + } + t.Run("VerifyCertExtensions with owner and repo", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "owner/repo", defaultIssuer) + err := VerifyCertExtensions(results, c) require.NoError(t, err) }) t.Run("VerifyCertExtensions with owner and repo, no tenant", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "owner/repo", defaultIssuer) + expectedCriteria := c + expectedCriteria.Extensions.SourceRepositoryOwnerURI = "https://github.com/owner" + expectedCriteria.Extensions.SourceRepositoryURI = "https://github.com/owner/repo" + err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://github.com/owner, got https://foo.ghe.com/owner") }) t.Run("VerifyCertExtensions with owner and repo, wrong tenant", func(t *testing.T) { - err := VerifyCertExtensions(results, "bar", "owner", "owner/repo", defaultIssuer) + expectedCriteria := c + expectedCriteria.Extensions.SourceRepositoryOwnerURI = "https://bar.ghe.com/owner" + expectedCriteria.Extensions.SourceRepositoryURI = "https://bar.ghe.com/owner/repo" + err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://bar.ghe.com/owner, got https://foo.ghe.com/owner") }) t.Run("VerifyCertExtensions with owner", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "", defaultIssuer) + expectedCriteria := c + expectedCriteria.Extensions.SourceRepositoryURI = "" + err := VerifyCertExtensions(results, expectedCriteria) require.NoError(t, err) }) t.Run("VerifyCertExtensions with wrong owner", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "wrong", "", defaultIssuer) + expectedCriteria := c + expectedCriteria.Extensions.SourceRepositoryOwnerURI = "https://foo.ghe.com/wrong" + err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://foo.ghe.com/wrong, got https://foo.ghe.com/owner") }) t.Run("VerifyCertExtensions with wrong repo", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "wrong", defaultIssuer) + expectedCriteria := c + expectedCriteria.Extensions.SourceRepositoryURI = "https://foo.ghe.com/owner/wrong" + err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected SourceRepositoryURI to be https://foo.ghe.com/wrong, got https://foo.ghe.com/owner/repo") }) t.Run("VerifyCertExtensions with correct, non-default issuer", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "owner/repo", "https://token.actions.foo.ghe.com") + expectedCriteria := c + expectedCriteria.OIDCIssuer = "https://token.actions.foo.ghe.com" + err := VerifyCertExtensions(results, expectedCriteria) require.NoError(t, err) }) t.Run("VerifyCertExtensions with wrong issuer", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "owner/repo", "wrong") + expectedCriteria := c + expectedCriteria.OIDCIssuer = "wrong" + err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected Issuer to be wrong, got https://token.actions.foo.ghe.com") }) } From 8b02c43085f78d966e24c9f6161df2c47072a2de Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 30 Oct 2024 16:05:39 -0600 Subject: [PATCH 10/33] add tests for newEnforcementCriteria Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 4 +- pkg/cmd/attestation/verify/policy_test.go | 159 ++++++++++++++++++++-- 2 files changed, 148 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index e2c357a0f..ff8a575af 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -83,8 +83,8 @@ func newEnforcementCriteria(opts *Options, a artifact.DigestedArtifact) (verific c.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) } - // if issuer is anything other than the default, use the user-provided value; - // otherwise, select the appropriate default based on the tenant + // if tenant is provided, select the appropriate default based on the tenant + // otherwise, use the provided OIDCIssuer if opts.Tenant != "" { c.OIDCIssuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant) } else { diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go index d50592ba2..9ef34e440 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -10,24 +10,157 @@ import ( "github.com/stretchr/testify/require" ) -// This tests that a policy can be built from a valid artifact -// Note that policy use is tested in verify_test.go in this package -func TestBuildPolicy(t *testing.T) { - ociClient := oci.MockClient{} +func TestNewEnforcementCriteria(t *testing.T) { artifactPath := "../test/data/sigstore-js-2.1.0.tgz" - digestAlg := "sha256" - artifact, err := artifact.NewDigestedArtifact(ociClient, artifactPath, digestAlg) + artifact, err := artifact.NewDigestedArtifact(oci.MockClient{}, artifactPath, "sha256") require.NoError(t, err) - opts := &Options{ - ArtifactPath: artifactPath, - Owner: "sigstore", - SANRegex: "^https://github.com/sigstore/", - } + t.Run("sets SANRegex using SignerRepo", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "bar", + SignerRepo: "foo/bar", + } - _, err = newEnforcementCriteria(opts, *artifact) - require.NoError(t, err) + c, err := newEnforcementCriteria(opts, *artifact) + require.NoError(t, err) + require.Equal(t, "^https://github.com/foo/bar", c.Extensions.SANRegex) + require.Zero(t, c.Extensions.SAN) + }) + + t.Run("sets SANRegex using SignerWorkflow", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "bar", + SignerWorkflow: "foo/bar/.github/workflows/attest.yml", + } + + c, err := newEnforcementCriteria(opts, *artifact) + require.NoError(t, err) + require.Equal(t, "^https://github.com/foo/bar/.github/workflows/attest.yml", c.Extensions.SANRegex) + require.Zero(t, c.Extensions.SAN) + }) + + t.Run("sets SANRegex and SAN using SANRegex and SAN", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "bar", + SAN: "https://github/foo/bar/.github/workflows/attest.yml", + SANRegex: "^https://github/foo", + } + + c, err := newEnforcementCriteria(opts, *artifact) + require.NoError(t, err) + require.Equal(t, "https://github/foo/bar/.github/workflows/attest.yml", c.Extensions.SANRegex) + require.Equal(t, "^https://github/foo", c.Extensions.SAN) + }) + + t.Run("sets Extensions.RunnerEnvironment to GitHubRunner value if opts.DenySelfHostedRunner is true", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "bar", + DenySelfHostedRunner: true, + } + + c, err := newEnforcementCriteria(opts, *artifact) + require.NoError(t, err) + require.Equal(t, GitHubRunner, c.Extensions.RunnerEnvironment) + }) + + t.Run("sets Extensions.RunnerEnvironment to * value if opts.DenySelfHostedRunner is false", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "bar", + DenySelfHostedRunner: false, + } + + c, err := newEnforcementCriteria(opts, *artifact) + require.NoError(t, err) + require.Equal(t, "*", c.Extensions.RunnerEnvironment) + }) + + t.Run("sets Extensions.BuildSourceRepoURI using opts.Repo and opts.Tenant", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "bar", + Tenant: "baz", + } + + c, err := newEnforcementCriteria(opts, *artifact) + require.NoError(t, err) + require.Equal(t, "https://baz.ghe.com/foo/bar", c.Extensions.BuildSourceRepoURI) + }) + + t.Run("sets Extensions.BuildSourceRepoURI using opts.Repo", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "bar", + } + + c, err := newEnforcementCriteria(opts, *artifact) + require.NoError(t, err) + require.Equal(t, "https://github.com/foo/bar", c.Extensions.BuildSourceRepoURI) + }) + + t.Run("sets Extensions.SourceRepositoryOwnerURI using opts.Owner and opts.Tenant", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "bar", + Tenant: "baz", + } + + c, err := newEnforcementCriteria(opts, *artifact) + require.NoError(t, err) + require.Equal(t, "https://baz.ghe.com/foo", c.Extensions.SourceRepositoryOwnerURI) + }) + + t.Run("sets Extensions.SourceRepositoryOwnerURI using opts.Owner", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "bar", + } + + c, err := newEnforcementCriteria(opts, *artifact) + require.NoError(t, err) + require.Equal(t, "https://github.com/foo", c.Extensions.SourceRepositoryOwnerURI) + }) + + t.Run("sets OIDCIssuer using opts.OIDCIssuer and opts.Tenant", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "bar", + Tenant: "baz", + OIDCIssuer: "https://foo.com", + } + + c, err := newEnforcementCriteria(opts, *artifact) + require.NoError(t, err) + require.Equal(t, "https://token.actions.baz.ghe.com", c.OIDCIssuer) + }) + + t.Run("sets OIDCIssuer using opts.OIDCIssuer", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "bar", + OIDCIssuer: "https://foo.com", + } + + c, err := newEnforcementCriteria(opts, *artifact) + require.NoError(t, err) + require.Equal(t, "https://foo.com", c.OIDCIssuer) + }) } func TestValidateSignerWorkflow(t *testing.T) { From 84c823c55fec23d4d306f43cd198d880840e90e6 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 30 Oct 2024 16:12:57 -0600 Subject: [PATCH 11/33] clean up extension verification tests Signed-off-by: Meredith Lancaster --- .../verification/extensions_test.go | 152 +----------------- 1 file changed, 8 insertions(+), 144 deletions(-) diff --git a/pkg/cmd/attestation/verification/extensions_test.go b/pkg/cmd/attestation/verification/extensions_test.go index e1a1555c5..734ee1943 100644 --- a/pkg/cmd/attestation/verification/extensions_test.go +++ b/pkg/cmd/attestation/verification/extensions_test.go @@ -33,172 +33,36 @@ func TestVerifyCertExtensions(t *testing.T) { OIDCIssuer: GitHubOIDCIssuer, } - t.Run("VerifyCertExtensions with owner and repo", func(t *testing.T) { + t.Run("success", func(t *testing.T) { err := VerifyCertExtensions(results, c) require.NoError(t, err) }) - t.Run("VerifyCertExtensions with owner and repo, but wrong tenant", func(t *testing.T) { - expectedCriteria := c - expectedCriteria.Extensions.SourceRepositoryOwnerURI = "https://foo.ghe.com/owner" - expectedCriteria.Extensions.SourceRepositoryURI = "https://foo.ghe.com/owner/repo" - err := VerifyCertExtensions(results, expectedCriteria) - require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://foo.ghe.com/owner, got https://github.com/owner") - }) - - t.Run("VerifyCertExtensions with owner", func(t *testing.T) { - expectedCriteria := c - expectedCriteria.Extensions.SourceRepositoryURI = "" - err := VerifyCertExtensions(results, expectedCriteria) - require.NoError(t, err) - }) - - t.Run("VerifyCertExtensions with wrong owner", func(t *testing.T) { + t.Run("with wrong SourceRepositoryOwnerURI", func(t *testing.T) { expectedCriteria := c expectedCriteria.Extensions.SourceRepositoryOwnerURI = "https://github.com/wrong" - expectedCriteria.Extensions.SourceRepositoryURI = "" err := VerifyCertExtensions(results, expectedCriteria) - require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://github.com/wrong, got https://github.com/owner") + require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://github.com/owner, got https://github.com/wrong") }) - t.Run("VerifyCertExtensions with wrong repo", func(t *testing.T) { + t.Run("with wrong SourceRepositoryURI", func(t *testing.T) { expectedCriteria := c - expectedCriteria.Extensions.SourceRepositoryURI = "https://github.com/owner/wrong" + expectedCriteria.Extensions.SourceRepositoryURI = "https://github.com/foo/wrong" err := VerifyCertExtensions(results, expectedCriteria) - require.ErrorContains(t, err, "expected SourceRepositoryURI to be https://github.com/wrong, got https://github.com/owner/repo") + require.ErrorContains(t, err, "expected SourceRepositoryURI to be https://github.com/owner/wrong, got https://github.com/wrong/bar") }) - t.Run("VerifyCertExtensions with wrong issuer", func(t *testing.T) { + t.Run("with wrong OIDCIssuer", func(t *testing.T) { expectedCriteria := c expectedCriteria.OIDCIssuer = "wrong" err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected Issuer to be wrong, got https://token.actions.githubusercontent.com") }) -} -func TestVerifyCertExtensionsCustomizedIssuer(t *testing.T) { - results := []*AttestationProcessingResult{ - { - VerificationResult: &verify.VerificationResult{ - Signature: &verify.SignatureVerificationResult{ - Certificate: &certificate.Summary{ - Extensions: certificate.Extensions{ - SourceRepositoryOwnerURI: "https://github.com/owner", - SourceRepositoryURI: "https://github.com/owner/repo", - Issuer: "https://token.actions.githubusercontent.com/foo-bar", - }, - }, - }, - }, - }, - } - - c := EnforcementCriteria{ - Extensions: Extensions{ - SourceRepositoryOwnerURI: "https://github.com/owner", - SourceRepositoryURI: "https://github.com/owner/repo", - }, - OIDCIssuer: "https://token.actions.githubusercontent.com/foo-bar", - } - - t.Run("VerifyCertExtensions with exact issuer match", func(t *testing.T) { - err := VerifyCertExtensions(results, c) - require.NoError(t, err) - }) - - t.Run("VerifyCertExtensions with partial issuer match", func(t *testing.T) { + t.Run("with partial OIDCIssuer match", func(t *testing.T) { expectedCriteria := c expectedCriteria.OIDCIssuer = "https://token.actions.githubusercontent.com" err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected Issuer to be https://token.actions.githubusercontent.com, got https://token.actions.githubusercontent.com/foo-bar -- if you have a custom OIDC issuer") }) - - t.Run("VerifyCertExtensions with wrong issuer", func(t *testing.T) { - expectedCriteria := c - expectedCriteria.OIDCIssuer = "wrong" - err := VerifyCertExtensions(results, expectedCriteria) - require.ErrorContains(t, err, "expected Issuer to be wrong, got https://token.actions.githubusercontent.com/foo-bar") - }) -} - -func TestVerifyTenancyCertExtensions(t *testing.T) { - results := []*AttestationProcessingResult{ - { - VerificationResult: &verify.VerificationResult{ - Signature: &verify.SignatureVerificationResult{ - Certificate: &certificate.Summary{ - Extensions: certificate.Extensions{ - SourceRepositoryOwnerURI: "https://foo.ghe.com/owner", - SourceRepositoryURI: "https://foo.ghe.com/owner/repo", - Issuer: "https://token.actions.foo.ghe.com", - }, - }, - }, - }, - }, - } - - c := EnforcementCriteria{ - Extensions: Extensions{ - SourceRepositoryOwnerURI: "https://foo.ghe.com/owner", - SourceRepositoryURI: "https://foo.ghe.com/owner/repo", - }, - OIDCIssuer: GitHubOIDCIssuer, - } - - t.Run("VerifyCertExtensions with owner and repo", func(t *testing.T) { - err := VerifyCertExtensions(results, c) - require.NoError(t, err) - }) - - t.Run("VerifyCertExtensions with owner and repo, no tenant", func(t *testing.T) { - expectedCriteria := c - expectedCriteria.Extensions.SourceRepositoryOwnerURI = "https://github.com/owner" - expectedCriteria.Extensions.SourceRepositoryURI = "https://github.com/owner/repo" - err := VerifyCertExtensions(results, expectedCriteria) - require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://github.com/owner, got https://foo.ghe.com/owner") - }) - - t.Run("VerifyCertExtensions with owner and repo, wrong tenant", func(t *testing.T) { - expectedCriteria := c - expectedCriteria.Extensions.SourceRepositoryOwnerURI = "https://bar.ghe.com/owner" - expectedCriteria.Extensions.SourceRepositoryURI = "https://bar.ghe.com/owner/repo" - err := VerifyCertExtensions(results, expectedCriteria) - require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://bar.ghe.com/owner, got https://foo.ghe.com/owner") - }) - - t.Run("VerifyCertExtensions with owner", func(t *testing.T) { - expectedCriteria := c - expectedCriteria.Extensions.SourceRepositoryURI = "" - err := VerifyCertExtensions(results, expectedCriteria) - require.NoError(t, err) - }) - - t.Run("VerifyCertExtensions with wrong owner", func(t *testing.T) { - expectedCriteria := c - expectedCriteria.Extensions.SourceRepositoryOwnerURI = "https://foo.ghe.com/wrong" - err := VerifyCertExtensions(results, expectedCriteria) - require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://foo.ghe.com/wrong, got https://foo.ghe.com/owner") - }) - - t.Run("VerifyCertExtensions with wrong repo", func(t *testing.T) { - expectedCriteria := c - expectedCriteria.Extensions.SourceRepositoryURI = "https://foo.ghe.com/owner/wrong" - err := VerifyCertExtensions(results, expectedCriteria) - require.ErrorContains(t, err, "expected SourceRepositoryURI to be https://foo.ghe.com/wrong, got https://foo.ghe.com/owner/repo") - }) - - t.Run("VerifyCertExtensions with correct, non-default issuer", func(t *testing.T) { - expectedCriteria := c - expectedCriteria.OIDCIssuer = "https://token.actions.foo.ghe.com" - err := VerifyCertExtensions(results, expectedCriteria) - require.NoError(t, err) - }) - - t.Run("VerifyCertExtensions with wrong issuer", func(t *testing.T) { - expectedCriteria := c - expectedCriteria.OIDCIssuer = "wrong" - err := VerifyCertExtensions(results, expectedCriteria) - require.ErrorContains(t, err, "expected Issuer to be wrong, got https://token.actions.foo.ghe.com") - }) } From 318bd9035604030f1e6941cf388231d2f1c71827 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 30 Oct 2024 16:21:15 -0600 Subject: [PATCH 12/33] update extensions tests Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verification/extensions_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/attestation/verification/extensions_test.go b/pkg/cmd/attestation/verification/extensions_test.go index 734ee1943..e01c14a6f 100644 --- a/pkg/cmd/attestation/verification/extensions_test.go +++ b/pkg/cmd/attestation/verification/extensions_test.go @@ -42,14 +42,14 @@ func TestVerifyCertExtensions(t *testing.T) { expectedCriteria := c expectedCriteria.Extensions.SourceRepositoryOwnerURI = "https://github.com/wrong" err := VerifyCertExtensions(results, expectedCriteria) - require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://github.com/owner, got https://github.com/wrong") + require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://github.com/wrong, got https://github.com/owner") }) t.Run("with wrong SourceRepositoryURI", func(t *testing.T) { expectedCriteria := c expectedCriteria.Extensions.SourceRepositoryURI = "https://github.com/foo/wrong" err := VerifyCertExtensions(results, expectedCriteria) - require.ErrorContains(t, err, "expected SourceRepositoryURI to be https://github.com/owner/wrong, got https://github.com/wrong/bar") + require.ErrorContains(t, err, "expected SourceRepositoryURI to be https://github.com/foo/wrong, got https://github.com/foo/bar") }) t.Run("with wrong OIDCIssuer", func(t *testing.T) { @@ -60,9 +60,9 @@ func TestVerifyCertExtensions(t *testing.T) { }) t.Run("with partial OIDCIssuer match", func(t *testing.T) { - expectedCriteria := c - expectedCriteria.OIDCIssuer = "https://token.actions.githubusercontent.com" - err := VerifyCertExtensions(results, expectedCriteria) + expectedResults := results + expectedResults[0].VerificationResult.Signature.Certificate.Extensions.Issuer = "https://token.actions.githubusercontent.com/foo-bar" + err := VerifyCertExtensions(expectedResults, c) require.ErrorContains(t, err, "expected Issuer to be https://token.actions.githubusercontent.com, got https://token.actions.githubusercontent.com/foo-bar -- if you have a custom OIDC issuer") }) } From bb0dcd9db4112f31cecee836af4d236f9208a58b Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 30 Oct 2024 17:19:15 -0600 Subject: [PATCH 13/33] fix wrong field settings Signed-off-by: Meredith Lancaster --- .../verification/extensions_test.go | 2 +- pkg/cmd/attestation/verification/policy.go | 1 - pkg/cmd/attestation/verify/policy.go | 5 ++- pkg/cmd/attestation/verify/policy_test.go | 41 ++++++++++--------- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/pkg/cmd/attestation/verification/extensions_test.go b/pkg/cmd/attestation/verification/extensions_test.go index e01c14a6f..f34cc8304 100644 --- a/pkg/cmd/attestation/verification/extensions_test.go +++ b/pkg/cmd/attestation/verification/extensions_test.go @@ -49,7 +49,7 @@ func TestVerifyCertExtensions(t *testing.T) { expectedCriteria := c expectedCriteria.Extensions.SourceRepositoryURI = "https://github.com/foo/wrong" err := VerifyCertExtensions(results, expectedCriteria) - require.ErrorContains(t, err, "expected SourceRepositoryURI to be https://github.com/foo/wrong, got https://github.com/foo/bar") + require.ErrorContains(t, err, "expected SourceRepositoryURI to be https://github.com/foo/wrong, got https://github.com/owner/repo") }) t.Run("with wrong OIDCIssuer", func(t *testing.T) { diff --git a/pkg/cmd/attestation/verification/policy.go b/pkg/cmd/attestation/verification/policy.go index 21210e730..974eae4e2 100644 --- a/pkg/cmd/attestation/verification/policy.go +++ b/pkg/cmd/attestation/verification/policy.go @@ -23,7 +23,6 @@ type Extensions struct { RunnerEnvironment string SANRegex string SAN string - BuildSourceRepoURI string SignerWorkflow string SourceRepositoryOwnerURI string SourceRepositoryURI string diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index ff8a575af..789b7dfb8 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -72,9 +72,10 @@ func newEnforcementCriteria(opts *Options, a artifact.DigestedArtifact) (verific if opts.Repo != "" { if opts.Tenant != "" { - c.Extensions.BuildSourceRepoURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo) + c.Extensions.SourceRepositoryURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo) + } else { + c.Extensions.SourceRepositoryURI = fmt.Sprintf("https://github.com/%s", opts.Repo) } - c.Extensions.BuildSourceRepoURI = fmt.Sprintf("https://github.com/%s", opts.Repo) } if opts.Tenant != "" { diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go index 9ef34e440..daadcf346 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -20,22 +20,23 @@ func TestNewEnforcementCriteria(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, Owner: "foo", - Repo: "bar", + Repo: "foo/bar", SignerRepo: "foo/bar", } c, err := newEnforcementCriteria(opts, *artifact) require.NoError(t, err) - require.Equal(t, "^https://github.com/foo/bar", c.Extensions.SANRegex) + require.Equal(t, "(?i)^https://github.com/foo/bar/", c.Extensions.SANRegex) require.Zero(t, c.Extensions.SAN) }) - t.Run("sets SANRegex using SignerWorkflow", func(t *testing.T) { + t.Run("sets SANRegex using SignerWorkflow matching host regex", func(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, Owner: "foo", - Repo: "bar", + Repo: "foo/bar", SignerWorkflow: "foo/bar/.github/workflows/attest.yml", + Hostname: "github.com", } c, err := newEnforcementCriteria(opts, *artifact) @@ -48,22 +49,22 @@ func TestNewEnforcementCriteria(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, Owner: "foo", - Repo: "bar", + Repo: "foo/bar", SAN: "https://github/foo/bar/.github/workflows/attest.yml", - SANRegex: "^https://github/foo", + SANRegex: "(?i)^https://github/foo", } c, err := newEnforcementCriteria(opts, *artifact) require.NoError(t, err) - require.Equal(t, "https://github/foo/bar/.github/workflows/attest.yml", c.Extensions.SANRegex) - require.Equal(t, "^https://github/foo", c.Extensions.SAN) + require.Equal(t, "https://github/foo/bar/.github/workflows/attest.yml", c.Extensions.SAN) + require.Equal(t, "(?i)^https://github/foo", c.Extensions.SANRegex) }) t.Run("sets Extensions.RunnerEnvironment to GitHubRunner value if opts.DenySelfHostedRunner is true", func(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, Owner: "foo", - Repo: "bar", + Repo: "foo/bar", DenySelfHostedRunner: true, } @@ -76,7 +77,7 @@ func TestNewEnforcementCriteria(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, Owner: "foo", - Repo: "bar", + Repo: "foo/bar", DenySelfHostedRunner: false, } @@ -85,36 +86,36 @@ func TestNewEnforcementCriteria(t *testing.T) { require.Equal(t, "*", c.Extensions.RunnerEnvironment) }) - t.Run("sets Extensions.BuildSourceRepoURI using opts.Repo and opts.Tenant", func(t *testing.T) { + t.Run("sets Extensions.SourceRepositoryURI using opts.Repo and opts.Tenant", func(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, Owner: "foo", - Repo: "bar", + Repo: "foo/bar", Tenant: "baz", } c, err := newEnforcementCriteria(opts, *artifact) require.NoError(t, err) - require.Equal(t, "https://baz.ghe.com/foo/bar", c.Extensions.BuildSourceRepoURI) + require.Equal(t, "https://baz.ghe.com/foo/bar", c.Extensions.SourceRepositoryURI) }) - t.Run("sets Extensions.BuildSourceRepoURI using opts.Repo", func(t *testing.T) { + t.Run("sets Extensions.SourceRepositoryURI using opts.Repo", func(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, Owner: "foo", - Repo: "bar", + Repo: "foo/bar", } c, err := newEnforcementCriteria(opts, *artifact) require.NoError(t, err) - require.Equal(t, "https://github.com/foo/bar", c.Extensions.BuildSourceRepoURI) + require.Equal(t, "https://github.com/foo/bar", c.Extensions.SourceRepositoryURI) }) t.Run("sets Extensions.SourceRepositoryOwnerURI using opts.Owner and opts.Tenant", func(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, Owner: "foo", - Repo: "bar", + Repo: "foo/bar", Tenant: "baz", } @@ -127,7 +128,7 @@ func TestNewEnforcementCriteria(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, Owner: "foo", - Repo: "bar", + Repo: "foo/bar", } c, err := newEnforcementCriteria(opts, *artifact) @@ -139,7 +140,7 @@ func TestNewEnforcementCriteria(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, Owner: "foo", - Repo: "bar", + Repo: "foo/bar", Tenant: "baz", OIDCIssuer: "https://foo.com", } @@ -153,7 +154,7 @@ func TestNewEnforcementCriteria(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, Owner: "foo", - Repo: "bar", + Repo: "foo/bar", OIDCIssuer: "https://foo.com", } From 61b60e9430e9e704e87db3793c9a68f0755fcacf Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 31 Oct 2024 08:19:33 -0600 Subject: [PATCH 14/33] fix runner setting Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 2 -- pkg/cmd/attestation/verify/policy_test.go | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index 789b7dfb8..d52497664 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -66,8 +66,6 @@ func newEnforcementCriteria(opts *Options, a artifact.DigestedArtifact) (verific if opts.DenySelfHostedRunner { c.Extensions.RunnerEnvironment = GitHubRunner - } else { - c.Extensions.RunnerEnvironment = "*" } if opts.Repo != "" { diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go index daadcf346..f194edb74 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -83,7 +83,7 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts, *artifact) require.NoError(t, err) - require.Equal(t, "*", c.Extensions.RunnerEnvironment) + require.Zero(t, c.Extensions.RunnerEnvironment) }) t.Run("sets Extensions.SourceRepositoryURI using opts.Repo and opts.Tenant", func(t *testing.T) { From 9cdeb31fc684dcae1caa001b5f4d8110ac365cad Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 31 Oct 2024 08:32:35 -0600 Subject: [PATCH 15/33] reorganize funcs Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 38 ++++++++++++++-------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index d52497664..b79ebacf0 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -25,25 +25,6 @@ func expandToGitHubURL(tenant, ownerOrRepo string) string { return fmt.Sprintf("(?i)^https://%s.ghe.com/%s/", tenant, ownerOrRepo) } -func validateSignerWorkflow(opts *Options) (string, error) { - // we expect a provided workflow argument be in the format [HOST/]///path/to/workflow.yml - // if the provided workflow does not contain a host, set the host - match, err := regexp.MatchString(hostRegex, opts.SignerWorkflow) - if err != nil { - return "", err - } - - if match { - return fmt.Sprintf("^https://%s", opts.SignerWorkflow), nil - } - - if opts.Hostname == "" { - return "", errors.New("unknown host") - } - - return fmt.Sprintf("^https://%s/%s", opts.Hostname, opts.SignerWorkflow), nil -} - func newEnforcementCriteria(opts *Options, a artifact.DigestedArtifact) (verification.EnforcementCriteria, error) { c := verification.EnforcementCriteria{ Artifact: a, @@ -131,3 +112,22 @@ func SigstorePolicy(c verification.EnforcementCriteria) (verify.PolicyBuilder, e policy := verify.NewPolicy(artifactDigestPolicyOption, certIdOption) return policy, nil } + +func validateSignerWorkflow(opts *Options) (string, error) { + // we expect a provided workflow argument be in the format [HOST/]///path/to/workflow.yml + // if the provided workflow does not contain a host, set the host + match, err := regexp.MatchString(hostRegex, opts.SignerWorkflow) + if err != nil { + return "", err + } + + if match { + return fmt.Sprintf("^https://%s", opts.SignerWorkflow), nil + } + + if opts.Hostname == "" { + return "", errors.New("unknown host") + } + + return fmt.Sprintf("^https://%s/%s", opts.Hostname, opts.SignerWorkflow), nil +} From 6f4b5ddc40e045e3869bf79c0f0f6e5d4aedb1f7 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 31 Oct 2024 16:07:25 -0600 Subject: [PATCH 16/33] remove artifact from EnforcementCriteria Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verification/policy.go | 1 - pkg/cmd/attestation/verify/policy.go | 10 ++++---- pkg/cmd/attestation/verify/policy_test.go | 27 +++++++++------------- pkg/cmd/attestation/verify/verify.go | 4 ++-- 4 files changed, 17 insertions(+), 25 deletions(-) diff --git a/pkg/cmd/attestation/verification/policy.go b/pkg/cmd/attestation/verification/policy.go index 974eae4e2..fabdbc0aa 100644 --- a/pkg/cmd/attestation/verification/policy.go +++ b/pkg/cmd/attestation/verification/policy.go @@ -31,6 +31,5 @@ type Extensions struct { type EnforcementCriteria struct { Extensions Extensions PredicateType string - Artifact artifact.DigestedArtifact OIDCIssuer string } diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index b79ebacf0..38931dfc1 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -25,10 +25,8 @@ func expandToGitHubURL(tenant, ownerOrRepo string) string { return fmt.Sprintf("(?i)^https://%s.ghe.com/%s/", tenant, ownerOrRepo) } -func newEnforcementCriteria(opts *Options, a artifact.DigestedArtifact) (verification.EnforcementCriteria, error) { - c := verification.EnforcementCriteria{ - Artifact: a, - } +func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, error) { + c := verification.EnforcementCriteria{} if opts.SignerRepo != "" { signedRepoRegex := expandToGitHubURL(opts.Tenant, opts.SignerRepo) @@ -98,8 +96,8 @@ func buildCertificateIdentityOption(c verification.EnforcementCriteria) (verify. return verify.WithCertificateIdentity(certId), nil } -func SigstorePolicy(c verification.EnforcementCriteria) (verify.PolicyBuilder, error) { - artifactDigestPolicyOption, err := verification.BuildDigestPolicyOption(c.Artifact) +func SigstorePolicy(c verification.EnforcementCriteria, a artifact.DigestedArtifact) (verify.PolicyBuilder, error) { + artifactDigestPolicyOption, err := verification.BuildDigestPolicyOption(a) if err != nil { return verify.PolicyBuilder{}, err } diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go index f194edb74..13a7d2822 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -3,8 +3,6 @@ package verify import ( "testing" - "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" - "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" "github.com/cli/cli/v2/pkg/cmd/factory" "github.com/stretchr/testify/require" @@ -13,9 +11,6 @@ import ( func TestNewEnforcementCriteria(t *testing.T) { artifactPath := "../test/data/sigstore-js-2.1.0.tgz" - artifact, err := artifact.NewDigestedArtifact(oci.MockClient{}, artifactPath, "sha256") - require.NoError(t, err) - t.Run("sets SANRegex using SignerRepo", func(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, @@ -24,7 +19,7 @@ func TestNewEnforcementCriteria(t *testing.T) { SignerRepo: "foo/bar", } - c, err := newEnforcementCriteria(opts, *artifact) + c, err := newEnforcementCriteria(opts) require.NoError(t, err) require.Equal(t, "(?i)^https://github.com/foo/bar/", c.Extensions.SANRegex) require.Zero(t, c.Extensions.SAN) @@ -39,7 +34,7 @@ func TestNewEnforcementCriteria(t *testing.T) { Hostname: "github.com", } - c, err := newEnforcementCriteria(opts, *artifact) + c, err := newEnforcementCriteria(opts) require.NoError(t, err) require.Equal(t, "^https://github.com/foo/bar/.github/workflows/attest.yml", c.Extensions.SANRegex) require.Zero(t, c.Extensions.SAN) @@ -54,7 +49,7 @@ func TestNewEnforcementCriteria(t *testing.T) { SANRegex: "(?i)^https://github/foo", } - c, err := newEnforcementCriteria(opts, *artifact) + c, err := newEnforcementCriteria(opts) require.NoError(t, err) require.Equal(t, "https://github/foo/bar/.github/workflows/attest.yml", c.Extensions.SAN) require.Equal(t, "(?i)^https://github/foo", c.Extensions.SANRegex) @@ -68,7 +63,7 @@ func TestNewEnforcementCriteria(t *testing.T) { DenySelfHostedRunner: true, } - c, err := newEnforcementCriteria(opts, *artifact) + c, err := newEnforcementCriteria(opts) require.NoError(t, err) require.Equal(t, GitHubRunner, c.Extensions.RunnerEnvironment) }) @@ -81,7 +76,7 @@ func TestNewEnforcementCriteria(t *testing.T) { DenySelfHostedRunner: false, } - c, err := newEnforcementCriteria(opts, *artifact) + c, err := newEnforcementCriteria(opts) require.NoError(t, err) require.Zero(t, c.Extensions.RunnerEnvironment) }) @@ -94,7 +89,7 @@ func TestNewEnforcementCriteria(t *testing.T) { Tenant: "baz", } - c, err := newEnforcementCriteria(opts, *artifact) + c, err := newEnforcementCriteria(opts) require.NoError(t, err) require.Equal(t, "https://baz.ghe.com/foo/bar", c.Extensions.SourceRepositoryURI) }) @@ -106,7 +101,7 @@ func TestNewEnforcementCriteria(t *testing.T) { Repo: "foo/bar", } - c, err := newEnforcementCriteria(opts, *artifact) + c, err := newEnforcementCriteria(opts) require.NoError(t, err) require.Equal(t, "https://github.com/foo/bar", c.Extensions.SourceRepositoryURI) }) @@ -119,7 +114,7 @@ func TestNewEnforcementCriteria(t *testing.T) { Tenant: "baz", } - c, err := newEnforcementCriteria(opts, *artifact) + c, err := newEnforcementCriteria(opts) require.NoError(t, err) require.Equal(t, "https://baz.ghe.com/foo", c.Extensions.SourceRepositoryOwnerURI) }) @@ -131,7 +126,7 @@ func TestNewEnforcementCriteria(t *testing.T) { Repo: "foo/bar", } - c, err := newEnforcementCriteria(opts, *artifact) + c, err := newEnforcementCriteria(opts) require.NoError(t, err) require.Equal(t, "https://github.com/foo", c.Extensions.SourceRepositoryOwnerURI) }) @@ -145,7 +140,7 @@ func TestNewEnforcementCriteria(t *testing.T) { OIDCIssuer: "https://foo.com", } - c, err := newEnforcementCriteria(opts, *artifact) + c, err := newEnforcementCriteria(opts) require.NoError(t, err) require.Equal(t, "https://token.actions.baz.ghe.com", c.OIDCIssuer) }) @@ -158,7 +153,7 @@ func TestNewEnforcementCriteria(t *testing.T) { OIDCIssuer: "https://foo.com", } - c, err := newEnforcementCriteria(opts, *artifact) + c, err := newEnforcementCriteria(opts) require.NoError(t, err) require.Equal(t, "https://foo.com", c.OIDCIssuer) }) diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 3c95b17b7..3771cc315 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -258,13 +258,13 @@ func runVerify(opts *Options) error { opts.Logger.VerbosePrintf("Verifying attestations with predicate type: %s\n", opts.PredicateType) - ec, err := newEnforcementCriteria(opts, *artifact) + ec, err := newEnforcementCriteria(opts) if err != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy")) return err } - sp, err := SigstorePolicy(ec) + sp, err := SigstorePolicy(ec, *artifact) if err != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build Sigstore verification policy")) return err From 7948ce4dc9703045965ef342093efcf295811ff3 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 31 Oct 2024 16:09:08 -0600 Subject: [PATCH 17/33] rename function Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 2 +- pkg/cmd/attestation/verify/verify.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index 38931dfc1..8ee34bc7a 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -96,7 +96,7 @@ func buildCertificateIdentityOption(c verification.EnforcementCriteria) (verify. return verify.WithCertificateIdentity(certId), nil } -func SigstorePolicy(c verification.EnforcementCriteria, a artifact.DigestedArtifact) (verify.PolicyBuilder, error) { +func buildSigstoreVerifyPolicy(c verification.EnforcementCriteria, a artifact.DigestedArtifact) (verify.PolicyBuilder, error) { artifactDigestPolicyOption, err := verification.BuildDigestPolicyOption(a) if err != nil { return verify.PolicyBuilder{}, err diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 3771cc315..5f1b1d6f3 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -264,7 +264,7 @@ func runVerify(opts *Options) error { return err } - sp, err := SigstorePolicy(ec, *artifact) + sp, err := buildSigstoreVerifyPolicy(ec, *artifact) if err != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build Sigstore verification policy")) return err From e6d0a067e64e397637da8ca62628c5a5a0cb1ea9 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 31 Oct 2024 16:09:45 -0600 Subject: [PATCH 18/33] Update pkg/cmd/attestation/verification/extensions.go Co-authored-by: Phill MV --- pkg/cmd/attestation/verification/extensions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index 13c83651e..67cc48f18 100644 --- a/pkg/cmd/attestation/verification/extensions.go +++ b/pkg/cmd/attestation/verification/extensions.go @@ -31,7 +31,7 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, ec Enforcement } } -func verifyCertExtensions(attestation *AttestationProcessingResult, c EnforcementCriteria) error { +func verifyCertExtensions(verifiedCert certificate.Summary, criteria EnforcementCriteria) error { if c.Extensions.SourceRepositoryOwnerURI != "" { sourceRepositoryOwnerURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI if !strings.EqualFold(c.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { From a81cb730fcd987051dbdfe9b2665178bc051f05f Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 31 Oct 2024 16:14:28 -0600 Subject: [PATCH 19/33] update VerifyCertExtensions args Signed-off-by: Meredith Lancaster --- .../attestation/verification/extensions.go | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index 67cc48f18..b41d7f559 100644 --- a/pkg/cmd/attestation/verification/extensions.go +++ b/pkg/cmd/attestation/verification/extensions.go @@ -4,6 +4,8 @@ import ( "errors" "fmt" "strings" + + "github.com/sigstore/sigstore-go/pkg/fulcio/certificate" ) var ( @@ -18,7 +20,7 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, ec Enforcement var atLeastOneVerified bool for _, attestation := range results { - if err := verifyCertExtensions(attestation, ec); err != nil { + if err := verifyCertExtensions(*attestation.VerificationResult.Signature.Certificate, ec); err != nil { return err } atLeastOneVerified = true @@ -32,30 +34,30 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, ec Enforcement } func verifyCertExtensions(verifiedCert certificate.Summary, criteria EnforcementCriteria) error { - if c.Extensions.SourceRepositoryOwnerURI != "" { - sourceRepositoryOwnerURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI - if !strings.EqualFold(c.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { - return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", c.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) + if criteria.Extensions.SourceRepositoryOwnerURI != "" { + sourceRepositoryOwnerURI := verifiedCert.Extensions.SourceRepositoryOwnerURI + if !strings.EqualFold(criteria.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { + return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", criteria.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) } } // if repo is set, check the SourceRepositoryURI field - if c.Extensions.SourceRepositoryURI != "" { - sourceRepositoryURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryURI - if !strings.EqualFold(c.Extensions.SourceRepositoryURI, sourceRepositoryURI) { - return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", c.Extensions.SourceRepositoryURI, sourceRepositoryURI) + if criteria.Extensions.SourceRepositoryURI != "" { + sourceRepositoryURI := verifiedCert.Extensions.SourceRepositoryURI + if !strings.EqualFold(criteria.Extensions.SourceRepositoryURI, sourceRepositoryURI) { + return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", criteria.Extensions.SourceRepositoryURI, sourceRepositoryURI) } } // if issuer is anything other than the default, use the user-provided value; // otherwise, select the appropriate default based on the tenant - if c.OIDCIssuer != "" { - certIssuer := attestation.VerificationResult.Signature.Certificate.Extensions.Issuer - if !strings.EqualFold(c.OIDCIssuer, certIssuer) { - if strings.Index(certIssuer, c.OIDCIssuer+"/") == 0 { - return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", c.OIDCIssuer, certIssuer) + if criteria.OIDCIssuer != "" { + certIssuer := verifiedCert.Extensions.Issuer + if !strings.EqualFold(criteria.OIDCIssuer, certIssuer) { + if strings.Index(certIssuer, criteria.OIDCIssuer+"/") == 0 { + return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", criteria.OIDCIssuer, certIssuer) } - return fmt.Errorf("expected Issuer to be %s, got %s", c.OIDCIssuer, certIssuer) + return fmt.Errorf("expected Issuer to be %s, got %s", criteria.OIDCIssuer, certIssuer) } } From 9f3d00960c43aaee2b9be3bd106a57853043bac7 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 31 Oct 2024 16:16:09 -0600 Subject: [PATCH 20/33] keep comment Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index 8ee34bc7a..bd614263d 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -45,6 +45,11 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er if opts.DenySelfHostedRunner { c.Extensions.RunnerEnvironment = GitHubRunner + } else { + // if Extensions.RunnerEnvironment value is set to the empty string + // through the second function argument, + // no certificate matching will happen on the RunnerEnvironment field + c.Extensions.RunnerEnvironment = "" } if opts.Repo != "" { From 8336f797ad35f40c6b10c886bd3b874d90f7ed8c Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 31 Oct 2024 16:27:21 -0600 Subject: [PATCH 21/33] use sigstore-go certificate.Summary type for criteria Signed-off-by: Meredith Lancaster --- .../attestation/verification/extensions.go | 22 +++++++------- .../verification/extensions_test.go | 17 ++++++----- pkg/cmd/attestation/verification/policy.go | 15 +++------- pkg/cmd/attestation/verify/policy.go | 30 +++++++++---------- pkg/cmd/attestation/verify/policy_test.go | 28 ++++++++--------- 5 files changed, 53 insertions(+), 59 deletions(-) diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index b41d7f559..6f9945661 100644 --- a/pkg/cmd/attestation/verification/extensions.go +++ b/pkg/cmd/attestation/verification/extensions.go @@ -34,30 +34,30 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, ec Enforcement } func verifyCertExtensions(verifiedCert certificate.Summary, criteria EnforcementCriteria) error { - if criteria.Extensions.SourceRepositoryOwnerURI != "" { + if criteria.Certificate.SourceRepositoryOwnerURI != "" { sourceRepositoryOwnerURI := verifiedCert.Extensions.SourceRepositoryOwnerURI - if !strings.EqualFold(criteria.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { - return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", criteria.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) + if !strings.EqualFold(criteria.Certificate.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { + return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", criteria.Certificate.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) } } // if repo is set, check the SourceRepositoryURI field - if criteria.Extensions.SourceRepositoryURI != "" { + if criteria.Certificate.SourceRepositoryURI != "" { sourceRepositoryURI := verifiedCert.Extensions.SourceRepositoryURI - if !strings.EqualFold(criteria.Extensions.SourceRepositoryURI, sourceRepositoryURI) { - return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", criteria.Extensions.SourceRepositoryURI, sourceRepositoryURI) + if !strings.EqualFold(criteria.Certificate.SourceRepositoryURI, sourceRepositoryURI) { + return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", criteria.Certificate.SourceRepositoryURI, sourceRepositoryURI) } } // if issuer is anything other than the default, use the user-provided value; // otherwise, select the appropriate default based on the tenant - if criteria.OIDCIssuer != "" { + if criteria.Certificate.Issuer != "" { certIssuer := verifiedCert.Extensions.Issuer - if !strings.EqualFold(criteria.OIDCIssuer, certIssuer) { - if strings.Index(certIssuer, criteria.OIDCIssuer+"/") == 0 { - return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", criteria.OIDCIssuer, certIssuer) + if !strings.EqualFold(criteria.Certificate.Issuer, certIssuer) { + if strings.Index(certIssuer, criteria.Certificate.Issuer+"/") == 0 { + return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", criteria.Certificate.Issuer, certIssuer) } - return fmt.Errorf("expected Issuer to be %s, got %s", criteria.OIDCIssuer, certIssuer) + return fmt.Errorf("expected Issuer to be %s, got %s", criteria.Certificate.Issuer, certIssuer) } } diff --git a/pkg/cmd/attestation/verification/extensions_test.go b/pkg/cmd/attestation/verification/extensions_test.go index f34cc8304..9548502be 100644 --- a/pkg/cmd/attestation/verification/extensions_test.go +++ b/pkg/cmd/attestation/verification/extensions_test.go @@ -25,12 +25,13 @@ func TestVerifyCertExtensions(t *testing.T) { }, } + certSummary := certificate.Summary{} + certSummary.SourceRepositoryOwnerURI = "https://github.com/owner" + certSummary.SourceRepositoryURI = "https://github.com/owner/repo" + certSummary.Issuer = GitHubOIDCIssuer + c := EnforcementCriteria{ - Extensions: Extensions{ - SourceRepositoryOwnerURI: "https://github.com/owner", - SourceRepositoryURI: "https://github.com/owner/repo", - }, - OIDCIssuer: GitHubOIDCIssuer, + Certificate: certSummary, } t.Run("success", func(t *testing.T) { @@ -40,21 +41,21 @@ func TestVerifyCertExtensions(t *testing.T) { t.Run("with wrong SourceRepositoryOwnerURI", func(t *testing.T) { expectedCriteria := c - expectedCriteria.Extensions.SourceRepositoryOwnerURI = "https://github.com/wrong" + expectedCriteria.Certificate.SourceRepositoryOwnerURI = "https://github.com/wrong" err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://github.com/wrong, got https://github.com/owner") }) t.Run("with wrong SourceRepositoryURI", func(t *testing.T) { expectedCriteria := c - expectedCriteria.Extensions.SourceRepositoryURI = "https://github.com/foo/wrong" + expectedCriteria.Certificate.SourceRepositoryURI = "https://github.com/foo/wrong" err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected SourceRepositoryURI to be https://github.com/foo/wrong, got https://github.com/owner/repo") }) t.Run("with wrong OIDCIssuer", func(t *testing.T) { expectedCriteria := c - expectedCriteria.OIDCIssuer = "wrong" + expectedCriteria.Certificate.Issuer = "wrong" err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected Issuer to be wrong, got https://token.actions.githubusercontent.com") }) diff --git a/pkg/cmd/attestation/verification/policy.go b/pkg/cmd/attestation/verification/policy.go index fabdbc0aa..9f8672628 100644 --- a/pkg/cmd/attestation/verification/policy.go +++ b/pkg/cmd/attestation/verification/policy.go @@ -5,6 +5,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" + "github.com/sigstore/sigstore-go/pkg/fulcio/certificate" "github.com/sigstore/sigstore-go/pkg/verify" ) @@ -19,17 +20,9 @@ func BuildDigestPolicyOption(a artifact.DigestedArtifact) (verify.ArtifactPolicy return verify.WithArtifactDigest(a.Algorithm(), decoded), nil } -type Extensions struct { - RunnerEnvironment string - SANRegex string - SAN string - SignerWorkflow string - SourceRepositoryOwnerURI string - SourceRepositoryURI string -} - type EnforcementCriteria struct { - Extensions Extensions + Certificate certificate.Summary PredicateType string - OIDCIssuer string + SANRegex string + SAN string } diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index bd614263d..f69c75c52 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -30,55 +30,55 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er if opts.SignerRepo != "" { signedRepoRegex := expandToGitHubURL(opts.Tenant, opts.SignerRepo) - c.Extensions.SANRegex = signedRepoRegex + c.SANRegex = signedRepoRegex } else if opts.SignerWorkflow != "" { validatedWorkflowRegex, err := validateSignerWorkflow(opts) if err != nil { return verification.EnforcementCriteria{}, err } - c.Extensions.SANRegex = validatedWorkflowRegex + c.SANRegex = validatedWorkflowRegex } else { - c.Extensions.SANRegex = opts.SANRegex - c.Extensions.SAN = opts.SAN + c.SANRegex = opts.SANRegex + c.SAN = opts.SAN } if opts.DenySelfHostedRunner { - c.Extensions.RunnerEnvironment = GitHubRunner + c.Certificate.RunnerEnvironment = GitHubRunner } else { - // if Extensions.RunnerEnvironment value is set to the empty string + // if Certificate.RunnerEnvironment value is set to the empty string // through the second function argument, // no certificate matching will happen on the RunnerEnvironment field - c.Extensions.RunnerEnvironment = "" + c.Certificate.RunnerEnvironment = "" } if opts.Repo != "" { if opts.Tenant != "" { - c.Extensions.SourceRepositoryURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo) + c.Certificate.SourceRepositoryURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo) } else { - c.Extensions.SourceRepositoryURI = fmt.Sprintf("https://github.com/%s", opts.Repo) + c.Certificate.SourceRepositoryURI = fmt.Sprintf("https://github.com/%s", opts.Repo) } } if opts.Tenant != "" { - c.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Owner) + c.Certificate.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Owner) } else { - c.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) + c.Certificate.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) } // if tenant is provided, select the appropriate default based on the tenant // otherwise, use the provided OIDCIssuer if opts.Tenant != "" { - c.OIDCIssuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant) + c.Certificate.Issuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant) } else { - c.OIDCIssuer = opts.OIDCIssuer + c.Certificate.Issuer = opts.OIDCIssuer } return c, nil } func buildCertificateIdentityOption(c verification.EnforcementCriteria) (verify.PolicyOption, error) { - sanMatcher, err := verify.NewSANMatcher(c.Extensions.SAN, c.Extensions.SANRegex) + sanMatcher, err := verify.NewSANMatcher(c.SAN, c.SANRegex) if err != nil { return nil, err } @@ -90,7 +90,7 @@ func buildCertificateIdentityOption(c verification.EnforcementCriteria) (verify. } extensions := certificate.Extensions{ - RunnerEnvironment: c.Extensions.RunnerEnvironment, + RunnerEnvironment: c.Certificate.RunnerEnvironment, } certId, err := verify.NewCertificateIdentity(sanMatcher, issuerMatcher, extensions) diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go index 13a7d2822..d4516e844 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -21,8 +21,8 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts) require.NoError(t, err) - require.Equal(t, "(?i)^https://github.com/foo/bar/", c.Extensions.SANRegex) - require.Zero(t, c.Extensions.SAN) + require.Equal(t, "(?i)^https://github.com/foo/bar/", c.SANRegex) + require.Zero(t, c.SAN) }) t.Run("sets SANRegex using SignerWorkflow matching host regex", func(t *testing.T) { @@ -36,8 +36,8 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts) require.NoError(t, err) - require.Equal(t, "^https://github.com/foo/bar/.github/workflows/attest.yml", c.Extensions.SANRegex) - require.Zero(t, c.Extensions.SAN) + require.Equal(t, "^https://github.com/foo/bar/.github/workflows/attest.yml", c.SANRegex) + require.Zero(t, c.SAN) }) t.Run("sets SANRegex and SAN using SANRegex and SAN", func(t *testing.T) { @@ -51,8 +51,8 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts) require.NoError(t, err) - require.Equal(t, "https://github/foo/bar/.github/workflows/attest.yml", c.Extensions.SAN) - require.Equal(t, "(?i)^https://github/foo", c.Extensions.SANRegex) + require.Equal(t, "https://github/foo/bar/.github/workflows/attest.yml", c.SAN) + require.Equal(t, "(?i)^https://github/foo", c.SANRegex) }) t.Run("sets Extensions.RunnerEnvironment to GitHubRunner value if opts.DenySelfHostedRunner is true", func(t *testing.T) { @@ -65,7 +65,7 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts) require.NoError(t, err) - require.Equal(t, GitHubRunner, c.Extensions.RunnerEnvironment) + require.Equal(t, GitHubRunner, c.Certificate.RunnerEnvironment) }) t.Run("sets Extensions.RunnerEnvironment to * value if opts.DenySelfHostedRunner is false", func(t *testing.T) { @@ -78,7 +78,7 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts) require.NoError(t, err) - require.Zero(t, c.Extensions.RunnerEnvironment) + require.Zero(t, c.Certificate.RunnerEnvironment) }) t.Run("sets Extensions.SourceRepositoryURI using opts.Repo and opts.Tenant", func(t *testing.T) { @@ -91,7 +91,7 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts) require.NoError(t, err) - require.Equal(t, "https://baz.ghe.com/foo/bar", c.Extensions.SourceRepositoryURI) + require.Equal(t, "https://baz.ghe.com/foo/bar", c.Certificate.SourceRepositoryURI) }) t.Run("sets Extensions.SourceRepositoryURI using opts.Repo", func(t *testing.T) { @@ -103,7 +103,7 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts) require.NoError(t, err) - require.Equal(t, "https://github.com/foo/bar", c.Extensions.SourceRepositoryURI) + require.Equal(t, "https://github.com/foo/bar", c.Certificate.SourceRepositoryURI) }) t.Run("sets Extensions.SourceRepositoryOwnerURI using opts.Owner and opts.Tenant", func(t *testing.T) { @@ -116,7 +116,7 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts) require.NoError(t, err) - require.Equal(t, "https://baz.ghe.com/foo", c.Extensions.SourceRepositoryOwnerURI) + require.Equal(t, "https://baz.ghe.com/foo", c.Certificate.SourceRepositoryOwnerURI) }) t.Run("sets Extensions.SourceRepositoryOwnerURI using opts.Owner", func(t *testing.T) { @@ -128,7 +128,7 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts) require.NoError(t, err) - require.Equal(t, "https://github.com/foo", c.Extensions.SourceRepositoryOwnerURI) + require.Equal(t, "https://github.com/foo", c.Certificate.SourceRepositoryOwnerURI) }) t.Run("sets OIDCIssuer using opts.OIDCIssuer and opts.Tenant", func(t *testing.T) { @@ -142,7 +142,7 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts) require.NoError(t, err) - require.Equal(t, "https://token.actions.baz.ghe.com", c.OIDCIssuer) + require.Equal(t, "https://token.actions.baz.ghe.com", c.Certificate.Issuer) }) t.Run("sets OIDCIssuer using opts.OIDCIssuer", func(t *testing.T) { @@ -155,7 +155,7 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts) require.NoError(t, err) - require.Equal(t, "https://foo.com", c.OIDCIssuer) + require.Equal(t, "https://foo.com", c.Certificate.Issuer) }) } From 50cda0df44f987c7c8803a4984eddcae7cdd4574 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 31 Oct 2024 16:56:49 -0600 Subject: [PATCH 22/33] add Valid method for EnforcementCriteria Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verification/policy.go | 19 +++++++++++++++++++ pkg/cmd/attestation/verify/policy.go | 7 ++++--- pkg/cmd/attestation/verify/policy_test.go | 3 ++- pkg/cmd/attestation/verify/verify.go | 17 +++++++++++------ 4 files changed, 36 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/attestation/verification/policy.go b/pkg/cmd/attestation/verification/policy.go index 9f8672628..ae21dae48 100644 --- a/pkg/cmd/attestation/verification/policy.go +++ b/pkg/cmd/attestation/verification/policy.go @@ -2,6 +2,7 @@ package verification import ( "encoding/hex" + "fmt" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" @@ -9,6 +10,8 @@ import ( "github.com/sigstore/sigstore-go/pkg/verify" ) +const GitHubRunner = "github-hosted" + // BuildDigestPolicyOption builds a verify.ArtifactPolicyOption // from the given artifact digest and digest algorithm func BuildDigestPolicyOption(a artifact.DigestedArtifact) (verify.ArtifactPolicyOption, error) { @@ -26,3 +29,19 @@ type EnforcementCriteria struct { SANRegex string SAN string } + +func (c EnforcementCriteria) Valid() error { + if c.Certificate.Issuer == "" { + return fmt.Errorf("Issuer must be set") + } + if c.Certificate.RunnerEnvironment != "" && c.Certificate.RunnerEnvironment != GitHubRunner { + return fmt.Errorf("RunnerEnvironment must be set to either \"\" or %s", GitHubRunner) + } + if c.Certificate.SourceRepositoryOwnerURI == "" { + return fmt.Errorf("SourceRepositoryOwnerURI must be set") + } + if c.PredicateType == "" { + return fmt.Errorf("PredicateType must be set") + } + return nil +} diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index f69c75c52..c68cdb452 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -14,8 +14,7 @@ import ( const ( // represents the GitHub hosted runner in the certificate RunnerEnvironment extension - GitHubRunner = "github-hosted" - hostRegex = `^[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+.*$` + hostRegex = `^[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+.*$` ) func expandToGitHubURL(tenant, ownerOrRepo string) string { @@ -44,7 +43,7 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er } if opts.DenySelfHostedRunner { - c.Certificate.RunnerEnvironment = GitHubRunner + c.Certificate.RunnerEnvironment = verification.GitHubRunner } else { // if Certificate.RunnerEnvironment value is set to the empty string // through the second function argument, @@ -74,6 +73,8 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er c.Certificate.Issuer = opts.OIDCIssuer } + c.PredicateType = opts.PredicateType + return c, nil } diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go index d4516e844..f5755e9d5 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -3,6 +3,7 @@ package verify import ( "testing" + "github.com/cli/cli/v2/pkg/cmd/attestation/verification" "github.com/cli/cli/v2/pkg/cmd/factory" "github.com/stretchr/testify/require" @@ -65,7 +66,7 @@ func TestNewEnforcementCriteria(t *testing.T) { c, err := newEnforcementCriteria(opts) require.NoError(t, err) - require.Equal(t, GitHubRunner, c.Certificate.RunnerEnvironment) + require.Equal(t, verification.GitHubRunner, c.Certificate.RunnerEnvironment) }) t.Run("sets Extensions.RunnerEnvironment to * value if opts.DenySelfHostedRunner is false", func(t *testing.T) { diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 5f1b1d6f3..188bde7b1 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -203,6 +203,17 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command } func runVerify(opts *Options) error { + ec, err := newEnforcementCriteria(opts) + if err != nil { + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy")) + return err + } + + if err := ec.Valid(); err != nil { + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Invalid verification policy")) + return err + } + artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm) if err != nil { opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ Loading digest for %s failed\n"), opts.ArtifactPath) @@ -258,12 +269,6 @@ func runVerify(opts *Options) error { opts.Logger.VerbosePrintf("Verifying attestations with predicate type: %s\n", opts.PredicateType) - ec, err := newEnforcementCriteria(opts) - if err != nil { - opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy")) - return err - } - sp, err := buildSigstoreVerifyPolicy(ec, *artifact) if err != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build Sigstore verification policy")) From a7a70fc91c70ab9ff6f95bb4fabaabb45e56e0c0 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 31 Oct 2024 16:59:25 -0600 Subject: [PATCH 23/33] check for SAN and SANRegex Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verification/policy.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/cmd/attestation/verification/policy.go b/pkg/cmd/attestation/verification/policy.go index ae21dae48..6e8ce7ae4 100644 --- a/pkg/cmd/attestation/verification/policy.go +++ b/pkg/cmd/attestation/verification/policy.go @@ -43,5 +43,8 @@ func (c EnforcementCriteria) Valid() error { if c.PredicateType == "" { return fmt.Errorf("PredicateType must be set") } + if c.SANRegex == "" && c.SAN == "" { + return fmt.Errorf("SANRegex or SAN must be set") + } return nil } From 0fb82a6e7c94f66165876575db44152a9470f2b2 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 31 Oct 2024 17:11:02 -0600 Subject: [PATCH 24/33] comments Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index c68cdb452..c6be71ae3 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -27,6 +27,7 @@ func expandToGitHubURL(tenant, ownerOrRepo string) string { func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, error) { c := verification.EnforcementCriteria{} + // Set SANRegex using either the opts.SignerRepo or opts.SignerWorkflow values if opts.SignerRepo != "" { signedRepoRegex := expandToGitHubURL(opts.Tenant, opts.SignerRepo) c.SANRegex = signedRepoRegex @@ -38,10 +39,13 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er c.SANRegex = validatedWorkflowRegex } else { + // If neither of those values were set, default to the provided SANRegex and SAN values c.SANRegex = opts.SANRegex c.SAN = opts.SAN } + // if the DenySelfHostedRunner option is set to true, set the + // RunnerEnvironment extension to the GitHub hosted runner value if opts.DenySelfHostedRunner { c.Certificate.RunnerEnvironment = verification.GitHubRunner } else { @@ -51,7 +55,10 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er c.Certificate.RunnerEnvironment = "" } + // If the Repo option is provided, set the SourceRepositoryURI extension if opts.Repo != "" { + // If the Tenant options is also provided, set the SourceRepositoryURI extension + // using the specific URI format if opts.Tenant != "" { c.Certificate.SourceRepositoryURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo) } else { @@ -59,6 +66,8 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er } } + // If the Tenant option is provided, set the SourceRepositoryOwnerURI extension + // using the specific URI format if opts.Tenant != "" { c.Certificate.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Owner) } else { @@ -66,10 +75,10 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er } // if tenant is provided, select the appropriate default based on the tenant - // otherwise, use the provided OIDCIssuer if opts.Tenant != "" { c.Certificate.Issuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant) } else { + // otherwise, use the provided OIDCIssuer c.Certificate.Issuer = opts.OIDCIssuer } From 815fcb72b57f3d5386b58f0b124c41942bc1198f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 1 Nov 2024 14:10:24 +0000 Subject: [PATCH 25/33] Bump github.com/creack/pty from 1.1.23 to 1.1.24 Bumps [github.com/creack/pty](https://github.com/creack/pty) from 1.1.23 to 1.1.24. - [Release notes](https://github.com/creack/pty/releases) - [Commits](https://github.com/creack/pty/compare/v1.1.23...v1.1.24) --- updated-dependencies: - dependency-name: github.com/creack/pty dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 77be0c19d..40193c4f8 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/cli/oauth v1.1.1 github.com/cli/safeexec v1.0.1 github.com/cpuguy83/go-md2man/v2 v2.0.5 - github.com/creack/pty v1.1.23 + github.com/creack/pty v1.1.24 github.com/distribution/reference v0.5.0 github.com/gabriel-vasile/mimetype v1.4.6 github.com/gdamore/tcell/v2 v2.5.4 diff --git a/go.sum b/go.sum index fe7adc3e7..d068d9d11 100644 --- a/go.sum +++ b/go.sum @@ -115,8 +115,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46t github.com/cpuguy83/go-md2man/v2 v2.0.5 h1:ZtcqGrnekaHpVLArFSe4HK5DoKx1T0rq2DwVB0alcyc= github.com/cpuguy83/go-md2man/v2 v2.0.5/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= -github.com/creack/pty v1.1.23 h1:4M6+isWdcStXEf15G/RbrMPOQj1dZ7HPZCGwE4kOeP0= -github.com/creack/pty v1.1.23/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= +github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= +github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= github.com/cyberphone/json-canonicalization v0.0.0-20220623050100-57a0ce2678a7 h1:vU+EP9ZuFUCYE0NYLwTSob+3LNEJATzNfP/DC7SWGWI= github.com/cyberphone/json-canonicalization v0.0.0-20220623050100-57a0ce2678a7/go.mod h1:uzvlm1mxhHkdfqitSA92i7Se+S9ksOn3a3qmv/kyOCw= github.com/danieljoos/wincred v1.2.1 h1:dl9cBrupW8+r5250DYkYxocLeZ1Y4vB1kxgtjxw8GQs= From a5eca00d0d2e175959f55033c92623a57817a334 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Fri, 1 Nov 2024 08:20:32 -0600 Subject: [PATCH 26/33] remove emtpy string checks Signed-off-by: Meredith Lancaster --- .../attestation/verification/extensions.go | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index 6f9945661..5434974a2 100644 --- a/pkg/cmd/attestation/verification/extensions.go +++ b/pkg/cmd/attestation/verification/extensions.go @@ -34,11 +34,9 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, ec Enforcement } func verifyCertExtensions(verifiedCert certificate.Summary, criteria EnforcementCriteria) error { - if criteria.Certificate.SourceRepositoryOwnerURI != "" { - sourceRepositoryOwnerURI := verifiedCert.Extensions.SourceRepositoryOwnerURI - if !strings.EqualFold(criteria.Certificate.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { - return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", criteria.Certificate.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) - } + sourceRepositoryOwnerURI := verifiedCert.Extensions.SourceRepositoryOwnerURI + if !strings.EqualFold(criteria.Certificate.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { + return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", criteria.Certificate.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) } // if repo is set, check the SourceRepositoryURI field @@ -51,14 +49,12 @@ func verifyCertExtensions(verifiedCert certificate.Summary, criteria Enforcement // if issuer is anything other than the default, use the user-provided value; // otherwise, select the appropriate default based on the tenant - if criteria.Certificate.Issuer != "" { - certIssuer := verifiedCert.Extensions.Issuer - if !strings.EqualFold(criteria.Certificate.Issuer, certIssuer) { - if strings.Index(certIssuer, criteria.Certificate.Issuer+"/") == 0 { - return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", criteria.Certificate.Issuer, certIssuer) - } - return fmt.Errorf("expected Issuer to be %s, got %s", criteria.Certificate.Issuer, certIssuer) + certIssuer := verifiedCert.Extensions.Issuer + if !strings.EqualFold(criteria.Certificate.Issuer, certIssuer) { + if strings.Index(certIssuer, criteria.Certificate.Issuer+"/") == 0 { + return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", criteria.Certificate.Issuer, certIssuer) } + return fmt.Errorf("expected Issuer to be %s, got %s", criteria.Certificate.Issuer, certIssuer) } return nil From a6d15b4f60b4db8bd26689c3c74cb0f912a2c858 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Fri, 1 Nov 2024 09:02:23 -0600 Subject: [PATCH 27/33] update OIDC issuer logic Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 14 +++++++++----- pkg/cmd/attestation/verify/policy_test.go | 5 +++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index c6be71ae3..d6b55abc0 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -74,12 +74,16 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er c.Certificate.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) } - // if tenant is provided, select the appropriate default based on the tenant - if opts.Tenant != "" { - c.Certificate.Issuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant) - } else { - // otherwise, use the provided OIDCIssuer + // If the OIDCIssuer option has been set, use that custom value + // Otherwise check if tenant is provided, select the appropriate default based on that + if opts.OIDCIssuer != verification.GitHubOIDCIssuer { c.Certificate.Issuer = opts.OIDCIssuer + } else { + if opts.Tenant != "" { + c.Certificate.Issuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant) + } else { + c.Certificate.Issuer = verification.GitHubOIDCIssuer + } } c.PredicateType = opts.PredicateType diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go index f5755e9d5..420c57f3a 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -132,13 +132,13 @@ func TestNewEnforcementCriteria(t *testing.T) { require.Equal(t, "https://github.com/foo", c.Certificate.SourceRepositoryOwnerURI) }) - t.Run("sets OIDCIssuer using opts.OIDCIssuer and opts.Tenant", func(t *testing.T) { + t.Run("sets OIDCIssuer using opts.Tenant", func(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, Owner: "foo", Repo: "foo/bar", Tenant: "baz", - OIDCIssuer: "https://foo.com", + OIDCIssuer: verification.GitHubOIDCIssuer, } c, err := newEnforcementCriteria(opts) @@ -152,6 +152,7 @@ func TestNewEnforcementCriteria(t *testing.T) { Owner: "foo", Repo: "foo/bar", OIDCIssuer: "https://foo.com", + Tenant: "baz", } c, err := newEnforcementCriteria(opts) From bb1584b52afc45d3952c942046051c28d0f22e37 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Fri, 1 Nov 2024 09:02:56 -0600 Subject: [PATCH 28/33] comment Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index d6b55abc0..99e3ea94e 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -74,8 +74,8 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er c.Certificate.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) } - // If the OIDCIssuer option has been set, use that custom value - // Otherwise check if tenant is provided, select the appropriate default based on that + // if issuer is anything other than the default, use the user-provided value; + // otherwise, select the appropriate default based on the tenant if opts.OIDCIssuer != verification.GitHubOIDCIssuer { c.Certificate.Issuer = opts.OIDCIssuer } else { From 43810a5fc3f03c460707340e4a05d0715137f7a3 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Fri, 1 Nov 2024 09:17:47 -0600 Subject: [PATCH 29/33] use predicate type stored in enforcementCriteria Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/verify.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 188bde7b1..0d129a03e 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -260,7 +260,7 @@ func runVerify(opts *Options) error { } // Apply predicate type filter to returned attestations - filteredAttestations := verification.FilterAttestations(opts.PredicateType, attestations) + filteredAttestations := verification.FilterAttestations(ec.PredicateType, attestations) if len(filteredAttestations) == 0 { opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ No attestations found with predicate type: %s\n"), opts.PredicateType) return err From 91967cced8833e2b1b3ec9eeb0ebbd44d95d76ce Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Fri, 1 Nov 2024 09:51:05 -0600 Subject: [PATCH 30/33] Update pkg/cmd/attestation/verify/verify.go Co-authored-by: Phill MV --- pkg/cmd/attestation/verify/verify.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 0d129a03e..cbfc91f1c 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -267,7 +267,7 @@ func runVerify(opts *Options) error { } attestations = filteredAttestations - opts.Logger.VerbosePrintf("Verifying attestations with predicate type: %s\n", opts.PredicateType) + opts.Logger.VerbosePrintf("Verifying attestations with predicate type: %s\n", ec.PredicateType) sp, err := buildSigstoreVerifyPolicy(ec, *artifact) if err != nil { From 3281bd457cc5c38c4ca62ce9481f8691ea567bc0 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Mon, 4 Nov 2024 07:32:10 -0700 Subject: [PATCH 31/33] simplify logic, add comments Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/policy.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index 99e3ea94e..d158cf375 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -25,7 +25,7 @@ func expandToGitHubURL(tenant, ownerOrRepo string) string { } func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, error) { - c := verification.EnforcementCriteria{} + var c verification.EnforcementCriteria // Set SANRegex using either the opts.SignerRepo or opts.SignerWorkflow values if opts.SignerRepo != "" { @@ -66,7 +66,7 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er } } - // If the Tenant option is provided, set the SourceRepositoryOwnerURI extension + // If the tenant option is provided, set the SourceRepositoryOwnerURI extension // using the specific URI format if opts.Tenant != "" { c.Certificate.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Owner) @@ -74,16 +74,13 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er c.Certificate.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) } - // if issuer is anything other than the default, use the user-provided value; - // otherwise, select the appropriate default based on the tenant - if opts.OIDCIssuer != verification.GitHubOIDCIssuer { - c.Certificate.Issuer = opts.OIDCIssuer + // if the tenant is provided and OIDC issuer provided matches the default + // use the tenant-specific issuer + if opts.Tenant != "" && opts.OIDCIssuer == verification.GitHubOIDCIssuer { + c.Certificate.Issuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant) } else { - if opts.Tenant != "" { - c.Certificate.Issuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant) - } else { - c.Certificate.Issuer = verification.GitHubOIDCIssuer - } + // otherwise use the custom OIDC issuer provided as an option + c.Certificate.Issuer = opts.OIDCIssuer } c.PredicateType = opts.PredicateType @@ -142,6 +139,8 @@ func validateSignerWorkflow(opts *Options) (string, error) { return fmt.Sprintf("^https://%s", opts.SignerWorkflow), nil } + // if the provided workflow did not match the expect format + // we move onto creating a signer workflow using the provided host name if opts.Hostname == "" { return "", errors.New("unknown host") } From b9c9f0acc27b79008649fefc4cabc9c0b5623aa1 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Mon, 4 Nov 2024 07:35:42 -0700 Subject: [PATCH 32/33] move comment Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verification/policy.go | 1 + pkg/cmd/attestation/verify/policy.go | 5 +---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/attestation/verification/policy.go b/pkg/cmd/attestation/verification/policy.go index 6e8ce7ae4..f5f4010aa 100644 --- a/pkg/cmd/attestation/verification/policy.go +++ b/pkg/cmd/attestation/verification/policy.go @@ -10,6 +10,7 @@ import ( "github.com/sigstore/sigstore-go/pkg/verify" ) +// represents the GitHub hosted runner in the certificate RunnerEnvironment extension const GitHubRunner = "github-hosted" // BuildDigestPolicyOption builds a verify.ArtifactPolicyOption diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index d158cf375..c2e154fe2 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -12,10 +12,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/verification" ) -const ( - // represents the GitHub hosted runner in the certificate RunnerEnvironment extension - hostRegex = `^[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+.*$` -) +const hostRegex = `^[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+.*$` func expandToGitHubURL(tenant, ownerOrRepo string) string { if tenant == "" { From 1c4c8e51453088949fd445c3b45d07fa6962edf3 Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Mon, 4 Nov 2024 17:55:35 +0200 Subject: [PATCH 33/33] Fix verbiage for deleting workflow runs It's not deleting _workflows_ (which are specified in YAML)... --- acceptance/testdata/workflow/run-delete.txtar | 2 +- pkg/cmd/run/delete/delete.go | 2 +- pkg/cmd/run/delete/delete_test.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/acceptance/testdata/workflow/run-delete.txtar b/acceptance/testdata/workflow/run-delete.txtar index 72d098740..b78135330 100644 --- a/acceptance/testdata/workflow/run-delete.txtar +++ b/acceptance/testdata/workflow/run-delete.txtar @@ -40,7 +40,7 @@ exec gh run watch $RUN_ID --exit-status # Delete the workflow run exec gh run delete $RUN_ID -stdout '✓ Request to delete workflow submitted.' +stdout '✓ Request to delete workflow run submitted.' # It takes some time for a workflow run to be deleted sleep 5 diff --git a/pkg/cmd/run/delete/delete.go b/pkg/cmd/run/delete/delete.go index b785b5f0f..711e98c02 100644 --- a/pkg/cmd/run/delete/delete.go +++ b/pkg/cmd/run/delete/delete.go @@ -133,7 +133,7 @@ func runDelete(opts *DeleteOptions) error { return err } - fmt.Fprintf(opts.IO.Out, "%s Request to delete workflow submitted.\n", cs.SuccessIcon()) + fmt.Fprintf(opts.IO.Out, "%s Request to delete workflow run submitted.\n", cs.SuccessIcon()) return nil } diff --git a/pkg/cmd/run/delete/delete_test.go b/pkg/cmd/run/delete/delete_test.go index 3ded01182..5df69aa60 100644 --- a/pkg/cmd/run/delete/delete_test.go +++ b/pkg/cmd/run/delete/delete_test.go @@ -110,7 +110,7 @@ func TestRunDelete(t *testing.T) { httpmock.REST("DELETE", fmt.Sprintf("repos/OWNER/REPO/actions/runs/%d", shared.SuccessfulRun.ID)), httpmock.StatusStringResponse(204, "")) }, - wantOut: "✓ Request to delete workflow submitted.\n", + wantOut: "✓ Request to delete workflow run submitted.\n", }, { name: "not found", @@ -153,7 +153,7 @@ func TestRunDelete(t *testing.T) { httpmock.REST("DELETE", fmt.Sprintf("repos/OWNER/REPO/actions/runs/%d", shared.SuccessfulRun.ID)), httpmock.StatusStringResponse(204, "")) }, - wantOut: "✓ Request to delete workflow submitted.\n", + wantOut: "✓ Request to delete workflow run submitted.\n", }, }