-
Notifications
You must be signed in to change notification settings - Fork 111
Expand file tree
/
Copy pathnobypass_test.go
More file actions
264 lines (242 loc) · 8.78 KB
/
Copy pathnobypass_test.go
File metadata and controls
264 lines (242 loc) · 8.78 KB
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
/*
Copyright IBM Corp. All Rights Reserved.
SPDX-License-Identifier: Apache-2.0
*/
// The tests in this file enforce the no-bypass rule stated in
// docs/security/store_integrity_verification.md: the checks in this package are
// unconditional, and nothing — no functional option, no setter, no configuration
// key — may be added that turns one of them off. A security check a deployment
// can disable is not a security posture, and a check that is disabled by default
// in some deployment is worse than no check at all, because the contract clauses
// on the store interfaces claim it holds.
//
// The rule is enforced by reading the source, rather than by convention, because
// the failure it guards against is a future well-meaning change ("make this
// opt-in so it does not break my deployment") that no behavioural test would
// catch.
package integrity_test
import (
"go/ast"
"go/parser"
"go/token"
"os"
"path/filepath"
"regexp"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// checkedPackages are the packages that apply the checks in this package. A
// bypass would have to be introduced in one of them, or in this package itself.
var checkedPackages = []string{
".",
"../ttxdb",
"../auditdb",
"../endorserdb",
"../db/sql/common",
"../db/kvs",
"../../identity",
"../../identity/wallet",
"../../ttx",
"../../..",
}
// bypassNames matches identifiers that would name a way to skip verification.
// It is deliberately broad: the point is to fail on the attempt, and a rename to
// something this does not match is a conscious act rather than an oversight. Add
// to it rather than narrowing it.
var bypassNames = regexp.MustCompile(
`(?i)(skip|disable|without|no|bypass|unsafe|unchecked|ignore)_?` +
`(integrity|verification|verify|check|validation|validate)`,
)
// parsePackageDir parses the non-test Go files of one package directory.
func parsePackageDir(t *testing.T, dir string) (*token.FileSet, []*ast.File) {
t.Helper()
entries, err := os.ReadDir(dir)
require.NoError(t, err, "cannot read package directory [%s]", dir)
fset := token.NewFileSet()
files := make([]*ast.File, 0, len(entries))
for _, entry := range entries {
name := entry.Name()
if entry.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
continue
}
file, err := parser.ParseFile(fset, filepath.Join(dir, name), nil, parser.SkipObjectResolution)
require.NoError(t, err, "cannot parse [%s]", name)
files = append(files, file)
}
require.NotEmpty(t, files, "no source files found in [%s]", dir)
return fset, files
}
// TestNoBypassIdentifiers asserts that no package applying the integrity checks
// declares anything named like a way to turn them off — no function, method,
// type, field, variable, or constant.
func TestNoBypassIdentifiers(t *testing.T) {
for _, dir := range checkedPackages {
t.Run(dir, func(t *testing.T) {
fset, files := parsePackageDir(t, dir)
for _, file := range files {
ast.Inspect(file, func(n ast.Node) bool {
ident, ok := n.(*ast.Ident)
if !ok {
return true
}
assert.False(t, bypassNames.MatchString(ident.Name),
"%s: identifier [%s] names a way to skip verification; the checks in "+
"token/services/storage/integrity are unconditional by design — see "+
"docs/security/store_integrity_verification.md",
fset.Position(ident.Pos()), ident.Name)
return true
})
}
})
}
}
// TestChecksTakeNoOptions asserts that the exported checks of this package are
// plain functions of their inputs: not variadic, and returning only an error.
// A variadic parameter is how an option that weakens a check would be added
// without changing any call site, so it must not exist in the first place.
func TestChecksTakeNoOptions(t *testing.T) {
fset, files := parsePackageDir(t, ".")
found := 0
for _, file := range files {
for _, decl := range file.Decls {
fn, ok := decl.(*ast.FuncDecl)
if !ok || fn.Recv != nil || !fn.Name.IsExported() || !strings.HasPrefix(fn.Name.Name, "Check") {
continue
}
found++
for _, param := range fn.Type.Params.List {
_, variadic := param.Type.(*ast.Ellipsis)
assert.False(t, variadic,
"%s: %s takes a variadic parameter; the checks must not be configurable",
fset.Position(param.Pos()), fn.Name.Name)
}
require.NotNil(t, fn.Type.Results, "%s must return an error", fn.Name.Name)
assert.Len(t, fn.Type.Results.List, 1,
"%s must return an error and nothing else, so that a caller cannot ignore the "+
"verdict while still using a result", fn.Name.Name)
}
}
assert.GreaterOrEqual(t, found, 6, "expected to have found the exported Check functions")
}
// TestChecksExposeNoMutableState asserts that this package holds no mutable
// package-level state. Anything settable at runtime — a flag, a hook, a
// replaceable function value — is a bypass, whether or not it is named like one.
// The only package-level values allowed are the sentinel errors, which are
// compared against and never assigned.
func TestChecksExposeNoMutableState(t *testing.T) {
fset, files := parsePackageDir(t, ".")
for _, file := range files {
for _, decl := range file.Decls {
gen, ok := decl.(*ast.GenDecl)
if !ok || gen.Tok != token.VAR {
continue
}
for _, spec := range gen.Specs {
value, ok := spec.(*ast.ValueSpec)
if !ok {
continue
}
for _, name := range value.Names {
assert.True(t, strings.HasPrefix(name.Name, "Err") || strings.HasPrefix(name.Name, "err"),
"%s: package-level variable [%s] is not a sentinel error; the integrity "+
"package must hold no state a deployment could change",
fset.Position(name.Pos()), name.Name)
}
}
}
}
}
// TestNoVerificationConfigKey asserts that no configuration key read by the
// storage layer controls verification. The keys are declared as constants, so
// this reads them from the source rather than from a hand-maintained list that
// would drift.
func TestNoVerificationConfigKey(t *testing.T) {
for _, dir := range []string{"../db/sql/common", "../services/cleanup", "../services/recovery"} {
t.Run(dir, func(t *testing.T) {
fset, files := parsePackageDir(t, dir)
for _, file := range files {
for _, decl := range file.Decls {
gen, ok := decl.(*ast.GenDecl)
if !ok || gen.Tok != token.CONST {
continue
}
for _, spec := range gen.Specs {
value, ok := spec.(*ast.ValueSpec)
if !ok {
continue
}
for i, name := range value.Names {
if !strings.HasPrefix(name.Name, "ConfigKey") {
continue
}
assert.False(t, bypassNames.MatchString(name.Name),
"%s: configuration key constant [%s] controls verification",
fset.Position(name.Pos()), name.Name)
if i < len(value.Values) {
if lit, ok := value.Values[i].(*ast.BasicLit); ok {
assert.False(t, bypassNames.MatchString(lit.Value),
"%s: configuration key [%s] controls verification",
fset.Position(lit.Pos()), lit.Value)
}
}
}
}
}
}
})
}
}
// TestCheckResultsAreNotDiscarded asserts that no caller of an integrity check
// throws its verdict away. A check whose error is assigned to the blank
// identifier, or called as a bare statement, reports nothing and is
// indistinguishable at runtime from a check that was never added — which is
// exactly the bypass this file exists to prevent, arrived at by accident rather
// than by design.
func TestCheckResultsAreNotDiscarded(t *testing.T) {
for _, dir := range checkedPackages {
if dir == "." {
continue // the checks do not call each other
}
t.Run(dir, func(t *testing.T) {
fset, files := parsePackageDir(t, dir)
for _, file := range files {
ast.Inspect(file, func(n ast.Node) bool {
switch stmt := n.(type) {
case *ast.ExprStmt:
// integrity.CheckX(...) as a statement of its own
assert.False(t, isIntegrityCheckCall(stmt.X),
"%s: the result of this integrity check is discarded",
fset.Position(stmt.Pos()))
case *ast.AssignStmt:
if len(stmt.Rhs) != 1 || !isIntegrityCheckCall(stmt.Rhs[0]) {
return true
}
for _, lhs := range stmt.Lhs {
ident, ok := lhs.(*ast.Ident)
assert.False(t, ok && ident.Name == "_",
"%s: the result of this integrity check is assigned to the blank identifier",
fset.Position(stmt.Pos()))
}
}
return true
})
}
})
}
}
// isIntegrityCheckCall reports whether expr is a call of the form
// integrity.CheckSomething(...).
func isIntegrityCheckCall(expr ast.Expr) bool {
call, ok := expr.(*ast.CallExpr)
if !ok {
return false
}
sel, ok := call.Fun.(*ast.SelectorExpr)
if !ok || !strings.HasPrefix(sel.Sel.Name, "Check") {
return false
}
pkg, ok := sel.X.(*ast.Ident)
return ok && pkg.Name == "integrity"
}