Skip to content

Commit 42a8184

Browse files
alcaeuskevinAlbs
andcommitted
CDRIVER-4635 Reset speculative_auth_response when resetting auth state (#1258)
* CDRIVER-4635 add regression test * reset authentication state before creating stream * remove unnecessary state reset in `mongoc_topology_scanner_node_disconnect` `mongoc_topology_scanner_node_disconnect` sets `stream` to NULL. That will result in the stream being recreated and state being reset in `mongoc_topology_scanner_node_setup` * skip `/Client/authenticate_cached/client` when using speculativeAuthentication This previously passed because SCRAM would fail speculativeAuthentication and restart the authentication steps. The restart would use the SCRAM cache. Now speculativeAuthentication succeeds. speculativeAuthentication in the topology scanner does not use the SCRAM cache (refer: CDRIVER-3642). This does not result in the expected error. * Reset scram step along with speculative authentication --------- Co-authored-by: Kevin Albertson <[email protected]>
1 parent ef75b3c commit 42a8184

File tree

3 files changed

+187
-7
lines changed

3 files changed

+187
-7
lines changed

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

+15-6
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,10 @@ _begin_hello_cmd (mongoc_topology_scanner_node_t *node,
416416
ssl_opts = ts->ssl_opts;
417417
#endif
418418

419+
// _mongoc_topology_scanner_add_speculative_authentication is called with
420+
// NULL for the scram_cache argument. The scram cache is not used for
421+
// speculative authentication in the topology scanner. This is planned to
422+
// be improved in CDRIVER-3642.
419423
_mongoc_topology_scanner_add_speculative_authentication (
420424
&cmd, ts->uri, ssl_opts, NULL, &node->scram);
421425
}
@@ -601,10 +605,6 @@ mongoc_topology_scanner_node_disconnect (mongoc_topology_scanner_node_t *node,
601605
}
602606

603607
node->stream = NULL;
604-
memset (
605-
&node->sasl_supported_mechs, 0, sizeof (node->sasl_supported_mechs));
606-
node->negotiated_sasl_supported_mechs = false;
607-
bson_reinit (&node->speculative_auth_response);
608608
}
609609
mongoc_server_description_destroy (node->handshake_sd);
610610
node->handshake_sd = NULL;
@@ -1094,6 +1094,17 @@ mongoc_topology_scanner_node_setup (mongoc_topology_scanner_node_t *node,
10941094

10951095
BSON_ASSERT (!node->retired);
10961096

1097+
// If a new stream is needed, reset state authentication state.
1098+
// Authentication state is tied to a stream.
1099+
{
1100+
node->has_auth = false;
1101+
bson_reinit (&node->speculative_auth_response);
1102+
node->scram.step = 0;
1103+
memset (
1104+
&node->sasl_supported_mechs, 0, sizeof (node->sasl_supported_mechs));
1105+
node->negotiated_sasl_supported_mechs = false;
1106+
}
1107+
10971108
if (node->ts->initiator) {
10981109
stream = node->ts->initiator (
10991110
node->ts->uri, &node->host, node->ts->initiator_context, error);
@@ -1124,8 +1135,6 @@ mongoc_topology_scanner_node_setup (mongoc_topology_scanner_node_t *node,
11241135
node->ts->setup_err_cb (node->id, node->ts->cb_data, error);
11251136
return;
11261137
}
1127-
1128-
node->has_auth = false;
11291138
}
11301139

11311140
/*

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -4230,7 +4230,11 @@ test_client_install (TestSuite *suite)
42304230
test_mongoc_client_authenticate_cached_single,
42314231
NULL,
42324232
NULL,
4233-
test_framework_skip_if_no_auth);
4233+
test_framework_skip_if_no_auth,
4234+
// speculativeAuthentication in single-threaded clients
4235+
// does not use the scram cache. speculativeAuthentication
4236+
// was introduced in server 4.4 (maxWireVersion=9)
4237+
test_framework_skip_if_max_wire_version_more_than_8);
42344238
TestSuite_AddFull (suite,
42354239
"/Client/authenticate_failure",
42364240
test_mongoc_client_authenticate_failure,

src/libmongoc/tests/test-mongoc-speculative-auth.c

+167
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,169 @@ test_mongoc_speculative_auth_request_x509_pool (void)
383383
bson_destroy (response);
384384
}
385385

386+
// test_mongoc_speculative_auth_request_x509_network_error is a regression test
387+
// for CDRIVER-4635.
388+
static void
389+
test_mongoc_speculative_auth_request_x509_network_error (void)
390+
{
391+
// Start mock server.
392+
mock_server_t *server;
393+
{
394+
mongoc_ssl_opt_t server_ssl_opts = {0};
395+
server_ssl_opts.ca_file = CERT_CA;
396+
server_ssl_opts.pem_file = CERT_SERVER;
397+
server = mock_server_new ();
398+
mock_server_set_ssl_opts (server, &server_ssl_opts);
399+
mock_server_run (server);
400+
}
401+
402+
// Create URI configured for X509 authentication.
403+
mongoc_uri_t *uri;
404+
{
405+
uri = mongoc_uri_copy (mock_server_get_uri (server));
406+
mongoc_uri_set_option_as_int32 (
407+
uri, MONGOC_URI_HEARTBEATFREQUENCYMS, 15000);
408+
_setup_speculative_auth_x_509 (uri);
409+
}
410+
411+
// Create single threaded client.
412+
mongoc_client_t *client;
413+
{
414+
mongoc_ssl_opt_t client_ssl_opts = {0};
415+
client_ssl_opts.ca_file = CERT_CA;
416+
client_ssl_opts.pem_file = CERT_CLIENT;
417+
client = test_framework_client_new_from_uri (uri, NULL);
418+
ASSERT (client);
419+
mongoc_client_set_ssl_opts (client, &client_ssl_opts);
420+
}
421+
422+
// Send a ping, and receive a network error.
423+
{
424+
bson_error_t error;
425+
426+
// Send ping.
427+
future_t *future = future_client_command_simple (
428+
client, "admin", tmp_bson ("{'ping': 1}"), NULL, NULL, &error);
429+
430+
// Expect a hello including speculativeAuthenticate field.
431+
{
432+
request_t *request = mock_server_receives_any_hello (server);
433+
ASSERT (request);
434+
const bson_t *request_doc = request_get_doc (request, 0);
435+
ASSERT (request_doc);
436+
char *request_str =
437+
bson_as_canonical_extended_json (request_doc, NULL);
438+
ASSERT_WITH_MSG (
439+
bson_has_field (request_doc, "speculativeAuthenticate"),
440+
"expected hello to contain 'speculativeAuthenticate', got: %s",
441+
request_str);
442+
// Respond with a non-empty document "speculativeAuthenticate" field.
443+
// The C driver will interpret this as a successful X509
444+
// authentication.
445+
bson_t *response = BCON_NEW ("ok",
446+
BCON_INT32 (1),
447+
"isWritablePrimary",
448+
BCON_BOOL (true),
449+
"minWireVersion",
450+
BCON_INT32 (WIRE_VERSION_MIN),
451+
"maxWireVersion",
452+
BCON_INT32 (WIRE_VERSION_MAX),
453+
"speculativeAuthenticate",
454+
"{",
455+
"foo",
456+
"bar",
457+
"}");
458+
459+
char *response_str = bson_as_canonical_extended_json (response, NULL);
460+
mock_server_replies_simple (request, response_str);
461+
bson_free (response_str);
462+
bson_destroy (response);
463+
bson_free (request_str);
464+
request_destroy (request);
465+
}
466+
467+
// Expect a ping command. Respond with a network error.
468+
{
469+
request_t *request = mock_server_receives_msg (
470+
server, MONGOC_MSG_NONE, tmp_bson ("{'ping': 1}"));
471+
ASSERT (request);
472+
// Cause a network error.
473+
mock_server_hangs_up (request);
474+
request_destroy (request);
475+
}
476+
477+
// Expect error.
478+
ASSERT (!future_get_bool (future));
479+
ASSERT_ERROR_CONTAINS (error,
480+
MONGOC_ERROR_STREAM,
481+
MONGOC_ERROR_STREAM_SOCKET,
482+
"socket error");
483+
future_destroy (future);
484+
}
485+
486+
// Send another ping, expect another "speculativeAuthenticate" attempt.
487+
{
488+
bson_error_t error;
489+
490+
// Send ping.
491+
future_t *future = future_client_command_simple (
492+
client, "admin", tmp_bson ("{'ping': 1}"), NULL, NULL, &error);
493+
494+
// Expect a hello including speculativeAuthenticate field.
495+
{
496+
request_t *request = mock_server_receives_any_hello (server);
497+
ASSERT (request);
498+
const bson_t *request_doc = request_get_doc (request, 0);
499+
ASSERT (request_doc);
500+
char *request_str =
501+
bson_as_canonical_extended_json (request_doc, NULL);
502+
ASSERT_WITH_MSG (
503+
bson_has_field (request_doc, "speculativeAuthenticate"),
504+
"expected hello to contain 'speculativeAuthenticate', got: %s",
505+
request_str);
506+
// Respond with a non-empty document "speculativeAuthenticate" field.
507+
// The C driver will interpret this as a successful X509
508+
// authentication.
509+
bson_t *response = BCON_NEW ("ok",
510+
BCON_INT32 (1),
511+
"isWritablePrimary",
512+
BCON_BOOL (true),
513+
"minWireVersion",
514+
BCON_INT32 (WIRE_VERSION_MIN),
515+
"maxWireVersion",
516+
BCON_INT32 (WIRE_VERSION_MAX),
517+
"speculativeAuthenticate",
518+
"{",
519+
"foo",
520+
"bar",
521+
"}");
522+
523+
char *response_str = bson_as_canonical_extended_json (response, NULL);
524+
mock_server_replies_simple (request, response_str);
525+
bson_free (response_str);
526+
bson_destroy (response);
527+
request_destroy (request);
528+
bson_free (request_str);
529+
}
530+
531+
// Expect a ping command. Respond with {"ok": 1}.
532+
{
533+
request_t *request = mock_server_receives_msg (
534+
server, MONGOC_MSG_NONE, tmp_bson ("{'ping': 1}"));
535+
ASSERT (request);
536+
mock_server_replies_ok_and_destroys (request);
537+
}
538+
539+
// Expect success.
540+
ASSERT_OR_PRINT (future_get_bool (future), error);
541+
future_destroy (future);
542+
}
543+
544+
mongoc_client_destroy (client);
545+
mongoc_uri_destroy (uri);
546+
mock_server_destroy (server);
547+
}
548+
386549
static void
387550
test_mongoc_speculative_auth_request_scram (void)
388551
{
@@ -450,6 +613,10 @@ test_speculative_auth_install (TestSuite *suite)
450613
TestSuite_AddMockServerTest (suite,
451614
"/speculative_auth_pool/request_x509",
452615
test_mongoc_speculative_auth_request_x509_pool);
616+
TestSuite_AddMockServerTest (
617+
suite,
618+
"/speculative_auth/request_x509/network_error",
619+
test_mongoc_speculative_auth_request_x509_network_error);
453620
#endif /* MONGOC_ENABLE_SSL_* */
454621
TestSuite_AddMockServerTest (
455622
suite,

0 commit comments

Comments
 (0)