Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: encrypt secrets for internal use #333

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.1.47
1.1.50
57 changes: 57 additions & 0 deletions bench/enc_dec.exs
Original file line number Diff line number Diff line change
@@ -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
6 changes: 3 additions & 3 deletions lib/supavisor.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
31 changes: 20 additions & 11 deletions lib/supavisor/client_handler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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"}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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}
Expand All @@ -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,
Expand All @@ -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)

Expand Down
14 changes: 7 additions & 7 deletions lib/supavisor/db_handler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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)
Expand All @@ -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]
Expand Down Expand Up @@ -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

Expand Down
37 changes: 37 additions & 0 deletions lib/supavisor/helpers.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)}"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Emitting the deprecation warning is fine, but have you considered using the @deprecated module tag - https://hexdocs.pm/elixir/Module.html#module-deprecated-since-v1-6-0 - with a different function name? This would make easier to spot the locations you are still using this, and would make easier to migrate. Of course is totally fine to keep as it is :)
WDYT?


fun.()
end

@spec decode_secret(binary()) :: term()
def decode_secret(encoded) do
encoded
|> Supavisor.Vault.decrypt!()
|> :erlang.binary_to_term()
end
end
29 changes: 20 additions & 9 deletions lib/supavisor/hot_upgrade.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ defmodule Supavisor.HotUpgrade do
import Cachex.Spec
require Record

alias Supavisor.Helpers, as: H

Record.defrecord(
:state,
[
Expand Down Expand Up @@ -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 ->
Expand All @@ -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))
abc3 marked this conversation as resolved.
Show resolved Hide resolved
end)

{[^db_handler], %{^db_handler => child}} = state(state, :children)
Expand All @@ -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 ->
Expand All @@ -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
5 changes: 3 additions & 2 deletions test/supavisor/db_handler_test.exs
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
},
Expand Down
3 changes: 2 additions & 1 deletion test/supavisor/syn_handler_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ 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"}

test "resolving conflict" do
node2 = :"[email protected]"

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)
Expand Down
4 changes: 3 additions & 1 deletion test/support/fixtures/helpers.ex
Original file line number Diff line number Diff line change
@@ -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
Loading