Skip to content

Commit b759523

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 5f72dc0 commit b759523

File tree

11 files changed

+290
-17
lines changed

11 files changed

+290
-17
lines changed

lib/supavisor/client_handler.ex

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,13 @@ defmodule Supavisor.ClientHandler do
313313
{:next_state, :auth_md5_wait, %{data | auth_context: auth_context},
314314
{:timeout, 15_000, :auth_timeout}}
315315

316+
:auth_query_jit ->
317+
auth_context = Auth.create_auth_context(method, secrets, info)
318+
:ok = HandlerHelpers.sock_send(sock, Server.password_request())
319+
320+
{:next_state, :auth_password_wait, %{data | auth_context: auth_context},
321+
{:timeout, 15_000, :auth_timeout}}
322+
316323
_scram_method ->
317324
:ok = HandlerHelpers.sock_send(sock, Server.scram_request())
318325
auth_context = Auth.create_auth_context(method, secrets, info)
@@ -980,7 +987,9 @@ defmodule Supavisor.ClientHandler do
980987
method: proxy_type,
981988
upstream_ssl: info.tenant.upstream_ssl,
982989
upstream_tls_ca: info.tenant.upstream_tls_ca,
983-
upstream_verify: info.tenant.upstream_verify
990+
upstream_verify: info.tenant.upstream_verify,
991+
use_jit: info.tenant.use_jit,
992+
jit_api_url: info.tenant.jit_api_url
984993
}
985994

986995
%{

lib/supavisor/client_handler/auth.ex

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ defmodule Supavisor.ClientHandler.Auth do
1414
alias Supavisor.{Helpers, Protocol.Server}
1515
alias Supavisor.ClientHandler.Auth.{MD5Secrets, PasswordSecrets, SASLSecrets}
1616

17-
@type auth_method :: :password | :auth_query | :auth_query_md5
17+
@type auth_method :: :password | :auth_query | :auth_query_md5 | :auth_query_jit
1818
@type auth_secrets :: {auth_method(), function()}
1919

2020
## Secret Management
@@ -83,6 +83,57 @@ defmodule Supavisor.ClientHandler.Auth do
8383
else: {:error, :wrong_password}
8484
end
8585

86+
def validate_credentials(:auth_query_jit, secrets, password, ip) do
87+
# check if incomming password looks like PAT or a JWT
88+
# otherwise handle as password,
89+
secret = secrets.()
90+
91+
if Helpers.token_matches?(password) do
92+
Logger.debug("Looks like a PAT/JWT - #{inspect(secret.jit_api_url)}")
93+
rhost = ip |> :inet.ntoa() |> to_string()
94+
95+
case Helpers.check_user_has_jit_role(secret.jit_api_url, password, secret.user, rhost) do
96+
{:ok, true} ->
97+
# set a fake client_key incase upstream switches away from pam mid auth
98+
{:ok, :crypto.hash(:sha256, password)}
99+
100+
{:ok, false} ->
101+
Logger.debug("User token is valid but can't assume this role")
102+
{:error, :wrong_password}
103+
104+
{:error, :unauthorized_or_forbidden} ->
105+
{:error, :wrong_password}
106+
107+
{:error, _} ->
108+
Logger.debug("Unexpected error while calling API")
109+
{:error, :wrong_password}
110+
end
111+
else
112+
# match against the scram-sha-256 / md5 we have from auth_query
113+
case secret.digest do
114+
:md5 ->
115+
if Helpers.md5([password, secret.user]) == secret.secret do
116+
{:ok, nil}
117+
else
118+
{:error, :wrong_password}
119+
end
120+
121+
_ ->
122+
salt = Base.decode64!(secret.salt)
123+
124+
salted_password =
125+
:crypto.pbkdf2_hmac(:sha256, password, salt, secret.iterations, 32)
126+
127+
client_key = :crypto.mac(:hmac, :sha256, salted_password, "Client Key")
128+
stored_key = :crypto.hash(:sha256, client_key)
129+
130+
if :crypto.hash_equals(stored_key, secret.stored_key),
131+
do: {:ok, client_key},
132+
else: {:error, :wrong_password}
133+
end
134+
end
135+
end
136+
86137
## Challenge Preparation
87138

88139
@doc """
@@ -233,7 +284,8 @@ defmodule Supavisor.ClientHandler.Auth do
233284
}
234285
end
235286

236-
def create_auth_context(method, secrets, info) when method in [:password, :auth_query] do
287+
def create_auth_context(method, secrets, info)
288+
when method in [:password, :auth_query, :auth_query_jit] do
237289
%{
238290
method: method,
239291
secrets: secrets,
@@ -312,9 +364,10 @@ defmodule Supavisor.ClientHandler.Auth do
312364

313365
with {:ok, secret} <- Helpers.get_user_secret(conn, tenant.auth_query, db_user) do
314366
auth_type =
315-
case secret do
316-
%MD5Secrets{} -> :auth_query_md5
317-
%SASLSecrets{} -> :auth_query
367+
case {tenant.use_jit, secret} do
368+
{true, %PasswordSecrets{}} -> :auth_query_jit
369+
{_, %MD5Secrets{}} -> :auth_query_md5
370+
{_, %SASLSecrets{}} -> :auth_query
318371
end
319372

320373
{:ok, {auth_type, fn -> secret end}}

lib/supavisor/client_handler/auth/password_secrets.ex

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ defmodule Supavisor.ClientHandler.Auth.PasswordSecrets do
66

77
@type t :: %__MODULE__{
88
user: String.t(),
9-
password: String.t()
9+
password: String.t(),
10+
use_jit: Bool.t(),
11+
jit_api_url: String.t()
1012
}
1113
end

lib/supavisor/client_handler/error.ex

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ defmodule Supavisor.ClientHandler.Error do
157157
log_message =
158158
case context do
159159
:auth_md5_wait -> "Timeout while waiting for MD5 password"
160+
:auth_password_wait -> "Timeout while waiting for password"
160161
:auth_scram_first_wait -> "Timeout while waiting for first SCRAM message"
161162
:auth_scram_final_wait -> "Timeout while waiting for final SCRAM message"
162163
_ -> "Authentication timeout"
@@ -274,6 +275,7 @@ defmodule Supavisor.ClientHandler.Error do
274275
end
275276

276277
defp auth_context_description(:auth_md5_wait), do: "MD5"
278+
defp auth_context_description(:auth_password_wait), do: "PASSWORD"
277279
defp auth_context_description(:auth_scram_first_wait), do: "SCRAM first"
278280
defp auth_context_description(:auth_scram_final_wait), do: "SCRAM final"
279281
defp auth_context_description(_), do: "Unknown"

lib/supavisor/db_handler.ex

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,12 @@ defmodule Supavisor.DbHandler do
251251
Logger.error("DbHandler: Auth error #{inspect(error)}")
252252
{:stop, :invalid_password, data}
253253

254+
{:error_response, %{"S" => "FATAL", "C" => "28000"} = error} ->
255+
reason = error["M"] || "Authentication failed"
256+
handle_authentication_error(data, reason)
257+
Logger.error("DbHandler: Auth error #{inspect(error)}")
258+
{:stop, :invalid_password, data}
259+
254260
{:error_response, %{"S" => "FATAL", "C" => "3D000"} = error} ->
255261
Logger.error("DbHandler: Database does not exist: #{inspect(error)}")
256262
encode_and_forward_error(error, data)
@@ -656,10 +662,17 @@ defmodule Supavisor.DbHandler do
656662
defp handle_auth_pkts(%{payload: :authentication_cleartext_password} = dec_pkt, _, data) do
657663
Logger.debug("DbHandler: dec_pkt, #{inspect(dec_pkt, pretty: true)}")
658664

659-
{_method, secrets_fn} = data.auth.secrets
665+
{method, secrets_fn} = data.auth.secrets
660666
secrets = secrets_fn.()
661667

662-
payload = <<secrets.password::binary, 0>>
668+
password =
669+
if method == :password do
670+
secrets.password
671+
else
672+
secrets.cls_password
673+
end
674+
675+
payload = <<password::binary, 0>>
663676
bin = [?p, <<IO.iodata_length(payload) + 4::signed-32>>, payload]
664677
:ok = HandlerHelpers.sock_send(data.sock, bin)
665678
:authentication_cleartext

lib/supavisor/helpers.ex

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,4 +435,74 @@ defmodule Supavisor.Helpers do
435435
raise "Invalid boolean value for #{env_var}: #{inspect(value)}. Expected: true, false, 1, or 0"
436436
end
437437
end
438+
439+
@doc """
440+
Checks if a token matches either a PAT or JWT.
441+
442+
## Parameters
443+
444+
- `token`: The token string
445+
"""
446+
def token_matches?(<<"sbp_", _rest::binary>>), do: true
447+
448+
def token_matches?(token) do
449+
case String.split(token, ".") do
450+
["eyJ" <> _, "eyJ" <> _, _sig] ->
451+
true
452+
453+
_ ->
454+
false
455+
end
456+
end
457+
458+
@doc """
459+
Makes an HTTPS GET request to `url` with a Bearer `token`
460+
and checks if the given `role` is present in the returned `user_roles` list.
461+
462+
Returns:
463+
- `{:ok, true}` if role is present
464+
- `{:ok, false}` if role is absent
465+
- `{:error, :unauthorized}` for 401
466+
- `{:error, :forbidden}` for 403
467+
- `{:error, {:unexpected_status, status}}` for other HTTP codes
468+
"""
469+
def check_user_has_jit_role(url, token, role \\ "postgres", rhost, opts \\ []) do
470+
opts =
471+
Keyword.merge(opts,
472+
headers: [
473+
authorization: "Bearer #{token}",
474+
"content-type": "application/json"
475+
]
476+
)
477+
478+
body =
479+
%{
480+
rhost: rhost,
481+
role: role
482+
}
483+
|> Jason.encode!()
484+
485+
response =
486+
Req.post!(
487+
url,
488+
opts
489+
|> Keyword.put(:body, body)
490+
)
491+
492+
case response.status do
493+
200 ->
494+
%{"user_role" => %{"role" => urole}} = response.body
495+
496+
# double check server response
497+
has_valid_role? = urole == role
498+
499+
{:ok, has_valid_role?}
500+
501+
status when status in [401, 403] ->
502+
{:error, :unauthorized_or_forbidden}
503+
504+
status ->
505+
{:error, {:unexpected_status, status}}
506+
end
507+
end
438508
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 =
@@ -388,11 +391,17 @@ defmodule Supavisor.Protocol.Server do
388391
end
389392

390393
@spec decode_payload(:password_message, binary()) ::
391-
{:first_msg_response, map()} | :undefined
394+
{:first_msg_response, map()} | {:cleartext_password, binary()} | :undefined
392395
defp decode_payload(:password_message, bin) do
393396
case kv_to_map(bin) do
394-
{:ok, map} -> {:first_msg_response, map}
395-
{:error, _} -> :undefined
397+
{:ok, map} ->
398+
{:first_msg_response, map}
399+
400+
{:error, _} ->
401+
case :binary.split(bin, <<0>>) do
402+
[password, ""] -> {:cleartext_password, password}
403+
_ -> :undefined
404+
end
396405
end
397406
end
398407

lib/supavisor/secret_checker.ex

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ defmodule Supavisor.SecretChecker do
6565
ip_version: Supavisor.Helpers.ip_version(tenant.ip_version, tenant.db_host),
6666
upstream_ssl: tenant.upstream_ssl,
6767
upstream_verify: tenant.upstream_verify,
68-
upstream_tls_ca: Supavisor.Helpers.upstream_cert(tenant.upstream_tls_ca)
68+
upstream_tls_ca: Supavisor.Helpers.upstream_cert(tenant.upstream_tls_ca),
69+
use_jit: tenant.use_jit,
70+
jit_api_url: tenant.jit_api_url
6971
}
7072
else
7173
nil
@@ -117,12 +119,19 @@ defmodule Supavisor.SecretChecker do
117119
do: Process.send_after(self(), :check, interval + jitter())
118120

119121
def check_secrets(user, %{auth: auth, conn: conn} = state) do
122+
alias Supavisor.ClientHandler.Auth
123+
124+
# get tenant information so that we can check configuration information
125+
# that affects secrets, like :use_jit
126+
t = Supavisor.Tenants.get_tenant_cache(state.tenant, state.auth.sni_hostname)
127+
120128
case Helpers.get_user_secret(conn, auth.auth_query, user) do
121129
{:ok, secret} ->
122130
method =
123-
case secret do
124-
%Auth.MD5Secrets{} -> :auth_query_md5
125-
%Auth.SASLSecrets{} -> :auth_query
131+
case {t.use_jit, secret} do
132+
{true, %Auth.PasswordSecrets{}} -> :auth_query_jit
133+
{_, %Auth.MD5Secrets{}} -> :auth_query_md5
134+
{_, %Auth.SASLSecrets{}} -> :auth_query
126135
end
127136

128137
update_cache =

lib/supavisor/tenants/tenant.ex

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ defmodule Supavisor.Tenants.Tenant do
3333
field(:allow_list, {:array, :string}, default: ["0.0.0.0/0", "::/0"])
3434
field(:availability_zone, :string)
3535
field(:feature_flags, :map, default: %{})
36+
field(:use_jit, :boolean, default: false)
37+
field(:jit_api_url, :string)
3638

3739
has_many(:users, User,
3840
foreign_key: :tenant_external_id,
@@ -67,7 +69,9 @@ defmodule Supavisor.Tenants.Tenant do
6769
:client_heartbeat_interval,
6870
:allow_list,
6971
:availability_zone,
70-
:feature_flags
72+
:feature_flags,
73+
:use_jit,
74+
:jit_api_url
7175
])
7276
|> check_constraint(:upstream_ssl, name: :upstream_constraints, prefix: "_supavisor")
7377
|> 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)