From 88edca65d3b1b7fb533f5abe7c1a95a417a5d016 Mon Sep 17 00:00:00 2001
From: Theofanis Despoudis <thdespou@hotmail.com>
Date: Thu, 5 Apr 2018 07:04:06 +0100
Subject: [PATCH] proxy: Fix transparent pass-thru of existing X-Forwarded-For
 headers

* Fixes #1960 Transparent proxy not appending
existing X-Forwarded-For header

* Fixes #1960 Formatting Code
---
 caddyhttp/proxy/proxy_test.go    | 82 ++++++++++++++++++++++++++++++--
 caddyhttp/proxy/upstream.go      |  3 +-
 caddyhttp/proxy/upstream_test.go |  7 ++-
 3 files changed, 85 insertions(+), 7 deletions(-)

diff --git a/caddyhttp/proxy/proxy_test.go b/caddyhttp/proxy/proxy_test.go
index 5876a29a..213f8741 100644
--- a/caddyhttp/proxy/proxy_test.go
+++ b/caddyhttp/proxy/proxy_test.go
@@ -913,6 +913,67 @@ func TestHostSimpleProxyNoHeaderForward(t *testing.T) {
 	}
 }
 
+func TestReverseProxyTransparentHeaders(t *testing.T) {
+	testCases := []struct {
+		name               string
+		remoteAddr         string
+		forwardedForHeader string
+		expected           []string
+	}{
+		{"No header", "192.168.0.1:80", "", []string{"192.168.0.1"}},
+		{"Existing", "192.168.0.1:80", "1.1.1.1, 2.2.2.2", []string{"1.1.1.1, 2.2.2.2, 192.168.0.1"}},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			testReverseProxyTransparentHeaders(t, tc.remoteAddr, tc.forwardedForHeader, tc.expected)
+		})
+	}
+}
+
+func testReverseProxyTransparentHeaders(t *testing.T, remoteAddr, forwardedForHeader string, expected []string) {
+	// Arrange
+	log.SetOutput(ioutil.Discard)
+	defer log.SetOutput(os.Stderr)
+
+	var actualHeaders http.Header
+	backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		actualHeaders = r.Header
+	}))
+	defer backend.Close()
+
+	config := "proxy / " + backend.URL + " {\n transparent \n}"
+
+	// make proxy
+	upstreams, err := NewStaticUpstreams(caddyfile.NewDispenser("Testfile", strings.NewReader(config)), "")
+	if err != nil {
+		t.Errorf("Expected no error. Got: %s", err.Error())
+	}
+
+	// set up proxy
+	p := &Proxy{
+		Next:      httpserver.EmptyNext, // prevents panic in some cases when test fails
+		Upstreams: upstreams,
+	}
+
+	// create request and response recorder
+	r := httptest.NewRequest("GET", backend.URL, nil)
+	r.RemoteAddr = remoteAddr
+	if forwardedForHeader != "" {
+		r.Header.Set("X-Forwarded-For", forwardedForHeader)
+	}
+
+	w := httptest.NewRecorder()
+
+	// Act
+	p.ServeHTTP(w, r)
+
+	// Assert
+	if got := actualHeaders["X-Forwarded-For"]; !reflect.DeepEqual(got, expected) {
+		t.Errorf("Transparent proxy response does not contain expected %v header: expect %v, but got %v",
+			"X-Forwarded-For", expected, got)
+	}
+}
+
 func TestHostHeaderReplacedUsingForward(t *testing.T) {
 	var requestHost string
 	backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -943,11 +1004,22 @@ func TestHostHeaderReplacedUsingForward(t *testing.T) {
 }
 
 func TestBasicAuth(t *testing.T) {
-	basicAuthTestcase(t, nil, nil)
-	basicAuthTestcase(t, nil, url.UserPassword("username", "password"))
-	basicAuthTestcase(t, url.UserPassword("usename", "password"), nil)
-	basicAuthTestcase(t, url.UserPassword("unused", "unused"),
-		url.UserPassword("username", "password"))
+	testCases := []struct {
+		name         string
+		upstreamUser *url.Userinfo
+		clientUser   *url.Userinfo
+	}{
+		{"Nil Both", nil, nil},
+		{"Nil Upstream User", nil, url.UserPassword("username", "password")},
+		{"Nil Client User", url.UserPassword("usename", "password"), nil},
+		{"Both Provided", url.UserPassword("unused", "unused"),
+			url.UserPassword("username", "password")},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			basicAuthTestcase(t, tc.upstreamUser, tc.clientUser)
+		})
+	}
 }
 
 func basicAuthTestcase(t *testing.T, upstreamUser, clientUser *url.Userinfo) {
diff --git a/caddyhttp/proxy/upstream.go b/caddyhttp/proxy/upstream.go
index ae15a6dc..df93d390 100644
--- a/caddyhttp/proxy/upstream.go
+++ b/caddyhttp/proxy/upstream.go
@@ -431,9 +431,10 @@ func parseBlock(c *caddyfile.Dispenser, u *staticUpstream, hasSrv bool) error {
 		}
 		u.downstreamHeaders.Add(header, value)
 	case "transparent":
+		// Note: X-Forwarded-For header is always being appended for proxy connections
+		// See implementation of createUpstreamRequest in proxy.go
 		u.upstreamHeaders.Add("Host", "{host}")
 		u.upstreamHeaders.Add("X-Real-IP", "{remote}")
-		u.upstreamHeaders.Add("X-Forwarded-For", "{remote}")
 		u.upstreamHeaders.Add("X-Forwarded-Proto", "{scheme}")
 	case "websocket":
 		u.upstreamHeaders.Add("Connection", "{>Connection}")
diff --git a/caddyhttp/proxy/upstream_test.go b/caddyhttp/proxy/upstream_test.go
index 23fd4831..18c65230 100644
--- a/caddyhttp/proxy/upstream_test.go
+++ b/caddyhttp/proxy/upstream_test.go
@@ -282,7 +282,8 @@ func TestStop(t *testing.T) {
 	}
 }
 
-func TestParseBlock(t *testing.T) {
+func TestParseBlockTransparent(t *testing.T) {
+	// tests for transparent proxy presets
 	r, _ := http.NewRequest("GET", "/", nil)
 	tests := []struct {
 		config string
@@ -316,6 +317,10 @@ func TestParseBlock(t *testing.T) {
 			if _, ok := headers["X-Forwarded-Proto"]; !ok {
 				t.Errorf("Test %d: Could not find the X-Forwarded-Proto header", i+1)
 			}
+
+			if _, ok := headers["X-Forwarded-For"]; ok {
+				t.Errorf("Test %d: Found unexpected X-Forwarded-For header", i+1)
+			}
 		}
 	}
 }