Loosen verification predicate type + better error messages (#2737)

* test policy-controller only. Signed-off-by: Ville Aikas <vaikas@chainguard.dev> * Better error messages. Signed-off-by: Ville Aikas <vaikas@chainguard.dev> --------- Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
sigstore · Feb 23, 2023 · d6b9001 · d6b9001
1 parent f9d8e5d
commit d6b9001
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 35 deletions.
diff --git a/cmd/cosign/cli/verify/verify_attestation.go b/cmd/cosign/cli/verify/verify_attestation.go
@@ -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"
@@ -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
@@ -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`.

diff --git a/cmd/cosign/cli/verify/verify_blob_attestation.go b/cmd/cosign/cli/verify/verify_blob_attestation.go
@@ -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")

diff --git a/cmd/cosign/cli/verify/verify_blob_attestation_test.go b/cmd/cosign/cli/verify/verify_blob_attestation_test.go
@@ -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",

diff --git a/pkg/policy/attestation.go b/pkg/policy/attestation.go
@@ -19,6 +19,7 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 
 	"github.com/in-toto/in-toto-golang/in_toto"
@@ -37,44 +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) {
-	// Check the predicate up front, no point in wasting time if it's invalid.
-	predicateURI, err := options.ParsePredicateType(predicateType)
-
-	if err != nil {
-		return nil, fmt.Errorf("invalid predicate type: %s", predicateType)
+// 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")
+	}
+	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 != 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
@@ -85,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
 }
diff --git a/pkg/policy/attestation_test.go b/pkg/policy/attestation_test.go
@@ -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"},
@@ -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)
 	}
 }
@@ -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) {
@@ -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)
 		}
@@ -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")
 		}