From e0b2181ae931f31a794c346d5e71d7ab3418eec1 Mon Sep 17 00:00:00 2001 From: archipelago Date: Mon, 5 Oct 2026 15:26:28 -0400 Subject: [PATCH] fix: isolate NPM upstream TLS sessions across public domains --- docs/npm-certificate-handoff-20261001.md | 36 +++++++++++++++++++++ docs/release-1.9.0-acceptance.md | 36 +++++++++++++++++++++ scripts/npm-public-bridge.py | 3 ++ scripts/tests/test_npm_public_bridge.py | 17 ++++++++++ tests/lifecycle/npm-public-bridge.py | 40 ++++++++++++++++++------ 5 files changed, 122 insertions(+), 10 deletions(-) diff --git a/docs/npm-certificate-handoff-20261001.md b/docs/npm-certificate-handoff-20261001.md index 461694d1..b721a7d7 100644 --- a/docs/npm-certificate-handoff-20261001.md +++ b/docs/npm-certificate-handoff-20261001.md @@ -256,3 +256,39 @@ IDs/start times are unchanged. Restored shop/www/indexer/relay HTTPS returns200. Shorty's old backend remains active under containment. Rebuild and requalify the migration, external security and restart persistence before closing this gate. The signed catalog contents are unchanged and need no further operator signature. + +### NPM multi-domain TLS regression found by public acceptance + +After the private-listener migration, local first requests passed, but public +Angor health intermittently returned502. Host nginx recorded upstream certificate +hostname mismatches when different public domains used the same NPM TLS listener. +The generated bridge inherited upstream TLS session reuse. This matches nginx's +[documented cross-SNI session-cache behaviour](https://trac.nginx.org/nginx/ticket/1340). + +The bridge now explicitly sets `proxy_ssl_session_reuse off` while retaining +SNI, hostname/chain verification and the existing trusted certificates. A real +NPM fixture with two distinct certificates reproduces failure with the old +configuration on the second hostname; the fixed fixture passes40 alternating +trusted TLS requests plus ACLs, WSS, certificate replacement, restart, failed-bind +rollback and disable/delete propagation.24 Python NPM regressions pass. +`/tmp/archy-190-npm-multicert-before.log` is the expected failing reproduction; +`/tmp/archy-190-npm-multicert-integration.log` is the fixed flat-layout pass. +Nested-layout issuance/renewal qualification is running separately. + +The exact helper correction is temporarily installed on Shorty and transactional +sync succeeds.40 mixed local TLS requests across four hostnames pass. Public +read-only Angor browser acceptance now passes TLS, WSS, funding/event commitment, +Explore discovery of the known fixture and full project details/statistics: +`/tmp/archy-190-shorty-migrated-angor-browser-2.log`. This remains one known fixture, +not all35-project recovery.32 external IPv4 management-denial checks and tailnet +access pass;10 HTTP/HTTPS ACME routes return the exact probe written in NPM's data +mount. Six NPM database tables and42 certificate/renewal files exactly match the +pre-migration backup (`/tmp/archy-190-shorty-state-preservation.log`). + +The previously running optimized build was stopped because it predates this +embedded-helper correction. A complete new build/deployment remains mandatory. +Shorty currently has the prior A5 candidate backend plus protected runtime nginx +and this qualified helper; an A5 restart can reinstall its older helper. Do not +claim final persistence until the new binary is deployed and restart is retested. +No certificate verification was disabled for a public application or upstream. +Raw-IP/unknown-SNI negative routing probes alone bypass hostname matching. diff --git a/docs/release-1.9.0-acceptance.md b/docs/release-1.9.0-acceptance.md index 385cb3b2..3cd6ba1d 100644 --- a/docs/release-1.9.0-acceptance.md +++ b/docs/release-1.9.0-acceptance.md @@ -660,3 +660,39 @@ IDs/start times are unchanged. Restored shop/www/indexer/relay HTTPS returns200. Shorty's old backend remains active under containment. Rebuild and requalify the migration, external security and restart persistence before closing this gate. The signed catalog contents are unchanged and need no further operator signature. + +### NPM multi-domain TLS regression found by public acceptance + +After the private-listener migration, local first requests passed, but public +Angor health intermittently returned502. Host nginx recorded upstream certificate +hostname mismatches when different public domains used the same NPM TLS listener. +The generated bridge inherited upstream TLS session reuse. This matches nginx's +[documented cross-SNI session-cache behaviour](https://trac.nginx.org/nginx/ticket/1340). + +The bridge now explicitly sets `proxy_ssl_session_reuse off` while retaining +SNI, hostname/chain verification and the existing trusted certificates. A real +NPM fixture with two distinct certificates reproduces failure with the old +configuration on the second hostname; the fixed fixture passes40 alternating +trusted TLS requests plus ACLs, WSS, certificate replacement, restart, failed-bind +rollback and disable/delete propagation.24 Python NPM regressions pass. +`/tmp/archy-190-npm-multicert-before.log` is the expected failing reproduction; +`/tmp/archy-190-npm-multicert-integration.log` is the fixed flat-layout pass. +Nested-layout issuance/renewal qualification is running separately. + +The exact helper correction is temporarily installed on Shorty and transactional +sync succeeds.40 mixed local TLS requests across four hostnames pass. Public +read-only Angor browser acceptance now passes TLS, WSS, funding/event commitment, +Explore discovery of the known fixture and full project details/statistics: +`/tmp/archy-190-shorty-migrated-angor-browser-2.log`. This remains one known fixture, +not all35-project recovery.32 external IPv4 management-denial checks and tailnet +access pass;10 HTTP/HTTPS ACME routes return the exact probe written in NPM's data +mount. Six NPM database tables and42 certificate/renewal files exactly match the +pre-migration backup (`/tmp/archy-190-shorty-state-preservation.log`). + +The previously running optimized build was stopped because it predates this +embedded-helper correction. A complete new build/deployment remains mandatory. +Shorty currently has the prior A5 candidate backend plus protected runtime nginx +and this qualified helper; an A5 restart can reinstall its older helper. Do not +claim final persistence until the new binary is deployed and restart is retested. +No certificate verification was disabled for a public application or upstream. +Raw-IP/unknown-SNI negative routing probes alone bypass hostname matching. diff --git a/scripts/npm-public-bridge.py b/scripts/npm-public-bridge.py index 1fea4e59..706c931f 100644 --- a/scripts/npm-public-bridge.py +++ b/scripts/npm-public-bridge.py @@ -279,6 +279,9 @@ def render(rows, paths, http_address, https_address, acme_root, trust_file): tls = '' if scheme == 'http' else f''' proxy_ssl_server_name on; proxy_ssl_name $host; + # One NPM listener serves different certificates. A shared upstream + # session cache can resume another hostname's session and fail SNI. + proxy_ssl_session_reuse off; proxy_ssl_verify on; proxy_ssl_verify_depth 5; proxy_ssl_trusted_certificate {quote(trust_file)};''' diff --git a/scripts/tests/test_npm_public_bridge.py b/scripts/tests/test_npm_public_bridge.py index e1b076cf..f834b5ae 100644 --- a/scripts/tests/test_npm_public_bridge.py +++ b/scripts/tests/test_npm_public_bridge.py @@ -291,6 +291,23 @@ class RoutingTests(unittest.TestCase): with self.assertRaisesRegex(ValueError, 'Duplicate'): bridge.render(rows + rows, {}, '127.0.0.1:8088', '127.0.0.1:8444', '/acme', '/trust.pem') + def test_distinct_sni_certificates_do_not_share_upstream_tls_sessions(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + rows = [] + for number, name in [(1, 'first.example'), (2, 'second.example')]: + directory = root / 'custom_ssl' / f'npm-{number}' + directory.mkdir(parents=True) + (directory / 'fullchain.pem').write_text('fixture certificate ' + name) + (directory / 'privkey.pem').write_text('fixture key ' + name) + rows.append({'id': number, 'domain_names': json.dumps([name]), + 'certificate_id': number, 'certificate_deleted': 0, 'provider': 'other'}) + config, _, _ = bridge.render(rows, {'data': str(root)}, '127.0.0.1:8088', + '127.0.0.1:8444', '/acme', '/trust.pem') + self.assertEqual(config.count(b'proxy_ssl_session_reuse off;'), 2) + self.assertEqual(config.count(b'proxy_ssl_name $host;'), 2) + self.assertEqual(config.count(b'proxy_ssl_verify on;'), 2) + class TransactionTests(unittest.TestCase): def test_idempotent_sync_and_certificate_renewal_reload(self): diff --git a/tests/lifecycle/npm-public-bridge.py b/tests/lifecycle/npm-public-bridge.py index 26da6c74..237f0819 100644 --- a/tests/lifecycle/npm-public-bridge.py +++ b/tests/lifecycle/npm-public-bridge.py @@ -25,6 +25,14 @@ NPM_IMAGE = yaml.safe_load((ROOT / 'apps/nginx-proxy-manager/manifest.yml').read spec = importlib.util.spec_from_file_location('bridge', ROOT / 'scripts/npm-public-bridge.py') bridge = importlib.util.module_from_spec(spec) spec.loader.exec_module(bridge) +# Opt-in reproduction of the previous generated configuration. Normal acceptance +# never enables this; a legacy run must fail the alternating-certificate check. +if os.environ.get('ARCHY_NPM_TEST_LEGACY_TLS_REUSE') == '1': + fixed_render = bridge.render + def legacy_render(*args, **kwargs): + config, trust, fingerprints = fixed_render(*args, **kwargs) + return config.replace(b'proxy_ssl_session_reuse off;', b'proxy_ssl_session_reuse on;'), trust, fingerprints + bridge.render = legacy_render def run(*args): @@ -210,17 +218,17 @@ server {{ listen 127.0.0.1:{http_port} default_server; acme.verify(api, sync, public, payload, tls_port, root/'access.log') certificate = api('/nginx/certificates', {'provider': 'other', 'nice_name': 'Disposable TLS fixture'}, 'POST') cert_path, key_path = root / 'leaf.pem', root / 'key.pem' - def upload_certificate(): + def upload_certificate(record=certificate, leaf=cert_path, key=key_path, names=('fixture.example', 'second.example')): run('openssl', 'req', '-x509', '-newkey', 'rsa:2048', '-nodes', '-days', '2', - '-subj', '/CN=fixture.example', '-addext', 'subjectAltName=DNS:fixture.example,DNS:second.example', - '-keyout', str(key_path), '-out', str(cert_path)) + '-subj', '/CN=' + names[0], '-addext', 'subjectAltName=' + ','.join('DNS:' + domain for domain in names), + '-keyout', str(key), '-out', str(leaf)) boundary = 'archy-' + uuid.uuid4().hex parts = [] - for field, path in [('certificate', cert_path), ('certificate_key', key_path)]: + for field, path in [('certificate', leaf), ('certificate_key', key)]: parts.append((f'--{boundary}\r\nContent-Disposition: form-data; name="{field}"; filename="{path.name}"\r\n' 'Content-Type: application/octet-stream\r\n\r\n').encode() + path.read_bytes() + b'\r\n') body = b''.join(parts) + f'--{boundary}--\r\n'.encode() - status, _, _ = request(admin + '/api/nginx/certificates/' + str(certificate['id']) + '/upload', + status, _, _ = request(admin + '/api/nginx/certificates/' + str(record['id']) + '/upload', body, 'POST', {'Authorization': 'Bearer ' + token, 'Content-Type': 'multipart/form-data; boundary=' + boundary}) assert status == 200, f'Fixture certificate upload failed ({status})' @@ -228,18 +236,30 @@ server {{ listen 127.0.0.1:{http_port} default_server; secure_payload = {**payload, 'certificate_id': certificate['id'], 'ssl_forced': True} api('/nginx/proxy-hosts/' + str(host['id']), secure_payload, 'PUT') assert sync(); time.sleep(.3) - def secure(headers=None): - context = ssl.create_default_context(cafile=str(cert_path)) + def secure(headers=None, hostname='fixture.example', cafile=cert_path): + context = ssl.create_default_context(cafile=str(cafile)) stream = context.wrap_socket(socket.create_connection(('127.0.0.1', tls_port), timeout=15), - server_hostname='fixture.example') - connection = http.client.HTTPConnection('fixture.example', tls_port, timeout=15) + server_hostname=hostname) + connection = http.client.HTTPConnection(hostname, tls_port, timeout=15) connection.sock = stream try: - connection.request('GET', '/', headers={'Host': 'fixture.example', **(headers or {})}) + connection.request('GET', '/', headers={'Host': hostname, **(headers or {})}) response = connection.getresponse() return response.status, response.read() finally: connection.close() + # Distinct certificate on the SAME upstream listener. Sharing a TLS + # session across SNI names causes intermittent certificate mismatch502s. + other_certificate = api('/nginx/certificates', {'provider': 'other', 'nice_name': 'Different SNI certificate'}, 'POST') + other_leaf, other_key = root / 'other-leaf.pem', root / 'other-key.pem' + upload_certificate(other_certificate, other_leaf, other_key, ('other.example',)) + other_host = api('/nginx/proxy-hosts', {**payload, 'domain_names': ['other.example'], + 'certificate_id': other_certificate['id'], 'ssl_forced': True}, 'POST') + assert sync(); time.sleep(.3) + for _ in range(20): + assert secure()[0] == 200, 'First hostname inherited another TLS session' + assert secure(hostname='other.example', cafile=other_leaf)[0] == 200, 'Second hostname inherited another TLS session' + print('PASS alternating40 trusted TLS requests across distinct SNI certificates on one NPM listener', flush=True) assert public()[0] == 301, 'NPM forced HTTPS was not preserved' assert secure()[0] == 200, 'Verified TLS bridge failed or redirected in a loop' run('podman', 'exec', name, 'sh', '-c',