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 error generating gpg subkeys for nistp256 and ed25519 primary keys #483

Closed
wants to merge 1 commit into from

Commits on Aug 2, 2024

  1. Fix error generating gpg subkeys for nistp256 and ed25519 primary keys

    This error would occur whenever the agent received a HAVEKEY request for
    the primary key that has a TREZOR-generated subkey, if the primary key
    algorithm was nistp256 or ed25519.
    
    This does not fix the AssertionError that occurs when using
    brainpoolP256r1 or secp256k1 primary keys.
    
    The specific error backtrace addressed by this patch is:
    
        Traceback (most recent call last):
          File ".../libagent/gpg/__init__.py", line 290, in run_agent_internal
            handler.handle(conn)
          File ".../libagent/gpg/agent.py", line 250, in handle
            handler(conn, args)
          File ".../libagent/gpg/agent.py", line 102, in <lambda>
            b'HAVEKEY': lambda conn, args: self.have_key(conn, *args),
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^
          File ".../libagent/gpg/agent.py", line 213, in have_key
            self.get_identity(keygrip=keygrip)
          File ".../libagent/util.py", line 231, in wrapper
            result = method(self, *args, **kwargs)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
          File ".../libagent/gpg/agent.py", line 174, in get_identity
            assert pubkey.key_id() == pubkey_dict['key_id']
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        AssertionError
    
    Results before and after this commit:
    
                          |  before this change   |  after this change    |
                          |    subkey algo:       |    subkey algo:       |
        primary key algo  |  ed25519  | nist256p1 |  ed25519  | nist256p1 |
        ------------------+-----------+-----------+-----------+-----------+
        ed25519           | FAILED(1) | FAILED(1) |    OK     |    OK     |
        nistp256          | FAILED(1) | FAILED(1) |    OK     |    OK     |
        secp256k1         | FAILED(2) | FAILED(2) | FAILED(2) | FAILED(2) |
        brainpoolP256r1   | FAILED(2) | FAILED(2) | FAILED(2) | FAILED(2) |
        dsa3072           |    OK     |   OK      |    OK     |    OK     |
        rsa3072           |    OK     |   OK      |    OK     |    OK     |
    
        (1) Fails on `assert pubkey.key_id() == pubkey_dict['key_id']`
            This patch fixes these.
    
        (2) Fails on `assert oid in SUPPORTED_CURVES, util.hexlify(oid)`
            This patch doesn't fix these.
    
    I used the following script for testing on a Trezor Model T:
    
        #!/bin/bash
        # Usage: run-tests-combined.sh
        #        run-tests-combined.sh primary_key_algo subkey_algo
        #
        # When run with no argument, this script re-invokes itself multiple
        # times with different combinations of algorithms and outputs the
        # result to ./test-results-$UNIX_TIMESTAMP.log.
        #
        # Otherwise, this script runs a single test with the given algorithms.
        set -euC
    
        if [ "$#" -eq 0 ]; then
            logfile="./test-results-$(date +%s).log"
            echo "Writing to $logfile"
            > "$logfile"
    
            for pka in rsa3072 dsa3072 brainpoolP256r1 secp256k1 nistp256 ed25519; do
                for ska in nist256p1 ed25519; do
                    if "$0" "$pka" "$ska"; then
                        printf '(%s, %s): OK\n' "$pka" "$ska" >> "$logfile"
                        printf '\033[1m(%s, %s): OK\033[m\n' "$pka" "$ska"
                    else
                        printf '(%s, %s): FAILED\n' "$pka" "$ska" >> "$logfile"
                        printf '\033[1m(%s, %s): FAILED\033[m\n' "$pka" "$ska"
                    fi
                done
            done
    
            echo ""
            echo "Test results:"
            cat "$logfile"
    
            exit 0
        fi
    
        # Individual test begins here
    
        primary_key_algo="$1"
        subkey_algo="$2"
    
        killall gpg-agent trezor-gpg-agent || true
    
        userid='Test User <[email protected]>'
        export GNUPGHOME=$(mktemp --tmpdir -d "trezor-gpg.XXXXXXXXXX" )
        echo "GNUPGHOME is $GNUPGHOME"
    
        # Generate primary key locally
        gpg --batch --passphrase '' \
            --quick-gen-key "$userid" "$primary_key_algo" cert
    
        # Generate subkey on Trezor
        trezor-gpg init --homedir "$GNUPGHOME/trezor" \
            -e "$subkey_algo" -t "$(date +%s)" --subkey "$userid"
    dlitz committed Aug 2, 2024
    Configuration menu
    Copy the full SHA
    6297ee1 View commit details
    Browse the repository at this point in the history