From d29a4a751ad5e81b0eaac0e492342e055d7eef98 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 31 Oct 2024 10:44:36 -0600 Subject: [PATCH] update extension verification logic Signed-off-by: Meredith Lancaster --- .../attestation/verification/extensions.go | 18 ++- .../verification/extensions_test.go | 118 ++++++++++-------- 2 files changed, 71 insertions(+), 65 deletions(-) diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index 046f24509..274e833c4 100644 --- a/pkg/cmd/attestation/verification/extensions.go +++ b/pkg/cmd/attestation/verification/extensions.go @@ -16,22 +16,20 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, tenant, owner, return errors.New("no attestations proccessing results") } - var atLeastOneVerified bool for _, attestation := range results { - if err := verifyCertExtensions(attestation, tenant, owner, repo, issuer); err != nil { - return err + if err := verifyCertExtension(attestation, tenant, owner, repo, issuer); 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 + return nil } - atLeastOneVerified = true } - if !atLeastOneVerified { - return ErrNoAttestationsVerified - } - - return nil + // if we have exited the for loop without returning early due to successful + // verification, we need to return an error + return ErrNoAttestationsVerified } -func verifyCertExtensions(attestation *AttestationProcessingResult, tenant, owner, repo, issuer string) error { +func verifyCertExtension(attestation *AttestationProcessingResult, tenant, owner, repo, issuer string) error { var want string if tenant == "" { diff --git a/pkg/cmd/attestation/verification/extensions_test.go b/pkg/cmd/attestation/verification/extensions_test.go index 445234652..a03cf79d6 100644 --- a/pkg/cmd/attestation/verification/extensions_test.go +++ b/pkg/cmd/attestation/verification/extensions_test.go @@ -27,12 +27,12 @@ func createSampleResult() *AttestationProcessingResult { func TestVerifyCertExtensions(t *testing.T) { results := []*AttestationProcessingResult{createSampleResult()} - t.Run("VerifyCertExtensions with owner and repo", func(t *testing.T) { + t.Run("passes with one result", func(t *testing.T) { err := VerifyCertExtensions(results, "", "owner", "owner/repo", GitHubOIDCIssuer) require.NoError(t, err) }) - t.Run("VerifyCertExtensions passes with at least one successful verification", func(t *testing.T) { + t.Run("passes with 1/2 valid results", func(t *testing.T) { twoResults := []*AttestationProcessingResult{createSampleResult(), createSampleResult()} require.Len(t, twoResults, 2) twoResults[1].VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI = "https://github.com/wrong" @@ -41,61 +41,71 @@ func TestVerifyCertExtensions(t *testing.T) { 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) + t.Run("fails when all results fail verification", func(t *testing.T) { + twoResults := []*AttestationProcessingResult{createSampleResult(), createSampleResult()} + require.Len(t, twoResults, 2) + 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) + require.NoError(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("VerifyCertExtensions with owner", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "", GitHubOIDCIssuer) + t.Run("with owner", func(t *testing.T) { + err := verifyCertExtension(createSampleResult(), "", "owner", "", GitHubOIDCIssuer) require.NoError(t, err) }) - t.Run("VerifyCertExtensions with wrong owner", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "wrong", "", GitHubOIDCIssuer) + t.Run("with wrong owner", func(t *testing.T) { + err := verifyCertExtension(createSampleResult(), "", "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", GitHubOIDCIssuer) + 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("VerifyCertExtensions with wrong issuer", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "", "wrong") + t.Run("with wrong issuer", func(t *testing.T) { + err := verifyCertExtension(createSampleResult(), "", "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", - }, +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("VerifyCertExtensions with exact issuer match", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "owner/repo", "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("VerifyCertExtensions with partial issuer match", func(t *testing.T) { - err := VerifyCertExtensions(results, "", "owner", "owner/repo", "https://token.actions.githubusercontent.com") + t.Run("with partial issuer match", func(t *testing.T) { + err := verifyCertExtension(result, "", "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") + 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") }) } @@ -103,59 +113,57 @@ func TestVerifyCertExtensionsCustomizedIssuer(t *testing.T) { func TestVerifyTenancyCertExtensions(t *testing.T) { defaultIssuer := GitHubOIDCIssuer - 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", - }, + 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("VerifyCertExtensions with owner and repo", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "owner/repo", defaultIssuer) + t.Run("with owner and repo", func(t *testing.T) { + err := verifyCertExtension(result, "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", defaultIssuer) + 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("VerifyCertExtensions with owner and repo, wrong tenant", func(t *testing.T) { - err := VerifyCertExtensions(results, "bar", "owner", "owner/repo", defaultIssuer) + 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("VerifyCertExtensions with owner", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "", defaultIssuer) + t.Run("with owner", func(t *testing.T) { + err := verifyCertExtension(result, "foo", "owner", "", defaultIssuer) require.NoError(t, err) }) - t.Run("VerifyCertExtensions with wrong owner", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "wrong", "", defaultIssuer) + 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("VerifyCertExtensions with wrong repo", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "wrong", defaultIssuer) + 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("VerifyCertExtensions with correct, non-default issuer", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "owner/repo", "https://token.actions.foo.ghe.com") + 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("VerifyCertExtensions with wrong issuer", func(t *testing.T) { - err := VerifyCertExtensions(results, "foo", "owner", "owner/repo", "wrong") + 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") }) }