From 9cc5fe9a5fb98130b627733c01ed4de0ca7e9aa3 Mon Sep 17 00:00:00 2001
From: Oneric <oneric@oneric.stub>
Date: Mon, 13 Jan 2025 00:24:01 +0100
Subject: [PATCH] signature: refetch key upon verification failure

This matches behaviour prioir to the SigningKey migration
and the expected semantics of the http_signatures lib.
Additionally add a min interval paramter, to avoid
refetch floods on bugs causing incompatible signatures
(like e.g. currently with Bridgy)
---
 config/config.exs               |  1 +
 lib/pleroma/signature.ex        | 12 +++++++++---
 lib/pleroma/user/signing_key.ex | 17 +++++++++++++++++
 test/pleroma/signature_test.exs | 11 +++++++++++
 4 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/config/config.exs b/config/config.exs
index 48ebe91d2..be6a0d81c 100644
--- a/config/config.exs
+++ b/config/config.exs
@@ -370,6 +370,7 @@
   note_replies_output_limit: 5,
   sign_object_fetches: true,
   authorized_fetch_mode: false,
+  min_key_refetch_interval: 86_400,
   max_collection_objects: 50
 
 config :pleroma, :streamer,
diff --git a/lib/pleroma/signature.ex b/lib/pleroma/signature.ex
index 39de8e9f1..f926f1aa6 100644
--- a/lib/pleroma/signature.ex
+++ b/lib/pleroma/signature.ex
@@ -27,12 +27,18 @@ def fetch_public_key(conn) do
 
   def refetch_public_key(conn) do
     with {_, %{"keyId" => kid}} <- {:keyid, HTTPSignatures.signature_for_conn(conn)},
-         # TODO: force a refetch of stale keys (perhaps with a backoff time based on updated_at)
-         {_, {:ok, %SigningKey{} = sk}, _} <-
-           {:fetch, SigningKey.get_or_fetch_by_key_id(kid), kid},
+         {_, {:ok, %SigningKey{} = sk}, _} <- {:fetch, SigningKey.refresh_by_key_id(kid), kid},
          {_, {:ok, decoded_key}} <- {:decode, SigningKey.public_key_decoded(sk)} do
       {:ok, decoded_key}
     else
+      {:fetch, {:error, :too_young}, kid} ->
+        Logger.debug("Refusing to refetch recently updated key: #{kid}")
+        {:error, {:fetch, :too_young}}
+
+      {:fetch, {:error, :unknown}, kid} ->
+        Logger.warning("Attempted to refresh unknown key; this should not happen: #{kid}")
+        {:error, {:fetch, :unknown}}
+
       {:fetch, error, kid} ->
         Logger.error("Failed to refresh stale key from signature: #{kid} #{inspect(error)}")
         {:error, {:fetch, error}}
diff --git a/lib/pleroma/user/signing_key.ex b/lib/pleroma/user/signing_key.ex
index 708bbbe19..91aa25a4e 100644
--- a/lib/pleroma/user/signing_key.ex
+++ b/lib/pleroma/user/signing_key.ex
@@ -218,6 +218,23 @@ def fetch_remote_key(key_id) do
     end
   end
 
+  defp refresh_key(%__MODULE__{} = key) do
+    min_backoff = Pleroma.Config.get!([:activitypub, :min_key_refetch_interval])
+
+    if Timex.diff(Timex.now(), key.updated_at, :seconds) >= min_backoff do
+      fetch_remote_key(key.key_id)
+    else
+      {:error, :too_young}
+    end
+  end
+
+  def refresh_by_key_id(key_id) do
+    case Repo.get_by(__MODULE__, key_id: key_id) do
+      nil -> {:error, :unknown}
+      key -> refresh_key(key)
+    end
+  end
+
   # Take the response from the remote instance and extract the key details
   # will check if the key ID matches the owner of the key, if not, error
   defp extract_key_details(%{"id" => ap_id, "publicKey" => public_key}) do
diff --git a/test/pleroma/signature_test.exs b/test/pleroma/signature_test.exs
index 0e0105534..731605804 100644
--- a/test/pleroma/signature_test.exs
+++ b/test/pleroma/signature_test.exs
@@ -56,8 +56,19 @@ test "it returns error if public key is nil" do
 
   describe "refetch_public_key/1" do
     test "it returns key" do
+      clear_config([:activitypub, :min_key_refetch_interval], 0)
       ap_id = "https://mastodon.social/users/lambadalambda"
 
+      %Pleroma.User{signing_key: sk} =
+        Pleroma.User.get_or_fetch_by_ap_id(ap_id)
+        |> then(fn {:ok, u} -> u end)
+        |> Pleroma.User.SigningKey.load_key()
+
+      {:ok, _} =
+        %{sk | public_key: "-----BEGIN PUBLIC KEY-----\nasdfghjkl"}
+        |> Ecto.Changeset.change()
+        |> Pleroma.Repo.update()
+
       assert Signature.refetch_public_key(make_fake_conn(ap_id)) == {:ok, @rsa_public_key}
     end
   end