Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

caddyhttp: Reject 0-RTT early data in IP matchers and set Early-Data header when proxying #6427

Merged
merged 3 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions listeners.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ type NetworkAddress struct {
// ListenAll calls Listen() for all addresses represented by this struct, i.e. all ports in the range.
// (If the address doesn't use ports or has 1 port only, then only 1 listener will be created.)
// It returns an error if any listener failed to bind, and closes any listeners opened up to that point.
//
// TODO: Experimental API: subject to change or removal.
func (na NetworkAddress) ListenAll(ctx context.Context, config net.ListenConfig) ([]any, error) {
var listeners []any
var err error
Expand Down Expand Up @@ -130,8 +128,6 @@ func (na NetworkAddress) ListenAll(ctx context.Context, config net.ListenConfig)
// Unix sockets will be unlinked before being created, to ensure we can bind to
// it even if the previous program using it exited uncleanly; it will also be
// unlinked upon a graceful exit (or when a new config does not use that socket).
//
// TODO: Experimental API: subject to change or removal.
func (na NetworkAddress) Listen(ctx context.Context, portOffset uint, config net.ListenConfig) (any, error) {
if na.IsUnixNetwork() {
unixSocketsMu.Lock()
Expand Down Expand Up @@ -221,8 +217,6 @@ func (na NetworkAddress) JoinHostPort(offset uint) string {
}

// Expand returns one NetworkAddress for each port in the port range.
//
// This is EXPERIMENTAL and subject to change or removal.
func (na NetworkAddress) Expand() []NetworkAddress {
size := na.PortRangeSize()
addrs := make([]NetworkAddress, size)
Expand Down
6 changes: 6 additions & 0 deletions modules/caddyhttp/ip_matchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ func (m *MatchRemoteIP) Provision(ctx caddy.Context) error {

// Match returns true if r matches m.
func (m MatchRemoteIP) Match(r *http.Request) bool {
if r.TLS != nil && !r.TLS.HandshakeComplete {
return false // if handshake is not finished, we infer 0-RTT that has not verified remote IP; could be spoofed
}
address := r.RemoteAddr
clientIP, zoneID, err := parseIPZoneFromString(address)
if err != nil {
Expand Down Expand Up @@ -228,6 +231,9 @@ func (m *MatchClientIP) Provision(ctx caddy.Context) error {

// Match returns true if r matches m.
func (m MatchClientIP) Match(r *http.Request) bool {
if r.TLS != nil && !r.TLS.HandshakeComplete {
return false // if handshake is not finished, we infer 0-RTT that has not verified remote IP; could be spoofed
}
address := GetVar(r.Context(), ClientIPVarKey).(string)
clientIP, zoneID, err := parseIPZoneFromString(address)
if err != nil {
Expand Down
39 changes: 39 additions & 0 deletions modules/caddyhttp/matchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,22 @@ type (
// "http/2", "http/3", or minimum versions: "http/2+", etc.
MatchProtocol string

// MatchTLS matches HTTP requests based on the underlying
// TLS connection state. If this matcher is specified but
// the request did not come over TLS, it will never match.
// If this matcher is specified but is empty and the request
// did come in over TLS, it will always match.
MatchTLS struct {
// Matches if the TLS handshake has completed. QUIC 0-RTT early
// data may arrive before the handshake completes. Generally, it
// is unsafe to replay these requests if they are not idempotent;
// additionally, the remote IP of early data packets can more
// easily be spoofed. It is conventional to respond with HTTP 425
// Too Early if the request cannot risk being processed in this
// state.
HandshakeComplete *bool `json:"handshake_complete,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not familiar with the architecture here. Does this make 0-RTT request rejection configurable? Is there a compelling use case for that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes it matchable. Server handlers are configured like "if-then" blocks, if the condition is true, then chain in the middleware handler. The matchers are like "if" statements. So this makes 0-RTT/Early Data "matchable" or "detectable". It allows users to do what @MWedl and the RFCs suggest, that is to reply with HTTP 425. Once I add Caddyfile support, it would look like this:

@early tls handshake_incomplete
respond @early 425

I'm trying to decide if I should improve the syntax a bit. Maybe I could rename handshake_incomplete to early_data, as that may be more canonical?

}

// MatchNot matches requests by negating the results of its matcher
// sets. A single "not" matcher takes one or more matcher sets. Each
// matcher set is OR'ed; in other words, if any matcher set returns
Expand Down Expand Up @@ -213,6 +229,7 @@ func init() {
caddy.RegisterModule(MatchHeader{})
caddy.RegisterModule(MatchHeaderRE{})
caddy.RegisterModule(new(MatchProtocol))
caddy.RegisterModule(MatchTLS{})
caddy.RegisterModule(MatchNot{})
}

Expand Down Expand Up @@ -1236,6 +1253,28 @@ func (MatchProtocol) CELLibrary(_ caddy.Context) (cel.Library, error) {
)
}

// CaddyModule returns the Caddy module information.
func (MatchTLS) CaddyModule() caddy.ModuleInfo {
return caddy.ModuleInfo{
ID: "http.matchers.tls",
New: func() caddy.Module { return new(MatchTLS) },
}
}

// Match returns true if r matches m.
func (m MatchTLS) Match(r *http.Request) bool {
if r.TLS == nil {
return false
}
if m.HandshakeComplete != nil {
if (!*m.HandshakeComplete && r.TLS.HandshakeComplete) ||
(*m.HandshakeComplete && !r.TLS.HandshakeComplete) {
return false
}
}
return true
}

// CaddyModule returns the Caddy module information.
func (MatchNot) CaddyModule() caddy.ModuleInfo {
return caddy.ModuleInfo{
Expand Down
8 changes: 8 additions & 0 deletions modules/caddyhttp/replacer.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,16 @@ func addHTTPVarsToReplacer(repl *caddy.Replacer, req *http.Request, w http.Respo
}
return port, true
case "http.request.remote":
if req.TLS != nil && !req.TLS.HandshakeComplete {
// without a complete handshake (QUIC "early data") we can't trust the remote IP address to not be spoofed
return nil, true
}
return req.RemoteAddr, true
case "http.request.remote.host":
if req.TLS != nil && !req.TLS.HandshakeComplete {
// without a complete handshake (QUIC "early data") we can't trust the remote IP address to not be spoofed
return nil, true
}
mohammed90 marked this conversation as resolved.
Show resolved Hide resolved
host, _, err := net.SplitHostPort(req.RemoteAddr)
if err != nil {
// req.RemoteAddr is host:port for tcp and udp sockets and /unix/socket.path
Expand Down
12 changes: 12 additions & 0 deletions modules/caddyhttp/reverseproxy/reverseproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,18 @@ func (h Handler) prepareRequest(req *http.Request, repl *caddy.Replacer) (*http.
req.Header.Set("User-Agent", "")
}

// Indicate if request has been conveyed in early data.
// RFC 8470: "An intermediary that forwards a request prior to the
// completion of the TLS handshake with its client MUST send it with
// the Early-Data header field set to “1” (i.e., it adds it if not
// present in the request). An intermediary MUST use the Early-Data
// header field if the request might have been subject to a replay and
// might already have been forwarded by it or another instance
// (see Section 6.2)."
if req.TLS != nil && !req.TLS.HandshakeComplete {
req.Header.Set("Early-Data", "1")
}

reqUpType := upgradeType(req.Header)
removeConnectionHeaders(req.Header)

Expand Down
Loading