From 70e31cad0c48497761d98842dad894266360ecd0 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 21 May 2019 19:07:36 -0400 Subject: [PATCH 1/4] tests: Properly escape arguments for cli interface --- test/test_device.py | 8 ++++++-- test/test_keepkey.py | 8 ++++++-- test/test_trezor.py | 8 ++++++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/test/test_device.py b/test/test_device.py index 40fcdb2..d0d0863 100644 --- a/test/test_device.py +++ b/test/test_device.py @@ -3,6 +3,7 @@ import atexit import json import os +import shlex import shutil import subprocess import tempfile @@ -79,12 +80,15 @@ class DeviceTestCase(unittest.TestCase): return suite def do_command(self, args): + cli_args = [] + for arg in args: + cli_args.append(shlex.quote(arg)) if self.interface == 'cli': - proc = subprocess.Popen(['hwi ' + ' '.join(args)], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, shell=True) + proc = subprocess.Popen(['hwi ' + ' '.join(cli_args)], stdout=subprocess.PIPE, shell=True) result = proc.communicate() return json.loads(result[0].decode()) elif self.interface == 'bindist': - proc = subprocess.Popen(['../dist/hwi ' + ' '.join(args)], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, shell=True) + proc = subprocess.Popen(['../dist/hwi ' + ' '.join(cli_args)], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, shell=True) result = proc.communicate() return json.loads(result[0].decode()) elif self.interface == 'stdin': diff --git a/test/test_keepkey.py b/test/test_keepkey.py index d20d383..91eef05 100755 --- a/test/test_keepkey.py +++ b/test/test_keepkey.py @@ -4,6 +4,7 @@ import argparse import atexit import json import os +import shlex import socket import subprocess import sys @@ -82,12 +83,15 @@ class KeepkeyTestCase(unittest.TestCase): return suite def do_command(self, args): + cli_args = [] + for arg in args: + cli_args.append(shlex.quote(arg)) if self.interface == 'cli': - proc = subprocess.Popen(['hwi ' + ' '.join(args)], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, shell=True) + proc = subprocess.Popen(['hwi ' + ' '.join(cli_args)], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, shell=True) result = proc.communicate() return json.loads(result[0].decode()) elif self.interface == 'bindist': - proc = subprocess.Popen(['../dist/hwi ' + ' '.join(args)], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, shell=True) + proc = subprocess.Popen(['../dist/hwi ' + ' '.join(cli_args)], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, shell=True) result = proc.communicate() return json.loads(result[0].decode()) elif self.interface == 'stdin': diff --git a/test/test_trezor.py b/test/test_trezor.py index 366ee17..d3f42f8 100755 --- a/test/test_trezor.py +++ b/test/test_trezor.py @@ -4,6 +4,7 @@ import argparse import atexit import json import os +import shlex import socket import subprocess import sys @@ -82,12 +83,15 @@ class TrezorTestCase(unittest.TestCase): return suite def do_command(self, args): + cli_args = [] + for arg in args: + cli_args.append(shlex.quote(arg)) if self.interface == 'cli': - proc = subprocess.Popen(['hwi ' + ' '.join(args)], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, shell=True) + proc = subprocess.Popen(['hwi ' + ' '.join(cli_args)], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, shell=True) result = proc.communicate() return json.loads(result[0].decode()) elif self.interface == 'bindist': - proc = subprocess.Popen(['../dist/hwi ' + ' '.join(args)], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, shell=True) + proc = subprocess.Popen(['../dist/hwi ' + ' '.join(cli_args)], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, shell=True) result = proc.communicate() return json.loads(result[0].decode()) elif self.interface == 'stdin': From cea784ef606ff732447e82dec6436621a7177be6 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 21 May 2019 19:09:18 -0400 Subject: [PATCH 2/4] tests: Replace remaining process_commands with self.do_command in tests --- test/test_device.py | 20 ++++++++++---------- test/test_digitalbitbox.py | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/test/test_device.py b/test/test_device.py index d0d0863..712168c 100644 --- a/test/test_device.py +++ b/test/test_device.py @@ -469,36 +469,36 @@ class TestDisplayAddress(DeviceTestCase): self.assertEquals(result['code'], -7) def test_display_address_descriptor(self): - account_xpub = process_commands(self.dev_args + ['getxpub', 'm/84h/1h/0h'])['xpub'] - p2sh_segwit_account_xpub = process_commands(self.dev_args + ['getxpub', 'm/49h/1h/0h'])['xpub'] - legacy_account_xpub = process_commands(self.dev_args + ['getxpub', 'm/44h/1h/0h'])['xpub'] + account_xpub = self.do_command(self.dev_args + ['getxpub', 'm/84h/1h/0h'])['xpub'] + p2sh_segwit_account_xpub = self.do_command(self.dev_args + ['getxpub', 'm/49h/1h/0h'])['xpub'] + legacy_account_xpub = self.do_command(self.dev_args + ['getxpub', 'm/44h/1h/0h'])['xpub'] # Native SegWit address using xpub: - process_commands(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h)]' + account_xpub + '/0/0)']) + self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h)]' + account_xpub + '/0/0)']) # Native SegWit address using hex encoded pubkey: - process_commands(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h)]' + xpub_to_pub_hex(account_xpub) + '/0/0)']) + self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h)]' + xpub_to_pub_hex(account_xpub) + '/0/0)']) # P2SH wrapped SegWit address using xpub: - process_commands(self.dev_args + ['displayaddress', '--desc', 'sh(wpkh([' + self.fingerprint + '/49h/1h/0h)]' + p2sh_segwit_account_xpub + '/0/0))']) + self.do_command(self.dev_args + ['displayaddress', '--desc', 'sh(wpkh([' + self.fingerprint + '/49h/1h/0h)]' + p2sh_segwit_account_xpub + '/0/0))']) # Legacy address - process_commands(self.dev_args + ['displayaddress', '--desc', 'pkh([' + self.fingerprint + '/44h/1h/0h)]' + legacy_account_xpub + '/0/0)']) + self.do_command(self.dev_args + ['displayaddress', '--desc', 'pkh([' + self.fingerprint + '/44h/1h/0h)]' + legacy_account_xpub + '/0/0)']) # Should check xpub - result = process_commands(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h)]' + "not_and_xpub" + '/0/0)']) + result = self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h)]' + "not_and_xpub" + '/0/0)']) self.assertIn('error', result) self.assertIn('code', result) self.assertEqual(result['code'], -7) # Should check hex pub - result = process_commands(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h)]' + "not_and_xpub" + '/0/0)']) + result = self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h)]' + "not_and_xpub" + '/0/0)']) self.assertIn('error', result) self.assertIn('code', result) self.assertEqual(result['code'], -7) # Should check fingerprint - process_commands(self.dev_args + ['displayaddress', '--desc', 'wpkh([00000000/84h/1h/0h)]' + account_xpub + '/0/0)']) + self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([00000000/84h/1h/0h)]' + account_xpub + '/0/0)']) self.assertIn('error', result) self.assertIn('code', result) self.assertEqual(result['code'], -7) diff --git a/test/test_digitalbitbox.py b/test/test_digitalbitbox.py index 0a7d631..419aa02 100755 --- a/test/test_digitalbitbox.py +++ b/test/test_digitalbitbox.py @@ -93,7 +93,7 @@ def digitalbitbox_test_suite(rpc, userpass, simulator, interface): self.assertTrue(result['success']) # Reset back to original - result = process_commands(self.dev_args + ['wipe']) + result = self.do_command(self.dev_args + ['wipe']) self.assertTrue(result['success']) send_plain(b'{"password":"0000"}', dev) send_encrypt(json.dumps({"seed":{"source":"backup","filename":"test_backup.pdf","key":"key"}}), '0000', dev) From 0974f6944caf975ddba9ac5b0a5429f79b9976ad Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 21 May 2019 19:09:33 -0400 Subject: [PATCH 3/4] Ensure the client's fingerprint is available for descriptor displayaddress --- hwilib/commands.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hwilib/commands.py b/hwilib/commands.py index a420467..e7be608 100644 --- a/hwilib/commands.py +++ b/hwilib/commands.py @@ -155,6 +155,10 @@ def displayaddress(client, path=None, desc=None, sh_wpkh=False, wpkh=False): return {'error':'Both `--wpkh` and `--sh_wpkh` can not be selected at the same time.','code':BAD_ARGUMENT} return client.display_address(path, sh_wpkh, wpkh) elif desc is not None: + if client.fingerprint is None: + master_xpub = client.get_pubkey_at_path('m/0h')['xpub'] + client.fingerprint = get_xpub_fingerprint_hex(master_xpub) + if sh_wpkh == True or wpkh == True: return {'error':' `--wpkh` and `--sh_wpkh` can not be combined with --desc','code':BAD_ARGUMENT} descriptor = Descriptor.parse(desc, client.is_testnet) From 54bd6687cc55408f5c54df708e72c6470edc6057 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 21 May 2019 19:09:37 -0400 Subject: [PATCH 4/4] Actually test that displayaddress is returning correct output --- test/test_device.py | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/test/test_device.py b/test/test_device.py index 712168c..d432b38 100644 --- a/test/test_device.py +++ b/test/test_device.py @@ -460,9 +460,20 @@ class TestDisplayAddress(DeviceTestCase): self.assertEqual(result['code'], -7) def test_display_address_path(self): - self.do_command(self.dev_args + ['displayaddress', '--path', 'm/44h/1h/0h/0/0']) - self.do_command(self.dev_args + ['displayaddress', '--sh_wpkh', '--path', 'm/49h/1h/0h/0/0']) - self.do_command(self.dev_args + ['displayaddress', '--wpkh', '--path', 'm/84h/1h/0h/0/0']) + result = self.do_command(self.dev_args + ['displayaddress', '--path', 'm/44h/1h/0h/0/0']) + self.assertNotIn('error', result) + self.assertNotIn('code', result) + self.assertIn('address', result) + + result = self.do_command(self.dev_args + ['displayaddress', '--sh_wpkh', '--path', 'm/49h/1h/0h/0/0']) + self.assertNotIn('error', result) + self.assertNotIn('code', result) + self.assertIn('address', result) + + result = self.do_command(self.dev_args + ['displayaddress', '--wpkh', '--path', 'm/84h/1h/0h/0/0']) + self.assertNotIn('error', result) + self.assertNotIn('code', result) + self.assertIn('address', result) def test_display_address_bad_path(self): result = self.do_command(self.dev_args + ['displayaddress', '--path', 'f']) @@ -474,31 +485,43 @@ class TestDisplayAddress(DeviceTestCase): legacy_account_xpub = self.do_command(self.dev_args + ['getxpub', 'm/44h/1h/0h'])['xpub'] # Native SegWit address using xpub: - self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h)]' + account_xpub + '/0/0)']) + result = self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h]' + account_xpub + '/0/0)']) + self.assertNotIn('error', result) + self.assertNotIn('code', result) + self.assertIn('address', result) # Native SegWit address using hex encoded pubkey: - self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h)]' + xpub_to_pub_hex(account_xpub) + '/0/0)']) + result = self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h]' + xpub_to_pub_hex(account_xpub) + '/0/0)']) + self.assertNotIn('error', result) + self.assertNotIn('code', result) + self.assertIn('address', result) # P2SH wrapped SegWit address using xpub: - self.do_command(self.dev_args + ['displayaddress', '--desc', 'sh(wpkh([' + self.fingerprint + '/49h/1h/0h)]' + p2sh_segwit_account_xpub + '/0/0))']) + result = self.do_command(self.dev_args + ['displayaddress', '--desc', 'sh(wpkh([' + self.fingerprint + '/49h/1h/0h]' + p2sh_segwit_account_xpub + '/0/0))']) + self.assertNotIn('error', result) + self.assertNotIn('code', result) + self.assertIn('address', result) # Legacy address - self.do_command(self.dev_args + ['displayaddress', '--desc', 'pkh([' + self.fingerprint + '/44h/1h/0h)]' + legacy_account_xpub + '/0/0)']) + result = self.do_command(self.dev_args + ['displayaddress', '--desc', 'pkh([' + self.fingerprint + '/44h/1h/0h]' + legacy_account_xpub + '/0/0)']) + self.assertNotIn('error', result) + self.assertNotIn('code', result) + self.assertIn('address', result) # Should check xpub - result = self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h)]' + "not_and_xpub" + '/0/0)']) + result = self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h]' + "not_and_xpub" + '/0/0)']) self.assertIn('error', result) self.assertIn('code', result) self.assertEqual(result['code'], -7) # Should check hex pub - result = self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h)]' + "not_and_xpub" + '/0/0)']) + result = self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([' + self.fingerprint + '/84h/1h/0h]' + "not_and_xpub" + '/0/0)']) self.assertIn('error', result) self.assertIn('code', result) self.assertEqual(result['code'], -7) # Should check fingerprint - self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([00000000/84h/1h/0h)]' + account_xpub + '/0/0)']) + self.do_command(self.dev_args + ['displayaddress', '--desc', 'wpkh([00000000/84h/1h/0h]' + account_xpub + '/0/0)']) self.assertIn('error', result) self.assertIn('code', result) self.assertEqual(result['code'], -7)