Skip to content

Commit 389b06b

Browse files
committed
Add documentation for verification tests.
In particular, add documentation for verify-govet-levee
1 parent f0d4b23 commit 389b06b

File tree

1 file changed

+78
-0
lines changed

1 file changed

+78
-0
lines changed
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# Verification Tests
2+
3+
**Table of Contents**
4+
5+
- [Verification Tests](#verification-tests)
6+
- [Overview](#overview)
7+
- [`verify-govet-leveee`](#verify-govet-levee)
8+
9+
## Overview
10+
11+
Verification tests for Kubernetes provide a mechanism to verify contributions for adherence to project conventions
12+
and best practices, and to validate generated build artifacts for soundness.
13+
14+
All blocking verification tests can be executed via `make verify`.
15+
Individual verification tests also can be found in vestigial shell scripts at `hack/verify-*.sh`.
16+
17+
Most verification tests are self-explanatory.
18+
`verify-golint` and `verify-gofmt`, for instance, fail when a contribution does not adhere to lint and formatting conventions.
19+
More complex verification tests are described below.
20+
21+
### `verify-govet-levee`
22+
23+
Verification in `verify-govet-levee.sh` uses taint propagation analysis
24+
to defend against accidental logging of credentials.
25+
Struct fields which may contain credentials should be annotated as such using the `datapolicy` field tag.
26+
Field tagging was introduced by [KEP-1753](https://github.com/kubernetes/enhancements/issues/1753), and analysis was introduced by [KEP-1993](https://github.com/kubernetes/enhancements/issues/1933).
27+
Additional credential sources may be identified in analysis configuration (see below).
28+
29+
Taint propagation analysis defends against both direct and indirect logging of credentials.
30+
For consider the following hypothetical snippet.
31+
32+
```golang
33+
// kubernetes/cmd/kubelet/app/server.go
34+
35+
// kubeConfigSpec struct holds info required to build a KubeConfig object
36+
type kubeConfigSpec struct {
37+
CACert *x509.Certificate
38+
APIServer string
39+
ClientName string
40+
TokenAuth *tokenAuth `datapolicy:"token"`
41+
ClientCertAuth *clientCertAuth `datapolicy:"security-key"`
42+
}
43+
44+
func MyDangerousFunction(spec kubeConfigSpec) error {
45+
if spec.CACert == nil {
46+
err := fmt.Errorf("kubeConfigSpec missing expected CACert, got %#v", spec) // Dangerous logging!
47+
klog.Error(err)
48+
return err
49+
}
50+
51+
if err := DoSomethingElse(spec); err != nil {
52+
klog.Error(err) // Dangerous logging!
53+
}
54+
55+
return nil
56+
}
57+
```
58+
59+
In the above, when `spec.CACert == nil`, we log the `spec`.
60+
However, we know from the datapolicy field tags that the spec could contain one or more credentials and should not be logged.
61+
The analysis will detect this and cause the verification test to fail.
62+
The log call should be adjusted to extract exactly what information is relevant from the `spec`.
63+
64+
The second `klog.Error` call is also problematic.
65+
The error returned by `DoSomethingElse` could potentially encapsulate the credential passed in by `spec`, and so we must not log it.
66+
That is, we consider `err` to be "tainted" by the call which has access to the credentials.
67+
The analysis will detect this as well and call the verification test to fail.
68+
69+
When this analysis causes the verification test to fail, a developer has several options.
70+
In order of decreasing preference:
71+
* Reconstruct logging calls such that only relevant information is passed.
72+
* If analysis warning is produced by a tainted value reaching logs, reconstruct the method which caused taint to spread so that it only takes non-credential values.
73+
* Reconstruct the method which caused taint to spread to return indicators which are not logged directly, e.g. return `value, ok` rather than `value, err`.
74+
* Write a *sanitizer* whose return value is guaranteed to be log-safe. Add this sanitizer to the analysis configuration (see below).
75+
* Add the method where the log call occurs to the analysis configuration exclude-list.
76+
77+
Analysis configuration can be found at [kubernetes/kubernetes/hack/testdata/levee/levee-config.yaml](https://github.com/kubernetes/kubernetes/blob/master/hack/testdata/levee/levee-config.yaml).
78+
Contact SIG-Security with any additional questions.

0 commit comments

Comments
 (0)