aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorFrancis Lavoie <[email protected]>2022-03-06 18:51:55 -0500
committerGitHub <[email protected]>2022-03-06 18:51:55 -0500
commitc50094fc9d34099efd705700e6d2efa2fa065412 (patch)
treeffa916e82f197d0d067d7cbc01c1a4f4e703b55f
parentd058dee11d7cfcf0b711f0378d10c9e5cabc8982 (diff)
downloadcaddy-c50094fc9d34099efd705700e6d2efa2fa065412.tar.gz
caddy-c50094fc9d34099efd705700e6d2efa2fa065412.zip
reverseproxy: Implement trusted proxies for `X-Forwarded-*` headers (#4507)
-rw-r--r--caddytest/integration/caddyfile_adapt/reverse_proxy_trusted_proxies.txt56
-rw-r--r--modules/caddyhttp/reverseproxy/caddyfile.go23
-rw-r--r--modules/caddyhttp/reverseproxy/reverseproxy.go187
3 files changed, 243 insertions, 23 deletions
diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_trusted_proxies.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_trusted_proxies.txt
new file mode 100644
index 000000000..5e112847f
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_trusted_proxies.txt
@@ -0,0 +1,56 @@
+:8884
+
+reverse_proxy 127.0.0.1:65535 {
+ trusted_proxies 127.0.0.1
+}
+
+reverse_proxy 127.0.0.1:65535 {
+ trusted_proxies private_ranges
+}
+----------
+{
+ "apps": {
+ "http": {
+ "servers": {
+ "srv0": {
+ "listen": [
+ ":8884"
+ ],
+ "routes": [
+ {
+ "handle": [
+ {
+ "handler": "reverse_proxy",
+ "trusted_proxies": [
+ "127.0.0.1"
+ ],
+ "upstreams": [
+ {
+ "dial": "127.0.0.1:65535"
+ }
+ ]
+ },
+ {
+ "handler": "reverse_proxy",
+ "trusted_proxies": [
+ "192.168.0.0/16",
+ "172.16.0.0/12",
+ "10.0.0.0/8",
+ "127.0.0.1/8",
+ "fd00::/8",
+ "::1"
+ ],
+ "upstreams": [
+ {
+ "dial": "127.0.0.1:65535"
+ }
+ ]
+ }
+ ]
+ }
+ ]
+ }
+ }
+ }
+ }
+}
diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go
index a1fbeb32c..e1272375d 100644
--- a/modules/caddyhttp/reverseproxy/caddyfile.go
+++ b/modules/caddyhttp/reverseproxy/caddyfile.go
@@ -82,6 +82,7 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
// buffer_requests
//
// # header manipulation
+// trusted_proxies [private_ranges] <ranges...>
// header_up [+|-]<field> [<value|regexp> [<replacement>]]
// header_down [+|-]<field> [<value|regexp> [<replacement>]]
//
@@ -485,6 +486,22 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
}
h.MaxBufferSize = int64(size)
+ case "trusted_proxies":
+ for d.NextArg() {
+ if d.Val() == "private_ranges" {
+ h.TrustedProxies = append(h.TrustedProxies, []string{
+ "192.168.0.0/16",
+ "172.16.0.0/12",
+ "10.0.0.0/8",
+ "127.0.0.1/8",
+ "fd00::/8",
+ "::1",
+ }...)
+ continue
+ }
+ h.TrustedProxies = append(h.TrustedProxies, d.Val())
+ }
+
case "header_up":
var err error
@@ -504,9 +521,15 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
if strings.EqualFold(args[0], "host") && (args[1] == "{hostport}" || args[1] == "{http.request.hostport}") {
log.Printf("[WARNING] Unnecessary header_up ('Host' field): the reverse proxy's default behavior is to pass headers to the upstream")
}
+ if strings.EqualFold(args[0], "x-forwarded-for") && (args[1] == "{remote}" || args[1] == "{http.request.remote}" || args[1] == "{remote_host}" || args[1] == "{http.request.remote.host}") {
+ log.Printf("[WARNING] Unnecessary header_up ('X-Forwarded-For' field): the reverse proxy's default behavior is to pass headers to the upstream")
+ }
if strings.EqualFold(args[0], "x-forwarded-proto") && (args[1] == "{scheme}" || args[1] == "{http.request.scheme}") {
log.Printf("[WARNING] Unnecessary header_up ('X-Forwarded-Proto' field): the reverse proxy's default behavior is to pass headers to the upstream")
}
+ if strings.EqualFold(args[0], "x-forwarded-host") && (args[1] == "{host}" || args[1] == "{http.request.host}" || args[1] == "{hostport}" || args[1] == "{http.request.hostport}") {
+ log.Printf("[WARNING] Unnecessary header_up ('X-Forwarded-Host' field): the reverse proxy's default behavior is to pass headers to the upstream")
+ }
err = headers.CaddyfileHeaderOp(h.Headers.Request, args[0], args[1], "")
case 3:
err = headers.CaddyfileHeaderOp(h.Headers.Request, args[0], args[1], args[2])
diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go
index 53911c86c..a5bdc3104 100644
--- a/modules/caddyhttp/reverseproxy/reverseproxy.go
+++ b/modules/caddyhttp/reverseproxy/reverseproxy.go
@@ -90,13 +90,20 @@ type Handler struct {
// to the client immediately.
FlushInterval caddy.Duration `json:"flush_interval,omitempty"`
+ // A list of IP ranges (supports CIDR notation) from which
+ // X-Forwarded-* header values should be trusted. By default,
+ // no proxies are trusted, so existing values will be ignored
+ // when setting these headers. If the proxy is trusted, then
+ // existing values will be used when constructing the final
+ // header values.
+ TrustedProxies []string `json:"trusted_proxies,omitempty"`
+
// Headers manipulates headers between Caddy and the backend.
// By default, all headers are passed-thru without changes,
// with the exceptions of special hop-by-hop headers.
//
- // X-Forwarded-For and X-Forwarded-Proto are also set
- // implicitly, but this may change in the future if the official
- // standardized Forwarded header field gains more adoption.
+ // X-Forwarded-For, X-Forwarded-Proto and X-Forwarded-Host
+ // are also set implicitly.
Headers *headers.Handler `json:"headers,omitempty"`
// If true, the entire request body will be read and buffered
@@ -133,6 +140,9 @@ type Handler struct {
Transport http.RoundTripper `json:"-"`
CB CircuitBreaker `json:"-"`
+ // Holds the parsed CIDR ranges from TrustedProxies
+ trustedProxies []*net.IPNet
+
// Holds the named response matchers from the Caddyfile while adapting
responseMatchers map[string]caddyhttp.ResponseMatcher
@@ -192,6 +202,30 @@ func (h *Handler) Provision(ctx caddy.Context) error {
h.CB = mod.(CircuitBreaker)
}
+ // parse trusted proxy CIDRs ahead of time
+ for _, str := range h.TrustedProxies {
+ if strings.Contains(str, "/") {
+ _, ipNet, err := net.ParseCIDR(str)
+ if err != nil {
+ return fmt.Errorf("parsing CIDR expression: %v", err)
+ }
+ h.trustedProxies = append(h.trustedProxies, ipNet)
+ } else {
+ ip := net.ParseIP(str)
+ if ip == nil {
+ return fmt.Errorf("invalid IP address: %s", str)
+ }
+ if ipv4 := ip.To4(); ipv4 != nil {
+ ip = ipv4
+ }
+ mask := len(ip) * 8
+ h.trustedProxies = append(h.trustedProxies, &net.IPNet{
+ IP: ip,
+ Mask: net.CIDRMask(mask, mask),
+ })
+ }
+ }
+
// ensure any embedded headers handler module gets provisioned
// (see https://caddy.community/t/set-cookie-manipulation-in-reverse-proxy/7666?u=matt
// for what happens if we forget to provision it)
@@ -514,32 +548,103 @@ func (h Handler) prepareRequest(req *http.Request) (*http.Request, error) {
req.Header.Set("Upgrade", reqUpType)
}
- if clientIP, _, err := net.SplitHostPort(req.RemoteAddr); err == nil {
- // If we aren't the first proxy retain prior
- // X-Forwarded-For information as a comma+space
- // separated list and fold multiple headers into one.
- prior, ok := req.Header["X-Forwarded-For"]
- omit := ok && prior == nil // Issue 38079: nil now means don't populate the header
- if len(prior) > 0 {
- clientIP = strings.Join(prior, ", ") + ", " + clientIP
- }
- if !omit {
- req.Header.Set("X-Forwarded-For", clientIP)
- }
+ // Add the supported X-Forwarded-* headers
+ err := h.addForwardedHeaders(req)
+ if err != nil {
+ return nil, err
+ }
+
+ return req, nil
+}
+
+// addForwardedHeaders adds the de-facto standard X-Forwarded-*
+// headers to the request before it is sent upstream.
+//
+// These headers are security sensitive, so care is taken to only
+// use existing values for these headers from the incoming request
+// if the client IP is trusted (i.e. coming from a trusted proxy
+// sitting in front of this server). If the request didn't have
+// the headers at all, then they will be added with the values
+// that we can glean from the request.
+func (h Handler) addForwardedHeaders(req *http.Request) error {
+ // Parse the remote IP, ignore the error as non-fatal,
+ // but the remote IP is required to continue, so we
+ // just return early. This should probably never happen
+ // though, unless some other module manipulated the request's
+ // remote address and used an invalid value.
+ clientIP, _, err := net.SplitHostPort(req.RemoteAddr)
+ if err != nil {
+ // Remove the `X-Forwarded-*` headers to avoid upstreams
+ // potentially trusting a header that came from the client
+ req.Header.Del("X-Forwarded-For")
+ req.Header.Del("X-Forwarded-Proto")
+ req.Header.Del("X-Forwarded-Host")
+ return nil
+ }
+
+ // Client IP may contain a zone if IPv6, so we need
+ // to pull that out before parsing the IP
+ if idx := strings.IndexByte(clientIP, '%'); idx >= 0 {
+ clientIP = clientIP[:idx]
+ }
+ ip := net.ParseIP(clientIP)
+ if ip == nil {
+ return fmt.Errorf("invalid client IP address: %s", clientIP)
}
- prior, ok := req.Header["X-Forwarded-Proto"]
- omit := ok && prior == nil
- if len(prior) == 0 && !omit {
- // set X-Forwarded-Proto; many backend apps expect this too
- proto := "https"
- if req.TLS == nil {
- proto = "http"
+ // Check if the client is a trusted proxy
+ trusted := false
+ for _, ipRange := range h.trustedProxies {
+ if ipRange.Contains(ip) {
+ trusted = true
+ break
}
+ }
+
+ // If we aren't the first proxy, and the proxy is trusted,
+ // retain prior X-Forwarded-For information as a comma+space
+ // separated list and fold multiple headers into one.
+ clientXFF := clientIP
+ prior, ok, omit := allHeaderValues(req.Header, "X-Forwarded-For")
+ if trusted && ok && prior != "" {
+ clientXFF = prior + ", " + clientXFF
+ }
+ if !omit {
+ req.Header.Set("X-Forwarded-For", clientXFF)
+ }
+
+ // Set X-Forwarded-Proto; many backend apps expect this,
+ // so that they can properly craft URLs with the right
+ // scheme to match the original request
+ proto := "https"
+ if req.TLS == nil {
+ proto = "http"
+ }
+ prior, ok, omit = lastHeaderValue(req.Header, "X-Forwarded-Proto")
+ if trusted && ok && prior != "" {
+ proto = prior
+ }
+ if !omit {
req.Header.Set("X-Forwarded-Proto", proto)
}
- return req, nil
+ // Set X-Forwarded-Host; often this is redundant because
+ // we pass through the request Host as-is, but in situations
+ // where we proxy over HTTPS, the user may need to override
+ // Host themselves, so it's helpful to send the original too.
+ host, _, err := net.SplitHostPort(req.Host)
+ if err != nil {
+ host = req.Host // OK; there probably was no port
+ }
+ prior, ok, omit = lastHeaderValue(req.Header, "X-Forwarded-Host")
+ if trusted && ok && prior != "" {
+ host = prior
+ }
+ if !omit {
+ req.Header.Set("X-Forwarded-Host", host)
+ }
+
+ return nil
}
// reverseProxy performs a round-trip to the given backend and processes the response with the client.
@@ -868,6 +973,42 @@ func copyHeader(dst, src http.Header) {
}
}
+// allHeaderValues gets all values for a given header field,
+// joined by a comma and space if more than one is set. If the
+// header field is nil, then the omit is true, meaning some
+// earlier logic in the server wanted to prevent this header from
+// getting written at all. If the header is empty, then ok is
+// false. Callers should still check that the value is not empty
+// (the header field may be set but have an empty value).
+func allHeaderValues(h http.Header, field string) (value string, ok bool, omit bool) {
+ values, ok := h[http.CanonicalHeaderKey(field)]
+ if ok && values == nil {
+ return "", true, true
+ }
+ if len(values) == 0 {
+ return "", false, false
+ }
+ return strings.Join(values, ", "), true, false
+}
+
+// lastHeaderValue gets the last value for a given header field
+// if more than one is set. If the header field is nil, then
+// the omit is true, meaning some earlier logic in the server
+// wanted to prevent this header from getting written at all.
+// If the header is empty, then ok is false. Callers should
+// still check that the value is not empty (the header field
+// may be set but have an empty value).
+func lastHeaderValue(h http.Header, field string) (value string, ok bool, omit bool) {
+ values, ok := h[http.CanonicalHeaderKey(field)]
+ if ok && values == nil {
+ return "", true, true
+ }
+ if len(values) == 0 {
+ return "", false, false
+ }
+ return values[len(values)-1], true, false
+}
+
func upgradeType(h http.Header) string {
if !httpguts.HeaderValuesContainsToken(h["Connection"], "Upgrade") {
return ""