Repository navigation
fix(ingress): validate tls-alpn ports, DOMAINS and renewal settings - #136
Merged
Merged
Conversation
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
force-pushed
the
fix/ingress-env-sanitize
branch
from
October 15, 2026 11:00
cde8bea to
9a262de
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Encrypted env is not covered by the attestation, so whoever deploys can set it to anything:
TLSALPN_ADDRESS,TLSALPN_PORTandTLS_TERMINATE_PORTwent raw intohaproxy.cfg. A newline inTLSALPN_PORTcould add afrontendserving/etc/haproxy/certs/<domain>.pem, private key included (reproduced in the 2.6 image).ACME_EMAIL, so../../../../evidences/x@x.comwrote the account key into/evidences.DOMAINSentries reach file paths without going throughsanitize_domain.Other settings failed quietly or late:
RENEW_INTERVAL=12hmade the dns-01 loop sleepless and killed the tls-alpn-01 renewal loop;RENEW_DAYS_BEFORE=30dbroke lego and was ignored by certbot;EVIDENCE_SERVER=True/SET_CAA=Truesilently turned the feature off;DNS_SETUP_TIMEOUT=30mand friends gave a traceback on every pass; a config with no domain or nothing to route to started and closed every connection.What
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_porttakes a name for its error.certman.pydrops its now unreachable invalidRENEW_DAYS_BEFOREbranch.DETERMINISTIC_TLS_KEYis 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,DOMAINSentries with a trailing dot or underscore,EVIDENCE_SERVER=True/SET_CAA=yes,DNS_SETUP_TIMEOUT=30m,DNS_SETUP_MODE=Wait,EVIDENCE_PORT=0443, anACME_EMAILwith/, and a config with neitherTARGET_ENDPOINTnorROUTING_MAP.Tests
test_sanitizers.sh(new cases for newline injection, path traversal, leading zeros, email) andtest_certman.pypass;shellcheck -xno new findings.ghcr.io/dstack-tee/dstack-ingress:2.6with these scripts mounted: exits 1 with a clear error for theTLSALPN_PORTnewline payload, theACME_EMAILtraversal,DOMAINS=../../tmp/x,RENEW_INTERVAL=12h, emptyDNS_WEBHOOK_TIMEOUTin webhook mode,EVIDENCE_PORT=0443, no backend,SET_CAA=True. Passes validation: dns-01 with*.example.com+RENEW_DAYS_BEFORE=30, tls-alpn-01 withTLSALPN_ADDRESS=[::1].