#449 - Replaced existing key derivation function using a hardened scheme (#459)

* #449 - replaced existing key derivation function using a hardened scheme

* removed unused code

* rerun tests

Author: thehenrytsai

README.md

@@ -3,7 +3,7 @@
 # Decentralized Web Node (DWN) SDK <!-- omit in toc -->
 
 Code Coverage
-![Statements](https://img.shields.io/badge/statements-97.55%25-brightgreen.svg?style=flat) ![Branches](https://img.shields.io/badge/branches-94.53%25-brightgreen.svg?style=flat) ![Functions](https://img.shields.io/badge/functions-93.82%25-brightgreen.svg?style=flat) ![Lines](https://img.shields.io/badge/lines-97.55%25-brightgreen.svg?style=flat)
+![Statements](https://img.shields.io/badge/statements-97.54%25-brightgreen.svg?style=flat) ![Branches](https://img.shields.io/badge/branches-94.51%25-brightgreen.svg?style=flat) ![Functions](https://img.shields.io/badge/functions-93.8%25-brightgreen.svg?style=flat) ![Lines](https://img.shields.io/badge/lines-97.54%25-brightgreen.svg?style=flat)
 
 
 - [Introduction](#introduction)

package-lock.json

@@ -86,7 +86,6 @@
     "multiformats": "11.0.2",
     "randombytes": "2.1.0",
     "readable-stream": "4.4.0",
-    "secp256k1": "5.0.0",
     "ulid": "2.3.0",
     "uuid": "8.3.2",
     "varint": "6.0.0"

package.json

@@ -1,7 +1,6 @@
 import type { PrivateJwk, PublicJwk } from '../types/jose-types.js';
 
 import * as secp256k1 from '@noble/secp256k1';
-import secp256k1Derivation from 'secp256k1';
 
 import { Encoder } from '../utils/encoder.js';
 import { sha256 } from 'multiformats/hashes/sha2';
@@ -157,67 +156,42 @@ export class Secp256k1 {
   }
 
   /**
-   * Derives a hierarchical deterministic public key.
-   * @param key Either a private or an uncompressed public key used to derive the descendant public key.
+   * Derives a hardened hierarchical deterministic public key.
    * @returns uncompressed public key
    */
-  public static async derivePublicKey(key: Uint8Array, relativePath: string[]): Promise<Uint8Array> {
+  public static async derivePublicKey(privateKey: Uint8Array, relativePath: string[]): Promise<Uint8Array> {
     Secp256k1.validateKeyDerivationPath(relativePath);
 
-    let currentPublicKey: Uint8Array;
-    if (key.length === 32) {
-      // private key is always 32 bytes
-      currentPublicKey = secp256k1.getPublicKey(key, false); // `false` = uncompressed
-    } else {
-      currentPublicKey = key;
-    }
-
-    for (const segment of relativePath) {
-      const hash = await sha256.encode(Encoder.stringToBytes(segment));
-      currentPublicKey = Secp256k1.deriveChildPublicKey(currentPublicKey, hash);
-    }
-
-    return currentPublicKey;
+    // derive the private key first then compute the derived public key from the derive private key
+    const derivedPrivateKey = await Secp256k1.derivePrivateKey(privateKey, relativePath);
+    const derivedPublicKey = await Secp256k1.getPublicKey(derivedPrivateKey);
+    return derivedPublicKey;
   }
 
   /**
-   * Derives a hierarchical deterministic private key.
+   * Derives a hardened hierarchical deterministic private key.
    */
   public static async derivePrivateKey(privateKey: Uint8Array, relativePath: string[]): Promise<Uint8Array> {
     Secp256k1.validateKeyDerivationPath(relativePath);
 
     let currentPrivateKey = privateKey;
     for (const segment of relativePath) {
-      const hash = await sha256.encode(Encoder.stringToBytes(segment));
-      currentPrivateKey = Secp256k1.deriveChildPrivateKey(currentPrivateKey, hash);
+      const derivationSegment = Encoder.stringToBytes(segment);
+      currentPrivateKey = await Secp256k1.deriveChildPrivateKey(currentPrivateKey, derivationSegment);
     }
 
     return currentPrivateKey;
   }
 
   /**
-   * Derives a child public key using the given tweak input.
-   */
-  public static deriveChildPublicKey(uncompressedPublicKey: Uint8Array, tweakInput: Uint8Array): Uint8Array {
-    // underlying library requires Buffer as input
-    const compressedPublicKey = false;
-    const publicKeyBuffer = Buffer.from(uncompressedPublicKey);
-    const tweakBuffer = Buffer.from(tweakInput);
-    const derivedPublicKey = secp256k1Derivation.publicKeyTweakAdd(publicKeyBuffer, tweakBuffer, compressedPublicKey);
-    return derivedPublicKey;
-  }
-
-  /**
-   * Derives a child private key using the given tweak input.
+   * Derives a child private key using the given derivation path segment.
    */
-  public static deriveChildPrivateKey(privateKey: Uint8Array, tweakInput: Uint8Array): Uint8Array {
-    // NOTE: passing in private key to v5.0.0 of `secp256k1.privateKeyTweakAdd()` has the side effect of modifying the input private key bytes.
-    // `secp256k1.publicKeyTweakAdd()` does not have this side effect.
-    // before there is a fix for it (we can also investigate and submit a PR), cloning the private key to workaround is a MUST
-    // also underlying library requires Buffer as input
-    const privateKeyBuffer = Buffer.from(privateKey);
-    const tweakBuffer = Buffer.from(tweakInput);
-    const derivedPrivateKey = secp256k1Derivation.privateKeyTweakAdd(privateKeyBuffer, tweakBuffer);
+  public static async deriveChildPrivateKey(privateKey: Uint8Array, derivationPathSegment: Uint8Array): Promise<Uint8Array> {
+    // hash the private key & derivation segment
+    const privateKeyHash = await sha256.encode(privateKey);
+    const derivationPathSegmentHash = await sha256.encode(derivationPathSegment);
+    const combinedBytes = secp256k1.etc.concatBytes(privateKeyHash, derivationPathSegmentHash);
+    const derivedPrivateKey = secp256k1.etc.hashToPrivateKey(combinedBytes);
     return derivedPrivateKey;
   }
 

src/utils/secp256k1.ts

@@ -42,55 +42,27 @@ describe('Secp256k1', () => {
     });
   });
 
-  describe('deriveChildPublic/PrivateKey()', () => {
-    it('should derive the same key pair counterparts when given the same tweak input', async () => {
-      const { publicKey, privateKey } = await Secp256k1.generateKeyPairRaw();
-
-      const tweakInput = TestDataGenerator.randomBytes(32);
-      const derivedPrivateKey = Secp256k1.deriveChildPrivateKey(privateKey, tweakInput);
-      const derivedPublicKey = Secp256k1.deriveChildPublicKey(publicKey, tweakInput);
-
-      const publicKeyFromDerivedPrivateKey = await Secp256k1.getPublicKey(derivedPrivateKey);
-      expect(ArrayUtility.byteArraysEqual(derivedPublicKey, publicKeyFromDerivedPrivateKey)).to.be.true;
-    });
-  });
-
   describe('derivePublic/PrivateKey()', () => {
     it('should be able to derive same key using different ancestor along the chain path', async () => {
-      const { publicKey, privateKey } = await Secp256k1.generateKeyPairRaw();
+      const { privateKey } = await Secp256k1.generateKeyPairRaw();
 
       const fullPathToG = ['a', 'b', 'c', 'd', 'e', 'f', 'g'];
       const fullPathToD = ['a', 'b', 'c', 'd'];
       const relativePathFromDToG = ['e', 'f', 'g'];
 
-      // testing public key derivation from different ancestor in the same chain
-      const publicKeyG = await Secp256k1.derivePublicKey(publicKey, fullPathToG);
-      const publicKeyD = await Secp256k1.derivePublicKey(publicKey, fullPathToD);
-      const publicKeyGFromD = await Secp256k1.derivePublicKey(publicKeyD, relativePathFromDToG);
-      expect(ArrayUtility.byteArraysEqual(publicKeyG, publicKeyGFromD)).to.be.true;
-
       // testing private key derivation from different ancestor in the same chain
       const privateKeyG = await Secp256k1.derivePrivateKey(privateKey, fullPathToG);
       const privateKeyD = await Secp256k1.derivePrivateKey(privateKey, fullPathToD);
       const privateKeyGFromD = await Secp256k1.derivePrivateKey(privateKeyD, relativePathFromDToG);
       expect(ArrayUtility.byteArraysEqual(privateKeyG, privateKeyGFromD)).to.be.true;
 
-      // testing that the derived private key matches up with the derived public key
-      const publicKeyGFromPrivateKeyG = await Secp256k1.getPublicKey(privateKeyG);
-      expect(ArrayUtility.byteArraysEqual(publicKeyGFromPrivateKeyG, publicKeyG)).to.be.true;
-    });
-
-    it('should derive the same public key using either the private or public counterpart of the same key pair', async () => {
-      const { publicKey, privateKey } = await Secp256k1.generateKeyPairRaw();
-
-      const path = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '10'];
-
-      const derivedKeyFromPublicKey = await Secp256k1.derivePublicKey(publicKey, path);
-      const derivedKeyFromPrivateKey = await Secp256k1.derivePublicKey(privateKey, path);
-      expect(ArrayUtility.byteArraysEqual(derivedKeyFromPublicKey, derivedKeyFromPrivateKey)).to.be.true;
+      // testing public key derivation from different ancestor private key in the same chain
+      const publicKeyG = await Secp256k1.derivePublicKey(privateKey, fullPathToG);
+      const publicKeyGFromD = await Secp256k1.derivePublicKey(privateKeyD, relativePathFromDToG);
+      expect(ArrayUtility.byteArraysEqual(publicKeyG, publicKeyGFromD)).to.be.true;
     });
 
-    it('should derive the same public key using either the private or public counterpart of the same key pair', async () => {
+    it('should throw if derivation path is invalid', async () => {
       const { publicKey, privateKey } = await Secp256k1.generateKeyPairRaw();
 
       const invalidPath = ['should not have segment with empty string', ''];