add new (alphabetic) signature logic

lib/diaspora_federation/entities/relayable.rb

@@ -62,7 +62,7 @@ module DiasporaFederation
             privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key_by_diaspora_id, diaspora_id)
             raise AuthorPrivateKeyNotFound, "author=#{diaspora_id} guid=#{guid}" if privkey.nil?
             hash[:author_signature] = sign_with_key(privkey, hash)
-            logger.info "event=sign_with_key signature=author_signature author=#{diaspora_id} guid=#{guid}"
+            logger.info "event=sign status=complete signature=author_signature author=#{diaspora_id} guid=#{guid}"
           end
 
           try_sign_with_parent_author(hash) if parent_author_signature.nil?
@@ -101,7 +101,7 @@ module DiasporaFederation
         )
         unless privkey.nil?
           hash[:parent_author_signature] = sign_with_key(privkey, hash)
-          logger.info "event=sign_with_key signature=parent_author_signature guid=#{guid}"
+          logger.info "event=sign status=complete signature=parent_author_signature guid=#{guid}"
         end
       end
 
@@ -121,23 +121,55 @@ module DiasporaFederation
       # @param [Hash] hash data to sign
       # @return [String] A Base64 encoded signature of #signable_string with key
       def sign_with_key(privkey, hash)
-        Base64.strict_encode64(privkey.sign(DIGEST, legacy_signature_data(hash)))
+        signature_data_string = if additional_xml_elements
+                                  logger.info "event=sign method=alphabetic guid=#{guid}"
+                                  signature_data(hash.merge(additional_xml_elements))
+                                else
+                                  logger.info "event=sign method=legacy guid=#{guid}"
+                                  legacy_signature_data(hash)
+                                end
+        Base64.strict_encode64(privkey.sign(DIGEST, signature_data_string))
       end
 
       # Check that signature is a correct signature
       #
       # @param [OpenSSL::PKey::RSA] pubkey An RSA key
       # @param [String] signature The signature to be verified.
-      # @return [Boolean]
+      # @return [Boolean] signature valid
       def verify_signature(pubkey, signature)
         if signature.nil?
           logger.warn "event=verify_signature status=abort reason=no_signature guid=#{guid}"
           return false
         end
 
-        validity = pubkey.verify(DIGEST, Base64.decode64(signature), legacy_signature_data(to_h))
+        valid = verify_legacy_signature(pubkey, signature)
-        logger.info "event=verify_signature status=complete guid=#{guid} validity=#{validity}"
+
-        validity
+        unless valid
+          logger.info "event=verify_signature method=alphabetic status=retry guid=#{guid}"
+          hash_to_verify = additional_xml_elements ? to_h.merge(additional_xml_elements) : to_h
+          valid = pubkey.verify(DIGEST, Base64.decode64(signature), signature_data(hash_to_verify))
+        end
+
+        logger.info "event=verify_signature status=complete guid=#{guid} valid=#{valid}"
+        valid
+      end
+
+      def verify_legacy_signature(pubkey, signature)
+        if additional_xml_elements
+          logger.info "event=verify_signature method=legacy status=skip guid=#{guid}"
+          false
+        else
+          valid = pubkey.verify(DIGEST, Base64.decode64(signature), legacy_signature_data(to_h))
+          logger.info "event=verify_signature method=legacy guid=#{guid} valid=#{valid}"
+          valid
+        end
+      end
+
+      # @param [Hash] hash data to sign
+      # @return [String] signature data string
+      def signature_data(hash)
+        # remove signatures from hash and sort properties alphabetically
+        Hash[hash.reject {|name, _| name =~ /signature/ }.map {|name, value| [name.to_s, value] }.sort].values.join(";")
       end
 
       # @param [Hash] hash data to sign

spec/lib/diaspora_federation/entities/relayable_spec.rb

@@ -2,14 +2,16 @@ module DiasporaFederation
   describe Entities::Relayable do
     let(:author_pkey) { OpenSSL::PKey::RSA.generate(1024) }
     let(:parent_pkey) { OpenSSL::PKey::RSA.generate(1024) }
-    let(:hash) {
+
-      {
+    let(:guid) { FactoryGirl.generate(:guid) }
-        guid:            FactoryGirl.generate(:guid),
+    let(:parent_guid) { FactoryGirl.generate(:guid) }
-        diaspora_id:     FactoryGirl.generate(:diaspora_id),
+    let(:diaspora_id) { FactoryGirl.generate(:diaspora_id) }
-        parent_guid:     FactoryGirl.generate(:guid),
+    let(:new_property) { "some text" }
-        some_other_data: "a_random_string"
+    let(:hash) { {guid: guid, diaspora_id: diaspora_id, parent_guid: parent_guid} }
-      }
+
-    }
+    let(:signature_data) { "#{diaspora_id};#{guid};#{parent_guid}" }
+    let(:signature_data_with_new_property) { "#{diaspora_id};#{guid};#{new_property};#{parent_guid}" }
+    let(:legacy_signature_data) { "#{guid};#{diaspora_id};#{parent_guid}" }
 
     class SomeRelayable < Entity
       LEGACY_SIGNATURE_ORDER = %i(guid diaspora_id parent_guid).freeze
@@ -25,10 +27,6 @@ module DiasporaFederation
     end
 
     describe "#verify_signatures" do
-      def legacy_signature_data
-        %i(guid diaspora_id parent_guid).map {|name| hash[name] }.join(";")
-      end
-
       def sign_with_key(privkey, signature_data)
         Base64.strict_encode64(privkey.sign(OpenSSL::Digest::SHA256.new, signature_data))
       end
@@ -39,15 +37,15 @@ module DiasporaFederation
         signed_hash[:parent_author_signature] = sign_with_key(parent_pkey, legacy_signature_data)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_public_key_by_diaspora_id, hash[:diaspora_id]
+          :fetch_public_key_by_diaspora_id, diaspora_id
         ).and_return(author_pkey.public_key)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_author_public_key_by_entity_guid, "Parent", hash[:parent_guid]
+          :fetch_author_public_key_by_entity_guid, "Parent", parent_guid
         ).and_return(parent_pkey.public_key)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :entity_author_is_local?, "Parent", hash[:parent_guid]
+          :entity_author_is_local?, "Parent", parent_guid
         ).and_return(false)
 
         expect { SomeRelayable.new(signed_hash).verify_signatures }.not_to raise_error
@@ -67,7 +65,7 @@ module DiasporaFederation
         hash[:author_signature] = nil
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_public_key_by_diaspora_id, hash[:diaspora_id]
+          :fetch_public_key_by_diaspora_id, diaspora_id
         ).and_return(author_pkey.public_key)
 
         expect {
@@ -79,15 +77,15 @@ module DiasporaFederation
         hash[:author_signature] = sign_with_key(author_pkey, legacy_signature_data)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_public_key_by_diaspora_id, hash[:diaspora_id]
+          :fetch_public_key_by_diaspora_id, diaspora_id
         ).and_return(author_pkey.public_key)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_author_public_key_by_entity_guid, "Parent", hash[:parent_guid]
+          :fetch_author_public_key_by_entity_guid, "Parent", parent_guid
         ).and_return(nil)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :entity_author_is_local?, "Parent", hash[:parent_guid]
+          :entity_author_is_local?, "Parent", parent_guid
         ).and_return(false)
 
         expect {
@@ -100,15 +98,15 @@ module DiasporaFederation
         hash[:parent_author_signature] = nil
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_public_key_by_diaspora_id, hash[:diaspora_id]
+          :fetch_public_key_by_diaspora_id, diaspora_id
         ).and_return(author_pkey.public_key)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_author_public_key_by_entity_guid, "Parent", hash[:parent_guid]
+          :fetch_author_public_key_by_entity_guid, "Parent", parent_guid
         ).and_return(parent_pkey.public_key)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :entity_author_is_local?, "Parent", hash[:parent_guid]
+          :entity_author_is_local?, "Parent", parent_guid
         ).and_return(false)
 
         expect {
@@ -121,48 +119,101 @@ module DiasporaFederation
         hash[:parent_author_signature] = nil
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_public_key_by_diaspora_id, hash[:diaspora_id]
+          :fetch_public_key_by_diaspora_id, diaspora_id
         ).and_return(author_pkey.public_key)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :entity_author_is_local?, "Parent", hash[:parent_guid]
+          :entity_author_is_local?, "Parent", parent_guid
         ).and_return(true)
 
         expect { SomeRelayable.new(hash).verify_signatures }.not_to raise_error
       end
+
+      context "new signatures" do
+        it "fallback to new signature verification if the legacy signature-check failed" do
+          signed_hash = hash.dup
+          signed_hash[:author_signature] = sign_with_key(author_pkey, signature_data)
+          signed_hash[:parent_author_signature] = sign_with_key(parent_pkey, signature_data)
+
+          expect(DiasporaFederation.callbacks).to receive(:trigger).with(
+            :fetch_public_key_by_diaspora_id, diaspora_id
+          ).and_return(author_pkey.public_key)
+
+          expect(DiasporaFederation.callbacks).to receive(:trigger).with(
+            :fetch_author_public_key_by_entity_guid, "Parent", parent_guid
+          ).and_return(parent_pkey.public_key)
+
+          expect(DiasporaFederation.callbacks).to receive(:trigger).with(
+            :entity_author_is_local?, "Parent", parent_guid
+          ).and_return(false)
+
+          expect { SomeRelayable.new(signed_hash).verify_signatures }.not_to raise_error
+        end
+
+        it "doesn't raise anything if correct signatures with new property were passed" do
+          signed_hash = hash.dup
+          signed_hash[:author_signature] = sign_with_key(author_pkey, signature_data_with_new_property)
+          signed_hash[:parent_author_signature] = sign_with_key(parent_pkey, signature_data_with_new_property)
+
+          expect(DiasporaFederation.callbacks).to receive(:trigger).with(
+            :fetch_public_key_by_diaspora_id, diaspora_id
+          ).and_return(author_pkey.public_key)
+
+          expect(DiasporaFederation.callbacks).to receive(:trigger).with(
+            :fetch_author_public_key_by_entity_guid, "Parent", parent_guid
+          ).and_return(parent_pkey.public_key)
+
+          expect(DiasporaFederation.callbacks).to receive(:trigger).with(
+            :entity_author_is_local?, "Parent", parent_guid
+          ).and_return(false)
+
+          expect { SomeRelayable.new(signed_hash, "new_property" => new_property).verify_signatures }.not_to raise_error
+        end
+
+        it "raises with legacy-signatures and with new property" do
+          signed_hash = hash.dup
+          signed_hash[:author_signature] = sign_with_key(author_pkey, legacy_signature_data)
+
+          expect(DiasporaFederation.callbacks).to receive(:trigger).with(
+            :fetch_public_key_by_diaspora_id, diaspora_id
+          ).and_return(author_pkey.public_key)
+
+          expect {
+            SomeRelayable.new(signed_hash, "new_property" => new_property).verify_signatures
+          }.to raise_error Entities::Relayable::SignatureVerificationFailed
+        end
+      end
     end
 
     describe "#to_signed_h" do
+      def verify_signature(pubkey, signature, signed_string)
+        pubkey.verify(OpenSSL::Digest::SHA256.new, Base64.decode64(signature), signed_string)
+      end
+
       it "updates signatures when they were nil and keys were supplied" do
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_private_key_by_diaspora_id, hash[:diaspora_id]
+          :fetch_private_key_by_diaspora_id, diaspora_id
         ).and_return(author_pkey)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_author_private_key_by_entity_guid, "Parent", hash[:parent_guid]
+          :fetch_author_private_key_by_entity_guid, "Parent", parent_guid
         ).and_return(parent_pkey)
 
-        signed_string = hash.reject {|key, _| key == :some_other_data }.values.join(";")
-
         signed_hash = SomeRelayable.new(hash).to_signed_h
 
-        def verify_signature(pubkey, signature, signed_string)
+        expect(verify_signature(author_pkey, signed_hash[:author_signature], legacy_signature_data)).to be_truthy
-          pubkey.verify(OpenSSL::Digest::SHA256.new, Base64.decode64(signature), signed_string)
+        expect(verify_signature(parent_pkey, signed_hash[:parent_author_signature], legacy_signature_data)).to be_truthy
-        end
-
-        expect(verify_signature(author_pkey, signed_hash[:author_signature], signed_string)).to be_truthy
-        expect(verify_signature(parent_pkey, signed_hash[:parent_author_signature], signed_string)).to be_truthy
       end
 
       it "doesn't change signatures if they are already set" do
-        hash.merge!(author_signature: "aa", parent_author_signature: "bb").delete(:some_other_data)
+        hash.merge!(author_signature: "aa", parent_author_signature: "bb")
 
         expect(SomeRelayable.new(hash).to_signed_h).to eq(hash)
       end
 
       it "raises when author_signature not set and key isn't supplied" do
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_private_key_by_diaspora_id, hash[:diaspora_id]
+          :fetch_private_key_by_diaspora_id, diaspora_id
         ).and_return(nil)
 
         expect {
@@ -172,17 +223,33 @@ module DiasporaFederation
 
       it "doesn't set parent_author_signature if key isn't supplied" do
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_private_key_by_diaspora_id, hash[:diaspora_id]
+          :fetch_private_key_by_diaspora_id, diaspora_id
         ).and_return(author_pkey)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_author_private_key_by_entity_guid, "Parent", hash[:parent_guid]
+          :fetch_author_private_key_by_entity_guid, "Parent", parent_guid
         ).and_return(nil)
 
         signed_hash = SomeRelayable.new(hash).to_signed_h
 
         expect(signed_hash[:parent_author_signature]).to eq(nil)
       end
+
+      context "new signatures" do
+        it "updates parent_author_signature with new signature-data if additional_xml_elements is present" do
+          hash[:author_signature] = "aa"
+
+          expect(DiasporaFederation.callbacks).to receive(:trigger).with(
+            :fetch_author_private_key_by_entity_guid, "Parent", parent_guid
+          ).and_return(parent_pkey)
+
+          signed_hash = SomeRelayable.new(hash, "new_property" => new_property).to_signed_h
+
+          expect(
+            verify_signature(parent_pkey, signed_hash[:parent_author_signature], signature_data_with_new_property)
+          ).to be_truthy
+        end
+      end
     end
   end
 end