Self-review

docs/config.md

@@ -568,8 +568,9 @@ Relevant flags to consider:
 * `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`: a comma-separated list of IP addresses that are removed from the forwarded header 
+* `proxy-trusted-addresses` is a comma-separated list of IP addresses that are removed from the forwarded header 
-  to determine the real IP address (default: empty)
+  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

server/server.yml

@@ -101,18 +101,17 @@
 # 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, ...)
+# - behind-proxy makes it so that the real visitor IP address is extracted from the header defined in
-# - proxy-forwarded-header defines the header used to determine the visitor IP address. This defaults
+#   proxy-forwarded-header. Without this, the remote address of the incoming connection is used.
-#   to "X-Forwarded-For", but can be set to any other header, e.g. "X-Real-IP", "X-Client-IP", ...
+# - proxy-forwarded-header is the header to use to identify visitors. It may be a single IP address (e.g. 1.2.3.4),
-# - proxy-trusted-addrs defines a list of trusted IP addresses that are stripped out of the
+#   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").
-#   forwarded header. This is useful if there are multiple trusted proxies involved.
+# - 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 parsing of the forwarded header is very lenient. Here are some examples:
+#   the forwarded header.
-# - X-Forwarded-For: 1.2.3.4, 5.6.7.8 (->
 #
 # 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".
@@ -149,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

@@ -75,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 {
@@ -92,8 +94,13 @@ 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,
+// It supports multiple formats:
-// only the right-most address can be trusted (as this is the one added by our proxy server).
+// - 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(strings.ToLower(r.Header.Get(forwardedHeader)))