Skip to content

Commit 9e30e53

Browse files
committed
test(gateway): pin what the CA refuses, not only what it accepts
The phase proved a certificate issues from a published record. It could not prove the record had to be that record: the only failing case it exercised was the domain having no record at all. Three cases the CA now decides, because it looks the record up rather than being run with PEBBLE_VA_ALWAYS_VALID: - A record correct in every respect except `policy=wildcard` does not answer the order. The gateway only ever orders `*.{domain}`, so every authorization it answers is a wildcard one, and the draft has a CA accept those only from a record carrying the parameter. - Adding that one parameter, and nothing else, issues. Without this the case above could pass for any reason at all. - A record naming a different account does not authorize. If it did, the record would authorize anybody who found it and the challenge would prove nothing. Plus the two gateway operations that change shape for a domain the gateway cannot write: `SetCaa` skips it rather than failing the whole run or writing to a provider it has no credential for, and `RotateAcmeCredentials` returns the records to republish, naming the account it just registered -- the old records name the account it replaced, and nothing else reports them. Each refusal gets its own domain. `DeleteZtDomain` keeps the certificate on purpose, so a domain that has already issued reports that certificate after being re-added, and a refusal would read as a pass. The assertions are inverted rather than matched against an error string, because what is being claimed is that no certificate appears.
1 parent 63fccb7 commit 9e30e53

2 files changed

Lines changed: 157 additions & 1 deletion

File tree

dstack/gateway/test-run/e2e/docker-compose.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ services:
5151
- PORT=8080
5252
- DEBUG=true
5353
# The zones certbot writes into and Pebble reads back out of.
54-
- MOCK_CF_ZONES=test0.local,test1.local,test2.local,persist0.local
54+
- MOCK_CF_ZONES=test0.local,test1.local,test2.local,persist0.local,persist1.local,persist2.local
5555
healthcheck:
5656
test: ["CMD", "python", "-c", "import urllib.request; urllib.request.urlopen('http://localhost:8080/health')"]
5757
interval: 5s

dstack/gateway/test-run/e2e/test.sh

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,19 @@ PERSIST_DOMAIN="persist0.local"
4141
# Mock zone ID, as server.py derives it from the zone name.
4242
PERSIST_ZONE_ID="zone-persist0-local"
4343

44+
# A second dns-persist-01 domain, used for the cases that must NOT issue. Kept
45+
# apart from PERSIST_DOMAIN so a deliberately unanswerable order cannot be
46+
# confused with the working one.
47+
PERSIST_NEG_DOMAIN="persist1.local"
48+
PERSIST_NEG_ZONE_ID="zone-persist1-local"
49+
50+
# A third domain, for the second refusal. Deleting a ZT domain deliberately
51+
# keeps its certificate ("kept for historical purposes"), so reusing a domain
52+
# that has already issued would report the old certificate and read as a
53+
# refusal that did not happen.
54+
PERSIST_NEG2_DOMAIN="persist2.local"
55+
PERSIST_NEG2_ZONE_ID="zone-persist2-local"
56+
4457
# Pebble answers to this name, not to letsencrypt.org, and a validation record
4558
# naming a CA the challenge does not list is ignored.
4659
PERSIST_ISSUER="pebble.letsencrypt.org"
@@ -232,6 +245,135 @@ test_persist_domain_issues_a_certificate() {
232245
return 1
233246
}
234247

248+
# ---- Cases that must NOT issue -------------------------------------------
249+
#
250+
# These are the reason the mock answers DNS at all. With PEBBLE_VA_ALWAYS_VALID
251+
# the CA accepts any challenge, so a wrong record and a right one are
252+
# indistinguishable and the record's grammar is pinned only by unit tests
253+
# written against a reading of the CA. Here the CA reads it.
254+
255+
# Publish one TXT record verbatim at the negative domain's validation name,
256+
# replacing whatever is there.
257+
publish_neg_record() {
258+
local domain="$1" zone="$2" rdata="$3"
259+
local records id
260+
records=$(curl -sf "${MOCK_CF_API}/api/records" 2>/dev/null || echo '{"records":[]}')
261+
for id in $(echo "$records" \
262+
| tr '{' '\n' \
263+
| grep -F "_validation-persist.${domain}" \
264+
| sed -e 's/.*"id": "//' -e 's/".*//'); do
265+
curl -sf -X DELETE \
266+
"${MOCK_CF_API}/client/v4/zones/${zone}/dns_records/${id}" \
267+
-H "Authorization: Bearer ${CF_API_TOKEN}" > /dev/null 2>&1 || true
268+
done
269+
curl -sf -X POST "${MOCK_CF_API}/client/v4/zones/${zone}/dns_records" \
270+
-H "Authorization: Bearer ${CF_API_TOKEN}" \
271+
-H "Content-Type: application/json" \
272+
-d '{
273+
"type": "TXT",
274+
"name": "_validation-persist.'"${domain}"'",
275+
"content": "'"${rdata}"'",
276+
"ttl": 1
277+
}' > /dev/null
278+
}
279+
280+
# The account URI the cluster actually holds, read off the domain the gateway
281+
# already renders records for.
282+
persist_account_uri() {
283+
admin_post GetZtDomain '{"domain": "'"${PERSIST_DOMAIN}"'"}' \
284+
| tr ',' '\n' \
285+
| grep -F "accounturi=" \
286+
| head -1 \
287+
| sed -e 's/.*accounturi=//' -e 's/[";\\].*//'
288+
}
289+
290+
# Try to issue for the negative domain and report whether a certificate
291+
# appeared. Deliberately short: the interesting outcome is "no certificate",
292+
# and an order that is going to be refused says so in one pass.
293+
neg_domain_issued() {
294+
local domain="$1"
295+
admin_post RenewZtDomainCert \
296+
'{"domain": "'"${domain}"'", "force": true}' > /dev/null 2>&1 || true
297+
local i=0
298+
while [ $i -lt 45 ]; do
299+
if admin_post GetZtDomain '{"domain": "'"${domain}"'"}' \
300+
| grep -q '"has_cert":true'; then
301+
return 0
302+
fi
303+
sleep 2
304+
i=$((i + 1))
305+
done
306+
return 1
307+
}
308+
309+
# The gateway only ever orders `*.{domain}`, so every authorization it answers
310+
# is a wildcard one -- and the draft has a CA accept a wildcard authorization
311+
# only from a record carrying `policy=wildcard`. A record that is correct in
312+
# every other respect must therefore not be enough.
313+
test_persist_record_without_wildcard_policy_is_refused() {
314+
admin_post DeleteZtDomain '{"domain": "'"${PERSIST_NEG_DOMAIN}"'"}' > /dev/null 2>&1 || true
315+
admin_post AddZtDomain \
316+
'{"domain": "'"${PERSIST_NEG_DOMAIN}"'", "port": 443, "challenge": "dns-persist-01"}' \
317+
> /dev/null || return 1
318+
local uri
319+
uri=$(persist_account_uri) || return 1
320+
[ -n "$uri" ] || return 1
321+
publish_neg_record "${PERSIST_NEG_DOMAIN}" "${PERSIST_NEG_ZONE_ID}" \
322+
"${PERSIST_ISSUER}; accounturi=${uri}" || return 1
323+
# Inverted: issuing here would mean the policy parameter is not enforced.
324+
! neg_domain_issued "${PERSIST_NEG_DOMAIN}"
325+
}
326+
327+
# Adding the one parameter that was missing, and nothing else, has to flip the
328+
# outcome. That is what makes the previous case a statement about `policy`
329+
# rather than about some unrelated breakage.
330+
test_persist_record_with_wildcard_policy_issues() {
331+
local uri
332+
uri=$(persist_account_uri) || return 1
333+
publish_neg_record "${PERSIST_NEG_DOMAIN}" "${PERSIST_NEG_ZONE_ID}" \
334+
"${PERSIST_ISSUER}; accounturi=${uri}; policy=wildcard" || return 1
335+
neg_domain_issued "${PERSIST_NEG_DOMAIN}"
336+
}
337+
338+
# The account URI in the record is the whole authorization. A record naming a
339+
# different account must not let this one issue -- otherwise publishing a record
340+
# authorizes anybody, and the challenge proves nothing.
341+
test_persist_record_for_another_account_is_refused() {
342+
admin_post DeleteZtDomain '{"domain": "'"${PERSIST_NEG2_DOMAIN}"'"}' > /dev/null 2>&1 || true
343+
admin_post AddZtDomain \
344+
'{"domain": "'"${PERSIST_NEG2_DOMAIN}"'", "port": 443, "challenge": "dns-persist-01"}' \
345+
> /dev/null || return 1
346+
publish_neg_record "${PERSIST_NEG2_DOMAIN}" "${PERSIST_NEG2_ZONE_ID}" \
347+
"${PERSIST_ISSUER}; accounturi=${ACME_URL%/dir}/my-account/0; policy=wildcard" || return 1
348+
! neg_domain_issued "${PERSIST_NEG2_DOMAIN}"
349+
}
350+
351+
# ---- Gateway operations that change shape for such a domain ---------------
352+
353+
# SetCaa reconciles CAA through the DNS provider, which the gateway has no
354+
# credential for here. It must skip the domain rather than fail the call, and
355+
# must not write anything.
356+
test_set_caa_skips_a_persist_domain() {
357+
admin_post SetCaa '{}' > /dev/null 2>&1 || true
358+
local records
359+
records=$(curl -sf "${MOCK_CF_API}/api/records" 2>/dev/null || echo "[]")
360+
! echo "$records" | grep -qF "\"type\": \"CAA\", \"name\": \"${PERSIST_DOMAIN}"
361+
}
362+
363+
# Rotation registers a new account, and every published record names the old
364+
# one. The response has to carry the replacements, because the gateway cannot
365+
# publish them and nothing else reports them.
366+
test_rotation_reports_the_records_to_republish() {
367+
local out
368+
out=$(admin_post RotateAcmeCredentials '{}') || return 1
369+
echo "$out" | grep -qF "_validation-persist.${PERSIST_DOMAIN}" || return 1
370+
# Naming the new account, not the one it replaced.
371+
local uri
372+
uri=$(echo "$out" | sed -e 's/.*"account_uri": *"//' -e 's/".*//')
373+
[ -n "$uri" ] || return 1
374+
echo "$out" | grep -qF "accounturi=${uri}"
375+
}
376+
235377
# The claim the whole challenge exists for: certbot reads DNS and never writes
236378
# it. Scoped to the record certbot would have written -- the zone is not empty,
237379
# because the harness published the validation record above standing in for the
@@ -491,6 +633,20 @@ main() {
491633
run_test "No DNS record is ever written for it" \
492634
"$(test_persist_domain_never_touches_the_provider; echo $?)"
493635

636+
# What the CA refuses, now that it actually looks the record up.
637+
run_test "A record without policy=wildcard cannot answer a wildcard order" \
638+
"$(test_persist_record_without_wildcard_policy_is_refused; echo $?)"
639+
run_test "Adding policy=wildcard is what makes it answerable" \
640+
"$(test_persist_record_with_wildcard_policy_issues; echo $?)"
641+
run_test "A record naming another account does not authorize" \
642+
"$(test_persist_record_for_another_account_is_refused; echo $?)"
643+
644+
# Gateway operations that change shape for a domain it cannot write.
645+
run_test "SetCaa skips it instead of failing or writing" \
646+
"$(test_set_caa_skips_a_persist_domain; echo $?)"
647+
run_test "Rotation reports the records to republish" \
648+
"$(test_rotation_reports_the_records_to_republish; echo $?)"
649+
494650
# Summary
495651
log_section "Test Summary"
496652
log_info "Passed: $TESTS_PASSED"

0 commit comments

Comments
 (0)