From 39181329dee3a14fa2210c72245811c07d5e72b6 Mon Sep 17 00:00:00 2001 From: Notealot <714804968@qq.com> Date: Wed, 6 Oct 2021 09:37:25 +0800 Subject: [PATCH 1/2] Tidy: Complete TrustedProxies feature (#2887) --- README.md | 12 +++++-- context.go | 2 +- context_test.go | 4 +++ gin.go | 70 +++++++++++++++++++++++------------------ gin_integration_test.go | 7 +++++ gin_test.go | 57 +++++++++++---------------------- 6 files changed, 80 insertions(+), 72 deletions(-) diff --git a/README.md b/README.md index 10be5e15..62094ff1 100644 --- a/README.md +++ b/README.md @@ -2202,11 +2202,17 @@ 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 network addresses or -network CIDRs from where clients which their request headers related to client +Use function `SetTrustedProxies()` on your `gin.Engine` to specify network addresses +or network CIDRs from where clients which their request headers related to client IP can be trusted. They can be IPv4 addresses, IPv4 CIDRs, IPv6 addresses or IPv6 CIDRs. +**Attention:** Gin trust all proxies by default if you don't specify a trusted +proxy using the function above, **this is NOT safe**. At the same time, if you don't +use any proxy, you can disable this feature by using `Engine.SetTrustedProxies(nil)`, +then `Context.ClientIP()` will return the remote address directly to avoid some +unnecessary computation. + ```go import ( "fmt" @@ -2217,7 +2223,7 @@ import ( func main() { router := gin.Default() - router.TrustedProxies = []string{"192.168.1.2"} + router.SetTrustedProxies([]string{"192.168.1.2"}) router.GET("/", func(c *gin.Context) { // If the client is 192.168.1.2, use the X-Forwarded-For diff --git a/context.go b/context.go index 370f6086..d211098b 100644 --- a/context.go +++ b/context.go @@ -778,7 +778,7 @@ func (c *Context) ClientIP() string { // RemoteIP parses the IP from Request.RemoteAddr, normalizes and returns the IP (without the port). // It also checks if the remoteIP is a trusted proxy or not. // In order to perform this validation, it will see if the IP is contained within at least one of the CIDR blocks -// defined in Engine.TrustedProxies +// defined by Engine.SetTrustedProxies() func (c *Context) RemoteIP() (net.IP, bool) { ip, _, err := net.SplitHostPort(strings.TrimSpace(c.Request.RemoteAddr)) if err != nil { diff --git a/context_test.go b/context_test.go index ad16fbb1..07a35f52 100644 --- a/context_test.go +++ b/context_test.go @@ -1409,6 +1409,10 @@ func TestContextClientIP(t *testing.T) { c.engine.RemoteIPHeaders = []string{"X-Forwarded-For"} assert.Equal(t, "40.40.40.40", c.ClientIP()) + // Disabled TrustedProxies feature + _ = c.engine.SetTrustedProxies(nil) + assert.Equal(t, "40.40.40.40", c.ClientIP()) + // Last proxy is trusted, but the RemoteAddr is not _ = c.engine.SetTrustedProxies([]string{"30.30.30.30"}) assert.Equal(t, "40.40.40.40", c.ClientIP()) diff --git a/gin.go b/gin.go index 02a1062c..af83161b 100644 --- a/gin.go +++ b/gin.go @@ -11,6 +11,7 @@ import ( "net/http" "os" "path" + "reflect" "strings" "sync" @@ -27,6 +28,8 @@ var ( var defaultPlatform string +var defaultTrustedCIDRs = []*net.IPNet{{IP: net.IP{0x0, 0x0, 0x0, 0x0}, Mask: net.IPMask{0x0, 0x0, 0x0, 0x0}}} // 0.0.0.0/0 + // HandlerFunc defines the handler used by gin middleware as return value. type HandlerFunc func(*Context) @@ -119,15 +122,9 @@ type Engine struct { // List of headers used to obtain the client IP when // `(*gin.Engine).ForwardedByClientIP` is `true` and // `(*gin.Context).Request.RemoteAddr` is matched by at least one of the - // network origins of `(*gin.Engine).TrustedProxies`. + // network origins of list defined by `(*gin.Engine).SetTrustedProxies()`. RemoteIPHeaders []string - // List of network origins (IPv4 addresses, IPv4 CIDRs, IPv6 addresses or - // IPv6 CIDRs) from which to trust request's headers that contain - // alternative client IP when `(*gin.Engine).ForwardedByClientIP` is - // `true`. - TrustedProxies []string - // If set to a constant of value gin.Platform*, trusts the headers set by // that platform, for example to determine the client IP TrustedPlatform string @@ -147,6 +144,7 @@ type Engine struct { pool sync.Pool trees methodTrees maxParams uint16 + trustedProxies []string trustedCIDRs []*net.IPNet } @@ -174,7 +172,6 @@ func New() *Engine { HandleMethodNotAllowed: false, ForwardedByClientIP: true, RemoteIPHeaders: []string{"X-Forwarded-For", "X-Real-IP"}, - TrustedProxies: []string{"0.0.0.0/0"}, TrustedPlatform: defaultPlatform, UseRawPath: false, RemoveExtraSlash: false, @@ -183,7 +180,8 @@ func New() *Engine { trees: make(methodTrees, 0, 9), delims: render.Delims{Left: "{{", Right: "}}"}, secureJSONPrefix: "while(1);", - trustedCIDRs: []*net.IPNet{{IP: net.IP{0x0, 0x0, 0x0, 0x0}, Mask: net.IPMask{0x0, 0x0, 0x0, 0x0}}}, + trustedProxies: []string{"0.0.0.0/0"}, + trustedCIDRs: defaultTrustedCIDRs, } engine.RouterGroup.engine = engine engine.pool.New = func() interface{} { @@ -342,9 +340,9 @@ func iterate(path, method string, routes RoutesInfo, root *node) RoutesInfo { func (engine *Engine) Run(addr ...string) (err error) { defer func() { debugPrintError(err) }() - err = engine.parseTrustedProxies() - if err != nil { - return err + if engine.isUnsafeTrustedProxies() { + debugPrint("[WARNING] You trusted all proxies, this is NOT safe. We recommend you to set a value.\n" + + "Please check https://pkg.go.dev/github.com/gin-gonic/gin#readme-don-t-trust-all-proxies for details.") } address := resolveAddress(addr) @@ -354,12 +352,12 @@ func (engine *Engine) Run(addr ...string) (err error) { } func (engine *Engine) prepareTrustedCIDRs() ([]*net.IPNet, error) { - if engine.TrustedProxies == nil { + if engine.trustedProxies == nil { return nil, nil } - cidr := make([]*net.IPNet, 0, len(engine.TrustedProxies)) - for _, trustedProxy := range engine.TrustedProxies { + cidr := make([]*net.IPNet, 0, len(engine.trustedProxies)) + for _, trustedProxy := range engine.trustedProxies { if !strings.Contains(trustedProxy, "/") { ip := parseIP(trustedProxy) if ip == nil { @@ -382,13 +380,25 @@ func (engine *Engine) prepareTrustedCIDRs() ([]*net.IPNet, error) { return cidr, nil } -// SetTrustedProxies set Engine.TrustedProxies +// SetTrustedProxies set a list of network origins (IPv4 addresses, +// IPv4 CIDRs, IPv6 addresses or IPv6 CIDRs) from which to trust +// request's headers that contain alternative client IP when +// `(*gin.Engine).ForwardedByClientIP` is `true`. `TrustedProxies` +// feature is enabled by default, and it also trusts all proxies +// by default. If you want to disable this feature, use +// Engine.SetTrustedProxies(nil), then Context.ClientIP() will +// return the remote address directly. func (engine *Engine) SetTrustedProxies(trustedProxies []string) error { - engine.TrustedProxies = trustedProxies + engine.trustedProxies = trustedProxies return engine.parseTrustedProxies() } -// parseTrustedProxies parse Engine.TrustedProxies to Engine.trustedCIDRs +// isUnsafeTrustedProxies compares Engine.trustedCIDRs and defaultTrustedCIDRs, it's not safe if equal (returns true) +func (engine *Engine) isUnsafeTrustedProxies() bool { + return reflect.DeepEqual(engine.trustedCIDRs, defaultTrustedCIDRs) +} + +// parseTrustedProxies parse Engine.trustedProxies to Engine.trustedCIDRs func (engine *Engine) parseTrustedProxies() error { trustedCIDRs, err := engine.prepareTrustedCIDRs() engine.trustedCIDRs = trustedCIDRs @@ -416,9 +426,9 @@ func (engine *Engine) RunTLS(addr, certFile, keyFile string) (err error) { debugPrint("Listening and serving HTTPS on %s\n", addr) defer func() { debugPrintError(err) }() - err = engine.parseTrustedProxies() - if err != nil { - return err + if engine.isUnsafeTrustedProxies() { + debugPrint("[WARNING] You trusted all proxies, this is NOT safe. We recommend you to set a value.\n" + + "Please check https://pkg.go.dev/github.com/gin-gonic/gin#readme-don-t-trust-all-proxies for details.") } err = http.ListenAndServeTLS(addr, certFile, keyFile, engine) @@ -432,9 +442,9 @@ func (engine *Engine) RunUnix(file string) (err error) { debugPrint("Listening and serving HTTP on unix:/%s", file) defer func() { debugPrintError(err) }() - err = engine.parseTrustedProxies() - if err != nil { - return err + if engine.isUnsafeTrustedProxies() { + debugPrint("[WARNING] You trusted all proxies, this is NOT safe. We recommend you to set a value.\n" + + "Please check https://pkg.go.dev/github.com/gin-gonic/gin#readme-don-t-trust-all-proxies for details.") } listener, err := net.Listen("unix", file) @@ -455,9 +465,9 @@ func (engine *Engine) RunFd(fd int) (err error) { debugPrint("Listening and serving HTTP on fd@%d", fd) defer func() { debugPrintError(err) }() - err = engine.parseTrustedProxies() - if err != nil { - return err + if engine.isUnsafeTrustedProxies() { + debugPrint("[WARNING] You trusted all proxies, this is NOT safe. We recommend you to set a value.\n" + + "Please check https://pkg.go.dev/github.com/gin-gonic/gin#readme-don-t-trust-all-proxies for details.") } f := os.NewFile(uintptr(fd), fmt.Sprintf("fd@%d", fd)) @@ -476,9 +486,9 @@ func (engine *Engine) RunListener(listener net.Listener) (err error) { debugPrint("Listening and serving HTTP on listener what's bind with address@%s", listener.Addr()) defer func() { debugPrintError(err) }() - err = engine.parseTrustedProxies() - if err != nil { - return err + if engine.isUnsafeTrustedProxies() { + debugPrint("[WARNING] You trusted all proxies, this is NOT safe. We recommend you to set a value.\n" + + "Please check https://pkg.go.dev/github.com/gin-gonic/gin#readme-don-t-trust-all-proxies for details.") } err = http.Serve(listener, engine) diff --git a/gin_integration_test.go b/gin_integration_test.go index 094c46e8..0b67b542 100644 --- a/gin_integration_test.go +++ b/gin_integration_test.go @@ -76,6 +76,12 @@ func TestRunEmpty(t *testing.T) { testRequest(t, "http://localhost:8080/example") } +func TestBadTrustedCIDRs(t *testing.T) { + router := New() + assert.Error(t, router.SetTrustedProxies([]string{"hello/world"})) +} + +/* legacy tests func TestBadTrustedCIDRsForRun(t *testing.T) { os.Setenv("PORT", "") router := New() @@ -143,6 +149,7 @@ func TestBadTrustedCIDRsForRunTLS(t *testing.T) { router.TrustedProxies = []string{"hello/world"} assert.Error(t, router.RunTLS(":8080", "./testdata/certificate/cert.pem", "./testdata/certificate/key.pem")) } +*/ func TestRunTLS(t *testing.T) { router := New() diff --git a/gin_test.go b/gin_test.go index 678d95f2..21c43d15 100644 --- a/gin_test.go +++ b/gin_test.go @@ -539,19 +539,15 @@ func TestPrepareTrustedCIRDsWith(t *testing.T) { // valid ipv4 cidr { expectedTrustedCIDRs := []*net.IPNet{parseCIDR("0.0.0.0/0")} - r.TrustedProxies = []string{"0.0.0.0/0"} - - trustedCIDRs, err := r.prepareTrustedCIDRs() + err := r.SetTrustedProxies([]string{"0.0.0.0/0"}) assert.NoError(t, err) - assert.Equal(t, expectedTrustedCIDRs, trustedCIDRs) + assert.Equal(t, expectedTrustedCIDRs, r.trustedCIDRs) } // invalid ipv4 cidr { - r.TrustedProxies = []string{"192.168.1.33/33"} - - _, err := r.prepareTrustedCIDRs() + err := r.SetTrustedProxies([]string{"192.168.1.33/33"}) assert.Error(t, err) } @@ -559,19 +555,16 @@ func TestPrepareTrustedCIRDsWith(t *testing.T) { // valid ipv4 address { expectedTrustedCIDRs := []*net.IPNet{parseCIDR("192.168.1.33/32")} - r.TrustedProxies = []string{"192.168.1.33"} - trustedCIDRs, err := r.prepareTrustedCIDRs() + err := r.SetTrustedProxies([]string{"192.168.1.33"}) assert.NoError(t, err) - assert.Equal(t, expectedTrustedCIDRs, trustedCIDRs) + assert.Equal(t, expectedTrustedCIDRs, r.trustedCIDRs) } // invalid ipv4 address { - r.TrustedProxies = []string{"192.168.1.256"} - - _, err := r.prepareTrustedCIDRs() + err := r.SetTrustedProxies([]string{"192.168.1.256"}) assert.Error(t, err) } @@ -579,19 +572,15 @@ func TestPrepareTrustedCIRDsWith(t *testing.T) { // valid ipv6 address { expectedTrustedCIDRs := []*net.IPNet{parseCIDR("2002:0000:0000:1234:abcd:ffff:c0a8:0101/128")} - r.TrustedProxies = []string{"2002:0000:0000:1234:abcd:ffff:c0a8:0101"} - - trustedCIDRs, err := r.prepareTrustedCIDRs() + err := r.SetTrustedProxies([]string{"2002:0000:0000:1234:abcd:ffff:c0a8:0101"}) assert.NoError(t, err) - assert.Equal(t, expectedTrustedCIDRs, trustedCIDRs) + assert.Equal(t, expectedTrustedCIDRs, r.trustedCIDRs) } // invalid ipv6 address { - r.TrustedProxies = []string{"gggg:0000:0000:1234:abcd:ffff:c0a8:0101"} - - _, err := r.prepareTrustedCIDRs() + err := r.SetTrustedProxies([]string{"gggg:0000:0000:1234:abcd:ffff:c0a8:0101"}) assert.Error(t, err) } @@ -599,19 +588,15 @@ func TestPrepareTrustedCIRDsWith(t *testing.T) { // valid ipv6 cidr { expectedTrustedCIDRs := []*net.IPNet{parseCIDR("::/0")} - r.TrustedProxies = []string{"::/0"} - - trustedCIDRs, err := r.prepareTrustedCIDRs() + err := r.SetTrustedProxies([]string{"::/0"}) assert.NoError(t, err) - assert.Equal(t, expectedTrustedCIDRs, trustedCIDRs) + assert.Equal(t, expectedTrustedCIDRs, r.trustedCIDRs) } // invalid ipv6 cidr { - r.TrustedProxies = []string{"gggg:0000:0000:1234:abcd:ffff:c0a8:0101/129"} - - _, err := r.prepareTrustedCIDRs() + err := r.SetTrustedProxies([]string{"gggg:0000:0000:1234:abcd:ffff:c0a8:0101/129"}) assert.Error(t, err) } @@ -623,36 +608,32 @@ func TestPrepareTrustedCIRDsWith(t *testing.T) { parseCIDR("192.168.0.0/16"), parseCIDR("172.16.0.1/32"), } - r.TrustedProxies = []string{ + err := r.SetTrustedProxies([]string{ "::/0", "192.168.0.0/16", "172.16.0.1", - } - - trustedCIDRs, err := r.prepareTrustedCIDRs() + }) assert.NoError(t, err) - assert.Equal(t, expectedTrustedCIDRs, trustedCIDRs) + assert.Equal(t, expectedTrustedCIDRs, r.trustedCIDRs) } // invalid combination { - r.TrustedProxies = []string{ + err := r.SetTrustedProxies([]string{ "::/0", "192.168.0.0/16", "172.16.0.256", - } - _, err := r.prepareTrustedCIDRs() + }) assert.Error(t, err) } // nil value { - r.TrustedProxies = nil - trustedCIDRs, err := r.prepareTrustedCIDRs() + err := r.SetTrustedProxies(nil) - assert.Nil(t, trustedCIDRs) + assert.Nil(t, r.trustedCIDRs) assert.Nil(t, err) } From 21125bbb3f550dbfa4c64151f7e01f58dd64e2b8 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 6 Oct 2021 09:38:42 +0800 Subject: [PATCH 2/2] Bump github.com/goccy/go-json from 0.7.8 to 0.7.9 (#2891) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 80807e85..c03439a0 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.13 require ( github.com/gin-contrib/sse v0.1.0 github.com/go-playground/validator/v10 v10.9.0 - github.com/goccy/go-json v0.7.8 + github.com/goccy/go-json v0.7.9 github.com/json-iterator/go v1.1.12 github.com/mattn/go-isatty v0.0.14 github.com/stretchr/testify v1.7.0 diff --git a/go.sum b/go.sum index 33ce8cd7..7bf71f9f 100644 --- a/go.sum +++ b/go.sum @@ -12,8 +12,8 @@ github.com/go-playground/universal-translator v0.18.0 h1:82dyy6p4OuJq4/CByFNOn/j github.com/go-playground/universal-translator v0.18.0/go.mod h1:UvRDBj+xPUEGrFYl+lu/H90nyDXpg0fqeB/AQUGNTVA= github.com/go-playground/validator/v10 v10.9.0 h1:NgTtmN58D0m8+UuxtYmGztBJB7VnPgjj221I1QHci2A= github.com/go-playground/validator/v10 v10.9.0/go.mod h1:74x4gJWsvQexRdW8Pn3dXSGrTK4nAUsbPlLADvpJkos= -github.com/goccy/go-json v0.7.8 h1:CvMH7LotYymYuLGEohBM1lTZWX4g6jzWUUl2aLFuBoE= -github.com/goccy/go-json v0.7.8/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= +github.com/goccy/go-json v0.7.9 h1:mSp3uo1tr6MXQTYopSNhHTUnJhd2zQ4Yk+HdJZP+ZRY= +github.com/goccy/go-json v0.7.9/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=