From a12b6454bb0a270732f9b55f8d4366c9add44136 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 16 Dec 2019 22:24:03 +0700 Subject: [PATCH 1/5] Add an option to require fetches to be signed --- CHANGELOG.md | 1 + config/config.exs | 3 +- docs/configuration/cheatsheet.md | 9 ++-- lib/pleroma/plugs/http_signature.ex | 43 ++++++++++++------ test/plugs/http_signature_plug_test.exs | 58 +++++++++++++++++++++++++ 5 files changed, 95 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c133cd9ec..ee9e1091c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - User notification settings: Add `privacy_option` option. - User settings: Add _This account is a_ option. - OAuth: admin scopes support (relevant setting: `[:auth, :enforce_oauth_admin_scope_usage]`). +- Add an option `authorized_fetch_mode` to requrie HTTP Signature for AP fetches.
API Changes diff --git a/config/config.exs b/config/config.exs index 370ddd855..541fcc2d4 100644 --- a/config/config.exs +++ b/config/config.exs @@ -343,7 +343,8 @@ config :pleroma, :activitypub, unfollow_blocked: true, outgoing_blocks: true, follow_handshake_timeout: 500, - sign_object_fetches: true + sign_object_fetches: true, + authorized_fetch_mode: false config :pleroma, :streamer, workers: 3, diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index ce2a14210..8fa4a1747 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -147,10 +147,11 @@ config :pleroma, :mrf_user_allowlist, * `:reject` rejects the message entirely ### :activitypub -* ``unfollow_blocked``: Whether blocks result in people getting unfollowed -* ``outgoing_blocks``: Whether to federate blocks to other instances -* ``deny_follow_blocked``: Whether to disallow following an account that has blocked the user in question -* ``sign_object_fetches``: Sign object fetches with HTTP signatures +* `unfollow_blocked`: Whether blocks result in people getting unfollowed +* `outgoing_blocks`: Whether to federate blocks to other instances +* `deny_follow_blocked`: Whether to disallow following an account that has blocked the user in question +* `sign_object_fetches`: Sign object fetches with HTTP signatures +* `authorized_fetch_mode`: Require HTTP Signature for AP fetches ### :fetch_initial_posts * `enabled`: if enabled, when a new user is federated with, fetch some of their latest posts diff --git a/lib/pleroma/plugs/http_signature.ex b/lib/pleroma/plugs/http_signature.ex index 23d22a712..ecd7a55bf 100644 --- a/lib/pleroma/plugs/http_signature.ex +++ b/lib/pleroma/plugs/http_signature.ex @@ -15,25 +15,23 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do end def call(conn, _opts) do - headers = get_req_header(conn, "signature") - signature = Enum.at(headers, 0) + conn + |> maybe_assign_valid_signature() + |> maybe_require_signature() + end - if signature do + defp maybe_assign_valid_signature(conn) do + if has_signature_header?(conn) do # set (request-target) header to the appropriate value # we also replace the digest header with the one we computed - conn = - conn - |> put_req_header( - "(request-target)", - String.downcase("#{conn.method}") <> " #{conn.request_path}" - ) + request_target = String.downcase("#{conn.method}") <> " #{conn.request_path}" conn = - if conn.assigns[:digest] do - conn - |> put_req_header("digest", conn.assigns[:digest]) - else - conn + conn + |> put_req_header("(request-target)", request_target) + |> case do + %{assigns: %{digest: digest}} = conn -> put_req_header(conn, "digest", digest) + conn -> conn end assign(conn, :valid_signature, HTTPSignatures.validate_conn(conn)) @@ -42,4 +40,21 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do conn end end + + defp has_signature_header?(conn) do + conn |> get_req_header("signature") |> Enum.at(0, false) + end + + defp maybe_require_signature(%{assigns: %{valid_signature: true}} = conn), do: conn + + defp maybe_require_signature(conn) do + if Pleroma.Config.get([:activitypub, :authorized_fetch_mode], false) do + conn + |> put_status(:unauthorized) + |> Phoenix.Controller.text("Request not signed") + |> halt() + else + conn + end + end end diff --git a/test/plugs/http_signature_plug_test.exs b/test/plugs/http_signature_plug_test.exs index d8ace36da..007193dd9 100644 --- a/test/plugs/http_signature_plug_test.exs +++ b/test/plugs/http_signature_plug_test.exs @@ -23,7 +23,65 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do |> HTTPSignaturePlug.call(%{}) assert conn.assigns.valid_signature == true + assert conn.halted == false assert called(HTTPSignatures.validate_conn(:_)) end end + + describe "requries a signature when `authorized_fetch_mode` is enabled" do + setup do + Pleroma.Config.put([:activitypub, :authorized_fetch_mode], true) + + on_exit(fn -> + Pleroma.Config.put([:activitypub, :authorized_fetch_mode], false) + end) + + params = %{"actor" => "http://mastodon.example.org/users/admin"} + conn = build_conn(:get, "/doesntmattter", params) + + [conn: conn] + end + + test "when signature header is present", %{conn: conn} do + with_mock HTTPSignatures, validate_conn: fn _ -> false end do + conn = + conn + |> put_req_header( + "signature", + "keyId=\"http://mastodon.example.org/users/admin#main-key" + ) + |> HTTPSignaturePlug.call(%{}) + + assert conn.assigns.valid_signature == false + assert conn.halted == true + assert conn.status == 401 + assert conn.state == :sent + assert conn.resp_body == "Request not signed" + assert called(HTTPSignatures.validate_conn(:_)) + end + + with_mock HTTPSignatures, validate_conn: fn _ -> true end do + conn = + conn + |> put_req_header( + "signature", + "keyId=\"http://mastodon.example.org/users/admin#main-key" + ) + |> HTTPSignaturePlug.call(%{}) + + assert conn.assigns.valid_signature == true + assert conn.halted == false + assert called(HTTPSignatures.validate_conn(:_)) + end + end + + test "halts the connection when `signature` header is not present", %{conn: conn} do + conn = HTTPSignaturePlug.call(conn, %{}) + assert conn.assigns[:valid_signature] == nil + assert conn.halted == true + assert conn.status == 401 + assert conn.state == :sent + assert conn.resp_body == "Request not signed" + end + end end From e1fa8c11a9ea26f54a231cbdacdc8befe634b57e Mon Sep 17 00:00:00 2001 From: minibikini Date: Mon, 16 Dec 2019 18:39:59 +0000 Subject: [PATCH 2/5] Apply suggestion to test/plugs/http_signature_plug_test.exs --- test/plugs/http_signature_plug_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugs/http_signature_plug_test.exs b/test/plugs/http_signature_plug_test.exs index 007193dd9..77e790288 100644 --- a/test/plugs/http_signature_plug_test.exs +++ b/test/plugs/http_signature_plug_test.exs @@ -28,7 +28,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do end end - describe "requries a signature when `authorized_fetch_mode` is enabled" do + describe "requires a signature when `authorized_fetch_mode` is enabled" do setup do Pleroma.Config.put([:activitypub, :authorized_fetch_mode], true) From d81c3afbb2f020226909afed851d2aa623fdcdb0 Mon Sep 17 00:00:00 2001 From: minibikini Date: Mon, 16 Dec 2019 18:40:12 +0000 Subject: [PATCH 3/5] Apply suggestion to CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee9e1091c..3c2b49962 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - User notification settings: Add `privacy_option` option. - User settings: Add _This account is a_ option. - OAuth: admin scopes support (relevant setting: `[:auth, :enforce_oauth_admin_scope_usage]`). -- Add an option `authorized_fetch_mode` to requrie HTTP Signature for AP fetches. +- Add an option `authorized_fetch_mode` to require HTTP Signatures for AP fetches.
API Changes From 36d66d965519037d086ad5080ccf833801c3381e Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Tue, 17 Dec 2019 02:06:58 +0700 Subject: [PATCH 4/5] Fix typo --- CHANGELOG.md | 2 +- docs/configuration/cheatsheet.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c2b49962..93c7485aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - User notification settings: Add `privacy_option` option. - User settings: Add _This account is a_ option. - OAuth: admin scopes support (relevant setting: `[:auth, :enforce_oauth_admin_scope_usage]`). -- Add an option `authorized_fetch_mode` to require HTTP Signatures for AP fetches. +- Add an option `authorized_fetch_mode` to require HTTP signatures for AP fetches.
API Changes diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index 8fa4a1747..cdc7c5ee0 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -151,7 +151,7 @@ config :pleroma, :mrf_user_allowlist, * `outgoing_blocks`: Whether to federate blocks to other instances * `deny_follow_blocked`: Whether to disallow following an account that has blocked the user in question * `sign_object_fetches`: Sign object fetches with HTTP signatures -* `authorized_fetch_mode`: Require HTTP Signature for AP fetches +* `authorized_fetch_mode`: Require HTTP signatures for AP fetches ### :fetch_initial_posts * `enabled`: if enabled, when a new user is federated with, fetch some of their latest posts From 775212121cc3eb108bca6c4b94a3fdf6d8d8fcd1 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Thu, 19 Dec 2019 20:17:18 +0700 Subject: [PATCH 5/5] Verify HTTP signatures only when request accepts "activity+json" type --- lib/pleroma/plugs/http_signature.ex | 13 +++++++++---- test/plugs/http_signature_plug_test.exs | 4 +++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/plugs/http_signature.ex b/lib/pleroma/plugs/http_signature.ex index ecd7a55bf..477a5b578 100644 --- a/lib/pleroma/plugs/http_signature.ex +++ b/lib/pleroma/plugs/http_signature.ex @@ -4,6 +4,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do import Plug.Conn + import Phoenix.Controller, only: [get_format: 1, text: 2] require Logger def init(options) do @@ -15,9 +16,13 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do end def call(conn, _opts) do - conn - |> maybe_assign_valid_signature() - |> maybe_require_signature() + if get_format(conn) == "activity+json" do + conn + |> maybe_assign_valid_signature() + |> maybe_require_signature() + else + conn + end end defp maybe_assign_valid_signature(conn) do @@ -51,7 +56,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do if Pleroma.Config.get([:activitypub, :authorized_fetch_mode], false) do conn |> put_status(:unauthorized) - |> Phoenix.Controller.text("Request not signed") + |> text("Request not signed") |> halt() else conn diff --git a/test/plugs/http_signature_plug_test.exs b/test/plugs/http_signature_plug_test.exs index 77e790288..55e8bafc0 100644 --- a/test/plugs/http_signature_plug_test.exs +++ b/test/plugs/http_signature_plug_test.exs @@ -7,6 +7,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do alias Pleroma.Web.Plugs.HTTPSignaturePlug import Plug.Conn + import Phoenix.Controller, only: [put_format: 2] import Mock test "it call HTTPSignatures to check validity if the actor sighed it" do @@ -20,6 +21,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do "signature", "keyId=\"http://mastodon.example.org/users/admin#main-key" ) + |> put_format("activity+json") |> HTTPSignaturePlug.call(%{}) assert conn.assigns.valid_signature == true @@ -37,7 +39,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do end) params = %{"actor" => "http://mastodon.example.org/users/admin"} - conn = build_conn(:get, "/doesntmattter", params) + conn = build_conn(:get, "/doesntmattter", params) |> put_format("activity+json") [conn: conn] end