fix(electric): Update SSL logic after upgrading to OTP 27.0 (#1396)

When we upgraded the sync service to OTP 27.0, we missed the changed
default from `verify_none` to `verify_peer` in SSL connections.

We now explicitly set `verify` to `verify_none` because it's currently
the only way to ensure encrypted connections work even when a faulty
certificate chain is presented by the PG host. This behaviour matches
that of `psql <DATABASE_URL>?sslmode=require`.

Here's an example of connecting to DigitalOcean's Managed PostgreSQL to
illustrate the point:
```
$ psql 'postgresql://...?sslmode=require'
Null display is "∅".
Output format is aligned.
Expanded display is used automatically.
Line style is unicode.
Border style is 1.
psql (16.1, server 16.3)
SSL connection (protocol: TLSv1.3, cipher: TLS_AES_256_GCM_SHA384, compression: off)
Type "help" for help.

[db-postgresql-do-user-13160360-0] doadmin:defaultdb=> \q
```
```
$ psql 'postgresql://...?sslmode=verify-full'
psql: error: connection to server at "***.db.ondigitalocean.com" (167.99.250.38), port 25060 failed: root certificate file "/home/alco/.postgresql/root.crt" does not exist
Either provide the file, use the system's trusted roots with sslrootcert=system, or change sslmode to disable server certificate verification.

$ psql 'sslrootcert=system sslmode=verify-full host=***.db.ondigitalocean.com ...'
psql: error: connection to server at "***.db.ondigitalocean.com" (167.99.250.38), port 25060 failed: SSL error: certificate verify failed
```

```
$ openssl s_client -starttls postgres -showcerts -connect ***.db.ondigitalocean.com:25060 -CApath /etc/ssl/certs/
[...]
SSL handshake has read 3990 bytes and written 885 bytes
Verification error: self-signed certificate in certificate chain
```
Fix #1395.

.changeset/warm-months-explain.md

@@ -0,0 +1,5 @@
+---
+"@core/electric": patch
+---
+
+Fix a bug that first appeared in v0.12.1 and prevented Electric from establishing SSL connections to the database.

components/electric/lib/electric/postgres/repo.ex

@@ -28,7 +28,7 @@ defmodule Electric.Postgres.Repo do
       username: conn_opts.username,
       password: conn_opts.password,
       database: conn_opts.database,
-      ssl: conn_opts.ssl == :required,
+      ssl: conn_opts[:ssl_opts] || false,
       # Pass TCP options to the Postgrex adapter. This is used to let the adapter know to
       # connect to the DB using IPv6, for example.
       socket_options: Map.get(conn_opts, :tcp_opts, []),

components/electric/lib/electric/replication/postgres_manager.ex

@@ -135,13 +135,24 @@ defmodule Electric.Replication.PostgresConnectorMng do
     case initialize_postgres(state) do
       {:ok, ssl_used?} ->
         state =
-          state
+          if ssl_used? do
+            # Connector config already has the right SSL configuration in this case.
+            state
+          else
+            # epgsql fell back to using an unencrypted connection. Modify the connector config
+            # accordingly, so that the correct SSL configuration is then used by
+            # `Electric.Postgres.Repo`.
+            fallback_to_nossl(state)
+          end
           |> set_status(:establishing_repl_conn)
-          |> update_connector_config(&force_ssl_mode(&1, ssl_used?))
 
         {:noreply, state, {:continue, :establish_repl_conn}}
 
       {:error, {:ssl_negotiation_failed, _}} when state.conn_opts.ssl != :required ->
+        Logger.warning(
+          "Falling back to trying an unencrypted connection to Postgres, since DATABASE_REQUIRE_SSL=false."
+        )
+
         state = fallback_to_nossl(state)
         {:noreply, state, {:continue, :init}}
 
@@ -305,17 +316,10 @@ defmodule Electric.Replication.PostgresConnectorMng do
       |> Keyword.put(:nulls, [nil, :null, :undefined])
       |> Keyword.put(:ip_addr, ip_addr)
       |> maybe_put_inet6(ip_addr)
-      |> maybe_put_sni()
-      |> maybe_verify_peer()
+      |> update_ssl_opts()
     end)
   end
 
-  def force_ssl_mode(connector_config, ssl_mode?) do
-    new_ssl_value = if ssl_mode?, do: :required, else: false
-
-    put_in(connector_config, [:connection, :ssl], new_ssl_value)
-  end
-
   # Perform a DNS lookup for an IPv6 IP address, followed by a lookup for an IPv4 address in case the first one fails.
   #
   # This is done in order to obviate the need for specifying the exact protocol a given database is reachable over,
@@ -337,30 +341,21 @@ defmodule Electric.Replication.PostgresConnectorMng do
 
   defp maybe_put_inet6(conn_opts, _), do: conn_opts
 
-  defp maybe_put_sni(conn_opts) do
+  defp update_ssl_opts(conn_opts) do
     if conn_opts[:ssl] do
-      sni_opt = {:server_name_indication, String.to_charlist(conn_opts[:host])}
-      update_in(conn_opts, [:ssl_opts], &[sni_opt | List.wrap(&1)])
-    else
-      conn_opts
-    end
-  end
+      ssl_opts =
+        ssl_verify_opts()
+        |> Keyword.put(:server_name_indication, String.to_charlist(conn_opts[:host]))
 
-  defp maybe_verify_peer(conn_opts) do
-    if conn_opts[:ssl] == :required do
-      ssl_opts = get_verify_peer_opts()
-      update_in(conn_opts, [:ssl_opts], &(ssl_opts ++ List.wrap(&1)))
+      Keyword.put(conn_opts, :ssl_opts, ssl_opts)
     else
-      conn_opts
+      Keyword.delete(conn_opts, :ssl_opts)
     end
   end
 
-  defp get_verify_peer_opts do
-    case :public_key.cacerts_load() do
-      :ok ->
-        cacerts = :public_key.cacerts_get()
-        Logger.info("Successfully loaded #{length(cacerts)} cacerts from the OS")
-
+  defp ssl_verify_opts do
+    case load_cacerts() do
+      {:ok, cacerts} ->
         [
           verify: :verify_peer,
           cacerts: cacerts,
@@ -371,22 +366,51 @@ defmodule Electric.Replication.PostgresConnectorMng do
           ]
         ]
 
+      :error ->
+        [verify: :verify_none]
+    end
+  end
+
+  # This attribute silences dialyzer's warning:
+  #
+  #     lib/electric/replication/postgres_manager.ex:381:pattern_match
+  #     The pattern can never match the type.
+  #
+  #     Pattern:
+  #     :undefined
+  #
+  #     Type:
+  #     :ok | {:error, _}
+  #
+  # As explained in https://github.com/erlang/otp/issues/8604, the function spec of
+  # `:public_key.cacerts_load()` is incorrect.
+  @dialyzer {:nowarn_function, load_cacerts: 0}
+
+  defp load_cacerts do
+    case :public_key.cacerts_load() do
+      :ok ->
+        cacerts = :public_key.cacerts_get()
+        Logger.info("Successfully loaded #{length(cacerts)} cacerts from the OS")
+        {:ok, cacerts}
+
       {:error, reason} ->
         Logger.warning("Failed to load cacerts from the OS: #{inspect(reason)}")
-        # We're not sure how reliable OS certificate stores are in general, so keep going even
-        # if the loading of cacerts has failed. A warning will be logged every time a new
-        # database connection is established without the `verify_peer` option, so the issue will be
-        # visible to the developer.
-        []
+        :error
+
+      :undefined ->
+        Logger.warning("Failed to load cacerts from the OS.")
+        :error
     end
   end
 
   defp fallback_to_nossl(state) do
-    Logger.warning(
-      "Falling back to trying an unencrypted connection to Postgres, since DATABASE_REQUIRE_SSL=false."
-    )
-
-    update_connector_config(state, &put_in(&1, [:connection, :ssl], false))
+    update_connector_config(state, fn connector_config ->
+      Keyword.update!(connector_config, :connection, fn conn_opts ->
+        conn_opts
+        |> Keyword.put(:ssl, false)
+        |> update_ssl_opts()
+      end)
+    end)
   end
 
   defp extra_error_description(:invalid_authorization_specification) do