From 22a89c12cb698026631b1578c9546dd561bf6c65 Mon Sep 17 00:00:00 2001 From: "Gerasimos (Makis) Maropoulos" Date: Sun, 26 Jul 2020 14:37:30 +0300 Subject: [PATCH] Add Configuration.RemoteAddrHeadersForce as requested at #1567 and change RemoteAddrHeaders from map to string slice Read HISTORY.md entry --- HISTORY.md | 3 + .../from-toml-file/configs/iris.tml | 2 +- .../from-yaml-file/configs/iris.yml | 6 +- configuration.go | 99 ++++++++++--------- configuration_test.go | 39 ++++---- context/configuration.go | 4 +- context/context.go | 32 ++++-- 7 files changed, 101 insertions(+), 84 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index dac34915..3c1f9671 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -375,6 +375,8 @@ Other Improvements: ![DBUG routes](https://iris-go.com/images/v12.2.0-dbug2.png?v=0) +- Add `Configuration.RemoteAddrHeadersForce bool` to force `Context.RemoteAddr() string` to return the first entry of request headers as a fallback instead of the `Request.RemoteAddr` one, as requested at: [1567#issuecomment-663972620](https://github.com/kataras/iris/issues/1567#issuecomment-663972620). + - Fix [#1569#issuecomment-663739177](https://github.com/kataras/iris/issues/1569#issuecomment-663739177). - Fix [#1564](https://github.com/kataras/iris/issues/1564). @@ -516,6 +518,7 @@ New Context Methods: Breaking Changes: +- `Configuration.RemoteAddrHeaders` from `map[string]bool` to `[]string`. If you used `With(out)RemoteAddrHeader` then you are ready to proceed without any code changes for that one. - `ctx.Gzip(boolean)` replaced with `ctx.CompressWriter(boolean) error`. - `ctx.GzipReader(boolean) error` replaced with `ctx.CompressReader(boolean) error`. - `iris.Gzip` and `iris.GzipReader` replaced with `iris.Compression` (middleware). diff --git a/_examples/configuration/from-toml-file/configs/iris.tml b/_examples/configuration/from-toml-file/configs/iris.tml index 449aed92..6eee1e11 100644 --- a/_examples/configuration/from-toml-file/configs/iris.tml +++ b/_examples/configuration/from-toml-file/configs/iris.tml @@ -4,6 +4,6 @@ FireMethodNotAllowed = true DisableBodyConsumptionOnUnmarshal = false TimeFormat = "Mon, 01 Jan 2006 15:04:05 GMT" Charset = "utf-8" - +RemoteAddrHeaders = ["X-Real-Ip", "X-Forwarded-For", "CF-Connecting-IP"] [Other] MyServerName = "iris" diff --git a/_examples/configuration/from-yaml-file/configs/iris.yml b/_examples/configuration/from-yaml-file/configs/iris.yml index c0c764d1..e68bbdf6 100644 --- a/_examples/configuration/from-yaml-file/configs/iris.yml +++ b/_examples/configuration/from-yaml-file/configs/iris.yml @@ -9,8 +9,8 @@ SSLProxyHeaders: HostProxyHeaders: X-Host: true RemoteAddrHeaders: - X-Real-Ip: true - X-Forwarded-For: true - CF-Connecting-IP: true + - X-Real-Ip + - X-Forwarded-For + - CF-Connecting-IP Other: Addr: :8080 diff --git a/configuration.go b/configuration.go index 202ffd2e..596cb541 100644 --- a/configuration.go +++ b/configuration.go @@ -351,48 +351,38 @@ func WithPostMaxMemory(limit int64) Configurator { } } -// WithRemoteAddrHeader enables or adds a new or existing request header name +// WithRemoteAddrHeader adds a new request header name // that can be used to validate the client's real IP. -// -// By-default no "X-" header is consired safe to be used for retrieving the -// client's IP address, because those headers can manually change by -// the client. But sometimes are useful e.g., when behind a proxy -// you want to enable the "X-Forwarded-For" or when cloudflare -// you want to enable the "CF-Connecting-IP", inneed you -// can allow the `ctx.RemoteAddr()` to use any header -// that the client may sent. -// -// Defaults to an empty map but an example usage is: -// WithRemoteAddrHeader("X-Forwarded-For", "CF-Connecting-IP") -// -// Look `context.RemoteAddr()` for more. func WithRemoteAddrHeader(header ...string) Configurator { return func(app *Application) { - if app.config.RemoteAddrHeaders == nil { - app.config.RemoteAddrHeaders = make(map[string]bool) - } + for _, h := range header { + exists := false + for _, v := range app.config.RemoteAddrHeaders { + if v == h { + exists = true + } + } - for _, k := range header { - app.config.RemoteAddrHeaders[k] = true + if !exists { + app.config.RemoteAddrHeaders = append(app.config.RemoteAddrHeaders, h) + } } } } -// WithoutRemoteAddrHeader disables an existing request header name +// WithoutRemoteAddrHeader removes an existing request header name // that can be used to validate and parse the client's real IP. // -// -// Keep note that RemoteAddrHeaders is already defaults to an empty map -// so you don't have to call this Configurator if you didn't -// add allowed headers via configuration or via `WithRemoteAddrHeader` before. -// // Look `context.RemoteAddr()` for more. func WithoutRemoteAddrHeader(headerName string) Configurator { return func(app *Application) { - if app.config.RemoteAddrHeaders == nil { - app.config.RemoteAddrHeaders = make(map[string]bool) + tmp := app.config.RemoteAddrHeaders[:0] + for _, v := range app.config.RemoteAddrHeaders { + if v != headerName { + tmp = append(tmp, v) + } } - app.config.RemoteAddrHeaders[headerName] = false + app.config.RemoteAddrHeaders = tmp } } @@ -783,21 +773,27 @@ type Configuration struct { // that can be valid to parse the client's IP based on. // By-default no "X-" header is consired safe to be used for retrieving the // client's IP address, because those headers can manually change by - // the client. But sometimes are useful e.g., when behind a proxy + // the client. But sometimes are useful e.g. when behind a proxy // you want to enable the "X-Forwarded-For" or when cloudflare - // you want to enable the "CF-Connecting-IP", inneed you + // you want to enable the "CF-Connecting-IP", indeed you // can allow the `ctx.RemoteAddr()` to use any header // that the client may sent. // - // Defaults to an empty map but an example usage is: + // Defaults to an empty slice but an example usage is: // RemoteAddrHeaders { - // "X-Real-Ip": true, - // "X-Forwarded-For": true, - // "CF-Connecting-IP": true, + // "X-Real-Ip", + // "X-Forwarded-For", + // "CF-Connecting-IP", // } // // Look `context.RemoteAddr()` for more. - RemoteAddrHeaders map[string]bool `json:"remoteAddrHeaders,omitempty" yaml:"RemoteAddrHeaders" toml:"RemoteAddrHeaders"` + RemoteAddrHeaders []string `json:"remoteAddrHeaders,omitempty" yaml:"RemoteAddrHeaders" toml:"RemoteAddrHeaders"` + // RemoteAddrHeadersForce forces the `Context.RemoteAddr()` method + // to return the first entry of a request header as a fallback, + // even if that IP is a part of the `RemoteAddrPrivateSubnets` list. + // The default behavior, if a remote address is part of the `RemoteAddrPrivateSubnets`, + // is to retrieve the IP from the `Request.RemoteAddr` field instead. + RemoteAddrHeadersForce bool `json:"remoteAddrHeadersForce,omitempty" yaml:"RemoteAddrHeadersForce" toml:"RemoteAddrHeadersForce"` // RemoteAddrPrivateSubnets defines the private sub-networks. // They are used to be compared against // IP Addresses fetched through `RemoteAddrHeaders` or `Context.Request.RemoteAddr`. @@ -960,10 +956,15 @@ func (c Configuration) GetViewDataContextKey() string { } // GetRemoteAddrHeaders returns the RemoteAddrHeaders field. -func (c Configuration) GetRemoteAddrHeaders() map[string]bool { +func (c Configuration) GetRemoteAddrHeaders() []string { return c.RemoteAddrHeaders } +// GetRemoteAddrHeadersForce returns RemoteAddrHeadersForce field. +func (c Configuration) GetRemoteAddrHeadersForce() bool { + return c.RemoteAddrHeadersForce +} + // GetSSLProxyHeaders returns the SSLProxyHeaders field. func (c Configuration) GetSSLProxyHeaders() map[string]string { return c.SSLProxyHeaders @@ -1102,12 +1103,11 @@ func WithConfiguration(c Configuration) Configurator { } if v := c.RemoteAddrHeaders; len(v) > 0 { - if main.RemoteAddrHeaders == nil { - main.RemoteAddrHeaders = make(map[string]bool, len(v)) - } - for key, value := range v { - main.RemoteAddrHeaders[key] = value - } + main.RemoteAddrHeaders = v + } + + if v := c.RemoteAddrHeadersForce; v { + main.RemoteAddrHeadersForce = v } if v := c.RemoteAddrPrivateSubnets; len(v) > 0 { @@ -1165,13 +1165,14 @@ func DefaultConfiguration() Configuration { // The request body the size limit // can be set by the middleware `LimitRequestBodySize` // or `context#SetMaxRequestBodySize`. - PostMaxMemory: 32 << 20, // 32MB - LocaleContextKey: "iris.locale", - LanguageContextKey: "iris.locale.language", - VersionContextKey: "iris.api.version", - ViewLayoutContextKey: "iris.viewLayout", - ViewDataContextKey: "iris.viewData", - RemoteAddrHeaders: make(map[string]bool), + PostMaxMemory: 32 << 20, // 32MB + LocaleContextKey: "iris.locale", + LanguageContextKey: "iris.locale.language", + VersionContextKey: "iris.api.version", + ViewLayoutContextKey: "iris.viewLayout", + ViewDataContextKey: "iris.viewData", + RemoteAddrHeaders: nil, + RemoteAddrHeadersForce: false, RemoteAddrPrivateSubnets: []netutil.IPRange{ { Start: net.ParseIP("10.0.0.0"), diff --git a/configuration_test.go b/configuration_test.go index 24c552a0..ae0e3edf 100644 --- a/configuration_test.go +++ b/configuration_test.go @@ -154,9 +154,9 @@ DisableBodyConsumptionOnUnmarshal: true TimeFormat: "Mon, 02 Jan 2006 15:04:05 GMT" Charset: "utf-8" RemoteAddrHeaders: - X-Real-Ip: true - X-Forwarded-For: true - CF-Connecting-IP: true + - X-Real-Ip + - X-Forwarded-For + - CF-Connecting-IP HostProxyHeaders: X-Host: true SSLProxyHeaders: @@ -210,19 +210,19 @@ Other: t.Fatalf("error on TestConfigurationYAML: Expected RemoteAddrHeaders to be filled") } - expectedRemoteAddrHeaders := map[string]bool{ - "X-Real-Ip": true, - "X-Forwarded-For": true, - "CF-Connecting-IP": true, + expectedRemoteAddrHeaders := []string{ + "X-Real-Ip", + "X-Forwarded-For", + "CF-Connecting-IP", } if expected, got := len(c.RemoteAddrHeaders), len(expectedRemoteAddrHeaders); expected != got { t.Fatalf("error on TestConfigurationYAML: Expected RemoteAddrHeaders' len(%d) and got(%d), len is not the same", expected, got) } - for k, v := range c.RemoteAddrHeaders { - if expected, got := expectedRemoteAddrHeaders[k], v; expected != got { - t.Fatalf("error on TestConfigurationYAML: Expected RemoteAddrHeaders[%s] = %t but got %t", k, expected, got) + for i, v := range c.RemoteAddrHeaders { + if expected, got := expectedRemoteAddrHeaders[i], v; expected != got { + t.Fatalf("error on TestConfigurationYAML: Expected RemoteAddrHeaders[%d] = %s but got %s", i, expected, got) } } @@ -285,10 +285,7 @@ DisableBodyConsumptionOnUnmarshal = true TimeFormat = "Mon, 02 Jan 2006 15:04:05 GMT" Charset = "utf-8" -[RemoteAddrHeaders] - X-Real-Ip = true - X-Forwarded-For = true - CF-Connecting-IP = true +RemoteAddrHeaders = ["X-Real-Ip", "X-Forwarded-For", "CF-Connecting-IP"] [Other] # Indentation (tabs and/or spaces) is allowed but not required @@ -337,19 +334,19 @@ Charset = "utf-8" t.Fatalf("error on TestConfigurationTOML: Expected RemoteAddrHeaders to be filled") } - expectedRemoteAddrHeaders := map[string]bool{ - "X-Real-Ip": true, - "X-Forwarded-For": true, - "CF-Connecting-IP": true, + expectedRemoteAddrHeaders := []string{ + "X-Real-Ip", + "X-Forwarded-For", + "CF-Connecting-IP", } if expected, got := len(c.RemoteAddrHeaders), len(expectedRemoteAddrHeaders); expected != got { t.Fatalf("error on TestConfigurationTOML: Expected RemoteAddrHeaders' len(%d) and got(%d), len is not the same", expected, got) } - for k, v := range c.RemoteAddrHeaders { - if expected, got := expectedRemoteAddrHeaders[k], v; expected != got { - t.Fatalf("error on TestConfigurationTOML: Expected RemoteAddrHeaders[%s] = %t but got %t", k, expected, got) + for i, got := range c.RemoteAddrHeaders { + if expected := expectedRemoteAddrHeaders[i]; expected != got { + t.Fatalf("error on TestConfigurationTOML: Expected RemoteAddrHeaders[%d] = %s but got %s", i, expected, got) } } diff --git a/context/configuration.go b/context/configuration.go index 557ac51c..26103e47 100644 --- a/context/configuration.go +++ b/context/configuration.go @@ -57,7 +57,9 @@ type ConfigurationReadOnly interface { GetViewDataContextKey() string // GetRemoteAddrHeaders returns RemoteAddrHeaders field. - GetRemoteAddrHeaders() map[string]bool + GetRemoteAddrHeaders() []string + // GetRemoteAddrHeadersForce returns RemoteAddrHeadersForce field. + GetRemoteAddrHeadersForce() bool // GetRemoteAddrPrivateSubnets returns the RemoteAddrPrivateSubnets field. GetRemoteAddrPrivateSubnets() []netutil.IPRange // GetSSLProxyHeaders returns the SSLProxyHeaders field. diff --git a/context/context.go b/context/context.go index 58e5ea5a..e48ce4a2 100644 --- a/context/context.go +++ b/context/context.go @@ -803,24 +803,38 @@ func (ctx *Context) FullRequestURI() string { // Based on allowed headers names that can be modified from Configuration.RemoteAddrHeaders. // // If parse based on these headers fail then it will return the Request's `RemoteAddr` field -// which is filled by the server before the HTTP handler. +// which is filled by the server before the HTTP handler, +// unless the Configuration.RemoteAddrHeadersForce was set to true +// which will force this method to return the first IP from RemoteAddrHeaders +// even if it's part of a private network. // // Look `Configuration.RemoteAddrHeaders`, +// `Configuration.RemoteAddrHeadersForce`, // `Configuration.WithRemoteAddrHeader(...)`, // `Configuration.WithoutRemoteAddrHeader(...)` and // `Configuration.RemoteAddrPrivateSubnets` for more. func (ctx *Context) RemoteAddr() string { - remoteHeaders := ctx.app.ConfigurationReadOnly().GetRemoteAddrHeaders() - privateSubnets := ctx.app.ConfigurationReadOnly().GetRemoteAddrPrivateSubnets() + if remoteHeaders := ctx.app.ConfigurationReadOnly().GetRemoteAddrHeaders(); len(remoteHeaders) > 0 { + privateSubnets := ctx.app.ConfigurationReadOnly().GetRemoteAddrPrivateSubnets() - for headerName, enabled := range remoteHeaders { - if !enabled { - continue + for _, headerName := range remoteHeaders { + ipAddresses := strings.Split(ctx.GetHeader(headerName), ",") + if ip, ok := netutil.GetIPAddress(ipAddresses, privateSubnets); ok { + return ip + } } - ipAddresses := strings.Split(ctx.GetHeader(headerName), ",") - if ip, ok := netutil.GetIPAddress(ipAddresses, privateSubnets); ok { - return ip + if ctx.app.ConfigurationReadOnly().GetRemoteAddrHeadersForce() { + for _, headerName := range remoteHeaders { + // return the first valid IP, + // even if it's a part of a private network. + ipAddresses := strings.Split(ctx.GetHeader(headerName), ",") + for _, addr := range ipAddresses { + if ip, _, err := net.SplitHostPort(addr); err == nil { + return ip + } + } + } } }