BitBoxSwiss / mcu

MCU code for the BitBox01 hardware wallet
https://shiftcrypto.ch
Other
63 stars 50 forks source link

Digital Bitbox fw 6.x fails to sign transactions with pubkeys on certain derivation paths. Multisig UTXOs unspendable #268

Closed JeffVandrewJr closed 5 years ago

JeffVandrewJr commented 5 years ago

The most recent firmware update has caused havoc on BitBoxes used for multisig.

It's a particularly hairy issue as existing multisig UTXOs have become unspendable absent manually exporting a private key.

Full details are here: https://github.com/spesmilo/electrum/issues/5275

benma commented 5 years ago

Hi @JeffVandrewJr

Thanks a lot for reaching out. We are aware of the issue and are working on releasing a fix. Sincere apologies for the inconveniences caused in the meantime!

SomberNight commented 5 years ago

I would be interested to know why this limitation was introduced in the first place. I can fully understand a limitation on the path to the change output, but not a limitation on the path to the input.

Somewhat unfortunately (in this context), Electrum allows (and has allowed) the user to choose basically any derivation path, so you cannot really just impose arbitrary restrictions now. :/

One thing you could do is to remove restrictions on input derivation path, and make the change path restriction "relative". E.g. change must be sent to either

KasparEtter commented 5 years ago

Hi @SomberNight, I'm happy to answer your questions. We started to enforce restrictions on the change keypath because it is chosen by the desktop app and is thus in the control of the attacker if your computer is compromised. As it turned out, we were overly restrictive by enforcing five derivation levels (with the first three having to match the keypath of the inputs). We are currently working on making this more flexible by allowing an arbitrary number of derivation levels up to six and simply enforce that the keypath prefix of the change up to the depth minus two (i.e. without the change and address index levels) matches the keypath of the inputs (and that they all have the same number of levels). As far as we can judge, this should solve the multisig with Electrum problem.

SomberNight commented 5 years ago

enforce restrictions on the change keypath because it is chosen by the desktop app and is thus in the control of the attacker if your computer is compromised

Yes, that makes sense, I thought that was the case, that explains the restriction on the change output.

What I am not sure about is why there are restrictions on the derivation path to the input. What could a compromised host do? (I understand the change output scenarios such as hiding the owner's coins on random paths)

allowing an arbitrary number of derivation levels up to six and simply enforce that the keypath prefix of the change up to the depth minus two (i.e. without the change and address index levels) matches the keypath of the inputs (and that they all have the same number of levels).

Sounds reasonable, I guess that fits my a path "similar" to the derivation path of at least one of the inputs idea :)

What about the restriction on the path to the input? Is that restriction currently for "the path to have 5 levels"? Are you changing that to "path to have at most 6 levels"? Just inferring from your change path description.

KasparEtter commented 5 years ago

What I am not sure about is why there are restrictions on the derivation path to the input. What could a compromised host do?

Sorry that I was not explicit on this. There is no reason to enforce restrictions on the keypaths of the inputs. However, since transactions typically have a change, we did not deem it necessary to treat transactions without a change differently (and simply enforced these restrictions on all keypaths).

I guess that fits my a path "similar" to the derivation path of at least one of the inputs idea

It does. 🙂 It is also more in line with what Trezor does. However, since the BitBox does not verify the transaction (this is left to the mobile verification app) and simply signs various hashes at various keypaths, we will continue to enforce that all inputs/utxos share the same keypath prefix up to the last two levels. If we only require that the change matches the keypath of at least one of the inputs, then an attacker could also request a signature at an arbitrary keypath, send the change "there" and simply discard/ignore the returned signature.

And yes, we will also make the keypath depth of the inputs more flexible again. In order to allocate memory, we have to cap the number of levels. Our patch under review made this configurable (before compilation) and set the value to 6. However, nothing speaks against increasing this value a bit more to, for example, 10, which we might do.

SomberNight commented 5 years ago

All right, thank you for explaining. I believe your described fix should be sufficient to restore previous functionality to Electrum.

JeffVandrewJr commented 5 years ago

@KasparEtter @benma Any ETA on the patch being released? I hate to be a nag; I'm asking only because the BitBoxes I bought are unusuable for my intended use in the meantime.

KasparEtter commented 5 years ago

Hi @JeffVandrewJr, please excuse the delay and the inconvenience. The patch is now ready to be signed and should be released tomorrow on Thursday or at the latest on Friday.

JeffVandrewJr commented 5 years ago

@KasparEtter Thanks man!

KasparEtter commented 5 years ago

Here is the signed firmware that should solve your issue with multisig in Electrum: https://github.com/digitalbitbox/mcu/releases/tag/v6.0.4

Please contact us if you still have problems.

In case you can't update the firmware via the command line with our Python script, we will also release a new version of the desktop app with the new firmware this week. Once the app is out as well, I will close this issue.

KasparEtter commented 5 years ago

And here is the desktop app with the firmware 6.0.4 that you can use to update the firmware on your BitBox: https://github.com/digitalbitbox/bitbox-wallet-app/releases/tag/v4.7.0

Thanks once more for bringing this to our attention!

TankPi commented 5 years ago

I assume I have a problem related to this issue, my setup:

Electrum p2wsh wallet with 2 out of 4 multisig, where I use a bitbox01 as an co-signing device. I've upgraded from:

to:

Now when I try to sign a transaction I get this window:

Screenshot from 2019-08-10 16-37-38

i.e. my funds are stuck. I'm not sufficiently knowledgeable to judge if the issue is due to Electrum or the BitBox. Would be really nice if you could help me out.

KasparEtter commented 5 years ago

Thanks for the report. Can you tell us the keypath you are using? If you haven't done so already, you should contact us at support@shiftcrypto.ch.