Skip to content

Commit c76bc91

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 a44394d commit c76bc91

File tree

9 files changed

+344
-20
lines changed

9 files changed

+344
-20
lines changed

lib/supavisor/client_handler.ex

Lines changed: 100 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,8 @@ defmodule Supavisor.ClientHandler do
357357
:no_retry -> handle_auth_failure(sock, reason, data)
358358
end
359359

360-
{:ok, client_key} ->
361-
handle_auth_success(sock, {method, secrets}, client_key, data)
360+
{:ok, client_key, password} ->
361+
handle_auth_success(sock, {method, secrets}, client_key, password, data)
362362
end
363363
end
364364

@@ -664,6 +664,7 @@ defmodule Supavisor.ClientHandler do
664664
def terminate(reason, state, _data) do
665665
Logger.metadata(state: state)
666666
Logger.debug("ClientHandler: socket closed with reason #{inspect(reason)}")
667+
667668
:ok
668669
end
669670

@@ -681,7 +682,7 @@ defmodule Supavisor.ClientHandler do
681682
# Retry authentication with fresh secrets
682683
case handle_exchange(sock, {method2, secrets2}) do
683684
{:ok, client_key} ->
684-
{:retry_success, handle_auth_success(sock, {method2, secrets2}, client_key, data)}
685+
{:retry_success, handle_auth_success(sock, {method2, secrets2}, client_key, nil, data)}
685686

686687
{:error, retry_reason} ->
687688
Logger.error("ClientHandler: Authentication retry failed: #{inspect(retry_reason)}")
@@ -747,11 +748,17 @@ defmodule Supavisor.ClientHandler do
747748
Cachex.update(Supavisor.Cache, {:secrets, tenant, user}, {:cached, value})
748749
end
749750

750-
defp handle_auth_success(sock, {method, secrets}, client_key, data) do
751+
defp handle_auth_success(sock, {method, secrets}, client_key, password, data) do
751752
final_secrets =
752-
if client_key,
753-
do: fn -> Map.put(secrets.(), :client_key, client_key) end,
754-
else: secrets
753+
fn ->
754+
Enum.reduce(
755+
[client_key: client_key, cls_password: password],
756+
secrets.(),
757+
fn {k, v}, acc ->
758+
if v != nil, do: Map.put(acc, k, v), else: acc
759+
end
760+
)
761+
end
755762

756763
Logger.debug("ClientHandler: Exchange success")
757764
:ok = HandlerHelpers.sock_send(sock, Server.authentication_ok())
@@ -793,7 +800,7 @@ defmodule Supavisor.ClientHandler do
793800
end
794801

795802
@spec handle_exchange(Supavisor.sock(), {atom(), fun()}) ::
796-
{:ok, binary() | nil}
803+
{:ok, binary() | nil, binary() | nil}
797804
| {:error, :wrong_password | {:timeout, String.t()} | {:unexpected_message, term()}}
798805
def handle_exchange({_, socket} = sock, {:auth_query_md5 = method, secrets}) do
799806
salt = :crypto.strong_rand_bytes(4)
@@ -805,7 +812,24 @@ defmodule Supavisor.ClientHandler do
805812
payload: {:md5, client_md5}
806813
}, _} <- receive_next(socket, "Timeout while waiting for the md5 exchange"),
807814
{:ok, key} <- authenticate_exchange(method, client_md5, secrets.().secret, salt) do
808-
{:ok, key}
815+
{:ok, key, nil}
816+
else
817+
{:error, message} -> {:error, message}
818+
other -> {:error, "Unexpected message #{inspect(other)}"}
819+
end
820+
end
821+
822+
def handle_exchange({_, socket} = sock, {:auth_query_jit = method, secrets}) do
823+
:ok = HandlerHelpers.sock_send(sock, Server.password_request())
824+
{:ok, {ip, _port}} = :inet.peername(socket)
825+
826+
with {:ok,
827+
%{
828+
tag: :password_message,
829+
payload: {:cleartext_password, password}
830+
}, _} <- receive_next(socket, "Timeout while waiting for the password exchange"),
831+
{:ok, key} <- authenticate_exchange(method, secrets, password, ip) do
832+
{:ok, key, password}
809833
else
810834
{:error, message} -> {:error, message}
811835
other -> {:error, "Unexpected message #{inspect(other)}"}
@@ -839,7 +863,7 @@ defmodule Supavisor.ClientHandler do
839863
{:ok, key} <- authenticate_exchange(method, secrets, signatures, p) do
840864
message = "v=#{Base.encode64(signatures.server)}"
841865
:ok = HandlerHelpers.sock_send(sock, Server.exchange_message(:final, message))
842-
{:ok, key}
866+
{:ok, key, nil}
843867
else
844868
{:error, message} -> {:error, message}
845869
other -> {:error, "Unexpected message #{inspect(other)}"}
@@ -886,6 +910,57 @@ defmodule Supavisor.ClientHandler do
886910
else: {:error, :wrong_password}
887911
end
888912

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

9551032
%{
@@ -1041,8 +1118,18 @@ defmodule Supavisor.ClientHandler do
10411118

10421119
resp =
10431120
with {:ok, secret} <- Helpers.get_user_secret(conn, tenant.auth_query, db_user) do
1044-
t = if secret.digest == :md5, do: :auth_query_md5, else: :auth_query
1045-
{:ok, {t, fn -> Map.put(secret, :alias, user.db_user_alias) end}}
1121+
t =
1122+
case {tenant.use_jit, secret.digest} do
1123+
{true, _} -> :auth_query_jit
1124+
{_, :md5} -> :auth_query_md5
1125+
_ -> :auth_query
1126+
end
1127+
1128+
{:ok,
1129+
{t,
1130+
fn ->
1131+
Map.merge(secret, %{alias: user.db_user_alias, jit_api_url: tenant.jit_api_url})
1132+
end}}
10461133
end
10471134

10481135
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
@@ -183,6 +183,11 @@ defmodule Supavisor.DbHandler do
183183
Logger.error("DbHandler: Auth error #{inspect(reason)}")
184184
{:stop, :invalid_password, data}
185185

186+
{:error_response, ["SFATAL", "VFATAL", "C28000", reason, _, _, _]} ->
187+
handle_authentication_error(data, reason)
188+
Logger.error("DbHandler: Auth error #{inspect(reason)}")
189+
{:stop, :invalid_password, data}
190+
186191
{:error_response, error} ->
187192
Logger.error("DbHandler: Error auth response #{inspect(error)}")
188193
{:stop, {:encode_and_forward, error}}
@@ -663,7 +668,14 @@ defmodule Supavisor.DbHandler do
663668
defp handle_auth_pkts(%{payload: :authentication_cleartext_password} = dec_pkt, _, data) do
664669
Logger.debug("DbHandler: dec_pkt, #{inspect(dec_pkt, pretty: true)}")
665670

666-
payload = <<data.auth.password.()::binary, 0>>
671+
password =
672+
if data.auth.method == :password do
673+
data.auth.password.()
674+
else
675+
data.auth.secrets.().cls_password
676+
end
677+
678+
payload = <<password::binary, 0>>
667679
bin = [?p, <<IO.iodata_length(payload) + 4::signed-32>>, payload]
668680
:ok = HandlerHelpers.sock_send(data.sock, bin)
669681
:authentication_cleartext

lib/supavisor/helpers.ex

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,4 +399,74 @@ defmodule Supavisor.Helpers do
399399
"""
400400
@spec mb_to_words(number()) :: pos_integer()
401401
def mb_to_words(mb), do: round(mb * 1_048_576 / :erlang.system_info(:wordsize))
402+
403+
@doc """
404+
Checks if a token matches either a PAT or JWT.
405+
406+
## Parameters
407+
408+
- `token`: The token string
409+
"""
410+
def token_matches?(<<"sbp_", _rest::binary>>), do: true
411+
412+
def token_matches?(token) do
413+
case String.split(token, ".") do
414+
["eyJ" <> _, "eyJ" <> _, _sig] ->
415+
true
416+
417+
_ ->
418+
false
419+
end
420+
end
421+
422+
@doc """
423+
Makes an HTTPS GET request to `url` with a Bearer `token`
424+
and checks if the given `role` is present in the returned `user_roles` list.
425+
426+
Returns:
427+
- `{:ok, true}` if role is present
428+
- `{:ok, false}` if role is absent
429+
- `{:error, :unauthorized}` for 401
430+
- `{:error, :forbidden}` for 403
431+
- `{:error, {:unexpected_status, status}}` for other HTTP codes
432+
"""
433+
def check_user_has_jit_role(url, token, role \\ "postgres", rhost, opts \\ []) do
434+
opts =
435+
Keyword.merge(opts,
436+
headers: [
437+
authorization: "Bearer #{token}",
438+
"content-type": "application/json"
439+
]
440+
)
441+
442+
body =
443+
%{
444+
rhost: rhost,
445+
role: role
446+
}
447+
|> Jason.encode!()
448+
449+
response =
450+
Req.post!(
451+
url,
452+
opts
453+
|> Keyword.put(:body, body)
454+
)
455+
456+
case response.status do
457+
200 ->
458+
%{"user_role" => %{"role" => urole}} = response.body
459+
460+
# double check server response
461+
has_valid_role? = urole == role
462+
463+
{:ok, has_valid_role?}
464+
465+
status when status in [401, 403] ->
466+
{:error, :unauthorized_or_forbidden}
467+
468+
status ->
469+
{:error, {:unexpected_status, status}}
470+
end
471+
end
402472
end

lib/supavisor/protocol/server.ex

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

76+
@spec password_request() :: iodata()
77+
def password_request, do: [<<?R, 8::32, 3::32>>]
78+
7679
@spec exchange_first_message(binary, binary | boolean, pos_integer) :: binary
7780
def exchange_first_message(nonce, salt \\ false, iterations \\ 4096) do
7881
server_nonce =
@@ -329,11 +332,17 @@ defmodule Supavisor.Protocol.Server do
329332
end
330333

331334
@spec decode_payload(:password_message, binary()) ::
332-
{:first_msg_response, map()} | :undefined
335+
{:first_msg_response, map()} | {:cleartext_password, binary()} | :undefined
333336
defp decode_payload(:password_message, bin) do
334337
case kv_to_map(bin) do
335-
{:ok, map} -> {:first_msg_response, map}
336-
{:error, _} -> :undefined
338+
{:ok, map} ->
339+
{:first_msg_response, map}
340+
341+
{:error, _} ->
342+
case :binary.split(bin, <<0>>) do
343+
[password, ""] -> {:cleartext_password, password}
344+
_ -> :undefined
345+
end
337346
end
338347
end
339348

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 & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +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(:use_jit, :boolean, default: false)
36+
field(:jit_api_url, :string)
3537

3638
has_many(:users, User,
3739
foreign_key: :tenant_external_id,
@@ -65,7 +67,9 @@ defmodule Supavisor.Tenants.Tenant do
6567
:client_idle_timeout,
6668
:client_heartbeat_interval,
6769
:allow_list,
68-
:availability_zone
70+
:availability_zone,
71+
:use_jit,
72+
:jit_api_url
6973
])
7074
|> check_constraint(:upstream_ssl, name: :upstream_constraints, prefix: "_supavisor")
7175
|> 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)