Skip to content

Commit

Permalink
Move signature verification stoplight to the requests themselves (mas…
Browse files Browse the repository at this point in the history
…todon#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
  • Loading branch information
ClearlyClaire authored and multiple creatures committed Nov 19, 2019
1 parent bd6534f commit 77323f4
Showing 1 changed file with 13 additions and 16 deletions.
29 changes: 13 additions & 16 deletions app/controllers/concerns/signature_verification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,7 @@ def signed_request_account
return
end

account_stoplight = Stoplight("source:#{request.ip}") { account_from_key_id(signature_params['keyId']) }
.with_fallback { nil }
.with_threshold(1)
.with_cool_off_time(5.minutes.seconds)
.with_error_handler { |error, handle| error.is_a?(HTTP::Error) ? handle.call(error) : raise(error) }

account = account_stoplight.run
account = account_from_key_id(signature_params['keyId'])

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

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

account_stoplight = Stoplight("source:#{request.ip}") { account.possibly_stale? ? account.refresh! : account_refresh_key(account) }
.with_fallback { nil }
.with_threshold(1)
.with_cool_off_time(5.minutes.seconds)
.with_error_handler { |error, handle| error.is_a?(HTTP::Error) ? handle.call(error) : raise(error) }

account = account_stoplight.run
account = stoplight_wrap_request { account.possibly_stale? ? account.refresh! : account_refresh_key(account) }

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

def account_from_key_id(key_id)
if key_id.start_with?('acct:')
ResolveAccountService.new.call(key_id.gsub(/\Aacct:/, ''))
stoplight_wrap_request { ResolveAccountService.new.call(key_id.gsub(/\Aacct:/, '')) }
elsif !ActivityPub::TagManager.instance.local_uri?(key_id)
account = ActivityPub::TagManager.instance.uri_to_resource(key_id, Account)
account ||= ActivityPub::FetchRemoteKeyService.new.call(key_id, id: false)
account ||= stoplight_wrap_request { ActivityPub::FetchRemoteKeyService.new.call(key_id, id: false) }
account
end
end

def stoplight_wrap_request(&block)
Stoplight("source:#{request.remote_ip}", &block)
.with_fallback { nil }
.with_threshold(1)
.with_cool_off_time(5.minutes.seconds)
.with_error_handler { |error, handle| error.is_a?(HTTP::Error) ? handle.call(error) : raise(error) }
.run
end

def account_refresh_key(account)
return if account.local?
ActivityPub::FetchRemoteAccountService.new.call(account.uri, only_key: true)
Expand Down

0 comments on commit 77323f4

Please sign in to comment.