From e58996d6c93eb8820831511bdc9765c215cabe38 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Fri, 9 Oct 2026 11:05:07 -0700 Subject: [PATCH 1/3] fix(ssl): retire SAN-overlapping certs, never publish expired lineages, report no-op renewals brainjar.net served an expired certificate for two days (2026-10-07..09): the old brain-jar.com bundle (CN brain-jar.com, SANs incl. all eight brainjar names) stayed in /etc/haproxy/certs and won the SNI match over the valid brainjar.net bundle, because superseded-cert cleanup matched on CN only, and renew-certificates.sh re-published the expired lineage every 12h. - Retirement now matches on ANY shared name (CN or SAN). The overlapping pem is moved to cert-retired/ and its lineage's live/ dir and renewal conf to live-retired/ and renewal-retired/ (after HAProxy loads the new bundle). Nothing is deleted any more: `certbot delete` is gone from this path. - Shared-PEM safety: a candidate is kept when it is still the only valid certificate for a name in the domains table (retirement_blockers()). Expired certs never block; names the caller lists in the new optional `unreachable_names` body field do not block. - /api/ssl/bundle passes --keep-until-expiring and reports `renewed` (fingerprint before vs after), `certbot_not_due` and the certificate's `not_after`, so WHP can stop recording certbot no-ops as renewals. - Never publish an expired certificate: renew-certificates.sh and sync-certificates.sh skip expired lineages (ERROR-logged, counted separately), and cert_publish / publish_pem_bundle refuse expired leaves. - VERSION 2026.10.3. Tests: test-cert-write-safety.py 31 -> 47, test-cert-scripts.py 38 -> 44, each new group verified to fail against a reverted fix. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 1 + VERSION | 2 +- haproxy_manager.py | 489 ++++++++++++++---- scripts/cert-publish-lib.sh | 30 ++ scripts/renew-certificates.sh | 13 +- scripts/sync-certificates.sh | 13 +- scripts/test-cert-scripts.py | 72 +++ scripts/test-cert-write-safety.py | 266 +++++++++- scripts/test-fixtures/san-certs/dead_old.crt | 18 + scripts/test-fixtures/san-certs/dead_old.key | 28 + .../test-fixtures/san-certs/expired_old.crt | 19 + .../test-fixtures/san-certs/expired_old.key | 28 + .../test-fixtures/san-certs/new_bundle.crt | 18 + .../test-fixtures/san-certs/new_bundle.key | 28 + .../test-fixtures/san-certs/shared_old.crt | 19 + .../test-fixtures/san-certs/shared_old.key | 28 + scripts/test-fixtures/san-certs/unrelated.crt | 18 + scripts/test-fixtures/san-certs/unrelated.key | 28 + 18 files changed, 1006 insertions(+), 112 deletions(-) create mode 100644 scripts/test-fixtures/san-certs/dead_old.crt create mode 100644 scripts/test-fixtures/san-certs/dead_old.key create mode 100644 scripts/test-fixtures/san-certs/expired_old.crt create mode 100644 scripts/test-fixtures/san-certs/expired_old.key create mode 100644 scripts/test-fixtures/san-certs/new_bundle.crt create mode 100644 scripts/test-fixtures/san-certs/new_bundle.key create mode 100644 scripts/test-fixtures/san-certs/shared_old.crt create mode 100644 scripts/test-fixtures/san-certs/shared_old.key create mode 100644 scripts/test-fixtures/san-certs/unrelated.crt create mode 100644 scripts/test-fixtures/san-certs/unrelated.key diff --git a/CLAUDE.md b/CLAUDE.md index f4c43cf..e2e69d4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,6 +10,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co - **Stick-table contract**: `python3 scripts/test-stick-table-contract.py` - offline; holds the templates' `store` clauses, `STICK_TABLE_FIELD_CONTRACT`, and every consumer to each other. Run it after touching any `stick-table` line. - **Runtime-map contract**: `python3 scripts/test-runtime-map-contract.py` - offline; asserts the runtime map commands are `@1`-prefixed, reference the map by FILE PATH (never `#`), carry the value `1`, and that every captured rejection is classified as a failure. Run it after touching any `add map`/`del map`/`clear map` path. - **Certificate destruction safety**: `python3 scripts/test-cert-write-safety.py` - offline; asserts a live `.pem` is never truncated, removed, or its certbot lineage deleted while any configured domain still references it (one bundle serves many names, so `ssl_cert_path` is routinely shared). Run it after touching any `os.remove`/`certbot delete`/PEM-write path. +- **Superseded-cert retirement**: covered by the same file (`TestRetirementRules`, `TestSanOverlapRetirement`). A new bundle retires any OTHER pem/lineage sharing ANY name (CN or SAN) with it - moved to `cert-retired/`, `live-retired/`, `renewal-retired/`, never deleted - unless that cert is still the only valid one for a name this edge serves (`retirement_blockers()`). Expired lineages are never published (`cert_is_expired` in `cert-publish-lib.sh`, `leaf_is_expired()` in Python). Fixtures: `scripts/test-fixtures/san-certs/`. - **Manual Testing**: Run `curl` commands against `http://localhost:8000` endpoints as shown in README.md ### Reading stick tables (and why it is easy to get silently wrong) diff --git a/VERSION b/VERSION index 3715deb..7df8db9 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2026.09.1 +2026.10.3 diff --git a/haproxy_manager.py b/haproxy_manager.py index 2e023bf..a04d23d 100644 --- a/haproxy_manager.py +++ b/haproxy_manager.py @@ -8,7 +8,7 @@ import socket import psutil import functools import logging -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone import json import ipaddress import shutil @@ -570,6 +570,19 @@ def backup_existing_pem(dest_path): return None +def leaf_is_expired(path): + """True only when openssl positively says the first certificate in `path` + has expired. Unreadable/missing openssl returns False: those cases are + rejected, with a better message, by validate_pem_bundle().""" + try: + proc = subprocess.run( + ['openssl', 'x509', '-checkend', '0', '-noout', '-in', path], + capture_output=True, text=True, stdin=subprocess.DEVNULL) + except Exception: + return False + return proc.returncode == 1 and 'will expire' in (proc.stdout or '') + + def publish_pem_bundle(dest_path, source_paths): """Publish cert+key as a combined PEM at dest_path, atomically. @@ -611,6 +624,15 @@ def publish_pem_bundle(dest_path, source_paths): raise CertificatePublishError( f'assembled bundle for {dest_path} is not usable: {msg}') + # Never publish an expired certificate. HAProxy loads the crt directory + # wholesale and serves the first file that claims an SNI name, so an + # expired file in there can only ever shadow a valid one (brain-jar.com.pem + # did exactly that to brainjar.net on 2026-10-07). + if leaf_is_expired(source_paths[0]): + raise CertificatePublishError( + f'refusing to publish {dest_path}: the certificate in ' + f'{source_paths[0]} has expired') + dest_dir = os.path.dirname(dest_path) or '.' try: os.makedirs(dest_dir, exist_ok=True) @@ -972,132 +994,336 @@ def request_ssl(): log_operation('request_ssl', False, str(e)) return jsonify({'status': 'error', 'message': str(e)}), 500 -def _quarantine_superseded_certs(keep_path, keep_lineage, bundle_names): - """Move aside cert files that the just-issued bundle supersedes. +# --------------------------------------------------------------------------- +# Superseded-certificate retirement (incident 2026-10-07, brainjar.net) +# --------------------------------------------------------------------------- +# HAProxy loads /etc/haproxy/certs as a DIRECTORY and, when two files claim the +# same SNI name, serves whichever it loaded first. So a new bundle has no effect +# while an older file that also lists one of its names is still in there. +# +# brainjar.net served an EXPIRED certificate for two days for exactly that +# reason. The orchestrator re-bundled brainjar.net and its seven siblings +# without the dead brain-jar.com, but the cleanup that was meant to move the old +# file aside matched on the old certificate's CN only. Its CN was brain-jar.com, +# which was not in the new bundle, so brain-jar.com.pem (which also listed all +# eight brainjar names) stayed put and kept winning the SNI match. On top of +# that, renew-certificates.sh re-published the old lineage from live/ every 12 +# hours, so moving the file alone would not have stuck. +# +# What retirement does now: +# * any OTHER pem/lineage that shares ANY name with the new bundle (CN or +# SAN) is a candidate, not only one whose CN is in the bundle; +# * a candidate is moved aside only if no name that this edge still serves +# would lose its last valid certificate (the shared-PEM hazard: one bundle +# routinely serves several sites' names, see domains_referencing_cert_path); +# * nothing is deleted. The pem goes to the cert-retired sibling directory and +# the lineage's live/ and renewal/.conf go to live-retired/ and +# renewal-retired/, so certbot stops renewing it and renew-certificates.sh +# stops re-publishing it. Every move is logged and can be reversed with mv. +# (This used to `certbot delete` the lineage, which is irreversible.) - 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 - file. We don't look at SANs of the OLD certs — being the CN is enough, - since that's what HAProxy SNI-matches against and what the file - convention names it after. +# Where certbot keeps its lineages. A module global so the tests can repoint +# it; find_certbot_live_dir() predates it and still hardcodes the live path. +LETSENCRYPT_DIR = '/etc/letsencrypt' - 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: - * 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. +def cert_retired_dir(): + """Where retired .pem files are moved. Sibling of the crt dir, never inside.""" + return _cert_sibling_dir('cert-retired') - 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. +def parse_cert_text(text): + """Parse `openssl x509 -noout -subject -nameopt multiline -enddate + -fingerprint -sha256 -ext subjectAltName` output. + + Returns {'cn', 'names', 'not_after', 'fingerprint'}; names is the lower-cased + set of the CN plus every DNS SAN. not_after is a UTC epoch or None. """ - summary = {'removed': [], 'errors': [], 'skipped': []} + cn = None + names = set() + not_after = None + fingerprint = None + in_san = False + for raw in text.splitlines(): + line = raw.strip() + if line.startswith('commonName') and '=' in line and cn is None: + cn = line.split('=', 1)[1].strip().lower() + continue + if line.startswith('notAfter='): + try: + not_after = int(datetime.strptime( + ' '.join(line.split('=', 1)[1].split()), + '%b %d %H:%M:%S %Y %Z').replace( + tzinfo=timezone.utc).timestamp()) + except ValueError: + not_after = None + continue + if '=' in line and line.lower().split('=', 1)[0].endswith('fingerprint'): + fingerprint = line.split('=', 1)[1].strip().upper() + continue + if line.startswith('X509v3 Subject Alternative Name'): + in_san = True + continue + if in_san: + for part in line.split(','): + part = part.strip() + if part.startswith('DNS:'): + names.add(part[4:].strip().lower()) + in_san = False + if cn: + names.add(cn) + return {'cn': cn, 'names': names, 'not_after': not_after, + 'fingerprint': fingerprint} + +def read_cert_info(path): + """parse_cert_text() for the first certificate in `path`, or None.""" + try: + proc = subprocess.run( + ['openssl', 'x509', '-in', path, '-noout', '-subject', + '-nameopt', 'multiline', '-enddate', '-fingerprint', '-sha256', + '-ext', 'subjectAltName'], + capture_output=True, text=True, stdin=subprocess.DEVNULL) + except Exception: + return None + if proc.returncode != 0: + return None + info = parse_cert_text(proc.stdout) + return info if info['names'] else None + + +def cert_name_covers(cert_name, host): + """Does certificate name `cert_name` (maybe `*.x`) cover hostname `host`?""" + cert_name = (cert_name or '').lower() + host = (host or '').lower() + if cert_name == host: + return True + if cert_name.startswith('*.') and not host.startswith('*.'): + suffix = cert_name[1:] # ".example.com" + head = host[:-len(suffix)] if host.endswith(suffix) else '' + return bool(head) and '.' not in head + return False + + +def _cert_covers(names, host): + return any(cert_name_covers(n, host) for n in names) + + +def _served_names(): + """Every hostname this edge has a route for (the SQLite domains table).""" + try: + with sqlite3.connect(DB_FILE) as conn: + return {row[0].lower() for row in + conn.execute('SELECT domain FROM domains').fetchall() + if row[0]} + except Exception as e: + # Unknown served set -> treat every name as served, i.e. never retire a + # still-valid certificate on the strength of a failed query. + logger.error("Could not read the domains table for retirement checks: %s", e) + return None + + +def retirement_blockers(candidate, others, bundle_names, served_names, + unreachable_names, now=None): + """Names that would lose their LAST valid certificate if `candidate` went. + + candidate/others are read_cert_info() dicts. `others` are the certificates + that would remain in the crt directory. Pure, so it is unit-testable. + + * An expired candidate blocks nothing: it is not a valid certificate for + anyone, and while it sits in the crt dir it can only shadow a good one. + * A name the caller asserts is not served here (unreachable_names - DNS + points elsewhere or nowhere) blocks nothing. + * A name with no route on this edge blocks nothing. + * served_names=None (could not read the DB) means "assume everything is + served", so a valid candidate is kept unless the bundle covers it all. + """ + now = time.time() if now is None else now + if candidate.get('not_after') is None: + return [''] + if candidate['not_after'] <= now: + return [] + unreachable = {n.lower() for n in (unreachable_names or ())} + bundle = {n.lower() for n in bundle_names} + if served_names is None: + candidates_to_check = set(candidate['names']) + else: + candidates_to_check = {s for s in served_names + if _cert_covers(candidate['names'], s)} + blockers = [] + for name in sorted(candidates_to_check): + if name in unreachable or _cert_covers(bundle, name): + continue + covered_elsewhere = any( + o is not candidate + and o.get('fingerprint') != candidate.get('fingerprint') + and o.get('not_after') is not None and o['not_after'] > now + and _cert_covers(o['names'], name) + for o in others) + if not covered_elsewhere: + blockers.append(name) + return blockers + + +def _overlaps_bundle(info, bundle_names): + return any(_cert_covers(info['names'], b) for b in bundle_names) or \ + any(n in {b.lower() for b in bundle_names} for n in info['names']) + + +def _unique_dest(directory, name): + dest = os.path.join(directory, name) + if not os.path.exists(dest): + return dest + stamp = datetime.now(timezone.utc).strftime('%Y%m%dT%H%M%SZ') + dest = os.path.join(directory, f'{name}.{stamp}') + n = 1 + while os.path.exists(dest): + dest = os.path.join(directory, f'{name}.{stamp}.{n}') + n += 1 + return dest + + +def _quarantine_superseded_certs(keep_path, keep_lineage, bundle_names, + unreachable_names=()): + """Move aside .pem files in the crt dir that overlap the new bundle. + + Runs after the new bundle is published and before HAProxy reloads, so the + replacement is never the only thing we have. See the block comment above + for the rules. Returns {'removed', 'kept', 'skipped', 'errors'}. + """ + summary = {'removed': [], 'kept': [], 'skipped': [], 'errors': []} if not os.path.isdir(SSL_CERTS_DIR): return summary keep_basename = os.path.basename(keep_path) + served = _served_names() + # Inventory once; `remaining` shrinks as files are retired so the coverage + # check for the next candidate sees what will actually be left. + inventory = {} for fname in sorted(os.listdir(SSL_CERTS_DIR)): if not fname.endswith('.pem'): continue + info = read_cert_info(os.path.join(SSL_CERTS_DIR, fname)) + if info is None: + summary['skipped'].append({'file': fname, 'reason': 'openssl could not read it'}) + continue + inventory[fname] = info + + remaining = dict(inventory) + for fname, info in inventory.items(): if fname == keep_basename: continue - fpath = os.path.join(SSL_CERTS_DIR, fname) - try: - cn_proc = subprocess.run( - ['openssl', 'x509', '-in', fpath, '-noout', '-subject', '-nameopt', 'multiline'], - capture_output=True, text=True - ) - if cn_proc.returncode != 0: - summary['skipped'].append({'file': fname, 'reason': 'openssl read failed'}) - continue - # `-nameopt multiline` lays out the subject one RDN per line; CN is - # the row matching `commonName`. Robust against unusual subject orderings. - cn = None - for line in cn_proc.stdout.splitlines(): - line = line.strip() - if line.startswith('commonName'): - # format: "commonName = example.com" - parts = line.split('=', 1) - if len(parts) == 2: - cn = parts[1].strip() - break - if not cn: - summary['skipped'].append({'file': fname, 'reason': 'no CN found'}) - continue - except Exception as e: - summary['skipped'].append({'file': fname, 'reason': f'inspect failed: {e}'}) + if not _overlaps_bundle(info, bundle_names): continue - - if cn not in bundle_names: - continue # not superseded — different domain group - - # This file's CN is now part of our new bundle — supersede it. - lineage_name = fname[:-len('.pem')] - if lineage_name == keep_lineage: - # Defensive: shouldn't happen because of keep_basename check, but - # don't accidentally drop the lineage we just wrote. + others = [i for f, i in remaining.items() if f != fname] + blockers = retirement_blockers(info, others, bundle_names, served, + unreachable_names) + if blockers: + logger.warning( + "Not retiring %s although it overlaps the new %s bundle: it is " + "still the only valid certificate for %s", fname, keep_lineage, + ', '.join(blockers)) + summary['kept'].append({'file': fname, 'blockers': blockers}) continue - try: - 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) + os.makedirs(cert_retired_dir(), exist_ok=True) + dest = _unique_dest(cert_retired_dir(), fname) + shutil.move(os.path.join(SSL_CERTS_DIR, fname), dest) + remaining.pop(fname, None) + logger.warning( + "Retired superseded certificate %s (names: %s; expires %s) -> %s " + "because it overlaps the new %s bundle", + fname, ', '.join(sorted(info['names'])), + datetime.fromtimestamp(info['not_after'], timezone.utc).isoformat() + if info['not_after'] else 'unknown', dest, keep_lineage) summary['removed'].append({ 'file': fname, - 'cn': cn, - 'lineage': lineage_name, - 'moved_to': quarantine_path, - 'lineage_deleted': False, + 'cn': info['cn'], + 'names': sorted(info['names']), + 'expired': bool(info['not_after'] and info['not_after'] <= time.time()), + 'moved_to': dest, }) 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(). +def _retire_superseded_lineages(keep_lineage, bundle_names, unreachable_names=(), + keep_fingerprint=None): + """Move overlapping certbot lineages out of live/ and renewal/. - 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. + Same overlap and safety rules as _quarantine_superseded_certs(), evaluated + against the pem files that are left in the crt dir. Runs only after HAProxy + has loaded the new bundle. Moving live/ to live-retired/ (same depth) + keeps its relative ../../archive symlinks valid, so `mv` back restores it. """ - for entry in summary.get('removed', []): - lineage_name = entry.get('lineage') - if not lineage_name: + summary = {'retired': [], 'kept': [], 'errors': []} + live_root = os.path.join(LETSENCRYPT_DIR, 'live') + if not os.path.isdir(live_root): + return summary + served = _served_names() + + remaining_pems = [] + if os.path.isdir(SSL_CERTS_DIR): + for fname in sorted(os.listdir(SSL_CERTS_DIR)): + if fname.endswith('.pem'): + info = read_cert_info(os.path.join(SSL_CERTS_DIR, fname)) + if info is not None: + info['_file'] = fname + remaining_pems.append(info) + + for lineage in sorted(os.listdir(live_root)): + lineage_dir = os.path.join(live_root, lineage) + if lineage == keep_lineage or not os.path.isdir(lineage_dir): continue + cert_file = os.path.join(lineage_dir, 'cert.pem') + if not os.path.exists(cert_file): + cert_file = os.path.join(lineage_dir, 'fullchain.pem') + info = read_cert_info(cert_file) + if info is None or not _overlaps_bundle(info, bundle_names): + continue + if keep_fingerprint and info.get('fingerprint') == keep_fingerprint: + continue + # The lineage's own published pem is not "another" certificate, even if + # its fingerprint drifted from the lineage (renewed, not republished). + others = [p for p in remaining_pems if p['_file'] != f'{lineage}.pem'] + blockers = retirement_blockers(info, others, bundle_names, served, + unreachable_names) + if blockers: + logger.warning( + "Not retiring certbot lineage %s although it overlaps the new %s " + "bundle: it is still the only valid certificate for %s", + lineage, keep_lineage, ', '.join(blockers)) + summary['kept'].append({'lineage': lineage, 'blockers': blockers}) + continue + entry = {'lineage': lineage, 'names': sorted(info['names'])} 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] + live_dest_root = os.path.join(LETSENCRYPT_DIR, 'live-retired') + os.makedirs(live_dest_root, exist_ok=True) + live_dest = _unique_dest(live_dest_root, lineage) + shutil.move(lineage_dir, live_dest) + entry['live_moved_to'] = live_dest + conf = os.path.join(LETSENCRYPT_DIR, 'renewal', f'{lineage}.conf') + if os.path.exists(conf): + conf_dest_root = os.path.join(LETSENCRYPT_DIR, 'renewal-retired') + os.makedirs(conf_dest_root, exist_ok=True) + conf_dest = _unique_dest(conf_dest_root, f'{lineage}.conf') + shutil.move(conf, conf_dest) + entry['renewal_moved_to'] = conf_dest + logger.warning( + "Retired certbot lineage %s (names: %s) because it overlaps the " + "new %s bundle: live -> %s, renewal conf -> %s. Archive left in " + "place; reverse with mv to restore.", lineage, + ', '.join(entry['names']), keep_lineage, entry['live_moved_to'], + entry.get('renewal_moved_to', '(none)')) + summary['retired'].append(entry) except Exception as e: - entry['certbot_error'] = str(e) + entry['error'] = str(e) + summary['errors'].append(entry) return summary + @app.route('/api/ssl/bundle', methods=['POST']) @require_api_key def request_ssl_bundle(): @@ -1108,7 +1334,18 @@ def request_ssl_bundle(): /api/ssl calls when a site has multiple domains. Body: - {"primary": "example.com", "sans": ["www.example.com", ...]} + {"primary": "example.com", "sans": ["www.example.com", ...], + "unreachable_names": ["dead.example.com", ...]} # optional + + unreachable_names: names the caller (WHP's orchestrator) has established do + not resolve to this host. They are left out of the bundle by the caller; + here they only tell superseded-certificate retirement that an older cert's + coverage of them is not worth keeping (see retirement_blockers()). + + The response says whether certbot actually issued ("renewed") or found the + existing lineage not yet due and did nothing, and carries the published + certificate's not_after, so the caller can schedule from the real expiry + instead of recording a no-op as a renewal. The cert lineage uses --cert-name , so renewal under the same name doesn't proliferate -0001/-0002 dirs (the issue we hit with the @@ -1146,6 +1383,13 @@ def request_ssl_bundle(): return jsonify({'status': 'error', 'message': f'invalid SAN: {s!r}'}), 400 cleaned_sans.append(s) + unreachable_names = data.get('unreachable_names') or [] + if not isinstance(unreachable_names, list) or not all( + isinstance(n, str) for n in unreachable_names): + return jsonify({'status': 'error', + 'message': '"unreachable_names" must be a list of strings'}), 400 + unreachable_names = [n.strip().lower() for n in unreachable_names if n.strip()] + seen = {primary} names = [primary] for s in cleaned_sans: @@ -1160,8 +1404,11 @@ def request_ssl_bundle(): 'message': f'Too many SANs ({len(names)}); Let\'s Encrypt limit is 100', }), 400 + # --keep-until-expiring is what -n does anyway when the lineage exists with + # the same names; it is spelled out so the no-op case is deliberate, and it + # is detected below rather than reported as an issuance. cmd = [ - 'certbot', 'certonly', '-n', '--standalone', + 'certbot', 'certonly', '-n', '--standalone', '--keep-until-expiring', '--preferred-challenges', 'http', '--http-01-port=8688', '--cert-name', primary, ] @@ -1172,6 +1419,11 @@ def request_ssl_bundle(): # Defensive: clear any stale lock left by a SIGKILLed prior run. clear_stale_certbot_locks() + # fullchain.pem starts with the leaf, so it identifies the certificate. + before_dir = find_certbot_live_dir(primary) + before_info = read_cert_info(os.path.join(before_dir, 'fullchain.pem')) \ + if before_dir else None + result = subprocess.run(cmd, capture_output=True, text=True) if result.returncode != 0: stderr_excerpt = (result.stderr or '').strip()[:800] @@ -1196,6 +1448,20 @@ def request_ssl_bundle(): key_path = os.path.join(live_dir, 'privkey.pem') combined_path = f'{SSL_CERTS_DIR}/{primary}.pem' + # Did certbot issue, or did it find the lineage not yet due? Measured by + # the certificate itself (fingerprint before vs after), with certbot's + # own wording as a second signal. + new_info = read_cert_info(cert_path) + output = (result.stdout or '') + (result.stderr or '') + certbot_not_due = ('not yet due for renewal' in output.lower() + or 'no action taken' in output.lower()) + if before_info is None: + renewed = new_info is not None + elif new_info is None: + renewed = not certbot_not_due + else: + renewed = new_info.get('fingerprint') != before_info.get('fingerprint') + os.makedirs(SSL_CERTS_DIR, exist_ok=True) try: publish_pem_bundle(combined_path, [cert_path, key_path]) @@ -1235,8 +1501,9 @@ def request_ssl_bundle(): # only copy we have. cleanup_summary = _quarantine_superseded_certs( keep_path=combined_path, - keep_lineage=primary, + keep_lineage=os.path.basename(live_dir), bundle_names=set(names), + unreachable_names=unreachable_names, ) # Raises if the config does not validate or HAProxy does not reload; @@ -1249,26 +1516,46 @@ def request_ssl_bundle(): 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 " + "their certbot lineages were NOT retired, so they can be " "restored by hand: %s", - primary, cert_backup_dir(), + primary, cert_retired_dir(), [e['file'] for e in cleanup_summary['removed']]) raise - _delete_superseded_lineages(cleanup_summary) + # Only now that HAProxy has loaded the new bundle: stop certbot renewing + # (and renew-certificates.sh re-publishing) overlapping old lineages. + lineage_summary = _retire_superseded_lineages( + keep_lineage=os.path.basename(live_dir), + bundle_names=set(names), + unreachable_names=unreachable_names, + keep_fingerprint=new_info.get('fingerprint') if new_info else None, + ) + cleanup_summary['lineages'] = lineage_summary + not_after = new_info.get('not_after') if new_info else None log_operation( 'request_ssl_bundle', True, - f'SSL bundle issued for {primary} covering {len(names)} names; ' - f'cleaned up {len(cleanup_summary["removed"])} superseded lineage(s)' + f'SSL bundle {"issued" if renewed else "not yet due (existing certificate kept)"} ' + f'for {primary} covering {len(names)} names; retired ' + f'{len(cleanup_summary["removed"])} superseded pem(s) and ' + f'{len(lineage_summary["retired"])} lineage(s)' ) return jsonify({ 'status': 'success', 'primary': primary, 'names': names, 'cert_path': combined_path, + 'renewed': bool(renewed), + 'certbot_not_due': bool(certbot_not_due), + 'not_after': not_after, + 'not_after_iso': datetime.fromtimestamp(not_after, timezone.utc).isoformat() + if not_after else None, + 'fingerprint': new_info.get('fingerprint') if new_info else None, 'cleanup': cleanup_summary, - 'message': f'Bundled certificate obtained for {len(names)} names', + 'message': (f'Bundled certificate obtained for {len(names)} names' + if renewed else + f'Existing certificate for {len(names)} names is not yet ' + 'due for renewal; nothing was issued'), }) except Exception as e: log_operation('request_ssl_bundle', False, str(e)) diff --git a/scripts/cert-publish-lib.sh b/scripts/cert-publish-lib.sh index a589f05..fcf2036 100644 --- a/scripts/cert-publish-lib.sh +++ b/scripts/cert-publish-lib.sh @@ -203,6 +203,31 @@ cert_bundle_valid() { return 0 } +# cert_is_expired CERT_FILE +# +# Returns 0 only when openssl positively reports that the first certificate in +# CERT_FILE has expired. Anything else (not expired, unreadable, no openssl) +# returns 1; unreadable material is rejected with a better message by +# cert_bundle_valid(). +# +# Why: HAProxy loads the crt directory wholesale and serves the FIRST file that +# claims an SNI name. An expired bundle in there is never useful and can only +# shadow a valid one - on 2026-10-07 renew-certificates.sh kept re-publishing +# the expired brain-jar.com lineage every 12 hours, and HAProxy served it for +# brainjar.net instead of the valid brainjar.net bundle next to it. +cert_is_expired() { + local file="$1" out + [ -f "$file" ] || return 1 + command -v openssl >/dev/null 2>&1 || return 1 + out="$(openssl x509 -checkend 0 -noout -in "$file" 2>/dev/null /dev/null Date: Fri, 9 Oct 2026 14:00:03 -0700 Subject: [PATCH 2/3] fix(ssl): review round 1 - exact --cert-name lineage, retire whole lineage tree, sweep expired pems - H1: request_ssl_bundle uses live/ exactly (certbot was given --cert-name ) instead of find_certbot_live_dir(), which returns the newest of /-NNNN. After a "not yet due" no-op that published a stale -0001 and retired the real lineage. keep_lineage is now the primary; the bundle's own lineage is never a retirement candidate. - H2: retire_lineage_tree() moves live/, archive/ and renewal/.conf together to /etc/letsencrypt/retired//{live,archive,renewal}/ (same depth, relative symlinks still resolve, restore is three mv's). Leaving archive/ behind made certbot's new_lineage() refuse the name forever (CertStorageError) and leave an empty renewal conf. Test re-issues the name through certbot's own storage code after retirement. - M2: cert_retire_expired_pems moves already-published EXPIRED pems out of /etc/haproxy/certs to cert-retired/ (never the last pem, never the default self-signed cert) and the scripts reload when anything was retired. - GET /api/capabilities + `capabilities` in the bundle response, and VERSION is copied into the image, so WHP can refuse whole-site re-bundling against an older edge. Tests: test-cert-write-safety.py 47 -> 52, test-cert-scripts.py 44 -> 50; each fix reverted in a scratch copy turns its suite red. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- Dockerfile | 2 + haproxy_manager.py | 128 ++++++++++++++++++++------- scripts/cert-publish-lib.sh | 51 +++++++++++ scripts/renew-certificates.sh | 9 +- scripts/sync-certificates.sh | 9 +- scripts/test-cert-scripts.py | 49 +++++++++++ scripts/test-cert-write-safety.py | 140 ++++++++++++++++++++++++++++-- 8 files changed, 345 insertions(+), 45 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e2e69d4..5b4df66 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,7 +10,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co - **Stick-table contract**: `python3 scripts/test-stick-table-contract.py` - offline; holds the templates' `store` clauses, `STICK_TABLE_FIELD_CONTRACT`, and every consumer to each other. Run it after touching any `stick-table` line. - **Runtime-map contract**: `python3 scripts/test-runtime-map-contract.py` - offline; asserts the runtime map commands are `@1`-prefixed, reference the map by FILE PATH (never `#`), carry the value `1`, and that every captured rejection is classified as a failure. Run it after touching any `add map`/`del map`/`clear map` path. - **Certificate destruction safety**: `python3 scripts/test-cert-write-safety.py` - offline; asserts a live `.pem` is never truncated, removed, or its certbot lineage deleted while any configured domain still references it (one bundle serves many names, so `ssl_cert_path` is routinely shared). Run it after touching any `os.remove`/`certbot delete`/PEM-write path. -- **Superseded-cert retirement**: covered by the same file (`TestRetirementRules`, `TestSanOverlapRetirement`). A new bundle retires any OTHER pem/lineage sharing ANY name (CN or SAN) with it - moved to `cert-retired/`, `live-retired/`, `renewal-retired/`, never deleted - unless that cert is still the only valid one for a name this edge serves (`retirement_blockers()`). Expired lineages are never published (`cert_is_expired` in `cert-publish-lib.sh`, `leaf_is_expired()` in Python). Fixtures: `scripts/test-fixtures/san-certs/`. +- **Superseded-cert retirement**: covered by the same file (`TestRetirementRules`, `TestSanOverlapRetirement`). A new bundle retires any OTHER pem/lineage sharing ANY name (CN or SAN) with it - the pem moved to `cert-retired/`, the lineage's live+archive+renewal moved TOGETHER to `/etc/letsencrypt/retired//` (leaving archive/ behind makes certbot refuse the name forever), never deleted; the bundle's own `--cert-name` lineage is `live/` exactly and is never retired - unless that cert is still the only valid one for a name this edge serves (`retirement_blockers()`). `GET /api/capabilities` advertises this to WHP, which must not use whole-site re-bundling without it. Expired pems already in the crt dir are moved to `cert-retired/` by `cert_retire_expired_pems` (never the last pem, never `default_self_signed_cert.pem`). Expired lineages are never published (`cert_is_expired` in `cert-publish-lib.sh`, `leaf_is_expired()` in Python). Fixtures: `scripts/test-fixtures/san-certs/`. - **Manual Testing**: Run `curl` commands against `http://localhost:8000` endpoints as shown in README.md ### Reading stick tables (and why it is easy to get silently wrong) diff --git a/Dockerfile b/Dockerfile index f66654f..094371b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -44,6 +44,8 @@ WORKDIR /haproxy COPY ./templates /haproxy/templates COPY requirements.txt /haproxy/ COPY haproxy_manager.py /haproxy/ +# Read by GET /api/capabilities (WHP gates whole-site re-bundling on it). +COPY VERSION /haproxy/VERSION COPY scripts /haproxy/scripts COPY trusted_ips.list /etc/haproxy/trusted_ips.list COPY trusted_ips.map /etc/haproxy/trusted_ips.map diff --git a/haproxy_manager.py b/haproxy_manager.py index a04d23d..66dee7b 100644 --- a/haproxy_manager.py +++ b/haproxy_manager.py @@ -774,6 +774,30 @@ def health_check(): 'error': str(e) }), 500 +# What this edge supports. WHP's orchestrator only switches to whole-site +# re-bundling with unreachable_names when it sees these (HARD RULE: the WHP side +# must not rely on 2026.10.3 behaviour against an older image, whose CN-only +# quarantine + `certbot delete` would act on the larger bundles). +EDGE_CAPABILITIES = ['bundle_renewed_flag', 'san_overlap_retirement', + 'unreachable_names', 'retired_lineage_tree'] + + +def _image_version(): + for path in ('/haproxy/VERSION', os.path.join(os.path.dirname(os.path.abspath(__file__)), 'VERSION')): + try: + with open(path) as fh: + return fh.read().strip() + except OSError: + continue + return 'unknown' + + +@app.route('/api/capabilities', methods=['GET']) +@require_api_key +def get_capabilities(): + return jsonify({'version': _image_version(), 'capabilities': EDGE_CAPABILITIES}) + + @app.route('/api/regenerate', methods=['GET']) @require_api_key def regenerate_conf(): @@ -1017,9 +1041,11 @@ def request_ssl(): # would lose its last valid certificate (the shared-PEM hazard: one bundle # routinely serves several sites' names, see domains_referencing_cert_path); # * nothing is deleted. The pem goes to the cert-retired sibling directory and -# the lineage's live/ and renewal/.conf go to live-retired/ and -# renewal-retired/, so certbot stops renewing it and renew-certificates.sh -# stops re-publishing it. Every move is logged and can be reversed with mv. +# the lineage's live/, archive/ and renewal/.conf go +# together to /etc/letsencrypt/retired// (retire_lineage_tree), so +# certbot stops renewing it, renew-certificates.sh stops re-publishing it, +# and the name can still be issued again later. Every move is logged and +# can be reversed with mv. # (This used to `certbot delete` the lineage, which is irreversible.) # Where certbot keeps its lineages. A module global so the tests can repoint @@ -1249,14 +1275,58 @@ def _quarantine_superseded_certs(keep_path, keep_lineage, bundle_names, return summary +def retire_lineage_tree(lineage, stamp=None): + """Move a certbot lineage COMPLETELY out of certbot's view, without deleting. + + live/, archive/ and renewal/.conf go together to + LETSENCRYPT_DIR/retired//{live,archive,renewal}/. All three must + move: certbot's storage.RenewableCert.new_lineage() refuses to create a + lineage while archive/ exists ("archive directory exists for ") - and + it has already written an empty renewal/.conf by then, which breaks every + later --cert-name and every `certbot renew` parse (review H2, verified + against certbot 2.11). Moving only live/ and renewal/ (as the first version + of this did) therefore made the name permanently un-issuable. + + The retired tree keeps the same depth as /etc/letsencrypt, so the relative + live//*.pem -> ../../archive//*N.pem symlinks still resolve, and a + restore is three mv's back. Returns {'retired_to': dir, 'moved': [...]}. + Raises on failure after moving back anything already moved. + """ + stamp = stamp or datetime.now(timezone.utc).strftime('%Y%m%dT%H%M%SZ') + root = os.path.join(LETSENCRYPT_DIR, 'retired', stamp) + n = 1 + while any(os.path.exists(os.path.join(root, part, name)) for part, name in + (('live', lineage), ('archive', lineage), ('renewal', f'{lineage}.conf'))): + root = os.path.join(LETSENCRYPT_DIR, 'retired', f'{stamp}.{n}') + n += 1 + plan = [(os.path.join(LETSENCRYPT_DIR, part, name), os.path.join(root, part, name)) + for part, name in (('renewal', f'{lineage}.conf'), ('live', lineage), + ('archive', lineage))] + moved = [] + try: + for src, dst in plan: + if os.path.lexists(src): + os.makedirs(os.path.dirname(dst), exist_ok=True) + shutil.move(src, dst) + moved.append((src, dst)) + except Exception: + for src, dst in reversed(moved): + try: + shutil.move(dst, src) + except Exception as e: # pragma: no cover - logged, nothing else to do + logger.critical("Could not restore %s from %s: %s", src, dst, e) + raise + return {'retired_to': root, 'moved': [d for _, d in moved]} + + def _retire_superseded_lineages(keep_lineage, bundle_names, unreachable_names=(), keep_fingerprint=None): - """Move overlapping certbot lineages out of live/ and renewal/. + """Move overlapping certbot lineages out of certbot's tree (retire_lineage_tree). Same overlap and safety rules as _quarantine_superseded_certs(), evaluated against the pem files that are left in the crt dir. Runs only after HAProxy - has loaded the new bundle. Moving live/ to live-retired/ (same depth) - keeps its relative ../../archive symlinks valid, so `mv` back restores it. + has loaded the new bundle. keep_lineage is the bundle's --cert-name and is + never retired. """ summary = {'retired': [], 'kept': [], 'errors': []} live_root = os.path.join(LETSENCRYPT_DIR, 'live') @@ -1275,6 +1345,8 @@ def _retire_superseded_lineages(keep_lineage, bundle_names, unreachable_names=() for lineage in sorted(os.listdir(live_root)): lineage_dir = os.path.join(live_root, lineage) + # Hard guard: the bundle's own --cert-name lineage is never a candidate, + # whatever any other check says (review H1). if lineage == keep_lineage or not os.path.isdir(lineage_dir): continue cert_file = os.path.join(lineage_dir, 'cert.pem') @@ -1299,24 +1371,12 @@ def _retire_superseded_lineages(keep_lineage, bundle_names, unreachable_names=() continue entry = {'lineage': lineage, 'names': sorted(info['names'])} try: - live_dest_root = os.path.join(LETSENCRYPT_DIR, 'live-retired') - os.makedirs(live_dest_root, exist_ok=True) - live_dest = _unique_dest(live_dest_root, lineage) - shutil.move(lineage_dir, live_dest) - entry['live_moved_to'] = live_dest - conf = os.path.join(LETSENCRYPT_DIR, 'renewal', f'{lineage}.conf') - if os.path.exists(conf): - conf_dest_root = os.path.join(LETSENCRYPT_DIR, 'renewal-retired') - os.makedirs(conf_dest_root, exist_ok=True) - conf_dest = _unique_dest(conf_dest_root, f'{lineage}.conf') - shutil.move(conf, conf_dest) - entry['renewal_moved_to'] = conf_dest + entry.update(retire_lineage_tree(lineage)) logger.warning( "Retired certbot lineage %s (names: %s) because it overlaps the " - "new %s bundle: live -> %s, renewal conf -> %s. Archive left in " - "place; reverse with mv to restore.", lineage, - ', '.join(entry['names']), keep_lineage, entry['live_moved_to'], - entry.get('renewal_moved_to', '(none)')) + "new %s bundle: live, archive and renewal conf moved to %s. " + "Restore with three mv's back.", lineage, + ', '.join(entry['names']), keep_lineage, entry['retired_to']) summary['retired'].append(entry) except Exception as e: entry['error'] = str(e) @@ -1419,10 +1479,16 @@ def request_ssl_bundle(): # Defensive: clear any stale lock left by a SIGKILLed prior run. clear_stale_certbot_locks() - # fullchain.pem starts with the leaf, so it identifies the certificate. - before_dir = find_certbot_live_dir(primary) - before_info = read_cert_info(os.path.join(before_dir, 'fullchain.pem')) \ - if before_dir else None + # The lineage is EXACTLY live/: certbot was given + # --cert-name , which is authoritative. find_certbot_live_dir() + # must NOT be used here - it returns the newest of and + # -NNNN, so after a "not yet due" no-op a stale, more recently + # renewed -0001 would be published as .pem and the + # real lineage would then be retired as "superseded" (review + # H1, reproduced with brainjar.net). fullchain.pem starts with the leaf. + lineage_dir = os.path.join(LETSENCRYPT_DIR, 'live', primary) + before_info = read_cert_info(os.path.join(lineage_dir, 'fullchain.pem')) \ + if os.path.isdir(lineage_dir) else None result = subprocess.run(cmd, capture_output=True, text=True) if result.returncode != 0: @@ -1436,9 +1502,8 @@ def request_ssl_bundle(): 'attempted_names': names, }), 500 - # Locate the lineage. With --cert-name primary, this should be a - # stable directory name (no -NNNN suffix on the first issuance). - live_dir = find_certbot_live_dir(primary) + # --cert-name means the lineage is live/, exactly. + live_dir = lineage_dir if os.path.isdir(lineage_dir) else None if not live_dir: error_msg = f'Bundle issued but live dir not found for {primary}' log_operation('request_ssl_bundle', False, error_msg) @@ -1501,7 +1566,7 @@ def request_ssl_bundle(): # only copy we have. cleanup_summary = _quarantine_superseded_certs( keep_path=combined_path, - keep_lineage=os.path.basename(live_dir), + keep_lineage=primary, bundle_names=set(names), unreachable_names=unreachable_names, ) @@ -1525,7 +1590,7 @@ def request_ssl_bundle(): # Only now that HAProxy has loaded the new bundle: stop certbot renewing # (and renew-certificates.sh re-publishing) overlapping old lineages. lineage_summary = _retire_superseded_lineages( - keep_lineage=os.path.basename(live_dir), + keep_lineage=primary, bundle_names=set(names), unreachable_names=unreachable_names, keep_fingerprint=new_info.get('fingerprint') if new_info else None, @@ -1546,6 +1611,7 @@ def request_ssl_bundle(): 'names': names, 'cert_path': combined_path, 'renewed': bool(renewed), + 'capabilities': EDGE_CAPABILITIES, 'certbot_not_due': bool(certbot_not_due), 'not_after': not_after, 'not_after_iso': datetime.fromtimestamp(not_after, timezone.utc).isoformat() diff --git a/scripts/cert-publish-lib.sh b/scripts/cert-publish-lib.sh index fcf2036..f8e0a9e 100644 --- a/scripts/cert-publish-lib.sh +++ b/scripts/cert-publish-lib.sh @@ -228,6 +228,57 @@ cert_not_after() { openssl x509 -enddate -noout -in "$1" 2>/dev/null /{cert,chain,fullchain,privkey}1.pem + live//*.pem + symlinks (../../archive//...1.pem) + renewal/.conf, like certbot.""" + archive = os.path.join(le_dir, 'archive', lineage) + live = os.path.join(le_dir, 'live', lineage) + os.makedirs(archive, exist_ok=True) + os.makedirs(live, exist_ok=True) + for kind, body in (('cert', cert), ('chain', cert), ('fullchain', cert), ('privkey', key)): + with open(os.path.join(archive, f'{kind}1.pem'), 'w') as fh: + fh.write(body) + link = os.path.join(live, f'{kind}.pem') + if os.path.lexists(link): + os.remove(link) + os.symlink(f'../../archive/{lineage}/{kind}1.pem', link) + os.makedirs(os.path.join(le_dir, 'renewal'), exist_ok=True) + with open(os.path.join(le_dir, 'renewal', f'{lineage}.conf'), 'w') as fh: + fh.write(f'archive_dir = {archive}\ncert = {live}/cert.pem\n') + return live + + +def retired_path(le_dir, part, name): + hits = sorted(__import__('glob').glob(os.path.join(le_dir, 'retired', '*', part, name))) + return hits[-1] if hits else os.path.join(le_dir, 'retired', '', part, name) + + FIX_ONLY_RETIRE = unittest.skipIf( TESTING_FOREIGN_TREE and not hasattr(hm, 'retirement_blockers'), 'HAPROXY_MANAGER_DIR points at a tree without SAN-overlap retirement') @@ -738,13 +764,14 @@ class TestSanOverlapRetirement(CertPublishTestCase): for name in ('brainjar.net', 'www.brainjar.net'): self.add_domain(name, name.replace('.', '_')) - def old_lineage(self, lineage, fixture): - live = os.path.join(self.le_live, lineage) - self.write(os.path.join(live, 'fullchain.pem'), san(f'{fixture}.crt')) - self.write(os.path.join(live, 'cert.pem'), san(f'{fixture}.crt')) - self.write(os.path.join(live, 'privkey.pem'), san(f'{fixture}.key')) - conf = self.write(os.path.join(self.le, 'renewal', f'{lineage}.conf'), - f'# renewal conf for {lineage}\n') + def old_lineage(self, lineage, fixture, publish=True): + """A lineage laid out the way certbot does it: real files in + archive//*1.pem, relative symlinks in live//, a renewal conf.""" + live = make_certbot_lineage(self.le, lineage, san(f'{fixture}.crt'), + san(f'{fixture}.key')) + conf = os.path.join(self.le, 'renewal', f'{lineage}.conf') + if not publish: + return live, conf, None pem = self.write(os.path.join(self.certs, f'{lineage}.pem'), san(f'{fixture}.crt') + san(f'{fixture}.key')) return live, conf, pem @@ -775,8 +802,14 @@ class TestSanOverlapRetirement(CertPublishTestCase): # re-publish it and certbot can no longer try to renew it. self.assertFalse(os.path.exists(live)) self.assertFalse(os.path.exists(conf)) - self.assertTrue(os.path.isdir(os.path.join(self.le, 'live-retired', 'brain-jar.com'))) - self.assertTrue(os.path.exists(os.path.join(self.le, 'renewal-retired', 'brain-jar.com.conf'))) + # live, archive AND renewal leave together (review H2), into one tree. + self.assertFalse(os.path.exists(os.path.join(self.le, 'archive', 'brain-jar.com'))) + self.assertTrue(os.path.isdir(retired_path(self.le, 'live', 'brain-jar.com'))) + self.assertTrue(os.path.isdir(retired_path(self.le, 'archive', 'brain-jar.com'))) + self.assertTrue(os.path.exists(retired_path(self.le, 'renewal', 'brain-jar.com.conf'))) + # Relative symlinks still resolve in the retired tree (restore = mv back). + self.assertIn('BEGIN CERTIFICATE', self.read( + os.path.join(retired_path(self.le, 'live', 'brain-jar.com'), 'cert.pem'))) self.assertEqual([], self.certbot_deletes()) self.assertTrue(os.path.exists(os.path.join(self.certs, 'brainjar.net.pem'))) self.assertTrue(self.edge_would_start()) @@ -857,6 +890,95 @@ class TestSanOverlapRetirement(CertPublishTestCase): self.assertIn('--keep-until-expiring', self.certbot_invocations()[-1]) + + @FIX_ONLY_RETIRE + @NEEDS_OPENSSL + def test_not_due_noop_never_retires_its_own_cert_name_lineage(self): + """Review H1: find_certbot_live_dir() returns the NEWEST of brainjar.net + and brainjar.net-0001. After a certbot no-op that was the stale -0001: + it got published as brainjar.net.pem and the real brainjar.net lineage + was retired as "superseded".""" + del os.environ['TEST_CERTBOT_INSTALL_SRC'] + os.environ['TEST_CERTBOT_NOT_DUE'] = '1' + real = make_certbot_lineage(self.le, 'brainjar.net', san('new_bundle.crt'), san('new_bundle.key')) + stale = make_certbot_lineage(self.le, 'brainjar.net-0001', san('dead_old.crt'), san('dead_old.key')) + later = time.time() + 60 + os.utime(os.path.join(self.le, 'archive', 'brainjar.net-0001', 'fullchain1.pem'), (later, later)) + # Production behaviour of find_certbot_live_dir(): newest wins. + hm.find_certbot_live_dir = lambda base: stale if base == 'brainjar.net' else None + + data = self.bundle() + + self.assertTrue(os.path.isdir(real), 'the --cert-name lineage itself was retired') + self.assertTrue(os.path.exists(os.path.join(self.le, 'renewal', 'brainjar.net.conf'))) + self.assertNotIn('brainjar.net', [e['lineage'] for e in data['cleanup']['lineages']['retired']]) + published = self.read(os.path.join(self.certs, 'brainjar.net.pem')) + self.assertIn(san('new_bundle.crt').strip().splitlines()[1], published, + 'published the stale -0001 certificate instead of live/brainjar.net') + self.assertFalse(data['renewed']) + + @FIX_ONLY_RETIRE + def test_bundle_reports_capabilities(self): + data = self.bundle() + self.assertIn('san_overlap_retirement', data.get('capabilities', [])) + caps = self.client.get('/api/capabilities').get_json() + self.assertIn('unreachable_names', caps['capabilities']) + + +class TestRetiredLineageCanBeReissued(CertPublishTestCase): + """Review H2: retirement must leave certbot able to issue the name again.""" + + def setUp(self): + super().setUp() + self.le = hm.LETSENCRYPT_DIR + + @FIX_ONLY_RETIRE + def test_retirement_removes_every_part_certbot_checks(self): + make_certbot_lineage(self.le, 'brain-jar.com', san('expired_old.crt'), san('expired_old.key')) + out = hm.retire_lineage_tree('brain-jar.com') + # certbot new_lineage() refuses if any of these exist for the name. + for part in (('live', 'brain-jar.com'), ('archive', 'brain-jar.com'), + ('renewal', 'brain-jar.com.conf')): + self.assertFalse(os.path.lexists(os.path.join(self.le, *part)), part) + self.assertTrue(os.path.lexists(os.path.join(out['retired_to'], *part)), part) + self.assertIn('BEGIN CERTIFICATE', + self.read(os.path.join(out['retired_to'], 'live', 'brain-jar.com', 'fullchain.pem'))) + + @FIX_ONLY_RETIRE + def test_second_retirement_of_the_same_name_does_not_collide(self): + make_certbot_lineage(self.le, 'x.example', san('new_bundle.crt'), san('new_bundle.key')) + a = hm.retire_lineage_tree('x.example', stamp='S') + make_certbot_lineage(self.le, 'x.example', san('new_bundle.crt'), san('new_bundle.key')) + b = hm.retire_lineage_tree('x.example', stamp='S') + self.assertNotEqual(a['retired_to'], b['retired_to']) + self.assertTrue(os.path.isdir(os.path.join(a['retired_to'], 'archive', 'x.example'))) + + @FIX_ONLY_RETIRE + def test_certbot_can_issue_the_name_again_after_retirement(self): + """Against certbot's own storage code (skipped where certbot is not + importable, e.g. the image's app python).""" + try: + from certbot._internal import storage + except ImportError: + self.skipTest('certbot not importable from this python') + import types + cfg = types.SimpleNamespace( + config_dir=self.le, renewal_configs_dir=os.path.join(self.le, 'renewal'), + default_archive_dir=os.path.join(self.le, 'archive'), + live_dir=os.path.join(self.le, 'live')) + real_rv = storage.relevant_values + storage.relevant_values = lambda c: {'server': 'https://acme.invalid/directory'} + self.addCleanup(setattr, storage, 'relevant_values', real_rv) + cert, key = san('new_bundle.crt').encode(), san('new_bundle.key').encode() + + first = storage.RenewableCert.new_lineage('brain-jar.com', cert, key, cert, cfg) + self.assertEqual('brain-jar.com', first.lineagename) + hm.retire_lineage_tree('brain-jar.com') + again = storage.RenewableCert.new_lineage('brain-jar.com', cert, key, cert, cfg) + self.assertEqual('brain-jar.com', again.lineagename, + 'certbot could not re-create the lineage under the same name') + self.assertTrue(os.path.getsize(os.path.join(self.le, 'renewal', 'brain-jar.com.conf')) > 0) + class TestExpiredCertificateIsNeverPublished(CertPublishTestCase): @FIX_ONLY_RETIRE From 17c9a8faf83534fda64d1c77d93ced00d0d1a838 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Fri, 9 Oct 2026 14:26:19 -0700 Subject: [PATCH 3/3] test(ssl): certbot re-issue test skips on ANY certbot import/init failure The skip guard caught only ImportError; a half-installed or version-skewed certbot can fail on import with other exceptions (verified with a stub that raises RuntimeError: previously an ERROR, now a skip). new_lineage() itself stays outside the guard - it is the assertion. Co-Authored-By: Claude Opus 5.5 --- scripts/test-cert-write-safety.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/scripts/test-cert-write-safety.py b/scripts/test-cert-write-safety.py index d58c3eb..c89e742 100755 --- a/scripts/test-cert-write-safety.py +++ b/scripts/test-cert-write-safety.py @@ -957,24 +957,31 @@ class TestRetiredLineageCanBeReissued(CertPublishTestCase): def test_certbot_can_issue_the_name_again_after_retirement(self): """Against certbot's own storage code (skipped where certbot is not importable, e.g. the image's app python).""" + # Import AND initialisation are both guarded: a half-installed or + # version-skewed certbot can fail with far more than ImportError (a + # missing transitive dependency, a pkg_resources/entry-point error, a + # moved internal). Any of those means "cannot run this check here", + # not "retirement is broken". new_lineage() itself is NOT inside the + # guard - that is the assertion. try: + import types from certbot._internal import storage - except ImportError: - self.skipTest('certbot not importable from this python') - import types - cfg = types.SimpleNamespace( - config_dir=self.le, renewal_configs_dir=os.path.join(self.le, 'renewal'), - default_archive_dir=os.path.join(self.le, 'archive'), - live_dir=os.path.join(self.le, 'live')) - real_rv = storage.relevant_values + real_rv = storage.relevant_values + new_lineage = storage.RenewableCert.new_lineage + cfg = types.SimpleNamespace( + config_dir=self.le, renewal_configs_dir=os.path.join(self.le, 'renewal'), + default_archive_dir=os.path.join(self.le, 'archive'), + live_dir=os.path.join(self.le, 'live')) + except Exception as e: # noqa: BLE001 - deliberately broad, see above + self.skipTest(f'certbot unusable from this python ({type(e).__name__}: {e})') storage.relevant_values = lambda c: {'server': 'https://acme.invalid/directory'} self.addCleanup(setattr, storage, 'relevant_values', real_rv) cert, key = san('new_bundle.crt').encode(), san('new_bundle.key').encode() - first = storage.RenewableCert.new_lineage('brain-jar.com', cert, key, cert, cfg) + first = new_lineage('brain-jar.com', cert, key, cert, cfg) self.assertEqual('brain-jar.com', first.lineagename) hm.retire_lineage_tree('brain-jar.com') - again = storage.RenewableCert.new_lineage('brain-jar.com', cert, key, cert, cfg) + again = new_lineage('brain-jar.com', cert, key, cert, cfg) self.assertEqual('brain-jar.com', again.lineagename, 'certbot could not re-create the lineage under the same name') self.assertTrue(os.path.getsize(os.path.join(self.le, 'renewal', 'brain-jar.com.conf')) > 0)