diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index 727feb72c..2ffb11a9d 100644 --- a/pkg/cmd/attestation/verification/extensions.go +++ b/pkg/cmd/attestation/verification/extensions.go @@ -6,20 +6,25 @@ import ( "strings" ) -func VerifyCertExtensions(results []*AttestationProcessingResult, tenant, owner, repo string) error { +var ( + GitHubOIDCIssuer = "https://token.actions.githubusercontent.com" + GitHubTenantOIDCIssuer = "https://token.actions.%s.ghe.com" +) + +func VerifyCertExtensions(results []*AttestationProcessingResult, tenant, owner, repo, issuer string) error { if len(results) == 0 { return errors.New("no attestations proccessing results") } for _, attestation := range results { - if err := verifyCertExtensions(attestation, tenant, owner, repo); err != nil { + if err := verifyCertExtensions(attestation, tenant, owner, repo, issuer); err != nil { return err } } return nil } -func verifyCertExtensions(attestation *AttestationProcessingResult, tenant, owner, repo string) error { +func verifyCertExtensions(attestation *AttestationProcessingResult, tenant, owner, repo, issuer string) error { var want string if tenant == "" { @@ -46,5 +51,26 @@ func verifyCertExtensions(attestation *AttestationProcessingResult, tenant, owne } } + // 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) + } + } + return nil } diff --git a/pkg/cmd/attestation/verification/extensions_test.go b/pkg/cmd/attestation/verification/extensions_test.go index c86f2030a..5eb28829d 100644 --- a/pkg/cmd/attestation/verification/extensions_test.go +++ b/pkg/cmd/attestation/verification/extensions_test.go @@ -17,6 +17,7 @@ func TestVerifyCertExtensions(t *testing.T) { Extensions: certificate.Extensions{ SourceRepositoryOwnerURI: "https://github.com/owner", SourceRepositoryURI: "https://github.com/owner/repo", + Issuer: "https://token.actions.githubusercontent.com", }, }, }, @@ -25,32 +26,72 @@ func TestVerifyCertExtensions(t *testing.T) { } t.Run("VerifyCertExtensions with owner and repo", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "owner/repo") + err := VerifyCertExtensions(results, "", "owner", "owner/repo", GitHubOIDCIssuer) 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") + err := VerifyCertExtensions(results, "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("VerifyCertExtensions with owner", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "") + err := VerifyCertExtensions(results, "", "owner", "", GitHubOIDCIssuer) require.NoError(t, err) }) t.Run("VerifyCertExtensions with wrong owner", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "wrong", "") + err := VerifyCertExtensions(results, "", "wrong", "", GitHubOIDCIssuer) 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") + err := VerifyCertExtensions(results, "", "owner", "wrong", GitHubOIDCIssuer) 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") + 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", + }, + }, + }, + }, + }, + } + + t.Run("VerifyCertExtensions with exact issuer match", func(t *testing.T) { + err := VerifyCertExtensions(results, "", "owner", "owner/repo", "https://token.actions.githubusercontent.com/foo-bar") + 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") + 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") + 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{ @@ -59,6 +100,7 @@ func TestVerifyTenancyCertExtensions(t *testing.T) { Extensions: certificate.Extensions{ SourceRepositoryOwnerURI: "https://foo.ghe.com/owner", SourceRepositoryURI: "https://foo.ghe.com/owner/repo", + Issuer: "https://token.actions.foo.ghe.com", }, }, }, @@ -67,32 +109,42 @@ func TestVerifyTenancyCertExtensions(t *testing.T) { } t.Run("VerifyCertExtensions with owner and repo", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "owner/repo") + err := VerifyCertExtensions(results, "foo", "owner", "owner/repo", defaultIssuer) require.NoError(t, err) }) t.Run("VerifyCertExtensions with owner and repo, no tenant", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "owner/repo") + err := VerifyCertExtensions(results, "", "owner", "owner/repo", defaultIssuer) 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") + err := VerifyCertExtensions(results, "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("VerifyCertExtensions with owner", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "") + err := VerifyCertExtensions(results, "foo", "owner", "", defaultIssuer) require.NoError(t, err) }) t.Run("VerifyCertExtensions with wrong owner", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "wrong", "") + err := VerifyCertExtensions(results, "foo", "wrong", "", defaultIssuer) 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") + err := VerifyCertExtensions(results, "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("VerifyCertExtensions with correct, non-default issuer", func(t *testing.T) { + err := VerifyCertExtensions(results, "foo", "owner", "owner/repo", "https://token.actions.foo.ghe.com") + require.NoError(t, err) + }) + + t.Run("VerifyCertExtensions with wrong issuer", func(t *testing.T) { + err := VerifyCertExtensions(results, "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/mock_verifier.go b/pkg/cmd/attestation/verification/mock_verifier.go index 9943e6a97..cb3a4c061 100644 --- a/pkg/cmd/attestation/verification/mock_verifier.go +++ b/pkg/cmd/attestation/verification/mock_verifier.go @@ -34,6 +34,7 @@ func (v *MockSigstoreVerifier) Verify(attestations []*api.Attestation, policy ve BuildSignerURI: "https://github.com/github/example/.github/workflows/release.yml@refs/heads/main", SourceRepositoryOwnerURI: "https://github.com/sigstore", SourceRepositoryURI: "https://github.com/sigstore/sigstore-js", + Issuer: "https://token.actions.githubusercontent.com", }, }, }, diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index 0c7a686c2..f41b2f66b 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -13,8 +13,6 @@ import ( ) const ( - GitHubOIDCIssuer = "https://token.actions.githubusercontent.com" - GitHubTenantOIDCIssuer = "https://token.actions.%s.ghe.com" // represents the GitHub hosted runner in the certificate RunnerEnvironment extension GitHubRunner = "github-hosted" hostRegex = `^[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+.*$` @@ -51,7 +49,8 @@ func buildCertificateIdentityOption(opts *Options, runnerEnv string) (verify.Pol return nil, err } - issuerMatcher, err := verify.NewIssuerMatcher(opts.OIDCIssuer, "") + // Accept any issuer, we will verify the issuer as part of the extension verification + issuerMatcher, err := verify.NewIssuerMatcher("", ".*") if err != nil { return nil, err } diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go index 89f989b45..ae1e52955 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -22,7 +22,6 @@ func TestBuildPolicy(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, - OIDCIssuer: GitHubOIDCIssuer, Owner: "sigstore", SANRegex: "^https://github.com/sigstore/", } diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 9e508e994..17b8a84d1 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -151,7 +151,6 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command } config.TrustDomain = td opts.Tenant = tenant - opts.OIDCIssuer = fmt.Sprintf(GitHubTenantOIDCIssuer, tenant) } // set policy flags based on what has been provided @@ -192,7 +191,7 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command verifyCmd.Flags().StringVarP(&opts.SignerRepo, "signer-repo", "", "", "Repository of reusable workflow that signed attestation in the format /") verifyCmd.Flags().StringVarP(&opts.SignerWorkflow, "signer-workflow", "", "", "Workflow that signed attestation in the format [host/]////") verifyCmd.MarkFlagsMutuallyExclusive("cert-identity", "cert-identity-regex", "signer-repo", "signer-workflow") - verifyCmd.Flags().StringVarP(&opts.OIDCIssuer, "cert-oidc-issuer", "", GitHubOIDCIssuer, "Issuer of the OIDC token") + verifyCmd.Flags().StringVarP(&opts.OIDCIssuer, "cert-oidc-issuer", "", verification.GitHubOIDCIssuer, "Issuer of the OIDC token") verifyCmd.Flags().StringVarP(&opts.Hostname, "hostname", "", "", "Configure host to use") return verifyCmd @@ -269,7 +268,7 @@ func runVerify(opts *Options) error { } // Verify extensions - if err := verification.VerifyCertExtensions(sigstoreRes.VerifyResults, opts.Tenant, opts.Owner, opts.Repo); err != nil { + 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 } diff --git a/pkg/cmd/attestation/verify/verify_integration_test.go b/pkg/cmd/attestation/verify/verify_integration_test.go index df24b31c4..4b0f0adfb 100644 --- a/pkg/cmd/attestation/verify/verify_integration_test.go +++ b/pkg/cmd/attestation/verify/verify_integration_test.go @@ -38,7 +38,7 @@ func TestVerifyIntegration(t *testing.T) { DigestAlgorithm: "sha512", Logger: logger, OCIClient: oci.NewLiveClient(), - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", SANRegex: "^https://github.com/sigstore/", SigstoreVerifier: verification.NewLiveSigstoreVerifier(sigstoreConfig), @@ -180,7 +180,7 @@ func TestVerifyIntegrationReusableWorkflow(t *testing.T) { DigestAlgorithm: "sha256", Logger: logger, OCIClient: oci.NewLiveClient(), - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, SigstoreVerifier: verification.NewLiveSigstoreVerifier(sigstoreConfig), } @@ -269,7 +269,7 @@ func TestVerifyIntegrationReusableWorkflowSignerWorkflow(t *testing.T) { DigestAlgorithm: "sha256", Logger: logger, OCIClient: oci.NewLiveClient(), - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "malancas", Repo: "malancas/attest-demo", SigstoreVerifier: verification.NewLiveSigstoreVerifier(sigstoreConfig), diff --git a/pkg/cmd/attestation/verify/verify_test.go b/pkg/cmd/attestation/verify/verify_test.go index 3ff309922..306ff8b35 100644 --- a/pkg/cmd/attestation/verify/verify_test.go +++ b/pkg/cmd/attestation/verify/verify_test.go @@ -71,7 +71,7 @@ func TestNewVerifyCmd(t *testing.T) { BundlePath: test.NormalizeRelativePath("../test/data/sigstore-js-2.1.0-bundle.json"), DigestAlgorithm: "sha384", Limit: 30, - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", SigstoreVerifier: verification.NewMockSigstoreVerifier(t), Hostname: "github.com", @@ -86,7 +86,7 @@ func TestNewVerifyCmd(t *testing.T) { BundlePath: test.NormalizeRelativePath("../test/data/sigstore-js-2.1.0-bundle.json"), DigestAlgorithm: "sha256", Limit: 30, - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", SANRegex: "(?i)^https://github.com/sigstore/", SigstoreVerifier: verification.NewMockSigstoreVerifier(t), @@ -102,7 +102,7 @@ func TestNewVerifyCmd(t *testing.T) { BundlePath: test.NormalizeRelativePath("../test/data/sigstore-js-2.1.0-bundle.json"), DigestAlgorithm: "sha256", Limit: 30, - OIDCIssuer: "https://token.actions.foo.ghe.com", + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", SANRegex: "(?i)^https://foo.ghe.com/sigstore/", SigstoreVerifier: verification.NewMockSigstoreVerifier(t), @@ -118,7 +118,7 @@ func TestNewVerifyCmd(t *testing.T) { BundlePath: test.NormalizeRelativePath("../test/data/sigstore-js-2.1.0-bundle.json"), DigestAlgorithm: "sha256", Limit: 30, - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", SANRegex: "(?i)^https://github.com/sigstore/", SigstoreVerifier: verification.NewMockSigstoreVerifier(t), @@ -134,7 +134,7 @@ func TestNewVerifyCmd(t *testing.T) { BundlePath: test.NormalizeRelativePath("../test/data/sigstore-js-2.1.0-bundle.json"), DigestAlgorithm: "sha512", Limit: 30, - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", SANRegex: "(?i)^https://github.com/sigstore/", SigstoreVerifier: verification.NewMockSigstoreVerifier(t), @@ -148,7 +148,7 @@ func TestNewVerifyCmd(t *testing.T) { wants: Options{ ArtifactPath: test.NormalizeRelativePath("../test/data/sigstore-js-2.1.0.tgz"), DigestAlgorithm: "sha256", - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", Limit: 30, SANRegex: "(?i)^https://github.com/sigstore/", @@ -163,7 +163,7 @@ func TestNewVerifyCmd(t *testing.T) { wants: Options{ ArtifactPath: artifactPath, DigestAlgorithm: "sha256", - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", Repo: "sigstore/sigstore-js", Limit: 30, @@ -179,7 +179,7 @@ func TestNewVerifyCmd(t *testing.T) { ArtifactPath: artifactPath, DigestAlgorithm: "sha256", Limit: 30, - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", SANRegex: "(?i)^https://github.com/sigstore/", SigstoreVerifier: verification.NewMockSigstoreVerifier(t), @@ -193,7 +193,7 @@ func TestNewVerifyCmd(t *testing.T) { wants: Options{ ArtifactPath: artifactPath, DigestAlgorithm: "sha256", - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", Limit: 101, SANRegex: "(?i)^https://github.com/sigstore/", @@ -208,7 +208,7 @@ func TestNewVerifyCmd(t *testing.T) { wants: Options{ ArtifactPath: artifactPath, DigestAlgorithm: "sha256", - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", Limit: 0, SANRegex: "(?i)^https://github.com/sigstore/", @@ -224,7 +224,7 @@ func TestNewVerifyCmd(t *testing.T) { ArtifactPath: artifactPath, DigestAlgorithm: "sha256", Limit: 30, - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", SAN: "https://github.com/sigstore/", SANRegex: "(?i)^https://github.com/sigstore/", @@ -241,7 +241,7 @@ func TestNewVerifyCmd(t *testing.T) { BundlePath: bundlePath, DigestAlgorithm: "sha256", Limit: 30, - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", SANRegex: "(?i)^https://github.com/sigstore/", SigstoreVerifier: verification.NewMockSigstoreVerifier(t), @@ -331,7 +331,7 @@ func TestJSONOutput(t *testing.T) { APIClient: api.NewTestClient(), Logger: io.NewHandler(testIO), OCIClient: oci.MockClient{}, - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", SANRegex: "^https://github.com/sigstore/", SigstoreVerifier: verification.NewMockSigstoreVerifier(t), @@ -354,7 +354,7 @@ func TestRunVerify(t *testing.T) { APIClient: api.NewTestClient(), Logger: logger, OCIClient: oci.MockClient{}, - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", SANRegex: "^https://github.com/sigstore/", SigstoreVerifier: verification.NewMockSigstoreVerifier(t), @@ -474,7 +474,7 @@ func TestRunVerify(t *testing.T) { APIClient: api.NewTestClient(), DigestAlgorithm: "sha512", Logger: logger, - OIDCIssuer: GitHubOIDCIssuer, + OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", SAN: SigstoreSanValue, SigstoreVerifier: verification.NewMockSigstoreVerifier(t),