diff --git a/custom-domain/dstack-ingress/README.md b/custom-domain/dstack-ingress/README.md index f209c6c..e089b71 100644 --- a/custom-domain/dstack-ingress/README.md +++ b/custom-domain/dstack-ingress/README.md @@ -191,9 +191,8 @@ environment: | `EVIDENCE_SERVER` | `true` | Serve evidence files at `/evidences/` on the TLS port | | `EVIDENCE_PORT` | `80` | Internal port for evidence HTTP server | | `ALPN` | | TLS ALPN protocols (e.g. `h2,http/1.1`). Only set if backends support h2c | -| `ACME_CHALLENGE_ALIAS` | | Delegate the ACME DNS-01 challenge to this zone (see below) so the DNS token needs no access to the served domain's own zone | -| `ACME_CHALLENGE_PROPAGATION_SECONDS` | `30` | Wait after writing the delegated challenge TXT before validation (only with `ACME_CHALLENGE_ALIAS`). Keep well under ~250s — certbot is killed after a 300s per-run timeout | -| `ALLOW_MISSING_CAA` | `false` | In delegation mode, treat an unconfirmed `accounturi` CAA as a warning instead of a blocker. Default fails closed (see below) | +| `DELEGATION_ZONE` | | Zone this container writes into, so the DNS token needs no access to the served domain's own zone (see below) | +| `DELEGATION_PROPAGATION_SECONDS` | `120` | Wait after writing the delegated challenge TXT before validation. Must outlast the record TTL (60s), or a resolver still serving the previous attempt's value fails validation. Keep well under ~250s — certbot is killed after a 300s per-run timeout | For DNS provider credentials, see [DNS_PROVIDERS.md](DNS_PROVIDERS.md). @@ -205,38 +204,66 @@ served name lives under a shared production zone (e.g. `svc.example.com` under `example.com`), that token can edit every record in the zone, which may be more privilege than you want. -Set `ACME_CHALLENGE_ALIAS=` to answer the DNS-01 challenge in a -separate zone that your token controls, so the token never touches the served -domain's zone. In this mode dstack-ingress **only** manages the challenge TXT in -the delegation zone; you set the following records **once, statically**, in the -served domain's production zone (the container prints the exact values on start): +> **Renamed.** This was `ACME_CHALLENGE_ALIAS`, from when the challenge was the +> only thing delegated. The old name is **not** accepted — a deployment still +> setting it silently leaves delegation mode and starts writing to the served +> domain's zone, which is what delegation exists to avoid. Rename it when +> upgrading. + +Set `DELEGATION_ZONE=` to move every name this deployment +needs into a zone your token controls. You create three CNAMEs in the served +domain's zone **once, before deploying**, and then never touch DNS again — not +when the app id moves, not when the ACME account is recreated, not when the +gateway moves: + +``` +svc.example.com CNAME svc.example.com.deleg.example.net +_dstack-app-address.svc.example.com CNAME _dstack-app-address.svc.example.com.deleg.example.net +_acme-challenge.svc.example.com CNAME _acme-challenge.svc.example.com.deleg.example.net +``` + +dstack-ingress publishes what they point at, in the delegation zone: + +``` +svc.example.com.deleg.example.net CNAME +svc.example.com.deleg.example.net CAA 0 issue "letsencrypt.org;validationmethods=dns-01;accounturi=…" +_dstack-app-address.svc.example.com.deleg.example.net TXT ":" +_acme-challenge.svc.example.com.deleg.example.net TXT (transient) +``` + +The gateway pointer and the CAA share a name, which DNS does not allow for a +CNAME. Which form works is a property of the provider — Cloudflare tolerates the +pair, Linode publishes an address record instead — and `set_alias_record` in the +provider layer is where that choice is made. Delegation asks for an alias and +takes what it gets. A provider that needs a form other than CNAME overrides that +method, as Linode does. + +**A wildcard needs a fourth CNAME.** RFC 8659 evaluates `*.app.example.com` at +`app.example.com`, so the base gets its own alias and the container publishes an +`issuewild` CAA behind it: ``` -svc.example.com CNAME -_dstack-app-address.svc.example.com TXT ":" -_acme-challenge.svc.example.com CNAME _acme-challenge.svc.example.com. -svc.example.com CAA 0 issue "letsencrypt.org;validationmethods=dns-01;accounturi=" +*.app.example.com CNAME app.example.com.deleg.example.net +app.example.com CNAME app.example.com.deleg.example.net +_dstack-app-address-wildcard.app.example.com CNAME _dstack-app-address-wildcard.app.example.com.deleg.example.net +_acme-challenge.app.example.com CNAME _acme-challenge.app.example.com.deleg.example.net ``` -> **Security note.** The `accounturi` CAA restricts issuance to this enclave's -> ACME account and is the control that prevents forged certificates. In -> delegation mode dstack-ingress cannot set it for you (no token for the served -> zone), so it prints the record and verifies it. **A CAA that is confirmed -> absent stops issuance** — set `ALLOW_MISSING_CAA=true` to downgrade that to a -> warning. Without this CAA, anyone who can satisfy the delegated challenge -> could obtain a certificate for the domain. -> -> Use a **dedicated** delegation zone and scope the token to only that zone — a -> zone shared with other tenants lets anyone with write access to it complete -> the challenge. `SET_CAA` does not affect delegation mode (this CAA path always -> runs). If a certificate was previously issued with the standard DNS-plugin -> authenticator, delete `/etc/letsencrypt/renewal/.conf` before enabling -> delegation, otherwise `certbot renew` reuses the old plugin (which needs the -> production-zone token this mode avoids). +The base has to be a name you can alias, which rules out a zone apex — an apex +carries SOA and NS records and a CNAME excludes them. `*.example.com` served +straight off the `example.com` zone is therefore not a fit for delegation; a +label down, `*.app.example.com`, is. The records above are checked the same way tls-alpn-01 checks its own: two DoH resolvers, both CAA wire formats, and `DNS_SETUP_MODE` deciding what happens -while they are missing. `wait` (the default) blocks until they appear, so you can +while they are missing. + +Only the `_acme-challenge` CNAME is checked on later passes. Under dns-01 the CA +reads a TXT record and never connects here, so the other two records are about +serving rather than issuance, and a renewal is not blocked on them — a routing +problem can be fixed at any time, an expired certificate cannot. They are still +checked on the first pass, where the point is to tell you whether you created +them correctly. `wait` (the default) blocks until they appear, so you can start the container and create the records afterwards; `print` lists them and continues without checking; `webhook` POSTs them to `DNS_WEBHOOK_URL` for an operator service to create automatically. diff --git a/custom-domain/dstack-ingress/TESTING.md b/custom-domain/dstack-ingress/TESTING.md index e2edc04..c1250e8 100644 --- a/custom-domain/dstack-ingress/TESTING.md +++ b/custom-domain/dstack-ingress/TESTING.md @@ -201,25 +201,28 @@ first issuance. ### Challenge delegation (dns-01) -`ACME_CHALLENGE_ALIAS` answers the DNS-01 challenge in a zone the container's +`DELEGATION_ZONE` answers the DNS-01 challenge in a zone the container's token controls, so the token needs no access to the served domain's own zone. Testing it looks like it needs a second registrar account. It does not. Any name under a zone you already control works as the delegation zone, because the provider resolves a zone by longest-suffix match on the record name. With -`ACME_CHALLENGE_ALIAS=deleg.example.com` and a served domain of +`DELEGATION_ZONE=deleg.example.com` and a served domain of `svc.example.com`, the challenge record is `_acme-challenge.svc.example.com.deleg.example.com`, which lands in `example.com` — the same zone, but through the delegation code path. -Play the operator and create the three static records the container prints: +Play the operator and create the three CNAMEs the container prints: ``` -svc.example.com CNAME -_dstack-app-address.svc.example.com TXT ":443" -_acme-challenge.svc.example.com CNAME _acme-challenge.svc.example.com.deleg.example.com +svc.example.com CNAME svc.example.com.deleg.example.com +_dstack-app-address.svc.example.com CNAME _dstack-app-address.svc.example.com.deleg.example.com +_acme-challenge.svc.example.com CNAME _acme-challenge.svc.example.com.deleg.example.com ``` +Everything they point at is published by the container, so this is the whole of +the operator's job — the app id can change afterwards and no DNS edit follows. + Then start the container with a real provider token and `ACME_STAGING=true`, and watch for: the three records verifying, `Executing (challenge-delegation):` with `--manual` and both hooks, the certificate arriving, and the challenge TXT being @@ -251,7 +254,9 @@ These fail fast and are cheap, so run them on every change: | Scenario | Expected | |---|---| | tls-alpn-01 + a wildcard domain | Refused before any ACME call, citing RFC 8737 | +| Delegation, CAA in the delegated zone changed to forbid the CA | Blocked before any ACME attempt — proves the published CAA is reachable through the CNAME | | Delegation with no CAA record at all | Blocked — unlike normal issuance, where "no CAA" means unrestricted and passes | +| Delegation, later pass, gateway CNAME broken | Renewal proceeds — that record is for serving, and dns-01 does not use it | | A CAA record with `validationmethods=dns-01`, mode tls-alpn-01 | Blocked with the restriction quoted back | | TXT holding the wrong value | Reported as `want , saw `, not "missing" | | An instance ID the gateway does not know | The CA reports a connection error — the gateway will not route to it | diff --git a/custom-domain/dstack-ingress/scripts/acme-dns-alias-hook.sh b/custom-domain/dstack-ingress/scripts/acme-dns-alias-hook.sh index 751ec2b..509b27e 100755 --- a/custom-domain/dstack-ingress/scripts/acme-dns-alias-hook.sh +++ b/custom-domain/dstack-ingress/scripts/acme-dns-alias-hook.sh @@ -3,10 +3,10 @@ # # Instead of writing the `_acme-challenge` TXT into the served domain's own # zone (which requires a DNS token for that zone), this writes it into a -# delegated zone that our token controls (ACME_CHALLENGE_ALIAS). The served +# delegated zone that our token controls (DELEGATION_ZONE). The served # domain's zone only needs one static, operator-managed CNAME: # -# _acme-challenge. CNAME _acme-challenge.. +# _acme-challenge. CNAME _acme-challenge.. # # Let's Encrypt follows that CNAME during validation and reads the TXT from the # delegated zone, so the enclave's token never needs access to the served @@ -18,8 +18,9 @@ set -euo pipefail action="${1:-}" -if [ -z "${ACME_CHALLENGE_ALIAS:-}" ]; then - echo "acme-dns-alias-hook: ACME_CHALLENGE_ALIAS is not set" >&2 +zone="${DELEGATION_ZONE:-}" +if [ -z "$zone" ]; then + echo "acme-dns-alias-hook: DELEGATION_ZONE is not set" >&2 exit 1 fi if [ -z "${CERTBOT_DOMAIN:-}" ]; then @@ -28,7 +29,7 @@ if [ -z "${CERTBOT_DOMAIN:-}" ]; then fi # certbot always passes the base domain (no leading "*.") in CERTBOT_DOMAIN. -record="_acme-challenge.${CERTBOT_DOMAIN}.${ACME_CHALLENGE_ALIAS}" +record="_acme-challenge.${CERTBOT_DOMAIN}.${zone}" case "$action" in auth) @@ -36,9 +37,16 @@ case "$action" in echo "acme-dns-alias-hook: writing challenge TXT to delegated record $record" dnsman.py set_txt --domain "$record" --content "$CERTBOT_VALIDATION" # certbot asks Let's Encrypt to validate immediately after this hook - # returns, so wait for the record to propagate on the delegated zone's - # authoritative servers before returning. - sleep "${ACME_CHALLENGE_PROPAGATION_SECONDS:-30}" + # returns, so wait for the record to propagate before returning. + # + # This has to outlast the record's own TTL, not just the time the write + # takes. dnsman.py publishes TXT with a 60s TTL, so a resolver that saw + # the *previous* challenge value keeps serving it for up to that long -- + # which is exactly the situation on the second of the two issuance + # attempts. At 30s Let's Encrypt's multi-perspective check fails with + # "During secondary validation: Incorrect TXT record found". The dns-01 + # plugin path waits 120s for the same reason. + sleep "${DELEGATION_PROPAGATION_SECONDS:-120}" ;; cleanup) echo "acme-dns-alias-hook: removing challenge TXT $record" diff --git a/custom-domain/dstack-ingress/scripts/certman.py b/custom-domain/dstack-ingress/scripts/certman.py index f9b1ede..d3993ac 100644 --- a/custom-domain/dstack-ingress/scripts/certman.py +++ b/custom-domain/dstack-ingress/scripts/certman.py @@ -279,11 +279,11 @@ def _build_certbot_command(self, action: str, domain: str, email: str) -> List[s """Build certbot command using provider configuration.""" certbot_cmd = self._get_certbot_command() - # Challenge-delegation mode: when ACME_CHALLENGE_ALIAS is set, answer the + # Challenge-delegation mode: when DELEGATION_ZONE is set, answer the # DNS-01 challenge in a delegated zone via a manual hook instead of the # provider's certbot plugin, so our DNS token never needs access to the # served domain's own zone. See scripts/acme-dns-alias-hook.sh. - if os.environ.get("ACME_CHALLENGE_ALIAS", "").strip(): + if os.environ.get("DELEGATION_ZONE", "").strip(): hook = "/scripts/acme-dns-alias-hook.sh" base_cmd = certbot_cmd + [action, "--non-interactive", "-v"] if action == "certonly": diff --git a/custom-domain/dstack-ingress/scripts/dns01.sh b/custom-domain/dstack-ingress/scripts/dns01.sh index 09be268..b8b5d6d 100644 --- a/custom-domain/dstack-ingress/scripts/dns01.sh +++ b/custom-domain/dstack-ingress/scripts/dns01.sh @@ -11,6 +11,10 @@ source /scripts/functions.sh source /scripts/haproxy-lib.sh source /scripts/evidence-lib.sh +# The zone this container writes into, and the switch that turns delegation on. +DELEGATION_ZONE=${DELEGATION_ZONE:-} +export DELEGATION_ZONE + # ACME_EMAIL / ACME_STAGING are normalised by entrypoint.sh and exported. The # contact address is optional; certman.py asks for a contactless account when # none is given. @@ -105,6 +109,49 @@ set_txt_record() { fi } +# --- Full delegation -------------------------------------------------------- +# +# The operator aliases three names into a zone this container can write, once, +# before deploying. Everything those names resolve to is published here, so DNS +# never needs touching again -- not when the app id changes with the compose, +# not when the ACME account is recreated, not when the gateway moves. +# +# The gateway pointer and the CAA end up on one name, which DNS does not allow +# for a CNAME. Which form works is a provider property -- Cloudflare tolerates +# the pair, Linode publishes an address record instead -- and set_alias_record +# is where that knowledge lives, so ask for an alias and let the provider pick. +delegated_name() { + local domain="${1#\*.}" + echo "${domain}.${DELEGATION_ZONE}" +} + +delegation_publish() { + local domain="$1" target txt_name + target=$(delegated_name "$domain") + + dnsman.py set_alias --domain "$target" --content "$GATEWAY_DOMAIN" || return 1 + + txt_name="$(txt_record_name "$domain").${DELEGATION_ZONE}" + dnsman.py set_txt --domain "$txt_name" --content "$(txt_record_value)" || return 1 +} + +# The accounturi CAA goes on the delegated name, beside the gateway pointer. +# +# A wildcard is evaluated at its base (RFC 8659), so it needs the `issuewild` +# tag and it needs the operator to have aliased the base as well -- dnsguide +# asks for that fourth CNAME. A base that is a zone apex cannot be aliased at +# all, so such a domain is not a fit for delegation; the CAA check reports the +# record missing rather than the container pretending to have set it. +delegation_publish_caa() { + local domain="$1" account_file account_uri + account_file=$(get_letsencrypt_account_file) || return 1 + account_uri=$(jq -j '.uri' "$account_file") + dnsman.py set_caa \ + --domain "$(delegated_name "$domain")" \ + --caa-tag "$(caa_tag_for "$domain")" \ + --caa-value "letsencrypt.org;validationmethods=dns-01;accounturi=$account_uri" +} + # Challenge delegation: this container holds no token for the served domain's # zone, so the operator creates those records. dnsguide.py is what prints and # verifies operator-managed records everywhere else in this image, so use it @@ -127,44 +174,23 @@ delegation_guide() { --caa-name "$caa_domain" \ --caa-tag "$caa_tag" \ --challenge dns-01 \ - --challenge-alias "$ACME_CHALLENGE_ALIAS" \ + --delegation-zone "$DELEGATION_ZONE" \ --mode "${DNS_SETUP_MODE:-wait}" \ --include "$include" \ "$@" } -# The accounturi CAA is what stops anyone else who can satisfy the delegated -# challenge from getting a certificate for this name, and we cannot write it, so -# a confirmed-absent record is fatal. ALLOW_MISSING_CAA downgrades it to a -# warning for operators who accept that risk. -delegation_verify_caa() { - local domain="$1" account_file account_uri - if ! account_file=$(get_letsencrypt_account_file); then - echo "ERROR: cannot read the ACME account file to determine the required CAA" >&2 - return 1 - fi - account_uri=$(jq -j '.uri' "$account_file") - # Absent is the state to block on here, not "absent means unrestricted": - # we cannot create this record, so nothing else protects the name. - local gate=(--caa-required) - if [ "${ALLOW_MISSING_CAA:-false}" = "true" ]; then - gate=(--caa-required --caa-advisory) - fi - - delegation_guide "$domain" caa \ - --caa-value "letsencrypt.org;validationmethods=dns-01;accounturi=$account_uri" \ - --account-uri "$account_uri" \ - "${gate[@]}" -} - set_caa_record() { local domain="$1" - # Delegation is handled separately and is deliberately NOT gated by - # SET_CAA: there we cannot write the record, so verifying it is the only - # protection left. - if [ -n "${ACME_CHALLENGE_ALIAS:-}" ]; then - delegation_verify_caa "$domain" || exit 1 + # Delegation is deliberately NOT gated by SET_CAA: this record is the only + # thing stopping anyone else who can satisfy the delegated challenge, so it + # is not optional here. + if [ -n "${DELEGATION_ZONE:-}" ]; then + if ! delegation_publish_caa "$domain"; then + echo "Error: could not publish the CAA record for $domain" >&2 + exit 1 + fi return fi @@ -203,11 +229,31 @@ set_caa_record() { process_domain() { local domain="$1" first status - if [ -n "${ACME_CHALLENGE_ALIAS:-}" ]; then - # Delegation: the operator owns these. Verify them before spending an - # ACME attempt that would fail on records nobody has created yet. CAA - # is checked after the first issuance, once the account URI exists. - if ! delegation_guide "$domain" cname,txt,challenge-cname; then + if [ -n "${DELEGATION_ZONE:-}" ]; then + # Delegation: the operator created three CNAMEs once, before deploying. + # Everything they point at is published here. Which of them block + # depends on the pass. + # + # Only the _acme-challenge alias is needed to issue: under dns-01 the CA + # reads a TXT record and never connects here, so the gateway CNAME and + # the app-address TXT are about serving, not about issuance. Blocking a + # renewal on them would turn a routing problem -- fixable at any time -- + # into an expired certificate, which is not. Renewal wins. + # + # The first pass still checks all three, because that is setup: the + # operator has just been handed a list of records to create and wants to + # know whether they got them right. + # Publish first: the aliases the operator created point at names that + # have to exist before checking them proves anything. + if ! delegation_publish "$domain"; then + echo "Error: could not publish $domain into $DELEGATION_ZONE" >&2 + return 1 + fi + local want=challenge-cname + if [ "$FIRST_PASS" = "true" ]; then + want=delegated + fi + if ! delegation_guide "$domain" "$want"; then echo "Error: required DNS records for $domain are not in place" >&2 return 1 fi @@ -278,6 +324,11 @@ collect_evidence() { evidence_finalize } +# True until the first pass finishes. The first pass is setup and checks more +# than issuance strictly needs; later passes check only what would stop a +# renewal. +FIRST_PASS=true + # Set when certificates have been renewed but not yet applied to haproxy. # Survives across passes, so a failed apply is retried on the next one. APPLY_PENDING=false @@ -319,6 +370,7 @@ run_pass() { echo "[$(date)] Certificate apply incomplete; retrying on the next pass" >&2 fi fi + FIRST_PASS=false [ "$failed" -eq 0 ] } diff --git a/custom-domain/dstack-ingress/scripts/dns_providers/base.py b/custom-domain/dstack-ingress/scripts/dns_providers/base.py index ff5319a..cbce3b6 100644 --- a/custom-domain/dstack-ingress/scripts/dns_providers/base.py +++ b/custom-domain/dstack-ingress/scripts/dns_providers/base.py @@ -160,6 +160,23 @@ def set_a_record( ) return self.create_dns_record(new_record) + def clear_conflicting_alias(self, name: str, keep: "RecordType") -> None: + """Remove address/alias records at `name` other than `keep`. + + DNS forbids a CNAME beside anything else, and the set_* helpers only + look at the type they are about to write, so a name that already holds + the other form blocks the write. That happens whenever the form changes + -- a provider gaining an override, or a deployment moving between + providers -- and the failure looks like a permissions problem rather + than a leftover record. + """ + for record_type in (RecordType.CNAME, RecordType.A, RecordType.AAAA): + if record_type == keep: + continue + for record in self.get_dns_records(name, record_type): + if record.id: + self.delete_dns_record(record.id, name) + def set_alias_record( self, name: str, @@ -167,20 +184,16 @@ def set_alias_record( ttl: int = 60, proxied: bool = False, ) -> bool: - """Set an alias record (delete existing and create new). + """Point `name` at `content`, in whatever form this provider allows. - Creates a CNAME record by default. Some providers may override this - to use A records instead (e.g., Linode to avoid CAA conflicts). + A CNAME by default. Providers override this where a CNAME will not do + -- Linode publishes an address record instead, because it refuses a CAA + beside a CNAME, and delegation needs both on one name. - Args: - name: The record name - content: The alias target (domain name) - ttl: Time to live - proxied: Whether to proxy through provider (if supported) - - Returns: - True if successful, False otherwise + Callers should not second-guess the form: which one works is a property + of the provider, and this is where that knowledge lives. """ + self.clear_conflicting_alias(name, RecordType.CNAME) return self.set_cname_record(name, content, ttl, proxied) def set_cname_record( @@ -257,26 +270,30 @@ def zone_is_resolvable(self, name: str) -> bool: """ return True - def unset_txt_record(self, name: str) -> bool: - """Delete all TXT records for a name. + def unset_records(self, name: str, record_type: RecordType) -> bool: + """Delete every record of one type at a name. - Used to clean up ACME DNS-01 challenge records (e.g. from a certbot - --manual cleanup hook). Missing records are treated as success. + Used to clean up an ACME challenge, and to clear a record type before + publishing a different one at the same name -- the set_* helpers only + look at the type they are about to write, so a leftover CNAME would + otherwise block an A record and vice versa. Args: name: The record name + record_type: Which type to remove Returns: True if all matching records were removed (or none existed). """ - # get_dns_records returns [] both for "this name has no TXT records" and - # for "the zone could not be resolved", so an empty list on its own would - # report a failed cleanup as a success. Ask whether the zone resolves - # before believing the emptiness. - records = self.get_dns_records(name, RecordType.TXT) + # get_dns_records returns [] both for "this name has no such records" + # and for "the zone could not be resolved", so an empty list on its own + # would report a failed cleanup as a success. Ask whether the zone + # resolves before believing the emptiness. + records = self.get_dns_records(name, record_type) if not records and not self.zone_is_resolvable(name): print( - f"Error: cannot delete TXT records for {name}: zone lookup failed", + f"Error: cannot delete {record_type.value} records for {name}: " + f"zone lookup failed", file=sys.stderr, ) return False @@ -284,13 +301,17 @@ def unset_txt_record(self, name: str) -> bool: ok = True for record in records: if not record.id: - print(f"Warning: TXT record for {name} has no id; cannot delete") + print(f"Warning: {record_type.value} record for {name} has no id; cannot delete") ok = False continue if not self.delete_dns_record(record.id, name): ok = False return ok + def unset_txt_record(self, name: str) -> bool: + """Delete all TXT records for a name.""" + return self.unset_records(name, RecordType.TXT) + def set_caa_record( self, name: str, diff --git a/custom-domain/dstack-ingress/scripts/dns_providers/linode.py b/custom-domain/dstack-ingress/scripts/dns_providers/linode.py index a4c307c..e186b27 100644 --- a/custom-domain/dstack-ingress/scripts/dns_providers/linode.py +++ b/custom-domain/dstack-ingress/scripts/dns_providers/linode.py @@ -292,6 +292,7 @@ def set_alias_record( """ # Resolve domain to IP domain = content + self.clear_conflicting_alias(name, RecordType.A) print(f"Trying to resolve: {domain}") ip_address = socket.gethostbyname(domain) print(f"✅ Resolved {domain} to IP: {ip_address}") diff --git a/custom-domain/dstack-ingress/scripts/dnsguide.py b/custom-domain/dstack-ingress/scripts/dnsguide.py index eb5ab6c..9db1278 100644 --- a/custom-domain/dstack-ingress/scripts/dnsguide.py +++ b/custom-domain/dstack-ingress/scripts/dnsguide.py @@ -274,19 +274,12 @@ def check_caa( want_method: str, account_uri: str, resolvers: str, - require_present: bool = False, ) -> Tuple[bool, str]: """Check CAA for domain, walking up to the closest ancestor that has one. An absent CAA record set means every CA may issue (RFC 8659), so "no CAA - anywhere" is normally a pass: we are asking whether *we* may issue, and - nothing forbids it. - - `require_present` inverts that for challenge delegation, where the question - is different. There the record is not a formality but the only thing stopping - someone else who can satisfy the delegated challenge from getting a - certificate for this name, and we hold no token to create it ourselves. An - unrestricted name is exactly the state that must block. + anywhere" is a pass: we are asking whether *we* may issue, and nothing + forbids it. """ labels = _fqdn(domain).split(".") for i in range(len(labels) - 1): @@ -320,11 +313,6 @@ def check_caa( reasons.append(reason) return False, f"CAA at {candidate} does not permit issuance: {'; '.join(reasons)}" - if require_present: - return False, ( - "no CAA record set, so any account that can satisfy the delegated " - "challenge could obtain a certificate for this name" - ) return True, "ok (no CAA record set; issuance is unrestricted)" @@ -357,8 +345,6 @@ def verify_once( caa_method: str, account_uri: str, resolvers: str, - caa_advisory: bool = False, - caa_required: bool = False, ) -> List[Tuple[str, bool, str]]: results = [] for rec in records: @@ -371,18 +357,27 @@ def verify_once( else: ok, why = check_alias(rec, resolvers) results.append((f"CNAME {rec.name}", ok, why)) - ok, why = check_caa( - domain, caa_tag, caa_method, account_uri, resolvers, caa_required - ) - if not ok and caa_advisory: - results.append((f"CAA {domain}", True, f"WARNING: {why} (continuing: CAA is advisory)")) - else: - results.append((f"CAA {domain}", ok, why)) + ok, why = check_caa(domain, caa_tag, caa_method, account_uri, resolvers) + results.append((f"CAA {domain}", ok, why)) return results +# `delegated` is a shape, not a record kind: it replaces the per-kind records +# rather than adding to them. Asking for both would emit two CNAMEs for the same +# name -- one at the gateway, one at the delegation zone -- so refuse instead. +PER_KIND = {"cname", "txt", "caa", "challenge-cname"} + + def build_records(args: argparse.Namespace) -> List[Record]: include = {part.strip() for part in args.include.split(",") if part.strip()} + unknown = include - PER_KIND - {"delegated"} + if unknown: + raise SystemExit(f"unknown --include value(s): {', '.join(sorted(unknown))}") + if "delegated" in include and include & (PER_KIND - {"caa"}): + raise SystemExit( + "--include delegated cannot be combined with cname/txt/challenge-cname: " + "delegated already covers them, and both would claim the same names" + ) records = [] if "cname" in include: records.append( @@ -402,7 +397,43 @@ def build_records(args: argparse.Namespace) -> List[Record]: note="tells the gateway which instance to route this hostname to", ) ) - if "challenge-cname" in include and args.challenge_alias: + if "delegated" in include and args.delegation_zone: + # Full delegation: every name this deployment needs is aliased into a + # zone the container can write, so the operator creates these three once + # and never touches DNS again -- not when the app id changes, not when + # the ACME account changes, not when the gateway moves. + # + # Each is exact: the target holds only what we put there, so there is no + # address to fall back to and a wrong target must be reported as wrong + # rather than as "does not resolve". + base = args.domain[2:] if args.domain.startswith("*.") else args.domain + alias = args.delegation_zone + wanted = [ + (args.domain, "routes traffic for this hostname into the delegated zone"), + (args.txt_name, "lets the container publish the gateway routing target"), + (f"_acme-challenge.{base}", "delegates the ACME challenge"), + ] + if args.domain.startswith("*."): + # RFC 8659 evaluates CAA for *.example.com at example.com, not at the + # wildcard, so the base needs its own alias for the CAA to be + # delegated too. Verified: with this in place the CA followed it and + # honoured an issuewild record in the delegated zone. + # + # The base has to be aliasable, which a zone apex is not (SOA and + # NS live there and a CNAME excludes them), so `*.example.com` + # served straight off its own zone is not a fit for delegation. + wanted.insert(1, (base, "delegates CAA, which a wildcard is evaluated at")) + for name, note in wanted: + records.append( + Record( + type="CNAME", + name=name, + value=f"{name}.{alias}" if name != args.domain else f"{base}.{alias}", + note=note, + exact=True, + ) + ) + if "challenge-cname" in include and args.delegation_zone: # Challenge delegation: the CA follows this CNAME when it looks for # _acme-challenge, so the TXT can live in a zone whose token we hold and # the served zone never needs one. certbot validates at the base name @@ -412,7 +443,7 @@ def build_records(args: argparse.Namespace) -> List[Record]: Record( type="CNAME", name=f"_acme-challenge.{base}", - value=f"_acme-challenge.{base}.{args.challenge_alias}", + value=f"_acme-challenge.{base}.{args.delegation_zone}", note="delegates the ACME challenge to a zone this container can write", exact=True, ) @@ -434,30 +465,19 @@ def build_records(args: argparse.Namespace) -> List[Record]: def main() -> int: parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--domain", required=True) - parser.add_argument("--alias-target", required=True, help="gateway domain") - parser.add_argument("--txt-name", required=True) - parser.add_argument("--txt-value", required=True) + parser.add_argument("--domain", default="") + parser.add_argument("--alias-target", default="", help="gateway domain") + parser.add_argument("--txt-name", default="") + parser.add_argument("--txt-value", default="") parser.add_argument("--caa-name", default="") parser.add_argument("--caa-tag", default="issue", choices=["issue", "issuewild"]) parser.add_argument("--caa-value", default="") parser.add_argument("--account-uri", default="") parser.add_argument("--challenge", default="tls-alpn-01") parser.add_argument( - "--challenge-alias", - default=os.environ.get("ACME_CHALLENGE_ALIAS", ""), - help="delegation zone for the _acme-challenge CNAME (dns-01 delegation)", - ) - parser.add_argument( - "--caa-required", - action="store_true", - help="treat an absent CAA record set as a failure (challenge delegation: " - "the record is the only protection and we cannot create it)", - ) - parser.add_argument( - "--caa-advisory", - action="store_true", - help="report a failing CAA check as a warning instead of blocking", + "--delegation-zone", + default=os.environ.get("DELEGATION_ZONE", ""), + help="zone this deployment is delegated into (dns-01 delegation)", ) parser.add_argument( "--mode", @@ -476,10 +496,17 @@ def main() -> int: "--include", default="cname,txt,caa", help="comma-separated subset of records to handle " - "(cname,txt,caa,challenge-cname)", + "(cname,txt,caa,challenge-cname,delegated)", ) args = parser.parse_args() + missing = [n for n, v in (("--domain", args.domain), + ("--alias-target", args.alias_target), + ("--txt-name", args.txt_name), + ("--txt-value", args.txt_value)) if not v] + if missing: + parser.error(f"{', '.join(missing)} are required") + if not args.caa_name: args.caa_name = args.domain.lstrip("*.") @@ -509,8 +536,7 @@ def main() -> int: try: results = verify_once( records, args.domain.lstrip("*."), args.caa_tag, args.challenge, - args.account_uri, args.resolver, args.caa_advisory, - args.caa_required, + args.account_uri, args.resolver, ) except ResolveError as exc: print(f"[dns-check #{attempt}] resolver problem: {exc}", flush=True) diff --git a/custom-domain/dstack-ingress/scripts/dnsman.py b/custom-domain/dstack-ingress/scripts/dnsman.py index 20dfc36..69915fd 100755 --- a/custom-domain/dstack-ingress/scripts/dnsman.py +++ b/custom-domain/dstack-ingress/scripts/dnsman.py @@ -17,7 +17,7 @@ def main(): choices=["set_cname", "set_alias", "set_txt", "unset_txt", "set_caa"], help="Action to perform", ) - parser.add_argument("--domain", required=True, help="Domain name") + parser.add_argument("--domain", default="", help="Domain name") parser.add_argument("--provider", help="DNS provider (cloudflare, linode)") # Zone ID is now handled internally by each provider parser.add_argument( @@ -30,6 +30,10 @@ def main(): args = parser.parse_args() + if not args.domain: + print("Error: --domain is required", file=sys.stderr) + sys.exit(1) + try: # Create DNS provider instance provider = DNSProviderFactory.create_provider(args.provider) diff --git a/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py b/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py index 8112e74..b694dc7 100644 --- a/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py +++ b/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py @@ -128,7 +128,7 @@ def _args(self, domain, alias): domain=domain, alias_target="gw.example.net", txt_name="_dstack-app-address." + domain.lstrip("*."), txt_value="appid:443", caa_name="", caa_tag="issue", caa_value="", - account_uri="", challenge="dns-01", challenge_alias=alias, + account_uri="", challenge="dns-01", delegation_zone=alias, include="challenge-cname", ) @@ -146,6 +146,29 @@ def test_absent_without_a_delegation_zone(self): self.assertEqual(dnsguide.build_records(self._args("svc.example.com", "")), []) +class TestWildcardDelegation(unittest.TestCase): + """A wildcard is evaluated at its base, so the base needs its own alias.""" + + def _records(self, domain, txt): + import argparse + return dnsguide.build_records(argparse.Namespace( + domain=domain, alias_target="gw", txt_name=txt, txt_value="x", + caa_name="", caa_tag="issue", caa_value="", account_uri="", + challenge="dns-01", delegation_zone="deleg.net", include="delegated")) + + def test_wildcard_aliases_its_base_as_well(self): + names = [r.name for r in self._records( + "*.app.example.com", "_dstack-app-address-wildcard.app.example.com")] + self.assertIn("app.example.com", names) + self.assertEqual(len(names), 4) + + def test_plain_domain_does_not(self): + names = [r.name for r in self._records( + "svc.example.com", "_dstack-app-address.svc.example.com")] + self.assertNotIn("example.com", names) + self.assertEqual(len(names), 3) + + class TestExactCnameCheck(unittest.TestCase): """check_alias falls back to comparing addresses; a delegation target has none.""" @@ -176,48 +199,6 @@ def test_reports_absence_without_claiming_it_does_not_resolve(self): self.assertIn("no CNAME record found", why) -class TestCaaRequiredForDelegation(unittest.TestCase): - """Delegation inverts the usual CAA question. - - Normally we ask "does anything forbid us from issuing", and no CAA at all - means no. Under challenge delegation the record is the only thing stopping - someone else who can satisfy the delegated challenge, and we cannot create - it, so its absence has to block. - """ - - def _no_caa(self): - saved = dnsguide.query_union - dnsguide.query_union = lambda name, rr, r: [] - self.addCleanup(lambda: setattr(dnsguide, "query_union", saved)) - - def test_absent_caa_passes_by_default(self): - self._no_caa() - ok, why = dnsguide.check_caa("app.example.com", "issue", "dns-01", "", "r") - self.assertTrue(ok) - self.assertIn("unrestricted", why) - - def test_absent_caa_blocks_when_required(self): - self._no_caa() - ok, why = dnsguide.check_caa( - "app.example.com", "issue", "dns-01", "", "r", require_present=True - ) - self.assertFalse(ok) - self.assertIn("no CAA record set", why) - - def test_present_and_matching_passes_when_required(self): - saved = dnsguide.query_union - dnsguide.query_union = lambda name, rr, r: ( - ['0 issue "letsencrypt.org;validationmethods=dns-01;accounturi=https://acme/1"'] - if name == "app.example.com" else [] - ) - self.addCleanup(lambda: setattr(dnsguide, "query_union", saved)) - ok, _ = dnsguide.check_caa( - "app.example.com", "issue", "dns-01", "https://acme/1", "r", - require_present=True, - ) - self.assertTrue(ok) - - class TestTxtNormalisation(unittest.TestCase): def test_strips_quotes(self): self.assertEqual(dnsguide._unquote_txt('"abc:443"'), "abc:443") @@ -260,5 +241,25 @@ def test_caa_omitted_without_value(self): self.assertEqual(types, ["CNAME", "TXT"]) +class TestNoShadowedDefinitions(unittest.TestCase): + """A duplicated class or method silently shadows the earlier one. + + Python does not complain, unittest does not notice, and the count still + goes up -- so a merge that lands the same block twice leaves dead tests + that look like passing ones. This caught exactly that. + """ + + def test_every_definition_is_unique(self): + import ast, collections, pathlib + + tree = ast.parse(pathlib.Path(__file__).read_text()) + names = [n.name for n in tree.body if isinstance(n, ast.ClassDef)] + for cls in (n for n in tree.body if isinstance(n, ast.ClassDef)): + names += [f"{cls.name}.{m.name}" for m in cls.body + if isinstance(m, (ast.FunctionDef, ast.AsyncFunctionDef))] + dupes = [n for n, c in collections.Counter(names).items() if c > 1] + self.assertEqual(dupes, [], f"defined more than once: {dupes}") + + if __name__ == "__main__": unittest.main(verbosity=2)