miracl / core

MIRACL Core
Apache License 2.0
206 stars 68 forks source link

BLS: bug in Rust implementation? #33

Closed chenyan-dfinity closed 3 years ago

chenyan-dfinity commented 3 years ago

I am trying to verify the following signature with BLS12381 and domain separator BLS_SIG_BLS12381G1_XMD:SHA-256_SSWU_RO_NUL_, without the multi-pairing mechanism and enabling ALLOW_ALT_COMPRESS.

The same test passed in C, but I get BLS_FAIL in Rust.

#[test]
fn bls_verify() {
    use bls12381::bls::{core_verify, init, BLS_OK};
    let pk = hex::decode("a7623a93cdb56c4d23d99c14216afaab3dfd6d4f9eb3db23d038280b6d5cb2caaee2a19dd92c9df7001dede23bf036bc0f33982dfb41e8fa9b8e96b5dc3e83d55ca4dd146c7eb2e8b6859cb5a5db815db86810b8d12cee1588b5dbf34a4dc9a5").unwrap();
    let sig = hex::decode("b89e13a212c830586eaa9ad53946cd968718ebecc27eda849d9232673dcd4f440e8b5df39bf14a88048c15e16cbcaabe").unwrap();
    let msg = "hello".to_owned().into_bytes();
    assert_eq!(init(), BLS_OK);
    assert_eq!(core_verify(&sig, &msg, &pk), BLS_OK);
}

Here is how I generate the library: https://github.com/dfinity/agent-rs/blob/6831121ce0963348346059a98d39c57786a8e9ca/ic-agent/src/bls/README.md

I think I'm patching the right code (see my last item in the README.md above), but the test still fails. Maybe something wrong in the Rust implementation of ALLOW_ALT_COMPRESS?

bitdivine commented 3 years ago

Format issue. The standard miracl serialisation is 48+1 and 96+1 bytes for signature and pk. The hex above is 48 and 96 bytes long, so is the wrong length for the miracl serialisation methods. Perhaps we can add .to_<standard name here> and .from_<standard name here> for the 48 and 96 byte serialisations used in the emerging standard for bls signatures.

mcarrickscott commented 3 years ago

Bad bug! Around line 474 of ecp.rs - was 20, should have been 0x20 !! :( Now fixed

chenyan-dfinity commented 3 years ago

Thanks for the quick fix! It works!