vechain / vechain-sdk-js

The official JavaScript SDK for VeChain.
24 stars 9 forks source link

Derive public key from Wallet extended public key without the need of the `elliptic` library. #712

Closed lucanicoladebiasi closed 8 months ago

lucanicoladebiasi commented 8 months ago

The #105 Try to substitute secp256k1 using noble-secp256k1 moves from elliptic lib to the secured noble-curves lib for cryptographic function.

The elliptic lib is used only to derive public keys from Wallet HD extended public keys hence no risk of security infringements, but the same functionalities seems to be implemented elsewhere in the SDK.

The hdnode.ts code should provide the same functionalities through the ether lib without the need of the elliptic one.

nwbrettski commented 8 months ago

This would result in being able to remove the dependency elliptic

lucanicoladebiasi commented 8 months ago

Experimenting libraries

to provide alternatives to elliptic libraries.

My notes so far:

lucanicoladebiasi commented 8 months ago

packages/core/src/secp256k1/secp256k1.ts provides the function randomBytes(bytesLength?: number | undefined): Buffer as drop-in replacement for the same function provided by crypto.js.

The new implementation is based on noble-hashes library, used for seckp256k1 ECDSA curve, hence

Source of randomness based on crypto.js are replaced. Tests simplified removing redundant random generator functions.

lucanicoladebiasi commented 8 months ago

packages/core/tests/mnemonic/mnemonic.unit.test.ts uses crypto.js in

because refactoring to complex at the moment.

lucanicoladebiasi commented 8 months ago

The PR at https://github.com/vechain/vechain-sdk-js/pull/751 removes the deprecated crypto-js vulnerabilities.

lucanicoladebiasi commented 8 months ago

The missing function to complete noble-curves with the functionalities of elliptic is the function inflating a compressed public key to its uncompressed form. My implementation in TS is

function inflate(compressedPublicKey: Uint8Array): Buffer {
    const a = new BN(_secp256k1.CURVE.a.toString());
    const b = new BN(_secp256k1.CURVE.b.toString());
    const p = new BN(_secp256k1.CURVE.p.toString());
    const prefix = compressedPublicKey[0];
    const x = new BN(compressedPublicKey.slice(1), 'hex');
    const y2 = x.pow(new BN('3')).add(x.mul(a)).add(b).umod(p);
    let y = y2.pow(p.addn(1).divn(4)).umod(p);
    const two = new BN(2);
    if (y.mod(two) !== new BN(prefix - 2)) {
        y = p.sub(y);
    }
    return Buffer.concat([
        Buffer.from([0x04]), // Prefix byte for uncompressed key
        Buffer.from(x.toArray()), // X-coordinate
        Buffer.from(y.toArray()) // Y-coordinate
    ]);
}

using the secp256k1 constants provided by the noble-curves library. The math at the base of the transformation is published at https://www.secg.org/sec2-v2.pdf, hence the above function is what must be certified. The mathematician developed the noble-curve library thought is trivial hence omitted to implement the function.

Unfortunately BN and BigInt are prone to time based side channel attack, hence I must derive a better way to decompose the finite field of the elliptic transoformation, working on the base of what published at https://paulmillr.com/posts/noble-secp256k1-fast-ecc/#public-key