[chore] skip trusted-proxies warning if ip excepted from rate limiting (#3699)

* [chore] skip `trusted-proxies` warning if ip excepted from rate limiting

* weep

* typo

* fix env parsing test

cmd/gotosocial/action/server/server.go

@@ -455,11 +455,10 @@ func(context.Context, time.Time) {
 	// create required middleware
 	// rate limiting
 	rlLimit := config.GetAdvancedRateLimitRequests()
-	rlExceptions := config.GetAdvancedRateLimitExceptions()
+	clLimit := middleware.RateLimit(rlLimit, config.GetAdvancedRateLimitExceptionsParsed())        // client api
-	clLimit := middleware.RateLimit(rlLimit, rlExceptions)        // client api
+	s2sLimit := middleware.RateLimit(rlLimit, config.GetAdvancedRateLimitExceptionsParsed())       // server-to-server (AP)
-	s2sLimit := middleware.RateLimit(rlLimit, rlExceptions)       // server-to-server (AP)
+	fsMainLimit := middleware.RateLimit(rlLimit, config.GetAdvancedRateLimitExceptionsParsed())    // fileserver / web templates
-	fsMainLimit := middleware.RateLimit(rlLimit, rlExceptions)    // fileserver / web templates
+	fsEmojiLimit := middleware.RateLimit(rlLimit*2, config.GetAdvancedRateLimitExceptionsParsed()) // fileserver (emojis only, use high limit)
-	fsEmojiLimit := middleware.RateLimit(rlLimit*2, rlExceptions) // fileserver (emojis only, use high limit)
 
 	// throttling
 	cpuMultiplier := config.GetAdvancedThrottlingMultiplier()

docs/configuration/trusted_proxies.md

@@ -27,17 +27,17 @@ Before (default config):
 
 ```yaml
 trusted-proxies:
-    - "127.0.0.1/32"
+  - "127.0.0.1/32"
-    - "::1"
+  - "::1"
 ```
 
 After (new config):
 
 ```yaml
 trusted-proxies:
-    - "172.17.0.1/16"
+  - "172.17.0.1/16"
-    - "127.0.0.1/32"
+  - "127.0.0.1/32"
-    - "::1"
+  - "::1"
 ```
 
 If you are using [environment variables](../configuration/index.md#environment-variables) to configure your instance, you can configure `trusted-proxies` by setting the environment variable `GTS_TRUSTED_PROXIES` to a comma-separated list of IP ranges, like so:
@@ -74,6 +74,98 @@ If you still see the warning message but with a different suggested IP range to
 
 ## I can't seem to get `trusted-proxies` configured properly, can I just disable the warning?
 
-There are some situations where it's not practically possible to get `trusted-proxies` configured correctly to detect the real client IP of incoming requests For example, if you're running GoToSocial behind a home internet router that cannot inject an `X-Forwarded-For` header, then your suggested entry to add to `trusted-proxies` will look something like `192.168.x.x`, but adding this to `trusted-proxies` won't resolve the issue.
+There are some situations where it's not practically possible to get `trusted-proxies` configured correctly to detect the real client IP of incoming requests, or where the real client IP is accurate but still shows as being within a private network.
 
-If you've tried everything, then you can disable the warning message by just turning off rate limiting entirely, ie., by setting `advanced-rate-limit-requests` to 0 in your config.yaml, or setting the environment variable `GTS_ADVANCED_RATE_LIMIT_REQUESTS` to 0. Don't forget to **restart your instance** after changing this setting.
+For example, if you're running GoToSocial on your home network, behind a home internet router that cannot inject an `X-Forwarded-For` header, then your suggested entry to add to `trusted-proxies` will look something like `192.168.x.x`, but adding this to `trusted-proxies` won't resolve the issue.
+
+Another example: you're running GoToSocial on your home network, behind a home internet router, and you are accessing the web frontend from a device that's *also* on your home network, like your laptop or phone. In this case, your router may send you directly to your GoToSocial instance without your request ever leaving the network, and so GtS will correctly see *your* client IP address as a private network address, but *other* requests coming in from the wider internet will show their real remote client IP addresses. In this scenario, the `trusted-proxies` warning does not really apply.
+
+If you've tried editing your `trusted-proxies` setting, but you still see the warning, then it's likely that one of the above examples applies to you. You can proceed in one of two ways:
+
+### Add specific exception for your home network (preferred)
+
+If the suggested IP range in the `trusted-proxies` warning looks something like `192.168.x.x`, but you still see other client IPs in your GoToSocial logs that don't start with `192.168`, then try adding a rate limiting exception only for devices on your home network, while leaving rate limiting in place for outside IP addresses.
+
+For example, if your suggestion is something like `192.168.1.128/32`, then swap the `/32` for `/24` so that the range covers `192.168.1.0` -> `192.168.1.255`, and add this to the `advanced-rate-limit-exceptions` setting in your `config.yaml` file.
+
+Before (default config):
+
+```yaml
+advanced-rate-limit-exceptions: []
+```
+
+After (new config):
+
+```yaml
+advanced-rate-limit-exceptions:
+  - "192.168.1.128/24"
+```
+
+If you are using [environment variables](../configuration/index.md#environment-variables) to configure your instance, you can configure `advanced-rate-limit-exceptions` by setting the environment variable `GTS_ADVANCED_RATE_LIMIT_EXCEPTIONS` to a comma-separated list of IP ranges, like so:
+
+```env
+GTS_ADVANCED_RATE_LIMIT_EXCEPTIONS="192.168.1.128/24"
+```
+
+If you are using docker compose, your docker-compose.yaml file should look something like this after the change (note that yaml uses `: ` and not `=`):
+
+```yaml
+################################
+# BLAH BLAH OTHER CONFIG STUFF #
+################################
+    environment:
+      ############################
+      # BLAH BLAH OTHER ENV VARS #
+      ############################
+      GTS_ADVANCED_RATE_LIMIT_EXCEPTIONS: "192.168.1.128/24"
+################################
+# BLAH BLAH OTHER CONFIG STUFF #
+################################
+```
+
+Once you have made the necessary configuration changes, **restart your instance** and refresh the home page.
+
+### Turn off rate limiting entirely (last resort)
+
+If nothing else works, you can disable rate limiting entirely, which will also disable the `trusted-proxies` check and warning.
+
+!!! warning
+    Turning off rate limiting entirely should be considered a last resort, as rate limiting helps protect your instance from spam and scrapers.
+
+To turn off rate limiting, set `advanced-rate-limit-requests` to 0 in your `config.yaml`.
+
+Before (default config):
+
+```yaml
+advanced-rate-limit-requests: 300
+```
+
+After (new config):
+
+```yaml
+advanced-rate-limit-requests: 0
+```
+
+If you are using [environment variables](../configuration/index.md#environment-variables) to configure your instance, you can configure `advanced-rate-limit-requests` by setting the environment variable `GTS_ADVANCED_RATE_LIMIT_REQUESTS` to 0, like so:
+
+```env
+GTS_ADVANCED_RATE_LIMIT_REQUESTS="0"
+```
+
+If you are using docker compose, your docker-compose.yaml file should look something like this after the change (note that yaml uses `: ` and not `=`):
+
+```yaml
+################################
+# BLAH BLAH OTHER CONFIG STUFF #
+################################
+    environment:
+      ############################
+      # BLAH BLAH OTHER ENV VARS #
+      ############################
+      GTS_ADVANCED_RATE_LIMIT_REQUESTS: "0"
+################################
+# BLAH BLAH OTHER CONFIG STUFF #
+################################
+```
+
+Once you have made the necessary configuration changes, **restart your instance** and refresh the home page.

internal/api/util/template.go

@@ -18,12 +18,13 @@
 package util
 
 import (
-	"net"
 	"net/http"
+	"net/netip"
 
 	"github.com/gin-gonic/gin"
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
 	"github.com/superseriousbusiness/gotosocial/internal/config"
+	"github.com/superseriousbusiness/gotosocial/internal/log"
 )
 
 // WebPage encapsulates variables for
@@ -137,7 +138,15 @@ func injectTrustedProxiesRec(
 		return
 	}
 
-	ip := net.ParseIP(clientIP)
+	ip, err := netip.ParseAddr(clientIP)
+	if err != nil {
+		log.Warnf(
+			c.Request.Context(),
+			"gin returned invalid clientIP %s: %v",
+			clientIP, err,
+		)
+	}
+
 	if !ip.IsPrivate() {
 		// Upstream set a remote IP
 		// header but final clientIP
@@ -147,8 +156,18 @@ func injectTrustedProxiesRec(
 		return
 	}
 
+	except := config.GetAdvancedRateLimitExceptionsParsed()
+	for _, prefix := range except {
+		if prefix.Contains(ip) {
+			// This ip is exempt from
+			// rate limiting, so don't
+			// inject the suggestion.
+			return
+		}
+	}
+
 	// Private IP, guess if Docker.
-	if dockerSubnet.Contains(ip) {
+	if DockerSubnet.Contains(ip) {
 		// Suggest a CIDR that likely
 		// covers this Docker subnet,
 		// eg., 172.17.0.0 -> 172.17.255.255.
@@ -163,16 +182,10 @@ func injectTrustedProxiesRec(
 	obj["trustedProxiesRec"] = trustedProxiesRec
 }
 
-// dockerSubnet is a CIDR that lets one make hazy guesses
+// DockerSubnet is a CIDR that lets one make hazy guesses
 // as to whether an address is within the ranges Docker
 // uses for subnets, ie., 172.16.0.0 -> 172.31.255.255.
-var dockerSubnet = func() *net.IPNet {
+var DockerSubnet = netip.MustParsePrefix("172.16.0.0/12")
-	_, subnet, err := net.ParseCIDR("172.16.0.0/12")
-	if err != nil {
-		panic(err)
-	}
-	return subnet
-}()
 
 // templateErrorPage renders the given
 // HTTP code, error, and request ID

internal/config/config.go

@@ -18,6 +18,7 @@
 package config
 
 import (
+	"net/netip"
 	"reflect"
 	"time"
 
@@ -163,14 +164,15 @@ type Configuration struct {
 	SyslogProtocol string `name:"syslog-protocol" usage:"Protocol to use when directing logs to syslog. Leave empty to connect to local syslog."`
 	SyslogAddress  string `name:"syslog-address" usage:"Address:port to send syslog logs to. Leave empty to connect to local syslog."`
 
-	AdvancedCookiesSamesite      string        `name:"advanced-cookies-samesite" usage:"'strict' or 'lax', see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite"`
+	AdvancedCookiesSamesite           string         `name:"advanced-cookies-samesite" usage:"'strict' or 'lax', see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite"`
-	AdvancedRateLimitRequests    int           `name:"advanced-rate-limit-requests" usage:"Amount of HTTP requests to permit within a 5 minute window. 0 or less turns rate limiting off."`
+	AdvancedRateLimitRequests         int            `name:"advanced-rate-limit-requests" usage:"Amount of HTTP requests to permit within a 5 minute window. 0 or less turns rate limiting off."`
-	AdvancedRateLimitExceptions  []string      `name:"advanced-rate-limit-exceptions" usage:"Slice of CIDRs to exclude from rate limit restrictions."`
+	AdvancedRateLimitExceptions       []string       `name:"advanced-rate-limit-exceptions" usage:"Slice of CIDRs to exclude from rate limit restrictions."`
-	AdvancedThrottlingMultiplier int           `name:"advanced-throttling-multiplier" usage:"Multiplier to use per cpu for http request throttling. 0 or less turns throttling off."`
+	AdvancedRateLimitExceptionsParsed []netip.Prefix `name:"advanced-rate-limit-exceptions-parsed"`
-	AdvancedThrottlingRetryAfter time.Duration `name:"advanced-throttling-retry-after" usage:"Retry-After duration response to send for throttled requests."`
+	AdvancedThrottlingMultiplier      int            `name:"advanced-throttling-multiplier" usage:"Multiplier to use per cpu for http request throttling. 0 or less turns throttling off."`
-	AdvancedSenderMultiplier     int           `name:"advanced-sender-multiplier" usage:"Multiplier to use per cpu for batching outgoing fedi messages. 0 or less turns batching off (not recommended)."`
+	AdvancedThrottlingRetryAfter      time.Duration  `name:"advanced-throttling-retry-after" usage:"Retry-After duration response to send for throttled requests."`
-	AdvancedCSPExtraURIs         []string      `name:"advanced-csp-extra-uris" usage:"Additional URIs to allow when building content-security-policy for media + images."`
+	AdvancedSenderMultiplier          int            `name:"advanced-sender-multiplier" usage:"Multiplier to use per cpu for batching outgoing fedi messages. 0 or less turns batching off (not recommended)."`
-	AdvancedHeaderFilterMode     string        `name:"advanced-header-filter-mode" usage:"Set incoming request header filtering mode."`
+	AdvancedCSPExtraURIs              []string       `name:"advanced-csp-extra-uris" usage:"Additional URIs to allow when building content-security-policy for media + images."`
+	AdvancedHeaderFilterMode          string         `name:"advanced-header-filter-mode" usage:"Set incoming request header filtering mode."`
 
 	// HTTPClient configuration vars.
 	HTTPClient HTTPClientConfiguration `name:"http-client"`

internal/config/gen/gen.go

@@ -65,6 +65,7 @@ func main() {
 	fmt.Fprint(output, license)
 	fmt.Fprint(output, "package config\n\n")
 	fmt.Fprint(output, "import (\n")
+	fmt.Fprint(output, "\t\"net/netip\"\n")
 	fmt.Fprint(output, "\t\"time\"\n\n")
 	fmt.Fprint(output, "\t\"codeberg.org/gruf/go-bytesize\"\n")
 	fmt.Fprint(output, "\t\"github.com/superseriousbusiness/gotosocial/internal/language\"\n")

internal/config/helpers.gen.go

@@ -19,6 +19,7 @@
 package config
 
 import (
+	"net/netip"
 	"time"
 
 	"codeberg.org/gruf/go-bytesize"
@@ -2681,6 +2682,35 @@ func GetAdvancedRateLimitExceptions() []string { return global.GetAdvancedRateLi
 // SetAdvancedRateLimitExceptions safely sets the value for global configuration 'AdvancedRateLimitExceptions' field
 func SetAdvancedRateLimitExceptions(v []string) { global.SetAdvancedRateLimitExceptions(v) }
 
+// GetAdvancedRateLimitExceptionsParsed safely fetches the Configuration value for state's 'AdvancedRateLimitExceptionsParsed' field
+func (st *ConfigState) GetAdvancedRateLimitExceptionsParsed() (v []netip.Prefix) {
+	st.mutex.RLock()
+	v = st.config.AdvancedRateLimitExceptionsParsed
+	st.mutex.RUnlock()
+	return
+}
+
+// SetAdvancedRateLimitExceptionsParsed safely sets the Configuration value for state's 'AdvancedRateLimitExceptionsParsed' field
+func (st *ConfigState) SetAdvancedRateLimitExceptionsParsed(v []netip.Prefix) {
+	st.mutex.Lock()
+	defer st.mutex.Unlock()
+	st.config.AdvancedRateLimitExceptionsParsed = v
+	st.reloadToViper()
+}
+
+// AdvancedRateLimitExceptionsParsedFlag returns the flag name for the 'AdvancedRateLimitExceptionsParsed' field
+func AdvancedRateLimitExceptionsParsedFlag() string { return "" }
+
+// GetAdvancedRateLimitExceptionsParsed safely fetches the value for global configuration 'AdvancedRateLimitExceptionsParsed' field
+func GetAdvancedRateLimitExceptionsParsed() []netip.Prefix {
+	return global.GetAdvancedRateLimitExceptionsParsed()
+}
+
+// SetAdvancedRateLimitExceptionsParsed safely sets the value for global configuration 'AdvancedRateLimitExceptionsParsed' field
+func SetAdvancedRateLimitExceptionsParsed(v []netip.Prefix) {
+	global.SetAdvancedRateLimitExceptionsParsed(v)
+}
+
 // GetAdvancedThrottlingMultiplier safely fetches the Configuration value for state's 'AdvancedThrottlingMultiplier' field
 func (st *ConfigState) GetAdvancedThrottlingMultiplier() (v int) {
 	st.mutex.RLock()

internal/config/validate.go

@@ -19,6 +19,7 @@
 
 import (
 	"fmt"
+	"net/netip"
 	"net/url"
 	"strings"
 
@@ -168,5 +169,22 @@ func Validate() error {
 		)
 	}
 
+	// Parse `advanced-rate-limit-exceptions` and set
+	// parsed versions on config to avoid reparsing calls.
+	rles := GetAdvancedRateLimitExceptions()
+	rlesParsed := make([]netip.Prefix, 0, len(rles))
+	for _, rle := range rles {
+		parsed, err := netip.ParsePrefix(rle)
+		if err != nil {
+			errf(
+				"invalid entry %s in %s: %w",
+				rle, AdvancedRateLimitExceptionsFlag(), err,
+			)
+			continue
+		}
+		rlesParsed = append(rlesParsed, parsed)
+	}
+	SetAdvancedRateLimitExceptionsParsed(rlesParsed)
+
 	return errs.Combine()
 }

internal/middleware/ratelimit.go

@@ -48,7 +48,7 @@
 //
 // If the config AdvancedRateLimitRequests value is <= 0, then a noop
 // handler will be returned, which performs no rate limiting.
-func RateLimit(limit int, exceptions []string) gin.HandlerFunc {
+func RateLimit(limit int, except []netip.Prefix) gin.HandlerFunc {
 	if limit <= 0 {
 		// Rate limiting is disabled.
 		// Return noop middleware.
@@ -63,12 +63,6 @@ func RateLimit(limit int, exceptions []string) gin.HandlerFunc {
 		},
 	)
 
-	// Convert exceptions IP ranges into prefixes.
-	exceptPrefs := make([]netip.Prefix, len(exceptions))
-	for i, str := range exceptions {
-		exceptPrefs[i] = netip.MustParsePrefix(str)
-	}
-
 	// It's prettymuch impossible to effectively
 	// rate limit the immense IPv6 address space
 	// unless we mask some of the bytes.
@@ -88,7 +82,7 @@ func RateLimit(limit int, exceptions []string) gin.HandlerFunc {
 
 		// Check if this IP is exempt from rate
 		// limits and skip further checks if so.
-		for _, prefix := range exceptPrefs {
+		for _, prefix := range except {
 			if prefix.Contains(clientIP) {
 				c.Next()
 				return

internal/middleware/ratelimit_test.go

@@ -20,6 +20,7 @@
 import (
 	"net/http"
 	"net/http/httptest"
+	"net/netip"
 	"strconv"
 	"testing"
 	"time"
@@ -47,60 +48,37 @@ func (suite *RateLimitTestSuite) TestRateLimit() {
 
 	type rlTest struct {
 		limit        int
-		exceptions   []string
+		exceptions   []netip.Prefix
 		clientIP     string
-		shouldPanic  bool
 		shouldExcept bool
 	}
 
 	for _, test := range []rlTest{
 		{
 			limit:        10,
-			exceptions:   []string{},
+			exceptions:   nil,
 			clientIP:     "192.0.2.0",
-			shouldPanic:  false,
 			shouldExcept: false,
 		},
 		{
 			limit:        10,
-			exceptions:   []string{},
+			exceptions:   nil,
 			clientIP:     "192.0.2.0",
-			shouldPanic:  false,
 			shouldExcept: false,
 		},
 		{
 			limit:        10,
-			exceptions:   []string{"192.0.2.0/24"},
+			exceptions:   []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")},
 			clientIP:     "192.0.2.0",
-			shouldPanic:  false,
 			shouldExcept: true,
 		},
 		{
 			limit:        10,
-			exceptions:   []string{"192.0.2.0/32"},
+			exceptions:   []netip.Prefix{netip.MustParsePrefix("192.0.2.0/32")},
 			clientIP:     "192.0.2.1",
-			shouldPanic:  false,
-			shouldExcept: false,
-		},
-		{
-			limit:        10,
-			exceptions:   []string{"Ceci n'est pas une CIDR"},
-			clientIP:     "192.0.2.0",
-			shouldPanic:  true,
 			shouldExcept: false,
 		},
 	} {
-		if test.shouldPanic {
-			// Try to trigger panic.
-			suite.Panics(func() {
-				_ = middleware.RateLimit(
-					test.limit,
-					test.exceptions,
-				)
-			})
-			continue
-		}
-
 		rlMiddleware := middleware.RateLimit(
 			test.limit,
 			test.exceptions,

test/envparsing.sh

@@ -16,6 +16,7 @@ EXPECT=$(cat << "EOF"
         "192.0.2.0/24",
         "127.0.0.1/32"
     ],
+    "advanced-rate-limit-exceptions-parsed": null,
     "advanced-rate-limit-requests": 6969,
     "advanced-sender-multiplier": -1,
     "advanced-throttling-multiplier": -1,