Gate findings on fix/cert-write-safety. One blocker, one latent truncation
path, a false premise in a load-bearing comment, and a set of tests that were
passing without testing anything.
THE BLOCKER - scripts/test-cert-scripts.py bricked the production container
------------------------------------------------------------------------
The openssl-availability tests build a stripped PATH out of SYMLINKS to real
system binaries (cat, grep, mktemp, mv, cp, rm, mkdir, basename, dirname, find,
date, chmod). _cleanup_tmp() then walked the temp tree calling os.chmod(p,
0o600) - and os.chmod FOLLOWS SYMLINKS. Run once as root in the real image,
which is where this file ships (COPY scripts /haproxy/scripts) and where an
operator would most plausibly run it after deploying a cert fix, it stripped
the exec bit off twelve core binaries INCLUDING chmod itself, so it could not
be undone from inside the container:
/haproxy/scripts/cert-publish-lib.sh: line 109: /usr/bin/grep: Permission denied
bash: /usr/bin/chmod: Permission denied
Certificate publishing stayed dead until the container was recreated. It was
invisible on a workstation because an unprivileged chmod of a root-owned file
fails EPERM straight into `except OSError: pass` - which is also why the
advertised "32 tests pass" was only ever true off-container. In the image the
shipped file measured FAILED (failures=22, skipped=1). Cleanup now skips
symlinks; the suites are green in the image and the binaries survive.
S1 - the shell half had no same-filesystem guard
------------------------------------------------
The header claimed a cross-device mv would "FAIL LOUDLY and leave the live pem
alone". GNU mv does the opposite: across filesystems it copies, so it opens and
truncates the DESTINATION and only then discovers it cannot finish - measured
as a 204800-byte partial live.pem, sentinel gone, before mv reported ENOSPC.
cert_publish() now compares stat -Lc %d of the staging and certs dirs before
writing anything, mirroring the st_dev check the Python half already had, and
the comment says what mv actually does. Latent today (both dirs share a device
on all five hosts) but both are env-overridable.
openssl is present - correct the premise, make the check mandatory
------------------------------------------------------------------
Both halves justified a fail-open with "the image does not necessarily install
the openssl CLI". It does: openssl 3.5.6 in the running container, pulled in by
ca-certificates which certbot needs, and generate_self_signed_cert() already
shells to `openssl req` with check=True during setup. The 'unavailable' branch
never fired, so the pairing check has always run - and that, not the stated
reasoning, is what made the fail-open harmless. Structural validation alone is
weak: a bundle of EMPTY pem blocks passes every structural rule and is caught
only by openssl. The check is now mandatory in both halves and a missing binary
is a loud refusal. No `cryptography` fallback: the app runs on
/usr/local/bin/python3 (3.12) where it is not importable - it belongs to
Debian's /usr/bin/python3 - and reaching for that would be a second unverified
premise.
Smaller items
-------------
* cert_bundle_valid() read the file six times; a concurrent swap between two of
them made openssl x509 and openssl pkey judge different files and log a bogus
"private key does not match the certificate" into the monitored error log. It
now reads one snapshot and feeds openssl from it on stdin.
* except OSError -> except (OSError, UnicodeDecodeError): a BINARY-corrupt live
pem made backup_existing_pem() raise out of publish_pem_bundle() entirely, so
the republish that would have healed the host was the one thing that could
not run. The shell half recovers fine.
* stat -c %a on a symlinked live pem reports the LINK's 0777 and produced a
world-writable private key in the crt directory; now stat -Lc.
* The staging reaper's '*.??????' glob matched mktemp names but not the Python
side's '<name>.<random>.tmp', so those leaked forever. Matches both now.
* renew-certificates.sh and sync-certificates.sh exited 0 even when every
domain failed to publish, so "0 updated, 12 failed" looked identical to a
clean run to cron, to host-renew-certificates.sh (which branches on it) and
to monitoring - a host could silently stop publishing renewals until the
certificates expired. They now exit 1 if any domain failed, still after
publishing the ones that worked.
Test-quality
------------
Four TestCertPublishLibrary tests passed with cert-publish-lib.sh DELETED -
they asserted only rc != 0, and `command not found` is 127. All four now assert
the rejection REASON via assert_rejected(), and setUp() fails if the library is
missing. test_missing_openssl_still_rejects... is replaced by
test_empty_pem_blocks_are_rejected, which pins the case that makes the pairing
check necessary.
Also: the staging-containment test used startswith(certs + os.sep), so
cert_staging_dir() returning the crt directory ITSELF - the exact hazard -
still passed; test_successful_renewal_still_publishes built its "renewed" cert
with a no-op .replace() and could not tell a renewal that published nothing;
test_no_temp_file_survives_a_failed_publish failed before the staging dir
existed and asserted [] == []; FIX_ONLY was skipUnless(hasattr(hm,
'publish_pem_bundle')), so renaming that function turned 10 of 17 tests into
skips while the run still printed OK. Each is fixed and each fix is
mutation-proved: the mutation that the old assertion waved through now fails.
File mode is pinned in both suites (it was pinned nowhere), and the rename
failure is injected with a stub mv instead of chmod 0500, which root ignored -
so that test no longer skips itself precisely where it matters.
Verification
------------
IN THE BUILT IMAGE, as root (the acceptance bar):
scripts/test-cert-scripts.py 38 tests, OK, 0 skipped
scripts/test-cert-write-safety.py 22 tests, OK, 0 skipped
scripts/test-config-rollback.py 17 tests, OK (neighbour, unchanged)
Workstation: 38/38 OK for the shell suite; the Python suite needs flask.
Against the pre-fix tree (main): shell 38 failures; python failures=4, errors=1,
skipped=15 - the FIX_ONLY skips are the 14 fix-only tests plus the API guard.
The old python suite against the pre-fix tree measures skipped=10, confirming
the gate's count.
32 mutation checks, all behaving as intended: every fix breaks a test when
reverted, and every rewritten test fails under the mutation its predecessor
passed. py_compile clean, bash -n clean, shellcheck clean, no new pyflakes
warnings (same 4 pre-existing).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
102 lines
3.5 KiB
Bash
Executable File
102 lines
3.5 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
|
|
# Certificate Sync Script for HAProxy Manager
|
|
# This script syncs all Let's Encrypt certificates to HAProxy format without running certbot renew
|
|
|
|
# Configuration
|
|
LOG_FILE="${LOG_FILE:-/var/log/haproxy-manager.log}"
|
|
ERROR_LOG_FILE="${ERROR_LOG_FILE:-/var/log/haproxy-manager-errors.log}"
|
|
DB_FILE="${DB_FILE:-/etc/haproxy/haproxy_config.db}"
|
|
SSL_CERTS_DIR="${SSL_CERTS_DIR:-/etc/haproxy/certs}"
|
|
LETSENCRYPT_LIVE_DIR="${LETSENCRYPT_LIVE_DIR:-/etc/letsencrypt/live}"
|
|
|
|
# Logging functions
|
|
log_info() {
|
|
echo "[$(date '+%Y-%m-%d %H:%M:%S')] [INFO] $*" | tee -a "$LOG_FILE"
|
|
}
|
|
|
|
log_error() {
|
|
echo "[$(date '+%Y-%m-%d %H:%M:%S')] [ERROR] $*" | tee -a "$LOG_FILE" >> "$ERROR_LOG_FILE"
|
|
}
|
|
|
|
# Safe certificate publication helpers (cert_publish / cert_bundle_valid /
|
|
# haproxy_config_ok). Sourced AFTER the log_* functions above so the library
|
|
# uses this script's logging rather than its own fallbacks.
|
|
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
|
# shellcheck source=cert-publish-lib.sh
|
|
if [ -r "${SCRIPT_DIR}/cert-publish-lib.sh" ]; then
|
|
. "${SCRIPT_DIR}/cert-publish-lib.sh"
|
|
else
|
|
log_error "Missing ${SCRIPT_DIR}/cert-publish-lib.sh - refusing to touch live certificates"
|
|
exit 1
|
|
fi
|
|
|
|
log_info "Starting certificate sync process"
|
|
|
|
# Ensure SSL certs directory exists
|
|
mkdir -p "$SSL_CERTS_DIR"
|
|
|
|
# Get all SSL-enabled domains from database
|
|
DOMAINS=$(find "$LETSENCRYPT_LIVE_DIR/" -mindepth 1 -maxdepth 1 -type d -printf '%f\n')
|
|
|
|
if [ -z "$DOMAINS" ]; then
|
|
log_info "No SSL-enabled domains found"
|
|
exit 0
|
|
fi
|
|
|
|
# Copy certificates for each domain
|
|
UPDATED=0
|
|
FAILED=0
|
|
|
|
while read -r domain; do
|
|
CERT_FILE="${LETSENCRYPT_LIVE_DIR}/${domain}/fullchain.pem"
|
|
KEY_FILE="${LETSENCRYPT_LIVE_DIR}/${domain}/privkey.pem"
|
|
COMBINED_FILE="${SSL_CERTS_DIR}/${domain}.pem"
|
|
|
|
if [ -f "$CERT_FILE" ] && [ -f "$KEY_FILE" ]; then
|
|
# Assemble in a staging dir and rename into place. NEVER redirect into
|
|
# $COMBINED_FILE: the shell truncates the live pem before cat runs, and
|
|
# HAProxy loads $SSL_CERTS_DIR as a directory, so one bad file there
|
|
# takes down the whole ssl bind. See scripts/cert-publish-lib.sh.
|
|
if cert_publish "$CERT_FILE" "$KEY_FILE" "$COMBINED_FILE"; then
|
|
log_info "Updated certificate for $domain"
|
|
UPDATED=$((UPDATED + 1))
|
|
else
|
|
log_error "Failed to combine certificate for $domain"
|
|
FAILED=$((FAILED + 1))
|
|
fi
|
|
else
|
|
log_error "Certificate files not found for $domain"
|
|
FAILED=$((FAILED + 1))
|
|
fi
|
|
done <<< "$DOMAINS"
|
|
|
|
log_info "Certificate sync completed: $UPDATED updated, $FAILED failed"
|
|
|
|
# Reload HAProxy if any certificates were updated
|
|
if [ $UPDATED -gt 0 ]; then
|
|
# Never reload onto unvalidated material: a reload that fails to load the
|
|
# certs directory drops HTTPS for every site on this host.
|
|
if ! haproxy_config_ok; then
|
|
log_error "HAProxy configuration does not validate - refusing to reload after certificate sync"
|
|
exit 1
|
|
fi
|
|
|
|
if echo "reload" | socat stdio /tmp/haproxy-cli 2>/dev/null; then
|
|
log_info "HAProxy reloaded successfully"
|
|
else
|
|
log_error "Failed to reload HAProxy"
|
|
exit 1
|
|
fi
|
|
fi
|
|
|
|
# See the matching block in renew-certificates.sh: a run in which every domain
|
|
# failed to publish must not look like a clean run to its caller.
|
|
if [ "$FAILED" -gt 0 ]; then
|
|
log_error "Certificate sync process completed with failures: $UPDATED updated, $FAILED failed"
|
|
exit 1
|
|
fi
|
|
|
|
log_info "Certificate sync process completed"
|
|
exit 0
|