From 2560626419eaaf2b85982bdb3a70f74953299c72 Mon Sep 17 00:00:00 2001 From: mpl Date: Mon, 7 Jun 2021 17:46:14 +0200 Subject: [PATCH] doc: clarify usage for ratelimit's excludedIPs --- docs/content/middlewares/ratelimit.md | 45 +++++++++++++++++++++------ pkg/config/dynamic/middlewares.go | 4 +-- pkg/ip/strategy.go | 16 +++++++--- pkg/ip/strategy_test.go | 25 ++++++++------- 4 files changed, 62 insertions(+), 28 deletions(-) diff --git a/docs/content/middlewares/ratelimit.md b/docs/content/middlewares/ratelimit.md index 764a16555..73a609867 100644 --- a/docs/content/middlewares/ratelimit.md +++ b/docs/content/middlewares/ratelimit.md @@ -325,19 +325,46 @@ http: ##### `ipStrategy.excludedIPs` -`excludedIPs` configures Traefik to scan the `X-Forwarded-For` header and select the first IP not in the list. +!!! important "Contrary to what the name might suggest, this option is _not_ about excluding an IP from the rate limiter, and therefore cannot be used to deactivate rate limiting for some IPs." !!! important "If `depth` is specified, `excludedIPs` is ignored." -!!! example "Example of ExcludedIPs & X-Forwarded-For" +`excludedIPs` is meant to address two classes of somewhat distinct use-cases: - | `X-Forwarded-For` | `excludedIPs` | clientIP | - |-----------------------------------------|-----------------------|--------------| - | `"10.0.0.1,11.0.0.1,12.0.0.1,13.0.0.1"` | `"12.0.0.1,13.0.0.1"` | `"11.0.0.1"` | - | `"10.0.0.1,11.0.0.1,12.0.0.1,13.0.0.1"` | `"15.0.0.1,13.0.0.1"` | `"12.0.0.1"` | - | `"10.0.0.1,11.0.0.1,12.0.0.1,13.0.0.1"` | `"10.0.0.1,13.0.0.1"` | `"12.0.0.1"` | - | `"10.0.0.1,11.0.0.1,12.0.0.1,13.0.0.1"` | `"15.0.0.1,16.0.0.1"` | `"13.0.0.1"` | - | `"10.0.0.1,11.0.0.1"` | `"10.0.0.1,11.0.0.1"` | `""` | +1. Distinguish IPs which are behind the same (set of) reverse-proxies so that each of them contributes, independently to the others, + to its own rate-limit "bucket" (cf the [leaky bucket analogy](https://wikipedia.org/wiki/Leaky_bucket)). + In this case, `excludedIPs` should be set to match the list of `X-Forwarded-For IPs` that are to be excluded, + in order to find the actual clientIP. + + !!! example "Each IP as a distinct source" + + | X-Forwarded-For | excludedIPs | clientIP | + |--------------------------------|-----------------------|--------------| + | `"10.0.0.1,11.0.0.1,12.0.0.1"` | `"11.0.0.1,12.0.0.1"` | `"10.0.0.1"` | + | `"10.0.0.2,11.0.0.1,12.0.0.1"` | `"11.0.0.1,12.0.0.1"` | `"10.0.0.2"` | + +2. Group together a set of IPs (also behind a common set of reverse-proxies) so that they are considered the same source, + and all contribute to the same rate-limit bucket. + + !!! example "Group IPs together as same source" + + | X-Forwarded-For | excludedIPs | clientIP | + |--------------------------------|--------------|--------------| + | `"10.0.0.1,11.0.0.1,12.0.0.1"` | `"12.0.0.1"` | `"11.0.0.1"` | + | `"10.0.0.2,11.0.0.1,12.0.0.1"` | `"12.0.0.1"` | `"11.0.0.1"` | + | `"10.0.0.3,11.0.0.1,12.0.0.1"` | `"12.0.0.1"` | `"11.0.0.1"` | + +For completeness, below are additional examples to illustrate how the matching works. +For a given request the list of `X-Forwarded-For` IPs is checked from most recent to most distant against the `excludedIPs` pool, +and the first IP that is _not_ in the pool (if any) is returned. + +!!! example "Matching for clientIP" + + | X-Forwarded-For | excludedIPs | clientIP | + |--------------------------------|-----------------------|--------------| + | `"10.0.0.1,11.0.0.1,13.0.0.1"` | `"11.0.0.1"` | `"13.0.0.1"` | + | `"10.0.0.1,11.0.0.1,13.0.0.1"` | `"15.0.0.1,16.0.0.1"` | `"13.0.0.1"` | + | `"10.0.0.1,11.0.0.1"` | `"10.0.0.1,11.0.0.1"` | `""` | ```yaml tab="Docker" labels: diff --git a/pkg/config/dynamic/middlewares.go b/pkg/config/dynamic/middlewares.go index b931de0d4..a1c2d69a3 100644 --- a/pkg/config/dynamic/middlewares.go +++ b/pkg/config/dynamic/middlewares.go @@ -241,7 +241,7 @@ type IPStrategy struct { // Get an IP selection strategy. // If nil return the RemoteAddr strategy -// else return a strategy base on the configuration using the X-Forwarded-For Header. +// else return a strategy based on the configuration using the X-Forwarded-For Header. // Depth override the ExcludedIPs. func (s *IPStrategy) Get() (ip.Strategy, error) { if s == nil { @@ -259,7 +259,7 @@ func (s *IPStrategy) Get() (ip.Strategy, error) { if err != nil { return nil, err } - return &ip.CheckerStrategy{ + return &ip.PoolStrategy{ Checker: checker, }, nil } diff --git a/pkg/ip/strategy.go b/pkg/ip/strategy.go index 5a32524f6..16e150e95 100644 --- a/pkg/ip/strategy.go +++ b/pkg/ip/strategy.go @@ -43,14 +43,16 @@ func (s *DepthStrategy) GetIP(req *http.Request) string { return strings.TrimSpace(xffs[len(xffs)-s.Depth]) } -// CheckerStrategy a strategy based on an IP Checker -// allows to check that addresses are in a trusted IPs. -type CheckerStrategy struct { +// PoolStrategy is a strategy based on an IP Checker. +// It allows to check whether addresses are in a given pool of IPs. +type PoolStrategy struct { Checker *Checker } -// GetIP return the selected IP. -func (s *CheckerStrategy) GetIP(req *http.Request) string { +// GetIP checks the list of Forwarded IPs (most recent first) against the +// Checker pool of IPs. It returns the first IP that is not in the pool, or the +// empty string otherwise. +func (s *PoolStrategy) GetIP(req *http.Request) string { if s.Checker == nil { return "" } @@ -60,9 +62,13 @@ func (s *CheckerStrategy) GetIP(req *http.Request) string { for i := len(xffs) - 1; i >= 0; i-- { xffTrimmed := strings.TrimSpace(xffs[i]) + if len(xffTrimmed) == 0 { + continue + } if contain, _ := s.Checker.Contains(xffTrimmed); !contain { return xffTrimmed } } + return "" } diff --git a/pkg/ip/strategy_test.go b/pkg/ip/strategy_test.go index 8a05acd63..1409b1d54 100644 --- a/pkg/ip/strategy_test.go +++ b/pkg/ip/strategy_test.go @@ -74,34 +74,35 @@ func TestDepthStrategy_GetIP(t *testing.T) { } } -func TestExcludedIPsStrategy_GetIP(t *testing.T) { +func TestTrustedIPsStrategy_GetIP(t *testing.T) { testCases := []struct { desc string - excludedIPs []string + trustedIPs []string xForwardedFor string expected string + useRemote bool }{ { - desc: "Use excluded all IPs", - excludedIPs: []string{"10.0.0.4", "10.0.0.3", "10.0.0.2", "10.0.0.1"}, + desc: "Trust all IPs", + trustedIPs: []string{"10.0.0.4", "10.0.0.3", "10.0.0.2", "10.0.0.1"}, xForwardedFor: "10.0.0.4,10.0.0.3,10.0.0.2,10.0.0.1", expected: "", }, { - desc: "Use excluded IPs", - excludedIPs: []string{"10.0.0.2", "10.0.0.1"}, + desc: "Do not trust all IPs", + trustedIPs: []string{"10.0.0.2", "10.0.0.1"}, xForwardedFor: "10.0.0.4,10.0.0.3,10.0.0.2,10.0.0.1", expected: "10.0.0.3", }, { - desc: "Use excluded IPs CIDR", - excludedIPs: []string{"10.0.0.1/24"}, + desc: "Do not trust all IPs with CIDR", + trustedIPs: []string{"10.0.0.1/24"}, xForwardedFor: "127.0.0.1,10.0.0.4,10.0.0.3,10.0.0.2,10.0.0.1", expected: "127.0.0.1", }, { - desc: "Use excluded all IPs CIDR", - excludedIPs: []string{"10.0.0.1/24"}, + desc: "Trust all IPs with CIDR", + trustedIPs: []string{"10.0.0.1/24"}, xForwardedFor: "10.0.0.4,10.0.0.3,10.0.0.2,10.0.0.1", expected: "", }, @@ -112,10 +113,10 @@ func TestExcludedIPsStrategy_GetIP(t *testing.T) { t.Run(test.desc, func(t *testing.T) { t.Parallel() - checker, err := NewChecker(test.excludedIPs) + checker, err := NewChecker(test.trustedIPs) require.NoError(t, err) - strategy := CheckerStrategy{Checker: checker} + strategy := PoolStrategy{Checker: checker} req := httptest.NewRequest(http.MethodGet, "http://127.0.0.1", nil) req.Header.Set(xForwardedFor, test.xForwardedFor) actual := strategy.GetIP(req)