Merge pull request #9877 from malancas/simplify-sigstore-verify-result-handling

Simplify Sigstore verification result handling in `gh attestation verify`
This commit is contained in:
Meredith Lancaster 2024-11-06 09:38:07 -07:00 committed by GitHub
commit 2f47d1c81c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 34 additions and 49 deletions

View file

@ -141,9 +141,9 @@ func runInspect(opts *Options) error {
return fmt.Errorf("failed to build policy: %v", err)
}
res := opts.SigstoreVerifier.Verify(attestations, policy)
if res.Error != nil {
return fmt.Errorf("at least one attestation failed to verify against Sigstore: %v", res.Error)
results, err := opts.SigstoreVerifier.Verify(attestations, policy)
if err != nil {
return fmt.Errorf("at least one attestation failed to verify against Sigstore: %v", err)
}
opts.Logger.VerbosePrint(opts.Logger.ColorScheme.Green(
@ -152,7 +152,7 @@ func runInspect(opts *Options) error {
// If the user provides the --format=json flag, print the results in JSON format
if opts.exporter != nil {
details, err := getAttestationDetails(opts.Tenant, res.VerifyResults)
details, err := getAttestationDetails(opts.Tenant, results)
if err != nil {
return fmt.Errorf("failed to get attestation detail: %v", err)
}
@ -165,7 +165,7 @@ func runInspect(opts *Options) error {
}
// otherwise, print results in a table
details, err := getDetailsAsSlice(opts.Tenant, res.VerifyResults)
details, err := getDetailsAsSlice(opts.Tenant, results)
if err != nil {
return fmt.Errorf("failed to parse attestation details: %v", err)
}

View file

@ -16,7 +16,7 @@ type MockSigstoreVerifier struct {
t *testing.T
}
func (v *MockSigstoreVerifier) Verify(attestations []*api.Attestation, policy verify.PolicyBuilder) *SigstoreResults {
func (v *MockSigstoreVerifier) Verify(attestations []*api.Attestation, policy verify.PolicyBuilder) ([]*AttestationProcessingResult, error) {
statement := &in_toto.Statement{}
statement.PredicateType = SLSAPredicateV1
@ -41,9 +41,7 @@ func (v *MockSigstoreVerifier) Verify(attestations []*api.Attestation, policy ve
results := []*AttestationProcessingResult{&result}
return &SigstoreResults{
VerifyResults: results,
}
return results, nil
}
func NewMockSigstoreVerifier(t *testing.T) *MockSigstoreVerifier {
@ -52,8 +50,6 @@ func NewMockSigstoreVerifier(t *testing.T) *MockSigstoreVerifier {
type FailSigstoreVerifier struct{}
func (v *FailSigstoreVerifier) Verify(attestations []*api.Attestation, policy verify.PolicyBuilder) *SigstoreResults {
return &SigstoreResults{
Error: fmt.Errorf("failed to verify attestations"),
}
func (v *FailSigstoreVerifier) Verify(attestations []*api.Attestation, policy verify.PolicyBuilder) ([]*AttestationProcessingResult, error) {
return nil, fmt.Errorf("failed to verify attestations")
}

View file

@ -28,11 +28,6 @@ type AttestationProcessingResult struct {
VerificationResult *verify.VerificationResult `json:"verificationResult"`
}
type SigstoreResults struct {
VerifyResults []*AttestationProcessingResult
Error error
}
type SigstoreConfig struct {
TrustedRoot string
Logger *io.Handler
@ -42,7 +37,7 @@ type SigstoreConfig struct {
}
type SigstoreVerifier interface {
Verify(attestations []*api.Attestation, policy verify.PolicyBuilder) *SigstoreResults
Verify(attestations []*api.Attestation, policy verify.PolicyBuilder) ([]*AttestationProcessingResult, error)
}
type LiveSigstoreVerifier struct {
@ -172,7 +167,7 @@ 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) *SigstoreResults {
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))
@ -192,9 +187,7 @@ func (v *LiveSigstoreVerifier) Verify(attestations []*api.Attestation, policy ve
// determine which verifier should attempt verification against the bundle
verifier, issuer, err := v.chooseVerifier(apr.Attestation.Bundle)
if err != nil {
return &SigstoreResults{
Error: fmt.Errorf("failed to find recognized issuer from bundle content: %v", err),
}
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)
@ -206,9 +199,7 @@ func (v *LiveSigstoreVerifier) Verify(attestations []*api.Attestation, policy ve
"Failed to verify against issuer \"%s\" \n\n", issuer,
))
return &SigstoreResults{
Error: fmt.Errorf("verifying with issuer \"%s\"", issuer),
}
return nil, fmt.Errorf("verifying with issuer \"%s\"", issuer)
}
// if verification is successful, add the result
@ -221,12 +212,10 @@ func (v *LiveSigstoreVerifier) Verify(attestations []*api.Attestation, policy ve
}
if atLeastOneVerified {
return &SigstoreResults{
VerifyResults: apResults,
}
} else {
return &SigstoreResults{Error: ErrNoAttestationsVerified}
return apResults, nil
}
return nil, ErrNoAttestationsVerified
}
func newCustomVerifier(trustedRoot *root.TrustedRoot) (*verify.SignedEntityVerifier, error) {

View file

@ -52,15 +52,15 @@ func TestLiveSigstoreVerifier(t *testing.T) {
Logger: io.NewTestHandler(),
})
res := verifier.Verify(tc.attestations, publicGoodPolicy(t))
results, err := verifier.Verify(tc.attestations, publicGoodPolicy(t))
if tc.expectErr {
require.Error(t, res.Error, "test case: %s", tc.name)
require.ErrorContains(t, res.Error, tc.errContains, "test case: %s", tc.name)
require.Nil(t, res.VerifyResults, "test case: %s", tc.name)
require.Error(t, err, "test case: %s", tc.name)
require.ErrorContains(t, err, tc.errContains, "test case: %s", tc.name)
require.Nil(t, results, "test case: %s", tc.name)
} else {
require.Equal(t, len(tc.attestations), len(res.VerifyResults), "test case: %s", tc.name)
require.NoError(t, res.Error, "test case: %s", tc.name)
require.Equal(t, len(tc.attestations), len(results), "test case: %s", tc.name)
require.NoError(t, err, "test case: %s", tc.name)
}
}
@ -77,9 +77,9 @@ func TestLiveSigstoreVerifier(t *testing.T) {
Logger: io.NewTestHandler(),
})
res := verifier.Verify(attestations, githubPolicy)
require.Len(t, res.VerifyResults, 1)
require.NoError(t, res.Error)
results, err := verifier.Verify(attestations, githubPolicy)
require.Len(t, results, 1)
require.NoError(t, err)
})
t.Run("with custom trusted root", func(t *testing.T) {
@ -90,9 +90,9 @@ func TestLiveSigstoreVerifier(t *testing.T) {
TrustedRoot: test.NormalizeRelativePath("../test/data/trusted_root.json"),
})
res := verifier.Verify(attestations, publicGoodPolicy(t))
require.Len(t, res.VerifyResults, 2)
require.NoError(t, res.Error)
results, err := verifier.Verify(attestations, publicGoodPolicy(t))
require.Len(t, results, 2)
require.NoError(t, err)
})
}

View file

@ -275,14 +275,14 @@ func runVerify(opts *Options) error {
return err
}
sigstoreRes := opts.SigstoreVerifier.Verify(attestations, sp)
if sigstoreRes.Error != nil {
verifyResults, err := opts.SigstoreVerifier.Verify(attestations, sp)
if err != nil {
opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Sigstore verification failed"))
return sigstoreRes.Error
return err
}
// Verify extensions
if err := verification.VerifyCertExtensions(sigstoreRes.VerifyResults, ec); err != nil {
if err := verification.VerifyCertExtensions(verifyResults, ec); err != nil {
opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Policy verification failed"))
return err
}
@ -292,7 +292,7 @@ func runVerify(opts *Options) error {
// If an exporter is provided with the --json flag, write the results to the terminal in JSON format
if opts.exporter != nil {
// print the results to the terminal as an array of JSON objects
if err = opts.exporter.Write(opts.Logger.IO, sigstoreRes.VerifyResults); err != nil {
if err = opts.exporter.Write(opts.Logger.IO, verifyResults); err != nil {
opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to write JSON output"))
return err
}
@ -302,7 +302,7 @@ func runVerify(opts *Options) error {
opts.Logger.Printf("%s was attested by:\n", artifact.DigestWithAlg())
// Otherwise print the results to the terminal in a table
tableContent, err := buildTableVerifyContent(opts.Tenant, sigstoreRes.VerifyResults)
tableContent, err := buildTableVerifyContent(opts.Tenant, verifyResults)
if err != nil {
opts.Logger.Println(opts.Logger.ColorScheme.Red("failed to parse results"))
return err