use verification mode in trusted resources

This commit uses verification policy's mode field in trusted resources. The mode can be set to "warn" or "enforce". The policy will use "enforce" mode if the `mode` is not set. If the mode is set `warn`, then fails to verify this policy will only log a warning and not fail the taskruns or pipelineruns. "enforce" mode policy will fail the taskruns or pipelineruns if fails to verify. Signed-off-by: Yongxuan Zhang [email protected]
tektoncd · Mar 22, 2023 · 8cd3da7 · 8cd3da7
1 parent 5deea1f
commit 8cd3da7
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 28 deletions.
diff --git a/docs/trusted-resources.md b/docs/trusted-resources.md
@@ -78,7 +78,7 @@ 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 needs to pass all of the "enforce" mode policies. If only matching "warn" mode policies, fail to pass the "warn" policy will not fail the taskrun/pipelinerun but only log a warning.
 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 +109,8 @@ spec:
       key:
         # data stores the inline public key data
         data: "STRING_ENCODED_PUBLIC_KEY"
+  # mode can be set to "enforce" or "warn".
+  mode: enforce
 ```
 
 ```yaml
@@ -141,6 +143,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,

diff --git a/pkg/trustedresources/errors.go b/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")
 )
diff --git a/pkg/trustedresources/verify.go b/pkg/trustedresources/verify.go
@@ -135,7 +135,8 @@ 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.
+// 1. If multiple policies are matched, the resource needs to pass all of "enforced" mode policies to pass verification. We use AND logic on matched "enforce" policies.
+//    or the resource only matches "warn" mode 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.
 func verifyResource(ctx context.Context, resource metav1.Object, k8s kubernetes.Interface, signature []byte, matchedPolicies []*v1alpha1.VerificationPolicy) error {
 	for _, p := range matchedPolicies {
@@ -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
@@ -207,7 +214,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, []byte{}, nil
 	}
 	// extract signature
 	signature, err := base64.StdEncoding.DecodeString(sig)

diff --git a/pkg/trustedresources/verify_test.go b/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,
@@ -427,7 +478,6 @@ func TestPrepareObjectMeta(t *testing.T) {
 		name       string
 		objectmeta *metav1.ObjectMeta
 		expected   metav1.ObjectMeta
-		wantErr    bool
 	}{{
 		name:       "Prepare signed objectmeta without labels",
 		objectmeta: signed,
@@ -436,7 +486,6 @@ func TestPrepareObjectMeta(t *testing.T) {
 			Namespace:   namespace,
 			Annotations: map[string]string{},
 		},
-		wantErr: false,
 	}, {
 		name:       "Prepare signed objectmeta with labels",
 		objectmeta: signedWithLabels,
@@ -446,7 +495,6 @@ func TestPrepareObjectMeta(t *testing.T) {
 			Labels:      map[string]string{"label": "foo"},
 			Annotations: map[string]string{},
 		},
-		wantErr: false,
 	}, {
 		name:       "Prepare signed objectmeta with extra annotations",
 		objectmeta: signedWithExtraAnnotations,
@@ -455,33 +503,20 @@ func TestPrepareObjectMeta(t *testing.T) {
 			Namespace:   namespace,
 			Annotations: map[string]string{},
 		},
-		wantErr: false,
-	}, {
-		name:       "Fail preparation without signature",
-		objectmeta: &unsigned,
-		expected: metav1.ObjectMeta{
-			Name:        "test-task",
-			Namespace:   namespace,
-			Annotations: map[string]string{"foo": "bar"},
-		},
-		wantErr: true,
 	}}
 
 	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.Errorf("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")
+				t.Error("signature is not extracted")
 			}
 		})
 	}