Add Digest header to requests with body, handle acct and URI keyId (#4565)
This commit is contained in:
		
							parent
							
								
									34c0f39d72
								
							
						
					
					
						commit
						1b6ced5887
					
				
					 3 changed files with 100 additions and 25 deletions
				
			
		|  | @ -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 | ||||
|  |  | |||
|  | @ -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 | ||||
|     @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 | ||||
|  |  | |||
|  | @ -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,6 +40,7 @@ describe ApplicationController, type: :controller do | |||
|   context 'with signature header' do | ||||
|     let!(:author) { Fabricate(:account) } | ||||
| 
 | ||||
|     context 'without body' do | ||||
|       before do | ||||
|         get :success | ||||
| 
 | ||||
|  | @ -71,4 +72,43 @@ describe ApplicationController, type: :controller do | |||
|         end | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     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 | ||||
| 
 | ||||
|       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 | ||||
|           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 | ||||
| end | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue