Commit Graph

161 Commits

Author SHA1 Message Date
Osama Sayegh b86127ad12
FEATURE: Apply rate limits per user instead of IP for trusted users (#14706)
Currently, Discourse rate limits all incoming requests by the IP address they
originate from regardless of the user making the request. This can be
frustrating if there are multiple users using Discourse simultaneously while
sharing the same IP address (e.g. employees in an office).

This commit implements a new feature to make Discourse apply rate limits by
user id rather than IP address for users at or higher than the configured trust
level (1 is the default).

For example, let's say a Discourse instance is configured to allow 200 requests
per minute per IP address, and we have 10 users at trust level 4 using
Discourse simultaneously from the same IP address. Before this feature, the 10
users could only make a total of 200 requests per minute before they got rate
limited. But with the new feature, each user is allowed to make 200 requests
per minute because the rate limits are applied on user id rather than the IP
address.

The minimum trust level for applying user-id-based rate limits can be
configured by the `skip_per_ip_rate_limit_trust_level` global setting. The
default is 1, but it can be changed by either adding the
`DISCOURSE_SKIP_PER_IP_RATE_LIMIT_TRUST_LEVEL` environment variable with the
desired value to your `app.yml`, or changing the setting's value in the
`discourse.conf` file.

Requests made with API keys are still rate limited by IP address and the
relevant global settings that control API keys rate limits.

Before this commit, Discourse's auth cookie (`_t`) was simply a 32 characters
string that Discourse used to lookup the current user from the database and the
cookie contained no additional information about the user. However, we had to
change the cookie content in this commit so we could identify the user from the
cookie without making a database query before the rate limits logic and avoid
introducing a bottleneck on busy sites.

Besides the 32 characters auth token, the cookie now includes the user id,
trust level and the cookie's generation date, and we encrypt/sign the cookie to
prevent tampering.

Internal ticket number: t54739.
2021-11-17 23:27:30 +03:00
Rafael dos Santos Silva 6645243a26
SECURITY: Disallow caching of MIME/Content-Type errors (#14907)
This will sign intermediary proxies and/or misconfigured CDNs to not
cache those error responses.
2021-11-12 15:52:25 -03:00
Dan Ungureanu 69f0f48dc0
DEV: Fix rubocop issues (#14715) 2021-10-27 11:39:28 +03:00
David Taylor 7a52ce0d6d
FIX: Strip `discourse-logged-in` header during `force_anonymous!` (#14533)
When the anonymous cache forces users into anonymous mode, it strips the cookies from their request. However, the discourse-logged-in header from the JS client remained.

When the discourse-logged-in header is present without any valid auth_token, the current_user_provider [marks the request as ['logged out'](dbbfad7ed0/lib/auth/default_current_user_provider.rb (L125-L125)), and a [discourse-logged-out header is returned to the client](dbbfad7ed0/lib/middleware/request_tracker.rb (L209-L211)). This causes the JS app to [popup a "you were logged out" modal](dbbfad7ed0/app/assets/javascripts/discourse/app/components/d-document.js (L29-L29)), which is very disruptive.

This commit strips the discourse-logged-in header from the request at the same time as the auth cookie.
2021-10-07 12:31:42 +01:00
Rafael dos Santos Silva b136375582
FEATURE: Rate limit exceptions via ENV (#14033)
Allow admins to configure exceptions to our Rails rate limiter.

Configuration happens in the environment variables, and work with both
IPs and CIDR blocks.

Example:

```
env:
  DISCOURSE_MAX_REQS_PER_IP_EXCEPTIONS: >-
    14.15.16.32/27
    216.148.1.2
```
2021-08-13 12:00:23 -03:00
Alan Guo Xiang Tan 8e3691d537 PERF: Eager load Theme associations in Stylesheet Manager.
Before this change, calling `StyleSheet::Manager.stylesheet_details`
for the first time resulted in multiple queries to the database. This is
because the code was modelled in a way where each `Theme` was loaded
from the database one at a time.

This PR restructures the code such that it allows us to load all the
theme records in a single query. It also allows us to eager load the
required associations upfront. In order to achieve this, I removed the
support of loading multiple themes per request. It was initially added
to support user selectable theme components but the feature was never
completed and abandoned because it wasn't a feature that we thought was
worth building.
2021-06-21 11:06:58 +08:00
Josh Soref 59097b207f
DEV: Correct typos and spelling mistakes (#12812)
Over the years we accrued many spelling mistakes in the code base. 

This PR attempts to fix spelling mistakes and typos in all areas of the code that are extremely safe to change 

- comments
- test descriptions
- other low risk areas
2021-05-21 11:43:47 +10:00
Bianca Nenciu 765ba1ab2d
FEATURE: Ignore anonymous page views on private sites (#12800)
For sites with login_required set to true, counting anonymous pageviews is
confusing. Requests to /login and other pages would make it look like
anonymous users have access to site's content.
2021-04-26 14:19:47 +03:00
Dan Ungureanu dce48d8aa7
FIX: Redirect to provided origin after auth (#12558)
It used to redirect to the destination_url cookie which sometimes is set
incorrectly.
2021-03-31 10:23:12 +01:00
Jarek Radosz 6ff888bd2c
DEV: Retry-after header values should be strings (#12475)
Fixes `Rack::Lint::LintError: a header value must be a String, but the value of 'Retry-After' is a Integer`. (see: 14a236b4f0/lib/rack/lint.rb (L676))

I found it when I got flooded by those warning a while back in a test-related accident 😉 (ember CLI tests were hitting a local rails server at a fast rate)
2021-03-23 20:32:36 +01:00
Martin Brennan 6eb0d0c38d
SECURITY: Fix is_private_ip for RateLimiter to cover all cases (#12464)
The regular expression to detect private IP addresses did not always detect them successfully.
Changed to use ruby's in-built IPAddr.new(ip_address).private? method instead
which does the same thing but covers all cases.
2021-03-22 13:56:32 +10:00
Vinoth Kannan a5923ad603
DEV: apply allow origin response header for CDN requests. (#11893)
Currently, it creates a CORS error while accessing those static files.
2021-01-29 07:44:49 +05:30
Dan Ungureanu 1f2f84a6df
FIX: Add Retry-Header to rate limited responses (#11736)
It returned a 429 error code with a 'Retry-After' header if a
RateLimiter::LimitExceeded was raised and unhandled, but the header was
missing if the request was limited in the 'RequestTracker' middleware.
2021-01-19 11:35:46 +02:00
Alan Guo Xiang Tan 38b6b098bc
FIX: Bypass `AnonymousCache` for `/srv/status` route. (#11491)
`/srv/status` routes should not be cached at all. Also, we want to
decouple the route from Redis which `AnonymouseCache` relies on. The
`/srv/status` should continue to return a success response even if Redis
is down.
2020-12-16 16:47:46 +11:00
Tobias Eigen 0a0fd6eace
DEV: fixed capitalization in rate limit message (#11193) 2020-11-11 12:35:03 +11:00
Sam 2686d14b9a
PERF: introduce aggressive rate limiting for anonymous (#11129)
Previous to this change our anonymous rate limits acted as a throttle.
New implementation means we now also consider rate limited requests towards
the limit.

This means that if an anonymous user is hammering the server it will not be
able to get any requests through until it subsides with traffic.
2020-11-05 16:36:17 +11:00
Vinoth Kannan af4938baf1
Revert "DEV: enable cors to all cdn get requests from workbox. (#10684)" (#11076)
This reverts commit e3de45359f.

We need to improve out strategy by adding a cache breaker with this change ... some assets on CDNs and clients may have incorrect CORS headers which can cause stuff to break.
2020-10-30 16:05:35 +11:00
Vinoth Kannan e3de45359f
DEV: enable cors to all cdn get requests from workbox. (#10685)
Now all external requests from the service worker will be in CORS mode without credentials.
2020-10-28 23:36:19 +05:30
Sam a6d9adf346
DEV: ensure queue_time and background_requests are floats (#10901)
GlobalSetting can end up with a String and we expect a Float
2020-10-13 18:08:38 +11:00
Sam 32393f72b1
PERF: backoff background requests when overloaded (#10888)
When the server gets overloaded and lots of requests start queuing server
will attempt to shed load by returning 429 errors on background requests.

The client can flag a request as background by setting the header:
`Discourse-Background` to `true`

Out-of-the-box we shed load when the queue time goes above 0.5 seconds.

The only request we shed at the moment is the request to load up a new post
when someone posts to a topic.

We can extend this as we go with a more general pattern on the client.

Previous to this change, rate limiting would "break" the post stream which
would make suggested topics vanish and users would have to scroll the page
to see more posts in the topic.

Server needs this protection for cases where tons of clients are navigated
to a topic and a new post is made. This can lead to a self inflicted denial
of service if enough clients are viewing the topic.

Due to the internal security design of Discourse it is hard for a large
number of clients to share a channel where we would pass the full post body
via the message bus.

It also renames (and deprecates) triggerNewPostInStream to triggerNewPostsInStream

This allows us to load a batch of new posts cleanly, so the controller can
keep track of a backlog

Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
2020-10-13 16:56:03 +11:00
Guo Xiang Tan fe83baa9b3
FIX: Exclude `DELETE` methods from invalid request with payload.
Follow-up 105d560177

Our client side code is sending params as part of the request payload so
that is going to be tricky to fix.
2020-08-03 17:05:11 +08:00
Guo Xiang Tan 105d560177
SECURITY: 413 for GET, HEAD or DELETE requests with payload. 2020-08-03 14:21:33 +08:00
David Taylor c09b5807f3
FIX: Include resolved locale in anonymous cache key (#10289)
This only applies when set_locale_from_accept_language_header is enabled
2020-07-22 18:00:07 +01:00
Aman Gupta Karmani 8a86705e51
FIX: handle heroku style HTTP_X_REQUEST_START (#10087) 2020-06-19 10:17:24 -04:00
Daniel Waterworth bca126f3f5 REFACTOR: Move the multisite middleware to the front
Both request tracking and message bus rely on multisite before the
middleware has run which is not ideal.

Follow-up-to: ca1208a636
2020-04-02 16:44:44 +01:00
Daniel Waterworth ca1208a636 Revert "REFACTOR: Move the multisite middleware to the front"
Looks like this is causing problems.

Follow-up-to: a91843f0dc
2020-04-02 15:20:28 +01:00
Daniel Waterworth a91843f0dc REFACTOR: Move the multisite middleware to the front
Both request tracking and message bus rely on multisite before the
middleware has run which is not ideal.
2020-04-02 10:15:38 +01:00
David Taylor 19814c5e81
FIX: Allow CSP to work correctly for non-default hostnames/schemes (#9180)
- Define the CSP based on the requested domain / scheme (respecting force_https)
- Update EnforceHostname middleware to allow secondary domains, add specs
- Add URL scheme to anon cache key so that CSP headers are cached correctly
2020-03-19 19:54:42 +00:00
Sam Saffron a9f90cdec3 FIX: avoid superflous logging when mime type is bad
Many security scanners ship invalid mime types, this ensures we return
a very cheap response to the clients and do not log anything.

Previous attempt still re-dispatched the request to get proper error page
but in this specific case we want no error page.
2020-01-02 12:34:38 +11:00
Sam Saffron 494fe335d3 DEV: allow handling crawler reqs with no user agent
Followup to e440ec25 we treat no user agent as crawler reqs.
2019-12-09 18:40:10 +11:00
Sam Saffron e440ec2519 FIX: crawler requests not tracked for non UTF-8 user agents
Non UTF-8 user_agent requests were bypassing logging due to PG always
wanting UTF-8 strings.

This adds some conversion to ensure we are always dealing with UTF-8
2019-12-09 17:43:51 +11:00
Vinoth Kannan e51091f199 REFACTOR: do `X-Frame-Options` header removal in application controller.
Co-authored-by: Sam <sam.saffron@gmail.com>
Previous commit: f7084a4339
2019-12-06 18:25:32 +05:30
Vinoth Kannan f7084a4339 FEATURE: add site setting to remove `X-Frame-Options` header. 2019-12-06 03:15:09 +05:30
Robin Ward 895d5cb592 FIX: Anonymous cache regression 2019-12-05 15:07:48 -05:00
Robin Ward 532fea1460
DEV: Provide API for anonymous cache segments (#8455)
This can be used from a plugin that needs to establish something new in
the anonymous cache. For example `is_ie` for an internet explorer
plugin.
2019-12-05 14:57:18 -05:00
Joffrey JAFFEUX 0d3d2c43a0
DEV: s/\$redis/Discourse\.redis (#8431)
This commit also adds a rubocop rule to prevent global variables.
2019-12-03 10:05:53 +01:00
Sam Saffron 423ad5f0a4 FIX: do not log if an invalid mime type is passed to app
Previously our custom exception handler was unable to handle situations
where an invalid mime type was sent, resulting in a warning log

This ensures we pretend a request is HTML for the purpose of rendering
the error page if an invalid mime type from a scanner is shipped to the app
2019-11-21 15:51:34 +11:00
Penar Musaraj 74869b8a7f FIX: Do not consider mobile app traffic as crawler visits
Followup to a4eb523a
2019-11-04 09:16:50 -05:00
Daniel Waterworth 563253e9ed FIX: Fix options given to per-minute rate limiter
Previously the options for the per-minute and per-10-second rate
limiters were the same.
2019-09-20 10:48:59 +01:00
Sam Saffron ed00f35306 FEATURE: improve performance of anonymous cache
This commit introduces 2 features:

1. DISCOURSE_COMPRESS_ANON_CACHE (true|false, default false): this allows
you to optionally compress the anon cache body entries in Redis, can be
useful for high load sites with Redis that lives on a separate server to
to webs

2. DISCOURSE_ANON_CACHE_STORE_THRESHOLD (default 2), only pop entries into
redis if we observe them more than N times. This avoids situations where
a crawler can walk a big pile of topics and store them all in Redis never
to be used. Our default anon cache time for topics is only 60 seconds. Anon
cache is in place to avoid the "slashdot" effect where a single topic is
hit by 100s of people in one minute.
2019-09-04 17:18:32 +10:00
Sam Saffron b9954b53bb FIX: report cached controller and action to loggers
Previously we would treat all cached hits in anon cache as "other"

This hinders analysis of cache performance and makes logging inaccurate
2019-09-03 10:55:16 +10:00
Sam Saffron 08743e8ac0 FEATURE: anon cache reports data to loggers
This allows custom plugins such as prometheus exporter to log how many
requests are stored in the anon cache vs used by the anon cache.

This metric allows us to fine tune cache behaviors
2019-09-02 18:45:35 +10:00
Régis Hanol 75eebc904e FEATURE: new 'Discourse-Render' HTTP header 2019-08-30 20:45:18 +02:00
Sam Saffron 8cea78c833 Revert "FEATURE: Protect against replay attacks when using TLS 1.3 0-RTT (#8020)"
This reverts commit 39c31a3d76.

Sorry about this, we have decided againse supporting 0-RTT directly in
core, this can be supported with similar hacks to this commit in a
plugin.

That said, we recommend against using a 0-RTT proxy for the Discourse
app due to inherit risk of replay attacks.
2019-08-26 08:56:49 +10:00
Rafael dos Santos Silva 39c31a3d76
FEATURE: Protect against replay attacks when using TLS 1.3 0-RTT (#8020) 2019-08-23 11:52:47 -03:00
David Taylor f4aa6096ab FIX: Convert omniauth authenticator names to symbols before comparing
This is necessary because some auth plugins define their name as a string
2019-08-14 12:57:11 +01:00
David Taylor 1a8fee11a0 DEV: If only one auth provider is enabled allow GET request
In this case, the auth provider is acting as a SSO provider, and can be trusted to maintain its own CSRF protections.
2019-08-12 11:03:05 +01:00
David Taylor 750802bf56
UX: Improve error handling for common OmniAuth exceptions (#7991)
This displays more useful messages for the most common issues we see:
- CSRF (when the user switches browser)
- Invalid IAT (when the server clock is wrong)
- OAuth::Unauthorized for OAuth1 providers, when the credentials are incorrect

This commit also stops earlier for disabled authenticators. Now we stop at the request phase, rather than the callback phase.
2019-08-12 10:55:02 +01:00
Sam Saffron 1f47ed1ea3 PERF: message_bus will be deferred by server when flooded
The message_bus performs a fair amount of work prior to hijacking requests
this change ensures that if there is a situation where the server is flooded
message_bus will inform client to back off for 30 seconds + random(120 secs)

This back-off is ultra cheap and happens very early in the middleware.

It corrects a situation where a flood to message bus could cause the app
to become unresponsive

MessageBus update is here to ensure message_bus gem properly respects
Retry-After header and status 429.

Under normal state this code should never trigger, to disable raise the
value of DISCOURSE_REJECT_MESSAGE_BUS_QUEUE_SECONDS, default is to tell
message bus to go away if we are queueing for 100ms or longer
2019-08-09 17:48:01 +10:00
David Taylor 3b8c468832 SECURITY: Require POST with CSRF token for OmniAuth request phase 2019-08-08 11:58:00 +01:00