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/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 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= diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index 4c748e3e9..e302d89c9 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 ( @@ -11,14 +13,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 lastErr error for _, attestation := range results { - err := verifyCertExtension(attestation, tenant, owner, repo, issuer) + err := verifyCertExtensions(*attestation.VerificationResult.Signature.Certificate, ec) if err == nil { // if at least one attestation is verified, we're good as verification // is defined as successful if at least one attestation is verified @@ -32,52 +34,28 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, tenant, owner, return lastErr } -func verifyCertExtension(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(verifiedCert certificate.Summary, criteria EnforcementCriteria) error { + 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 - if repo != "" { - if tenant == "" { - want = fmt.Sprintf("https://github.com/%s", repo) - } else { - want = fmt.Sprintf("https://%s.ghe.com/%s", tenant, repo) - } - - 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 criteria.Certificate.SourceRepositoryURI != "" { + sourceRepositoryURI := verifiedCert.Extensions.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 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) + 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 diff --git a/pkg/cmd/attestation/verification/extensions_test.go b/pkg/cmd/attestation/verification/extensions_test.go index 87249cb7b..b8ef2875f 100644 --- a/pkg/cmd/attestation/verification/extensions_test.go +++ b/pkg/cmd/attestation/verification/extensions_test.go @@ -27,8 +27,17 @@ func createSampleResult() *AttestationProcessingResult { func TestVerifyCertExtensions(t *testing.T) { results := []*AttestationProcessingResult{createSampleResult()} + certSummary := certificate.Summary{} + certSummary.SourceRepositoryOwnerURI = "https://github.com/owner" + certSummary.SourceRepositoryURI = "https://github.com/owner/repo" + certSummary.Issuer = GitHubOIDCIssuer + + c := EnforcementCriteria{ + Certificate: certSummary, + } + t.Run("passes with one result", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "owner/repo", GitHubOIDCIssuer) + err := VerifyCertExtensions(results, c) require.NoError(t, err) }) @@ -37,7 +46,7 @@ func TestVerifyCertExtensions(t *testing.T) { require.Len(t, twoResults, 2) twoResults[1].VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI = "https://github.com/wrong" - err := VerifyCertExtensions(twoResults, "", "owner", "owner/repo", GitHubOIDCIssuer) + err := VerifyCertExtensions(twoResults, c) require.NoError(t, err) }) @@ -47,123 +56,35 @@ func TestVerifyCertExtensions(t *testing.T) { twoResults[0].VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI = "https://github.com/wrong" twoResults[1].VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI = "https://github.com/wrong" - err := VerifyCertExtensions(twoResults, "", "owner", "owner/repo", GitHubOIDCIssuer) + err := VerifyCertExtensions(twoResults, c) require.Error(t, err) }) -} -func TestVerifyCertExtension(t *testing.T) { - t.Run("with owner and repo, but wrong tenant", func(t *testing.T) { - err := verifyCertExtension(createSampleResult(), "foo", "owner", "owner/repo", GitHubOIDCIssuer) - require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://foo.ghe.com/owner, got https://github.com/owner") - }) - - t.Run("with owner", func(t *testing.T) { - err := verifyCertExtension(createSampleResult(), "", "owner", "", GitHubOIDCIssuer) - require.NoError(t, err) - }) - - t.Run("with wrong owner", func(t *testing.T) { - err := verifyCertExtension(createSampleResult(), "", "wrong", "", GitHubOIDCIssuer) + t.Run("with wrong SourceRepositoryOwnerURI", func(t *testing.T) { + expectedCriteria := c + 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 repo", func(t *testing.T) { - err := verifyCertExtension(createSampleResult(), "", "owner", "wrong", GitHubOIDCIssuer) - require.ErrorContains(t, err, "expected SourceRepositoryURI to be https://github.com/wrong, got https://github.com/owner/repo") + t.Run("with wrong SourceRepositoryURI", func(t *testing.T) { + expectedCriteria := c + 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 issuer", func(t *testing.T) { - err := verifyCertExtension(createSampleResult(), "", "owner", "", "wrong") + t.Run("with wrong OIDCIssuer", func(t *testing.T) { + expectedCriteria := c + expectedCriteria.Certificate.Issuer = "wrong" + err := VerifyCertExtensions(results, expectedCriteria) require.ErrorContains(t, err, "expected Issuer to be wrong, got https://token.actions.githubusercontent.com") }) -} -func TestVerifyCertExtensionCustomizedIssuer(t *testing.T) { - result := &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", - }, - }, - }, - }, - } - - t.Run("with exact issuer match", func(t *testing.T) { - err := verifyCertExtension(result, "", "owner", "owner/repo", "https://token.actions.githubusercontent.com/foo-bar") - require.NoError(t, err) - }) - - t.Run("with partial issuer match", func(t *testing.T) { - err := verifyCertExtension(result, "", "owner", "owner/repo", "https://token.actions.githubusercontent.com") + t.Run("with partial OIDCIssuer match", func(t *testing.T) { + 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") }) - - t.Run("with wrong issuer", func(t *testing.T) { - err := verifyCertExtension(result, "", "owner", "", "wrong") - require.ErrorContains(t, err, "expected Issuer to be wrong, got https://token.actions.githubusercontent.com/foo-bar") - }) -} - -func TestVerifyTenancyCertExtensions(t *testing.T) { - defaultIssuer := GitHubOIDCIssuer - - result := &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", - }, - }, - }, - }, - } - - t.Run("with owner and repo", func(t *testing.T) { - err := verifyCertExtension(result, "foo", "owner", "owner/repo", defaultIssuer) - require.NoError(t, err) - }) - - t.Run("with owner and repo, no tenant", func(t *testing.T) { - err := verifyCertExtension(result, "", "owner", "owner/repo", defaultIssuer) - require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://github.com/owner, got https://foo.ghe.com/owner") - }) - - t.Run("with owner and repo, wrong tenant", func(t *testing.T) { - err := verifyCertExtension(result, "bar", "owner", "owner/repo", defaultIssuer) - require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://bar.ghe.com/owner, got https://foo.ghe.com/owner") - }) - - t.Run("with owner", func(t *testing.T) { - err := verifyCertExtension(result, "foo", "owner", "", defaultIssuer) - require.NoError(t, err) - }) - - t.Run("with wrong owner", func(t *testing.T) { - err := verifyCertExtension(result, "foo", "wrong", "", defaultIssuer) - require.ErrorContains(t, err, "expected SourceRepositoryOwnerURI to be https://foo.ghe.com/wrong, got https://foo.ghe.com/owner") - }) - - t.Run("with wrong repo", func(t *testing.T) { - err := verifyCertExtension(result, "foo", "owner", "wrong", defaultIssuer) - require.ErrorContains(t, err, "expected SourceRepositoryURI to be https://foo.ghe.com/wrong, got https://foo.ghe.com/owner/repo") - }) - - t.Run("with correct, non-default issuer", func(t *testing.T) { - err := verifyCertExtension(result, "foo", "owner", "owner/repo", "https://token.actions.foo.ghe.com") - require.NoError(t, err) - }) - - t.Run("with wrong issuer", func(t *testing.T) { - err := verifyCertExtension(result, "foo", "owner", "owner/repo", "wrong") - require.ErrorContains(t, err, "expected Issuer to be wrong, got https://token.actions.foo.ghe.com") - }) } diff --git a/pkg/cmd/attestation/verification/policy.go b/pkg/cmd/attestation/verification/policy.go index 91b54c75e..f5f4010aa 100644 --- a/pkg/cmd/attestation/verification/policy.go +++ b/pkg/cmd/attestation/verification/policy.go @@ -2,12 +2,17 @@ package verification import ( "encoding/hex" + "fmt" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" + "github.com/sigstore/sigstore-go/pkg/fulcio/certificate" "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 // from the given artifact digest and digest algorithm func BuildDigestPolicyOption(a artifact.DigestedArtifact) (verify.ArtifactPolicyOption, error) { @@ -18,3 +23,29 @@ func BuildDigestPolicyOption(a artifact.DigestedArtifact) (verify.ArtifactPolicy } return verify.WithArtifactDigest(a.Algorithm(), decoded), nil } + +type EnforcementCriteria struct { + Certificate certificate.Summary + PredicateType string + 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") + } + if c.SANRegex == "" && c.SAN == "" { + return fmt.Errorf("SANRegex or SAN must be set") + } + return nil +} diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index f41b2f66b..c2e154fe2 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -12,11 +12,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/verification" ) -const ( - // represents the GitHub hosted runner in the certificate RunnerEnvironment extension - GitHubRunner = "github-hosted" - 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 == "" { @@ -25,26 +21,72 @@ func expandToGitHubURL(tenant, ownerOrRepo string) string { return fmt.Sprintf("(?i)^https://%s.ghe.com/%s/", tenant, ownerOrRepo) } -func buildSANMatcher(opts *Options) (verify.SubjectAlternativeNameMatcher, error) { +func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, error) { + var c verification.EnforcementCriteria + + // Set SANRegex using either the opts.SignerRepo or opts.SignerWorkflow values if opts.SignerRepo != "" { signedRepoRegex := expandToGitHubURL(opts.Tenant, opts.SignerRepo) - return verify.NewSANMatcher("", signedRepoRegex) + c.SANRegex = signedRepoRegex } else if opts.SignerWorkflow != "" { validatedWorkflowRegex, err := validateSignerWorkflow(opts) if err != nil { - return verify.SubjectAlternativeNameMatcher{}, err + return verification.EnforcementCriteria{}, err } - return verify.NewSANMatcher("", validatedWorkflowRegex) - } else if opts.SAN != "" || opts.SANRegex != "" { - return verify.NewSANMatcher(opts.SAN, opts.SANRegex) + 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 } - return verify.SubjectAlternativeNameMatcher{}, nil + // 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 { + // 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.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 { + c.Certificate.SourceRepositoryURI = fmt.Sprintf("https://github.com/%s", opts.Repo) + } + } + + // 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 { + c.Certificate.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) + } + + // 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 { + // otherwise use the custom OIDC issuer provided as an option + c.Certificate.Issuer = opts.OIDCIssuer + } + + c.PredicateType = opts.PredicateType + + return c, nil } -func buildCertificateIdentityOption(opts *Options, runnerEnv string) (verify.PolicyOption, error) { - sanMatcher, err := buildSANMatcher(opts) +func buildCertificateIdentityOption(c verification.EnforcementCriteria) (verify.PolicyOption, error) { + sanMatcher, err := verify.NewSANMatcher(c.SAN, c.SANRegex) if err != nil { return nil, err } @@ -56,7 +98,7 @@ func buildCertificateIdentityOption(opts *Options, runnerEnv string) (verify.Pol } extensions := certificate.Extensions{ - RunnerEnvironment: runnerEnv, + RunnerEnvironment: c.Certificate.RunnerEnvironment, } certId, err := verify.NewCertificateIdentity(sanMatcher, issuerMatcher, extensions) @@ -67,34 +109,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) { +func buildSigstoreVerifyPolicy(c verification.EnforcementCriteria, a artifact.DigestedArtifact) (verify.PolicyBuilder, error) { artifactDigestPolicyOption, err := verification.BuildDigestPolicyOption(a) if err != nil { return verify.PolicyBuilder{}, err } - certIdOption, err := buildVerifyCertIdOption(opts) + certIdOption, err := buildCertificateIdentityOption(c) if err != nil { return verify.PolicyBuilder{}, err } @@ -103,10 +124,6 @@ func buildVerifyPolicy(opts *Options, a artifact.DigestedArtifact) (verify.Polic 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 @@ -116,12 +133,14 @@ func validateSignerWorkflow(opts *Options) (string, error) { } if match { - return addSchemeToRegex(opts.SignerWorkflow), nil + 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") } - return addSchemeToRegex(fmt.Sprintf("%s/%s", opts.Hostname, opts.SignerWorkflow)), nil + return fmt.Sprintf("^https://%s/%s", opts.Hostname, opts.SignerWorkflow), nil } diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go index 39e5d3707..420c57f3a 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -3,31 +3,162 @@ 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/attestation/verification" "github.com/cli/cli/v2/pkg/cmd/factory" "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) - require.NoError(t, err) + t.Run("sets SANRegex using SignerRepo", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "foo/bar", + SignerRepo: "foo/bar", + } - opts := &Options{ - ArtifactPath: artifactPath, - Owner: "sigstore", - SANRegex: "^https://github.com/sigstore/", - } + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "(?i)^https://github.com/foo/bar/", c.SANRegex) + require.Zero(t, c.SAN) + }) - _, err = buildVerifyPolicy(opts, *artifact) - require.NoError(t, err) + t.Run("sets SANRegex using SignerWorkflow matching host regex", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "foo/bar", + SignerWorkflow: "foo/bar/.github/workflows/attest.yml", + Hostname: "github.com", + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + 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) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "foo/bar", + SAN: "https://github/foo/bar/.github/workflows/attest.yml", + SANRegex: "(?i)^https://github/foo", + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + 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) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "foo/bar", + DenySelfHostedRunner: true, + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, verification.GitHubRunner, c.Certificate.RunnerEnvironment) + }) + + t.Run("sets Extensions.RunnerEnvironment to * value if opts.DenySelfHostedRunner is false", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "foo/bar", + DenySelfHostedRunner: false, + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Zero(t, c.Certificate.RunnerEnvironment) + }) + + t.Run("sets Extensions.SourceRepositoryURI using opts.Repo and opts.Tenant", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "foo/bar", + Tenant: "baz", + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "https://baz.ghe.com/foo/bar", c.Certificate.SourceRepositoryURI) + }) + + t.Run("sets Extensions.SourceRepositoryURI using opts.Repo", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "foo/bar", + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + 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) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "foo/bar", + Tenant: "baz", + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "https://baz.ghe.com/foo", c.Certificate.SourceRepositoryOwnerURI) + }) + + t.Run("sets Extensions.SourceRepositoryOwnerURI using opts.Owner", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "foo/bar", + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "https://github.com/foo", c.Certificate.SourceRepositoryOwnerURI) + }) + + t.Run("sets OIDCIssuer using opts.Tenant", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "foo/bar", + Tenant: "baz", + OIDCIssuer: verification.GitHubOIDCIssuer, + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "https://token.actions.baz.ghe.com", c.Certificate.Issuer) + }) + + t.Run("sets OIDCIssuer using opts.OIDCIssuer", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "foo/bar", + OIDCIssuer: "https://foo.com", + Tenant: "baz", + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "https://foo.com", c.Certificate.Issuer) + }) } func TestValidateSignerWorkflow(t *testing.T) { diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 206001f9b..cbfc91f1c 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) @@ -249,30 +260,30 @@ 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 } attestations = filteredAttestations - policy, err := buildVerifyPolicy(opts, *artifact) + opts.Logger.VerbosePrintf("Verifying attestations with predicate type: %s\n", ec.PredicateType) + + sp, err := buildSigstoreVerifyPolicy(ec, *artifact) if err != nil { - opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy")) + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build Sigstore verification policy")) return err } - opts.Logger.VerbosePrintf("Verifying attestations with predicate type: %s\n", opts.PredicateType) - - sigstoreRes := opts.SigstoreVerifier.Verify(attestations, policy) + sigstoreRes := opts.SigstoreVerifier.Verify(attestations, sp) if sigstoreRes.Error != nil { - opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Verification failed")) + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Sigstore 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")) + if err := verification.VerifyCertExtensions(sigstoreRes.VerifyResults, ec); err != nil { + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Policy verification failed")) return err } 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", }, }