trustcrypto / onlykey-agent

The OnlyKey agent is essentially middleware that lets you use OnlyKey as a hardware SSH/GPG device.
https://docs.crp.to/onlykey-agent.html
GNU Lesser General Public License v3.0
46 stars 15 forks source link

Python 3 compatibility #18

Closed haplo closed 3 years ago

haplo commented 4 years ago

Python 2.7 has now reached EOL, this project should be updated to run on a recent Python 3 version ASAP. I'm going to work on it, please let me know if there is anything I should know before I dive in.

Does anybody know the status of Python 3 support for trezor-agent? Should I look at their code for porting fixes?

onlykey commented 4 years ago

@haplo python-onlykey which is required for onlykey-agent was recently updated for python 3 compatibility, onlykey-agent may run now on python 3, we haven't had time to test it out yet. If you want to test it out that would be great!

haplo commented 4 years ago

It doesn't currently work, I originally installed onlykey-agent with pip3 but it failed to start because of an old-style print without parentheses.

I will start working on a python3 branch.

haplo commented 4 years ago

Today I upgraded to Kubuntu 20.04, which drops support for Python 2, and only after the fact I realized that onlykey-agent had stopped working. I have a branch with python3 support for onlykey-agent, but turns out that the onlykey Python library is also Python 2-exclusive.

Python 2 is not supported anymore, so I think onlykey should support only Python 3.5 and newer.

onlykey commented 4 years ago

@haplo I am working on the new onlykey-agent which we are looking to be made part of trezor agent - https://github.com/romanz/trezor-agent/issues/303

Its not ready yet though, help on this would be appreciated. The reason its taking so long is I am adding support for both selectable ECC keys (in addition to the derived keys) and RSA, and GPG support. I will push my latest changes later today.

I have not tested backwards compatibility with the old SSH agent yet

onlykey commented 4 years ago

@haplo To test

Git clone this branch https://github.com/onlykey/trezor-agent/tree/onlykey-agent-merge

pip install trezor-agent/ pip install trezor-agent/agents/onlykey/

and this

https://github.com/trustcrypto/python-onlykey

pip install python-onlykey/

The default now is ed25519 though, this used to be nist256p1 so to use that do

onlykey-agent test@domain -e nist256p1 -c

haplo commented 4 years ago

@onlykey I have pushed several changes to #21 , the onlykey-agent is working for me with these changes and with this fix to python-onlykey.

Please note that I haven't checked for Python 2.7 compatibility with these new changes, but I think Python 3 compatibility should have very high priority as more and more users will stop having Python 2 readily available.

haplo commented 4 years ago

I understand that the main focus now is the merge in trezor-agent, but consider making one release of onlykey-agent pinning the python-onlykey version to the current one, then making a new Python 3-compatible python-onlykey release and release onlykey-agent with that version and these Python 3 fixes. That would help people like me who found onlykey-agent didn't work without Python 2. README could be updated to instruct people to install the last Python 2-compatible release.

onlykey commented 4 years ago

@haplo Understood, I plan to release the new one next week. If you have time to test it out it should be backwards compatible with the old onlykey agent

$ git clone https://github.com/trustcrypto/python-onlykey $ pip install python-onlykey/ $ git clone https://github.com/onlykey/trezor-agent -b onlykey-agent-merge $ pip install trezor-agent/ $ pip install trezor-agent/agents/onlykey

We do now default to ed25519 keys instead of nist256p1 so if you used to do this:

onlykey-agent user@host -c

you would now do this:

onlykey-agent user@host -c -e nist256p1

if you used to do this:

onlykey-agent user@host -c -e ed25519

you would now do this:

onlykey-agent user@host -c

haplo commented 4 years ago

I finally found some time to try out the trezor-agent version. I think the new agent forces the key id to be in the form user@host, whereas the previous agent didn't. My key was just host, and now it fails with this error:

$ onlykey-agent -v host -- ssh user@host tmux attach
2020-07-29 20:08:38,036 INFO         identity #0: <ssh://host|ed25519>                                                          [__init__.py:289]
2020-07-29 20:08:38,048 INFO         running ['ssh', 'user@host', 'tmux', 'attach'] with {'SSH_AUTH_SOCK': '/tmp/trezor-ssh-agent-ip1m8qm6', 'SSH_AGENT_PID': '326994'} [server.py:156]
2020-07-29 20:08:38,363 INFO         Requesting public key from key slot =132                                                             [onlykey.py:116]
2020-07-29 20:08:38,364 INFO         disconnected from OnlyKey                                                                            [onlykey.py:98]
2020-07-29 20:08:38,365 WARNING      error: 'user'                                                                                        [server.py:100]
Traceback (most recent call last):
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/lib/python3.8/site-packages/libagent/server.py", line 95, in handle_connection
    reply = handler.handle(msg=msg)
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/lib/python3.8/site-packages/libagent/ssh/protocol.py", line 104, in handle
    reply = method(buf=buf)
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/lib/python3.8/site-packages/libagent/ssh/protocol.py", line 111, in list_pubs
    keys = self.conn.parse_public_keys()
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/lib/python3.8/site-packages/libagent/ssh/__init__.py", line 227, in parse_public_keys
    for pk in self.public_keys()]
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/lib/python3.8/site-packages/libagent/ssh/__init__.py", line 221, in public_keys
    self.public_keys_cache = conn.export_public_keys(self.identities)
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/lib/python3.8/site-packages/libagent/ssh/client.py", line 27, in export_public_keys
    pubkey = self.device.pubkey(identity=i)
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/lib/python3.8/site-packages/libagent/device/onlykey.py", line 123, in pubkey
    id_parts = unidecode.unidecode(identity.identity_dict['user'] + '@' + identity.identity_dict['host']).encode('ascii')
KeyError: 'user'
user@host: Permission denied (publickey).
onlykey commented 4 years ago

@haplo Thanks for testing that, I will see about fixing that this week

onlykey commented 4 years ago

@haplo Actually, it was an easy fix. Pushed fix onlykey-agent-merge branch so that it will work with host or user@host if you want to try it out.

haplo commented 4 years ago

Still failing I'm afraid:

$ onlykey-agent host
Traceback (most recent call last):
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/bin/onlykey-agent", line 8, in <module>
    sys.exit(ssh_agent())
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/bin/onlykey_agent.py", line 5, in <lambda>
    ssh_agent = lambda: libagent.ssh.main(DeviceType)
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/lib/python3.8/site-packages/libagent/ssh/__init__.py", line 185, in wrapper
    return func(*args, **kwargs)
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/lib/python3.8/site-packages/libagent/ssh/__init__.py", line 325, in main
    for pk in conn.public_keys():
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/lib/python3.8/site-packages/libagent/ssh/__init__.py", line 221, in public_keys
    self.public_keys_cache = conn.export_public_keys(self.identities)
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/lib/python3.8/site-packages/libagent/ssh/client.py", line 27, in export_public_keys
    pubkey = self.device.pubkey(identity=i)
  File "/home/user/src/virtualenvs/onlykey-trezor-agent/lib/python3.8/site-packages/libagent/device/onlykey.py", line 123, in pubkey
    id_parts = unidecode.unidecode(identity.identity_dict['user'] + '@' + identity.identity_dict['host']).encode('ascii')
KeyError: 'user'

I will debug it and create PR.

haplo commented 4 years ago

Nevermind, I was running old code, the fix works. :+1: