Skip to content

Commit b945fbd

Browse files
benjirewisbenjirewis
authored
GODRIVER-2013 Security-sensitive command-monitoring redactions (#681)
1 parent 75c61d6 commit b945fbd

File tree

8 files changed

+815
-23
lines changed

8 files changed

+815
-23
lines changed

‎data/command-monitoring/unified/redacted-commands.json

Lines changed: 492 additions & 0 deletions
Large diffs are not rendered by default.

‎data/command-monitoring/unified/redacted-commands.yml

Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
description: "redacted-commands"
2+
3+
schemaVersion: "1.5"
4+
5+
runOnRequirements:
6+
- minServerVersion: "5.0"
7+
auth: false
8+
9+
createEntities:
10+
- client:
11+
id: &client client
12+
observeEvents:
13+
- commandStartedEvent
14+
observeSensitiveCommands: true
15+
- database:
16+
id: &database database
17+
client: *client
18+
databaseName: &databaseName command-monitoring-tests
19+
20+
tests:
21+
- description: "authenticate"
22+
operations:
23+
- name: runCommand
24+
object: *database
25+
arguments:
26+
commandName: authenticate
27+
command:
28+
authenticate: "private"
29+
# Malformed authentication commands will fail against the server, but we
30+
# just want to assert that security-related commands are redacted
31+
# in command monitoring.
32+
expectError:
33+
isError: true
34+
expectEvents:
35+
- client: *client
36+
events:
37+
- commandStartedEvent:
38+
commandName: authenticate
39+
# This only asserts that the command name has been redacted from the published command;
40+
# however, it's unlikely that a driver would redact this but leave other, sensitive fields.
41+
# We cannot simply assert that command is an empty document because it's at root-level, so
42+
# additional fields in the actual document will be permitted
43+
command: { authenticate: { $$exists: false } }
44+
45+
- description: "saslStart"
46+
operations:
47+
- name: runCommand
48+
object: *database
49+
arguments:
50+
commandName: saslStart
51+
command:
52+
saslStart: "private"
53+
expectError:
54+
isError: true
55+
expectEvents:
56+
- client: *client
57+
events:
58+
- commandStartedEvent:
59+
commandName: saslStart
60+
command: { saslStart: { $$exists: false } }
61+
62+
- description: "saslContinue"
63+
operations:
64+
- name: runCommand
65+
object: *database
66+
arguments:
67+
commandName: saslContinue
68+
command:
69+
saslContinue: "private"
70+
expectError:
71+
isError: true
72+
expectEvents:
73+
- client: *client
74+
events:
75+
- commandStartedEvent:
76+
commandName: saslContinue
77+
command: { saslContinue: { $$exists: false } }
78+
79+
- description: "getnonce"
80+
operations:
81+
- name: runCommand
82+
object: *database
83+
arguments:
84+
commandName: getnonce
85+
command:
86+
getnonce: "private"
87+
expectEvents:
88+
- client: *client
89+
events:
90+
- commandStartedEvent:
91+
commandName: getnonce
92+
command: { getnonce: { $$exists: false } }
93+
94+
- description: "createUser"
95+
operations:
96+
- name: runCommand
97+
object: *database
98+
arguments:
99+
commandName: createUser
100+
command:
101+
createUser: "private"
102+
expectError:
103+
isError: true
104+
expectEvents:
105+
- client: *client
106+
events:
107+
- commandStartedEvent:
108+
commandName: createUser
109+
command: { createUser: { $$exists: false } }
110+
111+
- description: "updateUser"
112+
operations:
113+
- name: runCommand
114+
object: *database
115+
arguments:
116+
commandName: updateUser
117+
command:
118+
updateUser: "private"
119+
expectError:
120+
isError: true
121+
expectEvents:
122+
- client: *client
123+
events:
124+
- commandStartedEvent:
125+
commandName: updateUser
126+
command: { updateUser: { $$exists: false } }
127+
128+
- description: "copydbgetnonce"
129+
operations:
130+
- name: runCommand
131+
object: *database
132+
arguments:
133+
commandName: copydbgetnonce
134+
command:
135+
copydbgetnonce: "private"
136+
expectError:
137+
isError: true
138+
expectEvents:
139+
- client: *client
140+
events:
141+
- commandStartedEvent:
142+
commandName: copydbgetnonce
143+
command: { copydbgetnonce: { $$exists: false } }
144+
145+
- description: "copydbsaslstart"
146+
operations:
147+
- name: runCommand
148+
object: *database
149+
arguments:
150+
commandName: copydbsaslstart
151+
command:
152+
copydbsaslstart: "private"
153+
expectError:
154+
isError: true
155+
expectEvents:
156+
- client: *client
157+
events:
158+
- commandStartedEvent:
159+
commandName: copydbsaslstart
160+
command: { copydbsaslstart: { $$exists: false } }
161+
162+
- description: "copydb"
163+
operations:
164+
- name: runCommand
165+
object: *database
166+
arguments:
167+
commandName: copydb
168+
command:
169+
copydb: "private"
170+
expectError:
171+
isError: true
172+
expectEvents:
173+
- client: *client
174+
events:
175+
- commandStartedEvent:
176+
commandName: copydb
177+
command: { copydb: { $$exists: false } }
178+
179+
- description: "hello with speculative authenticate"
180+
operations:
181+
- name: runCommand
182+
object: *database
183+
arguments:
184+
commandName: hello
185+
command:
186+
hello: "private"
187+
speculativeAuthenticate: "foo"
188+
expectError:
189+
isError: true
190+
- name: runCommand
191+
object: *database
192+
arguments:
193+
commandName: ismaster
194+
command:
195+
ismaster: "private"
196+
speculativeAuthenticate: "foo"
197+
expectError:
198+
isError: true
199+
- name: runCommand
200+
object: *database
201+
arguments:
202+
commandName: isMaster
203+
command:
204+
isMaster: "private"
205+
speculativeAuthenticate: "foo"
206+
expectError:
207+
isError: true
208+
expectEvents:
209+
- client: *client
210+
events:
211+
- commandStartedEvent:
212+
commandName: hello
213+
command: { hello: { $$exists: false } }
214+
- commandStartedEvent:
215+
commandName: ismaster
216+
command: { ismaster: { $$exists: false } }
217+
- commandStartedEvent:
218+
commandName: isMaster
219+
command: { isMaster: { $$exists: false } }
220+
221+
- description: "hello without speculative authenticate is not redacted"
222+
operations:
223+
- name: runCommand
224+
object: *database
225+
arguments:
226+
commandName: hello
227+
command:
228+
hello: "public"
229+
- name: runCommand
230+
object: *database
231+
arguments:
232+
commandName: ismaster
233+
command:
234+
ismaster: "public"
235+
- name: runCommand
236+
object: *database
237+
arguments:
238+
commandName: isMaster
239+
command:
240+
isMaster: "public"
241+
expectEvents:
242+
- client: *client
243+
events:
244+
- commandStartedEvent:
245+
commandName: hello
246+
command: { hello: "public" }
247+
- commandStartedEvent:
248+
commandName: ismaster
249+
command: { ismaster: "public" }
250+
- commandStartedEvent:
251+
commandName: isMaster
252+
command: { isMaster: "public" }

‎mongo/integration/unified/client_entity.go

Lines changed: 50 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ package unified
99
import (
1010
"context"
1111
"fmt"
12+
"strings"
1213
"sync/atomic"
1314
"time"
1415

@@ -22,18 +23,23 @@ import (
2223
"go.mongodb.org/mongo-driver/x/bsonx/bsoncore"
2324
)
2425

26+
// Security-sensitive commands that should be ignored in command monitoring by default.
27+
var securitySensitiveCommands = []string{"authenticate", "saslStart", "saslContinue", "getnonce",
28+
"createUser", "updateUser", "copydbgetnonce", "copydbsaslstart", "copydb"}
29+
2530
// clientEntity is a wrapper for a mongo.Client object that also holds additional information required during test
2631
// execution.
2732
type clientEntity struct {
2833
*mongo.Client
2934

30-
recordEvents atomic.Value
31-
started []*event.CommandStartedEvent
32-
succeeded []*event.CommandSucceededEvent
33-
failed []*event.CommandFailedEvent
34-
pooled []*event.PoolEvent
35-
ignoredCommands map[string]struct{}
36-
numConnsCheckedOut int32
35+
recordEvents atomic.Value
36+
started []*event.CommandStartedEvent
37+
succeeded []*event.CommandSucceededEvent
38+
failed []*event.CommandFailedEvent
39+
pooled []*event.PoolEvent
40+
ignoredCommands map[string]struct{}
41+
observeSensitiveCommands *bool
42+
numConnsCheckedOut int32
3743

3844
// These should not be changed after the clientEntity is initialized
3945
observedEvents map[monitoringEventType]struct{}
@@ -43,14 +49,23 @@ type clientEntity struct {
4349
}
4450

4551
func newClientEntity(ctx context.Context, em *EntityMap, entityOptions *entityOptions) (*clientEntity, error) {
52+
// The "configureFailPoint" command should always be ignored.
53+
ignoredCommands := map[string]struct{}{
54+
"configureFailPoint": {},
55+
}
56+
// If not observing sensitive commands, add security-sensitive commands
57+
// to ignoredCommands by default.
58+
if entityOptions.ObserveSensitiveCommands == nil || !*entityOptions.ObserveSensitiveCommands {
59+
for _, cmd := range securitySensitiveCommands {
60+
ignoredCommands[cmd] = struct{}{}
61+
}
62+
}
4663
entity := &clientEntity{
47-
// The "configureFailPoint" command should always be ignored.
48-
ignoredCommands: map[string]struct{}{
49-
"configureFailPoint": {},
50-
},
51-
observedEvents: make(map[monitoringEventType]struct{}),
52-
storedEvents: make(map[monitoringEventType][]string),
53-
entityMap: em,
64+
ignoredCommands: ignoredCommands,
65+
observedEvents: make(map[monitoringEventType]struct{}),
66+
storedEvents: make(map[monitoringEventType][]string),
67+
entityMap: em,
68+
observeSensitiveCommands: entityOptions.ObserveSensitiveCommands,
5469
}
5570
entity.setRecordEvents(true)
5671

@@ -144,10 +159,30 @@ func (c *clientEntity) stopListeningForEvents() {
144159
c.setRecordEvents(false)
145160
}
146161

162+
func (c *clientEntity) isIgnoredEvent(event *event.CommandStartedEvent) bool {
163+
// Check if command is in ignoredCommands.
164+
if _, ok := c.ignoredCommands[event.CommandName]; ok {
165+
return true
166+
}
167+
168+
if event.CommandName == "hello" || strings.ToLower(event.CommandName) == "ismaster" {
169+
_, err := event.Command.LookupErr("speculativeAuthenticate")
170+
speculativeAuth := err == nil
171+
172+
// If observeSensitiveCommands is false (or unset) and hello command is with
173+
// speculative authenticate, command should be ignored.
174+
if (c.observeSensitiveCommands == nil || !*c.observeSensitiveCommands) &&
175+
speculativeAuth {
176+
return true
177+
}
178+
}
179+
return false
180+
}
181+
147182
func (c *clientEntity) startedEvents() []*event.CommandStartedEvent {
148183
var events []*event.CommandStartedEvent
149184
for _, evt := range c.started {
150-
if _, ok := c.ignoredCommands[evt.CommandName]; !ok {
185+
if !c.isIgnoredEvent(evt) {
151186
events = append(events, evt)
152187
}
153188
}

‎mongo/integration/unified/entity.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,13 @@ type entityOptions struct {
3434
ID string `bson:"id"`
3535

3636
// Options for client entities.
37-
URIOptions bson.M `bson:"uriOptions"`
38-
UseMultipleMongoses *bool `bson:"useMultipleMongoses"`
39-
ObserveEvents []string `bson:"observeEvents"`
40-
IgnoredCommands []string `bson:"ignoreCommandMonitoringEvents"`
41-
StoreEventsAsEntities []storeEventsAsEntitiesConfig `bson:"storeEventsAsEntities"`
42-
ServerAPIOptions *serverAPIOptions `bson:"serverApi"`
37+
URIOptions bson.M `bson:"uriOptions"`
38+
UseMultipleMongoses *bool `bson:"useMultipleMongoses"`
39+
ObserveEvents []string `bson:"observeEvents"`
40+
IgnoredCommands []string `bson:"ignoreCommandMonitoringEvents"`
41+
ObserveSensitiveCommands *bool `bson:"observeSensitiveCommands"`
42+
StoreEventsAsEntities []storeEventsAsEntitiesConfig `bson:"storeEventsAsEntities"`
43+
ServerAPIOptions *serverAPIOptions `bson:"serverApi"`
4344

4445
// Options for database entities.
4546
DatabaseName string `bson:"databaseName"`

‎mongo/integration/unified/event_verification.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,16 @@ func verifyCommandEvents(ctx context.Context, client *clientEntity, expectedEven
167167
if expected.Command != nil {
168168
expectedDoc := documentToRawValue(expected.Command)
169169
actualDoc := documentToRawValue(actual.Command)
170+
171+
// If actual.Command is empty, as is the case with redacted commands,
172+
// verifyValuesMatch will return an error from DocumentOK() because
173+
// there are not enough bytes to read a document from bson.RawValue{}.
174+
// In the case of an empty Command, hardcode an empty bson.RawValue document.
175+
if len(actual.Command) == 0 {
176+
emptyDoc := []byte{5, 0, 0, 0, 0}
177+
actualDoc = bson.RawValue{Type: bsontype.EmbeddedDocument, Value: emptyDoc}
178+
}
179+
170180
if err := verifyValuesMatch(ctx, expectedDoc, actualDoc, true); err != nil {
171181
return newEventVerificationError(idx, client, "error comparing command documents: %v", err)
172182
}

0 commit comments

Comments
 (0)