CDRIVER-3668 support OCSP back to OpenSSL 1.0.1 (#623)

- change SSL_CTX_set_tlsext_status_type to SSL_set_tlsext_status_type. - polyfill SSL_get0_verified_chain, NID_tlsfeature, and hostname check. - check for status_request from the tlsfeature extension when inspecting peer certificate. - skip time check for older OpenSSL when updating cache entries. - perform the OCSP check after the handshake, since sometimes the peer certificate is not available in the callback in OpenSSL <= 1.0.2. - check tlsDisableOCSPEndpointCheck before reaching out to a responder. - make tlsDisableOCSPEndpointCheck and tlsDisableCertificateRevocationCheck URI options implicitly enable TLS. - enable OCSP tests on OpenSSL and macOS that were skipped. - add OCSP tests for OpenSSL 1.0.1. - update OCSP OpenSSL documentation. - change OCSP verification logs from MONGOC_DEBUG to TRACE in successful cases.
mongodb · Jun 5, 2020 · 733322e · 733322e
1 parent 8cb279d
commit 733322e
Show file tree

Hide file tree

Showing 15 changed files with 2,141 additions and 233 deletions.
diff --git a/.evergreen/config.yml b/.evergreen/config.yml
diff --git a/build/evergreen_config_lib/tasks.py b/build/evergreen_config_lib/tasks.py
@@ -31,14 +31,15 @@
 class CompileTask(NamedTask):
     def __init__(self, task_name, tags=None, config='debug',
                  compression='default', continue_on_err=False,
-                 extra_commands=None, depends_on=None,
-                 extra_script=None, **kwargs):
+                 suffix_commands=None, depends_on=None,
+                 extra_script=None, prefix_commands=None, **kwargs):
         super(CompileTask, self).__init__(task_name=task_name,
                                           depends_on=depends_on,
                                           tags=tags,
                                           **kwargs)
 
-        self.extra_commands = extra_commands or []
+        self.suffix_commands = suffix_commands or []
+        self.prefix_commands = prefix_commands or []
         if extra_script:
             self.extra_script = "\n" + extra_script
         else:
@@ -65,6 +66,8 @@ def __init__(self, task_name, tags=None, config='debug',
     def to_dict(self):
         task = super(CompileTask, self).to_dict()
 
+        task['commands'].extend(self.prefix_commands)
+
         script = ''
         for opt, value in sorted(self.compile_sh_opt.items()):
             script += 'export %s="%s"\n' % (opt, value)
@@ -73,7 +76,7 @@ def to_dict(self):
             self.extra_script
         task['commands'].append(shell_mongoc(script))
         task['commands'].append(func('upload build'))
-        task['commands'].extend(self.extra_commands)
+        task['commands'].extend(self.suffix_commands)
         return task
 
 
@@ -135,7 +138,7 @@ def __init__(self, *args, **kwargs):
 
 
 class LinkTask(NamedTask):
-    def __init__(self, task_name, extra_commands, orchestration=True, **kwargs):
+    def __init__(self, task_name, suffix_commands, orchestration=True, **kwargs):
         if orchestration == 'ssl':
             bootstrap_commands = [bootstrap(SSL=1)]
         elif orchestration:
@@ -147,7 +150,7 @@ def __init__(self, task_name, extra_commands, orchestration=True, **kwargs):
             task_name=task_name,
             depends_on=OD([('name', 'make-release-archive'),
                            ('variant', 'releng')]),
-            commands=bootstrap_commands + extra_commands,
+            commands=bootstrap_commands + suffix_commands,
             **kwargs)
 
 
@@ -201,7 +204,7 @@ def __init__(self, task_name, extra_commands, orchestration=True, **kwargs):
     SpecialTask('debug-compile-coverage',
                 tags=['debug-compile', 'coverage'],
                 COVERAGE='ON',
-                extra_commands=[func('upload coverage')]),
+                suffix_commands=[func('upload coverage')]),
     CompileTask('debug-compile-no-counters',
                 tags=['debug-compile', 'no-counters'],
                 ENABLE_SHM_COUNTERS='OFF'),
@@ -244,7 +247,7 @@ def __init__(self, task_name, extra_commands, orchestration=True, **kwargs):
                 continue_on_err=True,
                 ANALYZE='ON',
                 CC='clang',
-                extra_commands=[
+                suffix_commands=[
                     func('upload scan artifacts'),
                     shell_mongoc('''
                         if find scan -name \*.html | grep -q html; then
@@ -300,70 +303,70 @@ def __init__(self, task_name, extra_commands, orchestration=True, **kwargs):
                 tags=['debug-compile'],
                 SRV='OFF'),
     LinkTask('link-with-cmake',
-             extra_commands=[
+             suffix_commands=[
                  func('link sample program', BUILD_SAMPLE_WITH_CMAKE=1)]),
     LinkTask('link-with-cmake-ssl',
-             extra_commands=[
+             suffix_commands=[
                  func('link sample program',
                       BUILD_SAMPLE_WITH_CMAKE=1,
                       ENABLE_SSL=1)]),
     LinkTask('link-with-cmake-snappy',
-             extra_commands=[
+             suffix_commands=[
                  func('link sample program',
                       BUILD_SAMPLE_WITH_CMAKE=1,
                       ENABLE_SNAPPY=1)]),
     LinkTask('link-with-cmake-mac',
-             extra_commands=[
+             suffix_commands=[
                  func('link sample program', BUILD_SAMPLE_WITH_CMAKE=1)]),
     LinkTask('link-with-cmake-deprecated',
-             extra_commands=[
+             suffix_commands=[
                  func('link sample program',
                       BUILD_SAMPLE_WITH_CMAKE=1,
                       BUILD_SAMPLE_WITH_CMAKE_DEPRECATED=1)]),
     LinkTask('link-with-cmake-ssl-deprecated',
-             extra_commands=[
+             suffix_commands=[
                  func('link sample program',
                       BUILD_SAMPLE_WITH_CMAKE=1,
                       BUILD_SAMPLE_WITH_CMAKE_DEPRECATED=1,
                       ENABLE_SSL=1)]),
     LinkTask('link-with-cmake-snappy-deprecated',
-             extra_commands=[
+             suffix_commands=[
                  func('link sample program',
                       BUILD_SAMPLE_WITH_CMAKE=1,
                       BUILD_SAMPLE_WITH_CMAKE_DEPRECATED=1,
                       ENABLE_SNAPPY=1)]),
     LinkTask('link-with-cmake-mac-deprecated',
-             extra_commands=[
+             suffix_commands=[
                  func('link sample program',
                       BUILD_SAMPLE_WITH_CMAKE=1,
                       BUILD_SAMPLE_WITH_CMAKE_DEPRECATED=1)]),
     LinkTask('link-with-cmake-windows',
-             extra_commands=[func('link sample program MSVC')]),
+             suffix_commands=[func('link sample program MSVC')]),
     LinkTask('link-with-cmake-windows-ssl',
-             extra_commands=[func('link sample program MSVC', ENABLE_SSL=1)],
+             suffix_commands=[func('link sample program MSVC', ENABLE_SSL=1)],
              orchestration='ssl'),
     LinkTask('link-with-cmake-windows-snappy',
-             extra_commands=[
+             suffix_commands=[
                  func('link sample program MSVC', ENABLE_SNAPPY=1)]),
     LinkTask('link-with-cmake-mingw',
-             extra_commands=[func('link sample program mingw')]),
+             suffix_commands=[func('link sample program mingw')]),
     LinkTask('link-with-pkg-config',
-             extra_commands=[func('link sample program')]),
+             suffix_commands=[func('link sample program')]),
     LinkTask('link-with-pkg-config-mac',
-             extra_commands=[func('link sample program')]),
+             suffix_commands=[func('link sample program')]),
     LinkTask('link-with-pkg-config-ssl',
-             extra_commands=[func('link sample program', ENABLE_SSL=1)]),
+             suffix_commands=[func('link sample program', ENABLE_SSL=1)]),
     LinkTask('link-with-bson',
-             extra_commands=[func('link sample program bson')],
+             suffix_commands=[func('link sample program bson')],
              orchestration=False),
     LinkTask('link-with-bson-mac',
-             extra_commands=[func('link sample program bson')],
+             suffix_commands=[func('link sample program bson')],
              orchestration=False),
     LinkTask('link-with-bson-windows',
-             extra_commands=[func('link sample program MSVC bson')],
+             suffix_commands=[func('link sample program MSVC bson')],
              orchestration=False),
     LinkTask('link-with-bson-mingw',
-             extra_commands=[func('link sample program mingw bson')],
+             suffix_commands=[func('link sample program mingw bson')],
              orchestration=False),
     NamedTask('debian-package-build',
               commands=[
@@ -408,7 +411,10 @@ def __init__(self, task_name, extra_commands, orchestration=True, **kwargs):
     CompileWithClientSideEncryption('debug-compile-sasl-winssl-cse', tags=[
                                     'debug-compile', 'sasl', 'winssl'], SASL="AUTO", SSL="WINDOWS"),
     CompileWithClientSideEncryptionAsan('debug-compile-asan-openssl-cse', tags=[
-                                        'debug-compile', 'asan-clang'], SSL="OPENSSL")
+                                        'debug-compile', 'asan-clang'], SSL="OPENSSL"),
+    CompileTask ('debug-compile-nosasl-openssl-1.0.1',
+        prefix_commands=[func("install ssl", SSL="openssl-1.0.1u")],
+        CFLAGS="-Wno-redundant-decls", SSL="OPENSSL", SASL="OFF")
 ]
 
 
@@ -667,11 +673,11 @@ def _compressor_list(self):
 
 class SpecialIntegrationTask(NamedTask):
     def __init__(self, task_name, depends_on='debug-compile-sasl-openssl',
-                 extra_commands=None, uri=None,
+                 suffix_commands=None, uri=None,
                  tags=None, version='latest', topology='server'):
         commands = [func('fetch build', BUILD_NAME=depends_on),
                     bootstrap(VERSION=version, TOPOLOGY=topology),
-                    run_tests(uri)] + (extra_commands or [])
+                    run_tests(uri)] + (suffix_commands or [])
         super(SpecialIntegrationTask, self).__init__(task_name,
                                                      commands=commands,
                                                      depends_on=depends_on,
@@ -824,7 +830,7 @@ def name(self):
     SSLTask('openssl-1.0.2', 'l'),
     SSLTask('openssl-1.1.0', 'f'),
     SSLTask('libressl-2.5', '.2', require_tls12=True),
-    SSLTask('libressl-3.0', '.2', require_tls12=True, enable_ssl="AUTO"),
+    SSLTask('libressl-3.0', '.2', require_tls12=True, enable_ssl="AUTO", cflags="-Wno-redundant-decls"),
     SSLTask('libressl-3.0', '.2', require_tls12=True),
 ])
 
@@ -900,7 +906,7 @@ class OCSPTask(MatrixTask):
     axes = OD([('test', ['test_1', 'test_2', 'test_3', 'test_4', 'soft_fail_test', 'malicious_server_test_1', 'malicious_server_test_2', 'cache']),
                ('delegate', ['delegate', 'nodelegate']),
                ('cert', ['rsa', 'ecdsa']),
-               ('ssl', ['openssl', 'darwinssl', 'winssl'])])
+               ('ssl', ['openssl', 'openssl-1.0.1', 'darwinssl', 'winssl'])])
 
     name_prefix = 'test-ocsp'
 
@@ -942,11 +948,9 @@ def to_dict(self):
 
     # Testing in OCSP has a lot of exceptions.
     def _check_allowed(self):
-        # Current latest macOS does not support the disableStapling failpoint.
-        # There are no tests that can run on macOS in current evergreen configuration.
         if self.ssl == 'darwinssl':
-            # TODO: remove this when macOS latest download is updated
-            prohibit (True)
+            # Secure Transport quietly ignores a must-staple certificate with no stapled response.
+            prohibit (self.test == 'malicious_server_test_2')
 
         # ECDSA certs can't be loaded (in the PEM format they're stored) on Windows/macOS. Skip them.
         if self.ssl == 'darwinssl' or self.ssl == 'winssl':
@@ -959,9 +963,6 @@ def _check_allowed(self):
         if self.test == 'soft_fail_test' or self.test == 'malicious_server_test_2' or self.test == 'cache':
             prohibit(self.delegate == 'delegate')
 
-        # Until OCSP is supported in OpenSSL, skip tests that expect to be revoked.
-        if self.ssl == 'openssl':
-            prohibit (self.test in ['test_2', 'test_4', 'malicious_server_test_1', 'malicious_server_test_2'])
 
 
 all_tasks = chain(all_tasks, OCSPTask.matrix())

diff --git a/build/evergreen_config_lib/variants.py b/build/evergreen_config_lib/variants.py
@@ -573,11 +573,13 @@ def days(n):
         batchtime=days(1)),
     Variant ('ocsp', 'OCSP tests', 'ubuntu1804-test', [
         OD([('name', 'debug-compile-nosasl-openssl'), ('distros', ['ubuntu1804-test'])]),
-        #OD([('name', 'debug-compile-nosasl-darwinssl'), ('distros', ['macos-1014'])]),
+        OD([('name', 'debug-compile-nosasl-darwinssl'), ('distros', ['macos-1014'])]),
         OD([('name', 'debug-compile-nosasl-winssl'), ('distros', ['windows-64-vs2017-test'])]),
         OD([('name', '.ocsp-openssl'), ('distros', ['ubuntu1804-test'])]),
-        #OD([('name', '.ocsp-darwinssl'), ('distros', ['macos-1014'])]),
-        OD([('name', '.ocsp-winssl'), ('distros', ['windows-64-vs2017-test'])])
+        OD([('name', '.ocsp-darwinssl'), ('distros', ['macos-1014'])]),
+        OD([('name', '.ocsp-winssl'), ('distros', ['windows-64-vs2017-test'])]),
+        OD([('name', 'debug-compile-nosasl-openssl-1.0.1'), ('distros', ['ubuntu1804-test'])]),
+        OD([('name', '.ocsp-openssl-1.0.1'), ('distros', ['ubuntu1804-test'])])
     ], {}, batchtime=days(14)),
     Variant ('packaging', 'Linux Distro Packaging', 'ubuntu1604-test', [
         'debian-package-build',

diff --git a/src/libmongoc/CMakeLists.txt b/src/libmongoc/CMakeLists.txt
@@ -629,10 +629,8 @@ if (NOT ENABLE_SSL STREQUAL OFF)
          ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-stream-tls-openssl.c
          ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-stream-tls-openssl-bio.c
          ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-openssl.c
+         ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-ocsp-cache.c
       )
-      if (OPENSSL_VERSION GREATER 1.1.1 OR OPENSSL_VERSION EQUAL 1.1.1)
-         set (SOURCES ${SOURCES} ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-ocsp-cache.c)
-      endif()
       set (SSL_LIBRARIES ${OPENSSL_LIBRARIES})
       include_directories (${OPENSSL_INCLUDE_DIR})
       if (WIN32)
@@ -950,10 +948,8 @@ if (MONGOC_ENABLE_SSL)
       ${PROJECT_SOURCE_DIR}/tests/test-mongoc-stream-tls-error.c
       ${PROJECT_SOURCE_DIR}/tests/test-mongoc-stream-tls.c
       ${PROJECT_SOURCE_DIR}/tests/test-mongoc-x509.c
+      ${PROJECT_SOURCE_DIR}/tests/test-mongoc-ocsp-cache.c
    )
-   if (OPENSSL_VERSION GREATER 1.1.1 OR OPENSSL_VERSION EQUAL 1.1.1)
-      set (test-libmongoc-sources ${test-libmongoc-sources} ${PROJECT_SOURCE_DIR}/tests/test-mongoc-ocsp-cache.c)
-   endif()
 endif ()
 
 if (MONGOC_ENABLE_SASL_CYRUS)

diff --git a/src/libmongoc/doc/configuring_tls.rst b/src/libmongoc/doc/configuring_tls.rst
@@ -88,9 +88,10 @@ Ensure your system's OpenSSL is a recent version (at least 1.0.1), or install a
 
 When compiled against OpenSSL, the driver will attempt to load the system default certificate store, as configured by the distribution. That can be overridden by setting the ``tlsCAFile`` URI option or with the fields ``ca_file`` and ``ca_dir`` in the :symbol:`mongoc_ssl_opt_t`.
 
-Setting ``tlsDisableOCSPEndpointCheck`` and ``tlsDisableCertificateRevocationCheck`` has no effect.
+Setting ``tlsDisableCertificateRevocationCheck`` disables OCSP revocation checking.
+Setting ``tlsDisableOCSPEndpointCheck`` disables OCSP responders from being contacted when OCSP revocation checking is enabled, and a server presents a certificate without stapled OCSP response.
 
-The Online Certificate Status Protocol (OCSP) is partially supported (see `RFC 6960 <https://tools.ietf.org/html/rfc6960>`_). Support requires OpenSSL 1.1.0 and has the following behavior:
+The Online Certificate Status Protocol (OCSP) is partially supported (see `RFC 6960 <https://tools.ietf.org/html/rfc6960>`_). Support requires OpenSSL 1.0.1 and has the following behavior:
 
 - Stapled OCSP responses are validated on certificates presented by the server.
 - Server certificates with a Must-Staple extension (see `RFC 7633 <https://tools.ietf.org/html/rfc7633>`_) are required to have stapled responses.

diff --git a/src/libmongoc/src/mongoc/mongoc-ocsp-cache.c b/src/libmongoc/src/mongoc/mongoc-ocsp-cache.c
@@ -80,6 +80,26 @@ update_entry (cache_entry_list_t *entry,
    entry->cert_status = cert_status;
    entry->reason = reason;
 }
+
+#if OPENSSL_VERSION_NUMBER >= 0x10101000L
+static int
+_cmp_time (ASN1_TIME *a, ASN1_TIME *b)
+{
+   return ASN1_TIME_compare (a, b);
+}
+#else
+static int
+_cmp_time (ASN1_TIME *a, ASN1_TIME *b)
+{
+   /* For older OpenSSL, always report that "a" is before "b". I.e. do not
+    * replace the entry.
+    * If a driver would accept a stapled OCSP response and that response has a
+    * later nextUpdate than the response already in the cache, drivers SHOULD
+    * replace the older entry in the cache with the fresher response. */
+   return -1;
+}
+#endif
+
 void
 _mongoc_ocsp_cache_set_resp (OCSP_CERTID *id,
                              int cert_status,
@@ -96,8 +116,7 @@ _mongoc_ocsp_cache_set_resp (OCSP_CERTID *id,
       entry->id = OCSP_CERTID_dup (id);
       LL_APPEND (cache, entry);
       update_entry (entry, cert_status, reason, this_update, next_update);
-   } else if (next_update &&
-              ASN1_TIME_compare (next_update, entry->next_update) == 1) {
+   } else if (next_update && _cmp_time (next_update, entry->next_update) == 1) {
       update_entry (entry, cert_status, reason, this_update, next_update);
    } else {
       /* Do nothing; our next_update is at a later date */

diff --git a/src/libmongoc/src/mongoc/mongoc-openssl-private.h b/src/libmongoc/src/mongoc/mongoc-openssl-private.h
@@ -25,8 +25,9 @@
 #include <openssl/err.h>
 
 #include "mongoc-ssl.h"
+#include "mongoc-stream-tls-openssl-private.h"
 
-#if (OPENSSL_VERSION_NUMBER >= 0x10101000L) && !defined(OPENSSL_NO_OCSP) && \
+#if (OPENSSL_VERSION_NUMBER >= 0x10001000L) && !defined(OPENSSL_NO_OCSP) && \
    !defined(LIBRESSL_VERSION_NUMBER)
 #define MONGOC_ENABLE_OCSP_OPENSSL
 #endif
@@ -35,9 +36,9 @@
 BSON_BEGIN_DECLS
 
 bool
-_mongoc_openssl_check_cert (SSL *ssl,
-                            const char *host,
-                            bool allow_invalid_hostname);
+_mongoc_openssl_check_peer_hostname (SSL *ssl,
+                                     const char *host,
+                                     bool allow_invalid_hostname);
 SSL_CTX *
 _mongoc_openssl_ctx_new (mongoc_ssl_opt_t *opt);
 char *
@@ -49,9 +50,11 @@ _mongoc_openssl_cleanup (void);
 
 #ifdef MONGOC_ENABLE_OCSP_OPENSSL
 int
-_mongoc_ocsp_tlsext_status_cb (SSL *ssl, void *arg);
+_mongoc_ocsp_tlsext_status (SSL *ssl, mongoc_openssl_ocsp_opt_t *opts);
 #endif
 
+bool
+_mongoc_tlsfeature_has_status_request (const uint8_t *data, int length);
 
 BSON_END_DECLS