Skip to content

Commit 3edee00

Browse files
ceshercesher
authored
[POA-3819] Redact sensitive paths on all witnesses regardless of status (#120)
@mgritter, @jhzeba I added a test to confirm we still zero out primitives with this change but I wasn't sure how to best test that we also redact path segments, I didn't see an existing example of that, so I could use some help coming up with more tests.
1 parent 009b014 commit 3edee00

9 files changed

+511
-68
lines changed

‎data_masks/redaction_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,54 @@ func TestRedaction(t *testing.T) {
115115
}
116116
}
117117

118+
func TestZeroAllPrimitives(t *testing.T) {
119+
testCases := map[string]struct {
120+
agentConfig optionals.Optional[*kgxapi.FieldRedactionConfig]
121+
inputFile string
122+
expectedFile string
123+
}{
124+
"zero all primitives": {
125+
inputFile: "001-witness.pb.txt",
126+
expectedFile: "001-expected-zero-all-primitives.pb.txt",
127+
},
128+
"redact secret in path": {
129+
inputFile: "005-witness.pb.txt",
130+
expectedFile: "005-expected-redacted-path.pb.txt",
131+
},
132+
}
133+
134+
for testName, testCase := range testCases {
135+
func() {
136+
ctrl := gomock.NewController(t)
137+
mockClient := mockrest.NewMockLearnClient(ctrl)
138+
defer ctrl.Finish()
139+
140+
agentConfig := kgxapi.NewServiceAgentConfig()
141+
if fieldsToRedact, exists := testCase.agentConfig.Get(); exists {
142+
agentConfig.FieldsToRedact = fieldsToRedact
143+
}
144+
145+
mockClient.
146+
EXPECT().
147+
GetDynamicAgentConfigForService(gomock.Any(), gomock.Any()).
148+
AnyTimes().
149+
Return(agentConfig, nil)
150+
151+
o, err := NewRedactor(akid.GenerateServiceID(), mockClient)
152+
assert.NoError(t, err)
153+
154+
testWitness := test.LoadWitnessFromFileOrDie(filepath.Join("testdata", testCase.inputFile))
155+
expectedWitness := test.LoadWitnessFromFileOrDie(filepath.Join("testdata", testCase.expectedFile))
156+
157+
o.ZeroAllPrimitives(testWitness.Method)
158+
159+
if diff := cmp.Diff(expectedWitness, testWitness, cmpOptions...); diff != "" {
160+
t.Errorf("found unexpected diff in test case %q:\n%s", testName, diff)
161+
}
162+
}()
163+
}
164+
}
165+
118166
func BenchmarkRedaction(b *testing.B) {
119167
ctrl := gomock.NewController(b)
120168
mockClient := mockrest.NewMockLearnClient(ctrl)
@@ -137,3 +185,26 @@ func BenchmarkRedaction(b *testing.B) {
137185
o.RedactSensitiveData(testWitness.Method)
138186
}
139187
}
188+
189+
func BenchmarkZeroAllPrimitives(b *testing.B) {
190+
ctrl := gomock.NewController(b)
191+
mockClient := mockrest.NewMockLearnClient(ctrl)
192+
defer ctrl.Finish()
193+
194+
mockClient.
195+
EXPECT().
196+
GetDynamicAgentConfigForService(gomock.Any(), gomock.Any()).
197+
AnyTimes().
198+
Return(kgxapi.NewServiceAgentConfig(), nil)
199+
200+
o, err := NewRedactor(akid.GenerateServiceID(), mockClient)
201+
assert.NoError(b, err)
202+
203+
testWitness := test.LoadWitnessFromFileOrDie(filepath.Join("testdata", "001-witness.pb.txt"))
204+
205+
b.ResetTimer()
206+
207+
for i := 0; i < b.N; i++ {
208+
o.ZeroAllPrimitives(testWitness.Method)
209+
}
210+
}

‎data_masks/redactor.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,18 @@ func (o *Redactor) RedactSensitiveData(m *pb.Method) {
131131
vis.Apply(&pov, m)
132132
}
133133

134+
func (o *Redactor) ZeroAllPrimitives(m *pb.Method) {
135+
o.userConfig.RLock()
136+
defer o.userConfig.RUnlock()
137+
138+
zpv := zeroPrimitivesVisitor{
139+
redactionOptions: o,
140+
}
141+
vis.Apply(&zpv, m)
142+
143+
m.GetMeta().GetHttp().Obfuscation = pb.HTTPMethodMeta_ZERO_VALUE
144+
}
145+
134146
// Determines whether fields with the given name should be redacted. Caller must
135147
// hold at least a read lock on muUserConfig.
136148
func (o *Redactor) redactFieldsNamed(fieldName string) bool {
@@ -261,3 +273,53 @@ func (*redactPrimitivesVisitor) EnterData(self interface{}, _ vis.SpecVisitorCon
261273
func redactPrimitive(p *pb.Primitive) {
262274
p.Value = spec_util.NewPrimitiveString(RedactionString).Value
263275
}
276+
277+
// Replaces all primitive values with zero values.
278+
type zeroPrimitivesVisitor struct {
279+
vis.DefaultSpecVisitorImpl
280+
redactionOptions *Redactor
281+
}
282+
283+
var _ vis.DefaultSpecVisitor = (*zeroPrimitivesVisitor)(nil)
284+
285+
// EnterData processes the given data and replaces all the primitive values
286+
// with zero values, regardless of its metadata.
287+
func (*zeroPrimitivesVisitor) EnterData(self interface{}, _ vis.SpecVisitorContext, d *pb.Data) Cont {
288+
dp := d.GetPrimitive()
289+
if dp == nil {
290+
return Continue
291+
}
292+
293+
pv, err := spec_util.PrimitiveValueFromProto(dp)
294+
if err != nil {
295+
printer.Warningf("failed to zero out raw value, dropping\n")
296+
d.Value = nil
297+
return Continue
298+
}
299+
300+
dp.Value = pv.Obfuscate().ToProto().Value
301+
return Continue
302+
}
303+
304+
func (s *zeroPrimitivesVisitor) EnterHTTPMethodMeta(self interface{}, ctx vis.SpecVisitorContext, meta *pb.HTTPMethodMeta) Cont {
305+
pathSegments := strings.Split(meta.PathTemplate, "/")
306+
307+
for i, segment := range pathSegments {
308+
// Check if the path segment contains sensitive information.
309+
if s.isSensitiveString(segment) {
310+
pathSegments[i] = RedactionString
311+
}
312+
}
313+
314+
meta.PathTemplate = strings.Join(pathSegments, "/")
315+
return SkipChildren
316+
}
317+
318+
func (s *zeroPrimitivesVisitor) isSensitiveString(v string) bool {
319+
for _, pattern := range s.redactionOptions.SensitiveDataValuePatterns {
320+
if pattern.MatchString(v) {
321+
return true
322+
}
323+
}
324+
return s.redactionOptions.userConfig.redactStringRegex(v)
325+
}

‎data_masks/testdata/001-expected-zero-all-primitives.pb.txt

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
# api_spec.Witness proto
2+
3+
method: {
4+
meta: {
5+
http: {
6+
method: "POST"
7+
path_template: "/v1/doggos"
8+
host: "example.com"
9+
}
10+
}
11+
args: {
12+
key: "KC2RO-pCNJA="
13+
value: {
14+
primitive: {
15+
string_value: {}
16+
}
17+
meta: {
18+
http: {
19+
header: {
20+
key: "Normal-Header"
21+
}
22+
}
23+
}
24+
}
25+
}
26+
args: {
27+
key: "4F1vWo8G_-Q="
28+
value: {
29+
primitive: {
30+
string_value: {}
31+
}
32+
meta: {
33+
http: {
34+
header: {
35+
key: "x-access-token"
36+
}
37+
}
38+
}
39+
}
40+
}
41+
args: {
42+
key: "MWeG2T99uHI="
43+
value: {
44+
struct: {
45+
fields: {
46+
key: "name"
47+
value: {
48+
primitive: {
49+
string_value: {}
50+
}
51+
}
52+
}
53+
fields: {
54+
key: "number"
55+
value: {
56+
primitive: {
57+
int64_value: {}
58+
}
59+
}
60+
}
61+
fields: {
62+
key: "secret-value"
63+
value: {
64+
primitive: {
65+
string_value: {}
66+
}
67+
}
68+
}
69+
}
70+
meta: {
71+
http: {
72+
body: {
73+
content_type: JSON
74+
other_type: "application/json"
75+
}
76+
}
77+
}
78+
}
79+
}
80+
responses: {
81+
key: "T7Jfr4mf1Zs="
82+
value: {
83+
struct: {
84+
fields: {
85+
key: "homes"
86+
value: {
87+
list: {
88+
elems: {
89+
primitive: {
90+
string_value: {}
91+
}
92+
}
93+
elems: {
94+
primitive: {
95+
string_value: {}
96+
}
97+
}
98+
elems: {
99+
primitive: {
100+
string_value: {}
101+
}
102+
}
103+
}
104+
}
105+
}
106+
}
107+
meta: {
108+
http: {
109+
body: {
110+
content_type: JSON
111+
other_type: "application/json"
112+
}
113+
response_code: 404
114+
}
115+
}
116+
}
117+
}
118+
}

‎data_masks/testdata/005-expected-redacted-path.pb.txt

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
# api_spec.Witness proto
2+
3+
method: {
4+
meta: {
5+
http: {
6+
method: "POST"
7+
path_template: "/v1/doggos/*REDACTED*"
8+
host: "example.com"
9+
}
10+
}
11+
args: {
12+
key: "KC2RO-pCNJA="
13+
value: {
14+
primitive: {
15+
string_value: {}
16+
}
17+
meta: {
18+
http: {
19+
header: {
20+
key: "Normal-Header"
21+
}
22+
}
23+
}
24+
}
25+
}
26+
args: {
27+
key: "4F1vWo8G_-Q="
28+
value: {
29+
primitive: {
30+
string_value: {}
31+
}
32+
meta: {
33+
http: {
34+
header: {
35+
key: "x-access-token"
36+
}
37+
}
38+
}
39+
}
40+
}
41+
args: {
42+
key: "MWeG2T99uHI="
43+
value: {
44+
struct: {
45+
fields: {
46+
key: "name"
47+
value: {
48+
primitive: {
49+
string_value: {}
50+
}
51+
}
52+
}
53+
fields: {
54+
key: "number"
55+
value: {
56+
primitive: {
57+
int64_value: {}
58+
}
59+
}
60+
}
61+
fields: {
62+
key: "secret-value"
63+
value: {
64+
primitive: {
65+
string_value: {}
66+
}
67+
}
68+
}
69+
}
70+
meta: {
71+
http: {
72+
body: {
73+
content_type: JSON
74+
other_type: "application/json"
75+
}
76+
}
77+
}
78+
}
79+
}
80+
responses: {
81+
key: "T7Jfr4mf1Zs="
82+
value: {
83+
struct: {
84+
fields: {
85+
key: "homes"
86+
value: {
87+
list: {
88+
elems: {
89+
primitive: {
90+
string_value: {}
91+
}
92+
}
93+
elems: {
94+
primitive: {
95+
string_value: {}
96+
}
97+
}
98+
elems: {
99+
primitive: {
100+
string_value: {}
101+
}
102+
}
103+
}
104+
}
105+
}
106+
}
107+
meta: {
108+
http: {
109+
body: {
110+
content_type: JSON
111+
other_type: "application/json"
112+
}
113+
response_code: 404
114+
}
115+
}
116+
}
117+
}
118+
}

0 commit comments

Comments
 (0)