From e3e7c7143d647d21fa0b8284f618fdd88003d04c Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Mon, 20 May 2024 10:46:18 +0200 Subject: [PATCH] Streamline SSL experience --- lib/myxql.ex | 15 +++++++---- lib/myxql/client.ex | 52 ++++++++++++++++++++++++++++++-------- lib/myxql/protocol.ex | 4 +-- lib/myxql/protocol/auth.ex | 11 ++++---- test/myxql/client_test.exs | 20 +++++++-------- test/myxql_test.exs | 14 +++++++--- test/test_helper.exs | 13 +++++++--- 7 files changed, 89 insertions(+), 40 deletions(-) diff --git a/lib/myxql.ex b/lib/myxql.ex index c636d71..93174f2 100644 --- a/lib/myxql.ex +++ b/lib/myxql.ex @@ -16,8 +16,7 @@ defmodule MyXQL do | {:password, String.t() | nil} | {:charset, String.t() | nil} | {:collation, String.t() | nil} - | {:ssl, boolean()} - | {:ssl_opts, [:ssl.tls_client_option()]} + | {:ssl, boolean | [:ssl.tls_client_option()]} | {:connect_timeout, timeout()} | {:handshake_timeout, timeout()} | {:ping_timeout, timeout()} @@ -70,9 +69,10 @@ defmodule MyXQL do * `:collation` - A connection collation. Must be given with `:charset` option, and if set it overwrites the default collation for the given charset. (default: `nil`) - * `:ssl` - Set to `true` if SSL should be used (default: `false`) - - * `:ssl_opts` - A list of SSL options, see `:ssl.connect/2` (default: `[]`) + * `:ssl` - Enables SSL. Setting it to `true` enables SSL without server certificate verification, + which emits a warning. Instead, prefer to set it to a keyword list, with either + `:cacerts` or `:cacertfile` set to a CA trust store, to enable server certificate + verification. (default: `false`) * `:connect_timeout` - Socket connect timeout in milliseconds (default: `15_000`) @@ -127,6 +127,11 @@ defmodule MyXQL do iex> {:ok, pid} = MyXQL.start_link(protocol: :tcp) {:ok, #PID<0.69.0>} + Start connection with SSL using CA certificate file: + + iex> {:ok, pid} = MyXQL.start_link(ssl: [cacertfile: System.fetch_env!("DB_CA_CERT_FILE")]) + {:ok, #PID<0.69.0>} + Run a query after connection has been established: iex> {:ok, pid} = MyXQL.start_link(after_connect: &MyXQL.query!(&1, "SET time_zone = '+00:00'")) diff --git a/lib/myxql/client.ex b/lib/myxql/client.ex index d9cb773..596e014 100644 --- a/lib/myxql/client.ex +++ b/lib/myxql/client.ex @@ -20,7 +20,6 @@ defmodule MyXQL.Client do :username, :password, :database, - :ssl?, :ssl_opts, :connect_timeout, :handshake_timeout, @@ -36,6 +35,24 @@ defmodule MyXQL.Client do def new(opts) do {address, port} = address_and_port(opts) + {ssl_opts, opts} = + case Keyword.pop(opts, :ssl, false) do + {false, opts} -> + {nil, opts} + + {true, opts} -> + Logger.warning( + "setting ssl: true on your database connection offers only limited protection, " <> + "as the server's certificate is not verified. Set \"ssl: [cacertfile: \"/path/to/cacert.crt\"]\" instead" + ) + + # Read ssl_opts for backwards compatibility + Keyword.pop(opts, :ssl_opts, []) + + {ssl_opts, opts} when is_list(ssl_opts) -> + {Keyword.merge(default_ssl_opts(), ssl_opts), opts} + end + %__MODULE__{ address: address, port: port, @@ -43,8 +60,7 @@ defmodule MyXQL.Client do Keyword.get(opts, :username, System.get_env("USER")) || raise(":username is missing"), password: nilify(Keyword.get(opts, :password, System.get_env("MYSQL_PWD"))), database: Keyword.get(opts, :database), - ssl?: Keyword.get(opts, :ssl, false), - ssl_opts: Keyword.get(opts, :ssl_opts, []), + ssl_opts: ssl_opts, connect_timeout: Keyword.get(opts, :connect_timeout, @default_timeout), handshake_timeout: Keyword.get(opts, :handshake_timeout, @default_timeout), socket_options: (opts[:socket_options] || []) ++ @sock_opts, @@ -54,6 +70,15 @@ defmodule MyXQL.Client do } end + defp default_ssl_opts do + [ + verify: :verify_peer, + customize_hostname_check: [ + match_fun: :public_key.pkix_verify_hostname_match_fun(:https) + ] + ] + end + defp nilify(""), do: nil defp nilify(other), do: other @@ -379,9 +404,20 @@ defmodule MyXQL.Client do com_query(client, "SET NAMES '#{charset}' COLLATE '#{collation}'") end - defp maybe_upgrade_to_ssl(client, %{ssl?: true} = config, capability_flags, sequence_id) do + defp maybe_upgrade_to_ssl(client, %{ssl_opts: nil}, _capability_flags, sequence_id) do + {:ok, sequence_id, client} + end + + defp maybe_upgrade_to_ssl(client, %{ssl_opts: ssl_opts} = config, capability_flags, sequence_id) do {_, sock} = client.sock + ssl_opts = + if is_list(config.address) do + Keyword.put_new(ssl_opts, :server_name_indication, config.address) + else + ssl_opts + end + ssl_request = ssl_request( capability_flags: capability_flags, @@ -392,15 +428,11 @@ defmodule MyXQL.Client do payload = encode_ssl_request(ssl_request) with :ok <- send_packet(client, payload, sequence_id), - {:ok, ssl_sock} <- :ssl.connect(sock, config.ssl_opts, config.connect_timeout) do + {:ok, ssl_sock} <- :ssl.connect(sock, ssl_opts, config.connect_timeout) do {:ok, sequence_id + 1, %{client | sock: {:ssl, ssl_sock}}} end end - defp maybe_upgrade_to_ssl(client, %{ssl?: false}, _capability_flags, sequence_id) do - {:ok, sequence_id, client} - end - defp recv_handshake(client) do recv_packet(client, &decode_initial_handshake/1) end @@ -477,7 +509,7 @@ defmodule MyXQL.Client do defp perform_full_auth(client, config, "caching_sha2_password", auth_plugin_data, sequence_id) do auth_response = - if config.ssl? do + if config.ssl_opts do [config.password, 0] else # request public key diff --git a/lib/myxql/protocol.ex b/lib/myxql/protocol.ex index b09eb08..74dd083 100644 --- a/lib/myxql/protocol.ex +++ b/lib/myxql/protocol.ex @@ -179,9 +179,9 @@ defmodule MyXQL.Protocol do :client_transactions ]) |> maybe_put_capability_flag(:client_connect_with_db, !is_nil(config.database)) - |> maybe_put_capability_flag(:client_ssl, config.ssl?) + |> maybe_put_capability_flag(:client_ssl, is_list(config.ssl_opts)) - if config.ssl? && !has_capability_flag?(server_capability_flags, :client_ssl) do + if config.ssl_opts && !has_capability_flag?(server_capability_flags, :client_ssl) do {:error, :server_does_not_support_ssl} else client_capability_flags = diff --git a/lib/myxql/protocol/auth.ex b/lib/myxql/protocol/auth.ex index 6168a63..9cffb4f 100644 --- a/lib/myxql/protocol/auth.ex +++ b/lib/myxql/protocol/auth.ex @@ -39,11 +39,12 @@ defmodule MyXQL.Protocol.Auth do auth_plugin_name == "mysql_native_password" -> mysql_native_password(config.password, initial_auth_plugin_data) - auth_plugin_name == "sha256_password" and config.ssl? -> - config.password <> <<0>> - - auth_plugin_name == "sha256_password" and not config.ssl? -> - <<1>> + auth_plugin_name == "sha256_password" -> + if config.ssl_opts do + config.password <> <<0>> + else + <<1>> + end auth_plugin_name == "caching_sha2_password" -> sha256_password(config.password, initial_auth_plugin_data) diff --git a/test/myxql/client_test.exs b/test/myxql/client_test.exs index 21fedb3..d4de415 100644 --- a/test/myxql/client_test.exs +++ b/test/myxql/client_test.exs @@ -4,6 +4,7 @@ defmodule MyXQL.ClientTest do import MyXQL.Protocol.{Flags, Records} @opts TestHelper.opts() + @opts_with_ssl TestHelper.opts_with_ssl() describe "connect" do @tag public_key_exchange: true @@ -15,7 +16,7 @@ defmodule MyXQL.ClientTest do @tag ssl: true test "default auth plugin (ssl)" do - opts = [username: "default_auth", password: "secret", ssl: true] ++ @opts + opts = [username: "default_auth", password: "secret"] ++ @opts_with_ssl assert {:ok, client} = Client.connect(opts) Client.com_quit(client) end @@ -47,7 +48,7 @@ defmodule MyXQL.ClientTest do @tag ssl: true test "no password (ssl)" do - opts = [username: "nopassword", ssl: true] ++ @opts + opts = [username: "nopassword"] ++ @opts_with_ssl assert {:ok, client} = Client.connect(opts) Client.com_quit(client) @@ -73,7 +74,7 @@ defmodule MyXQL.ClientTest do @tag mysql_native_password: true, ssl: true test "mysql_native_password (ssl)" do - opts = [username: "mysql_native", password: "secret", ssl: true] ++ @opts + opts = [username: "mysql_native", password: "secret"] ++ @opts_with_ssl assert {:ok, client} = Client.connect(opts) Client.com_quit(client) end @@ -106,7 +107,7 @@ defmodule MyXQL.ClientTest do @tag sha256_password: true, ssl: true test "sha256_password (ssl)" do - opts = [username: "sha256_password", password: "secret", ssl: true] ++ @opts + opts = [username: "sha256_password", password: "secret"] ++ @opts_with_ssl assert {:ok, client} = Client.connect(opts) Client.com_quit(client) end @@ -119,13 +120,13 @@ defmodule MyXQL.ClientTest do @tag sha256_password: true, ssl: true test "sha256_password (bad password) (ssl)" do - opts = [username: "sha256_password", password: "bad", ssl: true] ++ @opts + opts = [username: "sha256_password", password: "bad"] ++ @opts_with_ssl {:error, err_packet(message: "Access denied" <> _)} = Client.connect(opts) end @tag sha256_password: true, ssl: true test "sha256_password (empty password) (ssl)" do - opts = [username: "sha256_empty", ssl: true] ++ @opts + opts = [username: "sha256_empty"] ++ @opts_with_ssl assert {:ok, client} = Client.connect(opts) Client.com_quit(client) end @@ -156,7 +157,7 @@ defmodule MyXQL.ClientTest do @tag caching_sha2_password: true, ssl: true test "caching_sha2_password (ssl)" do - opts = [username: "caching_sha2_password", password: "secret", ssl: true] ++ @opts + opts = [username: "caching_sha2_password", password: "secret"] ++ @opts_with_ssl assert {:ok, client} = Client.connect(opts) Client.com_quit(client) end @@ -169,7 +170,7 @@ defmodule MyXQL.ClientTest do @tag caching_sha2_password: true, ssl: true test "caching_sha2_password (bad password) (ssl)" do - opts = [username: "caching_sha2_password", password: "bad", ssl: true] ++ @opts + opts = [username: "caching_sha2_password", password: "bad"] ++ @opts_with_ssl {:error, err_packet(message: "Access denied" <> _)} = Client.connect(opts) end @@ -194,8 +195,7 @@ defmodule MyXQL.ClientTest do @tag ssl: false test "client requires ssl but server does not support it" do - opts = [ssl: true] ++ @opts - assert {:error, :server_does_not_support_ssl} = Client.connect(opts) + assert {:error, :server_does_not_support_ssl} = Client.connect(@opts_with_ssl) end test "default charset" do diff --git a/test/myxql_test.exs b/test/myxql_test.exs index 61b414b..c7bda07 100644 --- a/test/myxql_test.exs +++ b/test/myxql_test.exs @@ -3,12 +3,13 @@ defmodule MyXQLTest do import ExUnit.CaptureLog @opts TestHelper.opts() + @opts_with_ssl TestHelper.opts_with_ssl() describe "connect" do @tag ssl: true test "connect with bad SSL opts" do assert capture_log(fn -> - opts = [ssl: true, ssl_opts: [ciphers: [:bad]]] ++ @opts + opts = put_in(@opts_with_ssl[:ssl][:ciphers], [:bad]) assert_start_and_killed(opts) end) =~ "** (DBConnection.ConnectionError) (127.0.0.1:3306) Invalid TLS option: {ciphers,[bad]}" @@ -164,7 +165,9 @@ defmodule MyXQLTest do test "#{@protocol}: query with multiple rows", c do %MyXQL.Result{num_rows: 2} = - MyXQL.query!(c.conn, "INSERT INTO integers VALUES (10), (20)", [], query_type: @protocol) + MyXQL.query!(c.conn, "INSERT INTO integers VALUES (10), (20)", [], + query_type: @protocol + ) assert {:ok, %MyXQL.Result{columns: ["x"], rows: [[10], [20]]}} = MyXQL.query(c.conn, "SELECT * FROM integers") @@ -176,7 +179,9 @@ defmodule MyXQLTest do values = Enum.map_join(1..num, ", ", &"(#{&1})") result = - MyXQL.query!(c.conn, "INSERT INTO integers VALUES " <> values, [], query_type: @protocol) + MyXQL.query!(c.conn, "INSERT INTO integers VALUES " <> values, [], + query_type: @protocol + ) assert result.num_rows == num @@ -784,8 +789,9 @@ defmodule MyXQLTest do end end - @tag :skip describe "idle ping" do + @describetag :skip + test "query before and after" do opts = [backoff_type: :stop, idle_interval: 1] ++ @opts {:ok, pid} = MyXQL.start_link(opts) diff --git a/test/test_helper.exs b/test/test_helper.exs index 9370cc3..b6b89b1 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1,11 +1,10 @@ defmodule TestHelper do - def opts() do + def opts do [ hostname: "127.0.0.1", username: "root", database: "myxql_test", timeout: 5000, - ssl_opts: ssl_opts(), backoff_type: :stop, max_restarts: 0, pool_size: 1, @@ -13,8 +12,14 @@ defmodule TestHelper do ] end - defp ssl_opts() do - [versions: [:"tlsv1.2"]] + def opts_with_ssl do + opts() ++ + [ + ssl: [ + verify: :verify_none, + versions: [:"tlsv1.2"] + ] + ] end def setup_server() do