feat(all): make chain_id mandatory

common/protob/messages-ethereum.proto

@@ -65,7 +65,7 @@ message EthereumSignTx {
     optional bytes value = 6;               // <=256 bit unsigned big endian (in wei)
     optional bytes data_initial_chunk = 7;  // The initial data chunk (<= 1024 bytes)
     optional uint32 data_length = 8;        // Length of transaction payload
-    optional uint64 chain_id = 9;           // Chain Id for EIP 155
+    required uint64 chain_id = 9;           // Chain Id for EIP 155
     optional uint32 tx_type = 10;           // Used for Wanchain
 }
 

core/.changelog.d/1794.incompatible

@@ -0,0 +1 @@
+Ethereum non-EIP-155 cross-chain signing is no longer supported.

core/src/apps/ethereum/sign_tx.py

@@ -75,10 +75,9 @@ async def sign_tx(ctx, msg, keychain):
         sha.extend(resp.data_chunk)
 
     # eip 155 replay protection
-    if msg.chain_id:
+    rlp.write(sha, msg.chain_id)
-        rlp.write(sha, msg.chain_id)
+    rlp.write(sha, 0)
-        rlp.write(sha, 0)
+    rlp.write(sha, 0)
-        rlp.write(sha, 0)
 
     digest = sha.get_digest()
     result = sign_digest(msg, keychain, digest)
@@ -119,14 +118,12 @@ def get_total_length(msg: EthereumSignTx, data_total: int) -> int:
         msg.gas_limit,
         address.bytes_from_address(msg.to),
         msg.value,
+        msg.chain_id,
+        0,
+        0,
     ):
         length += rlp.length(item)
 
-    if msg.chain_id:  # forks replay protection
-        length += rlp.length(msg.chain_id)
-        length += rlp.length(0)
-        length += rlp.length(0)
-
     length += rlp.header_length(data_total, msg.data_initial_chunk)
     length += data_total
 
@@ -154,7 +151,7 @@ def sign_digest(msg: EthereumSignTx, keychain, digest):
     req.signature_v = signature[0]
     if msg.chain_id > MAX_CHAIN_ID:
         req.signature_v -= 27
-    elif msg.chain_id:
+    else:
         req.signature_v += 2 * msg.chain_id + 8
 
     req.signature_r = signature[1:33]
@@ -217,6 +214,4 @@ def sanitize(msg):
         msg.to = ""
     if msg.nonce is None:
         msg.nonce = b""
-    if msg.chain_id is None:
-        msg.chain_id = 0
     return msg

core/src/trezor/messages.py

@@ -2991,12 +2991,13 @@ if TYPE_CHECKING:
         value: "bytes | None"
         data_initial_chunk: "bytes | None"
         data_length: "int | None"
-        chain_id: "int | None"
+        chain_id: "int"
         tx_type: "int | None"
 
         def __init__(
             self,
             *,
+            chain_id: "int",
             address_n: "list[int] | None" = None,
             nonce: "bytes | None" = None,
             gas_price: "bytes | None" = None,
@@ -3005,7 +3006,6 @@ if TYPE_CHECKING:
             value: "bytes | None" = None,
             data_initial_chunk: "bytes | None" = None,
             data_length: "int | None" = None,
-            chain_id: "int | None" = None,
             tx_type: "int | None" = None,
         ) -> None:
             pass

core/tests/test_apps.ethereum.keychain.py

@@ -155,44 +155,6 @@ class TestEthereumKeychain(unittest.TestCase):
                 )
             )
 
-    def test_missing_chain_id(self):
-        @with_keychain_from_chain_id
-        async def handler_chain_id(ctx, msg, keychain):
-            slip44_id = msg.address_n[1] & ~HARDENED
-            # standard tests
-            self._check_keychain(keychain, slip44_id)
-            # provided address should succeed too
-            keychain.derive(msg.address_n)
-
-        await_result(  # Ethereum
-            handler_chain_id(
-                wire.DUMMY_CONTEXT,
-                EthereumSignTx(
-                    address_n=[44 | HARDENED, 60 | HARDENED, 0 | HARDENED],
-                    chain_id=None,
-                ),
-            )
-        )
-
-        await_result(  # Ethereum Classic
-            handler_chain_id(
-                wire.DUMMY_CONTEXT,
-                EthereumSignTx(
-                    address_n=[44 | HARDENED, 61 | HARDENED, 0 | HARDENED],
-                ),
-            )
-        )
-
-        with self.assertRaises(wire.DataError):
-            await_result(  # unknown slip44 id
-                handler_chain_id(
-                    wire.DUMMY_CONTEXT,
-                    EthereumSignTx(
-                        address_n=[44 | HARDENED, 0 | HARDENED, 0 | HARDENED],
-                    ),
-                )
-            )
-
     def test_wanchain(self):
         @with_keychain_from_chain_id
         async def handler_wanchain(ctx, msg, keychain):

legacy/firmware/.changelog.d/1794.incompatible

@@ -0,0 +1 @@
+Ethereum non-EIP-155 cross-chain signing is no longer supported.

legacy/firmware/ethereum.c

@@ -193,12 +193,10 @@ static void send_signature(void) {
   layoutProgress(_("Signing"), 1000);
 
   /* eip-155 replay protection */
-  if (chain_id) {
+  /* hash v=chain_id, r=0, s=0 */
-    /* hash v=chain_id, r=0, s=0 */
+  hash_rlp_number(chain_id);
-    hash_rlp_number(chain_id);
+  hash_rlp_length(0, 0);
-    hash_rlp_length(0, 0);
+  hash_rlp_length(0, 0);
-    hash_rlp_length(0, 0);
-  }
 
   keccak_Final(&keccak_ctx, hash);
   if (ecdsa_sign_digest(&secp256k1, privkey, hash, sig, &v,
@@ -216,10 +214,8 @@ static void send_signature(void) {
   msg_tx_request.has_signature_v = true;
   if (chain_id > MAX_CHAIN_ID) {
     msg_tx_request.signature_v = v;
-  } else if (chain_id) {
-    msg_tx_request.signature_v = v + 2 * chain_id + 35;
   } else {
-    msg_tx_request.signature_v = v + 27;
+    msg_tx_request.signature_v = v + 2 * chain_id + 35;
   }
 
   msg_tx_request.has_signature_r = true;
@@ -438,17 +434,12 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node) {
   if (!msg->has_nonce) msg->nonce.size = 0;
 
   /* eip-155 chain id */
-  if (msg->has_chain_id) {
+  if (msg->chain_id < 1) {
-    if (msg->chain_id < 1) {
+    fsm_sendFailure(FailureType_Failure_DataError, _("Chain ID out of bounds"));
-      fsm_sendFailure(FailureType_Failure_DataError,
+    ethereum_signing_abort();
-                      _("Chain Id out of bounds"));
+    return;
-      ethereum_signing_abort();
-      return;
-    }
-    chain_id = msg->chain_id;
-  } else {
-    chain_id = 0;
   }
+  chain_id = msg->chain_id;
 
   /* Wanchain txtype */
   if (msg->has_tx_type) {
@@ -558,11 +549,9 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node) {
   if (tx_type) {
     rlp_length += rlp_calculate_number_length(tx_type);
   }
-  if (chain_id) {
+  rlp_length += rlp_calculate_number_length(chain_id);
-    rlp_length += rlp_calculate_number_length(chain_id);
+  rlp_length += rlp_calculate_length(0, 0);
-    rlp_length += rlp_calculate_length(0, 0);
+  rlp_length += rlp_calculate_length(0, 0);
-    rlp_length += rlp_calculate_length(0, 0);
-  }
 
   /* Stage 2: Store header fields */
   hash_rlp_list_length(rlp_length);

python/src/trezorlib/messages.py

@@ -4264,13 +4264,14 @@ class EthereumSignTx(protobuf.MessageType):
         6: protobuf.Field("value", "bytes", repeated=False, required=False),
         7: protobuf.Field("data_initial_chunk", "bytes", repeated=False, required=False),
         8: protobuf.Field("data_length", "uint32", repeated=False, required=False),
-        9: protobuf.Field("chain_id", "uint64", repeated=False, required=False),
+        9: protobuf.Field("chain_id", "uint64", repeated=False, required=True),
         10: protobuf.Field("tx_type", "uint32", repeated=False, required=False),
     }
 
     def __init__(
         self,
         *,
+        chain_id: "int",
         address_n: Optional[List["int"]] = None,
         nonce: Optional["bytes"] = None,
         gas_price: Optional["bytes"] = None,
@@ -4279,10 +4280,10 @@ class EthereumSignTx(protobuf.MessageType):
         value: Optional["bytes"] = None,
         data_initial_chunk: Optional["bytes"] = None,
         data_length: Optional["int"] = None,
-        chain_id: Optional["int"] = None,
         tx_type: Optional["int"] = None,
     ) -> None:
         self.address_n = address_n if address_n is not None else []
+        self.chain_id = chain_id
         self.nonce = nonce
         self.gas_price = gas_price
         self.gas_limit = gas_limit
@@ -4290,7 +4291,6 @@ class EthereumSignTx(protobuf.MessageType):
         self.value = value
         self.data_initial_chunk = data_initial_chunk
         self.data_length = data_length
-        self.chain_id = chain_id
         self.tx_type = tx_type
 
 

tests/device_tests/ethereum/test_signtx.py

@@ -178,6 +178,7 @@ def test_data_streaming(client):
             to=TO_ADDR,
             value=0,
             data=b"ABCDEFGHIJKLMNOP" * 256 + b"!!!",
+            chain_id=1,
         )