Merge #644: Use Optional for passphrase

9ed9dbe trezor: Default to empty string passphrase (Andrew Chow)
0c4df0c trezor, test: Test enumerate with empty passphrase (Andrew Chow)
d74fcbc test: Handle Optional emulator password (Andrew Chow)
d4cf550 Change passphrase (password) to Optional (Andrew Chow)

Pull request description:

  We should be distinguishing from the empty passphrase and no passphrase provided, so instead of defaulting to the empty passphrase, change passphrase (aka password) to an `Optional`. This way, if it is not provided, it is `None` and empty passphrases can be handled correctly.

  Fixes #639

Top commit has no ACKs.

Tree-SHA512: aac6763934dd40fd55c7ad0ed9bb57e387ce9eefeb4c198521fb09faaeed538966bc50f598c024dfffdc8e6a5a646f87ff64cb77103c296ec2c30ba992763274

hwilib/_cli.py

@@ -136,7 +136,7 @@ def get_parser() -> HWIArgumentParser:
     parser = HWIArgumentParser(description='Hardware Wallet Interface, version {}.\nAccess and send commands to a hardware wallet device. Responses are in JSON format.'.format(__version__))
     parser.add_argument('--device-path', '-d', help='Specify the device path of the device to connect to')
     parser.add_argument('--device-type', '-t', help='Specify the type of device that will be connected. If `--device-path` not given, the first device of this type enumerated is used.')
-    parser.add_argument('--password', '-p', help='Device password if it has one (e.g. DigitalBitbox)', default='')
+    parser.add_argument('--password', '-p', help='Device password if it has one (e.g. DigitalBitbox)')
     parser.add_argument('--stdinpass', help='Enter the device password on the command line', action='store_true')
     parser.add_argument('--chain', help='Select chain to work with', type=Chain.argparse, choices=list(Chain), default=Chain.MAIN) # type: ignore
     parser.add_argument('--debug', help='Print debug statements', action='store_true')

hwilib/commands.py

@@ -71,7 +71,7 @@
 
 
 # Get the client for the device
-def get_client(device_type: str, device_path: str, password: str = "", expert: bool = False, chain: Chain = Chain.MAIN) -> Optional[HardwareWalletClient]:
+def get_client(device_type: str, device_path: str, password: Optional[str] = None, expert: bool = False, chain: Chain = Chain.MAIN) -> Optional[HardwareWalletClient]:
     """
     Returns a HardwareWalletClient for the given device type at the device path
 
@@ -101,7 +101,7 @@ def get_client(device_type: str, device_path: str, password: str = "", expert: b
     return client
 
 # Get a list of all available hardware wallets
-def enumerate(password: str = "") -> List[Dict[str, Any]]:
+def enumerate(password: Optional[str] = None) -> List[Dict[str, Any]]:
     """
     Enumerate all of the devices that HWI can potentially access.
 
@@ -124,7 +124,7 @@ def enumerate(password: str = "") -> List[Dict[str, Any]]:
 
 # Fingerprint or device type required
 def find_device(
-    password: str = "",
+    password: Optional[str] = None,
     device_type: Optional[str] = None,
     fingerprint: Optional[str] = None,
     expert: bool = False,

hwilib/devices/bitbox02.py

@@ -174,7 +174,7 @@ def _xpubs_equal_ignoring_version(xpub1: bytes, xpub2: bytes) -> bool:
     return xpub1[4:] == xpub2[4:]
 
 
-def enumerate(password: str = "") -> List[Dict[str, object]]:
+def enumerate(password: Optional[str] = None) -> List[Dict[str, object]]:
     """
     Enumerate all BitBox02 devices. Bootloaders excluded.
     """
@@ -259,15 +259,15 @@ def func(*args, **kwargs):  # type: ignore
 
 # This class extends the HardwareWalletClient for BitBox02 specific things
 class Bitbox02Client(HardwareWalletClient):
-    def __init__(self, path: str, password: str = "", expert: bool = False, chain: Chain = Chain.MAIN) -> None:
+    def __init__(self, path: str, password: Optional[str] = None, expert: bool = False, chain: Chain = Chain.MAIN) -> None:
         """
         Initializes a new BitBox02 client instance.
         """
-        super().__init__(path, password=password, expert=expert, chain=chain)
-        if password:
+        if password is not None:
             raise BadArgumentError(
                 "The BitBox02 does not accept a passphrase from the host. Please enable the passphrase option and enter the passphrase on the device during unlock."
             )
+        super().__init__(path, password=password, expert=expert, chain=chain)
 
         hid_device = hid.device()
         hid_device.open_path(path.encode())

hwilib/devices/coldcard.py

@@ -67,6 +67,7 @@
 from typing import (
     Any,
     Callable,
+    Optional,
 )
 
 CC_SIMULATOR_SOCK = '/tmp/ckcc-simulator.sock'
@@ -89,7 +90,7 @@ def func(*args: Any, **kwargs: Any) -> Any:
 # This class extends the HardwareWalletClient for ColdCard specific things
 class ColdcardClient(HardwareWalletClient):
 
-    def __init__(self, path: str, password: str = "", expert: bool = False, chain: Chain = Chain.MAIN) -> None:
+    def __init__(self, path: str, password: Optional[str] = None, expert: bool = False, chain: Chain = Chain.MAIN) -> None:
         super(ColdcardClient, self).__init__(path, password, expert, chain)
         # Simulator hard coded pipe socket
         if path == CC_SIMULATOR_SOCK:
@@ -398,7 +399,7 @@ def can_sign_taproot(self) -> bool:
         return False
 
 
-def enumerate(password: str = "") -> List[Dict[str, Any]]:
+def enumerate(password: Optional[str] = None) -> List[Dict[str, Any]]:
     results = []
     devices = hid.enumerate(COINKITE_VID, CKCC_PID)
     devices.append({'path': CC_SIMULATOR_SOCK.encode()})

hwilib/devices/digitalbitbox.py

@@ -22,6 +22,7 @@
     Callable,
     Dict,
     List,
+    Optional,
     Tuple,
     Union,
 )
@@ -345,17 +346,17 @@ def format_backup_filename(name: str) -> str:
 # This class extends the HardwareWalletClient for Digital Bitbox specific things
 class DigitalbitboxClient(HardwareWalletClient):
 
-    def __init__(self, path: str, password: str, expert: bool = False, chain: Chain = Chain.MAIN) -> None:
+    def __init__(self, path: str, password: Optional[str], expert: bool = False, chain: Chain = Chain.MAIN) -> None:
         """
         The `DigitalbitboxClient` is a `HardwareWalletClient` for interacting with BitBox01 devices (previously known as the Digital BitBox).
 
         :param path: Path to the device as given by `enumerate`
         :param password: The password required to communicate with the device. Must be provided.
         :param expert: Whether to be in expert mode and return additional information.
         """
-        super(DigitalbitboxClient, self).__init__(path, password, expert, chain)
-        if not password:
+        if password is None:
             raise NoPasswordError('Password must be supplied for digital BitBox')
+        super(DigitalbitboxClient, self).__init__(path, password, expert, chain)
         if path.startswith('udp:'):
             split_path = path.split(':')
             ip = split_path[1]
@@ -364,7 +365,7 @@ def __init__(self, path: str, password: str, expert: bool = False, chain: Chain
         else:
             self.device = hid.device()
             self.device.open_path(path.encode())
-        self.password = password
+        self.password: str = password
 
     @digitalbitbox_exception
     def get_pubkey_at_path(self, path: str) -> ExtendedKey:
@@ -678,7 +679,7 @@ def can_sign_taproot(self) -> bool:
         return False
 
 
-def enumerate(password: str = "") -> List[Dict[str, Any]]:
+def enumerate(password: Optional[str] = None) -> List[Dict[str, Any]]:
     results = []
     devices = hid.enumerate(DBB_VENDOR_ID, DBB_DEVICE_ID)
     # Try connecting to simulator
@@ -707,7 +708,7 @@ def enumerate(password: str = "") -> List[Dict[str, Any]]:
                 client = DigitalbitboxClient(path, password)
 
                 # Check initialized
-                reply = send_encrypt('{"device" : "info"}', password, client.device)
+                reply = send_encrypt('{"device" : "info"}', "" if password is None else password, client.device)
                 if 'error' in reply and (reply['error']['code'] == 101 or reply['error']['code'] == '101'):
                     d_data['error'] = 'Not initialized'
                     d_data['code'] = DEVICE_NOT_INITIALIZED

hwilib/devices/jade.py

@@ -125,7 +125,7 @@ def _get_multisig_name(type: str, threshold: int, signers: List[Tuple[bytes, Seq
         hash_summary = sha256(summary.encode()).hex()
         return 'hwi' + hash_summary[:12]
 
-    def __init__(self, path: str, password: str = '', expert: bool = False, chain: Chain = Chain.MAIN, timeout: Optional[int] = None) -> None:
+    def __init__(self, path: str, password: Optional[str] = None, expert: bool = False, chain: Chain = Chain.MAIN, timeout: Optional[int] = None) -> None:
         super(JadeClient, self).__init__(path, password, expert, chain)
         self.jade = JadeAPI.create_serial(path, timeout=timeout)
         self.jade.connect()
@@ -508,7 +508,7 @@ def can_sign_taproot(self) -> bool:
         return False
 
 
-def enumerate(password: str = '') -> List[Dict[str, Any]]:
+def enumerate(password: Optional[str] = None) -> List[Dict[str, Any]]:
     results = []
 
     def _get_device_entry(device_model: str, device_path: str) -> Dict[str, Any]:

hwilib/devices/keepkey.py

@@ -141,7 +141,7 @@ def __init__(
 
 
 class KeepkeyClient(TrezorClient):
-    def __init__(self, path: str, password: str = "", expert: bool = False, chain: Chain = Chain.MAIN) -> None:
+    def __init__(self, path: str, password: Optional[str] = None, expert: bool = False, chain: Chain = Chain.MAIN) -> None:
         """
         The `KeepkeyClient` is a `HardwareWalletClient` for interacting with the Keepkey.
 
@@ -171,7 +171,7 @@ def can_sign_taproot(self) -> bool:
         return False
 
 
-def enumerate(password: str = "") -> List[Dict[str, Any]]:
+def enumerate(password: Optional[str] = None) -> List[Dict[str, Any]]:
     results = []
     devs = hid.HidTransport.enumerate(usb_ids=KEEPKEY_HID_IDS)
     devs.extend(webusb.WebUsbTransport.enumerate(usb_ids=KEEPKEY_WEBUSB_IDS))
@@ -202,7 +202,7 @@ def enumerate(password: str = "") -> List[Dict[str, Any]]:
             d_data['needs_passphrase_sent'] = client.client.features.passphrase_protection # always need the passphrase sent for Keepkey if it has passphrase protection enabled
             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:
+            if d_data['needs_passphrase_sent'] and password is None:
                 raise DeviceNotReadyError("Passphrase needs to be specified before the fingerprint information can be retrieved")
             if client.client.features.initialized:
                 d_data['fingerprint'] = client.get_master_fingerprint().hex()

hwilib/devices/ledger.py

@@ -141,7 +141,7 @@ def func(*args: Any, **kwargs: Any) -> Any:
 # This class extends the HardwareWalletClient for Ledger Nano S and Nano X specific things
 class LedgerClient(HardwareWalletClient):
 
-    def __init__(self, path: str, password: str = "", expert: bool = False, chain: Chain = Chain.MAIN) -> None:
+    def __init__(self, path: str, password: Optional[str] = None, expert: bool = False, chain: Chain = Chain.MAIN) -> None:
         super(LedgerClient, self).__init__(path, password, expert, chain)
 
         is_debug = logging.getLogger().getEffectiveLevel() == logging.DEBUG
@@ -539,7 +539,7 @@ def can_sign_taproot(self) -> bool:
         return isinstance(self.client, NewClient)
 
 
-def enumerate(password: str = '') -> List[Dict[str, Any]]:
+def enumerate(password: Optional[str] = None) -> List[Dict[str, Any]]:
     results = []
     devices = []
     devices.extend(hid.enumerate(LEDGER_VENDOR_ID, 0))

hwilib/devices/trezor.py

@@ -281,14 +281,16 @@ class TrezorClient(HardwareWalletClient):
     def __init__(
         self,
         path: str,
-        password: str = "",
+        password: Optional[str] = None,
         expert: bool = False,
         chain: Chain = Chain.MAIN,
         hid_ids: Set[Tuple[int, int]] = HID_IDS,
         webusb_ids: Set[Tuple[int, int]] = WEBUSB_IDS,
         sim_path: str = SIMULATOR_PATH,
         model: Optional[TrezorModel] = None
     ) -> None:
+        if password is None:
+            password = ""
         super(TrezorClient, self).__init__(path, password, expert, chain)
         self.simulator = False
         transport = get_path_transport(path, hid_ids, webusb_ids, sim_path)
@@ -326,6 +328,8 @@ def _check_unlocked(self) -> None:
             self.client.ui.disallow_passphrase()
         if self.client.features.pin_protection and not self.client.features.unlocked:
             raise DeviceNotReadyError('{} is locked. Unlock by using \'promptpin\' and then \'sendpin\'.'.format(self.type))
+        if self.client.features.passphrase_protection and self.password is None:
+            raise NoPasswordError("Passphrase protection is enabled, passphrase must be provided")
 
     def _supports_external(self) -> bool:
         if self.client.features.model == "1" and self.client.version <= (1, 10, 5):
@@ -843,7 +847,7 @@ def can_sign_taproot(self) -> bool:
         return bool(self.client.version >= (1, 10, 4))
 
 
-def enumerate(password: str = "") -> List[Dict[str, Any]]:
+def enumerate(password: Optional[str] = None) -> List[Dict[str, Any]]:
     results = []
     devs = hid.HidTransport.enumerate()
     devs.extend(webusb.WebUsbTransport.enumerate())
@@ -876,8 +880,8 @@ def enumerate(password: str = "") -> List[Dict[str, Any]]:
                 d_data['needs_passphrase_sent'] = False
             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 d_data['needs_passphrase_sent'] and password is None:
+                d_data["warnings"] = [["Passphrase protection enabled but passphrase was not provided. Using default passphrase of the empty string (\"\")"]]
             if client.client.features.initialized:
                 d_data['fingerprint'] = client.get_master_fingerprint().hex()
                 d_data['needs_passphrase_sent'] = False # Passphrase is always needed for the above to have worked, so it's already sent

hwilib/hwwclient.py

@@ -27,7 +27,7 @@ class HardwareWalletClient(object):
     that hardware wallet subclasses should implement.
     """
 
-    def __init__(self, path: str, password: str, expert: bool, chain: Chain = Chain.MAIN) -> None:
+    def __init__(self, path: str, password: Optional[str], expert: bool, chain: Chain = Chain.MAIN) -> None:
         """
         :param path: Path to the device as returned by :func:`~hwilib.commands.enumerate`
         :param password: A password/passphrase to use with the device.

test/test_device.py

@@ -137,7 +137,7 @@ def __init__(self, bitcoind, emulator=None, interface='library', methodName='run
         self.emulator = emulator
 
         self.dev_args = ['-t', self.emulator.type, '-d', self.emulator.path, '--chain', 'test']
-        if self.emulator.password:
+        if self.emulator.password is not None:
             self.dev_args.extend(['-p', self.emulator.password])
 
         self.interface = interface
@@ -173,7 +173,7 @@ def do_command(self, args):
             return process_commands(args)
 
     def get_password_args(self):
-        if self.emulator.password:
+        if self.emulator.password is not None:
             return ['-p', self.emulator.password]
         return []
 

test/test_trezor.py

@@ -345,6 +345,14 @@ def test_passphrase(self):
         else:
             self.fail("Did not enumerate device")
         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']
+                break
+        else:
+            self.fail("Did not enumerate device")
+        result = self.do_command(self.dev_args + ['-p', '', 'enumerate'])
         for dev in result:
             if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:21324':
                 self.assertFalse(dev['needs_passphrase_sent'])