Remove dependency on goldfinger gem (#14919)

There are edge cases where requests to certain hosts timeout when
using the vanilla HTTP.rb gem, which the goldfinger gem uses. Now
that we no longer need to support OStatus servers, webfinger logic
is so simple that there is no point encapsulating it in a gem, so
we can just use our own Request class. With that, we benefit from
more robust timeout code and IPv4/IPv6 resolution.

Fix #14091
This commit is contained in:
Eugen Rochko 2020-10-08 00:34:57 +02:00 committed by GitHub
parent a37732ef33
commit 7d985f2aac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 114 additions and 67 deletions

View File

@ -54,7 +54,6 @@ gem 'doorkeeper', '~> 5.4'
gem 'ed25519', '~> 1.2' gem 'ed25519', '~> 1.2'
gem 'fast_blank', '~> 1.0' gem 'fast_blank', '~> 1.0'
gem 'fastimage' gem 'fastimage'
gem 'goldfinger', '~> 2.1'
gem 'hiredis', '~> 0.6' gem 'hiredis', '~> 0.6'
gem 'redis-namespace', '~> 1.8' gem 'redis-namespace', '~> 1.8'
gem 'health_check', git: 'https://github.com/ianheggie/health_check', ref: '0b799ead604f900ed50685e9b2d469cd2befba5b' gem 'health_check', git: 'https://github.com/ianheggie/health_check', ref: '0b799ead604f900ed50685e9b2d469cd2befba5b'

View File

@ -240,11 +240,6 @@ GEM
ruby-progressbar (~> 1.4) ruby-progressbar (~> 1.4)
globalid (0.4.2) globalid (0.4.2)
activesupport (>= 4.2.0) activesupport (>= 4.2.0)
goldfinger (2.1.1)
addressable (~> 2.5)
http (~> 4.0)
nokogiri (~> 1.8)
oj (~> 3.0)
hamlit (2.13.0) hamlit (2.13.0)
temple (>= 0.8.2) temple (>= 0.8.2)
thor thor
@ -714,7 +709,6 @@ DEPENDENCIES
fog-core (<= 2.1.0) fog-core (<= 2.1.0)
fog-openstack (~> 0.3) fog-openstack (~> 0.3)
fuubar (~> 2.5) fuubar (~> 2.5)
goldfinger (~> 2.1)
hamlit-rails (~> 0.2) hamlit-rails (~> 0.2)
health_check! health_check!
hiredis (~> 0.6) hiredis (~> 0.6)

View File

@ -1,38 +1,7 @@
# frozen_string_literal: true # frozen_string_literal: true
# Monkey-patch on monkey-patch.
# Because it conflicts with the request.rb patch.
class HTTP::Timeout::PerOperationOriginal < HTTP::Timeout::PerOperation
def connect(socket_class, host, port, nodelay = false)
::Timeout.timeout(@connect_timeout, HTTP::TimeoutError) do
@socket = socket_class.open(host, port)
@socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay
end
end
end
module WebfingerHelper module WebfingerHelper
def webfinger!(uri) def webfinger!(uri)
hidden_service_uri = /\.(onion|i2p)(:\d+)?$/.match(uri) Webfinger.new(uri).perform
raise Mastodon::HostValidationError, 'Instance does not support hidden service connections' if !Rails.configuration.x.access_to_hidden_service && hidden_service_uri
opts = {
ssl: !hidden_service_uri,
headers: {
'User-Agent': Mastodon::Version.user_agent,
},
timeout_class: HTTP::Timeout::PerOperationOriginal,
timeout_options: {
write_timeout: 10,
connect_timeout: 5,
read_timeout: 10,
},
}
Goldfinger::Client.new(uri, opts.merge(Rails.configuration.x.http_client_proxy)).finger
end end
end end

93
app/lib/webfinger.rb Normal file
View File

@ -0,0 +1,93 @@
# frozen_string_literal: true
class Webfinger
class Error < StandardError; end
class Response
def initialize(body)
@json = Oj.load(body, mode: :strict)
end
def subject
@json['subject']
end
def link(rel, attribute)
links.dig(rel, attribute)
end
private
def links
@links ||= @json['links'].map { |link| [link['rel'], link] }.to_h
end
end
def initialize(uri)
_, @domain = uri.split('@')
raise ArgumentError, 'Webfinger requested for local account' if @domain.nil?
@uri = uri
end
def perform
Response.new(body_from_webfinger)
rescue Oj::ParseError
raise Webfinger::Error, "Invalid JSON in response for #{@uri}"
rescue Addressable::URI::InvalidURIError
raise Webfinger::Error, "Invalid URI for #{@uri}"
end
private
def body_from_webfinger(url = standard_url, use_fallback = true)
webfinger_request(url).perform do |res|
if res.code == 200
res.body_with_limit
elsif res.code == 404 && use_fallback
body_from_host_meta
else
raise Webfinger::Error, "Request for #{@uri} returned HTTP #{res.code}"
end
end
end
def body_from_host_meta
host_meta_request.perform do |res|
if res.code == 200
body_from_webfinger(url_from_template(res.body_with_limit), false)
else
raise Webfinger::Error, "Request for #{@uri} returned HTTP #{res.code}"
end
end
end
def url_from_template(str)
link = Nokogiri::XML(str).at_xpath('//xmlns:Link[@rel="lrdd"]')
if link.present?
link['template'].gsub('{uri}', @uri)
else
raise Webfinger::Error, "Request for #{@uri} returned host-meta without link to Webfinger"
end
rescue Nokogiri::XML::XPath::SyntaxError
raise Webfinger::Error, "Invalid XML encountered in host-meta for #{@uri}"
end
def host_meta_request
Request.new(:get, host_meta_url).add_headers('Accept' => 'application/xrd+xml, application/xml, text/xml')
end
def webfinger_request(url)
Request.new(:get, url).add_headers('Accept' => 'application/jrd+json, application/json')
end
def standard_url
"https://#{@domain}/.well-known/webfinger?resource=#{@uri}"
end
def host_meta_url
"https://#{@domain}/.well-known/host-meta"
end
end

View File

@ -33,7 +33,7 @@ class AccountAlias < ApplicationRecord
def set_uri def set_uri
target_account = ResolveAccountService.new.call(acct) target_account = ResolveAccountService.new.call(acct)
self.uri = ActivityPub::TagManager.instance.uri_for(target_account) unless target_account.nil? self.uri = ActivityPub::TagManager.instance.uri_for(target_account) unless target_account.nil?
rescue Goldfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error
# Validation will take care of it # Validation will take care of it
end end

View File

@ -54,7 +54,7 @@ class AccountMigration < ApplicationRecord
def set_target_account def set_target_account
self.target_account = ResolveAccountService.new.call(acct) self.target_account = ResolveAccountService.new.call(acct)
rescue Goldfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error
# Validation will take care of it # Validation will take care of it
end end

View File

@ -32,7 +32,7 @@ class Form::Redirect
def set_target_account def set_target_account
@target_account = ResolveAccountService.new.call(acct) @target_account = ResolveAccountService.new.call(acct)
rescue Goldfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error
# Validation will take care of it # Validation will take care of it
end end

View File

@ -56,7 +56,7 @@ class RemoteFollow
if domain.nil? if domain.nil?
@addressable_template = Addressable::Template.new("#{authorize_interaction_url}?uri={uri}") @addressable_template = Addressable::Template.new("#{authorize_interaction_url}?uri={uri}")
elsif redirect_url_link.nil? || redirect_url_link.template.nil? elsif redirect_uri_template.nil?
missing_resource_error missing_resource_error
else else
@addressable_template = Addressable::Template.new(redirect_uri_template) @addressable_template = Addressable::Template.new(redirect_uri_template)
@ -64,16 +64,12 @@ class RemoteFollow
end end
def redirect_uri_template def redirect_uri_template
redirect_url_link.template acct_resource&.link('http://ostatus.org/schema/1.0/subscribe', 'template')
end
def redirect_url_link
acct_resource&.link('http://ostatus.org/schema/1.0/subscribe')
end end
def acct_resource def acct_resource
@acct_resource ||= webfinger!("acct:#{acct}") @acct_resource ||= webfinger!("acct:#{acct}")
rescue Goldfinger::Error, HTTP::ConnectionError rescue Webfinger::Error, HTTP::ConnectionError
nil nil
end end

View File

@ -39,17 +39,16 @@ class ActivityPub::FetchRemoteAccountService < BaseService
webfinger = webfinger!("acct:#{@username}@#{@domain}") webfinger = webfinger!("acct:#{@username}@#{@domain}")
confirmed_username, confirmed_domain = split_acct(webfinger.subject) confirmed_username, confirmed_domain = split_acct(webfinger.subject)
return webfinger.link('self')&.href == @uri if @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero? return webfinger.link('self', 'href') == @uri if @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero?
webfinger = webfinger!("acct:#{confirmed_username}@#{confirmed_domain}") webfinger = webfinger!("acct:#{confirmed_username}@#{confirmed_domain}")
@username, @domain = split_acct(webfinger.subject) @username, @domain = split_acct(webfinger.subject)
self_reference = webfinger.link('self')
return false unless @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero? return false unless @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero?
return false if self_reference&.href != @uri return false if webfinger.link('self', 'href') != @uri
true true
rescue Goldfinger::Error rescue Webfinger::Error
false false
end end

View File

@ -29,7 +29,7 @@ class ProcessMentionsService < BaseService
if mention_undeliverable?(mentioned_account) if mention_undeliverable?(mentioned_account)
begin begin
mentioned_account = resolve_account_service.call(Regexp.last_match(1)) mentioned_account = resolve_account_service.call(Regexp.last_match(1))
rescue Goldfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::UnexpectedResponseError rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::UnexpectedResponseError
mentioned_account = nil mentioned_account = nil
end end
end end

View File

@ -26,11 +26,10 @@ class ResolveAccountService < BaseService
@account ||= Account.find_remote(@username, @domain) @account ||= Account.find_remote(@username, @domain)
return @account if @account&.local? || !webfinger_update_due? return @account if @account&.local? || @domain.nil? || !webfinger_update_due?
# At this point we are in need of a Webfinger query, which may # At this point we are in need of a Webfinger query, which may
# yield us a different username/domain through a redirect # yield us a different username/domain through a redirect
process_webfinger!(@uri) process_webfinger!(@uri)
# Because the username/domain pair may be different than what # Because the username/domain pair may be different than what
@ -47,7 +46,7 @@ class ResolveAccountService < BaseService
# either needs to be created, or updated from fresh data # either needs to be created, or updated from fresh data
process_account! process_account!
rescue Goldfinger::Error, WebfingerRedirectError, Oj::ParseError => e rescue Webfinger::Error, WebfingerRedirectError, Oj::ParseError => e
Rails.logger.debug "Webfinger query for #{@uri} failed: #{e}" Rails.logger.debug "Webfinger query for #{@uri} failed: #{e}"
nil nil
end end
@ -118,11 +117,11 @@ class ResolveAccountService < BaseService
end end
def activitypub_ready? def activitypub_ready?
!@webfinger.link('self').nil? && ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(@webfinger.link('self').type) ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(@webfinger.link('self', 'type'))
end end
def actor_url def actor_url
@actor_url ||= @webfinger.link('self').href @actor_url ||= @webfinger.link('self', 'href')
end end
def actor_json def actor_json

View File

@ -5,7 +5,6 @@ doc << Ox::Element.new('XRD').tap do |xrd|
xrd << Ox::Element.new('Link').tap do |link| xrd << Ox::Element.new('Link').tap do |link|
link['rel'] = 'lrdd' link['rel'] = 'lrdd'
link['type'] = 'application/xrd+xml'
link['template'] = @webfinger_template link['template'] = @webfinger_template
end end
end end

View File

@ -43,8 +43,7 @@ describe RemoteFollowController do
end end
it 'renders new when template is nil' do it 'renders new when template is nil' do
link_with_nil_template = double(template: nil) resource_with_link = double(link: nil)
resource_with_link = double(link: link_with_nil_template)
allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_return(resource_with_link) allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_return(resource_with_link)
post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } } post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } }
@ -55,8 +54,7 @@ describe RemoteFollowController do
context 'when webfinger values are good' do context 'when webfinger values are good' do
before do before do
link_with_template = double(template: 'http://example.com/follow_me?acct={uri}') resource_with_link = double(link: 'http://example.com/follow_me?acct={uri}')
resource_with_link = double(link: link_with_template)
allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_return(resource_with_link) allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_return(resource_with_link)
post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } } post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } }
end end
@ -78,8 +76,8 @@ describe RemoteFollowController do
expect(response).to render_template(:new) expect(response).to render_template(:new)
end end
it 'renders new with error when goldfinger fails' do it 'renders new with error when webfinger fails' do
allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_raise(Goldfinger::Error) allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_raise(Webfinger::Error)
post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } } post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } }
expect(response).to render_template(:new) expect(response).to render_template(:new)

View File

@ -12,7 +12,7 @@ describe WellKnown::HostMetaController, type: :controller do
expect(response.body).to eq <<XML expect(response.body).to eq <<XML
<?xml version="1.0" encoding="UTF-8"?> <?xml version="1.0" encoding="UTF-8"?>
<XRD xmlns="http://docs.oasis-open.org/ns/xri/xrd-1.0"> <XRD xmlns="http://docs.oasis-open.org/ns/xri/xrd-1.0">
<Link rel="lrdd" type="application/xrd+xml" template="https://cb6e6126.ngrok.io/.well-known/webfinger?resource={uri}"/> <Link rel="lrdd" template="https://cb6e6126.ngrok.io/.well-known/webfinger?resource={uri}"/>
</XRD> </XRD>
XML XML
end end

View File

@ -108,6 +108,7 @@ RSpec.describe FeedManager do
it 'returns false for status by followee mentioning another account' do it 'returns false for status by followee mentioning another account' do
bob.follow!(alice) bob.follow!(alice)
jeff.follow!(alice)
status = PostStatusService.new.call(alice, text: 'Hey @jeff') status = PostStatusService.new.call(alice, text: 'Hey @jeff')
expect(FeedManager.instance.filter?(:home, status, bob)).to be false expect(FeedManager.instance.filter?(:home, status, bob)).to be false
end end