Skip to content

fix(ingress): validate tls-alpn ports, DOMAINS and renewal settings - #136

Merged
kvinwang merged 1 commit into
mainfrom
fix/ingress-env-sanitize
Oct 15, 2026
Merged

kvinwang merged 1 commit into
mainfrom
fix/ingress-env-sanitize

Conversation

@kvinwang

@kvinwang kvinwang commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Why

Encrypted env is not covered by the attestation, so whoever deploys can set it to anything:

  • TLSALPN_ADDRESS, TLSALPN_PORT and TLS_TERMINATE_PORT went raw into haproxy.cfg. A newline in TLSALPN_PORT could add a frontend serving /etc/haproxy/certs/<domain>.pem, private key included (reproduced in the 2.6 image).
  • lego builds the account key path from ACME_EMAIL, so ../../../../evidences/x@x.com wrote the account key into /evidences.
  • DOMAINS entries reach file paths without going through sanitize_domain.

Other settings failed quietly or late: RENEW_INTERVAL=12h made the dns-01 loop sleepless and killed the tls-alpn-01 renewal loop; RENEW_DAYS_BEFORE=30d broke lego and was ignored by certbot; EVIDENCE_SERVER=True / SET_CAA=True silently turned the feature off; DNS_SETUP_TIMEOUT=30m and friends gave a traceback on every pass; a config with no domain or nothing to route to started and closed every connection.

What

  • New sanitize_host, sanitize_nonnegative_integer, sanitize_email (only rejects what is unsafe as a path component: not exactly one @ with text on both sides, /, whitespace/control characters, a leading .); sanitize_port takes a name for its error.
  • Startup checks for the settings above; ports and integers with leading zeros are rejected rather than read as octal.
  • certman.py drops its now unreachable invalid RENEW_DAYS_BEFORE branch.
  • DETERMINISTIC_TLS_KEY is left alone: it is already case-insensitive.

Upgrade note

Values that used to be tolerated now stop startup with an error, e.g. RENEW_DAYS_BEFORE=30d, DOMAINS entries with a trailing dot or underscore, EVIDENCE_SERVER=True / SET_CAA=yes, DNS_SETUP_TIMEOUT=30m, DNS_SETUP_MODE=Wait, EVIDENCE_PORT=0443, an ACME_EMAIL with /, and a config with neither TARGET_ENDPOINT nor ROUTING_MAP.

Tests

  • test_sanitizers.sh (new cases for newline injection, path traversal, leading zeros, email) and test_certman.py pass; shellcheck -x no new findings.
  • ghcr.io/dstack-tee/dstack-ingress:2.6 with these scripts mounted: exits 1 with a clear error for the TLSALPN_PORT newline payload, the ACME_EMAIL traversal, DOMAINS=../../tmp/x, RENEW_INTERVAL=12h, empty DNS_WEBHOOK_TIMEOUT in webhook mode, EVIDENCE_PORT=0443, no backend, SET_CAA=True. Passes validation: dns-01 with *.example.com + RENEW_DAYS_BEFORE=30, tls-alpn-01 with TLSALPN_ADDRESS=[::1].

Encrypted env is not covered by the attestation, so whoever deploys can
set it to anything:
- TLSALPN_ADDRESS, TLSALPN_PORT and TLS_TERMINATE_PORT went raw into
  haproxy.cfg; a newline in TLSALPN_PORT could add a frontend serving
  /etc/haproxy/certs/<domain>.pem, private key included.
- lego builds the account key path from ACME_EMAIL, so
  ../../../../evidences/x@x.com wrote the account key into /evidences.
- DOMAINS entries reach file paths without going through sanitize_domain.

Also reject at startup what used to fail quietly later: a non-integer
RENEW_INTERVAL (a sleepless loop under dns-01, a dead renewal loop under
tls-alpn-01), RENEW_DAYS_BEFORE, DNS_SETUP_TIMEOUT/INTERVAL/MODE,
DNS_WEBHOOK_TIMEOUT, DNS_SETTLE_SECONDS, DELEGATION_PROPAGATION_SECONDS,
EVIDENCE_SERVER/SET_CAA other than true/false, EVIDENCE_PORT out of
range, and a config with no domain or nothing to route to. Ports and
integers with leading zeros are rejected rather than read as octal.

certman.py drops its now unreachable invalid RENEW_DAYS_BEFORE branch.

Signed-off-by: Kevin Wang <wy721@qq.com>
@kvinwang
kvinwang force-pushed the fix/ingress-env-sanitize branch from cde8bea to 9a262de Compare October 15, 2026 11:00
@kvinwang
kvinwang merged commit a3410a0 into main Oct 15, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant