spesmilo / electrum

Electrum Bitcoin Wallet
https://electrum.org
MIT License
7.41k stars 3.08k forks source link

The Ledger plugin is currently broken for old Ledgers HW1 (can get account addresses, but Electrum hanging when signing a Tx) #7022

Open loupiote opened 3 years ago

loupiote commented 3 years ago

The Ledger plugin is currently broken for old Ledgers HW1 (the type of ledger hardware wallets that have no display). Electrum displays a "read error" pop-up when attempting to send (i.e. to sign a Tx). Electrum is capable of getting the account addresses and display the balances, so communication with the WD1 ledger is working, but not possible to sign the Tx and send BTC (using Electrum 4.0.9).

I could maybe try to find the problem myself. What would you recommend to debug an Electrum Plugin?

I.e. if I need to print things from within the ledger.py plugin, should I use the regular Python print() or is there a way to print in the Electrum console? (I'll be using Linux)

SomberNight commented 3 years ago

I've just tested on git master with an old Ledger HW.1 on testnet and I could spend coins from p2pkh and p2wpkh-p2sh scripts. That is, it worked for me, I could not reproduce the issue.

Note that the HW.1 (1) cannot spend from p2wpkh scripts, and (2) it cannot send to any bech32 address.

Re (1) one could already not create such a wallet, the wizard gives an error. Re (2), I've now added a commit (https://github.com/spesmilo/electrum/commit/b56fe237cdc604e5b527248acd5fb61e46a8a48c) to make this clear and show an error, as the behaviour was really confusing (when trying to sign the tx, it was as if the user had cancelled the flow).

I could maybe try to find the problem myself. What would you recommend to debug an Electrum Plugin? I.e. if I need to print things from within the ledger.py plugin, should I use the regular Python print() or is there a way to print in the Electrum console? (I'll be using Linux)

Yes, print() statements are fine. You can also use the logging system (which uses the python stdlib), see e.g. https://github.com/spesmilo/electrum/blob/b56fe237cdc604e5b527248acd5fb61e46a8a48c/electrum/plugins/ledger/ledger.py#L571

Note that if you will be using the Console tab in the Qt GUI, depending on how you log, you should monitor both the Console tab itself and stdout/stderr. I recommend you start Electrum from the terminal, with the -v flag, e.g. ./run_electrum -v, to see more logs.

loupiote commented 3 years ago

Note that the HW.1 (1) cannot spend from p2wpkh scripts, and (2) it cannot send to any bech32 address.

Ok, thanks, maybe that was the issue. I just received a report from another user, but I don't yet have a HW.1 device to test myself. So, it should work only when sending from a legacy address to either a legacy address (starts with 1) or a p2wpkh address (starts with 2 or 3), right?

The user sent me a screenshot that says "read error" after they entered their PIN to get the Tx signed.

I will run some tests after I receive my HW.1 device. Thanks again for all the advice!

SomberNight commented 3 years ago

So, it should work only when sending from a legacy address to either a legacy address (starts with 1) or a p2wpkh address (starts with 2 or 3), right?

p2pkh scripts have addresses that start with 1 on mainnet. p2wpkh-p2sh starts with 3. p2wpkh starts with bc1q.

So the HW.1 can spend from 1 and 3 addresses and send to those, but cannot do anything with bc1 addresses.

The user sent me a screenshot that says "read error" after they entered their PIN to get the Tx signed.

I did not manage to get a "read error" in any of the scenarios.

loupiote commented 3 years ago

I can confirm that Electrum does not work with the Ledger 1 (i.e. according to the Ledger company, same hardware as the ledger HW.1 with a different packaging. It has no screen and no buttons). It was tested on Mac, Win10 and Linux. Test was a transfer to a legacy address. After cliquing the "Send" button, Electrum asks for the PIN. After the PIN is entered, Electrum asks to confirm the Transaction on the device (which cannot be done, since the device has no screen and no button!). Then it hangs on "Signing Transaction", and the Tx is never sent.

I have attached the log file obtained using the -v option on Linux. Also attached is the screen-shot. The addresses with the unspent BTC can be seen in the log.

electrum2.log.txt WhatsApp Image 2021-02-15 at 17 31 54 WhatsApp Image 2021-02-15 at 13 13 11

loupiote commented 3 years ago

We did some mode debugging and we found out the one particular call causes the ledger to disconnect (and Electrum to stay hanging).

The call is line 503 of the ledger.py plugin:

503: outputData = client_ledger.finalizeInput(b'', 0, 0, changePath, bfh(rawTx))

The parameters are: changePath = 44'/0'/0'/1/0

rawTx = 0200000001c16dae699593c713bd04709fad1474d9ed1e140ef3fb43af9815da948711e89500000000230021022a5c4bdac8788757528feef374a16370ce9fffeb685ea7337c6d8eb514055c35fdffffff02a0860100000000001976a914f814983680610d9833cb00581c6aefb4a7c066de88ace4117800000000001976a914d732eccc8898cd964d033e23e7432ed298de29d688accc3c0a00

I could not find any Python code for finalizeInput, so I imagine that this is defined in a library from Ledger?

Let me know what you think.

Below is the decoded rawTx: (using https://live.blockcypher.com/btc/decodetx/) { "addresses": [ "16sNNMdm7PTHZqznJvTbKnKCXAaehTcP6x", "1PcjFWcXg4PGHhCnK9VqqNoCdbXyPsnhNo", "1LcsHaJuekqwtZypQjmTQVTYcaJKkLnfBW" ], "block_height": -1, "block_index": -1, "confirmations": 0, "double_spend": false, "fees": 36100, "hash": "92af8d8761440cc43b452826e4c325bcd6581d69e36ed2ecc7d7b7c9b0168117", "inputs": [ { "addresses": [ "16sNNMdm7PTHZqznJvTbKnKCXAaehTcP6x" ], "age": 350244, "output_index": 0, "output_value": 8005000, "prev_hash": "95e8118794da1598af43fbf30e141eedd97414ad9f7004bd13c7939569ae6dc1", "script": "0021022a5c4bdac8788757528feef374a16370ce9fffeb685ea7337c6d8eb514055c35", "script_type": "pay-to-pubkey-hash", "sequence": 4294967293 } ], "lock_time": 670924, "opt_in_rbf": true, "outputs": [ { "addresses": [ "1PcjFWcXg4PGHhCnK9VqqNoCdbXyPsnhNo" ], "script": "76a914f814983680610d9833cb00581c6aefb4a7c066de88ac", "script_type": "pay-to-pubkey-hash", "value": 100000 }, { "addresses": [ "1LcsHaJuekqwtZypQjmTQVTYcaJKkLnfBW" ], "script": "76a914d732eccc8898cd964d033e23e7432ed298de29d688ac", "script_type": "pay-to-pubkey-hash", "value": 7868900 } ], "preference": "high", "received": "2021-02-17T03:49:03.325874223Z", "relayed_by": "54.86.109.156", "size": 154, "total": 7968900, "ver": 2, "vin_sz": 1, "vout_sz": 2, "vsize": 154 }

SomberNight commented 3 years ago

The call is line 503 of the ledger.py plugin: https://github.com/spesmilo/electrum/blob/ae15bccb8192532afc4908b43f68797cd6239fd6/electrum/plugins/ledger/ledger.py#L503 I could not find any Python code for finalizeInput, so I imagine that this is defined in a library from Ledger?

Yes, it is here: https://github.com/LedgerHQ/btchip-python/blob/edf0517d6673288f7e0df46fe95ab13e7eaf4c16/btchip/btchip.py#L268


Maybe the reason I cannot reproduce is due to firmware version differences?

You can get the firmware version using the "Console" tab (View>Show Console):

>>> wallet.keystore.get_client().getFirmwareVersion()
{'compressedKeys': True, 'version': '1.0.4', 'specialVersion': 32}
loupiote commented 3 years ago

You can get the firmware version using the "Console" tab (View>Show Console):

Ok, I'll try, thanks!

loupiote commented 3 years ago

here is the version:

{'compressedKeys': True, 'version': '1.0.0', 'specialVersion': 32}

Here is more info, from https://www.reddit.com/r/ledgerwallet/comments/lf8bf9/are_the_discontinued_ledger_nano_1_and_ledger_hw1/gntl5ez/?utm_source=reddit&utm_medium=web2x&context=3

The issue occurs when this function is called:

outputData = client_ledger.finalizeInput(b'', 0, 0, changePath, bfh(rawTx))

=> b'e04aff0015058000002c80000000800000000000000100000000'

<= b''6985

=> b'e0460200260000000000000000000000000000000000058000002c80000000800000000000000100000000'

at that point device disconnects... Not sure exactly if it disconnects just after returning the error, or after receiving the next call (that does not get any answer)...

I see somewhere that error code 6985 means: Wrong Device Status - correct?

What would be the correct way to put it in the correct "status" before calling finalizeInput ? Maybe that's what is missing, somehow?

The call causing the 6985 error is this block:

params = []
params.extend(donglePath)
apdu.append(len(params))
apdu.extend(params)
response = self.dongle.exchange(bytearray(apdu))

with changePath = "44'/0'/0'/1/0" and donglePath derived from changePath:

donglePath = parse_bip32_path(changePath)

The parameter seems just correct in the call to the ledger, as you can see.

because of the error code returned, an Exception is raised by dongle.exchange, which is ignored in ledger.py (i checked that we pass in the Exception block):

        except Exception:
                      pass

Maybe this Ledger needs to be put it in the correct "status" before calling finalizeInput (or before setting the donglePath, which returns an error)?

Maybe that's what is missing, somehow?

Maybe the PIN needs to be sent again to set the ledger in the correct mode?

SomberNight commented 3 years ago

here is the version: {'compressedKeys': True, 'version': '1.0.0', 'specialVersion': 32}

That is a very old version. It is almost certainly the cause of the issue. The old firmware probably does not support the protocol we are using yet.

Your best bet would be to upgrade the firmware somehow. Barring that, you could look at the git blame and patch the code to make the library (btchip-python) method calls more similar to what the old code was doing. E.g. the old firmware might not support the alternateEncoding branch added here: https://github.com/LedgerHQ/btchip-python/commit/c013597dcf3fca1604cd48ebf18a05c52e3b2a68 which the ledger.py plugin code now uses

Maybe try this patch:

diff --git a/electrum/plugins/ledger/ledger.py b/electrum/plugins/ledger/ledger.py
index c6797e9231..f2a3210990 100644
--- a/electrum/plugins/ledger/ledger.py
+++ b/electrum/plugins/ledger/ledger.py
@@ -480,9 +480,7 @@ class Ledger_KeyStore(Hardware_KeyStore):
             if segwitTransaction:
                 client_ledger.startUntrustedTransaction(True, inputIndex,
                                                             chipInputs, redeemScripts[inputIndex], version=tx.version)
-                # we don't set meaningful outputAddress, amount and fees
-                # as we only care about the alternateEncoding==True branch
-                outputData = client_ledger.finalizeInput(b'', 0, 0, changePath, bfh(rawTx))
+                outputData = client_ledger.finalizeInputFull(txOutput)
                 outputData['outputData'] = txOutput
                 if outputData['confirmationNeeded']:
                     outputData['address'] = output
@@ -507,9 +505,7 @@ class Ledger_KeyStore(Hardware_KeyStore):
                 while inputIndex < len(inputs):
                     client_ledger.startUntrustedTransaction(firstTransaction, inputIndex,
                                                                 chipInputs, redeemScripts[inputIndex], version=tx.version)
-                    # we don't set meaningful outputAddress, amount and fees
-                    # as we only care about the alternateEncoding==True branch
-                    outputData = client_ledger.finalizeInput(b'', 0, 0, changePath, bfh(rawTx))
+                    outputData = client_ledger.finalizeInputFull(txOutput)
                     outputData['outputData'] = txOutput
                     if outputData['confirmationNeeded']:
                         outputData['address'] = output

Why are you still using a Ledger HW.1 anyway?

loupiote commented 3 years ago

Thanks!

So maybe we could even test the HW1 firmware version and use the old API conditionally?

Why are you still using a Ledger HW.1 anyway?

I am working on a recovery for a client who lost their seed, and have a large sum in BTC still controlled by this very old ledger device, for which they still have the PIN. Upgrading the firmware is not possible on this device.

I'll try your suggestion. But in that case, since the "change" path is not specified, will the "change" always be sent to one of the input address?

SomberNight commented 3 years ago

So maybe we could even test the HW1 firmware version and use the old API conditionally?

Maybe... if we fully understood what is going on. It would be better if someone who does suggested a patch.

I'll try your suggestion. But in that case, since the "change" path is not specified, will the "change" always be sent to one of the input address?

Given your firmware is very old, this patch might only work if there is just a single output (no change). I don't know. I cannot test. As you are just rescuing funds, that should be sufficient.

Again, feel free to look around using git blame.

loupiote commented 3 years ago

Thanks! I will definitely look at the Tx before sending it to the network. I may be able to test using an old HW1 ledger that I should receive in a few days. I asked btchip to confirm whether they think it could work.

Based on comments in btchip.py, it looks like the old API must be used "before firmware 1.0.2", i.e. 1.0.0 and 1.0.1 presumably...

loupiote commented 3 years ago

So we did the test using exactly the modified code according to https://github.com/spesmilo/electrum/issues/7022#issuecomment-781024756 We now go through:

    def finalizeInputFull(self, outputData):
        result = {}
        offset = 0
        encryptedOutputData = b""
        while (offset < len(outputData)):
            blockLength = self.scriptBlockLength
            if ((offset + blockLength) < len(outputData)):
                dataLength = blockLength
                p1 = 0x00
            else:
                dataLength = len(outputData) - offset
                p1 = 0x80
            apdu = [ self.BTCHIP_CLA, self.BTCHIP_INS_HASH_INPUT_FINALIZE_FULL, \
            p1, 0x00, dataLength ]
            apdu.extend(outputData[offset : offset + dataLength])
            response = self.dongle.exchange(bytearray(apdu))
            encryptedOutputData = encryptedOutputData + response[1 : 1 + response[0]]
            offset += dataLength

We still get an error: => b'e04a80004502a0860100000000001976a914f814983680610d9833cb00581c6aefb4a7c066de88ac04fc7700000000001976a914d732eccc8898cd964d033e23e7432ed298de29d688ac' <= b''6985

Due to the error an exception is thrown by self.dongle.exchange(...), and since the block finalizeInputFull has no "try", the exception get caught higher by Electrum. So, it still does not work. Any idea? Is there something else important that could have changed in the ledger.py plugin around the "btchip.py commit on Jun 14, 2015 " ?

SomberNight commented 3 years ago

Try using an old version of Electrum. E.g. 2.7.x, just to create and sign the tx, and then broadcast it elsewhere (or using new version).

loupiote commented 3 years ago

Try using an old version of Electrum. E.g. 2.7.x, just to create and sign the tx, and then broadcast it elsewhere (or using new version).

Yeah, but I would have to hard-code a lot of things, like the account balances etc, so it would probably not be fun.

Maybe recovering the ledger.py and the btchip.py (and its dependencies) from may 2015 could still work in the current Electrum, if the Electrum plugin API has not changed too much?

grocid commented 3 years ago

@loupiote Did you solve this problem? I have an old Ledger HW1 with an almost as old firmware:

>> wallet.keystore.get_client().getFirmwareVersion()
{'compressedKeys': True, 'version': '1.0.1', 'specialVersion': 32}

I have tried using several versions of Electrum and I get the read error problem too.

loupiote commented 3 years ago

@loupiote Did you solve this problem? I have an old Ledger HW1 with an almost as old firmware:

See https://www.reddit.com/r/ledgerwallet/comments/m4pk7q/successful_recovery_of_btc_from_a_hw1_ledger/

loupiote commented 3 years ago

@grocid

{'compressedKeys': True, 'version': '1.0.1', 'specialVersion': 32} I have tried using several versions of Electrum and I get the read error problem too.

FYI We are able to recover from those old ledger (older than 1.0.2), that are unsupported by Electrum.

ericlau792 commented 3 years ago

@loupiote I have the same problem than your customer and saw on Reddit you solved the problem. I will gladly exchange with you to recover my old mBTC from my HW.1 (1.0.1). Regards Eric

SomberNight commented 3 years ago

Maybe consider contributing code to the project to fix the issue... But I guess that might not help your business model of helping users privately :)

ericlau792 commented 3 years ago

Hi,

i would like to, but I have no solution... I'm asking loupiote who succeded...

Eric

------ Message d'origine ------ De: "ghost43" @.> À: "spesmilo/electrum" @.> Cc : "ericlau792" @.>; "Comment" @.> Envoyé : 21/07/2021 15:39:36 Objet : Re: [spesmilo/electrum] The Ledger plugin is currently broken for old Ledgers HW1 (can get account addresses, but Electrum hanging when signing a Tx) (#7022)

Maybe consider contributing code to the project to fix the issue... But I guess that might not help your business model of helping users privately :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/spesmilo/electrum/issues/7022#issuecomment-884197863, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALDGMBAAFHV6N2GEKQQCQ3DTY3EZRANCNFSM4XPXCEFA.

SomberNight commented 3 years ago

Right, my comment was aimed at them :)

loupiote commented 3 years ago

@SomberNight

Maybe consider contributing code to the project to fix the issue... But I guess that might not help your business model of helping users privately :)

Not only it would not help our business model, but our recovery code is very unsafe for public consumption since it is a hack that is very poorly tested, and almost impossible to test due to the scarcity of those antiquated ledger devices with the very early firmware. The raw transactions that we generate for the recovery must be decompiled and manually checked, before being manually sent to the Bitcoin network. If people were using this code, many would likely lose large amount of BTC, and may turn against the author of the code.

@ericlau792 contact us privately via loupiote [at] gmail

raghavkashi commented 2 years ago

Please advice , what is the resolution for this issue? I am not a developer, rather an end user and struggling to send BTC out of my Electrum wallet. The electrum wallet is configured to use the Ledger X BTC app. I am trying to send BTC to an address starting with "3"

loupiote commented 2 years ago

@raghavkashi If you are using a Ledger Nano X, your issue does not have anything to do with the issue discussed in this thread., which is about Ledger HW-1.

In addition, you can also use Ledger Live to get access to your segwit account (address starting with "3"). You don't need to use Electrum. Also, Electrum does not have any issue communicating with ledger Nano X and S (but in some cases you may have to run Electrum in Administrator mode). Feel free to contact us privately via loupiote [at] gmail.