From aaea0166e2824f12454c8f7d785bd7481eddebe1 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Wed, 9 Oct 2024 16:51:00 -0400 Subject: [PATCH 1/2] If provided with zero attestations to verify, the LiveSigstoreVerifier.Verify func should return an error. --- pkg/cmd/attestation/verification/sigstore.go | 22 ++++++++++++++----- .../verification/sigstore_integration_test.go | 15 +++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/attestation/verification/sigstore.go b/pkg/cmd/attestation/verification/sigstore.go index ca4015db2..e237a3eb9 100644 --- a/pkg/cmd/attestation/verification/sigstore.go +++ b/pkg/cmd/attestation/verification/sigstore.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "crypto/x509" + "errors" "fmt" "os" @@ -48,6 +49,8 @@ type LiveSigstoreVerifier struct { config SigstoreConfig } +var ErrNoAttestationsVerified = errors.New("no attestations were verified") + // NewLiveSigstoreVerifier creates a new LiveSigstoreVerifier struct // that is used to verify artifacts and attestations against the // Public Good, GitHub, or a custom trusted root. @@ -170,18 +173,20 @@ func getLowestCertInChain(ca *root.CertificateAuthority) (*x509.Certificate, err } func (v *LiveSigstoreVerifier) Verify(attestations []*api.Attestation, policy verify.PolicyBuilder) *SigstoreResults { - // initialize the processing results before attempting to verify + // initialize the processing apResults before attempting to verify // with multiple verifiers - results := make([]*AttestationProcessingResult, len(attestations)) + apResults := make([]*AttestationProcessingResult, len(attestations)) for i, att := range attestations { apr := &AttestationProcessingResult{ Attestation: att, } - results[i] = apr + apResults[i] = apr } + var atLeastOneVerified bool + totalAttestations := len(attestations) - for i, apr := range results { + for i, apr := range apResults { v.config.Logger.VerbosePrintf("Verifying attestation %d/%d against the configured Sigstore trust roots\n", i+1, totalAttestations) // determine which verifier should attempt verification against the bundle @@ -212,10 +217,15 @@ func (v *LiveSigstoreVerifier) Verify(attestations []*api.Attestation, policy ve "SUCCESS - attestation signature verified with \"%s\"\n", issuer, )) apr.VerificationResult = result + atLeastOneVerified = true } - return &SigstoreResults{ - VerifyResults: results, + if atLeastOneVerified { + return &SigstoreResults{ + VerifyResults: apResults, + } + } else { + return &SigstoreResults{Error: ErrNoAttestationsVerified} } } diff --git a/pkg/cmd/attestation/verification/sigstore_integration_test.go b/pkg/cmd/attestation/verification/sigstore_integration_test.go index 97b44581e..1d3ec2d75 100644 --- a/pkg/cmd/attestation/verification/sigstore_integration_test.go +++ b/pkg/cmd/attestation/verification/sigstore_integration_test.go @@ -85,6 +85,21 @@ func TestLiveSigstoreVerifier(t *testing.T) { require.Len(t, res.VerifyResults, 0) require.ErrorContains(t, res.Error, "unsupported bundle version") }) + + t.Run("with no attestations", func(t *testing.T) { + attestations := []*api.Attestation{} + require.Len(t, attestations, 0) + + verifier := NewLiveSigstoreVerifier(SigstoreConfig{ + Logger: io.NewTestHandler(), + TrustedRoot: test.NormalizeRelativePath("../test/data/trusted_root.json"), + }) + + res := verifier.Verify(attestations, publicGoodPolicy(t)) + require.Len(t, res.VerifyResults, 0) + require.NotNil(t, res.Error) + }) + } func publicGoodPolicy(t *testing.T) verify.PolicyBuilder { From 28c2308458261f3961d25bde6bd7091062c41825 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 10 Oct 2024 11:22:22 -0400 Subject: [PATCH 2/2] While we're at it, let's ensure VerifyCertExtensions can't be tricked the same way. --- pkg/cmd/attestation/verification/extensions.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index 2ffb11a9d..94ba88208 100644 --- a/pkg/cmd/attestation/verification/extensions.go +++ b/pkg/cmd/attestation/verification/extensions.go @@ -16,12 +16,19 @@ 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 } + atLeastOneVerified = true + } + + if atLeastOneVerified { + return nil + } else { + return ErrNoAttestationsVerified } - return nil } func verifyCertExtensions(attestation *AttestationProcessingResult, tenant, owner, repo, issuer string) error {