SignMessage / VerifyMessage does not work correctly for Segwit addresses

[prusnak]

... because:

1) SignMessage specifies address by address_n - add segwit flag?

2) VerifyMessage specifies address by string - segwit prefix is not recognized

[karelbilek]

As of interest

bitcoin core does not support signing with segwit (what BIP141/BIP49 calls P2WPKH-nested-in-P2SH) addresses. The RPC just throw an error "Address does not refer to key".

I will ask bitcoin core if that is a feature or a bug.

edit: ...here

https://github.com/bitcoin/bitcoin/issues/10542

[jhoenicke]

SignMessage/VerifyMessage work on public keys. There is nothing in the signature that indicates, whether the public key should be encoded as p2pkh address, as p2sh, or p2wpkh (optionally over p2sh). I think nobody has made a standard for this.

We could easily change VerifyMessage in the firmware to detect p2wpkh over p2sh address or the besh32 p2wpkh address and accept them if they match the public key. For signing, the firmware doesn't need any change (unless someone defines a new format for segwit message signatures). The web wallet need to detect its segwit addresses, though.

[karelbilek]

I am not sure if you can get the pubkey hash from just the address, which is just scripthash.

From the discussion on the linked thread, it seems that you cannot, so we would somehow need to attach the pubkey hash.

Which complicates everything.

As luke notes in the other thread, someone should make a standard if this is to be supported on segwit addresses. Because we want to release the segwit wallet+firmware sooner than later, I suggest we just disable the sign/verify for segwit accounts for now until the standard is made, if it ever is.

[jhoenicke]

You can always go from pubkey to pubkey hash to script hash. The VerifyMessage call already has to recover the public key from the signature to check that its hash matches the address. Then going from there to the p2wpkh or p2sh address is possible.

[karelbilek]

Oh you are right. I was confused about that. So it's not that hard actually

[karelbilek]

Hm, I am trying to find what exactly signMessage does :D and how is the pubkey encoded there, right now, and how much could that change with segwit

[karelbilek]

Hm, when I am reading more, without change of the format for "segwit" p2sh, the signatures would not be very helpful.

An "evil" signer could use the same private key for both p2pkh and p2sh-p2wpkh addresses, since the address itself is not in the signature/message.

So the signature would be meaningless, since you are not sure for which address it actually is - you are not signing with the address, but with the private key. You could not do such a trick with trezor, but you could with desktop software. [edit] - actually yes you can do it also with trezor, with the same path you get the same privkey. :)

We would somehow need to add into the format that it is p2sh-p2wpkh or there could be confusion. I am not sure if it could be used for actual attacks though; if you own a p2pkh address with a given key, you also own the other one and vice versa.

[jhoenicke]

The signature consists of 1 byte value "v", 32 byte value "r", 32 byte value "s". r and s is the ECDSA signature. Q can be reconstructed using the ECSDA equation except that there are up to four possible values. Value v encodes which of the four values and whether it is compressed or uncompressed. It would be possible to extend v to also encode whether segwit p2wpkh or p2sh address should be used. Currently only eight different values for v are defined (ethereum uses different values for v for replay protection, though).

[prusnak]

I think we'll have to come with the Segwit signing scheme. There are more and more people asking about this and it would be good to have this ready for Bitcoin once Segwit is activated there.

Probably the best way would be to encode type of address into v. Let's start just with P2PKH and P2WPKH-in-P2SH, or do we want to include other options as well from the beginning?

[karelbilek]

We can send those people to bitcoin-core, there is already an issue for that :)

see https://github.com/bitcoin/bitcoin/issues/10542 (linked above already)

[jhoenicke]

If we want it soon, we should do something simple like encoding the address type in v. And then hope that others wallets will recognize these signatures in the future. Maybe we can ask electrum developers if they want to do something similar.

Current encoding:

• low two bit encode parity and overflow
• 27-30 uncompressed pubkey,
• 31-34 compressed pubkey.

My suggestion (segwit pubkeys are always compressed, so there is no need to distinguish compressed and uncompressed):

• 35-38 p2sh segwit pubkey (base58).
• 39-42 segwit pubkey (bech32).

Note that all these signatures are malleable, since v does not go into the hash. If you have a signature for somebody's segwit address, you can change it into a signature for the uncompressed or compressed 1... address, or even into the signature of the negative address. It's not an attack vector as I can even trivially change it into a signature of my own address ☺.

[prusnak]

35-38 p2sh segwit pubkey (base58) 39-42 segwit pubkey (bech32)

My thoughts exactly!

[prusnak]

We'll proceed by adding optional InputScriptType script_type; to SignMessage like we do in GetAddress. We won't need this field for VerifyMessage, because we can guess the address type like we do when providing address for output.

Correct?

[prusnak]

Merged to Master in https://github.com/trezor/trezor-mcu/commit/b5f9a5738f558002c3c41d67ff8230483a98047d (segwit-in-p2sh only)

Unit tests added in python-trezor here: https://github.com/trezor/python-trezor/commit/11b686a9f2b4cb3a1d53e50390c61b9e2c607fb2