From a90c45b7e92ef5fb77e16bd103894e58cd4e17c6 Mon Sep 17 00:00:00 2001 From: "@luna@f.l4.pm" Date: Sat, 26 Nov 2022 19:22:56 +0000 Subject: [PATCH] Add Signed Fetch Statistics (#312) Close #304. Notes: - This patch was made on top of Pleroma develop, so I created a separate cachex worker for request signature actions, instead of Akkoma's instance cache. If that is a merge blocker, I can attempt to move logic around for that. - Regarding the `has_request_signatures: true -> false` state transition: I think that is a higher level thing (resetting instance state on new instance actor key) which is separate from the changes relevant to this one. Co-authored-by: Luna Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/312 Co-authored-by: @luna@f.l4.pm Co-committed-by: @luna@f.l4.pm --- lib/pleroma/application.ex | 3 +- lib/pleroma/instances.ex | 2 + lib/pleroma/instances/instance.ex | 30 +++- lib/pleroma/web/plugs/http_signature_plug.ex | 45 +++++ ...21123221956_add_has_request_signatures.exs | 9 + test/pleroma/instances_test.exs | 18 ++ .../web/plugs/http_signature_plug_test.exs | 167 ++++++++++++------ 7 files changed, 216 insertions(+), 58 deletions(-) create mode 100644 priv/repo/migrations/20221123221956_add_has_request_signatures.exs diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index b9bcad40c..ec8839e0f 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -157,7 +157,8 @@ defp cachex_children do build_cachex("failed_proxy_url", limit: 2500), build_cachex("banned_urls", default_ttl: :timer.hours(24 * 30), limit: 5_000), build_cachex("translations", default_ttl: :timer.hours(24 * 30), limit: 2500), - build_cachex("instances", default_ttl: :timer.hours(24), ttl_interval: 1000, limit: 2500) + build_cachex("instances", default_ttl: :timer.hours(24), ttl_interval: 1000, limit: 2500), + build_cachex("request_signatures", default_ttl: :timer.hours(24 * 30), limit: 3000) ] end diff --git a/lib/pleroma/instances.ex b/lib/pleroma/instances.ex index 6b57e56da..daea102db 100644 --- a/lib/pleroma/instances.ex +++ b/lib/pleroma/instances.ex @@ -43,4 +43,6 @@ def host(url_or_host) when is_binary(url_or_host) do url_or_host end end + + defdelegate set_request_signatures(url_or_host), to: Instance end diff --git a/lib/pleroma/instances/instance.ex b/lib/pleroma/instances/instance.ex index 27dbf7661..6ddfa5042 100644 --- a/lib/pleroma/instances/instance.ex +++ b/lib/pleroma/instances/instance.ex @@ -26,6 +26,7 @@ defmodule Pleroma.Instances.Instance do field(:favicon, :string) field(:metadata_updated_at, :naive_datetime) field(:nodeinfo, :map, default: %{}) + field(:has_request_signatures, :boolean) timestamps() end @@ -34,7 +35,14 @@ defmodule Pleroma.Instances.Instance do def changeset(struct, params \\ %{}) do struct - |> cast(params, [:host, :unreachable_since, :favicon, :nodeinfo, :metadata_updated_at]) + |> cast(params, [ + :host, + :unreachable_since, + :favicon, + :nodeinfo, + :metadata_updated_at, + :has_request_signatures + ]) |> validate_required([:host]) |> unique_constraint(:host) end @@ -316,4 +324,24 @@ def get_cached_by_url(url_or_host) do end) end end + + def set_request_signatures(url_or_host) when is_binary(url_or_host) do + host = host(url_or_host) + existing_record = Repo.get_by(Instance, %{host: host}) + changes = %{has_request_signatures: true} + + cond do + is_nil(existing_record) -> + %Instance{} + |> changeset(Map.put(changes, :host, host)) + |> Repo.insert() + + true -> + existing_record + |> changeset(changes) + |> Repo.update() + end + end + + def set_request_signatures(_), do: {:error, :invalid_input} end diff --git a/lib/pleroma/web/plugs/http_signature_plug.ex b/lib/pleroma/web/plugs/http_signature_plug.ex index c906a4eec..5ed3235e2 100644 --- a/lib/pleroma/web/plugs/http_signature_plug.ex +++ b/lib/pleroma/web/plugs/http_signature_plug.ex @@ -7,8 +7,12 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do import Phoenix.Controller, only: [get_format: 1, text: 2] alias Pleroma.Activity alias Pleroma.Web.Router + alias Pleroma.Signature + alias Pleroma.Instances require Logger + @cachex Pleroma.Config.get([:cachex, :provider], Cachex) + def init(options) do options end @@ -57,6 +61,7 @@ defp assign_valid_signature_on_route_aliases(conn, [path | rest]) do conn |> assign(:valid_signature, HTTPSignatures.validate_conn(conn)) + |> assign(:signature_actor_id, signature_host(conn)) |> assign_valid_signature_on_route_aliases(rest) end @@ -78,6 +83,36 @@ defp has_signature_header?(conn) do conn |> get_req_header("signature") |> Enum.at(0, false) end + defp maybe_require_signature( + %{assigns: %{valid_signature: true, signature_actor_id: actor_id}} = conn + ) do + # inboxes implicitly need http signatures for authentication + # so we don't really know if the instance will have broken federation after + # we turn on authorized_fetch_mode. + # + # to "check" this is a signed fetch, verify if method is GET + if conn.method == "GET" do + actor_host = URI.parse(actor_id).host + + case @cachex.get(:request_signatures_cache, actor_host) do + {:ok, nil} -> + Logger.debug("Successful signature from #{actor_host}") + Instances.set_request_signatures(actor_host) + @cachex.put(:request_signatures_cache, actor_host, true) + + {:ok, true} -> + :noop + + any -> + Logger.warn( + "expected request signature cache to return a boolean, instead got #{inspect(any)}" + ) + end + end + + conn + end + defp maybe_require_signature(%{assigns: %{valid_signature: true}} = conn), do: conn defp maybe_require_signature(conn) do @@ -90,4 +125,14 @@ defp maybe_require_signature(conn) do conn end end + + defp signature_host(conn) do + with %{"keyId" => kid} <- HTTPSignatures.signature_for_conn(conn), + {:ok, actor_id} <- Signature.key_id_to_actor_id(kid) do + actor_id + else + e -> + {:error, e} + end + end end diff --git a/priv/repo/migrations/20221123221956_add_has_request_signatures.exs b/priv/repo/migrations/20221123221956_add_has_request_signatures.exs new file mode 100644 index 000000000..b174b6f3a --- /dev/null +++ b/priv/repo/migrations/20221123221956_add_has_request_signatures.exs @@ -0,0 +1,9 @@ +defmodule Pleroma.Repo.Migrations.AddHasRequestSignatures do + use Ecto.Migration + + def change do + alter table(:instances) do + add(:has_request_signatures, :boolean, default: false, null: false) + end + end +end diff --git a/test/pleroma/instances_test.exs b/test/pleroma/instances_test.exs index 03f9e4e97..744ee8df3 100644 --- a/test/pleroma/instances_test.exs +++ b/test/pleroma/instances_test.exs @@ -4,6 +4,7 @@ defmodule Pleroma.InstancesTest do alias Pleroma.Instances + alias Pleroma.Instances.Instance use Pleroma.DataCase @@ -121,4 +122,21 @@ test "keeps unreachable url or host unreachable" do refute Instances.reachable?(host) end end + + describe "set_request_signatures/1" do + test "sets instance has request signatures" do + host = "domain.com" + + {:ok, instance} = Instances.set_request_signatures(host) + assert instance.has_request_signatures + + {:ok, cached_instance} = Instance.get_cached_by_url(host) + assert cached_instance.has_request_signatures + end + + test "returns error status on non-binary input" do + assert {:error, _} = Instances.set_request_signatures(nil) + assert {:error, _} = Instances.set_request_signatures(1) + end + 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 8ce956510..49e0b9808 100644 --- a/test/pleroma/web/plugs/http_signature_plug_test.exs +++ b/test/pleroma/web/plugs/http_signature_plug_test.exs @@ -1,34 +1,90 @@ # Pleroma: A lightweight social networking server -# Copyright © 2017-2021 Pleroma Authors +# Copyright © 2017-2022 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do - use Pleroma.Web.ConnCase + use Pleroma.Web.ConnCase, async: false import Pleroma.Factory alias Pleroma.Web.Plugs.HTTPSignaturePlug + alias Pleroma.Instances.Instance + alias Pleroma.Repo 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 + setup_with_mocks([ + {HTTPSignatures, [], + [ + signature_for_conn: fn _ -> + %{"keyId" => "http://mastodon.example.org/users/admin#main-key"} + end, + validate_conn: fn conn -> + Map.get(conn.assigns, :valid_signature, true) + end + ]} + ]) do + :ok + end + + defp submit_to_plug(host), do: submit_to_plug(host, :get, "/doesntmattter") + + defp submit_to_plug(host, method, path) do + params = %{"actor" => "http://#{host}/users/admin"} + + build_conn(method, path, params) + |> put_req_header( + "signature", + "keyId=\"http://#{host}/users/admin#main-key" + ) + |> put_format("activity+json") + |> HTTPSignaturePlug.call(%{}) + end + + test "it call HTTPSignatures to check validity if the actor signed it" do params = %{"actor" => "http://mastodon.example.org/users/admin"} conn = build_conn(:get, "/doesntmattter", params) - with_mock HTTPSignatures, validate_conn: fn _ -> true end do - conn = - conn - |> put_req_header( - "signature", - "keyId=\"http://mastodon.example.org/users/admin#main-key" - ) - |> put_format("activity+json") - |> HTTPSignaturePlug.call(%{}) + conn = + conn + |> put_req_header( + "signature", + "keyId=\"http://mastodon.example.org/users/admin#main-key" + ) + |> put_format("activity+json") + |> HTTPSignaturePlug.call(%{}) - assert conn.assigns.valid_signature == true - assert conn.halted == false - assert called(HTTPSignatures.validate_conn(:_)) - end + assert conn.assigns.valid_signature == true + assert conn.assigns.signature_actor_id == params["actor"] + assert conn.halted == false + assert called(HTTPSignatures.validate_conn(:_)) + end + + test "it sets request signatures property on the instance" do + host = "mastodon.example.org" + conn = submit_to_plug(host) + assert conn.assigns.valid_signature == true + instance = Repo.get_by(Instance, %{host: host}) + assert instance.has_request_signatures + end + + test "it does not set request signatures property on the instance when using inbox" do + host = "mastodon.example.org" + conn = submit_to_plug(host, :post, "/inbox") + assert conn.assigns.valid_signature == true + + # we don't even create the instance entry if its just POST /inbox + refute Repo.get_by(Instance, %{host: host}) + end + + test "it does not set request signatures property on the instance when its cached" do + host = "mastodon.example.org" + Cachex.put(:request_signatures_cache, host, true) + conn = submit_to_plug(host) + assert conn.assigns.valid_signature == true + + # we don't even create the instance entry if it was already done + refute Repo.get_by(Instance, %{host: host}) end describe "requires a signature when `authorized_fetch_mode` is enabled" do @@ -41,40 +97,39 @@ test "it call HTTPSignatures to check validity if the actor sighed it" do [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(%{}) + test "and signature is present and incorrect", %{conn: conn} do + conn = + conn + |> assign(:valid_signature, false) + |> 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 + 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 - test "halts the connection when `signature` header is not present", %{conn: conn} do + test "and signature is correct", %{conn: conn} 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 + + 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 @@ -82,16 +137,16 @@ test "halts the connection when `signature` header is not present", %{conn: conn assert conn.state == :sent assert conn.resp_body == "Request not signed" end + end - test "aliases redirected /object endpoints", _ do - obj = insert(:note) - act = insert(:note_activity, note: obj) - params = %{"actor" => "someparam"} - path = URI.parse(obj.data["id"]).path - conn = build_conn(:get, path, params) + test "aliases redirected /object endpoints", _ do + obj = insert(:note) + act = insert(:note_activity, note: obj) + params = %{"actor" => "someparam"} + path = URI.parse(obj.data["id"]).path + conn = build_conn(:get, path, params) - assert ["/notice/#{act.id}", "/notice/#{act.id}?actor=someparam"] == - HTTPSignaturePlug.route_aliases(conn) - end + assert ["/notice/#{act.id}", "/notice/#{act.id}?actor=someparam"] == + HTTPSignaturePlug.route_aliases(conn) end end