CDRIVER-2257 security rules for mongodb+srv://

Hostnames returned from an SRV lookup must share the service name's
parent domain, and SSL is turned on with mongodb+srv URIs by default.
At most one TXT record is allowed, only "replicaSet" and "authSource"
may be set from a TXT record.

Author: ajdavis

.evergreen/config.yml

@@ -296,7 +296,7 @@ functions:
         working_dir: "mongoc"
         script: |
           set -o errexit
-          COMPRESSORS="${COMPRESSORS}" CC="${CC}" AUTH=${AUTH} SSL=${SSL} URI=${URI} IPV4_ONLY=${IPV4_ONLY} VALGRIND=${VALGRIND} MONGOC_TEST_URI=${URI} sh .evergreen/run-tests.sh
+          COMPRESSORS="${COMPRESSORS}" CC="${CC}" AUTH=${AUTH} SSL=${SSL} URI=${URI} IPV4_ONLY=${IPV4_ONLY} VALGRIND=${VALGRIND} MONGOC_TEST_URI=${URI} DNS=${DNS} sh .evergreen/run-tests.sh
 
   "run auth tests":
     - command: shell.exec
@@ -912,21 +912,56 @@ tasks:
           vars:
             ENABLE_SSL: "1"
 
-    - name: test-dns
+    - name: test-dns-openssl
       depends_on:
-        - name: "debug-compile-nosasl-nossl"
+        - name: "debug-compile-sasl-openssl"
       commands:
         - func: "fetch build"
           vars:
-            BUILD_NAME: "debug-compile-nosasl-nossl"
-        # The Initial DNS Seedlist Discovery Spec's tests use 3-node replica set
+            BUILD_NAME: "debug-compile-sasl-openssl"
         - func: "bootstrap mongo-orchestration"
           vars:
-            VERSION: "3.4"
+            VERSION: "latest"
+            TOPOLOGY: "replica_set"
+            SSL: "ssl"
+        - func: "run tests"
+          vars:
+            DNS: on
+            SSL: "ssl"
+
+    - name: test-dns-winssl
+      depends_on:
+        - name: "debug-compile-sspi-winssl"
+      commands:
+        - func: "fetch build"
+          vars:
+            BUILD_NAME: "debug-compile-sspi-winssl"
+        - func: "bootstrap mongo-orchestration"
+          vars:
+            VERSION: "latest"
             TOPOLOGY: "replica_set"
+            SSL: "winssl"
         - func: "run tests"
           vars:
             DNS: on
+            SSL: "winssl"
+
+    - name: test-dns-darwinssl
+      depends_on:
+        - name: "debug-compile-sasl-darwinssl"
+      commands:
+        - func: "fetch build"
+          vars:
+            BUILD_NAME: "debug-compile-sasl-darwinssl"
+        - func: "bootstrap mongo-orchestration"
+          vars:
+            VERSION: "latest"
+            TOPOLOGY: "replica_set"
+            SSL: "darwinssl"
+        - func: "run tests"
+          vars:
+            DNS: on
+            SSL: "darwinssl"
 
     - name: debug-compile-nosasl-nossl
       tags: ["debug-compile", "nosasl", "nossl"]
@@ -7271,12 +7306,6 @@ buildvariants:
     - name: "link-with-pkg-config-mac"
       distros:
       - macos-1012
-    - name: "test-dns"
-      distros:
-      - ubuntu1604-build
-      - ubuntu1604-test
-      - ubuntu1404-build
-      - ubuntu1404-test
 
 
 - name: clang34ubuntu
@@ -7532,6 +7561,7 @@ buildvariants:
     - "retry-true-latest-server"
     # Ubuntu16.04 only supports MongoDB 3.4+
     - ".3.6 .openssl !.nosasl .server"
+    - "test-dns-openssl"
 
 - name: darwin
   display_name: "*Darwin, macOS (Apple LLVM)"
@@ -7556,7 +7586,7 @@ buildvariants:
     - ".3.2 .darwinssl !.nosasl .server"
     - ".3.2 .nossl !.nosasl"
     - ".2.6 .nossl !.nosasl" # No MongoDB SSL builds available
-    - "test-dns"
+    - "test-dns-darwinssl"
 
 - name: windows-2015
   display_name: "Windows (VS 2015)"
@@ -7584,7 +7614,7 @@ buildvariants:
     - ".3.2 .winssl !.nosasl .server"
     - ".3.0 .nossl !.nosasl"
     - ".2.6 .nossl !.nosasl"
-    - "test-dns"
+    - "test-dns-winssl"
 
 - name: windows-2015-32
   display_name: "Windows (i386) (VS 2015)"
@@ -7696,7 +7726,6 @@ buildvariants:
     - ".latest .nossl .nosasl"
     - ".latest .sspi"
     - ".3.6 .winssl .nosasl .server"
-    - "test-dns"
 
 - name: mingw
   display_name: "MinGW-W64"
@@ -7752,7 +7781,7 @@ buildvariants:
     - ".latest .nossl !.nosasl"
     - ".3.6 .openssl !.nosasl .server"
       # Ubuntu16.04 ppc64le is only supported by MongoDB 3.4+
-    - "test-dns"
+    - "test-dns-openssl"
 
 - name: arm-ubuntu1604
   display_name: "*ARM (aarch64) (Ubuntu 16.04)"
@@ -7776,7 +7805,7 @@ buildvariants:
     - ".latest .nossl !.nosasl"
     - ".3.6 .openssl !.nosasl .server"
       # Ubuntu16.04 aarch64 is only supported by MongoDB 3.4+
-    - "test-dns"
+    - "test-dns-openssl"
 
 - name: zseries-rhel72
   display_name: "*zSeries (s390x) (RHEL 7.2)"
@@ -7818,7 +7847,7 @@ buildvariants:
     - ".latest .nossl !.nosasl"
     - ".3.6 .openssl !.nosasl .server"
       # Ubuntu16.04 s390x is only supported by MongoDB 3.4+
-    - "test-dns"
+    - "test-dns-openssl"
 
 - name: zseries-suse12
   display_name: "zSeries (s390x) SUSE12"

.evergreen/run-tests.sh

@@ -46,7 +46,7 @@ fi
 
 if [ "$DNS" != "nodns" ]; then
    export MONGOC_TEST_DNS=on
-   TEST_ARGS="$TEST_ARGS -l '/initial_dns_seedlist_discovery*'"
+   TEST_ARGS="$TEST_ARGS -l /initial_dns_seedlist_discovery*"
 fi
 
 if [ "$CC" = "mingw" ]; then

doc/mongoc_uri_t.rst

@@ -70,11 +70,11 @@ You would use a connection string that resembles the following.
 SRV Example
 -----------
 
-If you have configured an `SRV record <https://www.ietf.org/rfc/rfc2782.txt>`_ with a name like "_mongodb._tcp.example.com" whose records are a list of one or more MongoDB server hostnames, use a connection string like this:
+If you have configured an `SRV record <https://www.ietf.org/rfc/rfc2782.txt>`_ with a name like "_mongodb._tcp.server.example.com" whose records are a list of one or more MongoDB server hostnames, use a connection string like this:
 
 .. code-block:: c
 
-  uri = mongoc_uri_new ("mongodb+srv://example.com/?replicaSet=rs&appName=applicationName");
+  uri = mongoc_uri_new ("mongodb+srv://server.example.com/?replicaSet=rs&appName=applicationName");
 
 The driver prefixes the service name with "_mongodb._tcp.", then performs a DNS SRV query to resolve the service name to one or more hostnames. If this query succeeds, the driver performs a DNS TXT query on the service name (without the "_mongodb._tcp" prefix) for additional URI options configured as TXT records.
 

src/mongoc/mongoc-client.c

@@ -109,10 +109,8 @@ srv_callback (const char *service,
               mongoc_uri_t *uri,
               bson_error_t *error)
 {
-   mongoc_uri_append_host (
-      uri, pdns->Data.SRV.pNameTarget, pdns->Data.SRV.wPort);
-
-   return true;
+   return mongoc_uri_append_host (
+      uri, pdns->Data.SRV.pNameTarget, pdns->Data.SRV.wPort, error);
 }
 
 static bool
@@ -122,20 +120,19 @@ txt_callback (const char *service,
               bson_error_t *error)
 {
    DWORD i;
+   bson_string_t *txt;
+   bool r;
+
+   txt = bson_string_new (NULL);
 
    for (i = 0; i < pdns->Data.TXT.dwStringCount; i++) {
-      /* Initial DNS Seedlist Discovery Spec: "Client MUST use options specified
-       * in the Connection String to override options provided through TXT
-       * records." So, do NOT override existing options with TXT options. */
-      if (!mongoc_uri_parse_options (uri,
-                                     pdns->Data.TXT.pStringArray[i],
-                                     false /* override */,
-                                     error)) {
-         return false;
-      }
+      bson_string_append (txt, pdns->Data.TXT.pStringArray[i]);
    }
 
-   return true;
+   r = mongoc_uri_parse_options (uri, txt->str, true /* from_dns */, error);
+   bson_string_free (txt, true);
+
+   return r;
 }
 
 /*
@@ -149,6 +146,12 @@ txt_callback (const char *service,
  * Returns:
  *       Success or failure.
  *
+ *       For an SRV lookup, returns false if there is any error.
+ *
+ *       For TXT lookup, ignores any error fetching the resource record, but
+ *       returns false if the resource record is found and there is an error
+ *       reading its contents as URI options.
+ *
  * Side effects:
  *       @error is set if there is a failure.
  *
@@ -167,15 +170,21 @@ _mongoc_get_rr_dnsapi (const char *service,
    PDNS_RECORD pdns = NULL;
    DNS_STATUS res;
    LPVOID lpMsgBuf = NULL;
-   bool ret = false;
+   bool dns_success;
+   bool callback_success = true;
+   int i;
 
    ENTRY;
 
    if (rr_type == MONGOC_RR_SRV) {
+      /* return true only if DNS succeeds */
+      dns_success = false;
       rr_type_name = "SRV";
       nst = DNS_TYPE_SRV;
       callback = srv_callback;
    } else {
+      /* return true whether or not DNS succeeds */
+      dns_success = true;
       rr_type_name = "TXT";
       nst = DNS_TYPE_TEXT;
       callback = txt_callback;
@@ -214,14 +223,25 @@ _mongoc_get_rr_dnsapi (const char *service,
       DNS_ERROR ("No %s records for \"%s\"", rr_type_name, service);
    }
 
+   dns_success = true;
+   i = 0;
+
    do {
+      if (i > 0 && rr_type == MONGOC_RR_TXT) {
+         /* Initial DNS Seedlist Discovery Spec: a client "MUST raise an error
+          * when multiple TXT records are encountered". */
+         callback_success = false;
+         DNS_ERROR ("Multiple TXT records for \"%s\"", service);
+      }
+
       if (!callback (service, pdns, uri, error)) {
+         callback_success = false;
          GOTO (done);
       }
       pdns = pdns->pNext;
+      i++;
    } while (pdns);
 
-   ret = true;
 done:
    if (pdns) {
       DnsRecordListFree (pdns, DnsFreeRecordList);
@@ -231,7 +251,7 @@ _mongoc_get_rr_dnsapi (const char *service,
       LocalFree (lpMsgBuf);
    }
 
-   RETURN (ret);
+   RETURN (dns_success && callback_success);
 }
 
 #elif (defined(MONGOC_HAVE_RES_NSEARCH) || defined(MONGOC_HAVE_RES_SEARCH))
@@ -269,8 +289,7 @@ srv_callback (const char *service,
                  strerror (h_errno));
    }
 
-   mongoc_uri_append_host (uri, name, port);
-   ret = true;
+   ret = mongoc_uri_append_host (uri, name, port, error);
 
 done:
    return ret;
@@ -309,10 +328,7 @@ txt_callback (const char *service,
       pos += len;
    }
 
-   /* Initial DNS Seedlist Discovery Spec: "Client MUST use options specified in
-    * the Connection String to override options provided through TXT records."
-    * So, do NOT override existing options with TXT options. */
-   r = mongoc_uri_parse_options (uri, txt->str, false /* override */, error);
+   r = mongoc_uri_parse_options (uri, txt->str, true /* from_dns */, error);
    bson_string_free (txt, true);
 
 done:
@@ -329,6 +345,12 @@ txt_callback (const char *service,
  * Returns:
  *       Success or failure.
  *
+ *       For an SRV lookup, returns false if there is any error.
+ *
+ *       For TXT lookup, ignores any error fetching the resource record, but
+ *       returns false if the resource record is found and there is an error
+ *       reading its contents as URI options.
+ *
  * Side effects:
  *       @error is set if there is a failure.
  *
@@ -353,15 +375,20 @@ _mongoc_get_rr_search (const char *service,
    ns_type nst;
    mongoc_rr_callback_t callback;
    ns_rr resource_record;
-   bool ret = false;
+   bool dns_success;
+   bool callback_success = true;
 
    ENTRY;
 
    if (rr_type == MONGOC_RR_SRV) {
+      /* return true only if DNS succeeds */
+      dns_success = false;
       rr_type_name = "SRV";
       nst = ns_t_srv;
       callback = srv_callback;
    } else {
+      /* return true whether or not DNS succeeds */
+      dns_success = true;
       rr_type_name = "TXT";
       nst = ns_t_txt;
       callback = txt_callback;
@@ -393,6 +420,13 @@ _mongoc_get_rr_search (const char *service,
    }
 
    for (i = 0; i < n; i++) {
+      if (i > 0 && rr_type == MONGOC_RR_TXT) {
+         /* Initial DNS Seedlist Discovery Spec: a client "MUST raise an error
+          * when multiple TXT records are encountered". */
+         callback_success = false;
+         DNS_ERROR ("Multiple TXT records for \"%s\"", service);
+      }
+
       if (ns_parserr (&ns_answer, ns_s_an, i, &resource_record)) {
          DNS_ERROR ("Invalid record %d of %s answer for \"%s\": \"%s\"",
                     i,
@@ -402,11 +436,12 @@ _mongoc_get_rr_search (const char *service,
       }
 
       if (!callback (service, &ns_answer, &resource_record, uri, error)) {
+         callback_success = false;
          GOTO (done);
       }
    }
 
-   ret = true;
+   dns_success = true;
 
 done:
 
@@ -417,7 +452,7 @@ _mongoc_get_rr_search (const char *service,
    /* defined on Linux, and only if MONGOC_HAVE_RES_NSEARCH is defined */
    res_nclose (&state);
 #endif
-   RETURN (ret);
+   RETURN (dns_success && callback_success);
 }
 #endif
 
@@ -930,7 +965,11 @@ mongoc_client_new_from_uri (const mongoc_uri_t *uri)
 
    topology = mongoc_topology_new (uri, true);
 
-   return _mongoc_client_new_from_uri (uri, topology);
+   /* topology->uri may be different from uri: if this is a mongodb+srv:// URI
+    * then mongoc_topology_new has fetched SRV and TXT records and updated its
+    * uri from them.
+    */
+   return _mongoc_client_new_from_uri (topology->uri, topology);
 }
 
 /*