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

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 this issue Feb 11, 2021 · 25 comments

Comments

@loupiote
Copy link

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
Copy link
Member

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 (b56fe23) 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.

self.logger.exception('')

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Author

loupiote commented Feb 17, 2021

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
}

@loupiote loupiote changed the title The Ledger plugin is currently broken for old Ledgers HW1 (can get account addresses, but "read error" when signing a Tx) The Ledger plugin is currently broken for old Ledgers HW1 (can get account addresses, but Electrum hanging when signing a Tx) Feb 17, 2021
@SomberNight
Copy link
Member

SomberNight commented Feb 17, 2021

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

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

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
Copy link
Author

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

Ok, I'll try, thanks!

@loupiote
Copy link
Author

loupiote commented Feb 18, 2021

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
Copy link
Member

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: LedgerHQ/btchip-python@c013597 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
Copy link
Author

loupiote commented Feb 18, 2021

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
Copy link
Member

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
Copy link
Author

loupiote commented Feb 18, 2021

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
Copy link
Author

loupiote commented Feb 20, 2021

So we did the test using exactly the modified code according to
#7022 (comment)
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
Copy link
Member

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
Copy link
Author

loupiote commented Feb 20, 2021

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
Copy link

grocid commented May 11, 2021

@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
Copy link
Author

@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
Copy link
Author

loupiote commented Jul 3, 2021

@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
Copy link

@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
Copy link
Member

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
Copy link

ericlau792 commented Jul 21, 2021 via email

@SomberNight
Copy link
Member

Right, my comment was aimed at them :)

@loupiote
Copy link
Author

@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
Copy link

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
Copy link
Author

loupiote commented Feb 8, 2022

@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.

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

No branches or pull requests

5 participants