From 645c7bd5ad99cb2484cddbd5cd9e09b3c37a7d93 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 17 May 2016 00:02:22 +0200 Subject: [PATCH] rewrite attack vector specs using new federation --- spec/integration/attack_vectors_spec.rb | 265 ------------------ .../federation/attack_vectors_spec.rb | 128 +++++++++ .../federation/federation_helper.rb | 6 + .../receive_federation_messages_spec.rb | 13 +- 4 files changed, 135 insertions(+), 277 deletions(-) delete mode 100644 spec/integration/attack_vectors_spec.rb create mode 100644 spec/integration/federation/attack_vectors_spec.rb diff --git a/spec/integration/attack_vectors_spec.rb b/spec/integration/attack_vectors_spec.rb deleted file mode 100644 index 2dc9dba0f..000000000 --- a/spec/integration/attack_vectors_spec.rb +++ /dev/null @@ -1,265 +0,0 @@ -# Copyright (c) 2010-2011, Diaspora Inc. This file is -# licensed under the Affero General Public License version 3 or later. See -# the COPYRIGHT file. - -require 'spec_helper' - - -def receive_post(post, opts) - sender = opts.fetch(:from) - receiver = opts.fetch(:by) - salmon_xml = sender.salmon(post).xml_for(receiver.person) - zord = Postzord::Receiver::Private.new(receiver, :salmon_xml => salmon_xml) - zord.perform! -end - -def receive_public(post, opts) - sender = opts.fetch(:from) - salmon_xml = Salmon::Slap.create_by_user_and_activity(sender, post.to_diaspora_xml).xml_for(nil) - post.destroy - zord = Postzord::Receiver::Public.new(salmon_xml) - zord.perform! -end - -def temporary_user(&block) - user = FactoryGirl.create(:user) - block_return_value = yield user - user.delete - block_return_value -end - -def temporary_post(user, &block) - temp_post = user.post(:status_message, :text => 'hi') - block_return_value = yield temp_post - temp_post.delete - block_return_value -end - -def expect_error(partial_message, &block)# DOES NOT REQUIRE ERROR!! - begin - yield - rescue => e - expect(e.message).to match partial_message - - ensure - raise "no error occured where expected" unless e.present? - end -end - -def bogus_retraction(&block) - ret = Retraction.new - yield ret - ret -end - -def user_should_not_see_guid(user, guid) - expect(user.reload.visible_shareables(Post).where(:guid => guid)).to be_blank -end - #returns the message -def legit_post_from_user1_to_user2(user1, user2) - original_message = user1.post(:status_message, :text => 'store this!', :to => user1.aspects.find_by_name("generic").id) - receive_post(original_message, :from => user1, :by => user2) - original_message -end - -describe "attack vectors", :type => :request do - before do - skip # TODO - end - - let(:eves_aspect) { eve.aspects.find_by_name("generic") } - let(:alices_aspect) { alice.aspects.find_by_name("generic") } - - context "testing side effects of validation phase" do - - describe 'Contact Required Unless Request' do - #CUSTOM SETUP; cant use helpers here - it 'does not save a post from a non-contact as a side effect' do - salmon_xml = nil - bad_post_guid = nil - - temporary_user do |bad_user| - temporary_post(bad_user) do |post_from_non_contact| - bad_post_guid = post_from_non_contact.guid - salmon_xml = bad_user.salmon(post_from_non_contact).xml_for(bob.person) - end - end - - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - - expect { - expect { - zord.perform! - }.to raise_error Diaspora::ContactRequiredUnlessRequest - }.to_not change(Post, :count) - - user_should_not_see_guid(bob, bad_post_guid) - end - - - #CUSTOM SETUP; cant use helpers here - it 'other users can not grant visiblity to another users posts by sending their friends post to themselves (even if they are contacts)' do - #setup: eve has a message. then, alice is connected to eve. - #(meaning alice can not see the old post, but it exists in the DB) - # bob takes eves message, changes the post author to himself - # bob trys to send a message to alice - original_message = eve.post(:status_message, :text => 'store this!', :to => eves_aspect.id) - original_message.diaspora_handle = bob.diaspora_handle - - alice.contacts.create(:person => eve.person, :aspects => [alice.aspects.first]) - - salmon_xml = bob.salmon(original_message).xml_for(alice.person) - - #bob sends it to himself????? - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - - expect { - zord.perform! - }.to raise_error Diaspora::ContactRequiredUnlessRequest - - #alice still should not see eves original post, even though bob sent it to her - user_should_not_see_guid(alice, original_message.guid) - end - end - - describe 'author does not match xml author' do - it 'should not overwrite another persons profile profile' do - profile = eve.profile.clone - profile.first_name = "Not BOB" - - expect { - expect { - receive_post(profile, :from => alice, :by => bob) - }.to raise_error Diaspora::AuthorXMLAuthorMismatch - }.to_not change(eve.profile, :first_name) - end - end - - - it 'public stuff should not be spoofed from another author' do - post = FactoryGirl.build(:status_message, :public => true, :author => eve.person) - expect { - receive_public(post, :from => alice) - }.to raise_error Diaspora::AuthorXMLAuthorMismatch - end - end - - - - context 'malicious contact attack vector' do - describe 'mass assignment on id' do - it "does not save a message over an old message with a different author" do - #setup: A user has a message with a given guid and author - original_message = legit_post_from_user1_to_user2(eve, bob) - - #someone else tries to make a message with the same guid - malicious_message = FactoryGirl.build(:status_message, :id => original_message.id, :guid => original_message.guid, :author => alice.person) - - expect{ - receive_post(malicious_message, :from => alice, :by => bob) - }.to_not change(original_message, :author_id) - end - - it 'does not save a message over an old message with the same author' do - #setup: - # i have a legit message from eve - original_message = legit_post_from_user1_to_user2(eve, bob) - - #eve tries to send me another message with the same ID - malicious_message = FactoryGirl.build( :status_message, :id => original_message.id, :text => 'BAD!!!', :author => eve.person) - - expect { - receive_post(malicious_message, :from => eve, :by => bob) - }.to_not change(original_message, :text) - end - end - - - it "ignores retractions on a post not owned by the retraction's sender" do - original_message = legit_post_from_user1_to_user2(eve, bob) - - ret = bogus_retraction do |retraction| - retraction.post_guid = original_message.guid - retraction.diaspora_handle = alice.person.diaspora_handle - retraction.type = original_message.class.to_s - end - - expect { - receive_post(ret, :from => alice, :by => bob) - }.to_not change(StatusMessage, :count) - end - - it "silently disregards retractions for non-existent posts(that are from someone other than the post's author)" do - bogus_retraction = temporary_post(eve) do |original_message| - bogus_retraction do |ret| - ret.post_guid = original_message.guid - ret.diaspora_handle = alice.person.diaspora_handle - ret.type = original_message.class.to_s - end - end - expect{ - receive_post(bogus_retraction, :from => alice, :by => bob) - }.to_not raise_error - end - - it 'should not receive retractions where the retractor and the salmon author do not match' do - original_message = legit_post_from_user1_to_user2(eve, bob) - - retraction = bogus_retraction do |ret| - ret.post_guid = original_message.guid - ret.diaspora_handle = eve.person.diaspora_handle - ret.type = original_message.class.to_s - end - - expect { - expect { - receive_post(retraction, :from => alice, :by => bob) - }.to raise_error Diaspora::AuthorXMLAuthorMismatch - }.to_not change { bob.visible_shareables(Post).count(:all) } - - end - - it 'it should not allow you to send retractions for other people' do - #we are banking on bob being friends with alice and eve - #here, alice is trying to disconnect bob and eve - - retraction = bogus_retraction do |ret| - ret.post_guid = eve.person.guid - ret.diaspora_handle = alice.person.diaspora_handle - ret.type = eve.person.class.to_s - end - - expect{ - receive_post(retraction, :from => alice, :by => bob) - }.to_not change{bob.reload.contacts.count} - end - - it 'it should not allow you to send retractions with xml and salmon handle mismatch' do - retraction = bogus_retraction do |ret| - ret.post_guid = eve.person.guid - ret.diaspora_handle = eve.person.diaspora_handle - ret.type = eve.person.class.to_s - end - - expect{ - expect { - receive_post(retraction, :from => alice, :by => bob) - }.to raise_error Diaspora::AuthorXMLAuthorMismatch - }.to_not change(bob.contacts, :count) - end - - it 'does not let another user update other persons post' do - original_message = eve.post(:photo, :user_file => uploaded_photo, :text => "store this!", :to => eves_aspect.id) - receive_post(original_message, :from => eve, :by => bob) - - #is this testing two things? - new_message = original_message.dup - new_message.diaspora_handle = alice.diaspora_handle - new_message.text = "bad bad bad" - - expect{ - receive_post(new_message, :from => alice, :by => bob) - }.to_not change(original_message, :text) - end - end -end diff --git a/spec/integration/federation/attack_vectors_spec.rb b/spec/integration/federation/attack_vectors_spec.rb new file mode 100644 index 000000000..033a90990 --- /dev/null +++ b/spec/integration/federation/attack_vectors_spec.rb @@ -0,0 +1,128 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +require "spec_helper" +require "integration/federation/federation_helper" + +describe "attack vectors", type: :request do + before do + allow_callbacks(%i(queue_public_receive queue_private_receive receive_entity fetch_related_entity fetch_public_key)) + end + + let(:eves_aspect) { eve.aspects.find_by_name("generic") } + let(:alices_aspect) { alice.aspects.find_by_name("generic") } + + it "other users can not grant visiblity to another users posts by sending their friends post to themselves" do + # setup: eve has a message. then, alice is connected to eve. + # (meaning alice can not see the old post, but it exists in the DB) + # bob takes eves message, changes the post author to himself + # bob trys to send a message to alice + + original_message = eve.post(:status_message, text: "store this!", to: eves_aspect.id) + original_message.diaspora_handle = bob.diaspora_handle + + alice.share_with(eve.person, alices_aspect) + + post_message(generate_xml(Diaspora::Federation::Entities.post(original_message), bob, alice), alice) + + # alice still should not see eves original post, even though bob sent it to her + expect(alice.reload.visible_shareables(Post).where(guid: original_message.guid)).to be_blank + end + + context "author does not match xml author" do + it "should not overwrite another persons profile" do + profile = eve.profile.clone + profile.first_name = "Not BOB" + + post_message(generate_xml(Diaspora::Federation::Entities.profile(profile), alice, bob), bob) + + expect(eve.profile(true).first_name).not_to eq("Not BOB") + end + + it "public post should not be spoofed from another author" do + post = FactoryGirl.build(:status_message, public: true, author: eve.person) + + post_message(generate_xml(Diaspora::Federation::Entities.post(post), alice)) + + expect(StatusMessage.exists?(guid: post.guid)).to be_falsey + end + + it "should not receive retractions where the retractor and the salmon author do not match" do + original_message = eve.post(:status_message, text: "store this!", to: eves_aspect.id) + + expect { + post_message(generate_xml(Diaspora::Federation::Entities.retraction(original_message), alice, bob), bob) + }.to_not change { bob.visible_shareables(Post).count(:all) } + end + + it "should not receive contact retractions from another person" do + # we are banking on bob being friends with alice and eve + # here, alice is trying to disconnect bob and eve + expect(bob.contacts(true).find_by(person_id: eve.person.id)).to be_sharing + + post_message(generate_xml(Diaspora::Federation::Entities.retraction(eve.person), alice, bob), bob) + + expect(bob.contacts(true).find_by(person_id: eve.person.id)).to be_sharing + end + end + + it "does not save a message over an old message with a different author" do + # setup: A user has a message with a given guid and author + original_message = eve.post(:status_message, text: "store this!", to: eves_aspect.id) + + # someone else tries to make a message with the same guid + malicious_message = FactoryGirl.build( + :status_message, + id: original_message.id, + guid: original_message.guid, + author: alice.person + ) + + post_message(generate_xml(Diaspora::Federation::Entities.post(malicious_message), alice, bob), bob) + + expect(original_message.reload.author_id).to eq(eve.person.id) + end + + it "does not save a message over an old message with the same author" do + # setup: + # I have a legit message from eve + original_message = eve.post(:status_message, text: "store this!", to: eves_aspect.id) + + # eve tries to send me another message with the same ID + malicious_message = FactoryGirl.build(:status_message, id: original_message.id, text: "BAD!!!", author: eve.person) + + post_message(generate_xml(Diaspora::Federation::Entities.post(malicious_message), eve, bob), bob) + + expect(original_message.reload.text).to eq("store this!") + end + + it "ignores retractions on a post not owned by the retraction's sender" do + original_message = eve.post(:status_message, text: "store this!", to: eves_aspect.id) + + retraction = DiasporaFederation::Entities::Retraction.new( + target_guid: original_message.guid, + target_type: original_message.class.to_s, + target: Diaspora::Federation::Entities.related_entity(original_message), + author: alice.person.diaspora_handle + ) + + expect { + post_message(generate_xml(retraction, alice, bob), bob) + }.to_not change(StatusMessage, :count) + end + + it "does not let another user update other persons post" do + original_message = eve.post(:photo, user_file: uploaded_photo, text: "store this!", to: eves_aspect.id) + + new_message = original_message.dup + new_message.diaspora_handle = alice.diaspora_handle + new_message.text = "bad bad bad" + new_message.height = 23 + new_message.width = 42 + + post_message(generate_xml(Diaspora::Federation::Entities.photo(new_message), alice, bob), bob) + + expect(original_message.reload.text).to eq("store this!") + end +end diff --git a/spec/integration/federation/federation_helper.rb b/spec/integration/federation/federation_helper.rb index fabe94c0e..9e5377707 100644 --- a/spec/integration/federation/federation_helper.rb +++ b/spec/integration/federation/federation_helper.rb @@ -24,6 +24,12 @@ def create_remote_user(pod) end end +def allow_callbacks(callbacks) + callbacks.each do |callback| + allow(DiasporaFederation.callbacks).to receive(:trigger).with(callback, any_args).and_call_original + end +end + def create_relayable_entity(entity_name, parent, diaspora_id) expect(DiasporaFederation.callbacks).to receive(:trigger).with( :fetch_private_key, alice.diaspora_handle diff --git a/spec/integration/federation/receive_federation_messages_spec.rb b/spec/integration/federation/receive_federation_messages_spec.rb index 4ee743d5c..7144ffdfa 100644 --- a/spec/integration/federation/receive_federation_messages_spec.rb +++ b/spec/integration/federation/receive_federation_messages_spec.rb @@ -6,18 +6,7 @@ require "integration/federation/shared_receive_stream_items" describe "Receive federation messages feature" do before do - allow(DiasporaFederation.callbacks).to receive(:trigger).with( - :queue_public_receive, any_args - ).and_call_original - allow(DiasporaFederation.callbacks).to receive(:trigger).with( - :queue_private_receive, any_args - ).and_call_original - allow(DiasporaFederation.callbacks).to receive(:trigger).with( - :receive_entity, any_args - ).and_call_original - allow(DiasporaFederation.callbacks).to receive(:trigger).with( - :fetch_related_entity, any_args - ).and_call_original + allow_callbacks(%i(queue_public_receive queue_private_receive receive_entity fetch_related_entity)) end let(:sender) { remote_user_on_pod_b }