From 22dab685d03d904333fc456b53153aeecc2716b0 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Thu, 6 Aug 2026 08:58:47 -0700 Subject: [PATCH] fix(certs): never truncate a live PEM; validate before publishing Every path that refreshed a combined certificate did with open(combined_path, 'w') as combined: # TRUNCATES subprocess.run(['cat', cert, key], stdout=combined) # rc ignored where combined_path is the bundle HAProxy is currently serving. The live file was emptied before any source material had been read and the cat status was never checked, so an unreadable source, a zero-length privkey, a full disk or a killed container 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 ssl bind - HTTPS down for every site on the host, and unlike a bad haproxy.cfg it is not recoverable by config rollback. The bundle endpoint compounded it: superseded .pem files were unlinked and their lineages `certbot delete`d before anything had checked the replacement, destroying both copies of a working certificate. Recovery there means fresh, rate-limited ACME orders. Bundles are now assembled in a staging directory beside the crt directory (never inside it - HAProxy would try to load a temp file), validated there, and swapped in with os.replace(). Validation is mandatory structural checks in pure Python plus a best-effort openssl key/leaf pairing check; a missing openssl warns loudly and does not silently pass. The previous bundle is copied to /etc/haproxy/cert-backups first. Superseded certs are moved aside rather than deleted, and `certbot delete` runs only after HAProxy has reloaded onto the replacement. The same guarantees are implemented for the cron/renewal shell path in scripts/cert-publish-lib.sh, which also gates the reload on `haproxy -c`. Reuses write_config_atomically() from the config-rollback fix (extended with staging_dir/validate) rather than adding a second atomic writer. Also heals a zero-byte QUIC cluster-secret file, which previously made get_or_create_cluster_secret() return '' forever. Tests: scripts/test-cert-write-safety.py (17) and scripts/test-cert-scripts.py (32) in the existing stdlib-unittest, stub-binary convention. Both bug classes reproduce against the pre-fix tree via HAPROXY_MANAGER_DIR. Co-Authored-By: Claude Opus 5 (1M context) --- haproxy_manager.py | 578 +++++++++++++++++++++-- scripts/cert-publish-lib.sh | 294 ++++++++++++ scripts/renew-certificates.sh | 33 +- scripts/sync-certificates.sh | 33 +- scripts/test-cert-scripts.py | 636 ++++++++++++++++++++++++++ scripts/test-cert-write-safety.py | 737 ++++++++++++++++++++++++++++++ 6 files changed, 2254 insertions(+), 57 deletions(-) create mode 100644 scripts/cert-publish-lib.sh create mode 100755 scripts/test-cert-scripts.py create mode 100755 scripts/test-cert-write-safety.py diff --git a/haproxy_manager.py b/haproxy_manager.py index 7390cd6..2780673 100644 --- a/haproxy_manager.py +++ b/haproxy_manager.py @@ -113,6 +113,12 @@ BLOCKED_IPS_MAP_BACKUP_PATH = '/etc/haproxy/blocked_ips.map.backup' 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 @@ -309,6 +315,298 @@ 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. Accepting it here would let one through on a host without the +# openssl CLI, where the pairing check (which would also reject it) is skipped. +_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. + """ + 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. The Dockerfile installs haproxy, certbot, + socat and curl but not the openssl CLI, so this is a real possibility. + Callers treat it as a loud warning rather than a failure: the failure modes + this whole module exists to prevent (truncation, missing key, partial + write) are fully covered by validate_pem_structure(), whereas refusing to + publish whenever the checker is missing would stall renewals fleet-wide and + let certificates expire - a guaranteed outage traded for a hypothetical + one. A mismatch, when we CAN check, is always fatal. + """ + 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). + + Mandatory structural validation plus a best-effort cryptographic pairing + check - see _openssl_pairing_status() for what happens when openssl is + missing (loud warning, structural verdict stands). + """ + try: + with open(path, 'r') as fh: + text = fh.read() + except OSError as e: + 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.warning( + "Certificate key/leaf pairing check SKIPPED for %s (%s). The " + "bundle passed structural validation only.", path, pair_msg) + 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 as e: + 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 as e: + 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) @@ -337,12 +635,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 @@ -581,8 +893,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: @@ -611,8 +930,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 @@ -620,10 +939,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': []} @@ -675,26 +1008,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(): @@ -794,8 +1155,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. @@ -816,15 +1186,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; ' @@ -861,11 +1251,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 @@ -876,15 +1267,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: @@ -1077,9 +1493,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() @@ -1700,11 +2126,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: @@ -1750,6 +2178,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) @@ -2111,7 +2573,7 @@ def _config_backup_pairs(): ) -def write_config_atomically(path, content): +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 @@ -2119,8 +2581,26 @@ def write_config_atomically(path, content): 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 = os.path.dirname(path) or '.' + 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: @@ -2136,6 +2616,10 @@ def write_config_atomically(path, 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: diff --git a/scripts/cert-publish-lib.sh b/scripts/cert-publish-lib.sh new file mode 100644 index 0000000..58a382d --- /dev/null +++ b/scripts/cert-publish-lib.sh @@ -0,0 +1,294 @@ +# 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. If the mv ever fails (EXDEV because someone +# mounted the certs dir separately, permissions, ...) we FAIL LOUDLY and leave +# the live pem alone. There is deliberately no "just write it directly" path. + +# 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 (MANDATORY, pure shell/grep, always available): structural checks. +# A missing or failed structural check is a HARD FAIL. This layer is +# what actually covers the truncation / partial-write / key-less +# failure modes this library exists to prevent. +# Layer 2 (BEST EFFORT): cryptographic pairing via the openssl CLI. +# If openssl runs and says the cert and key do not match, that is a +# HARD FAIL. If the openssl BINARY IS ABSENT we log a loud warning and +# accept the bundle on the structural checks alone. +# +# Rationale for not hard-failing on a missing checker: the container +# image (see Dockerfile) installs haproxy, certbot and socat, but not +# necessarily the openssl CLI. Refusing to publish when the checker is +# missing would stall every renewal fleet-wide and let certificates +# expire - a guaranteed outage - which is strictly worse than the risk +# it prevents, since a mismatched pair can only arise from a +# mis-assembled source tree, whereas the truncation modes we are +# actually defending against are fully covered by layer 1. +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 + if [ ! -s "$file" ]; then + log_error "Certificate bundle $file is empty" + return 1 + fi + + # --- layer 1: structure ------------------------------------------------- + if ! grep -qF -- '-----BEGIN CERTIFICATE-----' "$file"; then + log_error "Certificate bundle $file contains no certificate block" + return 1 + fi + if ! grep -qF -- '-----END CERTIFICATE-----' "$file"; 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-----' "$file")" + 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" "$file"; then + log_error "Certificate bundle $file has an unterminated private key block (truncated?)" + return 1 + fi + + # --- layer 2: cert/key pairing ------------------------------------------ + if ! command -v openssl >/dev/null 2>&1; then + # Once per process: this fires per domain otherwise, and a renewal run + # walks every certificate on the host. + if [ -z "${_CERT_OPENSSL_WARNED:-}" ]; then + _CERT_OPENSSL_WARNED=1 + log_warn "openssl binary not found - SKIPPING the cert/key pairing check" \ + "(openssl x509 -pubkey vs openssl pkey -pubout);" \ + "certificate bundles are being published on structural checks alone" + fi + return 0 + fi + + local cert_pub key_pub + # /dev/null /dev/null /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 + + # (c) 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 + + # (d) 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. + local mode + mode="$(stat -c '%a' "$dest_file" 2>/dev/null)" + [ -n "$mode" ] || mode=644 + chmod "$mode" "$tmp" 2>/dev/null + + # (e) atomic swap. Same filesystem by construction; 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 diff --git a/scripts/sync-certificates.sh b/scripts/sync-certificates.sh index 4743cb0..e84dd1f 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 diff --git a/scripts/test-cert-scripts.py b/scripts/test-cert-scripts.py new file mode 100755 index 0000000..3133280 --- /dev/null +++ b/scripts/test-cert-scripts.py @@ -0,0 +1,636 @@ +#!/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. The one test that +needs openssl to *verify* pairing skips itself if the binary is absent. +""" + +import os +import re +import shutil +import subprocess +import tempfile +import textwrap +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----- +""" + +# 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 + +# --- 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): + 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. + for root, dirs, files in os.walk(self.tmp): + for name in files: + try: + os.chmod(os.path.join(root, name), 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') + + +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() + self.assertEqual(result.returncode, 0, + 'a per-domain failure should not change the exit code') + + 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: the cert/key pairing ' + 'check is best-effort and is skipped by design') + 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): + return subprocess.run( + ['bash', '-c', '. "$1"; shift; ' + snippet, '_', LIB, *args], + env=self.env(), 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.assertNotEqual(self.call('cert_bundle_valid "$1"', empty).returncode, 0) + missing = os.path.join(self.tmp, 'nope.pem') + self.assertNotEqual(self.call('cert_bundle_valid "$1"', missing).returncode, 0) + + 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.assertNotEqual(self.call('cert_bundle_valid "$1"', truncated).returncode, 0) + + 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.""" + self.seed_previous_bundle() + before = read(self.live_pem) + os.chmod(self.certs_dir, 0o500) # no writes: mv will fail + self.addCleanup(os.chmod, self.certs_dir, 0o755) + if os.geteuid() == 0: + self.skipTest('root ignores directory permissions') + + result = self.call('cert_publish "$1" "$2" "$3"', + self.src_cert, self.src_key, self.live_pem) + + self.assertNotEqual(result.returncode, 0, + 'a failed rename was reported as success') + self.assertEqual(read(self.live_pem), before, + 'the live pem was damaged by a failed rename') + self.assert_no_staging_leftovers() + + 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 test_missing_openssl_warns_but_does_not_block(self): + """Best-effort layer: a missing checker must not stall renewals.""" + fake_path = os.path.join(self.tmp, 'no-openssl-bin') + os.makedirs(fake_path) + for tool in ('cat', 'grep', 'mktemp', 'mv', 'cp', 'rm', 'mkdir', + 'basename', 'dirname', 'find', 'date', 'chmod'): + real = shutil.which(tool) + if real: + os.symlink(real, os.path.join(fake_path, tool)) + 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.assertEqual(result.returncode, 0, + 'a missing openssl blocked publication') + self.assertRegex(result.stdout + result.stderr, + r'(?i)warning.*openssl', + 'the skipped pairing check was not announced loudly') + + def test_missing_openssl_still_rejects_a_structurally_broken_bundle(self): + fake_path = os.path.join(self.tmp, 'no-openssl-bin2') + os.makedirs(fake_path) + for tool in ('cat', 'grep', 'date'): + real = shutil.which(tool) + if real: + os.symlink(real, os.path.join(fake_path, tool)) + path = write(os.path.join(self.tmp, 'nokey.pem'), TEST_CERT) + + # 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.assertNotEqual(result.returncode, 0, + 'structural checks stopped being mandatory') + + +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..9125f53 --- /dev/null +++ b/scripts/test-cert-write-safety.py @@ -0,0 +1,737 @@ +#!/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 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 = unittest.skipUnless( + HAS_PUBLISHER, 'requires 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 + +# 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): + """A good bundle already being served for `domain`.""" + return self.write(os.path.join(self.certs, f'{domain}.pem'), GOOD_BUNDLE) + + 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.""" + cert_path = self.publish_live_bundle('ok.example.com') + self.add_domain('ok.example.com', 'ok_backend', ssl_cert_path=cert_path) + renewed = LEAF_CERT.replace('\n-----END CERTIFICATE-----', + '\n-----END CERTIFICATE-----') + self.make_lineage('ok.example.com', cert=renewed) + + resp = self.client.post('/api/certificates/renew') + + self.assertEqual(200, resp.status_code, resp.get_data(as_text=True)) + self.assertTrue(structurally_valid(self.read(cert_path))) + self.assertTrue(self.edge_would_start()) + self.assert_only_final_pems_in_certs_dir() + + +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.""" + for path in (hm.cert_staging_dir(), hm.cert_backup_dir()): + self.assertFalse( + os.path.abspath(path).startswith(os.path.abspath(self.certs) + os.sep), + f'{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 + def test_no_temp_file_survives_a_failed_publish(self): + dest = os.path.join(self.certs, 'leak.example.com.pem') + cert = self.write(os.path.join(self.tmp, 'src4', 'fullchain.pem'), + LEAF_CERT) + empty = self.write(os.path.join(self.tmp, 'src4', 'privkey.pem'), '') + with self.assertRaises(hm.CertificatePublishError): + hm.publish_pem_bundle(dest, [cert, empty]) + self.assert_only_final_pems_in_certs_dir() + self.assertEqual( + [], os.listdir(hm.cert_staging_dir()) if + os.path.isdir(hm.cert_staging_dir()) else [], + 'staged files must be cleaned up when a publish fails') + + +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)