Skip to content

Commit 4ac614d

Browse files
committed
fix: check cache for secret
Only using data.auth.password would reuse the password for the is_manager user when trying to authenticate against the upstream. The first request, from the is_manager to the upstream should use the password, while the subsequent connection with the authenticating user should use secret. feat: support e2e password flow chore: check password against auth_query chore: set client_key Sets the client_key when using password auth (jit) and the incomming password is a valid password that matches the scram-sha-256. This allows supporting the case where the tenant has switched back to scram-sha-256 but pooler is still checking for cleartext password. chore: todo - api implementation feat: support api lookup chore: ensure user secret caching works with jit chore: add jit expire checks chore: cleanup credo warnings chore: store client_key for cleartext auth exchange If the cleartext auth exchange was for a normal password (not PAT/JWT), we will have a valid client_key that was calculated for the scram-sha-256. Ensure this is always present in secrets, along with the clear password if we have it. This allows for the case where the upstream server either stops using JIT and reverts to scram-sha-256. Or has a pg_hba entry for the specific user to use scram-sha-256 rather than pam. This allows both JIT users and non-JIT users to exist in the same tenant. chore: support md5 on JIT, lazy eval secrets fix: migration fix: terminate does not need additional message Update lib/supavisor/helpers.ex Co-authored-by: felipe stival <14948182+v0idpwn@users.noreply.github.com> chore: start adding more tests test: add jit api tester chore: use simplified API with role, rhost
1 parent 20fa39f commit 4ac614d

File tree

8 files changed

+327
-22
lines changed

8 files changed

+327
-22
lines changed

lib/supavisor/client_handler.ex

Lines changed: 102 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,8 @@ defmodule Supavisor.ClientHandler do
364364
:no_retry -> handle_auth_failure(sock, reason, data)
365365
end
366366

367-
{:ok, client_key} ->
368-
handle_auth_success(sock, {method, secrets}, client_key, data)
367+
{:ok, client_key, password} ->
368+
handle_auth_success(sock, {method, secrets}, client_key, password, data)
369369
end
370370
end
371371

@@ -670,6 +670,7 @@ defmodule Supavisor.ClientHandler do
670670
def terminate(reason, state, _data) do
671671
Logger.metadata(state: state)
672672
Logger.debug("ClientHandler: socket closed with reason #{inspect(reason)}")
673+
673674
:ok
674675
end
675676

@@ -686,8 +687,9 @@ defmodule Supavisor.ClientHandler do
686687

687688
# Retry authentication with fresh secrets
688689
case handle_exchange(sock, {method2, secrets2}) do
689-
{:ok, client_key} ->
690-
{:retry_success, handle_auth_success(sock, {method2, secrets2}, client_key, data)}
690+
{:ok, client_key, password} ->
691+
{:retry_success,
692+
handle_auth_success(sock, {method2, secrets2}, client_key, password, data)}
691693

692694
{:error, retry_reason} ->
693695
Logger.error("ClientHandler: Authentication retry failed: #{inspect(retry_reason)}")
@@ -753,11 +755,17 @@ defmodule Supavisor.ClientHandler do
753755
Cachex.update(Supavisor.Cache, {:secrets, tenant, user}, {:cached, value})
754756
end
755757

756-
defp handle_auth_success(sock, {method, secrets}, client_key, data) do
758+
defp handle_auth_success(sock, {method, secrets}, client_key, password, data) do
757759
final_secrets =
758-
if client_key,
759-
do: fn -> Map.put(secrets.(), :client_key, client_key) end,
760-
else: secrets
760+
fn ->
761+
Enum.reduce(
762+
[client_key: client_key, cls_password: password],
763+
secrets.(),
764+
fn {k, v}, acc ->
765+
if v != nil, do: Map.put(acc, k, v), else: acc
766+
end
767+
)
768+
end
761769

762770
Logger.debug("ClientHandler: Exchange success")
763771
:ok = HandlerHelpers.sock_send(sock, Server.authentication_ok())
@@ -799,7 +807,7 @@ defmodule Supavisor.ClientHandler do
799807
end
800808

801809
@spec handle_exchange(Supavisor.sock(), {atom(), fun()}) ::
802-
{:ok, binary() | nil}
810+
{:ok, binary() | nil, binary() | nil}
803811
| {:error, :wrong_password | {:timeout, String.t()} | {:unexpected_message, term()}}
804812
def handle_exchange({_, socket} = sock, {:auth_query_md5 = method, secrets}) do
805813
salt = :crypto.strong_rand_bytes(4)
@@ -811,7 +819,24 @@ defmodule Supavisor.ClientHandler do
811819
payload: {:md5, client_md5}
812820
}, _} <- receive_next(socket, "Timeout while waiting for the md5 exchange"),
813821
{:ok, key} <- authenticate_exchange(method, client_md5, secrets.().secret, salt) do
814-
{:ok, key}
822+
{:ok, key, nil}
823+
else
824+
{:error, message} -> {:error, message}
825+
other -> {:error, "Unexpected message #{inspect(other)}"}
826+
end
827+
end
828+
829+
def handle_exchange({_, socket} = sock, {:auth_query_jit = method, secrets}) do
830+
:ok = HandlerHelpers.sock_send(sock, Server.password_request())
831+
{:ok, {ip, _port}} = :inet.peername(socket)
832+
833+
with {:ok,
834+
%{
835+
tag: :password_message,
836+
payload: {:cleartext_password, password}
837+
}, _} <- receive_next(socket, "Timeout while waiting for the password exchange"),
838+
{:ok, key} <- authenticate_exchange(method, secrets, password, ip) do
839+
{:ok, key, password}
815840
else
816841
{:error, message} -> {:error, message}
817842
other -> {:error, "Unexpected message #{inspect(other)}"}
@@ -845,7 +870,7 @@ defmodule Supavisor.ClientHandler do
845870
{:ok, key} <- authenticate_exchange(method, secrets, signatures, p) do
846871
message = "v=#{Base.encode64(signatures.server)}"
847872
:ok = HandlerHelpers.sock_send(sock, Server.exchange_message(:final, message))
848-
{:ok, key}
873+
{:ok, key, nil}
849874
else
850875
{:error, message} -> {:error, message}
851876
other -> {:error, "Unexpected message #{inspect(other)}"}
@@ -892,6 +917,57 @@ defmodule Supavisor.ClientHandler do
892917
else: {:error, :wrong_password}
893918
end
894919

920+
defp authenticate_exchange(:auth_query_jit, secrets, password, ip) do
921+
# check if incomming password looks like PAT or a JWT
922+
# otherwise handle as password,
923+
secret = secrets.()
924+
925+
if Helpers.token_matches?(password) do
926+
Logger.debug("Looks like a PAT/JWT - #{inspect(secret.jit_api_url)}")
927+
rhost = ip |> :inet.ntoa() |> to_string()
928+
929+
case Helpers.check_user_has_jit_role(secret.jit_api_url, password, secret.user, rhost) do
930+
{:ok, true} ->
931+
# set a fake client_key incase upstream switches away from pam mid auth
932+
{:ok, :crypto.hash(:sha256, password)}
933+
934+
{:ok, false} ->
935+
Logger.debug("User token is valid but can't assume this role")
936+
{:error, :wrong_password}
937+
938+
{:error, :unauthorized_or_forbidden} ->
939+
{:error, :wrong_password}
940+
941+
{:error, _} ->
942+
Logger.debug("Unexpected error while calling API")
943+
{:error, :wrong_password}
944+
end
945+
else
946+
# match against the scram-sha-256 / md5 we have from auth_query
947+
case secret.digest do
948+
:md5 ->
949+
if Helpers.md5([password, secret.user]) == secret.secret do
950+
{:ok, nil}
951+
else
952+
{:error, :wrong_password}
953+
end
954+
955+
_ ->
956+
salt = Base.decode64!(secret.salt)
957+
958+
salted_password =
959+
:crypto.pbkdf2_hmac(:sha256, password, salt, secret.iterations, 32)
960+
961+
client_key = :crypto.mac(:hmac, :sha256, salted_password, "Client Key")
962+
stored_key = :crypto.hash(:sha256, client_key)
963+
964+
if :crypto.hash_equals(stored_key, secret.stored_key),
965+
do: {:ok, client_key},
966+
else: {:error, :wrong_password}
967+
end
968+
end
969+
end
970+
895971
@spec db_checkout(:write | :read | :both, :on_connect | :on_query, map) ::
896972
{pid, pid, Supavisor.sock()} | nil
897973
defp db_checkout(_, _, %{mode: mode, db_pid: {pool, db_pid, db_sock}})
@@ -955,7 +1031,9 @@ defmodule Supavisor.ClientHandler do
9551031
require_user: info.tenant.require_user,
9561032
upstream_ssl: info.tenant.upstream_ssl,
9571033
upstream_tls_ca: info.tenant.upstream_tls_ca,
958-
upstream_verify: info.tenant.upstream_verify
1034+
upstream_verify: info.tenant.upstream_verify,
1035+
use_jit: info.tenant.use_jit,
1036+
jit_api_url: info.tenant.jit_api_url
9591037
}
9601038

9611039
%{
@@ -1048,8 +1126,18 @@ defmodule Supavisor.ClientHandler do
10481126

10491127
resp =
10501128
with {:ok, secret} <- Helpers.get_user_secret(conn, tenant.auth_query, db_user) do
1051-
t = if secret.digest == :md5, do: :auth_query_md5, else: :auth_query
1052-
{:ok, {t, fn -> Map.put(secret, :alias, user.db_user_alias) end}}
1129+
t =
1130+
case {tenant.use_jit, secret.digest} do
1131+
{true, _} -> :auth_query_jit
1132+
{_, :md5} -> :auth_query_md5
1133+
_ -> :auth_query
1134+
end
1135+
1136+
{:ok,
1137+
{t,
1138+
fn ->
1139+
Map.merge(secret, %{alias: user.db_user_alias, jit_api_url: tenant.jit_api_url})
1140+
end}}
10531141
end
10541142

10551143
Logger.info("ClientHandler: Get secrets finished")

lib/supavisor/db_handler.ex

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,11 @@ defmodule Supavisor.DbHandler do
193193
Logger.error("DbHandler: Auth error #{inspect(reason)}")
194194
{:stop, :invalid_password, data}
195195

196+
{:error_response, ["SFATAL", "VFATAL", "C28000", reason, _, _, _]} ->
197+
handle_authentication_error(data, reason)
198+
Logger.error("DbHandler: Auth error #{inspect(reason)}")
199+
{:stop, :invalid_password, data}
200+
196201
{:error_response, error} ->
197202
Logger.error("DbHandler: Error auth response #{inspect(error)}")
198203
{:stop, {:encode_and_forward, error}}
@@ -648,7 +653,14 @@ defmodule Supavisor.DbHandler do
648653
defp handle_auth_pkts(%{payload: :authentication_cleartext_password} = dec_pkt, _, data) do
649654
Logger.debug("DbHandler: dec_pkt, #{inspect(dec_pkt, pretty: true)}")
650655

651-
payload = <<data.auth.password.()::binary, 0>>
656+
password =
657+
if data.auth.method == :password do
658+
data.auth.password.()
659+
else
660+
data.auth.secrets.().cls_password
661+
end
662+
663+
payload = <<password::binary, 0>>
652664
bin = [?p, <<IO.iodata_length(payload) + 4::signed-32>>, payload]
653665
:ok = HandlerHelpers.sock_send(data.sock, bin)
654666
:authentication_cleartext

lib/supavisor/helpers.ex

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,4 +412,74 @@ defmodule Supavisor.Helpers do
412412
raise "Invalid boolean value for #{env_var}: #{inspect(value)}. Expected: true, false, 1, or 0"
413413
end
414414
end
415+
416+
@doc """
417+
Checks if a token matches either a PAT or JWT.
418+
419+
## Parameters
420+
421+
- `token`: The token string
422+
"""
423+
def token_matches?(<<"sbp_", _rest::binary>>), do: true
424+
425+
def token_matches?(token) do
426+
case String.split(token, ".") do
427+
["eyJ" <> _, "eyJ" <> _, _sig] ->
428+
true
429+
430+
_ ->
431+
false
432+
end
433+
end
434+
435+
@doc """
436+
Makes an HTTPS GET request to `url` with a Bearer `token`
437+
and checks if the given `role` is present in the returned `user_roles` list.
438+
439+
Returns:
440+
- `{:ok, true}` if role is present
441+
- `{:ok, false}` if role is absent
442+
- `{:error, :unauthorized}` for 401
443+
- `{:error, :forbidden}` for 403
444+
- `{:error, {:unexpected_status, status}}` for other HTTP codes
445+
"""
446+
def check_user_has_jit_role(url, token, role \\ "postgres", rhost, opts \\ []) do
447+
opts =
448+
Keyword.merge(opts,
449+
headers: [
450+
authorization: "Bearer #{token}",
451+
"content-type": "application/json"
452+
]
453+
)
454+
455+
body =
456+
%{
457+
rhost: rhost,
458+
role: role
459+
}
460+
|> Jason.encode!()
461+
462+
response =
463+
Req.post!(
464+
url,
465+
opts
466+
|> Keyword.put(:body, body)
467+
)
468+
469+
case response.status do
470+
200 ->
471+
%{"user_role" => %{"role" => urole}} = response.body
472+
473+
# double check server response
474+
has_valid_role? = urole == role
475+
476+
{:ok, has_valid_role?}
477+
478+
status when status in [401, 403] ->
479+
{:error, :unauthorized_or_forbidden}
480+
481+
status ->
482+
{:error, {:unexpected_status, status}}
483+
end
484+
end
415485
end

lib/supavisor/protocol/server.ex

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ defmodule Supavisor.Protocol.Server do
112112
@spec md5_request(<<_::32>>) :: iodata()
113113
def md5_request(salt), do: [<<?R, 12::32, 5::32>>, salt]
114114

115+
@spec password_request() :: iodata()
116+
def password_request, do: [<<?R, 8::32, 3::32>>]
117+
115118
@spec exchange_first_message(binary, binary | boolean, pos_integer) :: binary
116119
def exchange_first_message(nonce, salt \\ false, iterations \\ 4096) do
117120
server_nonce =
@@ -376,11 +379,17 @@ defmodule Supavisor.Protocol.Server do
376379
end
377380

378381
@spec decode_payload(:password_message, binary()) ::
379-
{:first_msg_response, map()} | :undefined
382+
{:first_msg_response, map()} | {:cleartext_password, binary()} | :undefined
380383
defp decode_payload(:password_message, bin) do
381384
case kv_to_map(bin) do
382-
{:ok, map} -> {:first_msg_response, map}
383-
{:error, _} -> :undefined
385+
{:ok, map} ->
386+
{:first_msg_response, map}
387+
388+
{:error, _} ->
389+
case :binary.split(bin, <<0>>) do
390+
[password, ""] -> {:cleartext_password, password}
391+
_ -> :undefined
392+
end
384393
end
385394
end
386395

lib/supavisor/secret_checker.ex

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,31 @@ defmodule Supavisor.SecretChecker do
101101
do: Process.send_after(self(), :check, interval)
102102

103103
def check_secrets(user, %{auth: auth, conn: conn} = state) do
104+
# get tenant information so that we can check configuration information
105+
# that affects secrets, like :use_jit
106+
t = Supavisor.Tenants.get_tenant_cache(state.tenant, state.auth.sni_hostname)
107+
104108
case Helpers.get_user_secret(conn, auth.auth_query, user) do
105109
{:ok, secret} ->
106-
method = if secret.digest == :md5, do: :auth_query_md5, else: :auth_query
107-
secrets = Map.put(secret, :alias, auth.alias)
110+
# when jit is in use, make sure to keep the jit_api_url in the cache, and
111+
# set the method to also be otherwise
112+
# we'll try use scram-sha-256 or md5 even though the upstream server is expecting
113+
# jit. Something that is configured in the tenant.
114+
# make sure to bring across the jit_api_url too, which will never be in the secrets
115+
# returned by the db
116+
method =
117+
cond do
118+
t.use_jit -> :auth_query_jit
119+
secret.digest == :md5 -> :auth_query_md5
120+
true -> :auth_query
121+
end
122+
123+
secrets =
124+
if t.use_jit do
125+
Map.merge(secret, %{alias: auth.alias, jit_api_url: t.jit_api_url})
126+
else
127+
Map.put(secret, :alias, auth.alias)
128+
end
108129

109130
update_cache =
110131
case Cachex.get(Supavisor.Cache, state.key) do

lib/supavisor/tenants/tenant.ex

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ defmodule Supavisor.Tenants.Tenant do
3232
field(:client_heartbeat_interval, :integer, default: 60)
3333
field(:allow_list, {:array, :string}, default: ["0.0.0.0/0", "::/0"])
3434
field(:availability_zone, :string)
35-
field(:feature_flags, :map, default: %{})
35+
field(:use_jit, :boolean, default: false)
36+
field(:jit_api_url, :string)
3637

3738
has_many(:users, User,
3839
foreign_key: :tenant_external_id,
@@ -67,7 +68,9 @@ defmodule Supavisor.Tenants.Tenant do
6768
:client_heartbeat_interval,
6869
:allow_list,
6970
:availability_zone,
70-
:feature_flags
71+
:feature_flags,
72+
:use_jit,
73+
:jit_api_url
7174
])
7275
|> check_constraint(:upstream_ssl, name: :upstream_constraints, prefix: "_supavisor")
7376
|> check_constraint(:upstream_verify, name: :upstream_constraints, prefix: "_supavisor")
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
defmodule Supavisor.Repo.Migrations.AddJitConfig do
2+
use Ecto.Migration
3+
4+
def change do
5+
alter table("tenants", prefix: "_supavisor") do
6+
add(:use_jit, :boolean, default: false)
7+
add(:jit_api_url, :string)
8+
end
9+
end
10+
end

0 commit comments

Comments
 (0)