forked from glitch-soc/mastodon
-
Notifications
You must be signed in to change notification settings - Fork 3
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request glitch-soc#2721 from ClearlyClaire/glitch-soc/merg…
…e-upstream Merge upstream changes up to 7f808ff
- Loading branch information
Showing
9 changed files
with
194 additions
and
18 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
# frozen_string_literal: true | ||
|
||
# Mastodon is not made to be directly accessed without a reverse proxy. | ||
# This monkey-patch prevents remote IP address spoofing when being accessed | ||
# directly. | ||
# | ||
# See PR: https://github.com/rails/rails/pull/51610 | ||
|
||
# In addition to the PR above, it also raises an error if a request with | ||
# `X-Forwarded-For` or `Client-Ip` comes directly from a client without | ||
# going through a trusted proxy. | ||
|
||
# rubocop:disable all -- This is a mostly vendored file | ||
|
||
module ActionDispatch | ||
class RemoteIp | ||
module GetIpExtensions | ||
def calculate_ip | ||
# Set by the Rack web server, this is a single value. | ||
remote_addr = ips_from(@req.remote_addr).last | ||
|
||
# Could be a CSV list and/or repeated headers that were concatenated. | ||
client_ips = ips_from(@req.client_ip).reverse! | ||
forwarded_ips = ips_from(@req.x_forwarded_for).reverse! | ||
|
||
# `Client-Ip` and `X-Forwarded-For` should not, generally, both be set. If they | ||
# are both set, it means that either: | ||
# | ||
# 1) This request passed through two proxies with incompatible IP header | ||
# conventions. | ||
# | ||
# 2) The client passed one of `Client-Ip` or `X-Forwarded-For` | ||
# (whichever the proxy servers weren't using) themselves. | ||
# | ||
# Either way, there is no way for us to determine which header is the right one | ||
# after the fact. Since we have no idea, if we are concerned about IP spoofing | ||
# we need to give up and explode. (If you're not concerned about IP spoofing you | ||
# can turn the `ip_spoofing_check` option off.) | ||
should_check_ip = @check_ip && client_ips.last && forwarded_ips.last | ||
if should_check_ip && !forwarded_ips.include?(client_ips.last) | ||
# We don't know which came from the proxy, and which from the user | ||
raise IpSpoofAttackError, "IP spoofing attack?! " \ | ||
"HTTP_CLIENT_IP=#{@req.client_ip.inspect} " \ | ||
"HTTP_X_FORWARDED_FOR=#{@req.x_forwarded_for.inspect}" | ||
end | ||
|
||
# NOTE: Mastodon addition to make sure we don't get requests from a non-trusted client | ||
if @check_ip && (forwarded_ips.last || client_ips.last) && !@proxies.any? { |proxy| proxy === remote_addr } | ||
raise IpSpoofAttackError, "IP spoofing attack?! client #{remote_addr} is not a trusted proxy " \ | ||
"HTTP_CLIENT_IP=#{@req.client_ip.inspect} " \ | ||
"HTTP_X_FORWARDED_FOR=#{@req.x_forwarded_for.inspect}" | ||
end | ||
|
||
# We assume these things about the IP headers: | ||
# | ||
# - X-Forwarded-For will be a list of IPs, one per proxy, or blank | ||
# - Client-Ip is propagated from the outermost proxy, or is blank | ||
# - REMOTE_ADDR will be the IP that made the request to Rack | ||
ips = forwarded_ips + client_ips | ||
ips.compact! | ||
|
||
# If every single IP option is in the trusted list, return the IP that's | ||
# furthest away | ||
filter_proxies([remote_addr] + ips).first || ips.last || remote_addr | ||
end | ||
end | ||
end | ||
end | ||
|
||
ActionDispatch::RemoteIp::GetIp.prepend(ActionDispatch::RemoteIp::GetIpExtensions) | ||
|
||
# rubocop:enable all |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ def patch | |
end | ||
|
||
def default_prerelease | ||
'alpha.3' | ||
'alpha.4' | ||
end | ||
|
||
def prerelease | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,19 +56,20 @@ def above_limit | |
end | ||
|
||
def throttle_count | ||
described_class.cache.read("#{counter_prefix}:#{throttle}:#{remote_ip}") || 0 | ||
described_class.cache.read("#{counter_prefix}:#{throttle}:#{discriminator}") || 0 | ||
end | ||
|
||
def counter_prefix | ||
(Time.now.to_i / period.seconds).to_i | ||
end | ||
|
||
def increment_counter | ||
described_class.cache.count("#{throttle}:#{remote_ip}", period) | ||
described_class.cache.count("#{throttle}:#{discriminator}", period) | ||
end | ||
end | ||
|
||
let(:remote_ip) { '1.2.3.5' } | ||
let(:discriminator) { remote_ip } | ||
|
||
describe 'throttle excessive sign-up requests by IP address' do | ||
context 'when accessed through the website' do | ||
|
@@ -149,4 +150,30 @@ def increment_counter | |
|
||
it_behaves_like 'throttled endpoint' | ||
end | ||
|
||
describe 'throttle excessive password change requests by account' do | ||
let(:user) { Fabricate(:user, email: '[email protected]') } | ||
let(:throttle) { 'throttle_password_change/account' } | ||
let(:limit) { 10 } | ||
let(:period) { 10.minutes } | ||
let(:request) { -> { put path, headers: { 'REMOTE_ADDR' => remote_ip } } } | ||
let(:path) { '/auth' } | ||
let(:discriminator) { user.id } | ||
|
||
before do | ||
sign_in user, scope: :user | ||
|
||
# Unfortunately, devise's `sign_in` helper causes the `session` to be | ||
# loaded in the next request regardless of whether it's actually accessed | ||
# by the client code. | ||
# | ||
# So, we make an extra query to clear issue a session cookie instead. | ||
# | ||
# A less resource-intensive way to deal with that would be to generate the | ||
# session cookie manually, but this seems pretty involved. | ||
get '/' | ||
end | ||
|
||
it_behaves_like 'throttled endpoint' | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters