failed to verify signatures generated by `polkadot-js`

[EclesioMeloJunior]

Description

Hello! I'm trying to use the repository https://github.com/w3f/schnorrkel, more specifically the function verify_simple to verify signatures generated by polkadot-js using thesignRaw function and unforunately it's not working. However the function signatureVerify from @polkadot/util-crypto verify correctly the same signature.

Steps to reproduce

1. I've used the polkadot extension and the polkadot-js to generate the signature

scren-record.mov

output at console.log 

Public Key (hex) 0xf84d048da2ddae2d9d8fd6763f469566e8817a26114f39408de15547f6d47805
Message => message to sign
Signature (hex) 0x48ce2c90e08651adfc8ecef84e916f6d1bb51ebebd16150ee12df247841a5437951ea0f9d632ca165e6ab391532e75e701be6a1caa88c8a6bcca3511f55b4183

2. With the https://github.com/w3f/schnorrkel cloned at my machine I created the following test case (which fails) at src/sign.rs file:

#[test]
fn verify_polkadot_js_signature() {
      use hex_literal::hex;
      const CTX : &'static [u8] = b"substrate";
        
      let message = b"message to sign";
      let public = hex!("f84d048da2ddae2d9d8fd6763f469566e8817a26114f39408de15547f6d47805");
      let public = PublicKey::from_bytes(&public[..]).unwrap();
        
      let signature = hex!("48ce2c90e08651adfc8ecef84e916f6d1bb51ebebd16150ee12df247841a5437951ea0f9d632ca165e6ab391532e75e701be6a1caa88c8a6bcca3511f55b4183");
      let signature = Signature::from_bytes(&signature).unwrap();
        
      let ok = public.verify_simple(CTX, message, &signature).is_ok();
      assert!(ok)
}

Debugging the polkadot-js signatureVerify function I noticed that this function calls the rust function ext_sr_verify under the hoods by a WASM biding (very interestingly 😄 ) and the rust function uses the same w3f/schnorrkel repository, then I assumed the function ext_sr_sign is used to generate the signature so I decided to create the following test case that uses these 2 functions to generate and verify a signature:

    #[test]
fn sign_and_verify() {
        const CTX : &'static [u8] = b"substrate";
        let sec = SecretKey::generate();
        let pub_bytes = sec.to_public().to_bytes();
        
        // ext_sr_sign function
        let sig = match (SecretKey::from_ed25519_bytes(&sec.to_bytes()), PublicKey::from_bytes(&pub_bytes)) {
            (Ok(s), Ok(k)) => s
                .sign_simple(CTX, message, &k)
                .to_bytes()
                .to_vec(),
            _ => panic!("Invalid secret or pubkey provided.")
        };

        // ext_sr_verify function
        let res =   match (Signature::from_bytes(&sig), PublicKey::from_bytes(&pub_bytes)) {
            (Ok(s), Ok(k)) => k
                .verify_simple(CTX, message, &s)
                .is_ok(),
            _ => false
        };

        assert!(res)
}

And unfortunately, this test case fails. The same thing happens with the https://github.com/ChainSafe/go-schnorrkel package, the original issue was reported into Gossamer discord channel.

Thanks in advance!

[burdges]

You've handled the key derivation path somehow? We stupidly split that between schnorrkel and other libraries. If not @jacogr can tell you where the other code is.

I'd happily argue in favor of W3F giving a grant to bring the full key derivation code stuff into schnorrkel, or a connected crate, which should simply problems like this.

[EclesioMeloJunior]

Wrapping the message with <Bytes>...</Bytes> works! as @jacogr said in polkadot-js/wasm#256.

Could you give me more context about the key derivation path solution?

[burdges]

I'm unsure about where the key derivation path decoding happens outside this create. Jaco should know, but I'll ask someone else too. They write:

https://docs.rs/sp-core/latest/sp_core/crypto/trait.Pair.html <- derive does that; there are implementations of this trait in https://github.com/paritytech/substrate/blob/master/primitives/core/src/sr25519.rs

[georgiosd]

I still don't understand how this code can fail - did anyone here figure it out?

const CTX : &'static [u8] = b"substrate";
let sec = SecretKey::generate();
let pub_bytes = sec.to_public().to_bytes();

let message = b"<Bytes>message to sign</Bytes>";

let sig = match (SecretKey::from_ed25519_bytes(&sec.to_bytes()), PublicKey::from_bytes(&pub_bytes)) {
(Ok(s), Ok(k)) => s
	.sign_simple(CTX, message, &k)
	.to_bytes()
	.to_vec(),
_ => panic!("Invalid secret or pubkey provided.")
};


// ext_sr_verify function
let res =   match (Signature::from_bytes(&sig), PublicKey::from_bytes(&pub_bytes)) {
(Ok(s), Ok(k)) => k
	.verify_simple(CTX, message, &s)
	.is_ok(),
_ => false
};

assert!(res)

[burdges]

I'd need to track down someone to answer, since this winds up more on the substrate side, so..

Can you ask this on https://substrate.stackexchange.com/ ? It'll likely be answered quickly there.

Again I'm not adverse to someone doing a PR that puts a bit more of the required substrate functionality here, if that's the best thing to do. I donno..

[georgiosd]

@burdges respectfully, how is this related to substrate? The code generates a keypair, signs it and then verifies the signature. Shouldn't that work regardless or the message?

[burdges]

I'd assumed we stayed on the same topic, and did not read the question, sorry.

SecretKey::from_ed25519_bytes(&sec.to_bytes()) changed your secret key by the cofactor.

Afaik there is no reason to use the ed25519 functions here, maybe they should be feature gated.

[georgiosd]

Would you kindly expand on that? I'm clueless as to what "changed by the cofactor" means.

IMO, it's more about having method name and/or documentation that can prevent such user errors.

[burdges]

ed25519 is not prime order, but has order the cofactor 8 times a prime l. In theory schnorr keys exist mod l, but ed25519 uses scalars mode 8*l and zeros the low bits to clear the cofactor when used.

A real ed25519 key has high bits set too, which is impossible here, but these ed25519 methods exist to make it possible to translate to some internal ed25519-dalek types using unsafe code. Just never use them for anything or you'll have problems like this. In future, I might remove them, feature gate them, or mark them unsafe, even though they're not unsafe themselves.

[georgiosd]

I can't say that I understood what you said, but thank you for elaborating :)