Fix handling of policy in verify-attestation

cmd/cosign/cli/verify/verify_attestation.go

@@ -195,22 +195,6 @@ func (c *VerifyAttestationCommand) Exec(ctx context.Context, images []string) (e
 				return errors.Wrap(err, "unmarshal payload data")
 			}
 
-			predicateURI, ok := options.PredicateTypeMap[c.PredicateType]
-			if !ok {
-				return fmt.Errorf("invalid predicate type: %s", c.PredicateType)
-			}
-
-			// sanity checks
-			if val, ok := payloadData["payloadType"]; ok {
-				// we need to check only given type from the cli flag
-				// so we are skipping other types
-				if predicateURI != val {
-					continue
-				}
-			} else {
-				return fmt.Errorf("could not find 'payloadType' in payload data")
-			}
-
 			var decodedPayload []byte
 			if val, ok := payloadData["payload"]; ok {
 				decodedPayload, err = base64.StdEncoding.DecodeString(val.(string))
@@ -221,14 +205,24 @@ func (c *VerifyAttestationCommand) Exec(ctx context.Context, images []string) (e
 				return fmt.Errorf("could not find 'payload' in payload data")
 			}
 
+			predicateURI, ok := options.PredicateTypeMap[c.PredicateType]
+			if !ok {
+				return fmt.Errorf("invalid predicate type: %s", c.PredicateType)
+			}
+
+			// Only apply the policy against the requested predicate type
+			var statement in_toto.Statement
+			if err := json.Unmarshal(decodedPayload, &statement); err != nil {
+				return fmt.Errorf("unmarshal in-toto statement: %w", err)
+			}
+			if statement.PredicateType != predicateURI {
+				continue
+			}
+
 			var payload []byte
 			switch c.PredicateType {
 			case options.PredicateCustom:
-				var cosignStatement in_toto.Statement
-				if err := json.Unmarshal(decodedPayload, &cosignStatement); err != nil {
-					return fmt.Errorf("unmarshal CosignStatement: %w", err)
-				}
-				payload, err = json.Marshal(cosignStatement)
+				payload, err = json.Marshal(statement)
 				if err != nil {
 					return fmt.Errorf("error when generating CosignStatement: %w", err)
 				}
@@ -259,6 +253,8 @@ func (c *VerifyAttestationCommand) Exec(ctx context.Context, images []string) (e
 				if err != nil {
 					return fmt.Errorf("error when generating SPDXStatement: %w", err)
 				}
+			default:
+				return fmt.Errorf("unsupported predicate type: %s", c.PredicateType)
 			}
 
 			if len(cuePolicies) > 0 {

pkg/cosign/rego/rego.go

@@ -24,11 +24,16 @@ import (
 	"github.com/open-policy-agent/opa/rego"
 )
 
+// The query below should meet the following requirements:
+// * Provides no Bindings. Do not use a query that sets a variable, e.g. x := data.signature.allow
+// * Queries for a single value.
+const QUERY = "data.signature.allow"
+
 func ValidateJSON(jsonBody []byte, entrypoints []string) []error {
 	ctx := context.Background()
 
 	r := rego.New(
-		rego.Query("data.signature.deny"), // hardcoded, ? data.cosign.allow→
+		rego.Query(QUERY),
 		rego.Load(entrypoints, nil))
 
 	query, err := r.PrepareForEval(ctx)
@@ -48,21 +53,23 @@ func ValidateJSON(jsonBody []byte, entrypoints []string) []error {
 		return []error{err}
 	}
 
+	// Ensure the resultset contains a single result where the Expression contains a single value
+	// which is true and there are no Bindings.
 	if rs.Allowed() {
 		return nil
 	}
 
 	var errs []error
 	for _, result := range rs {
 		for _, expression := range result.Expressions {
-			for _, v := range expression.Value.([]interface{}) {
-				if s, ok := v.(string); ok {
-					errs = append(errs, fmt.Errorf(s))
-				} else {
-					errs = append(errs, fmt.Errorf("%s", v))
-				}
-			}
+			errs = append(errs, fmt.Errorf("expression value, %v, is not true", expression))
 		}
 	}
+
+	// When rs.Allowed() is not true and len(rs) is 0, the result is undefined. This is a policy
+	// check failure.
+	if len(errs) == 0 {
+		errs = append(errs, fmt.Errorf("result is undefined for query '%s'", QUERY))
+	}
 	return errs
 }

pkg/cosign/rego/rego_test.go

@@ -0,0 +1,100 @@
+//
+// Copyright 2022 The Sigstore Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package rego
+
+import (
+	"fmt"
+	"os"
+	"testing"
+)
+
+const simpleJSONBody = `{
+	"_type": "https://in-toto.io/Statement/v0.1",
+	"predicateType": "https://slsa.dev/provenance/v0.2"
+}`
+
+func TestValidationJSON(t *testing.T) {
+	cases := []struct {
+		name     string
+		jsonBody string
+		policy   string
+		pass     bool
+		errors   []string
+	}{
+		{
+			name:     "passing policy",
+			jsonBody: simpleJSONBody,
+			policy: `
+				package signature
+
+				allow {
+					input.predicateType == "https://slsa.dev/provenance/v0.2"
+				}
+			`,
+			pass: true,
+		},
+		{
+			name:     "undefined result due to no matching rules",
+			jsonBody: simpleJSONBody,
+			policy: `
+				package signature
+
+				allow {
+					input.predicateType == "https://slsa.dev/provenance/v99.9"
+				}
+			`,
+			pass:   false,
+			errors: []string{"result is undefined for query 'data.signature.allow'"},
+		},
+		{
+			name:     "policy query evaluates to false",
+			jsonBody: simpleJSONBody,
+			policy: `
+				package signature
+
+				default allow = false
+			`,
+			pass:   false,
+			errors: []string{"expression value, false, is not true"},
+		},
+	}
+
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			// Do not use t.TempDir() because rego has issues loading policy files
+			// from an absolute path on Windows. Use a relative path instead. See:
+			//	https://github.com/open-policy-agent/opa/issues/4521
+			policyFileName := "tmp-policy.rego"
+			if err := os.WriteFile(policyFileName, []byte(tt.policy), 0644); err != nil {
+				t.Fatal(err)
+			}
+			defer os.Remove(policyFileName)
+
+			if errs := ValidateJSON([]byte(tt.jsonBody), []string{policyFileName}); (errs == nil) != tt.pass {
+				t.Fatalf("Unexpected result: %v", errs)
+			} else if errs != nil {
+				if len(errs) != len(tt.errors) {
+					t.Fatalf("Expected %d errors, got %d errors: %v", len(tt.errors), len(errs), errs)
+				}
+				for i, err := range errs {
+					if fmt.Sprintf("%s", err) != tt.errors[i] {
+						t.Errorf("Expected error %q, got %q", tt.errors[i], err)
+					}
+				}
+			}
+		})
+	}
+}