Skip to content

Commit 484f16f

Browse files
levblevb
committed
[FIXED] make natsOptions_SetSSLVerificationCallback experimental (#860)
* [FIXED] wrapped natsOptions_SetSSLVerificationCallback in ifdef NATS_WITH_EXPERIMENTAL * PR feedback
1 parent 3bf08f6 commit 484f16f

File tree

9 files changed

+94
-31
lines changed

9 files changed

+94
-31
lines changed

‎.github/workflows/build-test.yml

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ on:
44
arch:
55
type: string
66
default: "64"
7+
benchmark:
8+
type: string
9+
default: "OFF"
710
compiler:
811
type: string
912
default: "gcc"
@@ -13,18 +16,12 @@ on:
1316
dev_mode:
1417
type: string
1518
default: "OFF"
19+
experimental:
20+
type: string
21+
default: "OFF"
1622
pool_dispatch:
1723
type: string
1824
default: "NO-pool"
19-
write_deadline:
20-
type: string
21-
default: "NO-write_deadline"
22-
tls:
23-
type: string
24-
default: "TLS"
25-
verify_host:
26-
type: string
27-
default: "verify_host"
2825
repeat:
2926
type: string
3027
default: "1"
@@ -38,6 +35,9 @@ on:
3835
streaming:
3936
type: string
4037
default: "ON"
38+
tls:
39+
type: string
40+
default: "TLS"
4141
type:
4242
type: string
4343
description: "Debug or Release."
@@ -46,15 +46,18 @@ on:
4646
type: string
4747
description: "Ubuntu version to use, e.g. '20.04'"
4848
default: "latest"
49-
verbose_test_output:
50-
type: string
51-
default: "OFF"
5249
verbose_make_output:
5350
type: string
5451
default: "OFF"
55-
benchmark:
52+
verbose_test_output:
5653
type: string
5754
default: "OFF"
55+
verify_host:
56+
type: string
57+
default: "verify_host"
58+
write_deadline:
59+
type: string
60+
default: "NO-write_deadline"
5861

5962
secrets:
6063
CODECOV_TOKEN:
@@ -88,8 +91,8 @@ jobs:
8891
flags: -DNATS_BUILD_ARCH=${{ inputs.arch }}
8992
-DCMAKE_BUILD_TYPE=${{ inputs.type }}
9093
-DNATS_BUILD_STREAMING=${{ inputs.streaming }}
91-
-DNATS_PROTOBUF_DIR=${{ github.workspace}}/deps/pbuf
9294
-DNATS_BUILD_USE_SODIUM=ON
95+
-DNATS_PROTOBUF_DIR=${{ github.workspace}}/deps/pbuf
9396
-DNATS_SODIUM_DIR=${{ github.workspace}}/deps/sodium
9497
run: |
9598
if [[ "${{ inputs.tls }}" == "TLS" ]]; then
@@ -102,15 +105,18 @@ jobs:
102105
else
103106
flags="$flags -DNATS_BUILD_WITH_TLS=OFF"
104107
fi
105-
if [[ -n "${{ inputs.sanitize }}" ]]; then
106-
flags="$flags -DNATS_SANITIZE=ON -DCMAKE_C_FLAGS=-fsanitize=${{ inputs.sanitize }}"
107-
fi
108108
if [[ "${{ inputs.coverage }}" == "ON" ]]; then
109109
flags="$flags -DNATS_COVERAGE=ON"
110110
fi
111111
if [[ "${{ inputs.dev_mode }}" == "ON" ]]; then
112112
flags="$flags -DDEV_MODE=ON"
113113
fi
114+
if [[ "${{ inputs.experimental }}" == "ON" ]]; then
115+
flags="$flags -DNATS_EXPERIMENTAL=ON"
116+
fi
117+
if [[ -n "${{ inputs.sanitize }}" ]]; then
118+
flags="$flags -DNATS_SANITIZE=ON -DCMAKE_C_FLAGS=-fsanitize=${{ inputs.sanitize }}"
119+
fi
114120
if [[ "${{ inputs.verbose_make_output }}" == "ON" ]]; then
115121
flags="$flags -DCMAKE_VERBOSE_MAKEFILE=ON"
116122
fi

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ jobs:
1717
compiler: ${{ matrix.compiler }}
1818
server_version: main
1919
type: Debug
20+
experimental: ON
2021

2122
dev-mode:
2223
name: "DEV_MODE"
@@ -27,6 +28,7 @@ jobs:
2728
type: Debug
2829
verbose_test_output: ON
2930
verbose_make_output: ON
31+
experimental: ON
3032

3133
sanitize:
3234
name: "Sanitize"
@@ -43,6 +45,7 @@ jobs:
4345
compiler: ${{ matrix.compiler }}
4446
sanitize: ${{ matrix.sanitize }}
4547
pool_dispatch: ${{ matrix.pooled_dispatch }}
48+
experimental: ON
4649

4750
coverage-TLS:
4851
name: "Coverage: TLS"
@@ -61,6 +64,7 @@ jobs:
6164
verify_host: verify_host
6265
pool_dispatch: ${{ matrix.pooled_dispatch }}
6366
write_deadline: ${{ matrix.write_deadline }}
67+
experimental: ON
6468
secrets:
6569
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
6670

@@ -74,6 +78,7 @@ jobs:
7478
compiler: gcc
7579
tls: TLS
7680
verify_host: NO-verify_host
81+
experimental: ON
7782
secrets:
7883
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
7984

@@ -116,7 +121,7 @@ jobs:
116121
env:
117122
VCPKG_BINARY_SOURCES: "clear;x-gha,readwrite"
118123
run: |
119-
cmake -B build -S . -DCMAKE_BUILD_TYPE=Debug -DNATS_BUILD_STREAMING=OFF
124+
cmake -B build -S . -DCMAKE_BUILD_TYPE=Debug -DNATS_BUILD_STREAMING=OFF -DNATS_WITH_EXPERIMENTAL=ON
120125
cmake --build build
121126
122127
- name: Test

‎CMakeLists.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ option(NATS_BUILD_NO_PREFIX_CONNSTS "No prefix for connection status enum" OFF)
4646
option(NATS_BUILD_LIB_STATIC "Build static library" ON)
4747
option(NATS_BUILD_LIB_SHARED "Build shared library" ON)
4848
option(NATS_COMPILER_HARDENING "Compiler hardening flags" OFF)
49+
option(NATS_WITH_EXPERIMENTAL "Build with EXPERIMENTAL API support" OFF)
4950
if(UNIX AND APPLE)
5051
option(CMAKE_MACOSX_RPATH "Build with macOS RPath" ON)
5152
endif()
@@ -149,6 +150,10 @@ if(NATS_BUILD_NO_PREFIX_CONNSTS)
149150
add_definitions(-DNATS_CONN_STATUS_NO_PREFIX)
150151
endif(NATS_BUILD_NO_PREFIX_CONNSTS)
151152

153+
if(NATS_WITH_EXPERIMENTAL)
154+
add_definitions(-DNATS_WITH_EXPERIMENTAL)
155+
endif(NATS_WITH_EXPERIMENTAL)
156+
152157
# Platform specific settings
153158
if(UNIX)
154159
#---------------------------------------------------------------------------

‎README.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ This NATS Client implementation is heavily based on the [NATS GO Client](https:/
1818
- [Building](#building)
1919
* [TLS Support](#tls-support)
2020
* [Link statically](#link-statically)
21+
* [Building with EXPERIMENTAL API support](#building-with-experimental-API-support)
2122
* [Building with Streaming](#building-with-streaming)
2223
* [Building with Libsodium](#building-with-libsodium)
2324
* [Testing](#testing)
@@ -177,6 +178,18 @@ cmake .. <build options that you may already use> -DNATS_BUILD_OPENSSL_STATIC_LI
177178
```
178179
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.
179180

181+
## Building with EXPERIMENTAL API support
182+
183+
At times we may introduce APIs that are not stable yet. To build with them, define NATS_WITH_EXPERIMENTAL:
184+
```
185+
# cd ./build
186+
rm CMakeCache.txt
187+
cmake .. <build options that you may already use> -DNATS_WITH_EXPERIMENTALON
188+
make
189+
```
190+
191+
Note that experimental APIs may have other dependencies (like SSL).
192+
180193
## Building with Streaming
181194

182195
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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -736,7 +736,14 @@ _makeTLSConn(natsConnection *nc)
736736
s = nats_setError(NATS_SSL_ERROR, "unable to set expected hostname '%s'", nc->tlsName);
737737
}
738738
if (s == NATS_OK)
739-
SSL_set_verify(ssl, SSL_VERIFY_PEER, nc->opts->sslCtx->callback != NULL ? nc->opts->sslCtx->callback : _collectSSLErr);
739+
{
740+
SSL_verify_cb cb = _collectSSLErr;
741+
#ifdef NATS_WITH_EXPERIMENTAL
742+
if (nc->opts->sslCtx->callback != NULL)
743+
cb = nc->opts->sslCtx->callback;
744+
#endif // NATS_WITH_EXPERIMENTAL
745+
SSL_set_verify(ssl, SSL_VERIFY_PEER, cb);
746+
}
740747
}
741748
}
742749
#if defined(NATS_USE_OPENSSL_1_1)

‎src/nats.h

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,15 @@ extern "C" {
2727
#include "status.h"
2828
#include "version.h"
2929

30-
#if defined(NATS_HAS_TLS)
30+
#ifdef NATS_WITH_EXPERIMENTAL
31+
32+
#if !defined(NATS_HAS_TLS)
33+
#error "natsOptions_SetSSLVerificationCallback requires NATS_HAS_TLS to be defined"
34+
#endif
3135
#include <openssl/ssl.h>
3236
#include <openssl/x509v3.h>
33-
#else
34-
#define X509_STORE_CTX void
35-
typedef int (*SSL_verify_cb)(int preverify_ok, X509_STORE_CTX* x509_ctx);
36-
#endif
37+
38+
#endif // NATS_WITH_EXPERIMENTAL
3739

3840
/** \def NATS_EXTERN
3941
* \brief Needed for shared library.
@@ -2628,18 +2630,29 @@ natsOptions_SetExpectedHostname(natsOptions *opts, const char *hostname);
26282630
NATS_EXTERN natsStatus
26292631
natsOptions_SkipServerVerification(natsOptions *opts, bool skip);
26302632

2631-
/** \brief Sets the certificate validation callback.
2633+
#ifdef NATS_WITH_EXPERIMENTAL
2634+
2635+
/** \brief EXPERIMENTAL Sets the certificate validation callback.
26322636
*
26332637
* Sets a callback used to verify the SSL certificate.
26342638
*
2635-
* \note Setting a callback will enable SSL verification if disabled via natsOptions_SkipServerVerification().
2639+
* \note Setting a callback will enable SSL verification if disabled via
2640+
* natsOptions_SkipServerVerification().
2641+
*
2642+
* \warning This is an experimental API and is subject to change in future
2643+
* versions. To use this API compile the client code with
2644+
* `-DNATS_WITH_EXPERIMENTAL -DNATS_HAS_TLS`. `openssl` library must be
2645+
* installed and added to the include/link paths.
26362646
*
26372647
* @param opts the pointer to the #natsOptions object.
2638-
* @param callback the custom SSL verification handler to invoke. see https://docs.openssl.org/master/man3/SSL_CTX_set_verify/
2648+
* @param callback the custom SSL verification handler to invoke. see
2649+
* https://docs.openssl.org/master/man3/SSL_CTX_set_verify/
26392650
*/
26402651
NATS_EXTERN natsStatus
26412652
natsOptions_SetSSLVerificationCallback(natsOptions *opts, SSL_verify_cb callback);
26422653

2654+
#endif // NATS_WITH_EXPERIMENTAL
2655+
26432656
/** \brief Sets the verbose mode.
26442657
*
26452658
* Sets the verbose mode. If `true`, sends are echoed by the server with

‎src/natsp.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,10 @@ typedef struct __natsSSLCtx
187187
SSL_CTX *ctx;
188188
char *expectedHostname;
189189
bool skipVerify;
190+
191+
#ifdef NATS_WITH_EXPERIMENTAL
190192
SSL_verify_cb callback;
193+
#endif // NATS_WITH_EXPERIMENTAL
191194

192195
} natsSSLCtx;
193196

‎src/opts.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -695,17 +695,19 @@ natsOptions_SkipServerVerification(natsOptions *opts, bool skip)
695695
if (s == NATS_OK)
696696
{
697697
opts->sslCtx->skipVerify = skip;
698+
#ifdef NATS_WITH_EXPERIMENTAL
698699
if (skip)
699-
{
700700
opts->sslCtx->callback = NULL;
701-
}
701+
#endif // NATS_WITH_EXPERIMENTAL
702702
}
703703

704704
UNLOCK_OPTS(opts);
705705

706706
return s;
707707
}
708708

709+
#ifdef NATS_WITH_EXPERIMENTAL
710+
709711
natsStatus
710712
natsOptions_SetSSLVerificationCallback(natsOptions *opts, SSL_verify_cb callback)
711713
{
@@ -728,6 +730,8 @@ natsOptions_SetSSLVerificationCallback(natsOptions *opts, SSL_verify_cb callback
728730
return s;
729731
}
730732

733+
#endif // NATS_WITH_EXPERIMENTAL
734+
731735
#else
732736

733737
natsStatus
@@ -786,12 +790,17 @@ natsOptions_SkipServerVerification(natsOptions *opts, bool skip)
786790
return nats_setError(NATS_ILLEGAL_STATE, "%s", NO_SSL_ERR);
787791
}
788792

793+
794+
#ifdef NATS_WITH_EXPERIMENTAL
795+
789796
natsStatus
790797
natsOptions_SetSSLVerificationCallback(natsOptions *opts, SSL_verify_cb callback)
791798
{
792799
return nats_setError(NATS_ILLEGAL_STATE, "%s", NO_SSL_ERR);
793800
}
794801

802+
#endif // NATS_WITH_EXPERIMENTAL
803+
795804
#endif
796805

797806
natsStatus

‎test/test.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21195,7 +21195,8 @@ _sslVerifyCallback(int preverify_ok, X509_STORE_CTX *ctx)
2119521195

2119621196
void test_SSLVerificationCallback(void)
2119721197
{
21198-
#if defined(NATS_HAS_TLS)
21198+
#ifdef NATS_WITH_EXPERIMENTAL
21199+
#ifdef NATS_HAS_TLS
2119921200
natsStatus s;
2120021201
natsConnection *nc = NULL;
2120121202
natsOptions *opts = NULL;
@@ -21228,7 +21229,8 @@ void test_SSLVerificationCallback(void)
2122821229
#else
2122921230
test("Skipped when built with no SSL support: ");
2123021231
testCond(true);
21231-
#endif
21232+
#endif // NATS_HAS_TLS
21233+
#endif // NATS_WITH_EXPERIMENTAL
2123221234
}
2123321235

2123421236
void test_SSLCiphers(void)

0 commit comments

Comments
 (0)