Skip to content

Commit db30806

Browse files
committed
feat(dstack-ingress): honour RENEW_DAYS_BEFORE on dns-01, and document testing
RENEW_DAYS_BEFORE was wired into legoman.py only, so it silently did nothing on the dns-01 path -- while the README listed it in the shared table, with every other tls-alpn-01-only setting explicitly marked as such and this one not. The asymmetry also cost test coverage. lego takes the renewal window as a flag, which makes "certificate is due" reachable on demand and lets both renewal branches be exercised. certbot has no equivalent flag: it reads `renew_before_expiry` from the lineage's renewal config. With nothing writing that, a fresh certificate could not be made due, and the dns-01 renewal path could only ever be observed in its "nothing to do" branch. Write the setting before `certbot renew` so the variable means the same thing in both modes. Unset keeps certbot's own 30-day default. TESTING.md documents how to exercise this component against real infrastructure -- three tiers by cost, what each can and cannot observe, how to reach both renewal branches, and the traps that make a test pass for the wrong reason (leftover records changing which branch runs, single-resolver negative caching, gateway TXT caching).
1 parent 4a822fc commit db30806

3 files changed

Lines changed: 347 additions & 1 deletion

File tree

custom-domain/dstack-ingress/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ environment:
182182
| `DOH_RESOLVERS` | Google + Cloudflare | Comma-separated DoH endpoints used to verify records |
183183
| `TLSALPN_PORT` | `9443` | Loopback port lego's ACME responder binds to |
184184
| `TLS_TERMINATE_PORT` | `9444` | Loopback port the TLS frontend moves to in tls-alpn-01 mode |
185-
| `RENEW_DAYS_BEFORE` | lego default | Days of remaining lifetime that trigger renewal |
185+
| `RENEW_DAYS_BEFORE` | client default | Days of remaining lifetime that trigger renewal. Applies to both modes: passed to lego as `--renew-days`, and written to certbot's `renew_before_expiry` |
186186
| `RENEW_INTERVAL` | `43200` | Seconds between successful certificate passes |
187187
| `DNS_SETTLE_SECONDS` | `30` | Wait after DNS verifies, so the gateway's own TXT cache expires |
188188
| `MAXCONN` | `4096` | HAProxy max connections |
Lines changed: 277 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,277 @@
1+
# Testing dstack-ingress
2+
3+
This component's entire job is to obtain and serve TLS certificates. Almost
4+
everything that can go wrong involves a party we do not control — a CA, a DNS
5+
resolver, a gateway, a TEE — so a test that mocks those tests very little. What
6+
follows is how to exercise it against the real things, cheaply enough to do
7+
routinely.
8+
9+
Use **Let's Encrypt staging** throughout (`CERTBOT_STAGING=true` /
10+
`ACME_STAGING=true`). Its rate limits are generous and its certificates are
11+
untrusted, which is exactly what you want: a test certificate that browsers
12+
reject cannot be mistaken for a production one.
13+
14+
## Three tiers, in increasing cost
15+
16+
Pick the cheapest tier that can actually observe what you changed.
17+
18+
| Tier | Setup | Can test | Cannot test |
19+
|---|---|---|---|
20+
| **1. Container + simulator** | Docker, dstack simulator | dns-01 end to end, config generation, negative paths, `DNS_SETUP_MODE`, evidence, webhook | Anything needing inbound traffic |
21+
| **2. Tier 1 + real DNS credentials** | + a zone you control | Everything the DNS provider code does | Same |
22+
| **3. Real CVM behind a gateway** | + TDX host, `dstack-vmm`, `dstack-gateway`, public port 443 | tls-alpn-01, ALPN routing, real TDX quotes, gateway interaction ||
23+
24+
dns-01 needs no inbound connection: the CA reads a TXT record and never talks to
25+
you. That is why the whole dns-01 path fits in tier 1/2. tls-alpn-01 is the
26+
opposite — the CA connects to port 443 and completes a TLS handshake — so it can
27+
only be tested for real in tier 3.
28+
29+
## Tier 1/2: containers
30+
31+
The container asks the guest agent for its app and instance ID and for TDX
32+
quotes, so it needs a socket at `/var/run/dstack.sock`. Outside a CVM, use the
33+
simulator:
34+
35+
```bash
36+
cd dstack/sdk/simulator && ./build.sh && ./dstack-simulator &
37+
```
38+
39+
Then mount its socket:
40+
41+
```yaml
42+
volumes:
43+
- /path/to/sdk/simulator/dstack.sock:/var/run/dstack.sock
44+
```
45+
46+
Keep provider credentials out of the compose file — put them in a `.env` that
47+
compose reads, and `chmod 600` it:
48+
49+
```bash
50+
umask 077 && printf 'CLOUDFLARE_API_TOKEN=%s\n' "$TOKEN" > .env
51+
```
52+
53+
Give each scenario its own project name and its own volumes
54+
(`docker compose -p <name>`), so a leftover certificate from a previous run
55+
cannot make the next one pass for the wrong reason. This matters more than it
56+
sounds — see "Start from a clean zone" below.
57+
58+
## Tier 3: a real CVM
59+
60+
You need a TDX host running `dstack-vmm`, `dstack-kms` and `dstack-gateway`, and
61+
the gateway must be reachable from the internet on **port 443** — the CA picks
62+
that port and RFC 8737 gives you no say. If the gateway listens elsewhere,
63+
redirect:
64+
65+
```bash
66+
sudo iptables -t nat -A PREROUTING -d <public-ip> -p tcp --dport 443 \
67+
-m comment --comment 'ingress-lab' -j REDIRECT --to-ports <gateway-port>
68+
```
69+
70+
Tag every rule with a `--comment` so teardown can find them all later.
71+
72+
Point the test hostname at the gateway (an `A` record works; the container's
73+
CNAME check compares resolved addresses, not record types), then publish the
74+
routing TXT:
75+
76+
```
77+
_dstack-app-address.<domain> TXT "<instance_id>:443"
78+
```
79+
80+
**Instance ID, not app ID.** In tls-alpn-01 mode the challenge is answered by
81+
whichever instance holds the ACME order, while the gateway load-balances across
82+
every instance of the app and the CA validates from several vantage points at
83+
once — five distinct source IPs within ~2 seconds, in practice. Publishing the
84+
app ID sends the challenge to the wrong replica almost every time.
85+
86+
Reading container logs inside a CVM: with `--public-logs`, the guest agent serves
87+
them over the gateway's WireGuard network.
88+
89+
```bash
90+
curl -s 'http://10.44.0.<n>:8090/logs/dstack-dstack-ingress-1?text&bare&tail=500'
91+
```
92+
93+
The container name is the compose-mangled one (`dstack-<service>-1`), not the
94+
service name. Find the instance's WireGuard address by probing the peers listed
95+
in `wg show <interface> allowed-ips`; stale peers from removed VMs stay in the
96+
list, so probe for an open port rather than trusting it.
97+
98+
## Exercising each area
99+
100+
### Issuance
101+
102+
Nothing special — start the container and wait. What to assert:
103+
104+
- the certificate is served on port 443 and its issuer is a staging CA;
105+
- SAN matches the requested name;
106+
- for `ROUTING_MAP`, each hostname reaches *its own* backend. Routing is by SNI,
107+
so send the SNI, not just a `Host` header.
108+
109+
### The ALPN split (tls-alpn-01)
110+
111+
The peek frontend forwards `acme-tls/1` to lego and everything else to the TLS
112+
frontend. You can drive both sides by hand:
113+
114+
```bash
115+
openssl s_client -connect <domain>:443 -servername <domain> -alpn acme-tls/1
116+
openssl s_client -connect <domain>:443 -servername <domain> -alpn h2,http/1.1
117+
```
118+
119+
and then confirm in the container's haproxy log that they took different paths:
120+
121+
```
122+
tls_peek be_acme/lego ← the ACME probe
123+
tls_peek be_tls_terminate/local ← normal traffic
124+
```
125+
126+
This is the mechanism the whole mode rests on, and it is worth checking directly
127+
rather than inferring it from "issuance worked".
128+
129+
### Renewal — both branches
130+
131+
Renewal is the part most likely to break in production and the least likely to be
132+
covered, because a fresh certificate is not due for ~60 days. Both clients take a
133+
renewal window, and `RENEW_DAYS_BEFORE` sets it in either mode:
134+
135+
- **not due**: leave `RENEW_DAYS_BEFORE` unset and restart the container. Expect
136+
lego `Skip renewal` / certbot `No certificates need renewal`, **no haproxy
137+
reload, and no new evidence**. A pass that reloads on every check is a bug —
138+
it means the renewal decision is not being read correctly.
139+
- **due**: set `RENEW_DAYS_BEFORE=365` and restart. Expect a new certificate,
140+
a reload, and regenerated evidence.
141+
142+
Then check the certificate actually changed on the wire — the serial number, not
143+
just the log line:
144+
145+
```bash
146+
openssl s_client -connect <domain>:443 -servername <domain> </dev/null 2>/dev/null \
147+
| openssl x509 -noout -serial -dates
148+
```
149+
150+
Shorten `RENEW_INTERVAL` (default 43200s) if you want the loop to come round
151+
again without restarting.
152+
153+
Do not assert on the ACME client's log wording. lego 4.x wrote `no renewal` and
154+
5.x writes `Skip renewal`; certbot's phrasing has moved too. The container uses
155+
lego's `--deploy-hook`, which fires only on an actual renewal, and certbot's exit
156+
behaviour — test the observable effect (reload / no reload), not the prose.
157+
158+
### Evidence
159+
160+
`/evidences/` is served on the TLS port. The chain to verify:
161+
162+
```
163+
quote.report_data == sha256(sha256sum.txt)
164+
sha256sum.txt covers acme-account.json and every cert-<domain>.pem
165+
cert-<domain>.pem == the certificate actually served
166+
```
167+
168+
so:
169+
170+
```bash
171+
curl -sk https://<domain>/evidences/sha256sum.txt -o s.txt && sha256sum -c s.txt
172+
python3 -c "import json,hashlib;print(json.load(open('quote.json'))['report_data'][:64] \
173+
== hashlib.sha256(open('s.txt','rb').read()).hexdigest())"
174+
```
175+
176+
Compare the evidence copy against a live `openssl s_client` fingerprint — the
177+
point of the chain is that the quote commits to what is *being served*, and only
178+
that comparison tests it.
179+
180+
To verify the quote itself rather than trusting it, run it through `dcap-qvl`
181+
against Intel collateral and expect `status = UpToDate`. Re-run the whole chain
182+
after a renewal: the evidence must track the new certificates, not go stale at
183+
first issuance.
184+
185+
### DNS setup modes (tls-alpn-01)
186+
187+
- `wait` — the default. Start the container *before* creating the records and
188+
confirm it blocks and says which record is missing. Then create them and
189+
confirm it proceeds. This is also the only mode that runs the CAA
190+
compatibility check.
191+
- `print` — skips all verification, including CAA. Confirm it continues
192+
immediately.
193+
- `webhook` — set `DNS_WEBHOOK_URL` and `DNS_WEBHOOK_TOKEN` and point them at a
194+
throwaway HTTP server. The body carries the records, an HMAC-SHA256 over the
195+
payload, and a TDX quote whose `report_data` is `sha256(payload)`. Verify both:
196+
the HMAC proves the shared secret, the quote proves the enclave.
197+
198+
### Negative paths
199+
200+
These fail fast and are cheap, so run them on every change:
201+
202+
| Scenario | Expected |
203+
|---|---|
204+
| tls-alpn-01 + a wildcard domain | Refused before any ACME call, citing RFC 8737 |
205+
| A CAA record with `validationmethods=dns-01`, mode tls-alpn-01 | Blocked with the restriction quoted back |
206+
| TXT holding the wrong value | Reported as `want <x>, saw <y>`, not "missing" |
207+
| An instance ID the gateway does not know | The CA reports a connection error — the gateway will not route to it |
208+
209+
The last one is worth keeping: it is the cross-tenant impersonation defence, and
210+
it is observable rather than merely argued.
211+
212+
## Traps
213+
214+
**Start from a clean zone.** Records left over from an earlier run change which
215+
branch executes. A stale CAA record naming a previous ACME account makes dns-01's
216+
first issuance attempt fail and the second succeed — which happens to be the path
217+
where the code was correct, hiding a bug in the ordinary path where the first
218+
attempt succeeds. Delete test records between runs and verify against the
219+
authoritative nameserver, not a cached resolver.
220+
221+
**One resolver is not enough to trust either answer.** Public resolvers cache
222+
negative answers for the zone's SOA minimum, so a resolver queried before a
223+
record existed can keep saying "no such record" for up to an hour. They also
224+
disagree on wire format: Cloudflare returns CAA in RFC 3597 generic hex
225+
(`\# 47 00 05 69 73 73 75 65 ...`), Google in presentation form. The container
226+
queries two resolvers with deliberately opposite quorums — propagation needs all
227+
of them to agree, CAA takes the union so any resolver seeing a restriction stops
228+
issuance — and if you are checking DNS by hand you should do the same.
229+
230+
**Public DNS being correct is not the same as the gateway acting on it.** The
231+
gateway caches the app-address TXT for its TTL, so immediately after the value
232+
changes it can still route to the previous target. `DNS_SETTLE_SECONDS`
233+
(default 30, first pass only) covers this. A challenge that fails with
234+
`Error getting validation data` right after a DNS change is usually this, not a
235+
broken challenge.
236+
237+
**lego buffers its output until it exits.** A renewal can show nothing at all in
238+
the log for several minutes. Check whether the process is still running before
239+
concluding it hung.
240+
241+
**`ALPN` is not validated against the backend.** Advertising `h2` in front of an
242+
HTTP/1.1-only backend makes haproxy negotiate HTTP/2 on its behalf and the
243+
connection then breaks. If a test client fails but `--http1.1` works, suspect
244+
this before suspecting the proxy.
245+
246+
## Unit tests
247+
248+
```bash
249+
python3 scripts/tests/test_dnsguide.py # DNS/CAA parsing and record building
250+
bash scripts/tests/test_sanitizers.sh # env var validation
251+
```
252+
253+
The parsing tests are built from rdata real resolvers actually returned, in both
254+
encodings. When you touch DNS handling, add the real string you saw rather than
255+
one you composed — the encodings are the part that surprises people.
256+
257+
## haproxy config equivalence
258+
259+
The proxy config is assembled by `haproxy-lib.sh` and composed differently by
260+
each mode. When refactoring that assembly, generate the config both before and
261+
after your change for all four combinations — `dns-01` and `tls-alpn-01`, each
262+
with a single domain and with `ROUTING_MAP` — and diff them. Byte-identical
263+
output is a much stronger statement than "the tests still pass", and it is easy
264+
to obtain because the emitters only write to stdout.
265+
266+
## Cleanup
267+
268+
Leftovers from a certificate test are unusually annoying: a stray CAA record can
269+
block issuance for a domain weeks later, and a stray DNAT rule silently
270+
intercepts port 443. Tear down in this order and verify each:
271+
272+
1. containers and volumes (`docker compose -p <name> down -v`);
273+
2. the CVM (`vmm-cli remove`);
274+
3. iptables rules — delete by comment tag, in a loop, until none match;
275+
4. DNS records — list them from the provider API afterwards, not by `dig`,
276+
which can serve a cached answer for a record you already deleted;
277+
5. the credentials file, the simulator, and any images built for the test.

custom-domain/dstack-ingress/scripts/certman.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from dns_providers import DNSProviderFactory
44
import argparse
55
import os
6+
import re
67
import subprocess
78
import sys
89
import pkg_resources
@@ -414,6 +415,7 @@ def renew_certificate(self, domain: str) -> Tuple[bool, bool]:
414415
print(f"Failed to install plugin for renewal", file=sys.stderr)
415416
return False, False
416417

418+
self.apply_renewal_window(domain)
417419
cmd = self._build_certbot_command("renew", domain, "")
418420

419421
try:
@@ -459,6 +461,73 @@ def certificate_exists(self, domain: str) -> bool:
459461
cert_path = f"/etc/letsencrypt/live/{domain}/fullchain.pem"
460462
return os.path.isfile(cert_path)
461463

464+
@staticmethod
465+
def _lineage_name(domain: str) -> str:
466+
"""certbot stores a wildcard lineage under the bare name."""
467+
return domain[2:] if domain.startswith("*.") else domain
468+
469+
def apply_renewal_window(self, domain: str) -> None:
470+
"""Make RENEW_DAYS_BEFORE mean the same thing here as it does for lego.
471+
472+
lego takes the renewal window as a flag (`--renew-days`). certbot has no
473+
equivalent: it reads `renew_before_expiry` out of the lineage's renewal
474+
config, so the setting has to be written there before `certbot renew`
475+
runs. Without this the variable is silently tls-alpn-01 only, which also
476+
left the dns-01 renewal branch with no way to be exercised on demand.
477+
478+
Leaving it unset keeps certbot's own default (30 days).
479+
"""
480+
days = os.environ.get("RENEW_DAYS_BEFORE", "").strip()
481+
if not days:
482+
return
483+
if not days.isdigit() or int(days) < 1:
484+
print(
485+
f"Warning: ignoring invalid RENEW_DAYS_BEFORE={days!r} "
486+
f"(expected a positive number of days)",
487+
file=sys.stderr,
488+
)
489+
return
490+
491+
path = f"/etc/letsencrypt/renewal/{self._lineage_name(domain)}.conf"
492+
if not os.path.isfile(path):
493+
# No lineage yet: the first issuance has not happened, and certbot
494+
# writes this file itself. Nothing to do.
495+
return
496+
497+
setting = f"renew_before_expiry = {days} days"
498+
try:
499+
with open(path, encoding="utf-8") as fh:
500+
lines = fh.read().splitlines()
501+
except OSError as exc:
502+
print(f"Warning: cannot read {path}: {exc}", file=sys.stderr)
503+
return
504+
505+
out, replaced = [], False
506+
for line in lines:
507+
# The key ships commented out, so match both forms.
508+
if re.match(r"\s*#?\s*renew_before_expiry\s*=", line):
509+
if not replaced:
510+
out.append(setting)
511+
replaced = True
512+
continue
513+
out.append(line)
514+
515+
if not replaced:
516+
# Must land before the first section header; the key is top-level.
517+
insert_at = next(
518+
(i for i, line in enumerate(out) if line.strip().startswith("[")),
519+
len(out),
520+
)
521+
out.insert(insert_at, setting)
522+
523+
try:
524+
with open(path, "w", encoding="utf-8") as fh:
525+
fh.write("\n".join(out) + "\n")
526+
except OSError as exc:
527+
print(f"Warning: cannot write {path}: {exc}", file=sys.stderr)
528+
return
529+
print(f"Renewal window for {domain} set to {days} days before expiry")
530+
462531
def acme_account_exists(self) -> bool:
463532
"""Check if an ACME account exists for the current server (staging or production).
464533

0 commit comments

Comments
 (0)