fix(ssl): review round 1 - exact --cert-name lineage, retire whole lineage tree, sweep expired pems

- H1: request_ssl_bundle uses live/<primary> exactly (certbot was given
  --cert-name <primary>) instead of find_certbot_live_dir(), which returns the
  newest of <primary>/<primary>-NNNN. After a "not yet due" no-op that
  published a stale -0001 and retired the real <primary> lineage. keep_lineage
  is now the primary; the bundle's own lineage is never a retirement candidate.
- H2: retire_lineage_tree() moves live/<x>, archive/<x> and renewal/<x>.conf
  together to /etc/letsencrypt/retired/<stamp>/{live,archive,renewal}/ (same
  depth, relative symlinks still resolve, restore is three mv's). Leaving
  archive/<x> 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 <noreply@anthropic.com>
This commit is contained in:
shadowdaoandClaude Opus 5.5 committed 2026-10-09 14:00:03 -07:00
1 parent e58996d6c9
commit 8c7fd93387
8 files changed
+345 -45

No files matched your search

+51
View File
@@ -228,6 +228,57 @@ cert_not_after() {
openssl x509 -enddate -noout -in "$1" 2>/dev/null </dev/null | sed 's/^notAfter=//'
}
cert_retired_dir() {
if [ -n "${CERT_RETIRED_DIR:-}" ]; then
echo "$CERT_RETIRED_DIR"
else
echo "$(dirname "${SSL_CERTS_DIR:-/etc/haproxy/certs}")/cert-retired"
fi
}
# cert_retire_expired_pems
#
# Move every EXPIRED *.pem out of $SSL_CERTS_DIR into cert-retired/ (a sibling,
# never inside the crt dir). Sets CERT_RETIRED_EXPIRED to the number moved.
#
# Not publishing expired lineages stops the re-publish loop, but a pem that was
# published while valid and has expired since stays in the crt dir and keeps
# shadowing - that was exactly brain-jar.com.pem (review M2). An expired
# certificate is never a valid certificate for anyone, so it can always go: the
# only-valid-cert safety rule cannot be violated by moving it. Two exceptions:
# * default_self_signed_cert.pem - HAProxy's fallback, regenerated separately;
# * the LAST .pem in the directory - a crt directory with no certificate at
# all fails the whole :443 bind, which is worse than serving an expired one.
# Nothing is deleted; reverse with mv.
cert_retire_expired_pems() {
local dir="${SSL_CERTS_DIR:-/etc/haproxy/certs}" retired f base dest remaining
CERT_RETIRED_EXPIRED=0
retired="$(cert_retired_dir)"
for f in "$dir"/*.pem; do
[ -f "$f" ] || continue
base="$(basename "$f")"
[ "$base" = "default_self_signed_cert.pem" ] && continue
cert_is_expired "$f" || continue
remaining="$(find "$dir" -maxdepth 1 -type f -name '*.pem' | wc -l)"
if [ "$remaining" -le 1 ]; then
log_error "Expired certificate $f is the only certificate left in $dir - NOT retiring it (an empty crt directory fails the :443 bind)"
continue
fi
if ! mkdir -p "$retired"; then
log_error "cannot create $retired - expired $f left in place"
continue
fi
dest="$retired/$base"
[ -e "$dest" ] && dest="$dest.$(date -u +%Y%m%dT%H%M%SZ)"
if mv -- "$f" "$dest"; then
log_error "Retired EXPIRED certificate $base (notAfter $(cert_not_after "$dest")) from $dir to $dest - it could only shadow valid certificates"
CERT_RETIRED_EXPIRED=$((CERT_RETIRED_EXPIRED + 1))
else
log_error "failed to move expired $f to $dest"
fi
done
}
# cert_publish CERT_FILE KEY_FILE DEST_FILE
#
# Assemble CERT_FILE + KEY_FILE into DEST_FILE without ever exposing a
+7 -2
View File
@@ -98,10 +98,15 @@ while read -r domain; do
fi
done <<< "$DOMAINS"
log_info "Certificate update completed: $UPDATED updated, $FAILED failed, $SKIPPED_EXPIRED expired lineage(s) skipped"
# Already-published pems that have expired since are moved out too (they
# can only shadow; see cert_retire_expired_pems).
cert_retire_expired_pems
RETIRED_EXPIRED=$CERT_RETIRED_EXPIRED
log_info "Certificate update completed: $UPDATED updated, $FAILED failed, $SKIPPED_EXPIRED expired lineage(s) skipped, $RETIRED_EXPIRED expired pem(s) retired"
# Reload HAProxy if any certificates were updated
if [ $UPDATED -gt 0 ]; then
if [ $UPDATED -gt 0 ] || [ "$RETIRED_EXPIRED" -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
+7 -2
View File
@@ -80,10 +80,15 @@ while read -r domain; do
fi
done <<< "$DOMAINS"
log_info "Certificate sync completed: $UPDATED updated, $FAILED failed, $SKIPPED_EXPIRED expired lineage(s) skipped"
# Already-published pems that have expired since are moved out too (they
# can only shadow; see cert_retire_expired_pems).
cert_retire_expired_pems
RETIRED_EXPIRED=$CERT_RETIRED_EXPIRED
log_info "Certificate sync completed: $UPDATED updated, $FAILED failed, $SKIPPED_EXPIRED expired lineage(s) skipped, $RETIRED_EXPIRED expired pem(s) retired"
# Reload HAProxy if any certificates were updated
if [ $UPDATED -gt 0 ]; then
if [ $UPDATED -gt 0 ] || [ "$RETIRED_EXPIRED" -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
+49
View File
@@ -662,6 +662,36 @@ class CertScriptBehaviour:
self.assertEqual(read(existing), OTHER_BUNDLE,
'an expired lineage overwrote the file in the crt dir')
def test_already_published_expired_pem_is_retired(self):
"""Review M2: skipping publication is not enough - an expired pem that
is ALREADY in the crt dir (brain-jar.com.pem) keeps shadowing."""
expired = write(os.path.join(self.certs_dir, 'brain-jar.com.pem'),
san_fixture('expired_old.crt') + san_fixture('expired_old.key'))
result = self.run_it()
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertFalse(os.path.exists(expired), 'expired pem still in the crt dir')
retired = os.path.join(self.haproxy_dir, 'cert-retired', 'brain-jar.com.pem')
self.assertTrue(os.path.exists(retired), 'expired pem was not moved to cert-retired/ (never delete)')
self.assertIn('Retired EXPIRED certificate brain-jar.com.pem', self.logs())
self.assertIn('1 expired pem(s) retired', self.logs())
self.assertTrue(self.reload_attempted())
self.assertEqual(read(self.live_pem), NEW_BUNDLE, 'valid neighbours untouched')
self.assert_certs_dir_is_clean()
def test_retiring_an_expired_pem_alone_triggers_a_reload(self):
self.seed_previous_bundle(NEW_BUNDLE)
shutil.rmtree(self.domain_dir)
other = os.path.join(self.le_live, 'other.example')
os.makedirs(other)
write(os.path.join(other, 'fullchain.pem'), san_fixture('expired_old.crt'))
write(os.path.join(other, 'privkey.pem'), san_fixture('expired_old.key'))
write(os.path.join(self.certs_dir, 'other.example.pem'),
san_fixture('expired_old.crt') + san_fixture('expired_old.key'))
result = self.run_it()
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertIn('0 updated', self.logs())
self.assertTrue(self.reload_attempted(), 'HAProxy must reload to drop the retired pem')
class TestRenewCertificates(CertScriptBehaviour, CertScriptFixture):
SCRIPT = 'renew-certificates.sh'
@@ -693,6 +723,25 @@ class TestCertPublishLibrary(CertScriptFixture):
self.assertEqual(1, self.call('cert_is_expired "$1"',
os.path.join(self.tmp, 'missing')).returncode)
def test_expired_sweep_keeps_the_last_pem_and_the_default_cert(self):
exp = san_fixture('expired_old.crt') + san_fixture('expired_old.key')
only = write(os.path.join(self.certs_dir, 'only.pem'), exp)
r = self.call('cert_retire_expired_pems; echo "n=$CERT_RETIRED_EXPIRED"')
self.assertIn('n=0', r.stdout)
self.assertTrue(os.path.exists(only), 'emptied the crt dir: that fails the whole :443 bind')
dflt = write(os.path.join(self.certs_dir, 'default_self_signed_cert.pem'), exp)
r = self.call('cert_retire_expired_pems; echo "n=$CERT_RETIRED_EXPIRED"')
self.assertIn('n=1', r.stdout)
self.assertTrue(os.path.exists(dflt), 'the default self-signed cert is not ours to retire')
self.assertFalse(os.path.exists(only))
def test_expired_sweep_never_touches_a_valid_pem(self):
valid = write(os.path.join(self.certs_dir, 'a.pem'), NEW_BUNDLE)
write(os.path.join(self.certs_dir, 'b.pem'), OTHER_BUNDLE)
r = self.call('cert_retire_expired_pems; echo "n=$CERT_RETIRED_EXPIRED"')
self.assertIn('n=0', r.stdout)
self.assertEqual(read(valid), NEW_BUNDLE)
def test_cert_publish_refuses_an_expired_certificate(self):
cert = write(os.path.join(self.tmp, 'expired.crt'), san_fixture('expired_old.crt'))
key = write(os.path.join(self.tmp, 'expired.key'), san_fixture('expired_old.key'))
+131 -9
View File
@@ -57,6 +57,7 @@ import subprocess
import sys
import tempfile
import textwrap
import time
import unittest
BROKEN_TOKEN = '__BROKEN__'
@@ -644,6 +645,31 @@ def san(name):
return fh.read()
def make_certbot_lineage(le_dir, lineage, cert, key):
"""archive/<x>/{cert,chain,fullchain,privkey}1.pem + live/<x>/*.pem
symlinks (../../archive/<x>/...1.pem) + renewal/<x>.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', '<none>', 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/<x>/*1.pem, relative symlinks in live/<x>/, 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