Upgrade to support Erlang/OTP 25

README.md

@@ -16,6 +16,17 @@ By itself, `:ssh_client_key_api` does not provide SSH functionality, it only add
 a way to send private key information to an SSH connection. It is meant to be
 used alongside an SSH library such as `:ssh`, `SSHex`, `SSHKit`, or the like.
 
+Note: Upgrade to ssh_client_key_api 0.3.0 or higher for use with Erlang/OTP 25
+
+## Supported Key Types
+
+- rsa - with or without passphrase
+- ed25519 - only supported without a passphrase
+- ecdsa - only supported without a passphrase
+- dsa - with or without passphrase (but DSA keys are [not recommended](https://security.stackexchange.com/a/46781))
+  - OpenSSH 7.0 and higher no longer accept DSA keys by default
+  - Note tested on OTP 25+, but still expected to work
+
 ## Installation
 
 The package can be installed by adding `:ssh_client_key_api` to your list of
@@ -35,8 +46,9 @@ end
 `with_options/1`. See `with_options/1` for full list of available options.
 
 ```elixir
-key = File.open!("path/to/keyfile.pem")
-known_hosts = File.open!("path/to/known_hosts")
+key = File.open!("path/to/id_rsa") # Other key types supported as well
+known_hosts = File.open!("path/to/known_hosts", [:read, :write])
+
 cb = SSHClientKeyAPI.with_options(
   identity: key,
   known_hosts: known_hosts,

lib/ssh_client_key_api.ex

@@ -1,17 +1,20 @@
+# Note: Logger.warn exceptions because the :ssh_client_key_api will silently
+# catches exceptions
 defmodule SSHClientKeyAPI do
   @external_resource "README.md"
   @moduledoc "README.md"
              |> File.read!()
              |> String.split("<!-- MDOC !-->")
              |> Enum.fetch!(1)
 
-  alias SSHClientKeyAPI.KeyError
+  require Logger
+  require Record
 
   @behaviour :ssh_client_key_api
-  @key_algorithms :ssh.default_algorithms()[:public_key]
 
   @doc """
-  Returns a tuple suitable for passing the `SSHKit.SSH.connect/2` as the `key_cb` option.
+  Returns a tuple suitable for passing to `:ssh.connect/3` or
+  `SSHKit.SSH.connect/2` as the `key_cb` option.
 
   ## Options
 
@@ -35,7 +38,6 @@ defmodule SSHClientKeyAPI do
       SSHKit.SSH.connect("example.com", key_cb: cb)
 
   """
-  @spec with_options(opts :: list) :: {atom, list}
   def with_options(opts \\ []) do
     opts = with_defaults(opts)
 
@@ -47,17 +49,17 @@ defmodule SSHClientKeyAPI do
     {__MODULE__, opts}
   end
 
-  def add_host_key(hostname, key, opts) do
+  @impl :ssh_client_key_api
+  def add_host_key(hostname, _port, key, opts) do
+    hostname = normalize_hostname(hostname)
+
     case silently_accept_hosts(opts) do
       true ->
-        opts
-        |> known_hosts_data
-        |> :public_key.ssh_decode(:known_hosts)
-        |> (fn decoded -> decoded ++ [{key, [{:hostnames, [hostname]}]}] end).()
-        |> :public_key.ssh_encode(:known_hosts)
-        |> (fn encoded -> IO.binwrite(known_hosts(opts), encoded) end).()
+        # Don't save this to a file
+        :ok
 
       _ ->
+        # TODO: This seems to be missing a case to check if the host key is actually in the file
         message = """
         Error: unknown fingerprint found for #{inspect(hostname)} #{inspect(key)}.
         You either need to add a known good fingerprint to your known hosts file for this host,
@@ -66,83 +68,119 @@ defmodule SSHClientKeyAPI do
 
         {:error, message}
     end
+  rescue
+    e ->
+      Logger.warn("Exception in add_host_key: #{inspect(e)}")
+      raise e
   end
 
-  def is_host_key(key, hostname, alg, opts) when alg in @key_algorithms do
-    silently_accept_hosts(opts) ||
+  @impl :ssh_client_key_api
+  def is_host_key(key, host, port, alg, opts) do
+    :ssh_file.is_host_key(key, host, port, alg, opts)
+  rescue
+    e ->
+      Logger.warn("Exception in is_host_key: #{inspect(e)}")
+  end
+
+  # There's a fundamental disconnect between how the key_cb option works and how
+  # we want to use it. The key_cb option is expecting us to receive the
+  # algorithm type and then find the matching key, but we already know the exact
+  # key we want to use. So instead we return the key for every algorithm type
+  # and erlang will ignore the keys we return for an incorrect algorithm type.
+  #
+  # Ideally we could instead find the matching algorithm type for the key
+  # provided by the user without requiring the user to manually provide the key
+  # type but thus far I've been unable to find a way to find the algorithm type
+  # for the key
+  @impl :ssh_client_key_api
+  def user_key(_alg, opts) do
+    raw_key =
       opts
-      |> known_hosts_data
-      |> to_string
-      |> :public_key.ssh_decode(:known_hosts)
-      |> has_fingerprint(key, hostname)
-  end
-
-  def is_host_key(_, _, alg, _) do
-    IO.puts("unsupported host key algorithm #{inspect(alg)}")
-    false
-  end
+      |> identity_data()
+      |> to_string()
 
-  def user_key(alg, opts) when alg in @key_algorithms do
-    opts
-    |> identity_data
-    |> to_string
+    raw_key
     |> :public_key.pem_decode()
     |> List.first()
-    |> decode_pem_entry(passphrase(opts))
-  end
+    |> case do
+      {{:no_asn1, :new_openssh}, _data, :not_encrypted} ->
+        :ssh_file.decode(raw_key, :public_key)
+        |> case do
+          [{key, _comments} | _rest] ->
+            {:ok, key}
 
-  def user_key(alg, _) do
-    raise KeyError, {:unsupported_algorithm, alg}
-  end
+          {:error, :key_decode_failed} ->
+            message =
+              "unable to decode key, possibly because the key type does not support a passphrase"
 
-  defp decode_pem_entry(nil, _phrase) do
-    raise KeyError, {:unsupported_algorithm, :unknown}
-  end
+            Logger.warn(message)
+            {:error, :key_decode_failed}
 
-  defp decode_pem_entry({_type, _data, :not_encrypted} = entry, _) do
-    {:ok, :public_key.pem_entry_decode(entry)}
-  end
+          other ->
+            Logger.warn("Unexpected return value from :ssh_file.decode/2 #{inspect(other)}")
+            {:error, :ssh_client_key_api_unable_to_decode_key}
+        end
 
-  defp decode_pem_entry({_type, _data, {alg, _}}, nil) do
-    raise KeyError, {:passphrase_required, alg}
-  end
+      {_type, _data, :not_encrypted} = entry ->
+        result = :public_key.pem_entry_decode(entry)
+
+        {:ok, result}
 
-  defp decode_pem_entry({_type, _data, {alg, _}} = entry, phrase) do
-    {:ok, :public_key.pem_entry_decode(entry, phrase)}
+      {_type, _data, {_alg, _}} = entry ->
+        result = :public_key.pem_entry_decode(entry, passphrase(opts))
+        {:ok, result}
+
+      error ->
+        Logger.warn("Unexpected return value from :public_key.decode/2 #{inspect(error)}")
+        {:error, :ssh_client_key_api_unable_to_decode_key}
+    end
   rescue
-    _e in MatchError ->
-      # credo:disable-for-next-line Credo.Check.Warning.RaiseInsideRescue
-      raise KeyError, {:incorrect_passphrase, alg}
+    e ->
+      Logger.warn("user_key exception: #{inspect(e)}")
+      raise e
   end
 
-  defp identity_data(opts) do
-    cb_opts(opts)[:identity_data]
+  defp cb_opts(opts) do
+    opts[:key_cb_private]
   end
 
-  defp silently_accept_hosts(opts) do
-    cb_opts(opts)[:silently_accept_hosts]
+  defp known_hosts_data(opts) do
+    cb_opts(opts)[:known_hosts_data]
   end
 
   defp known_hosts(opts) do
     cb_opts(opts)[:known_hosts]
   end
 
-  defp known_hosts_data(opts) do
-    cb_opts(opts)[:known_hosts_data]
+  defp silently_accept_hosts(opts) do
+    cb_opts(opts)[:silently_accept_hosts]
+  end
+
+  defp identity_data(opts) do
+    cb_opts(opts)[:identity_data]
   end
 
   defp passphrase(opts) do
     cb_opts(opts)[:passphrase]
-  end
+    |> case do
+      # Needs to be a charlist
+      passphrase when is_list(passphrase) ->
+        passphrase
 
-  defp cb_opts(opts) do
-    opts[:key_cb_private]
-  end
+      passphrase when is_binary(passphrase) ->
+        Logger.warn("Passphrase must be a charlist, not a binary. Ignoring.")
+        nil
 
-  defp has_fingerprint(fingerprints, key, hostname) do
-    Enum.any?(fingerprints, fn {k, v} -> k == key && Enum.member?(v[:hostnames], hostname) end)
+      nil ->
+        nil
+    end
   end
 
+  # Handles the case where the ype of hostname is
+  # `[inet:ip_address() | inet:hostname()]`
+  defp normalize_hostname([hostname, _ip_addr]), do: hostname
+  defp normalize_hostname(hostname), do: hostname
+
   defp default_user_dir, do: Path.join(System.user_home!(), ".ssh")
 
   defp default_identity do