From d3f7aa7008971ef11e905d46af54796ba4d13417 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sun, 1 Jun 2025 10:12:06 -0400 Subject: [PATCH] Self-review --- docs/config.md | 5 +++-- server/server.yml | 19 +++++++++---------- server/server_test.go | 14 ++++++++++++++ server/util.go | 11 +++++++++-- 4 files changed, 35 insertions(+), 14 deletions(-) diff --git a/docs/config.md b/docs/config.md index 1dd0ee5e..1687c2ec 100644 --- a/docs/config.md +++ b/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 - to determine the real IP address (default: empty) +* `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 diff --git a/server/server.yml b/server/server.yml index c044efc5..669805b8 100644 --- a/server/server.yml +++ b/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, ...) -# - 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. -# -# The parsing of the forwarded header is very lenient. Here are some examples: -# - X-Forwarded-For: 1.2.3.4, 5.6.7.8 (-> +# - 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". @@ -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: diff --git a/server/server_test.go b/server/server_test.go index 2342759d..be0610ac 100644 --- a/server/server_test.go +++ b/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 diff --git a/server/util.go b/server/util.go index 006dcce1..3db9e322 100644 --- a/server/util.go +++ b/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, -// 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(strings.ToLower(r.Header.Get(forwardedHeader)))