diff --git a/README.md b/README.md index 0c2632441b..abca79064d 100644 --- a/README.md +++ b/README.md @@ -2117,6 +2117,43 @@ func main() { } ``` +## Don't trust all proxies + +Gin lets you specify which headers to hold the real client IP (if any), +as well as specifying which proxies (or direct clients) you trust to +specify one of these headers. + +The `TrustedProxies` slice on your `gin.Engine` specifes the clients +truest to specify unspoofed client IP headers. Proxies can be specified +as IP's, CIDR's, or hostnames. Hostnames are resolved on each query, +such that changes in your proxy pool take effect immediately. The +hostname option is handy, but also costly, so only use if you have no +other option. + +```go +import ( + "fmt" + + "github.com/gin-gonic/gin" +) + +func main() { + + router := gin.Default() + router.TrustedProxies = []string{"192.168.1.2"} + + router.GET("/", func(c *gin.Context) { + + // If the client is 192.168.1.2, use the X-Forwarded-For + // header to deduce the original client IP from the trust- + // worthy parts of that header. + // Otherwise, simply return the direct client IP + fmt.Printf("ClientIP: %s\n", c.ClientIP()) + }) + + router.Run() +} +``` ## Testing diff --git a/context.go b/context.go index 3d6b56d6b3..9e7a65782d 100644 --- a/context.go +++ b/context.go @@ -729,14 +729,12 @@ func (c *Context) ShouldBindBodyWith(obj interface{}, bb binding.BindingBody) (e // X-Real-IP and X-Forwarded-For in order to work properly with reverse-proxies such us: nginx or haproxy. // Use X-Forwarded-For before X-Real-Ip as nginx uses X-Real-Ip with the proxy's IP. func (c *Context) ClientIP() string { - if c.engine.ForwardedByClientIP { - clientIP := c.requestHeader("X-Forwarded-For") - clientIP = strings.TrimSpace(strings.Split(clientIP, ",")[0]) - if clientIP == "" { - clientIP = strings.TrimSpace(c.requestHeader("X-Real-Ip")) - } - if clientIP != "" { - return clientIP + if c.engine.ForwardedByClientIP && c.engine.RemoteIPHeaders != nil { + for _, header := range c.engine.RemoteIPHeaders { + ipChain := filterIPsFromUntrustedProxies(c.requestHeader(header), c.Request, c.engine) + if len(ipChain) > 0 { + return ipChain[0] + } } } @@ -746,11 +744,82 @@ func (c *Context) ClientIP() string { } } - if ip, _, err := net.SplitHostPort(strings.TrimSpace(c.Request.RemoteAddr)); err == nil { - return ip + ip, _ := getTransportPeerIPForRequest(c.Request) + + return ip +} + +func filterIPsFromUntrustedProxies(XForwardedForHeader string, req *http.Request, e *Engine) []string { + var items, out []string + if XForwardedForHeader != "" { + items = strings.Split(XForwardedForHeader, ",") + } else { + return []string{} + } + if peerIP, err := getTransportPeerIPForRequest(req); err == nil { + items = append(items, peerIP) } - return "" + for i := len(items) - 1; i >= 0; i-- { + item := strings.TrimSpace(items[i]) + ip := net.ParseIP(item) + if ip == nil { + return out + } + + out = prependString(ip.String(), out) + if !isTrustedProxy(ip, e) { + return out + } + // out = prependString(ip.String(), out) + } + return out +} + +func isTrustedProxy(ip net.IP, e *Engine) bool { + for _, trustedProxy := range e.TrustedProxies { + if _, ipnet, err := net.ParseCIDR(trustedProxy); err == nil { + if ipnet.Contains(ip) { + return true + } + continue + } + + if proxyIP := net.ParseIP(trustedProxy); proxyIP != nil { + if proxyIP.Equal(ip) { + return true + } + continue + } + + if addrs, err := e.lookupHost(trustedProxy); err == nil { + for _, proxyAddr := range addrs { + proxyIP := net.ParseIP(proxyAddr) + if proxyIP == nil { + continue + } + if proxyIP.Equal(ip) { + return true + } + } + } + } + return false +} + +func prependString(ip string, ipList []string) []string { + ipList = append(ipList, "") + copy(ipList[1:], ipList) + ipList[0] = string(ip) + return ipList +} + +func getTransportPeerIPForRequest(req *http.Request) (string, error) { + var err error + if ip, _, err := net.SplitHostPort(strings.TrimSpace(req.RemoteAddr)); err == nil { + return ip, nil + } + return "", err } // ContentType returns the Content-Type header of the request. diff --git a/context_test.go b/context_test.go index 8e1e3b57ff..16c4dd2861 100644 --- a/context_test.go +++ b/context_test.go @@ -1392,11 +1392,23 @@ func TestContextClientIP(t *testing.T) { c, _ := CreateTestContext(httptest.NewRecorder()) c.Request, _ = http.NewRequest("POST", "/", nil) - c.Request.Header.Set("X-Real-IP", " 10.10.10.10 ") - c.Request.Header.Set("X-Forwarded-For", " 20.20.20.20, 30.30.30.30") - c.Request.Header.Set("X-Appengine-Remote-Addr", "50.50.50.50") - c.Request.RemoteAddr = " 40.40.40.40:42123 " + c.engine.lookupHost = func(host string) ([]string, error) { + if host == "foo" { + return []string{"40.40.40.40", "30.30.30.30"}, nil + } + if host == "bar" { + return nil, errors.New("hostname lookup failed") + } + if host == "baz" { + return []string{"thisshouldneverhappen"}, nil + } + return []string{}, nil + } + + resetContextForClientIPTests(c) + // Legacy tests (validating that the defaults don't break the + // (insecure!) old behaviour) assert.Equal(t, "20.20.20.20", c.ClientIP()) c.Request.Header.Del("X-Forwarded-For") @@ -1416,6 +1428,74 @@ func TestContextClientIP(t *testing.T) { // no port c.Request.RemoteAddr = "50.50.50.50" assert.Empty(t, c.ClientIP()) + + // Tests exercising the TrustedProxies functionality + resetContextForClientIPTests(c) + + // No trusted proxies + c.engine.TrustedProxies = []string{} + c.engine.RemoteIPHeaders = []string{"X-Forwarded-For"} + assert.Equal(t, "40.40.40.40", c.ClientIP()) + + // Last proxy is trusted, but the RemoteAddr is not + c.engine.TrustedProxies = []string{"30.30.30.30"} + assert.Equal(t, "40.40.40.40", c.ClientIP()) + + // Only trust RemoteAddr + c.engine.TrustedProxies = []string{"40.40.40.40"} + assert.Equal(t, "30.30.30.30", c.ClientIP()) + + // All steps are trusted + c.engine.TrustedProxies = []string{"40.40.40.40", "30.30.30.30", "20.20.20.20"} + assert.Equal(t, "20.20.20.20", c.ClientIP()) + + // Use CIDR + c.engine.TrustedProxies = []string{"40.40.25.25/16", "30.30.30.30"} + assert.Equal(t, "20.20.20.20", c.ClientIP()) + + // Use hostname that resolves to all the proxies + c.engine.TrustedProxies = []string{"foo"} + assert.Equal(t, "20.20.20.20", c.ClientIP()) + + // Use hostname that returns an error + c.engine.TrustedProxies = []string{"bar"} + assert.Equal(t, "40.40.40.40", c.ClientIP()) + + // X-Forwarded-For has a non-IP element + c.engine.TrustedProxies = []string{"40.40.40.40"} + c.Request.Header.Set("X-Forwarded-For", " blah ") + assert.Equal(t, "40.40.40.40", c.ClientIP()) + + // Result from LookupHost has non-IP element. This should never + // happen, but we should test it to make sure we handle it + // gracefully. + c.engine.TrustedProxies = []string{"baz"} + c.Request.Header.Set("X-Forwarded-For", " 30.30.30.30 ") + assert.Equal(t, "40.40.40.40", c.ClientIP()) + + c.engine.TrustedProxies = []string{"40.40.40.40"} + c.Request.Header.Del("X-Forwarded-For") + c.engine.RemoteIPHeaders = []string{"X-Forwarded-For", "X-Real-IP"} + assert.Equal(t, "10.10.10.10", c.ClientIP()) + + c.engine.RemoteIPHeaders = []string{} + c.engine.AppEngine = true + assert.Equal(t, "50.50.50.50", c.ClientIP()) + + c.Request.Header.Del("X-Appengine-Remote-Addr") + assert.Equal(t, "40.40.40.40", c.ClientIP()) + + // no port + c.Request.RemoteAddr = "50.50.50.50" + assert.Empty(t, c.ClientIP()) +} + +func resetContextForClientIPTests(c *Context) { + c.Request.Header.Set("X-Real-IP", " 10.10.10.10 ") + c.Request.Header.Set("X-Forwarded-For", " 20.20.20.20, 30.30.30.30") + c.Request.Header.Set("X-Appengine-Remote-Addr", "50.50.50.50") + c.Request.RemoteAddr = " 40.40.40.40:42123 " + c.engine.AppEngine = false } func TestContextContentType(t *testing.T) { diff --git a/gin.go b/gin.go index 1e12617904..876bfe8a69 100644 --- a/gin.go +++ b/gin.go @@ -82,6 +82,8 @@ type Engine struct { // handler. HandleMethodNotAllowed bool ForwardedByClientIP bool + RemoteIPHeaders []string + TrustedProxies []string // #726 #755 If enabled, it will thrust some headers starting with // 'X-AppEngine...' for better integration with that PaaS. @@ -103,6 +105,8 @@ type Engine struct { // See the PR #1817 and issue #1644 RemoveExtraSlash bool + lookupHost func(string) ([]string, error) + delims render.Delims secureJSONPrefix string HTMLRender render.HTMLRender @@ -139,11 +143,14 @@ func New() *Engine { RedirectFixedPath: false, HandleMethodNotAllowed: false, ForwardedByClientIP: true, + RemoteIPHeaders: []string{"X-Forwarded-For", "X-Real-IP"}, + TrustedProxies: []string{"0.0.0.0/0"}, AppEngine: defaultAppEngine, UseRawPath: false, RemoveExtraSlash: false, UnescapePathValues: true, MaxMultipartMemory: defaultMultipartMemory, + lookupHost: net.LookupHost, trees: make(methodTrees, 0, 9), delims: render.Delims{Left: "{{", Right: "}}"}, secureJSONPrefix: "while(1);",