Skip to content

Commit

Permalink
Merge #473: ledger: ensure only Bitcoin and Bitcoin Test apps are active
Browse files Browse the repository at this point in the history
34003f8 ledger: require the app name is Bitcoin or Bitcoin Test (Andrew Chow)
d1d0574 Expose app name of ledger devices (João Barbosa)

Pull request description:

  When the user is in an app on the Ledger that is not Bitcoin or Bitcoin Test, we should not be enumerating the device or allowing them to use HWI as unexpected and unsupported behavior may occur. To do this, this PR pulls in (part of) #362 in order to get the current app name. Then it checks that the app name is one of `Bitcoin`, `Bitcoin Test`, or `app` (for the app included in Speculos). For older devices which do not implement this instruction, the name is returned as `None`. Such devices will be enumerated and accessible as previously as we do not want to lock out users using old devices.

  Closes #158

ACKs for top commit:
  instagibbs:
    tACK 34003f8

Tree-SHA512: 3870c3f7e0d5a36345111cf5aba86f993a7b849fea4a8a09f148b80e38b595e5a9f24b13a04b3cff45806018be2c51df37c5e0fdbe9b8b30991dde9c8922f462
  • Loading branch information
instagibbs committed Mar 2, 2021
2 parents 7b34fc7 + 34003f8 commit eb1ce2c
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 0 deletions.
19 changes: 19 additions & 0 deletions hwilib/devices/btchip/btchip.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@

class btchip:
BTCHIP_CLA = 0xe0
BTCHIP_CLA_COMMON_SDK = 0xb0
BTCHIP_JC_EXT_CLA = 0xf0

BTCHIP_INS_GET_APP_NAME_AND_VERSION = 0x01
BTCHIP_INS_SET_ALTERNATE_COIN_VERSION = 0x14
BTCHIP_INS_SETUP = 0x20
BTCHIP_INS_VERIFY_PIN = 0x22
Expand Down Expand Up @@ -407,6 +409,23 @@ def signMessageSign(self, pin=""):
response = self.dongle.exchange(bytearray(apdu))
return response

def getAppName(self):
apdu = [ self.BTCHIP_CLA_COMMON_SDK, self.BTCHIP_INS_GET_APP_NAME_AND_VERSION, 0x00, 0x00, 0x00 ]
try:
response = self.dongle.exchange(bytearray(apdu))
name_len = response[1]
name = response[2:][:name_len]
if b'OLOS' not in name:
return name.decode('ascii')
except BTChipException as e:
if e.sw == 0x6faa:
# ins not implemented"
return None
if e.sw == 0x6d00:
# Not in an app, return just a string saying that
return "not in an app"
raise

def getFirmwareVersion(self):
result = {}
apdu = [ self.BTCHIP_CLA, self.BTCHIP_INS_GET_FIRMWARE_VERSION, 0x00, 0x00, 0x00 ]
Expand Down
7 changes: 7 additions & 0 deletions hwilib/devices/ledger.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
DeviceConnectionError,
DeviceFailureError,
UnavailableActionError,
UnknownDeviceError,
common_err_msgs,
handle_errors,
)
Expand Down Expand Up @@ -136,6 +137,9 @@ def __init__(self, path: str, password: str = "", expert: bool = False) -> None:

self.app = btchip(self.dongle)

if self.app.getAppName() not in ["Bitcoin", "Bitcoin Test", "app"]:
raise UnknownDeviceError("Ledger is not in either the Bitcoin or Bitcoin Testnet app")

@ledger_exception
def get_pubkey_at_path(self, path: str) -> ExtendedKey:
if not check_keypath(path):
Expand Down Expand Up @@ -460,6 +464,9 @@ def enumerate(password: str = '') -> List[Dict[str, Any]]:
continue
else:
raise
except UnknownDeviceError:
# This only happens if the ledger is not in the Bitcoin app, so skip it
continue

if client:
client.close()
Expand Down

0 comments on commit eb1ce2c

Please sign in to comment.