[TEP-0091] use verification mode in trusted resources

docs/trusted-resources.md

@@ -78,7 +78,8 @@ How does VerificationPolicy work?
 You can create multiple `VerificationPolicy` and apply them to the cluster.
 1. Trusted resources will look up policies from the resource namespace (usually this is the same as taskrun/pipelinerun namespace).
 2. If multiple policies are found. For each policy we will check if the resource url is matching any of the `patterns` in the `resources` list. If matched then this policy will be used for verification.
-3. If multiple policies are matched, the resource needs to pass all of them to pass verification.
+3. If multiple policies are matched, the resource must pass all the "enforce" mode policies. If the resource only matches policies in "warn" mode and fails to pass the "warn" policy, it will not fail the taskrun or pipelinerun, but log a warning instead.
+
 4. To pass one policy, the resource can pass any public keys in the policy.
 
 Take the following `VerificationPolicies` for example, a resource from "https://github.com/tektoncd/catalog.git", needs to pass both `verification-policy-a` and `verification-policy-b`, to pass `verification-policy-a` the resource needs to pass either `key1` or `key2`.
@@ -109,6 +110,8 @@ spec:
       key:
         # data stores the inline public key data
         data: "STRING_ENCODED_PUBLIC_KEY"
+  # mode can be set to "enforce" (default) or "warn".
+  mode: enforce
 ```
 
 ```yaml
@@ -141,6 +144,9 @@ To learn more about `ConfigSource` please refer to resolvers doc for more contex
 
 `hashAlgorithm` is the algorithm for the public key, by default is `sha256`. It also supports `SHA224`, `SHA384`, `SHA512`.
 
+`mode` controls whether a failing policy will fail the taskrun/pipelinerun, or only log the a warning
+ * enforce (default) - fail the taskrun/pipelinerun if verification fails
+ * warn - don't fail the taskrun/pipelinerun if verification fails but log a warning
 
 #### Migrate Config key at configmap to VerificationPolicy
 **Note:** key configuration in configmap is deprecated,

pkg/trustedresources/errors.go

@@ -24,6 +24,4 @@ var (
 	ErrNoMatchedPolicies = errors.New("no policies are matched")
 	// ErrRegexMatch is returned when regex match returns error
 	ErrRegexMatch = errors.New("regex failed to match")
-	// ErrSignatureMissing is returned when signature is missing in resource
-	ErrSignatureMissing = errors.New("signature is missing")
 )

pkg/trustedresources/verify.go

@@ -135,8 +135,9 @@ func getMatchedPolicies(resourceName string, source string, policies []*v1alpha1
 
 // verifyResource verifies resource which implements metav1.Object by provided signature and public keys from verification policies.
 // For matched policies, `verifyResource“ will adopt the following rules to do verification:
-// 1. If multiple policies are matched, the resource needs to pass all of them to pass verification. We use AND logic on matched policies.
-// 2. To pass one policy, the resource can pass any public keys in the policy. We use OR logic on public keys of one policy.
+//  1. If multiple policies match, the resource must satisfy all the "enforce" policies to pass verification. The matching "enforce" policies are evaluated using AND logic.
+//     Alternatively, if the resource only matches policies in "warn" mode, it will still pass verification and only log a warning if these policies are not satisfied.
+//  2. To pass one policy, the resource can pass any public keys in the policy. We use OR logic on public keys of one policy.
 func verifyResource(ctx context.Context, resource metav1.Object, k8s kubernetes.Interface, signature []byte, matchedPolicies []*v1alpha1.VerificationPolicy) error {
 	for _, p := range matchedPolicies {
 		passVerification := false
@@ -151,9 +152,15 @@ func verifyResource(ctx context.Context, resource metav1.Object, k8s kubernetes.
 				break
 			}
 		}
-		// if this policy fails the verification, should return error directly. No need to check other policies
+		// if this policy fails the verification and the mode is not "warn", should return error directly. No need to check other policies
 		if !passVerification {
-			return fmt.Errorf("%w: resource %s in namespace %s fails verification", ErrResourceVerificationFailed, resource.GetName(), resource.GetNamespace())
+			if p.Spec.Mode == v1alpha1.ModeWarn {
+				logger := logging.FromContext(ctx)
+				logger.Warnf("%w: resource %s in namespace %s fails verification", ErrResourceVerificationFailed, resource.GetName(), resource.GetNamespace())
+			} else {
+				// if the mode is "enforce" or not set, return error.
+				return fmt.Errorf("%w: resource %s in namespace %s fails verification", ErrResourceVerificationFailed, resource.GetName(), resource.GetNamespace())
+			}
 		}
 	}
 	return nil
@@ -177,7 +184,10 @@ func verifyInterface(obj interface{}, verifier signature.Verifier, signature []b
 }
 
 // prepareObjectMeta will remove annotations not configured from user side -- "kubectl-client-side-apply" and "kubectl.kubernetes.io/last-applied-configuration"
-// to avoid verification failure and extract the signature.
+// (added when an object is created with `kubectl apply`) to avoid verification failure and extract the signature.
+// Returns a copy of the input object metadata with the annotations removed and the object's signature,
+// if it is present in the metadata.
+// Returns a non-nil error if the signature cannot be decoded.
 func prepareObjectMeta(in metav1.ObjectMeta) (metav1.ObjectMeta, []byte, error) {
 	out := metav1.ObjectMeta{}
 
@@ -207,7 +217,7 @@ func prepareObjectMeta(in metav1.ObjectMeta) (metav1.ObjectMeta, []byte, error)
 	// signature should be contained in annotation
 	sig, ok := in.Annotations[SignatureAnnotation]
 	if !ok {
-		return out, nil, ErrSignatureMissing
+		return out, nil, nil
 	}
 	// extract signature
 	signature, err := base64.StdEncoding.DecodeString(sig)

pkg/trustedresources/verify_test.go

@@ -131,6 +131,10 @@ func TestVerifyTask_Success(t *testing.T) {
 	if err != nil {
 		t.Fatal("fail to sign task", err)
 	}
+
+	modifiedTask := signedTask.DeepCopy()
+	modifiedTask.Name = "modified"
+
 	signer384, _, pub, err := test.GenerateKeys(elliptic.P384(), crypto.SHA384)
 	if err != nil {
 		t.Fatalf("failed to generate keys %v", err)
@@ -160,7 +164,32 @@ func TestVerifyTask_Success(t *testing.T) {
 			},
 		},
 	}
-	vps = append(vps, sha384Vp)
+
+	warnPolicy := &v1alpha1.VerificationPolicy{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "VerificationPolicy",
+			APIVersion: "v1alpha1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "warnPolicy",
+			Namespace: namespace,
+		},
+		Spec: v1alpha1.VerificationPolicySpec{
+			Resources: []v1alpha1.ResourcePattern{
+				{Pattern: "https://github.com/tektoncd/catalog.git"},
+			},
+			Authorities: []v1alpha1.Authority{
+				{
+					Name: "key",
+					Key: &v1alpha1.KeyRef{
+						Data:          string(pub),
+						HashAlgorithm: "sha384",
+					},
+				},
+			},
+			Mode: v1alpha1.ModeWarn,
+		},
+	}
 
 	signedTask384, err := test.GetSignedTask(unsignedTask, signer384, "signed384")
 	if err != nil {
@@ -174,21 +203,25 @@ func TestVerifyTask_Success(t *testing.T) {
 		source                    string
 		signer                    signature.SignerVerifier
 		verificationNoMatchPolicy string
+		verificationPolicies      []*v1alpha1.VerificationPolicy
 	}{{
 		name:                      "signed git source task passes verification",
 		task:                      signedTask,
 		source:                    "git+https://github.com/tektoncd/catalog.git",
 		verificationNoMatchPolicy: config.FailNoMatchPolicy,
+		verificationPolicies:      vps,
 	}, {
 		name:                      "signed bundle source task passes verification",
 		task:                      signedTask,
 		source:                    "gcr.io/tekton-releases/catalog/upstream/git-clone",
 		verificationNoMatchPolicy: config.FailNoMatchPolicy,
+		verificationPolicies:      vps,
 	}, {
 		name:                      "signed task with sha384 key",
 		task:                      signedTask384,
 		source:                    "gcr.io/tekton-releases/catalog/upstream/sha384",
 		verificationNoMatchPolicy: config.FailNoMatchPolicy,
+		verificationPolicies:      []*v1alpha1.VerificationPolicy{sha384Vp},
 	}, {
 		name:                      "ignore no match policy skips verification when no matching policies",
 		task:                      unsignedTask,
@@ -199,12 +232,24 @@ func TestVerifyTask_Success(t *testing.T) {
 		task:                      unsignedTask,
 		source:                    mismatchedSource,
 		verificationNoMatchPolicy: config.WarnNoMatchPolicy,
+	}, {
+		name:                      "unsigned task matches warn policy doesn't fail verification",
+		task:                      unsignedTask,
+		source:                    "git+https://github.com/tektoncd/catalog.git",
+		verificationNoMatchPolicy: config.FailNoMatchPolicy,
+		verificationPolicies:      []*v1alpha1.VerificationPolicy{warnPolicy},
+	}, {
+		name:                      "modified task matches warn policy doesn't fail verification",
+		task:                      modifiedTask,
+		source:                    "git+https://github.com/tektoncd/catalog.git",
+		verificationNoMatchPolicy: config.FailNoMatchPolicy,
+		verificationPolicies:      []*v1alpha1.VerificationPolicy{warnPolicy},
 	}}
 
 	for _, tc := range tcs {
 		t.Run(tc.name, func(t *testing.T) {
 			ctx := test.SetupTrustedResourceConfig(context.Background(), tc.verificationNoMatchPolicy)
-			err := VerifyTask(ctx, tc.task, k8sclient, tc.source, vps)
+			err := VerifyTask(ctx, tc.task, k8sclient, tc.source, tc.verificationPolicies)
 			if err != nil {
 				t.Fatalf("VerifyTask() get err %v", err)
 			}
@@ -236,6 +281,12 @@ func TestVerifyTask_Error(t *testing.T) {
 		verificationPolicy []*v1alpha1.VerificationPolicy
 		expectedError      error
 	}{{
+		name:               "unsigned Task fails verification",
+		task:               unsignedTask,
+		source:             "git+https://github.com/tektoncd/catalog.git",
+		verificationPolicy: vps,
+		expectedError:      ErrResourceVerificationFailed,
+	}, {
 		name:               "modified Task fails verification",
 		task:               tamperedTask,
 		source:             matchingSource,
@@ -414,7 +465,8 @@ func TestPrepareObjectMeta(t *testing.T) {
 	unsigned := test.GetUnsignedTask("test-task").ObjectMeta
 
 	signed := unsigned.DeepCopy()
-	signed.Annotations = map[string]string{SignatureAnnotation: "tY805zV53PtwDarK3VD6dQPx5MbIgctNcg/oSle+MG0="}
+	sig := "tY805zV53PtwDarK3VD6dQPx5MbIgctNcg/oSle+MG0="
+	signed.Annotations = map[string]string{SignatureAnnotation: sig}
 
 	signedWithLabels := signed.DeepCopy()
 	signedWithLabels.Labels = map[string]string{"label": "foo"}
@@ -424,10 +476,10 @@ func TestPrepareObjectMeta(t *testing.T) {
 	signedWithExtraAnnotations.Annotations["kubectl.kubernetes.io/last-applied-configuration"] = "config"
 
 	tcs := []struct {
-		name       string
-		objectmeta *metav1.ObjectMeta
-		expected   metav1.ObjectMeta
-		wantErr    bool
+		name              string
+		objectmeta        *metav1.ObjectMeta
+		expected          metav1.ObjectMeta
+		expectedSignature string
 	}{{
 		name:       "Prepare signed objectmeta without labels",
 		objectmeta: signed,
@@ -436,7 +488,7 @@ func TestPrepareObjectMeta(t *testing.T) {
 			Namespace:   namespace,
 			Annotations: map[string]string{},
 		},
-		wantErr: false,
+		expectedSignature: sig,
 	}, {
 		name:       "Prepare signed objectmeta with labels",
 		objectmeta: signedWithLabels,
@@ -446,7 +498,7 @@ func TestPrepareObjectMeta(t *testing.T) {
 			Labels:      map[string]string{"label": "foo"},
 			Annotations: map[string]string{},
 		},
-		wantErr: false,
+		expectedSignature: sig,
 	}, {
 		name:       "Prepare signed objectmeta with extra annotations",
 		objectmeta: signedWithExtraAnnotations,
@@ -455,33 +507,30 @@ func TestPrepareObjectMeta(t *testing.T) {
 			Namespace:   namespace,
 			Annotations: map[string]string{},
 		},
-		wantErr: false,
+		expectedSignature: sig,
 	}, {
-		name:       "Fail preparation without signature",
+		name:       "resource without signature shouldn't fail",
 		objectmeta: &unsigned,
 		expected: metav1.ObjectMeta{
 			Name:        "test-task",
 			Namespace:   namespace,
 			Annotations: map[string]string{"foo": "bar"},
 		},
-		wantErr: true,
+		expectedSignature: "",
 	}}
 
 	for _, tc := range tcs {
 		t.Run(tc.name, func(t *testing.T) {
 			task, signature, err := prepareObjectMeta(*tc.objectmeta)
-			if (err != nil) != tc.wantErr {
-				t.Fatalf("prepareObjectMeta() get err %v, wantErr %t", err, tc.wantErr)
+			if err != nil {
+				t.Fatalf("got unexpected err: %v", err)
 			}
 			if d := cmp.Diff(task, tc.expected); d != "" {
 				t.Error(diff.PrintWantGot(d))
 			}
-
-			if tc.wantErr {
-				return
-			}
-			if signature == nil {
-				t.Fatal("signature is not extracted")
+			got := base64.StdEncoding.EncodeToString(signature)
+			if d := cmp.Diff(got, tc.expectedSignature); d != "" {
+				t.Error(diff.PrintWantGot(d))
 			}
 		})
 	}