http_signatures: short-circuit gracefully on MRF rejects
And adjust log details
This commit is contained in:
parent
6cb6e3886c
commit
3b9055f7d9
4 changed files with 41 additions and 4 deletions
|
@ -45,6 +45,9 @@ defp handle_common_errors(error, kid, action_name) do
|
|||
{:fetch, {:error, :not_found}} ->
|
||||
{:halt, {:error, :gone}}
|
||||
|
||||
{:fetch, {:reject, reason}} ->
|
||||
{:halt, {:error, {:reject, reason}}}
|
||||
|
||||
{:fetch, error} ->
|
||||
Logger.error("Failed to #{action_name} key from signature: #{kid} #{inspect(error)}")
|
||||
{:error, {:fetch, error}}
|
||||
|
|
|
@ -211,6 +211,7 @@ def fetch_remote_key(key_id) do
|
|||
|
||||
case e do
|
||||
{:error, e} -> {:error, e}
|
||||
{:reject, reason} -> {:reject, reason}
|
||||
_ -> {:error, {"Could not fetch key", e}}
|
||||
end
|
||||
end
|
||||
|
|
|
@ -12,7 +12,6 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do
|
|||
alias Pleroma.Activity
|
||||
require Logger
|
||||
|
||||
|
||||
def init(options) do
|
||||
options
|
||||
end
|
||||
|
@ -43,21 +42,36 @@ def route_aliases(%{path_info: ["objects", id], query_string: query_string}) do
|
|||
def route_aliases(_), do: []
|
||||
|
||||
defp maybe_log_error(conn, verification_error) do
|
||||
siginfo_str =
|
||||
"<#{conn.method} #{conn.request_path}> #{inspect(get_req_header(conn, "signature"))}"
|
||||
|
||||
case verification_error do
|
||||
:gone ->
|
||||
# We can't verify the data since the actor was deleted and not previously known.
|
||||
# Likely we just received the actor’s Delete activity, so just silently drop.
|
||||
Logger.debug("Unable to verify request signature of deleted actor; dropping (#{inspect(conn)})")
|
||||
Logger.debug(
|
||||
"Unable to verify request signature of deleted actor; dropping (#{siginfo_str})"
|
||||
)
|
||||
|
||||
{:reject, reason} ->
|
||||
Logger.debug(
|
||||
"Refusing to validate signature from rejected key due to #{inspect(reason)} #{siginfo_str}"
|
||||
)
|
||||
|
||||
:wrong_signature ->
|
||||
Logger.warning("Received request with invalid signature!\n#{inspect(conn)}")
|
||||
|
||||
{:fetch_key, e} ->
|
||||
Logger.info("Unable to verify request since key cannot be retrieved: #{inspect(e)}")
|
||||
Logger.info(
|
||||
"Unable to verify request since key cannot be retrieved: #{inspect(e)} #{siginfo_str}"
|
||||
)
|
||||
|
||||
error ->
|
||||
Logger.error("Failed to verify request signature due to fatal error: #{inspect(error)}")
|
||||
Logger.error(
|
||||
"Failed to verify request signature due to fatal error: #{inspect(error)} #{inspect(conn)}"
|
||||
)
|
||||
end
|
||||
|
||||
conn
|
||||
end
|
||||
|
||||
|
|
|
@ -31,6 +31,9 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do
|
|||
Map.get(conn.assigns, :gone_signature_key, false) ->
|
||||
{:error, :gone}
|
||||
|
||||
Map.get(conn.assigns, :rejected_key_id, false) ->
|
||||
{:error, {:reject, :mrf}}
|
||||
|
||||
Map.get(conn.assigns, :valid_signature, true) ->
|
||||
{:ok, user} = Pleroma.User.get_or_fetch_by_ap_id(@user_ap_id)
|
||||
{:ok, %HTTPSignatures.HTTPKey{key: "aaa", user_data: %{"key_user" => user}}}
|
||||
|
@ -144,4 +147,20 @@ test "fails on gone key for non-Delete" do
|
|||
assert conn.assigns.valid_signature == false
|
||||
assert conn.assigns.signature_user == nil
|
||||
end
|
||||
|
||||
test "fails on rejected keys", %{user: user} do
|
||||
conn =
|
||||
build_conn(:post, "/inbox", %{"type" => "Note"})
|
||||
|> put_format("activity+json")
|
||||
|> assign(:rejected_key_id, true)
|
||||
|> put_req_header(
|
||||
"signature",
|
||||
"keyId=\"#{user.signing_key.key_id}\""
|
||||
)
|
||||
|> HTTPSignaturePlug.call(%{})
|
||||
|
||||
refute conn.halted
|
||||
assert conn.assigns.valid_signature == false
|
||||
assert conn.assigns.signature_user == nil
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Add table
Reference in a new issue