Support external inputs

[andrewkozlik]

Implements #1052

This PR implements support for signing transactions with external inputs on Trezor. The external input must either be pre-signed or accompanied with a proof of (non-)ownership. This should work for all Bitcoinlike coins except Decred, which didn't seem worth the effort as it has a signing flow of its own.

@onvej please review SLIP-0019 spec compliance and security in general, namely in terms of changes to the signing flow and correctness of signature verification.
@matejcik please review the overall design and organization of the code, protobuf messages and changes to the signing flow from the perspective of the protocol.

Test coverage
For pre-signed inputs the test coverage should be fairly complete, but I am using transactions generated by Trezor, which means that they might not actually be fully valid, as we discussed. Nevertheless this still tests the relevant functionality, so it's not a problem, I am just not very happy about introducing more of these. For external inputs with proofs of ownership I implemented only two device tests and those are for P2WPKH. I am leaving the other script types for later when we decide on a general approach to constructing fully valid testing transactions. However, the other script types are covered by unit tests, so that should actually be sufficient.

UI

1. There is a new special dialog for transactions with external inputs, which shows the total amount being spent and the amount being contributed by the user, but it doesn't show the fee. So the user will only get to see the fee on Trezor if it's too high. I can add a separate screen showing the fee, but since any malicious fee bump would show up in the total amount and the user can see the fee and total amount in the desktop wallet, it seems redundant.
   
2. Technically it's possible for the user's contribution to show up as negative (if his change output is larger than the sum of his inputs). If that happens we might want to reformulate the dialog to say something like "You are receiving ... BTC" instead of "You are contributing ... BTC". Probably not important at the moment.
3. It would be nice if we could hide the other participant's change outputs from the output confirmation dialogs. I haven't given this much thought, but it seems to me that we could just let the client software mark them as external change outputs to hide them. Their amounts would get subtracted from the transaction total and we would only check that total >= spending. Seems rather risky though, needs further analysis.

Signature verification
There are several ways to go about implementing signature verification. The easy way would be to delegate the parsing of the witness/scriptSig to client software and then just call some pre-existing functions in Trezor to check the hashes. I chose to parse the scripts in the firmware, because it seems to me that it's not that much trouble and considering that in time we will probably be implementing PSBT in Trezor, this approach seemed more future-proof, because we would need to do it anyway.

Verifying validity of external inputs
The validity of external inputs can be verified before or after user confirmation. I chose to do it after confirmation.
Advantages:

• In case of a large number of inputs, this reduces the amount of time that the user has to wait for the confirmation screen to appear, because the signature verification and streaming of the previous transaction is shifted to after confirmation for external inputs. We should probably do it this way for all inputs, not just the external ones.

Disadvantages:

• There is one additional TxRequest call for each external input with a proof. This is needed anyway for pre-signed external inputs, but it could be avoided for external inputs with proof of non-ownership. I haven't measured how long the extra TxRequest call takes, but the current solution gives cleaner code, which I would generally prefer to a minor speedup.
• If the ownership proof or a signature or an amount in an external input is invalid, then the user will find out only after confirming the transaction. Under normal circumstances this should not happen, so the usability improvement outweighs this disadvantage.

Verifying all previous transactions after confirmation
I haven't implemented this, but am in favor of doing so, especially if we are unable to speed up the prev_tx streaming. The disadvantages are the same as above (mainly one additional TxRequest per input). Ideally we could even do the prev_tx verification in the background while the confirmation dialogs are showing up, which would require these kind of changes to the code anyway. All of this can be done in a separate task, but I wanted to bring it up here, given that it's closely related.

Changes to test vectors
You will notice that due to some refactoring around hash143_preimage_hash(), I had to regenerate the test vectors, because I couldn't figure out the preimage to the pubkey hashes. I used the firmware to regenerate them, so if anyone cares to check them with a 3rd party tool, you are welcome to do so.

[matejcik]

Disadvantages:

• If the ownership proof or a signature or an amount in an external input is invalid, then the user will find out only after confirming the transaction. Under normal circumstances this should not happen,
  so the usability improvement outweighs this disadvantage.

This seems significant in terms of UX: we are deliberately introducing a failure mode where the trusted device can say "please confirm -> signing ... -> signing failed!". This is almost exactly the thing that was exploited in the recent BIP-143 vulnerability.
(of course in that case the "failed" thing was shown on the host, but at the moment this is a small difference for the end-user)

OTOH if we implement the proposed "success" screen at the end of every flow, this becomes less significant.

Ideally we could even do the prev_tx verification in the background

This is a very interesting idea. It might interfere with Suite wrt #35 though: no good way to report user progress to the host while streaming data.
But let's keep it in mind.

[andrewkozlik]

This seems significant in terms of UX: we are deliberately introducing a failure mode where the trusted device can say "please confirm -> signing ... -> signing failed!". This is almost exactly the thing that was exploited in the recent BIP-143 vulnerability.

I don't see a problem here. Let's assume we would have an error dialog for failed signing on Trezor and an attacker used the new code to invoke this dialog after user confirmation. Since the signing takes place after step4_verify_external_inputs(), he would actually interrupt the flow before having a chance to get his hands on any signatures. So the new code wouldn't be a problem in that scenario, but rather the already existing code in step6_serialize_outputs() or step7_sign_segwit_inputs() could be used to invoke a failure by sending an invalid response or simply cutting off communication at any time.

[matejcik]

partial review, i'll still need to go back to the Bitcoin and Verifier classes

[matejcik]

I don't see a problem here. Let's assume we would have an error dialog for failed signing on Trezor and an attacker used the new code to invoke this dialog after user confirmation. Since the signing takes place after step4_verify_external_inputs(), he would actually interrupt the flow before having a chance to get his hands on any signatures. So the new code wouldn't be a problem in that scenario, but rather the already existing code in step6_serialize_outputs() or step7_sign_segwit_inputs() could be used to invoke a failure by sending an invalid response or simply cutting off communication at any time.

I'm not claiming this is exploitable on the technical level.
I am claiming that introducing a failure mode of "the device can let you confirm and then change its mind and say it failed", makes it easier for the host to claim that this has happened.

[matejcik]

Verifying all previous transactions after confirmation
I haven't implemented this, but am in favor of doing so, especially if we are unable to speed up the prev_tx streaming. The disadvantages are the same as above (mainly one additional TxRequest per input). Ideally we could even do the prev_tx verification in the background while the confirmation dialogs are showing up, which would require these kind of changes to the code anyway. All of this can be done in a separate task, but I wanted to bring it up here, given that it's closely related.

There is also the fact that we would require txi.amount to always be set. I'm in favor of that, actually, but it will probably break at least some 3rd parties.

[matejcik]

i think this is all from me 🎉

[onvej-sl]

It seems to be OK in terms of security and compliance.

I added a few comments mostly suggesting better naming.

[onvej-sl]

I consider the "total amount" misleading since it includes external change outputs but not your change output. I can's see any benefit in showing such an amount.

[andrewkozlik]

It's the sum of the outputs that the user confirmed, plus the fee. Including the user's change output wouldn't make any sense. It would just confuse the user, because the number would look totally random to him given that his change output is hidden from the confirmation process. An alternative I was considering is below. But what I'd be most interested in is investigating the possibility of hiding the external change outputs as I explained above. It's up for discussion. I copied it into the CoinJoin document we were looking at yesterday so that it doesn't get lost.

[onvej-sl]

Including the user's change output wouldn't make any sense

I'm not saying it makes any sense.

I copied it into the CoinJoin document we were looking at yesterday so that it doesn't get lost.

Great.