Skip to content

PHPLIB-664, PHPLIB-676, and PHPLIB-703: APM redaction and observeSensitiveCommands #848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions tests/UnifiedSpecTests/Context.php
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,21 @@ static function (string $tag): array {

private function createClient(string $id, stdClass $o): void
{
Util::assertHasOnlyKeys($o, ['id', 'uriOptions', 'useMultipleMongoses', 'observeEvents', 'ignoreCommandMonitoringEvents', 'serverApi', 'storeEventsAsEntities']);
Util::assertHasOnlyKeys($o, [
'id',
'uriOptions',
'useMultipleMongoses',
'observeEvents',
'ignoreCommandMonitoringEvents',
'observeSensitiveCommands',
'serverApi',
'storeEventsAsEntities',
]);

$useMultipleMongoses = $o->useMultipleMongoses ?? null;
$observeEvents = $o->observeEvents ?? null;
$ignoreCommandMonitoringEvents = $o->ignoreCommandMonitoringEvents ?? [];
$observeSensitiveCommands = $o->observeSensitiveCommands ?? false;
$serverApi = $o->serverApi ?? null;
$storeEventsAsEntities = $o->storeEventsAsEntities ?? null;

Expand Down Expand Up @@ -289,8 +299,9 @@ private function createClient(string $id, stdClass $o): void
if (isset($observeEvents)) {
assertIsArray($observeEvents);
assertIsArray($ignoreCommandMonitoringEvents);
assertIsBool($observeSensitiveCommands);

$this->eventObserversByClient[$id] = new EventObserver($observeEvents, $ignoreCommandMonitoringEvents, $id, $this);
$this->eventObserversByClient[$id] = new EventObserver($observeEvents, $ignoreCommandMonitoringEvents, $observeSensitiveCommands, $id, $this);
}

if (isset($storeEventsAsEntities)) {
Expand Down
88 changes: 66 additions & 22 deletions tests/UnifiedSpecTests/EventObserver.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
use PHPUnit\Framework\Assert;
use stdClass;

use function array_fill_keys;
use function array_reverse;
use function count;
use function current;
Expand All @@ -38,22 +37,36 @@
*/
final class EventObserver implements CommandSubscriber
{
/** @var array */
private static $defaultIgnoreCommands = [
// failPoint and targetedFailPoint operations
'configureFailPoint',
// See: https://github.com/mongodb/specifications/blob/master/source/command-monitoring/command-monitoring.rst#security
'authenticate',
'saslStart',
'saslContinue',
'getnonce',
'createUser',
'updateUser',
'copydbgetnonce',
'copydbsaslstart',
'copydb',
'isMaster',
'hello',
/**
* These commands are always considered sensitive (i.e. command and reply
* documents should be redacted).
*
* @see https://github.com/mongodb/specifications/blob/master/source/command-monitoring/command-monitoring.rst#security
* @var array
*/
private static $sensitiveCommands = [
'authenticate' => 1,
'saslStart' => 1,
'saslContinue' => 1,
'getnonce' => 1,
'createUser' => 1,
'updateUser' => 1,
'copydbgetnonce' => 1,
'copydbsaslstart' => 1,
'copydb' => 1,
];

/**
* These commands are only considered sensitive when the command or reply
* document includes a speculativeAuthenticate field.
*
* @see https://github.com/mongodb/specifications/blob/master/source/command-monitoring/command-monitoring.rst#security
* @var array
*/
private static $sensitiveCommandsWithSpeculativeAuthenticate = [
'ismaster' => 1,
'isMaster' => 1,
'hello' => 1,
];

/** @var array */
Expand All @@ -72,13 +85,21 @@ final class EventObserver implements CommandSubscriber
/** @var Context */
private $context;

/** @var array */
private $ignoreCommands = [];
/**
* The configureFailPoint command (used by failPoint and targetedFailPoint
* operations) is always ignored.
*
* @var array
*/
private $ignoreCommands = ['configureFailPoint' => 1];

/** @var array */
private $observeEvents = [];

public function __construct(array $observeEvents, array $ignoreCommands, string $clientId, Context $context)
/** @var bool */
private $observeSensitiveCommands;

public function __construct(array $observeEvents, array $ignoreCommands, bool $observeSensitiveCommands, string $clientId, Context $context)
{
assertNotEmpty($observeEvents);

Expand All @@ -88,13 +109,12 @@ public function __construct(array $observeEvents, array $ignoreCommands, string
$this->observeEvents[self::$supportedEvents[$event]] = 1;
}

$this->ignoreCommands = array_fill_keys(self::$defaultIgnoreCommands, 1);

foreach ($ignoreCommands as $command) {
assertIsString($command);
$this->ignoreCommands[$command] = 1;
}

$this->observeSensitiveCommands = $observeSensitiveCommands;
$this->clientId = $clientId;
$this->context = $context;
}
Expand Down Expand Up @@ -270,6 +290,30 @@ private function handleEvent($event): void
return;
}

if (! $this->observeSensitiveCommands && $this->isSensitiveCommand($event)) {
return;
}

$this->actualEvents[] = $event;
}

/** @param CommandStartedEvent|CommandSucceededEvent|CommandFailedEvent $event */
private function isSensitiveCommand($event): bool
{
if (isset(self::$sensitiveCommands[$event->getCommandName()])) {
return true;
}

/* If the command or reply included a speculativeAuthenticate field,
* libmongoc will already have redacted it (CDRIVER-4000). Therefore, we
* can infer that the command was sensitive if its command or reply is
* empty. */
if (isset(self::$sensitiveCommandsWithSpeculativeAuthenticate[$event->getCommandName()])) {
$commandOrReply = $event instanceof CommandStartedEvent ? $event->getCommand() : $event->getReply();

return (array) $commandOrReply === [];
}

return false;
}
}
13 changes: 13 additions & 0 deletions tests/UnifiedSpecTests/UnifiedSpecTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,19 @@ public function provideCollectionManagementTests()
return $this->provideTests(__DIR__ . '/collection-management/*.json');
}

/**
* @dataProvider provideCommandMonitoringTests
*/
public function testCommandMonitoring(UnifiedTestCase $test): void
{
self::$runner->run($test);
}

public function provideCommandMonitoringTests()
{
return $this->provideTests(__DIR__ . '/command-monitoring/*.json');
}

/**
* @dataProvider provideCrudTests
*/
Expand Down
2 changes: 1 addition & 1 deletion tests/UnifiedSpecTests/UnifiedTestRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ final class UnifiedTestRunner
public const SERVER_ERROR_UNAUTHORIZED = 13;

public const MIN_SCHEMA_VERSION = '1.0';
public const MAX_SCHEMA_VERSION = '1.4';
public const MAX_SCHEMA_VERSION = '1.5';

/** @var MongoDB\Client */
private $internalClient;
Expand Down
Loading