Merge pull request #1405 from mickem/test/checknet-external-connectivity

ci+fix: Validate RedHat/Rocky builds, and fix the CA bundle they exposed

Author: mickem

.github/workflows/integration-tests-linux.yml

@@ -180,14 +180,19 @@ jobs:
       # dependent scenario (icinga, nrpe, nsca, …) skip itself so this stage
       # runs without a docker daemon. Each rest-* test spins its own nscp
       # via NscpInstance, so no manual start/kill/log-tail is needed here.
+      #
+      # This used to be deb-only, from when the harness still needed a docker
+      # daemon. NSCP_SKIP_DOCKER removed that, but the condition stayed - which
+      # left the rpm packages with only acceptance-tests.sh for coverage, so
+      # anything that behaves differently on RHEL went untested. The RPM base
+      # dependencies already install Node 20 from nodesource, so both package
+      # types run the same suite.
       - name: Install integration harness deps
-        if: inputs.package-type == 'deb'
         working-directory: tests
         shell: bash
         run: npm install
 
       - name: Run REST scenarios (no-docker)
-        if: inputs.package-type == 'deb'
         working-directory: tests
         shell: bash
         env:

CMakeLists.txt

@@ -22,7 +22,7 @@ endif()
 
 # ##############################################################################
 #
-# Setup cmake enviornment and include custom config overrides
+# Setup cmake environment and include custom config overrides
 #
 # ##############################################################################
 if(NSCP_CMAKE_CONFIG)
@@ -328,7 +328,7 @@ if(NOT HAVE_REQUIRED_DEPENDENCIES)
 else(NOT HAVE_REQUIRED_DEPENDENCIES)
     message(
         STATUS
-        " - All required dependecies found! (some modules and features migh stil be missing though)"
+        " - All required dependencies found! (some modules and features might still be missing though)"
     )
 endif(NOT HAVE_REQUIRED_DEPENDENCIES)
 
@@ -729,7 +729,7 @@ else(WIN32)
     set(MAIN_CONFIGURATION
         /etc/nscp/nscp.xml
         CACHE PATH
-        "Path for the client configurtion file"
+        "Path for the client configuration file"
     )
     set(DEPLOYROOT /usr/bin CACHE PATH "Path to deploy examples into)")
 endif(WIN32)
@@ -863,6 +863,13 @@ if(WIN32)
         CACHE STRING
         "security (certificates) folder"
     )
+    # The service exports the Windows ROOT store here at boot (windows_ca_store),
+    # so unlike unix this is a path we own rather than one the platform ships.
+    set(CONFIG_CA_PATH
+        "\${certificate-path}/windows-ca.pem"
+        CACHE STRING
+        "Trusted CA bundle used when a check does not name its own"
+    )
     set(CONFIG_DEFAULT_CACHE_PATH
         "\${shared-path}/cache"
         CACHE STRING
@@ -920,8 +927,53 @@ else()
     set(CONFIG_CERT_FOLDER
         "\${shared-path}/security"
         CACHE STRING
-        "securoty (certificates) folder"
+        "security (certificates) folder"
     )
+
+    # The trusted CA bundle belongs to the distribution, so this is an absolute
+    # path that must NOT track CMAKE_INSTALL_PREFIX: a --prefix=/usr/local build
+    # still reads the system bundle, not /usr/local/etc/ssl/...
+    #
+    # Where that bundle lives differs per family, and picking wrong is silent -
+    # every TLS check that does not name its own CA then fails with "Failed to
+    # load CA <path>: No such file or directory", including checks against a
+    # local self-signed server with verification disabled, since the file is
+    # loaded before the verify mode is considered. Detecting it here works
+    # because a package is built in a container of the distribution it targets;
+    # a packager who knows better can override with -DCONFIG_CA_PATH=...
+    if(NOT CONFIG_CA_PATH)
+        foreach(
+            _ca_candidate
+            "/etc/ssl/certs/ca-certificates.crt" # Debian, Ubuntu, Gentoo
+            "/etc/pki/tls/certs/ca-bundle.crt" # RHEL, Rocky, Alma, Fedora
+            "/etc/ssl/ca-bundle.pem" # SUSE
+            "/etc/ssl/cert.pem" # Alpine, FreeBSD, macOS
+        )
+            if(EXISTS "${_ca_candidate}")
+                set(_ca_detected "${_ca_candidate}")
+                break()
+            endif()
+        endforeach()
+        if(NOT _ca_detected)
+            # Nothing found: keep the historical value so the define is never
+            # empty, and say so - a build host without a CA bundle is unusual
+            # enough to be worth a line in the configure output.
+            set(_ca_detected "/etc/ssl/certs/ca-certificates.crt")
+            message(
+                WARNING
+                "No system CA bundle found; defaulting ca-path to ${_ca_detected}. "
+                "TLS checks that do not name their own CA will fail unless that file exists at runtime "
+                "(override with -DCONFIG_CA_PATH=/path/to/bundle)."
+            )
+        endif()
+        set(CONFIG_CA_PATH
+            "${_ca_detected}"
+            CACHE STRING
+            "Trusted CA bundle used when a check does not name its own"
+        )
+    endif()
+    message(STATUS "System CA bundle (ca-path): ${CONFIG_CA_PATH}")
+
     set(CONFIG_DEFAULT_CACHE_PATH
         "\${shared-path}/cache"
         CACHE STRING
@@ -1188,7 +1240,7 @@ set(NSCP_CLIENT_HPP
 
 # ##############################################################################
 #
-# Build everything (not already built lik libraries)!
+# Build everything (not already built like libraries)!
 #
 # ##############################################################################
 

check_deps.cmake

@@ -11,7 +11,7 @@ set(CMAKE_FIND_LIBRARY_PREFIXES
 set(CMAKE_FIND_LIBRARY_SUFFIXES .lib)
 #############################################################################
 #
-# Setup cmake enviornment and include custom config overrides
+# Setup cmake environment and include custom config overrides
 #
 #############################################################################
 if(EXISTS ${TARGET}/build.cmake)

include/config.h.in

@@ -46,6 +46,11 @@
 #define WEB_FOLDER "@CONFIG_WEB_FOLDER@"
 #define SCRIPTS_FOLDER "@CONFIG_SCRIPTS_FOLDER@"
 #define CERT_FOLDER "@CONFIG_CERT_FOLDER@"
+// Trusted CA bundle used whenever a check does not name its own. On Windows the
+// service exports the ROOT store to this file at boot; on unix it is the system
+// bundle, detected at configure time because its location differs per
+// distribution (see CONFIG_CA_PATH in CMakeLists.txt).
+#define CA_PATH "@CONFIG_CA_PATH@"
 #define LOG_FOLDER "@CONFIG_LOG_FOLDER@"
 #ifndef WIN32
 #define UNIX_SHARED_PATH_FOLDER "@UNIX_SHARED_PATH_FOLDER@"

installer_lib/CMakeLists.txt

@@ -171,7 +171,7 @@ set(CONFIG_SCRIPTS_FOLDER "\${exe-path}/scripts" CACHE STRING "script folder")
 set(CONFIG_CERT_FOLDER
     "\${shared-path}/security"
     CACHE STRING
-    "securoty (certificates) folder"
+    "security (certificates) folder"
 )
 set(CONFIG_DEFAULT_CACHE_PATH
     "\${shared-path}/cache"

modules/CheckNSCP/CheckNSCP.cpp

@@ -38,9 +38,10 @@ bool CheckNSCP::loadModuleEx(std::string alias, NSCAPI::moduleLoadMode) {
   NSC_DEBUG_MSG_STD("Crash folder is: " + crashFolder.string());
 
   // Default the CA bundle to the trusted system store (${ca-path} expands to
-  // certificate-path/windows-ca.pem on Windows, /etc/ssl/certs/ca-certificates.crt
-  // on Linux). The same setting is used by CheckNet's check_http and lets the
-  // update check validate api.github.com out of the box.
+  // certificate-path/windows-ca.pem on Windows, and on unix to the
+  // distribution's own bundle, detected at configure time - see CONFIG_CA_PATH).
+  // The same setting is used by CheckNet's check_http and lets the update check
+  // validate api.github.com out of the box.
   const std::string default_ca = get_core()->expand_path("${ca-path}");
 
   // clang-format off

modules/CheckNet/CheckNet.cpp

@@ -31,8 +31,9 @@ boost::atomic<unsigned short> identifier(0);
 bool CheckNet::loadModuleEx(const std::string &, NSCAPI::moduleLoadMode) {
   // Resolve the trusted CA bundle path once, at module load. ${ca-path}
   // expands to ${certificate-path}/windows-ca.pem on Windows (the auto-
-  // generated system ROOT bundle) and to /etc/ssl/certs/ca-certificates.crt
-  // on Linux. check_http hands this through as the default `ca` so HTTPS
+  // generated system ROOT bundle) and on unix to the distribution's own bundle,
+  // detected at configure time because its location differs per family
+  // (CONFIG_CA_PATH). check_http hands this through as the default `ca` so HTTPS
   // checks against public-CA-signed servers validate out of the box.
   default_ca_ = get_core()->expand_path("${ca-path}");
   return true;

service/NSClient++.cpp

@@ -266,7 +266,7 @@ bool NSClientT::load_configuration_2(const bool override_log) {
     LOG_ERROR_CORE_STD("COM exception: " + e.reason());
     return false;
   } catch (...) {
-    LOG_ERROR_CORE("Unknown exception iniating COM...");
+    LOG_ERROR_CORE("Unknown exception initiating COM...");
     return false;
   }
 
@@ -486,7 +486,7 @@ bool NSClientT::stop_nsclient() {
   } catch (com_helper::com_exception &e) {
     LOG_ERROR_CORE_STD("COM exception: " + e.reason());
   } catch (...) {
-    LOG_ERROR_CORE("Unknown exception uniniating COM...");
+    LOG_ERROR_CORE("Unknown exception uninitiating COM...");
   }
 #endif
   LOG_DEBUG_CORE("Stopping: Settings instance");

service/path_manager.cpp

@@ -85,25 +85,20 @@ std::string nsclient::core::path_manager::get_path_for_key(const std::string &ke
   // Static defaults baked in by CMake via config.h. Most are templated on
   // ${shared-path} or ${certificate-path}; expand_path resolves the chain.
   // Note on ca-path: on Windows the service exports the system ROOT store to
-  // this file at boot (see windows_ca_store); on Linux this is the de-facto
-  // Debian/Ubuntu location, overridable via boot.ini's [paths] section
-  // (or the --path-override CLI flag) for other distros.
+  // this file at boot (see windows_ca_store); on unix it is the distribution's
+  // own bundle, whose location differs per family and is therefore detected at
+  // configure time (CONFIG_CA_PATH). Either way it stays overridable via
+  // boot.ini's [paths] section or --path-override ca-path=...
   static const std::map<std::string, std::string> defaults = {
       {"certificate-path", CERT_FOLDER},
       {"module-path", MODULE_FOLDER},
       {"web-path", WEB_FOLDER},
       {"scripts", SCRIPTS_FOLDER},
       {"log-path", LOG_FOLDER},
+      {"ca-path", CA_PATH},
       {CACHE_FOLDER_KEY, DEFAULT_CACHE_PATH},
       {CRASH_ARCHIVE_FOLDER_KEY, "${shared-path}/crash-dumps"},
-#ifdef WIN32
-      {"ca-path", "${certificate-path}/windows-ca.pem"},
-#else
-      // ca-path stays a literal absolute path: the system CA bundle belongs to
-      // the distro, lives at /etc/ssl/... regardless of our install prefix, and
-      // must NOT track ${etc}/NSCP_SYSCONFDIR (a --prefix=/usr/local build still
-      // reads /etc/ssl/certs/..., not /usr/local/etc/ssl/...).
-      {"ca-path", "/etc/ssl/certs/ca-certificates.crt"},
+#ifndef WIN32
       {"shared-path", UNIX_SHARED_PATH_FOLDER},
       {"data-path", UNIX_DATA_PATH_FOLDER},
       // ${etc} tracks this build's config root (NSCP_SYSCONFDIR) so user

service/path_manager_test.cpp

@@ -6,6 +6,8 @@
 #include <config.h>
 #include <gtest/gtest.h>
 
+#include <boost/filesystem.hpp>
+
 #include <memory>
 #include <nsclient/logger/logger.hpp>
 
@@ -79,6 +81,16 @@ TEST_F(PathManagerTest, CaPathExpandsToBundleFile) {
   EXPECT_NE(expanded.find("windows-ca.pem"), std::string::npos);
 #else
   EXPECT_EQ(expanded.rfind("/etc/", 0), 0u);
+
+  // ...and it must name a bundle that is actually there. This used to be one
+  // hardcoded path for every non-Windows platform - the Debian one - so on
+  // RHEL-family it named a file that does not exist, and every TLS check that
+  // did not carry its own ca= failed with "Failed to load CA <path>: No such
+  // file or directory". CONFIG_CA_PATH now detects the platform bundle at
+  // configure time, and this asserts the detection actually found it: the build
+  // host is the distribution the package targets, so a regression to a
+  // hardcoded value shows up here rather than in the field.
+  EXPECT_TRUE(boost::filesystem::exists(expanded)) << "ca-path does not exist: " << expanded;
 #endif
 }