diff --git a/app/assets/javascripts/app/collections/posts.js b/app/assets/javascripts/app/collections/posts.js index 47c60b682..9805e75e8 100644 --- a/app/assets/javascripts/app/collections/posts.js +++ b/app/assets/javascripts/app/collections/posts.js @@ -2,3 +2,7 @@ app.collections.Posts = Backbone.Collection.extend({ model: app.models.Post, url : "/posts" }); + +app.collection.PublicPosts = app.collection.Posts.extend({ + url : '/public' +}) \ No newline at end of file diff --git a/db/schema.rb b/db/schema.rb index 7cb2c7b83..b71f2408e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -107,8 +107,6 @@ ActiveRecord::Schema.define(:version => 20120521191429) do t.datetime "updated_at", :null => false end - add_index "conversations", ["author_id"], :name => "conversations_author_id_fk" - create_table "invitation_codes", :force => true do |t| t.string "token" t.integer "user_id" @@ -146,7 +144,6 @@ ActiveRecord::Schema.define(:version => 20120521191429) do t.string "target_type", :limit => 60, :null => false end - add_index "likes", ["author_id"], :name => "likes_author_id_fk" add_index "likes", ["guid"], :name => "index_likes_on_guid", :unique => true add_index "likes", ["target_id", "author_id", "target_type"], :name => "index_likes_on_target_id_and_author_id_and_target_type", :unique => true add_index "likes", ["target_id"], :name => "index_likes_on_post_id" @@ -172,7 +169,6 @@ ActiveRecord::Schema.define(:version => 20120521191429) do end add_index "messages", ["author_id"], :name => "index_messages_on_author_id" - add_index "messages", ["conversation_id"], :name => "messages_conversation_id_fk" create_table "notification_actors", :force => true do |t| t.integer "notification_id" @@ -204,7 +200,7 @@ ActiveRecord::Schema.define(:version => 20120521191429) do t.text "data", :null => false end - add_index "o_embed_caches", ["url"], :name => "index_o_embed_caches_on_url", :length => {"url"=>255} + add_index "o_embed_caches", ["url"], :name => "index_o_embed_caches_on_url" create_table "participations", :force => true do |t| t.string "guid" diff --git a/lib/federation_logger.rb b/lib/federation_logger.rb index c9ac91a5f..aa6097d15 100644 --- a/lib/federation_logger.rb +++ b/lib/federation_logger.rb @@ -46,7 +46,7 @@ class Formatter end def random - @random ||= COLORS['FG'].keys[Random.rand(8)] + @random ||= COLORS['FG'].keys[3] end def colors? diff --git a/lib/postzord/receiver.rb b/lib/postzord/receiver.rb index 9d9bbee19..9b0ad6408 100644 --- a/lib/postzord/receiver.rb +++ b/lib/postzord/receiver.rb @@ -10,5 +10,12 @@ class Postzord::Receiver def perform! self.receive! end + + def author_does_not_match_xml_author? + if (@author.diaspora_handle != xml_author) + FEDERATION_LOGGER.info("event=receive status=abort reason='author in xml does not match retrieved person' payload_type=#{@object.class} sender=#{@author.diaspora_handle}") + return true + end + end end diff --git a/lib/postzord/receiver/private.rb b/lib/postzord/receiver/private.rb index be786657a..6215e0476 100644 --- a/lib/postzord/receiver/private.rb +++ b/lib/postzord/receiver/private.rb @@ -62,6 +62,17 @@ class Postzord::Receiver::Private < Postzord::Receiver @salmon ||= Salmon::EncryptedSlap.from_xml(@salmon_xml, @user) end + def validate_object + raise "Contact required unless request" if contact_required_unless_request + raise "Relayable object, but no parent object found" if relayable_without_parent? + + assign_sender_handle_if_request + + raise "Author does not match XML author" if author_does_not_match_xml_author? + + @object + end + def xml_author if @object.respond_to?(:relayable?) #if A and B are friends, and A sends B a comment from C, we delegate the validation to the owner of the post being commented on @@ -73,16 +84,6 @@ class Postzord::Receiver::Private < Postzord::Receiver xml_author end - def validate_object - raise "Contact required unless request" if contact_required_unless_request - raise "Relayable object, but no parent object found" if relayable_without_parent? - - assign_sender_handle_if_request - - raise "Author does not match XML author" if author_does_not_match_xml_author? - - @object - end def set_author! return unless @author @@ -100,13 +101,6 @@ class Postzord::Receiver::Private < Postzord::Receiver end end - def author_does_not_match_xml_author? - if (@author.diaspora_handle != xml_author) - FEDERATION_LOGGER.error("event=receive status=abort reason='author in xml does not match retrieved person' payload_type=#{@object.class} recipient=#{@user_person.diaspora_handle} sender=#{@sender.diaspora_handle}") - return true - end - end - def contact_required_unless_request unless @object.is_a?(Request) || @user.contact_for(@sender) FEDERATION_LOGGER.error("event=receive status=abort reason='sender not connected to recipient' recipient=#{@user_person.diaspora_handle} sender=#{@sender.diaspora_handle}") diff --git a/lib/postzord/receiver/public.rb b/lib/postzord/receiver/public.rb index ac92ca3cd..834f8e2ab 100644 --- a/lib/postzord/receiver/public.rb +++ b/lib/postzord/receiver/public.rb @@ -21,6 +21,8 @@ class Postzord::Receiver::Public < Postzord::Receiver # @return [void] def receive! return false unless verified_signature? + # return false unless account_deletion_is_from_author + return false unless save_object FEDERATION_LOGGER.info("received a #{@object.inspect}") @@ -50,6 +52,7 @@ class Postzord::Receiver::Public < Postzord::Receiver def save_object @object = Diaspora::Parser::from_xml(@salmon.parsed_data) raise "Object is not public" if object_can_be_public_and_it_is_not? + raise "Author does not match XML author" if author_does_not_match_xml_author? @object.save! if @object end @@ -58,8 +61,23 @@ class Postzord::Receiver::Public < Postzord::Receiver User.all_sharing_with_person(@author).select('users.id').map!{ |u| u.id } end + def xml_author + if @object.respond_to?(:relayable?) + #this is public, so it would only be owners sending us other people comments etc + @object.parent.author.local? ? @object.diaspora_handle : @object.parent.diaspora_handle + else + @object.diaspora_handle + end + end + private + def account_deletion_is_from_author + return true unless @object.is_a?(AccountDeletion) + return false if @object.diaspora_handle != @author.diaspora_handle + return true + end + # @return [Boolean] def object_can_be_public_and_it_is_not? @object.respond_to?(:public) && !@object.public? diff --git a/spec/integration/attack_vectors_spec.rb b/spec/integration/attack_vectors_spec.rb index c75129abc..110c1d500 100644 --- a/spec/integration/attack_vectors_spec.rb +++ b/spec/integration/attack_vectors_spec.rb @@ -13,6 +13,14 @@ def receive(post, opts) 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 = Factory(:user) block_return_value = yield user @@ -27,11 +35,14 @@ def temporary_post(user, &block) block_return_value end -def expect_error(partial_message, &block) +def expect_error(partial_message, &block)# DOES NOT REQUIRE ERROR!! begin yield rescue => e e.message.should match partial_message + + ensure + raise "no error occured where expected" unless e.present? end end @@ -120,6 +131,14 @@ describe "attack vectors" do }.should_not change(eve.profile, :first_name) end end + + + it 'public stuff should not be spoofed from another author' do + post = Factory(:status_message, :public => true, :author => eve.person) + expect_error /Author does not match XML author/ do + receive_public(post, :from => alice) + end + end end diff --git a/spec/lib/postzord/receiver/public_spec.rb b/spec/lib/postzord/receiver/public_spec.rb index 5eb84c230..d0a8bc8bd 100644 --- a/spec/lib/postzord/receiver/public_spec.rb +++ b/spec/lib/postzord/receiver/public_spec.rb @@ -16,9 +16,13 @@ describe Postzord::Receiver::Public do context 'round trips works with' do it 'a comment' do - comment = bob.build_comment(:text => 'yo', :post => Factory(:status_message)) + sm = Factory(:status_message, :author => alice.person) + + comment = bob.build_comment(:text => 'yo', :post => sm) comment.save + #bob signs his comment, and then sends it up xml = Salmon::Slap.create_by_user_and_activity(bob, comment.to_diaspora_xml).xml_for(nil) + bob.destroy comment.destroy expect{ receiver = Postzord::Receiver::Public.new(xml)