From 0aeb3c06f9ceeb0f0cb81685bf3971b48d263bb3 Mon Sep 17 00:00:00 2001 From: Stas Date: Fri, 21 Jun 2024 15:00:27 +0200 Subject: [PATCH 1/5] feat: validate username and db_name format --- VERSION | 2 +- lib/supavisor/client_handler.ex | 12 +++++++----- test/integration/proxy_test.exs | 7 +++---- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/VERSION b/VERSION index 0516ac10..0041a435 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.1.65 +1.1.66 diff --git a/lib/supavisor/client_handler.ex b/lib/supavisor/client_handler.ex index 4c46f5fe..b99c99bb 100644 --- a/lib/supavisor/client_handler.ex +++ b/lib/supavisor/client_handler.ex @@ -154,14 +154,16 @@ defmodule Supavisor.ClientHandler do Logger.debug("ClientHandler: Client startup message: #{inspect(hello)}") {type, {user, tenant_or_alias, db_name}} = HH.parse_user_info(hello.payload) - not_allowed = ["\"", "\\"] + # Validate user and db_name according to PostgreSQL rules. + # The rules are: 1-63 characters, alphanumeric and underscore + rule = ~r/^[a-z_][a-z0-9_$]*$/ - if String.contains?(user, not_allowed) or String.contains?(db_name, not_allowed) do - reason = "Invalid characters in user or db_name" + if !String.match?(user, rule) or !String.match?(db_name, rule) do + reason = "Invalid format for user or db_name" Logger.error("ClientHandler: #{inspect(reason)}") - Telem.client_join(:fail, data.id) + Telem.client_join(:fail, tenant_or_alias) HH.send_error(data.sock, "XX000", "Authentication error, reason: #{inspect(reason)}") - {:stop, {:shutdown, :invalid_characters}} + {:stop, {:shutdown, :invalid_format}} else log_level = case hello.payload["options"]["log_level"] do diff --git a/test/integration/proxy_test.exs b/test/integration/proxy_test.exs index 689912f8..168146ba 100644 --- a/test/integration/proxy_test.exs +++ b/test/integration/proxy_test.exs @@ -249,12 +249,12 @@ defmodule Supavisor.Integration.ProxyTest do assert [%Postgrex.Result{rows: [["1"]]}] = P.SimpleConnection.call(pid, {:query, "select 1;"}) end - test "invalid characters in user or db_name" do + test "invalid format for user or db_name" do Process.flag(:trap_exit, true) db_conf = Application.get_env(:supavisor, Repo) url = - "postgresql://user\"user.#{@tenant}:#{db_conf[:password]}@#{db_conf[:hostname]}:#{Application.get_env(:supavisor, :proxy_port_transaction)}/postgres\\\\\\\\\"\\" + "postgresql://user\"user.#{@tenant}:#{db_conf[:password]}@#{db_conf[:hostname]}:#{Application.get_env(:supavisor, :proxy_port_transaction)}/post gres\\\\\\\\\"\\" assert {:error, {_, @@ -262,8 +262,7 @@ defmodule Supavisor.Integration.ProxyTest do %Postgrex.Error{ postgres: %{ code: :internal_error, - message: - "Authentication error, reason: \"Invalid characters in user or db_name\"", + message: "Authentication error, reason: \"Invalid format for user or db_name\"", pg_code: "XX000", severity: "FATAL", unknown: "FATAL" From c79ff1dfda9e1d3e9a9bef9b2f21e61d13586ced Mon Sep 17 00:00:00 2001 From: Stas Date: Fri, 21 Jun 2024 15:03:49 +0200 Subject: [PATCH 2/5] update description --- lib/supavisor/client_handler.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/supavisor/client_handler.ex b/lib/supavisor/client_handler.ex index b99c99bb..5cccb354 100644 --- a/lib/supavisor/client_handler.ex +++ b/lib/supavisor/client_handler.ex @@ -155,7 +155,7 @@ defmodule Supavisor.ClientHandler do {type, {user, tenant_or_alias, db_name}} = HH.parse_user_info(hello.payload) # Validate user and db_name according to PostgreSQL rules. - # The rules are: 1-63 characters, alphanumeric and underscore + # The rules are: 1-63 characters, alphanumeric, underscore and $ rule = ~r/^[a-z_][a-z0-9_$]*$/ if !String.match?(user, rule) or !String.match?(db_name, rule) do From 1da956fee74be136a9be38cb42d1f173ac4948ea Mon Sep 17 00:00:00 2001 From: Stas Date: Fri, 21 Jun 2024 16:18:23 +0200 Subject: [PATCH 3/5] update if statement --- lib/supavisor/client_handler.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/supavisor/client_handler.ex b/lib/supavisor/client_handler.ex index 5cccb354..cb727a55 100644 --- a/lib/supavisor/client_handler.ex +++ b/lib/supavisor/client_handler.ex @@ -158,7 +158,7 @@ defmodule Supavisor.ClientHandler do # The rules are: 1-63 characters, alphanumeric, underscore and $ rule = ~r/^[a-z_][a-z0-9_$]*$/ - if !String.match?(user, rule) or !String.match?(db_name, rule) do + if !(user =~ rule && db_name =~ rule) do do reason = "Invalid format for user or db_name" Logger.error("ClientHandler: #{inspect(reason)}") Telem.client_join(:fail, tenant_or_alias) From e5b6c1f1776752313b88e786e18300f800a2da6c Mon Sep 17 00:00:00 2001 From: Stas Date: Fri, 21 Jun 2024 16:22:35 +0200 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=A4=A6=E2=80=8D=E2=99=82=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/supavisor/client_handler.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/supavisor/client_handler.ex b/lib/supavisor/client_handler.ex index cb727a55..7e95ebc6 100644 --- a/lib/supavisor/client_handler.ex +++ b/lib/supavisor/client_handler.ex @@ -158,7 +158,7 @@ defmodule Supavisor.ClientHandler do # The rules are: 1-63 characters, alphanumeric, underscore and $ rule = ~r/^[a-z_][a-z0-9_$]*$/ - if !(user =~ rule && db_name =~ rule) do do + if !(user =~ rule && db_name =~ rule) do reason = "Invalid format for user or db_name" Logger.error("ClientHandler: #{inspect(reason)}") Telem.client_join(:fail, tenant_or_alias) From 46d0e331d03d28b8b0b6fa3f8d314bd590af4250 Mon Sep 17 00:00:00 2001 From: Stas Date: Fri, 21 Jun 2024 16:52:50 +0200 Subject: [PATCH 5/5] swap cases --- lib/supavisor/client_handler.ex | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/supavisor/client_handler.ex b/lib/supavisor/client_handler.ex index 7e95ebc6..495f98c0 100644 --- a/lib/supavisor/client_handler.ex +++ b/lib/supavisor/client_handler.ex @@ -158,13 +158,7 @@ defmodule Supavisor.ClientHandler do # The rules are: 1-63 characters, alphanumeric, underscore and $ rule = ~r/^[a-z_][a-z0-9_$]*$/ - if !(user =~ rule && db_name =~ rule) do - reason = "Invalid format for user or db_name" - Logger.error("ClientHandler: #{inspect(reason)}") - Telem.client_join(:fail, tenant_or_alias) - HH.send_error(data.sock, "XX000", "Authentication error, reason: #{inspect(reason)}") - {:stop, {:shutdown, :invalid_format}} - else + if user =~ rule and db_name =~ rule do log_level = case hello.payload["options"]["log_level"] do nil -> nil @@ -175,6 +169,12 @@ defmodule Supavisor.ClientHandler do {:keep_state, %{data | log_level: log_level}, {:next_event, :internal, {:hello, {type, {user, tenant_or_alias, db_name}}}}} + else + reason = "Invalid format for user or db_name" + Logger.error("ClientHandler: #{inspect(reason)}") + Telem.client_join(:fail, tenant_or_alias) + HH.send_error(data.sock, "XX000", "Authentication error, reason: #{inspect(reason)}") + {:stop, {:shutdown, :invalid_format}} end {:error, error} ->