dearsky/ntfy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Change to "proxy-forwarded-header" and add "proxy-trusted-addrs"

Browse Source
This commit is contained in:
binwiederhier
2025-05-31 22:39:18 -04:00
parent 2cb4d089ab
commit 849884c947
12 changed files with 482 additions and 280 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
server/config.go
View File

@@ -143,8 +143,9 @@ type Config struct {
VisitorAuthFailureLimitReplenish time.Duration
VisitorStatsResetTime time.Time // Time of the day at which to reset visitor stats
VisitorSubscriberRateLimiting bool // Enable subscriber-based rate limiting for UnifiedPush topics
BehindProxy bool
ProxyForwardedHeader string
BehindProxy bool // If true, the server will trust the proxy client IP header to determine the client IP address
ProxyForwardedHeader string // The header field to read the real/client IP address from, if BehindProxy is true, defaults to "X-Forwarded-For"
ProxyTrustedAddrs []string // List of trusted proxy addresses that will be stripped from the Forwarded header if BehindProxy is true
StripeSecretKey string
StripeWebhookKey string
StripePriceCacheDuration time.Duration

6
server/server.go
View File

@@ -1936,8 +1936,8 @@ func (s *Server) authorizeTopic(next handleFunc, perm user.Permission) handleFun
// This function will ALWAYS return a visitor, even if an error occurs (e.g. unauthorized), so
// that subsequent logging calls still have a visitor context.
func (s *Server) maybeAuthenticate(r *http.Request) (*visitor, error) {
// Read "Authorization" header value, and exit out early if it's not set
ip := extractIPAddress(r, s.config.BehindProxy, s.config.ProxyForwardedHeader)
// Read the "Authorization" header value and exit out early if it's not set
ip := extractIPAddress(r, s.config.BehindProxy, s.config.ProxyForwardedHeader, s.config.ProxyTrustedAddrs)
vip := s.visitor(ip, nil)
if s.userManager == nil {
return vip, nil
@@ -2012,7 +2012,7 @@ func (s *Server) authenticateBearerAuth(r *http.Request, token string) (*user.Us
if err != nil {
return nil, err
}
ip := extractIPAddress(r, s.config.BehindProxy, s.config.ProxyForwardedHeader)
ip := extractIPAddress(r, s.config.BehindProxy, s.config.ProxyForwardedHeader, s.config.ProxyTrustedAddrs)
go s.userManager.EnqueueTokenUpdate(token, &user.TokenUpdate{
LastAccess: time.Now(),
LastOrigin: ip,

12
server/server.yml
View File

@@ -95,13 +95,21 @@
# auth-default-access: "read-write"
# auth-startup-queries:
# If set, the X-Forwarded-For header is used to determine the visitor IP address
# 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.
#
# WARNING: If you are behind a proxy, you must set this, otherwise all visitors are rate limited
# 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: false
# proxy-forwarded-header: "X-Forwarded-For"
# proxy-trusted-addrs:
# If enabled, clients can attach files to notifications as attachments. Minimum settings to enable attachments
# are "attachment-cache-dir" and "base-url".

28
server/server_test.go
View File

@@ -2200,7 +2200,7 @@ func TestServer_Visitor_XForwardedFor_None(t *testing.T) {
c.BehindProxy = true
s := newTestServer(t, c)
r, _ := http.NewRequest("GET", "/bla", nil)
r.RemoteAddr = "8.9.10.11"
r.RemoteAddr = "8.9.10.11:1234"
r.Header.Set("X-Forwarded-For", " ") // Spaces, not empty!
v, err := s.maybeAuthenticate(r)
require.Nil(t, err)
@@ -2212,7 +2212,7 @@ func TestServer_Visitor_XForwardedFor_Single(t *testing.T) {
c.BehindProxy = true
s := newTestServer(t, c)
r, _ := http.NewRequest("GET", "/bla", nil)
r.RemoteAddr = "8.9.10.11"
r.RemoteAddr = "8.9.10.11:1234"
r.Header.Set("X-Forwarded-For", "1.1.1.1")
v, err := s.maybeAuthenticate(r)
require.Nil(t, err)
@@ -2224,7 +2224,7 @@ func TestServer_Visitor_XForwardedFor_Multiple(t *testing.T) {
c.BehindProxy = true
s := newTestServer(t, c)
r, _ := http.NewRequest("GET", "/bla", nil)
r.RemoteAddr = "8.9.10.11"
r.RemoteAddr = "8.9.10.11:1234"
r.Header.Set("X-Forwarded-For", "1.2.3.4 , 2.4.4.2,234.5.2.1 ")
v, err := s.maybeAuthenticate(r)
require.Nil(t, err)
@@ -2237,7 +2237,7 @@ func TestServer_Visitor_Custom_ClientIP_Header(t *testing.T) {
c.ProxyForwardedHeader = "X-Client-IP"
s := newTestServer(t, c)
r, _ := http.NewRequest("GET", "/bla", nil)
r.RemoteAddr = "8.9.10.11"
r.RemoteAddr = "8.9.10.11:1234"
r.Header.Set("X-Client-IP", "1.2.3.4")
v, err := s.maybeAuthenticate(r)
require.Nil(t, err)
@@ -2333,7 +2333,7 @@ func TestServer_SubscriberRateLimiting_Success(t *testing.T) {
// "Register" visitor 1.2.3.4 to topic "upAAAAAAAAAAAA" as a rate limit visitor
subscriber1Fn := func(r *http.Request) {
r.RemoteAddr = "1.2.3.4"
r.RemoteAddr = "1.2.3.4:1234"
}
rr := request(t, s, "GET", "/upAAAAAAAAAAAA/json?poll=1", "", nil, subscriber1Fn)
require.Equal(t, 200, rr.Code)
@@ -2342,7 +2342,7 @@ func TestServer_SubscriberRateLimiting_Success(t *testing.T) {
// "Register" visitor 8.7.7.1 to topic "up012345678912" as a rate limit visitor (implicitly via topic name)
subscriber2Fn := func(r *http.Request) {
r.RemoteAddr = "8.7.7.1"
r.RemoteAddr = "8.7.7.1:1234"
}
rr = request(t, s, "GET", "/up012345678912/json?poll=1", "", nil, subscriber2Fn)
require.Equal(t, 200, rr.Code)
@@ -2385,7 +2385,7 @@ func TestServer_SubscriberRateLimiting_NotWrongTopic(t *testing.T) {
s := newTestServer(t, c)
subscriberFn := func(r *http.Request) {
r.RemoteAddr = "1.2.3.4"
r.RemoteAddr = "1.2.3.4:1234"
}
rr := request(t, s, "GET", "/alerts,upAAAAAAAAAAAA,upBBBBBBBBBBBB/json?poll=1", "", nil, subscriberFn)
require.Equal(t, 200, rr.Code)
@@ -2405,7 +2405,7 @@ func TestServer_SubscriberRateLimiting_NotEnabled_Failed(t *testing.T) {
// Registering visitor 1.2.3.4 to topic has no effect
rr := request(t, s, "GET", "/upAAAAAAAAAAAA/json?poll=1", "", nil, func(r *http.Request) {
r.RemoteAddr = "1.2.3.4"
r.RemoteAddr = "1.2.3.4:1234"
})
require.Equal(t, 200, rr.Code)
require.Equal(t, "", rr.Body.String())
@@ -2413,7 +2413,7 @@ func TestServer_SubscriberRateLimiting_NotEnabled_Failed(t *testing.T) {
// Registering visitor 8.7.7.1 to topic has no effect
rr = request(t, s, "GET", "/up012345678912/json?poll=1", "", nil, func(r *http.Request) {
r.RemoteAddr = "8.7.7.1"
r.RemoteAddr = "8.7.7.1:1234"
})
require.Equal(t, 200, rr.Code)
require.Equal(t, "", rr.Body.String())
@@ -2439,7 +2439,7 @@ func TestServer_SubscriberRateLimiting_UP_Only(t *testing.T) {
// "Register" 5 different UnifiedPush visitors
for i := 0; i < 5; i++ {
subscriberFn := func(r *http.Request) {
r.RemoteAddr = fmt.Sprintf("1.2.3.%d", i+1)
r.RemoteAddr = fmt.Sprintf("1.2.3.%d:1234", i+1)
}
rr := request(t, s, "GET", fmt.Sprintf("/up12345678901%d/json?poll=1", i), "", nil, subscriberFn)
require.Equal(t, 200, rr.Code)
@@ -2463,7 +2463,7 @@ func TestServer_Matrix_SubscriberRateLimiting_UP_Only(t *testing.T) {
// "Register" 5 different UnifiedPush visitors
for i := 0; i < 5; i++ {
rr := request(t, s, "GET", fmt.Sprintf("/up12345678901%d/json?poll=1", i), "", nil, func(r *http.Request) {
r.RemoteAddr = fmt.Sprintf("1.2.3.%d", i+1)
r.RemoteAddr = fmt.Sprintf("1.2.3.%d:1234", i+1)
})
require.Equal(t, 200, rr.Code)
}
@@ -2490,7 +2490,7 @@ func TestServer_SubscriberRateLimiting_VisitorExpiration(t *testing.T) {
// "Register" rate visitor
subscriberFn := func(r *http.Request) {
r.RemoteAddr = "1.2.3.4"
r.RemoteAddr = "1.2.3.4:1234"
}
rr := request(t, s, "GET", "/upAAAAAAAAAAAA/json?poll=1", "", nil, subscriberFn)
require.Equal(t, 200, rr.Code)
@@ -2529,7 +2529,7 @@ func TestServer_SubscriberRateLimiting_ProtectedTopics_WithDefaultReadWrite(t *t
// - "up123456789012": Allowed, because no ACLs and nobody owns the topic
// - "announcements": NOT allowed, because it has read-only permissions for everyone
rr := request(t, s, "GET", "/up123456789012,announcements/json?poll=1", "", nil, func(r *http.Request) {
r.RemoteAddr = "1.2.3.4"
r.RemoteAddr = "1.2.3.4:1234"
})
require.Equal(t, 200, rr.Code)
require.Equal(t, "1.2.3.4", s.topics["up123456789012"].rateVisitor.ip.String())
@@ -2971,7 +2971,7 @@ func request(t *testing.T, s *Server, method, url, body string, headers map[stri
if err != nil {
t.Fatal(err)
}
r.RemoteAddr = "9.9.9.9" // Used for tests
r.RemoteAddr = "9.9.9.9:1234" // Used for tests
for k, v := range headers {
r.Header.Set(k, v)
}

60
server/util.go
View File

@@ -10,6 +10,7 @@ import (
"net/http"
"net/netip"
"regexp"
"slices"
"strings"
)
@@ -73,34 +74,43 @@ func readQueryParam(r *http.Request, names ...string) string {
return ""
}
func extractIPAddress(r *http.Request, behindProxy bool, proxyForwardedHeader string) netip.Addr {
remoteAddr := r.RemoteAddr
addrPort, err := netip.ParseAddrPort(remoteAddr)
ip := addrPort.Addr()
func extractIPAddress(r *http.Request, behindProxy bool, proxyForwardedHeader string, proxyTrustedAddrs []string) netip.Addr {
if behindProxy && proxyForwardedHeader != "" {
if addr, err := extractIPAddressFromHeader(r, proxyForwardedHeader, proxyTrustedAddrs); err == nil {
return addr
}
// Fall back to the remote address if the header is not found or invalid
}
addrPort, err := netip.ParseAddrPort(r.RemoteAddr)
if err != nil {
// This should not happen in real life; only in tests. So, using falling back to 0.0.0.0 if address unspecified
ip, err = netip.ParseAddr(remoteAddr)
if err != nil {
ip = netip.IPv4Unspecified()
if remoteAddr != "@" && !behindProxy { // RemoteAddr is @ when unix socket is used
logr(r).Err(err).Warn("unable to parse IP (%s), new visitor with unspecified IP (0.0.0.0) created", remoteAddr)
}
}
logr(r).Err(err).Warn("unable to parse IP (%s), new visitor with unspecified IP (0.0.0.0) created", r.RemoteAddr)
return netip.IPv4Unspecified()
}
if behindProxy && strings.TrimSpace(r.Header.Get(proxyForwardedHeader)) != "" {
// 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).
// See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For for details.
ips := util.SplitNoEmpty(r.Header.Get(proxyForwardedHeader), ",")
realIP, err := netip.ParseAddr(strings.TrimSpace(util.LastString(ips, remoteAddr)))
if err != nil {
logr(r).Err(err).Error("invalid IP address %s received in %s header", ip, proxyForwardedHeader)
// Fall back to the regular remote address if X-Forwarded-For is damaged
} else {
ip = realIP
}
return addrPort.Addr()
}
// 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).
// See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For for details.
func extractIPAddressFromHeader(r *http.Request, forwardedHeader string, trustedAddrs []string) (netip.Addr, error) {
value := strings.TrimSpace(r.Header.Get(forwardedHeader))
if value == "" {
return netip.IPv4Unspecified(), fmt.Errorf("no %s header found", forwardedHeader)
}
return ip
addrs := util.Map(util.SplitNoEmpty(value, ","), strings.TrimSpace)
clientAddrs := util.Filter(addrs, func(addr string) bool {
return !slices.Contains(trustedAddrs, addr)
})
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
}
func readJSONWithLimit[T any](r io.ReadCloser, limit int, allowEmpty bool) (*T, error) {

26
server/util_test.go
View File

@@ -88,3 +88,29 @@ func TestMaybeDecodeHeaders(t *testing.T) {
r.Header.Set("X-Priority", "5") // ntfy priority header
require.Equal(t, "5", readHeaderParam(r, "x-priority", "priority", "p"))
}
func TestExtractIPAddress(t *testing.T) {
r, _ := http.NewRequest("GET", "http://ntfy.sh/mytopic/json?since=all", nil)
r.RemoteAddr = "10.0.0.1:1234"
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")
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, "10.0.0.1", extractIPAddress(r, false, "X-Forwarded-For", trustedProxies).String())
}
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")
trustedProxies := []string{"1.1.1.1"}
require.Equal(t, "5.6.7.8", extractIPAddress(r, true, "X-Forwarded-For", trustedProxies).String())
require.Equal(t, "0.0.0.0", extractIPAddress(r, false, "X-Forwarded-For", trustedProxies).String())
}