Skip to content

Commit 56e87d0

Browse files
ClearlyClairehiyuki2578
authored andcommitted
Move signature verification stoplight to the requests themselves (mastodon#10813)
* Move signature verification stoplight to the requests themselves This avoids blocking messages from known keys for 5 minutes when only one fails… * Put the stoplight on the actual client IP, not a potential reverse proxy
1 parent 19c9773 commit 56e87d0

1 file changed

Lines changed: 13 additions & 16 deletions

File tree

app/controllers/concerns/signature_verification.rb

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,7 @@ def signed_request_account
4343
return
4444
end
4545

46-
account_stoplight = Stoplight("source:#{request.ip}") { account_from_key_id(signature_params['keyId']) }
47-
.with_fallback { nil }
48-
.with_threshold(1)
49-
.with_cool_off_time(5.minutes.seconds)
50-
.with_error_handler { |error, handle| error.is_a?(HTTP::Error) ? handle.call(error) : raise(error) }
51-
52-
account = account_stoplight.run
46+
account = account_from_key_id(signature_params['keyId'])
5347

5448
if account.nil?
5549
@signature_verification_failure_reason = "Public key not found for key #{signature_params['keyId']}"
@@ -62,13 +56,7 @@ def signed_request_account
6256

6357
return account unless verify_signature(account, signature, compare_signed_string).nil?
6458

65-
account_stoplight = Stoplight("source:#{request.ip}") { account.possibly_stale? ? account.refresh! : account_refresh_key(account) }
66-
.with_fallback { nil }
67-
.with_threshold(1)
68-
.with_cool_off_time(5.minutes.seconds)
69-
.with_error_handler { |error, handle| error.is_a?(HTTP::Error) ? handle.call(error) : raise(error) }
70-
71-
account = account_stoplight.run
59+
account = stoplight_wrap_request { account.possibly_stale? ? account.refresh! : account_refresh_key(account) }
7260

7361
if account.nil?
7462
@signature_verification_failure_reason = "Public key not found for key #{signature_params['keyId']}"
@@ -136,14 +124,23 @@ def incompatible_signature?(signature_params)
136124

137125
def account_from_key_id(key_id)
138126
if key_id.start_with?('acct:')
139-
ResolveAccountService.new.call(key_id.gsub(/\Aacct:/, ''))
127+
stoplight_wrap_request { ResolveAccountService.new.call(key_id.gsub(/\Aacct:/, '')) }
140128
elsif !ActivityPub::TagManager.instance.local_uri?(key_id)
141129
account = ActivityPub::TagManager.instance.uri_to_resource(key_id, Account)
142-
account ||= ActivityPub::FetchRemoteKeyService.new.call(key_id, id: false)
130+
account ||= stoplight_wrap_request { ActivityPub::FetchRemoteKeyService.new.call(key_id, id: false) }
143131
account
144132
end
145133
end
146134

135+
def stoplight_wrap_request(&block)
136+
Stoplight("source:#{request.remote_ip}", &block)
137+
.with_fallback { nil }
138+
.with_threshold(1)
139+
.with_cool_off_time(5.minutes.seconds)
140+
.with_error_handler { |error, handle| error.is_a?(HTTP::Error) ? handle.call(error) : raise(error) }
141+
.run
142+
end
143+
147144
def account_refresh_key(account)
148145
return if account.local? || !account.activitypub?
149146
ActivityPub::FetchRemoteAccountService.new.call(account.uri, only_key: true)

0 commit comments

Comments
 (0)