Add :verify_peer config option and handle expired certificates better. Fixes #116

Author: kipcole9

CHANGELOG.md

@@ -1,3 +1,15 @@
+# Changelog for Money v5.2.0
+
+This is the changelog for Money v5.2.0 released on May 30th, 2020.  For older changelogs please consult the release tag on [GitHub](https://github.com/kipcole9/money/tags)
+
+### Enhancements
+
+* Adds a configuration option `:verify_peer` which is a boolean that determines whether to verify the client certificate for any exchange rate service API call. The default is `true`. This option should not be changed without a very clear understanding of the security implications. This option will remain undocumented but supported for now.
+
+### Bug fixes
+
+* Handle expired certificate errors on the exchange rates API service and log them. Thanks to @coladarci. Fixes #116
+
 # Changelog for Money v5.1.0
 
 This is the changelog for Money v5.1.0 released on May 26th, 2020.  For older changelogs please consult the release tag on [GitHub](https://github.com/kipcole9/money/tags)

lib/money/exchange_rates.ex

@@ -187,7 +187,8 @@ defmodule Money.ExchangeRates do
             log_levels: map(),
             preload_historic_rates: Date.t() | Date.Range.t() | {Date.t(), Date.t()} | nil,
             retriever_options: map() | nil,
-            cache_module: module() | nil
+            cache_module: module() | nil,
+            verify_peer: boolean()
           }
 
     defstruct retrieve_every: nil,
@@ -196,7 +197,8 @@ defmodule Money.ExchangeRates do
               log_levels: %{},
               preload_historic_rates: nil,
               retriever_options: nil,
-              cache_module: nil
+              cache_module: nil,
+              verify_peer: true
   end
 
   @doc """
@@ -214,7 +216,8 @@ defmodule Money.ExchangeRates do
         success: Money.get_env(:log_success, nil),
         failure: Money.get_env(:log_failure, :warn),
         info: Money.get_env(:log_info, :info)
-      }
+        },
+      verify_peer: Money.get_env(:verify_peer, true, :boolean)
     }
   end
 

lib/money/exchange_rates/exchange_rates_retriever.ex

@@ -195,7 +195,7 @@ defmodule Money.ExchangeRates.Retriever do
   def retrieve_rates(url, config) when is_list(url) do
     headers = if_none_match_header(url)
 
-    :httpc.request(:get, {url, headers}, https_opts(), [])
+    :httpc.request(:get, {url, headers}, https_opts(config), [])
     |> process_response(url, config)
   end
 
@@ -222,6 +222,10 @@ defmodule Money.ExchangeRates.Retriever do
     {:error, sys_message}
   end
 
+  defp process_response({:error, {:tls_alert, {:certificate_expired, _message}}}, url, _config) do
+    {:error, "Certificate for #{inspect url} has expired"}
+  end
+
   defp if_none_match_header(url) do
     case get_etag(url) do
       {etag, date} ->
@@ -562,16 +566,23 @@ defmodule Money.ExchangeRates.Retriever do
     file
   end
 
-  defp https_opts do
-    [ssl:
+  defp https_opts(%Money.ExchangeRates.Config{verify_peer: true}) do
+    [
+      ssl:
       [
         verify: :verify_peer,
         cacertfile: certificate_store(),
         depth: 99,
+        log_level: :alert,
+        log_alert: false,
         customize_hostname_check: [
           match_fun: :public_key.pkix_verify_hostname_match_fun(:https)
         ]
       ]
     ]
   end
+
+  defp https_opts(%Money.ExchangeRates.Config{verify_peer: false}) do
+    []
+  end
 end

mix.exs

@@ -1,7 +1,7 @@
 defmodule Money.Mixfile do
   use Mix.Project
 
-  @version "5.1.0"
+  @version "5.2.0"
 
   def project do
     [

test/money_exchange_rates_test.exs

@@ -76,9 +76,13 @@ defmodule Money.ExchangeRates.Test do
       config = Money.ExchangeRates.OpenExchangeRates.init(Money.ExchangeRates.default_config())
       config = Map.put(config, :log_levels, %{failure: nil, info: nil, success: nil})
 
+      # Testing only, should not be used in production
+      # config = Map.put(config, :verify_peer, false)
+
       case Money.ExchangeRates.OpenExchangeRates.get_latest_rates(config) do
         {:ok, rates} -> assert is_map(rates)
         {:error, :nxdomain} -> :no_network
+        {:error, other} -> IO.warn(inspect(other))
       end
     end
   end