From 94f6d2d5b506b2d021ac44a056ea14afb0999520 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Fri, 3 Mar 2023 20:23:18 -0500 Subject: [PATCH] Rename flag --- cmd/serve.go | 6 ++--- docs/config.md | 20 ++++++++++++++++ server/config.go | 4 +++- server/server.go | 6 ++--- server/server.yml | 9 +++---- server/server_test.go | 55 ++++++++++++++++++++++++++++++++++++------- 6 files changed, 81 insertions(+), 19 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index 4e663024..7883c8d3 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -63,7 +63,6 @@ var flagsServe = append( altsrc.NewBoolFlag(&cli.BoolFlag{Name: "enable-signup", Aliases: []string{"enable_signup"}, EnvVars: []string{"NTFY_ENABLE_SIGNUP"}, Value: false, Usage: "allows users to sign up via the web app, or API"}), altsrc.NewBoolFlag(&cli.BoolFlag{Name: "enable-login", Aliases: []string{"enable_login"}, EnvVars: []string{"NTFY_ENABLE_LOGIN"}, Value: false, Usage: "allows users to log in via the web app, or API"}), altsrc.NewBoolFlag(&cli.BoolFlag{Name: "enable-reservations", Aliases: []string{"enable_reservations"}, EnvVars: []string{"NTFY_ENABLE_RESERVATIONS"}, Value: false, Usage: "allows users to reserve topics (if their tier allows it)"}), - altsrc.NewBoolFlag(&cli.BoolFlag{Name: "enable-rate-visitor", Aliases: []string{"enable_rate_visitor"}, EnvVars: []string{"NTFY_ENABLE_RATE_VISITOR"}, Value: false, Usage: "enables subscriber-based rate limiting for UnifiedPush topics"}), altsrc.NewStringFlag(&cli.StringFlag{Name: "upstream-base-url", Aliases: []string{"upstream_base_url"}, EnvVars: []string{"NTFY_UPSTREAM_BASE_URL"}, Value: "", Usage: "forward poll request to an upstream server, this is needed for iOS push notifications for self-hosted servers"}), altsrc.NewStringFlag(&cli.StringFlag{Name: "smtp-sender-addr", Aliases: []string{"smtp_sender_addr"}, EnvVars: []string{"NTFY_SMTP_SENDER_ADDR"}, Usage: "SMTP server address (host:port) for outgoing emails"}), altsrc.NewStringFlag(&cli.StringFlag{Name: "smtp-sender-user", Aliases: []string{"smtp_sender_user"}, EnvVars: []string{"NTFY_SMTP_SENDER_USER"}, Usage: "SMTP user (if e-mail sending is enabled)"}), @@ -82,6 +81,7 @@ var flagsServe = append( altsrc.NewIntFlag(&cli.IntFlag{Name: "visitor-message-daily-limit", Aliases: []string{"visitor_message_daily_limit"}, EnvVars: []string{"NTFY_VISITOR_MESSAGE_DAILY_LIMIT"}, Value: server.DefaultVisitorMessageDailyLimit, Usage: "max messages per visitor per day, derived from request limit if unset"}), altsrc.NewIntFlag(&cli.IntFlag{Name: "visitor-email-limit-burst", Aliases: []string{"visitor_email_limit_burst"}, EnvVars: []string{"NTFY_VISITOR_EMAIL_LIMIT_BURST"}, Value: server.DefaultVisitorEmailLimitBurst, Usage: "initial limit of e-mails per visitor"}), altsrc.NewDurationFlag(&cli.DurationFlag{Name: "visitor-email-limit-replenish", Aliases: []string{"visitor_email_limit_replenish"}, EnvVars: []string{"NTFY_VISITOR_EMAIL_LIMIT_REPLENISH"}, Value: server.DefaultVisitorEmailLimitReplenish, Usage: "interval at which burst limit is replenished (one per x)"}), + altsrc.NewBoolFlag(&cli.BoolFlag{Name: "visitor-subscriber-rate-limiting", Aliases: []string{"enable_rate_visitor"}, EnvVars: []string{"NTFY_ENABLE_RATE_VISITOR"}, Value: false, Usage: "enables subscriber-based rate limiting for UnifiedPush topics"}), altsrc.NewBoolFlag(&cli.BoolFlag{Name: "behind-proxy", Aliases: []string{"behind_proxy", "P"}, EnvVars: []string{"NTFY_BEHIND_PROXY"}, Value: false, Usage: "if set, use X-Forwarded-For header to determine visitor IP address (for rate limiting)"}), altsrc.NewStringFlag(&cli.StringFlag{Name: "stripe-secret-key", Aliases: []string{"stripe_secret_key"}, EnvVars: []string{"NTFY_STRIPE_SECRET_KEY"}, Value: "", Usage: "key used for the Stripe API communication, this enables payments"}), altsrc.NewStringFlag(&cli.StringFlag{Name: "stripe-webhook-key", Aliases: []string{"stripe_webhook_key"}, EnvVars: []string{"NTFY_STRIPE_WEBHOOK_KEY"}, Value: "", Usage: "key required to validate the authenticity of incoming webhooks from Stripe"}), @@ -140,7 +140,6 @@ func execServe(c *cli.Context) error { enableSignup := c.Bool("enable-signup") enableLogin := c.Bool("enable-login") enableReservations := c.Bool("enable-reservations") - enableRateVisitor := c.Bool("enable-rate-visitor") upstreamBaseURL := c.String("upstream-base-url") smtpSenderAddr := c.String("smtp-sender-addr") smtpSenderUser := c.String("smtp-sender-user") @@ -151,6 +150,7 @@ func execServe(c *cli.Context) error { smtpServerAddrPrefix := c.String("smtp-server-addr-prefix") totalTopicLimit := c.Int("global-topic-limit") visitorSubscriptionLimit := c.Int("visitor-subscription-limit") + visitorSubscriberRateLimiting := c.Bool("visitor-subscriber-rate-limiting") visitorAttachmentTotalSizeLimitStr := c.String("visitor-attachment-total-size-limit") visitorAttachmentDailyBandwidthLimitStr := c.String("visitor-attachment-daily-bandwidth-limit") visitorRequestLimitBurst := c.Int("visitor-request-limit-burst") @@ -306,6 +306,7 @@ func execServe(c *cli.Context) error { conf.VisitorMessageDailyLimit = visitorMessageDailyLimit conf.VisitorEmailLimitBurst = visitorEmailLimitBurst conf.VisitorEmailLimitReplenish = visitorEmailLimitReplenish + conf.VisitorSubscriberRateLimiting = visitorSubscriberRateLimiting conf.BehindProxy = behindProxy conf.StripeSecretKey = stripeSecretKey conf.StripeWebhookKey = stripeWebhookKey @@ -314,7 +315,6 @@ func execServe(c *cli.Context) error { conf.EnableSignup = enableSignup conf.EnableLogin = enableLogin conf.EnableReservations = enableReservations - conf.EnableRateVisitor = enableRateVisitor conf.Version = c.App.Version // Set up hot-reloading of config diff --git a/docs/config.md b/docs/config.md index 59ee30ce..33195eb8 100644 --- a/docs/config.md +++ b/docs/config.md @@ -932,6 +932,25 @@ If this ever happens, there will be a log message that looks something like this WARN Firebase quota exceeded (likely for topic), temporarily denying Firebase access to visitor ``` +### Subscriber-based rate limiting +By default, ntfy puts almost all rate limits on the message publisher, e.g. number of messages, requests, and attachment +size are all based on the visitor who publishes a message. **Subscriber-based rate limiting is a way to use the rate limits +of a topic's subscriber, instead of the limits of the publisher.** + +If enabled, subscribers may opt to have published messages counted against their own rate limits, as opposed +to the publisher's rate limits. This is especially useful to increase the amount of messages that high-volume +publishers (e.g. Matrix/Mastodon servers) are allowed to send. + +Once enabled, a client may send a `Rate-Topics: ,,...` header when subscribing to topics via +HTTP stream, or websockets, thereby registering itself as the "rate visitor", i.e. the visitor whose rate limits +to use when publishing on this topic. Note that setting the rate visitor requires **read-write permission** on the topic. + +UnifiedPush only: If this setting is enabled, publishing to UnifiedPush topics will lead to an `HTTP 507 Insufficient Storage` +response if no "rate visitor" has been previously registered. This is to avoid burning the publisher's +`visitor-message-daily-limit`. + +To enable subscriber-based rate limiting, set `visitor-subscriber-rate-limiting: true`. + ## Tuning for scale If you're running ntfy for your home server, you probably don't need to worry about scale at all. In its default config, if it's not behind a proxy, the ntfy server can keep about **as many connections as the open file limit allows**. @@ -1191,6 +1210,7 @@ variable before running the `ntfy` command (e.g. `export NTFY_LISTEN_HTTP=:80`). | `visitor-request-limit-replenish` | `NTFY_VISITOR_REQUEST_LIMIT_REPLENISH` | *duration* | 5s | Rate limiting: Strongly related to `visitor-request-limit-burst`: The rate at which the bucket is refilled | | `visitor-request-limit-exempt-hosts` | `NTFY_VISITOR_REQUEST_LIMIT_EXEMPT_HOSTS` | *comma-separated host/IP list* | - | Rate limiting: List of hostnames and IPs to be exempt from request rate limiting | | `visitor-subscription-limit` | `NTFY_VISITOR_SUBSCRIPTION_LIMIT` | *number* | 30 | Rate limiting: Number of subscriptions per visitor (IP address) | +| `visitor-subscriber-rate-limiting` | `NTFY_VISITOR_SUBSCRIBER_RATE_LIMITING` | *bool* | `false` | Rate limiting: Enables subscriber-based rate limiting | | `web-root` | `NTFY_WEB_ROOT` | `app`, `home` or `disable` | `app` | Sets web root to landing page (home), web app (app) or disables the web app entirely (disable) | | `enable-signup` | `NTFY_ENABLE_SIGNUP` | *boolean* (`true` or `false`) | `false` | Allows users to sign up via the web app, or API | | `enable-login` | `NTFY_ENABLE_LOGIN` | *boolean* (`true` or `false`) | `false` | Allows users to log in via the web app, or API | diff --git a/server/config.go b/server/config.go index 747cbf17..cc9539ba 100644 --- a/server/config.go +++ b/server/config.go @@ -124,6 +124,7 @@ type Config struct { VisitorAuthFailureLimitBurst int 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 StripeSecretKey string StripeWebhookKey string @@ -133,7 +134,6 @@ type Config struct { EnableSignup bool // Enable creation of accounts via API and UI EnableLogin bool EnableReservations bool // Allow users with role "user" to own/reserve topics - EnableRateVisitor bool // Enable subscriber-based rate limiting for UnifiedPush topics AccessControlAllowOrigin string // CORS header field to restrict access from web clients Version string // injected by App } @@ -199,10 +199,12 @@ func NewConfig() *Config { VisitorAuthFailureLimitBurst: DefaultVisitorAuthFailureLimitBurst, VisitorAuthFailureLimitReplenish: DefaultVisitorAuthFailureLimitReplenish, VisitorStatsResetTime: DefaultVisitorStatsResetTime, + VisitorSubscriberRateLimiting: false, BehindProxy: false, StripeSecretKey: "", StripeWebhookKey: "", StripePriceCacheDuration: DefaultStripePriceCacheDuration, + BillingContact: "", EnableWeb: true, EnableSignup: false, EnableLogin: false, diff --git a/server/server.go b/server/server.go index 27f20478..2df289bf 100644 --- a/server/server.go +++ b/server/server.go @@ -597,7 +597,7 @@ func (s *Server) handlePublishWithoutResponse(r *http.Request, v *visitor) (*mes if e != nil { return nil, e.With(t) } - if unifiedpush && s.config.EnableRateVisitor && t.RateVisitor() == nil { + if unifiedpush && s.config.VisitorSubscriberRateLimiting && t.RateVisitor() == nil { // UnifiedPush clients must subscribe before publishing to allow proper subscriber-based rate limiting (see // Rate-Topics header). The 5xx response is because some app servers (in particular Mastodon) will remove // the subscription as invalid if any 400-499 code (except 429/408) is returned. @@ -1188,7 +1188,7 @@ func parseSubscribeParams(r *http.Request) (poll bool, since sinceMarker, schedu // maybeSetRateVisitors sets the rate visitor on a topic (v.SetRateVisitor), indicating that all messages published // to that topic will be rate limited against the rate visitor instead of the publishing visitor. // -// Setting the rate visitor is ony allowed if the `enable-rate-visitor` setting is enabled, AND +// Setting the rate visitor is ony allowed if the `visitor-subscriber-rate-limiting` setting is enabled, AND // - auth-file is not set (everything is open by default) // - or the topic is reserved, and v.user is the owner // - or the topic is not reserved, and v.user has write access @@ -1197,7 +1197,7 @@ func parseSubscribeParams(r *http.Request) (poll bool, since sinceMarker, schedu // until the Android app will send the "Rate-Topics" header. func (s *Server) maybeSetRateVisitors(r *http.Request, v *visitor, topics []*topic, rateTopics []string) error { // Bail out if not enabled - if !s.config.EnableRateVisitor { + if !s.config.VisitorSubscriberRateLimiting { return nil } diff --git a/server/server.yml b/server/server.yml index 8f4d646e..241b5377 100644 --- a/server/server.yml +++ b/server/server.yml @@ -238,16 +238,17 @@ # Rate limiting: Enable subscriber-based rate limiting (mostly used for UnifiedPush) # # If enabled, subscribers may opt to have published messages counted against their own rate limits, as opposed -# to the publisher's rate limits. This is especially useful to increase the amount of messages that UnifiedPush +# to the publisher's rate limits. This is especially useful to increase the amount of messages that high-volume # publishers (e.g. Matrix/Mastodon servers) are allowed to send. # # Once enabled, a client may send a "Rate-Topics: ,,..." header when subscribing to topics via # HTTP stream, or websockets, thereby registering itself as the "rate visitor", i.e. the visitor whose rate limits -# to use when publishing on this topic. +# to use when publishing on this topic. Note: Setting the rate visitor requires READ-WRITE permission on the topic. # -# For your home server, you likely DO NOT NEED THIS setting. +# UnifiedPush only: If this setting is enabled, publishing to UnifiedPush topics will lead to a HTTP 507 response if +# no "rate visitor" has been previously registered. This is to avoid burning the publisher's "visitor-message-daily-limit". # -# enable-rate-visitors: false +# visitor-subscriber-rate-limiting: false # Payments integration via Stripe # diff --git a/server/server_test.go b/server/server_test.go index 2dfb5643..707c7d88 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1293,7 +1293,7 @@ func TestServer_MatrixGateway_Push_Success(t *testing.T) { func TestServer_MatrixGateway_Push_Failure_NoSubscriber(t *testing.T) { c := newTestConfig(t) - c.EnableRateVisitor = true + c.VisitorSubscriberRateLimiting = true s := newTestServer(t, c) notification := `{"notification":{"devices":[{"pushkey":"http://127.0.0.1:12345/mytopic?up=1"}]}}` response := request(t, s, "POST", "/_matrix/push/v1/notify", notification, nil) @@ -2032,7 +2032,7 @@ func TestServer_AnonymousUser_And_NonTierUser_Are_Same_Visitor(t *testing.T) { func TestServer_SubscriberRateLimiting_Success(t *testing.T) { c := newTestConfigWithAuthFile(t) c.VisitorRequestLimitBurst = 3 - c.EnableRateVisitor = true + c.VisitorSubscriberRateLimiting = true s := newTestServer(t, c) // "Register" visitor 1.2.3.4 to topic "subscriber1topic" as a rate limit visitor @@ -2044,6 +2044,7 @@ func TestServer_SubscriberRateLimiting_Success(t *testing.T) { }, subscriber1Fn) require.Equal(t, 200, rr.Code) require.Equal(t, "", rr.Body.String()) + require.Equal(t, "1.2.3.4", s.topics["subscriber1topic"].rateVisitor.ip.String()) // "Register" visitor 8.7.7.1 to topic "up012345678912" as a rate limit visitor (implicitly via topic name) subscriber2Fn := func(r *http.Request) { @@ -2052,6 +2053,7 @@ func TestServer_SubscriberRateLimiting_Success(t *testing.T) { rr = request(t, s, "GET", "/up012345678912/json?poll=1", "", nil, subscriber2Fn) require.Equal(t, 200, rr.Code) require.Equal(t, "", rr.Body.String()) + require.Equal(t, "8.7.7.1", s.topics["up012345678912"].rateVisitor.ip.String()) // Publish 2 messages to "subscriber1topic" as visitor 9.9.9.9. It'd be 3 normally, but the // GET request before is also counted towards the request limiter. @@ -2083,10 +2085,47 @@ func TestServer_SubscriberRateLimiting_Success(t *testing.T) { require.Equal(t, 429, rr.Code) } +func TestServer_SubscriberRateLimiting_NotEnabled_Failed(t *testing.T) { + c := newTestConfigWithAuthFile(t) + c.VisitorRequestLimitBurst = 3 + c.VisitorSubscriberRateLimiting = false + s := newTestServer(t, c) + + // Subscriber rate limiting is disabled! + + // Registering visitor 1.2.3.4 to topic has no effect + rr := request(t, s, "GET", "/subscriber1topic/json?poll=1", "", map[string]string{ + "Rate-Topics": "subscriber1topic", + }, func(r *http.Request) { + r.RemoteAddr = "1.2.3.4" + }) + require.Equal(t, 200, rr.Code) + require.Equal(t, "", rr.Body.String()) + require.Nil(t, s.topics["subscriber1topic"].rateVisitor) + + // 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" + }) + require.Equal(t, 200, rr.Code) + require.Equal(t, "", rr.Body.String()) + require.Nil(t, s.topics["up012345678912"].rateVisitor) + + // Publish 3 messages to "subscriber1topic" as visitor 9.9.9.9 + for i := 0; i < 3; i++ { + rr := request(t, s, "PUT", "/subscriber1topic", "some message", nil) + require.Equal(t, 200, rr.Code) + } + rr = request(t, s, "PUT", "/subscriber1topic", "some message", nil) + require.Equal(t, 429, rr.Code) + rr = request(t, s, "PUT", "/up012345678912", "some message", nil) + require.Equal(t, 429, rr.Code) +} + func TestServer_SubscriberRateLimiting_UP_Only(t *testing.T) { c := newTestConfigWithAuthFile(t) c.VisitorRequestLimitBurst = 3 - c.EnableRateVisitor = true + c.VisitorSubscriberRateLimiting = true s := newTestServer(t, c) // "Register" 5 different UnifiedPush visitors @@ -2110,7 +2149,7 @@ func TestServer_SubscriberRateLimiting_UP_Only(t *testing.T) { func TestServer_Matrix_SubscriberRateLimiting_UP_Only(t *testing.T) { c := newTestConfig(t) c.VisitorRequestLimitBurst = 3 - c.EnableRateVisitor = true + c.VisitorSubscriberRateLimiting = true s := newTestServer(t, c) // "Register" 5 different UnifiedPush visitors @@ -2138,7 +2177,7 @@ func TestServer_Matrix_SubscriberRateLimiting_UP_Only(t *testing.T) { func TestServer_SubscriberRateLimiting_VisitorExpiration(t *testing.T) { c := newTestConfig(t) c.VisitorRequestLimitBurst = 3 - c.EnableRateVisitor = true + c.VisitorSubscriberRateLimiting = true s := newTestServer(t, c) // "Register" rate visitor @@ -2174,7 +2213,7 @@ func TestServer_SubscriberRateLimiting_VisitorExpiration(t *testing.T) { func TestServer_SubscriberRateLimiting_ProtectedTopics(t *testing.T) { c := newTestConfigWithAuthFile(t) c.AuthDefault = user.PermissionDenyAll - c.EnableRateVisitor = true + c.VisitorSubscriberRateLimiting = true s := newTestServer(t, c) // Create some ACLs @@ -2222,7 +2261,7 @@ func TestServer_SubscriberRateLimiting_ProtectedTopics(t *testing.T) { func TestServer_SubscriberRateLimiting_ProtectedTopics_WithDefaultReadWrite(t *testing.T) { c := newTestConfigWithAuthFile(t) c.AuthDefault = user.PermissionReadWrite - c.EnableRateVisitor = true + c.VisitorSubscriberRateLimiting = true s := newTestServer(t, c) // Create some ACLs @@ -2342,5 +2381,5 @@ func waitForWithMaxWait(t *testing.T, maxWait time.Duration, f func() bool) { } time.Sleep(100 * time.Millisecond) } - t.Fatalf("Function f did not succeed after %v: %s", maxWait, debug.Stack()) + t.Fatalf("Function f did not succeed after %v: %v", maxWait, string(debug.Stack())) }