From 6f2efb1c450daa75d848dab8e3729ced81ed3904 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 27 Feb 2020 18:46:05 +0300 Subject: Runtime configurability of RateLimiter. Refactoring. Disabled default rate limits in tests. --- lib/pleroma/plugs/rate_limiter/rate_limiter.ex | 162 ++++++++++++++----------- 1 file changed, 89 insertions(+), 73 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/plugs/rate_limiter/rate_limiter.ex b/lib/pleroma/plugs/rate_limiter/rate_limiter.ex index 7fb92489c..d2067060d 100644 --- a/lib/pleroma/plugs/rate_limiter/rate_limiter.ex +++ b/lib/pleroma/plugs/rate_limiter/rate_limiter.ex @@ -7,12 +7,14 @@ defmodule Pleroma.Plugs.RateLimiter do ## Configuration - A keyword list of rate limiters where a key is a limiter name and value is the limiter configuration. The basic configuration is a tuple where: + A keyword list of rate limiters where a key is a limiter name and value is the limiter configuration. + The basic configuration is a tuple where: * The first element: `scale` (Integer). The time scale in milliseconds. * The second element: `limit` (Integer). How many requests to limit in the time scale provided. - It is also possible to have different limits for unauthenticated and authenticated users: the keyword value must be a list of two tuples where the first one is a config for unauthenticated users and the second one is for authenticated. + It is also possible to have different limits for unauthenticated and authenticated users: the keyword value must be a + list of two tuples where the first one is a config for unauthenticated users and the second one is for authenticated. To disable a limiter set its value to `nil`. @@ -64,91 +66,100 @@ defmodule Pleroma.Plugs.RateLimiter do import Pleroma.Web.TranslationHelpers import Plug.Conn + alias Pleroma.Config alias Pleroma.Plugs.RateLimiter.LimiterSupervisor alias Pleroma.User require Logger - def init(opts) do - limiter_name = Keyword.get(opts, :name) - - case Pleroma.Config.get([:rate_limit, limiter_name]) do - nil -> - nil - - config -> - name_root = Keyword.get(opts, :bucket_name, limiter_name) + @doc false + def init(plug_opts) do + plug_opts + end - %{ - name: name_root, - limits: config, - opts: opts - } + def call(conn, plug_opts) do + if disabled?() do + handle_disabled(conn) + else + action_settings = action_settings(plug_opts) + handle(conn, action_settings) end end - # Do not limit if there is no limiter configuration - def call(conn, nil), do: conn + defp handle_disabled(conn) do + if Config.get(:env) == :prod do + Logger.warn("Rate limiter is disabled for localhost/socket") + end + + conn + end - def call(conn, settings) do - case disabled?() do - true -> - if Pleroma.Config.get(:env) == :prod, - do: Logger.warn("Rate limiter is disabled for localhost/socket") + defp handle(conn, nil), do: conn + defp handle(conn, action_settings) do + action_settings + |> incorporate_conn_info(conn) + |> check_rate() + |> case do + {:ok, _count} -> conn - false -> - settings - |> incorporate_conn_info(conn) - |> check_rate() - |> case do - {:ok, _count} -> - conn - - {:error, _count} -> - render_throttled_error(conn) - end + {:error, _count} -> + render_throttled_error(conn) end end def disabled? do localhost_or_socket = - Pleroma.Config.get([Pleroma.Web.Endpoint, :http, :ip]) + Config.get([Pleroma.Web.Endpoint, :http, :ip]) |> Tuple.to_list() |> Enum.join(".") |> String.match?(~r/^local|^127.0.0.1/) - remote_ip_disabled = not Pleroma.Config.get([Pleroma.Plugs.RemoteIp, :enabled]) + remote_ip_disabled = not Config.get([Pleroma.Plugs.RemoteIp, :enabled]) localhost_or_socket and remote_ip_disabled end - def inspect_bucket(conn, name_root, settings) do - settings = - settings - |> incorporate_conn_info(conn) + def inspect_bucket(conn, bucket_name_root, plug_opts) do + with %{name: _} = action_settings <- action_settings(plug_opts) do + action_settings = incorporate_conn_info(action_settings, conn) + bucket_name = make_bucket_name(%{action_settings | name: bucket_name_root}) + key_name = make_key_name(action_settings) + limit = get_limits(action_settings) - bucket_name = make_bucket_name(%{settings | name: name_root}) - key_name = make_key_name(settings) - limit = get_limits(settings) + case Cachex.get(bucket_name, key_name) do + {:error, :no_cache} -> + {:err, :not_found} - case Cachex.get(bucket_name, key_name) do - {:error, :no_cache} -> - {:err, :not_found} + {:ok, nil} -> + {0, limit} + + {:ok, value} -> + {value, limit - value} + end + else + _ -> {:err, :not_found} + end + end - {:ok, nil} -> - {0, limit} + def action_settings(plug_opts) do + with limiter_name when not is_nil(limiter_name) <- plug_opts[:name], + limits when not is_nil(limits) <- Config.get([:rate_limit, limiter_name]) do + bucket_name_root = Keyword.get(plug_opts, :bucket_name, limiter_name) - {:ok, value} -> - {value, limit - value} + %{ + name: bucket_name_root, + limits: limits, + opts: plug_opts + } end end - defp check_rate(settings) do - bucket_name = make_bucket_name(settings) - key_name = make_key_name(settings) - limit = get_limits(settings) + defp check_rate(action_settings) do + bucket_name = make_bucket_name(action_settings) + key_name = make_key_name(action_settings) + limit = get_limits(action_settings) case Cachex.get_and_update(bucket_name, key_name, &increment_value(&1, limit)) do {:commit, value} -> @@ -158,8 +169,8 @@ defmodule Pleroma.Plugs.RateLimiter do {:error, value} {:error, :no_cache} -> - initialize_buckets(settings) - check_rate(settings) + initialize_buckets(action_settings) + check_rate(action_settings) end end @@ -169,16 +180,19 @@ defmodule Pleroma.Plugs.RateLimiter do defp increment_value(val, _limit), do: {:commit, val + 1} - defp incorporate_conn_info(settings, %{assigns: %{user: %User{id: user_id}}, params: params}) do - Map.merge(settings, %{ + defp incorporate_conn_info(action_settings, %{ + assigns: %{user: %User{id: user_id}}, + params: params + }) do + Map.merge(action_settings, %{ mode: :user, conn_params: params, conn_info: "#{user_id}" }) end - defp incorporate_conn_info(settings, %{params: params} = conn) do - Map.merge(settings, %{ + defp incorporate_conn_info(action_settings, %{params: params} = conn) do + Map.merge(action_settings, %{ mode: :anon, conn_params: params, conn_info: "#{ip(conn)}" @@ -197,10 +211,10 @@ defmodule Pleroma.Plugs.RateLimiter do |> halt() end - defp make_key_name(settings) do + defp make_key_name(action_settings) do "" - |> attach_params(settings) - |> attach_identity(settings) + |> attach_selected_params(action_settings) + |> attach_identity(action_settings) end defp get_scale(_, {scale, _}), do: scale @@ -215,21 +229,23 @@ defmodule Pleroma.Plugs.RateLimiter do defp get_limits(%{limits: [{_, limit}, _]}), do: limit - defp make_bucket_name(%{mode: :user, name: name_root}), - do: user_bucket_name(name_root) + defp make_bucket_name(%{mode: :user, name: bucket_name_root}), + do: user_bucket_name(bucket_name_root) - defp make_bucket_name(%{mode: :anon, name: name_root}), - do: anon_bucket_name(name_root) + defp make_bucket_name(%{mode: :anon, name: bucket_name_root}), + do: anon_bucket_name(bucket_name_root) - defp attach_params(input, %{conn_params: conn_params, opts: opts}) do - param_string = - opts + defp attach_selected_params(input, %{conn_params: conn_params, opts: plug_opts}) do + params_string = + plug_opts |> Keyword.get(:params, []) |> Enum.sort() |> Enum.map(&Map.get(conn_params, &1, "")) |> Enum.join(":") - "#{input}#{param_string}" + [input, params_string] + |> Enum.join(":") + |> String.replace_leading(":", "") end defp initialize_buckets(%{name: _name, limits: nil}), do: :ok @@ -245,6 +261,6 @@ defmodule Pleroma.Plugs.RateLimiter do defp attach_identity(base, %{mode: :anon, conn_info: conn_info}), do: "ip:#{base}:#{conn_info}" - defp user_bucket_name(name_root), do: "user:#{name_root}" |> String.to_atom() - defp anon_bucket_name(name_root), do: "anon:#{name_root}" |> String.to_atom() + defp user_bucket_name(bucket_name_root), do: "user:#{bucket_name_root}" |> String.to_atom() + defp anon_bucket_name(bucket_name_root), do: "anon:#{bucket_name_root}" |> String.to_atom() end -- cgit v1.2.3 From 3759b146c4332f4026370fd1292085fbbb92d536 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Fri, 28 Feb 2020 13:33:42 +0000 Subject: Apply suggestion to lib/pleroma/plugs/rate_limiter/rate_limiter.ex --- lib/pleroma/plugs/rate_limiter/rate_limiter.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/plugs/rate_limiter/rate_limiter.ex b/lib/pleroma/plugs/rate_limiter/rate_limiter.ex index d2067060d..3a27d6eb7 100644 --- a/lib/pleroma/plugs/rate_limiter/rate_limiter.ex +++ b/lib/pleroma/plugs/rate_limiter/rate_limiter.ex @@ -144,7 +144,7 @@ defmodule Pleroma.Plugs.RateLimiter do end def action_settings(plug_opts) do - with limiter_name when not is_nil(limiter_name) <- plug_opts[:name], + with limiter_name when is_atom(limiter_name) <- plug_opts[:name], limits when not is_nil(limits) <- Config.get([:rate_limit, limiter_name]) do bucket_name_root = Keyword.get(plug_opts, :bucket_name, limiter_name) -- cgit v1.2.3 From c747260989fdba32a8f319f88f0840c811ff8b50 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sat, 29 Feb 2020 22:04:09 +0300 Subject: [#2250] Tiny refactoring per merge request review. --- lib/pleroma/plugs/rate_limiter/rate_limiter.ex | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/plugs/rate_limiter/rate_limiter.ex b/lib/pleroma/plugs/rate_limiter/rate_limiter.ex index 3a27d6eb7..9c362a392 100644 --- a/lib/pleroma/plugs/rate_limiter/rate_limiter.ex +++ b/lib/pleroma/plugs/rate_limiter/rate_limiter.ex @@ -121,6 +121,8 @@ defmodule Pleroma.Plugs.RateLimiter do localhost_or_socket and remote_ip_disabled end + @inspect_bucket_not_found {:error, :not_found} + def inspect_bucket(conn, bucket_name_root, plug_opts) do with %{name: _} = action_settings <- action_settings(plug_opts) do action_settings = incorporate_conn_info(action_settings, conn) @@ -130,7 +132,7 @@ defmodule Pleroma.Plugs.RateLimiter do case Cachex.get(bucket_name, key_name) do {:error, :no_cache} -> - {:err, :not_found} + @inspect_bucket_not_found {:ok, nil} -> {0, limit} @@ -139,7 +141,7 @@ defmodule Pleroma.Plugs.RateLimiter do {value, limit - value} end else - _ -> {:err, :not_found} + _ -> @inspect_bucket_not_found end end -- cgit v1.2.3 From df2173343accec7a7a311d85df2f13d5141b7bc7 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Fri, 28 Feb 2020 17:29:53 +0300 Subject: pagination: limit the number of elements returned at one time to 40 --- lib/pleroma/pagination.ex | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/pagination.ex b/lib/pleroma/pagination.ex index 4535ca7c5..43fb7babf 100644 --- a/lib/pleroma/pagination.ex +++ b/lib/pleroma/pagination.ex @@ -13,6 +13,7 @@ defmodule Pleroma.Pagination do alias Pleroma.Repo @default_limit 20 + @max_limit 40 @page_keys ["max_id", "min_id", "limit", "since_id", "order"] def page_keys, do: @page_keys @@ -130,7 +131,11 @@ defmodule Pleroma.Pagination do end defp restrict(query, :limit, options, _table_binding) do - limit = Map.get(options, :limit, @default_limit) + limit = + case Map.get(options, :limit, @default_limit) do + limit when limit < @max_limit -> limit + _ -> @max_limit + end query |> limit(^limit) -- cgit v1.2.3 From 4d416343fae4a9e0b1654b12bd476017be63a7e9 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Fri, 28 Feb 2020 17:35:01 +0300 Subject: rate limiter: Fix a race condition When multiple requests are processed by rate limiter plug at the same time and the bucket is not yet initialized, both would try to initialize the bucket resulting in an internal server error. --- lib/pleroma/plugs/rate_limiter/limiter_supervisor.ex | 10 ++++++++-- lib/pleroma/plugs/rate_limiter/rate_limiter.ex | 15 ++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/plugs/rate_limiter/limiter_supervisor.ex b/lib/pleroma/plugs/rate_limiter/limiter_supervisor.ex index 187582ede..884268d96 100644 --- a/lib/pleroma/plugs/rate_limiter/limiter_supervisor.ex +++ b/lib/pleroma/plugs/rate_limiter/limiter_supervisor.ex @@ -7,8 +7,8 @@ defmodule Pleroma.Plugs.RateLimiter.LimiterSupervisor do DynamicSupervisor.start_link(__MODULE__, init_arg, name: __MODULE__) end - def add_limiter(limiter_name, expiration) do - {:ok, _pid} = + def add_or_return_limiter(limiter_name, expiration) do + result = DynamicSupervisor.start_child( __MODULE__, %{ @@ -28,6 +28,12 @@ defmodule Pleroma.Plugs.RateLimiter.LimiterSupervisor do ]} } ) + + case result do + {:ok, _pid} = result -> result + {:error, {:already_started, pid}} -> {:ok, pid} + _ -> result + end end @impl true diff --git a/lib/pleroma/plugs/rate_limiter/rate_limiter.ex b/lib/pleroma/plugs/rate_limiter/rate_limiter.ex index 9c362a392..b9cbe9716 100644 --- a/lib/pleroma/plugs/rate_limiter/rate_limiter.ex +++ b/lib/pleroma/plugs/rate_limiter/rate_limiter.ex @@ -171,7 +171,7 @@ defmodule Pleroma.Plugs.RateLimiter do {:error, value} {:error, :no_cache} -> - initialize_buckets(action_settings) + initialize_buckets!(action_settings) check_rate(action_settings) end end @@ -250,11 +250,16 @@ defmodule Pleroma.Plugs.RateLimiter do |> String.replace_leading(":", "") end - defp initialize_buckets(%{name: _name, limits: nil}), do: :ok + defp initialize_buckets!(%{name: _name, limits: nil}), do: :ok - defp initialize_buckets(%{name: name, limits: limits}) do - LimiterSupervisor.add_limiter(anon_bucket_name(name), get_scale(:anon, limits)) - LimiterSupervisor.add_limiter(user_bucket_name(name), get_scale(:user, limits)) + defp initialize_buckets!(%{name: name, limits: limits}) do + {:ok, _pid} = + LimiterSupervisor.add_or_return_limiter(anon_bucket_name(name), get_scale(:anon, limits)) + + {:ok, _pid} = + LimiterSupervisor.add_or_return_limiter(user_bucket_name(name), get_scale(:user, limits)) + + :ok end defp attach_identity(base, %{mode: :user, conn_info: conn_info}), -- cgit v1.2.3 From ffcebe7e22b4c5ccaf3ba63f3ed2885ac55a6b4d Mon Sep 17 00:00:00 2001 From: rinpatch Date: Fri, 28 Feb 2020 17:44:59 +0300 Subject: timeline controller: rate limit timelines to 3 requests per 500ms per timeline per ip/user --- .../web/mastodon_api/controllers/timeline_controller.ex | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'lib') diff --git a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex index 29964a1d4..f58c1f93c 100644 --- a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex @@ -10,9 +10,20 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do alias Pleroma.Pagination alias Pleroma.Plugs.OAuthScopesPlug + alias Pleroma.Plugs.RateLimiter alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub + # XXX: Ideally these would be generated instead of copypasted, + # but I haven't been able to overcome an issue with guards when + # trying to generate these. + # See: https://elixirforum.com/t/trouble-plugging-plugs-with-generated-options-in-guards-in-a-phoenix-controller/29465 + plug(RateLimiter, [name: :timeline, bucket_name: :direct_timeline] when action == :direct) + plug(RateLimiter, [name: :timeline, bucket_name: :public_timeline] when action == :public) + plug(RateLimiter, [name: :timeline, bucket_name: :home_timeline] when action == :home) + plug(RateLimiter, [name: :timeline, bucket_name: :hashtag_timeline] when action == :hashtag) + plug(RateLimiter, [name: :timeline, bucket_name: :list_timeline] when action == :list) + plug(OAuthScopesPlug, %{scopes: ["read:statuses"]} when action in [:home, :direct]) plug(OAuthScopesPlug, %{scopes: ["read:lists"]} when action == :list) -- cgit v1.2.3 From b5465bf385800d52998bca472a19ea1b9db4c252 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sun, 1 Mar 2020 02:03:46 +0300 Subject: timeline controller: add a TODO for replacing copypaste with a macro --- lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex index f58c1f93c..a3110c722 100644 --- a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex @@ -14,10 +14,10 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub - # XXX: Ideally these would be generated instead of copypasted, - # but I haven't been able to overcome an issue with guards when - # trying to generate these. - # See: https://elixirforum.com/t/trouble-plugging-plugs-with-generated-options-in-guards-in-a-phoenix-controller/29465 + # TODO: Replace with a macro when there is a Phoenix release with + # https://github.com/phoenixframework/phoenix/commit/2e8c63c01fec4dde5467dbbbf9705ff9e780735e + # in it + plug(RateLimiter, [name: :timeline, bucket_name: :direct_timeline] when action == :direct) plug(RateLimiter, [name: :timeline, bucket_name: :public_timeline] when action == :public) plug(RateLimiter, [name: :timeline, bucket_name: :home_timeline] when action == :home) -- cgit v1.2.3