STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #9616 from cli/bdehamer/custom-issuer-error

Browse source 
Better messaging for `attestation verify` custom issuer mismatch error
This commit is contained in:
Brian DeHamer 2024-09-16 12:52:12 -07:00 • committed by GitHub
commit 2e13ec5d80
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 115 additions and 39 deletions
Download patch file Download diff file Expand all files Collapse all files

32
pkg/cmd/attestation/verification/extensions.go
View file

@ -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
}

74
pkg/cmd/attestation/verification/extensions_test.go
View file

@ -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")
})
}

1
pkg/cmd/attestation/verification/mock_verifier.go
View file

@ -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",
},
},
},

5
pkg/cmd/attestation/verify/policy.go
View file

@ -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
}

1
pkg/cmd/attestation/verify/policy_test.go
View file

@ -22,7 +22,6 @@ func TestBuildPolicy(t *testing.T) {
opts := &Options{
ArtifactPath: artifactPath,
OIDCIssuer: GitHubOIDCIssuer,
Owner: "sigstore",
SANRegex: "^https://github.com/sigstore/",
}

5
pkg/cmd/attestation/verify/verify.go
View file

@ -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 <owner>/<repo>")
verifyCmd.Flags().StringVarP(&opts.SignerWorkflow, "signer-workflow", "", "", "Workflow that signed attestation in the format [host/]<owner>/<repo>/<path>/<to>/<workflow>")
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
}

6
pkg/cmd/attestation/verify/verify_integration_test.go
View file

@ -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),

30
pkg/cmd/attestation/verify/verify_test.go
View file

@ -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),