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

Signmessage doesn't work with segwit addresses #10542

Open
karel-3d opened this issue Jun 6, 2017 · 46 comments

Comments

@karel-3d
Copy link
Contributor

commented Jun 6, 2017

Describe the issue

Signmessage doesn't work on "segwit addresses" (what BIP141/BIP49 calls "P2WPKH-nested-in-P2SH").

Steps to reproduce

On testnet (or on litecoin :)):

  1. NEW_ADDRESS=$(bitcoin-cli getnewaddress)
  2. SEGWIT_ADDRESS=$(bitcoin-cli addwitnessaddress $NEW_ADDRESS)
  3. bitcoin-cli signmessage $SEGWIT_ADDRESS "shiny"

Expected behaviour

Signature is printed

Actual behaviour

error code: -3
error message:
Address does not refer to key

What version of bitcoin-core are you using?

v0.14.1

self-compiled from github repo

Machine specs:

  • OS: Linux
  • CPU: can reproduce on x86 and x64 CPUs
@karel-3d

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2017

Looking at the code, it seems to me that verification will have the exact same problem. That is,

addr.GetKeyID(keyID)

will return false.

@achow101

This comment has been minimized.

Copy link
Member

commented Jun 6, 2017

The address is a p2sh address, it doesn't have an associated public or private key to sign and verify messages with.

@karel-3d

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2017

I don't understand

It does have associated private key - the same private key as the first address I get from bitcoin-cli getnewaddress. I was under the impression that is what addwitnessaddress does (as was explained to me here - https://bitcoin.stackexchange.com/questions/52961/can-i-get-the-wallet-in-bitcoind-to-use-p2sh-segwit-on-testnet )

@achow101

This comment has been minimized.

Copy link
Member

commented Jun 6, 2017

Yes, but it is still technically a P2SH address and is treated as such. The underlying script has a private key associated with it, but not the address itself.

@sipa

This comment has been minimized.

Copy link
Member

commented Jun 6, 2017

The confusion here comes from the ambiguitiy in whether an address is an identifier of a key, or a shorthand for a script.

In the time when there was only one type of addresses, this was an innocent confusion to have: every address was indeed a shorthand for a P2PKH script, but also uniquely identified a private/public keypair. This is exploited in the signmessage command. It works with keys, not addresses, but uses addresses to refer to these keys.

Since P2SH, and certainly now with P2WSH/P2WPKH, this no longer works. You can't sign with an arbitrary P2SH address - even if you have the key for it - since the receiver wouldn't have the public key to verify with.

@luke-jr

This comment has been minimized.

Copy link
Member

commented Jun 6, 2017

IMO it is intentional that the old sign/verify message does not work with P2SH/newer addresses. If someone wants to do message signing as a non-obsolete thing, I would recommend writing a BIP that specifies how to handle it for newer address formats, and fixes the confusion surrounding it (I have seen people try to use signed messages to prove they sent bitcoins, which the current stuff does NOT prove.)

In the meantime, perhaps verifymessage ought to throw an error rather than returning false?

@karel-3d

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2017

You can't sign with an arbitrary P2SH address

But you can sight with your P2SH address that you have private key to, right?

And similarly, if the receiver knows that the address is "nested witness address", he can check whether a given public key belongs to that address.

I am not talking about arbitrary P2SH addresses, but really when the receiver knows it's "nested witness" address.

@achow101

This comment has been minimized.

Copy link
Member

commented Jun 6, 2017

But you can sight with your P2SH address that you have private key to, right?

No, you can't.

And similarly, if the receiver knows that the address is "nested witness address", he can check whether a given public key belongs to that address.

I am not talking about arbitrary P2SH addresses, but really when the receiver knows it's "nested witness" address.

There is no way for the software that is verifying the signature to know whether the public key corresponds to a nested-p2wpkh address. It simply does not know what p2sh address it would correspond to since a public key can be a part of multiple p2sh addresses. The reason it works for p2pkh is because a public key can only correspond to one p2pkh address.

@karel-3d

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2017

Oh. I thought (pubkey) -> (nested-p2wpkh address) function is deterministic, based on BIP 141.

@sipa

This comment has been minimized.

Copy link
Member

commented Jun 6, 2017

Oh. I thought (pubkey) -> (nested-p2wpkh address) function is deterministic, based on BIP 141.

It is.

And it would in theory be possible to make signmessage work for a P2SH-P2WPKH address, in cases where the verifier knows the embedded pubkeyhash already. But in that case you don't need "sign with a witness address" functionality - you could just sign with the embedded key (see validateaddress), and have the verifier check that.

The point is to not further the misunderstanding that signmessage signs with an address - it never did. It signs with a keyhash, and verify with a keyhash.

@karel-3d

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2017

So, in theory, the current behaviour could be "emulated" by using some different format, where the address would have to be P2SH-PWPKH and the pubkeyhash would be the part of the signature somehow.

However, that would need some new standards for the new types of addresses, as @luke-jr noted. And it's a question if it's worth it.

Got it. Thanks for the comments. Should I close the issue now?

@karel-3d

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2017

nope I still misunderstood all the formats, sorry. It could still be possible now I think.

Since you have to recover pubkey from the signature right now, you can go to pubkeyhash and scripthash, given you somehow know the script format (in this case, p2sh-p2wpkh).

The signer would sign the message with his private key; the verifier would try that the pubkey in the signature generates the given address through p2sh, which would mean the signature is valid.

The only problem I can think of is that you are then proving both the ownership of the p2pkh and the p2s-p2wpkh addresses, but it doesn't really matter.

@gmaxwell

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2017

We constructed a new kind of signmessage for elements which is conceptually a lot better and supports arbitrary scripts-- but it immediately runs into a problem that softfork semantics only work within the context of a consensus network... it's not clear how to handle them.

E.g. if a new softfork is defined on bitcoin and you make a pubkey that uses it... old verifiers need to return indeterminate-- before segwit script versioning this was intractable but it could be done now.

@karel-3d

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2017

just for interest - our users (Trezor) are asking for this feature (on Litecoin addresses) and we will maybe have to write some custom format for this

see discussion here

trezor/trezor-mcu#169

@prusnak

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2017

trezor/trezor-mcu#169

TL;DR: As a quick solution we are thinking about using introducing more v values to distinguish among various address formats:

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

@karel-3d

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2017

sign/verify for p2wpkh-in-p2sh is now implemented in trezor firmware (only in code now). See the linked discussion.

@Sjors

This comment has been minimized.

Copy link
Member

commented Dec 9, 2017

New dev mailinglist discussion here.

@jakubtrnka

This comment has been minimized.

Copy link

commented Dec 18, 2017

I'm currently working on it

@prusnak

This comment has been minimized.

Copy link
Contributor

commented Dec 18, 2017

@jakubtrnka working on what?

@karel-3d

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2017

@jakubtrnka are you working on BIP?

@sipa

This comment has been minimized.

Copy link
Member

commented Dec 18, 2017

@prusnak I'm totally fine with extending signmessage using just new header bytes as you're suggesting:

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

As SegWit keys are always compressed you only need 4 for each, indeed.

@jakubtrnka

This comment has been minimized.

Copy link

commented Dec 18, 2017

@karel-3d @prusnak At first I'm going to implement verification for P2SH encapsulated P2WPKH. If that works then also "signmessage". This is not a problem. ECDSA enables to recover public key from signature as described in § 4.1.6 . is actually very similar procedure of how "normal" message/address verification works, am I right? The public key is reconstructed from signature, hashed and compared. Right now the only way I found to verify such signatures is in Trezor web wallet. I didn't find anyone else working on this.

@Willtech

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2018

@luke-jr In this case, the person puporting to offer some Bitcoin's for sale is a Bitcoin Core user. I can understand that the situation may be confusing if the wallet is with a web wallet provider, with address amalgamation and possibly multiple wallet accounts under one key set that you do not own.

Surely utxo's stay associated to the receiving address? Okay, once again with web wallets, utxo's on the receiving address may have no relationship to your balance but, once again, you do not own the priv keys.

I think that the whole idea of a web wallet is nice for people but I am yet to see an implementation that I like.

@luke-jr

This comment has been minimized.

Copy link
Member

commented Mar 13, 2018

The message-verifier cannot prove the message-signer is using any particular software. And it's not a good idea to rely on accidental/unintended behaviour anyway.

Addresses are intentionally explicitly opaque and disconnected from UTXOs and balances.

@Willtech

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2018

@luke-jr Okay, but then the message just should not verify if the same message signing rules are not used.

If I send 1BTC to {address} of mine then utxo does not move until I spend it. That is not opaque, unless you are once again referring to amalgamated web wallets. Utxo balances are distinct from wallet balances.

@luke-jr

This comment has been minimized.

Copy link
Member

commented Mar 13, 2018

@Willtech Core does not guarantee the behaviour you assume either. The message signing rules for signmessage guarantee only that the signer will receive coins sent to the address in question. It does not guarantee possession of a balance or UTXOs in any sense.

@Willtech

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2018

@luke-jr How do you sign without the priv key proving ownership of the address and, in the case of a full node wallet, ownership of the utxo's?

@sipa

This comment has been minimized.

Copy link
Member

commented Mar 13, 2018

Owning money and having access to private keys that can spend coins are independent concepts. Exchanges have private keys for UTXOs money they hold on behalf of their customers. They can clearly sign messages using those keys, but they certainly don't own all those coins.

@luke-jr

This comment has been minimized.

Copy link
Member

commented Mar 14, 2018

@Willtech I'm not sure what you're asking. There is no mechanism that supports proving present ownership of UTXOs/coins (for messages, anyway).

@Willtech

This comment has been minimized.

Copy link
Contributor

commented Mar 14, 2018

Well yes, but I am really only speaking about Bitcoin Core as in the use case. That's where we are after all.

jakubtrnka pushed a commit to jakubtrnka/bitcoin that referenced this issue Mar 24, 2018
Jakub Trnka
bitcoin-cli verifymessage was extended to verify signatures against both
bech32 and segwit-p2sh. Class PubkeyConsistencyVisitor has been added
which takes extracted public key as constructor argument and verifies
signatures against all meaningful CTxDestinations.

Signmessage will be implemented soon.
jakubtrnka pushed a commit to jakubtrnka/bitcoin that referenced this issue Mar 24, 2018
Jakub Trnka
Tabs have been replaced by spaces
@jakubtrnka

This comment has been minimized.

Copy link

commented Mar 26, 2018

@luke-jr Why would we need to prove ownership of specific UTXO? Neither do I understand how you mean that signature does not prove ownership of keys. I thought the whole idea of message signing is to prove the ability to spend UTXOs associated with specific keys, therefore key hashes, therefore addresses. So why not to allow to verify message-signatures associated with those public keys if it's possible given whatever address?

@dooglus

This comment has been minimized.

Copy link
Contributor

commented Jan 14, 2019

When attempting to sign a message with a bech32 address, Bitcoin Core complains that the address doesn't refer to a key. But it does. As a result I am unable to verify signatures made by users of the Electrum wallet (which can sign using bech32 addresses). I heard the Trezor also has similar functionality.

I made the following change to allow signing and verifying messages with bech32 addresses in Core. It appears to be compatible with Electrum's signatures.

diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
index 18c867bff..a8e75ddc7 100644
--- a/src/rpc/misc.cpp
+++ b/src/rpc/misc.cpp
@@ -179,7 +179,11 @@ static UniValue verifymessage(const JSONRPCRequest& request)
 
     const CKeyID *keyID = boost::get<CKeyID>(&destination);
     if (!keyID) {
-        throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
+        const WitnessV0KeyHash *witnessV0KeyHash = boost::get<WitnessV0KeyHash>(&destination);
+        if (!witnessV0KeyHash) {
+            throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
+        }
+        keyID = (CKeyID *)(witnessV0KeyHash);
     }
 
     bool fInvalid = false;
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 38397a394..a01b17ce8 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -540,7 +540,11 @@ static UniValue signmessage(const JSONRPCRequest& request)
 
     const CKeyID *keyID = boost::get<CKeyID>(&dest);
     if (!keyID) {
-        throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
+        const WitnessV0KeyHash *witnessV0KeyHash = boost::get<WitnessV0KeyHash>(&dest);
+        if (!witnessV0KeyHash) {
+            throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
+        }
+        keyID = (CKeyID *)(witnessV0KeyHash);
     }
 
     CKey key;
@ripper234

This comment has been minimized.

Copy link

commented Aug 9, 2019

Hi all,

Any updates on this issue? It's been 8 months since the last comment.

@dooglus

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

If you don't like my fix in the comment immediately before yours, you can always take a two-step approach. First get the privkey from the address, and then sign your message using the privkey.

1. dumpprivkey "address"
2. signmessagewithprivkey "privkey" "message"

If this is something you need to do frequently I'd recommend scripting it. Something like this perhaps:

signmessage() {
    if [[ $# != 2 ]]; then
        echo "usage: signmessage <address> <message>" 1>&2; return
    fi

    address=$1; message=$2

    privkey=$(bitcoin-cli dumpprivkey $address)
    if (($?)); then
        echo "error getting privkey" 1>&2; return
    fi

    bitcoin-cli signmessagewithprivkey $privkey "$message"
    if (($?)); then
        echo "error signing message" 1>&2; return
    fi
}
@Willtech

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2019

It seems that some believe the use case for sign message is discredited and intend that nothing will happen, the feature will eventually be removed. FWIW I disagree.

@sipa

This comment has been minimized.

Copy link
Member

commented Aug 23, 2019

See #16440.

@wspi

This comment has been minimized.

Copy link

commented Sep 2, 2019

If you don't like my fix in the comment immediately before yours, you can always take a two-step approach. First get the privkey from the address, and then sign your message using the privkey.

1. dumpprivkey "address"
2. signmessagewithprivkey "privkey" "message"

If this is something you need to do frequently I'd recommend scripting it. Something like this perhaps:

signmessage() {
    if [[ $# != 2 ]]; then
        echo "usage: signmessage <address> <message>" 1>&2; return
    fi

    address=$1; message=$2

    privkey=$(bitcoin-cli dumpprivkey $address)
    if (($?)); then
        echo "error getting privkey" 1>&2; return
    fi

    bitcoin-cli signmessagewithprivkey $privkey "$message"
    if (($?)); then
        echo "error signing message" 1>&2; return
    fi
}

@dooglus when you sign with the private key (signmessagewithprivkey), how do you verify it?

verifymessage is not able to verify it

@dooglus

This comment has been minimized.

Copy link
Contributor

commented Sep 2, 2019

@wspi I use this patch to verify it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.