Skip to content

Commit

Permalink
Always set server_name_indicator based on hostname
Browse files Browse the repository at this point in the history
  • Loading branch information
josevalim committed May 17, 2024
1 parent 80f748c commit 325a7e2
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 71 deletions.
55 changes: 5 additions & 50 deletions lib/postgrex.ex
Original file line number Diff line number Diff line change
Expand Up @@ -191,37 +191,13 @@ defmodule Postgrex do
## SSL client authentication
When connecting to Postgres or CockroachDB instances over SSL it is idiomatic to use
certificate authentication. Config files do not allowing passing functions,
so use the `init` callback of the Ecto supervisor.
certificate authentication:
In your Repository configuration:
[ssl: [cacertfile: System.get_env("DB_CA_CERT_FILE")]]
config :app, App.Repo,
ssl: String.to_existing_atom(System.get_env("DB_SSL_ENABLED", "true")),
verify_ssl: true
And in App.Repo, set your `:ssl_opts`:
def init(_type, config) do
config =
if config[:verify_ssl] do
Keyword.put(config, :ssl_opts, my_ssl_opts(config[:hostname]))
else
config
end
{:ok, config}
end
def my_ssl_opts(server) do
[
verify: :verify_peer,
cacertfile: System.get_env("DB_CA_CERT_FILE"),
server_name_indication: String.to_charlist(server),
customize_hostname_check: [match_fun: :public_key.pkix_verify_hostname_match_fun(:https)],
depth: 3
]
end
The server name indication (SNI) will be automatically set based on the `:hostname`
configuration, if one was provided. Other options, such as `depth: 3`, may be necessary
depending on the server.
## PgBouncer
Expand Down Expand Up @@ -269,27 +245,6 @@ defmodule Postgrex do
{"test-instance-N.xyz.eu-west-1.rds.amazonaws.com", 5432}
]
### Failover with SSL support
As specified in Erlang [:ssl.connect](https://erlang.org/doc/man/ssl.html#connect-3),
host verification using `:public_key.pkix_verify_hostname_match_fun(:https)`
requires that the ssl_opt `server_name_indication` is set, and for this reason,
the aforementioned `endpoints` list can become a three element tuple as:
endpoints: [
{
"test-instance-1.xyz.eu-west-1.rds.amazonaws.com",
5432,
[ssl_opts: [server_name_indication: String.to_charlist("test-instance-1.xyz.eu-west-1.rds.amazonaws.com")]]
},
(...),
{
"test-instance-2.xyz.eu-west-1.rds.amazonaws.com",
5432,
[ssl_opts: [server_name_indication: String.to_charlist("test-instance-2.xyz.eu-west-1.rds.amazonaws.com")]]
}
]
"""
@spec start_link([start_option]) :: {:ok, pid} | {:error, Postgrex.Error.t() | term}
def start_link(opts) do
Expand Down
48 changes: 27 additions & 21 deletions lib/postgrex/protocol.ex
Original file line number Diff line number Diff line change
Expand Up @@ -143,36 +143,40 @@ defmodule Postgrex.Protocol do
end

defp default_ssl_opts(opts) do

Check warning on line 145 in lib/postgrex/protocol.ex

View workflow job for this annotation

GitHub Actions / test (9.4, skip_wal, 1.11.4, 23.3.3)

variable "opts" is unused (if the variable is not meant to be used, prefix it with an underscore)

Check warning on line 145 in lib/postgrex/protocol.ex

View workflow job for this annotation

GitHub Actions / test (9.5, skip_wal, 1.11.4, 23.3.3)

variable "opts" is unused (if the variable is not meant to be used, prefix it with an underscore)

Check warning on line 145 in lib/postgrex/protocol.ex

View workflow job for this annotation

GitHub Actions / test (9.6, skip_wal, 1.11.4, 23.3.3)

variable "opts" is unused (if the variable is not meant to be used, prefix it with an underscore)

Check warning on line 145 in lib/postgrex/protocol.ex

View workflow job for this annotation

GitHub Actions / test (10, 1.11.4, 23.3.3)

variable "opts" is unused (if the variable is not meant to be used, prefix it with an underscore)

Check warning on line 145 in lib/postgrex/protocol.ex

View workflow job for this annotation

GitHub Actions / test (11, 1.11.4, 23.3.3)

variable "opts" is unused (if the variable is not meant to be used, prefix it with an underscore)

Check warning on line 145 in lib/postgrex/protocol.ex

View workflow job for this annotation

GitHub Actions / test (12, 1.11.4, 23.3.3)

variable "opts" is unused (if the variable is not meant to be used, prefix it with an underscore)

Check warning on line 145 in lib/postgrex/protocol.ex

View workflow job for this annotation

GitHub Actions / test (13, 1.11.4, 23.3.3)

variable "opts" is unused (if the variable is not meant to be used, prefix it with an underscore)

Check warning on line 145 in lib/postgrex/protocol.ex

View workflow job for this annotation

GitHub Actions / test (14, 1.11.4, 23.3.3)

variable "opts" is unused (if the variable is not meant to be used, prefix it with an underscore)
case Keyword.fetch(opts, :hostname) do
{:ok, hostname} -> [server_name_indication: String.to_charlist(hostname)]
:error -> []
end ++
[
verify: :verify_peer,
customize_hostname_check: [
match_fun: :public_key.pkix_verify_hostname_match_fun(:https)
]
[
verify: :verify_peer,
customize_hostname_check: [
match_fun: :public_key.pkix_verify_hostname_match_fun(:https)
]
]
end

defp endpoints(opts) do
port = opts[:port] || 5432

case Keyword.fetch(opts, :socket) do
{:ok, file} ->
[{{:local, file}, 0, []}]
[{{:local, file}, 0}]

:error ->
case Keyword.fetch(opts, :socket_dir) do
{:ok, dir} ->
[{{:local, "#{dir}/.s.PGSQL.#{port}"}, 0, []}]
[{{:local, "#{dir}/.s.PGSQL.#{port}"}, 0}]

:error ->
case Keyword.fetch(opts, :endpoints) do
{:ok, endpoints} when is_list(endpoints) ->
Enum.map(endpoints, fn
{hostname, port} -> {to_charlist(hostname), port, []}
{hostname, port, extra_opts} -> {to_charlist(hostname), port, extra_opts}
{hostname, port} ->
{to_charlist(hostname), port}

{hostname, port, _extra_opts} ->
Logger.warn(
"Returning a triplet from :endpoints is deprecated, " <>
"the server name indicator is automatically set based on the hostname if SSL is enabled"
)

{to_charlist(hostname), port}
end)

{:ok, _} ->
Expand All @@ -181,7 +185,7 @@ defmodule Postgrex.Protocol do
:error ->
case Keyword.fetch(opts, :hostname) do
{:ok, hostname} ->
[{to_charlist(hostname), port, []}]
[{to_charlist(hostname), port}]

:error ->
raise ArgumentError,
Expand All @@ -193,15 +197,14 @@ defmodule Postgrex.Protocol do
end

defp connect_endpoints(
[{host, port, extra_opts} | remaining_endpoints],
[{host, port} | remaining_endpoints],
sock_opts,
timeout,
s,
%{opts: opts, types_mod: types_mod} = status,
previous_errors
) do
with {:ok, database} <- fetch_database(opts),
opts = Config.Reader.merge(opts, extra_opts),
status = %{status | types_key: if(types_mod, do: {host, port, database}), opts: opts},
{:ok, ret} <- connect_and_handshake(host, port, sock_opts, timeout, s, status) do
{:ok, ret}
Expand Down Expand Up @@ -246,7 +249,7 @@ defmodule Postgrex.Protocol do
defp connect_and_handshake(host, port, sock_opts, timeout, s, status) do
case connect(host, port, sock_opts, timeout, s) do
{:ok, s} ->
handshake(s, status)
handshake(host, s, status)

{:error, _} = error ->
error
Expand Down Expand Up @@ -712,13 +715,13 @@ defmodule Postgrex.Protocol do

## handshake

defp handshake(%{sock: {:gen_tcp, sock}, timeout: timeout} = s, status) do
defp handshake(host, %{sock: {:gen_tcp, sock}, timeout: timeout} = s, status) do
{:ok, peer} = :inet.peername(sock)
%{opts: opts} = status
handshake_timeout = Keyword.get(opts, :handshake_timeout, timeout)
timer = start_handshake_timer(handshake_timeout, sock)

case do_handshake(%{s | peer: peer}, status) do
case do_handshake(host, %{s | peer: peer}, status) do
{:ok, %{parameters: parameters} = s} ->
cancel_handshake_timer(timer)
ref = Postgrex.Parameters.insert(parameters)
Expand Down Expand Up @@ -763,8 +766,11 @@ defmodule Postgrex.Protocol do
:ok
end

defp do_handshake(s, %{ssl: nil} = status), do: startup(s, status)
defp do_handshake(s, %{ssl: ssl_opts} = status), do: ssl(s, status, ssl_opts)
defp do_handshake(_host, s, %{ssl: nil} = status), do: startup(s, status)

defp do_handshake(host, s, %{ssl: ssl_opts} = status) do
ssl(s, status, Keyword.put_new(ssl_opts, :server_name_indicator, host))
end

## ssl

Expand Down

0 comments on commit 325a7e2

Please sign in to comment.