Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix AssertionError on decryption whenever there is another ECC key in the keychain #470

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

prometheanSolutions
Copy link

@prometheanSolutions prometheanSolutions commented Dec 23, 2023

Whenever there are other public keys in the GPG keychain with the ECC alogrithm, decryption is not possible.

How to reproduce:

Short:
Suppose our trezor-key has the uuid: 'trezor'. We create another public key called 'Alice' and encrypt a file with both keys.
Decryption is not possible.

Long:

  1. Generate second key with the following properties: ECC and ECC (encryption) - nistp256
    gpg --full-generate-key --expert
    (9) ECC and ECC
    (3) NIST P-256
    Key is valid for? (0) 0
    Real name: Alice
  2. Delete secret key for key 'Alice'
    gpg --delete-secret-key 'Alice'
  3. Encrypt a test file with both keys
    gpg -e -r Alice -r trezor test
  4. Restart trezor gpg agent
  5. Try to decrypt
    gpg -d test.gpg

Result: There is an AssertionError:
File "/home/user/projects/trezor-agent/libagent/gpg/agent.py", line 174, in get_identity
assert pubkey.key_id() == pubkey_dict['key_id']
AssertionError

This is the case, because all compatible keys in the keychain are being tried and when the key is not the same with the key on the HW device an Assertion error is thrown.

Fix by: Ignore keys which do not correspond to key on device instead of throwing an Assertion Error

@dlitz
Copy link

dlitz commented Aug 2, 2024

Looks like I duplicated this work in #483. The comment there has a script to automate key generation & testing/repro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants