Skip to content

Commit aa0ff79

Browse files
dunglasdunglas
committed
implement proposal #50465 
1 parent 744aa33 commit aa0ff79

File tree

2 files changed

+277
-46
lines changed

2 files changed

+277
-46
lines changed

‎src/net/http/httputil/reverseproxy.go

Lines changed: 59 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,37 @@ import (
2424
"golang.org/x/net/http/httpguts"
2525
)
2626

27+
type ForwardedHeaderBehavior uint8
28+
29+
const (
30+
// Overwrite existing X-Forwarded-For, X-Forwarded-Proto
31+
// and X-Forwarded-Host HTTP headers with values extracted
32+
// from the current connection with the client.
33+
// As a special case, headers explicitly set to nil
34+
// are never modified.
35+
ForwardedHeaderOverwrite ForwardedHeaderBehavior = iota
36+
37+
// Add client IP to the existing IPs in X-Forwarded-For.
38+
// Preserve X-Forwarded-Proto and X-Forwarded-Host if set.
39+
// Ensure that the previous proxies in the chain are trusted
40+
// As a special case, headers explicitly set to nil
41+
// are never modified.
42+
// when using this value, or it will lead to security issues.
43+
ForwardedHeaderAdd
44+
45+
// Preserve all existing forwarded HTTP headers.
46+
// Ensure that the previous proxies in the chain are trusted
47+
// when using this value, or it will lead to security issues.
48+
ForwardedHeaderPreserve
49+
)
50+
2751
// ReverseProxy is an HTTP Handler that takes an incoming request and
2852
// sends it to another server, proxying the response back to the
2953
// client.
3054
//
3155
// ReverseProxy by default sets the client IP as the value of the
32-
// X-Forwarded-For header.
33-
//
34-
// If an X-Forwarded-For header already exists, the client IP is
35-
// appended to the existing values. As a special case, if the header
36-
// exists in the Request.Header map but has a nil value (such as when
37-
// set by the Director func), the X-Forwarded-For header is
38-
// not modified.
39-
//
40-
// To prevent IP spoofing, be sure to delete any pre-existing
41-
// X-Forwarded-For header coming from the client or
42-
// an untrusted proxy, for instance, by setting
43-
// OverwriteForwardedHeaders to true.
56+
// X-Forwarded-For header. To control this behavior,
57+
// set the ForwardedHeaderBehavior field.
4458
type ReverseProxy struct {
4559
// Director must be a function which modifies
4660
// the request into a new request to be sent
@@ -94,19 +108,10 @@ type ReverseProxy struct {
94108
// a 502 Status Bad Gateway response.
95109
ErrorHandler func(http.ResponseWriter, *http.Request, error)
96110

97-
// OverwriteForwardedHeaders specifies if X-Forwarded-For,
98-
// X-Forwarded-Proto and X-Forwarded-Host headers coming from
99-
// the previous proxy must be replaced or not.
100-
//
101-
// If true, these 3 headers will be set regardless of any
102-
// existing value.
103-
//
104-
// If false, X-Forwarded-Proto and X-Forwarded-Host will not be
105-
// touched (not even created if they don't exist), and the
106-
// current client IP will be appended to the list in
107-
// X-Forwarded-For. If X-Forwarded-For doesn't exist, it will be
108-
// created.
109-
OverwriteForwardedHeaders bool
111+
// OverwriteForwardedHeaders specifies how to deal with the
112+
// X-Forwarded-For, X-Forwarded-Proto, and X-Forwarded-Host
113+
// headers.
114+
ForwardedHeaderBehavior ForwardedHeaderBehavior
110115
}
111116

112117
// A BufferPool is an interface for getting and returning temporary
@@ -297,28 +302,38 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
297302
outreq.Header.Set("Upgrade", reqUpType)
298303
}
299304

300-
if p.OverwriteForwardedHeaders {
301-
proto := "https"
302-
if req.TLS == nil {
303-
proto = "http"
305+
if p.ForwardedHeaderBehavior != ForwardedHeaderPreserve {
306+
if clientIP, _, err := net.SplitHostPort(req.RemoteAddr); err == nil {
307+
// If we aren't the first proxy retain prior
308+
// X-Forwarded-For information as a comma+space
309+
// separated list and fold multiple headers into one.
310+
prior, ok := outreq.Header["X-Forwarded-For"]
311+
omit := ok && prior == nil // Issue 38079: nil now means don't populate the header
312+
log.Printf("%#v %#v", prior, omit)
313+
if p.ForwardedHeaderBehavior == ForwardedHeaderAdd && len(prior) > 0 {
314+
clientIP = strings.Join(prior, ", ") + ", " + clientIP
315+
}
316+
if !omit {
317+
outreq.Header.Set("X-Forwarded-For", clientIP)
318+
}
304319
}
305320

306-
outreq.Header.Set("X-Forwarded-Proto", proto)
307-
outreq.Header.Set("X-Forwarded-Host", outreq.Host)
308-
outreq.Header.Del("X-Forwarded-For")
309-
}
321+
if p.ForwardedHeaderBehavior == ForwardedHeaderOverwrite {
322+
prior, ok := outreq.Header["X-Forwarded-Host"]
323+
omit := ok && prior == nil // nil means don't populate the header
324+
if !omit {
325+
outreq.Header.Set("X-Forwarded-Host", req.Host)
326+
}
310327

311-
if clientIP, _, err := net.SplitHostPort(req.RemoteAddr); err == nil {
312-
// If we aren't the first proxy retain prior
313-
// X-Forwarded-For information as a comma+space
314-
// separated list and fold multiple headers into one.
315-
prior, ok := outreq.Header["X-Forwarded-For"]
316-
omit := ok && prior == nil // Issue 38079: nil now means don't populate the header
317-
if len(prior) > 0 {
318-
clientIP = strings.Join(prior, ", ") + ", " + clientIP
319-
}
320-
if !omit {
321-
outreq.Header.Set("X-Forwarded-For", clientIP)
328+
prior, ok = outreq.Header["X-Forwarded-Proto"]
329+
omit = ok && prior == nil // nil means don't populate the header
330+
if !omit {
331+
if req.TLS == nil {
332+
outreq.Header.Set("X-Forwarded-Proto", "http")
333+
} else {
334+
outreq.Header.Set("X-Forwarded-Proto", "https")
335+
}
336+
}
322337
}
323338
}
324339

‎src/net/http/httputil/reverseproxy_test.go

Lines changed: 218 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ func TestXForwardedFor(t *testing.T) {
347347
}
348348
}
349349

350-
func TestOverwriteForwardedHeaders(t *testing.T) {
350+
func TestForwardedHeaderOverwrite(t *testing.T) {
351351
const prevForwardedFor = "client ip"
352352
const prevForwardedProto = "https"
353353
const prevForwardedHost = "example.com"
@@ -375,7 +375,223 @@ func TestOverwriteForwardedHeaders(t *testing.T) {
375375
t.Fatal(err)
376376
}
377377
proxyHandler := NewSingleHostReverseProxy(backendURL)
378-
proxyHandler.OverwriteForwardedHeaders = true
378+
frontend := httptest.NewServer(proxyHandler)
379+
defer frontend.Close()
380+
381+
getReq, _ := http.NewRequest("GET", frontend.URL, nil)
382+
getReq.Host = "some-name"
383+
getReq.Header.Set("Connection", "close")
384+
getReq.Header.Set("X-Forwarded-For", prevForwardedFor)
385+
getReq.Header.Set("X-Forwarded-Proto", prevForwardedProto)
386+
getReq.Header.Set("X-Forwarded-Host", prevForwardedHost)
387+
getReq.Close = true
388+
res, err := frontend.Client().Do(getReq)
389+
if err != nil {
390+
t.Fatalf("Get: %v", err)
391+
}
392+
if g, e := res.StatusCode, backendStatus; g != e {
393+
t.Errorf("got res.StatusCode %d; expected %d", g, e)
394+
}
395+
bodyBytes, _ := ioutil.ReadAll(res.Body)
396+
if g, e := string(bodyBytes), backendResponse; g != e {
397+
t.Errorf("got body %q; expected %q", g, e)
398+
}
399+
}
400+
401+
func TestForwardedHeaderOverwrite_Omit(t *testing.T) {
402+
const prevForwardedFor = "client ip"
403+
const prevForwardedProto = "https"
404+
const prevForwardedHost = "example.com"
405+
const backendResponse = "I am the backend"
406+
const backendStatus = 404
407+
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
408+
if r.Header["X-Forwarded-For"] != nil {
409+
t.Errorf("X-Forwarded-For header is not nil")
410+
}
411+
if r.Header["X-Forwarded-Proto"] != nil {
412+
t.Errorf("X-Forwarded-Proto header is not nil")
413+
}
414+
if r.Header["X-Forwarded-For"] != nil {
415+
t.Errorf("X-Forwarded-For header is not nil")
416+
}
417+
w.WriteHeader(backendStatus)
418+
w.Write([]byte(backendResponse))
419+
}))
420+
defer backend.Close()
421+
backendURL, err := url.Parse(backend.URL)
422+
if err != nil {
423+
t.Fatal(err)
424+
}
425+
proxyHandler := NewSingleHostReverseProxy(backendURL)
426+
frontend := httptest.NewServer(proxyHandler)
427+
defer frontend.Close()
428+
429+
oldDirector := proxyHandler.Director
430+
proxyHandler.Director = func(r *http.Request) {
431+
r.Header["X-Forwarded-For"] = nil
432+
r.Header["X-Forwarded-Proto"] = nil
433+
r.Header["X-Forwarded-Host"] = nil
434+
oldDirector(r)
435+
}
436+
437+
getReq, _ := http.NewRequest("GET", frontend.URL, nil)
438+
getReq.Host = "some-name"
439+
getReq.Header.Set("Connection", "close")
440+
getReq.Header.Set("X-Forwarded-For", prevForwardedFor)
441+
getReq.Header.Set("X-Forwarded-Proto", prevForwardedProto)
442+
getReq.Header.Set("X-Forwarded-Host", prevForwardedHost)
443+
getReq.Close = true
444+
res, err := frontend.Client().Do(getReq)
445+
if err != nil {
446+
t.Fatalf("Get: %v", err)
447+
}
448+
if g, e := res.StatusCode, backendStatus; g != e {
449+
t.Errorf("got res.StatusCode %d; expected %d", g, e)
450+
}
451+
bodyBytes, _ := ioutil.ReadAll(res.Body)
452+
if g, e := string(bodyBytes), backendResponse; g != e {
453+
t.Errorf("got body %q; expected %q", g, e)
454+
}
455+
}
456+
457+
func TestForwardedHeaderAdd(t *testing.T) {
458+
const prevForwardedFor = "127.0.0.42"
459+
const prevForwardedProto = "https"
460+
const prevForwardedHost = "example.com"
461+
const backendResponse = "I am the backend"
462+
const backendStatus = 404
463+
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
464+
if r.Header.Get("X-Forwarded-For") == "" {
465+
t.Errorf("didn't get X-Forwarded-For header")
466+
}
467+
if r.Header.Get("X-Forwarded-For") == "127.0.0.42,127.0.0.1" {
468+
t.Errorf("X-Forwarded-For does not contain prior and current data")
469+
}
470+
if r.Header.Get("X-Forwarded-Proto") != prevForwardedProto {
471+
t.Errorf("X-Forwarded-Proto does not contain prior data")
472+
}
473+
if r.Header.Get("X-Forwarded-Host") != prevForwardedHost {
474+
t.Errorf("X-Forwarded-Host does not contain prior data")
475+
}
476+
w.WriteHeader(backendStatus)
477+
w.Write([]byte(backendResponse))
478+
}))
479+
defer backend.Close()
480+
backendURL, err := url.Parse(backend.URL)
481+
if err != nil {
482+
t.Fatal(err)
483+
}
484+
proxyHandler := NewSingleHostReverseProxy(backendURL)
485+
proxyHandler.ForwardedHeaderBehavior = ForwardedHeaderAdd
486+
frontend := httptest.NewServer(proxyHandler)
487+
defer frontend.Close()
488+
489+
getReq, _ := http.NewRequest("GET", frontend.URL, nil)
490+
getReq.Host = "some-name"
491+
getReq.Header.Set("Connection", "close")
492+
getReq.Header.Set("X-Forwarded-For", prevForwardedFor)
493+
getReq.Header.Set("X-Forwarded-Proto", prevForwardedProto)
494+
getReq.Header.Set("X-Forwarded-Host", prevForwardedHost)
495+
getReq.Close = true
496+
res, err := frontend.Client().Do(getReq)
497+
if err != nil {
498+
t.Fatalf("Get: %v", err)
499+
}
500+
if g, e := res.StatusCode, backendStatus; g != e {
501+
t.Errorf("got res.StatusCode %d; expected %d", g, e)
502+
}
503+
bodyBytes, _ := ioutil.ReadAll(res.Body)
504+
if g, e := string(bodyBytes), backendResponse; g != e {
505+
t.Errorf("got body %q; expected %q", g, e)
506+
}
507+
}
508+
509+
func TestForwardedHeaderAdd_Omit(t *testing.T) {
510+
const prevForwardedFor = "127.0.0.42"
511+
const prevForwardedProto = "https"
512+
const prevForwardedHost = "example.com"
513+
const backendResponse = "I am the backend"
514+
const backendStatus = 404
515+
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
516+
if r.Header["X-Forwarded-For"] != nil {
517+
t.Errorf("X-Forwarded-For header is not nil")
518+
}
519+
if r.Header["X-Forwarded-Proto"] != nil {
520+
t.Errorf("X-Forwarded-Proto header is not nil")
521+
}
522+
if r.Header["X-Forwarded-For"] != nil {
523+
t.Errorf("X-Forwarded-For header is not nil")
524+
}
525+
w.WriteHeader(backendStatus)
526+
w.Write([]byte(backendResponse))
527+
}))
528+
defer backend.Close()
529+
backendURL, err := url.Parse(backend.URL)
530+
if err != nil {
531+
t.Fatal(err)
532+
}
533+
proxyHandler := NewSingleHostReverseProxy(backendURL)
534+
proxyHandler.ForwardedHeaderBehavior = ForwardedHeaderAdd
535+
frontend := httptest.NewServer(proxyHandler)
536+
defer frontend.Close()
537+
538+
oldDirector := proxyHandler.Director
539+
proxyHandler.Director = func(r *http.Request) {
540+
r.Header["X-Forwarded-For"] = nil
541+
r.Header["X-Forwarded-Proto"] = nil
542+
r.Header["X-Forwarded-Host"] = nil
543+
oldDirector(r)
544+
}
545+
546+
getReq, _ := http.NewRequest("GET", frontend.URL, nil)
547+
getReq.Host = "some-name"
548+
getReq.Header.Set("Connection", "close")
549+
getReq.Header.Set("X-Forwarded-For", prevForwardedFor)
550+
getReq.Header.Set("X-Forwarded-Proto", prevForwardedProto)
551+
getReq.Header.Set("X-Forwarded-Host", prevForwardedHost)
552+
getReq.Close = true
553+
res, err := frontend.Client().Do(getReq)
554+
if err != nil {
555+
t.Fatalf("Get: %v", err)
556+
}
557+
if g, e := res.StatusCode, backendStatus; g != e {
558+
t.Errorf("got res.StatusCode %d; expected %d", g, e)
559+
}
560+
bodyBytes, _ := ioutil.ReadAll(res.Body)
561+
if g, e := string(bodyBytes), backendResponse; g != e {
562+
t.Errorf("got body %q; expected %q", g, e)
563+
}
564+
}
565+
566+
func TestForwardedHeaderPreserve(t *testing.T) {
567+
const prevForwardedFor = "127.0.0.42"
568+
const prevForwardedProto = "https"
569+
const prevForwardedHost = "example.com"
570+
const backendResponse = "I am the backend"
571+
const backendStatus = 404
572+
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
573+
if r.Header.Get("X-Forwarded-For") == "" {
574+
t.Errorf("didn't get X-Forwarded-For header")
575+
}
576+
if r.Header.Get("X-Forwarded-For") != "127.0.0.42" {
577+
t.Errorf("X-Forwarded-For does not contain prior data")
578+
}
579+
if r.Header.Get("X-Forwarded-Proto") != prevForwardedProto {
580+
t.Errorf("X-Forwarded-Proto does not contain prior data")
581+
}
582+
if r.Header.Get("X-Forwarded-Host") != prevForwardedHost {
583+
t.Errorf("X-Forwarded-Host does not contain prior data")
584+
}
585+
w.WriteHeader(backendStatus)
586+
w.Write([]byte(backendResponse))
587+
}))
588+
defer backend.Close()
589+
backendURL, err := url.Parse(backend.URL)
590+
if err != nil {
591+
t.Fatal(err)
592+
}
593+
proxyHandler := NewSingleHostReverseProxy(backendURL)
594+
proxyHandler.ForwardedHeaderBehavior = ForwardedHeaderPreserve
379595
frontend := httptest.NewServer(proxyHandler)
380596
defer frontend.Close()
381597

0 commit comments

Comments
 (0)