diff --git a/custom-domain/dstack-ingress/DNS_PROVIDERS.md b/custom-domain/dstack-ingress/DNS_PROVIDERS.md index 14d8909..0e8799f 100644 --- a/custom-domain/dstack-ingress/DNS_PROVIDERS.md +++ b/custom-domain/dstack-ingress/DNS_PROVIDERS.md @@ -21,7 +21,7 @@ This guide explains how to configure dstack-ingress to work with different DNS p - `DNS_PROVIDER` - DNS provider to use (`cloudflare`, `linode`, `namecheap`, `route53`). Detected from the provider's credential variable when unset; set it explicitly for `route53` - `ACME_EMAIL` - Contact address for the ACME account (`CERTBOT_EMAIL` still works). Published in the evidence, so leave it unset unless you want one -- `SET_CAA` - Enable CAA record setup: `true` or `false` (default). Except on Cloudflare, this makes a non-wildcard domain's pointer an A record; see the README +- `SET_CAA` - Enable CAA record setup: `true` or `false` (default `false`; `true` with `DELEGATION_ZONE`). Except on Cloudflare, this makes a non-wildcard domain's pointer an A record; see the README - `PORT` - HTTPS port (default: 443) - `TXT_PREFIX` - Prefix for TXT records (default: "_dstack-app-address") @@ -112,7 +112,7 @@ PolicyDocument: **Important Notes for Route53:** - The certbot plugin uses the format `certbot-dns-route53` package - CAA values are merged into the existing record set at the name: only values with the same tag are replaced, so `iodef` and other tags are kept -- Where a CAA record shares the name (`DELEGATION_ZONE`, `SET_CAA=true` on a non-wildcard domain, or any CAA already there), the gateway pointer is an A record to the resolved gateway IP instead of a CNAME, replacing any CNAME or AAAA in the same change. It is refreshed every `RENEW_INTERVAL` +- Where a CAA record shares the name (`SET_CAA=true` under `DELEGATION_ZONE`, where it is the default, or on a non-wildcard domain, or any CAA already there), the gateway pointer is an A record to the resolved gateway IP instead of a CNAME, replacing any CNAME or AAAA in the same change. It is refreshed every `RENEW_INTERVAL` - Wildcard record names are listed by Route53 as `\052.`; they are matched as `*.` - It is essential that the AWS service account used can only assume the limited role. See cloudformation example. diff --git a/custom-domain/dstack-ingress/README.md b/custom-domain/dstack-ingress/README.md index a91dab0..f2136c4 100644 --- a/custom-domain/dstack-ingress/README.md +++ b/custom-domain/dstack-ingress/README.md @@ -175,7 +175,7 @@ otherwise arbitrary — the gateway answers every name under its domain. |----------|---------|-------------| | `PORT` | `443` | HAProxy listen port | | `ROUTING_MAP` | | Multi-domain routing: `domain=host:port` per line. A wildcard entry (`*.example.com=host:port`) matches one label, and exact entries take precedence over it. Unmatched names go to the first entry. It routes only: every name still needs to be in `DOMAIN`/`DOMAINS` to get a certificate | -| `SET_CAA` | `false` | Enable CAA DNS record (dns-01 only; tls-alpn-01 cannot write DNS). Replaces existing values with the same tag; for a wildcard whose base is a zone apex this restricts the whole zone (see [Wildcard Domain Support](#wildcard-domain-support)). Except on Cloudflare, a non-wildcard domain is then published as an A record to the gateway's current address instead of a CNAME, because a CNAME would hide the CAA beside it; the address is refreshed every `RENEW_INTERVAL`. The same happens whenever a CAA record already exists at the name, whoever wrote it, so turning `SET_CAA` off later keeps the A record until you delete the CAA. Linode always uses an A record. With `DELEGATION_ZONE` the CAA is always published, whatever this says | +| `SET_CAA` | `false` | Enable CAA DNS record (dns-01 only; tls-alpn-01 cannot write DNS). Replaces existing values with the same tag; for a wildcard whose base is a zone apex this restricts the whole zone (see [Wildcard Domain Support](#wildcard-domain-support)). Except on Cloudflare, a non-wildcard domain is then published as an A record to the gateway's current address instead of a CNAME, because a CNAME would hide the CAA beside it; the address is refreshed every `RENEW_INTERVAL`. The same happens whenever a CAA record already exists at the name, whoever wrote it, so turning `SET_CAA` off later keeps the A record until you delete the CAA. Linode always uses an A record. Defaults to `true` with `DELEGATION_ZONE`; turning it off there lets anyone who can write the delegation zone get certificates for the domain. Either way, the CAA in force is checked before every issuance and a domain it refuses is not sent to the CA | | `TXT_PREFIX` | `_dstack-app-address` | DNS TXT record prefix | | `ACME_EMAIL` | | ACME contact address, in either mode. `CERTBOT_EMAIL` is the historical name and still works. See [below](#the-acme-contact-address-is-optional-and-public) — it is optional, and published | | `ACME_STAGING` | `false` | Use Let's Encrypt staging, in either mode. `CERTBOT_STAGING` is the historical name and still works | @@ -188,7 +188,7 @@ otherwise arbitrary — the gateway answers every name under its domain. | `DNS_WEBHOOK_URL` | | `DNS_SETUP_MODE=webhook`: endpoint to notify | | `DNS_WEBHOOK_TOKEN` | | Shared secret; the payload is HMAC-SHA256 signed with it | | `DNS_WEBHOOK_TIMEOUT` | `15` | Seconds per webhook POST attempt | -| `DOH_RESOLVERS` | Google + Cloudflare | Comma-separated DoH endpoints used to verify records. `DOH_RESOLVER` is the historical name and still works | +| `DOH_RESOLVERS` | Google + Cloudflare | Comma-separated DoH endpoints used to verify records, and to check CAA before issuing. `DOH_RESOLVER` is the historical name and still works | | `TLSALPN_ADDRESS` | `127.0.0.1` | Address lego's ACME responder binds to, and haproxy forwards `acme-tls/1` connections to | | `TLSALPN_PORT` | `9443` | Loopback port lego's ACME responder binds to | | `TLS_TERMINATE_PORT` | `9444` | Loopback port the TLS frontend moves to in tls-alpn-01 mode | @@ -243,12 +243,12 @@ dstack-ingress publishes what they point at, in the delegation zone: ``` svc.example.com.deleg.example.net A
-svc.example.com.deleg.example.net CAA 0 issue "letsencrypt.org;validationmethods=dns-01;accounturi=…" +svc.example.com.deleg.example.net CAA 0 issue "letsencrypt.org;validationmethods=dns-01;accounturi=…" (SET_CAA, on by default here) _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. Cloudflare answers a CAA query +With `SET_CAA` the gateway pointer and the CAA share a name. Cloudflare answers a CAA query with the CAA beside a CNAME, so the pointer stays a CNAME there. Elsewhere it is an A record: by the DNS rules resolvers follow a CNAME without looking beside it, and Route53 and Linode refuse the pair. The address diff --git a/custom-domain/dstack-ingress/TESTING.md b/custom-domain/dstack-ingress/TESTING.md index c279cff..1d48c18 100644 --- a/custom-domain/dstack-ingress/TESTING.md +++ b/custom-domain/dstack-ingress/TESTING.md @@ -284,8 +284,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 | Proceeds, reporting `no CAA record set; issuance is unrestricted` — see below | +| Delegation, `SET_CAA=false`, CAA in the delegated zone changed to forbid the CA | Blocked before any ACME attempt — proves a CAA there is reachable through the CNAME. With `SET_CAA` on, the next pass replaces it instead | +| dns-01, `SET_CAA=false`, a CAA naming another CA or pinned to another account | Blocked before any ACME attempt with `CAA forbids issuing with this account` and the reason quoted back | +| Delegation, `SET_CAA=false`, no CAA record at all | Proceeds, reporting `no CAA record set; issuance is unrestricted` — see below | | 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 `TXT present but wrong value`, not "missing" | @@ -298,7 +299,7 @@ it is observable rather than merely argued. had to create the CAA record, its absence was the state that mattered: nothing then stopped anyone else who could satisfy the delegated challenge, and the container held no token to create it. Once the container began publishing the -delegated CAA itself, absence stopped meaning that — it means "not published +delegated CAA itself (`SET_CAA`, the default under delegation), absence stopped meaning that — it means "not published yet", and the next pass fixes it — so the gate was removed along with `ALLOW_MISSING_CAA` and dnsguide's `--caa-required`. `check_caa` now follows RFC 8659 for delegation as it always did elsewhere: an absent CAA record set @@ -310,19 +311,20 @@ attempt, which is what proves the published record is reachable through the operator's CNAME. Expect the reason quoted back, e.g. `issue allows 'example.com', not letsencrypt.org`. -Note also that the first pass on a fresh deployment necessarily runs before any -CAA exists: the record names the ACME account, so the account has to exist -first. `process_domain` issues, writes the CAA, and issues again only if the -first attempt failed and had to create the account. +Note also that the CAA record names the ACME account, so `process_domain` +registers the account on its own first. Every dns-01 pass runs the same steps +in the same order — register, publish the records (CAA with `SET_CAA`), check +that the CAA in force lets this account issue, issue once — so even a fresh +deployment's first certificate is issued under the pin. ## Traps -**Start from a clean zone.** Records left over from an earlier run change which -branch executes. A stale CAA record naming a previous ACME account makes dns-01's -first issuance attempt fail and the second succeed — which happens to be the path -where the code was correct, hiding a bug in the ordinary path where the first -attempt succeeds. Delete test records between runs and verify against the -authoritative nameserver, not a cached resolver. +**Start from a clean zone.** Records left over from an earlier run change what a +pass does. A stale CAA record naming a previous ACME account is rewritten with +`SET_CAA=true`, but resolvers may keep the old one for its TTL, so the CAA check +fails a pass or two before issuance; with `SET_CAA=false` it blocks for good. +Delete test records between runs and verify against the authoritative +nameserver, not a cached resolver. **One resolver is not enough to trust either answer.** Public resolvers cache negative answers for the zone's SOA minimum, so a resolver queried before a diff --git a/custom-domain/dstack-ingress/scripts/cert-lib.sh b/custom-domain/dstack-ingress/scripts/cert-lib.sh index 8b80208..8d2557a 100644 --- a/custom-domain/dstack-ingress/scripts/cert-lib.sh +++ b/custom-domain/dstack-ingress/scripts/cert-lib.sh @@ -90,10 +90,9 @@ run_pass() { # "nothing to do", and under `set -e` a bare non-zero would kill the # whole script on every quiet renewal pass. if ( process_domain "$domain" ); then status=0; else status=$?; fi - # A step after issuance can still fail (a CAA write, a timeout that - # kills the client after it saved the certificate). The new certificate - # is on disk by then and the next pass will find it current, so apply - # it now or never. + # Issuance can fail after saving the certificate (a timeout that kills + # the client). The new certificate is on disk by then and the next pass + # will find it current, so apply it now or never. [ "$(cert_checksum "$domain")" = "$before" ] || changed=1 case $status in 0) changed=1 ;; diff --git a/custom-domain/dstack-ingress/scripts/certman.py b/custom-domain/dstack-ingress/scripts/certman.py index cd725d6..3b5165d 100644 --- a/custom-domain/dstack-ingress/scripts/certman.py +++ b/custom-domain/dstack-ingress/scripts/certman.py @@ -706,6 +706,51 @@ def apply_renewal_window(self, domain: str) -> None: return print(f"Renewal window for {domain} set to {days} days before expiry") + def register_account(self, email: str) -> bool: + """Create the ACME account without issuing, so the CAA can name it + before the first order.""" + cmd = self._get_certbot_command() + [ + "register", "--non-interactive", "--agree-tos", "--no-eff-email", + "--server", acme_server()] + # Contactless unless an address is given; see _build_certbot_command. + cmd += ["--email", email] if email else ["--register-unsafely-without-email"] + try: + result = subprocess.run(cmd, capture_output=True, text=True, timeout=120) + except subprocess.TimeoutExpired: + print("ACME account registration timed out", file=sys.stderr) + return False + if result.returncode != 0 or not self.acme_account_exists(): + print(f"ACME account registration failed: {result.stderr.strip()}", file=sys.stderr) + return False + print(f"✓ ACME account registered with {acme_server()}") + return True + + def _lineage_account_current(self, domain: str) -> bool: + """Whether `certbot renew` would find the account the lineage names. + + certbot renews with the account recorded in the lineage and fails once + that one is gone, and register_account runs before every pass, so + "an account exists" no longer tells. Deterministic certificates are + issued outside any lineage, with whatever account is current. A + lineage that records no account is left to certbot rather than + replaced on every pass. + """ + current = account_file() + if current is None: + return False + if deterministic_key.enabled(): + return True + recorded = None + try: + with open(self._renewal_conf_path(domain), encoding="utf-8") as fh: + for line in fh: + match = re.match(r"\s*account\s*=\s*(\S+)", line) + if match: + recorded = match.group(1) + except OSError: + pass + return recorded is None or recorded == os.path.basename(os.path.dirname(current)) + def acme_account_exists(self) -> bool: """Check if an ACME account exists for the current server.""" return account_file() is not None @@ -731,14 +776,14 @@ def run_action( # certbot always records the server; a lineage without one has # a broken renewal config, which re-issuing would not repair. server = acme_server() - if server == acme_server() and self.acme_account_exists(): + if server == acme_server() and self._lineage_account_current(domain): success, renewed = self.renew_certificate(domain, email) return success, renewed # Only generate evidence if actually renewed if server != acme_server(): print(f"Certificate for {domain} came from {server or 'an unknown ACME server'}, " f"replacing it from {acme_server()}") else: - print(f"Certificate exists for {domain} but the ACME account is missing, " + print(f"Certificate for {domain} belongs to an ACME account that is gone, " f"re-obtaining") success = self.obtain_certificate(domain, email, replace=True) return success, success @@ -758,7 +803,7 @@ def main(): ) parser.add_argument( "action", choices=["obtain", "renew", "auto", "setup", "cert-dir", - "account-file"], + "account-file", "register"], help="Action to perform" ) parser.add_argument("--domain", help="Domain name") @@ -775,6 +820,8 @@ def main(): sys.exit(1) print(path) return + if args.action == "register" and account_file(): + return # every pass asks; skip loading the provider SDKs if args.action == "cert-dir": if not args.domain: parser.error("--domain is required") @@ -793,6 +840,15 @@ def main(): print(f"Setup completed for {manager.provider_type} provider") return + # The contact address is optional; see _build_certbot_command. + if not args.email: + args.email = os.environ.get("ACME_EMAIL") or os.environ.get( + "CERTBOT_EMAIL", "" + ) + + if args.action == "register": + sys.exit(0 if manager.register_account(args.email) else 1) + # Domain is required for certificate operations if not args.domain: print( @@ -801,12 +857,6 @@ def main(): ) sys.exit(1) - # The contact address is optional; see _build_certbot_command. - if not args.email: - args.email = os.environ.get("ACME_EMAIL") or os.environ.get( - "CERTBOT_EMAIL", "" - ) - success, needs_evidence = manager.run_action( args.domain, args.email, args.action ) diff --git a/custom-domain/dstack-ingress/scripts/dns01.sh b/custom-domain/dstack-ingress/scripts/dns01.sh index c3e35b5..86adff8 100644 --- a/custom-domain/dstack-ingress/scripts/dns01.sh +++ b/custom-domain/dstack-ingress/scripts/dns01.sh @@ -22,10 +22,16 @@ if [ -n "$DELEGATION_ZONE" ] && [ -n "${DELEGATION_PROPAGATION_SECONDS:-}" ]; th export DELEGATION_PROPAGATION_SECONDS fi -SET_CAA=${SET_CAA:-false} +# Whether this container publishes the CAA record that pins issuance to its +# ACME account. On by default under delegation: there, everyone else who can +# write the delegation zone can also answer the delegated challenge. +if [ -n "$DELEGATION_ZONE" ]; then SET_CAA=${SET_CAA:-true}; else SET_CAA=${SET_CAA:-false}; fi if ! SET_CAA=$(sanitize_boolean "$SET_CAA" "SET_CAA"); then exit 1 fi +if [ -n "$DELEGATION_ZONE" ] && [ "$SET_CAA" = "false" ]; then + echo "Warning: SET_CAA=false with DELEGATION_ZONE: anyone who can write $DELEGATION_ZONE can get certificates for these domains" >&2 +fi # ACME_EMAIL / ACME_STAGING are normalised by entrypoint.sh and exported. The # contact address is optional; certman.py asks for a contactless account when @@ -36,6 +42,7 @@ cert_privkey_path() { echo "$(certman.py cert-dir --domain "$1")/privkey.pem"; # certman.py decides which ACME server is in use (ACME_STAGING, # ACME_DIRECTORY_URL), and so where its account lives. acme_account_file() { certman.py account-file; } +acme_account_uri() { jq -j '.uri' "$(acme_account_file)"; } setup_py_env() { if [ ! -d /opt/app-venv ]; then @@ -97,13 +104,10 @@ EOF } set_alias_record() { - local domain="$1" with_caa=() - # A plain name gets its CAA beside this record. - if [ "$SET_CAA" = "true" ] && [[ "$domain" != \*.* ]]; then - with_caa=(--with-caa) - fi + local domain="$1" echo "Setting alias record for $domain" - dnsman.py set_alias "${with_caa[@]}" \ + # shellcheck disable=SC2046 # one flag or none + dnsman.py set_alias $(with_caa_flag "$domain") \ --domain "$domain" \ --content "$GATEWAY_DOMAIN" @@ -138,7 +142,7 @@ set_txt_record() { # 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 form of pointer +# With SET_CAA the gateway pointer and the CAA share a name. Which form of pointer # lets the CAA take effect is a provider property, so tell the provider and let # it pick (an address record unless its servers answer CAA beside a CNAME). delegated_name() { @@ -150,32 +154,13 @@ delegation_publish() { local domain="$1" target txt_name target=$(delegated_name "$domain") - dnsman.py set_alias --with-caa --domain "$target" --content "$GATEWAY_DOMAIN" || return 1 + # shellcheck disable=SC2046 # one flag or none + dnsman.py set_alias $(with_caa_flag "$domain") --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 (beside `issue`) 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 tag - account_file=$(acme_account_file) || return 1 - account_uri=$(jq -j '.uri' "$account_file") - for tag in $(caa_tags_for "$domain"); do - dnsman.py set_caa \ - --domain "$(delegated_name "$domain")" \ - --caa-tag "$tag" \ - --caa-value "letsencrypt.org;validationmethods=dns-01;accounturi=$account_uri" || - return 1 - done -} - # 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 @@ -214,81 +199,97 @@ is_zone_apex() { >/dev/null } -set_caa_record() { - local domain="$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 - - if [ "$SET_CAA" != "true" ]; then - echo "Skipping CAA record setup" - return - fi - - local ACCOUNT_URI - local account_file - - if ! account_file=$(acme_account_file); then - echo "Warning: Cannot set CAA record - account file not found" - echo "This is not critical - certificates can still be issued without CAA records" - return - fi +# --- CAA ------------------------------------------------------------------- +# +# The same steps in both dns-01 setups: publish the record (SET_CAA), then check +# that the CAA in force lets this account issue, then issue once. The account +# is registered before any of it, so the record can name it and the check can +# compare against it. + +# Where the CAA goes: the base of a wildcard (RFC 8659), in the delegation zone +# under delegation, behind the operator's CNAME. A wildcard needs the operator +# to alias its base as well, which dnsguide asks for; a base that is a zone apex +# cannot be aliased, so such a domain is not a fit for delegation. +caa_name() { + if [ -n "$DELEGATION_ZONE" ]; then delegated_name "$1"; else echo "${1#\*.}"; fi +} - local caa_domain caa_tag - caa_domain="${domain#\*.}" +# --with-caa when the gateway pointer shares its name with the CAA, so the +# provider can pick a pointer that leaves the CAA visible. +with_caa_flag() { + [ "$SET_CAA" = "true" ] || return 0 + if [ -n "$DELEGATION_ZONE" ] || [[ "$1" != \*.* ]]; then echo --with-caa; fi +} - if [ "$FIRST_PASS" = "true" ] && [[ "$domain" == \*.* ]] && is_zone_apex "$caa_domain"; then - echo "Warning: $caa_domain is a zone apex. The CAA issue record published there for $domain" +publish_caa() { + local domain="$1" name tag + [ "$SET_CAA" = "true" ] || return 0 + name=$(caa_name "$domain") + if [ -z "$DELEGATION_ZONE" ] && [ "$FIRST_PASS" = "true" ] && [[ "$domain" == \*.* ]] && is_zone_apex "$name"; then + echo "Warning: $name is a zone apex. The CAA issue record published there for $domain" echo "replaces every existing issue value and governs every name in the zone that has no CAA" echo "record of its own, so other CAs and ACME accounts can no longer issue for them." echo "Give those names their own CAA records, or set SET_CAA=false." fi - - ACCOUNT_URI=$(jq -j '.uri' "$account_file") - for caa_tag in $(caa_tags_for "$domain"); do - echo "Adding CAA record ($caa_tag) for $caa_domain, accounturi=$ACCOUNT_URI" - if ! dnsman.py set_caa \ - --domain "$caa_domain" \ - --caa-tag "$caa_tag" \ - --caa-value "letsencrypt.org;validationmethods=dns-01;accounturi=$ACCOUNT_URI"; then - echo "Warning: Failed to set CAA record for $domain" - echo "This is not critical - certificates can still be issued without CAA records" - echo "Consider disabling CAA records by setting SET_CAA=false if this continues to fail" - fi + for tag in $(caa_tags_for "$domain"); do + echo "Setting CAA record ($tag) for $name" + dnsman.py set_caa \ + --domain "$name" \ + --caa-tag "$tag" \ + --caa-value "letsencrypt.org;validationmethods=dns-01;accounturi=$(acme_account_uri)" || + return 1 done } +# Exits 3 when the CAA in force forbids this account to issue for $1; anything +# else leaves the decision to the CA. With SET_CAA the record is our own, just +# written, and resolvers may still cache the one it replaced, so wait for it; +# anyone else's will not change by waiting. +check_caa() { + local timeout=0 + [ "$SET_CAA" = "true" ] && timeout=$DNS_SETUP_TIMEOUT + dnsguide.py --caa-only \ + --domain "$1" \ + --caa-tag "$(caa_tag_for "$1")" \ + --challenge dns-01 \ + --account-uri "$(acme_account_uri)" \ + --timeout "$timeout" +} + process_domain() { - local domain="$1" status retry=false - - 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 + local domain="$1" caa=0 + + if ! certman.py register ${ACME_EMAIL:+--email "$ACME_EMAIL"}; then + echo "Error: could not register the ACME account" >&2 + return 1 + fi + + if [ -n "$DELEGATION_ZONE" ]; then + # Delegation: the operator created the CNAMEs once, before deploying, + # and everything they point at is published here -- first, because + # checking the aliases proves nothing until their targets exist. + delegation_publish "$domain" || { echo "Error: could not publish $domain into $DELEGATION_ZONE" >&2 return 1 - fi + } + else + set_alias_record "$domain" + set_txt_record "$domain" + fi + if ! publish_caa "$domain"; then + echo "Error: could not publish the CAA record for $domain" >&2 + return 1 + fi + + if [ -n "$DELEGATION_ZONE" ]; then + # 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 of them, 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. local want=challenge-cname if [ "$FIRST_PASS" = "true" ]; then want=delegated @@ -297,34 +298,21 @@ process_domain() { echo "Error: required DNS records for $domain are not in place" >&2 return 1 fi - else - set_alias_record "$domain" - set_txt_record "$domain" fi - # The CAA record names our ACME account, so the account has to exist first: - # try once, write CAA, and try again if the first attempt failed after - # creating the account. That is the attempt a CAA record naming an earlier - # account refuses, and set_caa_record has just replaced it. Any other - # failure would only fail again, spending another of Let's Encrypt's five - # failed validations per hostname per hour on every pass. - if [ -n "${DELEGATION_ZONE:-}" ] || [ "$SET_CAA" = "true" ]; then - acme_account_file >/dev/null 2>&1 || retry=true - fi - if issue_certificate "$domain"; then status=0; else status=$?; fi - - set_caa_record "$domain" - - # Never after a success (0) or "nothing to renew" (2): a second attempt - # would issue another certificate whenever renewal is due. Nor when the - # attempt did not get as far as creating the account (the CA unreachable, - # say): then there is no CAA naming it either. - if [ "$retry" = true ] && [ "$status" -ne 0 ] && [ "$status" -ne 2 ] && - acme_account_file >/dev/null 2>&1; then - echo "First certificate attempt failed for $domain, retrying now that the CAA record names the new account" - if issue_certificate "$domain"; then status=0; else status=$?; fi + # A refusal the CA would give anyway costs one of Let's Encrypt's five + # failed validations per hostname per hour; ask DNS first. + check_caa "$domain" || caa=$? + if [ "$caa" -eq 3 ]; then + if [ "$SET_CAA" = "true" ]; then + echo "Error: the CAA published for $domain was not in force within ${DNS_SETUP_TIMEOUT}s" >&2 + else + echo "Error: CAA forbids issuing $domain with this account. Fix or delete that CAA record, or set SET_CAA=true to have this container publish one" >&2 + fi + return 1 fi - return "$status" + + issue_certificate "$domain" } issue_certificate() { diff --git a/custom-domain/dstack-ingress/scripts/dnsguide.py b/custom-domain/dstack-ingress/scripts/dnsguide.py index d1540ff..b17c7ba 100644 --- a/custom-domain/dstack-ingress/scripts/dnsguide.py +++ b/custom-domain/dstack-ingress/scripts/dnsguide.py @@ -356,6 +356,7 @@ def verify_once( caa_method: str, account_uri: str, resolvers: str, + with_caa: bool, ) -> List[Tuple[str, bool, str]]: results = [] for rec in records: @@ -368,11 +369,44 @@ 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) - results.append((f"CAA {domain}", ok, why)) + if with_caa: + ok, why = check_caa(domain, caa_tag, caa_method, account_uri, resolvers) + results.append((f"CAA {domain}", ok, why)) return results +# Distinct from 1, which is also what an uncaught exception exits with. +CAA_FORBIDS = 3 + + +def caa_only(args: argparse.Namespace) -> int: + """Whether CAA lets this account issue for --domain, for a caller about + to ask the CA. + + CAA_FORBIDS means the CA would refuse, so do not spend a failed validation + on it. Anything else, DNS that cannot be read included, leaves the decision + to the CA. A refusal is retried every --interval until --timeout, for a + record the caller has just replaced and resolvers may still cache. + """ + domain = args.domain[2:] if args.domain.startswith("*.") else args.domain + deadline = time.monotonic() + args.timeout + while True: + try: + ok, why = check_caa(domain, args.caa_tag, args.challenge, args.account_uri, args.resolver) + except ResolveError as exc: + print(f"Warning: could not check CAA for {args.domain}: {exc}", file=sys.stderr, flush=True) + return 0 + if ok: + print(f"CAA {args.domain}: {why}", flush=True) + return 0 + remaining = deadline - time.monotonic() + if remaining <= 0: + print(f"Error: {why}", file=sys.stderr, flush=True) + return CAA_FORBIDS + print(f"[caa-check] WAIT {args.domain}: {why}", flush=True) + time.sleep(min(args.interval, max(1, int(remaining)))) + + # `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. @@ -513,7 +547,15 @@ def main() -> int: help="comma-separated subset of records to handle " "(cname,txt,caa,challenge-cname,delegated)", ) + parser.add_argument( + "--caa-only", + action="store_true", + help="only check that CAA permits --challenge for --account-uri, " + f"waiting up to --timeout; exit {CAA_FORBIDS} if it forbids issuance", + ) args = parser.parse_args() + if args.caa_only: + return caa_only(args) missing = [n for n, v in (("--domain", args.domain), ("--alias-target", args.alias_target), @@ -551,7 +593,7 @@ def main() -> int: try: results = verify_once( records, args.domain.lstrip("*."), args.caa_tag, args.challenge, - args.account_uri, args.resolver, + args.account_uri, args.resolver, "caa" in args.include.split(","), ) except ResolveError as exc: print(f"[dns-check #{attempt}] resolver problem: {exc}", flush=True) diff --git a/custom-domain/dstack-ingress/scripts/tests/test_certman.py b/custom-domain/dstack-ingress/scripts/tests/test_certman.py index ba07e3f..e46c627 100644 --- a/custom-domain/dstack-ingress/scripts/tests/test_certman.py +++ b/custom-domain/dstack-ingress/scripts/tests/test_certman.py @@ -275,18 +275,24 @@ def test_directory_url_applies_to_every_command(self): os.environ["ACME_DIRECTORY_URL"] = "https://pebble:14000/dir" self.assertEqual(self.servers(), {"https://pebble:14000/dir"}) - def run_auto(self, lineage_server, account=True): + def run_auto(self, lineage_server, lineage_account="abc", account="abc"): tmp = tempfile.mkdtemp() self.addCleanup(shutil.rmtree, tmp, ignore_errors=True) conf = os.path.join(tmp, "a.example.com.conf") with open(conf, "w", encoding="utf-8") as fh: + fh.write("version = 5.8.0\n[renewalparams]\n") if lineage_server: - fh.write(f"version = 5.8.0\n[renewalparams]\nserver = {lineage_server}\n") + fh.write(f"server = {lineage_server}\n") + if lineage_account: + fh.write(f"account = {lineage_account}\n") + account_path = os.path.join(tmp, "accounts", account, "regr.json") if account else None + saved = certman.account_file + certman.account_file = lambda: account_path + self.addCleanup(setattr, certman, "account_file", saved) calls = [] mgr = object.__new__(certman.CertManager) mgr._renewal_conf_path = lambda domain: conf # noqa: SLF001 mgr.certificate_exists = lambda domain: True - mgr.acme_account_exists = lambda: account mgr.renew_certificate = lambda d, e: calls.append("renew") or (True, False) mgr.obtain_certificate = lambda d, e, replace=False: calls.append( ("obtain", replace)) or True @@ -304,11 +310,15 @@ def test_production_to_staging_replaces_the_certificate(self): self.assertEqual(self.run_auto(certman.ACME_PROD), [("obtain", True)]) def test_missing_account_replaces_the_certificate(self): - self.assertEqual(self.run_auto(certman.ACME_PROD, account=False), + self.assertEqual(self.run_auto(certman.ACME_PROD, account=None), + [("obtain", True)]) + + def test_replaced_account_replaces_the_certificate(self): + self.assertEqual(self.run_auto(certman.ACME_PROD, account="new"), [("obtain", True)]) def test_unknown_server_does_not_replace_every_pass(self): - self.assertEqual(self.run_auto(None), ["renew"]) + self.assertEqual(self.run_auto(None, lineage_account=None), ["renew"]) class RenewRandomSleepTest(EnvTestCase): diff --git a/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py b/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py index 15cf558..f515ad4 100644 --- a/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py +++ b/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py @@ -4,9 +4,13 @@ Run: python3 scripts/tests/test_dnsguide.py """ +import argparse +import contextlib +import io import os import sys import unittest +from unittest import mock sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "..")) @@ -163,6 +167,72 @@ def test_issuewild_overrides_issue_for_a_wildcard(self): self.assertFalse(ok) +class TestCaaOnly(unittest.TestCase): + """--caa-only: the check dns-01 runs before asking the CA.""" + + ACCOUNT = "https://acme-v02.api.letsencrypt.org/acme/acct/1" + + def _run(self, query, domain="svc.example.com", tag="issue", timeout=0): + saved = dnsguide.query_union, dnsguide.time.sleep + dnsguide.query_union = query + dnsguide.time.sleep = lambda s: None + self.addCleanup(lambda: (setattr(dnsguide, "query_union", saved[0]), + setattr(dnsguide.time, "sleep", saved[1]))) + args = argparse.Namespace(domain=domain, caa_tag=tag, challenge="dns-01", + account_uri=self.ACCOUNT, resolver="", + timeout=timeout, interval=1) + with contextlib.redirect_stderr(io.StringIO()), contextlib.redirect_stdout(io.StringIO()): + return dnsguide.caa_only(args) + + def test_pinned_to_another_account_fails(self): + other = '0 issue "letsencrypt.org;accounturi=https://acme-v02.api.letsencrypt.org/acme/acct/2"' + self.assertEqual(self._run(lambda n, rr, r: [other] if n == "svc.example.com" else []), + dnsguide.CAA_FORBIDS) + + def test_inherited_other_ca_fails_a_wildcard(self): + query = lambda n, rr, r: ['0 issue "digicert.com"'] if n == "example.com" else [] + self.assertEqual(self._run(query, domain="*.svc.example.com", tag="issuewild"), + dnsguide.CAA_FORBIDS) + + def test_waits_for_a_replaced_record(self): + old = '0 issue "letsencrypt.org;accounturi=https://acme-v02.api.letsencrypt.org/acme/acct/2"' + answers = iter([[old], [old], []]) + query = lambda n, rr, r: next(answers) if n == "svc.example.com" else [] + self.assertEqual(self._run(query, timeout=60), 0) + + def test_no_caa_passes(self): + self.assertEqual(self._run(lambda n, rr, r: []), 0) + + def test_unreadable_dns_does_not_block(self): + def broken(name, rr, resolvers): + raise dnsguide.ResolveError("all resolvers failed") + self.assertEqual(self._run(broken), 0) + + +class TestMainChecksCaaOnlyWhenAsked(unittest.TestCase): + """The record check runs CAA only when `caa` is included.""" + + def _main(self, *extra): + asked = [] + union = lambda name, rr, r: asked.append(rr) or (["gw.example."] if rr == dnsguide.RR_CNAME else []) + per_resolver = lambda name, rr, r: [("r", ['"x:443"'])] + argv = ["dnsguide.py", "--domain", "a.example.com", "--alias-target", "gw.example", + "--txt-name", "_dstack-app-address.a.example.com", "--txt-value", "x:443", + "--mode", "wait", "--timeout", "0", *extra] + with mock.patch.object(sys, "argv", argv), \ + mock.patch.object(dnsguide, "query_union", union), \ + mock.patch.object(dnsguide, "query_per_resolver", per_resolver), \ + contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()): + code = dnsguide.main() + return code, dnsguide.RR_CAA in asked + + def test_default_include_checks_caa(self): + self.assertEqual(self._main(), (0, True)) + + def test_without_caa_it_is_not_checked(self): + self.assertEqual(self._main("--include", "cname,txt"), (0, False)) + + class TestDelegationRecord(unittest.TestCase): """The _acme-challenge CNAME that makes challenge delegation work."""