From 336e37d98f1b86c0332c9f260e27455a14714fa6 Mon Sep 17 00:00:00 2001 From: Ekaterina Vaartis Date: Fri, 21 Dec 2018 00:32:37 +0300 Subject: [PATCH] Make captcha (kocaptcha) stateless Also rename seconds_retained to seconds_valid since that's how it is now. Put it down from 180 to 20 seconds. The answer data is now stored in an encrypted text transfered to the client and back, so no ETS is needed --- config/config.exs | 2 +- docs/config.md | 2 +- lib/pleroma/captcha/captcha.ex | 29 +------ lib/pleroma/captcha/captcha_service.ex | 12 +-- lib/pleroma/captcha/kocaptcha.ex | 90 ++++++++++++---------- lib/pleroma/web/twitter_api/twitter_api.ex | 16 ++-- test/captcha_test.exs | 18 +++-- test/support/captcha_mock.ex | 5 +- 8 files changed, 82 insertions(+), 92 deletions(-) diff --git a/config/config.exs b/config/config.exs index 65d7346de..b8ef448b4 100644 --- a/config/config.exs +++ b/config/config.exs @@ -12,7 +12,7 @@ config :pleroma, Pleroma.Captcha, enabled: false, - seconds_retained: 180, + seconds_valid: 20, method: Pleroma.Captcha.Kocaptcha config :pleroma, Pleroma.Captcha.Kocaptcha, endpoint: "https://captcha.kotobank.ch" diff --git a/docs/config.md b/docs/config.md index c35769f21..cfeca8eb4 100644 --- a/docs/config.md +++ b/docs/config.md @@ -168,7 +168,7 @@ Web Push Notifications configuration. You can use the mix task `mix web_push.gen ## Pleroma.Captcha * `enabled`: Whether the captcha should be shown on registration * `method`: The method/service to use for captcha -* `seconds_retained`: The time in seconds for which the captcha is valid (stored in the cache) +* `seconds_valid`: The time in seconds for which the captcha is valid ### Pleroma.Captcha.Kocaptcha Kocaptcha is a very simple captcha service with a single API endpoint, diff --git a/lib/pleroma/captcha/captcha.ex b/lib/pleroma/captcha/captcha.ex index 5630f6b57..61a0f907f 100644 --- a/lib/pleroma/captcha/captcha.ex +++ b/lib/pleroma/captcha/captcha.ex @@ -1,8 +1,6 @@ defmodule Pleroma.Captcha do use GenServer - @ets_options [:ordered_set, :private, :named_table, {:read_concurrency, true}] - @doc false def start_link() do GenServer.start_link(__MODULE__, [], name: __MODULE__) @@ -10,14 +8,6 @@ def start_link() do @doc false def init(_) do - # Create a ETS table to store captchas - ets_name = Module.concat(method(), Ets) - ^ets_name = :ets.new(Module.concat(method(), Ets), @ets_options) - - # Clean up old captchas every few minutes - seconds_retained = Pleroma.Config.get!([__MODULE__, :seconds_retained]) - Process.send_after(self(), :cleanup, 1000 * seconds_retained) - {:ok, nil} end @@ -31,8 +21,8 @@ def new() do @doc """ Ask the configured captcha service to validate the captcha """ - def validate(token, captcha) do - GenServer.call(__MODULE__, {:validate, token, captcha}) + def validate(token, captcha, answer_data) do + GenServer.call(__MODULE__, {:validate, token, captcha, answer_data}) end @doc false @@ -47,19 +37,8 @@ def handle_call(:new, _from, state) do end @doc false - def handle_call({:validate, token, captcha}, _from, state) do - {:reply, method().validate(token, captcha), state} - end - - @doc false - def handle_info(:cleanup, state) do - :ok = method().cleanup() - - seconds_retained = Pleroma.Config.get!([__MODULE__, :seconds_retained]) - # Schedule the next clenup - Process.send_after(self(), :cleanup, 1000 * seconds_retained) - - {:noreply, state} + def handle_call({:validate, token, captcha, answer_data}, _from, state) do + {:reply, method().validate(token, captcha, answer_data), state} end defp method, do: Pleroma.Config.get!([__MODULE__, :method]) diff --git a/lib/pleroma/captcha/captcha_service.ex b/lib/pleroma/captcha/captcha_service.ex index 8d0b76f86..6f36d29b0 100644 --- a/lib/pleroma/captcha/captcha_service.ex +++ b/lib/pleroma/captcha/captcha_service.ex @@ -14,15 +14,15 @@ defmodule Pleroma.Captcha.Service do Arguments: * `token` the captcha is associated with * `captcha` solution of the captcha to validate + * `answer_data` is the data needed to validate the answer (presumably encrypted) Returns: `true` if captcha is valid, `false` if not """ - @callback validate(token :: String.t(), captcha :: String.t()) :: boolean - - @doc """ - This function is called periodically to clean up old captchas - """ - @callback cleanup() :: :ok + @callback validate( + token :: String.t(), + captcha :: String.t(), + answer_data :: String.t() + ) :: :ok | {:error, String.t()} end diff --git a/lib/pleroma/captcha/kocaptcha.ex b/lib/pleroma/captcha/kocaptcha.ex index 51900d123..f881c7b65 100644 --- a/lib/pleroma/captcha/kocaptcha.ex +++ b/lib/pleroma/captcha/kocaptcha.ex @@ -1,11 +1,11 @@ defmodule Pleroma.Captcha.Kocaptcha do + alias Plug.Crypto.KeyGenerator + alias Plug.Crypto.MessageEncryptor alias Calendar.DateTime alias Pleroma.Captcha.Service @behaviour Service - @ets __MODULE__.Ets - @impl Service def new() do endpoint = Pleroma.Config.get!([__MODULE__, :endpoint]) @@ -18,50 +18,56 @@ def new() do json_resp = Poison.decode!(res.body) token = json_resp["token"] + answer_md5 = json_resp["md5"] - true = - :ets.insert( - @ets, - {token, json_resp["md5"], DateTime.now_utc() |> DateTime.Format.unix()} - ) + secret_key_base = Pleroma.Config.get!([Pleroma.Web.Endpoint, :secret_key_base]) - %{type: :kocaptcha, token: token, url: endpoint <> json_resp["url"]} - end - end + # This make salt a little different for two keys + secret = KeyGenerator.generate(secret_key_base, token <> "_encrypt") + sign_secret = KeyGenerator.generate(secret_key_base, token <> "_sign") + # Basicallty copy what Phoenix.Token does here, add the time to + # the actual data and make it a binary to then encrypt it + encrypted_captcha_answer = + %{ + at: DateTime.now_utc(), + answer_md5: answer_md5 + } + |> :erlang.term_to_binary() + |> MessageEncryptor.encrypt(secret, sign_secret) - @impl Service - def validate(token, captcha) do - with false <- is_nil(captcha), - [{^token, saved_md5, _}] <- :ets.lookup(@ets, token), - true <- :crypto.hash(:md5, captcha) |> Base.encode16() == String.upcase(saved_md5) do - # Clear the saved value - :ets.delete(@ets, token) - - true - else - _ -> false - end - end - - @impl Service - def cleanup() do - seconds_retained = Pleroma.Config.get!([Pleroma.Captcha, :seconds_retained]) - # If the time in ETS is less than current_time - seconds_retained, then the time has - # already passed - delete_after = - DateTime.subtract!(DateTime.now_utc(), seconds_retained) |> DateTime.Format.unix() - - :ets.select_delete( - @ets, - [ - { - {:_, :_, :"$1"}, - [{:<, :"$1", {:const, delete_after}}], - [true] + %{ + type: :kocaptcha, + token: token, + url: endpoint <> json_resp["url"], + answer_data: encrypted_captcha_answer } - ] - ) + end + end - :ok + @impl Service + def validate(token, captcha, answer_data) do + secret_key_base = Pleroma.Config.get!([Pleroma.Web.Endpoint, :secret_key_base]) + secret = KeyGenerator.generate(secret_key_base, token <> "_encrypt") + sign_secret = KeyGenerator.generate(secret_key_base, token <> "_sign") + + # If the time found is less than (current_time - seconds_valid), then the time has already passed. + # Later we check that the time found is more than the presumed invalidatation time, that means + # that the data is still valid and the captcha can be checked + seconds_valid = Pleroma.Config.get!([Pleroma.Captcha, :seconds_valid]) + valid_if_after = DateTime.subtract!(DateTime.now_utc(), seconds_valid) + + with {:ok, data} <- MessageEncryptor.decrypt(answer_data, secret, sign_secret), + %{at: at, answer_md5: answer_md5} <- :erlang.binary_to_term(data) do + if DateTime.after?(at, valid_if_after) do + if not is_nil(captcha) and + :crypto.hash(:md5, captcha) |> Base.encode16() == String.upcase(answer_md5), + do: :ok, + else: {:error, "Invalid CAPTCHA"} + else + {:error, "CAPTCHA expired"} + end + else + _ -> {:error, "Invalid answer data"} + end end end diff --git a/lib/pleroma/web/twitter_api/twitter_api.ex b/lib/pleroma/web/twitter_api/twitter_api.ex index d816dc3bc..9e15f2c33 100644 --- a/lib/pleroma/web/twitter_api/twitter_api.ex +++ b/lib/pleroma/web/twitter_api/twitter_api.ex @@ -136,22 +136,28 @@ def register_user(params) do password: params["password"], password_confirmation: params["confirm"], captcha_solution: params["captcha_solution"], - captcha_token: params["captcha_token"] + captcha_token: params["captcha_token"], + captcha_answer_data: params["captcha_answer_data"] } captcha_enabled = Pleroma.Config.get([Pleroma.Captcha, :enabled]) # true if captcha is disabled or enabled and valid, false otherwise captcha_ok = if !captcha_enabled do - true + :ok else - Pleroma.Captcha.validate(params[:captcha_token], params[:captcha_solution]) + Pleroma.Captcha.validate( + params[:captcha_token], + params[:captcha_solution], + params[:captcha_answer_data] + ) end # Captcha invalid - if not captcha_ok do + if captcha_ok != :ok do + {:error, error} = captcha_ok # I have no idea how this error handling works - {:error, %{error: Jason.encode!(%{captcha: ["Invalid CAPTCHA"]})}} + {:error, %{error: Jason.encode!(%{captcha: [error]})}} else registrations_open = Pleroma.Config.get([:instance, :registrations_open]) diff --git a/test/captcha_test.exs b/test/captcha_test.exs index 54ffbd92f..93b8930da 100644 --- a/test/captcha_test.exs +++ b/test/captcha_test.exs @@ -25,16 +25,18 @@ defmodule Pleroma.CaptchaTest do end test "new and validate" do - assert Kocaptcha.new() == %{ - type: :kocaptcha, - token: "afa1815e14e29355e6c8f6b143a39fa2", - url: "https://captcha.kotobank.ch/captchas/afa1815e14e29355e6c8f6b143a39fa2.png" - } + new = Kocaptcha.new() + assert new[:type] == :kocaptcha + assert new[:token] == "afa1815e14e29355e6c8f6b143a39fa2" + + assert new[:url] == + "https://captcha.kotobank.ch/captchas/afa1815e14e29355e6c8f6b143a39fa2.png" assert Kocaptcha.validate( - "afa1815e14e29355e6c8f6b143a39fa2", - "7oEy8c" - ) + new[:token], + "7oEy8c", + new[:answer_data] + ) == :ok end end end diff --git a/test/support/captcha_mock.ex b/test/support/captcha_mock.ex index 898aa17b8..410318dc4 100644 --- a/test/support/captcha_mock.ex +++ b/test/support/captcha_mock.ex @@ -6,8 +6,5 @@ defmodule Pleroma.Captcha.Mock do def new(), do: %{type: :mock} @impl Service - def validate(_token, _captcha), do: true - - @impl Service - def cleanup(), do: :ok + def validate(_token, _captcha, _data), do: :ok end