commit: fdea173237cfcd3a6b36f6ebccb0cb1a21cf9294
parent: 4e1bf082ce83ca941f20993fcfb8b6f9597624c6
Author: Eugen Rochko <eugen@zeonfederated.com>
Date: Wed, 9 Aug 2017 23:54:14 +0200
Add Digest header to requests with body, handle acct and URI keyId (#4565)
Diffstat:
3 files changed, 100 insertions(+), 25 deletions(-)
diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb
@@ -31,7 +31,7 @@ module SignatureVerification
return
end
- account = ResolveRemoteAccountService.new.call(signature_params['keyId'].gsub(/\Aacct:/, ''))
+ account = account_from_key_id(signature_params['keyId'])
if account.nil?
@signed_request_account = nil
@@ -49,6 +49,10 @@ module SignatureVerification
end
end
+ def request_body
+ @request_body ||= request.raw_post
+ end
+
private
def build_signed_string(signed_headers)
@@ -57,6 +61,8 @@ module SignatureVerification
signed_headers.split(' ').map do |signed_header|
if signed_header == Request::REQUEST_TARGET
"#{Request::REQUEST_TARGET}: #{request.method.downcase} #{request.path}"
+ elsif signed_header == 'digest'
+ "digest: #{body_digest}"
else
"#{signed_header}: #{request.headers[to_header_name(signed_header)]}"
end
@@ -73,6 +79,10 @@ module SignatureVerification
(Time.now.utc - time_sent).abs <= 30
end
+ def body_digest
+ "SHA-256=#{Digest::SHA256.base64digest(request_body)}"
+ end
+
def to_header_name(name)
name.split(/-/).map(&:capitalize).join('-')
end
@@ -81,7 +91,14 @@ module SignatureVerification
signature_params['keyId'].blank? ||
signature_params['signature'].blank? ||
signature_params['algorithm'].blank? ||
- signature_params['algorithm'] != 'rsa-sha256' ||
- !signature_params['keyId'].start_with?('acct:')
+ signature_params['algorithm'] != 'rsa-sha256'
+ end
+
+ def account_from_key_id(key_id)
+ if key_id.start_with?('acct:')
+ ResolveRemoteAccountService.new.call(key_id.gsub(/\Aacct:/, ''))
+ elsif !ActivityPub::TagManager.instance.local_uri?(key_id)
+ ActivityPub::FetchRemoteAccountService.new.call(key_id)
+ end
end
end
diff --git a/app/lib/request.rb b/app/lib/request.rb
@@ -12,15 +12,21 @@ class Request
@headers = {}
set_common_headers!
+ set_digest! if options.key?(:body)
end
- def on_behalf_of(account)
+ def on_behalf_of(account, key_id_format = :acct)
raise ArgumentError unless account.local?
- @account = account
+
+ @account = account
+ @key_id_format = key_id_format
+
+ self
end
def add_headers(new_headers)
@headers.merge!(new_headers)
+ self
end
def perform
@@ -40,8 +46,11 @@ class Request
@headers['Date'] = Time.now.utc.httpdate
end
+ def set_digest!
+ @headers['Digest'] = "SHA-256=#{Digest::SHA256.base64digest(@options[:body])}"
+ end
+
def signature
- key_id = @account.to_webfinger_s
algorithm = 'rsa-sha256'
signature = Base64.strict_encode64(@account.keypair.sign(OpenSSL::Digest::SHA256.new, signed_string))
@@ -60,6 +69,15 @@ class Request
@user_agent ||= "#{HTTP::Request::USER_AGENT} (Mastodon/#{Mastodon::Version}; +#{root_url})"
end
+ def key_id
+ case @key_id_format
+ when :acct
+ @account.to_webfinger_s
+ when :uri
+ [ActivityPub::TagManager.instance.uri_for(@account), '#main-key'].join
+ end
+ end
+
def timeout
{ write: 10, connect: 10, read: 10 }
end
diff --git a/spec/controllers/concerns/signature_verification_spec.rb b/spec/controllers/concerns/signature_verification_spec.rb
@@ -16,7 +16,7 @@ describe ApplicationController, type: :controller do
end
before do
- routes.draw { get 'success' => 'anonymous#success' }
+ routes.draw { match via: [:get, :post], 'success' => 'anonymous#success' }
end
context 'without signature header' do
@@ -40,34 +40,74 @@ describe ApplicationController, type: :controller do
context 'with signature header' do
let!(:author) { Fabricate(:account) }
- before do
- get :success
+ context 'without body' do
+ before do
+ get :success
- fake_request = Request.new(:get, request.url)
- fake_request.on_behalf_of(author)
+ fake_request = Request.new(:get, request.url)
+ fake_request.on_behalf_of(author)
- request.headers.merge!(fake_request.headers)
- end
+ request.headers.merge!(fake_request.headers)
+ end
- describe '#signed_request?' do
- it 'returns true' do
- expect(controller.signed_request?).to be true
+ describe '#signed_request?' do
+ it 'returns true' do
+ expect(controller.signed_request?).to be true
+ end
+ end
+
+ describe '#signed_request_account' do
+ it 'returns an account' do
+ expect(controller.signed_request_account).to eq author
+ end
+
+ it 'returns nil when path does not match' do
+ request.path = '/alternative-path'
+ expect(controller.signed_request_account).to be_nil
+ end
+
+ it 'returns nil when method does not match' do
+ post :success
+ expect(controller.signed_request_account).to be_nil
+ end
end
end
- describe '#signed_request_account' do
- it 'returns an account' do
- expect(controller.signed_request_account).to eq author
+ context 'with body' do
+ before do
+ post :success, body: 'Hello world'
+
+ fake_request = Request.new(:post, request.url, body: 'Hello world')
+ fake_request.on_behalf_of(author)
+
+ request.headers.merge!(fake_request.headers)
end
- it 'returns nil when path does not match' do
- request.path = '/alternative-path'
- expect(controller.signed_request_account).to be_nil
+ describe '#signed_request?' do
+ it 'returns true' do
+ expect(controller.signed_request?).to be true
+ end
end
- it 'returns nil when method does not match' do
- post :success
- expect(controller.signed_request_account).to be_nil
+ describe '#signed_request_account' do
+ it 'returns an account' do
+ expect(controller.signed_request_account).to eq author
+ end
+
+ it 'returns nil when path does not match' do
+ request.path = '/alternative-path'
+ expect(controller.signed_request_account).to be_nil
+ end
+
+ it 'returns nil when method does not match' do
+ get :success
+ expect(controller.signed_request_account).to be_nil
+ end
+
+ it 'returns nil when body has been tampered' do
+ request.headers['RAW_POST_DATA'] = 'doo doo doo'
+ expect(controller.signed_request_account).to be_nil
+ end
end
end
end
