From 06445901f8539184f43622992c61590c3527242c Mon Sep 17 00:00:00 2001 From: maxwell Date: Mon, 1 Nov 2010 16:19:14 -0700 Subject: [PATCH 1/2] IZ MS retrations for posts now green --- app/models/retraction.rb | 20 ++++++-------- config/deploy_config.yml | 2 +- lib/diaspora/user/receiving.rb | 14 +++++----- spec/models/user/attack_vectors_spec.rb | 36 ++++++++++++++++++++++--- 4 files changed, 48 insertions(+), 24 deletions(-) diff --git a/app/models/retraction.rb b/app/models/retraction.rb index 10b055a38..9f1db49e0 100644 --- a/app/models/retraction.rb +++ b/app/models/retraction.rb @@ -7,12 +7,13 @@ class Retraction include Diaspora::Webhooks xml_accessor :post_id - xml_accessor :person_id + xml_accessor :diaspora_handle xml_accessor :type attr_accessor :post_id - attr_accessor :person_id + attr_accessor :diaspora_handle attr_accessor :type + attr_accessor :person def self.for(object) retraction = self.new @@ -23,12 +24,16 @@ class Retraction retraction.post_id = object.id retraction.type = object.class.to_s end - retraction.person_id = person_id_from(object) + retraction.diaspora_handle = object.diaspora_handle retraction end def perform receiving_user_id Rails.logger.debug "Performing retraction for #{post_id}" + unless Post.first(:diaspora_handle => person.diaspora_handle, :id => post_id) + raise "#{person.inspect} is trying to retract a post they do not own" + end + begin Rails.logger.debug("Retracting #{self.type} id: #{self.post_id}") target = self.type.constantize.first(:id => self.post_id) @@ -38,13 +43,4 @@ class Retraction Rails.logger.info("Retraction for unknown type recieved.") end end - - def self.person_id_from(object) - object.is_a?(Person) ? object.id : object.person.id - end - - def person - Person.find_by_id(self.person_id) - end - end diff --git a/config/deploy_config.yml b/config/deploy_config.yml index f99ef75d1..06a6747e1 100644 --- a/config/deploy_config.yml +++ b/config/deploy_config.yml @@ -6,7 +6,7 @@ cross_server: deploy_to: '/usr/local/app/diaspora' user: 'root' repo: 'git://github.com/diaspora/diaspora.git' - branch: 'diaspora-handle-request' + branch: 'master' default_env: 'development' servers: tom: diff --git a/lib/diaspora/user/receiving.rb b/lib/diaspora/user/receiving.rb index 586fcc904..5bb5d9bd9 100644 --- a/lib/diaspora/user/receiving.rb +++ b/lib/diaspora/user/receiving.rb @@ -21,13 +21,14 @@ module Diaspora Rails.logger.debug("From: #{object.person.inspect}") if object.person - if object.is_a?(Comment) || object.is_a?(Post)|| object.is_a?(Request) + if object.is_a?(Comment) || object.is_a?(Post)|| object.is_a?(Request) || object.is_a?(Retraction) e = EMWebfinger.new(object.diaspora_handle) e.on_person { |person| if person.class == Person object.person = person + sender_in_xml = sender(object, xml, person) if (salmon_author != sender_in_xml) raise "Malicious Post, #{salmon_author.real_name} with id #{salmon_author.id} is sending a #{object.class} as #{sender_in_xml.real_name} with id #{sender_in_xml.id} " @@ -41,6 +42,8 @@ module Diaspora if object.is_a?(Comment) receive_comment object, xml + elsif object.is_a? Retraction + receive_retraction object, xml else receive_post object, xml end @@ -57,18 +60,15 @@ module Diaspora raise "Not friends with that person" unless self.contact_for(salmon_author) - if object.is_a? Retraction - receive_retraction object, xml - elsif object.is_a? Profile + + if object.is_a? Profile receive_profile object, xml end end end def sender(object, xml, webfingered_person = nil) - if object.is_a? Retraction - sender = object.person - elsif object.is_a? Profile + if object.is_a? Profile sender = Diaspora::Parser.owner_id_from_xml xml else diff --git a/spec/models/user/attack_vectors_spec.rb b/spec/models/user/attack_vectors_spec.rb index 8a6198fab..86dc6c2dd 100644 --- a/spec/models/user/attack_vectors_spec.rb +++ b/spec/models/user/attack_vectors_spec.rb @@ -80,20 +80,48 @@ describe "attack vectors" do user2.profile.first_name.should == first_name end - it 'can send retractions on post you do not own' do - pending + it 'should not receive retractions on post you do not own' do original_message = user2.post :status_message, :message => 'store this!', :to => aspect2.id user.receive_salmon(user2.salmon(original_message).xml_for(user.person)) user.raw_visible_posts.count.should be 1 ret = Retraction.new ret.post_id = original_message.id - ret.person_id = user3.person.id + ret.diaspora_handle = user3.person.diaspora_handle ret.type = original_message.class.to_s - user.receive_salmon(user3.salmon(ret).xml_for(user.person)) + proc{ user.receive_salmon(user3.salmon(ret).xml_for(user.person)) }.should raise_error /is trying to retract a post they do not own/ StatusMessage.count.should be 1 user.reload.raw_visible_posts.count.should be 1 end + + it 'should not receive retractions where the retractor and the salmon author do not match' do + original_message = user2.post :status_message, :message => 'store this!', :to => aspect2.id + user.receive_salmon(user2.salmon(original_message).xml_for(user.person)) + user.raw_visible_posts.count.should be 1 + + ret = Retraction.new + ret.post_id = original_message.id + ret.diaspora_handle = user2.person.diaspora_handle + ret.type = original_message.class.to_s + + proc{ user.receive_salmon(user3.salmon(ret).xml_for(user.person)) }.should raise_error /Malicious Post/ + StatusMessage.count.should be 1 + user.reload.raw_visible_posts.count.should be 1 + end + + it 'it should not allow you to send retractions for other people' do + pending + ret = Retraction.new + ret.post_id = user2.person.id + ret.diaspora_handle = user3.person.diaspora_handle + ret.type = user2.person.class.to_s + + #proc{ + user.receive_salmon(user3.salmon(ret).xml_for(user.person)) + #}.should raise_error /Malicious Post/ + + # user.reload.friends.count.should == 2 + end end end From 3c8b40c427365e7ce4ee410ee69ac79c9afca406 Mon Sep 17 00:00:00 2001 From: maxwell Date: Mon, 1 Nov 2010 17:10:36 -0700 Subject: [PATCH 2/2] IZ MS retractions now use diaspora handle --- app/helpers/sockets_helper.rb | 2 +- lib/diaspora/user/receiving.rb | 3 +++ spec/lib/diaspora/parser_spec.rb | 2 +- spec/models/user/attack_vectors_spec.rb | 20 ++++++++++++++++---- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/app/helpers/sockets_helper.rb b/app/helpers/sockets_helper.rb index 049c3a17f..eba28dd5f 100644 --- a/app/helpers/sockets_helper.rb +++ b/app/helpers/sockets_helper.rb @@ -23,7 +23,7 @@ module SocketsHelper action_hash[:photo_hash] = object.thumb_hash end - if object.person.owner_id == uid + if object.person && object.person.owner_id == uid action_hash[:mine?] = true end diff --git a/lib/diaspora/user/receiving.rb b/lib/diaspora/user/receiving.rb index 5bb5d9bd9..8010b7e96 100644 --- a/lib/diaspora/user/receiving.rb +++ b/lib/diaspora/user/receiving.rb @@ -83,6 +83,9 @@ module Diaspora def receive_retraction retraction, xml if retraction.type == 'Person' + unless retraction.person.id.to_s == retraction.post_id.to_s + raise "#{retraction.diaspora_handle} trying to unfriend #{retraction.post_id} from #{self.id}" + end Rails.logger.info( "the person id is #{retraction.post_id} the friend found is #{visible_person_by_id(retraction.post_id).inspect}") unfriended_by visible_person_by_id(retraction.post_id) else diff --git a/spec/lib/diaspora/parser_spec.rb b/spec/lib/diaspora/parser_spec.rb index a6a306f03..6ad1da099 100644 --- a/spec/lib/diaspora/parser_spec.rb +++ b/spec/lib/diaspora/parser_spec.rb @@ -27,7 +27,7 @@ describe Diaspora::Parser do it 'should accept retractions' do friend_users(user, aspect, user2, aspect2) - message = Factory.create(:status_message, :person => user2.person) + message = user2.post(:status_message, :message => "cats", :to => aspect2.id) retraction = Retraction.for(message) xml = retraction.to_diaspora_xml diff --git a/spec/models/user/attack_vectors_spec.rb b/spec/models/user/attack_vectors_spec.rb index 86dc6c2dd..8ee200dce 100644 --- a/spec/models/user/attack_vectors_spec.rb +++ b/spec/models/user/attack_vectors_spec.rb @@ -111,17 +111,29 @@ describe "attack vectors" do end it 'it should not allow you to send retractions for other people' do - pending ret = Retraction.new ret.post_id = user2.person.id ret.diaspora_handle = user3.person.diaspora_handle ret.type = user2.person.class.to_s - #proc{ + proc{ user.receive_salmon(user3.salmon(ret).xml_for(user.person)) - #}.should raise_error /Malicious Post/ + }.should raise_error /#{user3.diaspora_handle} trying to unfriend #{user2.person.id} from #{user.id}/ - # user.reload.friends.count.should == 2 + user.reload.friends.count.should == 2 + end + + it 'it should not allow you to send retractions with xml and salmon handle mismatch' do + ret = Retraction.new + ret.post_id = user2.person.id + ret.diaspora_handle = user2.person.diaspora_handle + ret.type = user2.person.class.to_s + + proc{ + user.receive_salmon(user3.salmon(ret).xml_for(user.person)) + }.should raise_error /Malicious Post/ + + user.reload.friends.count.should == 2 end end end