diff --git a/common/protob/messages-ethereum.proto b/common/protob/messages-ethereum.proto index 1b0bd52bf..8ac7d768e 100644 --- a/common/protob/messages-ethereum.proto +++ b/common/protob/messages-ethereum.proto @@ -159,7 +159,7 @@ message EthereumVerifyMessage { message EthereumSignTypedHash { repeated uint32 address_n = 1; // BIP-32 path to derive the key from master node required bytes domain_separator_hash = 2; // Hash of domainSeparator of typed data to be signed - required bytes message_hash = 3; // Hash of the data of typed data to be signed + optional bytes message_hash = 3; // Hash of the data of typed data to be signed (empty if domain-only data) } /** diff --git a/common/tests/fixtures/ethereum/sign_typed_data.json b/common/tests/fixtures/ethereum/sign_typed_data.json index dabb9dd49..ae98bb401 100644 --- a/common/tests/fixtures/ethereum/sign_typed_data.json +++ b/common/tests/fixtures/ethereum/sign_typed_data.json @@ -4,6 +4,77 @@ "passphrase": "" }, "tests": [ + { + "name": "bare_minimum", + "comment": "Bare minimum EIP-712 message (domain only)", + "parameters": { + "path": "m/44'/60'/0'/0/0", + "metamask_v4_compat": true, + "data": { + "types": { + "EIP712Domain": [] + }, + "primaryType": "EIP712Domain", + "message": {}, + "domain": {} + }, + "message_hash": null, + "domain_separator_hash": "0x6192106f129ce05c9075d319c1fa6ea9b3ae37cbd0c1ef92e2be7137bb07baa1" + }, + "result": { + "address": "0x73d0385F4d8E00C5e6504C6030F47BF6212736A8", + "sig": "0x18aaea9abed7cd88d3763a9a420e2e7b71a9f991685fbc62d74da86326cffa680644862d459d1973e422777a3933bc74190b1cae9a5418ddaea645a7d7630dd91c" + } + }, + { + "name": "full_domain_empty_message", + "comment": "Domain only EIP-712 message", + "parameters": { + "path": "m/44'/60'/0'/0/0", + "metamask_v4_compat": true, + "data": { + "types": { + "EIP712Domain": [ + { + "name": "name", + "type": "string" + }, + { + "name": "version", + "type": "string" + }, + { + "name": "chainId", + "type": "uint256" + }, + { + "name": "verifyingContract", + "type": "address" + }, + { + "name": "salt", + "type": "bytes32" + } + ] + }, + "primaryType": "EIP712Domain", + "message": {}, + "domain": { + "name": "Trezor", + "version": "Test v0.0.0", + "chainId": 1, + "verifyingContract": "0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC", + "salt": "0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + } + }, + "message_hash": null, + "domain_separator_hash": "0xf85aaf157e9a36dc6e12643fff450fdf8d98fd0d0e41c5b42bb1f7aae6c83388" + }, + "result": { + "address": "0x73d0385F4d8E00C5e6504C6030F47BF6212736A8", + "sig": "0x98a3e66f738002da98c70b976ef131c11ed8b94aad872140574ed2a2d4a2bac53a9350e284994274f0a7ce1191cf79bf13f2f0d0a862dcf0dd86ad8141eb90dc1c" + } + }, { "name": "basic_data", "parameters": { diff --git a/core/.changelog.d/2036.fixed b/core/.changelog.d/2036.fixed new file mode 100644 index 000000000..c402937a7 --- /dev/null +++ b/core/.changelog.d/2036.fixed @@ -0,0 +1 @@ +Fix domain-only EIP-712 hashes (i.e. when `primaryType`=`EIP712Domain`) diff --git a/core/src/apps/ethereum/layout.py b/core/src/apps/ethereum/layout.py index 3480c36ec..863661e4f 100644 --- a/core/src/apps/ethereum/layout.py +++ b/core/src/apps/ethereum/layout.py @@ -6,6 +6,7 @@ from trezor.enums import ButtonRequestType, EthereumDataType from trezor.messages import EthereumFieldType, EthereumStructMember from trezor.strings import format_amount, format_plural from trezor.ui.layouts import ( + confirm_action, confirm_address, confirm_amount, confirm_blob, @@ -111,16 +112,27 @@ def require_confirm_data(ctx: Context, data: bytes, data_total: int) -> Awaitabl ) -async def confirm_hash(ctx: Context, message_hash: bytes) -> None: - await confirm_blob( +async def confirm_typed_data_final(ctx: Context) -> None: + await confirm_action( ctx, - "confirm_hash", - title="Confirm hash", - data="0x" + hexlify(message_hash).decode(), + "confirm_typed_data_final", + title="Confirm typed data", + action="Really sign EIP-712 typed data?", + verb="Hold to confirm", hold=True, ) +def confirm_empty_typed_message(ctx: Context) -> Awaitable[None]: + return confirm_text( + ctx, + "confirm_empty_typed_message", + title="Confirm message", + data="", + description="No message field", + ) + + async def should_show_domain(ctx: Context, name: bytes, version: bytes) -> bool: domain_name = decode_typed_data(name, "string") domain_version = decode_typed_data(version, "string") diff --git a/core/src/apps/ethereum/sign_typed_data.py b/core/src/apps/ethereum/sign_typed_data.py index d423068c8..87bd8b31d 100644 --- a/core/src/apps/ethereum/sign_typed_data.py +++ b/core/src/apps/ethereum/sign_typed_data.py @@ -20,7 +20,8 @@ from apps.common import paths from .helpers import address_from_bytes, get_type_name from .keychain import PATTERNS_ADDRESS, with_keychain_from_path from .layout import ( - confirm_hash, + confirm_empty_typed_message, + confirm_typed_data_final, confirm_typed_value, should_show_array, should_show_domain, @@ -82,23 +83,30 @@ async def generate_typed_data_hash( parent_objects=["EIP712Domain"], ) - show_message = await should_show_struct( - ctx, - description=primary_type, - data_members=typed_data_envelope.types[primary_type].members, - title="Confirm message", - button_text="Show full message", - ) - message_hash = await typed_data_envelope.hash_struct( - primary_type=primary_type, - member_path=[1], - show_data=show_message, - parent_objects=[primary_type], - ) + # Setting the primary_type to "EIP712Domain" is technically in spec + # In this case, we ignore the "message" part and only use the "domain" part + # https://ethereum-magicians.org/t/eip-712-standards-clarification-primarytype-as-domaintype/3286 + if primary_type == "EIP712Domain": + await confirm_empty_typed_message(ctx) + message_hash = b"" + else: + show_message = await should_show_struct( + ctx, + description=primary_type, + data_members=typed_data_envelope.types[primary_type].members, + title="Confirm message", + button_text="Show full message", + ) + message_hash = await typed_data_envelope.hash_struct( + primary_type=primary_type, + member_path=[1], + show_data=show_message, + parent_objects=[primary_type], + ) - await confirm_hash(ctx, message_hash) + await confirm_typed_data_final(ctx) - return keccak256(b"\x19" + b"\x01" + domain_separator + message_hash) + return keccak256(b"\x19\x01" + domain_separator + message_hash) def get_hash_writer() -> HashWriter: diff --git a/core/src/trezor/messages.py b/core/src/trezor/messages.py index f59e4b841..71bea4642 100644 --- a/core/src/trezor/messages.py +++ b/core/src/trezor/messages.py @@ -3234,14 +3234,14 @@ if TYPE_CHECKING: class EthereumSignTypedHash(protobuf.MessageType): address_n: "list[int]" domain_separator_hash: "bytes" - message_hash: "bytes" + message_hash: "bytes | None" def __init__( self, *, domain_separator_hash: "bytes", - message_hash: "bytes", address_n: "list[int] | None" = None, + message_hash: "bytes | None" = None, ) -> None: pass diff --git a/legacy/firmware/.changelog.d/2036.fixed b/legacy/firmware/.changelog.d/2036.fixed new file mode 100644 index 000000000..c402937a7 --- /dev/null +++ b/legacy/firmware/.changelog.d/2036.fixed @@ -0,0 +1 @@ +Fix domain-only EIP-712 hashes (i.e. when `primaryType`=`EIP712Domain`) diff --git a/legacy/firmware/ethereum.c b/legacy/firmware/ethereum.c index ae9c88559..66289f393 100644 --- a/legacy/firmware/ethereum.c +++ b/legacy/firmware/ethereum.c @@ -976,14 +976,20 @@ int ethereum_message_verify(const EthereumVerifyMessage *msg) { return 0; } +/* + * EIP-712 hashes might have no message_hash if primaryType="EIP712Domain". + * In this case, set has_message_hash=false. + */ static void ethereum_typed_hash(const uint8_t domain_separator_hash[32], const uint8_t message_hash[32], - uint8_t hash[32]) { + bool has_message_hash, uint8_t hash[32]) { struct SHA3_CTX ctx = {0}; sha3_256_Init(&ctx); sha3_Update(&ctx, (const uint8_t *)"\x19\x01", 2); sha3_Update(&ctx, domain_separator_hash, 32); - sha3_Update(&ctx, message_hash, 32); + if (has_message_hash) { + sha3_Update(&ctx, message_hash, 32); + } keccak_Final(&ctx, hash); } @@ -991,8 +997,9 @@ void ethereum_typed_hash_sign(const EthereumSignTypedHash *msg, const HDNode *node, EthereumTypedDataSignature *resp) { uint8_t hash[32] = {0}; + ethereum_typed_hash(msg->domain_separator_hash.bytes, msg->message_hash.bytes, - hash); + msg->has_message_hash, hash); uint8_t v = 0; if (ecdsa_sign_digest(&secp256k1, node->private_key, hash, diff --git a/legacy/firmware/fsm_msg_ethereum.h b/legacy/firmware/fsm_msg_ethereum.h index 41813a58a..226dbb26e 100644 --- a/legacy/firmware/fsm_msg_ethereum.h +++ b/legacy/firmware/fsm_msg_ethereum.h @@ -216,7 +216,8 @@ void fsm_msgEthereumSignTypedHash(const EthereumSignTypedHash *msg) { CHECK_PIN - if (msg->domain_separator_hash.size != 32 || msg->message_hash.size != 32) { + if (msg->domain_separator_hash.size != 32 || + (msg->has_message_hash && msg->message_hash.size != 32)) { fsm_sendFailure(FailureType_Failure_DataError, _("Invalid hash length")); return; } @@ -256,12 +257,17 @@ void fsm_msgEthereumSignTypedHash(const EthereumSignTypedHash *msg) { layoutHome(); return; } - layoutConfirmHash(&bmp_icon_warning, _("EIP-712 message hash"), - msg->message_hash.bytes, 32); - if (!protectButton(ButtonRequestType_ButtonRequest_Other, false)) { - fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); - layoutHome(); - return; + + // No message hash when setting primaryType="EIP712Domain" + // https://ethereum-magicians.org/t/eip-712-standards-clarification-primarytype-as-domaintype/3286 + if (msg->has_message_hash) { + layoutConfirmHash(&bmp_icon_warning, _("EIP-712 message hash"), + msg->message_hash.bytes, 32); + if (!protectButton(ButtonRequestType_ButtonRequest_Other, false)) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); + layoutHome(); + return; + } } ethereum_typed_hash_sign(msg, node, resp); diff --git a/python/.changelog.d/2036.fixed b/python/.changelog.d/2036.fixed new file mode 100644 index 000000000..16f663897 --- /dev/null +++ b/python/.changelog.d/2036.fixed @@ -0,0 +1,2 @@ +Allow passing empty `message_hash` for domain-only EIP-712 hashes +for Trezor T1 (i.e. when `primaryType`=`EIP712Domain`) diff --git a/python/src/trezorlib/cli/ethereum.py b/python/src/trezorlib/cli/ethereum.py index 7d5d4c779..61c5d3ddb 100644 --- a/python/src/trezorlib/cli/ethereum.py +++ b/python/src/trezorlib/cli/ethereum.py @@ -455,10 +455,12 @@ def sign_typed_data_hash( Sign hash of typed data (EIP-712) with Ethereum address. For T1 backward compatibility. + + MESSAGE_HASH_HEX can be set to an empty string '' for domain-only hashes. """ address_n = tools.parse_path(address) domain_hash = ethereum.decode_hex(domain_hash_hex) - message_hash = ethereum.decode_hex(message_hash_hex) + message_hash = ethereum.decode_hex(message_hash_hex) if message_hash_hex else None ret = ethereum.sign_typed_data_hash(client, address_n, domain_hash, message_hash) output = { "domain_hash": domain_hash_hex, diff --git a/python/src/trezorlib/ethereum.py b/python/src/trezorlib/ethereum.py index bc3147e1d..098dbecc2 100644 --- a/python/src/trezorlib/ethereum.py +++ b/python/src/trezorlib/ethereum.py @@ -361,7 +361,10 @@ def verify_message( @expect(messages.EthereumTypedDataSignature) def sign_typed_data_hash( - client: "TrezorClient", n: "Address", domain_hash: bytes, message_hash: bytes + client: "TrezorClient", + n: "Address", + domain_hash: bytes, + message_hash: Optional[bytes], ) -> "MessageType": return client.call( messages.EthereumSignTypedHash( diff --git a/python/src/trezorlib/messages.py b/python/src/trezorlib/messages.py index 9548b0a32..162b78872 100644 --- a/python/src/trezorlib/messages.py +++ b/python/src/trezorlib/messages.py @@ -4742,15 +4742,15 @@ class EthereumSignTypedHash(protobuf.MessageType): FIELDS = { 1: protobuf.Field("address_n", "uint32", repeated=True, required=False), 2: protobuf.Field("domain_separator_hash", "bytes", repeated=False, required=True), - 3: protobuf.Field("message_hash", "bytes", repeated=False, required=True), + 3: protobuf.Field("message_hash", "bytes", repeated=False, required=False), } def __init__( self, *, domain_separator_hash: "bytes", - message_hash: "bytes", address_n: Optional[Sequence["int"]] = None, + message_hash: Optional["bytes"] = None, ) -> None: self.address_n: Sequence["int"] = address_n if address_n is not None else [] self.domain_separator_hash = domain_separator_hash diff --git a/tests/device_tests/ethereum/test_sign_typed_data.py b/tests/device_tests/ethereum/test_sign_typed_data.py index 9b184ee1b..47186cef4 100644 --- a/tests/device_tests/ethereum/test_sign_typed_data.py +++ b/tests/device_tests/ethereum/test_sign_typed_data.py @@ -50,7 +50,10 @@ def test_ethereum_sign_typed_data_blind(client, parameters, result): client, address_n, ethereum.decode_hex(parameters["domain_separator_hash"]), - ethereum.decode_hex(parameters["message_hash"]), + # message hash is empty for domain-only hashes + ethereum.decode_hex(parameters["message_hash"]) + if parameters["message_hash"] + else None, ) assert ret.address == result["address"] assert f"0x{ret.signature.hex()}" == result["sig"] diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index 4add89ba5..5b3e28e93 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -602,13 +602,15 @@ "ethereum-test_getpublickey.py::test_ethereum_getpublickey[parameters0-result0]": "095af81ec79e9b510c90d9fa34fed343f3840807190c67bc237af885695ae687", "ethereum-test_getpublickey.py::test_ethereum_getpublickey[parameters1-result1]": "095af81ec79e9b510c90d9fa34fed343f3840807190c67bc237af885695ae687", "ethereum-test_getpublickey.py::test_ethereum_getpublickey[parameters2-result2]": "095af81ec79e9b510c90d9fa34fed343f3840807190c67bc237af885695ae687", -"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[basic_data]": "4f512d6beb0222079aaa878a2bcd2c41ac389fdd47edad3e0b0b5779677d8697", -"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[complex_data]": "275a8630a795c966419f6fc6de834bb576bfc3dbc16a1fd7605aa2b8ceae666e", -"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[struct_list_non_v4]": "7dd23b14bd273b937836a24cf056c745e7b3461139f895caefd4624e0d5545f5", -"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[struct_list_v4]": "b7e3475d4906942bc0e8d62203ae91a13ea0d702c3a7a53b9777bea670c4a7f7", -"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[structs_arrays_v4]": "31cc5b5c1d9d94f0761208f5dc6423d283b9118b12b87cf878d20fa144f4f252", +"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[bare_minimum]": "e13b237f58a9977c6edf1917730748b881749acecc3d040b1fd9d794e6e02299", +"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[basic_data]": "84f1d23e031919fedb4ef6b3f3dc1cb667836b9efcd0c5a0da586010e38e1fd6", +"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[complex_data]": "27ee23894c19e5704d02ae684d50a0f0d14678d3f74a95e67fc4975818a1faa8", +"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[full_domain_empty_message]": "c3c299087e9cac4d7554314abc8637d041da69f0c0ee2863c5526f1f89448ae2", +"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[struct_list_non_v4]": "84f1d23e031919fedb4ef6b3f3dc1cb667836b9efcd0c5a0da586010e38e1fd6", +"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[struct_list_v4]": "84f1d23e031919fedb4ef6b3f3dc1cb667836b9efcd0c5a0da586010e38e1fd6", +"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[structs_arrays_v4]": "84f1d23e031919fedb4ef6b3f3dc1cb667836b9efcd0c5a0da586010e38e1fd6", "ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data_cancel": "08712efae2d007610289bbfb3a8fe6800547e884636c83c5bf0e25f33728789e", -"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data_show_more_button": "1adbea797586685ce09aae58b0a2b89e1617e4eaad23a8c1ac6fc10b041e57a5", +"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data_show_more_button": "72959d95278c136cb2071f6452b2a61782b9ac3e6946d8fb5f8db8f153171e6f", "ethereum-test_sign_verify_message.py::test_signmessage[parameters0-result0]": "7aa14b29e5005d8fdc0a8b497ed5d3ebea15c7017f9c457d09214f2d05fbc532", "ethereum-test_sign_verify_message.py::test_signmessage[parameters1-result1]": "c5fb9393267c3d9b9bf5839aab6c641d3931286411f291cd1d8e937cb224ae2d", "ethereum-test_sign_verify_message.py::test_signmessage[parameters2-result2]": "8499b87474becc06010e9b4356398a3e29ee2ef152e04653f15dcc227fc486d6",