From d2696cfadf6b09bfffe606334e4083250dd7e72d Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 22 Apr 2019 10:34:34 -0400 Subject: [PATCH 1/5] Add needs_passphrase_sent to enumerate output --- hwilib/devices/coldcard.py | 2 ++ hwilib/devices/digitalbitbox.py | 1 + hwilib/devices/keepkey.py | 4 ++++ hwilib/devices/ledger.py | 4 +--- hwilib/devices/trezor.py | 4 ++++ 5 files changed, 12 insertions(+), 3 deletions(-) diff --git a/hwilib/devices/coldcard.py b/hwilib/devices/coldcard.py index 91689a4..bed73aa 100644 --- a/hwilib/devices/coldcard.py +++ b/hwilib/devices/coldcard.py @@ -225,6 +225,7 @@ def enumerate(password=''): path = d['path'].decode() d_data['type'] = 'coldcard' d_data['path'] = path + d_data['needs_passphrase'] = False client = None try: @@ -251,6 +252,7 @@ def enumerate(password=''): d_data['fingerprint'] = client._get_fingerprint_hex() d_data['type'] = 'coldcard' d_data['path'] = CC_SIMULATOR_SOCK + d_data['needs_passphrase_sent'] = False results.append(d_data) except RuntimeError as e: if str(e) == 'Cannot connect to simulator. Is it running?': diff --git a/hwilib/devices/digitalbitbox.py b/hwilib/devices/digitalbitbox.py index 1ef0dbb..d229005 100644 --- a/hwilib/devices/digitalbitbox.py +++ b/hwilib/devices/digitalbitbox.py @@ -603,6 +603,7 @@ def enumerate(password=''): else: master_xpub = client.get_pubkey_at_path('m/0h')['xpub'] d_data['fingerprint'] = get_xpub_fingerprint_hex(master_xpub) + d_data['needs_passphrase_sent'] = True except HWWError as e: d_data['error'] = "Could not open client or get fingerprint information: " + e.get_msg() d_data['code'] = e.get_code() diff --git a/hwilib/devices/keepkey.py b/hwilib/devices/keepkey.py index 463fe95..15e7d8a 100644 --- a/hwilib/devices/keepkey.py +++ b/hwilib/devices/keepkey.py @@ -26,9 +26,13 @@ def enumerate(password=''): client.client.init_device() if not 'keepkey' in client.client.features.vendor: continue + d_data['needs_passphrase_sent'] = client.client.features.passphrase_protection and not client.client.features.passphrase_cached + if d_data['needs_passphrase_sent'] and not password: + raise DeviceNotReadyError("Passphrase needs to be specified before the fingerprint information can be retrieved") if client.client.features.initialized: master_xpub = client.get_pubkey_at_path('m/0h')['xpub'] d_data['fingerprint'] = get_xpub_fingerprint_hex(master_xpub) + d_data['needs_passphrase_sent'] = False # Passphrase is always needed for the above to have worked, so it's already sent else: d_data['error'] = 'Not initialized' except HWWError as e: diff --git a/hwilib/devices/ledger.py b/hwilib/devices/ledger.py index 8c2151f..1d6984e 100644 --- a/hwilib/devices/ledger.py +++ b/hwilib/devices/ledger.py @@ -350,9 +350,7 @@ def enumerate(password=''): client = LedgerClient(path, password) master_xpub = client.get_pubkey_at_path('m/0h')['xpub'] d_data['fingerprint'] = get_xpub_fingerprint_hex(master_xpub) - except HWWError as e: - d_data['error'] = "Could not open client or get fingerprint information: " + e.get_msg() - d_data['code'] = e.get_code() + d_data['needs_passphrase_sent'] = False except Exception as e: d_data['error'] = "Could not open client or get fingerprint information: " + str(e) d_data['code'] = UNKNOWN_ERROR diff --git a/hwilib/devices/trezor.py b/hwilib/devices/trezor.py index 025249f..05cea6d 100644 --- a/hwilib/devices/trezor.py +++ b/hwilib/devices/trezor.py @@ -421,9 +421,13 @@ def enumerate(password=''): client.client.init_device() if not 'trezor' in client.client.features.vendor: continue + d_data['needs_passphrase_sent'] = client.client.features.passphrase_protection and not client.client.features.passphrase_cached + if d_data['needs_passphrase_sent'] and not password: + raise DeviceNotReadyError("Passphrase needs to be specified before the fingerprint information can be retrieved") if client.client.features.initialized: master_xpub = client.get_pubkey_at_path('m/0h')['xpub'] d_data['fingerprint'] = get_xpub_fingerprint_hex(master_xpub) + d_data['needs_passphrase_sent'] = False # Passphrase is always needed for the above to have worked, so it's already sent else: d_data['error'] = 'Not initialized' except HWWError as e: From bca85354f912c772b06ce90c54057857f04d7136 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 22 Apr 2019 19:34:46 -0400 Subject: [PATCH 2/5] Add needs_pin_sent to indicate whether promptpin needs to be used --- hwilib/devices/coldcard.py | 1 + hwilib/devices/digitalbitbox.py | 1 + hwilib/devices/keepkey.py | 3 +++ hwilib/devices/ledger.py | 1 + hwilib/devices/trezor.py | 3 +++ 5 files changed, 9 insertions(+) diff --git a/hwilib/devices/coldcard.py b/hwilib/devices/coldcard.py index bed73aa..2aea3b2 100644 --- a/hwilib/devices/coldcard.py +++ b/hwilib/devices/coldcard.py @@ -252,6 +252,7 @@ def enumerate(password=''): d_data['fingerprint'] = client._get_fingerprint_hex() d_data['type'] = 'coldcard' d_data['path'] = CC_SIMULATOR_SOCK + d_data['needs_pin_sent'] = False d_data['needs_passphrase_sent'] = False results.append(d_data) except RuntimeError as e: diff --git a/hwilib/devices/digitalbitbox.py b/hwilib/devices/digitalbitbox.py index d229005..3ec3922 100644 --- a/hwilib/devices/digitalbitbox.py +++ b/hwilib/devices/digitalbitbox.py @@ -603,6 +603,7 @@ def enumerate(password=''): else: master_xpub = client.get_pubkey_at_path('m/0h')['xpub'] d_data['fingerprint'] = get_xpub_fingerprint_hex(master_xpub) + d_data['needs_pin_sent'] = False d_data['needs_passphrase_sent'] = True except HWWError as e: d_data['error'] = "Could not open client or get fingerprint information: " + e.get_msg() diff --git a/hwilib/devices/keepkey.py b/hwilib/devices/keepkey.py index 15e7d8a..a4bfaf4 100644 --- a/hwilib/devices/keepkey.py +++ b/hwilib/devices/keepkey.py @@ -26,7 +26,10 @@ def enumerate(password=''): client.client.init_device() if not 'keepkey' in client.client.features.vendor: continue + d_data['needs_pin_sent'] = client.client.features.pin_protection and not client.client.features.pin_cached d_data['needs_passphrase_sent'] = client.client.features.passphrase_protection and not client.client.features.passphrase_cached + if d_data['needs_pin_sent']: + raise DeviceNotReadyError('Keepkey is locked. Unlock by using \'promptpin\' and then \'sendpin\'.') if d_data['needs_passphrase_sent'] and not password: raise DeviceNotReadyError("Passphrase needs to be specified before the fingerprint information can be retrieved") if client.client.features.initialized: diff --git a/hwilib/devices/ledger.py b/hwilib/devices/ledger.py index 1d6984e..d35fac5 100644 --- a/hwilib/devices/ledger.py +++ b/hwilib/devices/ledger.py @@ -350,6 +350,7 @@ def enumerate(password=''): client = LedgerClient(path, password) master_xpub = client.get_pubkey_at_path('m/0h')['xpub'] d_data['fingerprint'] = get_xpub_fingerprint_hex(master_xpub) + d_data['needs_pin_sent'] = False d_data['needs_passphrase_sent'] = False except Exception as e: d_data['error'] = "Could not open client or get fingerprint information: " + str(e) diff --git a/hwilib/devices/trezor.py b/hwilib/devices/trezor.py index 05cea6d..fb0d9e9 100644 --- a/hwilib/devices/trezor.py +++ b/hwilib/devices/trezor.py @@ -421,7 +421,10 @@ def enumerate(password=''): client.client.init_device() if not 'trezor' in client.client.features.vendor: continue + d_data['needs_pin_sent'] = client.client.features.pin_protection and not client.client.features.pin_cached d_data['needs_passphrase_sent'] = client.client.features.passphrase_protection and not client.client.features.passphrase_cached + if d_data['needs_pin_sent']: + raise DeviceNotReadyError('Trezor is locked. Unlock by using \'promptpin\' and then \'sendpin\'.') if d_data['needs_passphrase_sent'] and not password: raise DeviceNotReadyError("Passphrase needs to be specified before the fingerprint information can be retrieved") if client.client.features.initialized: From 1c33312bb1b1fd7fe74e5c193efd8c1ab1123759 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 22 Apr 2019 23:34:00 -0400 Subject: [PATCH 3/5] Test need_passphrase_sent and need_pin_sent --- hwilib/devices/trezor.py | 1 + test/test_keepkey.py | 58 ++++++++++++++++++++++++++++++++++++++++ test/test_trezor.py | 57 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+) diff --git a/hwilib/devices/trezor.py b/hwilib/devices/trezor.py index fb0d9e9..6289303 100644 --- a/hwilib/devices/trezor.py +++ b/hwilib/devices/trezor.py @@ -100,6 +100,7 @@ class TrezorClient(HardwareWalletClient): transport = get_transport(path) self.client = TrezorClientDebugLink(transport=transport) self.simulator = True + self.client.set_passphrase(password) else: self.client = Trezor(transport=get_transport(path), ui=PassphraseUI(password)) diff --git a/test/test_keepkey.py b/test/test_keepkey.py index 91eef05..ef0ff4c 100755 --- a/test/test_keepkey.py +++ b/test/test_keepkey.py @@ -180,11 +180,19 @@ class TestKeepkeyManCommands(KeepkeyTestCase): result = self.do_command(self.dev_args + ['sendpin', '1234']) self.assertEqual(result['error'], 'This device does not need a PIN') self.assertEqual(result['code'], -11) + result = self.do_command(self.dev_args + ['enumerate']) + for dev in result: + if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_pin_sent']) # Set a PIN device.wipe(self.client) load_device_by_mnemonic(client=self.client, mnemonic='alcohol woman abuse must during monitor noble actual mixed trade anger aisle', pin='1234', passphrase_protection=False, label='test') self.client.call(messages.ClearSession()) + result = self.do_command(self.dev_args + ['enumerate']) + for dev in result: + if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertTrue(dev['needs_pin_sent']) result = self.do_command(self.dev_args + ['promptpin']) self.assertTrue(result['success']) @@ -212,6 +220,11 @@ class TestKeepkeyManCommands(KeepkeyTestCase): result = self.do_command(self.dev_args + ['sendpin', pin]) self.assertTrue(result['success']) + result = self.do_command(self.dev_args + ['enumerate']) + for dev in result: + if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_pin_sent']) + # Sending PIN after unlock result = self.do_command(self.dev_args + ['promptpin']) self.assertEqual(result['error'], 'The PIN has already been sent to this device') @@ -220,6 +233,51 @@ class TestKeepkeyManCommands(KeepkeyTestCase): self.assertEqual(result['error'], 'The PIN has already been sent to this device') self.assertEqual(result['code'], -11) + def test_passphrase(self): + # There's no passphrase + result = self.do_command(self.dev_args + ['enumerate']) + for dev in result: + if dev['type'] == 'keepkey' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_passphrase_sent']) + self.assertEquals(dev['fingerprint'], '95d8f670') + # Setting a passphrase won't change the fingerprint + result = self.do_command(self.dev_args + ['-p', 'pass', 'enumerate']) + for dev in result: + if dev['type'] == 'keepkey' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_passphrase_sent']) + self.assertEquals(dev['fingerprint'], '95d8f670') + + # Set a passphrase + device.wipe(self.client) + self.client.set_passphrase('pass') + load_device_by_mnemonic(client=self.client, mnemonic='alcohol woman abuse must during monitor noble actual mixed trade anger aisle', pin='', passphrase_protection=True, label='test') + self.client.call(messages.ClearSession()) + + # A passphrase will need to be sent + result = self.do_command(self.dev_args + ['enumerate']) + for dev in result: + if dev['type'] == 'keepkey' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertTrue(dev['needs_passphrase_sent']) + result = self.do_command(self.dev_args + ['-p', 'pass', 'enumerate']) + for dev in result: + if dev['type'] == 'keepkey' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_passphrase_sent']) + fpr = dev['fingerprint'] + # A different passphrase would not change the fingerprint + result = self.do_command(self.dev_args + ['-p', 'pass2', 'enumerate']) + for dev in result: + if dev['type'] == 'keepkey' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_passphrase_sent']) + self.assertEqual(dev['fingerprint'], fpr) + + # Clearing the session and starting a new one with a new passphrase should change the passphrase + self.client.call(messages.ClearSession()) + result = self.do_command(self.dev_args + ['-p', 'pass3', 'enumerate']) + for dev in result: + if dev['type'] == 'keepkey' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_passphrase_sent']) + self.assertNotEqual(dev['fingerprint'], fpr) + def keepkey_test_suite(emulator, rpc, userpass, interface): # Redirect stderr to /dev/null as it's super spammy sys.stderr = open(os.devnull, 'w') diff --git a/test/test_trezor.py b/test/test_trezor.py index d3f42f8..9e3f1fb 100755 --- a/test/test_trezor.py +++ b/test/test_trezor.py @@ -180,11 +180,19 @@ class TestTrezorManCommands(TrezorTestCase): result = self.do_command(self.dev_args + ['sendpin', '1234']) self.assertEqual(result['error'], 'This device does not need a PIN') self.assertEqual(result['code'], -11) + result = self.do_command(self.dev_args + ['enumerate']) + for dev in result: + if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_pin_sent']) # Set a PIN device.wipe(self.client) load_device_by_mnemonic(client=self.client, mnemonic='alcohol woman abuse must during monitor noble actual mixed trade anger aisle', pin='1234', passphrase_protection=False, label='test') self.client.call(messages.ClearSession()) + result = self.do_command(self.dev_args + ['enumerate']) + for dev in result: + if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertTrue(dev['needs_pin_sent']) result = self.do_command(self.dev_args + ['promptpin']) self.assertTrue(result['success']) @@ -212,6 +220,11 @@ class TestTrezorManCommands(TrezorTestCase): result = self.do_command(self.dev_args + ['sendpin', pin]) self.assertTrue(result['success']) + result = self.do_command(self.dev_args + ['enumerate']) + for dev in result: + if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_pin_sent']) + # Sending PIN after unlock result = self.do_command(self.dev_args + ['promptpin']) self.assertEqual(result['error'], 'The PIN has already been sent to this device') @@ -220,6 +233,50 @@ class TestTrezorManCommands(TrezorTestCase): self.assertEqual(result['error'], 'The PIN has already been sent to this device') self.assertEqual(result['code'], -11) + def test_passphrase(self): + # There's no passphrase + result = self.do_command(self.dev_args + ['enumerate']) + for dev in result: + if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_passphrase_sent']) + self.assertEquals(dev['fingerprint'], '95d8f670') + # Setting a passphrase won't change the fingerprint + result = self.do_command(self.dev_args + ['-p', 'pass', 'enumerate']) + for dev in result: + if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_passphrase_sent']) + self.assertEquals(dev['fingerprint'], '95d8f670') + + # Set a passphrase + device.wipe(self.client) + load_device_by_mnemonic(client=self.client, mnemonic='alcohol woman abuse must during monitor noble actual mixed trade anger aisle', pin='', passphrase_protection=True, label='test') + self.client.call(messages.ClearSession()) + + # A passphrase will need to be sent + result = self.do_command(self.dev_args + ['enumerate']) + for dev in result: + if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertTrue(dev['needs_passphrase_sent']) + result = self.do_command(self.dev_args + ['-p', 'pass', 'enumerate']) + for dev in result: + if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_passphrase_sent']) + fpr = dev['fingerprint'] + # A different passphrase would not change the fingerprint + result = self.do_command(self.dev_args + ['-p', 'pass2', 'enumerate']) + for dev in result: + if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_passphrase_sent']) + self.assertEqual(dev['fingerprint'], fpr) + + # Clearing the session and starting a new one with a new passphrase should change the passphrase + self.client.call(messages.Initialize()) + result = self.do_command(self.dev_args + ['-p', 'pass3', 'enumerate']) + for dev in result: + if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324': + self.assertFalse(dev['needs_passphrase_sent']) + self.assertNotEqual(dev['fingerprint'], fpr) + def trezor_test_suite(emulator, rpc, userpass, interface): # Redirect stderr to /dev/null as it's super spammy sys.stderr = open(os.devnull, 'w') From 54e14ba301ad57769261b94e9d4fc22d24de94ab Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 3 May 2019 18:56:15 -0400 Subject: [PATCH 4/5] Change the order devices are tested in Test the digitalbitbox first because it will self wipe if the wrong password is given too many times (by other tests) which causes test to fail. Then, group up tests by simulator, those with simulators started at the beginning (dbb and coldcard) go first. Then those with simulators started for each test (trezor and keepkey). Lastly the test that requires a physical device. --- test/run_tests.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/run_tests.py b/test/run_tests.py index 6817a61..635a9ba 100755 --- a/test/run_tests.py +++ b/test/run_tests.py @@ -49,15 +49,15 @@ if not args.no_trezor or not args.no_coldcard or args.ledger or not args.no_bitb # Start bitcoind rpc, userpass = start_bitcoind(args.bitcoind) -if not args.no_trezor: - suite.addTest(trezor_test_suite(args.trezor, rpc, userpass, args.interface)) -if not args.no_coldcard: - suite.addTest(coldcard_test_suite(args.coldcard, rpc, userpass, args.interface)) -if args.ledger: - suite.addTest(ledger_test_suite(rpc, userpass, args.interface)) if not args.no_bitbox: suite.addTest(digitalbitbox_test_suite(rpc, userpass, args.bitbox, args.interface)) +if not args.no_coldcard: + suite.addTest(coldcard_test_suite(args.coldcard, rpc, userpass, args.interface)) +if not args.no_trezor: + suite.addTest(trezor_test_suite(args.trezor, rpc, userpass, args.interface)) if not args.no_keepkey: suite.addTest(keepkey_test_suite(args.keepkey, rpc, userpass, args.interface)) +if args.ledger: + suite.addTest(ledger_test_suite(rpc, userpass, args.interface)) result = unittest.TextTestRunner(stream=sys.stdout, verbosity=2).run(suite) sys.exit(not result.wasSuccessful()) From 4d21f2912723036be581c60eac3a24f15c27e761 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 3 May 2019 19:02:05 -0400 Subject: [PATCH 5/5] ignore other devices when checking that the coldcard has started --- test/test_coldcard.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/test_coldcard.py b/test/test_coldcard.py index 792e37d..6998bc6 100755 --- a/test/test_coldcard.py +++ b/test/test_coldcard.py @@ -18,7 +18,12 @@ def coldcard_test_suite(simulator, rpc, userpass, interface): # Wait for simulator to be up while True: enum_res = process_commands(['enumerate']) - if len(enum_res) > 0 and 'error' not in enum_res[0]: + found = False + for dev in enum_res: + if dev['type'] == 'coldcard' and 'error' not in dev: + found = True + break + if found: break time.sleep(0.5) # Cleanup