ci+fix: Validate RedHat/Rocky builds, and fix the CA bundle they exposed - #1405
Merged
Conversation
Every target in the CheckNet suite is a listener on loopback, and every
TLS case passes `verify=none` or an explicit `ca=`. That keeps the suite
hermetic, but it means the one thing never exercised is the *default* CA
bundle: `${ca-path}`, which CheckNet::loadModuleEx resolves once at load
and hands to check_http whenever the caller does not override it.
That default is a single hardcoded path (service/path_manager.cpp), and
on every non-Windows platform it is the Debian/Ubuntu one, so on
RHEL-family - where the bundle is /etc/pki/tls/certs/ca-bundle.crt - any
HTTPS check fails before it opens a socket:
CRITICAL: https://www.google.com -> 0 error: Failed to load CA
/etc/ssl/certs/ca-certificates.crt: load_verify_file: asio.ssl error
The integration suite already runs inside rockylinux containers
(build-redhat.yml), so a single test that leaves the default alone
catches this there while passing on Debian. check_ping gets the same
treatment: nothing in the suite has ever sent an ICMP packet off the
machine, so "the check works" was never actually asserted.
Both tests assert reachability rather than latency - thresholds are
pinned wide, so a slow or busy runner cannot turn a working check into a
red build - and they are the only tests in the file that need egress,
which the header now says.
Verified by hand before adding: check_http is OK on Ubuntu 24.04 and
CRITICAL on Rocky 9 with the message above; check_ping is OK on both;
both are OK on Windows, where ${ca-path} points at the ROOT-store export
and is regenerated on demand when missing.
Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Michael Medin <michael@medin.name>
The first cut put both tests in the jest suite, on the assumption that it runs everywhere. It does not: the jest steps in integration-tests-linux.yml are gated `if: inputs.package-type == 'deb'`, so the RPM jobs never ran them - and RHEL-family is the only place the hardcoded Debian CA path is wrong. The run proved it: check_http passed on Ubuntu and Windows, and the RedHat jobs reported green without executing either test. Move the trust-store assertion into tests/acceptance-tests.sh, which runs for both deb and rpm, so the RPM jobs fail on the bug they actually have. It greps for an OK, so the failure carries the "Failed to load CA /etc/ssl/certs/ca-certificates.crt" line with it. check_ping becomes opt-in behind NSCP_EXTERNAL_ICMP=1. GitHub-hosted runners drop outbound ICMP, so it came back CRITICAL with 100% loss on Ubuntu and Windows alike, while the identical query is OK from an Azure VM on both distros and from a Windows desktop. Deleting it would lose the only test that proves check_ping puts a packet on the wire; skipping it by default keeps CI honest without pretending the check is untested. It also now asserts on the message, so a failure says which of the two it was rather than only "expected 0, received 2". Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Michael Medin <michael@medin.name>
The jest harness has been gated `if: inputs.package-type == 'deb'` since b83d6bf, from when it still needed a docker daemon. NSCP_SKIP_DOCKER=1 removed that need - it is how the deb job runs today - but the condition stayed, so the rpm packages were covered by acceptance-tests.sh alone and anything that behaves differently on RHEL went untested. The RPM base dependencies already install Node 20 from nodesource, so nothing else was missing. Turning it on immediately finds a real bug. ${ca-path} is a single hardcoded path (service/path_manager.cpp) and it is the Debian one on every non-Windows platform, so on RHEL-family it names a file that is not there. make_context loads the CA whenever `ca` is non-empty, before verify mode is considered, so this is not limited to public hosts: `verify=none` against a local self-signed server fails too. Verified by running the whole RPM job locally in a rockylinux:10 container (the CI image), against the rocky-10 artifact from the last build: Test Suites: 1 failed, 20 skipped, 32 passed, 33 of 53 total Tests: 5 failed, 148 skipped, 433 passed, 586 total The 20 skips are the docker-gated scenarios, as on deb. All five failures are in checknet-commands and all five are the same missing bundle - four of them pre-existing tests that have never run on RHEL: ssl_expiry_days, redirect-lands-on-plain-http, check_nsclient_web_online x2, and the new public-HTTPS test CRITICAL: https://www.google.com -> 0 error: Failed to load CA /etc/ssl/certs/ca-certificates.crt: load_verify_file: No such file OK: https://www.google.com -> 200 ok (with ca=/etc/pki/tls/certs/ca-bundle.crt) The trust-store assertion therefore lives in the jest suite rather than in acceptance-tests.sh, since the suite now runs where it matters. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Michael Medin <michael@medin.name>
mickem
changed the title
${ca-path} was one hardcoded path for every non-Windows platform,
/etc/ssl/certs/ca-certificates.crt, which is the Debian/Ubuntu location.
On RHEL-family the bundle is /etc/pki/tls/certs/ca-bundle.crt, on SUSE
/etc/ssl/ca-bundle.pem, so the token named a file that is not there and
every TLS check that did not carry its own ca= failed with
Failed to load CA /etc/ssl/certs/ca-certificates.crt: No such file
That is not limited to public hosts: make_context loads the CA whenever
`ca` is non-empty, before the verify mode is considered, so a check
against a local self-signed server with verify=none failed too. It also
took nscp enroll with it, since 0.16.0 defaults --ca to this token - so
fleet enrollment was impossible on RHEL without naming a bundle by hand.
Detect it at configure time instead. A package is built in a container of
the distribution it targets, so the build host has the right answer, and
a packager who knows better can override with -DCONFIG_CA_PATH=... The
value now reaches path_manager through config.h like the other path
defaults, which also removes the WIN32/else pair for this key.
The unit test now asserts the bundle exists rather than only that it sits
under /etc/, so a regression to a hardcoded value fails on the build host
instead of in the field.
Verified in a rockylinux:10 container (the CI image): detection resolves
to /etc/pki/tls/certs/ca-bundle.crt there and to
/etc/ssl/certs/ca-certificates.crt on ubuntu:24.04, -DCONFIG_CA_PATH wins
over both, and the generated config.h carries the value. Configuring the
real project with and without this patch produces the same (pre-existing,
invocation-specific) CMake error, so it adds none. Pointing ca-path at the
real bundle takes checknet-commands on Rocky from 5 failed / 48 passed to
53 passed.
Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Michael Medin <michael@medin.name>
mickem
changed the title
The codespell workflow runs with only_warn, so its ten annotations ride along on every pull request without ever failing a build - and stay there. Clear them: enviornment, dependecies, migh, stil, configurtion, lik, iniating and the `contect` parameter of switch_context (renamed in both the declaration and the definition). Two the dictionary does not know, fixed while here: "securoty (certificates) folder" in both config.h generators, and the matching "uniniating COM" next to the "iniating COM" codespell did flag. Comments, cache doc strings and one log message only - no behaviour change. Signed-off-by: Michael Medin <michael@medin.name> Assisted-by: Claude Code:claude-opus-5
It was there to work out whether check_ping actually puts a packet on
the wire when GitHub-hosted runners report 100% loss; that question is
answered, and an opt-in test nothing sets the variable for is dead
weight. The check_http case stays - it is what exercises the default
${ca-path} bundle this branch fixes.
Signed-off-by: Michael Medin <michael@medin.name>
Assisted-by: Claude Code:claude-opus-5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The jest integration harness only ever ran on Debian builds. This turns it on for the RPM jobs too — and fixes the bug that immediately surfaced.
Why RHEL was untested
The harness has been gated
if: inputs.package-type == 'deb'sinceb83d6bf7, from when it still needed a docker daemon.NSCP_SKIP_DOCKER=1removed that need — it is how the deb job runs today — but the condition stayed. RPM packages were covered byacceptance-tests.shalone, so anything that behaves differently on RHEL went untested. Nothing else was missing: the RPM base dependencies already install Node 20 from nodesource.What it caught
${ca-path}was one hardcoded path for every non-Windows platform (service/path_manager.cpp):{"ca-path", "/etc/ssl/certs/ca-certificates.crt"},That is the Debian/Ubuntu location. On RHEL-family the bundle is
/etc/pki/tls/certs/ca-bundle.crt, on SUSE/etc/ssl/ca-bundle.pem— so the token named a file that does not exist, and every TLS check that did not carry its ownca=failed.It was never only a public-internet problem:
make_contextloads the CA whenevercais non-empty, before the verify mode is considered, so a check against a local self-signed server withverify=nonefailed too. Turning the suite on for RPM failed five tests, four of them pre-existing tests that had simply never run on RHEL:check_http over TLS exposes the certificate expiry via ssl_expiry_dayscheck_http reports no certificate when a redirect lands on plain httpcheck_nsclient_web_online reports a reachable REST APIcheck_nsclient_web_online passes a remote check result throughcheck_http validates a public HTTPS site using the platform CA bundleIt also took
nscp enrollwith it: 0.16.0 defaults--cato the same token, so fleet enrollment was impossible on RHEL-family without naming a bundle by hand.The fix
Detect the bundle at configure time and pass it through
config.hlike the other path defaults, rather than hardcoding one per#ifdef. A package is built in a container of the distribution it targets, so the build host has the right answer; a packager who knows better can override with-DCONFIG_CA_PATH=…. This also removes theWIN32/elsepair for this key.path_manager_test.cppnow asserts the bundle exists, not merely that it lives under/etc/— so a regression to a hardcoded value fails on the build host instead of in the field. That is the hermetic regression test that was missing all along.Verified locally, in the CI image
Everything below was run in
rockylinux/rockylinux:10rather than by pushing and waiting:/etc/pki/tls/certs/ca-bundle.crton Rocky 10 and/etc/ssl/certs/ca-certificates.crtonubuntu:24.04;-DCONFIG_CA_PATH=/opt/custom/bundle.pemoverrides both; the generatedconfig.hcarries the value.file COPY cannot find …/check_nsclient, an artefact of the local invocation), so the change adds none.ca-pathresolving to a real bundle,checknet-commandson Rocky goes from5 failed, 48 passedto53 passed, 0 failed.Test Suites: 32 passed,Tests: 433 passedbefore the fix, with the 20 docker-gated suites skipping exactly as on deb. The existing python acceptance tests pass on Rocky (OK: 14 test(s) successfull).Also added
check_pingagainst a public host, opt-in behindNSCP_EXTERNAL_ICMP=1. Nothing in the suite has ever sent an ICMP packet off the machine. It is opt-in because GitHub-hosted runners drop outbound ICMP — it returns CRITICAL with 100% loss on the Ubuntu containers and the Windows runners alike, while the identical query is OK from an Azure VM on both distros and from a Windows desktop.Worth doing separately
build-redhat.yml:177-182), so only Rocky 10 runs this. The bug reproduced on both.include/net/http/client.hpptreats an emptycaas "trust nothing";set_default_verify_paths()there would be a sensible second line of defence, and the three call sites that load a CA (client.hppthrows,check_tcp.cpp:233returns a string,socket_helpers.cpp:230collects and continues) could converge.🤖 Generated with Claude Code