romanz
Support Signing and Authentication subkeys
This pull request is for adding a Signing subkey and an Authentication subkey, in addition to the existing Encryption subkey. It also changes the Primary key to be Certify only. This structure matches best practices found here and here.
One of the benefits of this structure is that it becomes possible to use the Trezor to deterministically generate the Primary and Subkeys, use the included recovery script to mimic the generation and recover the secret portions (suggested to only do this in an offline Tails environment), then move the secret subkeys to a smartcard device like a Yubikey 5. This allows one to achieve the security and determinism of the Trezor (especially when utilizing Shamir), while maintaining the compatability and convienience of a smartcard like the Yubikey. Note that with this scheme, the Primary (certifying) key remains completely offline, and is not intended to be used.
The current code creates a Primary key with Certify and Sign capabilities, and a single Encryption Subkey. The Primary key uses SLIP 13, while the Encryption Subkey uses SLIP 17. Both use derivation index 0 of their respective schemes. The code uses a single boolean named ecdh to differentiate the two.
This pull request adds an enum named KeyFlags. This enum accomplishes 3 things simultaneously:
- Determines the capabilities of the keys
- Determines the derivation index
- Determines the curve algorithm
The derivation index comes into play because we want the Primary Key, Signing Subkey, and Authentication Subkey to all be different. I chose to use derivation indices 0, 1, 2 for these respectively. The derivation index is normally passed to the Trezor via the IdentityType message, using a field named index. Currently, this value is not set by trezor-agent, so it defaults to 0. In order for _identity_proto to properly set the index, it needs to be available in the identity_dict of the Identity class. Per SLIP 13/17, the derivation index is separate from the identity_str (RFC 3986 URI), therefore the index is derived from a KeyFlag parameter added to the constructor. This change does end up rippling through some code, notably necessatating an Identity for each Public Key created in init.
A majority of the rest of the changes involve replacing the ecdh variable with the KeyFlags enum and adjusting functions and calls accordingly.
A second change was needed to accomplish the goal. Now that multiple subkeys (and therefore multiple KeyFlags) are at play, the decode class needed to be able to parse these KeyFlags from the keys. The KeyFlags are stored inside the hashed area of the signature packets for a key. According to page 69 of RFC 4880, the signature packet always follows the pubkey/subkey packet, so the load_by_keygrip was rewritten to map them together and return it to get_identity. A unit test was added for this.
Finally, gpg version 2.1.14 adds a new subpacket type 33, which is not part of RFC 4880 (and has caused some problems for other clients). It is used for verifying cross-certification of signing subkeys, and is only specifically required for signing subkeys (not authentication subkeys, this was verified by using gpg to generate a new key with subkeys, then dump the packets). Here are some handy links:
- Definition of the subpacket type
- The format of the packet (version number followed by fingerprint)
- Locating the cross-certification signature
That last link shows a "gotcha", where the signature creation date must be greater than 0 for gpg to register it as a cross-certification signature. I chose to add 1 to the embedded signature creation time. This does not affect the public keys themselves.
Lastly, I rearranged some fields to match the same order that gpg produces, refactored libagent/gpg/__init__.py since the inclusion of the subkeys has introduced some repetition.
I did multiple tests to ensure backward compatability. First, I wiped a Trezor and initialized it with a mnemonic of all all all all all all all all all all all all. Then, I used the current version of trezor-agent to both sign and encrypt a file.
Next, I used the version of trezor-agent from this pull request to both verify and decrypt the file, using the "normal" key layout. The fingerprints and keygrips were also compared and found to be the same. Afterwards, I did the test again by passing the --smartcard flag to generate the three subkeys. The fingerprint and keygrip of the certifying key and encryption key are the same, as expected, and the "smartcard" layout is able to both verify and decrypt the same file.
Lastly, I used the contrib/trezor_agent_recovery.py script to recover the public and private keys. The public key was compared to the one generated by trezor-agent init --smartcard and found to be the same, and the private key was the proper pair. Using the private key, I was able to decrypt the file without the Trezor.
This confirms the ability to both generate and recover the private subkeys, for moving to a Yubikey 5.
I tried to maintain the same coding style that was present, and made sure that pylint and pytest both passed. If you have any questions, let me know. Thank you!
@jrruethe Have you tried generating a ssh key after these changes? I just tried and am getting the following error:
Traceback (most recent call last): File "/home/cameron/miniconda3/envs/trezor/bin/trezor-agent", line 8, in
sys.exit(ssh_agent()) File "/home/cameron/miniconda3/envs/trezor/bin/trezor_agent.py", line 4, in ssh_agent = lambda: ssh.main(DeviceType) File "/home/cameron/miniconda3/envs/trezor/lib/python3.9/site-packages/libagent/ssh/init.py", line 173, in wrapper return func(*args, **kwargs) File "/home/cameron/miniconda3/envs/trezor/lib/python3.9/site-packages/libagent/ssh/init.py", line 269, in main identities = [device.interface.Identity( TypeError: init() missing 1 required positional argument: 'keyflag'
It appears there is a parameter in Identity: keyflag that is not set from "libagent/ssh/init.py"
I have not, but I can look at that and fix it. I only did testing with GPG, not SSH. Thanks for the heads up!
Hey, awesome work on this @jrruethe! Just wanted to check what the status of this is?
@lukechilds I've been using my branch without issues for my own purposes, so for what I needed it is complete. I used it to generate deterministic subkeys from the Trezor and load them onto my Yubikey 5 NFC, which can then be used with the standard GPG installation.
Sorry for the very delayed response...
Could you please rebase over the latest master?
@lukechilds I've been using my branch without issues for my own purposes, so for what I needed it is complete. I used it to generate deterministic subkeys from the Trezor and load them onto my Yubikey 5 NFC, which can then be used with the standard GPG installation.
@jrruethe Do you have a writeup or can you point me in the right direction for moving loading subkeys onto the Yubikey 5 NFC.
@lukechilds I've been using my branch without issues for my own purposes, so for what I needed it is complete. I used it to generate deterministic subkeys from the Trezor and load them onto my Yubikey 5 NFC, which can then be used with the standard GPG installation.
@jrruethe Hi Joe! This is exactly what I wanted to get working. As @McCodeman would you please be able to provide instructions on how to do exactly what you said? I would love to backup the Trezor master key and create/revoke subkeys and use them with my Yubikey. Sounds amazing! Thanks!!
Essentially, I used the script in this pull request "contrib/trezor_agent_recover.py" with the "--smartcard" option. I used a Tails environment that was not connected to the internet, and entered my Trezor seed into the script, which output the private GPG keys (both the master and subkeys). After that, I just used the standard instructions for moving the subkeys to a Yubikey, such as this guide: https://github.com/drduh/YubiKey-Guide
There isn't a way to extract the private keys directly from the Trezor itself, which is why I supplied the recovery script. The recovery script generates the same keys as the Trezor when the same seed is used. This means that signing and encryption operations done with the Trezor can be used with the Yubikey, and vice versa.
Essentially, I used the script in this pull request "contrib/trezor_agent_recover.py" with the "--smartcard" option. I used a Tails environment that was not connected to the internet, and entered my Trezor seed into the script, which output the private GPG keys (both the master and subkeys). After that, I just used the standard instructions for moving the subkeys to a Yubikey, such as this guide: https://github.com/drduh/YubiKey-Guide
There isn't a way to extract the private keys directly from the Trezor itself, which is why I supplied the recovery script. The recovery script generates the same keys as the Trezor when the same seed is used. This means that signing and encryption operations done with the Trezor can be used with the Yubikey, and vice versa.
Thanks for replying so quick! I'm an absolute novice on github. I don't mind running the python file on my regular computer as I'm just testing around with my Trezor, there's nothing on it. My question is how do I just run the file? Just open Terminal and run the saved file? Or do I enter my seed somewhere into the python script and run it? I did a shamir seed, so I would have enter the entire thing? Then do I remove do a "rm -r trezor" that I currently have and then run the script as if I'm doing the "trezor-gpg init... -- smartcard" command? I sent you an email in hopes that you can guide me, I don't mind compensating for your time as I'm very interested in this. I came across GPG not long ago and became very interested. Thanks!
@jrruethe sorry, how do you run this version of the script instead of the usual "trezor-gpg init..." ?
For an ubuntu-based system, the following commands should get you started:
sudo apt install python3-setuptools
git clone https://github.com/jrruethe/trezor-agent.git
cd trezor-agent
git checkout subkeys
sudo python3 ./setup.py install
python3 ./contrib/trezor_agent_recover.py --smartcard true
@jrruethe thanks for getting back! I got a traceback, maybe I'm doing something wrong?
Traceback (most recent call last): File "trezor_agent_recover.py", line 25, in <module> from libagent.formats import KeyFlags # type: ignore ImportError: cannot import name 'KeyFlags' from 'libagent.formats' (/home/granttesler/.local/lib/python3.8/site-packages/libagent/formats.py)
@jrruethe any advice for why I'm getting the Traceback for the Keyflags error that I listed above?
@jrruethe your contrib/trezor_agent_recover.py is really useful to me. I'm not too familiar with the internals of this lib, would it be simple to modify it to also export the SSH key?
Unfortunately, I don't have the capacity to work on this any more right now.
For SSH, I have my GPG agent set up to also be an SSH agent. SSH connections utilize the authorization subkey on the Yubikey.
This guide has instructions on how to go that route: https://github.com/drduh/YubiKey-Guide
@jrruethe Could you please rebase your PR over latest master?
I would be happy to merge it :)
After rebasing, I can't seem to decrypt files, so apparently I messed something up. I'll try to dig in deeper if I get a chance
@romanz Ok, I got it all working and retested it with a real trezor. I also tested the trezor_agent_recover.py script again, and everything looks good.
Ready to merge! Thanks!
EDIT - I certainly hope I did that right, my git is not as strong as it should be :)
I have squashed all the changes above into a single commit: f16310f03eb71bf4c40b124e2c04df15251bbd41. Please review it - it should contain the same code as https://github.com/romanz/trezor-agent/pull/358/commits/e11afff47d48dc45b13dce3f727be26417364757 above:
$ git diff e11afff f16310f
It seems that CI didn't run on this PR...
My bad - fixed in 2b49eacc01b45caa5c7da379b54378d58e60bdd7 (please rebase over latest master).
@jrruethe any chance of getting the same capabilities for ed25519? It seems to be more advised these days than nist's.
Thanks
@andrevmatos I don't have any plans to implement that right now. I am just trying to get the current PR completed.
@romanz Anything else needed to get this merged?
Will do the final review this weekend - many thanks for the contribution!
I also tested it using fake-device agent (on 3c45889), and it seems that regular GPG identity fails to sign:
$ gpg --version
gpg (GnuPG) 2.2.27
$ export GNUPGHOME=/tmp/gnupg
$ rm -r /tmp/gnupg/
$ fake-device-gpg init "User"
2023-06-09 19:09:11,039 WARNING This GPG tool is still in EXPERIMENTAL mode, so please note that the API and features may change without backwards compatibility! [__init__.py:171]
2023-06-09 19:09:11,045 WARNING NOTE: in order to re-generate the exact same GPG key later, run this command with "--time=0" commandline flag (to set the timestamp of the GPG key manually). [__init__.py:86]
2023-06-09 19:09:11,045 CRITICAL NEVER USE THIS CODE FOR REAL-LIFE USE-CASES!!! [fake_device.py:31]
2023-06-09 19:09:11,045 CRITICAL ONLY FOR DEBUGGING AND TESTING!!! [fake_device.py:32]
2023-06-09 19:09:11,045 CRITICAL NEVER USE THIS CODE FOR REAL-LIFE USE-CASES!!! [fake_device.py:31]
2023-06-09 19:09:11,045 CRITICAL ONLY FOR DEBUGGING AND TESTING!!! [fake_device.py:32]
2023-06-09 19:09:11,051 CRITICAL NEVER USE THIS CODE FOR REAL-LIFE USE-CASES!!! [fake_device.py:31]
2023-06-09 19:09:11,051 CRITICAL ONLY FOR DEBUGGING AND TESTING!!! [fake_device.py:32]
2023-06-09 19:09:11,052 CRITICAL NEVER USE THIS CODE FOR REAL-LIFE USE-CASES!!! [fake_device.py:31]
2023-06-09 19:09:11,052 CRITICAL ONLY FOR DEBUGGING AND TESTING!!! [fake_device.py:32]
gpg: inserting ownertrust of 6
gpg: checking the trustdb
gpg: marginals needed: 3 completes needed: 1 trust model: pgp
gpg: depth: 0 valid: 1 signed: 0 trust: 0-, 0q, 0n, 0m, 0f, 1u
sec nistp256 1970-01-01 [C]
8F37E3F7339D2542B8133BE64D8F4714A14548AC
uid [ultimate] User
ssb nistp256 1970-01-01 [E]
$ gpg --sign README.md
gpg: using "User" as default secret key for signing
gpg: no default secret key: Unusable secret key
gpg: signing failed: Unusable secret key
It does seems to work on master:
$ rm -r /tmp/gnupg/
$ fake-device-gpg init "User"
2023-06-09 19:10:56,352 WARNING This GPG tool is still in EXPERIMENTAL mode, so please note that the API and features may change without backwards compatibility! [__init__.py:118]
2023-06-09 19:10:56,357 WARNING NOTE: in order to re-generate the exact same GPG key later, run this command with "--time=0" commandline flag (to set the timestamp of the GPG key manually). [__init__.py:33]
2023-06-09 19:10:56,358 CRITICAL NEVER USE THIS CODE FOR REAL-LIFE USE-CASES!!! [fake_device.py:31]
2023-06-09 19:10:56,358 CRITICAL ONLY FOR DEBUGGING AND TESTING!!! [fake_device.py:32]
2023-06-09 19:10:56,358 CRITICAL NEVER USE THIS CODE FOR REAL-LIFE USE-CASES!!! [fake_device.py:31]
2023-06-09 19:10:56,358 CRITICAL ONLY FOR DEBUGGING AND TESTING!!! [fake_device.py:32]
2023-06-09 19:10:56,358 CRITICAL NEVER USE THIS CODE FOR REAL-LIFE USE-CASES!!! [fake_device.py:31]
2023-06-09 19:10:56,358 CRITICAL ONLY FOR DEBUGGING AND TESTING!!! [fake_device.py:32]
2023-06-09 19:10:56,365 CRITICAL NEVER USE THIS CODE FOR REAL-LIFE USE-CASES!!! [fake_device.py:31]
2023-06-09 19:10:56,365 CRITICAL ONLY FOR DEBUGGING AND TESTING!!! [fake_device.py:32]
gpg: inserting ownertrust of 6
gpg: checking the trustdb
gpg: marginals needed: 3 completes needed: 1 trust model: pgp
gpg: depth: 0 valid: 1 signed: 0 trust: 0-, 0q, 0n, 0m, 0f, 1u
sec nistp256 1970-01-01 [SC]
8F37E3F7339D2542B8133BE64D8F4714A14548AC
uid [ultimate] User
ssb nistp256 1970-01-01 [E]
$ rm README.md.gpg
$ gpg --sign README.md
gpg: using "User" as default secret key for signing
$ gpg --verify README.md.gpg
gpg: Signature made Fri 09 Jun 2023 19:11:08 IDT
gpg: using ECDSA key 8F37E3F7339D2542B8133BE64D8F4714A14548AC
gpg: Good signature from "User" [uncertain]
@jrruethe Could you please take a look?