From a63c991e8379959379d78d83f3a543213346fedb Mon Sep 17 00:00:00 2001 From: "ma.backes" Date: Sat, 25 Jul 2026 23:04:50 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(portal,tls):=20DNS-Pr=C3=BCfung=20des?= =?UTF-8?q?=20zusammengesetzten=20Portal-Hosts=20+=20Verwaltungs-Host=20in?= =?UTF-8?q?=20TLS-Automation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zwei ACME-Defekte aus einem externen Bug-Report (1.118.5): 1. validatePortalHost prüfte nur die DNS-Verifizierung der Basisdomain, nie den daraus gebauten Host .. Ohne dessen A/AAAA-Eintrag ging er trotzdem in buildTlsAutomation und Caddy versuchte 30 Tage lang (max_duration) beim produktiven Let's-Encrypt-CA ein Zertifikat zu holen — das Limit von 5 fehlgeschlagenen Prüfungen/Stunde/Hostname war sofort erschöpft, ohne Rückmeldung im UI. Der Host wird jetzt selbst aufgelöst (domains.verify) und mit der Server-IP verglichen; Status 'failed' → 400 mit eigenem Fehlertext host_unresolved. 'pending' (Resolver nicht erreichbar) blockiert bewusst nicht, und der Apex-Fall (leeres Präfix) spart den zweiten Roundtrip. 2. Der Verwaltungs-Host aus GC_BASE_URL erreichte buildTlsAutomation nie, weil seine Route erst danach in caddyRoutes eingehängt wird. Er fiel damit auf Caddys Standard-Automation zurück — ein ACME-Konto ohne Kontaktadresse trotz gesetztem GC_CADDY_EMAIL, also keine Ablaufbenachrichtigungen. Er wird jetzt wie homeHost ausdrücklich übergeben; die Domain-Liste ist dedupliziert, damit ein Host, der zugleich Route ist, nicht doppelt als Subject auftaucht. Regressionstests für beide Defekte (ohne Fix rot), bestehende Portal-Host-Tests auf den nun asynchronen Validator umgestellt und ihren Resolver gestubbt (liefen vorher zufällig über den 'pending'-Pfad durch). --- CHANGELOG.md | 2 + src/i18n/de.json | 1 + src/i18n/en.json | 1 + src/routes/api/settings/portal.js | 4 +- src/services/caddyConfig.js | 14 +++--- src/services/portalConfig.js | 11 ++++- tests/caddy_tls_management_host.test.js | 45 +++++++++++++++++++ tests/portal_host_helper.test.js | 57 ++++++++++++++++++++----- tests/portal_settings_host.test.js | 27 +++++++++++- 9 files changed, 141 insertions(+), 21 deletions(-) create mode 100644 tests/caddy_tls_management_host.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d386006..f02c524c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ ### Fixed - Einstellungen → Allgemein: Das Standard-Design ließ sich in den Themes Classic und Pro nicht auf Aurora stellen — der Aurora-Knopf fehlte dort, obwohl der Server den Wert längst akzeptiert. Beide Templates bieten jetzt alle drei Designs an, ein Paritätstest hält die Auswahl über alle Themes und beide Seiten (Profil + Einstellungen) synchron. - Caddy versuchte dauerhaft, für den internen Ownership-Marker `gc-owner.invalid` ein öffentliches Zertifikat zu beziehen, was den Log mit fehlschlagenden ACME-Versuchen füllte. Die Marker-Route wird erst nach dem Aufbau der TLS-Automation angehängt und erreichte deren TLD-Klassifizierung nie; sie ist jetzt ausdrücklich vom automatischen HTTPS ausgenommen. +- Einstellungen → Portal: Beim Setzen einer Portal-Basisdomain wurde nur die Basisdomain auf ihre DNS-Verifizierung geprüft, nie der daraus zusammengesetzte Host `.`. Fehlte dessen A/AAAA-Eintrag, landete er trotzdem in der TLS-Automation und Caddy versuchte 30 Tage lang (`max_duration`) erfolglos ein Zertifikat beim produktiven Let's-Encrypt-CA zu beziehen — das Kontingent von 5 fehlgeschlagenen Prüfungen pro Stunde und Hostname war sofort erschöpft, ohne jede Rückmeldung im UI. Der zusammengesetzte Host wird jetzt vor dem Speichern selbst aufgelöst und mit der Server-IP verglichen; passt er nicht, lehnt die API mit einem eigenen Fehlertext ab. Ein nicht erreichbarer Resolver blockiert nicht. +- Der Host des Verwaltungs-UI (`GC_BASE_URL`) wurde der TLS-Automation nie mitgegeben, weil seine Route erst danach angelegt wird. Ohne passende Route-Domain fiel er auf Caddys Standard-Automation zurück — ein ACME-Konto ganz ohne Kontaktadresse, obwohl `GC_CADDY_EMAIL` gesetzt war, sodass für dieses Zertifikat nie Ablaufbenachrichtigungen verschickt wurden. Er wird jetzt wie der Portal-Host ausdrücklich übergeben. --- diff --git a/src/i18n/de.json b/src/i18n/de.json index e482f8c2..4efd7018 100644 --- a/src/i18n/de.json +++ b/src/i18n/de.json @@ -1950,6 +1950,7 @@ "settings.portal.saved": "Portal-Einstellungen gespeichert", "settings.portal.host_not_verified": "Domain ist nicht verifiziert", "settings.portal.host_invalid_prefix": "Ungültiges Subdomain-Präfix", + "settings.portal.host_unresolved": "Der Portal-Host hat keinen DNS-Eintrag, der auf diesen Server zeigt \u2014 bitte zuerst einen A/AAAA-Eintrag anlegen", "settings.portal.host_collision": "Host kollidiert mit der GateControl-Adresse, einer Route-Domain oder einem Peer-Hostnamen", "settings.portal.address": "Portal-Adresse", "settings.portal.base_domain": "Basis-Domain", diff --git a/src/i18n/en.json b/src/i18n/en.json index ab4e85d4..a3b1b748 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -2006,6 +2006,7 @@ "settings.portal.saved": "Portal settings saved", "settings.portal.host_not_verified": "Domain is not verified", "settings.portal.host_invalid_prefix": "Invalid subdomain prefix", + "settings.portal.host_unresolved": "The portal host has no DNS record pointing at this server — create an A/AAAA record for it first", "settings.portal.host_collision": "Host collides with the GateControl address, a route domain, or a peer hostname", "settings.portal.address": "Portal address", "settings.portal.base_domain": "Base domain", diff --git a/src/routes/api/settings/portal.js b/src/routes/api/settings/portal.js index 871a7c87..93e5ed67 100644 --- a/src/routes/api/settings/portal.js +++ b/src/routes/api/settings/portal.js @@ -37,7 +37,7 @@ router.get('/portal', (req, res) => { * { enabled: bool, widgets: { device: bool, traffic: bool, services: bool, pihole: bool }, * trust_owner_mapping: bool, base_domain: string, prefix: string } */ -router.put('/portal', (req, res) => { +router.put('/portal', async (req, res) => { try { const body = req.body || {}; const widgets = body.widgets || {}; @@ -78,7 +78,7 @@ router.put('/portal', (req, res) => { if (body.base_domain !== undefined || body.prefix !== undefined) { const base = String(body.base_domain !== undefined ? body.base_domain : settings.get('portal.base_domain', '') || '').trim().toLowerCase(); const prefix = String(body.prefix !== undefined ? (body.prefix == null ? '' : body.prefix) : settings.get('portal.prefix', 'home')).trim().toLowerCase(); - const v = validatePortalHost(base, prefix); + const v = await validatePortalHost(base, prefix); if (!v.ok) return res.status(400).json({ ok: false, error: req.t('settings.portal.host_' + v.error) }); // NOTE: GC_CADDY_EMAIL is intentionally NOT required. ACME issuance (Let's Encrypt) // does not need an account email; when GC_CADDY_EMAIL is empty, buildTlsAutomation diff --git a/src/services/caddyConfig.js b/src/services/caddyConfig.js index 8a1b4260..b00ffd02 100644 --- a/src/services/caddyConfig.js +++ b/src/services/caddyConfig.js @@ -677,16 +677,20 @@ function buildCaddyConfig(injectedRoutes, options = {}) { // split a single `.test`/`.local`/`.internal` route would hammer the // Let's Encrypt rate-limit endpoint with retries every hour and // pollute acme logs. - // homeHost is passed explicitly because it is added to caddyRoutes below, - // AFTER this call, so it would otherwise be absent from the TLS policy. + // homeHost and gcHost are passed explicitly because both are added to + // caddyRoutes below, AFTER this call, so they would otherwise be absent from + // the TLS policy. For gcHost that meant the management UI fell through to + // Caddy's DEFAULT automation — an ACME account with no contact email, i.e. no + // expiry notices, even with GC_CADDY_EMAIL set. + let gcHost = ''; + try { gcHost = new URL(config.app.baseUrl || '').hostname.toLowerCase(); } catch { /* unset/invalid baseUrl */ } const forceInternal = portal.public ? [] : [homeHost]; - const tlsConfig = buildTlsAutomation([...Object.keys(caddyRoutes), homeHost], config.caddy, forceInternal); + const tlsDomains = [...new Set([...Object.keys(caddyRoutes), homeHost, gcHost].filter(Boolean))]; + const tlsConfig = buildTlsAutomation(tlsDomains, config.caddy, forceInternal); if (tlsConfig) caddyConfig.apps.tls = tlsConfig; // GateControl management UI route - const baseUrl = config.app.baseUrl || ''; try { - const gcHost = new URL(baseUrl).hostname; if (gcHost && !caddyRoutes[gcHost]) { caddyRoutes[gcHost] = { listen: [':443', ':80'], diff --git a/src/services/portalConfig.js b/src/services/portalConfig.js index cd2ebeb4..82e4c2b5 100644 --- a/src/services/portalConfig.js +++ b/src/services/portalConfig.js @@ -55,7 +55,7 @@ function collidesWithPeer(host) { const rows = getDb().prepare("SELECT hostname FROM peers WHERE hostname IS NOT NULL AND hostname != ''").all(); return rows.some(r => `${String(r.hostname).trim().toLowerCase()}.${config.dns.domain}` === host); } -function validatePortalHost(base, prefix) { +async function validatePortalHost(base, prefix) { base = String(base || '').trim().toLowerCase(); prefix = String(prefix == null ? 'home' : prefix).trim().toLowerCase(); if (!base) return { ok: true }; // internal default @@ -66,6 +66,15 @@ function validatePortalHost(base, prefix) { if (collidesWithGateControl(host) || collidesWithRoute(host) || collidesWithPeer(host)) { return { ok: false, error: 'collision' }; } + // The apex being verified says NOTHING about .. Committing an + // unresolvable host hands it to buildTlsAutomation, and Caddy then retries ACME + // against the production CA for max_duration = 30 days (burning Let's Encrypt's + // 5 failed-validations/hour/hostname budget). So resolve the composed host too. + if (host !== base) { + const v = await domains.verify(host); + // 'pending' = our resolver is unreachable — can't decide, so don't block. + if (v.status === 'failed') return { ok: false, error: 'unresolved' }; + } return { ok: true }; } diff --git a/tests/caddy_tls_management_host.test.js b/tests/caddy_tls_management_host.test.js new file mode 100644 index 00000000..50639032 --- /dev/null +++ b/tests/caddy_tls_management_host.test.js @@ -0,0 +1,45 @@ +'use strict'; +// Regression: the management UI host (GC_BASE_URL) is added to caddyRoutes AFTER +// buildTlsAutomation ran, so with an empty routes table it was absent from the TLS +// policy entirely and fell through to Caddy's DEFAULT automation — an ACME account +// with NO contact email, even with GC_CADDY_EMAIL configured. +const crypto = require('crypto'); +process.env.GC_ENCRYPTION_KEY = process.env.GC_ENCRYPTION_KEY || crypto.randomBytes(32).toString('hex'); +process.env.GC_CADDY_EMAIL = 'admin@example.com'; // TLS policies only emit when email set +const { test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { setup, teardown } = require('./helpers/setup'); + +const GC_HOST = 'gc.example.com'; +let buildCaddyConfig; +beforeEach(async () => { + await setup(); + // helpers/setup pins GC_BASE_URL to localhost — whose TLD is private, so it would + // never reach the ACME branch. Re-point it and drop the two modules that captured + // it at require-time (their own dependencies stay cached). + process.env.GC_BASE_URL = `https://${GC_HOST}`; + delete require.cache[require.resolve('../config/default')]; + delete require.cache[require.resolve('../src/services/caddyConfig')]; + buildCaddyConfig = require('../src/services/caddyConfig').buildCaddyConfig; +}); +afterEach(teardown); + +const policies = (cfg) => ((cfg.apps.tls || {}).automation || {}).policies || []; + +test('management host gets an explicit ACME policy carrying the account email', async () => { + const cfg = await buildCaddyConfig(); + const mgmt = policies(cfg) + .filter(p => (p.issuers || []).some(i => i.module === 'acme')) + .find(p => (p.subjects || []).includes(GC_HOST)); + assert.ok(mgmt, `${GC_HOST} must have an explicit ACME policy, not fall through to the default`); + assert.equal(mgmt.issuers.find(i => i.module === 'acme').email, 'admin@example.com'); +}); + +test('management host appears exactly once even when a route already serves it', async () => { + require('../src/db/connection').getDb() + .prepare("INSERT INTO routes (description, domain, target_ip, target_port, enabled, route_type) VALUES ('r',?,'10.0.0.2','80',1,'http')") + .run(GC_HOST); + const cfg = await buildCaddyConfig(); + const hits = policies(cfg).flatMap(p => p.subjects || []).filter(s => s === GC_HOST); + assert.equal(hits.length, 1, 'no duplicate subject when the host is both a route and the management host'); +}); diff --git a/tests/portal_host_helper.test.js b/tests/portal_host_helper.test.js index de59b137..0c6be292 100644 --- a/tests/portal_host_helper.test.js +++ b/tests/portal_host_helper.test.js @@ -5,12 +5,18 @@ const { test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const { setup, teardown } = require('./helpers/setup'); -let pc, settings, getDb; +let pc, settings, getDb, domains; +// The composed portal host is DNS-checked, so every validatePortalHost test needs a +// resolver seam. Default: the host points at us (server.public_ip) — the happy path. +const SERVER_IP = '198.51.100.7'; beforeEach(async () => { await setup(); pc = require('../src/services/portalConfig'); settings = require('../src/services/settings'); getDb = require('../src/db/connection').getDb; + domains = require('../src/services/domains'); + settings.set('server.public_ip', SERVER_IP); + domains._setResolverForTest(async (host, family) => (family === 4 ? [SERVER_IP] : [])); }); afterEach(teardown); @@ -28,31 +34,60 @@ test('effectivePortalHost: prefix.base when base set; empty prefix -> apex', () assert.deepEqual(pc.effectivePortalHost(), { host: 'domaincaster.com', public: true }); }); -test('validatePortalHost: empty base ok; unverified rejected; verified ok', () => { - assert.equal(pc.validatePortalHost('', 'home').ok, true); - assert.equal(pc.validatePortalHost('nope.com', 'home').ok, false); // not in domains +test('validatePortalHost: empty base ok; unverified rejected; verified ok', async () => { + assert.equal((await pc.validatePortalHost('', 'home')).ok, true); + assert.equal((await pc.validatePortalHost('nope.com', 'home')).ok, false); // not in domains getDb().prepare("INSERT INTO domains (domain, status) VALUES ('domaincaster.com','verified')").run(); - assert.equal(pc.validatePortalHost('domaincaster.com', 'home').ok, true); + assert.equal((await pc.validatePortalHost('domaincaster.com', 'home')).ok, true); }); -test('validatePortalHost: rejects collision with a route domain', () => { +test('validatePortalHost: rejects collision with a route domain', async () => { getDb().prepare("INSERT INTO domains (domain, status) VALUES ('domaincaster.com','verified')").run(); getDb().prepare("INSERT INTO routes (description, domain, target_ip, target_port, enabled, route_type) VALUES ('r','home.domaincaster.com','10.0.0.2','80',1,'http')").run(); - const r = pc.validatePortalHost('domaincaster.com', 'home'); + const r = await pc.validatePortalHost('domaincaster.com', 'home'); assert.equal(r.ok, false); assert.equal(r.error, 'collision'); }); -test('validatePortalHost: rejects collision with a peer FQDN', () => { +test('validatePortalHost: rejects collision with a peer FQDN', async () => { getDb().prepare("INSERT INTO domains (domain, status) VALUES ('gc.internal','verified')").run(); getDb().prepare("INSERT INTO peers (name, public_key, allowed_ips, enabled, peer_type, hostname) VALUES ('p','k','10.8.0.9/32',1,'regular','alice')").run(); // peer FQDN = alice.; choosing base=gc.internal + prefix=alice collides - const r = pc.validatePortalHost('gc.internal', 'alice'); + const r = await pc.validatePortalHost('gc.internal', 'alice'); assert.equal(r.ok, false); assert.equal(r.error, 'collision'); }); -test('validatePortalHost: rejects invalid prefix', () => { +test('validatePortalHost: rejects invalid prefix', async () => { getDb().prepare("INSERT INTO domains (domain, status) VALUES ('domaincaster.com','verified')").run(); - assert.equal(pc.validatePortalHost('domaincaster.com', 'bad_prefix!').ok, false); + assert.equal((await pc.validatePortalHost('domaincaster.com', 'bad_prefix!')).ok, false); +}); + +// ── Regression: verified apex + NXDOMAIN subdomain → 30-day ACME retry loop ── +test('validatePortalHost: rejects a composed host with no DNS record (NXDOMAIN)', async () => { + getDb().prepare("INSERT INTO domains (domain, status) VALUES ('domaincaster.com','verified')").run(); + domains._setResolverForTest(async () => []); // NXDOMAIN for home.domaincaster.com + const r = await pc.validatePortalHost('domaincaster.com', 'home'); + assert.equal(r.ok, false); + assert.equal(r.error, 'unresolved'); +}); + +test('validatePortalHost: rejects a composed host pointing at a foreign IP', async () => { + getDb().prepare("INSERT INTO domains (domain, status) VALUES ('domaincaster.com','verified')").run(); + domains._setResolverForTest(async (host, family) => (family === 4 ? ['203.0.113.9'] : [])); + const r = await pc.validatePortalHost('domaincaster.com', 'home'); + assert.equal(r.ok, false); + assert.equal(r.error, 'unresolved'); +}); + +test('validatePortalHost: unreachable resolver does not block (pending, not failed)', async () => { + getDb().prepare("INSERT INTO domains (domain, status) VALUES ('domaincaster.com','verified')").run(); + domains._setResolverForTest(async () => { throw new Error('timeout'); }); + assert.equal((await pc.validatePortalHost('domaincaster.com', 'home')).ok, true); +}); + +test('validatePortalHost: apex portal host (empty prefix) is not re-resolved', async () => { + getDb().prepare("INSERT INTO domains (domain, status) VALUES ('domaincaster.com','verified')").run(); + domains._setResolverForTest(async () => { throw new Error('resolver must not be called'); }); + assert.equal((await pc.validatePortalHost('domaincaster.com', '')).ok, true); }); diff --git a/tests/portal_settings_host.test.js b/tests/portal_settings_host.test.js index ad876de8..dc44e309 100644 --- a/tests/portal_settings_host.test.js +++ b/tests/portal_settings_host.test.js @@ -8,8 +8,17 @@ const { test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const { setup, teardown, getAgent, getCsrf } = require('./helpers/setup'); -let getDb; -beforeEach(async () => { await setup(); getDb = require('../src/db/connection').getDb; }); +let getDb, domains; +// The composed portal host is DNS-checked before it is committed. Pin the server IP and +// the resolver so these tests never touch the network: default = the host points at us. +const SERVER_IP = '198.51.100.7'; +beforeEach(async () => { + await setup(); + getDb = require('../src/db/connection').getDb; + domains = require('../src/services/domains'); + require('../src/services/settings').set('server.public_ip', SERVER_IP); + domains._setResolverForTest(async (host, family) => (family === 4 ? [SERVER_IP] : [])); +}); afterEach(teardown); test('PUT accepts a verified base domain + prefix and GET reflects it', async () => { @@ -39,6 +48,20 @@ test('PUT rejects an unverified base domain (400)', async () => { .send({ base_domain: 'unverified.com', prefix: 'home' }).expect(400); }); +// Regression: a verified apex says nothing about .. Committing an +// unresolvable portal host started a 30-day production-ACME retry loop in Caddy. +test('PUT rejects a verified base whose composed host has no DNS record (400)', async () => { + getDb().prepare("INSERT INTO domains (domain, status) VALUES ('domaincaster.com','verified')").run(); + domains._setResolverForTest(async () => []); // NXDOMAIN for home.domaincaster.com + const agent = getAgent(); const csrf = getCsrf(); + await agent.put('/api/v1/settings/portal').set('X-CSRF-Token', csrf) + .send({ base_domain: 'domaincaster.com', prefix: 'home' }).expect(400); + // and nothing was persisted — the ACME policy must never see the bad host + const get = await agent.get('/api/v1/settings/portal').expect(200); + assert.equal(get.body.data.base_domain, ''); + assert.equal(get.body.data.isPublic, false); +}); + test('widget toggles still work (no regression)', async () => { const agent = getAgent(); const csrf = getCsrf(); await agent.put('/api/v1/settings/portal').set('X-CSRF-Token', csrf).send({ enabled: false }).expect(200); From eaf9db6deca994765b3e648aa1b9c89ae5ee8a64 Mon Sep 17 00:00:00 2001 From: "ma.backes" Date: Sat, 25 Jul 2026 23:10:21 +0200 Subject: [PATCH 2/2] test: exakter Subject-Vergleich statt Array#includes (CodeQL-Falschtreffer) --- tests/caddy_tls_management_host.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/caddy_tls_management_host.test.js b/tests/caddy_tls_management_host.test.js index 50639032..4cc16f79 100644 --- a/tests/caddy_tls_management_host.test.js +++ b/tests/caddy_tls_management_host.test.js @@ -30,7 +30,8 @@ test('management host gets an explicit ACME policy carrying the account email', const cfg = await buildCaddyConfig(); const mgmt = policies(cfg) .filter(p => (p.issuers || []).some(i => i.module === 'acme')) - .find(p => (p.subjects || []).includes(GC_HOST)); + // exact subject equality — Array#includes here reads to CodeQL as URL substring matching + .find(p => (p.subjects || []).some(s => s === GC_HOST)); assert.ok(mgmt, `${GC_HOST} must have an explicit ACME policy, not fall through to the default`); assert.equal(mgmt.issuers.find(i => i.module === 'acme').email, 'admin@example.com'); });