From 4f19da67f801c61560c652bb84680ba021e042d7 Mon Sep 17 00:00:00 2001 From: Ivan Kozlovic Date: Tue, 13 Aug 2024 10:34:08 -0600 Subject: [PATCH] Fixed test SSLVerifyHostName when build NATS_FORCE_HOST_VERIFICATION=OFF (#788) * Fixed test SSLVerifyHostName when build NATS_FORCE_HOST_VERIFICATION=OFF This test would fail unless we force host verification, so adapt test to take into consideration the expected result based on the build environment variable. Signed-off-by: Ivan Kozlovic * Added a GHA job for NATS_FORCE_HOST_VERIFICATION * fixed default: ON * job: no-host-sanitize * job names * [CI only] Streamlined/fixed sanitize/coverage matrices (#790) --------- Signed-off-by: Ivan Kozlovic Co-authored-by: Lev Brouk Co-authored-by: Lev <1187448+levb@users.noreply.github.com> --- .github/workflows/build-test.yml | 32 ++++++++----- .github/workflows/on-pr-debug.yml | 74 +++++++++++++++++++------------ test/test.c | 13 +++++- 3 files changed, 79 insertions(+), 40 deletions(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index e4eda5c70..cf96b75e6 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -13,12 +13,18 @@ on: dev_mode: type: string default: "OFF" - lib_msg_delivery: + pool_dispatch: type: string - default: "OFF" - lib_write_deadline: + default: "NO-pool" + write_deadline: type: string - default: "OFF" + default: "NO-write_deadline" + tls: + type: string + default: "TLS" + verify_host: + type: string + default: "verify_host" repeat: type: string default: "1" @@ -32,9 +38,6 @@ on: streaming: type: string default: "ON" - tls: - type: string - default: "ON" type: type: string description: "Debug or Release." @@ -85,11 +88,20 @@ jobs: flags: -DNATS_BUILD_ARCH=${{ inputs.arch }} -DCMAKE_BUILD_TYPE=${{ inputs.type }} -DNATS_BUILD_STREAMING=${{ inputs.streaming }} - -DNATS_BUILD_WITH_TLS=${{ inputs.tls }} -DNATS_PROTOBUF_DIR=${{ github.workspace}}/deps/pbuf -DNATS_BUILD_USE_SODIUM=ON -DNATS_SODIUM_DIR=${{ github.workspace}}/deps/sodium run: | + if [[ "${{ inputs.tls }}" == "TLS" ]]; then + flags="$flags -DNATS_BUILD_WITH_TLS=ON" + if [[ "${{ inputs.verify_host }}" == "verify_host" ]]; then + flags="$flags -DNATS_BUILD_TLS_FORCE_HOST_VERIFY=ON" + else + flags="$flags -DNATS_BUILD_TLS_FORCE_HOST_VERIFY=OFF" + fi + 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 @@ -110,10 +122,10 @@ jobs: if [[ -n "${{ inputs.sanitize }}" ]]; then echo "NATS_TEST_VALGRIND=yes" >> $GITHUB_ENV fi - if [[ "${{ inputs.lib_msg_delivery }}" == "ON" ]]; then + if [[ "${{ inputs.pool_dispatch }}" == "pool" ]]; then echo "NATS_DEFAULT_TO_LIB_MSG_DELIVERY=yes" >> $GITHUB_ENV fi - if [[ "${{ inputs.lib_write_deadline }}" == "ON" ]]; then + if [[ "${{ inputs.write_deadline }}" == "write_deadline" ]]; then echo "NATS_DEFAULT_LIB_WRITE_DEADLINE=2000" >> $GITHUB_ENV fi echo "CC=${{ inputs.compiler }}" >> $GITHUB_ENV diff --git a/.github/workflows/on-pr-debug.yml b/.github/workflows/on-pr-debug.yml index 6ec95f12f..fe1a1d0e7 100644 --- a/.github/workflows/on-pr-debug.yml +++ b/.github/workflows/on-pr-debug.yml @@ -18,27 +18,6 @@ jobs: server_version: main type: Debug - coverage: - name: "Coverage" - uses: ./.github/workflows/build-test.yml - with: - coverage: ON - server_version: main - type: Debug - secrets: - CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} - - coverage-pooled: - name: "Coverage (pooled delivery)" - uses: ./.github/workflows/build-test.yml - with: - coverage: ON - server_version: main - type: Debug - lib_msg_delivery: ON - secrets: - CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} - dev-mode: name: "DEV_MODE" uses: ./.github/workflows/build-test.yml @@ -56,24 +35,61 @@ jobs: matrix: compiler: [gcc, clang] sanitize: [address, thread] - pooled_delivery: [ON, OFF] + pooled_dispatch: [pool, NO-pool] uses: ./.github/workflows/build-test.yml with: server_version: main - type: Debug + type: RelWithDebInfo compiler: ${{ matrix.compiler }} sanitize: ${{ matrix.sanitize }} - lib_msg_delivery: ${{ matrix.pooled_delivery }} + pool_dispatch: ${{ matrix.pooled_dispatch }} - san-addr-deadline: - name: "Sanitize address (lib_write_deadline)" + coverage-TLS: + name: "Coverage: TLS" + strategy: + fail-fast: false + matrix: + pooled_dispatch: [pool, NO-pool] + write_deadline: [write_deadline, NO-write_deadline] uses: ./.github/workflows/build-test.yml with: - type: Debug - sanitize: address + coverage: ON + type: RelWithDebInfo server_version: main - lib_write_deadline: ON + compiler: gcc + tls: TLS + verify_host: verify_host + pool_dispatch: ${{ matrix.pooled_dispatch }} + write_deadline: ${{ matrix.write_deadline }} + secrets: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + coverage-NO-verify_host: + name: "Coverage: NO-verify_host" + uses: ./.github/workflows/build-test.yml + with: + coverage: ON + type: RelWithDebInfo + server_version: main + compiler: gcc + tls: TLS + verify_host: NO-verify_host + secrets: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + + coverage-NO-TLS: + name: "Coverage NO-TLS" + uses: ./.github/workflows/build-test.yml + with: + coverage: ON + type: RelWithDebInfo + server_version: main + compiler: gcc + tls: NO-TLS + verify_host: NO-verify_host + secrets: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + bench: name: "Benchmark" uses: ./.github/workflows/build-test.yml diff --git a/test/test.c b/test/test.c index 021c983a0..5c46669a4 100644 --- a/test/test.c +++ b/test/test.c @@ -20895,20 +20895,31 @@ void test_SSLVerifyHostname(void) serverPid = _startServer("nats://127.0.0.1:4443", "-config tls_noip.conf", true); CHECK_SERVER_STARTED(serverPid); +#if defined(NATS_FORCE_HOST_VERIFICATION) test("Check that connect fails if url is IP: "); +#else + test("Check that connect succeeds if url is IP: "); +#endif s = natsOptions_SetURL(opts, "nats://127.0.0.1:4443"); IFOK(s, natsOptions_SetSecure(opts, true)); - // For test purposes, we provide the CA trusted certs IFOK(s, natsOptions_LoadCATrustedCertificates(opts, "certs/ca.pem")); IFOK(s, natsOptions_SetReconnectedCB(opts, _reconnectedCb, &args)); IFOK(s, natsConnection_Connect(&nc, opts)); +#if defined(NATS_FORCE_HOST_VERIFICATION) testCond(s == NATS_SSL_ERROR); + nats_clearLastError(); +#else + testCond(s == NATS_OK); + natsConnection_Destroy(nc); + nc = NULL; +#endif test("Check that connect fails if wrong expected hostname: "); s = natsOptions_SetURL(opts, "nats://localhost:4443"); IFOK(s, natsOptions_SetExpectedHostname(opts, "foo")); IFOK(s, natsConnection_Connect(&nc, opts)); testCond(s == NATS_SSL_ERROR); + nats_clearLastError(); test("Check that connect succeeds if hostname ok and no expected hostname set: "); s = natsOptions_SetURL(opts, "nats://localhost:4443");