Merge pull request #464 from LedgerHQ/feat/apa/getpk_on_other_chains

Get PK on other chains

client/src/ledger_app_clients/ethereum/client.py

@@ -124,6 +124,16 @@ def send_fund(self,
     def get_challenge(self):
         return self._send(self._cmd_builder.get_challenge())
 
+    def get_public_addr(self,
+                        display: bool = True,
+                        chaincode: bool = False,
+                        bip32_path: str = "m/44'/60'/0'/0/0",
+                        chain_id: Optional[int] = None):
+        return self._send(self._cmd_builder.get_public_addr(display,
+                                                            chaincode,
+                                                            bip32_path,
+                                                            chain_id))
+
     def provide_domain_name(self, challenge: int, name: str, addr: bytes):
         payload = format_tlv(DOMAIN_NAME_TAG.STRUCTURE_TYPE, 3)  # TrustedDomainName
         payload += format_tlv(DOMAIN_NAME_TAG.STRUCTURE_VERSION, 1)

client/src/ledger_app_clients/ethereum/command_builder.py

@@ -1,12 +1,14 @@
 import struct
 from enum import IntEnum
+from typing import Optional
 from ragger.bip import pack_derivation_path
 from typing import List
 
 from .eip712 import EIP712FieldType
 
 
 class InsType(IntEnum):
+    GET_PUBLIC_ADDR = 0x02
     SIGN = 0x04
     EIP712_SEND_STRUCT_DEF = 0x1a
     EIP712_SEND_STRUCT_IMPL = 0x1c
@@ -204,3 +206,16 @@ def provide_domain_name(self, tlv_payload: bytes) -> list[bytes]:
             payload = payload[0xff:]
             p1 = 0
         return chunks
+
+    def get_public_addr(self,
+                        display: bool,
+                        chaincode: bool,
+                        bip32_path: str,
+                        chain_id: Optional[int]) -> bytes:
+        payload = pack_derivation_path(bip32_path)
+        if chain_id is not None:
+            payload += struct.pack(">Q", chain_id)
+        return self._serialize(InsType.GET_PUBLIC_ADDR,
+                               int(display),
+                               int(chaincode),
+                               payload)

client/src/ledger_app_clients/ethereum/response_parser.py

@@ -13,3 +13,39 @@ def signature(data: bytes) -> tuple[bytes, bytes, bytes]:
 def challenge(data: bytes) -> int:
     assert len(data) == 4
     return int.from_bytes(data, "big")
+
+def pk_addr(data: bytes, has_chaincode: bool = False):
+    idx = 0
+
+    if len(data) < (idx + 1):
+        return None
+    pk_len = data[idx]
+    idx += 1
+
+    if len(data) < (idx + pk_len):
+        return None
+    pk = data[idx:idx + pk_len]
+    idx += pk_len
+
+    if len(data) < (idx + 1):
+        return None
+    addr_len = data[idx]
+    idx += 1
+
+    if len(data) < (idx + addr_len):
+        return None
+    addr = data[idx:idx + addr_len]
+    idx += addr_len
+
+    if has_chaincode:
+        if len(data) < (idx + 32):
+            return None
+        chaincode = data[idx:idx + 32]
+        idx += 32
+    else:
+        chaincode = None
+
+    if idx != len(data):
+        return None
+
+    return pk, addr.decode(), chaincode

doc/ethapp.adoc

@@ -87,6 +87,7 @@ The address can be optionally checked on the device before being returned.
 | First derivation index (big endian)                                               | 4
 | ...                                                                               | 4
 | Last derivation index (big endian)                                                | 4
+| Chain ID (big endian) (optional)                                                  | 8
 |==============================================================================================================================
 
 'Output data'

src/common_ui.h

@@ -2,13 +2,14 @@
 #define _COMMON_UI_H_
 
 #include <stdbool.h>
+#include <stdint.h>
 
 void ui_idle(void);
 void ui_warning_contract_data(void);
 void ui_display_public_eth2(void);
 void ui_display_privacy_public_key(void);
 void ui_display_privacy_shared_secret(void);
-void ui_display_public_key(void);
+void ui_display_public_key(const uint64_t *chain_id);
 void ui_sign_712_v0(void);
 void ui_display_stark_public(void);
 void ui_confirm_selector(void);

src_bagl/common_ui.c

@@ -28,7 +28,8 @@ void ui_display_privacy_shared_secret(void) {
     ux_flow_init(0, ux_display_privacy_shared_secret_flow, NULL);
 }
 
-void ui_display_public_key(void) {
+void ui_display_public_key(const uint64_t *chain_id) {
+    (void) chain_id;
     ux_flow_init(0, ux_display_public_flow, NULL);
 }
 

src_bagl/ui_domain_name.c

@@ -1,6 +1,6 @@
 #ifdef HAVE_DOMAIN_NAME
 
-#include "ux.h"
+#include "ui_domain_name.h"
 #include "domain_name.h"
 
 //////////////////////////////////////////////////////////////////////

src_bagl/ui_flow_signTx.c

@@ -158,6 +158,7 @@ UX_STEP_NOCB(
       .title = "Max Fees",
       .text = strings.common.maxFee,
     });
+
 UX_STEP_NOCB(
     ux_approval_network_step,
     bnnn_paging,
@@ -240,7 +241,7 @@ void ux_approve_tx(bool fromPlugin) {
     }
 
     uint64_t chain_id = get_tx_chain_id();
-    if (chainConfig->chainId == ETHEREUM_MAINNET_CHAINID && chain_id != chainConfig->chainId) {
+    if ((chainConfig->chainId == ETHEREUM_MAINNET_CHAINID) && (chain_id != chainConfig->chainId)) {
         ux_approval_tx_flow[step++] = &ux_approval_network_step;
     }
 

src_features/getPublicKey/cmd_getPublicKey.c

@@ -1,6 +1,6 @@
 #include "shared_context.h"
 #include "apdu_constants.h"
-
+#include "utils.h"
 #include "feature_getPublicKey.h"
 #include "ethUtils.h"
 #include "common_ui.h"
@@ -21,16 +21,18 @@ void handleGetPublicKey(uint8_t p1,
     }
 
     if ((p1 != P1_CONFIRM) && (p1 != P1_NON_CONFIRM)) {
-        THROW(0x6B00);
+        PRINTF("Error: Unexpected P1 (%u)!\n", p1);
+        THROW(APDU_RESPONSE_INVALID_P1_P2);
     }
     if ((p2 != P2_CHAINCODE) && (p2 != P2_NO_CHAINCODE)) {
-        THROW(0x6B00);
+        PRINTF("Error: Unexpected P2 (%u)!\n", p2);
+        THROW(APDU_RESPONSE_INVALID_P1_P2);
     }
 
     dataBuffer = parseBip32(dataBuffer, &dataLength, &bip32);
 
     if (dataBuffer == NULL) {
-        THROW(0x6a80);
+        THROW(APDU_RESPONSE_INVALID_DATA);
     }
 
     tmpCtx.publicKeyContext.getChaincode = (p2 == P2_CHAINCODE);
@@ -51,12 +53,26 @@ void handleGetPublicKey(uint8_t p1,
                                tmpCtx.publicKeyContext.address,
                                &global_sha3,
                                chainConfig->chainId);
+
+    uint64_t chain_id = chainConfig->chainId;
+    if (dataLength >= sizeof(chain_id)) {
+        chain_id = u64_from_BE(dataBuffer, sizeof(chain_id));
+        dataLength -= sizeof(chain_id);
+        dataBuffer += sizeof(chain_id);
+    }
+
+    (void) dataBuffer;  // to prevent dead increment warning
+    if (dataLength > 0) {
+        PRINTF("Error: Leftover unwanted data (%u bytes long)!\n", dataLength);
+        THROW(APDU_RESPONSE_INVALID_DATA);
+    }
+
 #ifndef NO_CONSENT
     if (p1 == P1_NON_CONFIRM)
 #endif  // NO_CONSENT
     {
         *tx = set_result_get_publicKey();
-        THROW(0x9000);
+        THROW(APDU_RESPONSE_OK);
     }
 #ifndef NO_CONSENT
     else {
@@ -65,7 +81,8 @@ void handleGetPublicKey(uint8_t p1,
                  "0x%.*s",
                  40,
                  tmpCtx.publicKeyContext.address);
-        ui_display_public_key();
+        // don't unnecessarily pass the current app's chain ID
+        ui_display_public_key(chainConfig->chainId == chain_id ? NULL : &chain_id);
 
         *flags |= IO_ASYNCH_REPLY;
     }

src_features/signTx/logic_signTx.c

@@ -266,6 +266,7 @@ static void nonce_to_string(const txInt256_t *nonce, char *out, size_t out_size)
 static void get_network_as_string(char *out, size_t out_size) {
     uint64_t chain_id = get_tx_chain_id();
     const char *name = get_network_name_from_chain_id(&chain_id);
+
     if (name == NULL) {
         // No network name found so simply copy the chain ID as the network name.
         u64_to_string(chain_id, out, out_size);
@@ -504,7 +505,7 @@ void finalizeParsing(bool direct) {
                     sizeof(strings.common.nonce));
     PRINTF("Nonce: %s\n", strings.common.nonce);
 
-    // Prepare chainID field
+    // Prepare network field
     get_network_as_string(strings.common.network_name, sizeof(strings.common.network_name));
     PRINTF("Network: %s\n", strings.common.network_name);
 

src_nbgl/ui_get_public_key.c

@@ -2,27 +2,57 @@
 #include "shared_context.h"
 #include "ui_callbacks.h"
 #include "ui_nbgl.h"
+#include "network.h"
 
-static void reviewReject(void) {
+static void cancel_send(void) {
     io_seproxyhal_touch_address_cancel(NULL);
 }
 
-static void confirmTransation(void) {
+static void confirm_send(void) {
     io_seproxyhal_touch_address_ok(NULL);
 }
 
-static void reviewChoice(bool confirm) {
+static void confirm_addr(void) {
+    // display a status page and go back to main
+    nbgl_useCaseStatus("ADDRESS\nVERIFIED", true, confirm_send);
+}
+
+static void reject_addr(void) {
+    nbgl_useCaseStatus("Address verification\ncancelled", false, cancel_send);
+}
+
+static void review_choice(bool confirm) {
     if (confirm) {
-        // display a status page and go back to main
-        nbgl_useCaseStatus("ADDRESS\nVERIFIED", true, confirmTransation);
+        confirm_addr();
     } else {
-        nbgl_useCaseStatus("Address verification\ncancelled", false, reviewReject);
+        reject_addr();
     }
 }
 
-static void buildScreen(void) {
-    nbgl_useCaseAddressConfirmation(strings.common.fullAddress, reviewChoice);
+static void display_addr(void) {
+    nbgl_useCaseAddressConfirmation(strings.common.fullAddress, review_choice);
+}
+
+void ui_display_public_key(const uint64_t *chain_id) {
+    // - if a chain_id is given and it's - known, we specify its network name
+    //                                   - unknown, we don't specify anything
+    // - if no chain_id is given we specify the APPNAME (legacy behaviour)
+    strlcpy(g_stax_shared_buffer, "Verify ", sizeof(g_stax_shared_buffer));
+    if (chain_id != NULL) {
+        if (chain_is_ethereum_compatible(chain_id)) {
+            strlcat(g_stax_shared_buffer,
+                    get_network_name_from_chain_id(chain_id),
+                    sizeof(g_stax_shared_buffer));
+            strlcat(g_stax_shared_buffer, "\n", sizeof(g_stax_shared_buffer));
+        }
+    } else {
+        strlcat(g_stax_shared_buffer, APPNAME "\n", sizeof(g_stax_shared_buffer));
+    }
+    strlcat(g_stax_shared_buffer, "address", sizeof(g_stax_shared_buffer));
+    nbgl_useCaseReviewStart(get_app_icon(false),
+                            g_stax_shared_buffer,
+                            NULL,
+                            "Cancel",
+                            display_addr,
+                            reject_addr);
 }
-void ui_display_public_key(void) {
-    buildScreen();
-}

tests/ragger/test_get_address.py

@@ -0,0 +1,80 @@
+import pytest
+from typing import Optional
+from pathlib import Path
+from ragger.error import ExceptionRAPDU
+from ragger.firmware import Firmware
+from ragger.backend import BackendInterface
+from ragger.navigator import Navigator, NavInsID
+from ledger_app_clients.ethereum.client import EthAppClient, StatusWord
+from ledger_app_clients.ethereum.settings import SettingID, settings_toggle
+import ledger_app_clients.ethereum.response_parser as ResponseParser
+from ragger.bip import calculate_public_key_and_chaincode, CurveChoice
+
+ROOT_SCREENSHOT_PATH = Path(__file__).parent
+
+@pytest.fixture(params=[True, False])
+def with_chaincode(request) -> bool:
+    return request.param
+
+@pytest.fixture(params=[None, 1, 2, 5, 137])
+def chain(request) -> Optional[int]:
+    return request.param
+
+def get_moves(firmware: Firmware,
+              navigator: BackendInterface,
+              chain: Optional[int] = None,
+              reject: bool = False):
+    moves = list()
+
+    if firmware.is_nano:
+        moves += [ NavInsID.RIGHT_CLICK ]
+        if firmware.device == "nanos":
+            moves += [ NavInsID.RIGHT_CLICK ] * 3
+        else:
+            moves += [ NavInsID.RIGHT_CLICK ]
+        if reject:
+            moves += [ NavInsID.RIGHT_CLICK ]
+        moves += [ NavInsID.BOTH_CLICK ]
+    else:
+        moves += [ NavInsID.USE_CASE_REVIEW_TAP ]
+        if chain is not None and chain > 1:
+            moves += [ NavInsID.USE_CASE_ADDRESS_CONFIRMATION_TAP ]
+        if reject:
+            moves += [ NavInsID.USE_CASE_ADDRESS_CONFIRMATION_CANCEL ]
+        else:
+            moves += [ NavInsID.USE_CASE_ADDRESS_CONFIRMATION_CONFIRM ]
+
+    return moves
+
+def test_get_pk_rejected(firmware: Firmware,
+                         backend: BackendInterface,
+                         navigator: Navigator):
+    app_client = EthAppClient(backend)
+
+    try:
+        with app_client.get_public_addr():
+            navigator.navigate_and_compare(ROOT_SCREENSHOT_PATH,
+                                           "get_pk_rejected",
+                                           get_moves(firmware, navigator, reject=True))
+    except ExceptionRAPDU as e:
+        assert e.status == StatusWord.CONDITION_NOT_SATISFIED
+    else:
+        assert False # An exception should have been raised
+
+def test_get_pk(firmware: Firmware,
+                backend: BackendInterface,
+                navigator: Navigator,
+                with_chaincode: bool,
+                chain: Optional[int]):
+    app_client = EthAppClient(backend)
+
+    with app_client.get_public_addr(chaincode=with_chaincode, chain_id=chain):
+        navigator.navigate_and_compare(ROOT_SCREENSHOT_PATH,
+                                       "get_pk_%s" % (chain),
+                                       get_moves(firmware, navigator, chain=chain))
+    pk, addr, chaincode = ResponseParser.pk_addr(app_client.response().data, with_chaincode)
+    ref_pk, ref_chaincode = calculate_public_key_and_chaincode(curve=CurveChoice.Secp256k1,
+                                                               path="m/44'/60'/0'/0/0")
+    assert pk.hex() == ref_pk
+    if with_chaincode:
+        assert chaincode.hex() == ref_chaincode