Merge pull request #1381 from aledbf/refactor-template-headers

Refactor X-Forwarded-* headers

controllers/nginx/pkg/cmd/controller/nginx.go

@@ -676,6 +676,7 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error {
 		RedirectServers:         redirectServers,
 		IsSSLPassthroughEnabled: n.isSSLPassthroughEnabled,
 		ListenPorts:             n.ports,
+		PublishService:          n.controller.GetPublishService(),
 	}
 
 	content, err := n.t.Write(tc)

controllers/nginx/pkg/config/config.go

@@ -23,6 +23,8 @@ import (
 
 	"github.com/golang/glog"
 
+	apiv1 "k8s.io/api/core/v1"
+
 	"k8s.io/ingress/core/pkg/ingress"
 	"k8s.io/ingress/core/pkg/ingress/defaults"
 )
@@ -259,6 +261,11 @@ type Configuration struct {
 	// https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_headers_hash_bucket_size
 	ProxyHeadersHashBucketSize int `json:"proxy-headers-hash-bucket-size,omitempty"`
 
+	// RealClientFrom defines the trusted source of the client source IP address
+	// The valid values are "auto", "http-proxy" and "tcp-proxy"
+	// Default: auto
+	RealClientFrom string `json:"real-client-from,omitempty"`
+
 	// Enables or disables emitting nginx version in error messages and in the “Server” response header field.
 	// http://nginx.org/en/docs/http/ngx_http_core_module.html#server_tokens
 	// Default: true
@@ -463,6 +470,7 @@ func NewDefault() Configuration {
 		LimitConnZoneVariable:        defaultLimitConnZoneVariable,
 		BindAddressIpv4:              defBindAddress,
 		BindAddressIpv6:              defBindAddress,
+		RealClientFrom:               "auto",
 		ZipkinCollectorPort:          9411,
 		ZipkinServiceName:            "nginx",
 	}
@@ -503,6 +511,7 @@ type TemplateConfig struct {
 	IsSSLPassthroughEnabled bool
 	RedirectServers         map[string]string
 	ListenPorts             *ListenPorts
+	PublishService          *apiv1.Service
 }
 
 // ListenPorts describe the ports required to run the

controllers/nginx/pkg/template/configmap.go

@@ -19,6 +19,7 @@ package template
 import (
 	"fmt"
 	"net"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -38,6 +39,10 @@ const (
 	bindAddress          = "bind-address"
 )
 
+var (
+	realClientRegex = regexp.MustCompile(`auto|http-proxy|tcp-proxy`)
+)
+
 // ReadConfig obtains the configuration defined by the user merged with the defaults.
 func ReadConfig(src map[string]string) config.Configuration {
 	conf := map[string]string{}
@@ -120,6 +125,11 @@ func ReadConfig(src map[string]string) config.Configuration {
 		glog.Warningf("unexpected error merging defaults: %v", err)
 	}
 
+	if !realClientRegex.MatchString(to.RealClientFrom) {
+		glog.Warningf("unexpected value for RealClientFromSetting (%v). Using default \"auto\"", to.RealClientFrom)
+		to.RealClientFrom = "auto"
+	}
+
 	return to
 }
 

controllers/nginx/pkg/template/template.go

@@ -32,6 +32,7 @@ import (
 
 	"github.com/pborman/uuid"
 
+	apiv1 "k8s.io/api/core/v1"
 	extensions "k8s.io/api/extensions/v1beta1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/ingress/controllers/nginx/pkg/config"
@@ -151,6 +152,8 @@ var (
 		},
 		"isValidClientBodyBufferSize": isValidClientBodyBufferSize,
 		"buildForwardedFor":           buildForwardedFor,
+		"trustHTTPHeaders":            trustHTTPHeaders,
+		"trustProxyProtocol":          trustProxyProtocol,
 	}
 )
 
@@ -666,3 +669,27 @@ func buildForwardedFor(input interface{}) string {
 	ffh = strings.ToLower(ffh)
 	return fmt.Sprintf("$http_%v", ffh)
 }
+
+func trustHTTPHeaders(input interface{}) bool {
+	conf, ok := input.(config.TemplateConfig)
+	if !ok {
+		glog.Errorf("%v", input)
+		return true
+	}
+
+	return conf.Cfg.RealClientFrom == "http-proxy" ||
+		(conf.Cfg.RealClientFrom == "auto" && !conf.Cfg.UseProxyProtocol ||
+			(conf.Cfg.RealClientFrom == "auto" && conf.PublishService != nil &&
+				conf.PublishService.Spec.Type == apiv1.ServiceTypeLoadBalancer))
+}
+
+func trustProxyProtocol(input interface{}) bool {
+	conf, ok := input.(config.TemplateConfig)
+	if !ok {
+		glog.Errorf("%v", input)
+		return true
+	}
+
+	return conf.Cfg.RealClientFrom == "tcp-proxy" ||
+		(conf.Cfg.RealClientFrom == "auto" && conf.Cfg.UseProxyProtocol)
+}

controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl

@@ -151,6 +151,14 @@ http {
         ''               close;
     }
 
+    {{ if (trustHTTPHeaders $all) }}
+    # Trust HTTP X-Forwarded-* Headers, but use direct values if they're missing.
+    map {{ buildForwardedFor $cfg.ForwardedForHeader }} $the_real_ip {
+        # Get IP address from X-Forwarded-For HTTP header
+        default          $realip_remote_addr;
+        ''               $remote_addr;
+    }
+
     # trust http_x_forwarded_proto headers correctly indicate ssl offloading
     map $http_x_forwarded_proto $pass_access_scheme {
         default          $http_x_forwarded_proto;
@@ -158,20 +166,39 @@ http {
     }
 
     map $http_x_forwarded_port $pass_server_port {
-       default           $http_x_forwarded_port;
-       ''                $server_port;
+        default           $http_x_forwarded_port;
+        ''                $server_port;
+    }
+
+    map $http_x_forwarded_host $best_http_host {
+        default          $http_x_forwarded_host;
+        ''               $this_host;
     }
 
-    map {{ buildForwardedFor $cfg.ForwardedForHeader }} $the_real_ip {
-        default          {{ buildForwardedFor $cfg.ForwardedForHeader }};
-        "~*(?<ip>[0-9\.]+).*"     $ip;
-    {{ if $cfg.UseProxyProtocol }}
-        ''               $proxy_protocol_addr;
     {{ else }}
-        ''               $realip_remote_addr;
+    # Do not trust HTTP X-Forwarded-* Headers
+    map {{ buildForwardedFor $cfg.ForwardedForHeader }} $the_real_ip {
+    {{ if (trustProxyProtocol $cfg) }}
+        # Get IP address from Proxy Protocol
+        default          $proxy_protocol_addr;
+    {{ else }}
+        # Get IP from direct remote address
+        default          $realip_remote_addr;
     {{ end }}
     }
 
+    map $http_x_forwarded_host $best_http_host {
+        default          $this_host;
+    }
+    map $http_x_forwarded_proto $pass_access_scheme {
+        default          $scheme;
+    }
+    map $http_x_forwarded_port $pass_server_port {
+        default           $server_port;
+    }
+
+    {{ end }}
+
     {{ if $all.IsSSLPassthroughEnabled }}
     # map port {{ $all.ListenPorts.SSLProxy }} to 443 for header X-Forwarded-Port
     map $pass_server_port $pass_port {
@@ -206,11 +233,6 @@ http {
         ''               $host;
     }
 
-    map $http_x_forwarded_host $best_http_host {
-        default          $http_x_forwarded_host;
-        ''               $this_host;
-    }
-
     server_name_in_redirect off;
     port_in_redirect        off;
 
@@ -270,10 +292,12 @@ http {
 
         {{ range $server := $upstream.Endpoints }}server {{ $server.Address | formatIP }}:{{ $server.Port }} max_fails={{ $server.MaxFails }} fail_timeout={{ $server.FailTimeout }};
         {{ end }}
+
     }
 
     {{ end }}
 
+
     upstream {{ $upstream.Name }} {
         # Load balance algorithm; empty for round robin, which is the default
         {{ if ne $cfg.LoadBalanceAlgorithm "round_robin" }}
@@ -364,7 +388,7 @@ http {
 
         {{ template "CUSTOM_ERRORS" $all }}
     }
-    
+
     {{ if $server.Alias }}
     server {
         server_name {{ $server.Alias }};