Conversation
|
Just transferring Matt's comments to make this a bit easier to read. Regarding aacb430 modules/caddyhttp/autohttps.go (557-559)
modules/caddyhttp/autohttps.go (176)
modules/caddytls/tls.go (729-732)
|
Reuse shared protocol defaults instead of repeating the default HTTP protocol list, unify server name registration to carry ALPN in one experimental API and reuse the TLS default ALPN ordering for HTTPS RR publication
|
I took the narrower route here and pushed the ALPN data through the existing server-name registration path, rather than layering on a second parallel mechanism just for ECH. Basically This should not break existing setups in practice. The API is explicitly experimental and within Caddy the behavioural change is only that ECH publication now gets the effective ALPN data from the same registration flow used to collect names. The default protocol/ALPN lists are also centralised rather than changed so 904f9fd is mainly just deduplication and data-shape cleanup. |
mholt
left a comment
There was a problem hiding this comment.
Thanks, this is making progress; I started a review but realized the last comment here is a big ask. What do you think?
Summary
This adds
alpnto the HTTPS records Caddy already manages for ECH publication.This is a good fit for Caddy because ALPN is derived from Caddy's own configured HTTP protocol support. If Caddy is already authoritative for the HTTPS RR it is publishing, it is also authoritative for the effective ALPN set exposed by that server.
This PR keeps the scope deliberately narrow.
It does not add automatic
ipv4hintoripv6hint. Those do not fit Caddy's information boundary nearly as well because Caddy is often not the source of truth for client-facing IP hints. In many real deployments, Caddy may be behind NAT, behind a CDN, behind a load balancer, behind Kubernetes ingress, using split DNS or serving through separately managed authoritative DNS. In those cases, deriving IP hints from local listener state would be misleading in the best case and wrong in many cases and wrong hints are worse than omitted hints.Concretely, this change:
alpnalongsideechwhen Caddy updates managed HTTPS recordsOne implementation detail worth calling out is that Automatic HTTPS runs before later provisioning fills in the default protocol set so the helper derives the effective protocol set from the raw server config rather than relying on already-normalised
Server.Protocols. That preserves the common default case instead of silently omitting ALPN.Tests
Added focused tests to verify:
h2cechandalpnImplements/closes #7403.
Assistance Disclosure
No AI was used.