chaintope / bitcoinrb

The implementation of the bitcoin protocol for ruby.
MIT License
61 stars 20 forks source link

Bitcoin::MessageVerify.verify_message always returns true #73

Open ybce opened 1 month ago

ybce commented 1 month ago

Hello,

I am trying to verify a signed message using the MessageVerify module but I'm running into some strange behaviour.

I have a signature from the demo app located here: https://developer.onekey.so/connect-to-software/webapp-connect-onekey/btc/api-reference/signmessage This gives me a signature in base64: IAheSKap+fzPF7Ny+21r87GqcyNSAwCG+pCmpE9gUBT7ANHuQEnk3zBsvQI27a1bknE8JI5ZyFvVksIJVZd1NxQ=

And when I try verifying the signature using the address and the plain text message I get true back: Bitcoin::MessageSign.verify_message(address, signature, "010203") \\true

But swapping the signature or message for anything random always returns true, I tried with mainnet and signet and both give me the same result, returning true for any signature/message

Is there something else I'm missing here that should be setup? I'm not too familiar with Bitcoin message signing and verification so I apologize in advance if I'm missing something obvious here.

azuchi commented 1 month ago

I tried running the following and it came out false:

signature = "IAheSKap+fzPF7Ny+21r87GqcyNSAwCG+pCmpE9gUBT7ANHuQEnk3zBsvQI27a1bknE8JI5ZyFvVksIJVZd1NxQ="
addr = "191arn68nSLRiNJXD8srnmw4bRykBkVv6o"
msg = "010203"
Bitcoin::MessageSign.verify_message(addr, signature, msg)

And when I try verifying the signature using the address and the plain text message I get true back: Bitcoin::MessageSign.verify_message(address, signature, "010203") \true

Can you provide the value of address in this case?

ybce commented 1 month ago

Yes the address I was using was this: tb1prgrtw5sag3gsxrafu2f36dpzuyfpf7lhaea5zvk8f0rtuepqtepqsma6za (taproot on signet) I had to set Bitcoin.chain_params = :signet before calling verify_message or else the parse_from_addr function would throw an error.

I just tried everything again with a mainnet address and got the same issue happening:

Bitcoin.chain_params = :mainnet
address = "bc1q54xd8kn6qayzqeyxmltsfyagjjaftayr3tem7s" // Native Segwit Address
signature = "IAheSKap+fzPF7Ny+21r87GqcyNSAwCG+pCmpE9gUBT7ANHuQEnk3zBsvQI27a1bknE8JI5ZyFvVksIJVZd1NxQ="

Bitcoin::MessageSign.verify_message(address, signature, "010203") \\ true
Bitcoin::MessageSign.verify_message(address, signature, "12345") \\ true

Edit:

Okay I tried with a "Legacy" address according to OneKey which start with 1 like in your example and it worked fine, does this mean this is the only address type that the library supports for message verification?

azuchi commented 1 month ago

@ybce There was a bug in the interpreter flag settings when checking BIP322. I fixed it, please use the fixed version v1.7.0.

ybce commented 4 weeks ago

Thank you for the quick fix, I'll test out the new version today

ybce commented 3 weeks ago

@azuchi I tried this again and now it always returns false

Bitcoin.chain_params = :mainnet
address = "bc1q54xd8kn6qayzqeyxmltsfyagjjaftayr3tem7s" // Native Segwit Address
signature = "IAheSKap+fzPF7Ny+21r87GqcyNSAwCG+pCmpE9gUBT7ANHuQEnk3zBsvQI27a1bknE8JI5ZyFvVksIJVZd1NxQ="

Bitcoin::MessageSign.verify_message(address, signature, "010203") \\ false
Bitcoin::MessageSign.verify_message(address, signature, "12345") \\ false
azuchi commented 3 weeks ago

@ybce How did you create this signature? Regarding native segwit, the library currently only supports simple scheme in BIP322.

ybce commented 3 weeks ago

I created it using the example app here in the OneKey documentation: https://developer.onekey.so/connect-to-software/webapp-connect-onekey/btc/api-reference/signmessage

Thanks for the clarification

If I try anything other than a legacy address it doesn't work. Can you provide an example of an address that's has simple scheme in BIP-322?

Thanks again for your help

azuchi commented 3 weeks ago

A signature for the BIP322 simple scheme can be generated using this library as follows:

key = Bitcoin::Key.generate
address = key.to_p2wpkh
message = "010203"
signature = Bitcoin::MessageSign.sign_message(
  key, message, address: address, format: Bitcoin::MessageSign::FORMAT_SIMPLE
)

It seems that OneKey also supports bip322-simple as a type. In BIP-322, a simple signature is defined as a witness stack, but your signature "IAheSKap+fzPF7Ny+21r87GqcyNSAwCG+pCmpE9gUBT7ANHuQEnk3zBsvQI27a1bknE8JI5ZyFvVksIJVZd1NxQ=" appears to have less data than a P2WPKH witness stack (public key and signature).

azuchi commented 3 weeks ago

IAheSKap+fzPF7Ny+21r87GqcyNSAwCG+pCmpE9gUBT7ANHuQEnk3zBsvQI27a1bknE8JI5ZyFvVksIJVZd1NxQ=

This signature appears to be a legacy P2PKH signature. Therefore, when verifying this signature, you must use the legacy address (P2PKH) instead of the Segwit address.

If you want to create a signature for a segwit address, you need to generate the signature by specifying bip322-simple.

ybce commented 3 weeks ago

Thanks for the information @azuchi I'll look at this and see what we can do, we need to support multiple address types like taproot, native segwit and legacy on signet and mainne but OneKey doesn't really offer many options when signing messages which is the way we're generating these signatures