1. caddy_manager: embed ddns.token (registration bearer token) in Caddyfile, not DDNS_TOTP_SECRET. The pic_ngo plugin sends the token to POST /api/v1/dns-challenge; using the TOTP secret caused 401 on every attempt. 2. firewall_manager: add _acme-challenge.<zone> forwarding block before each split-horizon zone in the Corefile. Without this, CoreDNS was authoritative for the challenge name and returned NODATA for TXT queries (wildcard A record matches but wrong type), blocking Caddy's internal DNS pre-verification step. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -286,8 +286,11 @@ class CaddyManager(BaseServiceManager):
|
|||||||
# Resolve credentials at write time — Caddy runs in its own container
|
# Resolve credentials at write time — Caddy runs in its own container
|
||||||
# and does not inherit the API's environment variables, so we embed the
|
# and does not inherit the API's environment variables, so we embed the
|
||||||
# actual values instead of {$VAR} placeholders.
|
# actual values instead of {$VAR} placeholders.
|
||||||
ddns_token = (os.environ.get('DDNS_TOTP_SECRET') or '').strip()
|
# Use the registration bearer token (ddns.token), NOT the TOTP secret —
|
||||||
ddns_api = (os.environ.get('DDNS_URL') or 'https://ddns.pic.ngo/api/v1').strip()
|
# the pic_ngo plugin authenticates to POST /api/v1/dns-challenge with this token.
|
||||||
|
ddns_cfg = self.config_manager.configs.get('ddns', {})
|
||||||
|
ddns_token = (ddns_cfg.get('token') or os.environ.get('DDNS_TOKEN') or '').strip()
|
||||||
|
ddns_api = (os.environ.get('DDNS_URL') or ddns_cfg.get('url') or 'https://ddns.pic.ngo/api/v1').strip()
|
||||||
|
|
||||||
return (
|
return (
|
||||||
f"{self._global_acme_block(email)}\n"
|
f"{self._global_acme_block(email)}\n"
|
||||||
|
|||||||
@@ -758,6 +758,18 @@ def generate_corefile(peers: List[Dict[str, Any]], corefile_path: str = COREFILE
|
|||||||
# *.pic1.pic.ngo to the internal Caddy IP without hairpin NAT.
|
# *.pic1.pic.ngo to the internal Caddy IP without hairpin NAT.
|
||||||
if split_horizon_zones:
|
if split_horizon_zones:
|
||||||
for sz in split_horizon_zones:
|
for sz in split_horizon_zones:
|
||||||
|
# More-specific block for ACME DNS-01 challenge records: forward
|
||||||
|
# to public DNS so Caddy can verify TXT records it creates on the
|
||||||
|
# DDNS server. Without this, the wildcard A record in the zone
|
||||||
|
# file causes CoreDNS to return NODATA for TXT queries, blocking
|
||||||
|
# Caddy's internal pre-verification step.
|
||||||
|
corefile += (
|
||||||
|
f'\n_acme-challenge.{sz} {{\n'
|
||||||
|
f' forward . 8.8.8.8 1.1.1.1\n'
|
||||||
|
f' cache\n'
|
||||||
|
f' log\n'
|
||||||
|
f'}}\n'
|
||||||
|
)
|
||||||
corefile += (
|
corefile += (
|
||||||
f'\n{sz} {{\n'
|
f'\n{sz} {{\n'
|
||||||
f' file /data/{sz}.zone\n'
|
f' file /data/{sz}.zone\n'
|
||||||
|
|||||||
@@ -59,17 +59,16 @@ class TestGenerateCaddyfileLan(unittest.TestCase):
|
|||||||
class TestGenerateCaddyfilePicNgo(unittest.TestCase):
|
class TestGenerateCaddyfilePicNgo(unittest.TestCase):
|
||||||
def test_pic_ngo_has_dns_plugin_and_wildcard(self):
|
def test_pic_ngo_has_dns_plugin_and_wildcard(self):
|
||||||
mgr = _mgr()
|
mgr = _mgr()
|
||||||
|
mgr.config_manager.configs = {
|
||||||
|
'ddns': {'token': 'TESTSECRET123', 'url': 'https://ddns.pic.ngo/api/v1'},
|
||||||
|
}
|
||||||
identity = {'cell_name': 'alpha', 'domain_mode': 'pic_ngo'}
|
identity = {'cell_name': 'alpha', 'domain_mode': 'pic_ngo'}
|
||||||
import os
|
with unittest.mock.patch.dict(os.environ, {'DDNS_URL': 'https://ddns.pic.ngo/api/v1'}):
|
||||||
with unittest.mock.patch.dict(os.environ, {
|
|
||||||
'DDNS_TOTP_SECRET': 'TESTSECRET123',
|
|
||||||
'DDNS_URL': 'https://ddns.pic.ngo/api/v1',
|
|
||||||
}):
|
|
||||||
out = mgr.generate_caddyfile(identity, [])
|
out = mgr.generate_caddyfile(identity, [])
|
||||||
self.assertIn('dns pic_ngo', out)
|
self.assertIn('dns pic_ngo', out)
|
||||||
self.assertIn('*.alpha.pic.ngo', out)
|
self.assertIn('*.alpha.pic.ngo', out)
|
||||||
self.assertIn('alpha.pic.ngo', out)
|
self.assertIn('alpha.pic.ngo', out)
|
||||||
# Credentials are resolved at write time and embedded — no {$VAR} placeholders
|
# Registration token (not TOTP secret) is embedded — no {$VAR} placeholders
|
||||||
self.assertIn('token TESTSECRET123', out)
|
self.assertIn('token TESTSECRET123', out)
|
||||||
self.assertIn('api_base_url https://ddns.pic.ngo/api/v1', out)
|
self.assertIn('api_base_url https://ddns.pic.ngo/api/v1', out)
|
||||||
self.assertNotIn('{$PIC_NGO_DDNS_TOKEN}', out)
|
self.assertNotIn('{$PIC_NGO_DDNS_TOKEN}', out)
|
||||||
@@ -80,10 +79,9 @@ class TestGenerateCaddyfilePicNgo(unittest.TestCase):
|
|||||||
|
|
||||||
def test_pic_ngo_acme_ca_included_when_env_set(self):
|
def test_pic_ngo_acme_ca_included_when_env_set(self):
|
||||||
mgr = _mgr()
|
mgr = _mgr()
|
||||||
|
mgr.config_manager.configs = {'ddns': {'token': 'TESTSECRET123'}}
|
||||||
identity = {'cell_name': 'alpha', 'domain_mode': 'pic_ngo'}
|
identity = {'cell_name': 'alpha', 'domain_mode': 'pic_ngo'}
|
||||||
import os
|
|
||||||
with unittest.mock.patch.dict(os.environ, {
|
with unittest.mock.patch.dict(os.environ, {
|
||||||
'DDNS_TOTP_SECRET': 'TESTSECRET123',
|
|
||||||
'DDNS_URL': 'https://ddns.pic.ngo/api/v1',
|
'DDNS_URL': 'https://ddns.pic.ngo/api/v1',
|
||||||
'ACME_CA_URL': 'https://acme-staging-v02.api.letsencrypt.org/directory',
|
'ACME_CA_URL': 'https://acme-staging-v02.api.letsencrypt.org/directory',
|
||||||
}):
|
}):
|
||||||
|
|||||||
@@ -241,13 +241,21 @@ class TestGenerateCorefileSplitHorizon(unittest.TestCase):
|
|||||||
self.assertIn('pic1.pic.ngo {', content)
|
self.assertIn('pic1.pic.ngo {', content)
|
||||||
self.assertIn('file /data/pic1.pic.ngo.zone', content)
|
self.assertIn('file /data/pic1.pic.ngo.zone', content)
|
||||||
|
|
||||||
def test_split_horizon_zone_does_not_add_forward(self):
|
def test_split_horizon_zone_has_acme_challenge_forward(self):
|
||||||
"""Split-horizon blocks must use 'file', not 'forward'."""
|
"""Each split-horizon zone gets a more-specific _acme-challenge forwarding block
|
||||||
|
so Caddy's DNS-01 pre-verification bypasses the authoritative local zone."""
|
||||||
firewall_manager.generate_corefile([], self.path, split_horizon_zones=['pic1.pic.ngo'])
|
firewall_manager.generate_corefile([], self.path, split_horizon_zones=['pic1.pic.ngo'])
|
||||||
content = self._content()
|
content = self._content()
|
||||||
# Only the default internet forwarder; no extra forward for the horizon zone
|
self.assertIn('_acme-challenge.pic1.pic.ngo {', content)
|
||||||
forward_lines = [l for l in content.splitlines() if 'forward' in l and 'pic1' in l]
|
# ACME block appears before the zone file block so CoreDNS matches it first
|
||||||
self.assertEqual(len(forward_lines), 0)
|
acme_pos = content.index('_acme-challenge.pic1.pic.ngo {')
|
||||||
|
zone_pos = content.index('\npic1.pic.ngo {')
|
||||||
|
self.assertLess(acme_pos, zone_pos)
|
||||||
|
# ACME block contains a forward directive, not a local file
|
||||||
|
acme_block_end = content.index('}', acme_pos)
|
||||||
|
acme_block = content[acme_pos:acme_block_end]
|
||||||
|
self.assertIn('forward .', acme_block)
|
||||||
|
self.assertNotIn('file /data/', acme_block)
|
||||||
|
|
||||||
def test_multiple_split_horizon_zones(self):
|
def test_multiple_split_horizon_zones(self):
|
||||||
"""Multiple zones all get their own file block."""
|
"""Multiple zones all get their own file block."""
|
||||||
|
|||||||
Reference in New Issue
Block a user