diff --git a/lib/pleroma/signature.ex b/lib/pleroma/signature.ex index bc3baf433..86065a603 100644 --- a/lib/pleroma/signature.ex +++ b/lib/pleroma/signature.ex @@ -9,24 +9,11 @@ defmodule Pleroma.Signature do alias Pleroma.User.SigningKey require Logger - def key_id_to_actor_id(key_id) do - case SigningKey.key_id_to_ap_id(key_id) do - nil -> - # hm, we SHOULD have gotten this in the pipeline before we hit here! - Logger.error("Could not figure out who owns the key #{key_id}") - {:error, :key_owner_not_found} - - key -> - {:ok, key} - end - end - def fetch_public_key(conn) do with %{"keyId" => kid} <- HTTPSignatures.signature_for_conn(conn), - {:ok, %SigningKey{}} <- SigningKey.get_or_fetch_by_key_id(kid), - {:ok, actor_id} <- key_id_to_actor_id(kid), - {:ok, public_key} <- User.get_public_key_for_ap_id(actor_id) do - {:ok, public_key} + {:ok, %SigningKey{} = sk} <- SigningKey.get_or_fetch_by_key_id(kid), + {:ok, decoded_key} <- SigningKey.public_key_decoded(sk) do + {:ok, decoded_key} else e -> {:error, e} @@ -35,10 +22,10 @@ def fetch_public_key(conn) do def refetch_public_key(conn) do with %{"keyId" => kid} <- HTTPSignatures.signature_for_conn(conn), - {:ok, %SigningKey{}} <- SigningKey.get_or_fetch_by_key_id(kid), - {:ok, actor_id} <- key_id_to_actor_id(kid), - {:ok, public_key} <- User.get_public_key_for_ap_id(actor_id) do - {:ok, public_key} + # TODO: force a refetch of stale keys (perhaps with a backoff time based on updated_at) + {:ok, %SigningKey{} = sk} <- SigningKey.get_or_fetch_by_key_id(kid), + {:ok, decoded_key} <- SigningKey.public_key_decoded(sk) do + {:ok, decoded_key} else e -> {:error, e} diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 666464cab..5f3ddf64a 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -2068,17 +2068,6 @@ defp create_service_actor(uri, nickname) do defdelegate public_key(user), to: SigningKey - def get_public_key_for_ap_id(ap_id) do - with {:ok, %User{} = user} <- get_or_fetch_by_ap_id(ap_id), - {:ok, public_key} <- SigningKey.public_key(user) do - {:ok, public_key} - else - e -> - Logger.error("Could not get public key for #{ap_id}.\n#{inspect(e)}") - {:error, e} - end - end - @doc "Gets or fetch a user by uri or nickname." @spec get_or_fetch(String.t()) :: {:ok, User.t()} | {:error, String.t()} def get_or_fetch("http://" <> _host = uri), do: get_or_fetch_by_ap_id(uri) diff --git a/lib/pleroma/user/signing_key.ex b/lib/pleroma/user/signing_key.ex index 705302013..708bbbe19 100644 --- a/lib/pleroma/user/signing_key.ex +++ b/lib/pleroma/user/signing_key.ex @@ -115,24 +115,18 @@ def private_pem_to_public_pem(private_pem) do {:ok, :public_key.pem_encode([public_key])} end - @spec public_key(User.t()) :: {:ok, binary()} | {:error, String.t()} + @spec public_key(__MODULE__) :: {:ok, binary()} | {:error, String.t()} @doc """ - Given a user, return the public key for that user in binary format. + Return public key data in binary format. """ - def public_key(%User{} = user) do - case Repo.preload(user, :signing_key) do - %User{signing_key: %__MODULE__{public_key: public_key_pem}} -> - key = - public_key_pem - |> :public_key.pem_decode() - |> hd() - |> :public_key.pem_entry_decode() + def public_key_decoded(%__MODULE__{public_key: public_key_pem}) do + decoded = + public_key_pem + |> :public_key.pem_decode() + |> hd() + |> :public_key.pem_entry_decode() - {:ok, key} - - _ -> - {:error, "key not found"} - end + {:ok, decoded} end def public_key(_), do: {:error, "key not found"} diff --git a/lib/pleroma/web/plugs/http_signature_plug.ex b/lib/pleroma/web/plugs/http_signature_plug.ex index 06527cada..c8df805fe 100644 --- a/lib/pleroma/web/plugs/http_signature_plug.ex +++ b/lib/pleroma/web/plugs/http_signature_plug.ex @@ -8,8 +8,8 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do use Pleroma.Web, :verified_routes alias Pleroma.Activity - alias Pleroma.Signature alias Pleroma.Instances + alias Pleroma.User.SigningKey require Logger @cachex Pleroma.Config.get([:cachex, :provider], Cachex) @@ -140,15 +140,17 @@ defp maybe_require_signature(conn), do: conn defp signature_host(conn) do with {:key_id, %{"keyId" => kid}} <- {:key_id, HTTPSignatures.signature_for_conn(conn)}, - {:actor_id, {:ok, actor_id}} <- {:actor_id, Signature.key_id_to_actor_id(kid)} do + {:actor_id, actor_id, _} when actor_id != nil <- + {:actor_id, SigningKey.key_id_to_ap_id(kid), kid} do actor_id else {:key_id, e} -> Logger.error("Failed to extract key_id from signature: #{inspect(e)}") nil - {:actor_id, e} -> - Logger.error("Failed to extract actor_id from signature: #{inspect(e)}") + {:actor_id, _, kid} -> + # SigningKeys SHOULD have been fetched before this gets called! + Logger.error("Failed to extract actor_id from signature: signing key #{kid} not known") nil end end diff --git a/test/pleroma/signature_test.exs b/test/pleroma/signature_test.exs index 86e2b138a..0e0105534 100644 --- a/test/pleroma/signature_test.exs +++ b/test/pleroma/signature_test.exs @@ -114,16 +114,6 @@ test "it returns signature headers" do end end - describe "key_id_to_actor_id/1" do - test "it reverses the key id to actor id" do - user = - insert(:user) - |> with_signing_key() - - assert Signature.key_id_to_actor_id(user.signing_key.key_id) == {:ok, user.ap_id} - end - end - describe "signed_date" do test "it returns formatted current date" do with_mock(NaiveDateTime, utc_now: fn -> ~N[2019-08-23 18:11:24.822233] end) do diff --git a/test/pleroma/user_test.exs b/test/pleroma/user_test.exs index f68bf29bc..be8e21dcb 100644 --- a/test/pleroma/user_test.exs +++ b/test/pleroma/user_test.exs @@ -1820,10 +1820,6 @@ test "unsuggests a user" do end end - test "get_public_key_for_ap_id fetches a user that's not in the db" do - assert {:ok, _key} = User.get_public_key_for_ap_id("http://mastodon.example.org/users/admin") - end - describe "per-user rich-text filtering" do test "html_filter_policy returns default policies, when rich-text is enabled" do user = insert(:user)