Add signing eip712 for hardware accounts (#12265)

(cherry picked from commit e8a688c)

browser/brave_wallet/brave_wallet_provider_impl_unittest.cc

@@ -81,6 +81,12 @@ void ValidateErrorCode(BraveWalletProviderImpl* provider,
   ASSERT_TRUE(callback_is_called);
 }
 
+std::vector<uint8_t> DecodeHexHash(const std::string& hash_hex) {
+  std::vector<uint8_t> hash;
+  base::HexStringToBytes(hash_hex, &hash);
+  return hash;
+}
+
 }  // namespace
 
 class TestEventsListener : public brave_wallet::mojom::EventsListener {
@@ -425,7 +431,8 @@ class BraveWalletProviderImplUnitTest : public testing::Test {
   void SignTypedMessage(absl::optional<bool> user_approved,
                         const std::string& address,
                         const std::string& message,
-                        const std::string& message_to_sign,
+                        const std::vector<uint8_t>& domain_hash,
+                        const std::vector<uint8_t>& primary_hash,
                         base::Value&& domain,
                         std::string* signature_out,
                         mojom::ProviderError* error_out,
@@ -435,7 +442,7 @@ class BraveWalletProviderImplUnitTest : public testing::Test {
 
     base::RunLoop run_loop;
     provider()->SignTypedMessage(
-        address, message, message_to_sign, std::move(domain),
+        address, message, domain_hash, primary_hash, std::move(domain),
         base::BindLambdaForTesting([&](const std::string& signature,
                                        mojom::ProviderError error,
                                        const std::string& error_message) {
@@ -1197,47 +1204,50 @@ TEST_F(BraveWalletProviderImplUnitTest, SignTypedMessage) {
   EXPECT_EQ(json_rpc_service()->GetChainId(), "0x1");
   CreateWallet();
   AddAccount();
-  const std::string valid_message_to_sign =
-      "be609aee343fb3c4b28e1df9e632fca64fcfaede20f02e86244efddf30957bd2";
   std::string signature;
   mojom::ProviderError error;
   std::string error_message;
   base::Value domain(base::Value::Type::DICTIONARY);
+  std::vector<uint8_t> domain_hash = DecodeHexHash(
+      "f2cee375fa42b42143804025fc449deafd50cc031ca257e0b194a650a912090f");
+  std::vector<uint8_t> primary_hash = DecodeHexHash(
+      "c52c0ee5d84264471806290a3f2c4cecfc5490626bf912d01f240d7a274b371e");
   domain.SetIntKey("chainId", 1);
-  SignTypedMessage(absl::nullopt, "1234", "{...}", valid_message_to_sign,
+  SignTypedMessage(absl::nullopt, "1234", "{...}", domain_hash, primary_hash,
                    domain.Clone(), &signature, &error, &error_message);
   EXPECT_TRUE(signature.empty());
   EXPECT_EQ(error, mojom::ProviderError::kInvalidParams);
   EXPECT_EQ(error_message,
             l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
 
-  SignTypedMessage(absl::nullopt, "0x12345678", "{...}", valid_message_to_sign,
-                   domain.Clone(), &signature, &error, &error_message);
+  SignTypedMessage(absl::nullopt, "0x12345678", "{...}", domain_hash,
+                   primary_hash, domain.Clone(), &signature, &error,
+                   &error_message);
   EXPECT_TRUE(signature.empty());
   EXPECT_EQ(error, mojom::ProviderError::kInvalidParams);
   EXPECT_EQ(error_message,
             l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
 
   const std::string address = "0x1234567890123456789012345678901234567890";
   // domain not dict
-  SignTypedMessage(absl::nullopt, address, "{...}", valid_message_to_sign,
+  SignTypedMessage(absl::nullopt, address, "{...}", domain_hash, primary_hash,
                    base::Value("not dict"), &signature, &error, &error_message);
   EXPECT_TRUE(signature.empty());
   EXPECT_EQ(error, mojom::ProviderError::kInvalidParams);
   EXPECT_EQ(error_message,
             l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
 
-  // not valid hex
-  SignTypedMessage(absl::nullopt, address, "{...}", "brave", domain.Clone(),
-                   &signature, &error, &error_message);
+  // not valid domain hash
+  SignTypedMessage(absl::nullopt, address, "{...}", {}, primary_hash,
+                   domain.Clone(), &signature, &error, &error_message);
   EXPECT_TRUE(signature.empty());
   EXPECT_EQ(error, mojom::ProviderError::kInvalidParams);
   EXPECT_EQ(error_message,
             l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
 
-  // not valid eip712 hash
-  SignTypedMessage(absl::nullopt, address, "{...}", "deadbeef", domain.Clone(),
-                   &signature, &error, &error_message);
+  // not valid primary hash
+  SignTypedMessage(absl::nullopt, address, "{...}", domain_hash, {},
+                   domain.Clone(), &signature, &error, &error_message);
   EXPECT_TRUE(signature.empty());
   EXPECT_EQ(error, mojom::ProviderError::kInvalidParams);
   EXPECT_EQ(error_message,
@@ -1246,7 +1256,7 @@ TEST_F(BraveWalletProviderImplUnitTest, SignTypedMessage) {
   domain.SetIntKey("chainId", 4);
   std::string chain_id = "0x4";
   // not active network
-  SignTypedMessage(absl::nullopt, address, "{...}", valid_message_to_sign,
+  SignTypedMessage(absl::nullopt, address, "{...}", domain_hash, primary_hash,
                    domain.Clone(), &signature, &error, &error_message);
   EXPECT_TRUE(signature.empty());
   EXPECT_EQ(error, mojom::ProviderError::kInternalError);
@@ -1256,7 +1266,7 @@ TEST_F(BraveWalletProviderImplUnitTest, SignTypedMessage) {
                 base::ASCIIToUTF16(chain_id)));
   domain.SetIntKey("chainId", 1);
 
-  SignTypedMessage(absl::nullopt, address, "{...}", valid_message_to_sign,
+  SignTypedMessage(absl::nullopt, address, "{...}", domain_hash, primary_hash,
                    domain.Clone(), &signature, &error, &error_message);
   EXPECT_TRUE(signature.empty());
   EXPECT_EQ(error, mojom::ProviderError::kUnauthorized);
@@ -1267,8 +1277,9 @@ TEST_F(BraveWalletProviderImplUnitTest, SignTypedMessage) {
   // No permission
   const std::vector<std::string> addresses = GetAddresses();
   ASSERT_FALSE(address.empty());
-  SignTypedMessage(absl::nullopt, addresses[0], "{...}", valid_message_to_sign,
-                   domain.Clone(), &signature, &error, &error_message);
+  SignTypedMessage(absl::nullopt, addresses[0], "{...}", domain_hash,
+                   primary_hash, domain.Clone(), &signature, &error,
+                   &error_message);
   EXPECT_TRUE(signature.empty());
   EXPECT_EQ(error, mojom::ProviderError::kUnauthorized);
   EXPECT_EQ(error_message,
@@ -1277,27 +1288,43 @@ TEST_F(BraveWalletProviderImplUnitTest, SignTypedMessage) {
   GURL url("https://brave.com");
   Navigate(url);
   AddEthereumPermission(url);
-  SignTypedMessage(true, addresses[0], "{...}", valid_message_to_sign,
+  SignTypedMessage(true, addresses[0], "{...}", domain_hash, primary_hash,
                    domain.Clone(), &signature, &error, &error_message);
 
   EXPECT_FALSE(signature.empty());
   EXPECT_EQ(error, mojom::ProviderError::kSuccess);
   EXPECT_TRUE(error_message.empty());
 
   // User reject request
-  SignTypedMessage(false, addresses[0], "{...}", valid_message_to_sign,
+  SignTypedMessage(false, addresses[0], "{...}", domain_hash, primary_hash,
                    domain.Clone(), &signature, &error, &error_message);
   EXPECT_TRUE(signature.empty());
   EXPECT_EQ(error, mojom::ProviderError::kUserRejectedRequest);
   EXPECT_EQ(error_message,
             l10n_util::GetStringUTF8(IDS_WALLET_USER_REJECTED_REQUEST));
-
+  // not valid eip712 domain hash
+  SignTypedMessage(absl::nullopt, address, "{...}", DecodeHexHash("brave"),
+                   primary_hash, domain.Clone(), &signature, &error,
+                   &error_message);
+  EXPECT_TRUE(signature.empty());
+  EXPECT_EQ(error, mojom::ProviderError::kInvalidParams);
+  EXPECT_EQ(error_message,
+            l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
+  // not valid eip712 primary hash
+  SignTypedMessage(absl::nullopt, address, "{...}", domain_hash,
+                   DecodeHexHash("primary"), domain.Clone(), &signature, &error,
+                   &error_message);
+  EXPECT_TRUE(signature.empty());
+  EXPECT_EQ(error, mojom::ProviderError::kInvalidParams);
+  EXPECT_EQ(error_message,
+            l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
   keyring_service()->Lock();
 
   // nullopt for the first param here because we don't AddSignMessageRequest
   // whent here are no accounts returned.
-  SignTypedMessage(absl::nullopt, addresses[0], "{...}", valid_message_to_sign,
-                   domain.Clone(), &signature, &error, &error_message);
+  SignTypedMessage(absl::nullopt, addresses[0], "{...}", domain_hash,
+                   primary_hash, domain.Clone(), &signature, &error,
+                   &error_message);
   EXPECT_TRUE(signature.empty());
   EXPECT_EQ(error, mojom::ProviderError::kUnauthorized);
   EXPECT_EQ(error_message,

browser/brave_wallet/brave_wallet_service_unittest.cc

@@ -4,6 +4,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "brave/components/brave_wallet/browser/brave_wallet_service.h"
+#include <string>
 
 #include "base/memory/raw_ptr.h"
 #include "base/test/bind.h"
@@ -1315,7 +1316,8 @@ TEST_F(BraveWalletServiceUnitTest, SignMessageHardware) {
   std::string address = "0xbe862ad9abfe6f22bcb087716c7d89a26051f74c";
   std::string message = "0xAB";
   auto request1 = mojom::SignMessageRequest::New(
-      1, address, std::string(message.begin(), message.end()));
+      1, address, std::string(message.begin(), message.end()), false,
+      absl::nullopt, absl::nullopt);
   bool callback_is_called = false;
   service_->AddSignMessageRequest(
       std::move(request1), base::BindLambdaForTesting(
@@ -1337,7 +1339,8 @@ TEST_F(BraveWalletServiceUnitTest, SignMessageHardware) {
   callback_is_called = false;
   std::string expected_error = "error";
   auto request2 = mojom::SignMessageRequest::New(
-      2, address, std::string(message.begin(), message.end()));
+      2, address, std::string(message.begin(), message.end()), false,
+      absl::nullopt, absl::nullopt);
   service_->AddSignMessageRequest(
       std::move(request2), base::BindLambdaForTesting(
                                [&](bool approved, const std::string& signature,
@@ -1359,7 +1362,8 @@ TEST_F(BraveWalletServiceUnitTest, SignMessage) {
   std::string address = "0xbe862ad9abfe6f22bcb087716c7d89a26051f74c";
   std::string message = "0xAB";
   auto request1 = mojom::SignMessageRequest::New(
-      1, address, std::string(message.begin(), message.end()));
+      1, address, std::string(message.begin(), message.end()), false,
+      absl::nullopt, absl::nullopt);
   bool callback_is_called = false;
   service_->AddSignMessageRequest(
       std::move(request1), base::BindLambdaForTesting(
@@ -1377,7 +1381,8 @@ TEST_F(BraveWalletServiceUnitTest, SignMessage) {
   callback_is_called = false;
   std::string expected_error = "error";
   auto request2 = mojom::SignMessageRequest::New(
-      2, address, std::string(message.begin(), message.end()));
+      2, address, std::string(message.begin(), message.end()), false,
+      absl::nullopt, absl::nullopt);
   service_->AddSignMessageRequest(
       std::move(request2), base::BindLambdaForTesting(
                                [&](bool approved, const std::string& signature,
@@ -1569,7 +1574,8 @@ TEST_F(BraveWalletServiceUnitTest, Reset) {
   std::string address = "0xbe862ad9abfe6f22bcb087716c7d89a26051f74c";
   std::string message = "0xAB";
   auto request1 = mojom::SignMessageRequest::New(
-      1, address, std::string(message.begin(), message.end()));
+      1, address, std::string(message.begin(), message.end()), false,
+      absl::nullopt, absl::nullopt);
   service_->AddSignMessageRequest(
       std::move(request1),
       base::BindLambdaForTesting(

components/brave_wallet/browser/brave_wallet_constants.h

@@ -588,7 +588,10 @@ constexpr webui::LocalizedString kLocalizedStrings[] = {
     {"braveWalletNFTDetailBlockchain", IDS_BRAVE_WALLET_NFT_DETAIL_BLOCKCHAIN},
     {"braveWalletNFTDetailTokenStandard",
      IDS_BRAVE_WALLET_NFT_DETAIL_TOKEN_STANDARD},
-    {"braveWalletNFTDetailTokenID", IDS_BRAVE_WALLET_NFT_DETAIL_TOKEN_ID}};
+    {"braveWalletNFTDetailTokenID", IDS_BRAVE_WALLET_NFT_DETAIL_TOKEN_ID},
+    {"braveWalletTrezorSignTypedDataError",
+     IDS_BRAVE_WALLET_TREZOR_SIGN_TYPED_DATA_ERROR},
+};
 
 const char kRopstenSwapBaseAPIURL[] = "https://ropsten.api.0x.org/";
 const char kRopstenBuyTokenPercentageFee[] = "0.00875";

components/brave_wallet/browser/brave_wallet_provider_impl.cc

@@ -5,6 +5,7 @@
 
 #include "brave/components/brave_wallet/browser/brave_wallet_provider_impl.h"
 
+#include <string>
 #include <utility>
 
 #include "base/json/json_reader.h"
@@ -20,6 +21,7 @@
 #include "brave/components/brave_wallet/browser/json_rpc_service.h"
 #include "brave/components/brave_wallet/browser/keyring_service.h"
 #include "brave/components/brave_wallet/common/eth_address.h"
+#include "brave/components/brave_wallet/common/eth_sign_typed_data_helper.h"
 #include "brave/components/brave_wallet/common/hex_utils.h"
 #include "brave/components/brave_wallet/common/value_conversion_utils.h"
 #include "brave/components/brave_wallet/common/web3_provider_constants.h"
@@ -364,11 +366,11 @@ void BraveWalletProviderImpl::SignMessage(const std::string& address,
   // Convert to checksum address
   auto checksum_address = EthAddress::FromHex(address);
   GetAllowedAccounts(
-      false,
-      base::BindOnce(&BraveWalletProviderImpl::ContinueSignMessage,
-                     weak_factory_.GetWeakPtr(),
-                     checksum_address.ToChecksumAddress(), message_str,
-                     std::move(message_bytes), std::move(callback), false));
+      false, base::BindOnce(&BraveWalletProviderImpl::ContinueSignMessage,
+                            weak_factory_.GetWeakPtr(),
+                            checksum_address.ToChecksumAddress(), message_str,
+                            std::move(message_bytes), absl::nullopt,
+                            absl::nullopt, false, std::move(callback)));
 }
 
 void BraveWalletProviderImpl::RecoverAddress(const std::string& message,
@@ -413,19 +415,17 @@ void BraveWalletProviderImpl::RecoverAddress(const std::string& message,
 void BraveWalletProviderImpl::SignTypedMessage(
     const std::string& address,
     const std::string& message,
-    const std::string& message_to_sign,
+    const std::vector<uint8_t>& domain_hash,
+    const std::vector<uint8_t>& primary_hash,
     base::Value domain,
     SignTypedMessageCallback callback) {
-  std::vector<uint8_t> eip712_hash;
-  if (!EthAddress::IsValidAddress(address) ||
-      !base::HexStringToBytes(message_to_sign, &eip712_hash) ||
-      eip712_hash.size() != 32 || !domain.is_dict()) {
+  if (!EthAddress::IsValidAddress(address) || !domain.is_dict() ||
+      domain_hash.empty() || primary_hash.empty()) {
     std::move(callback).Run(
         "", mojom::ProviderError::kInvalidParams,
         l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
     return;
   }
-
   auto chain_id = domain.FindDoubleKey("chainId");
   if (chain_id) {
     const std::string chain_id_hex =
@@ -440,21 +440,40 @@ void BraveWalletProviderImpl::SignTypedMessage(
     }
   }
 
+  if (domain_hash.empty() || primary_hash.empty()) {
+    std::move(callback).Run(
+        "", mojom::ProviderError::kInvalidParams,
+        l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
+    return;
+  }
+  auto message_to_sign = EthSignTypedDataHelper::GetTypedDataMessageToSign(
+      domain_hash, primary_hash);
+  if (!message_to_sign || message_to_sign->size() != 32) {
+    std::move(callback).Run(
+        "", mojom::ProviderError::kInvalidParams,
+        l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
+    return;
+  }
+
   // Convert to checksum address
   auto checksum_address = EthAddress::FromHex(address);
   GetAllowedAccounts(
-      false, base::BindOnce(&BraveWalletProviderImpl::ContinueSignMessage,
-                            weak_factory_.GetWeakPtr(),
-                            checksum_address.ToChecksumAddress(), message,
-                            std::move(eip712_hash), std::move(callback), true));
+      false,
+      base::BindOnce(&BraveWalletProviderImpl::ContinueSignMessage,
+                     weak_factory_.GetWeakPtr(),
+                     checksum_address.ToChecksumAddress(), message,
+                     std::move(*message_to_sign), base::HexEncode(domain_hash),
+                     base::HexEncode(primary_hash), true, std::move(callback)));
 }
 
 void BraveWalletProviderImpl::ContinueSignMessage(
     const std::string& address,
     const std::string& message,
     std::vector<uint8_t>&& message_to_sign,
-    SignMessageCallback callback,
+    const absl::optional<std::string>& domain_hash,
+    const absl::optional<std::string>& primary_hash,
     bool is_eip712,
+    SignMessageCallback callback,
     const std::vector<std::string>& allowed_accounts,
     mojom::ProviderError error,
     const std::string& error_message) {
@@ -472,7 +491,9 @@ void BraveWalletProviderImpl::ContinueSignMessage(
   }
 
   auto request =
-      mojom::SignMessageRequest::New(sign_message_id_++, address, message);
+      mojom::SignMessageRequest::New(sign_message_id_++, address, message,
+                                     is_eip712, domain_hash, primary_hash);
+
   if (keyring_service_->IsHardwareAccount(address)) {
     brave_wallet_service_->AddSignMessageRequest(
         std::move(request),

components/brave_wallet/browser/brave_wallet_provider_impl.h

@@ -80,7 +80,8 @@ class BraveWalletProviderImpl final
                       RecoverAddressCallback callback) override;
   void SignTypedMessage(const std::string& address,
                         const std::string& message,
-                        const std::string& message_to_sign,
+                        const std::vector<uint8_t>& domain_hash,
+                        const std::vector<uint8_t>& primary_hash,
                         base::Value domain,
                         SignTypedMessageCallback callback) override;
   void OnGetAllowedAccounts(GetAllowedAccountsCallback callback,
@@ -161,8 +162,10 @@ class BraveWalletProviderImpl final
   void ContinueSignMessage(const std::string& address,
                            const std::string& message,
                            std::vector<uint8_t>&& message_to_sign,
-                           SignMessageCallback callback,
+                           const absl::optional<std::string>& domain_hash,
+                           const absl::optional<std::string>& primary_hash,
                            bool is_eip712,
+                           SignMessageCallback callback,
                            const std::vector<std::string>& allowed_accounts,
                            mojom::ProviderError error,
                            const std::string& error_message);