Refactor X-Forwarded-* headers

kubernetes · Sep 17, 2017 · 2e9f4b7 · 2e9f4b7
1 parent 2c3b29c
commit 2e9f4b7
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 14 deletions.
diff --git a/controllers/nginx/pkg/cmd/controller/nginx.go b/controllers/nginx/pkg/cmd/controller/nginx.go
@@ -35,7 +35,6 @@ import (
 	"github.com/spf13/pflag"
 
 	proxyproto "github.com/armon/go-proxyproto"
-	api "k8s.io/api/core/v1"
 	api_v1 "k8s.io/api/core/v1"
 	extensions "k8s.io/api/extensions/v1beta1"
 
@@ -309,7 +308,7 @@ func (n NGINXController) DefaultEndpoint() ingress.Endpoint {
 	return ingress.Endpoint{
 		Address: "127.0.0.1",
 		Port:    fmt.Sprintf("%v", n.ports.Default),
-		Target:  &api.ObjectReference{},
+		Target:  &api_v1.ObjectReference{},
 	}
 }
 
@@ -657,6 +656,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)

diff --git a/controllers/nginx/pkg/config/config.go b/controllers/nginx/pkg/config/config.go
@@ -23,6 +23,8 @@ import (
 
 	"github.com/golang/glog"
 
+	api "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
@@ -486,6 +493,7 @@ type TemplateConfig struct {
 	IsSSLPassthroughEnabled bool
 	RedirectServers         map[string]string
 	ListenPorts             *ListenPorts
+	PublishService          *api.Service
 }
 
 // ListenPorts describe the ports required to run the

diff --git a/controllers/nginx/pkg/template/configmap.go b/controllers/nginx/pkg/template/configmap.go
@@ -19,6 +19,7 @@ package template
 import (
 	"fmt"
 	"net"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -37,6 +38,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{}
@@ -119,6 +124,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
 }
 

diff --git a/controllers/nginx/pkg/template/template.go b/controllers/nginx/pkg/template/template.go
@@ -32,6 +32,7 @@ import (
 	"github.com/golang/glog"
 	"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"
@@ -158,6 +159,8 @@ var (
 		"buildAuthSignURL":            buildAuthSignURL,
 		"isValidClientBodyBufferSize": isValidClientBodyBufferSize,
 		"buildForwardedFor":           buildForwardedFor,
+		"trustHTTPHeaders":            trustHTTPHeaders,
+		"trustProxyProtocol":          trustProxyProtocol,
 	}
 )
 
@@ -657,3 +660,24 @@ 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 {
+		return true
+	}
+
+	return conf.Cfg.RealClientFrom == "http-proxy" ||
+		(conf.Cfg.RealClientFrom == "auto" && !conf.Cfg.UseProxyProtocol &&
+			(conf.PublishService != nil && conf.PublishService.Spec.Type == apiv1.ServiceTypeLoadBalancer))
+}
+
+func trustProxyProtocol(input interface{}) bool {
+	conf, ok := input.(config.TemplateConfig)
+	if !ok {
+		return true
+	}
+
+	return conf.Cfg.RealClientFrom == "tcp-proxy" ||
+		(conf.Cfg.RealClientFrom == "auto" && !conf.Cfg.UseProxyProtocol)
+}
diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl
@@ -143,27 +143,59 @@ http {
         ''               close;
     }
 
+    {{ if (trustHTTPHeaders $cfg) }}
+    # 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          {{ buildForwardedFor $cfg.ForwardedForHeader }};
+        ''               $realip_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;
         ''               $scheme;
     }
 
     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;
+    }
+
+    {{ else }}
+    # Do not trust HTTP X-Forwarded-* Headers
     map {{ buildForwardedFor $cfg.ForwardedForHeader }} $the_real_ip {
-        default          {{ buildForwardedFor $cfg.ForwardedForHeader }};
-        "~*(?<ip>[0-9\.]+).*"     $ip;
-    {{ if $cfg.UseProxyProtocol }}
-        ''               $proxy_protocol_addr;
+    {{ if (trustProxyProtocol $cfg) }}
+        # Get IP address from Proxy Protocol
+    {{ if (ne (len $cfg.ProxyRealIPCIDR) 0) }}
+        # using trusted real IP CIDR
+        default          $realip_remote_addr;
     {{ else }}
-        ''               $realip_remote_addr;
+        default          $proxy_protocol_addr;
+    {{ end }}
+    {{ 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 {
@@ -198,11 +230,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;
 

diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go
@@ -410,6 +410,19 @@ func (ic GenericController) GetService(name string) (*api.Service, error) {
 	return s.(*api.Service), nil
 }
 
+// GetPublishService returns the service used to get status information
+func (ic GenericController) GetPublishService() *api.Service {
+	if ic.cfg.PublishService == "" {
+		return nil
+	}
+
+	svc, err := ic.GetService(ic.cfg.PublishService)
+	if err != nil {
+		return nil
+	}
+	return svc
+}
+
 func (ic *GenericController) getConfigMap(ns, name string) (*api.ConfigMap, error) {
 	s, exists, err := ic.mapLister.Store.GetByKey(fmt.Sprintf("%v/%v", ns, name))
 	if err != nil {