Skip to content

Commit

Permalink
Better error messages.
Browse files Browse the repository at this point in the history
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
  • Loading branch information
vaikas committed Feb 23, 2023
1 parent cc5e8e7 commit fa59a94
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 31 deletions.
10 changes: 8 additions & 2 deletions cmd/cosign/cli/verify/verify_attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"

"github.com/google/go-containerregistry/pkg/name"
"github.com/sigstore/cosign/v2/cmd/cosign/cli/fulcio"
Expand Down Expand Up @@ -269,11 +270,16 @@ func (c *VerifyAttestationCommand) Exec(ctx context.Context, images []string) (e

var checked []oci.Signature
var validationErrors []error
// To aid in determining if there's a mismatch in what predicateType
// we're looking for and what we checked, keep track of them here so
// that we can help the user figure out if there's a typo, etc.
checkedPredicateTypes := []string{}
for _, vp := range verified {
payload, err := policy.AttestationToPayloadJSON(ctx, c.PredicateType, vp)
payload, gotPredicateType, err := policy.AttestationToPayloadJSON(ctx, c.PredicateType, vp)
if err != nil {
return fmt.Errorf("converting to consumable policy validation: %w", err)
}
checkedPredicateTypes = append(checkedPredicateTypes, gotPredicateType)
if len(payload) == 0 {
// This is not the predicate type we're looking for.
continue
Expand Down Expand Up @@ -309,7 +315,7 @@ func (c *VerifyAttestationCommand) Exec(ctx context.Context, images []string) (e
}

if len(checked) == 0 {
return fmt.Errorf("none of the attestations matched the predicate type: %s", c.PredicateType)
return fmt.Errorf("none of the attestations matched the predicate type: %s, found: %s", c.PredicateType, strings.Join(checkedPredicateTypes, ","))
}

// TODO: add CUE validation report to `PrintVerificationHeader`.
Expand Down
4 changes: 2 additions & 2 deletions cmd/cosign/cli/verify/verify_blob_attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,8 @@ func (c *VerifyBlobAttestationCommand) Exec(ctx context.Context, artifactPath st

// This checks the predicate type -- if no error is returned and no payload is, then
// the attestation is not of the given predicate type.
if b, err := policy.AttestationToPayloadJSON(ctx, c.PredicateType, signature); b == nil && err == nil {
return fmt.Errorf("invalid predicate type, expected %s", c.PredicateType)
if b, gotPredicateType, err := policy.AttestationToPayloadJSON(ctx, c.PredicateType, signature); b == nil && err == nil {
return fmt.Errorf("invalid predicate type, expected %s got %s", c.PredicateType, gotPredicateType)
}

fmt.Fprintln(os.Stderr, "Verified OK")
Expand Down
6 changes: 6 additions & 0 deletions cmd/cosign/cli/verify/verify_blob_attestation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ func TestVerifyBlobAttestation(t *testing.T) {
predicateType: "slsaprovenance",
signature: dssePredicateMultipleSubjects,
blobPath: blobPath,
}, {
description: "dsse envelope has multiple subjects, one is valid, but we are looking for different predicatetype",
predicateType: "notreallyslsaprovenance",
signature: dssePredicateMultipleSubjects,
blobPath: blobPath,
shouldErr: true,
}, {
description: "dsse envelope has multiple subjects, none has correct sha256 digest",
predicateType: "slsaprovenance",
Expand Down
56 changes: 34 additions & 22 deletions pkg/policy/attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,40 +38,52 @@ import (
//
// If there's no error, and payload is empty means the predicateType did not
// match the attestation.
func AttestationToPayloadJSON(ctx context.Context, predicateType string, verifiedAttestation oci.Signature) ([]byte, error) {
// Returns the attestation type (PredicateType) if the payload was decoded
// before the error happened, or in the case the predicateType that was
// requested does not match. This is useful for callers to be able to provide
// better error messages. For example, if there's a typo in the predicateType,
// or the predicateType is not the one they are looking for. Without returning
// this, it's hard for users to know which attestations/predicateTypes were
// inspected.
func AttestationToPayloadJSON(ctx context.Context, predicateType string, verifiedAttestation oci.Signature) ([]byte, string, error) {
if predicateType == "" {
return nil, errors.New("missing predicate type")
return nil, "", errors.New("missing predicate type")
}
predicateURI, ok := options.PredicateTypeMap[predicateType]
if !ok {
// Not a custom one, use it as is.
predicateURI = predicateType
}
var payloadData map[string]interface{}

p, err := verifiedAttestation.Payload()
if err != nil {
return nil, fmt.Errorf("getting payload: %w", err)
return nil, "", fmt.Errorf("getting payload: %w", err)
}

err = json.Unmarshal(p, &payloadData)
if err != nil {
return nil, fmt.Errorf("unmarshaling payload data")
return nil, "", fmt.Errorf("unmarshaling payload data")
}

var decodedPayload []byte
if val, ok := payloadData["payload"]; ok {
decodedPayload, err = base64.StdEncoding.DecodeString(val.(string))
if err != nil {
return nil, fmt.Errorf("decoding payload: %w", err)
return nil, "", fmt.Errorf("decoding payload: %w", err)
}
} else {
return nil, fmt.Errorf("could not find payload in payload data")
return nil, "", fmt.Errorf("could not find payload in payload data")
}

// Only apply the policy against the requested predicate type
var statement in_toto.Statement
if err := json.Unmarshal(decodedPayload, &statement); err != nil {
return nil, fmt.Errorf("unmarshal in-toto statement: %w", err)
return nil, "", fmt.Errorf("unmarshal in-toto statement: %w", err)
}
if statement.PredicateType != predicateType {
if statement.PredicateType != predicateURI {
// This is not the predicate we're looking for, so skip it.
return nil, nil
return nil, statement.PredicateType, nil
}

// NB: In many (all?) of these cases, we could just return the
Expand All @@ -82,59 +94,59 @@ func AttestationToPayloadJSON(ctx context.Context, predicateType string, verifie
case options.PredicateCustom:
payload, err = json.Marshal(statement)
if err != nil {
return nil, fmt.Errorf("generating CosignStatement: %w", err)
return nil, statement.PredicateType, fmt.Errorf("generating CosignStatement: %w", err)
}
case options.PredicateLink:
var linkStatement in_toto.LinkStatement
if err := json.Unmarshal(decodedPayload, &linkStatement); err != nil {
return nil, fmt.Errorf("unmarshaling LinkStatement: %w", err)
return nil, statement.PredicateType, fmt.Errorf("unmarshaling LinkStatement: %w", err)
}
payload, err = json.Marshal(linkStatement)
if err != nil {
return nil, fmt.Errorf("marshaling LinkStatement: %w", err)
return nil, statement.PredicateType, fmt.Errorf("marshaling LinkStatement: %w", err)
}
case options.PredicateSLSA:
var slsaProvenanceStatement in_toto.ProvenanceStatement
if err := json.Unmarshal(decodedPayload, &slsaProvenanceStatement); err != nil {
return nil, fmt.Errorf("unmarshaling ProvenanceStatement): %w", err)
return nil, statement.PredicateType, fmt.Errorf("unmarshaling ProvenanceStatement): %w", err)
}
payload, err = json.Marshal(slsaProvenanceStatement)
if err != nil {
return nil, fmt.Errorf("marshaling ProvenanceStatement: %w", err)
return nil, statement.PredicateType, fmt.Errorf("marshaling ProvenanceStatement: %w", err)
}
case options.PredicateSPDX, options.PredicateSPDXJSON:
var spdxStatement in_toto.SPDXStatement
if err := json.Unmarshal(decodedPayload, &spdxStatement); err != nil {
return nil, fmt.Errorf("unmarshaling SPDXStatement: %w", err)
return nil, statement.PredicateType, fmt.Errorf("unmarshaling SPDXStatement: %w", err)
}
payload, err = json.Marshal(spdxStatement)
if err != nil {
return nil, fmt.Errorf("marshaling SPDXStatement: %w", err)
return nil, statement.PredicateType, fmt.Errorf("marshaling SPDXStatement: %w", err)
}
case options.PredicateCycloneDX:
var cyclonedxStatement in_toto.CycloneDXStatement
if err := json.Unmarshal(decodedPayload, &cyclonedxStatement); err != nil {
return nil, fmt.Errorf("unmarshaling CycloneDXStatement: %w", err)
return nil, statement.PredicateType, fmt.Errorf("unmarshaling CycloneDXStatement: %w", err)
}
payload, err = json.Marshal(cyclonedxStatement)
if err != nil {
return nil, fmt.Errorf("marshaling CycloneDXStatement: %w", err)
return nil, statement.PredicateType, fmt.Errorf("marshaling CycloneDXStatement: %w", err)
}
case options.PredicateVuln:
var vulnStatement attestation.CosignVulnStatement
if err := json.Unmarshal(decodedPayload, &vulnStatement); err != nil {
return nil, fmt.Errorf("unmarshaling CosignVulnStatement: %w", err)
return nil, statement.PredicateType, fmt.Errorf("unmarshaling CosignVulnStatement: %w", err)
}
payload, err = json.Marshal(vulnStatement)
if err != nil {
return nil, fmt.Errorf("marshaling CosignVulnStatement: %w", err)
return nil, statement.PredicateType, fmt.Errorf("marshaling CosignVulnStatement: %w", err)
}
default:
// Valid URI type reaches here.
payload, err = json.Marshal(statement)
if err != nil {
return nil, fmt.Errorf("generating Statement: %w", err)
return nil, statement.PredicateType, fmt.Errorf("generating Statement: %w", err)
}
}
return payload, nil
return payload, statement.PredicateType, nil
}
11 changes: 6 additions & 5 deletions pkg/policy/attestation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,7 @@ func TestFailures(t *testing.T) {
payload string
predicateType string
wantErrSubstring string
}{{payload: "", predicateType: "notvalidpredicate", wantErrSubstring: "invalid predicate type"},
{payload: "", wantErrSubstring: "unmarshaling payload data"}, {payload: "{badness", wantErrSubstring: "unmarshaling payload data"},
}{{payload: "", wantErrSubstring: "unmarshaling payload data"}, {payload: "{badness", wantErrSubstring: "unmarshaling payload data"},
{payload: `{"payloadType":"notmarshallable}`, wantErrSubstring: "unmarshaling payload data"},
{payload: `{"payload":"shou!ln'twork"}`, wantErrSubstring: "decoding payload"},
{payload: `{"payloadType":"finebutnopayload"}`, wantErrSubstring: "could not find payload"},
Expand All @@ -119,7 +118,7 @@ func TestFailures(t *testing.T) {
if predicateType == "" {
predicateType = "custom"
}
_, err = AttestationToPayloadJSON(context.TODO(), predicateType, att)
_, _, err = AttestationToPayloadJSON(context.TODO(), predicateType, att)
checkFailure(t, tc.wantErrSubstring, err)
}
}
Expand All @@ -130,7 +129,7 @@ func TestFailures(t *testing.T) {
// constructing different attestations there.
func TestErroringPayload(t *testing.T) {
// Payload() call fails
_, err := AttestationToPayloadJSON(context.TODO(), "custom", &failingAttestation{})
_, _, err := AttestationToPayloadJSON(context.TODO(), "custom", &failingAttestation{})
checkFailure(t, "inducing test failure", err)
}
func TestAttestationToPayloadJson(t *testing.T) {
Expand All @@ -142,7 +141,7 @@ func TestAttestationToPayloadJson(t *testing.T) {
if err != nil {
t.Fatal("Failed to create static.NewSignature: ", err)
}
jsonBytes, err := AttestationToPayloadJSON(context.TODO(), fileName, ociSig)
jsonBytes, gotPredicateType, err := AttestationToPayloadJSON(context.TODO(), fileName, ociSig)
if err != nil {
t.Fatalf("Failed to convert : %s", err)
}
Expand All @@ -153,12 +152,14 @@ func TestAttestationToPayloadJson(t *testing.T) {
t.Fatalf("[%s] Wanted custom statement, can't unmarshal to it: %v", fileName, err)
}
checkPredicateType(t, attestation.CosignCustomProvenanceV01, intoto.PredicateType)
checkPredicateType(t, gotPredicateType, intoto.PredicateType)
case "vuln":
var vulnStatement attestation.CosignVulnStatement
if err := json.Unmarshal(jsonBytes, &vulnStatement); err != nil {
t.Fatalf("[%s] Wanted vuln statement, can't unmarshal to it: %v", fileName, err)
}
checkPredicateType(t, attestation.CosignVulnProvenanceV01, vulnStatement.PredicateType)
checkPredicateType(t, gotPredicateType, vulnStatement.PredicateType)
case "default":
t.Fatal("non supported predicate file")
}
Expand Down

0 comments on commit fa59a94

Please sign in to comment.