CDRIVER-935 pooled clients require SSL if opts set

In 1.1.x, all clients (pooled and single-threaded) required an SSL
connection to the server only if "ssl=true" was in the URI. In 1.2.0
this changed unintentionally (but beneficially) for single-threaded
clients: setting a single-threaded client's SSL options put the client
in SSL mode, even if "ssl=true" is omitted from the URI.

Pooled clients, however, stopped working entirely if the options are
set but "ssl=true" is omitted. This patch fixes pooled clients and
tests the new behavior for both.

Author: ajdavis

src/mongoc/mongoc-client-private.h

@@ -54,6 +54,7 @@ struct _mongoc_client_t
    void                      *initiator_data;
 
 #ifdef MONGOC_ENABLE_SSL
+   bool                       use_ssl;
    mongoc_ssl_opt_t           ssl_opts;
    char                      *pem_subject;
 #endif

src/mongoc/mongoc-client.c

@@ -274,10 +274,8 @@ mongoc_client_default_stream_initiator (const mongoc_uri_t       *uri,
    mongoc_stream_t *base_stream = NULL;
 #ifdef MONGOC_ENABLE_SSL
    mongoc_client_t *client = (mongoc_client_t *)user_data;
-   const bson_t *options;
-   bson_iter_t iter;
    const char *mechanism;
-   int32_t connecttimeoutms = MONGOC_DEFAULT_CONNECTTIMEOUTMS;
+   int32_t connecttimeoutms;
 #endif
 
    BSON_ASSERT (uri);
@@ -314,11 +312,9 @@ mongoc_client_default_stream_initiator (const mongoc_uri_t       *uri,
 
 #ifdef MONGOC_ENABLE_SSL
    if (base_stream) {
-      options = mongoc_uri_get_options (uri);
       mechanism = mongoc_uri_get_auth_mechanism (uri);
 
-      if ((bson_iter_init_find_case (&iter, options, "ssl") &&
-           bson_iter_as_bool (&iter)) ||
+      if (client->use_ssl ||
           (mechanism && (0 == strcmp (mechanism, "MONGODB-X509")))) {
          base_stream = mongoc_stream_tls_new (base_stream, &client->ssl_opts,
                                               true);
@@ -646,6 +642,7 @@ mongoc_client_set_ssl_opts (mongoc_client_t        *client,
    BSON_ASSERT (client);
    BSON_ASSERT (opts);
 
+   client->use_ssl = true;
    memcpy (&client->ssl_opts, opts, sizeof client->ssl_opts);
 
    bson_free (client->pem_subject);
@@ -712,10 +709,6 @@ _mongoc_client_new_from_uri (const mongoc_uri_t *uri, mongoc_topology_t *topolog
    mongoc_client_t *client;
    const mongoc_read_prefs_t *read_prefs;
    const mongoc_write_concern_t *write_concern;
-#ifdef MONGOC_ENABLE_SSL
-   const bson_t *options;
-   bson_iter_t iter;
-#endif
 
    BSON_ASSERT (uri);
 
@@ -735,12 +728,10 @@ _mongoc_client_new_from_uri (const mongoc_uri_t *uri, mongoc_topology_t *topolog
    mongoc_cluster_init (&client->cluster, client->uri, client);
 
 #ifdef MONGOC_ENABLE_SSL
-   options = mongoc_uri_get_options (client->uri);
-
-   if (bson_iter_init_find (&iter, options, "ssl") &&
-       BSON_ITER_HOLDS_BOOL (&iter) &&
-       bson_iter_bool (&iter)) {
-         mongoc_client_set_ssl_opts (client, mongoc_ssl_opt_get_default ());
+   client->use_ssl = false;
+   if (mongoc_uri_get_ssl (client->uri)) {
+      /* sets use_ssl = true */
+      mongoc_client_set_ssl_opts (client, mongoc_ssl_opt_get_default ());
    }
 #endif
 

tests/debug-stream.c

@@ -180,7 +180,7 @@ debug_stream_initiator (const mongoc_uri_t       *uri,
 
    default_stream = mongoc_client_default_stream_initiator (uri,
                                                             host,
-                                                            user_data,
+                                                            stats->client,
                                                             error);
 
    return debug_stream_new (default_stream, stats);
@@ -191,6 +191,7 @@ void
 test_framework_set_debug_stream (mongoc_client_t *client,
                                  debug_stream_stats_t *stats)
 {
+   stats->client = client;
    mongoc_client_set_stream_initiator (client,
                                        debug_stream_initiator,
                                        stats);

tests/test-libmongoc.c

@@ -443,7 +443,7 @@ test_framework_add_user_password (const char *uri_str,
  *
  *--------------------------------------------------------------------------
  */
-static char *
+char *
 test_framework_add_user_password_from_env (const char *uri_str)
 {
    char *user;
@@ -858,6 +858,30 @@ test_framework_client_new ()
 }
 
 
+#ifdef MONGOC_ENABLE_SSL
+/*
+ *--------------------------------------------------------------------------
+ *
+ * test_framework_get_ssl_opts --
+ *
+ *       Get options for connecting to mongod over SSL (even if mongod
+ *       isn't actually SSL-enabled).
+ *
+ * Returns:
+ *       A pointer to constant global SSL-test options.
+ *
+ * Side effects:
+ *       None.
+ *
+ *--------------------------------------------------------------------------
+ */
+const mongoc_ssl_opt_t *
+test_framework_get_ssl_opts (void)
+{
+   return &gSSLOptions;
+}
+#endif
+
 /*
  *--------------------------------------------------------------------------
  *
@@ -874,7 +898,7 @@ test_framework_client_new ()
  *
  *--------------------------------------------------------------------------
  */
-static void
+void
 test_framework_set_pool_ssl_opts (mongoc_client_pool_t *pool)
 {
    assert (pool);

tests/test-libmongoc.h

@@ -33,12 +33,17 @@ bool test_framework_get_ssl (void);
 char *test_framework_add_user_password (const char *uri_str,
                                         const char *user,
                                         const char *password);
+char *test_framework_add_user_password_from_env (const char *uri_str);
 char *test_framework_get_uri_str_no_auth (const char *database_name);
 char *test_framework_get_uri_str (void);
 char *test_framework_get_unix_domain_socket_uri_str (void);
 char *test_framework_get_unix_domain_socket_path (void);
 mongoc_uri_t *test_framework_get_uri (void);
+#ifdef MONGOC_ENABLE_SSL
+const mongoc_ssl_opt_t *test_framework_get_ssl_opts (void);
+#endif
 void test_framework_set_ssl_opts (mongoc_client_t *client);
+void test_framework_set_pool_ssl_opts (mongoc_client_pool_t *pool);
 mongoc_client_t *test_framework_client_new (void);
 mongoc_client_pool_t *test_framework_client_pool_new (void);
 bool test_framework_max_wire_version_at_least (int version);
@@ -52,8 +57,9 @@ int test_framework_skip_if_single  (void);
 int test_framework_skip_if_windows (void);
 
 typedef struct _debug_stream_stats_t {
-    int n_destroyed;
-    int n_failed;
+   mongoc_client_t *client;
+   int n_destroyed;
+   int n_failed;
 } debug_stream_stats_t;
 
 void test_framework_set_debug_stream (mongoc_client_t *client,

tests/test-mongoc-client.c

@@ -880,6 +880,97 @@ test_mongoc_client_unix_domain_socket (void *context)
 }
 
 
+#ifdef MONGOC_ENABLE_SSL
+static void
+_test_mongoc_client_ssl_opts (bool pooled)
+{
+   char *host;
+   uint16_t port;
+   char *uri_str;
+   char *uri_str_auth;
+   char *uri_str_auth_ssl;
+   mongoc_uri_t *uri;
+   const mongoc_ssl_opt_t *ssl_opts;
+   mongoc_client_pool_t *pool = NULL;
+   mongoc_client_t *client;
+   bool ret;
+   bson_error_t error;
+   int add_ssl_to_uri;
+
+   host = test_framework_get_host ();
+   port = test_framework_get_port ();
+   uri_str = bson_strdup_printf (
+      "mongodb://%s:%d/?serverSelectionTimeoutMS=1000",
+      host, port);
+
+   uri_str_auth = test_framework_add_user_password_from_env (uri_str);
+   uri_str_auth_ssl = bson_strdup_printf ("%s&ssl=true", uri_str_auth);
+
+   ssl_opts = test_framework_get_ssl_opts ();
+
+   /* client uses SSL once SSL options are set, regardless of "ssl=true" */
+   for (add_ssl_to_uri = 0; add_ssl_to_uri < 2; add_ssl_to_uri++) {
+
+      if (add_ssl_to_uri) {
+         uri = mongoc_uri_new (uri_str_auth_ssl);
+      } else {
+         uri = mongoc_uri_new (uri_str_auth);
+      }
+
+      if (pooled) {
+         pool = mongoc_client_pool_new (uri);
+         mongoc_client_pool_set_ssl_opts (pool, ssl_opts);
+         client = mongoc_client_pool_pop (pool);
+      } else {
+         client = mongoc_client_new_from_uri (uri);
+         mongoc_client_set_ssl_opts (client, ssl_opts);
+      }
+
+      /* any operation */
+      ret = mongoc_client_command_simple (client, "admin",
+                                          tmp_bson ("{'ping': 1}"), NULL,
+                                          NULL, &error);
+
+      if (test_framework_get_ssl ()) {
+         ASSERT_OR_PRINT (ret, error);
+      } else {
+         /* TODO: CDRIVER-936 check the err msg has "SSL handshake failed" */
+         ASSERT (!ret);
+         ASSERT_CMPINT (MONGOC_ERROR_SERVER_SELECTION, ==, error.domain);
+      }
+
+      if (pooled) {
+         mongoc_client_pool_push (pool, client);
+         mongoc_client_pool_destroy (pool);
+      } else {
+         mongoc_client_destroy (client);
+      }
+
+      mongoc_uri_destroy (uri);
+   }
+
+   bson_free (uri_str_auth_ssl);
+   bson_free (uri_str_auth);
+   bson_free (uri_str);
+   bson_free (host);
+};
+
+
+static void
+test_ssl_single (void)
+{
+   _test_mongoc_client_ssl_opts (false);
+}
+
+
+static void
+test_ssl_pooled (void)
+{
+   _test_mongoc_client_ssl_opts (true);
+}
+#endif
+
+
 void
 test_client_install (TestSuite *suite)
 {
@@ -917,5 +1008,9 @@ test_client_install (TestSuite *suite)
 #ifdef TODO_CDRIVER_689
    TestSuite_Add (suite, "/Client/wire_version", test_wire_version);
 #endif
-}
 
+#ifdef MONGOC_ENABLE_SSL
+   TestSuite_Add (suite, "/Client/ssl_opts/single", test_ssl_single);
+   TestSuite_Add (suite, "/Client/ssl_opts/pooled", test_ssl_pooled);
+#endif
+}