Self-review

docs/config.md

@@ -563,10 +563,14 @@ ntfy server, they all share the proxy's IP address.
 
 Relevant flags to consider:
 
-* `behind-proxy`: if set, ntfy will use the `proxy-forwarded-header` to identify visitors (default: `false`)
-* `proxy-forwarded-header`: the header to use to identify visitors (default: `X-Forwarded-For`)
-* `proxy-trusted-addresses`: a comma-separated list of IP addresses that are removed from the forwarded header 
-  to determine the real IP address (default: empty)
+* `behind-proxy` makes it so that the real visitor IP address is extracted from the header defined in `proxy-forwarded-header`.
+  Without this, the remote address of the incoming connection is used (default: `false`).
+* `proxy-forwarded-header` is the header to use to identify visitors (default: `X-Forwarded-For`). It may be a single IP address (e.g. `1.2.3.4`),
+  a comma-separated list of IP addresses (e.g. `1.2.3.4, 5.6.7.8`), or an [RFC 7239](https://datatracker.ietf.org/doc/html/rfc7239)-style
+ header (e.g. `for=1.2.3.4;by=proxy.example.com, for=5.6.7.8`).
+* `proxy-trusted-addresses` is a comma-separated list of IP addresses that are removed from the forwarded header 
+  to determine the real IP address. This is only useful if there are multiple proxies involved that add themselves to
+  the forwarded header (default: empty).
 
 === "/etc/ntfy/server.yml (behind a proxy)"
     ``` yaml
@@ -578,7 +582,7 @@ Relevant flags to consider:
     behind-proxy: true
     ```
 
-=== "/etc/ntfy/server.yml (with custom header)"
+=== "/etc/ntfy/server.yml (X-Client-IP header)"
     ``` yaml
     # Tell ntfy to use "X-Client-IP" header to identify visitors for rate limiting
     #
@@ -589,6 +593,17 @@ Relevant flags to consider:
     proxy-forwarded-header: "X-Client-IP"
     ```
 
+=== "/etc/ntfy/server.yml (Forwarded header)"
+    ``` yaml
+    # Tell ntfy to use "Forwarded" header (RFC 7239) to identify visitors for rate limiting
+    #
+    # Example: If "Forwarded: for=1.2.3.4;by=proxy.example.com, for=9.9.9.9" is set, 
+    #          the visitor IP will be 9.9.9.9.
+    #
+    behind-proxy: true
+    proxy-forwarded-header: "Forwarded"
+    ```
+
 === "/etc/ntfy/server.yml (multiple proxies)"
     ``` yaml
     # Tell ntfy to use "X-Forwarded-For" header to identify visitors for rate limiting,

docs/releases.md

@@ -1433,6 +1433,12 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release
 
 ## Not released yet
 
+### ntfy server v2.13.0 (UNRELEASED)
+
+**Features:**
+
+* Support `X-Client-IP`, `X-Real-IP`, `Forwarded` headers for [rate limiting](config.md#ip-based-rate-limiting) via `proxy-forwarded-header` and `proxy-trusted-addresses` ([#1360](https://github.com/binwiederhier/ntfy/pull/1360)/[#1252](https://github.com/binwiederhier/ntfy/pull/1252), thanks to [@pixitha](https://github.com/pixitha)) 
+
 ### ntfy Android app v1.16.1 (UNRELEASED)
 
 **Features:**

server/server.yml

@@ -95,21 +95,23 @@
 # auth-default-access: "read-write"
 # auth-startup-queries:
 
-# If set, the X-Forwarded-For header (or whatever is configured) is used to determine the visitor IP address
-# instead of the remote address of the connection.
+# If set, the X-Forwarded-For header (or whatever is configured in proxy-forwarded-header) is used to determine
+# the visitor IP address instead of the remote address of the connection.
 #
 # WARNING: If you are behind a proxy, you must set this, otherwise all visitors are rate-limited
 #          as if they are one.
 #
-# - behind-proxy defines whether the server is behind a reverse proxy (e.g. nginx, traefik, ...)
-# - proxy-forwarded-header defines the header used to determine the visitor IP address. This defaults
-#   to "X-Forwarded-For", but can be set to any other header, e.g. "X-Real-IP", "X-Client-IP", ...
-# - proxy-trusted-addrs defines a list of trusted IP addresses that are stripped out of the
-#   forwarded header. This is useful if there are multiple trusted proxies involved.
+# - behind-proxy makes it so that the real visitor IP address is extracted from the header defined in
+#   proxy-forwarded-header. Without this, the remote address of the incoming connection is used.
+# - proxy-forwarded-header is the header to use to identify visitors. It may be a single IP address (e.g. 1.2.3.4),
+#   a comma-separated list of IP addresses (e.g. "1.2.3.4, 5.6.7.8"), or an RFC 7239-style header (e.g. "for=1.2.3.4;by=proxy.example.com, for=5.6.7.8").
+# - proxy-trusted-addresses is a comma-separated list of IP addresses that are removed from the forwarded header
+#   to determine the real IP address. This is only useful if there are multiple proxies involved that add themselves to
+#   the forwarded header.
 #
 # behind-proxy: false
 # proxy-forwarded-header: "X-Forwarded-For"
-# proxy-trusted-addrs:
+# proxy-trusted-addresses:
 
 # If enabled, clients can attach files to notifications as attachments. Minimum settings to enable attachments
 # are "attachment-cache-dir" and "base-url".
@@ -146,7 +148,7 @@
 # - smtp-server-domain is the e-mail domain, e.g. ntfy.sh
 # - smtp-server-addr-prefix is an optional prefix for the e-mail addresses to prevent spam. If set to "ntfy-",
 #   for instance, only e-mails to ntfy-$topic@ntfy.sh will be accepted. If this is not set, all emails to
-#   $topic@ntfy.sh will be accepted (which may obviously be a spam problem).
+#   $topic@ntfy.sh will be accepted (which may be a spam problem).
 #
 # smtp-server-listen:
 # smtp-server-domain:

server/server_test.go

@@ -2244,6 +2244,20 @@ func TestServer_Visitor_Custom_ClientIP_Header(t *testing.T) {
 	require.Equal(t, "1.2.3.4", v.ip.String())
 }
 
+func TestServer_Visitor_Custom_Forwarded_Header(t *testing.T) {
+	c := newTestConfig(t)
+	c.BehindProxy = true
+	c.ProxyForwardedHeader = "Forwarded"
+	c.ProxyTrustedAddresses = []string{"1.2.3.4"}
+	s := newTestServer(t, c)
+	r, _ := http.NewRequest("GET", "/bla", nil)
+	r.RemoteAddr = "8.9.10.11:1234"
+	r.Header.Set("Forwarded", " for=5.6.7.8, by=example.com;for=1.2.3.4")
+	v, err := s.maybeAuthenticate(r)
+	require.Nil(t, err)
+	require.Equal(t, "5.6.7.8", v.ip.String())
+}
+
 func TestServer_PublishWhileUpdatingStatsWithLotsOfMessages(t *testing.T) {
 	t.Parallel()
 	count := 50000

server/util.go

@@ -15,8 +15,13 @@ import (
 )
 
 var (
-	mimeDecoder               mime.WordDecoder
+	mimeDecoder mime.WordDecoder
+
+	// priorityHeaderIgnoreRegex matches specific patterns of the "Priority" header (RFC 9218), so that it can be ignored
 	priorityHeaderIgnoreRegex = regexp.MustCompile(`^u=\d,\s*(i|\d)$|^u=\d$`)
+
+	// forwardedHeaderRegex parses IPv4 addresses from the "Forwarded" header (RFC 7239)
+	forwardedHeaderRegex = regexp.MustCompile(`(?i)\bfor="?(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})"?`)
 )
 
 func readBoolParam(r *http.Request, defaultValue bool, names ...string) bool {
@@ -35,15 +40,11 @@ func toBool(value string) bool {
 	return value == "1" || value == "yes" || value == "true"
 }
 
-func readCommaSeparatedParam(r *http.Request, names ...string) (params []string) {
-	paramStr := readParam(r, names...)
-	if paramStr != "" {
-		params = make([]string, 0)
-		for _, s := range util.SplitNoEmpty(paramStr, ",") {
-			params = append(params, strings.TrimSpace(s))
-		}
+func readCommaSeparatedParam(r *http.Request, names ...string) []string {
+	if paramStr := readParam(r, names...); paramStr != "" {
+		return util.Map(util.SplitNoEmpty(paramStr, ","), strings.TrimSpace)
 	}
-	return params
+	return []string{}
 }
 
 func readParam(r *http.Request, names ...string) string {
@@ -74,6 +75,8 @@ func readQueryParam(r *http.Request, names ...string) string {
 	return ""
 }
 
+// extractIPAddress extracts the IP address of the visitor from the request,
+// either from the TCP socket or from a proxy header.
 func extractIPAddress(r *http.Request, behindProxy bool, proxyForwardedHeader string, proxyTrustedAddresses []string) netip.Addr {
 	if behindProxy && proxyForwardedHeader != "" {
 		if addr, err := extractIPAddressFromHeader(r, proxyForwardedHeader, proxyTrustedAddresses); err == nil {
@@ -91,26 +94,39 @@ func extractIPAddress(r *http.Request, behindProxy bool, proxyForwardedHeader st
 
 // extractIPAddressFromHeader extracts the last IP address from the specified header.
 //
-// X-Forwarded-For can contain multiple addresses (see #328). If we are behind a proxy,
-// only the right-most address can be trusted (as this is the one added by our proxy server).
+// It supports multiple formats:
+// - single IP address
+// - comma-separated list
+// - RFC 7239-style list (Forwarded header)
+//
+// If there are multiple addresses, we first remove the trusted IP addresses from the list, and
+// then take the right-most address in the list (as this is the one added by our proxy server).
 // See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For for details.
 func extractIPAddressFromHeader(r *http.Request, forwardedHeader string, trustedAddresses []string) (netip.Addr, error) {
-	value := strings.TrimSpace(r.Header.Get(forwardedHeader))
+	value := strings.TrimSpace(strings.ToLower(r.Header.Get(forwardedHeader)))
 	if value == "" {
 		return netip.IPv4Unspecified(), fmt.Errorf("no %s header found", forwardedHeader)
 	}
-	addrs := util.Map(util.SplitNoEmpty(value, ","), strings.TrimSpace)
-	clientAddrs := util.Filter(addrs, func(addr string) bool {
-		return !slices.Contains(trustedAddresses, addr)
+	// Extract valid addresses
+	addrsStrs := util.Map(util.SplitNoEmpty(value, ","), strings.TrimSpace)
+	var validAddrs []netip.Addr
+	for _, addrStr := range addrsStrs {
+		if addr, err := netip.ParseAddr(addrStr); err == nil {
+			validAddrs = append(validAddrs, addr)
+		} else if m := forwardedHeaderRegex.FindStringSubmatch(addrStr); len(m) == 2 {
+			if addr, err := netip.ParseAddr(m[1]); err == nil {
+				validAddrs = append(validAddrs, addr)
+			}
+		}
+	}
+	// Filter out proxy addresses
+	clientAddrs := util.Filter(validAddrs, func(addr netip.Addr) bool {
+		return !slices.Contains(trustedAddresses, addr.String())
 	})
 	if len(clientAddrs) == 0 {
 		return netip.IPv4Unspecified(), fmt.Errorf("no client IP address found in %s header: %s", forwardedHeader, value)
 	}
-	clientAddr, err := netip.ParseAddr(clientAddrs[len(clientAddrs)-1])
-	if err != nil {
-		return netip.IPv4Unspecified(), fmt.Errorf("invalid IP address %s received in %s header: %s: %w", clientAddr, forwardedHeader, value, err)
-	}
-	return clientAddr, nil
+	return clientAddrs[len(clientAddrs)-1], nil
 }
 
 func readJSONWithLimit[T any](r io.ReadCloser, limit int, allowEmpty bool) (*T, error) {
@@ -143,7 +159,7 @@ func fromContext[T any](r *http.Request, key contextKey) (T, error) {
 
 // maybeDecodeHeader decodes the given header value if it is MIME encoded, e.g. "=?utf-8?q?Hello_World?=",
 // or returns the original header value if it is not MIME encoded. It also calls maybeIgnoreSpecialHeader
-// to ignore new HTTP "Priority" header.
+// to ignore the new HTTP "Priority" header.
 func maybeDecodeHeader(name, value string) string {
 	decoded, err := mimeDecoder.DecodeHeader(value)
 	if err != nil {
@@ -152,7 +168,7 @@ func maybeDecodeHeader(name, value string) string {
 	return maybeIgnoreSpecialHeader(name, decoded)
 }
 
-// maybeIgnoreSpecialHeader ignores new HTTP "Priority" header (see https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-priority)
+// maybeIgnoreSpecialHeader ignores the new HTTP "Priority" header (RFC 9218, see https://datatracker.ietf.org/doc/html/rfc9218)
 //
 // Cloudflare (and potentially other providers) add this to requests when forwarding to the backend (ntfy),
 // so we just ignore it. If the "Priority" header is set to "u=*, i" or "u=*" (by Cloudflare), the header will be ignored.

server/util_test.go

@@ -95,12 +95,14 @@ func TestExtractIPAddress(t *testing.T) {
 	r.Header.Set("X-Forwarded-For", "  1.2.3.4  , 5.6.7.8")
 	r.Header.Set("X-Client-IP", "9.10.11.12")
 	r.Header.Set("X-Real-IP", "13.14.15.16, 1.1.1.1")
+	r.Header.Set("Forwarded", "for=17.18.19.20;by=proxy.example.com, by=2.2.2.2;for=1.1.1.1")
 
 	trustedProxies := []string{"1.1.1.1"}
 
 	require.Equal(t, "5.6.7.8", extractIPAddress(r, true, "X-Forwarded-For", trustedProxies).String())
 	require.Equal(t, "9.10.11.12", extractIPAddress(r, true, "X-Client-IP", trustedProxies).String())
 	require.Equal(t, "13.14.15.16", extractIPAddress(r, true, "X-Real-IP", trustedProxies).String())
+	require.Equal(t, "17.18.19.20", extractIPAddress(r, true, "Forwarded", trustedProxies).String())
 	require.Equal(t, "10.0.0.1", extractIPAddress(r, false, "X-Forwarded-For", trustedProxies).String())
 }
 
@@ -108,9 +110,11 @@ func TestExtractIPAddress_UnixSocket(t *testing.T) {
 	r, _ := http.NewRequest("GET", "http://ntfy.sh/mytopic/json?since=all", nil)
 	r.RemoteAddr = "@"
 	r.Header.Set("X-Forwarded-For", "1.2.3.4, 5.6.7.8, 1.1.1.1")
+	r.Header.Set("Forwarded", "by=bla.example.com;for=17.18.19.20")
 
 	trustedProxies := []string{"1.1.1.1"}
 
 	require.Equal(t, "5.6.7.8", extractIPAddress(r, true, "X-Forwarded-For", trustedProxies).String())
+	require.Equal(t, "17.18.19.20", extractIPAddress(r, true, "Forwarded", trustedProxies).String())
 	require.Equal(t, "0.0.0.0", extractIPAddress(r, false, "X-Forwarded-For", trustedProxies).String())
 }