From 1dc5794e2996d09dee22f0156c4a442c8338aa8d Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Mon, 22 Feb 2021 14:46:59 -0600 Subject: [PATCH 01/12] Never forward the client's user-agent through the media proxy --- lib/pleroma/reverse_proxy.ex | 26 +++++++------------- test/pleroma/reverse_proxy_test.exs | 38 +++++++++++++---------------- 2 files changed, 26 insertions(+), 38 deletions(-) diff --git a/lib/pleroma/reverse_proxy.ex b/lib/pleroma/reverse_proxy.ex index 466906f03..406f7e2b8 100644 --- a/lib/pleroma/reverse_proxy.ex +++ b/lib/pleroma/reverse_proxy.ex @@ -4,7 +4,7 @@ defmodule Pleroma.ReverseProxy do @range_headers ~w(range if-range) - @keep_req_headers ~w(accept user-agent accept-encoding cache-control if-modified-since) ++ + @keep_req_headers ~w(accept accept-encoding cache-control if-modified-since) ++ ~w(if-unmodified-since if-none-match) ++ @range_headers @resp_cache_headers ~w(etag date last-modified) @keep_resp_headers @resp_cache_headers ++ @@ -57,9 +57,6 @@ def default_cache_control_header, do: @default_cache_control_header * `false` will add `content-disposition: attachment` to any request, * a list of whitelisted content types - * `keep_user_agent` will forward the client's user-agent to the upstream. This may be useful if the upstream is - doing content transformation (encoding, …) depending on the request. - * `req_headers`, `resp_headers` additional headers. * `http`: options for [hackney](https://github.com/benoitc/hackney) or [gun](https://github.com/ninenines/gun). @@ -84,8 +81,7 @@ def default_cache_control_header, do: @default_cache_control_header import Plug.Conn @type option() :: - {:keep_user_agent, boolean} - | {:max_read_duration, :timer.time() | :infinity} + {:max_read_duration, :timer.time() | :infinity} | {:max_body_length, non_neg_integer() | :infinity} | {:failed_request_ttl, :timer.time() | :infinity} | {:http, []} @@ -291,17 +287,13 @@ defp build_req_range_or_encoding_header(headers, _opts) do end end - defp build_req_user_agent_header(headers, opts) do - if Keyword.get(opts, :keep_user_agent, false) do - List.keystore( - headers, - "user-agent", - 0, - {"user-agent", Pleroma.Application.user_agent()} - ) - else - headers - end + defp build_req_user_agent_header(headers, _opts) do + List.keystore( + headers, + "user-agent", + 0, + {"user-agent", Pleroma.Application.user_agent()} + ) end defp build_resp_headers(headers, opts) do diff --git a/test/pleroma/reverse_proxy_test.exs b/test/pleroma/reverse_proxy_test.exs index 499d29c06..863e0c50d 100644 --- a/test/pleroma/reverse_proxy_test.exs +++ b/test/pleroma/reverse_proxy_test.exs @@ -18,24 +18,23 @@ defmodule Pleroma.ReverseProxyTest do setup :verify_on_exit! - defp user_agent_mock(user_agent, invokes) do - json = Jason.encode!(%{"user-agent": user_agent}) - + defp user_agent_mock(invokes) do ClientMock - |> expect(:request, fn :get, url, _, _, _ -> + |> expect(:request, fn :get, url, headers, _body, _opts -> Registry.register(ClientMock, url, 0) + body = headers |> Enum.into(%{}) |> Jason.encode!() {:ok, 200, [ {"content-type", "application/json"}, - {"content-length", byte_size(json) |> to_string()} - ], %{url: url}} + {"content-length", byte_size(body) |> to_string()} + ], %{url: url, body: body}} end) - |> expect(:stream_body, invokes, fn %{url: url} = client -> + |> expect(:stream_body, invokes, fn %{url: url, body: body} = client -> case Registry.lookup(ClientMock, url) do [{_, 0}] -> Registry.update_value(ClientMock, url, &(&1 + 1)) - {:ok, json, client} + {:ok, body, client} [{_, 1}] -> Registry.unregister(ClientMock, url) @@ -46,7 +45,7 @@ defp user_agent_mock(user_agent, invokes) do describe "reverse proxy" do test "do not track successful request", %{conn: conn} do - user_agent_mock("hackney/1.15.1", 2) + user_agent_mock(2) url = "/success" conn = ReverseProxy.call(conn, url) @@ -56,18 +55,15 @@ test "do not track successful request", %{conn: conn} do end end - describe "user-agent" do - test "don't keep", %{conn: conn} do - user_agent_mock("hackney/1.15.1", 2) - conn = ReverseProxy.call(conn, "/user-agent") - assert json_response(conn, 200) == %{"user-agent" => "hackney/1.15.1"} - end + test "use Pleroma's user agent in the request; don't pass the client's", %{conn: conn} do + user_agent_mock(2) - test "keep", %{conn: conn} do - user_agent_mock(Pleroma.Application.user_agent(), 2) - conn = ReverseProxy.call(conn, "/user-agent-keep", keep_user_agent: true) - assert json_response(conn, 200) == %{"user-agent" => Pleroma.Application.user_agent()} - end + conn = + conn + |> Plug.Conn.put_req_header("user-agent", "fake/1.0") + |> ReverseProxy.call("/user-agent") + + assert json_response(conn, 200) == %{"user-agent" => Pleroma.Application.user_agent()} end test "closed connection", %{conn: conn} do @@ -114,7 +110,7 @@ defp stream_mock(invokes, with_close? \\ false) do describe "max_body" do test "length returns error if content-length more than option", %{conn: conn} do - user_agent_mock("hackney/1.15.1", 0) + user_agent_mock(0) assert capture_log(fn -> ReverseProxy.call(conn, "/huge-file", max_body_length: 4) From 7ebbe11e7589bdabd1199954f07df05107fd6c41 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Wed, 24 Feb 2021 21:37:30 -0600 Subject: [PATCH 02/12] user_agent_mock --> request_mock --- test/pleroma/reverse_proxy_test.exs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/pleroma/reverse_proxy_test.exs b/test/pleroma/reverse_proxy_test.exs index 863e0c50d..a4dd8e99a 100644 --- a/test/pleroma/reverse_proxy_test.exs +++ b/test/pleroma/reverse_proxy_test.exs @@ -18,7 +18,7 @@ defmodule Pleroma.ReverseProxyTest do setup :verify_on_exit! - defp user_agent_mock(invokes) do + defp request_mock(invokes) do ClientMock |> expect(:request, fn :get, url, headers, _body, _opts -> Registry.register(ClientMock, url, 0) @@ -45,7 +45,7 @@ defp user_agent_mock(invokes) do describe "reverse proxy" do test "do not track successful request", %{conn: conn} do - user_agent_mock(2) + request_mock(2) url = "/success" conn = ReverseProxy.call(conn, url) @@ -56,7 +56,7 @@ test "do not track successful request", %{conn: conn} do end test "use Pleroma's user agent in the request; don't pass the client's", %{conn: conn} do - user_agent_mock(2) + request_mock(2) conn = conn @@ -110,7 +110,7 @@ defp stream_mock(invokes, with_close? \\ false) do describe "max_body" do test "length returns error if content-length more than option", %{conn: conn} do - user_agent_mock(0) + request_mock(0) assert capture_log(fn -> ReverseProxy.call(conn, "/huge-file", max_body_length: 4) From 808e15b26479a2ae4ac98f4ba293b570106c7140 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 22 Feb 2021 16:19:08 -0600 Subject: [PATCH 03/12] Document user agent leak fix --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 812816f48..ead45f990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased +### Security + +- Fixed client user agent leaking through MediaProxy + ### Removed - `:auth, :enforce_oauth_admin_scope_usage` configuration option. From 9f71b63c2d2c621352d12d1b854afb5beadede68 Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 1 Mar 2021 21:12:26 +0100 Subject: [PATCH 04/12] Update changelog --- CHANGELOG.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ead45f990..82189336f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). -## Unreleased +## [2.3.0] - 2020-03-01 ### Security @@ -103,9 +103,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Mastodon API: Support for expires_in/expires_at in the Filters. -## Unreleased (Patch) - - ## [2.2.2] - 2020-01-18 ### Fixed From cd6aa9bcae84499b165fb4be25f4caaac94c2548 Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 1 Mar 2021 21:13:43 +0100 Subject: [PATCH 05/12] Mix: Update version number --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index 50d4b4080..46b7746fc 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Pleroma.Mixfile do def project do [ app: :pleroma, - version: version("2.2.50"), + version: version("2.3.0"), elixir: "~> 1.9", elixirc_paths: elixirc_paths(Mix.env()), compilers: [:phoenix, :gettext] ++ Mix.compilers(), From 024c11c18d289d4acd65d749f939ad3684f31905 Mon Sep 17 00:00:00 2001 From: lain Date: Tue, 2 Mar 2021 14:40:47 +0100 Subject: [PATCH 06/12] StatusController: Deactivate application support for now. Some more things to discuss about, so we'll remove it from 2.3.0 --- .../controllers/status_controller.ex | 19 ++++++++++--------- .../controllers/status_controller_test.exs | 1 + 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index b051fca74..834222740 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -21,7 +21,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do alias Pleroma.Web.CommonAPI alias Pleroma.Web.MastodonAPI.AccountView alias Pleroma.Web.MastodonAPI.ScheduledActivityView - alias Pleroma.Web.OAuth.Token + # alias Pleroma.Web.OAuth.Token alias Pleroma.Web.Plugs.OAuthScopesPlug alias Pleroma.Web.Plugs.RateLimiter @@ -420,14 +420,15 @@ def bookmarks(%{assigns: %{user: user}} = conn, params) do ) end - defp put_application(params, %{assigns: %{token: %Token{user: %User{} = user} = token}} = _conn) do - if user.disclose_client do - %{client_name: client_name, website: website} = Repo.preload(token, :app).app - Map.put(params, :generator, %{type: "Application", name: client_name, url: website}) - else - Map.put(params, :generator, nil) - end - end + # Deactivated for 2.3.0 + # defp put_application(params, %{assigns: %{token: %Token{user: %User{} = user} = token}} = _conn) do + # if user.disclose_client do + # %{client_name: client_name, website: website} = Repo.preload(token, :app).app + # Map.put(params, :generator, %{type: "Application", name: client_name, url: website}) + # else + # Map.put(params, :generator, nil) + # end + # end defp put_application(params, _), do: Map.put(params, :generator, nil) end diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs index bd385bccd..e76c2760d 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -358,6 +358,7 @@ test "posting a direct status", %{conn: conn} do assert activity.data["cc"] == [] end + @tag :skip test "discloses application metadata when enabled" do user = insert(:user, disclose_client: true) %{user: _user, token: token, conn: conn} = oauth_access(["write:statuses"], user: user) From 7d790bb27b7f50a404aaaf23f4768227c3c46ae6 Mon Sep 17 00:00:00 2001 From: lain Date: Tue, 2 Mar 2021 14:42:11 +0100 Subject: [PATCH 07/12] Changelog: Remove application support line. --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82189336f..a55ebbf8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,7 +63,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Ability to define custom HTTP headers per each frontend - MRF (`NoEmptyPolicy`): New MRF Policy which will deny empty statuses or statuses of only mentions from being created by local users - New users will receive a simple email confirming their registration if no other emails will be dispatched. (e.g., Welcome, Confirmation, or Approval Required) -- The `application` metadata returned with statuses is no longer hardcoded. Apps that want to display these details will now have valid data for new posts after this change.
API Changes From 7dac83eb6e8b7bf47633e629870bced590639bbf Mon Sep 17 00:00:00 2001 From: lain Date: Tue, 2 Mar 2021 15:03:16 +0100 Subject: [PATCH 08/12] Linting. --- .../mastodon_api/controllers/status_controller.ex | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index 834222740..d1a58d5e1 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -421,13 +421,14 @@ def bookmarks(%{assigns: %{user: user}} = conn, params) do end # Deactivated for 2.3.0 - # defp put_application(params, %{assigns: %{token: %Token{user: %User{} = user} = token}} = _conn) do - # if user.disclose_client do - # %{client_name: client_name, website: website} = Repo.preload(token, :app).app - # Map.put(params, :generator, %{type: "Application", name: client_name, url: website}) - # else - # Map.put(params, :generator, nil) - # end + # defp put_application(params, + # %{assigns: %{token: %Token{user: %User{} = user} = token}} = _conn) do + # if user.disclose_client do + # %{client_name: client_name, website: website} = Repo.preload(token, :app).app + # Map.put(params, :generator, %{type: "Application", name: client_name, url: website}) + # else + # Map.put(params, :generator, nil) + # end # end defp put_application(params, _), do: Map.put(params, :generator, nil) From 0a589c887bd4215e7d443a34c194fd0a3bde8f72 Mon Sep 17 00:00:00 2001 From: lain Date: Tue, 2 Mar 2021 17:03:14 +0100 Subject: [PATCH 09/12] Mix: Update linkify. --- mix.exs | 2 +- mix.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mix.exs b/mix.exs index 46b7746fc..436381f32 100644 --- a/mix.exs +++ b/mix.exs @@ -157,7 +157,7 @@ defp deps do {:floki, "~> 0.27"}, {:timex, "~> 3.6"}, {:ueberauth, "~> 0.4"}, - {:linkify, "~> 0.4.1"}, + {:linkify, "~> 0.5.0"}, {:http_signatures, "~> 0.1.0"}, {:telemetry, "~> 0.3"}, {:poolboy, "~> 1.5"}, diff --git a/mix.lock b/mix.lock index 3e5631c72..99be81826 100644 --- a/mix.lock +++ b/mix.lock @@ -65,7 +65,7 @@ "jose": {:hex, :jose, "1.10.1", "16d8e460dae7203c6d1efa3f277e25b5af8b659febfc2f2eb4bacf87f128b80a", [:mix, :rebar3], [], "hexpm", "3c7ddc8a9394b92891db7c2771da94bf819834a1a4c92e30857b7d582e2f8257"}, "jumper": {:hex, :jumper, "1.0.1", "3c00542ef1a83532b72269fab9f0f0c82bf23a35e27d278bfd9ed0865cecabff", [:mix], [], "hexpm", "318c59078ac220e966d27af3646026db9b5a5e6703cb2aa3e26bcfaba65b7433"}, "libring": {:hex, :libring, "1.4.0", "41246ba2f3fbc76b3971f6bce83119dfec1eee17e977a48d8a9cfaaf58c2a8d6", [:mix], [], "hexpm"}, - "linkify": {:hex, :linkify, "0.4.1", "f881eb3429ae88010cf736e6fb3eed406c187bcdd544902ec937496636b7c7b3", [:mix], [], "hexpm", "ce98693f54ae9ace59f2f7a8aed3de2ef311381a8ce7794804bd75484c371dda"}, + "linkify": {:hex, :linkify, "0.5.0", "e0ea8de73ff44742d6a889721221f4c4eccaad5284957ee9832ffeb347602d54", [:mix], [], "hexpm", "4ccd958350aee7c51c89e21f05b15d30596ebbba707e051d21766be1809df2d7"}, "majic": {:git, "https://git.pleroma.social/pleroma/elixir-libraries/majic.git", "289cda1b6d0d70ccb2ba508a2b0bd24638db2880", [ref: "289cda1b6d0d70ccb2ba508a2b0bd24638db2880"]}, "makeup": {:hex, :makeup, "1.0.3", "e339e2f766d12e7260e6672dd4047405963c5ec99661abdc432e6ec67d29ef95", [:mix], [{:nimble_parsec, "~> 0.5", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "2e9b4996d11832947731f7608fed7ad2f9443011b3b479ae288011265cdd3dad"}, "makeup_elixir": {:hex, :makeup_elixir, "0.14.1", "4f0e96847c63c17841d42c08107405a005a2680eb9c7ccadfd757bd31dabccfb", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "f2438b1a80eaec9ede832b5c41cd4f373b38fd7aa33e3b22d9db79e640cbde11"}, From c5352e90be363f88f011ed5a63129caf3ee1a9fc Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 3 Mar 2021 13:56:40 +0100 Subject: [PATCH 10/12] Changelog, mix: merge in stable --- CHANGELOG.md | 4 ++++ mix.exs | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a55ebbf8a..40c423273 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## Unreleased + +## Unreleased (Patch) + ## [2.3.0] - 2020-03-01 ### Security diff --git a/mix.exs b/mix.exs index 436381f32..ec6e92df7 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Pleroma.Mixfile do def project do [ app: :pleroma, - version: version("2.3.0"), + version: version("2.3.50"), elixir: "~> 1.9", elixirc_paths: elixirc_paths(Mix.env()), compilers: [:phoenix, :gettext] ++ Mix.compilers(), From 2e296c079f0666a8239a0d3ce5b5fba6baf45a29 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 3 Mar 2021 15:33:06 +0100 Subject: [PATCH 11/12] Revert "StatusController: Deactivate application support for now." This reverts commit 024c11c18d289d4acd65d749f939ad3684f31905. --- .../controllers/status_controller.ex | 20 +++++++++---------- .../controllers/status_controller_test.exs | 1 - 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index d1a58d5e1..b051fca74 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -21,7 +21,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do alias Pleroma.Web.CommonAPI alias Pleroma.Web.MastodonAPI.AccountView alias Pleroma.Web.MastodonAPI.ScheduledActivityView - # alias Pleroma.Web.OAuth.Token + alias Pleroma.Web.OAuth.Token alias Pleroma.Web.Plugs.OAuthScopesPlug alias Pleroma.Web.Plugs.RateLimiter @@ -420,16 +420,14 @@ def bookmarks(%{assigns: %{user: user}} = conn, params) do ) end - # Deactivated for 2.3.0 - # defp put_application(params, - # %{assigns: %{token: %Token{user: %User{} = user} = token}} = _conn) do - # if user.disclose_client do - # %{client_name: client_name, website: website} = Repo.preload(token, :app).app - # Map.put(params, :generator, %{type: "Application", name: client_name, url: website}) - # else - # Map.put(params, :generator, nil) - # end - # end + defp put_application(params, %{assigns: %{token: %Token{user: %User{} = user} = token}} = _conn) do + if user.disclose_client do + %{client_name: client_name, website: website} = Repo.preload(token, :app).app + Map.put(params, :generator, %{type: "Application", name: client_name, url: website}) + else + Map.put(params, :generator, nil) + end + end defp put_application(params, _), do: Map.put(params, :generator, nil) end diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs index e76c2760d..bd385bccd 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -358,7 +358,6 @@ test "posting a direct status", %{conn: conn} do assert activity.data["cc"] == [] end - @tag :skip test "discloses application metadata when enabled" do user = insert(:user, disclose_client: true) %{user: _user, token: token, conn: conn} = oauth_access(["write:statuses"], user: user) From 10f402af6d0f088aa6ad8a3f26b5e226a2287634 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 3 Mar 2021 15:35:25 +0100 Subject: [PATCH 12/12] Changelog: Re-add application support --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40c423273..ed08701fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased +- The `application` metadata returned with statuses is no longer hardcoded. Apps that want to display these details will now have valid data for new posts after this change. + ## Unreleased (Patch) ## [2.3.0] - 2020-03-01