polkascan / py-substrate-interface

Python Substrate Interface
https://polkascan.github.io/py-substrate-interface/
Apache License 2.0
240 stars 114 forks source link

False negative verifying a message signed with polkadot{.js} #185

Closed Khazbs closed 2 years ago

Khazbs commented 2 years ago

The message is signed client-side with polkadot{.js} using the default cryptography, and the signature is successfully verified with polkadot{.js}. However, when I try to verify the message server-side with py-substrate-interface, verification results with False.

The JS code used client-side to sign and verify the message:

const address = account.address;
const message = 'Of course I own this wallet!';
let signature;
try {
  ({signature} = await signRaw({
    address: address,
    data: util.stringToHex(message),
    type: 'bytes',
  }));
}
catch {
  return;
}
const publicKey = utilCrypto.decodeAddress(account.address);
const hexPublicKey = util.u8aToHex(publicKey);
const isValid = utilCrypto.signatureVerify(
  message,
  signature,
  hexPublicKey,
).isValid;
console.log('Is signature valid?', isValid); // true

Then, address, message and signature are POST-ed over to the server, where this code tries to verify the message:

import substrateinterface as sub

keypair = sub.Keypair(address, ss58_format=42)
print(keypair.verify(message, signature)) # False
arjanz commented 2 years ago

First I thought, maybe the message that you used is probably not an SCALE-encoded bytes (because a string in Substrate is in fact a Vec<u8> so proceeded with the length of the string):

keypair = Keypair.create_from_uri('//Alice', crypto_type=KeypairType.SR25519)
message_obj = substrate.create_scale_object("Bytes")
scale_data = message_obj.encode("Of course I own this wallet!")

# copied from Polkadot.JS/apps
signature = bytes.fromhex("f094900ab85cf4d1b243092b08e078492d66c45b5a4d6cf377ee843a8fe14b7bedbcd825484feb1ef31b6fc79ef2bd345114c4c57771ecf4886be73b2516c08e")
# # Generate signature 
# signature = keypair.sign(scale_data)

verified = keypair.verify(scale_data, signature)

However when I tested this out, it indeed somehow didn't verify and also vice-versa.. So there is somehow an inconsistency between the signing and verification process, maybe the message format changed that Polkadot.JS uses but not sure. I can remember it used to work in the past.

Because the signing is used to create extrinsic signatures and Substrate is successfully verifying those, so I have to look into it more to figure out why..

Khazbs commented 2 years ago

Thank you! Looking forward to any news.

metakeller commented 2 years ago

Yeah, we also need this update. Would be so thankful to get it!

When do u think you can finish it? 🙏

arjanz commented 2 years ago

It seems the issue resides in the RUST bindings of Sr25519. Signatures generated using Ed25519 cryptography in PolkadotJS verifies correctly with the Ed25519 bindings of the library. I'll link the issue to that repos

arjanz commented 2 years ago

I'm afraid I'm running out of ideas.. I confirmed the RUST bindings use the same SIGNING_CTX and verification function as in the Substrate repos: https://github.com/paritytech/substrate/blob/master/primitives/core/src/sr25519.rs#L564-L576 versus https://github.com/polkascan/py-sr25519-bindings/blob/master/src/lib.rs#L119-L130

And I even tried to use the old verify_simple_preaudit_deprecated as mentioned in https://github.com/paritytech/substrate/blob/master/primitives/core/src/sr25519.rs#L600-L609 but without success..

I will have to discuss this with @jacogr and hope he can give me some pointers.

jacogr commented 2 years ago

Where are you calling signRaw from? If the extension, it needs to be wrapped (it doesn’t sign as-is to ensure users cannot just sign transaction data via that interface)

The JS verify tests for both with and without wrapping.

Khazbs commented 2 years ago

Thank you all for contributing to solving this problem! I do call signRaw injected via the polkadot{.js} extension dapp, as instructed in this cookbook. Can you help me understand how exactly do I "wrap" it?

jacogr commented 2 years ago

The wrapping the extension applies is exposed here - https://github.com/polkadot-js/common/blob/master/packages/util/src/u8a/wrap.ts#L42

(In the JS libs the verification then first tries the data as-is and then the wrapped version before failing the signature)

TL;DR The extension signs <Bytes>...all data in here...</Bytes> (assuming it (a) is not already wrapped with <Bytes> or (b) it is not an Ethereum-style wrapped message, aka Frontier-like chains)

arjanz commented 2 years ago

The wrapping the extension applies is exposed here - https://github.com/polkadot-js/common/blob/master/packages/util/src/u8a/wrap.ts#L42

(In the JS libs the verification then first tries the data as-is and then the wrapped version before failing the signature)

TL;DR The extension signs <Bytes>...all data in here...</Bytes> (assuming it (a) is not already wrapped with <Bytes> or (b) it is not an Ethereum-style wrapped message, aka Frontier-like chains)

Ok that solved the mystery.. When I test with the message: b'<Bytes>Of course I own this wallet!</Bytes>' the signature totally verifies correctly. Thanks for pointing this out @jacogr

Khazbs commented 2 years ago

I can verify that wrapping the message around with <Bytes> and </Bytes> before verification works for me too. Thank you for your effort, gentlemen, case closed.

import substrateinterface as sub

keypair = sub.Keypair(address, ss58_format=42)
wrapped_message = '<Bytes>' + message + '</Bytes>'
print(keypair.verify(wrapped_message, signature)) # True
arjanz commented 2 years ago

Automatic retry verification with wrapping released in https://github.com/polkascan/py-substrate-interface/releases/tag/v1.2.0