From b25e21f980b50b2010032568f5f3559585d43d7b Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 5 Sep 2017 03:19:39 +0200 Subject: [PATCH 1/3] Add method to get the top entity to RelatedEntity --- lib/diaspora_federation/entities/related_entity.rb | 8 ++++++++ .../entities/related_entity_spec.rb | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/lib/diaspora_federation/entities/related_entity.rb b/lib/diaspora_federation/entities/related_entity.rb index 1ba37a8..b384748 100644 --- a/lib/diaspora_federation/entities/related_entity.rb +++ b/lib/diaspora_federation/entities/related_entity.rb @@ -24,6 +24,14 @@ module DiasporaFederation # @return [RelatedEntity] parent entity entity :parent, Entities::RelatedEntity, default: nil + # The root entity, this entity is responsible for relaying relayables + # @return [RelatedEntity] absolute parent entity + def root + root = self + root = root.parent until root.parent.nil? + root + end + # Get related entity from the backend or fetch it from remote if not available locally # @return [RelatedEntity] fetched related entity def self.fetch(author, type, guid) diff --git a/spec/lib/diaspora_federation/entities/related_entity_spec.rb b/spec/lib/diaspora_federation/entities/related_entity_spec.rb index 811a32c..902eab3 100644 --- a/spec/lib/diaspora_federation/entities/related_entity_spec.rb +++ b/spec/lib/diaspora_federation/entities/related_entity_spec.rb @@ -5,6 +5,20 @@ module DiasporaFederation it_behaves_like "an Entity subclass" + describe "#root" do + it "returns self if it's already the root" do + entity = Fabricate(:related_entity, parent: nil) + expect(entity.root).to eq(entity) + end + + it "returns the root entity if the current entity has parents" do + root = Fabricate(:related_entity, parent: nil) + parent = Fabricate(:related_entity, parent: root) + entity = Fabricate(:related_entity, parent: parent) + expect(entity.root).to eq(root) + end + end + describe ".fetch" do let(:guid) { Fabricate.sequence(:guid) } let(:entity) { Fabricate(:related_entity) } From 61c00b35968a7ed9c579dc9510047ac7b25c8b06 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 5 Sep 2017 23:54:11 +0200 Subject: [PATCH 2/3] Use top parent when relaying relayables of relayables --- lib/diaspora_federation/entities/relayable.rb | 6 +- .../entities/retraction.rb | 2 +- .../entities/relayable_spec.rb | 62 ++++++++++++++++++- .../entities/retraction_spec.rb | 31 ++++++++-- 4 files changed, 90 insertions(+), 11 deletions(-) diff --git a/lib/diaspora_federation/entities/relayable.rb b/lib/diaspora_federation/entities/relayable.rb index a61a467..6c44ebb 100644 --- a/lib/diaspora_federation/entities/relayable.rb +++ b/lib/diaspora_federation/entities/relayable.rb @@ -76,11 +76,11 @@ module DiasporaFederation # @raise [SignatureVerificationFailed] if the signature is not valid # @raise [PublicKeyNotFound] if no public key is found def verify_signature - super(author, :author_signature) unless author == parent.author + super(author, :author_signature) unless author == parent.root.author end def sender_valid?(sender) - (sender == author && parent.local) || sender == parent.author + (sender == author && parent.root.local) || sender == parent.root.author end # @return [String] string representation of this object @@ -121,7 +121,7 @@ module DiasporaFederation # Sign with parent author key, if the parent author is local (if the private key is found) # @return [String] A Base64 encoded signature of #signature_data with key def sign_with_parent_author_if_available - privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key, parent.author) + privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key, parent.root.author) return unless privkey sign_with_key(privkey).tap do diff --git a/lib/diaspora_federation/entities/retraction.rb b/lib/diaspora_federation/entities/retraction.rb index 67799b8..217c0a6 100644 --- a/lib/diaspora_federation/entities/retraction.rb +++ b/lib/diaspora_federation/entities/retraction.rb @@ -28,7 +28,7 @@ module DiasporaFederation def sender_valid?(sender) case target_type when "Comment", "Like", "PollParticipation" - sender == target.author || sender == target.parent.author + sender == target.author || sender == target.root.author else sender == target.author end diff --git a/spec/lib/diaspora_federation/entities/relayable_spec.rb b/spec/lib/diaspora_federation/entities/relayable_spec.rb index bbac670..c861c55 100644 --- a/spec/lib/diaspora_federation/entities/relayable_spec.rb +++ b/spec/lib/diaspora_federation/entities/relayable_spec.rb @@ -78,6 +78,16 @@ module DiasporaFederation }.not_to raise_error end + it "doesn't raise when no author signature was passed, but the author is also the author of the root entity" do + hash[:author_signature] = nil + root = Fabricate(:related_entity, author: author, local: false) + hash[:parent] = Fabricate(:related_entity, author: Fabricate.sequence(:diaspora_id), local: false, parent: root) + + expect { + Entities::SomeRelayable.new(hash, signature_order).verify_signature + }.not_to raise_error + end + it "raises when bad author signature was passed" do hash[:author_signature] = sign_with_key(author_pkey, "bad signed string") @@ -198,6 +208,22 @@ XML expect(verify_signature(parent_pkey, parent_author_signature, signature_data)).to be_truthy end + it "computes correct signatures for the entity when the parent is a relayable itself" do + intermediate_author = Fabricate.sequence(:diaspora_id) + parent = Fabricate(:related_entity, author: intermediate_author, local: true, parent: local_parent) + expect_callback(:fetch_private_key, author).and_return(author_pkey) + expect_callback(:fetch_private_key, local_parent.author).and_return(parent_pkey) + expect(DiasporaFederation.callbacks).not_to receive(:trigger).with(:fetch_private_key, intermediate_author) + + xml = Entities::SomeRelayable.new(hash.merge(parent: parent)).to_xml + + author_signature = xml.at_xpath("author_signature").text + parent_author_signature = xml.at_xpath("parent_author_signature").text + + expect(verify_signature(author_pkey, author_signature, signature_data)).to be_truthy + expect(verify_signature(parent_pkey, parent_author_signature, signature_data)).to be_truthy + end + it "computes correct signatures for the entity with new unknown xml elements" do expect_callback(:fetch_private_key, author).and_return(author_pkey) expect_callback(:fetch_private_key, local_parent.author).and_return(parent_pkey) @@ -549,9 +575,9 @@ XML end it "allows parent author" do - entity = Entities::SomeRelayable.new(hash) + entity = Entities::SomeRelayable.new(hash.merge(parent: remote_parent)) - expect(entity.sender_valid?(local_parent.author)).to be_truthy + expect(entity.sender_valid?(remote_parent.author)).to be_truthy end it "does not allow any random author" do @@ -560,6 +586,38 @@ XML expect(entity.sender_valid?(invalid_author)).to be_falsey end + + context "multi-layer relayable" do + let(:intermediate_author) { Fabricate.sequence(:diaspora_id) } + + it "allows author if the root entity is local" do + parent = Fabricate(:related_entity, author: intermediate_author, local: false, parent: local_parent) + entity = Entities::SomeRelayable.new(hash.merge(parent: parent)) + + expect(entity.sender_valid?(author)).to be_truthy + end + + it "does not allow the author if the root entity is not local" do + parent = Fabricate(:related_entity, author: intermediate_author, local: true, parent: remote_parent) + entity = Entities::SomeRelayable.new(hash.merge(parent: parent)) + + expect(entity.sender_valid?(author)).to be_falsey + end + + it "allows root entity author" do + parent = Fabricate(:related_entity, author: intermediate_author, local: false, parent: remote_parent) + entity = Entities::SomeRelayable.new(hash.merge(parent: parent)) + + expect(entity.sender_valid?(remote_parent.author)).to be_truthy + end + + it "does not allow an intermediate parent author" do + parent = Fabricate(:related_entity, author: intermediate_author, local: false, parent: remote_parent) + entity = Entities::SomeRelayable.new(hash.merge(parent: parent)) + + expect(entity.sender_valid?(intermediate_author)).to be_falsey + end + end end end end diff --git a/spec/lib/diaspora_federation/entities/retraction_spec.rb b/spec/lib/diaspora_federation/entities/retraction_spec.rb index 5b8328b..c3d1e9d 100644 --- a/spec/lib/diaspora_federation/entities/retraction_spec.rb +++ b/spec/lib/diaspora_federation/entities/retraction_spec.rb @@ -62,27 +62,48 @@ XML ) } let(:relayable_data) { data.merge(target_type: target_type, target: relayable_target) } + let(:entity) { Entities::Retraction.new(relayable_data) } it "allows target author" do - entity = Entities::Retraction.new(relayable_data) - expect(entity.sender_valid?(bob.diaspora_id)).to be_truthy end it "allows target parent author" do - entity = Entities::Retraction.new(relayable_data) - expect(entity.sender_valid?(alice.diaspora_id)).to be_truthy end it "does not allow any random author" do - entity = Entities::Retraction.new(relayable_data) invalid_author = Fabricate.sequence(:diaspora_id) expect(entity.sender_valid?(invalid_author)).to be_falsey end end end + + context "Like of a Comment" do + let(:comment) { + Fabricate( + :related_entity, + author: Fabricate.sequence(:diaspora_id), + parent: Fabricate(:related_entity, author: alice.diaspora_id) + ) + } + let(:relayable_target) { Fabricate(:related_entity, author: bob.diaspora_id, parent: comment) } + let(:relayable_data) { data.merge(target_type: "Like", target: relayable_target) } + let(:entity) { Entities::Retraction.new(relayable_data) } + + it "allows target author" do + expect(entity.sender_valid?(bob.diaspora_id)).to be_truthy + end + + it "allows target root entity author" do + expect(entity.sender_valid?(alice.diaspora_id)).to be_truthy + end + + it "does not allow the comment author" do + expect(entity.sender_valid?(comment.author)).to be_falsey + end + end end end end From 0801d4b2608fbac8c36f06f92601c2fb9568f282 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Wed, 6 Sep 2017 00:18:30 +0200 Subject: [PATCH 3/3] Update documentation to describe the behavior with the root entity --- docs/federation/relayable.md | 38 ++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/docs/federation/relayable.md b/docs/federation/relayable.md index 742b5ae..da62885 100644 --- a/docs/federation/relayable.md +++ b/docs/federation/relayable.md @@ -2,10 +2,14 @@ title: Relayable --- -If a person participates on an entity, it needs to be relayed via the author of the parent entity, because only the -parent author knows, to whom they shared the original entity. +If a person participates on an entity, it needs to be relayed via the author of the root entity, because only the +root author knows, to whom they shared the original entity. -Such entities are: +The root is the most top parent of the relayable: If the parent of a relayable is a relayable itself, +the parent of that should be used, until it isn't a relayable anymore. +For example, for both, a `Like` for a `Post` and a `Like` for a `Comment`, the author of the `Post` should be used. + +Such relayable entities are: * [Comment][comment] * [Like][like] @@ -24,16 +28,16 @@ All relayables have the following properties: ## Relaying -If the author is not the same as the parent author, the author of the relayable sends the entity to the parent author +If the author is not the same as the root author, the author of the relayable sends the entity to the root author and the author must include the `author_signature`. -The parent author then must envelop it in a new [Magic Envelope][magicsig] and send the entity to all the recipients -of the parent entity. If the author and the parent author are on the same server, the author must sign the -`author_signature` and the parent author needs to sign the Magic Envelope. +The root author then must envelop it in a new [Magic Envelope][magicsig] and send the entity to all the recipients +of the root entity. If the author and the root author are on the same server, the author must sign the +`author_signature` and the root author needs to sign the Magic Envelope. -If someone other then the parent author receives a relayable without a valid Magic Envelope signed from -the parent author, it must be ignored. If the author is not the same as the parent author and the `author_signature` -is missing or invalid, it also must be ignored. If the author is the same as the parent author, the `author_signature` +If someone other then the root author receives a relayable without a valid Magic Envelope signed from +the root author, it must be ignored. If the author is not the same as the root author and the `author_signature` +is missing or invalid, it also must be ignored. If the author is the same as the root author, the `author_signature` can be missing, because a valid signature in the Magic Envelope from the author is enough in that case. ## Signatures @@ -41,15 +45,15 @@ can be missing, because a valid signature in the Magic Envelope from the author The string to sign is built with the content of all properties (except the `author_signature` itself), concatenated using `;` as separator in the same order as they appear in the XML. The order in the XML is not specified. -This ensures that relayables even work, if the parent author or another recipient does not know all properties of the +This ensures that relayables even work, if the root author or another recipient does not know all properties of the relayable entity (e.g. older version of diaspora\*). This string is then signed with the private RSA key using the RSA-SHA256 algorithm and base64-encoded. -The parent author must use the same order as the relayable author. Unknown properties must be included again in the XML +The root author must use the same order as the relayable author. Unknown properties must be included again in the XML and the signature. -To support fetching of the relayables, the parent author should save the following information: +To support fetching of the relayables, the root author should save the following information: * order of the received XML * additional (unknown) properties @@ -57,13 +61,13 @@ To support fetching of the relayables, the parent author should save the followi ## Retraction / Reject -The parent author is allowed to retract the entity, so there are no additional signatures required for the +The root author is allowed to retract the entity, so there are no additional signatures required for the [Retraction][retraction] (only the [Salmon Magic Signature][magicsig]). -If the author retracts the entity, they send a [Retraction][retraction] to the parent author. The parent author also -must relay this retraction to all recipients of the parent entity. +If the author retracts the entity, they send a [Retraction][retraction] to the root author. The root author also +must relay this retraction to all recipients of the root entity. -If the parent author wants to reject the entity (e.g. if they ignore the author of the relayable), they can simply send +If the root author wants to reject the entity (e.g. if they ignore the author of the relayable), they can simply send a [Retraction][retraction] for it back to the author.