search

romanz / trezor-agent

Hardware-based SSH/GPG/age agent
GNU Lesser General Public License v3.0
564 stars 152 forks source link

trezor-agent not working under macOS #98

Closed VitGottwald closed 7 years ago

VitGottwald commented 7 years ago

After installation of trezor_agent on macOS Sierra (12.10.3) , I get this:

python --version
Python 2.7.13
trezor-agent root@example.com
Traceback (most recent call last):
  File "/usr/local/bin/trezor-agent", line 11, in <module>
    sys.exit(run_agent())
  File "/usr/local/lib/python2.7/site-packages/trezor_agent/__main__.py", line 130, in wrapper
    return func(*args, **kwargs)
  File "/usr/local/lib/python2.7/site-packages/trezor_agent/__main__.py", line 161, in run_agent
    public_keys = [conn.get_public_key(i) for i in identities]
  File "/usr/local/lib/python2.7/site-packages/trezor_agent/client.py", line 23, in get_public_key
    with self.device:
  File "/usr/local/lib/python2.7/site-packages/trezor_agent/device/interface.py", line 110, in __enter__
    self.conn = self.connect()
  File "/usr/local/lib/python2.7/site-packages/trezor_agent/device/trezor.py", line 30, in connect
    connection = self._defs.Client(transport)
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 431, in __init__
    self.init_device()
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 438, in init_device
    self.features = expect(proto.Features)(self.call)(proto.Initialize())
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 127, in wrapped_f
    ret = f(*args, **kwargs)
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 140, in wrapped_f
    return f(*args, **kwargs)
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 176, in call
    resp = self.call_raw(msg)
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 140, in wrapped_f
    return f(*args, **kwargs)
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 172, in call_raw
    return self.transport.read_blocking()
  File "/usr/local/lib/python2.7/site-packages/trezorlib/transport.py", line 86, in read_blocking
    data = self._read()
  File "/usr/local/lib/python2.7/site-packages/trezorlib/transport.py", line 146, in _read
    (msg_type, datalen, data) = self.parse_first(chunk)
  File "/usr/local/lib/python2.7/site-packages/trezorlib/transport.py", line 158, in parse_first
    raise Exception("Unexpected magic characters")
Exception: Unexpected magic characters

the funny thing is that if I try to run the same trezor-agent command a couple of times, sometimes it actually outputs the expected thing

ecdsa-sha2-nistp256....

in this case the display blinks with some text that is immediately replaced back by logo

romanz commented 7 years ago

Thanks for reporting this issue! It looks very similar to https://github.com/romanz/trezor-agent/issues/53, which was fixed by https://github.com/trezor/python-trezor/pull/77.

Could you please update your trezor package to the latest version (currently 0.7.12)?

Could you also run trezorctl get_features command a few times, to see if this bug can be reproduced without trezor-agent?

VitGottwald commented 7 years ago

It looks like I already have the latest version

$ pip list
Cython (0.25.2)
ecdsa (0.13)
ECPy (0.8.1)
ed25519 (1.4)
future (0.16.0)
hidapi (0.7.99.post15)
keepkey (0.7.3)
ledgerblue (0.1.12)
mnemonic (0.17)
olefile (0.44)
pbkdf2 (1.3)
Pillow (4.0.0)
pip (9.0.1)
protobuf (3.2.0)
pycrypto (2.6.1)
semver (2.7.6)
setuptools (32.1.0)
six (1.10.0)
trezor (0.7.12)
trezor-agent (0.8.2)
wheel (0.29.0)

running trezorctl has the same problem

$ trezorctl get_features
Traceback (most recent call last):
  File "/usr/local/bin/trezorctl", line 598, in <module>
    main()
  File "/usr/local/bin/trezorctl", line 581, in main
    client = TrezorClient(transport)
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 431, in __init__
    self.init_device()
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 438, in init_device
    self.features = expect(proto.Features)(self.call)(proto.Initialize())
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 127, in wrapped_f
    ret = f(*args, **kwargs)
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 140, in wrapped_f
    return f(*args, **kwargs)
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 176, in call
    resp = self.call_raw(msg)
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 140, in wrapped_f
    return f(*args, **kwargs)
  File "/usr/local/lib/python2.7/site-packages/trezorlib/client.py", line 172, in call_raw
    return self.transport.read_blocking()
  File "/usr/local/lib/python2.7/site-packages/trezorlib/transport.py", line 86, in read_blocking
    data = self._read()
  File "/usr/local/lib/python2.7/site-packages/trezorlib/transport.py", line 146, in _read
    (msg_type, datalen, data) = self.parse_first(chunk)
  File "/usr/local/lib/python2.7/site-packages/trezorlib/transport.py", line 158, in parse_first
    raise Exception("Unexpected magic characters")
Exception: Unexpected magic characters
romanz commented 7 years ago

It seems that there is an issue with the USB link with the device itself... Could you please retry with another USB cable?

@prusnak - what do you think?

VitGottwald commented 7 years ago

I use trezor with this cable for TREZOR password manager without any problem. I just tried another cable anyway, but the result is still the same.

VitGottwald commented 7 years ago

The really weird part is that the command sometimes succeeds and sometimes not and it looks totally random.

prusnak commented 7 years ago

Duplicate of https://github.com/trezor/python-trezor/issues/76 and https://github.com/romanz/trezor-agent/issues/50

Wonder why it is still present.

VitGottwald commented 7 years ago

Any ideas how to turn on some diagnostics to help you find the root cause?

romanz commented 7 years ago

@baremetalfreak , can you please attach your /usr/local/lib/python2.7/site-packages/trezorlib/transport_hid.py file, to make sure that its enumerate() function returns sorted(devices.values())? (that was the issue solved at https://github.com/trezor/python-trezor/pull/77)

VitGottwald commented 7 years ago 
# This file is part of the TREZOR project.
#
# Copyright (C) 2012-2016 Marek Palatinus <slush@satoshilabs.com>
# Copyright (C) 2012-2016 Pavol Rusnak <stick@satoshilabs.com>
#
# This library is free software: you can redistribute it and/or modify
# it under the terms of the GNU Lesser General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This library is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU Lesser General Public License for more details.
#
# You should have received a copy of the GNU Lesser General Public License
# along with this library.  If not, see <http://www.gnu.org/licenses/>.

'''USB HID implementation of Transport.'''

import hid
import time
from .transport import TransportV1, TransportV2, ConnectionError

def enumerate():
    """
    Return a list of available TREZOR devices.
    """
    devices = {}
    for d in hid.enumerate(0, 0):
        vendor_id = d['vendor_id']
        product_id = d['product_id']
        serial_number = d['serial_number']
        interface_number = d['interface_number']
        usage_page = d['usage_page']
        path = d['path']

        if (vendor_id, product_id) in DEVICE_IDS:
            devices.setdefault(serial_number, [None, None])
            # first match by usage_page, then try interface number
            if usage_page == 0xFF00 or interface_number == 0:  # normal link
                devices[serial_number][0] = path
            elif usage_page == 0xFF01 or interface_number == 1:  # debug link
                devices[serial_number][1] = path

    # List of two-tuples (path_normal, path_debuglink)
    return sorted(devices.values())

def path_to_transport(path):
    try:
        device = [ d for d in hid.enumerate(0, 0) if d['path'] == path ][0]
    except IndexError:
        raise ConnectionError("Connection failed")

    # VID/PID found, let's find proper transport
    try:
        transport = DEVICE_TRANSPORTS[(device['vendor_id'], device['product_id'])]
    except IndexError:
        raise Exception("Unknown transport for VID:PID %04x:%04x" % (vid, pid))

    return transport

class _HidTransport(object):
    def __init__(self, device, *args, **kwargs):
        self.hid = None
        self.hid_version = None

        device = device[int(bool(kwargs.get('debug_link')))]
        super(_HidTransport, self).__init__(device, *args, **kwargs)

    def is_connected(self):
        """
        Check if the device is still connected.
        """
        for d in hid.enumerate(0, 0):
            if d['path'] == self.device:
                return True
        return False

    def _open(self):
        self.hid = hid.device()
        self.hid.open_path(self.device)
        self.hid.set_nonblocking(True)

        # determine hid_version
        if isinstance(self, HidTransportV2):
            self.hid_version = 2
        else:
            r = self.hid.write([0, 63, ] + [0xFF] * 63)
            if r == 65:
                self.hid_version = 2
                return
            r = self.hid.write([63, ] + [0xFF] * 63)
            if r == 64:
                self.hid_version = 1
                return
            raise ConnectionError("Unknown HID version")

    def _close(self):
        self.hid.close()
        self.hid = None

    def _write_chunk(self, chunk):
        if len(chunk) != 64:
            raise Exception("Unexpected data length")

        if self.hid_version == 2:
            self.hid.write(b'\0' + chunk)
        else:
            self.hid.write(chunk)

    def _read_chunk(self):
        start = time.time()

        while True:
            data = self.hid.read(64)
            if not len(data):
                if time.time() - start > 10:
                    # Over 10 s of no response, let's check if
                    # device is still alive
                    if not self.is_connected():
                        raise ConnectionError("Connection failed")

                    # Restart timer
                    start = time.time()

                time.sleep(0.001)
                continue

            break

        if len(data) != 64:
            raise Exception("Unexpected chunk size: %d" % len(data))

        return bytearray(data)

class HidTransportV1(_HidTransport, TransportV1):
    pass

class HidTransportV2(_HidTransport, TransportV2):
    pass

DEVICE_IDS = [
    (0x534c, 0x0001),  # TREZOR
    (0x1209, 0x53C0),  # TREZORv2 Bootloader
    (0x1209, 0x53C1),  # TREZORv2
]

DEVICE_TRANSPORTS = {
    (0x534c, 0x0001): HidTransportV1,  # TREZOR
    (0x1209, 0x53C0): HidTransportV2,  # TREZORv2 Bootloader
    (0x1209, 0x53C1): HidTransportV2,  # TREZORv2
}

# Backward compatible wrapper, decides for proper transport
# based on VID/PID of given path
def HidTransport(device, *args, **kwargs):
    transport = path_to_transport(device[0])
    return transport(device, *args, **kwargs)

# Backward compatibility hack; HidTransport is a function, not a class like before
HidTransport.enumerate = enumerate
prusnak commented 7 years ago

What does the result of running the following in Python

import hid
hid.enumerate()

look like? (while TREZOR is connected?)

VitGottwald commented 7 years ago 
$ python
Python 2.7.13 (default, Dec 18 2016, 07:03:39)
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.42.1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import hid
>>> hid.enumerate()
[{'interface_number': -1,
  'manufacturer_string': u'Apple Inc.',
  'path': 'USB_05ac_0274_14400000',
  'product_id': 628,
  'product_string': u'Apple Internal Keyboard / Trackpad',
  'release_number': 1572,
  'serial_number': u'D3H62661FU1GHMFAF2FS',
  'usage': 11,
  'usage_page': 65280,
  'vendor_id': 1452},
 {'interface_number': -1,
  'manufacturer_string': u'Apple Inc.',
  'path': 'USB_05ac_0274_14400000',
  'product_id': 628,
  'product_string': u'Apple Internal Keyboard / Trackpad',
  'release_number': 1572,
  'serial_number': u'D3H62661FU1GHMFAF2FS',
  'usage': 6,
  'usage_page': 1,
  'vendor_id': 1452},
 {'interface_number': -1,
  'manufacturer_string': u'SatoshiLabs',
  'path': 'USB_534c_0001_14200000',
  'product_id': 1,
  'product_string': u'TREZOR',
  'release_number': 256,
  'serial_number': u'25EF44E0D2C99827292CC0AE',
  'usage': 1,
  'usage_page': 61904,
  'vendor_id': 21324},
 {'interface_number': -1,
  'manufacturer_string': u'Apple Inc.',
  'path': 'USB_05ac_0274_14400000',
  'product_id': 628,
  'product_string': u'Apple Internal Keyboard / Trackpad',
  'release_number': 1572,
  'serial_number': u'D3H62661FU1GHMFAF2FS',
  'usage': 2,
  'usage_page': 1,
  'vendor_id': 1452},
 {'interface_number': -1,
  'manufacturer_string': u'SatoshiLabs',
  'path': 'USB_534c_0001_14200000',
  'product_id': 1,
  'product_string': u'TREZOR',
  'release_number': 256,
  'serial_number': u'25EF44E0D2C99827292CC0AE',
  'usage': 1,
  'usage_page': 65280,
  'vendor_id': 21324},
 {'interface_number': -1,
  'manufacturer_string': u'Apple Inc.',
  'path': 'USB_05ac_0274_14400000',
  'product_id': 628,
  'product_string': u'Apple Internal Keyboard / Trackpad',
  'release_number': 1572,
  'serial_number': u'D3H62661FU1GHMFAF2FS',
  'usage': 13,
  'usage_page': 65280,
  'vendor_id': 1452},
 {'interface_number': -1,
  'manufacturer_string': u'Apple Inc.',
  'path': 'USB_05ac_0274_14400000',
  'product_id': 628,
  'product_string': u'Apple Internal Keyboard / Trackpad',
  'release_number': 1572,
  'serial_number': u'D3H62661FU1GHMFAF2FS',
  'usage': 3,
  'usage_page': 65280,
  'vendor_id': 1452}]
>>>
prusnak commented 7 years ago

Crappy OSX USB stack. :-/ Both interfaces (normal and U2F) report the same path (USB_534c_0001_14200000 in your case). And it also seems that random interface is opened each time.

I wonder if that changed recently or was always broken like this.

I have a big rewrite of all transport-related stuff in python-trezor, but this might take some time to finish it. Without it this issue is not fixable unfortunately (because our Python stack uses path to identify the correct interface it should talk to, not its native USB handle).

VitGottwald commented 7 years ago

Any idea why Chrome plugin for TREZOR password manager works flawlessly over the same USB port and cable?

prusnak commented 7 years ago

Because they use device handles, not paths to distinguish between the interfaces, while our Python stack uses strings to identify the interfaces.

VitGottwald commented 7 years ago

It seems that you already know the exact problem and what it would take to fix it.

So just for my better understanding - is the problem caused by the python code using https://github.com/signal11/hidapi while the chrome plugin using https://developer.chrome.com/apps/hid to deal with USB devices?

prusnak commented 7 years ago

I found the culprit ...

Older versions of hidapi used custom paths which had collisions I described above. Latest release of python-hidapi (0.7.99.post20) used updated version of hidapi from master, which uses IOKit paths, which do not have these collisions:

new hidapi:

IOService:/AppleACPIPlatformExpert/PCI0@0/AppleACPIPCI/XHC1@14/XHC1@14000000/PRT1@14100000/TREZOR@14100000/IOUSBHostInterface@0/IOUSBHostHIDDevice@14100000,0
IOService:/AppleACPIPlatformExpert/PCI0@0/AppleACPIPCI/XHC1@14/XHC1@14000000/PRT1@14100000/TREZOR@14100000/IOUSBHostInterface@1/IOUSBHostHIDDevice@14100000,1

vs old hidapi:

USB_534c_0001_14200000
USB_534c_0001_14200000

Please update to latest hidapi. I also added requirement for hidapi>=0.7.99.post20 to python-trezor.

VitGottwald commented 7 years ago

Upgrading to hidapi (0.7.99.post20) fixed the problem. Thank you!!!

romanz commented 7 years ago

Thanks @prusnak :)

optimator999 commented 7 years ago

Can confirm. I had the same issue. Solved with sudo pip install -I hidapi