From f0fc62e94d27d27e9fc6bc48455b415d555f5922 Mon Sep 17 00:00:00 2001 From: cmrd Senya Date: Wed, 2 Dec 2015 18:01:58 +0300 Subject: [PATCH] Fix a security issue that author_signature is not checked on the to-downstream receive of a federated relayable entity, allowing to forge relayables if you are an owner of the pod where a parent object is stored. closes #6539 --- Changelog.md | 2 ++ lib/diaspora/relayable.rb | 6 ++++++ .../federation/federation_messages_generation.rb | 7 +++++++ spec/integration/federation/shared_receive_relayable.rb | 8 ++++++++ 4 files changed, 23 insertions(+) diff --git a/Changelog.md b/Changelog.md index eece68961..51974a7db 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,10 +1,12 @@ # 0.5.6.0 ## Refactor +* Add more integration tests with the help of the new diaspora-federation gem [#6539](https://github.com/diaspora/diaspora/pull/6539) ## Bug fixes * Fix mention autocomplete when pasting the username [#6510](https://github.com/diaspora/diaspora/pull/6510) * Use and update updated\_at for notifications [#6573](https://github.com/diaspora/diaspora/pull/6573) +* Ensure the author signature is checked when receiving a relayable [#6539](https://github.com/diaspora/diaspora/pull/6539) ## Features diff --git a/lib/diaspora/relayable.rb b/lib/diaspora/relayable.rb index 87c19a2ff..3edd989c4 100644 --- a/lib/diaspora/relayable.rb +++ b/lib/diaspora/relayable.rb @@ -69,6 +69,12 @@ module Diaspora def receive(user, person=nil) comment_or_like = self.class.where(guid: self.guid).first || self + unless comment_or_like.signature_valid? + logger.warn "event=receive status=abort reason='object signature not valid' recipient=#{user.diaspora_handle} "\ + "sender=#{parent.author.diaspora_handle} payload_type=#{self.class} parent_id=#{parent.id}" + return + end + # Check to make sure the signature of the comment or like comes from the person claiming to author it unless comment_or_like.parent_author == user.person || comment_or_like.verify_parent_author_signature logger.warn "event=receive status=abort reason='object signature not valid' recipient=#{user.diaspora_handle} "\ diff --git a/spec/integration/federation/federation_messages_generation.rb b/spec/integration/federation/federation_messages_generation.rb index 6dff58e40..59bcc315e 100644 --- a/spec/integration/federation/federation_messages_generation.rb +++ b/spec/integration/federation/federation_messages_generation.rb @@ -117,6 +117,13 @@ def generate_relayable_local_parent_wrong_author_key(entity_name) generate_relayable_local_parent(entity_name) end +# Checks when a remote pod B wants to send us a relayable with authorship from a remote pod C user +# without having correct signature from him. +def generate_relayable_remote_parent_wrong_author_key(entity_name) + substitute_wrong_key(@remote_user2, 1) + generate_relayable_remote_parent(entity_name) +end + # Checks when a remote pod C wants to send us a relayable from its user, but bypassing the pod B where # remote status came from. def generate_relayable_remote_parent_wrong_parent_key(entity_name) diff --git a/spec/integration/federation/shared_receive_relayable.rb b/spec/integration/federation/shared_receive_relayable.rb index b4ad41db3..2d25cce46 100644 --- a/spec/integration/federation/shared_receive_relayable.rb +++ b/spec/integration/federation/shared_receive_relayable.rb @@ -21,6 +21,14 @@ shared_examples_for "it deals correctly with a relayable" do expect(received_entity.author.diaspora_handle).to eq(@remote_person2.diaspora_handle) end + it "rejects a downstream entity with a malformed author signature" do + Workers::ReceiveEncryptedSalmon.new.perform( + @user.id, + generate_relayable_remote_parent_wrong_author_key(entity_name) + ) + expect(klass.exists?(guid: @entity.guid)).to be(false) + end + it "declines downstream receive when sender signed with a wrong key" do Workers::ReceiveEncryptedSalmon.new.perform( @user.id,