From 010fcb73d7e308c15b3f2c10fa27bd49d25d56cf Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 17 Nov 2018 17:42:47 +0000 Subject: [PATCH 1/6] test: httpoison mock: add second spoofing activity test --- .../https___info.pleroma.site_actor.json | 17 +++++++++++++++++ .../https__info.pleroma.site_activity2.json | 14 ++++++++++++++ test/support/httpoison_mock.ex | 16 ++++++++++++++++ 3 files changed, 47 insertions(+) create mode 100644 test/fixtures/httpoison_mock/https___info.pleroma.site_actor.json create mode 100644 test/fixtures/httpoison_mock/https__info.pleroma.site_activity2.json diff --git a/test/fixtures/httpoison_mock/https___info.pleroma.site_actor.json b/test/fixtures/httpoison_mock/https___info.pleroma.site_actor.json new file mode 100644 index 000000000..9dabf0e52 --- /dev/null +++ b/test/fixtures/httpoison_mock/https___info.pleroma.site_actor.json @@ -0,0 +1,17 @@ +{ + "@context": "https://www.w3.org/ns/activitystreams", + "id": "https://info.pleroma.site/actor.json", + "type": "Person", + "following": "https://info.pleroma.site/following.json", + "followers": "https://info.pleroma.site/followers.json", + "inbox": "https://info.pleroma.site/inbox.json", + "outbox": "https://info.pleroma.site/outbox.json", + "preferredUsername": "admin", + "name": null, + "summary": "

", + "publicKey": { + "id": "https://info.pleroma.site/actor.json#main-key", + "owner": "https://info.pleroma.site/actor.json", + "publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtc4Tir+3ADhSNF6VKrtW\nOU32T01w7V0yshmQei38YyiVwVvFu8XOP6ACchkdxbJ+C9mZud8qWaRJKVbFTMUG\nNX4+6Q+FobyuKrwN7CEwhDALZtaN2IPbaPd6uG1B7QhWorrY+yFa8f2TBM3BxnUy\nI4T+bMIZIEYG7KtljCBoQXuTQmGtuffO0UwJksidg2ffCF5Q+K//JfQagJ3UzrR+\nZXbKMJdAw4bCVJYs4Z5EhHYBwQWiXCyMGTd7BGlmMkY6Av7ZqHKC/owp3/0EWDNz\nNqF09Wcpr3y3e8nA10X40MJqp/wR+1xtxp+YGbq/Cj5hZGBG7etFOmIpVBrDOhry\nBwIDAQAB\n-----END PUBLIC KEY-----\n" + } +} diff --git a/test/fixtures/httpoison_mock/https__info.pleroma.site_activity2.json b/test/fixtures/httpoison_mock/https__info.pleroma.site_activity2.json new file mode 100644 index 000000000..8ce35958d --- /dev/null +++ b/test/fixtures/httpoison_mock/https__info.pleroma.site_activity2.json @@ -0,0 +1,14 @@ +{ + "@context": "https://www.w3.org/ns/activitystreams", + "attributedTo": "https://info.pleroma.site/actor.json", + "attachment": [], + "actor": "https://mastodon.example.org/users/admin", + "content": "

this post was not actually written by Haelwenn

", + "id": "https://info.pleroma.site/activity2.json", + "published": "2018-09-01T22:15:00Z", + "tag": [], + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "type": "Note" +} diff --git a/test/support/httpoison_mock.ex b/test/support/httpoison_mock.ex index ab964334d..2ea10910e 100644 --- a/test/support/httpoison_mock.ex +++ b/test/support/httpoison_mock.ex @@ -40,6 +40,22 @@ def get("https://info.pleroma.site/activity.json", _, _) do }} end + def get("https://info.pleroma.site/activity2.json", _, _) do + {:ok, + %Response{ + status_code: 200, + body: File.read!("test/fixtures/httpoison_mock/https__info.pleroma.site_activity2.json") + }} + end + + def get("https://info.pleroma.site/actor.json", _, _) do + {:ok, + %Response{ + status_code: 200, + body: File.read!("test/fixtures/httpoison_mock/https___info.pleroma.site_actor.json") + }} + end + def get("https://puckipedia.com/", [Accept: "application/activity+json"], _) do {:ok, %Response{ From 2ab8e287289d3b4d6458f6d9cc29c5d29dfcd102 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 17 Nov 2018 18:11:31 +0000 Subject: [PATCH 2/6] transmogrifier tests: fix defective spoofing test --- test/web/activity_pub/transmogrifier_test.exs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 0278ef5d1..9250598f4 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -872,12 +872,10 @@ test "it rejects objects with a bogus origin" do end test "it rejects activities which reference objects with bogus origins" do - user = insert(:user, %{local: false}) - data = %{ "@context" => "https://www.w3.org/ns/activitystreams", - "id" => user.ap_id <> "/activities/1234", - "actor" => user.ap_id, + "id" => "https://mastodon.example.org/users/admin/activities/1234", + "actor" => "https://mastodon.example.org/users/admin", "to" => ["https://www.w3.org/ns/activitystreams#Public"], "object" => "https://info.pleroma.site/activity.json", "type" => "Announce" From d9cb081f0723881343b6dd71e1bb1b52b5492f2b Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 17 Nov 2018 18:12:11 +0000 Subject: [PATCH 3/6] tests: add additional spoofing tests --- test/web/activity_pub/transmogrifier_test.exs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 9250598f4..0ba969263 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -883,5 +883,22 @@ test "it rejects activities which reference objects with bogus origins" do :error = Transmogrifier.handle_incoming(data) end + + test "it rejects objects when the ID does not match the fetched URI" do + {:error, _} = ActivityPub.fetch_object_from_id("https://info.pleroma.site/activity2.json") + end + + test "it rejects activities which reference objects by mismatched URI" do + data = %{ + "@context" => "https://www.w3.org/ns/activitystreams", + "id" => "http://mastodon.example.org/users/admin/activities/1234", + "actor" => "http://mastodon.example.org/users/admin", + "to" => ["https://www.w3.org/ns/activitystreams#Public"], + "object" => "https://info.pleroma.site/activity2.json", + "type" => "Announce" + } + + :error = Transmogrifier.handle_incoming(data) + end end end From 9c8adfb6efb0adf0638f91533ff1bc9f9df36668 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 17 Nov 2018 18:16:55 +0000 Subject: [PATCH 4/6] test: fix more test defects --- .../httpoison_mock/https__info.pleroma.site_activity.json | 4 ++-- .../httpoison_mock/https__info.pleroma.site_activity2.json | 2 +- test/web/activity_pub/transmogrifier_test.exs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/fixtures/httpoison_mock/https__info.pleroma.site_activity.json b/test/fixtures/httpoison_mock/https__info.pleroma.site_activity.json index eab0341fe..a0dc4c830 100644 --- a/test/fixtures/httpoison_mock/https__info.pleroma.site_activity.json +++ b/test/fixtures/httpoison_mock/https__info.pleroma.site_activity.json @@ -1,8 +1,8 @@ { "@context": "https://www.w3.org/ns/activitystreams", - "actor": "https://mastodon.example.org/users/admin", + "actor": "http://mastodon.example.org/users/admin", "attachment": [], - "attributedTo": "https://mastodon.example.org/users/admin", + "attributedTo": "http://mastodon.example.org/users/admin", "content": "

this post was not actually written by Haelwenn

", "id": "https://info.pleroma.site/activity.json", "published": "2018-09-01T22:15:00Z", diff --git a/test/fixtures/httpoison_mock/https__info.pleroma.site_activity2.json b/test/fixtures/httpoison_mock/https__info.pleroma.site_activity2.json index 8ce35958d..b16a9279b 100644 --- a/test/fixtures/httpoison_mock/https__info.pleroma.site_activity2.json +++ b/test/fixtures/httpoison_mock/https__info.pleroma.site_activity2.json @@ -2,7 +2,7 @@ "@context": "https://www.w3.org/ns/activitystreams", "attributedTo": "https://info.pleroma.site/actor.json", "attachment": [], - "actor": "https://mastodon.example.org/users/admin", + "actor": "http://mastodon.example.org/users/admin", "content": "

this post was not actually written by Haelwenn

", "id": "https://info.pleroma.site/activity2.json", "published": "2018-09-01T22:15:00Z", diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 0ba969263..e5308d125 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -874,8 +874,8 @@ test "it rejects objects with a bogus origin" do test "it rejects activities which reference objects with bogus origins" do data = %{ "@context" => "https://www.w3.org/ns/activitystreams", - "id" => "https://mastodon.example.org/users/admin/activities/1234", - "actor" => "https://mastodon.example.org/users/admin", + "id" => "http://mastodon.example.org/users/admin/activities/1234", + "actor" => "http://mastodon.example.org/users/admin", "to" => ["https://www.w3.org/ns/activitystreams#Public"], "object" => "https://info.pleroma.site/activity.json", "type" => "Announce" From 603fccf175bd6f0d80cc52d0766b0208d2309790 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 17 Nov 2018 18:17:17 +0000 Subject: [PATCH 5/6] activitypub: fetch_object_from_id(): prefer `actor` over `attributedTo` to avoid spoofing --- lib/pleroma/web/activity_pub/activity_pub.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index c6733e487..51b787272 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -747,7 +747,7 @@ def fetch_object_from_id(id) do "type" => "Create", "to" => data["to"], "cc" => data["cc"], - "actor" => data["attributedTo"], + "actor" => data["actor"] || data["attributedTo"], "object" => data }, :ok <- Transmogrifier.contain_origin(id, params), From b483ae0a724a2b76e8c61f63a96d7867339dacb1 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 17 Nov 2018 18:24:58 +0000 Subject: [PATCH 6/6] tests: add a second spoofing variant --- .../https__info.pleroma.site_activity3.json | 13 ++++++++++++ test/support/httpoison_mock.ex | 8 +++++++ test/web/activity_pub/transmogrifier_test.exs | 21 +++++++++++++++++-- 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/httpoison_mock/https__info.pleroma.site_activity3.json diff --git a/test/fixtures/httpoison_mock/https__info.pleroma.site_activity3.json b/test/fixtures/httpoison_mock/https__info.pleroma.site_activity3.json new file mode 100644 index 000000000..1df73f2c5 --- /dev/null +++ b/test/fixtures/httpoison_mock/https__info.pleroma.site_activity3.json @@ -0,0 +1,13 @@ +{ + "@context": "https://www.w3.org/ns/activitystreams", + "attributedTo": "http://mastodon.example.org/users/admin", + "attachment": [], + "content": "

this post was not actually written by Haelwenn

", + "id": "https://info.pleroma.site/activity2.json", + "published": "2018-09-01T22:15:00Z", + "tag": [], + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "type": "Note" +} diff --git a/test/support/httpoison_mock.ex b/test/support/httpoison_mock.ex index 2ea10910e..ebd1e9c4d 100644 --- a/test/support/httpoison_mock.ex +++ b/test/support/httpoison_mock.ex @@ -48,6 +48,14 @@ def get("https://info.pleroma.site/activity2.json", _, _) do }} end + def get("https://info.pleroma.site/activity3.json", _, _) do + {:ok, + %Response{ + status_code: 200, + body: File.read!("test/fixtures/httpoison_mock/https__info.pleroma.site_activity3.json") + }} + end + def get("https://info.pleroma.site/actor.json", _, _) do {:ok, %Response{ diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index e5308d125..6320b5b6e 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -884,11 +884,11 @@ test "it rejects activities which reference objects with bogus origins" do :error = Transmogrifier.handle_incoming(data) end - test "it rejects objects when the ID does not match the fetched URI" do + test "it rejects objects when attributedTo is wrong (variant 1)" do {:error, _} = ActivityPub.fetch_object_from_id("https://info.pleroma.site/activity2.json") end - test "it rejects activities which reference objects by mismatched URI" do + test "it rejects activities which reference objects that have an incorrect attribution (variant 1)" do data = %{ "@context" => "https://www.w3.org/ns/activitystreams", "id" => "http://mastodon.example.org/users/admin/activities/1234", @@ -900,5 +900,22 @@ test "it rejects activities which reference objects by mismatched URI" do :error = Transmogrifier.handle_incoming(data) end + + test "it rejects objects when attributedTo is wrong (variant 2)" do + {:error, _} = ActivityPub.fetch_object_from_id("https://info.pleroma.site/activity3.json") + end + + test "it rejects activities which reference objects that have an incorrect attribution (variant 2)" do + data = %{ + "@context" => "https://www.w3.org/ns/activitystreams", + "id" => "http://mastodon.example.org/users/admin/activities/1234", + "actor" => "http://mastodon.example.org/users/admin", + "to" => ["https://www.w3.org/ns/activitystreams#Public"], + "object" => "https://info.pleroma.site/activity3.json", + "type" => "Announce" + } + + :error = Transmogrifier.handle_incoming(data) + end end end