Merge f56a1546e7 into 20c105c194

app/lib/signed_request.rb

@@ -25,14 +25,7 @@ class SignedRequest
 
     def verified?(keypair)
       signature = Base64.decode64(signature_params['signature'])
-      compare_signed_string = build_signed_string(include_query_string: true)
-
-      return true unless verify_signature(keypair, signature, compare_signed_string).nil?
-
-      compare_signed_string = build_signed_string(include_query_string: false)
-      return true unless verify_signature(keypair, signature, compare_signed_string).nil?
-
-      false
+      verify_signature(keypair, signature, build_signed_string)
     end
 
     def created_time
@@ -100,23 +93,16 @@ class SignedRequest
     end
 
     def verify_signature(keypair, signature, compare_signed_string)
-      true if keypair.keypair.public_key.verify(OpenSSL::Digest.new('SHA256'), signature, compare_signed_string)
+      keypair.keypair.public_key.verify(OpenSSL::Digest.new('SHA256'), signature, compare_signed_string)
     rescue OpenSSL::PKey::PKeyError
-      nil
+      false
     end
 
-    def build_signed_string(include_query_string: true)
+    def build_signed_string
       signed_headers.map do |signed_header|
         case signed_header
         when HttpSignatureDraft::REQUEST_TARGET
-          if include_query_string
-            "#{HttpSignatureDraft::REQUEST_TARGET}: #{@request.method.downcase} #{@request.original_fullpath}"
-          else
-            # Current versions of Mastodon incorrectly omit the query string from the (request-target) pseudo-header.
-            # Therefore, temporarily support such incorrect signatures for compatibility.
-            # TODO: remove eventually some time after release of the fixed version
-            "#{HttpSignatureDraft::REQUEST_TARGET}: #{@request.method.downcase} #{@request.path}"
-          end
+          "#{HttpSignatureDraft::REQUEST_TARGET}: #{@request.method.downcase} #{@request.original_fullpath}"
         when '(created)'
           raise Mastodon::SignatureVerificationError, 'Invalid pseudo-header (created) for rsa-sha256' unless signature_algorithm == 'hs2019'
           raise Mastodon::SignatureVerificationError, 'Pseudo-header (created) used but corresponding argument missing' if signature_params['created'].blank?

spec/requests/signature_verification_spec.rb

@@ -181,11 +181,14 @@ RSpec.describe 'signature verification concern' do
         end
       end
 
-      context 'when the query string is missing from the signature verification (compatibility quirk)' do
+      context 'when the query string is missing from the signature verification (dropped compatibility quirk)' do
         let(:signature_header) do
           'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="date host (request-target)",signature="Z8ilar3J7bOwqZkMp7sL8sRs4B1FT+UorbmvWoE+A5UeoOJ3KBcUmbsh+k3wQwbP5gMNUrra9rEWabpasZGphLsbDxfbsWL3Cf0PllAc7c1c7AFEwnewtExI83/qqgEkfWc2z7UDutXc2NfgAx89Ox8DXU/fA2GG0jILjB6UpFyNugkY9rg6oI31UnvfVi3R7sr3/x8Ea3I9thPvqI2byF6cojknSpDAwYzeKdngX3TAQEGzFHz3SDWwyp3jeMWfwvVVbM38FxhvAnSumw7YwWW4L7M7h4M68isLimoT3yfCn2ucBVL5Dz8koBpYf/40w7QidClAwCafZQFC29yDOg=="' # rubocop:disable Layout/LineLength
         end
 
+        # Signature verification will fail, so the key will be refetched
+        before { stub_key_requests }
+
         it 'successfuly verifies signature', :aggregate_failures do
           expect(signature_header).to eq build_signature_string(actor_keypair, 'https://remote.domain/users/bob#main-key', 'get /activitypub/success', { 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', 'Host' => 'www.example.com' })
 
@@ -195,10 +198,10 @@ RSpec.describe 'signature verification concern' do
             'Signature' => signature_header,
           }
 
-          expect(response).to have_http_status(200)
           expect(response.parsed_body).to match(
             signed_request: true,
-            signature_actor_id: actor.id.to_s
+            signature_actor_id: nil,
+            error: anything
           )
         end
       end