cli · malancas · Nov 7, 2024 · Oct 31, 2024 · Oct 31, 2024 · Oct 31, 2024
@@ -18,19 +18,20 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, ec Enforcement
 		return errors.New("no attestations proccessing results")
 	}
 
-	var atLeastOneVerified bool
+	var lastErr error
 	for _, attestation := range results {
-		if err := verifyCertExtensions(*attestation.VerificationResult.Signature.Certificate, ec); err != nil {
-			return err
+		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
+			return nil
 		}
-		atLeastOneVerified = true
+		lastErr = err
 	}
 
-	if atLeastOneVerified {
-		return nil
-	} else {
-		return ErrNoAttestationsVerified
-	}
+	// if we have exited the for loop without returning early due to successful
+	// verification, we need to return an error
+	return lastErr
 }
 
 func verifyCertExtensions(verifiedCert certificate.Summary, criteria EnforcementCriteria) error {

@@ -8,22 +8,24 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestVerifyCertExtensions(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",
-						},
+func createSampleResult() *AttestationProcessingResult {
+	return &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",
 					},
 				},
 			},
 		},
 	}
+}
+
+func TestVerifyCertExtensions(t *testing.T) {
+	results := []*AttestationProcessingResult{createSampleResult()}
 
 	certSummary := certificate.Summary{}
 	certSummary.SourceRepositoryOwnerURI = "https://github.com/owner"
@@ -34,11 +36,30 @@ func TestVerifyCertExtensions(t *testing.T) {
 		Certificate: certSummary,
 	}
 
-	t.Run("success", func(t *testing.T) {
+	t.Run("passes with one result", func(t *testing.T) {
 		err := VerifyCertExtensions(results, c)
 		require.NoError(t, err)
 	})
 
+	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"
+
+		err := VerifyCertExtensions(twoResults, c)
+		require.NoError(t, err)
+	})
+
+	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, c)
+		require.Error(t, err)
+	})
+
 	t.Run("with wrong SourceRepositoryOwnerURI", func(t *testing.T) {
 		expectedCriteria := c
 		expectedCriteria.Certificate.SourceRepositoryOwnerURI = "https://github.com/wrong"

@@ -167,55 +167,68 @@ func getLowestCertInChain(ca *root.CertificateAuthority) (*x509.Certificate, err
 	return nil, fmt.Errorf("certificate authority had no certificates")
 }
 
-func (v *LiveSigstoreVerifier) Verify(attestations []*api.Attestation, policy verify.PolicyBuilder) ([]*AttestationProcessingResult, error) {
-	// initialize the processing apResults before attempting to verify
-	// with multiple verifiers
-	apResults := make([]*AttestationProcessingResult, len(attestations))
-	for i, att := range attestations {
-		apr := &AttestationProcessingResult{
-			Attestation: att,
-		}
-		apResults[i] = apr
+func (v *LiveSigstoreVerifier) verify(attestation *api.Attestation, policy verify.PolicyBuilder) (*AttestationProcessingResult, error) {
+	// determine which verifier should attempt verification against the bundle
+	verifier, issuer, err := v.chooseVerifier(attestation.Bundle)
+	if err != nil {
+		return nil, fmt.Errorf("failed to find recognized issuer from bundle content: %v", err)
+	}
+
+	v.config.Logger.VerbosePrintf("Attempting verification against issuer \"%s\"\n", issuer)
+	// attempt to verify the attestation
+	result, err := verifier.Verify(attestation.Bundle, policy)
+	// if verification fails, create the error and exit verification early
+	if err != nil {
+		v.config.Logger.VerbosePrint(v.config.Logger.ColorScheme.Redf(
+			"Failed to verify against issuer \"%s\" \n\n", issuer,
+		))
+
+		return nil, fmt.Errorf("verifying with issuer \"%s\"", issuer)
 	}
 
-	var atLeastOneVerified bool
+	// if verification is successful, add the result
+	// to the AttestationProcessingResult entry
+	v.config.Logger.VerbosePrint(v.config.Logger.ColorScheme.Greenf(
+		"SUCCESS - attestation signature verified with \"%s\"\n", issuer,
+	))
+
+	return &AttestationProcessingResult{
+		Attestation:        attestation,
+		VerificationResult: result,
+	}, nil
+}
+
+func (v *LiveSigstoreVerifier) Verify(attestations []*api.Attestation, policy verify.PolicyBuilder) ([]*AttestationProcessingResult, error) {
+	if len(attestations) == 0 {
+		return nil, ErrNoAttestationsVerified
+	}
 
+	results := make([]*AttestationProcessingResult, len(attestations))
+	var verifyCount int
+	var lastError error
 	totalAttestations := len(attestations)
-	for i, apr := range apResults {
+	for i, a := range attestations {
 		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
-		verifier, issuer, err := v.chooseVerifier(apr.Attestation.Bundle)
-		if err != nil {
-			return nil, fmt.Errorf("failed to find recognized issuer from bundle content: %v", err)
-		}
-
-		v.config.Logger.VerbosePrintf("Attempting verification against issuer \"%s\"\n", issuer)
-		// attempt to verify the attestation
-		result, err := verifier.Verify(apr.Attestation.Bundle, policy)
-		// if verification fails, create the error and exit verification early
+		apr, err := v.verify(a, policy)
 		if err != nil {
-			v.config.Logger.VerbosePrint(v.config.Logger.ColorScheme.Redf(
-				"Failed to verify against issuer \"%s\" \n\n", issuer,
-			))
-
-			return nil, fmt.Errorf("verifying with issuer \"%s\"", issuer)
+			lastError = err
+			// move onto the next attestation in the for loop if verification fails
+			continue
 		}
-
-		// if verification is successful, add the result
-		// to the AttestationProcessingResult entry
-		v.config.Logger.VerbosePrint(v.config.Logger.ColorScheme.Greenf(
-			"SUCCESS - attestation signature verified with \"%s\"\n", issuer,
-		))
-		apr.VerificationResult = result
-		atLeastOneVerified = true
+		// otherwise, add the result to the results slice and increment verifyCount
+		results[verifyCount] = apr
+		verifyCount++
 	}
 
-	if atLeastOneVerified {
-		return apResults, nil
+	if verifyCount == 0 {
+		return nil, lastError
 	}
 
-	return nil, ErrNoAttestationsVerified
+	// truncate the results slice to only include verified attestations
+	results = results[:verifyCount]
+
+	return results, nil
 }
 
 func newCustomVerifier(trustedRoot *root.TrustedRoot) (*verify.SignedEntityVerifier, error) {

@@ -64,6 +64,37 @@ func TestLiveSigstoreVerifier(t *testing.T) {
 		}
 	}
 
+	t.Run("with 2/3 verified attestations", func(t *testing.T) {
+		verifier := NewLiveSigstoreVerifier(SigstoreConfig{
+			Logger: io.NewTestHandler(),
+		})
+
+		invalidBundle := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0-bundle-v0.1.json")
+		attestations := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0_with_2_bundles.jsonl")
+		attestations = append(attestations, invalidBundle[0])
+		require.Len(t, attestations, 3)
+
+		results, err := verifier.Verify(attestations, publicGoodPolicy(t))
+
+		require.Len(t, results, 2)
+		require.NoError(t, err)
+	})
+
+	t.Run("fail with 0/2 verified attestations", func(t *testing.T) {
+		verifier := NewLiveSigstoreVerifier(SigstoreConfig{
+			Logger: io.NewTestHandler(),
+		})
+
+		invalidBundle := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0-bundle-v0.1.json")
+		attestations := getAttestationsFor(t, "../test/data/sigstoreBundle-invalid-signature.json")
+		attestations = append(attestations, invalidBundle[0])
+		require.Len(t, attestations, 2)
+
+		results, err := verifier.Verify(attestations, publicGoodPolicy(t))
+		require.Nil(t, results)
+		require.Error(t, err)
+	})
+
 	t.Run("with GitHub Sigstore artifact", func(t *testing.T) {
 		githubArtifactPath := test.NormalizeRelativePath("../test/data/github_provenance_demo-0.0.12-py3-none-any.whl")
 		githubArtifact, err := artifact.NewDigestedArtifact(nil, githubArtifactPath, "sha256")