Add support for message signing/verification with SegWit addresses (BIP 137)

[johnzweng]

Overview

• This PR adds support for signing and verifying text messages with Segwit addresses (more precise: with the private keys of the address).
• Additionally it adds a helper tool MessageVerifyUtils which makes it easier to verify addresses without the need for taking care which address type has been used to produce the signature.

Short general background info

(not related to this PR, omit if you 're familiar with the topic)

• signing/verifying of text messages with Bitcoin address keys simply works by hashing the message (with a specific prefix) and creating an ECDSA signature for the message hash
• the format (magic string prefifx Bitcoin Signed Message:) and serialisation before hashing (i.e. using VarInt for message length) was never formally specified in a BIP but instead de-facto standardised by the implementation in the Satoshi client (now Bitcoin Core)
• Unlike transaction signatures the signature of text messages are not using DER encoding, but instead just consist of the plain signature (32 byte R value, 32 byte S value, prefixed with a single header byte containing meta information about the signature -> details see below), serialised as base64 string
• the public key is not stored, as with ECDSA it can be recovered from message+signature
• in general, public key recovery from ECDSA signature+message will result in more than one possible public key, therefore one information in the signatures prefix-header byte is, which one of the possible recovered keys is the one which had actually been used for signing. This parameter is called recId in bitcoinJ, and can have a range from 0-3 (0=first recovered key with even y, 1=first recovered key with odd y, 2=second key with even y, 3=second key with odd y)
• the second piece of information stored in this signature prefix-header-byte is if the address used the compressed or uncompressed representation of the pubKey, so that the verifier ends up generating the same address
• to verify a signature for a message, a wallet recovers the public key from the signature, regenerates the corresponding Bitcoin address from this public key and checks if this address matches the specified address
• all this was before BIP 137
• in 2019, the only notable change that came with BIP 137 was that the signatures prefix-header-byte had been extended by some more allowed values to represent the two new address types: P2WPKH (native SegWit) and P2WPKH-wrapped-in-P2SH (legacy SegWit)

What was missing in bitcoinJ?

1. As the possible allowed values for this signature's header byte had been extended in BIP 137 (new range 27-42), it was no longer possible to use ECKey.signedMessageToKey() with signatures from SegWit addresses as it would fail when checking the signature's header byte for allowed values

2. Until now the verifyMessage(...) method resides in the ECKey class, which was ok, as long as all message verification operations consisted of just comparing two pubKeyHashes (the one from the given address and the one recovered from the signature). This is no longer true with P2SH-P2WPKH addresses, as here we don't have the plain pubKeyHash in the address but instead RIPEMD160(SHA256(0x00 0x14 <20-byte-pubKeyHash>)). So to verify messages signed with this type of address, we also need to do the same with the recovered pubKey from the signature (before we can compare it with the data contained in the address). So message signature verification is no longer script type agnostic, it needs to know the type of address, which has been used.

What I did in this PR:

• The first two commits simply extend ECKey.signedMessageToKey (…) to allow the new header byte values added with BIP 137
• then I added new signMessage(...) methods with a new parameter ScriptType which set the header byte value according to BIP 137 when creating new signatures in this commit.
• and to make it easy to verify messages, without the need of manually taking care of address type and recreating P2SH-P2WPKH-scriptPubKeys, I added a new Utility class MessageVerifyUtils.java

Out of scope:

BIP 322 defines a new format for signing messages, which promises to be backwards compatible to the above described legacy message signing format, but additionally will allow more sophisticated proofs (not just proof of key ownership), but this BIP is still a draft and to my knowledge not implemented by any wallet (there is only one open PR against Bitcoin Core). Therefore BIP 322 is out-of-scope of this PR.

I am happy about any feedback. 🙂 If I should structure anything different, please let me know. 🙂

[schildbach]

Thanks for your contribution! A quick glance looks great, especially the large amount of tests. For now my main comment is this:

On our master branch, ideally each commit should have one specific goal, and build successfully. This meant the accompanying unit tests should go with the changes that implement the logic to be tested.

Do some of your added tests also run on the existing codebase? We could merge them earlier than the rest.

[johnzweng]

Hi!

our master branch, ideally each commit should have one specific goal, and build successfully. This meant the accompanying unit tests should go with the changes that implement the logic to be tested.

Ok, I definitely didn't follow this rule. The first rule adds test which fail because of missing functionality, and then the next commit adds the missing functionality (new range for signature header byte).

I will restructure the commits so that each commit only contains only one functionality and its test.

And regarding tests and existing codebase: the large set of test I added is in the new test class MessageVerifyUtilsTest which tests the MessageVerifyUtils.verifyMessage(..) method from the newly added MessageVerifyUtils util class. So they won't work in this form with existing codebase.

Of course, theoretically we could reuse the content of some of these tests (at least the signatures not created from SegWit addresses) and include it as new tests in the existing ECKeyTest, but we would need to write new tests in there (and also this is no @RunWith(Parameterized.class) testclass).

[johnzweng]

Hi! I've restructured my changes now into 3 commits (the change itself stayed the same). Each commit now has now one single goal and should build and test successfully:

• first commit: add support for verifying messages signed with SegWit addresses
• second commit: add support for creating BIP 137 compatible signatures from SegWit addresses (marking old methods as Deprecated, but not removing them and not changing their behaviour)
• and last commit: introduce a new utility to hide complexity of verification (this might be debatable if needed, I would find it useful)

As the test cases all use the API of the added utility class for verification, the majority of testcases for now also is included in the 3rd commit.

I also tried to use meaningful commit messages.

[edit] I also rebased on current master. [/edit]

[schildbach]

Thanks again! I just merged this, with my only change being renaming SegWit to segwit.