From 4a78c431cfe1373ba09a3c02bc56502063975890 Mon Sep 17 00:00:00 2001 From: Atsuko Karagi Date: Mon, 19 Dec 2022 20:41:48 +0000 Subject: [PATCH] Simplified HTTP signature processing --- CHANGELOG.md | 1 + lib/pleroma/web/o_auth/scopes.ex | 2 +- .../web/plugs/ensure_http_signature_plug.ex | 31 +++++++++ lib/pleroma/web/plugs/http_signature_plug.ex | 17 +---- lib/pleroma/web/router.ex | 1 + .../plugs/ensure_http_signature_plug_test.exs | 68 +++++++++++++++++++ .../web/plugs/http_signature_plug_test.exs | 9 --- 7 files changed, 105 insertions(+), 24 deletions(-) create mode 100644 lib/pleroma/web/plugs/ensure_http_signature_plug.ex create mode 100644 test/pleroma/web/plugs/ensure_http_signature_plug_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 73346617e..6b99a0e3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Non-admin users now cannot register `admin` scope tokens (not security-critical, they didn't work before, but you _could_ create them) - Admin scopes will be dropped on create - Rich media will now backoff for 20 minutes after a failure +- Simplified HTTP signature processing ### Fixed - /api/v1/accounts/lookup will now respect restrict\_unauthenticated diff --git a/lib/pleroma/web/o_auth/scopes.ex b/lib/pleroma/web/o_auth/scopes.ex index d5e7c29d6..344ecd631 100644 --- a/lib/pleroma/web/o_auth/scopes.ex +++ b/lib/pleroma/web/o_auth/scopes.ex @@ -61,7 +61,7 @@ defmodule Pleroma.Web.OAuth.Scopes do def validate(blank_scopes, _app_scopes, _user) when blank_scopes in [nil, []], do: {:error, :missing_scopes} - def validate(scopes, app_scopes, %Pleroma.User{is_admin: is_admin}) do + def validate(scopes, app_scopes, _user) do validate_scopes_are_supported(scopes, app_scopes) end diff --git a/lib/pleroma/web/plugs/ensure_http_signature_plug.ex b/lib/pleroma/web/plugs/ensure_http_signature_plug.ex new file mode 100644 index 000000000..c75501a2d --- /dev/null +++ b/lib/pleroma/web/plugs/ensure_http_signature_plug.ex @@ -0,0 +1,31 @@ +# Akkoma: Magically expressive social media +# Copyright © 2022-2022 Akkoma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.Plugs.EnsureHTTPSignaturePlug do + @moduledoc """ + Ensures HTTP signature has been validated by previous plugs on ActivityPub requests. + """ + import Plug.Conn + import Phoenix.Controller, only: [get_format: 1, text: 2] + + alias Pleroma.Config + + def init(options) do + options + end + + def call(%{assigns: %{valid_signature: true}} = conn, _), do: conn + + def call(conn, _) do + with true <- get_format(conn) in ["json", "activity+json"], + true <- Config.get([:activitypub, :authorized_fetch_mode], true) do + conn + |> put_status(:unauthorized) + |> text("Request not signed") + |> halt() + else + _ -> conn + end + end +end diff --git a/lib/pleroma/web/plugs/http_signature_plug.ex b/lib/pleroma/web/plugs/http_signature_plug.ex index 5ed3235e2..4ffaa6e98 100644 --- a/lib/pleroma/web/plugs/http_signature_plug.ex +++ b/lib/pleroma/web/plugs/http_signature_plug.ex @@ -4,7 +4,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do import Plug.Conn - import Phoenix.Controller, only: [get_format: 1, text: 2] + import Phoenix.Controller, only: [get_format: 1] alias Pleroma.Activity alias Pleroma.Web.Router alias Pleroma.Signature @@ -22,7 +22,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do end def call(conn, _opts) do - if get_format(conn) == "activity+json" do + if get_format(conn) in ["json", "activity+json"] do conn |> maybe_assign_valid_signature() |> maybe_require_signature() @@ -113,18 +113,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do conn 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) - |> text("Request not signed") - |> halt() - else - conn - end - end + defp maybe_require_signature(conn), do: conn defp signature_host(conn) do with %{"keyId" => kid} <- HTTPSignatures.signature_for_conn(conn), diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index f47041b0b..f984ad598 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -147,6 +147,7 @@ defmodule Pleroma.Web.Router do pipeline :http_signature do plug(Pleroma.Web.Plugs.HTTPSignaturePlug) plug(Pleroma.Web.Plugs.MappedSignatureToIdentityPlug) + plug(Pleroma.Web.Plugs.EnsureHTTPSignaturePlug) end pipeline :static_fe do diff --git a/test/pleroma/web/plugs/ensure_http_signature_plug_test.exs b/test/pleroma/web/plugs/ensure_http_signature_plug_test.exs new file mode 100644 index 000000000..35dabbe1c --- /dev/null +++ b/test/pleroma/web/plugs/ensure_http_signature_plug_test.exs @@ -0,0 +1,68 @@ +# Akkoma: Magically expressive social media +# Copyright © 2022-2022 Akkoma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.Plugs.EnsureHTTPSignaturePlugTest do + use Pleroma.Web.ConnCase + alias Pleroma.Web.Plugs.EnsureHTTPSignaturePlug + + import Plug.Conn + import Phoenix.Controller, only: [put_format: 2] + + import Pleroma.Tests.Helpers, only: [clear_config: 2] + + describe "requires a signature when `authorized_fetch_mode` is enabled" do + setup do + clear_config([:activitypub, :authorized_fetch_mode], true) + + conn = + build_conn(:get, "/doesntmatter") + |> put_format("activity+json") + + [conn: conn] + end + + test "and signature has been set as invalid", %{conn: conn} do + conn = + conn + |> assign(:valid_signature, false) + |> EnsureHTTPSignaturePlug.call(%{}) + + assert conn.halted == true + assert conn.status == 401 + assert conn.state == :sent + assert conn.resp_body == "Request not signed" + end + + test "and signature has been set as valid", %{conn: conn} do + conn = + conn + |> assign(:valid_signature, true) + |> EnsureHTTPSignaturePlug.call(%{}) + + assert conn.halted == false + end + + test "does nothing for non-ActivityPub content types", %{conn: conn} do + conn = + conn + |> assign(:valid_signature, false) + |> put_format("html") + |> EnsureHTTPSignaturePlug.call(%{}) + + assert conn.halted == false + end + end + + test "does nothing on invalid signature when `authorized_fetch_mode` is disabled" do + clear_config([:activitypub, :authorized_fetch_mode], false) + + conn = + build_conn(:get, "/doesntmatter") + |> put_format("activity+json") + |> assign(:valid_signature, false) + |> EnsureHTTPSignaturePlug.call(%{}) + + assert conn.halted == false + end +end diff --git a/test/pleroma/web/plugs/http_signature_plug_test.exs b/test/pleroma/web/plugs/http_signature_plug_test.exs index 49e0b9808..34d0dc00e 100644 --- a/test/pleroma/web/plugs/http_signature_plug_test.exs +++ b/test/pleroma/web/plugs/http_signature_plug_test.exs @@ -108,10 +108,6 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do |> 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 @@ -125,17 +121,12 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do |> HTTPSignaturePlug.call(%{}) assert conn.assigns.valid_signature == true - assert conn.halted == false assert called(HTTPSignatures.validate_conn(:_)) end test "and 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