Commit Graph

152 Commits

Author SHA1 Message Date
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
Sam Saffron
62141b6316 FEATURE: enable_performance_http_headers for performance diagnostics
This adds support for DISCOURSE_ENABLE_PERFORMANCE_HTTP_HEADERS
when set to `true` this will turn on performance related headers

```text
X-Redis-Calls: 10     # number of redis calls
X-Redis-Time: 1.02    # redis time in seconds
X-Sql-Commands: 102   # number of SQL commands
X-Sql-Time: 1.02      # duration in SQL in seconds
X-Queue-Time: 1.01    # time the request sat in queue (depends on NGINX)
```

To get queue time NGINX must provide: HTTP_X_REQUEST_START

We do not recommend you enable this without thinking, it exposes information
about what your page is doing, usually you would only enable this if you
intend to strip off the headers further down the stream in a proxy
2019-06-05 16:08:11 +10:00
Maja Komel
42809f4d69 FIX: use crawler layout when saving url in Wayback Machine (#7667) 2019-06-03 12:13:32 +10:00
Sam Saffron
30990006a9 DEV: enable frozen string literal on all files
This reduces chances of errors where consumers of strings mutate inputs
and reduces memory usage of the app.

Test suite passes now, but there may be some stuff left, so we will run
a few sites on a branch prior to merging
2019-05-13 09:31:32 +08:00
Penar Musaraj
a4eb523af6 Track Discourse user agent pageviews as crawler
Since 5bfe051e, Discourse user agents are marked as non-crawlers (to avoid accidental blacklisting). This makes sure pageviews for these agents are tracked as crawler hits.
2019-05-08 10:38:55 -04:00
David Taylor
8963f1af30
FEATURE: Optional detailed performance logging for Sidekiq jobs (#7091)
By default, this does nothing. Two environment variables are available:

- `DISCOURSE_LOG_SIDEKIQ`

  Set to `"1"` to enable logging. This will log all completed jobs to `log/rails/sidekiq.log`, along with various db/redis/network statistics. This is useful to track down poorly performing jobs.

- `DISCOURSE_LOG_SIDEKIQ_INTERVAL`

  (seconds) Check running jobs periodically, and log their current duration. They will appear in the logs with `status:pending`. This is useful to track down jobs which take a long time, then crash sidekiq before completing.
2019-03-05 11:19:11 +00:00
Guo Xiang Tan
c732ae9ca9 FIX: Don't update User#last_seen_at when PG is in readonly. 2019-01-21 13:29:29 +08:00
Guo Xiang Tan
e2a20d90fe FIX: Don't log request when Discourse is in readonly due to PG. 2019-01-21 11:04:32 +08:00
Sam
a19170a4c2 DEV: avoid require_dependency for some libs
This avoids require dependency on method_profiler and anon cache.

It means that if there is any change to these files the reloader will not pick it up.

Previously the reloader was picking up the anon cache twice causing it to double load on boot.

This caused warnings.

Long term my plan is to give up on require dependency and instead use:

https://github.com/Shopify/autoload_reloader
2018-12-31 10:53:30 +11:00
Sam
939b82ef0c DEV: correct intermittent test failure
ActionController::BadRequest can not be re-dispatched, under some conditions
we are getting this vs InvalidParameterError in the following test

59c56bd20f/spec/requests/application_controller_spec.rb (L34-L62)
2018-12-13 18:27:13 +11:00