refactor signature verification for relayables

lib/diaspora_federation/entities/message.rb

@@ -37,9 +37,7 @@ module DiasporaFederation
 
       # @deprecated remove after {Message} doesn't include {Relayable} anymore
       def verify_author_signature
-        pubkey = DiasporaFederation.callbacks.trigger(:fetch_public_key, author)
+        verify_signature(author, :author_signature)
-        raise PublicKeyNotFound, "author_signature author=#{author} obj=#{self}" if pubkey.nil?
-        raise SignatureVerificationFailed, "obj=#{self}" unless verify_signature(pubkey, author_signature)
         true
       end
 

lib/diaspora_federation/entities/relayable.rb

@@ -81,13 +81,10 @@ module DiasporaFederation
       # verifies the signatures (+author_signature+ and +parent_author_signature+ if needed)
       # @raise [SignatureVerificationFailed] if the signature is not valid or no public key is found
       def verify_signatures
-        pubkey = DiasporaFederation.callbacks.trigger(:fetch_public_key, author)
+        verify_signature(author, :author_signature)
-        raise PublicKeyNotFound, "author_signature author=#{author} obj=#{self}" if pubkey.nil?
-        unless verify_signature(pubkey, author_signature)
-          raise SignatureVerificationFailed, "wrong author_signature for #{self}"
-        end
 
-        verify_parent_author_signature unless parent.local
+        # this happens only on downstream federation
+        verify_signature(parent.author, :parent_author_signature) unless parent.local
       end
 
       def sender_valid?(sender)
@@ -101,29 +98,22 @@ module DiasporaFederation
 
       private
 
-      # this happens only on downstream federation
-      def verify_parent_author_signature
-        pubkey = DiasporaFederation.callbacks.trigger(:fetch_public_key, parent.author)
-        raise PublicKeyNotFound, "parent_author_signature parent_author=#{parent.author} obj=#{self}" if pubkey.nil?
-        unless verify_signature(pubkey, parent_author_signature)
-          raise SignatureVerificationFailed, "wrong parent_author_signature for #{self}"
-        end
-      end
-
       # Check that signature is a correct signature
       #
-      # @param [OpenSSL::PKey::RSA] pubkey An RSA key
+      # @param [String] author The author of the signature
-      # @param [String] signature The signature to be verified.
+      # @param [String] signature_key The signature to be verified
       # @return [Boolean] signature valid
-      def verify_signature(pubkey, signature)
+      def verify_signature(author, signature_key)
-        if signature.nil?
+        pubkey = DiasporaFederation.callbacks.trigger(:fetch_public_key, author)
-          logger.warn "event=verify_signature status=abort reason=no_signature guid=#{guid}"
+        raise PublicKeyNotFound, "signature=#{signature_key} person=#{author} obj=#{self}" if pubkey.nil?
-          return false
-        end
 
-        pubkey.verify(DIGEST, Base64.decode64(signature), signature_data).tap do |valid|
+        signature = public_send(signature_key)
-          logger.info "event=verify_signature status=complete obj=#{self} valid=#{valid}"
+        raise SignatureVerificationFailed, "no #{signature_key} for #{self}" if signature.nil?
-        end
+
+        valid = pubkey.verify(DIGEST, Base64.decode64(signature), signature_data)
+        raise SignatureVerificationFailed, "wrong #{signature_key} for #{self}" unless valid
+
+        logger.info "event=verify_signature signature=#{signature_key} status=valid obj=#{self}"
       end
 
       # sign with author key

spec/lib/diaspora_federation/entities/message_spec.rb

@@ -51,10 +51,10 @@ XML
       it "does not allow the parent author if the signature is invalid" do
         expect_callback(:fetch_related_entity, "Conversation", entity.conversation_guid).and_return(data[:parent])
         expect_callback(:fetch_public_key, alice.diaspora_id).and_return(alice.private_key)
-        invalid_entity = Entities::Message.new(data.merge(author_signature: "aa"))
+        invalid_msg = Entities::Message.new(data.merge(author_signature: "aa"))
         expect {
-          invalid_entity.sender_valid?(bob.diaspora_id)
+          invalid_msg.sender_valid?(bob.diaspora_id)
-        }.to raise_error Entities::Relayable::SignatureVerificationFailed, "obj=#{invalid_entity}"
+        }.to raise_error Entities::Relayable::SignatureVerificationFailed, "wrong author_signature for #{invalid_msg}"
       end
 
       it "raises NotFetchable if the parent Conversation can not be found" do