From f6900fcf530e80c921dac8e4f09996cffce7f436 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Fri, 6 May 2022 10:50:26 -0400 Subject: [PATCH] reverseproxy: Support performing pre-check requests (#4739) --- caddyconfig/httpcaddyfile/builtins.go | 16 +- caddyconfig/httpcaddyfile/builtins_test.go | 27 +- caddyconfig/httpcaddyfile/directives.go | 1 + caddyconfig/httpcaddyfile/httptype.go | 1 + .../caddyfile_adapt/forward_auth_authelia.txt | 137 +++++++++ .../caddyfile_adapt/reverse_proxy_options.txt | 34 +-- modules/caddyhttp/reverseproxy/caddyfile.go | 29 +- .../reverseproxy/fastcgi/caddyfile.go | 10 +- .../reverseproxy/forwardauth/caddyfile.go | 269 ++++++++++++++++++ .../caddyhttp/reverseproxy/reverseproxy.go | 56 +++- modules/caddyhttp/rewrite/rewrite.go | 4 +- modules/caddyhttp/rewrite/rewrite_test.go | 2 +- modules/caddyhttp/standard/imports.go | 1 + 13 files changed, 542 insertions(+), 45 deletions(-) create mode 100644 caddytest/integration/caddyfile_adapt/forward_auth_authelia.txt create mode 100644 modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go index 1e7c7016..5c539e2a 100644 --- a/caddyconfig/httpcaddyfile/builtins.go +++ b/caddyconfig/httpcaddyfile/builtins.go @@ -580,12 +580,24 @@ func parseRedir(h Helper) (caddyhttp.MiddlewareHandler, error) { body = fmt.Sprintf(metaRedir, safeTo, safeTo, safeTo, safeTo) code = "302" default: + // Allow placeholders for the code + if strings.HasPrefix(code, "{") { + break + } + // Try to validate as an integer otherwise codeInt, err := strconv.Atoi(code) if err != nil { return nil, h.Errf("Not a supported redir code type or not valid integer: '%s'", code) } - if codeInt < 300 || codeInt > 399 { - return nil, h.Errf("Redir code not in the 3xx range: '%v'", codeInt) + // Sometimes, a 401 with Location header is desirable because + // requests made with XHR will "eat" the 3xx redirect; so if + // the intent was to redirect to an auth page, a 3xx won't + // work. Responding with 401 allows JS code to read the + // Location header and do a window.location redirect manually. + // see https://stackoverflow.com/a/2573589/846934 + // see https://github.com/oauth2-proxy/oauth2-proxy/issues/1522 + if codeInt < 300 || (codeInt > 399 && codeInt != 401) { + return nil, h.Errf("Redir code not in the 3xx range or 401: '%v'", codeInt) } } diff --git a/caddyconfig/httpcaddyfile/builtins_test.go b/caddyconfig/httpcaddyfile/builtins_test.go index 991c649c..bd5e1169 100644 --- a/caddyconfig/httpcaddyfile/builtins_test.go +++ b/caddyconfig/httpcaddyfile/builtins_test.go @@ -148,6 +148,27 @@ func TestRedirDirectiveSyntax(t *testing.T) { }`, expectError: false, }, + { + // this is now allowed so a Location header + // can be written and consumed by JS + // in the case of XHR requests + input: `:8080 { + redir * :8081 401 + }`, + expectError: false, + }, + { + input: `:8080 { + redir * :8081 402 + }`, + expectError: true, + }, + { + input: `:8080 { + redir * :8081 {http.reverse_proxy.status_code} + }`, + expectError: false, + }, { input: `:8080 { redir /old.html /new.html htlm @@ -160,12 +181,6 @@ func TestRedirDirectiveSyntax(t *testing.T) { }`, expectError: true, }, - { - input: `:8080 { - redir * :8081 400 - }`, - expectError: true, - }, { input: `:8080 { redir * :8081 temp diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go index aaa8cb01..164b9126 100644 --- a/caddyconfig/httpcaddyfile/directives.go +++ b/caddyconfig/httpcaddyfile/directives.go @@ -57,6 +57,7 @@ var directiveOrder = []string{ // middleware handlers; some wrap responses "basicauth", + "forward_auth", "request_header", "encode", "push", diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index 242f0b82..9c723dbf 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/caddyconfig/httpcaddyfile/httptype.go @@ -130,6 +130,7 @@ func (st ServerType) Setup(inputServerBlocks []caddyfile.ServerBlock, {regexp.MustCompile(`{path\.([\w-]*)}`), "{http.request.uri.path.$1}"}, {regexp.MustCompile(`{re\.([\w-]*)\.([\w-]*)}`), "{http.regexp.$1.$2}"}, {regexp.MustCompile(`{vars\.([\w-]*)}`), "{http.vars.$1}"}, + {regexp.MustCompile(`{rp\.([\w-\.]*)}`), "{http.reverse_proxy.$1}"}, } for _, sb := range originalServerBlocks { diff --git a/caddytest/integration/caddyfile_adapt/forward_auth_authelia.txt b/caddytest/integration/caddyfile_adapt/forward_auth_authelia.txt new file mode 100644 index 00000000..e7cbb0f5 --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/forward_auth_authelia.txt @@ -0,0 +1,137 @@ +app.example.com { + forward_auth authelia:9091 { + uri /api/verify?rd=https://authelia.example.com + copy_headers Remote-User Remote-Groups Remote-Name Remote-Email + } + + reverse_proxy backend:8080 +} +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "app.example.com" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "handle_response": [ + { + "match": { + "status_code": [ + 2 + ] + }, + "routes": [ + { + "handle": [ + { + "handler": "headers", + "request": { + "set": { + "Remote-Email": [ + "{http.reverse_proxy.header.Remote-Email}" + ], + "Remote-Groups": [ + "{http.reverse_proxy.header.Remote-Groups}" + ], + "Remote-Name": [ + "{http.reverse_proxy.header.Remote-Name}" + ], + "Remote-User": [ + "{http.reverse_proxy.header.Remote-User}" + ] + } + } + } + ] + } + ] + }, + { + "routes": [ + { + "handle": [ + { + "exclude": [ + "Connection", + "Keep-Alive", + "Te", + "Trailers", + "Transfer-Encoding", + "Upgrade" + ], + "handler": "copy_response_headers" + } + ] + }, + { + "handle": [ + { + "handler": "copy_response" + } + ] + } + ] + } + ], + "handler": "reverse_proxy", + "headers": { + "request": { + "set": { + "X-Forwarded-Method": [ + "{http.request.method}" + ], + "X-Forwarded-Uri": [ + "{http.request.uri}" + ] + } + } + }, + "rewrite": { + "method": "GET", + "uri": "/api/verify?rd=https://authelia.example.com" + }, + "upstreams": [ + { + "dial": "authelia:9091" + } + ] + }, + { + "handler": "reverse_proxy", + "upstreams": [ + { + "dial": "backend:8080" + } + ] + } + ] + } + ] + } + ], + "terminal": true + } + ] + } + } + } + } +} \ No newline at end of file diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_options.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_options.txt index e41b9004..fc07698d 100644 --- a/caddytest/integration/caddyfile_adapt/reverse_proxy_options.txt +++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_options.txt @@ -1,11 +1,11 @@ https://example.com { - reverse_proxy /path http://localhost:54321 { - header_up Host {host} - header_up X-Real-IP {remote} - header_up X-Forwarded-For {remote} - header_up X-Forwarded-Port {server_port} - header_up X-Forwarded-Proto "http" + reverse_proxy /path https://localhost:54321 { + header_up Host {upstream_hostport} + header_up Foo bar + + method GET + rewrite /rewritten?uri={uri} buffer_requests @@ -58,24 +58,19 @@ https://example.com { "headers": { "request": { "set": { + "Foo": [ + "bar" + ], "Host": [ - "{http.request.host}" - ], - "X-Forwarded-For": [ - "{http.request.remote}" - ], - "X-Forwarded-Port": [ - "{server_port}" - ], - "X-Forwarded-Proto": [ - "http" - ], - "X-Real-Ip": [ - "{http.request.remote}" + "{http.reverse_proxy.upstream.hostport}" ] } } }, + "rewrite": { + "method": "GET", + "uri": "/rewritten?uri={http.request.uri}" + }, "transport": { "compression": false, "dial_fallback_delay": 5000000000, @@ -96,6 +91,7 @@ https://example.com { ] }, "response_header_timeout": 8000000000, + "tls": {}, "versions": [ "h2c", "2" diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 14de2300..ebea49eb 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -27,6 +27,7 @@ import ( "github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile" "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/caddyserver/caddy/v2/modules/caddyhttp/headers" + "github.com/caddyserver/caddy/v2/modules/caddyhttp/rewrite" "github.com/dustin/go-humanize" ) @@ -85,10 +86,12 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) // buffer_responses // max_buffer_size // -// # header manipulation +// # request manipulation // trusted_proxies [private_ranges] // header_up [+|-] [ []] // header_down [+|-] [ []] +// method +// rewrite // // # round trip // transport { @@ -600,6 +603,30 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return d.Err(err.Error()) } + case "method": + if !d.NextArg() { + return d.ArgErr() + } + if h.Rewrite == nil { + h.Rewrite = &rewrite.Rewrite{} + } + h.Rewrite.Method = d.Val() + if d.NextArg() { + return d.ArgErr() + } + + case "rewrite": + if !d.NextArg() { + return d.ArgErr() + } + if h.Rewrite == nil { + h.Rewrite = &rewrite.Rewrite{} + } + h.Rewrite.URI = d.Val() + if d.NextArg() { + return d.ArgErr() + } + case "transport": if !d.NextArg() { return d.ArgErr() diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go index 1b4cecf4..96b84f21 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go @@ -196,7 +196,15 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error // NOTE: we delete the tokens as we go so that the reverse_proxy // unmarshal doesn't see these subdirectives which it cannot handle for dispenser.Next() { - for dispenser.NextBlock(0) && dispenser.Nesting() == 1 { + for dispenser.NextBlock(0) { + // ignore any sub-subdirectives that might + // have the same name somewhere within + // the reverse_proxy passthrough tokens + if dispenser.Nesting() != 1 { + continue + } + + // parse the php_fastcgi subdirectives switch dispenser.Val() { case "root": if !dispenser.NextArg() { diff --git a/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go b/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go new file mode 100644 index 00000000..1571f09a --- /dev/null +++ b/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go @@ -0,0 +1,269 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package forwardauth + +import ( + "encoding/json" + "net/http" + + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/caddyconfig" + "github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" + "github.com/caddyserver/caddy/v2/modules/caddyhttp/headers" + "github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy" + "github.com/caddyserver/caddy/v2/modules/caddyhttp/rewrite" +) + +func init() { + httpcaddyfile.RegisterDirective("forward_auth", parseCaddyfile) +} + +// parseCaddyfile parses the forward_auth directive, which has the same syntax +// as the reverse_proxy directive (in fact, the reverse_proxy's directive +// Unmarshaler is invoked by this function) but the resulting proxy is specially +// configured for most™️ auth gateways that support forward auth. The typical +// config which looks something like this: +// +// forward_auth auth-gateway:9091 { +// uri /authenticate?redirect=https://auth.example.com +// copy_headers Remote-User Remote-Email +// } +// +// is equivalent to a reverse_proxy directive like this: +// +// reverse_proxy auth-gateway:9091 { +// method GET +// rewrite /authenticate?redirect=https://auth.example.com +// +// header_up X-Forwarded-Method {method} +// header_up X-Forwarded-Uri {uri} +// +// @good status 2xx +// handle_response @good { +// request_header { +// Remote-User {http.reverse_proxy.header.Remote-User} +// Remote-Email {http.reverse_proxy.header.Remote-Email} +// } +// } +// +// handle_response { +// copy_response_headers { +// exclude Connection Keep-Alive Te Trailers Transfer-Encoding Upgrade +// } +// copy_response +// } +// } +// +func parseCaddyfile(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) { + if !h.Next() { + return nil, h.ArgErr() + } + + // if the user specified a matcher token, use that + // matcher in a route that wraps both of our routes; + // either way, strip the matcher token and pass + // the remaining tokens to the unmarshaler so that + // we can gain the rest of the reverse_proxy syntax + userMatcherSet, err := h.ExtractMatcherSet() + if err != nil { + return nil, err + } + + // make a new dispenser from the remaining tokens so that we + // can reset the dispenser back to this point for the + // reverse_proxy unmarshaler to read from it as well + dispenser := h.NewFromNextSegment() + + // create the reverse proxy handler + rpHandler := &reverseproxy.Handler{ + // set up defaults for header_up; reverse_proxy already deals with + // adding the other three X-Forwarded-* headers, but for this flow, + // we want to also send along the incoming method and URI since this + // request will have a rewritten URI and method. + Headers: &headers.Handler{ + Request: &headers.HeaderOps{ + Set: http.Header{ + "X-Forwarded-Method": []string{"{http.request.method}"}, + "X-Forwarded-Uri": []string{"{http.request.uri}"}, + }, + }, + }, + + // we always rewrite the method to GET, which implicitly + // turns off sending the incoming request's body, which + // allows later middleware handlers to consume it + Rewrite: &rewrite.Rewrite{ + Method: "GET", + }, + + HandleResponse: []caddyhttp.ResponseHandler{}, + } + + // collect the headers to copy from the auth response + // onto the original request, so they can get passed + // through to a backend app + headersToCopy := []string{} + + // read the subdirectives for configuring the forward_auth shortcut + // NOTE: we delete the tokens as we go so that the reverse_proxy + // unmarshal doesn't see these subdirectives which it cannot handle + for dispenser.Next() { + for dispenser.NextBlock(0) { + // ignore any sub-subdirectives that might + // have the same name somewhere within + // the reverse_proxy passthrough tokens + if dispenser.Nesting() != 1 { + continue + } + + // parse the forward_auth subdirectives + switch dispenser.Val() { + case "uri": + if !dispenser.NextArg() { + return nil, dispenser.ArgErr() + } + rpHandler.Rewrite.URI = dispenser.Val() + dispenser.Delete() + dispenser.Delete() + + case "copy_headers": + args := dispenser.RemainingArgs() + dispenser.Delete() + for _, headerField := range args { + dispenser.Delete() + headersToCopy = append(headersToCopy, headerField) + } + if len(headersToCopy) == 0 { + return nil, dispenser.ArgErr() + } + } + } + } + + // reset the dispenser after we're done so that the reverse_proxy + // unmarshaler can read it from the start + dispenser.Reset() + + // the auth target URI must not be empty + if rpHandler.Rewrite.URI == "" { + return nil, dispenser.Errf("the 'uri' subdirective is required") + } + + // set up handler for good responses; when a response + // has 2xx status, then we will copy some headers from + // the response onto the original request, and allow + // handling to continue down the middleware chain, + // by _not_ executing a terminal handler. + goodResponseHandler := caddyhttp.ResponseHandler{ + Match: &caddyhttp.ResponseMatcher{ + StatusCode: []int{2}, + }, + Routes: []caddyhttp.Route{}, + } + if len(headersToCopy) > 0 { + handler := &headers.Handler{ + Request: &headers.HeaderOps{ + Set: http.Header{}, + }, + } + + for _, headerField := range headersToCopy { + handler.Request.Set[headerField] = []string{ + "{http.reverse_proxy.header." + headerField + "}", + } + } + + goodResponseHandler.Routes = append( + goodResponseHandler.Routes, + caddyhttp.Route{ + HandlersRaw: []json.RawMessage{caddyconfig.JSONModuleObject( + handler, + "handler", + "headers", + nil, + )}, + }, + ) + } + rpHandler.HandleResponse = append(rpHandler.HandleResponse, goodResponseHandler) + + // set up handler for denial responses; when a response + // has any other status than 2xx, then we copy the response + // back to the client, and terminate handling. + denialResponseHandler := caddyhttp.ResponseHandler{ + Routes: []caddyhttp.Route{ + { + HandlersRaw: []json.RawMessage{caddyconfig.JSONModuleObject( + &reverseproxy.CopyResponseHeadersHandler{ + Exclude: []string{ + "Connection", + "Keep-Alive", + "Te", + "Trailers", + "Transfer-Encoding", + "Upgrade", + }, + }, + "handler", + "copy_response_headers", + nil, + )}, + }, + { + HandlersRaw: []json.RawMessage{caddyconfig.JSONModuleObject( + &reverseproxy.CopyResponseHandler{}, + "handler", + "copy_response", + nil, + )}, + }, + }, + } + rpHandler.HandleResponse = append(rpHandler.HandleResponse, denialResponseHandler) + + // the rest of the config is specified by the user + // using the reverse_proxy directive syntax + err = rpHandler.UnmarshalCaddyfile(dispenser) + if err != nil { + return nil, err + } + err = rpHandler.FinalizeUnmarshalCaddyfile(h) + if err != nil { + return nil, err + } + + // create the final reverse proxy route + rpRoute := caddyhttp.Route{ + HandlersRaw: []json.RawMessage{caddyconfig.JSONModuleObject( + rpHandler, + "handler", + "reverse_proxy", + nil, + )}, + } + + // apply the user's matcher if any + if userMatcherSet != nil { + rpRoute.MatcherSetsRaw = []caddy.ModuleMap{userMatcherSet} + } + + return []httpcaddyfile.ConfigValue{ + { + Class: "route", + Value: rpRoute, + }, + }, nil +} diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index d69b04e1..07eb99ce 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -35,6 +35,7 @@ import ( "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/caddyserver/caddy/v2/modules/caddyhttp/headers" + "github.com/caddyserver/caddy/v2/modules/caddyhttp/rewrite" "go.uber.org/zap" "golang.org/x/net/http/httpguts" ) @@ -136,6 +137,18 @@ type Handler struct { // used for the requests and responses (in bytes). MaxBufferSize int64 `json:"max_buffer_size,omitempty"` + // If configured, rewrites the copy of the upstream request. + // Allows changing the request method and URI (path and query). + // Since the rewrite is applied to the copy, it does not persist + // past the reverse proxy handler. + // If the method is changed to `GET` or `HEAD`, the request body + // will not be copied to the backend. This allows a later request + // handler -- either in a `handle_response` route, or after -- to + // read the body. + // By default, no rewrite is performed, and the method and URI + // from the incoming request is used as-is for proxying. + Rewrite *rewrite.Rewrite `json:"rewrite,omitempty"` + // List of handlers and their associated matchers to evaluate // after successful roundtrips. The first handler that matches // the response from a backend will be invoked. The response @@ -258,6 +271,13 @@ func (h *Handler) Provision(ctx caddy.Context) error { } } + if h.Rewrite != nil { + err := h.Rewrite.Provision(ctx) + if err != nil { + return fmt.Errorf("provisioning rewrite: %v", err) + } + } + // set up transport if h.Transport == nil { t := &HTTPTransport{} @@ -385,7 +405,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) // prepare the request for proxying; this is needed only once - clonedReq, err := h.prepareRequest(r) + clonedReq, err := h.prepareRequest(r, repl) if err != nil { return caddyhttp.Error(http.StatusInternalServerError, fmt.Errorf("preparing request for upstream round-trip: %v", err)) @@ -412,7 +432,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht var proxyErr error for { var done bool - done, proxyErr = h.proxyLoopIteration(clonedReq, w, proxyErr, start, repl, reqHeader, reqHost, next) + done, proxyErr = h.proxyLoopIteration(clonedReq, r, w, proxyErr, start, repl, reqHeader, reqHost, next) if done { break } @@ -429,7 +449,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht // that has to be passed in, we brought this into its own method so that we could run defer more easily. // It returns true when the loop is done and should break; false otherwise. The error value returned should // be assigned to the proxyErr value for the next iteration of the loop (or the error handled after break). -func (h *Handler) proxyLoopIteration(r *http.Request, w http.ResponseWriter, proxyErr error, start time.Time, +func (h *Handler) proxyLoopIteration(r *http.Request, origReq *http.Request, w http.ResponseWriter, proxyErr error, start time.Time, repl *caddy.Replacer, reqHeader http.Header, reqHost string, next caddyhttp.Handler) (bool, error) { // get the updated list of upstreams upstreams := h.Upstreams @@ -503,7 +523,7 @@ func (h *Handler) proxyLoopIteration(r *http.Request, w http.ResponseWriter, pro } // proxy the request to that upstream - proxyErr = h.reverseProxy(w, r, repl, dialInfo, next) + proxyErr = h.reverseProxy(w, r, origReq, repl, dialInfo, next) if proxyErr == nil || proxyErr == context.Canceled { // context.Canceled happens when the downstream client // cancels the request, which is not our failure @@ -536,9 +556,20 @@ func (h *Handler) proxyLoopIteration(r *http.Request, w http.ResponseWriter, pro // properties of the cloned request and should be done just once (before // proxying) regardless of proxy retries. This assumes that no mutations // of the cloned request are performed by h during or after proxying. -func (h Handler) prepareRequest(req *http.Request) (*http.Request, error) { +func (h Handler) prepareRequest(req *http.Request, repl *caddy.Replacer) (*http.Request, error) { req = cloneRequest(req) + // if enabled, perform rewrites on the cloned request; if + // the method is GET or HEAD, prevent the request body + // from being copied to the upstream + if h.Rewrite != nil { + changed := h.Rewrite.Rewrite(req, repl) + if changed && (h.Rewrite.Method == "GET" || h.Rewrite.Method == "HEAD") { + req.ContentLength = 0 + req.Body = nil + } + } + // if enabled, buffer client request; this should only be // enabled if the upstream requires it and does not work // with "slow clients" (gunicorn, etc.) - this obviously @@ -547,7 +578,7 @@ func (h Handler) prepareRequest(req *http.Request) (*http.Request, error) { // attacks, so it is strongly recommended to only use this // feature if absolutely required, if read timeouts are // set, and if body size is limited - if h.BufferRequests { + if h.BufferRequests && req.Body != nil { req.Body = h.bufferedBody(req.Body) } @@ -673,10 +704,7 @@ func (h Handler) addForwardedHeaders(req *http.Request) error { // 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 - } + host := req.Host prior, ok, omit = lastHeaderValue(req.Header, "X-Forwarded-Host") if trusted && ok && prior != "" { host = prior @@ -691,7 +719,7 @@ func (h Handler) addForwardedHeaders(req *http.Request) error { // reverseProxy performs a round-trip to the given backend and processes the response with the client. // (This method is mostly the beginning of what was borrowed from the net/http/httputil package in the // Go standard library which was used as the foundation.) -func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, repl *caddy.Replacer, di DialInfo, next caddyhttp.Handler) error { +func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, origReq *http.Request, repl *caddy.Replacer, di DialInfo, next caddyhttp.Handler) error { _ = di.Upstream.Host.countRequest(1) //nolint:errcheck defer di.Upstream.Host.countRequest(-1) @@ -798,17 +826,19 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, repl * // we make some data available via request context to child routes // so that they may inherit some options and functions from the // handler, and be able to copy the response. + // we use the original request here, so that any routes from 'next' + // see the original request rather than the proxy cloned request. hrc := &handleResponseContext{ handler: h, response: res, start: start, logger: logger, } - ctx := req.Context() + ctx := origReq.Context() ctx = context.WithValue(ctx, proxyHandleResponseContextCtxKey, hrc) // pass the request through the response handler routes - routeErr := rh.Routes.Compile(next).ServeHTTP(rw, req.WithContext(ctx)) + routeErr := rh.Routes.Compile(next).ServeHTTP(rw, origReq.WithContext(ctx)) // if the response handler routes already finalized the response, // we can return early. It should be finalized if the routes executed diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go index cd340e35..690e7526 100644 --- a/modules/caddyhttp/rewrite/rewrite.go +++ b/modules/caddyhttp/rewrite/rewrite.go @@ -106,7 +106,7 @@ func (rewr Rewrite) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddy zap.Object("request", caddyhttp.LoggableHTTPRequest{Request: r}), ) - changed := rewr.rewrite(r, repl, logger) + changed := rewr.Rewrite(r, repl) if changed { logger.Debug("rewrote request", @@ -121,7 +121,7 @@ func (rewr Rewrite) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddy // rewrite performs the rewrites on r using repl, which should // have been obtained from r, but is passed in for efficiency. // It returns true if any changes were made to r. -func (rewr Rewrite) rewrite(r *http.Request, repl *caddy.Replacer, logger *zap.Logger) bool { +func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool { oldMethod := r.Method oldURI := r.RequestURI diff --git a/modules/caddyhttp/rewrite/rewrite_test.go b/modules/caddyhttp/rewrite/rewrite_test.go index 38d96fe9..3e2e13da 100644 --- a/modules/caddyhttp/rewrite/rewrite_test.go +++ b/modules/caddyhttp/rewrite/rewrite_test.go @@ -292,7 +292,7 @@ func TestRewrite(t *testing.T) { rep.re = re } - changed := tc.rule.rewrite(tc.input, repl, nil) + changed := tc.rule.Rewrite(tc.input, repl) if expected, actual := !reqEqual(originalInput, tc.input), changed; expected != actual { t.Errorf("Test %d: Expected changed=%t but was %t", i, expected, actual) diff --git a/modules/caddyhttp/standard/imports.go b/modules/caddyhttp/standard/imports.go index 8ce23957..435569d6 100644 --- a/modules/caddyhttp/standard/imports.go +++ b/modules/caddyhttp/standard/imports.go @@ -15,6 +15,7 @@ import ( _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/requestbody" _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy" _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy/fastcgi" + _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy/forwardauth" _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/rewrite" _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/templates" _ "github.com/caddyserver/caddy/v2/modules/caddyhttp/tracing"