Skip to content

Commit be4008a

Browse files
committed
CDRIVER-4699 fix leak on repeated attempts to authenticate (#1364)
* CDRIVER-4699 add regression test * Call `_mongoc_scram_destroy` when resetting auth state * only call if `MONGOC_ENABLE_CRYPTO` is defined `_mongoc_scram_destroy` is conditionally defined. If `MONGOC_ENABLE_CRYPTO` is not defined, scram auth is not supported.
1 parent 48d2577 commit be4008a

File tree

2 files changed

+41
-1
lines changed

2 files changed

+41
-1
lines changed

src/libmongoc/src/mongoc/mongoc-topology-scanner.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1102,7 +1102,10 @@ mongoc_topology_scanner_node_setup (mongoc_topology_scanner_node_t *node,
11021102
{
11031103
node->has_auth = false;
11041104
bson_reinit (&node->speculative_auth_response);
1105-
node->scram.step = 0;
1105+
#ifdef MONGOC_ENABLE_CRYPTO
1106+
// Destroy and zero `node->scram`.
1107+
_mongoc_scram_destroy (&node->scram);
1108+
#endif
11061109
memset (
11071110
&node->sasl_supported_mechs, 0, sizeof (node->sasl_supported_mechs));
11081111
node->negotiated_sasl_supported_mechs = false;

src/libmongoc/tests/test-mongoc-client.c

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4225,6 +4225,42 @@ test_mongoc_client_resends_handshake_on_network_error (void)
42254225
mock_server_destroy (server);
42264226
}
42274227

4228+
// test_failure_to_auth is a regression test for the leak reported in
4229+
// CDRIVER-4699.
4230+
static void
4231+
test_failure_to_auth (void)
4232+
{
4233+
mongoc_uri_t *uri = mongoc_uri_new_for_host_port ("localhost", 12345);
4234+
mongoc_uri_set_username (uri, "foo");
4235+
mongoc_uri_set_password (uri, "bar");
4236+
mongoc_uri_set_option_as_bool (
4237+
uri, MONGOC_URI_SERVERSELECTIONTRYONCE, false);
4238+
// Set a shorter serverSelectionTimeoutMS for a faster test.
4239+
// serverSelectionTimeoutMS must be long enough to require a second attempt
4240+
// of authentication. Experimentally: 100ms appears long enough to reproduce
4241+
// leak reported in CDRIVER-4699.
4242+
mongoc_uri_set_option_as_int32 (
4243+
uri, MONGOC_URI_SERVERSELECTIONTIMEOUTMS, 100);
4244+
mongoc_client_t *client = mongoc_client_new_from_uri (uri);
4245+
4246+
// Override minHeartbeatFrequencyMS to reduce the time between server checks.
4247+
client->topology->min_heartbeat_frequency_msec = 1;
4248+
4249+
// Disable the cooldown period to reduce the time between server checks.
4250+
// Single threaded clients wait for a default 5 second cooldown period after
4251+
// failing to connect to a server before making another attempt.
4252+
_mongoc_topology_bypass_cooldown (client->topology);
4253+
4254+
bool ok = mongoc_client_command_simple (client,
4255+
"admin",
4256+
tmp_bson ("{'ping': 1}"),
4257+
NULL /* read prefs */,
4258+
NULL /* reply */,
4259+
NULL /* error */);
4260+
ASSERT_WITH_MSG (!ok, "expected command to fail, got success");
4261+
mongoc_client_destroy (client);
4262+
mongoc_uri_destroy (uri);
4263+
}
42284264
void
42294265
test_client_install (TestSuite *suite)
42304266
{
@@ -4538,4 +4574,5 @@ test_client_install (TestSuite *suite)
45384574
suite,
45394575
"/Client/resends_handshake_on_network_error",
45404576
test_mongoc_client_resends_handshake_on_network_error);
4577+
TestSuite_Add (suite, "/Client/failure_to_auth", test_failure_to_auth);
45414578
}

0 commit comments

Comments
 (0)