add unit tests

README.md

@@ -263,7 +263,7 @@ Ethers has these built-in signers to use:
 
 - `Ethers.Signer.Local`\*: A local signer which loads a private key from `signer_opts` and signs the transactions.
 - `Ethers.Signer.JsonRPC`: Uses `eth_signTransaction` Json RPC function to sign transactions. (Using services like [Consensys/web3signer](https://github.com/Consensys/web3signer) or [geth](https://geth.ethereum.org/))
-- `Ethers.Signer.KMS`: A KMS-based signer that expects a valid `kms_key_id` from `signer_opts`. Transactions are sent to KMS to sign with the specified [Customer Managed Key](https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html#customer-cmk). Notice that the keys need to be created as `ECC_SECG_P256K1` asymmetric pairs as described on [AWS](https://docs.aws.amazon.com/kms/latest/developerguide/asymmetric-key-specs.html#key-spec-ecc).
+- `Ethers.Signer.KMS`: A KMS-based signer that expects a valid `kms_key_id` from `signer_opts`. Transactions are sent to KMS to sign with the specified [Customer Managed Key](https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html#customer-cmk). Notice that the keys need to pre-exist as `ECC_SECG_P256K1` asymmetric pairs as described on [AWS](https://docs.aws.amazon.com/kms/latest/developerguide/asymmetric-key-specs.html#key-spec-ecc). Also, please refer to [AWS Key configuration](https://github.com/ex-aws/ex_aws?tab=readme-ov-file#aws-key-configuration) for configuring your app to connect to AWS in order to use the signer.
 
 For more information on signers, visit [hexdocs](https://hexdocs.pm/ethers/Ethers.Signer.html).
 

lib/ethers/signer/kms.ex

@@ -16,12 +16,15 @@ defmodule Ethers.Signer.KMS do
 
   @impl true
   def sign_transaction(%Transaction{} = tx, opts) do
-    with {:ok, key_id} <- get_kms_key(opts) do
+    with {:ok, key_id} <- get_kms_key(opts),
+         {:ok, %{"PublicKey" => pem}} <- ExAws.KMS.get_public_key(key_id) |> ExAws.request(),
+         {:ok, public_key} <- Utils.public_key_from_pem(pem),
+         :ok <- validate_public_key(public_key, tx.from) do
       {:ok, {r, s, recovery_id}} =
         Transaction.encode(tx)
         |> keccak_module().hash_256()
         |> Base.encode64()
-        |> sign(key_id)
+        |> sign(key_id, public_key)
 
       signed =
         %{tx | signature_r: r, signature_s: s, signature_recovery_id: recovery_id}
@@ -42,10 +45,21 @@ defmodule Ethers.Signer.KMS do
     end
   end
 
-  defp sign(message, key_id) do
-    with {:ok, %{"PublicKey" => pem}} <- ExAws.KMS.get_public_key(key_id) |> ExAws.request(),
-         {:ok, public_key} <- Utils.public_key_from_pem(pem),
-         {:ok, %{"Signature" => signature}} <-
+  defp validate_public_key(_public_key, nil), do: {:error, :no_from_address}
+
+  defp validate_public_key(public_key, from_address) do
+    derived_address = Utils.public_key_to_address(public_key)
+    from_address = Utils.to_checksum_address(from_address)
+
+    if derived_address == from_address do
+      :ok
+    else
+      {:error, :wrong_public_key}
+    end
+  end
+
+  defp sign(message, key_id, public_key) do
+    with {:ok, %{"Signature" => signature}} <-
            ExAws.KMS.sign(message, key_id, "ECDSA_SHA_256", message_type: "DIGEST")
            |> ExAws.request() do
       # extract r and s from the signature

lib/ethers/utils.ex

@@ -330,9 +330,6 @@ defmodule Ethers.Utils do
   ## Examples
 
       iex> Utils.public_key_from_pem("MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsjtGIk8SxD+OEiBpP2/TJUAF0upwuKGMk6wH8Rwov88VvzJrVm2NCticTk5FUg+UG5r8JArrV4tJPRHQyvqKwF4NiksuvOjv3HyIf4oaOhZjT8hDne1Bfv+cFqZJ61Gk0MjANh/T5q9vxER/7TdUNHKpoRV+NVlKN5bEU/NQ5FQjVXicfswxh6Y6fl2PIFqT2CfjD+FkBPU1iT9qyJYHA38IRvwNtcitFgCeZwdGPoxiPPh1WHY8VxpUVBv/2JsUtrB/rAIbGqZoxAIWvijJPe9o1TY3VlOzk9ASZ1AeatvOir+iDVJ5OpKmLnzc46QgGPUsjIyo6Sje9dxpGtoGQQIDAQAB")
-      {:ok, <<48, 130, 1, 10, 2, 130, 1, 1, 0, 178, 59, 70, 34, 79, 18, 196, 63, 142, 18,
-        32, 105, 63, 111, 211, 37, 64, 5, 210, 234, 112, 184, 161, 140, 147, 172, 7,
-        241, 28, 40, 191, 207, 21, 191, 50, 107, 86, 109, 141>>}
   """
   @spec public_key_from_pem(String.t()) :: {:ok, binary()}
   def public_key_from_pem(pem) do

mix.exs

@@ -102,7 +102,8 @@ defmodule Ethers.MixProject do
       {:jason, "~> 1.4"},
       {:ex_aws, "~> 2.5.1"},
       {:ex_aws_kms, "~> 2.3.2"},
-      {:ex_aws_sts, "~> 2.3.0"}
+      {:ex_aws_sts, "~> 2.3.0"},
+      {:mimic, "~> 1.7", only: :test}
     ]
   end
 

mix.lock

@@ -7,6 +7,9 @@
   "erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"},
   "ethereumex": {:hex, :ethereumex, "0.10.6", "6d75cac39b5b7a720b064fe48563f205d3d9784e5bde25f983dd07cf306c2a6d", [:make, :mix], [{:finch, "~> 0.16", [hex: :finch, repo: "hexpm", optional: false]}, {:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "58cf926239dabf8bd1fc6cf50a37b926274240b7f58ba5b235a20b5500a9a7e1"},
   "ex_abi": {:hex, :ex_abi, "0.6.4", "f722a38298f176dab511cf94627b2815282669255bc2eb834674f23ca71f5cfb", [:mix], [{:ex_keccak, "~> 0.7.3", [hex: :ex_keccak, repo: "hexpm", optional: false]}, {:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "07eaf39b70dd3beac1286c10368d27091a9a64844830eb26a38f1c8d8b19dfbb"},
+  "ex_aws": {:hex, :ex_aws, "2.5.1", "7418917974ea42e9e84b25e88b9f3d21a861d5f953ad453e212f48e593d8d39f", [:mix], [{:configparser_ex, "~> 4.0", [hex: :configparser_ex, repo: "hexpm", optional: true]}, {:hackney, "~> 1.16", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:jsx, "~> 2.8 or ~> 3.0", [hex: :jsx, repo: "hexpm", optional: true]}, {:mime, "~> 1.2 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:sweet_xml, "~> 0.7", [hex: :sweet_xml, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "1b95431f70c446fa1871f0eb9b183043c5a625f75f9948a42d25f43ae2eff12b"},
+  "ex_aws_kms": {:hex, :ex_aws_kms, "2.3.2", "0a397bd4eb807dcb519dc738ef3ab4f89f371cbcb5880c4f94bbd3612b364446", [:mix], [{:ex_aws, "~> 2.1", [hex: :ex_aws, repo: "hexpm", optional: false]}], "hexpm", "1f2adcc51a3f160f27d0c4d2acab93dd1d7e75b3809d06477bf433b99b03f616"},
+  "ex_aws_sts": {:hex, :ex_aws_sts, "2.3.0", "ce48c4cba7f1595a7d544458d0202ca313124026dba7b1a0021bbb1baa3d66d0", [:mix], [{:ex_aws, "~> 2.2", [hex: :ex_aws, repo: "hexpm", optional: false]}], "hexpm", "f14e4c7da3454514bf253b331e9422d25825485c211896ab3b81d2a4bdbf62f5"},
   "ex_doc": {:hex, :ex_doc, "0.31.1", "8a2355ac42b1cc7b2379da9e40243f2670143721dd50748bf6c3b1184dae2089", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.1", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "3178c3a407c557d8343479e1ff117a96fd31bafe52a039079593fb0524ef61b0"},
   "ex_keccak": {:hex, :ex_keccak, "0.7.3", "33298f97159f6b0acd28f6e96ce5ea975a0f4a19f85fe615b4f4579b88b24d06", [:mix], [{:rustler, ">= 0.0.0", [hex: :rustler, repo: "hexpm", optional: true]}, {:rustler_precompiled, "~> 0.6.1", [hex: :rustler_precompiled, repo: "hexpm", optional: false]}], "hexpm", "4c5e6d9d5f77b64ab48769a0166a9814180d40ced68ed74ce60a5174ab55b3fc"},
   "ex_rlp": {:hex, :ex_rlp, "0.6.0", "985391d2356a7cb8712a4a9a2deb93f19f2fbca0323f5c1203fcaf64d077e31e", [:mix], [], "hexpm", "7135db93b861d9e76821039b60b00a6a22d2c4e751bf8c444bffe7a042f1abaf"},
@@ -21,6 +24,7 @@
   "makeup_elixir": {:hex, :makeup_elixir, "0.16.1", "cc9e3ca312f1cfeccc572b37a09980287e243648108384b97ff2b76e505c3555", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "e127a341ad1b209bd80f7bd1620a15693a9908ed780c3b763bccf7d200c767c6"},
   "makeup_erlang": {:hex, :makeup_erlang, "0.1.3", "d684f4bac8690e70b06eb52dad65d26de2eefa44cd19d64a8095e1417df7c8fd", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "b78dc853d2e670ff6390b605d807263bf606da3c82be37f9d7f68635bd886fc9"},
   "mime": {:hex, :mime, "2.0.5", "dc34c8efd439abe6ae0343edbb8556f4d63f178594894720607772a041b04b02", [:mix], [], "hexpm", "da0d64a365c45bc9935cc5c8a7fc5e49a0e0f9932a761c55d6c52b142780a05c"},
+  "mimic": {:hex, :mimic, "1.7.4", "cd2772ffbc9edefe964bc668bfd4059487fa639a5b7f1cbdf4fd22946505aa4f", [:mix], [], "hexpm", "437c61041ecf8a7fae35763ce89859e4973bb0666e6ce76d75efc789204447c3"},
   "mint": {:hex, :mint, "1.5.2", "4805e059f96028948870d23d7783613b7e6b0e2fb4e98d720383852a760067fd", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:hpax, "~> 0.1.1", [hex: :hpax, repo: "hexpm", optional: false]}], "hexpm", "d77d9e9ce4eb35941907f1d3df38d8f750c357865353e21d335bdcdf6d892a02"},
   "nimble_options": {:hex, :nimble_options, "1.1.0", "3b31a57ede9cb1502071fade751ab0c7b8dbe75a9a4c2b5bbb0943a690b63172", [:mix], [], "hexpm", "8bbbb3941af3ca9acc7835f5655ea062111c9c27bcac53e004460dfd19008a99"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},

test/ethers/signer/kms_test.exs

@@ -0,0 +1,110 @@
+defmodule Ethers.Signer.KMSTest do
+  use ExUnit.Case
+  use Mimic
+
+  alias Ethers.Signer
+  alias Ethers.SignerFixtures
+
+  describe "sign_transaction/2" do
+    test "signs the transaction with the correct data" do
+      expect(ExAws, :request, fn _ ->
+        SignerFixtures.kms_public_key_response()
+      end)
+
+      expect(ExAws, :request, fn _ ->
+        SignerFixtures.kms_sign_response()
+      end)
+
+      transaction = %Ethers.Transaction{
+        type: :eip1559,
+        chain_id: "0x539",
+        nonce: "0xb66",
+        gas: "0x5A82",
+        from: "0x4eed49289Ac2876C9c966FC16b22F6eC5bf0817c",
+        to: "0xFFcf8FDEE72ac11b5c542428B35EEF5769C409f0",
+        value: "0x0",
+        data: "0x06fdde03",
+        gas_price: "0x10e7467522",
+        max_fee_per_gas: "0x1448BAF2F5",
+        max_priority_fee_per_gas: "0x0"
+      }
+
+      assert {:ok,
+              "0x02f86f820539820b6680851448baf2f5825a8294ffcf8fdee72ac11b5c542428b35eef5769c409f0808406fdde03c001a032ba3398b3223445b858849e275d6dbb1a6708f305bb2b8c427143f9239bb9bea053fa0a993614ed279496088147827086767088e33a587e3c18b5978f8ac018e5"} ==
+               Signer.KMS.sign_transaction(transaction,
+                 kms_key_id: "ddb1aedd-77d1-4b90-a3a8-d77fb82ba533"
+               )
+    end
+
+    test "fails if no kms key id is given" do
+      transaction = %Ethers.Transaction{
+        type: :eip1559,
+        chain_id: "0x539",
+        nonce: "0xb66",
+        gas: "0x5A82",
+        from: "0x4eed49289Ac2876C9c966FC16b22F6eC5bf0817c",
+        to: "0xFFcf8FDEE72ac11b5c542428B35EEF5769C409f0",
+        value: "0x0",
+        data: "0x06fdde03",
+        gas_price: "0x10e7467522",
+        max_fee_per_gas: "0x1448BAF2F5",
+        max_priority_fee_per_gas: "0x0"
+      }
+
+      assert {:error, :kms_key_not_found} ==
+               Signer.KMS.sign_transaction(transaction,
+                 kms_key_id: nil
+               )
+    end
+
+    test "fails if no from address is given" do
+      expect(ExAws, :request, fn _ ->
+        SignerFixtures.kms_public_key_response()
+      end)
+
+      transaction = %Ethers.Transaction{
+        type: :eip1559,
+        chain_id: "0x539",
+        nonce: "0xb66",
+        gas: "0x5A82",
+        from: nil,
+        to: "0xFFcf8FDEE72ac11b5c542428B35EEF5769C409f0",
+        value: "0x0",
+        data: "0x06fdde03",
+        gas_price: "0x10e7467522",
+        max_fee_per_gas: "0x1448BAF2F5",
+        max_priority_fee_per_gas: "0x0"
+      }
+
+      assert {:error, :no_from_address} ==
+               Signer.KMS.sign_transaction(transaction,
+                 kms_key_id: "ddb1aedd-77d1-4b90-a3a8-d77fb82ba533"
+               )
+    end
+
+    test "fails if from address does not match the public key specified in the signer" do
+      expect(ExAws, :request, fn _ ->
+        SignerFixtures.kms_public_key_response()
+      end)
+
+      transaction = %Ethers.Transaction{
+        type: :eip1559,
+        chain_id: "0x539",
+        nonce: "0xb66",
+        gas: "0x5A82",
+        from: "0xFFcf8FDEE72ac11b5c542428B35EEF5769C409f0",
+        to: "0xFFcf8FDEE72ac11b5c542428B35EEF5769C409f0",
+        value: "0x0",
+        data: "0x06fdde03",
+        gas_price: "0x10e7467522",
+        max_fee_per_gas: "0x1448BAF2F5",
+        max_priority_fee_per_gas: "0x0"
+      }
+
+      assert {:error, :wrong_public_key} ==
+               Signer.KMS.sign_transaction(transaction,
+                 kms_key_id: "ddb1aedd-77d1-4b90-a3a8-d77fb82ba533"
+               )
+    end
+  end
+end

test/ethers/utils_test.exs

@@ -83,4 +83,29 @@ defmodule Ethers.UtilsTest do
                    fn -> Ethers.Utils.hex_decode!("0xrubbish") end
     end
   end
+
+  describe "public_key_from_pem" do
+    test "can extract public key from a pem" do
+      assert {:ok, pub_key} =
+               Utils.public_key_from_pem(
+                 "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsjtGIk8SxD+OEiBpP2/TJUAF0upwuKGMk6wH8Rwov88VvzJrVm2NCticTk5FUg+UG5r8JArrV4tJPRHQyvqKwF4NiksuvOjv3HyIf4oaOhZjT8hDne1Bfv+cFqZJ61Gk0MjANh/T5q9vxER/7TdUNHKpoRV+NVlKN5bEU/NQ5FQjVXicfswxh6Y6fl2PIFqT2CfjD+FkBPU1iT9qyJYHA38IRvwNtcitFgCeZwdGPoxiPPh1WHY8VxpUVBv/2JsUtrB/rAIbGqZoxAIWvijJPe9o1TY3VlOzk9ASZ1AeatvOir+iDVJ5OpKmLnzc46QgGPUsjIyo6Sje9dxpGtoGQQIDAQAB"
+               )
+
+      assert true == is_binary(pub_key)
+    end
+
+    test "can extract public key from a pem with head and tail" do
+      assert {:ok, pub_key} =
+               Utils.public_key_from_pem(
+                 "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsjtGIk8SxD+OEiBpP2/TJUAF0upwuKGMk6wH8Rwov88VvzJrVm2NCticTk5FUg+UG5r8JArrV4tJPRHQyvqKwF4NiksuvOjv3HyIf4oaOhZjT8hDne1Bfv+cFqZJ61Gk0MjANh/T5q9vxER/7TdUNHKpoRV+NVlKN5bEU/NQ5FQjVXicfswxh6Y6fl2PIFqT2CfjD+FkBPU1iT9qyJYHA38IRvwNtcitFgCeZwdGPoxiPPh1WHY8VxpUVBv/2JsUtrB/rAIbGqZoxAIWvijJPe9o1TY3VlOzk9ASZ1AeatvOir+iDVJ5OpKmLnzc46QgGPUsjIyo6Sje9dxpGtoGQQIDAQAB\n-----END PUBLIC KEY-----"
+               )
+
+      assert true == is_binary(pub_key)
+    end
+
+    test "can not extract public key from an invalid pem" do
+      assert_raise ErlangError,
+                   fn -> Utils.public_key_from_pem("invalid pem") end
+    end
+  end
 end

test/support/fixtures/signer_fixtures.ex

@@ -0,0 +1,21 @@
+defmodule Ethers.SignerFixtures do
+  @moduledoc """
+  This module defines fixtures for Signers.
+  """
+
+  def kms_public_key_response do
+    {:ok,
+     %{
+       "PublicKey" =>
+         "MFYwEAYHKoZIzj0CAQYFK4EEAAoDQgAEsdTvgjvTDk/BF/COdU/4/v6HgCaceKifvcBfxKnZpCt5wFzZEgwWSLTsz1T9YaCaS0Xb0D0g7TaT8VAD+Tesmg=="
+     }}
+  end
+
+  def kms_sign_response do
+    {:ok,
+     %{
+       "Signature" =>
+         "MEUCIDK6M5izIjRFuFiEniddbbsaZwjzBbsrjEJxQ/kjm7m+AiEArAX1ZsnrEthrafd+uH2PeEQ+VAN08CH/pxzG/UV2KFw="
+     }}
+  end
+end

test/test_helper.exs

@@ -1 +1,3 @@
 ExUnit.start()
+
+Mimic.copy(ExAws)