From 98ff8cbae0d7beda9df4a2e135d638529ad4d0b2 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 6 Feb 2016 01:29:25 +0100 Subject: [PATCH] add new (alphabetic) signature logic --- lib/diaspora_federation/entities/relayable.rb | 46 +++++- .../entities/relayable_spec.rb | 143 +++++++++++++----- 2 files changed, 144 insertions(+), 45 deletions(-) diff --git a/lib/diaspora_federation/entities/relayable.rb b/lib/diaspora_federation/entities/relayable.rb index bcec828..269526a 100644 --- a/lib/diaspora_federation/entities/relayable.rb +++ b/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)) - logger.info "event=verify_signature status=complete guid=#{guid} validity=#{validity}" - validity + valid = verify_legacy_signature(pubkey, signature) + + 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 diff --git a/spec/lib/diaspora_federation/entities/relayable_spec.rb b/spec/lib/diaspora_federation/entities/relayable_spec.rb index 1af4d53..c961462 100644 --- a/spec/lib/diaspora_federation/entities/relayable_spec.rb +++ b/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) { - { - guid: FactoryGirl.generate(:guid), - diaspora_id: FactoryGirl.generate(:diaspora_id), - parent_guid: FactoryGirl.generate(:guid), - some_other_data: "a_random_string" - } - } + + let(:guid) { FactoryGirl.generate(:guid) } + let(:parent_guid) { FactoryGirl.generate(:guid) } + let(:diaspora_id) { FactoryGirl.generate(:diaspora_id) } + let(:new_property) { "some text" } + 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) - pubkey.verify(OpenSSL::Digest::SHA256.new, Base64.decode64(signature), signed_string) - 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 + expect(verify_signature(author_pkey, signed_hash[:author_signature], legacy_signature_data)).to be_truthy + expect(verify_signature(parent_pkey, signed_hash[:parent_author_signature], legacy_signature_data)).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