ZenGo-X / multi-party-eddsa

Rust implementation of multi party Ed25519 signature scheme.
GNU General Public License v3.0
134 stars 47 forks source link

remove bit clamping to allow HKD #4

Open omershlo opened 6 years ago

omershlo commented 6 years ago

https://moderncrypto.org/mail-archive/curves/2017/000858.html

durgesh-pandey commented 3 years ago

Hello, Any plans to add this feature?

I see the following suggestion for removing bit clamping looks good : https://moderncrypto.org/mail-archive/curves/2017/000869.html , here it's mentioned that we need to do the scalar mod 8l instead of bit clamping for which just doing the h * (a / h mod l ) will handle the co factor related issue where h is 8. I guess this need to be added in the initial key derivation part.

Also, on the HKD part, considering only two party case, I see the following BIP32 ed25519 : https://github.com/LedgerHQ/orakolo/blob/master/papers/Ed25519_BIP%20Final.pdf which can also be used for deriving the non-hardened ( i < 2^31) in two party eddsa (or even multiparty eddsa) case. However, there are attacks mentioned on this approach here: https://forum.w3f.community/t/key-recovery-attack-on-bip32-ed25519/44 . They also mention the multiplicative approach used by next gen tor and additive version of it. Though later one use the secret key to derive the scalar, so it may not be possible for both parties to calculate and update their pubKey.

I think if we follow the co factor handling as above and remove bit clamping on the initial keys, then for the non hardened child key derivation, whatever the scalar we get after the HMAC (as calculated in 2p ECDSA case), if that is also sanitised according to above suggestion and then added to initial secret (mod 8l necessary or mod l is enough after addition?), It should give a valid Child keys.

omershlo commented 3 years ago

Thanks @durgeshp-crypto. About this specific feature - do you want to give it a try? about HD: @elichai , you might have some thoughts on this ?

durgesh-pandey commented 3 years ago

@omershlo , actually I encounter rust-gmp related linking issue while running this repo directly in my machine. I guess that's some tool chain related issue. But I have tried doing it in my coding env where I modified logic in key generation which maps to line from https://github.com/ZenGo-X/multi-party-eddsa/blob/9bf90ad91d8efd916e89fb9c4de5862d2e0b2fba/src/protocols/aggsig/mod.rs#L72 as follow:

    private_key_1.copy_from_slice(&h_vec_padded[00..32]);
    //private_key[0] &= 248;
    //private_key[31] &= 63;
    //private_key[31] |= 64;
    let mut private_key: FE = ECScalar::from(&BigInt::from(&private_key_1[..]));
    let cofactor : FE =  ECScalar::from(&BigInt::from_str_radix("8", 10).unwrap());
    let cofactor_inv : FE = cofactor.invert();
    private_key = private_key.mul(&cofactor_inv.get_element());
    private_key = private_key.mul(&cofactor.get_element()); 

and this looks to be working for my test cases. Though I am trying to see if the division by 8 and multiplication by 8 inverse will have the same effect or not on the original scalar.

Updated: Inverse cofactor multiplication then by multiplying it 8 again results into same value. So need to do bigInt division instead of inverse multiplication to ecscalar.

So modifying above code like this and added few print statements for comparison:

    private_key_1.copy_from_slice(&h_vec_padded[00..32]);
    let mut private_key_0= private_key_1.clone();

    // co factor handling via bit clamping 
    private_key_0[0] &= 248;
    private_key_0[31] &= 63;
    private_key_0[31] |= 64;
    let private_key_0_fe: FE = ECScalar::from(&BigInt::from(&secret_key_0[..]));

    let mut private_key: FE = ECScalar::from(&BigInt::from(&private_key_1[..]));
    println!("initial scalar is : {:?} \n ", private_key);

    // cofactor handling via divide and multiply
    let private_key_bigu = private_key.to_big_int()/8;
    private_key = ECScalar::from(&private_key_bigu);
    let cofactor : FE =  ECScalar::from(&BigInt::from_str_radix("8", 10).unwrap());
    private_key = private_key.mul(&cofactor.get_element()); 

    println!("scalar after cofactor handling via divide and multiply is : {:?} \n", private_key);
    println!("scalar after cofactor via bit clamping is : {:?} \n", private_key_0_fe);

This is two example of above prints:

Ex1: initial scalar is : Point { purpose: "from_big_int", bytes: [115, 158, 41, 34, 12, 186, 147, 255, 44, 21, 251, 95, 27, 209, 198, 36, 0, 173, 240, 66, 185, 116, 30, 228, 247, 136, 175, 120, 95, 187, 19, 10] }

scalar after cofactor handling via divide and multiply is : Point { purpose: "from_big_int", bytes: [112, 158, 41, 34, 12, 186, 147, 255, 44, 21, 251, 95, 27, 209, 198, 36, 0, 173, 240, 66, 185, 116, 30, 228, 247, 136, 175, 120, 95, 187, 19, 10] }

scalar after cofactor via bit clamping is : Point { purpose: "from_big_int", bytes: [115, 158, 41, 34, 12, 186, 147, 255, 44, 21, 251, 95, 27, 209, 198, 36, 0, 173, 240, 66, 185, 116, 30, 228, 247, 136, 175, 120, 95, 187, 19, 8] }

Ex2: initial scalar is : Point { purpose: "from_big_int", bytes: [167, 211, 227, 171, 215, 195, 80, 29, 184, 38, 29, 52, 22, 133, 29, 188, 243, 194, 140, 108, 64, 91, 172, 142, 255, 41, 46, 166, 181, 55, 157, 4] }

scalar after cofactor handling via divide and multiply is : Point { purpose: "from_big_int", bytes: [160, 211, 227, 171, 215, 195, 80, 29, 184, 38, 29, 52, 22, 133, 29, 188, 243, 194, 140, 108, 64, 91, 172, 142, 255, 41, 46, 166, 181, 55, 157, 4] }

scalar after cofactor via bit clamping is : Point { purpose: "from_big_int", bytes: [103, 211, 227, 171, 215, 195, 80, 29, 184, 38, 29, 52, 22, 133, 29, 188, 243, 194, 140, 108, 64, 91, 172, 142, 255, 41, 46, 166, 181, 55, 157, 0] }

durgesh-pandey commented 3 years ago

I got my env fixed and can run this repo now, I'll give it a try and create a PR for it. @omershlo

omershlo commented 3 years ago

thank you !!

durgesh-pandey commented 3 years ago

Hey, wanted to create a PR in "curv" (https://github.com/ZenGo-X/curv) for small torsion-safe ed25519 scalar test. How can I get permission to push or create a draft PR for it? It shows I do not have write access when pushing the changes.

omershlo commented 3 years ago

You should be able to make a PR as usual . What error do you get ?

durgesh-pandey commented 3 years ago

Okay. Tried both ssh & https cloning, cloning of repo works but while pushing it shows the following error:

via SSH clonning: ERROR: Permission to ZenGo-X/curv.git denied to durgeshp-crypto. fatal: Could not read from remote repository.

Please make sure you have the correct access rights and the repository exists.

HTTPS: remote: Permission to ZenGo-X/curv.git denied to durgeshp-crypto. fatal: unable to access 'https://github.com/ZenGo-X/curv.git/': The requested URL returned error: 403

Tried the one-time token too to avoid the use of a password but still shows the same issue. Trying to figure out if it's an issue from my github account or from curv side.

durgesh-pandey commented 3 years ago

It got resolved, was using a wrong method for creating PR :( https://github.com/ZenGo-X/curv/pull/136