Merge #157: Indicate in enumerate whether a device needs a passphrase or a pin

4d21f29 ignore other devices when checking that the coldcard has started (Andrew Chow)
54e14ba Change the order devices are tested in (Andrew Chow)
1c33312 Test need_passphrase_sent and need_pin_sent (Andrew Chow)
bca8535 Add needs_pin_sent to indicate whether promptpin needs to be used (Andrew Chow)
d2696cf Add needs_passphrase_sent to enumerate output (Andrew Chow)

Pull request description:

  This adds two fields to `numerate`: `needs_passphrase_sent` and `needs_pin_sent` which indicate whether a command with a `-p` option is needed or `promptpin` and `sendpin` need to be done. These fields will be true in the instances that a passphrase needs to be sent and have not been cached by the device. To avoid a chicken and egg problem with fingerprints on Trezors, if the device needs a passphrase but one was not specified in `enumerate`, the fingerprint won't be retrieved in order to not contaminate the passphrase cache on device.

  Built on #152 because it has some interactions with it that needed to be addressed.

ACKs for commit 4d21f2:

Tree-SHA512: 4164a8e541a9441b615205b84561966dd2f199f5f5e738cc55f12c60866483e458f3a5c05ac22c69baf862c37f6beeb3ca78f01c7b63770b9440f47a56811173
This commit is contained in:
Andrew Chow 2019-06-16 10:44:03 -04:00
commit d8cf9f120b
No known key found for this signature in database
GPG Key ID: 17565732E08E5E41
9 changed files with 149 additions and 10 deletions

View File

@ -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,8 @@ 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:
if str(e) == 'Cannot connect to simulator. Is it running?':

View File

@ -603,6 +603,8 @@ 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()
d_data['code'] = e.get_code()

View File

@ -26,9 +26,16 @@ 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:
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:

View File

@ -350,9 +350,8 @@ 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_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)
d_data['code'] = UNKNOWN_ERROR

View File

@ -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))
@ -421,9 +422,16 @@ 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:
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:

View File

@ -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())

View File

@ -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

View File

@ -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')

View File

@ -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')