From 1bd8ce9c6b8338f5e97b061003d3c7c7e2ad11f3 Mon Sep 17 00:00:00 2001 From: Ro Date: Fri, 1 Nov 2024 17:43:40 -0500 Subject: [PATCH] fix: Verify body digest vs signing header --- README.md | 15 ++++-- lib/activity_pub/headers.ex | 54 +++++++++++-------- lib/postland_web/cache_body_reader.ex | 18 +++++++ .../controllers/inbox_controller.ex | 2 +- .../controllers/outbox_controller.ex | 2 +- lib/postland_web/endpoint.ex | 1 + lib/postland_web/live/profile_live.ex | 1 + test/activity_pub/headers_test.exs | 2 +- .../user_session_controller_test.exs | 1 - 9 files changed, 66 insertions(+), 30 deletions(-) create mode 100644 lib/postland_web/cache_body_reader.ex diff --git a/README.md b/README.md index 5fb40ad..9fc494e 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,6 @@ ## Backend +- [x] Check that signature header (digest) matches digest of body contents - [ ] Posting - [x] Making posts locally - [x] Figuring out follower list @@ -7,24 +8,28 @@ - [x] Post formatting - [ ] Sending posts w/ images / videos - [ ] Receiving posts w/ images / videos -- [ ] Timeline +- [x] Timeline - [x] My posts - [x] Posts from accounts you follow - - [ ] Show the actor avatar and display name + - [x] Show the actor avatar and display name - [ ] Deleting posts -- [ ] Profile - - [ ] Name field (for display name) +- [x] Profile + - [x] Name field (for display name) + - [x] Bust actor cache when you update your profile - [x] Following - [ ] Scrape public posts from the outbox when you follow - [ ] Unfollowing - [x] Being followed - [x] Accepting follows -- [ ] Approving / declining follows and authorized instance list +- [ ] Blocking +- [ ] Approving / declining follows +- [ ] Manage authorized instance list - [ ] Liking - [ ] Unliking - [ ] CW posts - [ ] Polls - [ ] DMs +- [ ] Support authenticated fetch of outbox (by allowed domains / servers) - [ ] Followers-only posts (or maybe this is handled because we only send posts to followers? but we also include public in the TO field?) ## UX diff --git a/lib/activity_pub/headers.ex b/lib/activity_pub/headers.ex index 9524a53..87a9915 100644 --- a/lib/activity_pub/headers.ex +++ b/lib/activity_pub/headers.ex @@ -10,21 +10,25 @@ defmodule ActivityPub.Headers do ] def signing_headers(method, url, body, actor_url, private_key) do - url = case url do - url when is_struct(url, URI) -> - url - url when is_binary(url) -> - URI.parse(url) + url = + case url do + url when is_struct(url, URI) -> + url + + url when is_binary(url) -> + URI.parse(url) end - private_key = case private_key do - "-----BEGIN" <> _ = key_pem -> - key_pem - |> :public_key.pem_decode() - |> hd() - |> :public_key.pem_entry_decode() - private_key -> - private_key + private_key = + case private_key do + "-----BEGIN" <> _ = key_pem -> + key_pem + |> :public_key.pem_decode() + |> hd() + |> :public_key.pem_entry_decode() + + private_key -> + private_key end method = String.downcase("#{method}") @@ -51,7 +55,7 @@ defmodule ActivityPub.Headers do [{"signature", signature_header} | headers] end - def verify(method, path, headers, actor_fetcher \\ &fetch_actor_key/1) do + def verify(method, path, headers, body, actor_fetcher \\ &fetch_actor_key/1) do {_key, signature_header} = Enum.find(headers, fn {key, _} -> key == "signature" end) signature_kv = SignatureSplitter.split(signature_header) @@ -59,15 +63,23 @@ defmodule ActivityPub.Headers do signature = signature_kv |> find_value("signature") |> Base.decode64!() signing_text_headers = signature_kv |> find_value("headers") |> String.split(" ") - to_verify = - signing_text(signing_text_headers, [request_target_pseudoheader(method, path) | headers]) + digest = :crypto.hash(:sha256, body) + expected_digest_header = "SHA-256=#{Base.encode64(digest)}" + actual_digest_header = find_value(headers, "digest") - case actor_fetcher.(key_id) do - {:ok, public_key} -> - :public_key.verify(to_verify, :sha256, signature, public_key) + if expected_digest_header != actual_digest_header do + {:error, :digest} + else + to_verify = + signing_text(signing_text_headers, [request_target_pseudoheader(method, path) | headers]) - error -> - error + case actor_fetcher.(key_id) do + {:ok, public_key} -> + :public_key.verify(to_verify, :sha256, signature, public_key) + + error -> + error + end end end diff --git a/lib/postland_web/cache_body_reader.ex b/lib/postland_web/cache_body_reader.ex new file mode 100644 index 0000000..7bbf804 --- /dev/null +++ b/lib/postland_web/cache_body_reader.ex @@ -0,0 +1,18 @@ +defmodule PostlandWeb.CacheBodyReader do + @moduledoc """ + Inspired by https://hexdocs.pm/plug/1.6.0/Plug.Parsers.html#module-custom-body-reader + """ + + alias Plug.Conn + + @doc """ + Read the raw body and store it for later use in the connection. + It ignores the updated connection returned by `Plug.Conn.read_body/2` to not break CSRF. + """ + @spec read_body(Conn.t(), Plug.opts()) :: {:ok, String.t(), Conn.t()} + def read_body(conn, opts) do + {:ok, body, _conn} = Conn.read_body(conn, opts) + conn = update_in(conn.assigns[:raw_body], &[body | &1 || []]) + {:ok, body, conn} + end +end diff --git a/lib/postland_web/controllers/inbox_controller.ex b/lib/postland_web/controllers/inbox_controller.ex index bf76b36..2c4e86b 100644 --- a/lib/postland_web/controllers/inbox_controller.ex +++ b/lib/postland_web/controllers/inbox_controller.ex @@ -7,7 +7,7 @@ defmodule PostlandWeb.InboxController do alias Postland.Activities def post(conn, params) do - if Headers.verify(conn.method, conn.request_path, conn.req_headers) do + if Headers.verify(conn.method, conn.request_path, conn.req_headers, conn.assigns.raw_body) do case Activities.process_activity(params) do {:ok, _activity} -> send_resp(conn, 200, Jason.encode!(params)) diff --git a/lib/postland_web/controllers/outbox_controller.ex b/lib/postland_web/controllers/outbox_controller.ex index de62bea..2b8e08d 100644 --- a/lib/postland_web/controllers/outbox_controller.ex +++ b/lib/postland_web/controllers/outbox_controller.ex @@ -14,7 +14,7 @@ defmodule PostlandWeb.OutboxController do "orderedItems" => [] } - if Headers.verify(conn.method, conn.request_path, conn.req_headers) do + if Headers.verify(conn.method, conn.request_path, conn.req_headers, conn.assigns.raw_body) do Plug.Conn.send_resp(conn, 200, Jason.encode!(json)) else send_resp(conn, 403, "forbidden") diff --git a/lib/postland_web/endpoint.ex b/lib/postland_web/endpoint.ex index e2c1441..5e93c81 100644 --- a/lib/postland_web/endpoint.ex +++ b/lib/postland_web/endpoint.ex @@ -49,6 +49,7 @@ defmodule PostlandWeb.Endpoint do plug Plug.Telemetry, event_prefix: [:phoenix, :endpoint] plug Plug.Parsers, + body_reader: {PostlandWeb.CacheBodyReader, :read_body, []}, parsers: [:urlencoded, :multipart, :json], pass: ["*/*"], json_decoder: Phoenix.json_library() diff --git a/lib/postland_web/live/profile_live.ex b/lib/postland_web/live/profile_live.ex index 7458621..701ad66 100644 --- a/lib/postland_web/live/profile_live.ex +++ b/lib/postland_web/live/profile_live.ex @@ -88,6 +88,7 @@ defmodule PostlandWeb.ProfileLive do case Repo.update(changeset) do {:ok, account} -> form = account |> User.profile_changeset(%{}) |> to_form() + Cachex.del(:main_cache, "actor:#{Postland.my_actor_id()}") {:noreply, assign(socket, account: account, editing: false, form: form)} {:error, changeset} -> diff --git a/test/activity_pub/headers_test.exs b/test/activity_pub/headers_test.exs index f7cb091..249ede3 100644 --- a/test/activity_pub/headers_test.exs +++ b/test/activity_pub/headers_test.exs @@ -33,7 +33,7 @@ defmodule ActivityPub.HeadersTest do headers = signing_headers(method, url, body, actor_url, private) - verify(method, "/inbox", headers, actor_fetcher) + verify(method, "/inbox", headers, body, actor_fetcher) end end end diff --git a/test/postland_web/controllers/user_session_controller_test.exs b/test/postland_web/controllers/user_session_controller_test.exs index 668ca64..ccde3ae 100644 --- a/test/postland_web/controllers/user_session_controller_test.exs +++ b/test/postland_web/controllers/user_session_controller_test.exs @@ -20,7 +20,6 @@ defmodule PostlandWeb.UserSessionControllerTest do # Now do a logged in request and assert on the menu conn = get(conn, ~p"/") response = html_response(conn, 200) - assert response =~ user.username assert response =~ ~p"/users/settings" assert response =~ ~p"/users/log_out" end