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

BLS: bug in Rust implementation? #33

Closed
chenyan-dfinity opened this issue Nov 11, 2020 · 3 comments
Closed

BLS: bug in Rust implementation? #33

chenyan-dfinity opened this issue Nov 11, 2020 · 3 comments

Comments

@chenyan-dfinity
Copy link

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
Copy link
Contributor

bitdivine commented Nov 11, 2020

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
Copy link
Contributor

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

@chenyan-dfinity
Copy link
Author

Thanks for the quick fix! It works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants