diff --git a/VERSION b/VERSION index 0657c95..8ac114e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2026.07.3 +2026.08.1 diff --git a/haproxy_manager.py b/haproxy_manager.py index a70f812..b8b6a0b 100644 --- a/haproxy_manager.py +++ b/haproxy_manager.py @@ -12,6 +12,7 @@ from datetime import datetime, timedelta import json import ipaddress import shutil +import stat import tempfile import threading import time @@ -105,7 +106,19 @@ HAPROXY_CONFIG_PATH = '/etc/haproxy/haproxy.cfg' HAPROXY_BACKUP_PATH = '/etc/haproxy/haproxy.cfg.backup' BLOCKED_IPS_MAP_PATH = '/etc/haproxy/blocked_ips.map' BLOCKED_IPS_MAP_BACKUP_PATH = '/etc/haproxy/blocked_ips.map.backup' +# Coraza SPOE engine file. `haproxy -c` parses this too (the frontend's +# `filter spoe engine coraza config ` line points at it), so it is part +# of the same restorable config set as haproxy.cfg — rolling back haproxy.cfg +# while leaving a broken coraza-spoe.cfg behind still fails validation. +CORAZA_SPOE_CONFIG_PATH = '/etc/haproxy/coraza-spoe.cfg' +CORAZA_SPOE_BACKUP_PATH = '/etc/haproxy/coraza-spoe.cfg.backup' HAPROXY_SOCKET_PATH = '/var/run/haproxy.sock' +# HAProxy loads this path as a DIRECTORY (`bind ... ssl crt /etc/haproxy/certs`), +# which means it tries to load EVERY file it finds in here. Nothing but final, +# validated `.pem` bundles may ever exist in this directory - no temp +# files, no `.backup` copies. Staging and backups live in the sibling +# directories below, on the same filesystem so os.replace() stays atomic. +# See publish_pem_bundle(). SSL_CERTS_DIR = '/etc/haproxy/certs' # Stable per-host secret for QUIC Retry/address-validation tokens. Lives in the # /etc/haproxy named volume so it survives container recreates; self-healed on @@ -302,6 +315,340 @@ def find_certbot_live_dir(base_domain): candidates.sort(key=lambda x: x[1], reverse=True) return candidates[0][0] +# --------------------------------------------------------------------------- +# Certificate publishing +# --------------------------------------------------------------------------- +# Until 2026-08 every code path that refreshed a combined PEM did this: +# +# with open(combined_path, 'w') as combined: # TRUNCATES the file +# subprocess.run(['cat', cert, key], stdout=combined) # rc ignored +# +# `combined_path` is the live bundle HAProxy is serving. open(...,'w') empties +# it BEFORE a single byte of source material has been read, and the `cat` exit +# status was never checked. Any failure in between - source unreadable, disk +# full, container killed, certbot lineage half-written - left a truncated or +# key-less PEM in place. HAProxy loads /etc/haproxy/certs as a directory, so one +# unusable file there fails the whole `bind ... ssl crt` and takes down HTTPS +# for every site on the host. Unlike a bad haproxy.cfg this is NOT recoverable +# by config rollback, and re-issuing hits Let's Encrypt rate limits. +# +# Everything below exists to make publishing a bundle all-or-nothing: +# assemble in a staging dir -> validate -> back up the old one -> os.replace() +# The live file is only ever swapped for a complete, validated replacement. + + +class CertificatePublishError(Exception): + """A certificate bundle could not be published. The live PEM is untouched.""" + + +def _cert_sibling_dir(name): + """A directory next to SSL_CERTS_DIR (NOT inside it). + + HAProxy loads SSL_CERTS_DIR as a crt directory and tries to load every file + in it, so staging and backup copies must live outside. Siblings share the + /etc/haproxy filesystem, which is what keeps os.replace() atomic. + + Derived at call time so tests that repoint SSL_CERTS_DIR get matching + staging/backup dirs, the same pattern as _config_backup_pairs(). + """ + parent = os.path.dirname(SSL_CERTS_DIR.rstrip('/')) or '/' + return os.path.join(parent, name) + + +def cert_staging_dir(): + """Directory where bundles are assembled and validated before publishing.""" + return _cert_sibling_dir('cert-staging') + + +def cert_backup_dir(): + """Directory holding the previous copy of each published bundle. + + Mirrors the config backup on the parent branch (one `.backup` alongside + haproxy.cfg): one copy per cert, overwritten on each successful publish, so + an operator always has a manual path back to the bundle that was being + served before the last change. + """ + return _cert_sibling_dir('cert-backups') + + +# ENCRYPTED PRIVATE KEY is deliberately absent: HAProxy cannot use a +# passphrase-protected key from a crt file, so a bundle containing one is not +# publishable, and the structural layer is the only layer that names the +# problem ("no complete private key block") rather than reporting it as an +# unreadable key. +_PEM_KEY_LABELS = ('PRIVATE KEY', 'RSA PRIVATE KEY', 'EC PRIVATE KEY') + + +def _pem_labels(text): + """Labels of well-formed PEM blocks in text, in order. + + A block counts only if its BEGIN line is followed by the matching END line, + so a bundle truncated in the middle of a block yields no label for it - + which is exactly the corruption we are guarding against. + """ + labels = [] + open_label = None + for line in text.splitlines(): + line = line.strip() + if line.startswith('-----BEGIN ') and line.endswith('-----'): + open_label = line[len('-----BEGIN '):-len('-----')].strip() + elif line.startswith('-----END ') and line.endswith('-----'): + end_label = line[len('-----END '):-len('-----')].strip() + if open_label is not None and end_label == open_label: + labels.append(end_label) + open_label = None + return labels + + +def validate_pem_structure(text): + """Structural check on an assembled bundle. Returns (ok, message). + + Pure Python and therefore ALWAYS available - it can never be skipped for + lack of a tool. It catches every failure mode the truncation bug produced: + empty file, certificate without a key, key without a certificate, and a + block cut off mid-write. + + It is NOT sufficient on its own, which is why _openssl_pairing_status() is + mandatory rather than best-effort: a bundle of EMPTY pem blocks (a BEGIN + line immediately followed by its END line, no base64 between them) passes + every check here and is rejected only by openssl. + """ + if not text.strip(): + return False, 'bundle is empty' + labels = _pem_labels(text) + if not labels: + return False, 'bundle contains no complete PEM block (truncated?)' + if 'CERTIFICATE' not in labels: + return False, 'bundle contains no complete CERTIFICATE block' + if not any(label in _PEM_KEY_LABELS for label in labels): + return False, 'bundle contains no complete private key block' + return True, None + + +def _openssl_pairing_status(path): + """Does the private key in `path` match the leaf certificate in `path`? + + Returns (status, message) with status 'valid' | 'invalid' | 'unavailable'. + + `openssl x509` reads the FIRST certificate in the file (our bundles are + fullchain-then-key, so that is the leaf) and `openssl pkey` scans past the + certificate blocks to the first private key, so both run against the + assembled bundle directly. Comparing the two public keys proves the pair. + + 'unavailable' means the openssl BINARY is absent - a verdict about our + tooling, not about the bundle. validate_pem_bundle() treats it as a HARD + FAILURE. + + That is a deliberate reversal. This docstring used to say "the Dockerfile + installs haproxy, certbot, socat and curl but not the openssl CLI, so this + is a real possibility", and callers accepted the bundle on the structural + checks alone. The premise is false: openssl 3.x IS in the image, as a + dependency of ca-certificates (which certbot requires), and + generate_self_signed_cert() below already runs `openssl req` with + check=True during first-run setup - so no container has ever reached a + publish without it. The 'unavailable' branch never fired, which means the + pairing check has in fact always run, and THAT is what made the fail-open + harmless - not the stated reasoning. Structural validation on its own is + weak: a bundle of empty pem blocks passes validate_pem_structure() and is + caught only here. + + So an absent openssl now means the image is broken, and we say so and stop + instead of quietly downgrading to the weaker check. The cost is that a + hypothetical openssl-less image stops publishing renewals - but it does so + immediately and loudly, in the monitored error log, at the first renewal, + rather than 90 days later; and publishing an unverified bundle can take the + whole :443 bind, i.e. every site on the host, down at the next reload. + + There is no python `cryptography` fallback on purpose: this process runs on + /usr/local/bin/python3 (the base image's 3.12), where cryptography is not + importable. It is installed for Debian's /usr/bin/python3 as a certbot + dependency, and reaching for that interpreter would be a second unverified + premise of exactly the kind this comment is correcting. + """ + try: + cert_pub = subprocess.run( + ['openssl', 'x509', '-in', path, '-noout', '-pubkey'], + capture_output=True, text=True, stdin=subprocess.DEVNULL) + except FileNotFoundError: + return 'unavailable', 'openssl binary not found' + except Exception as e: + return 'unavailable', f'could not run openssl: {e}' + if cert_pub.returncode != 0: + return 'invalid', ('leaf certificate is unreadable: ' + f'{(cert_pub.stderr or "").strip()[:200]}') + try: + # -passin pass: plus a closed stdin so an (unexpected) encrypted key + # fails fast instead of blocking on a passphrase prompt. + key_pub = subprocess.run( + ['openssl', 'pkey', '-in', path, '-pubout', '-passin', 'pass:'], + capture_output=True, text=True, stdin=subprocess.DEVNULL) + except FileNotFoundError: + return 'unavailable', 'openssl binary not found' + except Exception as e: + return 'unavailable', f'could not run openssl: {e}' + if key_pub.returncode != 0: + return 'invalid', ('private key is unreadable: ' + f'{(key_pub.stderr or "").strip()[:200]}') + if cert_pub.stdout.strip() != key_pub.stdout.strip(): + return 'invalid', 'private key does not match the leaf certificate' + return 'valid', None + + +def validate_pem_bundle(path): + """Validate a bundle file on disk. Returns (ok, message). + + Structural validation AND the cryptographic pairing check, both mandatory - + see _openssl_pairing_status() for why a missing openssl is a failure rather + than a downgrade to structure-only. + """ + try: + with open(path, 'r') as fh: + text = fh.read() + except (OSError, UnicodeDecodeError) as e: + # UnicodeDecodeError, not just OSError: a bundle corrupted into binary + # is unreadable as text but reads perfectly well as bytes, so `except + # OSError` let the decode error escape as an unhandled traceback. + return False, f'cannot read assembled bundle: {e}' + + ok, msg = validate_pem_structure(text) + if not ok: + return False, msg + + status, pair_msg = _openssl_pairing_status(path) + if status == 'invalid': + return False, pair_msg + if status == 'unavailable': + logger.error( + "Certificate key/leaf pairing check could not run for %s (%s) - " + "REFUSING to publish. openssl is required; structural validation " + "alone cannot tell a real bundle from empty pem blocks.", + path, pair_msg) + return False, (f'cert/key pairing check unavailable ({pair_msg}); ' + 'refusing to publish on structural checks alone') + return True, None + + +def backup_existing_pem(dest_path): + """Copy the currently published bundle aside before it is replaced. + + Only a bundle that still validates is promoted to backup: overwriting a + good backup with an already-corrupt live file would turn "restore the + backup" into "restore different garbage". Same require_valid reasoning as + create_backup() for haproxy.cfg. + + Never fatal - failing to take a backup must not stop us replacing a cert + with a validated one - but always logged. + """ + if not os.path.exists(dest_path): + return None + try: + with open(dest_path, 'r') as fh: + ok, msg = validate_pem_structure(fh.read()) + except (OSError, UnicodeDecodeError) as e: + # UnicodeDecodeError, not just OSError. A live pem corrupted into + # BINARY (the exact state a republish is meant to heal) raises + # UnicodeDecodeError here, and with only OSError caught it escaped all + # the way out of publish_pem_bundle() - so the one operation that would + # have put a working certificate back blew up on the way to taking a + # backup of the broken one. The shell half recovers from this fine; + # this is the only reason the Python half did not. + ok, msg = False, str(e) + backup_path = os.path.join(cert_backup_dir(), os.path.basename(dest_path)) + if not ok: + logger.warning( + "Not backing up %s before replacing it: the file on disk is not a " + "valid bundle (%s). Keeping any previous backup at %s.", + dest_path, msg, backup_path) + return None + try: + os.makedirs(cert_backup_dir(), exist_ok=True) + shutil.copy2(dest_path, backup_path) + return backup_path + except Exception as e: + logger.error("Failed to back up %s to %s: %s", + dest_path, backup_path, e) + return None + + +def publish_pem_bundle(dest_path, source_paths): + """Publish cert+key as a combined PEM at dest_path, atomically. + + source_paths are concatenated in order (fullchain first, then privkey) into + a staging file OUTSIDE the crt directory, validated there, and only then + moved into place with os.replace(). If anything fails, the exception is + raised and dest_path still holds the previous, working bundle - the live + PEM is never opened for writing at any point. + + Returns the path of the backup taken (or None). Raises + CertificatePublishError on any failure. + """ + for src in source_paths: + if not os.path.exists(src): + raise CertificatePublishError( + f'source certificate material missing: {src}') + + parts = [] + for src in source_paths: + try: + with open(src, 'r') as fh: + data = fh.read() + except (OSError, UnicodeDecodeError) as e: + # Binary-corrupt source material must be a clean, reported refusal, + # not an unhandled UnicodeDecodeError out of the request handler. + raise CertificatePublishError(f'cannot read {src}: {e}') + if not data.strip(): + raise CertificatePublishError(f'source file is empty: {src}') + if not data.endswith('\n'): + # Guard against a cert whose last line runs into the key's BEGIN + # line; certbot always ends with a newline, but a hand-placed file + # might not. + data += '\n' + parts.append(data) + content = ''.join(parts) + + ok, msg = validate_pem_structure(content) + if not ok: + raise CertificatePublishError( + f'assembled bundle for {dest_path} is not usable: {msg}') + + dest_dir = os.path.dirname(dest_path) or '.' + try: + os.makedirs(dest_dir, exist_ok=True) + os.makedirs(cert_staging_dir(), exist_ok=True) + except OSError as e: + raise CertificatePublishError(f'cannot prepare directories: {e}') + + # os.replace() is only atomic within one filesystem. Checking up front + # turns an EXDEV rename failure into an operator-readable message; the + # outcome is the same either way (nothing published, live PEM untouched) + # because staging inside the crt directory is not an acceptable fallback. + try: + if os.stat(cert_staging_dir()).st_dev != os.stat(dest_dir).st_dev: + raise CertificatePublishError( + f'{cert_staging_dir()} and {dest_dir} are on different ' + 'filesystems, so a certificate cannot be swapped in atomically') + except OSError as e: + raise CertificatePublishError(f'cannot stat certificate dirs: {e}') + + backup_path = backup_existing_pem(dest_path) + + def _validate_staged(staged_path): + return validate_pem_bundle(staged_path) + + try: + write_config_atomically(dest_path, content, + staging_dir=cert_staging_dir(), + validate=_validate_staged) + except Exception as e: + raise CertificatePublishError( + f'refused to publish {dest_path}: {e}. The previously served ' + 'certificate is still in place.') + logger.info("Published validated certificate bundle to %s%s", dest_path, + f' (previous copy backed up to {backup_path})' + if backup_path else '') + return backup_path + + def certbot_register(): """Register with Let's Encrypt using the certbot client and agree to the terms of service""" result = subprocess.run(['certbot', 'show_account'], capture_output=True) @@ -330,12 +677,26 @@ def generate_self_signed_cert(ssl_certs_dir): '-subj', f'/CN={DOMAIN}' ], check=True) - # Combine cert and key for HAProxy - with open(self_sign_cert, 'wb') as combined: + # Combine cert and key for HAProxy. Same publisher as every other bundle: + # this file lands in the crt directory, so a half-written one would break + # the TLS bind for every site on the host, and because the function + # short-circuits on "file exists" a corrupt one would never be regenerated. + try: + publish_pem_bundle(self_sign_cert, ['/tmp/cert.pem', '/tmp/key.pem']) + except CertificatePublishError as e: + # do_initial_setup() calls this unguarded, so raising here would abort + # container startup before HAProxy is ever launched. Refusing to write + # an unusable default cert is right; taking the whole container down + # over it is not - start_haproxy() already degrades gracefully. + logger.critical("Could not publish the default self-signed certificate " + "to %s: %s", self_sign_cert, e) + return False + finally: for file in ['/tmp/cert.pem', '/tmp/key.pem']: - with open(file, 'rb') as f: - combined.write(f.read()) - os.remove(file) # Clean up temporary files + try: + os.remove(file) # Clean up temporary files + except OSError: + pass generate_config() return True @@ -574,8 +935,15 @@ def request_ssl(): # Ensure SSL certs directory exists os.makedirs(SSL_CERTS_DIR, exist_ok=True) - with open(combined_path, 'w') as combined: - subprocess.run(['cat', cert_path, key_path], stdout=combined) + try: + publish_pem_bundle(combined_path, [cert_path, key_path]) + except CertificatePublishError as e: + # Nothing was written: any previously served bundle for this + # name is untouched and HAProxy is not reloaded. + error_msg = f'Certificate issued but not published: {e}' + logger.critical(error_msg) + log_operation('request_ssl', False, error_msg) + return jsonify({'status': 'error', 'message': error_msg}), 500 # Update database with sqlite3.connect(DB_FILE) as conn: @@ -604,8 +972,8 @@ def request_ssl(): log_operation('request_ssl', False, str(e)) return jsonify({'status': 'error', 'message': str(e)}), 500 -def _cleanup_superseded_lineages(keep_path, keep_lineage, bundle_names): - """Remove cert files + certbot lineages that the just-issued bundle supersedes. +def _quarantine_superseded_certs(keep_path, keep_lineage, bundle_names): + """Move aside cert files that the just-issued bundle supersedes. A `.pem` in /etc/haproxy/certs/ is "superseded" iff its certificate's CN is one of the bundle's names AND the file isn't the bundle's own combined @@ -613,10 +981,24 @@ def _cleanup_superseded_lineages(keep_path, keep_lineage, bundle_names): since that's what HAProxy SNI-matches against and what the file convention names it after. - Also drops the corresponding certbot renewal config so `certbot renew` - stops trying to renew the dead lineage on its next 12h cron tick. + This used to os.remove() the file and immediately `certbot delete` the + lineage, both BEFORE anything had checked that the replacement bundle was + usable — destroying the only two copies of a working certificate on the + strength of a `cat` whose exit status nobody read. Now: - Returns a small summary dict for logging / API response. + * the superseded file is MOVED to the cert backup directory instead of + unlinked, so an operator can put it back by hand; + * the certbot lineage is left alone here. Deleting it is irreversible + (archive, live and renewal config all go) and recovering means fresh, + rate-limited ACME orders, so it happens only in + _delete_superseded_lineages(), after HAProxy has actually loaded the + replacement. + + Moving still solves the problem the removal existed for: the old file no + longer sits in the crt directory shadowing the new bundle's SNI match. + + Returns a summary dict for logging / API response. Entries in 'removed' + carry the lineage name for the deletion phase. """ summary = {'removed': [], 'errors': [], 'skipped': []} @@ -668,26 +1050,54 @@ def _cleanup_superseded_lineages(keep_path, keep_lineage, bundle_names): continue try: - os.remove(fpath) - removed_entry = {'file': fname, 'cn': cn, 'lineage_deleted': False} - # Best-effort certbot lineage delete. Some files may not have a - # corresponding lineage (e.g. self-signed dev certs); ignore those. - try: - cb_proc = subprocess.run( - ['certbot', 'delete', '--cert-name', lineage_name, '-n'], - capture_output=True, text=True - ) - removed_entry['lineage_deleted'] = (cb_proc.returncode == 0) - if cb_proc.returncode != 0: - removed_entry['certbot_stderr'] = (cb_proc.stderr or '').strip()[:200] - except Exception as e: - removed_entry['certbot_error'] = str(e) - summary['removed'].append(removed_entry) + os.makedirs(cert_backup_dir(), exist_ok=True) + quarantine_path = os.path.join(cert_backup_dir(), fname) + # Move, not unlink: out of the crt directory (so it stops shadowing + # the new bundle) but still on disk for manual recovery. + shutil.move(fpath, quarantine_path) + summary['removed'].append({ + 'file': fname, + 'cn': cn, + 'lineage': lineage_name, + 'moved_to': quarantine_path, + 'lineage_deleted': False, + }) except Exception as e: summary['errors'].append({'file': fname, 'error': str(e)}) return summary + +def _delete_superseded_lineages(summary): + """`certbot delete` the lineages quarantined by _quarantine_superseded_certs(). + + IRREVERSIBLE: certbot removes the lineage's archive, live symlinks and + renewal config. If it turns out we needed that certificate, the only way + back is a fresh ACME order, which Let's Encrypt rate-limits — an outage + measured in hours. So this is gated hardest of anything in this module: it + runs only after the replacement bundle has been assembled, validated, + published, AND loaded by a HAProxy that reloaded successfully. + + Mutates `summary` in place. Best-effort: some files have no corresponding + lineage (e.g. self-signed dev certs) and a failure here is harmless — a + dead lineage merely wastes a renewal attempt on the next 12h cron tick. + """ + for entry in summary.get('removed', []): + lineage_name = entry.get('lineage') + if not lineage_name: + continue + try: + cb_proc = subprocess.run( + ['certbot', 'delete', '--cert-name', lineage_name, '-n'], + capture_output=True, text=True + ) + entry['lineage_deleted'] = (cb_proc.returncode == 0) + if cb_proc.returncode != 0: + entry['certbot_stderr'] = (cb_proc.stderr or '').strip()[:200] + except Exception as e: + entry['certbot_error'] = str(e) + return summary + @app.route('/api/ssl/bundle', methods=['POST']) @require_api_key def request_ssl_bundle(): @@ -787,8 +1197,17 @@ def request_ssl_bundle(): combined_path = f'{SSL_CERTS_DIR}/{primary}.pem' os.makedirs(SSL_CERTS_DIR, exist_ok=True) - with open(combined_path, 'w') as combined: - subprocess.run(['cat', cert_path, key_path], stdout=combined) + try: + publish_pem_bundle(combined_path, [cert_path, key_path]) + except CertificatePublishError as e: + # Publishing is all-or-nothing, so at this point nothing has been + # written, no old certificate has been touched and no lineage has + # been deleted. Stop before any of that becomes untrue. + error_msg = f'Bundle issued but not published for {primary}: {e}' + logger.critical(error_msg) + log_operation('request_ssl_bundle', False, error_msg) + return jsonify({'status': 'error', 'message': error_msg, + 'primary': primary, 'names': names}), 500 # Mark every name in the bundle as ssl_enabled, all pointing at the # same combined .pem. HAProxy serves one file for many SNI hostnames. @@ -809,15 +1228,35 @@ def request_ssl_bundle(): # `bind ... ssl crt /etc/haproxy/certs` directive. HAProxy then picks # one of them by alphabetical/load order — frequently the older # single-SAN file — and the new bundle has no effect on what's served. - # This block deletes those superseded files (and their certbot lineage) + # This block moves those superseded files out of the crt directory # before the generate_config() reload so HAProxy picks up the bundle. - cleanup_summary = _cleanup_superseded_lineages( + # It only runs once publish_pem_bundle() above has validated the + # replacement and put it in place, so the old certificate is never the + # only copy we have. + cleanup_summary = _quarantine_superseded_certs( keep_path=combined_path, keep_lineage=primary, bundle_names=set(names), ) - generate_config() + # Raises if the config does not validate or HAProxy does not reload; + # the certbot lineages below are therefore only deleted once the new + # bundle is genuinely being served. + try: + generate_config() + except Exception: + if cleanup_summary['removed']: + logger.critical( + "HAProxy did not reload after publishing the bundle for " + "%s. The superseded certificate files were moved to %s and " + "their certbot lineages were NOT deleted, so they can be " + "restored by hand: %s", + primary, cert_backup_dir(), + [e['file'] for e in cleanup_summary['removed']]) + raise + + _delete_superseded_lineages(cleanup_summary) + log_operation( 'request_ssl_bundle', True, f'SSL bundle issued for {primary} covering {len(names)} names; ' @@ -854,11 +1293,12 @@ def renew_certificates(): # Check if any certificates were renewed if 'Congratulations' in result.stdout or 'renewed' in result.stdout: # Update combined certificates for HAProxy + publish_failures = [] with sqlite3.connect(DB_FILE) as conn: cursor = conn.cursor() cursor.execute('SELECT domain, ssl_cert_path FROM domains WHERE ssl_enabled = 1') domains = cursor.fetchall() - + for domain, cert_path in domains: if cert_path and os.path.exists(cert_path): # For wildcard domains, strip *. prefix for directory lookup @@ -869,15 +1309,40 @@ def renew_certificates(): letsencrypt_key = os.path.join(live_dir, 'privkey.pem') if os.path.exists(letsencrypt_cert) and os.path.exists(letsencrypt_key): - with open(cert_path, 'w') as combined: - subprocess.run(['cat', letsencrypt_cert, letsencrypt_key], stdout=combined) - - # Regenerate config and reload HAProxy + # A failure here leaves the currently + # served bundle in place. That certificate + # is still valid (renewal runs ~30 days + # before expiry and retries every 12h), so + # keeping it is strictly better than + # replacing it with something unverified. + try: + publish_pem_bundle( + cert_path, + [letsencrypt_cert, letsencrypt_key]) + except CertificatePublishError as e: + logger.critical( + "Renewed certificate for %s was NOT " + "published: %s", domain, e) + publish_failures.append( + {'domain': domain, 'error': str(e)}) + + # Regenerate config and reload HAProxy. Safe to do with + # publish failures present: those certificates were left + # untouched, so nothing unvalidated is being loaded. generate_config() reload_result = subprocess.run('echo "reload" | socat stdio /tmp/haproxy-cli', capture_output=True, text=True, shell=True) if reload_result.returncode == 0: + if publish_failures: + error_msg = ( + f'{len(publish_failures)} renewed certificate(s) could ' + 'not be published and are still being served from ' + 'their previous bundle') + log_operation('renew_certificates', False, error_msg) + return jsonify({'status': 'partial_success', + 'message': error_msg, + 'failures': publish_failures}), 500 log_operation('renew_certificates', True, 'Certificates renewed and HAProxy reloaded') return jsonify({'status': 'success', 'message': 'Certificates renewed and HAProxy reloaded'}) else: @@ -1070,9 +1535,19 @@ def request_certificates(): # Ensure SSL certs directory exists os.makedirs(SSL_CERTS_DIR, exist_ok=True) - with open(combined_path, 'w') as combined: - subprocess.run(['cat', cert_path, key_path], stdout=combined) - + try: + publish_pem_bundle(combined_path, [cert_path, key_path]) + except CertificatePublishError as e: + error_msg = f'Certificate issued but not published: {e}' + logger.critical('%s (%s)', error_msg, domain) + results.append({ + 'domain': domain, + 'status': 'error', + 'message': error_msg, + }) + error_count += 1 + continue + # Update database (add domain if it doesn't exist) with sqlite3.connect(DB_FILE) as conn: cursor = conn.cursor() @@ -1693,11 +2168,13 @@ def dns_challenge_verify(): os.makedirs(SSL_CERTS_DIR, exist_ok=True) combined_path = f'{SSL_CERTS_DIR}/_wildcard_.{base_domain}.pem' - with open(combined_path, 'w') as combined: - with open(cert_path, 'r') as cf: - combined.write(cf.read()) - with open(key_path, 'r') as kf: - combined.write(kf.read()) + try: + publish_pem_bundle(combined_path, [cert_path, key_path]) + except CertificatePublishError as e: + error_msg = f'Wildcard certificate obtained but not published: {e}' + logger.critical(error_msg) + log_operation('dns_challenge_verify', False, error_msg) + return jsonify({'success': False, 'error': error_msg}), 500 # Update database with sqlite3.connect(DB_FILE) as conn: @@ -1743,6 +2220,40 @@ def get_or_create_cluster_secret(): secret = f.read().strip() if secret: return secret + # File exists but is blank — e.g. a create that died between + # open() and write(), or a volume restored empty. Without healing + # it here we fall through to the O_EXCL create below, which fails + # with FileExistsError, and the handler re-reads the same blank + # file: this function would return '' forever and the host would + # never get the stable secret its docstring promises. + # + # Rewrite in place under an exclusive lock rather than unlinking + # and recreating: the file is never momentarily absent, and two + # workers healing at once serialise instead of racing to install + # two different secrets. + try: + fd = os.open(CLUSTER_SECRET_PATH, os.O_RDWR) + try: + fcntl.flock(fd, fcntl.LOCK_EX) + # Re-read under the lock: another worker may have healed it + # while we were waiting. + existing = os.read(fd, 4096).decode(errors='replace').strip() + if existing: + return existing + secret = os.urandom(32).hex() + os.ftruncate(fd, 0) + os.lseek(fd, 0, os.SEEK_SET) + os.write(fd, secret.encode()) + os.fchmod(fd, 0o600) + logger.warning( + "Healed empty QUIC cluster-secret at %s", + CLUSTER_SECRET_PATH) + return secret + finally: + os.close(fd) + except Exception as e: + logger.error("Failed to heal empty cluster-secret: %s", e) + return '' # Generate and persist exclusively (0600). hex => config-safe charset. secret = os.urandom(32).hex() fd = os.open(CLUSTER_SECRET_PATH, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) @@ -1795,6 +2306,21 @@ def generate_config(): config_parts = [] + # Snapshot the last-known-good config BEFORE anything below touches a + # file in /etc/haproxy. Everything this function writes (haproxy.cfg, + # blocked_ips.map, coraza-spoe.cfg) is validated as one set by + # `haproxy -c`, so the rollback point has to predate the first of them. + # Taking it here (rather than inside reload_haproxy_safely(), which runs + # after the writes) is what makes rollback real - see create_backup(). + backup_ok, backup_status = create_backup() + if not backup_ok: + # Could not even attempt a snapshot (I/O error). Writing a new + # config now would leave us with no way back, so refuse. + raise Exception( + "Refusing to regenerate config: failed to back up the current " + "configuration, so a failed change could not be rolled back" + ) + # Optional Coraza WAF integration. When HAPROXY_CORAZA_SPOE_BACKEND is # set on the haproxy-manager container, we render an extra TCP backend # pointing at a coraza-spoa sidecar AND inject a `filter spoe ...` line @@ -1831,8 +2357,12 @@ def generate_config(): ) config_parts.append(default_headers) - # Update blocked IPs map file first - update_blocked_ips_map() + # Update blocked IPs map file first. promote_backup=False: the rollback + # snapshot was taken a few lines above and this map is part of the + # not-yet-validated change, so refreshing the backup copy here would + # overwrite the bytes rollback needs - the same class of bug as backing + # up after the write. + update_blocked_ips_map(promote_backup=False) # Add Listener Block listener_block = template_env.get_template('hap_listener.tpl').render( @@ -1984,25 +2514,21 @@ backend default-backend # how the file was authored. if not coraza_spoe_cfg.endswith('\n'): coraza_spoe_cfg += '\n' - coraza_spoe_path = '/etc/haproxy/coraza-spoe.cfg' - with open(coraza_spoe_path, 'w') as f: - f.write(coraza_spoe_cfg) - logger.info(f"Coraza SPOE engine config written to {coraza_spoe_path} " + write_config_atomically(CORAZA_SPOE_CONFIG_PATH, coraza_spoe_cfg) + logger.info(f"Coraza SPOE engine config written to " + f"{CORAZA_SPOE_CONFIG_PATH} " f"(SPOA target: {coraza_spoe_backend})") - # Write complete configuration to tmp - temp_config_path = "/etc/haproxy/haproxy.cfg" - config_content = '\n'.join(config_parts) logger.debug("Generated HAProxy configuration") - # Write complete configuration to tmp - # Write new configuration to file - with open(HAPROXY_CONFIG_PATH, 'w') as f: - f.write(config_content) - + # Write new configuration to file (atomically - a truncated haproxy.cfg + # is as fatal as an invalid one). The rollback point was taken above, + # before this write. + write_config_atomically(HAPROXY_CONFIG_PATH, config_content) + # Use safe reload with validation and rollback - success, message = reload_haproxy_safely() + success, message = reload_haproxy_safely(backup_status=backup_status) if success: logger.info("Configuration generated and HAProxy reloaded safely") log_operation('generate_config', True, 'Configuration generated and HAProxy reloaded safely') @@ -2019,63 +2545,340 @@ backend default-backend traceback.print_exc() raise -def create_backup(): - """Create backup of current config and map files""" +# --------------------------------------------------------------------------- +# Config backup / rollback +# --------------------------------------------------------------------------- +# Rollback only works if the backup predates the write it is supposed to undo. +# Until 2026-08 create_backup() ran from inside reload_haproxy_safely(), i.e. +# AFTER generate_config() had already overwritten haproxy.cfg — so the "backup" +# was a copy of the new (possibly broken) config and restore_backup() restored +# the same broken bytes. The advertised rollback was a no-op and a fatal +# haproxy.cfg persisted on disk, where start_haproxy() refuses to launch (the +# June 2026 missing-template incident). create_backup() must now be called by +# the writer, BEFORE the first byte is written. + +# Statuses returned by create_backup() that mean a rollback target exists. +_ROLLBACK_AVAILABLE_STATUSES = ('created', 'kept_previous') + + +def _files_identical(path_a, path_b): + """Byte-compare two files. + + Deliberately not filecmp.cmp(): it memoises on (size, mtime), and + shutil.copy2() preserves mtime, so a stale cache entry could report a + changed config as unchanged. These files are small; read them. + """ try: - if os.path.exists(HAPROXY_CONFIG_PATH): - shutil.copy2(HAPROXY_CONFIG_PATH, HAPROXY_BACKUP_PATH) - if os.path.exists(BLOCKED_IPS_MAP_PATH): - shutil.copy2(BLOCKED_IPS_MAP_PATH, BLOCKED_IPS_MAP_BACKUP_PATH) - logger.info("Backups created successfully") - return True + if os.path.getsize(path_a) != os.path.getsize(path_b): + return False + with open(path_a, 'rb') as fa, open(path_b, 'rb') as fb: + while True: + chunk_a = fa.read(65536) + chunk_b = fb.read(65536) + if chunk_a != chunk_b: + return False + if not chunk_a: + return True + except OSError: + return False + + +def _config_set_matches_backup(): + """True if every live config file is byte-identical to its backup copy. + + After a successful reload the live set has already been recorded as + known-good (see promote_current_config_to_backup()), which is the common + case at the start of the next generation. Recognising it lets create_backup() + skip both the re-validation and the copy - worth doing because + `haproxy -c` on an edge with hundreds of certificates is not free and + generate_config() runs synchronously inside customer-facing API calls. + """ + for live_path, backup_path in _config_backup_pairs(): + if os.path.exists(live_path) != os.path.exists(backup_path): + return False + if (os.path.exists(live_path) + and not _files_identical(live_path, backup_path)): + return False + return True + + +def _config_backup_pairs(): + """(live, backup) pairs forming one restorable config set. + + Built at call time rather than at import so the module-level path constants + stay patchable (tests, alternate deployments). + """ + return ( + (HAPROXY_CONFIG_PATH, HAPROXY_BACKUP_PATH), + (BLOCKED_IPS_MAP_PATH, BLOCKED_IPS_MAP_BACKUP_PATH), + (CORAZA_SPOE_CONFIG_PATH, CORAZA_SPOE_BACKUP_PATH), + ) + + +def write_config_atomically(path, content, staging_dir=None, validate=None): + """Write content to path via temp file + rename. + + A half-written haproxy.cfg (disk full, container killed mid-write) is just + as fatal as an invalid one and is invisible to the caller. os.replace() is + atomic within a filesystem, so the file on disk is always either the whole + old config or the whole new one — never a truncated hybrid. This also keeps + the "existing config is already broken" case from being self-inflicted. + + The same guarantee is what certificate bundles need, so this is the single + atomic publisher for both (see publish_pem_bundle()); the two extra + arguments exist for that caller: + + staging_dir: where the temp file is created. Defaults to the destination's + own directory, which is right for /etc/haproxy but WRONG for + /etc/haproxy/certs — HAProxy loads that path as a crt directory and + tries to load every file in it, so a `.tmp` there (or one leaked by a + crash) can break the whole TLS bind. Must be on the same filesystem as + `path` or os.replace() cannot be atomic; if it is not, the rename fails + loudly and the destination is left untouched, which is the safe outcome. + + validate: optional callable(temp_path) -> (ok, message), run on the staged + file BEFORE it is moved into place. Returning False aborts the publish + with the temp file removed and `path` still holding its previous + contents. This is the only ordering that lets us validate the + replacement without having destroyed the original first. + """ + directory = staging_dir or os.path.dirname(path) or '.' + # Preserve the mode of the file we are replacing; mkstemp defaults to 0600 + # and HAProxy config files are conventionally 0644. + try: + mode = stat.S_IMODE(os.stat(path).st_mode) + except OSError: + mode = 0o644 + fd, tmp_path = tempfile.mkstemp( + dir=directory, prefix=os.path.basename(path) + '.', suffix='.tmp' + ) + try: + with os.fdopen(fd, 'w') as f: + f.write(content) + f.flush() + os.fsync(f.fileno()) + os.chmod(tmp_path, mode) + if validate is not None: + ok, message = validate(tmp_path) + if not ok: + raise ValueError(f'staged file failed validation: {message}') + os.replace(tmp_path, path) + except Exception: + try: + os.unlink(tmp_path) + except OSError: + pass + raise + + +def create_backup(require_valid=True): + """Snapshot the CURRENT on-disk config set as the rollback point. + + MUST be called BEFORE the new configuration is written — see the module + comment above. Calling it afterwards silently disarms rollback. + + require_valid=True (default) refuses to promote a config that HAProxy + already rejects. Backing up a broken config would make "rollback" mean + "restore a different broken config"; keeping the older, validated backup + instead means a rollback always lands on something HAProxy will actually + start with. Cost is one `haproxy -c` run per config generation. + + Returns (ok, status): + ok=False, status='error' - the copy itself failed; caller decides. + status='created' - backup now holds the current config. + status='kept_previous' - current config missing or invalid; the + existing (older, good) backup was kept. + status='unavailable' - nothing to roll back to at all (first + run, or broken config and no prior + backup). Rollback is NOT possible. + """ + try: + snapshot_ok = True + reason = None + + if not os.path.exists(HAPROXY_CONFIG_PATH): + snapshot_ok = False + reason = 'no existing HAProxy config on disk (first run?)' + elif _config_set_matches_backup(): + # The backup already IS the current config, recorded when it last + # loaded successfully. Nothing to copy and nothing to re-validate. + logger.debug("Config backup already matches the live config") + return True, 'created' + elif require_valid: + status, msg = validate_config_file(HAPROXY_CONFIG_PATH) + if status == 'invalid': + snapshot_ok = False + reason = f'current config on disk does not validate: {msg}' + elif status == 'unavailable': + # The validator itself could not run (no haproxy binary, etc). + # That is NOT evidence the config is bad, and refusing to back + # up would leave us with no rollback target at all, so fall + # back to last-written semantics and say so loudly. + logger.warning( + f"Could not verify current config before backup ({msg}); " + "backing it up unverified" + ) + + if not snapshot_ok: + if os.path.exists(HAPROXY_BACKUP_PATH): + logger.warning( + f"Not refreshing config backup: {reason}. Keeping the " + f"existing backup at {HAPROXY_BACKUP_PATH} as the rollback " + "target." + ) + return True, 'kept_previous' + logger.error( + f"No config backup could be taken: {reason}, and no previous " + f"backup exists at {HAPROXY_BACKUP_PATH}. ROLLBACK IS NOT " + "AVAILABLE for this configuration change." + ) + return True, 'unavailable' + + for live_path, backup_path in _config_backup_pairs(): + if os.path.exists(live_path): + shutil.copy2(live_path, backup_path) + logger.info("Backup of last-known-good config created successfully") + return True, 'created' except Exception as e: logger.error(f"Failed to create backup: {e}") - return False + return False, 'error' -def restore_backup(): - """Restore from backup files""" +def promote_current_config_to_backup(): + """Record the live config as the known-good rollback target. + + Called ONLY after the config has both validated and been loaded by HAProxy, + so "backup" really means "the last configuration this box was running". + Must never be called before a reload attempt: doing so would make the + backup a copy of the config we may still have to roll back from - the same + class of bug as backing up after the write. + + Without this, a box whose very first generation succeeded has no rollback + target at all until its second successful generation, and any corruption of + haproxy.cfg in between leaves nothing to recover to. + """ try: - if os.path.exists(HAPROXY_BACKUP_PATH): - shutil.copy2(HAPROXY_BACKUP_PATH, HAPROXY_CONFIG_PATH) - if os.path.exists(BLOCKED_IPS_MAP_BACKUP_PATH): - shutil.copy2(BLOCKED_IPS_MAP_BACKUP_PATH, BLOCKED_IPS_MAP_PATH) - logger.info("Backups restored successfully") + for live_path, backup_path in _config_backup_pairs(): + if os.path.exists(live_path): + shutil.copy2(live_path, backup_path) + logger.debug("Known-good config backup updated after successful reload") return True except Exception as e: - logger.error(f"Failed to restore backup: {e}") + # Non-fatal: the config is live and working, we just failed to record + # it. Loud, because the next change now has a staler rollback target. + logger.error(f"Failed to record known-good config backup: {e}") return False -def validate_haproxy_config(): - """Validate HAProxy configuration file""" - try: - result = subprocess.run(['haproxy', '-c', '-f', HAPROXY_CONFIG_PATH], - capture_output=True, text=True) - if result.returncode == 0: - logger.info("HAProxy configuration validation passed") - return True, None - else: - error_msg = f"HAProxy configuration validation failed: {result.stderr}" - logger.error(error_msg) - return False, error_msg - except Exception as e: - error_msg = f"Error validating HAProxy config: {e}" - logger.error(error_msg) - return False, error_msg -def reload_haproxy_safely(): - """Safely reload HAProxy with validation and rollback""" +def restore_backup(): + """Restore the backed-up config set over the live files. + + Returns (restored, message). restored=False means NOTHING was rolled back + and the live config is still whatever the failed change left on disk — + callers MUST surface that difference, it is the difference between "we + recovered" and "this edge is sitting on a config HAProxy will not load". + """ + if not os.path.exists(HAPROXY_BACKUP_PATH): + msg = (f"No config backup at {HAPROXY_BACKUP_PATH} - cannot roll back; " + f"{HAPROXY_CONFIG_PATH} still holds the failed configuration") + logger.critical(msg) + return False, msg try: - # Create backup before changes - if not create_backup(): - return False, "Failed to create backup" - + for live_path, backup_path in _config_backup_pairs(): + if os.path.exists(backup_path): + shutil.copy2(backup_path, live_path) + msg = f"Configuration restored from backup ({HAPROXY_BACKUP_PATH})" + logger.info(msg) + return True, msg + except Exception as e: + msg = (f"Failed to restore backup: {e} - {HAPROXY_CONFIG_PATH} may hold " + "a broken configuration") + logger.critical(msg) + return False, msg + + +def validate_config_file(config_path): + """Run `haproxy -c` against config_path. + + Returns (status, message) with status one of: + 'valid' - HAProxy parsed the file successfully + 'invalid' - HAProxy rejected it (message carries stderr) + 'unavailable' - the validator could not be run at all (binary missing, + timeout, ...). Deliberately distinct from 'invalid': + it tells us nothing about the config. + """ + try: + result = subprocess.run(['haproxy', '-c', '-f', config_path], + capture_output=True, text=True) + except Exception as e: + return 'unavailable', f"Error validating HAProxy config: {e}" + if result.returncode == 0: + return 'valid', None + return 'invalid', f"HAProxy configuration validation failed: {result.stderr}" + + +def validate_haproxy_config(): + """Validate the live HAProxy configuration file. Returns (is_valid, error). + + NOTE the invalid/unavailable distinction validate_config_file() draws does + NOT survive here: this returns a bare bool, so a validator that could not + run is reported the same as a rejected config and the reload path rolls + back, logging "Config validation failed". That is deliberate - without a + working validator we cannot claim the new config is safe, and the reload + path is the one place where guessing wrong takes the edge down. The + distinction is only acted on inside create_backup(), where treating + "cannot check" as "bad" would mean refusing to keep any rollback target at + all. (The 2026-08 commit message said flatly that "a missing haproxy binary + is not read as a bad config"; that is true of create_backup() only.) + """ + status, message = validate_config_file(HAPROXY_CONFIG_PATH) + if status == 'valid': + logger.info("HAProxy configuration validation passed") + return True, None + logger.error(message) + return False, message + +def reload_haproxy_safely(backup_status=None): + """Safely reload HAProxy with validation and rollback. + + PRECONDITION: the caller must already have called create_backup() BEFORE + writing the new config, and pass the status it returned. This function runs + after the new config is on disk, so it cannot take a meaningful backup + itself — doing so is exactly the bug this contract exists to prevent. + + backup_status=None means the caller did not take a pre-write backup. We do + NOT create one here (that would overwrite a genuinely good backup with the + unverified new config); we log it and fall back to whatever backup already + exists on disk. + """ + try: + if backup_status is None: + logger.error( + "reload_haproxy_safely() called without a pre-write backup " + "status - rollback will fall back to whatever backup already " + "exists on disk. Callers must call create_backup() BEFORE " + "writing the new configuration." + ) + elif backup_status not in _ROLLBACK_AVAILABLE_STATUSES: + logger.warning( + f"Proceeding with reload without a rollback target " + f"(backup status: {backup_status})" + ) + # Validate new configuration is_valid, error_msg = validate_haproxy_config() if not is_valid: # Restore backup on validation failure - restore_backup() + restored, restore_msg = restore_backup() + if not restored: + logger.critical( + "Config validation failed AND rollback was not possible - " + f"{HAPROXY_CONFIG_PATH} holds an invalid configuration that " + "HAProxy will refuse to start with" + ) + return False, (f"Config validation failed: {error_msg} | " + f"ROLLBACK FAILED: {restore_msg}") return False, f"Config validation failed: {error_msg}" - + # Attempt reload if is_process_running('haproxy'): # Use HAProxy stats socket for graceful reload @@ -2094,20 +2897,28 @@ def reload_haproxy_safely(): if reload_result.returncode == 0: logger.info("HAProxy reloaded successfully") + # Now - and only now - is this config known good. + promote_current_config_to_backup() return True, "HAProxy reloaded successfully" else: # Reload failed, restore backup - restore_backup() - # Try to reload with backup config - subprocess.run('echo "reload" | socat stdio /tmp/haproxy-cli', - shell=True, capture_output=True) + restored, restore_msg = restore_backup() + if restored: + # Try to reload with the restored (known-good) config + subprocess.run( + 'echo "reload" | socat stdio /tmp/haproxy-cli', + shell=True, capture_output=True) error_msg = f"HAProxy reload failed: {reload_result.stderr}" + if not restored: + error_msg += f" | ROLLBACK FAILED: {restore_msg}" logger.error(error_msg) return False, error_msg except Exception as e: # Critical error during reload, restore backup - restore_backup() + restored, restore_msg = restore_backup() error_msg = f"Critical error during reload: {e}" + if not restored: + error_msg += f" | ROLLBACK FAILED: {restore_msg}" logger.error(error_msg) return False, error_msg else: @@ -2118,20 +2929,101 @@ def reload_haproxy_safely(): check=True, capture_output=True, text=True ) logger.info("HAProxy started successfully") + # Now - and only now - is this config known good. + promote_current_config_to_backup() return True, "HAProxy started successfully" except subprocess.CalledProcessError as e: # Start failed, restore backup - restore_backup() + restored, restore_msg = restore_backup() error_msg = f"Failed to start HAProxy: {e.stderr}" + if not restored: + error_msg += f" | ROLLBACK FAILED: {restore_msg}" logger.error(error_msg) return False, error_msg except Exception as e: + # KNOWN GAP (pre-existing, unchanged by the 2026-08 backup-ordering + # fix): this outer handler does NOT roll back. The window is narrow - + # everything after the validation gate has its own handler - but an + # exception raised between the gate and those handlers leaves the new + # config on disk. Left as-is deliberately; rolling back from here would + # also undo changes that had in fact loaded. error_msg = f"Critical error in reload process: {e}" logger.error(error_msg) return False, error_msg -def update_blocked_ips_map(): - """Update the blocked IPs map file from database""" +def _blocked_ips_map_is_wellformed(path): + """True if every key in a blocked-IPs map is one HAProxy will parse. + + map_ip() rejects the ENTIRE configuration if a single key is not an IP or + CIDR ("'198.51.10' is not a valid IPv4 or IPv6 address at line 2 of file + ..."), so this is the property that decides whether the file is safe to + record as a rollback target. Pure Python over a small file - deliberately + not another `haproxy -c`, which is the cost this check exists to avoid. + """ + try: + with open(path) as fh: + for line in fh: + line = line.strip() + if not line or line.startswith('#'): + continue + ipaddress.ip_network(line.split()[0], strict=False) + except (OSError, ValueError, IndexError): + return False + return True + + +def _promote_blocked_ips_map_to_backup(): + """Keep blocked_ips.map.backup in step with the map just written. + + The /api/blocked-ips routes call update_blocked_ips_map() OUTSIDE + generate_config(): the map write IS the whole change there, published + against a config HAProxy is already running. Without promoting it, the live + map drifts from its backup, create_backup() misses its fast path on the + NEXT config change, and an edge that blocks IPs automatically (this fleet + does) pays an extra `haproxy -c` on the following customer-facing domain + add - permanently, since every block re-opens the drift. + + Guarded twice: + * a config backup set must already exist - never fabricate a rollback + target out of nothing (see restore_backup()); and + * the map must be well-formed, so promoting it cannot leave a "rollback + target" HAProxy refuses to load. That would be this module's own bug + on a different file. + When either fails we leave the older backup map alone and the next + create_backup() takes the slow, `haproxy -c`-validated path - i.e. the + behaviour before this function existed. Nothing here can lose data. + + Note the map is promoted after the write rather than after the caller's + reload: the routes only warn on a failed reload and carry on, so there is + no reload result to gate on. A well-formed map that HAProxy has not loaded + yet is still a loadable rollback target, which is the guarantee that + matters. + """ + if not os.path.exists(HAPROXY_BACKUP_PATH): + return False + if not _blocked_ips_map_is_wellformed(BLOCKED_IPS_MAP_PATH): + logger.warning( + f"Not recording {BLOCKED_IPS_MAP_PATH} as known-good: it contains " + "an entry HAProxy cannot parse, which would make the whole " + "configuration invalid. Keeping the previous backup map." + ) + return False + try: + shutil.copy2(BLOCKED_IPS_MAP_PATH, BLOCKED_IPS_MAP_BACKUP_PATH) + return True + except OSError as e: + logger.warning(f"Could not update the backup blocked IPs map: {e}") + return False + + +def update_blocked_ips_map(promote_backup=True): + """Update the blocked IPs map file from database. + + promote_backup=False is for callers that are in the middle of an + unvalidated configuration change - generate_config() has already taken the + rollback snapshot by the time it gets here, so refreshing the backup map + would overwrite the very bytes rollback needs. + """ try: with sqlite3.connect(DB_FILE) as conn: cursor = conn.cursor() @@ -2142,9 +3034,19 @@ def update_blocked_ips_map(): # For IP blocking, we use: 1 # This allows map_ip() to work with both single IPs and CIDR ranges os.makedirs(os.path.dirname(BLOCKED_IPS_MAP_PATH), exist_ok=True) - with open(BLOCKED_IPS_MAP_PATH, 'w') as f: - for ip in blocked_ips: - f.write(f"{ip} 1\n") + # Atomically, for the same reason haproxy.cfg is: `haproxy -c` LOADS + # this file (hap_listener.tpl matches on + # map_ip(/etc/haproxy/blocked_ips.map,0)), and a half-written final + # line is a FATAL config error rather than one dropped entry - verified + # against HAProxy 2.8. A truncated map is therefore exactly the failure + # this module's backup ordering exists to prevent, on a different file. + write_config_atomically( + BLOCKED_IPS_MAP_PATH, + ''.join(f"{ip} 1\n" for ip in blocked_ips) + ) + + if promote_backup: + _promote_blocked_ips_map_to_backup() logger.info(f"Updated blocked IPs map file with {len(blocked_ips)} IPs") return True diff --git a/scripts/cert-publish-lib.sh b/scripts/cert-publish-lib.sh new file mode 100644 index 0000000..a589f05 --- /dev/null +++ b/scripts/cert-publish-lib.sh @@ -0,0 +1,371 @@ +# shellcheck shell=bash +# cert-publish-lib.sh - safe publication of HAProxy certificate bundles. +# +# This file is SOURCED, never executed (hence no shebang / no exec bit). +# +# Why this exists +# --------------- +# renew-certificates.sh and sync-certificates.sh used to publish a bundle with +# +# cat "$CERT_FILE" "$KEY_FILE" > "$COMBINED_FILE" +# +# where $COMBINED_FILE is the LIVE pem HAProxy is serving right now. The shell +# truncates the destination to zero bytes when it sets up the redirect, BEFORE +# cat ever runs, so any failure after that point (unreadable source, ENOSPC, +# container killed mid-write) leaves a zero-length or key-less pem in place. +# Checking cat's exit status does not help: the damage is already done. +# +# That matters more here than for an ordinary file because HAProxy loads +# $SSL_CERTS_DIR as a DIRECTORY: +# +# bind 0.0.0.0:443 ssl crt /etc/haproxy/certs +# +# It tries to load *every* file in that directory, and one unloadable file +# fails the whole bind - i.e. HTTPS goes down for every customer on the host. +# +# Two consequences drive the design below: +# 1. Assemble somewhere else and rename into place, so the live pem is either +# the old bundle or the new one and never a half-written one. +# 2. NEVER create a temp file, .tmp, .backup or any other non-final file +# inside $SSL_CERTS_DIR. Staging and backups live in SIBLING directories. +# +# Directory layout (kept identical to the Python half in haproxy_manager.py): +# staging: $(dirname $SSL_CERTS_DIR)/cert-staging [$CERT_STAGING_DIR] +# backups: $(dirname $SSL_CERTS_DIR)/cert-backups [$CERT_BACKUP_DIR] +# Both siblings of the certs dir, so they are on the same filesystem and the +# final mv is a rename(2) - atomic. There is deliberately no "just write it +# directly into the certs dir" fallback path. +# +# The same-filesystem property is CHECKED, not assumed (see cert_publish step +# (c)). An earlier revision of this comment claimed a cross-device mv would +# "fail loudly and leave the live pem alone". It does not: GNU mv falls back to +# copy-then-unlink across filesystems, so it OPENS THE DESTINATION FOR WRITING +# and only then discovers it cannot finish - e.g. with a full destination +# filesystem the live pem is already overwritten when mv reports failure. That +# is precisely the truncation this library exists to prevent, so the device +# numbers of the staging dir and the certs dir are compared with stat(1) before +# anything is written, and a mismatch aborts the publish. Both directories are +# env-overridable ($CERT_STAGING_DIR / $SSL_CERTS_DIR), so "they are siblings" +# is not something the code can take on faith. This mirrors the explicit +# st_dev check in publish_pem_bundle() on the Python side. + +# Logging: the callers define their own log_info/log_error. Only provide +# fallbacks so this library is usable standalone (e.g. from a test or a shell). +declare -F log_info >/dev/null || log_info() { + echo "[$(date '+%Y-%m-%d %H:%M:%S')] [INFO] $*" +} +declare -F log_error >/dev/null || log_error() { + echo "[$(date '+%Y-%m-%d %H:%M:%S')] [ERROR] $*" >&2 +} +# log_warn is not part of the callers' vocabulary; route it through log_info +# with a loud prefix so it lands in the main log without tripping the +# error-log monitors (scripts/monitor-errors.sh) for non-fatal conditions. +declare -F log_warn >/dev/null || log_warn() { + log_info "WARNING: $*" +} + +cert_staging_dir() { + if [ -n "${CERT_STAGING_DIR:-}" ]; then + echo "$CERT_STAGING_DIR" + else + echo "$(dirname "${SSL_CERTS_DIR:-/etc/haproxy/certs}")/cert-staging" + fi +} + +cert_backup_dir() { + if [ -n "${CERT_BACKUP_DIR:-}" ]; then + echo "$CERT_BACKUP_DIR" + else + echo "$(dirname "${SSL_CERTS_DIR:-/etc/haproxy/certs}")/cert-backups" + fi +} + +# cert_bundle_valid FILE +# +# Returns 0 if FILE is publishable as an HAProxy pem bundle. +# +# Layer 1 (structure, pure shell/grep): covers the truncation / partial-write / +# key-less failure modes this library exists to prevent. +# Layer 2 (cryptographic pairing via the openssl CLI): covers what layer 1 +# cannot see. Structural checks are weak on their own - a bundle of +# EMPTY pem blocks ("-----BEGIN CERTIFICATE-----" immediately followed +# by "-----END CERTIFICATE-----") satisfies every grep below and is +# caught only by openssl. +# +# BOTH LAYERS ARE MANDATORY. An absent openssl binary is a hard failure, not a +# skip. +# +# The previous revision made layer 2 best-effort and justified it with "the +# container image installs haproxy, certbot and socat, but not necessarily the +# openssl CLI". That premise is false. openssl 3.x is present in the image: it +# is a dependency of ca-certificates, which certbot needs, and +# generate_self_signed_cert() in haproxy_manager.py shells out to `openssl req` +# with check=True during first-run setup, so a container that reached the point +# of publishing a bundle has always had it. The "unavailable" branch therefore +# never fired in production, which means the fail-open was safe only by +# accident - and a comment that justifies a decision on a false premise is +# worse than no comment, because the next person extends the reasoning. +# +# Making it mandatory does mean a hypothetical image without openssl stops +# publishing renewals. That is the right trade: it fails immediately and +# loudly, into the monitored error log, on the first renewal run, whereas +# publishing an unpaired or empty-block bundle takes the whole :443 bind (i.e. +# every site on the host) down at the next reload. There is deliberately no +# python `cryptography` fallback: the app runs on /usr/local/bin/python3 (the +# base image's 3.12), where cryptography is NOT importable - it is installed +# for Debian's /usr/bin/python3 as a certbot dependency. Coding to a hardcoded +# /usr/bin/python3 would just be a second unverified premise. +cert_bundle_valid() { + local file="$1" + + if [ -z "$file" ]; then + log_error "cert_bundle_valid: no file given" + return 1 + fi + if [ ! -f "$file" ]; then + log_error "Certificate bundle $file does not exist (or is not a regular file)" + return 1 + fi + + # openssl is checked BEFORE any content check so a broken image is reported + # as a broken image rather than as a bad certificate. + if ! command -v openssl >/dev/null 2>&1; then + log_error "openssl binary not found - REFUSING to publish $file." \ + "The cert/key pairing check (openssl x509 -pubkey vs openssl pkey -pubout)" \ + "is mandatory; structural checks alone cannot tell a real bundle from" \ + "empty pem blocks. Install openssl in this image." + return 1 + fi + + # Read the file ONCE and run every check against that snapshot. + # + # This used to open $file six times (two [ ] tests, three greps, two + # openssl invocations). cert_bundle_valid() is called on the LIVE pem in + # cert_publish() step (d), where a concurrent publisher can replace it + # between two of those opens - each open then sees a different file. The + # observable symptom was a spurious "private key does not match the + # certificate" ERROR in the monitored error log for a pair that was fine: + # openssl x509 read the old bundle and openssl pkey the new one. + local content + if ! content="$(cat -- "$file" 2>/dev/null)"; then + log_error "Certificate bundle $file could not be read" + return 1 + fi + if [ -z "$content" ]; then + log_error "Certificate bundle $file is empty" + return 1 + fi + + # --- layer 1: structure ------------------------------------------------- + if ! grep -qF -- '-----BEGIN CERTIFICATE-----' <<< "$content"; then + log_error "Certificate bundle $file contains no certificate block" + return 1 + fi + if ! grep -qF -- '-----END CERTIFICATE-----' <<< "$content"; then + log_error "Certificate bundle $file has an unterminated certificate block (truncated?)" + return 1 + fi + + local key_begin key_end + key_begin="$(grep -m1 -oE -- '-----BEGIN (RSA |EC )?PRIVATE KEY-----' <<< "$content")" + if [ -z "$key_begin" ]; then + log_error "Certificate bundle $file contains no private key block" + return 1 + fi + key_end="${key_begin/BEGIN/END}" + if ! grep -qF -- "$key_end" <<< "$content"; then + log_error "Certificate bundle $file has an unterminated private key block (truncated?)" + return 1 + fi + + # --- layer 2: cert/key pairing ------------------------------------------ + # Fed from the same snapshot on stdin (openssl reads stdin when -in is + # omitted) rather than re-opening $file, so layer 2 judges exactly the + # bytes layer 1 judged. -passin pass: means an encrypted key fails fast + # instead of prompting - a passphrase prompt in a cron job is a hang, not + # an error. + local cert_pub key_pub + if ! cert_pub="$(openssl x509 -noout -pubkey 2>/dev/null <<< "$content")" \ + || [ -z "$cert_pub" ]; then + log_error "Certificate bundle $file: openssl could not read the certificate" + return 1 + fi + if ! key_pub="$(openssl pkey -pubout -passin pass: 2>/dev/null <<< "$content")" \ + || [ -z "$key_pub" ]; then + log_error "Certificate bundle $file: openssl could not read the private key" + return 1 + fi + if [ "$cert_pub" != "$key_pub" ]; then + log_error "Certificate bundle $file: private key does not match the certificate" + return 1 + fi + + return 0 +} + +# cert_publish CERT_FILE KEY_FILE DEST_FILE +# +# Assemble CERT_FILE + KEY_FILE into DEST_FILE without ever exposing a +# partially written DEST_FILE to HAProxy. Returns 0 on success. +# +# On ANY failure DEST_FILE is left exactly as it was. +cert_publish() { + if [ $# -ne 3 ]; then + log_error "cert_publish: expected 3 arguments (cert key dest), got $#" + return 1 + fi + + local cert_file="$1" key_file="$2" dest_file="$3" + local staging_dir backup_dir dest_dir tmp base + + # (a) sources must exist and be non-empty before we touch anything. + if [ ! -s "$cert_file" ]; then + log_error "cert_publish: certificate $cert_file is missing or empty" + return 1 + fi + if [ ! -s "$key_file" ]; then + log_error "cert_publish: private key $key_file is missing or empty" + return 1 + fi + + # (b) assemble in the staging dir - NOT in the certs dir, which HAProxy + # scans wholesale. + staging_dir="$(cert_staging_dir)" + dest_dir="$(dirname "$dest_file")" + if ! mkdir -p "$staging_dir" || ! mkdir -p "$dest_dir"; then + log_error "cert_publish: cannot create staging directory $staging_dir or destination directory $dest_dir" + return 1 + fi + + # (c) the final swap is `mv`, which is only a rename(2) - and therefore only + # atomic - within one filesystem. Across filesystems GNU mv copies: + # it truncates and writes the DESTINATION, then unlinks the source, so a + # failure part-way through (ENOSPC is the realistic one) leaves exactly + # the half-written live pem this library exists to prevent. Both paths + # are env-overridable, so check instead of assuming. Same check as the + # st_dev comparison in publish_pem_bundle() on the Python side. + local staging_dev dest_dev + staging_dev="$(stat -Lc '%d' "$staging_dir" 2>/dev/null)" + dest_dev="$(stat -Lc '%d' "$dest_dir" 2>/dev/null)" + if [ -z "$staging_dev" ] || [ -z "$dest_dev" ]; then + log_error "cert_publish: cannot stat $staging_dir and/or $dest_dir - refusing to publish $dest_file" + return 1 + fi + if [ "$staging_dev" != "$dest_dev" ]; then + log_error "cert_publish: staging dir $staging_dir and destination dir $dest_dir" \ + "are on different filesystems, so the bundle cannot be swapped in atomically." \ + "Refusing to publish $dest_file (live file left untouched);" \ + "point CERT_STAGING_DIR at a directory on the same filesystem as $dest_dir" + return 1 + fi + + # Sweep temps orphaned by a kill -9 / OOM in an earlier run. Two shapes, + # because two publishers share this directory: mktemp's six-X suffix from + # this library, and `..tmp` from write_config_atomically() on + # the Python side (tempfile.mkstemp(prefix=name + '.', suffix='.tmp')). + # Matching only the mktemp shape - as this did - left every Python-side + # temp behind forever. + find "$staging_dir" -maxdepth 1 -type f \ + \( -name '*.??????' -o -name '*.tmp' \) -mmin +1440 -delete 2>/dev/null + + base="$(basename "$dest_file")" + tmp="$(mktemp "${staging_dir}/${base}.XXXXXX" 2>/dev/null)" + if [ -z "$tmp" ] || [ ! -f "$tmp" ]; then + log_error "cert_publish: cannot create a staging file in $staging_dir" + return 1 + fi + # 0600 while the temp file holds a private key; the final mode is matched + # to the file being replaced just before the swap (see below). + chmod 600 "$tmp" 2>/dev/null + + if ! cat "$cert_file" "$key_file" > "$tmp"; then + log_error "cert_publish: failed to assemble $cert_file + $key_file (live $dest_file left untouched)" + rm -f "$tmp" + return 1 + fi + if [ ! -s "$tmp" ]; then + log_error "cert_publish: assembled bundle for $dest_file is empty (live file left untouched)" + rm -f "$tmp" + return 1 + fi + + # (d) never promote something HAProxy would choke on. + if ! cert_bundle_valid "$tmp"; then + log_error "cert_publish: assembled bundle for $dest_file failed validation (live file left untouched)" + rm -f "$tmp" + return 1 + fi + + # (e) back up the bundle we are about to replace - but only if it is itself + # valid. Overwriting a good backup with garbage would turn "restore the + # backup" into "restore a different broken file". Same semantics as + # create_backup(require_valid=True) in haproxy_manager.py. + if [ -e "$dest_file" ]; then + backup_dir="$(cert_backup_dir)" + if cert_bundle_valid "$dest_file"; then + if mkdir -p "$backup_dir"; then + if ! cp -p "$dest_file" "${backup_dir}/${base}"; then + log_warn "could not back up $dest_file to ${backup_dir}/${base}; publishing anyway" + fi + else + log_warn "could not create backup directory $backup_dir; publishing without a backup" + fi + else + log_warn "existing $dest_file is not a valid bundle - KEEPING the previous backup" \ + "in $backup_dir rather than overwriting it with an unusable one" + fi + fi + + # Preserve the mode of the bundle being replaced (0644 by default, which is + # what `cat > file` produced under the standard umask). mktemp gives 0600, + # and mv carries the temp file's mode onto the destination, so without this + # every publish would silently tighten the live pem's permissions. Changing + # who can read these files is not something a write-safety fix should do as + # a side effect - and it must match write_config_atomically() on the Python + # side, which preserves the mode the same way. + # + # -L (follow symlinks) matters: stat without it reports the mode of the + # SYMLINK, which is 0777 on Linux and is not a permission at all. A live + # pem that is a symlink therefore produced a world-WRITABLE 0777 private + # key sitting in the crt directory. With -L we copy the mode of the file + # the link points at, which is the mode an operator actually chose. + local mode + mode="$(stat -Lc '%a' "$dest_file" 2>/dev/null)" + [ -n "$mode" ] || mode=644 + chmod "$mode" "$tmp" 2>/dev/null + + # (f) atomic swap. Same filesystem, verified in (c); if it still fails, + # stop - do not fall back to writing into the certs dir. + if ! mv -f "$tmp" "$dest_file"; then + log_error "cert_publish: failed to move $tmp into place as $dest_file" \ + "(live file left untouched; NOT falling back to a direct write)" + rm -f "$tmp" + return 1 + fi + + return 0 +} + +# haproxy_config_ok +# +# Gate a reload on `haproxy -c`. Returns 0 if the config validates, or if we +# cannot check (no haproxy binary) - a missing checker must not block a reload +# that is otherwise needed, but a checker that says "no" always wins. +haproxy_config_ok() { + local cfg="${HAPROXY_CONFIG:-/etc/haproxy/haproxy.cfg}" + local out rc + + if ! command -v haproxy >/dev/null 2>&1; then + log_warn "haproxy binary not found - skipping 'haproxy -c' validation before reload" + return 0 + fi + + out="$(haproxy -c -f "$cfg" 2>&1 > "$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 renewal process" # Run certbot renewal — don't exit on failure, some certs may have @@ -42,7 +55,7 @@ fi mkdir -p "$SSL_CERTS_DIR" # Get all SSL-enabled domains from database -DOMAINS=$(find /etc/letsencrypt/live/ -mindepth 1 -maxdepth 1 -type d -printf '%f\n') +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" @@ -54,13 +67,16 @@ UPDATED=0 FAILED=0 while read -r domain; do - CERT_FILE="/etc/letsencrypt/live/${domain}/fullchain.pem" - KEY_FILE="/etc/letsencrypt/live/${domain}/privkey.pem" + 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 - # Combine cert and key into single file for HAProxy - if cat "$CERT_FILE" "$KEY_FILE" > "$COMBINED_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 @@ -77,6 +93,13 @@ log_info "Certificate update 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 renewal" + exit 1 + fi + if echo "reload" | socat stdio /tmp/haproxy-cli 2>/dev/null; then log_info "HAProxy reloaded successfully" else @@ -85,5 +108,18 @@ if [ $UPDATED -gt 0 ]; then fi fi +# A per-domain publication failure is a real failure and must be reported as +# one. This script used to `exit 0` no matter how many domains failed, so +# "0 updated, 12 failed" - a host that has completely stopped publishing +# renewals - looked identical to a clean run to cron, to +# host-renew-certificates.sh (which branches on this exit code) and to any +# external monitoring. The first visible symptom would have been certificates +# expiring. The loop above deliberately continues past a failed domain so the +# others still get published; the status is reported here instead. +if [ "$FAILED" -gt 0 ]; then + log_error "Certificate renewal process completed with failures: $UPDATED updated, $FAILED failed" + exit 1 +fi + log_info "Certificate renewal process completed" exit 0 diff --git a/scripts/sync-certificates.sh b/scripts/sync-certificates.sh index 4743cb0..80cc038 100755 --- a/scripts/sync-certificates.sh +++ b/scripts/sync-certificates.sh @@ -8,6 +8,7 @@ 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() { @@ -18,13 +19,25 @@ 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 /etc/letsencrypt/live/ -mindepth 1 -maxdepth 1 -type d -printf '%f\n') +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" @@ -36,13 +49,16 @@ UPDATED=0 FAILED=0 while read -r domain; do - CERT_FILE="/etc/letsencrypt/live/${domain}/fullchain.pem" - KEY_FILE="/etc/letsencrypt/live/${domain}/privkey.pem" + 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 - # Combine cert and key into single file for HAProxy - if cat "$CERT_FILE" "$KEY_FILE" > "$COMBINED_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 @@ -59,6 +75,13 @@ 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 @@ -67,5 +90,12 @@ if [ $UPDATED -gt 0 ]; then 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 diff --git a/scripts/test-cert-scripts.py b/scripts/test-cert-scripts.py new file mode 100755 index 0000000..bc6a240 --- /dev/null +++ b/scripts/test-cert-scripts.py @@ -0,0 +1,958 @@ +#!/usr/bin/env python3 +"""Regression tests for the certificate publication shell scripts. + +Why this file exists +-------------------- +renew-certificates.sh and sync-certificates.sh published a bundle with + + cat "$CERT_FILE" "$KEY_FILE" > "$COMBINED_FILE" + +where $COMBINED_FILE is the pem HAProxy is serving *right now*. The shell +truncates the destination when it opens the redirect, before cat runs, so any +failure after that point - unreadable source key, ENOSPC, container killed +mid-write - left a zero-length or key-less pem behind. The exit status of cat +was checked, but by then the live file was already destroyed. + +HAProxy loads $SSL_CERTS_DIR as a DIRECTORY (`bind :443 ssl crt /etc/haproxy/ +certs`) and tries to load every file in it, so a single unloadable file fails +the whole bind: HTTPS down for every customer on the host. + +The fix (scripts/cert-publish-lib.sh) assembles into a sibling staging dir, +validates, backs up the outgoing bundle into a sibling backup dir, and renames +into place. These tests pin the observable guarantees: + + * a successful publish replaces the live pem and archives the old one; + * a FAILED publish leaves the previous, still-valid pem byte-for-byte intact; + * nothing that is not a final *.pem ever appears in the certs directory; + * HAProxy is not reloaded when `haproxy -c` rejects the configuration. + +Running +------- + python3 scripts/test-cert-scripts.py # tests the repo checkout + HAPROXY_MANAGER_DIR=/some/other/tree \ + python3 scripts/test-cert-scripts.py # tests another tree + +Self-contained stdlib unittest - no pytest, no venv, no bats, and nothing is +imported from the application. The scripts are driven as subprocesses with +every path they touch redirected by environment variable (SSL_CERTS_DIR, +LETSENCRYPT_LIVE_DIR, CERT_STAGING_DIR, CERT_BACKUP_DIR, LOG_FILE, +ERROR_LOG_FILE, HAPROXY_CONFIG) and stub `certbot`, `socat` and `haproxy` +binaries on PATH. + +The certificate material below is a real self-signed test certificate with its +matching key (plus a second, unrelated key for the mismatch case), embedded as +constants so the tests need no openssl to *create* material. openssl IS needed +to run them: the library's cert/key pairing check is mandatory (it is the only +layer that can reject a bundle of empty pem blocks), so without the binary +every publish is refused by design. The image ships openssl 3.x. + +The acceptance bar for this file is a green run INSIDE the built image, as +root, which is where these scripts actually execute - not on a workstation. +Several failure modes are invisible outside the container (root ignores the +directory permissions one test used to rely on) and one was actively +destructive there; see _cleanup_tmp(). +""" + +import os +import re +import shutil +import stat +import subprocess +import tempfile +import textwrap +import time +import unittest + +MODULE_DIR = os.path.abspath( + os.environ.get('HAPROXY_MANAGER_DIR', + os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')) +) +SCRIPTS_DIR = os.path.join(MODULE_DIR, 'scripts') +LIB = os.path.join(SCRIPTS_DIR, 'cert-publish-lib.sh') + +BROKEN_TOKEN = '__BROKEN__' +DOMAIN = 'test.example.com' + +# --- test key material ------------------------------------------------------- +# openssl req -x509 -newkey rsa:2048 -keyout key1 -out cert1 -days 3650 -nodes \ +# -subj /CN=test.example.com +TEST_CERT = """\ +-----BEGIN CERTIFICATE----- +MIIDFzCCAf+gAwIBAgIUeaz/lNOESOTHsB3Y97+Xja7Fy4gwDQYJKoZIhvcNAQEL +BQAwGzEZMBcGA1UEAwwQdGVzdC5leGFtcGxlLmNvbTAeFw0yNjA4MDYxNTQyMTda +Fw0zNjA4MDMxNTQyMTdaMBsxGTAXBgNVBAMMEHRlc3QuZXhhbXBsZS5jb20wggEi +MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDF5GL7Gjn+UnPFy5sqP2k4XHth +mkWFZj+mjK6cDhbBXYt60NrwVdrrgOFydMC75VeUceFxG/5GD7wrXZP23xzbnWKm +7FxfOSmr4y+1rVEZwi8IeWEz3W6C6y5rjZsCI+pBgdna+aJSpTQZHPfDpNtQm5vl +enj5BfizYixinORxm9kvXMGXV+Cw1CJkqB3mzScwWt40EtQoVxekebf8B7i4ZHyx +xT6/xwF+WY8OliZkY1pdqncoTLUAYcaE/HR/ojJKmSVIq1GswZE/y3E56LIwq+wJ +eGbgH46a+86z+VO2UX1jbad1kWKBCsRoOpaybZDEWAYTOqahW7kOH7umTUsRAgMB +AAGjUzBRMB0GA1UdDgQWBBTIqa3BNEjjcxkhXqnRwInrLM9yijAfBgNVHSMEGDAW +gBTIqa3BNEjjcxkhXqnRwInrLM9yijAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3 +DQEBCwUAA4IBAQB/lYzXb5PI3magMz/IXmwTsMCrVSdaUYEIKLEJggmbGxqpwO1a +iYagWZ/5H3B9KDvNQA+L4FkMJ726ZkdGEH/vkwvTAuhwU2NSWcbRJ8DK5u3Q4rnJ +VswPcW5njUF9mQq0NPX/PMCeOoFDEI8+RrgQZxtHhopwuKOgVA6HRBINKdEZJlrp +oLLQrHDNVLMYTclNHG6kBg0lOHUV31TgkJQ8kMgtq0WQX7RseKR10QKgN5iOBomU +3+y713Ibpac5B1zw5l3LjE/59xFteFbDENr2+A5VGhVNVZC+bs+YTYTzeAsGB0e3 +MQ+XJMJq3kaJmQ+QcTrRaKMtoMz2h1AIRbLd +-----END CERTIFICATE----- +""" + +# The private key that matches TEST_CERT. +TEST_KEY = """\ +-----BEGIN PRIVATE KEY----- +MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQDF5GL7Gjn+UnPF +y5sqP2k4XHthmkWFZj+mjK6cDhbBXYt60NrwVdrrgOFydMC75VeUceFxG/5GD7wr +XZP23xzbnWKm7FxfOSmr4y+1rVEZwi8IeWEz3W6C6y5rjZsCI+pBgdna+aJSpTQZ +HPfDpNtQm5vlenj5BfizYixinORxm9kvXMGXV+Cw1CJkqB3mzScwWt40EtQoVxek +ebf8B7i4ZHyxxT6/xwF+WY8OliZkY1pdqncoTLUAYcaE/HR/ojJKmSVIq1GswZE/ +y3E56LIwq+wJeGbgH46a+86z+VO2UX1jbad1kWKBCsRoOpaybZDEWAYTOqahW7kO +H7umTUsRAgMBAAECggEAA9SnFFqGfR1yO4XUlfmmgyZqJoJmnl2TdZlDT4bHyrwx +dSIKHO0iiNzEsHMhYHnA62EVdruunUNUdofwE28v9zHZnSbV5mt8OqUyERue5mdf +gvPbjXYXu63LBx61fZH9qME3WwFqUpx7UNGiW62LJ8ktWEC5ywNCNFG+D3YfR3Iu +v3PdIFXwkpJMaXO+42JSoSVoiqxlONqNiqcdQj64iYgCYdNxcgdLy+mHGrf1hAZs +dom2Jp0oEM4ZdOQu3Z6uyEyPsECiz1PdQjzagaEfPWtKCQk0kY8DDCn5X2xQif9w +3xeaISqho7bQgSo4IEgWHTP6V0v7brTP7VcK+gyMyQKBgQD15FrE4XAL89NBQ6iX +m2l6quZv5tIUtreuYxsOpFIMU22zfsRZOvA7sJR3ufOJ+b7tFHuJ8DAJUpK08Vvg +A930/LW9wUsY42d54XKIO/8DTsIrmjppoGdshs3axJQkqfN0zQkhqfbwaXRJ9X2k +Fvax+5jftaIaw0hQdWLnfTmGmQKBgQDOBue89D8THYl/LvDGg6WoVyK3ljBMK2s3 +4BljeZCJdWAjtido13Mltubc6YVHScmVoIZKTmx+fCjdQ3y1t92vZZSBeLsXhfFr +N+IOGZu3ZmJu64x3OukSYQ7x6agi5yP3+7k0siZgxOMXJRQDYZUcHxIHDrSGLeLZ +sj7LnbvLOQKBgE8murE1gEPYsOAJT3O96y45ZQQQYP+Z8XaJIGSOMHsXP/DPlZTD +jCEqrh/8E5EOe48FUN8OGehmVCM6rkBl/kSmNDpoxiu0x9JL5/pClcwSxh4S/0qQ +/7nHiuwo6ycCLgQjHBViCMNKrsw/4bm4SqDwRD1+0jebNOPxZWzuul3BAoGAZij0 +ZjSyxhbCZEdxau5CiYvTkjct8cch3k4IKNRRwGdsaajcN9eFqHDeXzKIPQYwqDo1 +/MiQcdO9K6JYR39JtLxo/B5Sn2JyiJjoRdea6EEjlB7GwyR6B/wKvhf/oHb+1euD +NccU0q0ucf6XwulzV8NsXAWFrHc6YnpJOwwW37kCgYAkEyjUc73jImiTyf/IXVOD +UHlRXZPvwtZUuPGe4RI0Gds97tKnvXnvFsPIRCOGfVzZ8z79DGiQ8TR2a0hgZec1 +Mo3J2dCjlv4Q6ACjHkCA1cmi13OHUPnpaeesrOk+SpEVJfR2k4qRWH4z3oxbPO/q +TDFnjWHSgjkDMrxVHZ8wPg== +-----END PRIVATE KEY----- +""" + +# A perfectly valid RSA key that has nothing to do with TEST_CERT. +UNRELATED_KEY = """\ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDE7mosrsdBhdn1 +ZIErYmMuPU69ws51JdyqwtRZlV2uLLby0dOfJpFKZPuBPEqTqDimd9N4FSL0CGjG +zXG23RYd9AbdSoorFORrUxeiPzFnbz4v38srGHckOS9ozKAbPLACUEjWMX1NwAZ+ +wGlSz3cWYTWVztFCUIxpvLVR87PSpTnpdCXIj0EABOc6WoLBLE+v2knZOa63LKg6 +GryInD43CnWFBKpH0gdgWqh+ie3NFMumLR8M3lZq2Mk0EFgVWrxPnJobYvOmavNp +NBR3ZugR4X57c1YFyLkYXQIdhxuYV1ZAY6NgmAIsaiCKFqdksF5AJSV7FZNBM43r +33ae/R5hAgMBAAECggEAIgNYsL9+OEWtU8ooWi0r2q5plXJaVNb1gkPUx+U5sS3V +amKNxbj0UrBW1Scr7U1afXwIOP8TkqkKKb4NpCsS2RkO/3USoKbC3fuTwyjdeFM5 +Hy0sytR2rXm4A8aF57ZnYvrpXZ9eGEnwhT9n4Y7mL2YaSnXWZDkDy3Z1rcIk/p5P +jUo4UQyzD/9Gab1stcBbGv+66B3mlVdRVJor5+tGn6zmr4TjvqupAexuXd+Q6+1L +OG4e2bAUmuOVOa+w8Xo4vwkiXSDLVEsl1z1x4Bcrv61bIg0rbZAeqcZ4EUPyrEyR +RpaaOLAiwdzjOj5pQvHAF2q/++IlY6xPhotTr11tvQKBgQD37C2UQRGRo622+2kx +qgsdA6jPM9vCE9S4aS4qaOj8XSMMCu6bQW5lU48DaONvXxDmT6x9IrgR3QN1Iz/t +17kUCCghuviEP1RjnahBRQqZR38KdDzKhaWt9jgdjCun9MzUXIaQRTFlFGA1z1pz +apZ9a8/ehYPSk4pv9h06/B3IjQKBgQDLWPBuhj203QOmwVF1v1k4yyuJ7UBqtRaY +I9jMW93sB2hPs6Se10UroQHlF95IHeRXvNKH/UXuAILIJMN8oTI4uU7t60shJHvI +o14RzEZoUQjES7BBVhePglndJmYIKoKKAX5lSFe9Lk0ei9B+1Dle5Q+9ZeP09cp4 +vVcGvOgqJQKBgQCtxe1srO8TlhZ821uwY+/GNnpsQX0XW68OUyr4rvAfc2jNWBxG +1mX6v8bOLQa9WXUO+Wl9jIhYfQGfaUW2AC7Jy63VdqgaigksiaUVmr8DEQoK2c6C +ZYrrlFlg3I78+qlXcEMhfF5S6yVEkkJkA6HX52mcHxl2z9OJBokWfwChQQKBgAzH +xzy7FS/D4FHfvpXu89Wc91yQ28aZIRVo01xsvbLy+DxiJwuQrhlC4lKawG656jsV +dAn2AiomQBICNYMkwnpMM0jCzBMGLv16PxRRSW+PAEUOGMLSfWKYp7s9iZYjzdaM +p3wIIvOR8GjmErGV9xEexnF58OzZceNKyyhyQQk9AoGAbvjJvYLOhQpAbbqErNNv +LK1P+TngKpukRHXjiUpPVEGNhN6krBBJCBWzY7ucrIy6jz8UBy7SbITBy7qIKhxZ +PVP7WVATMWEeW1AfdBfCYDI4jFKAD8SLECby45nRBuBllYdQnW1gBzLcCulCwB+w +FV0RvuQPDYkqsx8ibqpSv7c= +-----END PRIVATE KEY----- +""" + +# A SECOND, unrelated but internally consistent pair. Needed by the +# concurrency test: two bundles that are each perfectly valid but whose keys +# differ, so a validator that reads the certificate from one and the key from +# the other reports a mismatch. Two bundles sharing key material - which is +# what PREVIOUS_BUNDLE and NEW_BUNDLE are - cannot detect that at all. +# openssl req -x509 -newkey rsa:2048 -nodes -days 3650 \ +# -subj /CN=second.example.com +TEST_CERT_2 = """\ +-----BEGIN CERTIFICATE----- +MIIDGzCCAgOgAwIBAgIUKQAAvrVkden7gIg2zYMLb6dO1c8wDQYJKoZIhvcNAQEL +BQAwHTEbMBkGA1UEAwwSc2Vjb25kLmV4YW1wbGUuY29tMB4XDTI2MDgwNjE2NTcz +NVoXDTM2MDgwMzE2NTczNVowHTEbMBkGA1UEAwwSc2Vjb25kLmV4YW1wbGUuY29t +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAv2pfTOvN2zrZJ6d+biiD +VOczMqanuHPBCy2CnTBif+7VWf8AaywRzQ3ShfhmalVRNEFMn0MSO+GocmH71Ve1 +nl89oGiJPmvX1lpncbgM692ddhP9ez4xUeNj+QAWp9VBZhInNuM4Pawv5BPngtpj +2MGXf3ZlBSli8Ng7jBo1fTMT3bh8GcE1rIPRvmUuQwFIt2eGnLR8jQd+xGelhAjG +nnXtlc+ebo4r2OjljNgvtdUknBZdpiZXmjdFzyClYTeMuEen2uwMpJNc0wLbRjcU +khVF3nw4jUnkOhWH3JYGAoWslJyEqZSANwt/eOHwXgyVuxg31bCl297iskW0IRZL +wQIDAQABo1MwUTAdBgNVHQ4EFgQU7HH8GacJzc2j9s2UJCPwykgrIckwHwYDVR0j +BBgwFoAU7HH8GacJzc2j9s2UJCPwykgrIckwDwYDVR0TAQH/BAUwAwEB/zANBgkq +hkiG9w0BAQsFAAOCAQEAYE5jHX1dK091jVsFSZDdiw9AU5rrk8XpF1yuPmDisRnE +dJ4QQq3dzWXRnp0bzZnq7fdfiEz1m39zVixov7WFp24QhenD2n5K7/wew7RpXTnA +pAGBEdsGvBJ+3MgkRYklXCM9f9f4z21xXRNZ+BwBcM25D+gR4b+PRMQR6BhZx5R+ +y2jQsoM68cFjRApFWgmji4pBjg/eOaZMBCfVTjP+npVyqG7UtV5EyYXwPgPa/rm2 +FoP+eftzP6dszBEonkIVyyvkdscI4Wkr8hw3S0R/TP8l9lTnvN1HC3o7Es5VY53R +swNAXWBlgm0N7A96ISLtQjgvOfeMRTCSjxW9pm0wJA== +-----END CERTIFICATE----- +""" + +TEST_KEY_2 = """\ +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQC/al9M683bOtkn +p35uKINU5zMypqe4c8ELLYKdMGJ/7tVZ/wBrLBHNDdKF+GZqVVE0QUyfQxI74ahy +YfvVV7WeXz2gaIk+a9fWWmdxuAzr3Z12E/17PjFR42P5ABan1UFmEic24zg9rC/k +E+eC2mPYwZd/dmUFKWLw2DuMGjV9MxPduHwZwTWsg9G+ZS5DAUi3Z4actHyNB37E +Z6WECMaede2Vz55ujivY6OWM2C+11SScFl2mJleaN0XPIKVhN4y4R6fa7Aykk1zT +AttGNxSSFUXefDiNSeQ6FYfclgYChayUnISplIA3C3944fBeDJW7GDfVsKXb3uKy +RbQhFkvBAgMBAAECggEABQLqy6eedRPt31sLOMFEDkzbZdFHOMpZeuqThsNmjo7z +t9diokgeD4ZQXimbEQqsZYDAtmFGdiEp86I9JQBk/4TUiqFhHrOADBe1jQAptjfs +iyIb51gIPnjZ1RBA5Al8zohy28T9h1+Z7+/OeCvcgLyVAixf/U9pU8D1El9cu9zv +4q4WJPB5Tgkq+YwcmeuT8LzsKoSDmPQjFVY9v+gz6hoVUVyP6gswnlFnKjNcmfZU +0CKP90sCAc5mKZv7RyGG920LDU4u2ggnQoK05GhXK8R3amJmoAF3i+xGeTRvNEKe +wDC7NinTG2WDVo6y/FCvFKs0+qqlKww1u39fq166rQKBgQDtjPjvkDkSYFcWFznn +sfsxN5R1cLpxdutLZhfvtpHIj5NkQbmOk0r3LbDAwU+UNVQ6F8jWQVHUsni9jAY6 +3RNRF4ZabC51aqg/Ssj7d5mE4kj7y6Ch/nnSXIogRxLG4bWo2rtG1M2Dt3Ef08R4 +6gjcp9ZmELyVx7H4yXFJGIaDdQKBgQDOSCHFK/ggZzzy4iC7DOq1lrbR+jyk95rh +F5EGzyAkgJ4uYc9TCPkUDxXjWL17r61/obfbW4znaV9fHEKpz3R54osDXln3oLea +BBWkJI3ANe8iNrGDE4FN9to4DdUMFsWX/WEiRyBPIqDy9DTyadblMLTNZjNPuLmg +ZJOB3tRZnQKBgQCjeglya9Eq2Uv1MuSxk2VnmHU9YOed4BXLHKZKXFz1JgFr1GNL +QAguFK536FDIkO62z9lxwR/8fRnkb7F13uBFRSg7oAlU2qKQc/nePI9UyJkrVxXj +hYn2f6K61c6ROZFXc7e/5gDMrXhXS9gA0iZpG8PLF6eAeB39NTwV7p/bZQKBgQCV +v5eEY58FJu0ABVhtcbsRiA+/70EHIRi2Pz1xC/vxg81RLoArb2AiR7FEEa+8kpQJ +C4VFIPjxJXWuvf1G+Os9cFAqadw1/95JWJ29QywEVSL8W2gSF57O0l0oRCJdXEql +Q7O4BppV2HWu6clmEZ+HUgxu77pgLWHUJi9PIExXoQKBgEq6rYBXmsSQermnPTOn +YFx7c2ns97hsjYIbs497+gPW4/xQWwsN76t60SWqjXV4DHJCpi5Tnjo2fk5/OzK+ +HfshC9CKFDk8T0KGQWwaPwqP/OYbqOA88IlJ7xbPdSuJkjefHCtVVEtao6+HNgzm +lniLtDMpU0MLPgB98ClQ4HDA +-----END PRIVATE KEY----- +""" + +# The bundle already on disk when a run starts. Same key material, plus a +# trailing marker so "the live file was replaced" and "the old file was +# archived" can be told apart byte-for-byte. +PREVIOUS_BUNDLE = TEST_CERT + TEST_KEY + '# previous bundle\n' +NEW_BUNDLE = TEST_CERT + TEST_KEY +OTHER_BUNDLE = TEST_CERT_2 + TEST_KEY_2 + +# --- stub binaries ----------------------------------------------------------- +STUB_CERTBOT = """\ +#!/bin/sh +# Test stub for certbot: pretend there was nothing to renew. +echo "No renewals were attempted." +exit 0 +""" + +STUB_SOCAT = """\ +#!/bin/sh +# Test stub for socat: record that a reload was attempted, and what was sent. +{ printf 'socat %s <<' "$*"; cat; printf '>>\\n'; } >> "$SOCAT_LOG" +exit 0 +""" + +STUB_HAPROXY = """\ +#!/bin/sh +# Test stub for the haproxy binary: `haproxy -c -f FILE` rejects any config +# containing %(token)s, which is how the tests inject an invalid config. +cfg="" +while [ $# -gt 0 ]; do + case "$1" in -f) cfg="$2"; shift ;; esac + shift +done +if [ -n "$cfg" ] && grep -q '%(token)s' "$cfg" 2>/dev/null; then + echo "[ALERT] parsing [$cfg:1] : unknown keyword '%(token)s'" >&2 + exit 1 +fi +exit 0 +""" % {'token': BROKEN_TOKEN} + +GOOD_HAPROXY_CFG = textwrap.dedent("""\ + global + daemon + defaults + mode http + frontend fe + bind 0.0.0.0:443 ssl crt /etc/haproxy/certs +""") + + +def write(path, content, mode=None): + with open(path, 'w') as fh: + fh.write(content) + if mode is not None: + os.chmod(path, mode) + return path + + +def read(path): + with open(path) as fh: + return fh.read() + + +class CertScriptFixture(unittest.TestCase): + """An isolated fake /etc/haproxy + /etc/letsencrypt plus stub binaries.""" + + def setUp(self): + # Without the library every "the library rejects X" assertion in this + # file can be satisfied by bash exiting 127, so make its absence a + # failure of every test rather than a silent pass of several. + self.assertTrue(os.path.isfile(LIB), + f'{LIB} is missing - nothing below tests anything') + + self.tmp = tempfile.mkdtemp(prefix='haproxy-cert-test-') + self.addCleanup(self._cleanup_tmp) + + self.bindir = os.path.join(self.tmp, 'bin') + os.makedirs(self.bindir) + write(os.path.join(self.bindir, 'certbot'), STUB_CERTBOT, 0o755) + write(os.path.join(self.bindir, 'socat'), STUB_SOCAT, 0o755) + write(os.path.join(self.bindir, 'haproxy'), STUB_HAPROXY, 0o755) + self.socat_log = os.path.join(self.tmp, 'socat-invocations.log') + + # Mirrors the real layout: certs dir, with staging/backups as SIBLINGS. + self.haproxy_dir = os.path.join(self.tmp, 'etc', 'haproxy') + self.certs_dir = os.path.join(self.haproxy_dir, 'certs') + self.staging_dir = os.path.join(self.haproxy_dir, 'cert-staging') + self.backup_dir = os.path.join(self.haproxy_dir, 'cert-backups') + os.makedirs(self.certs_dir) + + self.le_live = os.path.join(self.tmp, 'etc', 'letsencrypt', 'live') + self.domain_dir = os.path.join(self.le_live, DOMAIN) + os.makedirs(self.domain_dir) + self.src_cert = write(os.path.join(self.domain_dir, 'fullchain.pem'), TEST_CERT) + self.src_key = write(os.path.join(self.domain_dir, 'privkey.pem'), TEST_KEY) + + self.live_pem = os.path.join(self.certs_dir, DOMAIN + '.pem') + self.haproxy_cfg = write(os.path.join(self.haproxy_dir, 'haproxy.cfg'), + GOOD_HAPROXY_CFG) + self.log_file = os.path.join(self.tmp, 'haproxy-manager.log') + self.error_log = os.path.join(self.tmp, 'haproxy-manager-errors.log') + + def _cleanup_tmp(self): + # A test may have chmod 000'd a fixture file, which would stop rmtree. + # + # SYMLINKS ARE SKIPPED, and that is not a nicety. os.chmod() FOLLOWS + # symlinks, and the openssl-availability tests below build a stripped + # PATH directory out of symlinks to real system binaries (/usr/bin/cat, + # /usr/bin/chmod, ...). Walking those with chmod 0600 as root - which is + # how this container runs - stripped the exec bit from a dozen core + # binaries of the machine running the tests, chmod itself included, so + # it could not even be undone from inside the container: every later + # test failed with "/usr/bin/grep: Permission denied" and certificate + # publishing stayed dead until the container was recreated. It never + # showed up on a workstation because an unprivileged chmod of a + # root-owned file fails EPERM and was swallowed by `except OSError`. + # This file ships in the image (COPY scripts /haproxy/scripts), so + # running it in place is a thing an operator will do. + for root, dirs, files in os.walk(self.tmp): + for name in files: + path = os.path.join(root, name) + if os.path.islink(path): + continue + try: + os.chmod(path, 0o600) + except OSError: + pass + shutil.rmtree(self.tmp, ignore_errors=True) + + # -- helpers --------------------------------------------------------- + def env(self, **overrides): + env = dict(os.environ) + env.update({ + 'PATH': self.bindir + os.pathsep + os.environ['PATH'], + 'SSL_CERTS_DIR': self.certs_dir, + 'LETSENCRYPT_LIVE_DIR': self.le_live, + 'CERT_STAGING_DIR': self.staging_dir, + 'CERT_BACKUP_DIR': self.backup_dir, + 'LOG_FILE': self.log_file, + 'ERROR_LOG_FILE': self.error_log, + 'HAPROXY_CONFIG': self.haproxy_cfg, + 'SOCAT_LOG': self.socat_log, + }) + env.update(overrides) + return env + + def run_script(self, name, **env_overrides): + path = os.path.join(SCRIPTS_DIR, name) + self.assertTrue(os.path.exists(path), f'{path} does not exist') + return subprocess.run(['bash', path], env=self.env(**env_overrides), + capture_output=True, text=True, timeout=120) + + def bundle_is_valid(self, path): + """Ask the shipped library whether HAProxy could use this bundle.""" + return subprocess.run( + ['bash', '-c', '. "$1"; cert_bundle_valid "$2"', '_', LIB, path], + env=self.env(), capture_output=True, text=True).returncode == 0 + + def reload_attempted(self): + return os.path.exists(self.socat_log) and os.path.getsize(self.socat_log) > 0 + + def logs(self): + text = '' + for path in (self.log_file, self.error_log): + if os.path.exists(path): + text += read(path) + return text + + def seed_previous_bundle(self, content=PREVIOUS_BUNDLE): + return write(self.live_pem, content) + + def assert_certs_dir_is_clean(self): + """HAProxy loads this directory wholesale: only final *.pem may be here.""" + entries = sorted(os.listdir(self.certs_dir)) + strays = [e for e in entries if not e.endswith('.pem')] + self.assertEqual(strays, [], + f'non-.pem files left in the certs directory HAProxy ' + f'loads wholesale: {strays} (dir: {entries})') + + def assert_no_staging_leftovers(self): + if os.path.isdir(self.staging_dir): + self.assertEqual(sorted(os.listdir(self.staging_dir)), [], + 'staging file was not cleaned up') + + def assert_rejected(self, result, because): + """The library refused, FOR THE STATED REASON. + + `assertNotEqual(rc, 0)` on its own proves nothing about this library. + Delete cert-publish-lib.sh and `. "$1"` fails, cert_bundle_valid is + never defined, bash exits 127 - and a bare rc!=0 assertion passes. Four + tests in TestCertPublishLibrary were doing exactly that; they were + pinning "some bash pipeline failed", not "the bundle was rejected". + """ + output = result.stdout + result.stderr + self.assertNotIn('command not found', output, + 'the shell could not find the function under test - ' + 'this asserts nothing about the library') + self.assertNotEqual(127, result.returncode, + f'exit 127 means "no such command", not "rejected": {output}') + self.assertNotEqual(0, result.returncode, + f'expected a rejection, got success: {output}') + self.assertIn(because, output, + f'rejected, but not for the expected reason ' + f'({because!r} not in output): {output}') + + +class CertScriptBehaviour: + """Behaviour shared by renew-certificates.sh and sync-certificates.sh. + + A mixin rather than a TestCase so the cases are collected once per concrete + script, not a third time for the base class. + """ + + SCRIPT = None + + def run_it(self, **env_overrides): + return self.run_script(self.SCRIPT, **env_overrides) + + # -- happy path ------------------------------------------------------ + def test_publish_replaces_live_pem_and_archives_the_previous_one(self): + self.seed_previous_bundle() + result = self.run_it() + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(read(self.live_pem), NEW_BUNDLE, + 'live pem is not the newly assembled cert+key') + backup = os.path.join(self.backup_dir, DOMAIN + '.pem') + self.assertTrue(os.path.exists(backup), + f'previous bundle was not archived to {backup}') + self.assertEqual(read(backup), PREVIOUS_BUNDLE, + 'the archived bundle is not the one that was replaced') + self.assertIn('1 updated, 0 failed', self.logs()) + self.assertTrue(self.reload_attempted(), + 'HAProxy was never reloaded after a successful update') + self.assert_certs_dir_is_clean() + self.assert_no_staging_leftovers() + + def test_first_publish_works_with_no_previous_bundle(self): + result = self.run_it() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(read(self.live_pem), NEW_BUNDLE) + self.assert_certs_dir_is_clean() + + # -- THE HEADLINE ---------------------------------------------------- + def test_unreadable_source_key_leaves_the_previous_bundle_intact(self): + """The bug this whole change exists for. + + Pre-fix: `cat cert key > live.pem` truncated live.pem before cat ran, + so an unreadable key left a key-less (or empty) pem in the directory + HAProxy loads wholesale -> the :443 bind fails -> every site on the + host loses HTTPS. Checking cat's exit status did not undo that. + """ + self.seed_previous_bundle() + before = read(self.live_pem) + self.assertTrue(self.bundle_is_valid(self.live_pem), + 'fixture precondition: the seeded bundle must be valid') + + how = self.break_source_key() + + result = self.run_it() + + self.assertEqual(read(self.live_pem), before, + f'the live pem was damaged by a failed publish ({how}); ' + f'HAProxy would fail to load the certs directory') + self.assertTrue(self.bundle_is_valid(self.live_pem), + 'the pem left on disk is no longer a usable bundle') + logs = self.logs() + self.assertIn(f'Failed to combine certificate for {DOMAIN}', logs) + self.assertIn('0 updated, 1 failed', logs) + self.assertFalse(self.reload_attempted(), + 'HAProxy was reloaded even though nothing was updated') + self.assert_certs_dir_is_clean() + self.assert_no_staging_leftovers() + # This used to assert returncode == 0 with the comment "a per-domain + # failure should not change the exit code", codifying the script's + # `exit 0`. That is wrong and it is the dangerous kind of wrong: a host + # where EVERY domain fails to publish looked, to cron and to + # host-renew-certificates.sh (which branches on this exit code), + # exactly like a clean run. Nothing would notice until the certificates + # expired. Continuing past a failed domain so the others still get + # published is right; reporting success afterwards is not. + self.assertNotEqual(result.returncode, 0, + 'a domain that failed to publish must be reported ' + 'in the exit code, not just in the log') + + def break_source_key(self): + """Make reading the source key fail, however this environment allows. + + chmod 000 is the faithful reproduction (file present, `-f` true, cat + fails), but it is a no-op for root, so as root we truncate the key + instead: pre-fix that is even nastier, because `cat` then *succeeds* + and silently publishes a key-less pem. + """ + if os.geteuid() == 0: + write(self.src_key, '') + return 'zero-length source key (running as root)' + os.chmod(self.src_key, 0o000) + return 'unreadable source key (chmod 000)' + + # -- other ways to end up with an unusable bundle -------------------- + def test_cert_only_bundle_is_rejected(self): + self.seed_previous_bundle() + before = read(self.live_pem) + write(self.src_key, TEST_CERT) # no private key block at all + + self.run_it() + + self.assertEqual(read(self.live_pem), before, + 'a key-less bundle was published over the live pem') + self.assertIn('0 updated, 1 failed', self.logs()) + self.assertFalse(self.reload_attempted()) + self.assert_certs_dir_is_clean() + self.assert_no_staging_leftovers() + + def test_truncated_certificate_block_is_rejected(self): + self.seed_previous_bundle() + before = read(self.live_pem) + write(self.src_cert, TEST_CERT.split('\n')[0] + '\nMIIDFzCCAf+gAwIBA\n') + + self.run_it() + + self.assertEqual(read(self.live_pem), before, + 'a truncated certificate was published over the live pem') + self.assertIn('0 updated, 1 failed', self.logs()) + self.assert_certs_dir_is_clean() + + def test_mismatched_key_is_rejected(self): + if shutil.which('openssl') is None: + self.skipTest('openssl CLI not available: without it every publish ' + 'is refused, so this test could not tell a pairing ' + 'rejection from a missing-checker rejection') + self.seed_previous_bundle() + before = read(self.live_pem) + write(self.src_key, UNRELATED_KEY) + + self.run_it() + + self.assertEqual(read(self.live_pem), before, + 'a bundle whose key does not match the cert was published') + self.assertIn('does not match the certificate', self.logs()) + self.assertIn('0 updated, 1 failed', self.logs()) + self.assert_certs_dir_is_clean() + + # -- the certs directory is HAProxy's, not ours ---------------------- + def test_no_stray_files_in_certs_dir_after_success_or_failure(self): + self.seed_previous_bundle() + self.run_it() + self.assert_certs_dir_is_clean() + self.assertEqual(sorted(os.listdir(self.certs_dir)), [DOMAIN + '.pem']) + + self.break_source_key() + self.run_it() + self.assert_certs_dir_is_clean() + self.assertEqual(sorted(os.listdir(self.certs_dir)), [DOMAIN + '.pem']) + self.assert_no_staging_leftovers() + + def test_staging_and_backup_dirs_are_outside_the_certs_dir(self): + """Belt and braces: even with the defaults, nothing lands under certs/.""" + self.seed_previous_bundle() + # Drop the explicit overrides so the derived defaults are exercised. + env = {'CERT_STAGING_DIR': '', 'CERT_BACKUP_DIR': ''} + self.run_it(**env) + + self.assert_certs_dir_is_clean() + self.assertEqual(sorted(os.listdir(self.certs_dir)), [DOMAIN + '.pem']) + self.assertTrue( + os.path.exists(os.path.join(self.haproxy_dir, 'cert-staging')), + 'default staging dir is not the documented sibling of the certs dir') + self.assertTrue( + os.path.exists(os.path.join(self.haproxy_dir, 'cert-backups', + DOMAIN + '.pem')), + 'default backup dir is not the documented sibling of the certs dir') + + # -- reload gating --------------------------------------------------- + def test_reload_is_not_attempted_when_haproxy_config_is_invalid(self): + write(self.haproxy_cfg, GOOD_HAPROXY_CFG + BROKEN_TOKEN + '\n') + + result = self.run_it() + + self.assertFalse(self.reload_attempted(), + 'HAProxy was reloaded with a configuration that ' + '`haproxy -c` rejects') + self.assertNotEqual(result.returncode, 0, + 'refusing to reload must be a loud, non-zero exit') + self.assertIn('does not validate', self.logs()) + + def test_reload_happens_when_the_config_validates(self): + self.run_it() + self.assertTrue(self.reload_attempted()) + self.assertIn('reload', read(self.socat_log)) + + def test_reload_is_not_attempted_when_nothing_was_updated(self): + shutil.rmtree(self.domain_dir) + result = self.run_it() + self.assertEqual(result.returncode, 0) + self.assertFalse(self.reload_attempted()) + + +class TestRenewCertificates(CertScriptBehaviour, CertScriptFixture): + SCRIPT = 'renew-certificates.sh' + + +class TestSyncCertificates(CertScriptBehaviour, CertScriptFixture): + SCRIPT = 'sync-certificates.sh' + + +class TestCertPublishLibrary(CertScriptFixture): + """Unit-level checks on cert-publish-lib.sh itself.""" + + def call(self, snippet, *args, **env_overrides): + return subprocess.run( + ['bash', '-c', '. "$1"; shift; ' + snippet, '_', LIB, *args], + env=self.env(**env_overrides), capture_output=True, text=True) + + def test_valid_bundle_accepted(self): + path = write(os.path.join(self.tmp, 'ok.pem'), NEW_BUNDLE) + self.assertEqual(self.call('cert_bundle_valid "$1"', path).returncode, 0) + + def test_empty_and_missing_bundles_rejected(self): + empty = write(os.path.join(self.tmp, 'empty.pem'), '') + self.assert_rejected(self.call('cert_bundle_valid "$1"', empty), + 'is empty') + missing = os.path.join(self.tmp, 'nope.pem') + self.assert_rejected(self.call('cert_bundle_valid "$1"', missing), + 'does not exist') + + def test_key_without_end_marker_rejected(self): + truncated = write(os.path.join(self.tmp, 'cut.pem'), + TEST_CERT + '-----BEGIN PRIVATE KEY-----\nMIIEvAIB\n') + self.assert_rejected(self.call('cert_bundle_valid "$1"', truncated), + 'unterminated private key block') + + def test_empty_pem_blocks_are_rejected(self): + """Why the pairing check is mandatory rather than best-effort. + + Every structural check in the library passes on this file: a complete + CERTIFICATE block and a complete PRIVATE KEY block, both with nothing + between BEGIN and END. Only openssl can tell it is not a certificate. + """ + hollow = write(os.path.join(self.tmp, 'hollow.pem'), + '-----BEGIN CERTIFICATE-----\n' + '-----END CERTIFICATE-----\n' + '-----BEGIN PRIVATE KEY-----\n' + '-----END PRIVATE KEY-----\n') + self.assert_rejected(self.call('cert_bundle_valid "$1"', hollow), + 'openssl could not read the certificate') + + def test_a_broken_live_pem_does_not_overwrite_a_good_backup(self): + """Mirrors create_backup(require_valid=True) in haproxy_manager.py. + + If the pem currently on disk is already garbage, archiving it would + replace a restorable backup with an unusable one. + """ + os.makedirs(self.backup_dir) + good_backup = write(os.path.join(self.backup_dir, DOMAIN + '.pem'), + PREVIOUS_BUNDLE) + write(self.live_pem, 'garbage, not a pem at all\n') + + result = self.call('cert_publish "$1" "$2" "$3"', + self.src_cert, self.src_key, self.live_pem) + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(read(self.live_pem), NEW_BUNDLE) + self.assertEqual(read(good_backup), PREVIOUS_BUNDLE, + 'a good backup was overwritten with an unusable pem') + + def test_publish_fails_loudly_when_the_rename_cannot_happen(self): + """No silent fallback to writing straight into the certs dir. + + The failure is injected with a stub `mv` that refuses, rather than by + chmod 0500 on the certs dir: the container these scripts run in is + root, root ignores directory permissions, so the chmod version skipped + itself exactly where it matters and only ever ran on a workstation. + """ + self.seed_previous_bundle() + before = read(self.live_pem) + write(os.path.join(self.bindir, 'mv'), + "#!/bin/sh\n" + "echo \"mv: cannot move '$2': Permission denied\" >&2\n" + "exit 1\n", 0o755) + self.addCleanup(os.unlink, os.path.join(self.bindir, 'mv')) + + result = self.call('cert_publish "$1" "$2" "$3"', + self.src_cert, self.src_key, self.live_pem) + + self.assert_rejected(result, 'NOT falling back to a direct write') + self.assertEqual(read(self.live_pem), before, + 'the live pem was damaged by a failed rename') + self.assert_no_staging_leftovers() + + def test_published_pem_keeps_the_mode_of_the_file_it_replaces(self): + """A write-safety fix must not silently re-permission private keys. + + Both directions matter: mktemp stages at 0600, so without the explicit + chmod every publish would tighten a 0644 bundle; and the mode must not + be copied from a symlink (see the next test). + """ + for mode in (0o644, 0o640, 0o600): + with self.subTest(oct(mode)): + write(self.live_pem, PREVIOUS_BUNDLE, mode) + result = self.call('cert_publish "$1" "$2" "$3"', + self.src_cert, self.src_key, self.live_pem) + self.assertEqual(result.returncode, 0, + result.stdout + result.stderr) + self.assertEqual(read(self.live_pem), NEW_BUNDLE) + self.assertEqual( + stat.S_IMODE(os.stat(self.live_pem).st_mode), mode, + 'publishing changed who can read the private key') + + def test_symlinked_live_pem_does_not_become_world_writable(self): + """`stat -c %a` on a symlink reports 0777 - the LINK's mode, not a + permission. Copying that onto the staged bundle put a world-writable + private key in the directory HAProxy serves from. -L is what makes the + preserved mode the mode of the file an operator actually chose. + """ + target = write(os.path.join(self.tmp, 'real-bundle.pem'), + PREVIOUS_BUNDLE, 0o640) + os.symlink(target, self.live_pem) + + result = self.call('cert_publish "$1" "$2" "$3"', + self.src_cert, self.src_key, self.live_pem) + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + mode = stat.S_IMODE(os.stat(self.live_pem).st_mode) + self.assertEqual( + 0, mode & 0o022, + f'published bundle is group/world writable ({oct(mode)}) - the ' + f'symlink mode was copied onto a real private key') + self.assertEqual(0o640, mode) + + def test_publish_refuses_when_staging_is_on_another_filesystem(self): + """The header used to claim a cross-device mv "fails loudly and leaves + the live pem alone". GNU mv does no such thing: across filesystems it + copies, so it truncates and writes the DESTINATION first and only then + discovers it cannot finish (ENOSPC being the realistic case) - the very + truncation this library exists to prevent. Both directories are + env-overridable, so the device numbers have to be checked up front. + """ + staging = os.path.join(self.a_dir_on_another_filesystem(), + 'cert-staging') + self.seed_previous_bundle() + before = read(self.live_pem) + + result = self.call('cert_publish "$1" "$2" "$3"', + self.src_cert, self.src_key, self.live_pem, + CERT_STAGING_DIR=staging) + + self.assert_rejected(result, 'different filesystems') + self.assertEqual(read(self.live_pem), before, + 'the live pem was disturbed by a refused publish') + self.assert_certs_dir_is_clean() + + def test_stale_python_side_staging_temps_are_reaped(self): + """The staging dir has two writers. + + write_config_atomically() on the Python side stages as + `..tmp` (tempfile.mkstemp(prefix=name + '.', + suffix='.tmp')). The reaper matched only mktemp's `*.??????` shape, so + every temp leaked by a SIGKILL on the Python side stayed there forever. + """ + os.makedirs(self.staging_dir, exist_ok=True) + stale_py = write(os.path.join(self.staging_dir, + DOMAIN + '.pem.ab12cd34.tmp'), 'stale\n') + stale_sh = write(os.path.join(self.staging_dir, + DOMAIN + '.pem.AbCdEf'), 'stale\n') + fresh = write(os.path.join(self.staging_dir, + 'recent.pem.zz99yy.tmp'), 'fresh\n') + old = time.time() - 3 * 24 * 3600 + for path in (stale_py, stale_sh): + os.utime(path, (old, old)) + + result = self.call('cert_publish "$1" "$2" "$3"', + self.src_cert, self.src_key, self.live_pem) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + self.assertFalse(os.path.exists(stale_py), + 'a stale Python-side staging temp was never reaped') + self.assertFalse(os.path.exists(stale_sh), + 'a stale shell-side staging temp was never reaped') + self.assertTrue(os.path.exists(fresh), + 'the reaper deleted a temp a concurrent publisher may ' + 'still be writing') + + def test_validation_of_a_live_pem_being_republished_is_not_spurious(self): + """cert_bundle_valid must judge ONE snapshot of the file. + + cert_publish() calls cert_bundle_valid() on the LIVE pem (step (e), to + decide whether it is worth backing up) while another publisher may be + renaming a new bundle over it. The function used to open the file six + times, so `openssl x509` could read the outgoing bundle and `openssl + pkey` the incoming one - and report "private key does not match the + certificate" about two files that were each perfectly fine. That ERROR + goes into the log monitor-errors.sh watches, which makes it a page. + + The two bundles alternated below are each internally valid but carry + DIFFERENT key material. That matters: NEW_BUNDLE and PREVIOUS_BUNDLE + share a cert and a key, so alternating those two could never produce a + mismatch no matter how badly the reads were interleaved - the test + would model a world in which the bug cannot happen and pass forever. + """ + write(self.live_pem, NEW_BUNDLE) + a = write(os.path.join(self.tmp, 'churn-a.pem'), NEW_BUNDLE) + b = write(os.path.join(self.tmp, 'churn-b.pem'), OTHER_BUNDLE) + + # A publisher renaming over the live pem as fast as it can. Staged + # outside the certs dir, then renamed, exactly like cert_publish(). + churn = subprocess.Popen( + ['bash', '-c', + 'end=$((SECONDS+8)); s="$4"; while [ $SECONDS -lt $end ]; do ' + ' cp "$1" "$s"; mv -f "$s" "$2"; ' + ' cp "$3" "$s"; mv -f "$s" "$2"; ' + 'done', '_', a, self.live_pem, b, + os.path.join(self.tmp, 'churn-staged.pem')], + env=self.env(), stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + def _stop(): + churn.kill() + churn.wait() + self.addCleanup(_stop) + + result = subprocess.run( + ['bash', '-c', + '. "$1"; for i in $(seq 1 200); do cert_bundle_valid "$2" || exit 1; done', + '_', LIB, self.live_pem], + env=self.env(), capture_output=True, text=True, timeout=120) + _stop() + + output = result.stdout + result.stderr + self.assertNotIn('does not match the certificate', output, + 'two valid bundles were reported as a mismatched pair ' + 'because the checks read different files') + self.assertEqual(0, result.returncode, + f'a concurrent republish made validation fail: {output}') + + def a_dir_on_another_filesystem(self): + certs_dev = os.stat(self.certs_dir).st_dev + for candidate in ('/dev/shm', '/run', '/var/tmp', '/tmp', '/'): + try: + if (os.path.isdir(candidate) + and os.access(candidate, os.W_OK) + and os.stat(candidate).st_dev != certs_dev): + path = tempfile.mkdtemp(prefix='cert-xdev-', dir=candidate) + self.addCleanup(shutil.rmtree, path, True) + return path + except OSError: + continue + self.skipTest('no writable directory on a second filesystem available') + + def test_haproxy_config_ok_follows_the_validator(self): + self.assertEqual(self.call('haproxy_config_ok').returncode, 0) + write(self.haproxy_cfg, GOOD_HAPROXY_CFG + BROKEN_TOKEN + '\n') + self.assertNotEqual(self.call('haproxy_config_ok').returncode, 0) + + def _openssl_free_path(self, name): + """A PATH directory with the library's tools but no openssl. + + Symlinks, so this directory must never be walked with a chmod that + follows them - see _cleanup_tmp(). + """ + fake_path = os.path.join(self.tmp, name) + os.makedirs(fake_path) + for tool in ('cat', 'grep', 'mktemp', 'mv', 'cp', 'rm', 'mkdir', + 'basename', 'dirname', 'find', 'date', 'chmod', 'stat'): + real = shutil.which(tool) + if real: + os.symlink(real, os.path.join(fake_path, tool)) + return fake_path + + def test_missing_openssl_is_a_hard_failure(self): + """The pairing check is MANDATORY: no openssl, no publication. + + This used to assert the opposite - that a missing openssl warns and + publishes anyway - justified by "the image does not necessarily install + the openssl CLI". The image does: openssl 3.x arrives with + ca-certificates, which certbot needs, and generate_self_signed_cert() + already runs `openssl req` with check=True at first-run setup. So the + fail-open never actually fired, and structural checks alone accept a + bundle of empty pem blocks (see test_empty_pem_blocks_are_rejected). + """ + fake_path = self._openssl_free_path('no-openssl-bin') + path = write(os.path.join(self.tmp, 'ok.pem'), NEW_BUNDLE) + + # bash by absolute path: the stripped PATH cannot resolve it. + result = subprocess.run( + [shutil.which('bash'), '-c', '. "$1"; cert_bundle_valid "$2"', + '_', LIB, path], + env=self.env(PATH=fake_path), capture_output=True, text=True) + + self.assert_rejected(result, 'openssl binary not found') + self.assertIn('REFUSING', result.stdout + result.stderr, + 'a broken image must be reported as a broken image') + + def test_missing_openssl_stops_a_publish_rather_than_weakening_it(self): + """cert_publish must inherit the refusal, and not touch the live pem.""" + fake_path = self._openssl_free_path('no-openssl-bin2') + self.seed_previous_bundle() + before = read(self.live_pem) + + result = subprocess.run( + [shutil.which('bash'), '-c', + '. "$1"; cert_publish "$2" "$3" "$4"', + '_', LIB, self.src_cert, self.src_key, self.live_pem], + env=self.env(PATH=fake_path), capture_output=True, text=True) + + self.assert_rejected(result, 'openssl binary not found') + self.assertEqual(read(self.live_pem), before, + 'the live pem was disturbed by a refused publish') + self.assert_certs_dir_is_clean() + self.assert_no_staging_leftovers() + + +class TestScriptsAreSane(unittest.TestCase): + """Cheap static guards against the failure mode coming back.""" + + SHELL_FILES = ('renew-certificates.sh', 'sync-certificates.sh', + 'cert-publish-lib.sh') + + def test_shell_files_parse(self): + for name in self.SHELL_FILES: + path = os.path.join(SCRIPTS_DIR, name) + result = subprocess.run(['bash', '-n', path], + capture_output=True, text=True) + self.assertEqual(result.returncode, 0, + f'{name}: {result.stderr}') + + def test_no_script_redirects_into_the_live_pem(self): + pattern = re.compile(r'>\s*"?\$\{?COMBINED_FILE') + for name in ('renew-certificates.sh', 'sync-certificates.sh'): + body = read(os.path.join(SCRIPTS_DIR, name)) + self.assertIsNone(pattern.search(body), + f'{name} still redirects output straight into the ' + f'live pem HAProxy is serving') + + +if __name__ == '__main__': + print(f'testing scripts from: {SCRIPTS_DIR}') + unittest.main(verbosity=2) diff --git a/scripts/test-cert-write-safety.py b/scripts/test-cert-write-safety.py new file mode 100755 index 0000000..9a94ea0 --- /dev/null +++ b/scripts/test-cert-write-safety.py @@ -0,0 +1,938 @@ +#!/usr/bin/env python3 +"""Regression tests for certificate bundle publishing. + +Why this file exists +-------------------- +Every code path that refreshed a combined PEM used to do this: + + with open(combined_path, 'w') as combined: # TRUNCATES + subprocess.run(['cat', cert, key], stdout=combined) # rc ignored + +`combined_path` is the live bundle HAProxy is serving. open(..., 'w') empties it +BEFORE any source material has been read, and the `cat` exit status was never +checked, so a half-written certbot lineage, an unreadable source or a full disk +left a truncated or key-less PEM in place. HAProxy loads /etc/haproxy/certs as a +directory and refuses to start if any file in it is unusable, so that is HTTPS +down for every site on the host - and unlike a broken haproxy.cfg it is not +recoverable by config rollback. + +The bundle endpoint made it worse: it deleted superseded .pem files AND ran +`certbot delete` on their lineages before anything had checked that the +replacement was usable, destroying both copies of a working certificate. +Recovery there means fresh, rate-limited ACME orders. + +These tests pin the invariants: + * a failed publish leaves the previously served bundle byte-for-byte intact + and the edge still able to start; + * nothing is published that is not a complete, validated cert+key pair; + * no old certificate file is removed and no lineage deleted until the + replacement is validated, in place, and actually loaded by HAProxy; + * only final .pem files ever exist in the crt directory. + +Running +------- + python3 scripts/test-cert-write-safety.py # tests the repo checkout + HAPROXY_MANAGER_DIR=/some/other/tree \ + python3 scripts/test-cert-write-safety.py # tests another tree + +Pointing HAPROXY_MANAGER_DIR at a pre-fix checkout is how the bugs above were +reproduced: the behavioural tests run there too (the fix-only tests skip +themselves), and they fail. + +Same conventions as scripts/test-config-rollback.py: self-contained stdlib +unittest, no pytest/venv/extra dependencies, stub binaries on PATH. The +certificate material below is real (a self-signed leaf plus its matching key, +and one unrelated key for the mismatch case) and embedded as constants so the +suite needs no crypto tooling to create fixtures. +""" + +import logging +import os +import shutil +import sqlite3 +import stat +import subprocess +import sys +import tempfile +import textwrap +import unittest + +BROKEN_TOKEN = '__BROKEN__' + +MODULE_DIR = os.path.abspath( + os.environ.get('HAPROXY_MANAGER_DIR', + os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')) +) + +# haproxy_manager builds its Jinja2 environment from the relative path +# Path('templates'), so it has to be imported with the module dir as cwd. +os.chdir(MODULE_DIR) +sys.path.insert(0, MODULE_DIR) + +# The module opens /var/log/haproxy-manager.log at import time via +# logging.FileHandler. Redirect that one call so the suite runs unprivileged. +_LOG_DIR = tempfile.mkdtemp(prefix='haproxy-mgr-test-logs-') +_real_file_handler = logging.FileHandler +logging.FileHandler = ( + lambda fn, *a, **kw: _real_file_handler( + os.path.join(_LOG_DIR, os.path.basename(fn)), *a, **kw) +) +try: + import haproxy_manager as hm +except ImportError as exc: # pragma: no cover - environment problem, not a failure + sys.stderr.write( + f"SKIP: cannot import haproxy_manager ({exc}).\n" + "Install the application requirements first: pip install -r requirements.txt\n" + ) + raise SystemExit(77) +finally: + logging.FileHandler = _real_file_handler + +logging.getLogger('haproxy_manager').setLevel(logging.CRITICAL) + +HAS_PUBLISHER = hasattr(hm, 'publish_pem_bundle') + +# FIX_ONLY marks tests that can only run against a tree that HAS the fix, so +# that pointing HAPROXY_MANAGER_DIR at a pre-fix checkout (how the bugs were +# reproduced) skips them instead of erroring. +# +# It used to be `skipUnless(HAS_PUBLISHER, ...)` unconditionally, which is +# tautological when testing THIS tree: rename or delete publish_pem_bundle and +# 10 of the 17 tests silently skip themselves while the run still reports OK. +# A guard that disappears when the thing it guards disappears is not a guard. +# So the escape hatch is now tied to the thing it exists for - testing a +# FOREIGN tree - and a missing publisher in the repo checkout is a hard +# failure (see TestPublisherApiIsPresent). +_REPO_ROOT = os.path.realpath( + os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')) +TESTING_FOREIGN_TREE = os.path.realpath(MODULE_DIR) != _REPO_ROOT +FIX_ONLY = unittest.skipIf( + TESTING_FOREIGN_TREE and not HAS_PUBLISHER, + 'HAPROXY_MANAGER_DIR points at a tree without the certificate publishing ' + 'fix (publish_pem_bundle)') +NEEDS_OPENSSL = unittest.skipUnless( + shutil.which('openssl'), 'needs the openssl CLI') + +# --- real test material ----------------------------------------------------- +# Self-signed leaf, CN=test.example.com, SAN test.example.com + +# www.test.example.com, valid until 2126. +LEAF_CERT = """\ +-----BEGIN CERTIFICATE----- +MIIDTjCCAjagAwIBAgIUTliK3dNIdYS3i7R3yxNMHfxVWTcwDQYJKoZIhvcNAQEL +BQAwGzEZMBcGA1UEAwwQdGVzdC5leGFtcGxlLmNvbTAgFw0yNjA4MDYxNTQxMjda +GA8yMTI2MDcxMzE1NDEyN1owGzEZMBcGA1UEAwwQdGVzdC5leGFtcGxlLmNvbTCC +ASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMRuorQDGdsyc/rk3WST8J70 +9oVcTz2QSTQ5QNxqa4jKX7vD0YdLK2Kz7TKLbR//ju+dnqFhLTAs38KqljmU5M1d +7hC1frSV9Y9heHTa51fc9hxewDl9535TdIsUga5HT+sc7q3Np7RparOf1NOm/aBd +27j+LGKclbJ5YaU3I39S05H+S8mgmFpLezwQ7uzFomkk/E5deUcqGpJrW8k2t6Uv +jvfejB8FGAM6DxEL4yTjizmIaJE+jadPTJRq1TtHG+LE4rt2UpF4ggNFXlK7xXfp +p0mcIE1b3MwGaPGmAe5Vw2+nl4uD9LRgQh3XxO2+as4QTTYhlR40Fn03ksXVo6cC +AwEAAaOBhzCBhDAdBgNVHQ4EFgQUjAt+yg/dpNHyeWNqH2oAURfW3iUwHwYDVR0j +BBgwFoAUjAt+yg/dpNHyeWNqH2oAURfW3iUwDwYDVR0TAQH/BAUwAwEB/zAxBgNV +HREEKjAoghB0ZXN0LmV4YW1wbGUuY29tghR3d3cudGVzdC5leGFtcGxlLmNvbTAN +BgkqhkiG9w0BAQsFAAOCAQEABo5x4n3/61XxwJEkNPyv/mCAN5t/+NrMxfadRFJK ++jBxtOO8w8vhi21zF1NDVQkt2bt69QGrVleP5X78FaIaI6sJpLKDE1nOyE9Dpt4y +mnLoRi0Ep7NaDV6rHmfbokLkVdd4Z9RKUESnYCc1Zt5x82oGhEe3GJ4ej2HS8sGY +r79qGVEhQIbLPDA3PD+RQCF6+xNU2CVgZUJ7ZtSeAaNaQQqTRUT2qCBvIKfV8fMS +VDmV6/YORV2jTzO3odKsKxXQY8oWCzfwosxD0dJ2zbZWvMAqcQQq3d/iuH8NiS4Y ++zZ6j9KI/RdRYJz/co2FKAy3FfQ/gZo8eWF2gf9RUDgNQg== +-----END CERTIFICATE----- +""" + +LEAF_KEY = """\ +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDEbqK0AxnbMnP6 +5N1kk/Ce9PaFXE89kEk0OUDcamuIyl+7w9GHSytis+0yi20f/47vnZ6hYS0wLN/C +qpY5lOTNXe4QtX60lfWPYXh02udX3PYcXsA5fed+U3SLFIGuR0/rHO6tzae0aWqz +n9TTpv2gXdu4/ixinJWyeWGlNyN/UtOR/kvJoJhaS3s8EO7sxaJpJPxOXXlHKhqS +a1vJNrelL4733owfBRgDOg8RC+Mk44s5iGiRPo2nT0yUatU7RxvixOK7dlKReIID +RV5Su8V36adJnCBNW9zMBmjxpgHuVcNvp5eLg/S0YEId18TtvmrOEE02IZUeNBZ9 +N5LF1aOnAgMBAAECggEAR0NcA7KcTsmfCga9yx9gzEpSpU838D3IUQn0XgK9wIKq ++JOyEENVGhnsk8nBbTppwMSOKD35BuFAzH7WwU0jNN4+4BD4RsugqsPRz5MbGuUu +5Fv7oN/sfAgK3+owoel9NO7qKGPT07/q1f/GVoLewK9MZ3DO6XelV3px0l6OokHn +Oy+ShWw4CnUXRIAKBuRNLLA0HQ4ld0dceV/kT428tZIluj9pkUhnp9xux5yDWkMu +dj26CXZxp2PMzVLD21TmI/BZO+3Ev+Nf5GaS8GrmROlXLKTE5YDmU1ZTECsSSFLK +Nuw7B3TT+xEQUf5lokDigkbTgIQ5kpnIy1LSbqLAAQKBgQDudSbhaqABZk6B09UL +PZVQZSrGZDXl4vwnkHXArrr2BRz1+B67tWebq3V0pCrZqwFc9eOtq1F0vH6q11Ns +vLZ9lZ7KUEsFS6InZkqSQSGrgWt4MIjocc8qHLlaEk/EGgHgtwBblm2hEyDwy487 +jhnc2wI82D6ZLvIc27pQQFNjAQKBgQDS4gXINPsaq9YFiQ9JQG8oJGSq6yn137wJ +DQYrTaB9tqcmIXWx6ZgMiKZiOGLQYVmlNFoh10cTxbebWjXjWL4VP3gg5nE87pQz +8KKvkXP07QWiSQLV5SJzEeDM/l3Lkyc9O69PYnusWjdFOrIPDTmxZ2l6wNyAVxpm +zEK17gYOpwKBgQC+glxAxZX16E2ajanspBPRujG1dMRW2MTJuzFIcpCuEyGzJbsw +DlsrVI2vVaViZ6vcIBr5WiDm2d19EjD1c8N8i/fj/Mgi/+0Z+zBirqR+yBQbXvNS +efKf23j+DBksO/b6GFqx0XnesVCk8IyLcRkaiOK9x6ojag1Gnwm4Kdw1AQKBgAen +ssQIwFDAih1bU1W6ZA6V+52Eudo2C/JcKawqvjeyCLFGp6oUq7NQxpFsMJIV5pYr +p1XxJaBfHgIirTAaiZPl4Ot40gV/N5wHETDEW+w5Kmowskyna6+3p2xpk2gPaG49 +m2iLT6f7AmSd89a+CSkacubE13xFLS0sHwPRpyCjAoGBAJfFYXWFAD2viAmBshCS +g6Ba78n0vkF74/DnFRWpIqxw8vufQ5nY/k67tGko+zZg3jzI1d1REvI1qHYctKFM +Ko7fmF3ny803cdJT8EuIlrU2+V5lh0GkPO/or+68qso+qBj3R8f6jRLbahkUbY5e +yR3PzoVVfGaftvLsdbAqD+VC +-----END PRIVATE KEY----- +""" + +# A perfectly valid key that simply does not belong to LEAF_CERT. +UNRELATED_KEY = """\ +-----BEGIN PRIVATE KEY----- +MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQDBpZAkTBLhiDnq +cCjsSsBwAW/fhYYfx+t2iauYPrvaLmiJHFkXEr7gaBlYWTKJRISY2jqYTM1RdLJB +DDjNThrewOLtcF7d+k+ArMPWXxqBotbCTKMkh7djRfnXEcjJ/mil31W351cR/11q +EPBdCcj4BYegBzc4GG2qIh/Nww+jd1KkkxUhnTMPp5Ie0myJafh0Pdsss9lbmqBH +fW6BO341C9jG8N/7C7FHCzzf74q7mf0Bx+isYgW/1YL0Ndg/R8yTRVKTbPvU3ZM2 +hEeLDXFbvcceREq1j0VEXGd9rcB2JHBE4TAQZq9WmeT51uynW8a1hnUcUs/li7jL +cuI6DswxAgMBAAECggEADeC4Z42HJeAeLHG80RBbYbuMobN/RPxOIOjlYgwO6Og+ +CCN+tANdKBZ1yInd8A33xb+QBvWsGjwXgUdns7j2/oNK0BLnTZfAhlt7Tnvy2ZsK +spKM95N9XlE3wkTNQ8Kmi8qpaTxcVlcbgfw0SaqnmzTEP0D9IVlI1LJM3rFtx7xn +xhrtKzhcv6WbnZKjrKlPpJq7dFC8+3WtbWFBvWFUTs2mQJ3eFWlc/EBh60PKuMAz +44aMrypzQSdcAs0+f0fOpnhpWa6Px4uEAIvSNPjPI7gt/7v4qb+fNrGGYrou9Pd9 +hW0MGDOxfTxr094YCdapklIV3OyCvrCeS8XVYnVddQKBgQDiZzBTNnc6O2EWgY2h +82VSEBMDTnk+rT7CTx4RGkzyq2z6oUg0itr+TQGhyjRX24VbROXtXlCBNXQkn9UA +aZsss+KnrF6KLEmdwQIoNKSgBIiTgX/PHmkf3a+scDShgL5aIItjazu9TEblBqKF ++H4eBwuPjKc20h2CctoZswE0HQKBgQDa9iZBOPCjyTtq4oSeJbyvjYfJN5EoI9nZ +hl0Yqa8ajbJ8nyxGziy5z5ktqBFYiVa53xamagJJp69DCmnm6vSy01KZIOtKNEKb +PCaNc1Lp+cf5SEIHX0Pakx/zmi0PzDx1V0DhtjLhF8dFUfQhWvvxF+m16LZeBYZ+ +0UP6NRAUJQKBgQCM/e/1UkzrocDzkBiQy4/EjCga/gq5gpA716OEyRk0YpdKeZgK +yJJancAvbkoskJO64+xAZ2TBInXCvRqb2Ch/rUKwYsK5T51EtcbPHQGMeWZIXfQn +GuwioR7exz2vegqQ/AVyE3yvhUn9JKWfwsFfl8mWSuRzWmRwMXArYvOT7QKBgQCV +6Y2LfjaTjMUXivsNY/zpnNbo1xiVCOawXaQDrLlsTrNzS29/Es3gcdgIQFeP7Ifq +Pmk9irsCPsJp/gk/xoG+pZyZpsYxSdKIgghLNDgCZbeaXvSGI51LWwu3N0m+1TBX +jmOnpZz0K9mNBm1FIQv5p0ul9ixV9yZ8UT5fYlEd2QKBgQDLWMgMD9rOORl+0s8Z +RcpSfu2E7KP0e2DaxP4dYRmUyNuE8iK8hglNqqVBLhrFHmDZ1yo1lFIDBROaJ9EW +pAxSb8kRi2dba8ZnpEDhhiqoDwkoXRDOrzd5bdHyiOJV39U1F5U6HqDE4URaq8iu +k49ABlblWHBsoUF63ka1PBrMgA== +-----END PRIVATE KEY----- +""" + +GOOD_BUNDLE = LEAF_CERT + LEAF_KEY + +# What a bundle already on disk looks like when a test starts: the same, valid +# material plus a trailing marker. Without it the "previous" and "new" bundles +# are byte-identical, and a test that meant to prove a publish HAPPENED cannot +# tell that from a publish that did nothing at all. Trailing text after the key +# is ignored by both HAProxy and openssl, so the file stays genuinely usable. +PREVIOUS_MARKER = '# previous bundle\n' +PREVIOUS_BUNDLE = GOOD_BUNDLE + PREVIOUS_MARKER + +# Stub haproxy. Beyond the config check the parent suite's stub does, this one +# also walks the crt directory the way HAProxy does when a `bind ... ssl crt +# ` is used: every file in there must be a loadable cert+key bundle, and +# one that is not takes the whole listener (i.e. the whole edge) down. That is +# what makes "the edge would still start" an assertion rather than a hope. +FAKE_HAPROXY = textwrap.dedent(f"""\ + #!/bin/sh + # haproxy -c -f FILE -> reject FILE containing {BROKEN_TOKEN} + # reject any unusable file in $TEST_CERTS_DIR + cfg="" + while [ $# -gt 0 ]; do + case "$1" in -f) cfg="$2"; shift ;; esac + shift + done + if [ -n "$cfg" ] && grep -q '{BROKEN_TOKEN}' "$cfg" 2>/dev/null; then + echo "[ALERT] parsing [$cfg:1] : unknown keyword '{BROKEN_TOKEN}'" >&2 + exit 1 + fi + if [ -n "$TEST_CERTS_DIR" ] && [ -d "$TEST_CERTS_DIR" ]; then + for f in "$TEST_CERTS_DIR"/*; do + [ -e "$f" ] || continue + if [ ! -s "$f" ]; then + echo "[ALERT] unable to load SSL certificate from empty file '$f'" >&2 + exit 1 + fi + if ! grep -q -- '-----END CERTIFICATE-----' "$f"; then + echo "[ALERT] unable to load SSL certificate from '$f'" >&2 + exit 1 + fi + if ! grep -q -- '-----END .*PRIVATE KEY-----' "$f"; then + echo "[ALERT] unable to load SSL private key from '$f'" >&2 + exit 1 + fi + done + fi + exit 0 +""") + +# certbot stub: succeeds, announces a renewal, and records every invocation so +# tests can assert that `certbot delete` did or did not run. +FAKE_CERTBOT = textwrap.dedent("""\ + #!/bin/sh + if [ -n "$TEST_CERTBOT_LOG" ]; then + echo "$@" >> "$TEST_CERTBOT_LOG" + fi + case "$1" in + renew) echo "Congratulations, all renewals succeeded" ;; + delete) [ -n "$TEST_CERTBOT_DELETE_FAILS" ] && exit 1 ;; + esac + exit 0 +""") + +# socat stub: records reload attempts so tests can assert HAProxy was NOT +# reloaded with unvalidated material. +FAKE_SOCAT = textwrap.dedent("""\ + #!/bin/sh + if [ -n "$TEST_SOCAT_LOG" ]; then + echo "$@" >> "$TEST_SOCAT_LOG" + fi + cat > /dev/null 2>&1 + exit 0 +""") + + +def structurally_valid(text): + """Is this text a usable cert+key bundle? + + Deliberately implemented here rather than calling into haproxy_manager, so + the assertions stay honest when the suite is pointed at a tree whose + validation code is the thing under test (or absent entirely). + """ + return ('-----BEGIN CERTIFICATE-----' in text + and '-----END CERTIFICATE-----' in text + and any(f'-----END {label}-----' in text for label in + ('PRIVATE KEY', 'RSA PRIVATE KEY', 'EC PRIVATE KEY'))) + + +class CertPublishTestCase(unittest.TestCase): + """Isolated fake /etc/haproxy + /etc/letsencrypt plus stub binaries.""" + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix='haproxy-cert-test-') + self.addCleanup(shutil.rmtree, self.tmp, True) + + bindir = os.path.join(self.tmp, 'bin') + os.makedirs(bindir) + for name, body in (('haproxy', FAKE_HAPROXY), + ('certbot', FAKE_CERTBOT), + ('socat', FAKE_SOCAT)): + path = os.path.join(bindir, name) + with open(path, 'w') as fh: + fh.write(body) + os.chmod(path, 0o755) + + self.certbot_log = os.path.join(self.tmp, 'certbot-invocations.log') + self.socat_log = os.path.join(self.tmp, 'socat-invocations.log') + self.etc = os.path.join(self.tmp, 'etc') + self.certs = os.path.join(self.etc, 'certs') + os.makedirs(self.certs) + + self._saved_env = dict(os.environ) + os.environ['PATH'] = bindir + os.pathsep + os.environ['PATH'] + os.environ['TEST_CERTS_DIR'] = self.certs + os.environ['TEST_CERTBOT_LOG'] = self.certbot_log + os.environ['TEST_SOCAT_LOG'] = self.socat_log + self.addCleanup(self._restore_env) + + overrides = { + 'DB_FILE': os.path.join(self.etc, 'haproxy_config.db'), + 'HAPROXY_CONFIG_PATH': os.path.join(self.etc, 'haproxy.cfg'), + 'HAPROXY_BACKUP_PATH': os.path.join(self.etc, 'haproxy.cfg.backup'), + 'BLOCKED_IPS_MAP_PATH': os.path.join(self.etc, 'blocked_ips.map'), + 'BLOCKED_IPS_MAP_BACKUP_PATH': os.path.join(self.etc, 'blocked_ips.map.backup'), + 'CORAZA_SPOE_CONFIG_PATH': os.path.join(self.etc, 'coraza-spoe.cfg'), + 'CORAZA_SPOE_BACKUP_PATH': os.path.join(self.etc, 'coraza-spoe.cfg.backup'), + 'CLUSTER_SECRET_PATH': os.path.join(self.etc, 'cluster-secret'), + 'SSL_CERTS_DIR': self.certs, + 'HAPROXY_SOCKET_PATH': os.path.join(self.etc, 'haproxy.sock'), + 'API_KEY': None, + } + self._saved = {} + for name, value in overrides.items(): + self._saved[name] = getattr(hm, name, None) + setattr(hm, name, value) + self.addCleanup(self._restore_globals) + + # find_certbot_live_dir() resolves /etc/letsencrypt/live, which we + # cannot repoint on older trees. Stubbing this one lookup keeps the + # suite runnable against a pre-fix checkout for bug reproduction; every + # line of code under test is downstream of it. + self.le_live = os.path.join(self.tmp, 'letsencrypt', 'live') + os.makedirs(self.le_live) + self._real_find = hm.find_certbot_live_dir + hm.find_certbot_live_dir = self._fake_find_live_dir + self.addCleanup( + lambda: setattr(hm, 'find_certbot_live_dir', self._real_find)) + + # log_operation() appends to a hardcoded /var/log path. Injecting `open` + # into the module namespace shadows the builtin for that module only. + real_open = open + log_dir = self.tmp + + def _redirecting_open(path, *args, **kwargs): + if isinstance(path, str) and path.startswith('/var/log/'): + path = os.path.join(log_dir, os.path.basename(path)) + return real_open(path, *args, **kwargs) + + hm.open = _redirecting_open + self.addCleanup(lambda: hm.__dict__.pop('open', None)) + + hm.init_db() + self.client = hm.app.test_client() + + def _restore_env(self): + os.environ.clear() + os.environ.update(self._saved_env) + + def _restore_globals(self): + for name, value in self._saved.items(): + if value is None: + setattr(hm, name, None) + else: + setattr(hm, name, value) + + def _fake_find_live_dir(self, *args, **kwargs): + base = args[0] if args else kwargs.get('base_domain') + path = os.path.join(self.le_live, base) + return path if os.path.isdir(path) else None + + # -- helpers --------------------------------------------------------- + def write(self, path, text): + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, 'w') as fh: + fh.write(text) + return path + + def read(self, path): + with open(path) as fh: + return fh.read() + + def make_lineage(self, domain, cert=LEAF_CERT, key=LEAF_KEY): + """Create a certbot live directory. key=None omits privkey.pem.""" + live = os.path.join(self.le_live, domain) + os.makedirs(live, exist_ok=True) + self.write(os.path.join(live, 'fullchain.pem'), cert) + if key is not None: + self.write(os.path.join(live, 'privkey.pem'), key) + return live + + def publish_live_bundle(self, domain, content=PREVIOUS_BUNDLE): + """A good bundle already being served for `domain`.""" + return self.write(os.path.join(self.certs, f'{domain}.pem'), content) + + def add_domain(self, domain, backend_name, ssl_cert_path=None): + with sqlite3.connect(hm.DB_FILE) as conn: + cur = conn.cursor() + cur.execute( + 'INSERT INTO domains (domain, ssl_enabled, ssl_cert_path) ' + 'VALUES (?, ?, ?)', + (domain, 1 if ssl_cert_path else 0, ssl_cert_path)) + domain_id = cur.lastrowid + cur.execute('INSERT INTO backends (name, domain_id) VALUES (?, ?)', + (backend_name, domain_id)) + backend_id = cur.lastrowid + cur.execute( + 'INSERT INTO backend_servers ' + '(backend_id, server_name, server_address, server_port) ' + 'VALUES (?, ?, ?, ?)', (backend_id, 'srv1', '10.0.0.1', 8080)) + conn.commit() + + def certbot_invocations(self): + if not os.path.exists(self.certbot_log): + return [] + return [line.strip() for line in self.read(self.certbot_log).splitlines()] + + def certbot_deletes(self): + return [c for c in self.certbot_invocations() if c.startswith('delete')] + + def edge_would_start(self): + """Would HAProxy load the current config + crt directory?""" + return subprocess.run( + ['haproxy', '-c', '-f', hm.HAPROXY_CONFIG_PATH], + capture_output=True).returncode == 0 + + def assert_only_final_pems_in_certs_dir(self): + strays = [n for n in os.listdir(self.certs) if not n.endswith('.pem')] + self.assertEqual( + [], strays, + 'HAProxy loads every file in the crt directory; only final .pem ' + f'bundles may exist there, found: {strays}') + + +class TestLivePemIsNeverTruncated(CertPublishTestCase): + """Bug 1: the live PEM was opened in truncate mode before any source read.""" + + def test_failed_renewal_leaves_previous_bundle_intact(self): + """HEADLINE: a failure mid-publish must not disturb what is served. + + The renewed lineage has a privkey.pem that exists but is empty - the + signature of a write that died half way, and the case the old + `os.path.exists(key)` guard waved straight through into + `cat fullchain emptykey > livepem`. + """ + cert_path = self.publish_live_bundle('renew.example.com') + before = self.read(cert_path) + self.add_domain('renew.example.com', 'renew_backend', + ssl_cert_path=cert_path) + self.make_lineage('renew.example.com', key='') + + resp = self.client.post('/api/certificates/renew') + + after = self.read(cert_path) + self.assertEqual(before, after, + 'the live PEM must still be the previous bundle, ' + 'byte for byte') + self.assertTrue(structurally_valid(after), + 'the served bundle must still contain a cert AND a key') + self.assertTrue(self.edge_would_start(), + 'HAProxy must still be able to load the crt directory') + self.assertEqual(500, resp.status_code, + 'a failed publish must be reported loudly, not as success') + self.assert_only_final_pems_in_certs_dir() + + def test_missing_source_key_leaves_previous_bundle_intact(self): + """Same invariant when privkey.pem is absent rather than empty.""" + cert_path = self.publish_live_bundle('gone.example.com') + before = self.read(cert_path) + self.add_domain('gone.example.com', 'gone_backend', + ssl_cert_path=cert_path) + self.make_lineage('gone.example.com', key=None) + + self.client.post('/api/certificates/renew') + + self.assertEqual(before, self.read(cert_path)) + self.assertTrue(self.edge_would_start()) + + def test_issuance_failure_leaves_previous_bundle_intact(self): + """/api/ssl re-issuing over an existing bundle must be all-or-nothing.""" + cert_path = self.publish_live_bundle('issue.example.com') + before = self.read(cert_path) + self.add_domain('issue.example.com', 'issue_backend') + self.make_lineage('issue.example.com', key='') + + resp = self.client.post('/api/ssl', json={'domain': 'issue.example.com'}) + + self.assertEqual(before, self.read(cert_path)) + self.assertTrue(structurally_valid(self.read(cert_path))) + self.assertTrue(self.edge_would_start()) + self.assertEqual(500, resp.status_code) + self.assertEqual('error', resp.get_json()['status']) + + def test_successful_renewal_still_publishes(self): + """The guard must not block the normal path. + + The old version built its "renewed" certificate with a no-op + `.replace('\\n-----END CERTIFICATE-----', '\\n-----END CERTIFICATE-----')`, + so the renewed lineage was byte-identical to what was already being + served. Every assertion still passed if the renewal published nothing + at all - which is the failure this test is supposed to catch. The live + bundle now carries a marker the renewed material does not, so "the file + on disk changed" is an actual assertion. + """ + cert_path = self.publish_live_bundle('ok.example.com') + self.assertIn(PREVIOUS_MARKER, self.read(cert_path), + 'fixture precondition: the live bundle is distinguishable') + self.add_domain('ok.example.com', 'ok_backend', ssl_cert_path=cert_path) + self.make_lineage('ok.example.com') + + resp = self.client.post('/api/certificates/renew') + + self.assertEqual(200, resp.status_code, resp.get_data(as_text=True)) + published = self.read(cert_path) + self.assertEqual(GOOD_BUNDLE, published, + 'the renewed cert+key was not written to the live pem') + self.assertNotIn(PREVIOUS_MARKER, published, + 'the previous bundle is still on disk: the renewal ' + 'published nothing') + self.assertTrue(structurally_valid(published)) + self.assertTrue(self.edge_would_start()) + self.assert_only_final_pems_in_certs_dir() + backup = os.path.join(hm.cert_backup_dir(), 'ok.example.com.pem') + self.assertTrue(os.path.exists(backup)) + self.assertEqual(PREVIOUS_BUNDLE, self.read(backup), + 'the archived copy is not the bundle that was replaced') + + +class TestOldCertificateIsNotDestroyedFirst(CertPublishTestCase): + """Bug 2: superseded .pem removed and lineage deleted before validation.""" + + def _setup_supersede(self, broken=True): + # An older single-SAN file whose CN is covered by the new bundle. + old_path = self.write( + os.path.join(self.certs, 'www.test.example.com.pem'), GOOD_BUNDLE) + self.add_domain('test.example.com', 'bundle_backend') + self.make_lineage('test.example.com', key='' if broken else LEAF_KEY) + return old_path + + @NEEDS_OPENSSL + def test_failed_bundle_does_not_remove_the_old_certificate(self): + old_path = self._setup_supersede(broken=True) + + resp = self.client.post('/api/ssl/bundle', json={ + 'primary': 'test.example.com', + 'sans': ['www.test.example.com'], + }) + + self.assertEqual(500, resp.status_code) + self.assertTrue( + os.path.exists(old_path), + 'the superseded certificate must survive a failed replacement - ' + 'it may be the only working copy left') + self.assertTrue(structurally_valid(self.read(old_path))) + self.assertEqual( + [], self.certbot_deletes(), + '`certbot delete` is irreversible and rate-limited to recover from; ' + 'it must never run for a replacement that was never published') + self.assertTrue(self.edge_would_start()) + + @FIX_ONLY + @NEEDS_OPENSSL + def test_lineage_is_deleted_only_after_haproxy_loads_the_bundle(self): + """A reload failure must leave the lineage intact and the file recoverable.""" + old_path = self._setup_supersede(broken=False) + # Make generate_config() produce a config the validator rejects, so the + # publish succeeds but HAProxy never loads it. + self.add_domain('other.example.com', BROKEN_TOKEN + '_backend') + + resp = self.client.post('/api/ssl/bundle', json={ + 'primary': 'test.example.com', + 'sans': ['www.test.example.com'], + }) + + self.assertEqual(500, resp.status_code) + self.assertEqual( + [], self.certbot_deletes(), + 'the lineage must not be deleted when HAProxy did not reload') + self.assertFalse(os.path.exists(old_path)) + quarantined = os.path.join(hm.cert_backup_dir(), + os.path.basename(old_path)) + self.assertTrue(os.path.exists(quarantined), + 'the superseded file must be recoverable by hand') + self.assertTrue(structurally_valid(self.read(quarantined))) + + @FIX_ONLY + @NEEDS_OPENSSL + def test_successful_bundle_still_supersedes_and_deletes(self): + """The cleanup must still do its job on the happy path.""" + old_path = self._setup_supersede(broken=False) + + resp = self.client.post('/api/ssl/bundle', json={ + 'primary': 'test.example.com', + 'sans': ['www.test.example.com'], + }) + + self.assertEqual(200, resp.status_code, resp.get_data(as_text=True)) + self.assertFalse(os.path.exists(old_path), + 'the superseded file must leave the crt directory or ' + 'it keeps shadowing the new bundle') + self.assertEqual(['delete --cert-name www.test.example.com -n'], + self.certbot_deletes()) + self.assertTrue(self.edge_would_start()) + self.assert_only_final_pems_in_certs_dir() + + +class TestBundleValidation(CertPublishTestCase): + """What may and may not be published.""" + + @FIX_ONLY + def test_structure_checks(self): + cases = [ + ('', False, 'empty'), + (' \n', False, 'whitespace only'), + (LEAF_CERT, False, 'certificate without a key'), + (LEAF_KEY, False, 'key without a certificate'), + (GOOD_BUNDLE[:len(LEAF_CERT) // 2], False, 'truncated mid-block'), + (LEAF_CERT + LEAF_KEY.replace('-----END PRIVATE KEY-----', ''), + False, 'key block never closed'), + (GOOD_BUNDLE, True, 'complete bundle'), + ] + for text, expected, label in cases: + with self.subTest(label): + ok, _ = hm.validate_pem_structure(text) + self.assertEqual(expected, ok) + + @FIX_ONLY + @NEEDS_OPENSSL + def test_key_must_match_the_leaf_certificate(self): + dest = os.path.join(self.certs, 'pair.example.com.pem') + cert = self.write(os.path.join(self.tmp, 'src', 'fullchain.pem'), LEAF_CERT) + bad_key = self.write(os.path.join(self.tmp, 'src', 'wrong.pem'), + UNRELATED_KEY) + with self.assertRaises(hm.CertificatePublishError): + hm.publish_pem_bundle(dest, [cert, bad_key]) + self.assertFalse(os.path.exists(dest)) + self.assert_only_final_pems_in_certs_dir() + + @FIX_ONLY + @NEEDS_OPENSSL + def test_mismatched_pair_does_not_disturb_the_live_bundle(self): + """The atomic-swap invariant, not just the pre-write content check. + + A cert+key that only turns out to be unusable once assembled (here: a + structurally perfect bundle whose key belongs to another certificate) + is the case that proves the live file is never opened for writing - + the failure is discovered with the replacement already fully staged. + """ + dest = self.publish_live_bundle('swap.example.com') + before = self.read(dest) + cert = self.write(os.path.join(self.tmp, 'srcm', 'fullchain.pem'), + LEAF_CERT) + bad_key = self.write(os.path.join(self.tmp, 'srcm', 'privkey.pem'), + UNRELATED_KEY) + + with self.assertRaises(hm.CertificatePublishError): + hm.publish_pem_bundle(dest, [cert, bad_key]) + + self.assertEqual(before, self.read(dest), + 'the previously served bundle must survive byte for byte') + self.assertTrue(self.edge_would_start()) + self.assert_only_final_pems_in_certs_dir() + + @FIX_ONLY + def test_unexpected_error_mid_publish_leaves_the_live_bundle_intact(self): + """A failure anywhere between staging and swap must be survivable. + + Stands in for the failures we cannot stage deterministically - disk + full, container killed, an exception in a future validation step. + """ + dest = self.publish_live_bundle('boom.example.com') + before = self.read(dest) + cert = self.write(os.path.join(self.tmp, 'srcb', 'fullchain.pem'), + LEAF_CERT) + key = self.write(os.path.join(self.tmp, 'srcb', 'privkey.pem'), LEAF_KEY) + + real_validate = hm.validate_pem_bundle + + def _explode(path): + raise RuntimeError('simulated failure while publishing') + + hm.validate_pem_bundle = _explode + self.addCleanup(lambda: setattr(hm, 'validate_pem_bundle', real_validate)) + + with self.assertRaises(Exception): + hm.publish_pem_bundle(dest, [cert, key]) + + self.assertEqual(before, self.read(dest)) + self.assertTrue(self.edge_would_start()) + self.assert_only_final_pems_in_certs_dir() + + @FIX_ONLY + def test_staging_and_backups_live_outside_the_crt_directory(self): + """A temp or backup file inside the crt dir would be loaded by HAProxy. + + `startswith(certs + os.sep)` alone let the worst case through: if + cert_staging_dir() returned the crt directory ITSELF - staging straight + into the directory HAProxy scans, the exact hazard this design exists + to prevent - the assertion passed, because the certs dir is not a + strict subpath of itself. + """ + certs = os.path.realpath(self.certs) + for name, path in (('staging', hm.cert_staging_dir()), + ('backup', hm.cert_backup_dir())): + real = os.path.realpath(path) + self.assertNotEqual( + certs, real, + f'the {name} directory IS the crt directory - HAProxy would ' + f'load every temp/backup file in it') + self.assertFalse( + real.startswith(certs + os.sep), + f'the {name} directory {path} must not be inside {self.certs}') + + @FIX_ONLY + def test_previous_bundle_is_backed_up_on_publish(self): + dest = self.publish_live_bundle('backup.example.com') + previous = self.read(dest) + cert = self.write(os.path.join(self.tmp, 'src2', 'fullchain.pem'), + LEAF_CERT) + key = self.write(os.path.join(self.tmp, 'src2', 'privkey.pem'), LEAF_KEY) + + hm.publish_pem_bundle(dest, [cert, key]) + + backup = os.path.join(hm.cert_backup_dir(), 'backup.example.com.pem') + self.assertTrue(os.path.exists(backup), + 'an operator needs a manual path back to the previous ' + 'certificate') + self.assertEqual(previous, self.read(backup)) + + @FIX_ONLY + def test_corrupt_live_bundle_does_not_overwrite_a_good_backup(self): + """Mirrors create_backup(require_valid=True) for haproxy.cfg.""" + dest = os.path.join(self.certs, 'guard.example.com.pem') + os.makedirs(hm.cert_backup_dir(), exist_ok=True) + good_backup = self.write( + os.path.join(hm.cert_backup_dir(), 'guard.example.com.pem'), + GOOD_BUNDLE) + self.write(dest, LEAF_CERT) # live file is key-less garbage + + cert = self.write(os.path.join(self.tmp, 'src3', 'fullchain.pem'), + LEAF_CERT) + key = self.write(os.path.join(self.tmp, 'src3', 'privkey.pem'), LEAF_KEY) + hm.publish_pem_bundle(dest, [cert, key]) + + self.assertEqual(GOOD_BUNDLE, self.read(good_backup), + 'a good backup must not be replaced by a corrupt live ' + 'file') + + @FIX_ONLY + @NEEDS_OPENSSL + def test_no_temp_file_survives_a_failed_publish(self): + """The failure must happen AFTER something has been staged. + + This used to feed publish_pem_bundle() an empty privkey, which is + rejected while reading the sources - before the staging directory is + even created. The test then asserted `[] == []` and proved nothing + about temp-file cleanup. A structurally perfect bundle whose key + belongs to a different certificate is rejected by the pairing check, + which runs on the fully staged file, so the temp definitely exists at + the moment the publish fails. + """ + dest = os.path.join(self.certs, 'leak.example.com.pem') + cert = self.write(os.path.join(self.tmp, 'src4', 'fullchain.pem'), + LEAF_CERT) + wrong = self.write(os.path.join(self.tmp, 'src4', 'privkey.pem'), + UNRELATED_KEY) + with self.assertRaises(hm.CertificatePublishError): + hm.publish_pem_bundle(dest, [cert, wrong]) + + staging = hm.cert_staging_dir() + self.assertTrue(os.path.isdir(staging), + 'the publish never got as far as staging, so this ' + 'asserts nothing about cleanup') + self.assertEqual([], os.listdir(staging), + 'staged files must be cleaned up when a publish fails') + self.assertFalse(os.path.exists(dest), + 'a rejected bundle was published anyway') + self.assert_only_final_pems_in_certs_dir() + + + @FIX_ONLY + @NEEDS_OPENSSL + def test_empty_pem_blocks_are_rejected(self): + """Why the pairing check may not be best-effort. + + Structural validation is weak on its own: BEGIN/END pairs with nothing + between them satisfy every rule in validate_pem_structure(). Only + openssl can say this is not a certificate. + """ + hollow = ('-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n' + '-----BEGIN PRIVATE KEY-----\n-----END PRIVATE KEY-----\n') + ok, _ = hm.validate_pem_structure(hollow) + self.assertTrue(ok, 'precondition: structure alone accepts this file') + + path = self.write(os.path.join(self.tmp, 'hollow.pem'), hollow) + ok, msg = hm.validate_pem_bundle(path) + self.assertFalse(ok, 'a bundle of empty pem blocks was accepted') + + @FIX_ONLY + def test_publish_refuses_when_the_pairing_check_cannot_run(self): + """No fail-open. 'unavailable' means the image is broken, not the cert. + + The previous behaviour logged a warning and published on structural + checks alone, justified by "the Dockerfile does not install the openssl + CLI". It does - openssl 3.x comes in with ca-certificates, and + generate_self_signed_cert() shells out to `openssl req` with + check=True during setup - so this branch never fired and the fail-open + was safe only by accident. + """ + real = hm._openssl_pairing_status + hm._openssl_pairing_status = lambda path: ('unavailable', + 'openssl binary not found') + self.addCleanup(setattr, hm, '_openssl_pairing_status', real) + + dest = self.publish_live_bundle('nocheck.example.com') + before = self.read(dest) + cert = self.write(os.path.join(self.tmp, 'src5', 'fullchain.pem'), + LEAF_CERT) + key = self.write(os.path.join(self.tmp, 'src5', 'privkey.pem'), LEAF_KEY) + + with self.assertRaises(hm.CertificatePublishError): + hm.publish_pem_bundle(dest, [cert, key]) + + self.assertEqual(before, self.read(dest), + 'the live pem was disturbed by a refused publish') + self.assert_only_final_pems_in_certs_dir() + + @FIX_ONLY + def test_binary_corrupt_live_bundle_can_still_be_republished(self): + """Republishing is how an operator heals a corrupt live pem. + + A pem corrupted into binary (a partial write from before this fix, a + bad restore) is unreadable as text. backup_existing_pem() opens it in + text mode and caught only OSError, so the UnicodeDecodeError escaped + publish_pem_bundle() entirely: the single operation that would have put + a working certificate back blew up trying to archive the broken one. + The shell half recovers from this without complaint. + """ + dest = os.path.join(self.certs, 'binary.example.com.pem') + with open(dest, 'wb') as fh: + fh.write(b'\x00\x01\x02\xfe\xff' * 128) + cert = self.write(os.path.join(self.tmp, 'src6', 'fullchain.pem'), + LEAF_CERT) + key = self.write(os.path.join(self.tmp, 'src6', 'privkey.pem'), LEAF_KEY) + + hm.publish_pem_bundle(dest, [cert, key]) + + self.assertEqual(GOOD_BUNDLE, self.read(dest), + 'a binary-corrupt live pem blocked its own repair') + self.assertTrue(self.edge_would_start()) + + @FIX_ONLY + def test_published_pem_file_mode(self): + """Publishing must not silently re-permission a private key. + + The staged file is created by mkstemp at 0600; without the explicit + mode copy in write_config_atomically() every publish would tighten a + 0644 bundle, and a future change in the other direction would loosen + one. Neither is a decision a write-safety fix gets to make as a side + effect, so pin it. + """ + cert = self.write(os.path.join(self.tmp, 'src7', 'fullchain.pem'), + LEAF_CERT) + key = self.write(os.path.join(self.tmp, 'src7', 'privkey.pem'), LEAF_KEY) + + for mode in (0o644, 0o640, 0o600): + with self.subTest(oct(mode)): + dest = self.publish_live_bundle(f'mode{mode:o}.example.com') + os.chmod(dest, mode) + hm.publish_pem_bundle(dest, [cert, key]) + self.assertEqual(mode, + stat.S_IMODE(os.stat(dest).st_mode), + 'publishing changed who can read the key') + + fresh = os.path.join(self.certs, 'fresh.example.com.pem') + hm.publish_pem_bundle(fresh, [cert, key]) + fresh_mode = stat.S_IMODE(os.stat(fresh).st_mode) + self.assertEqual(0o644, fresh_mode, + 'a newly created bundle should match the 0644 that ' + '`cat > file` produced under the standard umask') + self.assertEqual(0, fresh_mode & 0o022, + 'a private key must never be group/world writable') + + +@unittest.skipIf(TESTING_FOREIGN_TREE, + 'HAPROXY_MANAGER_DIR points at another tree') +class TestPublisherApiIsPresent(unittest.TestCase): + """FIX_ONLY must never be able to hide the fix going missing. + + With `FIX_ONLY = skipUnless(hasattr(hm, 'publish_pem_bundle'))`, renaming + that one function turned 10 of these 17 tests into skips and the run still + printed OK. This class fails loudly instead. + """ + + def test_publisher_api_is_present(self): + for name in ('publish_pem_bundle', 'validate_pem_bundle', + 'validate_pem_structure', 'backup_existing_pem', + 'cert_staging_dir', 'cert_backup_dir', + 'CertificatePublishError', '_openssl_pairing_status'): + self.assertTrue( + hasattr(hm, name), + f'haproxy_manager.{name} is gone - the tests that exercise it ' + f'would otherwise skip themselves and report OK') + + +class TestClusterSecretSelfHeal(CertPublishTestCase): + """Bug 4: a zero-byte secret file was never healed.""" + + def test_empty_secret_file_is_healed(self): + self.write(hm.CLUSTER_SECRET_PATH, '') + secret = hm.get_or_create_cluster_secret() + self.assertTrue( + secret, + 'an empty secret file must be regenerated, not returned as ""') + self.assertEqual(secret, self.read(hm.CLUSTER_SECRET_PATH).strip()) + self.assertEqual(secret, hm.get_or_create_cluster_secret(), + 'the healed secret must then be stable') + + def test_existing_secret_is_preserved(self): + self.write(hm.CLUSTER_SECRET_PATH, 'deadbeef\n') + self.assertEqual('deadbeef', hm.get_or_create_cluster_secret()) + + +if __name__ == '__main__': + print(f"testing haproxy_manager from: {MODULE_DIR}", file=sys.stderr) + unittest.main(verbosity=2) diff --git a/scripts/test-config-rollback.py b/scripts/test-config-rollback.py new file mode 100755 index 0000000..1b50725 --- /dev/null +++ b/scripts/test-config-rollback.py @@ -0,0 +1,775 @@ +#!/usr/bin/env python3 +"""Regression tests for HAProxy config backup / rollback ordering. + +Why this file exists +-------------------- +generate_config() used to write the new haproxy.cfg and only THEN call +reload_haproxy_safely() -> create_backup(), so the "backup" was a copy of the +config that had just been written. On a validation failure restore_backup() +restored the identical broken bytes: the advertised rollback was a no-op and a +fatal haproxy.cfg stayed on disk, where start_haproxy() refuses to launch. + +These tests pin the ordering invariant (backup predates the write) and the +observable end-to-end behaviour (after a failed validation the file on disk is +the previous working config and HAProxy will start with it). + +Running +------- + python3 scripts/test-config-rollback.py # tests the repo checkout + HAPROXY_MANAGER_DIR=/some/other/tree \ + python3 scripts/test-config-rollback.py # tests another tree + +The repo has no Python test framework (scripts/test-*.sh are curl-based +integration scripts against a running API), so this is a self-contained +stdlib-unittest script - no pytest, no venv, no new dependencies beyond the +application's own requirements.txt (Flask/Jinja2/psutil), which are already +present in the container image. + +No HAProxy binary is required: a stub `haproxy` is put on PATH that mimics +`haproxy -c -f ` by rejecting any config containing the token +__BROKEN__, which is how the tests inject an invalid configuration. +""" + +import os +import re +import sys +import shutil +import inspect +import sqlite3 +import logging +import tempfile +import textwrap +import unittest + +BROKEN_TOKEN = '__BROKEN__' + +MODULE_DIR = os.path.abspath( + os.environ.get('HAPROXY_MANAGER_DIR', + os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')) +) + +# haproxy_manager builds its Jinja2 environment from the relative path +# Path('templates'), so it has to be imported with the module dir as cwd. +os.chdir(MODULE_DIR) +sys.path.insert(0, MODULE_DIR) + +# The module opens /var/log/haproxy-manager.log at import time via +# logging.FileHandler. Redirect that one call so the suite runs unprivileged. +_LOG_DIR = tempfile.mkdtemp(prefix='haproxy-mgr-test-logs-') +_real_file_handler = logging.FileHandler +logging.FileHandler = ( + lambda fn, *a, **kw: _real_file_handler( + os.path.join(_LOG_DIR, os.path.basename(fn)), *a, **kw) +) +try: + import haproxy_manager as hm +except ImportError as exc: # pragma: no cover - environment problem, not a failure + sys.stderr.write( + f"SKIP: cannot import haproxy_manager ({exc}).\n" + "Install the application requirements first: pip install -r requirements.txt\n" + ) + raise SystemExit(77) +finally: + logging.FileHandler = _real_file_handler + +logging.getLogger('haproxy_manager').setLevel(logging.CRITICAL) + +FAKE_HAPROXY = textwrap.dedent(f"""\ + #!/bin/sh + # Test stub for the haproxy binary. + # haproxy -c -f FILE -> exit 1 if FILE contains {BROKEN_TOKEN}, else 0 + # haproxy -W -S ... -f FILE (start) -> same validation, then exit 0 + cfg="" + while [ $# -gt 0 ]; do + case "$1" in -f) cfg="$2"; shift ;; esac + shift + done + if [ -n "$cfg" ] && grep -q '{BROKEN_TOKEN}' "$cfg" 2>/dev/null; then + echo "[ALERT] parsing [$cfg:1] : unknown keyword '{BROKEN_TOKEN}'" >&2 + exit 1 + fi + exit 0 +""") + + +class RollbackTestCase(unittest.TestCase): + """Base fixture: an isolated fake /etc/haproxy plus a stub haproxy binary.""" + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix='haproxy-rollback-test-') + self.addCleanup(shutil.rmtree, self.tmp, True) + + bindir = os.path.join(self.tmp, 'bin') + os.makedirs(bindir) + stub = os.path.join(bindir, 'haproxy') + with open(stub, 'w') as fh: + fh.write(FAKE_HAPROXY) + os.chmod(stub, 0o755) + self._old_path = os.environ['PATH'] + os.environ['PATH'] = bindir + os.pathsep + self._old_path + self.addCleanup(lambda: os.environ.__setitem__('PATH', self._old_path)) + + self.etc = os.path.join(self.tmp, 'etc') + os.makedirs(self.etc) + + overrides = { + 'DB_FILE': os.path.join(self.etc, 'haproxy_config.db'), + 'HAPROXY_CONFIG_PATH': os.path.join(self.etc, 'haproxy.cfg'), + 'HAPROXY_BACKUP_PATH': os.path.join(self.etc, 'haproxy.cfg.backup'), + 'BLOCKED_IPS_MAP_PATH': os.path.join(self.etc, 'blocked_ips.map'), + 'BLOCKED_IPS_MAP_BACKUP_PATH': os.path.join(self.etc, 'blocked_ips.map.backup'), + 'CLUSTER_SECRET_PATH': os.path.join(self.etc, 'cluster-secret'), + 'SSL_CERTS_DIR': os.path.join(self.etc, 'certs'), + 'HAPROXY_SOCKET_PATH': os.path.join(self.etc, 'haproxy.sock'), + # Added by the rollback fix; older trees do not have it. + 'CORAZA_SPOE_CONFIG_PATH': os.path.join(self.etc, 'coraza-spoe.cfg'), + 'CORAZA_SPOE_BACKUP_PATH': os.path.join(self.etc, 'coraza-spoe.cfg.backup'), + } + self._saved = {} + for name, value in overrides.items(): + self._saved[name] = getattr(hm, name, None) + setattr(hm, name, value) + self.addCleanup(self._restore_globals) + os.makedirs(hm.SSL_CERTS_DIR) + + # log_operation() appends to a hardcoded /var/log path. Injecting `open` + # into the module namespace shadows the builtin for that module only + # (module globals are searched before builtins), so the real + # log_operation code still runs. + real_open = open + log_dir = self.tmp + + def _redirecting_open(path, *args, **kwargs): + if isinstance(path, str) and path.startswith('/var/log/'): + path = os.path.join(log_dir, os.path.basename(path)) + return real_open(path, *args, **kwargs) + + hm.open = _redirecting_open + self.addCleanup(lambda: hm.__dict__.pop('open', None)) + + hm.init_db() + + def _restore_globals(self): + for name, value in self._saved.items(): + if value is None: + hm.__dict__.pop(name, None) + else: + setattr(hm, name, value) + + # -- helpers --------------------------------------------------------- + def add_domain(self, domain, backend_name, address='10.0.0.1'): + with sqlite3.connect(hm.DB_FILE) as conn: + cur = conn.cursor() + cur.execute('INSERT INTO domains (domain, ssl_enabled) VALUES (?, 0)', + (domain,)) + domain_id = cur.lastrowid + cur.execute('INSERT INTO backends (name, domain_id) VALUES (?, ?)', + (backend_name, domain_id)) + backend_id = cur.lastrowid + cur.execute( + 'INSERT INTO backend_servers ' + '(backend_id, server_name, server_address, server_port) ' + 'VALUES (?, ?, ?, ?)', + (backend_id, 'srv1', address, 8080)) + conn.commit() + + def block_ip(self, ip): + with sqlite3.connect(hm.DB_FILE) as conn: + conn.execute('INSERT INTO blocked_ips (ip_address, reason) VALUES (?, ?)', + (ip, 'test')) + conn.commit() + + def read(self, path): + with open(path) as fh: + return fh.read() + + def config_is_loadable(self): + """True if HAProxy would accept the config currently on disk.""" + import subprocess + return subprocess.run( + ['haproxy', '-c', '-f', hm.HAPROXY_CONFIG_PATH], + capture_output=True).returncode == 0 + + def generate_good_config(self): + self.add_domain('good.example.com', 'good_backend') + hm.generate_config() + self.assertTrue(self.config_is_loadable(), + 'fixture precondition: first generated config must be valid') + return self.read(hm.HAPROXY_CONFIG_PATH) + + def break_the_config(self): + """Queue a domain whose rendered backend the validator rejects.""" + self.add_domain('bad.example.com', BROKEN_TOKEN + '_backend', '10.0.0.2') + + +class TestBackupOrdering(RollbackTestCase): + + def test_backup_is_taken_before_the_new_config_is_written(self): + """The ordering invariant, asserted directly. + + Whatever create_backup() sees on disk must be the OLD config; if the + write happens first the backup is a copy of the new config and rollback + is meaningless. + """ + good = self.generate_good_config() + + seen = {} + real_create_backup = hm.create_backup + + def spy(*args, **kwargs): + seen['config_on_disk'] = self.read(hm.HAPROXY_CONFIG_PATH) + return real_create_backup(*args, **kwargs) + + hm.create_backup = spy + self.addCleanup(setattr, hm, 'create_backup', real_create_backup) + + self.add_domain('second.example.com', 'second_backend', '10.0.0.3') + hm.generate_config() + + self.assertIn('config_on_disk', seen, + 'create_backup() was never called during generate_config()') + self.assertEqual( + seen['config_on_disk'], good, + 'create_backup() ran AFTER the new config was written - the backup ' + 'is a copy of the new config, so rollback cannot undo anything') + + def test_backup_tracks_the_last_known_good_config(self): + """After a change that validated AND loaded, the backup is that config. + + The rollback target is "the last configuration HAProxy actually ran", + not "the file that happened to be there last time". + """ + good = self.generate_good_config() + self.add_domain('second.example.com', 'second_backend', '10.0.0.3') + hm.generate_config() + + live = self.read(hm.HAPROXY_CONFIG_PATH) + self.assertNotEqual(live, good, 'fixture sanity: the new config should differ') + self.assertEqual(self.read(hm.HAPROXY_BACKUP_PATH), live, + 'the successful config was not recorded as known-good') + + def test_backup_is_not_promoted_when_the_change_fails(self): + """A config that never loaded must not become the rollback target.""" + good = self.generate_good_config() + self.break_the_config() + with self.assertRaises(Exception): + hm.generate_config() + + self.assertEqual(self.read(hm.HAPROXY_BACKUP_PATH), good, + 'a config that failed validation was promoted to backup') + + +class TestRollbackEndToEnd(RollbackTestCase): + + def test_failed_validation_leaves_the_last_good_config_on_disk(self): + good = self.generate_good_config() + + self.break_the_config() + with self.assertRaises(Exception): + hm.generate_config() + + on_disk = self.read(hm.HAPROXY_CONFIG_PATH) + self.assertNotIn(BROKEN_TOKEN, on_disk, + 'the rejected config is still on disk - rollback was a no-op') + self.assertEqual(on_disk, good, + 'on-disk config is not byte-identical to the last good one') + + def test_haproxy_would_still_start_after_a_failed_change(self): + """The operational consequence: the edge can still come up.""" + self.generate_good_config() + self.break_the_config() + with self.assertRaises(Exception): + hm.generate_config() + + self.assertTrue(self.config_is_loadable(), + 'HAProxy would refuse to start with the config left on disk') + with self.assertLogs('haproxy_manager', level='INFO') as captured: + hm.start_haproxy() + self.assertTrue( + any('HAProxy started successfully' in line for line in captured.output), + f'start_haproxy() did not succeed after rollback: {captured.output}') + + def test_blocked_ips_map_is_rolled_back_too(self): + """generate_config() rewrites the map file before writing haproxy.cfg.""" + self.block_ip('192.0.2.10') + self.generate_good_config() + good_map = self.read(hm.BLOCKED_IPS_MAP_PATH) + + self.block_ip('198.51.100.20') + self.break_the_config() + with self.assertRaises(Exception): + hm.generate_config() + + self.assertEqual(self.read(hm.BLOCKED_IPS_MAP_PATH), good_map, + 'blocked IPs map was not rolled back with the config') + + def test_first_run_failure_reports_that_rollback_was_impossible(self): + """No prior config: there is nothing to restore, and that must be said. + + A missing backup must never be reported as a successful restore, and it + must never be turned into "restore an empty file". + """ + self.break_the_config() + with self.assertRaises(Exception) as ctx: + hm.generate_config() + + self.assertIn('ROLLBACK FAILED', str(ctx.exception), + 'a failed change with no backup was not reported as such') + self.assertFalse(os.path.exists(hm.HAPROXY_BACKUP_PATH), + 'a backup was fabricated from the broken config') + # The broken config is deliberately left in place: start_haproxy() can + # then detect it and try to regenerate. It must not be blanked. + self.assertGreater(os.path.getsize(hm.HAPROXY_CONFIG_PATH), 0, + 'config file was emptied instead of left for diagnosis') + + +class TestBackupPrimitives(RollbackTestCase): + + def test_restore_backup_distinguishes_missing_backup_from_success(self): + restored, message = hm.restore_backup() + self.assertFalse(restored, + 'restore_backup() reported success with no backup present') + self.assertIn('cannot roll back', message.lower()) + + good = self.generate_good_config() + with open(hm.HAPROXY_CONFIG_PATH, 'w') as fh: + fh.write('scribbled over\n') + + restored, message = hm.restore_backup() + self.assertTrue(restored, message) + self.assertEqual(self.read(hm.HAPROXY_CONFIG_PATH), good) + + def test_a_successful_generation_records_a_rollback_target(self): + """Even the first-ever generation must leave something to roll back to.""" + good = self.generate_good_config() + self.assertTrue( + os.path.exists(hm.HAPROXY_BACKUP_PATH), + 'after a successful reload there is still no known-good backup') + self.assertEqual(self.read(hm.HAPROXY_BACKUP_PATH), good) + + def test_a_broken_current_config_does_not_replace_a_good_backup(self): + """The known-good marker. + + If the config already on disk is broken (previous failed write, manual + edit), snapshotting it would make "rollback" mean "restore a different + broken config". The older validated backup must survive. + """ + good = self.generate_good_config() + self.assertEqual(self.read(hm.HAPROXY_BACKUP_PATH), good, + 'fixture: a good backup should exist by now') + + with open(hm.HAPROXY_CONFIG_PATH, 'w') as fh: + fh.write(f'garbage {BROKEN_TOKEN} config\n') + + ok, status = hm.create_backup() + self.assertTrue(ok) + self.assertEqual(status, 'kept_previous') + self.assertEqual(self.read(hm.HAPROXY_BACKUP_PATH), good, + 'a broken config overwrote the known-good backup') + + def test_reload_does_not_take_its_own_backup(self): + """reload_haproxy_safely() runs after the write, so it must not back up.""" + good = self.generate_good_config() + with open(hm.HAPROXY_CONFIG_PATH, 'w') as fh: + fh.write(f'broken {BROKEN_TOKEN}\n') + + success, message = hm.reload_haproxy_safely(backup_status='created') + + self.assertFalse(success) + self.assertEqual(self.read(hm.HAPROXY_BACKUP_PATH), good, + 'reload_haproxy_safely() overwrote the good backup') + self.assertEqual(self.read(hm.HAPROXY_CONFIG_PATH), good, + 'reload_haproxy_safely() did not roll the config back') + + def test_unchanged_config_is_not_revalidated(self): + """Fast path: if the backup already is the live config, do no work. + + generate_config() runs inside customer-facing API calls and + `haproxy -c` is expensive on an edge with hundreds of certificates. + """ + self.generate_good_config() + + calls = [] + real_validate = hm.validate_config_file + hm.validate_config_file = lambda path: (calls.append(path), + real_validate(path))[1] + self.addCleanup(setattr, hm, 'validate_config_file', real_validate) + + ok, status = hm.create_backup() + self.assertTrue(ok) + self.assertEqual(status, 'created') + self.assertEqual(calls, [], + 'the unchanged live config was re-validated needlessly') + + def test_fast_path_does_not_hide_a_drifted_broken_config(self): + """If the live config drifted from the backup, the gate must still run.""" + good = self.generate_good_config() + with open(hm.HAPROXY_CONFIG_PATH, 'w') as fh: + fh.write(f'hand edited {BROKEN_TOKEN}\n') + + ok, status = hm.create_backup() + self.assertTrue(ok) + self.assertEqual(status, 'kept_previous', + 'a drifted broken config was silently accepted') + self.assertEqual(self.read(hm.HAPROXY_BACKUP_PATH), good) + + def test_backup_set_covers_every_file_generate_config_writes(self): + """Derived, not restated. + + An earlier version of this test listed the three files it expected and + checked they were in the backup set, so it could never have noticed a + FOURTH file being added. Here the set of files generate_config() writes + is observed (and, for env-gated branches this fixture cannot safely + execute, read out of the source), and anything not backed up has to be + on the documented exclusion list below. + """ + # Written by generate_config() but deliberately NOT restorable, with + # the reason. Everything else must be in the backup set: `haproxy -c` + # validates the config as a set, so a file it loads that is not + # restored alongside haproxy.cfg breaks rollback. + excluded = { + # Only ever created empty-when-missing (haproxy refuses to start + # with an ACL -f pointing at a missing file); its contents are + # owned by the /suspended API, not by generate_config(), so there + # is nothing here for a config rollback to undo. + 'suspended_domains.list', + # Generated once and then read, never rewritten with new content. + # Its value is rendered INTO haproxy.cfg, so restoring an older + # haproxy.cfg alongside the current secret file is consistent. + 'cluster-secret', + } + backed_up = {os.path.basename(p) for p, _ in hm._config_backup_pairs()} + + # 1. Observed: run a generation with every patchable optional branch on + # and see what actually changed on disk. + self.add_domain('derive.example.com', 'derive_backend') + os.environ['HAPROXY_CORAZA_SPOE_BACKEND'] = '127.0.0.1:9000' + self.addCleanup(os.environ.pop, 'HAPROXY_CORAZA_SPOE_BACKEND', None) + before = self._snapshot_etc() + hm.generate_config() + after = self._snapshot_etc() + touched = {name for name, blob in after.items() + if before.get(name) != blob} + self.assertIn('coraza-spoe.cfg', touched, + 'fixture precondition: the Coraza branch did not run') + + # 2. Read out of the source: branches this fixture must not execute + # (suspension writes a hardcoded /etc/haproxy path that no test + # global can redirect) still have to be accounted for. + touched |= { + os.path.basename(m) + for m in re.findall(r"'(/etc/haproxy/[\w.+-]+)'", + inspect.getsource(hm.generate_config)) + } + + unaccounted = touched - backed_up - excluded + self.assertEqual( + unaccounted, set(), + f'generate_config() writes {sorted(unaccounted)}, which is neither ' + 'in the backup set nor on the documented exclusion list - a ' + 'rollback would restore a mixed-vintage config set') + + def _snapshot_etc(self): + """Contents of every plain file in the fake /etc/haproxy. + + Skips the backup halves (they are the thing being maintained), the + SQLite database and its journals, and the stats socket. + """ + skip_prefixes = (os.path.basename(hm.DB_FILE), + os.path.basename(hm.HAPROXY_SOCKET_PATH)) + state = {} + for name in os.listdir(self.etc): + path = os.path.join(self.etc, name) + if not os.path.isfile(path) or name.endswith('.backup'): + continue + if name.startswith(skip_prefixes): + continue + with open(path, 'rb') as fh: + state[name] = fh.read() + return state + + def test_coraza_spoe_config_round_trips(self): + self.generate_good_config() + with open(hm.CORAZA_SPOE_CONFIG_PATH, 'w') as fh: + fh.write('spoe-good\n') + hm.create_backup() + with open(hm.CORAZA_SPOE_CONFIG_PATH, 'w') as fh: + fh.write('spoe-broken\n') + restored, message = hm.restore_backup() + self.assertTrue(restored, message) + self.assertEqual(self.read(hm.CORAZA_SPOE_CONFIG_PATH), 'spoe-good\n') + + +class TestAtomicWrite(RollbackTestCase): + + def test_write_is_atomic_and_preserves_mode(self): + path = os.path.join(self.etc, 'atomic.cfg') + with open(path, 'w') as fh: + fh.write('old') + os.chmod(path, 0o644) + + hm.write_config_atomically(path, 'new content\n') + + self.assertEqual(self.read(path), 'new content\n') + self.assertEqual(oct(os.stat(path).st_mode & 0o777), oct(0o644)) + leftovers = [n for n in os.listdir(self.etc) if n.endswith('.tmp')] + self.assertEqual(leftovers, [], f'temp files left behind: {leftovers}') + + def test_failed_write_leaves_the_previous_file_intact(self): + path = os.path.join(self.etc, 'atomic.cfg') + with open(path, 'w') as fh: + fh.write('old content\n') + + # Anything that makes f.write() blow up mid-flight stands in for a full + # disk / killed container. TypeError specifically, not Exception: a + # bare assertRaises(Exception) also swallows the AttributeError raised + # when write_config_atomically does not exist at all, so this test + # passed against the pre-fix tree and would keep passing if the + # function were deleted. + with self.assertRaises(TypeError): + hm.write_config_atomically(path, object()) + + self.assertEqual(self.read(path), 'old content\n', + 'a failed write clobbered the previous config') + leftovers = [n for n in os.listdir(self.etc) if n.endswith('.tmp')] + self.assertEqual(leftovers, [], f'temp files left behind: {leftovers}') + + +class TestBackupFailureGuard(RollbackTestCase): + """generate_config() refuses to write when no rollback target could be taken.""" + + def test_a_failed_backup_stops_the_config_from_being_written(self): + good = self.generate_good_config() + self.block_ip('192.0.2.10') + hm.update_blocked_ips_map() + good_map = self.read(hm.BLOCKED_IPS_MAP_PATH) + + real_create_backup = hm.create_backup + hm.create_backup = lambda *a, **kw: (False, 'error') + self.addCleanup(setattr, hm, 'create_backup', real_create_backup) + + self.add_domain('second.example.com', 'second_backend', '10.0.0.3') + with self.assertRaises(Exception) as ctx: + hm.generate_config() + + self.assertIn('Refusing to regenerate', str(ctx.exception), + 'a backup failure was not reported as a refusal') + self.assertEqual( + self.read(hm.HAPROXY_CONFIG_PATH), good, + 'a new config was written even though the snapshot failed - a bad ' + 'change could not have been rolled back') + self.assertEqual( + self.read(hm.BLOCKED_IPS_MAP_PATH), good_map, + 'the blocked IPs map was rewritten even though the snapshot failed') + + +class TestValidatorAvailability(RollbackTestCase): + """'the validator could not run' is not the same as 'the config is bad'.""" + + def _hide_the_haproxy_binary(self): + empty = os.path.join(self.tmp, 'empty-bin') + os.makedirs(empty, exist_ok=True) + os.environ['PATH'] = empty + # setUp's cleanup restores the original PATH. + + def test_a_missing_validator_is_unavailable_not_invalid(self): + good = self.generate_good_config() + self._hide_the_haproxy_binary() + + status, message = hm.validate_config_file(hm.HAPROXY_CONFIG_PATH) + self.assertEqual(status, 'unavailable', + 'a validator that could not run was reported as a ' + f'verdict on the config ({status}: {message})') + + # And the consequence create_backup() draws from it: a config it could + # not check is still snapshotted, because refusing would leave the box + # with no rollback target at all. Contrast + # test_a_broken_current_config_does_not_replace_a_good_backup, where a + # real 'invalid' verdict yields 'kept_previous'. + with open(hm.HAPROXY_CONFIG_PATH, 'w') as fh: + fh.write('hand written, unverifiable\n') + ok, status = hm.create_backup() + self.assertTrue(ok) + self.assertEqual(status, 'created', + 'an unverifiable config was treated as a rejected one') + self.assertEqual(self.read(hm.HAPROXY_BACKUP_PATH), + 'hand written, unverifiable\n') + self.assertNotEqual(self.read(hm.HAPROXY_BACKUP_PATH), good) + + +class TestFileComparison(RollbackTestCase): + """The fast path compares bytes, not sizes.""" + + def test_same_size_different_content_is_not_identical(self): + a = os.path.join(self.etc, 'a') + b = os.path.join(self.etc, 'b') + with open(a, 'w') as fh: + fh.write('aaaa\n') + with open(b, 'w') as fh: + fh.write('aaba\n') + self.assertEqual(os.path.getsize(a), os.path.getsize(b), + 'fixture: the two files must be the same size') + self.assertFalse(hm._files_identical(a, b), + 'two same-size files with different bytes compared equal') + + def test_a_same_size_drifted_config_still_hits_the_validation_gate(self): + """The case the byte-compare exists for. + + A config edited in place without changing its length (one character + swapped, a hostname replaced by another of the same width) must not be + mistaken for the known-good backup and waved through. + """ + good = self.generate_good_config() + # Sized in BYTES, not characters: the rendered config contains + # non-ASCII (em dashes in template comments), so len(str) would be + # smaller than the file and this test would pass for the wrong reason. + size = os.path.getsize(hm.HAPROXY_CONFIG_PATH) + broken = f'# {BROKEN_TOKEN}\n'.encode() + broken += b'#' * (size - len(broken) - 1) + b'\n' + with open(hm.HAPROXY_CONFIG_PATH, 'wb') as fh: + fh.write(broken) + self.assertEqual(os.path.getsize(hm.HAPROXY_CONFIG_PATH), size, + 'fixture: the drifted config must be the same size') + + ok, status = hm.create_backup() + self.assertTrue(ok) + self.assertEqual( + status, 'kept_previous', + 'a same-size broken config was accepted as unchanged and skipped ' + 'the validation gate') + self.assertEqual(self.read(hm.HAPROXY_BACKUP_PATH), good, + 'the known-good backup was overwritten') + + +class TestBlockedIpsMapWrites(RollbackTestCase): + """blocked_ips.map is loaded by `haproxy -c`, so it gets the same care. + + Verified against HAProxy 2.8: with the map referenced by + map_ip(/etc/haproxy/blocked_ips.map,0), a half-written final line makes the + WHOLE configuration invalid ("'198.51.10' is not a valid IPv4 or IPv6 + address at line 2 of file ..."), not merely a dropped entry. + """ + + def test_the_map_goes_through_the_atomic_writer(self): + seen = [] + real_write = hm.write_config_atomically + + def spy(path, content, *args, **kwargs): + seen.append(path) + return real_write(path, content, *args, **kwargs) + + hm.write_config_atomically = spy + self.addCleanup(setattr, hm, 'write_config_atomically', real_write) + + self.block_ip('192.0.2.10') + self.assertTrue(hm.update_blocked_ips_map()) + self.assertIn(hm.BLOCKED_IPS_MAP_PATH, seen, + 'the blocked IPs map was written without the atomic ' + 'writer - a truncated map is a fatal config') + + def test_a_crash_before_the_rename_leaves_the_old_map_intact(self): + self.block_ip('192.0.2.10') + hm.update_blocked_ips_map() + good_map = self.read(hm.BLOCKED_IPS_MAP_PATH) + + real_replace = os.replace + + def boom(src, dst, *args, **kwargs): + if dst == hm.BLOCKED_IPS_MAP_PATH: + raise OSError('simulated crash between write and rename') + return real_replace(src, dst, *args, **kwargs) + + os.replace = boom + self.addCleanup(setattr, os, 'replace', real_replace) + + self.block_ip('198.51.100.20') + self.assertFalse(hm.update_blocked_ips_map(), + 'a failed map write was reported as success') + os.replace = real_replace + + self.assertEqual( + self.read(hm.BLOCKED_IPS_MAP_PATH), good_map, + 'an interrupted map write clobbered the map HAProxy is running') + leftovers = [n for n in os.listdir(self.etc) if n.endswith('.tmp')] + self.assertEqual(leftovers, [], f'temp files left behind: {leftovers}') + + def test_a_malformed_map_is_not_recorded_as_known_good(self): + """The map backup must stay something HAProxy would actually load.""" + self.generate_good_config() + good_map = self.read(hm.BLOCKED_IPS_MAP_BACKUP_PATH) + + # Straight into the table, the way a bad row gets there in the first + # place - the API route is not the only writer. + self.block_ip('not-an-ip') + hm.update_blocked_ips_map() + + self.assertEqual( + self.read(hm.BLOCKED_IPS_MAP_BACKUP_PATH), good_map, + 'a map HAProxy cannot parse was promoted to the rollback target') + + def test_no_map_backup_is_fabricated_before_a_config_exists(self): + """Nothing to stay in step with means nothing to write.""" + self.block_ip('192.0.2.10') + self.assertTrue(hm.update_blocked_ips_map()) + self.assertFalse( + os.path.exists(hm.BLOCKED_IPS_MAP_BACKUP_PATH), + 'a rollback target was invented out of a map write alone') + + +class TestValidationCost(RollbackTestCase): + """`haproxy -c` runs are the customer-facing cost of a config change. + + generate_config() runs synchronously inside the API call that adds a + domain, and on an edge with hundreds of certificates `haproxy -c` is the + expensive part. These counts are the contract; changing them should be a + deliberate decision, not a side effect. + """ + + def _count_validations(self, action): + calls = [] + real_validate = hm.validate_config_file + + def spy(path): + calls.append(path) + return real_validate(path) + + hm.validate_config_file = spy + try: + action() + finally: + hm.validate_config_file = real_validate + return len(calls) + + def test_blocking_an_ip_does_not_add_a_validation_to_the_next_change(self): + self.generate_good_config() + + def add(domain, backend, address): + self.add_domain(domain, backend, address) + hm.generate_config() + + steady = self._count_validations( + lambda: add('a.example.com', 'a_backend', '10.0.0.4')) + self.assertEqual( + steady, 1, + 'a steady-state config change should cost exactly one `haproxy -c` ' + '(the pre-reload gate); the known-good fast path should skip the ' + f'other one, but {steady} ran') + + # What POST /api/blocked-ips does: rewrite the map outside + # generate_config(). This fleet blocks IPs automatically, so it happens + # between most config changes. + self.block_ip('192.0.2.10') + hm.update_blocked_ips_map() + + after_block = self._count_validations( + lambda: add('b.example.com', 'b_backend', '10.0.0.5')) + self.assertEqual( + after_block, steady, + 'an IP block left the map out of step with its backup, so the next ' + f'domain add paid {after_block} `haproxy -c` runs instead of ' + f'{steady} - on the customer-facing call') + + +if __name__ == '__main__': + print(f"testing haproxy_manager from: {MODULE_DIR}") + unittest.main(verbosity=2)