Skip to content

Commit d3f5003

Browse files
committed
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]
1 parent 5deea1f commit d3f5003

File tree

4 files changed

+92
-29
lines changed

4 files changed

+92
-29
lines changed

docs/trusted-resources.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ How does VerificationPolicy work?
7878
You can create multiple `VerificationPolicy` and apply them to the cluster.
7979
1. Trusted resources will look up policies from the resource namespace (usually this is the same as taskrun/pipelinerun namespace).
8080
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.
81-
3. If multiple policies are matched, the resource needs to pass all of them to pass verification.
81+
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.
82+
8283
4. To pass one policy, the resource can pass any public keys in the policy.
8384

8485
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:
109110
key:
110111
# data stores the inline public key data
111112
data: "STRING_ENCODED_PUBLIC_KEY"
113+
# mode can be set to "enforce" (default) or "warn".
114+
mode: enforce
112115
```
113116

114117
```yaml
@@ -141,6 +144,9 @@ To learn more about `ConfigSource` please refer to resolvers doc for more contex
141144

142145
`hashAlgorithm` is the algorithm for the public key, by default is `sha256`. It also supports `SHA224`, `SHA384`, `SHA512`.
143146

147+
`mode` controls whether a failing policy will fail the taskrun/pipelinerun, or only log the a warning
148+
* enforce (default) - fail the taskrun/pipelinerun if verification fails
149+
* warn - don't fail the taskrun/pipelinerun if verification fails but log a warning
144150

145151
#### Migrate Config key at configmap to VerificationPolicy
146152
**Note:** key configuration in configmap is deprecated,

pkg/trustedresources/errors.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,4 @@ var (
2424
ErrNoMatchedPolicies = errors.New("no policies are matched")
2525
// ErrRegexMatch is returned when regex match returns error
2626
ErrRegexMatch = errors.New("regex failed to match")
27-
// ErrSignatureMissing is returned when signature is missing in resource
28-
ErrSignatureMissing = errors.New("signature is missing")
2927
)

pkg/trustedresources/verify.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,9 @@ func getMatchedPolicies(resourceName string, source string, policies []*v1alpha1
135135

136136
// verifyResource verifies resource which implements metav1.Object by provided signature and public keys from verification policies.
137137
// For matched policies, `verifyResource“ will adopt the following rules to do verification:
138-
// 1. If multiple policies are matched, the resource needs to pass all of them to pass verification. We use AND logic on matched policies.
139-
// 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.
138+
// 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.
139+
// 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.
140+
// 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.
140141
func verifyResource(ctx context.Context, resource metav1.Object, k8s kubernetes.Interface, signature []byte, matchedPolicies []*v1alpha1.VerificationPolicy) error {
141142
for _, p := range matchedPolicies {
142143
passVerification := false
@@ -151,9 +152,15 @@ func verifyResource(ctx context.Context, resource metav1.Object, k8s kubernetes.
151152
break
152153
}
153154
}
154-
// if this policy fails the verification, should return error directly. No need to check other policies
155+
// if this policy fails the verification and the mode is not "warn", should return error directly. No need to check other policies
155156
if !passVerification {
156-
return fmt.Errorf("%w: resource %s in namespace %s fails verification", ErrResourceVerificationFailed, resource.GetName(), resource.GetNamespace())
157+
if p.Spec.Mode == v1alpha1.ModeWarn {
158+
logger := logging.FromContext(ctx)
159+
logger.Warnf("%w: resource %s in namespace %s fails verification", ErrResourceVerificationFailed, resource.GetName(), resource.GetNamespace())
160+
} else {
161+
// if the mode is "enforce" or not set, return error.
162+
return fmt.Errorf("%w: resource %s in namespace %s fails verification", ErrResourceVerificationFailed, resource.GetName(), resource.GetNamespace())
163+
}
157164
}
158165
}
159166
return nil
@@ -177,7 +184,10 @@ func verifyInterface(obj interface{}, verifier signature.Verifier, signature []b
177184
}
178185

179186
// prepareObjectMeta will remove annotations not configured from user side -- "kubectl-client-side-apply" and "kubectl.kubernetes.io/last-applied-configuration"
180-
// to avoid verification failure and extract the signature.
187+
// (added when an object is created with `kubectl apply`) to avoid verification failure and extract the signature.
188+
// Returns a copy of the input object metadata with the annotations removed and the object's signature,
189+
// if it is present in the metadata.
190+
// Returns a non-nil error if the signature cannot be decoded.
181191
func prepareObjectMeta(in metav1.ObjectMeta) (metav1.ObjectMeta, []byte, error) {
182192
out := metav1.ObjectMeta{}
183193

@@ -207,7 +217,7 @@ func prepareObjectMeta(in metav1.ObjectMeta) (metav1.ObjectMeta, []byte, error)
207217
// signature should be contained in annotation
208218
sig, ok := in.Annotations[SignatureAnnotation]
209219
if !ok {
210-
return out, nil, ErrSignatureMissing
220+
return out, nil, nil
211221
}
212222
// extract signature
213223
signature, err := base64.StdEncoding.DecodeString(sig)

pkg/trustedresources/verify_test.go

Lines changed: 69 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ func TestVerifyTask_Success(t *testing.T) {
131131
if err != nil {
132132
t.Fatal("fail to sign task", err)
133133
}
134+
135+
modifiedTask := signedTask.DeepCopy()
136+
modifiedTask.Name = "modified"
137+
134138
signer384, _, pub, err := test.GenerateKeys(elliptic.P384(), crypto.SHA384)
135139
if err != nil {
136140
t.Fatalf("failed to generate keys %v", err)
@@ -160,7 +164,32 @@ func TestVerifyTask_Success(t *testing.T) {
160164
},
161165
},
162166
}
163-
vps = append(vps, sha384Vp)
167+
168+
warnPolicy := &v1alpha1.VerificationPolicy{
169+
TypeMeta: metav1.TypeMeta{
170+
Kind: "VerificationPolicy",
171+
APIVersion: "v1alpha1",
172+
},
173+
ObjectMeta: metav1.ObjectMeta{
174+
Name: "warnPolicy",
175+
Namespace: namespace,
176+
},
177+
Spec: v1alpha1.VerificationPolicySpec{
178+
Resources: []v1alpha1.ResourcePattern{
179+
{Pattern: "https://github.com/tektoncd/catalog.git"},
180+
},
181+
Authorities: []v1alpha1.Authority{
182+
{
183+
Name: "key",
184+
Key: &v1alpha1.KeyRef{
185+
Data: string(pub),
186+
HashAlgorithm: "sha384",
187+
},
188+
},
189+
},
190+
Mode: v1alpha1.ModeWarn,
191+
},
192+
}
164193

165194
signedTask384, err := test.GetSignedTask(unsignedTask, signer384, "signed384")
166195
if err != nil {
@@ -174,21 +203,25 @@ func TestVerifyTask_Success(t *testing.T) {
174203
source string
175204
signer signature.SignerVerifier
176205
verificationNoMatchPolicy string
206+
verificationPolicies []*v1alpha1.VerificationPolicy
177207
}{{
178208
name: "signed git source task passes verification",
179209
task: signedTask,
180210
source: "git+https://github.com/tektoncd/catalog.git",
181211
verificationNoMatchPolicy: config.FailNoMatchPolicy,
212+
verificationPolicies: vps,
182213
}, {
183214
name: "signed bundle source task passes verification",
184215
task: signedTask,
185216
source: "gcr.io/tekton-releases/catalog/upstream/git-clone",
186217
verificationNoMatchPolicy: config.FailNoMatchPolicy,
218+
verificationPolicies: vps,
187219
}, {
188220
name: "signed task with sha384 key",
189221
task: signedTask384,
190222
source: "gcr.io/tekton-releases/catalog/upstream/sha384",
191223
verificationNoMatchPolicy: config.FailNoMatchPolicy,
224+
verificationPolicies: []*v1alpha1.VerificationPolicy{sha384Vp},
192225
}, {
193226
name: "ignore no match policy skips verification when no matching policies",
194227
task: unsignedTask,
@@ -199,12 +232,24 @@ func TestVerifyTask_Success(t *testing.T) {
199232
task: unsignedTask,
200233
source: mismatchedSource,
201234
verificationNoMatchPolicy: config.WarnNoMatchPolicy,
235+
}, {
236+
name: "unsigned task matches warn policy doesn't fail verification",
237+
task: unsignedTask,
238+
source: "git+https://github.com/tektoncd/catalog.git",
239+
verificationNoMatchPolicy: config.FailNoMatchPolicy,
240+
verificationPolicies: []*v1alpha1.VerificationPolicy{warnPolicy},
241+
}, {
242+
name: "modified task matches warn policy doesn't fail verification",
243+
task: modifiedTask,
244+
source: "git+https://github.com/tektoncd/catalog.git",
245+
verificationNoMatchPolicy: config.FailNoMatchPolicy,
246+
verificationPolicies: []*v1alpha1.VerificationPolicy{warnPolicy},
202247
}}
203248

204249
for _, tc := range tcs {
205250
t.Run(tc.name, func(t *testing.T) {
206251
ctx := test.SetupTrustedResourceConfig(context.Background(), tc.verificationNoMatchPolicy)
207-
err := VerifyTask(ctx, tc.task, k8sclient, tc.source, vps)
252+
err := VerifyTask(ctx, tc.task, k8sclient, tc.source, tc.verificationPolicies)
208253
if err != nil {
209254
t.Fatalf("VerifyTask() get err %v", err)
210255
}
@@ -236,6 +281,12 @@ func TestVerifyTask_Error(t *testing.T) {
236281
verificationPolicy []*v1alpha1.VerificationPolicy
237282
expectedError error
238283
}{{
284+
name: "unsigned Task fails verification",
285+
task: unsignedTask,
286+
source: "git+https://github.com/tektoncd/catalog.git",
287+
verificationPolicy: vps,
288+
expectedError: ErrResourceVerificationFailed,
289+
}, {
239290
name: "modified Task fails verification",
240291
task: tamperedTask,
241292
source: matchingSource,
@@ -414,7 +465,8 @@ func TestPrepareObjectMeta(t *testing.T) {
414465
unsigned := test.GetUnsignedTask("test-task").ObjectMeta
415466

416467
signed := unsigned.DeepCopy()
417-
signed.Annotations = map[string]string{SignatureAnnotation: "tY805zV53PtwDarK3VD6dQPx5MbIgctNcg/oSle+MG0="}
468+
sig := "tY805zV53PtwDarK3VD6dQPx5MbIgctNcg/oSle+MG0="
469+
signed.Annotations = map[string]string{SignatureAnnotation: sig}
418470

419471
signedWithLabels := signed.DeepCopy()
420472
signedWithLabels.Labels = map[string]string{"label": "foo"}
@@ -424,10 +476,10 @@ func TestPrepareObjectMeta(t *testing.T) {
424476
signedWithExtraAnnotations.Annotations["kubectl.kubernetes.io/last-applied-configuration"] = "config"
425477

426478
tcs := []struct {
427-
name string
428-
objectmeta *metav1.ObjectMeta
429-
expected metav1.ObjectMeta
430-
wantErr bool
479+
name string
480+
objectmeta *metav1.ObjectMeta
481+
expected metav1.ObjectMeta
482+
expectedSignature string
431483
}{{
432484
name: "Prepare signed objectmeta without labels",
433485
objectmeta: signed,
@@ -436,7 +488,7 @@ func TestPrepareObjectMeta(t *testing.T) {
436488
Namespace: namespace,
437489
Annotations: map[string]string{},
438490
},
439-
wantErr: false,
491+
expectedSignature: sig,
440492
}, {
441493
name: "Prepare signed objectmeta with labels",
442494
objectmeta: signedWithLabels,
@@ -446,7 +498,7 @@ func TestPrepareObjectMeta(t *testing.T) {
446498
Labels: map[string]string{"label": "foo"},
447499
Annotations: map[string]string{},
448500
},
449-
wantErr: false,
501+
expectedSignature: sig,
450502
}, {
451503
name: "Prepare signed objectmeta with extra annotations",
452504
objectmeta: signedWithExtraAnnotations,
@@ -455,33 +507,30 @@ func TestPrepareObjectMeta(t *testing.T) {
455507
Namespace: namespace,
456508
Annotations: map[string]string{},
457509
},
458-
wantErr: false,
510+
expectedSignature: sig,
459511
}, {
460-
name: "Fail preparation without signature",
512+
name: "resource without signature shouldn't fail",
461513
objectmeta: &unsigned,
462514
expected: metav1.ObjectMeta{
463515
Name: "test-task",
464516
Namespace: namespace,
465517
Annotations: map[string]string{"foo": "bar"},
466518
},
467-
wantErr: true,
519+
expectedSignature: "",
468520
}}
469521

470522
for _, tc := range tcs {
471523
t.Run(tc.name, func(t *testing.T) {
472524
task, signature, err := prepareObjectMeta(*tc.objectmeta)
473-
if (err != nil) != tc.wantErr {
474-
t.Fatalf("prepareObjectMeta() get err %v, wantErr %t", err, tc.wantErr)
525+
if err != nil {
526+
t.Fatalf("got unexpected err: %v", err)
475527
}
476528
if d := cmp.Diff(task, tc.expected); d != "" {
477529
t.Error(diff.PrintWantGot(d))
478530
}
479-
480-
if tc.wantErr {
481-
return
482-
}
483-
if signature == nil {
484-
t.Fatal("signature is not extracted")
531+
got := base64.StdEncoding.EncodeToString(signature)
532+
if d := cmp.Diff(got, tc.expectedSignature); d != "" {
533+
t.Error(diff.PrintWantGot(d))
485534
}
486535
})
487536
}

0 commit comments

Comments
 (0)