Merge pull request #68 from transmute-industries/feat/secp56k1-bugs

Fix secp256k1 bugs
transmute-industries · Feb 8, 2021 · bec59eb · bec59eb
2 parents d1f633d + aa5252a
commit bec59eb
Show file tree

Hide file tree

Showing 8 changed files with 91 additions and 47 deletions.
diff --git a/packages/secp256k1/src/ES256K-R.test.ts b/packages/secp256k1/src/ES256K-R.test.ts
@@ -34,7 +34,7 @@ it('sign / verify', async () => {
   );
   const jws = await sign(message, privateKeyJwk);
   const verified = await verify(jws, publicKeyJwk);
-  expect(verified).toEqual(message);
+  expect(verified).toEqual(true);
 });
 
 it('signDetached / recoverPublicKey', async () => {

diff --git a/packages/secp256k1/src/ES256K-R.ts b/packages/secp256k1/src/ES256K-R.ts
@@ -10,7 +10,7 @@ export const sign = async (
   payload: any,
   privateKeyJwk: any,
   header: any = { alg: 'ES256K-R' }
-) => {
+): Promise<string> => {
   const privateKeyUInt8Array = await privateKeyUInt8ArrayFromJwk(privateKeyJwk);
 
   const encodedHeader = base64url.encode(JSON.stringify(header));
@@ -34,7 +34,10 @@ export const sign = async (
   return `${encodedHeader}.${encodedPayload}.${encodedSignature}`;
 };
 
-export const verify = async (jws: string, publicKeyJwk: any) => {
+export const verify = async (
+  jws: string,
+  publicKeyJwk: any
+): Promise<boolean> => {
   const publicKeyUInt8Array = await publicKeyUInt8ArrayFromJwk(publicKeyJwk);
   const [encodedHeader, encodedPayload, encodedSignature] = jws.split('.');
 
@@ -58,17 +61,8 @@ export const verify = async (jws: string, publicKeyJwk: any) => {
     messageHashUInt8Array,
     publicKeyUInt8Array
   );
-  if (verified) {
-    return JSON.parse(base64url.decode(encodedPayload));
-  }
-  const erroObject = {
-    signature: signatureUInt8Array.toString('hex'),
-    message: messageHashUInt8Array.toString('hex'),
-    publicKey: publicKeyUInt8Array.toString('hex'),
-  };
-  throw new Error(
-    'ECDSA Verify Failed: ' + JSON.stringify(erroObject, null, 2)
-  );
+
+  return verified;
 };
 
 export const signDetached = async (
@@ -79,7 +73,7 @@ export const signDetached = async (
     b64: false,
     crit: ['b64'],
   }
-) => {
+): Promise<string> => {
   const privateKeyUInt8Array = await privateKeyUInt8ArrayFromJwk(privateKeyJwk);
   const encodedHeader = base64url.encode(JSON.stringify(header));
   const toBeSignedBuffer = Buffer.concat([

diff --git a/packages/secp256k1/src/ES256K.test.ts b/packages/secp256k1/src/ES256K.test.ts
@@ -42,7 +42,7 @@ it('compact sign / verify ', async () => {
     jose.JWK.asKey(keypair.publicKeyJwk as any)
   );
   expect(_verified2).toEqual(example.payload);
-  expect(_verified).toEqual(example.payload);
+  expect(_verified).toEqual(true);
 });
 
 it('detached sign / verify', async () => {

diff --git a/packages/secp256k1/src/ES256K.ts b/packages/secp256k1/src/ES256K.ts
@@ -39,7 +39,7 @@ export const signDetached = async (
     b64: false,
     crit: ['b64'],
   }
-) => {
+): Promise<string> => {
   const privateKeyUInt8Array = await privateKeyUInt8ArrayFromJwk(privateKeyJwk);
 
   const encodedHeader = base64url.encode(JSON.stringify(header));
@@ -72,7 +72,7 @@ export const verifyDetached = async (
   jws: string,
   payload: Buffer,
   publicKeyJwk: ISecp256k1PublicKeyJwk
-) => {
+): Promise<boolean> => {
   if (jws.indexOf('..') === -1) {
     throw new JWSVerificationFailed('not a valid rfc7797 jws.');
   }
@@ -109,25 +109,15 @@ export const verifyDetached = async (
     publicKeyUInt8Array
   );
 
-  if (verified) {
-    return true;
-  }
-  const erroObject = {
-    signature: signatureUInt8Array.toString('hex'),
-    // message: messageHashUInt8Array.toString('hex'),
-    // publicKey: publicKeyUInt8Array.toString('hex'),
-  };
-  throw new JWSVerificationFailed(
-    'ECDSA Verify Failed: ' + JSON.stringify(erroObject, null, 2)
-  );
+  return verified;
 };
 
 /** Produce a normal ES256K JWS */
 export const sign = async (
   payload: any,
   privateKeyJwk: ISecp256k1PrivateKeyJwk,
   header: IJWSHeader = { alg: 'ES256K' }
-) => {
+): Promise<string> => {
   const privateKeyUInt8Array = await privateKeyUInt8ArrayFromJwk(privateKeyJwk);
 
   const encodedHeader = base64url.encode(JSON.stringify(header));
@@ -155,7 +145,7 @@ export const sign = async (
 export const verify = async (
   jws: string,
   publicKeyJwk: ISecp256k1PublicKeyJwk
-) => {
+): Promise<boolean> => {
   const publicKeyUInt8Array = await publicKeyUInt8ArrayFromJwk(publicKeyJwk);
   const [encodedHeader, encodedPayload, encodedSignature] = jws.split('.');
   const toBeSigned = `${encodedHeader}.${encodedPayload}`;
@@ -176,21 +166,12 @@ export const verify = async (
     messageHashUInt8Array,
     publicKeyUInt8Array
   );
-  if (verified) {
-    return JSON.parse(base64url.decode(encodedPayload));
-  }
-  const erroObject = {
-    signature: signatureUInt8Array.toString('hex'),
-    message: messageHashUInt8Array.toString('hex'),
-    publicKey: publicKeyUInt8Array.toString('hex'),
-  };
-  throw new JWSVerificationFailed(
-    'ECDSA Verify Failed: ' + JSON.stringify(erroObject, null, 2)
-  );
+
+  return verified;
 };
 
 /** decode a JWS (without verifying it) */
-export const decode = (jws: string, options = { complete: false }) => {
+export const decode = (jws: string, options = { complete: false }): any => {
   const [encodedHeader, encodedPayload, encodedSignature] = jws.split('.');
 
   if (options.complete) {

diff --git a/packages/secp256k1/src/__tests__/31-byte-private-keys.test.ts b/packages/secp256k1/src/__tests__/31-byte-private-keys.test.ts
@@ -0,0 +1,21 @@
+import * as keyUtils from '../keyUtils';
+
+// Originally reported in:
+// https://github.com/transmute-industries/did-key.js/issues/63
+it('private keys MUST always be 32 bytes', async () => {
+  // this JWK generatetes 31 byte length private keys
+  const example = {
+    kty: 'EC',
+    crv: 'secp256k1',
+    d: 'H8IPdO0ZRDrma0Oc1ASGp4R-7ioP3HKC2o3dBYLHUg',
+    x: 'F0UpAkmilL3GafMgs_NMLqwGpUYPEnFphet8wS21jMg',
+    y: 'vTGyefjnlCj2-T7gYw3Det6m1UtDOfbB4CTzROlT6QA',
+    kid: 'y3hMPnp_oK9BM_V9vXu0aErpbzz0mDKe7xjEG44doUg',
+  };
+  const privateKeyBuffer = keyUtils.privateKeyUInt8ArrayFromJwk(example);
+  expect(privateKeyBuffer.length).toBe(32);
+  const privateKeyJwk = keyUtils.privateKeyJwkFromPrivateKeyHex(
+    privateKeyBuffer.toString('hex')
+  );
+  expect(privateKeyJwk).toEqual(example);
+});
diff --git a/packages/secp256k1/src/__tests__/jose.santity.test.ts b/packages/secp256k1/src/__tests__/jose.santity.test.ts
@@ -22,7 +22,7 @@ it('interop', async () => {
     expect(theirVerification).toEqual(payload);
     const ourJws = await ES256K.sign(payload, privateKeyJwk);
     const ourVerification = await ES256K.verify(ourJws, publicKeyJwk);
-    expect(ourVerification).toEqual(payload);
+    expect(ourVerification).toEqual(true);
 
     const theirVerificationOfOurs = await JWS.verify(
       ourJws,
@@ -35,7 +35,7 @@ it('interop', async () => {
         theirJws,
         publicKeyJwk
       );
-      expect(ourVerificationOfTheirs).toEqual(payload);
+      expect(ourVerificationOfTheirs).toEqual(true);
     } catch (e) {
       errorCount++;
     }

diff --git a/packages/secp256k1/src/keyUtils.test.ts b/packages/secp256k1/src/keyUtils.test.ts
@@ -62,3 +62,27 @@ describe('publicKeyHexFromJwk', () => {
     );
   });
 });
+
+describe('publicKeyJwkFromPublicKeyBase58', () => {
+  it('should convert a publicKeyBase58 to a publicKeyJwk', async () => {
+    const _publicKeyJwk = await keyUtils.publicKeyJwkFromPublicKeyBase58(
+      example.keypair['application/did+ld+json'].publicKeyBase58
+    );
+    delete _publicKeyJwk.kid;
+    expect(_publicKeyJwk).toEqual(
+      example.keypair['application/did+json'].publicKeyJwk
+    );
+  });
+});
+
+describe('privateKeyJwkFromPrivateKeyBase58', () => {
+  it('should convert a privateKeyBase58 to a privateKeyJwk', async () => {
+    const _privateKeyJwk = await keyUtils.privateKeyJwkFromPrivateKeyBase58(
+      example.keypair['application/did+ld+json'].privateKeyBase58
+    );
+    delete _privateKeyJwk.kid;
+    expect(_privateKeyJwk).toEqual(
+      example.keypair['application/did+json'].privateKeyJwk
+    );
+  });
+});
diff --git a/packages/secp256k1/src/keyUtils.ts b/packages/secp256k1/src/keyUtils.ts
@@ -174,13 +174,25 @@ export const publicKeyHexFromJwk = (jwk: ISecp256k1PublicKeyJwk) => {
 /** convert jwk to binary encoded private key */
 export const privateKeyUInt8ArrayFromJwk = (jwk: ISecp256k1PrivateKeyJwk) => {
   const privateKeyHex = privateKeyHexFromJwk(jwk);
-  return Buffer.from(privateKeyHex, 'hex');
+  let asBuffer = Buffer.from(privateKeyHex, 'hex');
+  let padding = 32 - asBuffer.length;
+  while (padding > 0) {
+    asBuffer = Buffer.concat([Buffer.from('00', 'hex'), asBuffer]);
+    padding--;
+  }
+  return asBuffer;
 };
 
 /** convert jwk to binary encoded public key */
 export const publicKeyUInt8ArrayFromJwk = (jwk: ISecp256k1PublicKeyJwk) => {
   const publicKeyHex = publicKeyHexFromJwk(jwk);
-  return Buffer.from(publicKeyHex, 'hex');
+  let asBuffer = Buffer.from(publicKeyHex, 'hex');
+  let padding = 32 - asBuffer.length;
+  while (padding > 0) {
+    asBuffer = Buffer.concat([Buffer.from('00', 'hex'), asBuffer]);
+    padding--;
+  }
+  return asBuffer;
 };
 
 /** convert publicKeyHex to base58 */
@@ -211,3 +223,15 @@ export const publicKeyHexFromPrivateKeyHex = (privateKeyHex: string) => {
   );
   return Buffer.from(publicKey).toString('hex');
 };
+
+export const publicKeyJwkFromPublicKeyBase58 = (publicKeybase58: string) => {
+  return publicKeyJwkFromPublicKeyHex(
+    bs58.decode(publicKeybase58).toString('hex')
+  );
+};
+
+export const privateKeyJwkFromPrivateKeyBase58 = (privateKeyBase58: string) => {
+  return privateKeyJwkFromPrivateKeyHex(
+    bs58.decode(privateKeyBase58).toString('hex')
+  );
+};