[FIXED] make natsOptions_SetSSLVerificationCallback experimental (nats-io#860)

* [FIXED] wrapped natsOptions_SetSSLVerificationCallback in ifdef NATS_WITH_EXPERIMENTAL

* PR feedback

Author: levb

.github/workflows/build-test.yml

@@ -4,6 +4,9 @@ on:
       arch:
         type: string
         default: "64"
+      benchmark:
+        type: string
+        default: "OFF"
       compiler:
         type: string
         default: "gcc"
@@ -13,18 +16,12 @@ on:
       dev_mode:
         type: string
         default: "OFF"
+      experimental:
+        type: string
+        default: "OFF"
       pool_dispatch:
         type: string
         default: "NO-pool"
-      write_deadline:
-        type: string
-        default: "NO-write_deadline"
-      tls:
-        type: string
-        default: "TLS"
-      verify_host:
-        type: string
-        default: "verify_host"
       repeat:
         type: string
         default: "1"
@@ -38,6 +35,9 @@ on:
       streaming:
         type: string
         default: "ON"
+      tls:
+        type: string
+        default: "TLS"
       type:
         type: string
         description: "Debug or Release."
@@ -46,15 +46,18 @@ on:
         type: string
         description: "Ubuntu version to use, e.g. '20.04'"
         default: "latest"
-      verbose_test_output:
-        type: string
-        default: "OFF"
       verbose_make_output:
         type: string
         default: "OFF"
-      benchmark:
+      verbose_test_output:
         type: string
         default: "OFF"
+      verify_host:
+        type: string
+        default: "verify_host"
+      write_deadline:
+        type: string
+        default: "NO-write_deadline"
 
     secrets:
       CODECOV_TOKEN:
@@ -88,8 +91,8 @@ jobs:
           flags: -DNATS_BUILD_ARCH=${{ inputs.arch }}
             -DCMAKE_BUILD_TYPE=${{ inputs.type }}
             -DNATS_BUILD_STREAMING=${{ inputs.streaming }}
-            -DNATS_PROTOBUF_DIR=${{ github.workspace}}/deps/pbuf
             -DNATS_BUILD_USE_SODIUM=ON
+            -DNATS_PROTOBUF_DIR=${{ github.workspace}}/deps/pbuf
             -DNATS_SODIUM_DIR=${{ github.workspace}}/deps/sodium
         run: |
           if [[ "${{ inputs.tls }}" == "TLS" ]]; then
@@ -102,15 +105,18 @@ jobs:
           else
             flags="$flags -DNATS_BUILD_WITH_TLS=OFF"
           fi
-          if [[ -n "${{ inputs.sanitize }}" ]]; then
-            flags="$flags -DNATS_SANITIZE=ON -DCMAKE_C_FLAGS=-fsanitize=${{ inputs.sanitize }}"
-          fi
           if [[ "${{ inputs.coverage }}" == "ON" ]]; then
             flags="$flags -DNATS_COVERAGE=ON"
           fi
           if [[ "${{ inputs.dev_mode }}" == "ON" ]]; then
             flags="$flags -DDEV_MODE=ON"
           fi
+          if [[ "${{ inputs.experimental }}" == "ON" ]]; then
+            flags="$flags -DNATS_EXPERIMENTAL=ON"
+          fi
+          if [[ -n "${{ inputs.sanitize }}" ]]; then
+            flags="$flags -DNATS_SANITIZE=ON -DCMAKE_C_FLAGS=-fsanitize=${{ inputs.sanitize }}"
+          fi
           if [[ "${{ inputs.verbose_make_output }}" == "ON" ]]; then
             flags="$flags -DCMAKE_VERBOSE_MAKEFILE=ON"
           fi

.github/workflows/on-pr-debug.yml

@@ -17,6 +17,7 @@ jobs:
       compiler: ${{ matrix.compiler }}
       server_version: main
       type: Debug
+      experimental: ON
 
   dev-mode:
     name: "DEV_MODE"
@@ -27,6 +28,7 @@ jobs:
       type: Debug
       verbose_test_output: ON
       verbose_make_output: ON
+      experimental: ON
 
   sanitize:
     name: "Sanitize"
@@ -43,6 +45,7 @@ jobs:
       compiler: ${{ matrix.compiler }}
       sanitize: ${{ matrix.sanitize }}
       pool_dispatch: ${{ matrix.pooled_dispatch }}
+      experimental: ON
 
   coverage-TLS:
     name: "Coverage: TLS"
@@ -61,6 +64,7 @@ jobs:
       verify_host: verify_host
       pool_dispatch: ${{ matrix.pooled_dispatch }}
       write_deadline: ${{ matrix.write_deadline }}
+      experimental: ON
     secrets:
       CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
 
@@ -74,6 +78,7 @@ jobs:
       compiler: gcc
       tls: TLS
       verify_host: NO-verify_host
+      experimental: ON
     secrets:
       CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
 
@@ -116,7 +121,7 @@ jobs:
         env:
           VCPKG_BINARY_SOURCES: "clear;x-gha,readwrite"
         run: |
-          cmake -B build -S . -DCMAKE_BUILD_TYPE=Debug -DNATS_BUILD_STREAMING=OFF
+          cmake -B build -S . -DCMAKE_BUILD_TYPE=Debug -DNATS_BUILD_STREAMING=OFF -DNATS_WITH_EXPERIMENTAL=ON
           cmake --build build
 
       - name: Test

CMakeLists.txt

@@ -46,6 +46,7 @@ option(NATS_BUILD_NO_PREFIX_CONNSTS "No prefix for connection status enum" OFF)
 option(NATS_BUILD_LIB_STATIC "Build static library" ON)
 option(NATS_BUILD_LIB_SHARED "Build shared library" ON)
 option(NATS_COMPILER_HARDENING "Compiler hardening flags" OFF)
+option(NATS_WITH_EXPERIMENTAL "Build with EXPERIMENTAL API support" OFF)
 if(UNIX AND APPLE)
   option(CMAKE_MACOSX_RPATH "Build with macOS RPath" ON)
 endif()
@@ -149,6 +150,10 @@ if(NATS_BUILD_NO_PREFIX_CONNSTS)
   add_definitions(-DNATS_CONN_STATUS_NO_PREFIX)
 endif(NATS_BUILD_NO_PREFIX_CONNSTS)
 
+if(NATS_WITH_EXPERIMENTAL)
+  add_definitions(-DNATS_WITH_EXPERIMENTAL)
+endif(NATS_WITH_EXPERIMENTAL)
+
 # Platform specific settings
 if(UNIX)
   #---------------------------------------------------------------------------

README.md

@@ -18,6 +18,7 @@ This NATS Client implementation is heavily based on the [NATS GO Client](https:/
 - [Building](#building)
 	* [TLS Support](#tls-support)
         * [Link statically](#link-statically)
+    * [Building with EXPERIMENTAL API support](#building-with-experimental-API-support)
     * [Building with Streaming](#building-with-streaming)
     * [Building with Libsodium](#building-with-libsodium)
     * [Testing](#testing)
@@ -177,6 +178,18 @@ cmake .. <build options that you may already use> -DNATS_BUILD_OPENSSL_STATIC_LI
 ```
 Then call `make` (or equivalent depending on your platform) and this should ensure that the library (and examples and/or test suite executable) are linked against the OpenSSL library, if it was found by CMake.
 
+## Building with EXPERIMENTAL API support
+
+At times we may introduce APIs that are not stable yet. To build with them, define NATS_WITH_EXPERIMENTAL:
+```
+# cd ./build
+rm CMakeCache.txt
+cmake .. <build options that you may already use> -DNATS_WITH_EXPERIMENTALON
+make
+```
+
+Note that experimental APIs may have other dependencies (like SSL).
+
 ## Building with Streaming
 
 When building the library with Streaming support, the NATS library uses the [libprotobuf-c](https://github.com/protobuf-c/protobuf-c) library.

src/conn.c

@@ -736,7 +736,14 @@ _makeTLSConn(natsConnection *nc)
                     s = nats_setError(NATS_SSL_ERROR, "unable to set expected hostname '%s'", nc->tlsName);
             }
             if (s == NATS_OK)
-                SSL_set_verify(ssl, SSL_VERIFY_PEER, nc->opts->sslCtx->callback != NULL ? nc->opts->sslCtx->callback : _collectSSLErr);
+            {
+                SSL_verify_cb cb = _collectSSLErr;
+#ifdef NATS_WITH_EXPERIMENTAL
+                if (nc->opts->sslCtx->callback != NULL)
+                    cb = nc->opts->sslCtx->callback;
+#endif // NATS_WITH_EXPERIMENTAL
+                SSL_set_verify(ssl, SSL_VERIFY_PEER, cb);
+            }
         }
     }
 #if defined(NATS_USE_OPENSSL_1_1)

src/nats.h

@@ -27,13 +27,15 @@ extern "C" {
 #include "status.h"
 #include "version.h"
 
-#if defined(NATS_HAS_TLS)
+#ifdef NATS_WITH_EXPERIMENTAL
+
+#if !defined(NATS_HAS_TLS)
+#error "natsOptions_SetSSLVerificationCallback requires NATS_HAS_TLS to be defined"
+#endif
 #include <openssl/ssl.h>
 #include <openssl/x509v3.h>
-#else
-#define X509_STORE_CTX void
-typedef int (*SSL_verify_cb)(int preverify_ok, X509_STORE_CTX* x509_ctx);
-#endif
+
+#endif // NATS_WITH_EXPERIMENTAL
 
 /** \def NATS_EXTERN
  *  \brief Needed for shared library.
@@ -2628,18 +2630,29 @@ natsOptions_SetExpectedHostname(natsOptions *opts, const char *hostname);
 NATS_EXTERN natsStatus
 natsOptions_SkipServerVerification(natsOptions *opts, bool skip);
 
-/** \brief Sets the certificate validation callback.
+#ifdef NATS_WITH_EXPERIMENTAL
+
+/** \brief EXPERIMENTAL Sets the certificate validation callback.
  *
  * Sets a callback used to verify the SSL certificate.
  *
- * \note Setting a callback will enable SSL verification if disabled via natsOptions_SkipServerVerification().
+ * \note Setting a callback will enable SSL verification if disabled via
+ * natsOptions_SkipServerVerification().
+ *
+ * \warning This is an experimental API and is subject to change in future
+ * versions. To use this API compile the client code with
+ * `-DNATS_WITH_EXPERIMENTAL -DNATS_HAS_TLS`. `openssl` library must be
+ * installed and added to the include/link paths.
  *
  * @param opts the pointer to the #natsOptions object.
- * @param callback the custom SSL verification handler to invoke. see https://docs.openssl.org/master/man3/SSL_CTX_set_verify/
+ * @param callback the custom SSL verification handler to invoke. see
+ * https://docs.openssl.org/master/man3/SSL_CTX_set_verify/
  */
 NATS_EXTERN natsStatus
 natsOptions_SetSSLVerificationCallback(natsOptions *opts, SSL_verify_cb callback);
 
+#endif // NATS_WITH_EXPERIMENTAL
+
 /** \brief Sets the verbose mode.
  *
  * Sets the verbose mode. If `true`, sends are echoed by the server with

src/natsp.h

@@ -187,7 +187,10 @@ typedef struct __natsSSLCtx
     SSL_CTX         *ctx;
     char            *expectedHostname;
     bool            skipVerify;
+
+#ifdef NATS_WITH_EXPERIMENTAL
     SSL_verify_cb   callback;
+#endif // NATS_WITH_EXPERIMENTAL
 
 } natsSSLCtx;
 

src/opts.c

@@ -695,17 +695,19 @@ natsOptions_SkipServerVerification(natsOptions *opts, bool skip)
     if (s == NATS_OK)
     {
         opts->sslCtx->skipVerify = skip;
+#ifdef NATS_WITH_EXPERIMENTAL
         if (skip)
-        {
             opts->sslCtx->callback = NULL;
-        }
+#endif // NATS_WITH_EXPERIMENTAL
     }
 
     UNLOCK_OPTS(opts);
 
     return s;
 }
 
+#ifdef NATS_WITH_EXPERIMENTAL
+
 natsStatus
 natsOptions_SetSSLVerificationCallback(natsOptions *opts, SSL_verify_cb callback)
 {
@@ -728,6 +730,8 @@ natsOptions_SetSSLVerificationCallback(natsOptions *opts, SSL_verify_cb callback
     return s;
 }
 
+#endif // NATS_WITH_EXPERIMENTAL
+
 #else
 
 natsStatus
@@ -786,12 +790,17 @@ natsOptions_SkipServerVerification(natsOptions *opts, bool skip)
     return nats_setError(NATS_ILLEGAL_STATE, "%s", NO_SSL_ERR);
 }
 
+
+#ifdef NATS_WITH_EXPERIMENTAL
+
 natsStatus
 natsOptions_SetSSLVerificationCallback(natsOptions *opts, SSL_verify_cb callback)
 {
     return nats_setError(NATS_ILLEGAL_STATE, "%s", NO_SSL_ERR);
 }
 
+#endif // NATS_WITH_EXPERIMENTAL
+
 #endif
 
 natsStatus

test/test.c

@@ -21195,7 +21195,8 @@ _sslVerifyCallback(int preverify_ok, X509_STORE_CTX *ctx)
 
 void test_SSLVerificationCallback(void)
 {
-#if defined(NATS_HAS_TLS)
+#ifdef NATS_WITH_EXPERIMENTAL
+#ifdef NATS_HAS_TLS
     natsStatus          s;
     natsConnection      *nc         = NULL;
     natsOptions         *opts       = NULL;
@@ -21228,7 +21229,8 @@ void test_SSLVerificationCallback(void)
 #else
     test("Skipped when built with no SSL support: ");
     testCond(true);
-#endif
+#endif // NATS_HAS_TLS
+#endif // NATS_WITH_EXPERIMENTAL
 }
 
 void test_SSLCiphers(void)