From 075523146925df788e27910d0b447167268491c4 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 5 Oct 2017 22:55:10 -0300 Subject: [PATCH] Fix source IP address --- controllers/nginx/README.md | 8 +++ controllers/nginx/pkg/config/config.go | 6 -- controllers/nginx/pkg/template/configmap.go | 10 ---- controllers/nginx/pkg/template/template.go | 28 +--------- .../nginx/pkg/template/template_test.go | 4 +- .../rootfs/etc/nginx/template/nginx.tmpl | 55 ++++--------------- 6 files changed, 22 insertions(+), 89 deletions(-) diff --git a/controllers/nginx/README.md b/controllers/nginx/README.md index c92a7c5991..0eafcef9ab 100644 --- a/controllers/nginx/README.md +++ b/controllers/nginx/README.md @@ -14,6 +14,7 @@ This is an nginx Ingress controller that uses [ConfigMap](https://kubernetes.io/ * [HTTPS enforcement](#server-side-https-enforcement) * [HSTS](#http-strict-transport-security) * [Kube-Lego](#automated-certificate-management-with-kube-lego) +* [Source IP address](#source-ip-address) * [TCP Services](#exposing-tcp-services) * [UDP Services](#exposing-udp-services) * [Proxy Protocol](#proxy-protocol) @@ -333,6 +334,13 @@ version to fully support Kube-Lego is nginx Ingress controller 0.8. [Kube-Lego]:https://github.com/jetstack/kube-lego [Let's Encrypt]:https://letsencrypt.org +## Source IP address + +By default NGINX uses the content of the header `X-Forwarded-For` as the source of truth to get information about the client IP address. This works without issues in L7 **if we configure the setting `proxy-real-ip-cidr`** with the correct information of the IP/network address of the external load balancer. +If the ingress controller is running in AWS we need to use the VPC IPv4 CIDR. This allows NGINX to avoid the spoofing of the header. +Another option is to enable proxy protocol using `use-proxy-protocol: "true"`. +In this mode NGINX do not uses the content of the header to get the source IP address of the connection. + ## Exposing TCP services Ingress does not support TCP services (yet). For this reason this Ingress controller uses the flag `--tcp-services-configmap` to point to an existing config map where the key is the external port to use and the value is `::[PROXY]:[PROXY]` diff --git a/controllers/nginx/pkg/config/config.go b/controllers/nginx/pkg/config/config.go index 3b4ddd6ec0..50f88f58f4 100644 --- a/controllers/nginx/pkg/config/config.go +++ b/controllers/nginx/pkg/config/config.go @@ -261,11 +261,6 @@ 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 @@ -479,7 +474,6 @@ func NewDefault() Configuration { LimitConnZoneVariable: defaultLimitConnZoneVariable, BindAddressIpv4: defBindAddress, BindAddressIpv6: defBindAddress, - RealClientFrom: "auto", ZipkinCollectorPort: 9411, ZipkinServiceName: "nginx", } diff --git a/controllers/nginx/pkg/template/configmap.go b/controllers/nginx/pkg/template/configmap.go index afed49b3ec..f07ef46b6e 100644 --- a/controllers/nginx/pkg/template/configmap.go +++ b/controllers/nginx/pkg/template/configmap.go @@ -19,7 +19,6 @@ package template import ( "fmt" "net" - "regexp" "strconv" "strings" @@ -39,10 +38,6 @@ 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{} @@ -125,11 +120,6 @@ 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 index 9fbc46bed4..cde9a2ac58 100644 --- a/controllers/nginx/pkg/template/template.go +++ b/controllers/nginx/pkg/template/template.go @@ -152,8 +152,6 @@ var ( }, "isValidClientBodyBufferSize": isValidClientBodyBufferSize, "buildForwardedFor": buildForwardedFor, - "trustHTTPHeaders": trustHTTPHeaders, - "trustProxyProtocol": trustProxyProtocol, "buildAuthSignURL": buildAuthSignURL, } ) @@ -671,28 +669,6 @@ func buildForwardedFor(input interface{}) string { 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) -} - -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) -} - func buildAuthSignURL(input interface{}) string { s, ok := input.(string) if !ok { @@ -703,12 +679,12 @@ func buildAuthSignURL(input interface{}) string { u, _ := url.Parse(s) q := u.Query() if len(q) == 0 { - return fmt.Sprintf("%v?rd=$request_uri", s) + return fmt.Sprintf("%v?rd=$scheme://$http_host$request_uri", s) } if q.Get("rd") != "" { return s } - return fmt.Sprintf("%v&rd=$request_uri", s) + return fmt.Sprintf("%v&rd=$scheme://$http_host$request_uri", s) } diff --git a/controllers/nginx/pkg/template/template_test.go b/controllers/nginx/pkg/template/template_test.go index e330b9ce60..1360eba4a1 100644 --- a/controllers/nginx/pkg/template/template_test.go +++ b/controllers/nginx/pkg/template/template_test.go @@ -359,8 +359,8 @@ func TestBuildAuthSignURL(t *testing.T) { cases := map[string]struct { Input, Output string }{ - "default url": {"http://google.com", "http://google.com?rd=$request_uri"}, - "with random field": {"http://google.com?cat=0", "http://google.com?cat=0&rd=$request_uri"}, + "default url": {"http://google.com", "http://google.com?rd=$scheme://$http_host$request_uri"}, + "with random field": {"http://google.com?cat=0", "http://google.com?cat=0&rd=$scheme://$http_host$request_uri"}, "with rd field": {"http://google.com?cat&rd=$request", "http://google.com?cat&rd=$request"}, } for k, tc := range cases { diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl index 5d4fd19745..3b76e25c21 100644 --- a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl +++ b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl @@ -151,12 +151,13 @@ 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; + {{ if $cfg.UseProxyProtocol }} + # Get IP address from Proxy Protocol + default $proxy_protocol_addr; + {{ else }} + default $remote_addr; + {{ end }} } # trust http_x_forwarded_proto headers correctly indicate ssl offloading @@ -175,30 +176,6 @@ http { '' $this_host; } - {{ else }} - # Do not trust HTTP X-Forwarded-* Headers - map {{ buildForwardedFor $cfg.ForwardedForHeader }} $the_real_ip { - {{ if (trustProxyProtocol $all) }} - # 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 { @@ -212,21 +189,6 @@ http { } {{ end }} - # Map a response error watching the header Content-Type - map $http_accept $httpAccept { - default html; - application/json json; - application/xml xml; - text/plain text; - } - - map $httpAccept $httpReturnType { - default text/html; - json application/json; - xml application/xml; - text text/plain; - } - # Obtain best http host map $http_host $this_host { default $http_host; @@ -688,8 +650,8 @@ stream { {{ end }} - location {{ $path }} { + location {{ $path }} { {{ if $all.Cfg.EnableVtsStatus }}{{ if $location.VtsFilterKey }} vhost_traffic_status_filter_by_set_key {{ $location.VtsFilterKey }};{{ end }}{{ end }} set $proxy_upstream_name "{{ buildUpstreamName $server.Hostname $all.Backends $location }}"; @@ -786,6 +748,9 @@ stream { proxy_set_header X-Original-URI $request_uri; proxy_set_header X-Scheme $pass_access_scheme; + # Pass the original X-Forwarded-For + proxy_set_header X-Original-Forwarded-For {{ buildForwardedFor $all.Cfg.ForwardedForHeader }}; + # mitigate HTTPoxy Vulnerability # https://www.nginx.com/blog/mitigating-the-httpoxy-vulnerability-with-nginx/ proxy_set_header Proxy "";