From 88d064d80e4a3272a2a7101089b5f924fd175866 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Thu, 18 Jul 2019 15:06:58 +0000 Subject: http signature plug: remove redundant checks handled by HTTPSignatures library the redundant checks assumed a POST request, which will not work for signed GETs. this check was originally needed because the HTTPSignatures adapter assumed that the requests were also POST requests. but now, the adapter has been corrected. --- lib/pleroma/plugs/http_signature.ex | 49 +++++++++++++++---------------------- 1 file changed, 20 insertions(+), 29 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/plugs/http_signature.ex b/lib/pleroma/plugs/http_signature.ex index e2874c469..d87fa52fa 100644 --- a/lib/pleroma/plugs/http_signature.ex +++ b/lib/pleroma/plugs/http_signature.ex @@ -3,7 +3,6 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do - alias Pleroma.Web.ActivityPub.Utils import Plug.Conn require Logger @@ -16,38 +15,30 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do end def call(conn, _opts) do - user = Utils.get_ap_id(conn.params["actor"]) - Logger.debug("Checking sig for #{user}") [signature | _] = get_req_header(conn, "signature") - cond do - signature && String.contains?(signature, user) -> - # 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}" - ) - - conn = - if conn.assigns[:digest] do - conn - |> put_req_header("digest", conn.assigns[:digest]) - else - conn - end - - assign(conn, :valid_signature, HTTPSignatures.validate_conn(conn)) + if signature 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}" + ) - signature -> - Logger.debug("Signature not from actor") - assign(conn, :valid_signature, false) + conn = + if conn.assigns[:digest] do + conn + |> put_req_header("digest", conn.assigns[:digest]) + else + conn + end - true -> - Logger.debug("No signature header!") - conn + assign(conn, :valid_signature, HTTPSignatures.validate_conn(conn)) + else + Logger.debug("No signature header!") + conn end end end -- cgit v1.2.3 From 18d8d12d53567b7c0c246bb793ee724d0d2e4c77 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Thu, 18 Jul 2019 15:35:42 +0000 Subject: signature: make key_id_to_actor_id() public --- lib/pleroma/signature.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/signature.ex b/lib/pleroma/signature.ex index a45c70a9d..2a0823ecf 100644 --- a/lib/pleroma/signature.ex +++ b/lib/pleroma/signature.ex @@ -9,7 +9,7 @@ defmodule Pleroma.Signature do alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub - defp key_id_to_actor_id(key_id) do + def key_id_to_actor_id(key_id) do URI.parse(key_id) |> Map.put(:fragment, nil) |> URI.to_string() -- cgit v1.2.3 From 184fa61fb3a1bc8c5d5515bb7748c12816b11ebf Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Thu, 18 Jul 2019 15:38:45 +0000 Subject: plugs: add MappedSignatureToIdentityPlug --- .../plugs/mapped_signature_to_identity_plug.ex | 64 ++++++++++++++++++++++ lib/pleroma/web/router.ex | 1 + 2 files changed, 65 insertions(+) create mode 100644 lib/pleroma/plugs/mapped_signature_to_identity_plug.ex (limited to 'lib') diff --git a/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex b/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex new file mode 100644 index 000000000..ae9339595 --- /dev/null +++ b/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex @@ -0,0 +1,64 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.Plugs.MappedSignatureToIdentityPlug do + alias Pleroma.Signature + alias Pleroma.User + alias Pleroma.Web.ActivityPub.Utils + + import Plug.Conn + require Logger + + def init(options), do: options + + defp key_id_from_conn(conn) do + with %{"keyId" => key_id} <- HTTPSignatures.signature_for_conn(conn) do + Signature.key_id_to_actor_id(key_id) + else + _ -> + nil + end + end + + defp user_from_key_id(conn) do + with key_actor_id when is_binary(key_actor_id) <- key_id_from_conn(conn), + %User{} = user <- User.get_or_fetch_by_ap_id(key_actor_id) do + user + else + _ -> + nil + end + end + + def call(%{assigns: %{mapped_identity: _}} = conn, _opts), do: conn + + # if this has payload make sure it is signed by the same actor that made it + def call(%{assigns: %{valid_signature: true}, params: %{"actor" => actor}} = conn, _opts) do + with actor_id <- Utils.get_ap_id(actor), + %User{} = user <- user_from_key_id(conn), + true <- user.ap_id == actor_id do + assign(conn, :mapped_identity, user) + else + _ -> + Logger.debug("Failed to map identity from signature (payload actor mismatch?)") + Logger.debug("key_id=#{key_id_from_conn(conn)}, actor=#{actor}") + conn + end + end + + # no payload, probably a signed fetch + def call(%{assigns: %{valid_signature: true}} = conn, _opts) do + with %User{} = user <- user_from_key_id(conn) do + assign(conn, :mapped_identity, user) + else + _ -> + Logger.debug("Failed to map identity from signature (no payload actor mismatch)") + Logger.debug("key_id=#{key_id_from_conn(conn)}") + conn + end + end + + # no signature at all + def call(conn, _opts), do: conn +end diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 8095ac4b1..518720d38 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -617,6 +617,7 @@ defmodule Pleroma.Web.Router do pipeline :activitypub do plug(:accepts, ["activity+json", "json"]) plug(Pleroma.Web.Plugs.HTTPSignaturePlug) + plug(Pleroma.Web.Plugs.MappedSignatureToIdentityPlug) end scope "/", Pleroma.Web.ActivityPub do -- cgit v1.2.3 From 5ea0cd69f7457086fc486f13e072f13d2c1ef547 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Thu, 18 Jul 2019 16:01:21 +0000 Subject: mapped signature plug: don't invalidate in cases where a signature is actually not present (testsuite) --- lib/pleroma/plugs/mapped_signature_to_identity_plug.ex | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex b/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex index ae9339595..2a8ed4470 100644 --- a/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex +++ b/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex @@ -36,12 +36,18 @@ defmodule Pleroma.Web.Plugs.MappedSignatureToIdentityPlug do # if this has payload make sure it is signed by the same actor that made it def call(%{assigns: %{valid_signature: true}, params: %{"actor" => actor}} = conn, _opts) do with actor_id <- Utils.get_ap_id(actor), - %User{} = user <- user_from_key_id(conn), - true <- user.ap_id == actor_id do + {:user, %User{} = user} <- {:user, user_from_key_id(conn)}, + {:user_match, true} <- {:user_match, user.ap_id == actor_id} do assign(conn, :mapped_identity, user) else - _ -> - Logger.debug("Failed to map identity from signature (payload actor mismatch?)") + {:user_match, false} -> + Logger.debug("Failed to map identity from signature (payload actor mismatch)") + Logger.debug("key_id=#{key_id_from_conn(conn)}, actor=#{actor}") + assign(conn, :valid_signature, false) + + # remove me once testsuite uses mapped capabilities instead of what we do now + {:user, nil} -> + Logger.debug("Failed to map identity from signature (lookup failure)") Logger.debug("key_id=#{key_id_from_conn(conn)}, actor=#{actor}") conn end @@ -55,7 +61,7 @@ defmodule Pleroma.Web.Plugs.MappedSignatureToIdentityPlug do _ -> Logger.debug("Failed to map identity from signature (no payload actor mismatch)") Logger.debug("key_id=#{key_id_from_conn(conn)}") - conn + assign(conn, :valid_signature, false) end end -- cgit v1.2.3 From a8af0ac053713102204418fe7a28d322f81eb3ea Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Thu, 18 Jul 2019 16:27:50 +0000 Subject: mapped signature plug: fix user lookup --- lib/pleroma/plugs/mapped_signature_to_identity_plug.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex b/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex index 2a8ed4470..1e7da4f50 100644 --- a/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex +++ b/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex @@ -23,7 +23,7 @@ defmodule Pleroma.Web.Plugs.MappedSignatureToIdentityPlug do defp user_from_key_id(conn) do with key_actor_id when is_binary(key_actor_id) <- key_id_from_conn(conn), - %User{} = user <- User.get_or_fetch_by_ap_id(key_actor_id) do + {:ok, %User{} = user} <- User.get_or_fetch_by_ap_id(key_actor_id) do user else _ -> -- cgit v1.2.3 From c947cfec5ab49f90fb2de83f61bda77568298d6f Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Thu, 18 Jul 2019 20:31:25 +0000 Subject: mapped signature plug: use `user` assign like authentication plug --- lib/pleroma/plugs/mapped_signature_to_identity_plug.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex b/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex index 1e7da4f50..ce8494b9d 100644 --- a/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex +++ b/lib/pleroma/plugs/mapped_signature_to_identity_plug.ex @@ -31,14 +31,14 @@ defmodule Pleroma.Web.Plugs.MappedSignatureToIdentityPlug do end end - def call(%{assigns: %{mapped_identity: _}} = conn, _opts), do: conn + def call(%{assigns: %{user: _}} = conn, _opts), do: conn # if this has payload make sure it is signed by the same actor that made it def call(%{assigns: %{valid_signature: true}, params: %{"actor" => actor}} = conn, _opts) do with actor_id <- Utils.get_ap_id(actor), {:user, %User{} = user} <- {:user, user_from_key_id(conn)}, {:user_match, true} <- {:user_match, user.ap_id == actor_id} do - assign(conn, :mapped_identity, user) + assign(conn, :user, user) else {:user_match, false} -> Logger.debug("Failed to map identity from signature (payload actor mismatch)") @@ -56,7 +56,7 @@ defmodule Pleroma.Web.Plugs.MappedSignatureToIdentityPlug do # no payload, probably a signed fetch def call(%{assigns: %{valid_signature: true}} = conn, _opts) do with %User{} = user <- user_from_key_id(conn) do - assign(conn, :mapped_identity, user) + assign(conn, :user, user) else _ -> Logger.debug("Failed to map identity from signature (no payload actor mismatch)") -- cgit v1.2.3