Add Digest header to requests with body, handle acct and URI keyId (#4565)

3 files changed, 100 insertions, 25 deletions

diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb
index abe845d93..aeb8da879 100644
--- 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
index e73c5ac20..c01e07925 100644
--- 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
index b371795ab..64648621e 100644
--- 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