Merge pull request #2616 from Dirbaio/xff

Add use-forwarded-headers configmap option.

docs/user-guide/nginx-configuration/configmap.md

@@ -102,6 +102,7 @@ The following table shows a configuration option's name, type, and the default v
 |[proxy-stream-timeout](#proxy-stream-timeout)|string|"600s"|
 |[proxy-stream-responses](#proxy-stream-responses)|int|1|
 |[bind-address](#bind-address)|[]string|""|
+|[use-forwarded-headers](#use-forwarded-headers)|bool|"true"|
 |[forwarded-for-header](#forwarded-for-header)|string|"X-Forwarded-For"|
 |[compute-full-forwarded-for](#compute-full-forwarded-for)|bool|"false"|
 |[proxy-add-original-uri-header](#proxy-add-original-uri-header)|bool|"true"|
@@ -589,6 +590,12 @@ _References:_
 
 Sets the addresses on which the server will accept requests instead of *. It should be noted that these addresses must exist in the runtime environment or the controller will crash loop.
 
+## use-forwarded-headers
+
+If true, NGINX passes the incoming `X-Forwarded-*` headers to upstreams. Use this option when NGINX is behind another L7 proxy / load balancer that is setting these headers.
+
+If false, NGINX ignores incoming `X-Forwarded-*` headers, filling them with the request information it sees. Use this option if NGINX is exposed directly to the internet, or it's behind a L3/packet-based load balancer that doesn't alter the source IP in the packets.
+
 ## forwarded-for-header
 
 Sets the header field for identifying the originating IP address of a client. _**default:**_ X-Forwarded-For
@@ -601,7 +608,7 @@ Append the remote address to the X-Forwarded-For header instead of replacing it.
 
 Adds an X-Original-Uri header with the original request URI to the backend request
 
-## generate-request-id 
+## generate-request-id
 
 Ensures that X-Request-ID is defaulted to a random value, if no X-Request-ID is present in the request
 

internal/ingress/controller/config/config.go

@@ -411,6 +411,9 @@ type Configuration struct {
 	// Sets the ipv6 addresses on which the server will accept requests.
 	BindAddressIpv6 []string `json:"bind-address-ipv6,omitempty"`
 
+	// Sets whether to use incoming X-Forwarded headers.
+	UseForwardedHeaders bool `json:"use-forwarded-headers"`
+
 	// Sets the header field for identifying the originating IP address of a client
 	// Default is X-Forwarded-For
 	ForwardedForHeader string `json:"forwarded-for-header,omitempty"`
@@ -552,6 +555,7 @@ func NewDefault() Configuration {
 		EnableDynamicTLSRecords:    true,
 		EnableUnderscoresInHeaders: false,
 		ErrorLogLevel:              errorLevel,
+		UseForwardedHeaders:        true,
 		ForwardedForHeader:         "X-Forwarded-For",
 		ComputeFullForwardedFor:    false,
 		ProxyAddOriginalUriHeader:  true,

rootfs/etc/nginx/template/nginx.tmpl

@@ -93,7 +93,10 @@ http {
     }
     {{ end }}
     {{ end }}
-    {{/* we use the value of the header X-Forwarded-For to be able to use the geo_ip module */}}
+
+    {{/* Enable the real_ip module only if we use either X-Forwarded headers or Proxy Protocol. */}}
+    {{/* we use the value of the real IP for the geo_ip module */}}
+    {{ if or $cfg.UseForwardedHeaders $cfg.UseProxyProtocol }}
     {{ if $cfg.UseProxyProtocol }}
     real_ip_header      proxy_protocol;
     {{ else }}
@@ -104,6 +107,7 @@ http {
     {{ range $trusted_ip := $cfg.ProxyRealIPCIDR }}
     set_real_ip_from    {{ $trusted_ip }};
     {{ end }}
+    {{ end }}
 
     {{ if $cfg.UseGeoIP }}
     {{/* databases used to determine the country depending on the client IP address */}}
@@ -241,7 +245,9 @@ http {
         ''               close;
     }
 
-    map {{ buildForwardedFor $cfg.ForwardedForHeader }} $the_real_ip {
+    # The following is a sneaky way to do "set $the_real_ip $remote_addr"
+    # Needed because using set is not allowed outside server blocks.
+    map '' $the_real_ip {
     {{ if $cfg.UseProxyProtocol }}
         # Get IP address from Proxy Protocol
         default          $proxy_protocol_addr;
@@ -250,12 +256,44 @@ http {
     {{ end }}
     }
 
+    {{ if $cfg.UseForwardedHeaders }}
     # trust http_x_forwarded_proto headers correctly indicate ssl offloading
     map $http_x_forwarded_proto $pass_access_scheme {
         default          $http_x_forwarded_proto;
         ''               $scheme;
     }
 
+    map $http_x_forwarded_port $pass_server_port {
+        default           $http_x_forwarded_port;
+        ''                $server_port;
+    }
+
+    # Obtain best http host
+    map $http_host $this_host {
+        default          $http_host;
+        ''               $host;
+    }
+
+    map $http_x_forwarded_host $best_http_host {
+        default          $http_x_forwarded_host;
+        ''               $this_host;
+    }
+    {{ else }}
+    map '' $pass_access_scheme {
+        default          $scheme;
+    }
+
+    map '' $pass_server_port {
+        default          $server_port;
+    }
+
+    # Obtain best http host
+    map $http_host $best_http_host {
+        default          $http_host;
+        ''               $host;
+    }
+    {{ end }}
+
     # validate $pass_access_scheme and $scheme are http to force a redirect
     map "$scheme:$pass_access_scheme" $redirect_to_https {
         default          0;
@@ -263,11 +301,6 @@ http {
         "https:http"     1;
     }
 
-    map $http_x_forwarded_port $pass_server_port {
-        default           $http_x_forwarded_port;
-        ''                $server_port;
-    }
-
     {{ if $all.IsSSLPassthroughEnabled }}
     # map port {{ $all.ListenPorts.SSLProxy }} to 443 for header X-Forwarded-Port
     map $pass_server_port $pass_port {
@@ -281,17 +314,6 @@ http {
     }
     {{ end }}
 
-    # Obtain best http host
-    map $http_host $this_host {
-        default          $http_host;
-        ''               $host;
-    }
-
-    map $http_x_forwarded_host $best_http_host {
-        default          $http_x_forwarded_host;
-        ''               $this_host;
-    }
-
     # Reverse proxies can detect if a client provides a X-Request-ID header, and pass it on to the backend server.
     # If no such header is provided, it can provide a random value.
     map $http_x_request_id $req_id {
@@ -301,7 +323,7 @@ http {
         {{ end }}
     }
 
-    {{ if $cfg.ComputeFullForwardedFor }}
+    {{ if and $cfg.UseForwardedHeaders $cfg.ComputeFullForwardedFor }}
     # We can't use $proxy_add_x_forwarded_for because the realip module
     # replaces the remote_addr too soon
     map $http_x_forwarded_for $full_x_forwarded_for {
@@ -1047,7 +1069,7 @@ stream {
 
             {{ $proxySetHeader }} X-Request-ID           $req_id;
             {{ $proxySetHeader }} X-Real-IP              $the_real_ip;
-            {{ if $all.Cfg.ComputeFullForwardedFor }}
+            {{ if and $all.Cfg.UseForwardedHeaders $all.Cfg.ComputeFullForwardedFor }}
             {{ $proxySetHeader }} X-Forwarded-For        $full_x_forwarded_for;
             {{ else }}
             {{ $proxySetHeader }} X-Forwarded-For        $the_real_ip;

test/e2e/settings/forwarded_headers.go

@@ -0,0 +1,117 @@
+/*
+Copyright 2017 The Kubernetes 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 settings
+
+import (
+	"fmt"
+	"net/http"
+	"strings"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/parnurzeal/gorequest"
+
+	"k8s.io/ingress-nginx/test/e2e/framework"
+)
+
+var _ = framework.IngressNginxDescribe("X-Forwarded headers", func() {
+	f := framework.NewDefaultFramework("forwarded-headers")
+
+	setting := "use-forwarded-headers"
+
+	BeforeEach(func() {
+		err := f.NewEchoDeployment()
+		Expect(err).NotTo(HaveOccurred())
+
+		err = f.UpdateNginxConfigMapData(setting, "false")
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	AfterEach(func() {
+	})
+
+	It("should trust X-Forwarded headers when setting is true", func() {
+		host := "forwarded-headers"
+
+		err := f.UpdateNginxConfigMapData(setting, "true")
+		Expect(err).NotTo(HaveOccurred())
+
+		ing, err := f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, nil))
+		Expect(err).NotTo(HaveOccurred())
+		Expect(ing).NotTo(BeNil())
+
+		err = f.WaitForNginxServer(host,
+			func(server string) bool {
+				return strings.Contains(server, "server_name forwarded-headers")
+			})
+		Expect(err).NotTo(HaveOccurred())
+
+		resp, body, errs := gorequest.New().
+			Get(f.IngressController.HTTPURL).
+			Set("Host", host).
+			Set("X-Forwarded-Port", "1234").
+			Set("X-Forwarded-Proto", "myproto").
+			Set("X-Forwarded-For", "1.2.3.4").
+			Set("X-Forwarded-Host", "myhost").
+			End()
+
+		Expect(len(errs)).Should(BeNumerically("==", 0))
+		Expect(resp.StatusCode).Should(Equal(http.StatusOK))
+		Expect(body).Should(ContainSubstring(fmt.Sprintf("host=myhost")))
+		Expect(body).Should(ContainSubstring(fmt.Sprintf("x-forwarded-host=myhost")))
+		Expect(body).Should(ContainSubstring(fmt.Sprintf("x-forwarded-proto=myproto")))
+		Expect(body).Should(ContainSubstring(fmt.Sprintf("x-forwarded-port=1234")))
+		Expect(body).Should(ContainSubstring(fmt.Sprintf("x-forwarded-for=1.2.3.4")))
+	})
+	It("should not trust X-Forwarded headers when setting is false", func() {
+		host := "forwarded-headers"
+
+		err := f.UpdateNginxConfigMapData(setting, "false")
+		Expect(err).NotTo(HaveOccurred())
+
+		ing, err := f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, nil))
+		Expect(err).NotTo(HaveOccurred())
+		Expect(ing).NotTo(BeNil())
+
+		err = f.WaitForNginxServer(host,
+			func(server string) bool {
+				return strings.Contains(server, "server_name forwarded-headers")
+			})
+		Expect(err).NotTo(HaveOccurred())
+
+		resp, body, errs := gorequest.New().
+			Get(f.IngressController.HTTPURL).
+			Set("Host", host).
+			Set("X-Forwarded-Port", "1234").
+			Set("X-Forwarded-Proto", "myproto").
+			Set("X-Forwarded-For", "1.2.3.4").
+			Set("X-Forwarded-Host", "myhost").
+			End()
+
+		Expect(len(errs)).Should(BeNumerically("==", 0))
+		Expect(resp.StatusCode).Should(Equal(http.StatusOK))
+		Expect(body).Should(ContainSubstring(fmt.Sprintf("host=forwarded-headers")))
+		Expect(body).Should(ContainSubstring(fmt.Sprintf("x-forwarded-port=80")))
+		Expect(body).Should(ContainSubstring(fmt.Sprintf("x-forwarded-proto=http")))
+		Expect(body).Should(ContainSubstring(fmt.Sprintf("x-original-forwarded-for=1.2.3.4")))
+		Expect(body).ShouldNot(ContainSubstring(fmt.Sprintf("host=myhost")))
+		Expect(body).ShouldNot(ContainSubstring(fmt.Sprintf("x-forwarded-host=myhost")))
+		Expect(body).ShouldNot(ContainSubstring(fmt.Sprintf("x-forwarded-proto=myproto")))
+		Expect(body).ShouldNot(ContainSubstring(fmt.Sprintf("x-forwarded-port=1234")))
+		Expect(body).ShouldNot(ContainSubstring(fmt.Sprintf("x-forwarded-for=1.2.3.4")))
+	})
+})