Description
An HTTP request may contain header fields which are intended for the immediate recipient ("hop-by-hop"), and ones intended for all recipients on the chain of a forwarded request ("end-to-end"). For example, the "Keep-Alive" header is a hop-by-hop header, because it applies to a single TCP connection. The "Cache-Control" header, in contrast, is an end-to-end header which should be forwarded by an HTTP proxy.
RFC 2616, section 13.5.1 specified a list of hop-by-hop headers which HTTP proxies should not forward.
RFC 7230, section 6.1 replaces the hardcoded list of hop-by-hop headers with the ability for the originator of a request to specify the hop-by-hop headers in the "Connection" header.
The httputil.ReverseProxy
proxy both understands the obsolete hardcoded list of hop-by-hop headers from RFC 2616 and the Connection
header from RFC 7230. ReverseProxy
will remove all hop-by-hop headers before forwarding a request.
When ReverseProxy forwards a request, it follows the following steps in order:
- Clone the incoming request to produce an outbound request,
outreq
. - Pass
outreq
to theDirector
function, which may modify it. - Remove hop-by-hop headers from
outreq
. - Send
outreq
to the backend.
This can cause headers added by the Director
function to be removed before the request is forwarded to the backend.
For example, if an inbound request contains a Connection: forwarded
header, then any Forwarded
header added by the Director
will not be sent to the backend. This is probably surprising; under some circumstances, it may be a security vulnerability.
The user of httputil.ReverseProxy
can prevent a header from being stripped in this fashion by removing any headers that should not be dropped from the Connection
header. This is cumbersome, and relies on the user knowing that they must do this.
Some proposed solutions:
-
Remove hop-by-hop headers before calling the
Director
function: This breaks any existingDirector
function that examines hop-by-hop headers likeProxy-Authorization
. -
Detect what headers were modified by the
Director
function and avoid removing them: This does not avoid the problem, because the attacker could send the exact header they want to remove. For example, an attacker can sendConnection: x-forwarded-proto X-Forwarded-Proto: https
and after
Director
runs we cannot tell whether it intends theX-Forwarded-Proto
header to stay. -
Only remove RFC 2616 hop-by-hop headers and do not remove headers listed in the
Connection
header: This is a violation of RFC 7230, which states that proxies MUST remove headers listed in theConnection
header.
I do not believe it is possible to fix this problem within the context of the existing ReverseProxy
API. The problem is that the Director
function makes no distinction between the inbound request and the outbound one: We cannot remove hop-by-hop headers before passing a request to Director
, since Director
may need to examine those headers, but removing those headers after Director
runs can strip headers that should be preserved for the next hop.
I propose that we add a new field to ReverseProxy
, superseding Director
, which takes both the inbound and outbound request.
type ReverseProxy struct {
// ModifyRequest is an optional function that modifies the Request
// sent using Transport. The inreq is the unmodified inbound request,
// including including hop-by-hop headers. The outreq is the request
// to be sent, with hop-by-hop headers removed and the `X-Forwarded-For`
// header added.
//
// If ModifyRequest returns an error, ErrorHandler is called
// with its error value. If ErrorHandler is nil, its default
// implementation is used.
ModifyRequest func(inreq, outreq *http.Request) error
}
We do not usually backport new APIs. Since in this case the existing API is inherently insecure, I propose that we backport ModifyRequest and mark Director as deprecated.