From 2ef8605d90f81300163ef8691b080ceef3af15e0 Mon Sep 17 00:00:00 2001 From: Stas Date: Wed, 3 Apr 2024 13:24:10 +0200 Subject: [PATCH 1/2] feat: encrypt secrets for internal use --- VERSION | 2 +- bench/enc_dec.exs | 57 +++++++++++++++++++++++++++++ lib/supavisor.ex | 6 +-- lib/supavisor/client_handler.ex | 31 ++++++++++------ lib/supavisor/db_handler.ex | 14 +++---- lib/supavisor/helpers.ex | 37 +++++++++++++++++++ lib/supavisor/hot_upgrade.ex | 29 ++++++++++----- test/supavisor/db_handler_test.exs | 5 ++- test/supavisor/syn_handler_test.exs | 3 +- test/support/fixtures/helpers.ex | 4 +- 10 files changed, 153 insertions(+), 35 deletions(-) create mode 100644 bench/enc_dec.exs diff --git a/VERSION b/VERSION index d388d619..da44c7f3 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.1.47 +1.1.50 diff --git a/bench/enc_dec.exs b/bench/enc_dec.exs new file mode 100644 index 00000000..9bb29944 --- /dev/null +++ b/bench/enc_dec.exs @@ -0,0 +1,57 @@ +alias Supavisor.Helpers, as: H + +map = %{:a => 1, "key" => "value", "ключ" => "значення"} +text = "big_secret" +map_enc = <<1, 10, 65, 69, 83, 46, 71, 67, 77, 46, 86, 49, 187, 130, 166, 189, 94, 38, 109, 83, 9, 109, 228, 202, 104, 33, 49, 31, 105, 142, 219, 106, 114, 21, 167, 60, 192, 204, 56, 9, 111, 223, 208, 47, 247, 74, 178, 192, 209, 153, 54, 23, 150, 155, 82, 180, 188, 2, 87, 189, 180, 225, 66, 243, 111, 73, 73, 199, 2, 240, 51, 227, 168, 202, 197, 247, 210, 197, 100, 67, 230, 32, 175, 110, 173, 21, 219, 110, 244, 166, 79, 70, 200, 75, 206, 222, 116, 177, 243, 239, 141, 80, 6, 33, 250, 188, 92, 73>> +text_enc = <<1, 10, 65, 69, 83, 46, 71, 67, 77, 46, 86, 49, 79, 49, 25, 197, 18, 255, 27, 3, 45, 198, 65, 15, 230, 155, 246, 84, 13, 125, 122, 178, 51, 203, 103, 149, 86, 117, 61, 106, 220, 97, 155, 204, 118, 20, 217, 71, 15, 250, 43, 171, 6, 68, 250, 58, 215, 45, 0, 60>> + +Benchee.run(%{ + "encode_secret map" => fn -> + H.encode_secret(map) + end, + "encode_secret text" => fn -> + H.encode_secret(text) + end, + "decode_secret map" => fn -> + H.decode_secret(map_enc) + end, + "decode_secret text" => fn -> + H.decode_secret(text_enc) + end +}) + + +# $ VAULT_ENC_KEY="aHD8DZRdk2emnkdktFZRh3E9RNg4aOY7" mix run bench/enc_dec.exs + +# Operating System: macOS +# CPU Information: Apple M1 Pro +# Number of Available Cores: 10 +# Available memory: 16 GB +# Elixir 1.14.3 +# Erlang 24.3.4 + +# Benchmark suite executing with the following configuration: +# warmup: 2 s +# time: 5 s +# memory time: 0 ns +# reduction time: 0 ns +# parallel: 1 +# inputs: none specified +# Estimated total run time: 28 s + +# Benchmarking decode_secret map ... +# Benchmarking decode_secret text ... +# Benchmarking encode_secret map ... +# Benchmarking encode_secret text ... + +# Name ips average deviation median 99th % +# decode_secret text 753.68 K 1.33 μs ±1293.76% 1.25 μs 1.50 μs +# encode_secret text 708.57 K 1.41 μs ±453.06% 1.33 μs 1.88 μs +# decode_secret map 692.11 K 1.44 μs ±945.34% 1.33 μs 1.67 μs +# encode_secret map 671.23 K 1.49 μs ±414.54% 1.42 μs 1.96 μs + +# Comparison: +# decode_secret text 753.68 K +# encode_secret text 708.57 K - 1.06x slower +0.0845 μs +# decode_secret map 692.11 K - 1.09x slower +0.118 μs +# encode_secret map 671.23 K - 1.12x slower +0.163 μs diff --git a/lib/supavisor.ex b/lib/supavisor.ex index cff685ea..6ccf0c9a 100644 --- a/lib/supavisor.ex +++ b/lib/supavisor.ex @@ -10,7 +10,7 @@ defmodule Supavisor do @type ssl_sock :: {:ssl, :ssl.sslsocket()} @type tcp_sock :: {:gen_tcp, :gen_tcp.socket()} @type workers :: %{manager: pid, pool: pid} - @type secrets :: {:password | :auth_query, fun()} + @type secrets :: {:password | :auth_query, binary()} @type mode :: :transaction | :session | :native @type id :: {{:single | :cluster, String.t()}, String.t(), mode, String.t()} @type subscribe_opts :: %{workers: workers, ps: list, idle_timeout: integer} @@ -250,7 +250,7 @@ defmodule Supavisor do def start_local_pool({{type, tenant}, _user, _mode, _db_name} = id, secrets, log_level \\ nil) do Logger.debug("Starting pool(s) for #{inspect(id)}") - user = elem(secrets, 1).().alias + user = H.decode_secret(elem(secrets, 1)).alias case type do :single -> T.get_pool_config(tenant, user) @@ -327,7 +327,7 @@ defmodule Supavisor do port: db_port, user: db_user, database: if(db_name != nil, do: db_name, else: db_database), - password: fn -> db_pass end, + password: H.encode_secret(db_pass), application_name: "Supavisor", ip_version: H.ip_version(ip_ver, db_host), upstream_ssl: tenant_record.upstream_ssl, diff --git a/lib/supavisor/client_handler.ex b/lib/supavisor/client_handler.ex index 76b94a1f..29e5eacb 100644 --- a/lib/supavisor/client_handler.ex +++ b/lib/supavisor/client_handler.ex @@ -287,7 +287,8 @@ defmodule Supavisor.ClientHandler do Cachex.get(Supavisor.Cache, key) == {:ok, nil} do case auth_secrets(info, data.user, key, 15_000) do {:ok, {method2, secrets2}} = value -> - if method != method2 || Map.delete(secrets.(), :client_key) != secrets2.() do + if method != method2 || + Map.delete(H.decode_secret(secrets), :client_key) != H.decode_secret(secrets2) do Logger.warning("ClientHandler: Update secrets and terminate pool") Cachex.update( @@ -315,9 +316,10 @@ defmodule Supavisor.ClientHandler do {:ok, client_key} -> secrets = if client_key do - fn -> - Map.put(secrets.(), :client_key, client_key) - end + secrets + |> H.decode_secret() + |> Map.put(:client_key, client_key) + |> H.encode_secret() else secrets end @@ -637,7 +639,8 @@ defmodule Supavisor.ClientHandler do tag: :password_message, payload: {:md5, client_md5} }, _} <- receive_next(socket, "Timeout while waiting for the md5 exchange"), - {:ok, key} <- authenticate_exchange(method, client_md5, secrets.().secret, salt) do + {:ok, key} <- + authenticate_exchange(method, client_md5, H.decode_secret(secrets).secret, salt) do {:ok, key} else {:error, message} -> {:error, message} @@ -708,7 +711,7 @@ defmodule Supavisor.ClientHandler do defp authenticate_exchange(:auth_query, secrets, signatures, p) do client_key = :crypto.exor(Base.decode64!(p), signatures.client) - if H.hash(client_key) == secrets.().stored_key do + if H.hash(client_key) == H.decode_secret(secrets).stored_key do {:ok, client_key} else {:error, "Wrong password"} @@ -791,7 +794,7 @@ defmodule Supavisor.ClientHandler do def auth_secrets(%{user: user, tenant: %{require_user: true}}, _, _, _) do secrets = %{db_user: user.db_user, password: user.db_password, alias: user.db_user_alias} - {:ok, {:password, fn -> secrets end}} + {:ok, {:password, H.encode_secret(secrets)}} end ## auth_query secrets @@ -810,7 +813,7 @@ defmodule Supavisor.ClientHandler do end end - @spec get_secrets(map, String.t()) :: {:ok, {:auth_query, fun()}} | {:error, term()} + @spec get_secrets(map, String.t()) :: {:ok, {:auth_query, binary()}} | {:error, term()} def get_secrets(%{user: user, tenant: tenant}, db_user) do ssl_opts = if tenant.upstream_ssl and tenant.upstream_verify == "peer" do @@ -843,7 +846,13 @@ defmodule Supavisor.ClientHandler do case H.get_user_secret(conn, tenant.auth_query, db_user) do {:ok, secret} -> t = if secret.digest == :md5, do: :auth_query_md5, else: :auth_query - {:ok, {t, fn -> Map.put(secret, :alias, user.db_user_alias) end}} + + enc = + secret + |> Map.put(:alias, user.db_user_alias) + |> H.encode_secret() + + {:ok, {t, enc}} {:error, reason} -> {:error, reason} @@ -862,7 +871,7 @@ defmodule Supavisor.ClientHandler do {client_final_message, server_proof} = H.get_client_final( :password, - secret.().password, + H.decode_secret(secret).password, server_first_parts, nonce, user, @@ -878,7 +887,7 @@ defmodule Supavisor.ClientHandler do end defp exchange_first(:auth_query, secret, nonce, user, channel) do - secret = secret.() + secret = H.decode_secret(secret) message = Server.exchange_first_message(nonce, secret.salt) server_first_parts = H.parse_server_first(message, nonce) diff --git a/lib/supavisor/db_handler.ex b/lib/supavisor/db_handler.ex index a5611268..4c62521d 100644 --- a/lib/supavisor/db_handler.ex +++ b/lib/supavisor/db_handler.ex @@ -181,10 +181,10 @@ defmodule Supavisor.DbHandler do {client_final_message, server_proof} = H.get_client_final( :auth_query, - data.auth.secrets.(), + H.decode_secret(data.auth.secrets), server_first_parts, nonce, - data.auth.secrets.().user, + H.decode_secret(data.auth.secrets).user, "biws" ) @@ -202,7 +202,7 @@ defmodule Supavisor.DbHandler do server_first_parts, nonce, data.auth.user, - data.auth.password.() + H.decode_secret(data.auth.password) ) bin = :pgo_protocol.encode_scram_response_message(client_final_message) @@ -218,9 +218,9 @@ defmodule Supavisor.DbHandler do digest = if data.auth.method == :password do - H.md5([data.auth.password.(), data.auth.user]) + H.md5([H.decode_secret(data.auth.password), data.auth.user]) else - data.auth.secrets.().secret + H.decode_secret(data.auth.secrets).secret end payload = ["md5", H.md5([digest, salt]), 0] @@ -546,9 +546,9 @@ defmodule Supavisor.DbHandler do defp get_user(auth) do if auth.require_user do - auth.secrets.().db_user + H.decode_secret(auth.secrets).db_user else - auth.secrets.().user + H.decode_secret(auth.secrets).user end end diff --git a/lib/supavisor/helpers.ex b/lib/supavisor/helpers.ex index bdb615c8..27d8f454 100644 --- a/lib/supavisor/helpers.ex +++ b/lib/supavisor/helpers.ex @@ -354,4 +354,41 @@ defmodule Supavisor.Helpers do Logger.notice("Setting log level to #{inspect(level)}") Logger.put_process_level(self(), level) end + + @doc """ + Encodes and Decodes a secret using the Vault encryption. + + ## Examples + + iex> Supavisor.Helpers.encode_secret("!#&%#.!@#DSFVАЯІЇQW") |> Supavisor.Helpers.decode_secret() + "!#&%#.!@#DSFVАЯІЇQW" + iex> Supavisor.Helpers.encode_secret(%{a: 1, b: 2}) |> Supavisor.Helpers.decode_secret() + %{a: 1, b: 2} + iex> Supavisor.Helpers.decode_secret(fn -> :secret end) + :secret + """ + + @spec encode_secret(term()) :: binary() + def encode_secret(term) do + term + |> :erlang.term_to_binary() + |> Supavisor.Vault.encrypt!() + end + + # for backward compatibility, will be removed in the future + @spec decode_secret(fun()) :: term() + def decode_secret(fun) when is_function(fun) do + Logger.warning( + "Using deprecated function `decode_secret/1` #{inspect(Process.info(self(), :current_stacktrace), pretty: true)}" + ) + + fun.() + end + + @spec decode_secret(binary()) :: term() + def decode_secret(encoded) do + encoded + |> Supavisor.Vault.decrypt!() + |> :erlang.binary_to_term() + end end diff --git a/lib/supavisor/hot_upgrade.ex b/lib/supavisor/hot_upgrade.ex index 5811c967..115fcdb3 100644 --- a/lib/supavisor/hot_upgrade.ex +++ b/lib/supavisor/hot_upgrade.ex @@ -5,6 +5,8 @@ defmodule Supavisor.HotUpgrade do import Cachex.Spec require Record + alias Supavisor.Helpers, as: H + Record.defrecord( :state, [ @@ -107,7 +109,8 @@ defmodule Supavisor.HotUpgrade do {:supervisor, :poolboy_sup, _}, Process.info(linked_pid)[:dictionary][:"$initial_call"] ), - state = :sys.get_state(linked_pid), + {status, state} = get_state(linked_pid), + match?(:ok, status), Record.is_record(state, :state), state(state, :module) == :poolboy_sup do :sys.replace_state(linked_pid, fn state -> @@ -116,8 +119,8 @@ defmodule Supavisor.HotUpgrade do args = Map.update!(args, :auth, fn auth -> - Map.put(auth, :password, enc(auth.password.())) - |> Map.put(:secrets, enc(auth.secrets.())) + Map.put(auth, :password, enc(auth.password)) + |> Map.put(:secrets, enc(auth.secrets)) end) {[^db_handler], %{^db_handler => child}} = state(state, :children) @@ -137,7 +140,7 @@ defmodule Supavisor.HotUpgrade do case value do {:cached, {:ok, {:auth_query, auth}}} when is_function(auth) -> Logger.debug("Reinitializing secret: #{inspect(key)}") - new = {:cached, {:ok, {:auth_query, enc(auth.())}}} + new = {:cached, {:ok, {:auth_query, enc(auth)}}} Cachex.put(Supavisor.Cache, key, new) other -> @@ -146,9 +149,17 @@ defmodule Supavisor.HotUpgrade do end) end - @spec enc(term) :: fun - def enc(val), do: apply(__MODULE__, :do_enc, [val]) - - @spec do_enc(term) :: fun - def do_enc(val), do: fn -> val end + @spec enc(term() | fun()) :: binary() + def enc(fun) when is_function(fun), do: H.encode_secret(fun.()) + def enc(val), do: val + + def get_state(pid) do + try do + {:ok, :sys.get_state(pid)} + catch + type, exception -> + IO.write("Error getting state: #{inspect(exception)}") + {:error, {type, exception}} + end + end end diff --git a/test/supavisor/db_handler_test.exs b/test/supavisor/db_handler_test.exs index e2610de6..cdcdf0b0 100644 --- a/test/supavisor/db_handler_test.exs +++ b/test/supavisor/db_handler_test.exs @@ -1,6 +1,7 @@ defmodule Supavisor.DbHandlerTest do use ExUnit.Case, async: false alias Supavisor.DbHandler, as: Db + alias Supavisor.Helpers, as: H alias Supavisor.Protocol.Server # import Mock @@ -40,7 +41,7 @@ defmodule Supavisor.DbHandlerTest do :meck.expect(:gen_tcp, :send, fn _sock, _msg -> :ok end) :meck.expect(:inet, :setopts, fn _sock, _opts -> :ok end) - secrets = fn -> %{user: "some user", db_user: "some user"} end + secrets = H.encode_secret(%{user: "some user", db_user: "some user"}) auth = %{ host: "host", @@ -148,7 +149,7 @@ defmodule Supavisor.DbHandlerTest do data = %{ auth: %{ - password: fn -> "some_password" end, + password: H.encode_secret("some_password"), user: "some_user", method: :password }, diff --git a/test/supavisor/syn_handler_test.exs b/test/supavisor/syn_handler_test.exs index edfa4f5a..5ebf46c1 100644 --- a/test/supavisor/syn_handler_test.exs +++ b/test/supavisor/syn_handler_test.exs @@ -3,6 +3,7 @@ defmodule Supavisor.SynHandlerTest do import ExUnit.CaptureLog require Logger alias Ecto.Adapters.SQL.Sandbox + alias Supavisor.Helpers, as: H @id {{:single, "syn_tenant"}, "postgres", :session, "postgres"} @@ -10,7 +11,7 @@ defmodule Supavisor.SynHandlerTest do node2 = :"secondary@127.0.0.1" secret = %{alias: "postgres"} - auth_secret = {:password, fn -> secret end} + auth_secret = {:password, H.encode_secret(secret)} {:ok, pid2} = :erpc.call(node2, Supavisor.FixturesHelpers, :start_pool, [@id, secret]) Process.sleep(500) assert pid2 == Supavisor.get_global_sup(@id) diff --git a/test/support/fixtures/helpers.ex b/test/support/fixtures/helpers.ex index 0e593c96..de6404d8 100644 --- a/test/support/fixtures/helpers.ex +++ b/test/support/fixtures/helpers.ex @@ -1,8 +1,10 @@ defmodule Supavisor.FixturesHelpers do @moduledoc false + alias Supavisor.Helpers, as: H + def start_pool(id, secret) do - secret = {:password, fn -> secret end} + secret = {:password, H.encode_secret(secret)} Supavisor.start(id, secret) end end From bb3071aaa58c61995101aaa48305ea2e69157967 Mon Sep 17 00:00:00 2001 From: Stas Date: Wed, 29 May 2024 15:50:26 +0300 Subject: [PATCH 2/2] Update lib/supavisor/hot_upgrade.ex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Filipe Cabaço --- lib/supavisor/hot_upgrade.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/supavisor/hot_upgrade.ex b/lib/supavisor/hot_upgrade.ex index 115fcdb3..a0fb468a 100644 --- a/lib/supavisor/hot_upgrade.ex +++ b/lib/supavisor/hot_upgrade.ex @@ -119,7 +119,8 @@ defmodule Supavisor.HotUpgrade do args = Map.update!(args, :auth, fn auth -> - Map.put(auth, :password, enc(auth.password)) + auth + |> Map.put(:password, enc(auth.password)) |> Map.put(:secrets, enc(auth.secrets)) end)