From 5a76ce1f8a2368c3abae5bb0a78327a5bd5c2433 Mon Sep 17 00:00:00 2001 From: Ivaylo Nikolov Date: Mon, 3 Feb 2025 03:06:50 +0200 Subject: [PATCH 1/3] feat: add validation for public key Signed-off-by: Ivaylo Nikolov --- packages/cryptography/package.json | 1 + packages/cryptography/pnpm-lock.yaml | 17 ++++++++ packages/cryptography/src/EcdsaPublicKey.js | 35 +++++++++++++++- packages/cryptography/src/Ed25519PublicKey.js | 28 ++++++++++++- test/unit/PublicKey.js | 40 +++++++++++++++++++ 5 files changed, 117 insertions(+), 4 deletions(-) diff --git a/packages/cryptography/package.json b/packages/cryptography/package.json index e20e17446..75dd9022c 100644 --- a/packages/cryptography/package.json +++ b/packages/cryptography/package.json @@ -59,6 +59,7 @@ "last 1 Firefox versions" ], "dependencies": { + "@noble/curves": "^1.8.1", "asn1js": "^3.0.5", "bignumber.js": "^9.1.1", "bn.js": "^5.2.1", diff --git a/packages/cryptography/pnpm-lock.yaml b/packages/cryptography/pnpm-lock.yaml index bd6816e41..a75eb5cfe 100644 --- a/packages/cryptography/pnpm-lock.yaml +++ b/packages/cryptography/pnpm-lock.yaml @@ -8,6 +8,9 @@ importers: .: dependencies: + '@noble/curves': + specifier: ^1.8.1 + version: 1.8.1 asn1js: specifier: ^3.0.5 version: 3.0.5 @@ -1215,6 +1218,14 @@ packages: '@nicolo-ribaudo/eslint-scope-5-internals@5.1.1-v1': resolution: {integrity: sha512-54/JRvkLIzzDWshCWfuhadfrfZVPiElY8Fcgmg1HroEly/EDSszzhBAsarCux+D/kOslTRquNzuyGSmUSTTHGg==} + '@noble/curves@1.8.1': + resolution: {integrity: sha512-warwspo+UYUPep0Q+vtdVB4Ugn8GGQj8iyB3gnRWsztmUHTI3S1nhdiWNsPUGL0vud7JlRRk1XEu7Lq1KGTnMQ==} + engines: {node: ^14.21.3 || >=16} + + '@noble/hashes@1.7.1': + resolution: {integrity: sha512-B8XBPsn4vT/KJAGqDzbwztd+6Yte3P4V7iafm24bxgDe/mlRuK6xmWPuCNrKt2vDafZ8MfJLlchDG/vYafQEjQ==} + engines: {node: ^14.21.3 || >=16} + '@nodelib/fs.scandir@2.1.5': resolution: {integrity: sha512-vq24Bq3ym5HEQm2NKCr3yXDwjc7vTsEThRDnkp2DK9p1uqLR+DHurm/NOTo0KG7HYHU7eppKZj3MyqYuMBf62g==} engines: {node: '>= 8'} @@ -6123,6 +6134,12 @@ snapshots: dependencies: eslint-scope: 5.1.1 + '@noble/curves@1.8.1': + dependencies: + '@noble/hashes': 1.7.1 + + '@noble/hashes@1.7.1': {} + '@nodelib/fs.scandir@2.1.5': dependencies: '@nodelib/fs.stat': 2.0.5 diff --git a/packages/cryptography/src/EcdsaPublicKey.js b/packages/cryptography/src/EcdsaPublicKey.js index 30863c618..0801500bf 100644 --- a/packages/cryptography/src/EcdsaPublicKey.js +++ b/packages/cryptography/src/EcdsaPublicKey.js @@ -4,6 +4,7 @@ import { arrayEqual } from "./util/array.js"; import * as hex from "./encoding/hex.js"; import * as ecdsa from "./primitive/ecdsa.js"; import { keccak256 } from "./primitive/keccak.js"; +import { secp256k1 } from "@noble/curves/secp256k1"; import elliptic from "elliptic"; const ec = new elliptic.ec("secp256k1"); @@ -92,7 +93,8 @@ export default class EcdsaPublicKey extends Key { `cannot decode ECDSA private key data from DER format`, ); } - return new EcdsaPublicKey(ecdsaPublicKeyBytes); + + return EcdsaPublicKey.fromBytesRaw(ecdsaPublicKeyBytes); } /** @@ -106,7 +108,36 @@ export default class EcdsaPublicKey extends Key { ); } - return new EcdsaPublicKey(data); + if (this.isValid(data)) { + return new EcdsaPublicKey(data); + } else { + throw new BadKeyError("invalid public key"); + } + } + + /** + * @param {Uint8Array} data + * @returns {boolean} + */ + static isValid(data) { + const EMPTY_BUFFER_33_BYTES = Buffer.from(new Uint8Array(33).fill(0)); + const EMPTY_BUFFER_65_BYTES = Buffer.from(new Uint8Array(65).fill(0)); + const bufferData = Buffer.from(data); + + // SEE HIP-540 + if ( + EMPTY_BUFFER_33_BYTES.equals(bufferData) || + EMPTY_BUFFER_65_BYTES.equals(bufferData) + ) { + return true; + } + + try { + secp256k1.ProjectivePoint.fromHex(data); + return true; + } catch (error) { + return false; + } } /** diff --git a/packages/cryptography/src/Ed25519PublicKey.js b/packages/cryptography/src/Ed25519PublicKey.js index b6606d12a..b1e6161f3 100644 --- a/packages/cryptography/src/Ed25519PublicKey.js +++ b/packages/cryptography/src/Ed25519PublicKey.js @@ -4,6 +4,7 @@ import nacl from "tweetnacl"; import { arrayEqual } from "./util/array.js"; import * as hex from "./encoding/hex.js"; import forge from "node-forge"; +import { ed25519 } from "@noble/curves/ed25519"; const derPrefix = "302a300506032b6570032100"; const derPrefixBytes = hex.decode(derPrefix); @@ -77,7 +78,7 @@ export default class Ed25519PublicKey extends Key { ); } - return new Ed25519PublicKey(publicKey); + return Ed25519PublicKey.fromBytesRaw(publicKey); } /** @@ -91,7 +92,11 @@ export default class Ed25519PublicKey extends Key { ); } - return new Ed25519PublicKey(data); + if (this.isValid(data)) { + return new Ed25519PublicKey(data); + } else { + throw new BadKeyError("invalid public key"); + } } /** @@ -106,6 +111,25 @@ export default class Ed25519PublicKey extends Key { return Ed25519PublicKey.fromBytes(hex.decode(text)); } + /** + * @param {Uint8Array} data + * @returns {boolean} + */ + static isValid(data) { + if (!data) return false; + + const EMPTY_BUFFER = Buffer.from(new Uint8Array(33).fill(0)); + const bufferData = Buffer.from(data); + if (bufferData.equals(EMPTY_BUFFER)) return true; + + try { + ed25519.ExtendedPoint.fromHex(data); + return true; + } catch { + return false; + } + } + /** * Verify a signature on a message with this public key. * @param {Uint8Array} message diff --git a/test/unit/PublicKey.js b/test/unit/PublicKey.js index f659ecad5..874348e1f 100644 --- a/test/unit/PublicKey.js +++ b/test/unit/PublicKey.js @@ -106,4 +106,44 @@ describe("PublicKey", function () { const ed25519PublicKey1 = PublicKey.fromString(PUBLIC_KEY_DER1); expect(ed25519PublicKey1.toStringRaw()).to.be.equal(PUBLIC_KEY1); }); + + it("should throw error when creating ED25519 key from invalid bytes", function () { + // Test cases with invalid byte + const invalidPrivateKeyBytes = Uint8Array.from([ + 0xca, 0x35, 0x4b, 0x7c, 0xf4, 0x87, 0xd1, 0xbc, 0x43, 0x5a, 0x25, + 0x66, 0x77, 0x09, 0xc1, 0xab, 0x98, 0x0c, 0x11, 0x4d, 0x35, 0x94, + 0xe6, 0x25, 0x9e, 0x81, 0x2e, 0x6a, 0x70, 0x3d, 0x4f, 0x51, + ]); + + expect(() => { + PublicKey.fromBytesED25519(invalidPrivateKeyBytes); + }).to.throw; + }); + + it("should throw error when creating ECDSA key from invalid bytes", function () { + // Test cases with invalid byte + const invalidPrivateKeyBytes = Uint8Array.from([ + 0x00, 0xca, 0x35, 0x4b, 0x7c, 0xf4, 0x87, 0xd1, 0xbc, 0x43, 0x5a, + 0x25, 0x66, 0x77, 0x09, 0xc1, 0xab, 0x98, 0x0c, 0x11, 0x4d, 0x35, + 0x94, 0xe6, 0x25, 0x9e, 0x81, 0x2e, 0x6a, 0x70, 0x3d, 0x4f, 0x51, + ]); + + expect(() => { + PublicKey.fromBytesECDSA(invalidPrivateKeyBytes); + }).to.throw; + }); + + it("should be able to create ECDSA key from valid bytes", function () { + const validPubKey = PrivateKey.generateECDSA().publicKey; + const bytes = validPubKey.toBytes(); + const publicKey = PublicKey.fromBytes(bytes); + expect(publicKey.toString()).to.be.equal(validPubKey.toString()); + }); + + it("should be able to create ED25519 key from valid bytes", function () { + const validPubKey = PrivateKey.generateED25519().publicKey; + const bytes = validPubKey.toBytes(); + const publicKey = PublicKey.fromBytes(bytes); + expect(publicKey.toString()).to.be.equal(validPubKey.toString()); + }); }); From c028b1b1e95794943b6d8c1ee2f0b7533fcbc90b Mon Sep 17 00:00:00 2001 From: Ivaylo Nikolov Date: Tue, 4 Feb 2025 15:55:51 +0200 Subject: [PATCH 2/3] refactor: use curve order of ecdsa and ed25519 for tests Signed-off-by: Ivaylo Nikolov --- packages/cryptography/src/EcdsaPublicKey.js | 31 +++++--------- packages/cryptography/src/Ed25519PublicKey.js | 42 +++++++++---------- test/unit/PublicKey.js | 23 +++++----- 3 files changed, 43 insertions(+), 53 deletions(-) diff --git a/packages/cryptography/src/EcdsaPublicKey.js b/packages/cryptography/src/EcdsaPublicKey.js index 0801500bf..9c2482f78 100644 --- a/packages/cryptography/src/EcdsaPublicKey.js +++ b/packages/cryptography/src/EcdsaPublicKey.js @@ -98,8 +98,10 @@ export default class EcdsaPublicKey extends Key { } /** - * @param {Uint8Array} data + * Creates an ECDSA public key from raw bytes after validating the input + * @param {Uint8Array} data - Raw byte data for the public key * @returns {EcdsaPublicKey} + * @throws {BadKeyError} If the key is invalid or has an incorrect length */ static fromBytesRaw(data) { if (data.length != 33) { @@ -108,35 +110,24 @@ export default class EcdsaPublicKey extends Key { ); } - if (this.isValid(data)) { - return new EcdsaPublicKey(data); - } else { - throw new BadKeyError("invalid public key"); - } - } - - /** - * @param {Uint8Array} data - * @returns {boolean} - */ - static isValid(data) { - const EMPTY_BUFFER_33_BYTES = Buffer.from(new Uint8Array(33).fill(0)); - const EMPTY_BUFFER_65_BYTES = Buffer.from(new Uint8Array(65).fill(0)); + const EMPTY_BUFFER_33_BYTES = Buffer.alloc(33); + const EMPTY_BUFFER_65_BYTES = Buffer.alloc(65); const bufferData = Buffer.from(data); - // SEE HIP-540 + // Check for empty buffers (as per HIP-540) if ( EMPTY_BUFFER_33_BYTES.equals(bufferData) || EMPTY_BUFFER_65_BYTES.equals(bufferData) ) { - return true; + return new EcdsaPublicKey(data); } + // Attempt to validate the key using secp256k1 library try { secp256k1.ProjectivePoint.fromHex(data); - return true; - } catch (error) { - return false; + return new EcdsaPublicKey(data); + } catch { + throw new BadKeyError("invalid public key"); } } diff --git a/packages/cryptography/src/Ed25519PublicKey.js b/packages/cryptography/src/Ed25519PublicKey.js index b1e6161f3..64dbc3085 100644 --- a/packages/cryptography/src/Ed25519PublicKey.js +++ b/packages/cryptography/src/Ed25519PublicKey.js @@ -82,19 +82,34 @@ export default class Ed25519PublicKey extends Key { } /** - * @param {Uint8Array} data + * Creates an Ed25519 public key from raw bytes after validating the input + * @param {Uint8Array} data - Raw byte data for the public key * @returns {Ed25519PublicKey} + * @throws {BadKeyError} If the key is invalid or has an incorrect length */ static fromBytesRaw(data) { - if (data.length != 32) { + // Check length first + if (data.length !== 32) { throw new BadKeyError( `invalid public key length: ${data.length} bytes`, ); } - if (this.isValid(data)) { + // Create a buffer from the input data + const bufferData = Buffer.from(data); + + // Check for empty buffer (all zeros) + // SEE HIP-540 + const emptyBuffer = Buffer.alloc(33); + if (bufferData.equals(emptyBuffer)) { return new Ed25519PublicKey(data); - } else { + } + + // Attempt to validate the key using ed25519 library + try { + ed25519.ExtendedPoint.fromHex(data); + return new Ed25519PublicKey(data); + } catch { throw new BadKeyError("invalid public key"); } } @@ -111,25 +126,6 @@ export default class Ed25519PublicKey extends Key { return Ed25519PublicKey.fromBytes(hex.decode(text)); } - /** - * @param {Uint8Array} data - * @returns {boolean} - */ - static isValid(data) { - if (!data) return false; - - const EMPTY_BUFFER = Buffer.from(new Uint8Array(33).fill(0)); - const bufferData = Buffer.from(data); - if (bufferData.equals(EMPTY_BUFFER)) return true; - - try { - ed25519.ExtendedPoint.fromHex(data); - return true; - } catch { - return false; - } - } - /** * Verify a signature on a message with this public key. * @param {Uint8Array} message diff --git a/test/unit/PublicKey.js b/test/unit/PublicKey.js index 874348e1f..947097493 100644 --- a/test/unit/PublicKey.js +++ b/test/unit/PublicKey.js @@ -7,6 +7,11 @@ import { Transaction, } from "../../src/index.js"; +const CURVE_ORDER_ED25519 = + 7237005577332262213973186563042994240857116359379907606001950938285454250989n; +const CURVE_ORDER_ECDSA = + 115792089237316195423570985008687907852837564279074904382605163141518161494337n; + describe("PublicKey", function () { it("`verifyTransaction` works", async function () { const key1 = PrivateKey.generateED25519(); @@ -109,11 +114,10 @@ describe("PublicKey", function () { it("should throw error when creating ED25519 key from invalid bytes", function () { // Test cases with invalid byte - const invalidPrivateKeyBytes = Uint8Array.from([ - 0xca, 0x35, 0x4b, 0x7c, 0xf4, 0x87, 0xd1, 0xbc, 0x43, 0x5a, 0x25, - 0x66, 0x77, 0x09, 0xc1, 0xab, 0x98, 0x0c, 0x11, 0x4d, 0x35, 0x94, - 0xe6, 0x25, 0x9e, 0x81, 0x2e, 0x6a, 0x70, 0x3d, 0x4f, 0x51, - ]); + + const invalidKey = (CURVE_ORDER_ED25519 + 1n).toString(16); + const invalidKeyBuffer = Buffer.from(invalidKey, "hex"); + const invalidPrivateKeyBytes = Uint8Array.from(invalidKeyBuffer); expect(() => { PublicKey.fromBytesED25519(invalidPrivateKeyBytes); @@ -122,11 +126,10 @@ describe("PublicKey", function () { it("should throw error when creating ECDSA key from invalid bytes", function () { // Test cases with invalid byte - const invalidPrivateKeyBytes = Uint8Array.from([ - 0x00, 0xca, 0x35, 0x4b, 0x7c, 0xf4, 0x87, 0xd1, 0xbc, 0x43, 0x5a, - 0x25, 0x66, 0x77, 0x09, 0xc1, 0xab, 0x98, 0x0c, 0x11, 0x4d, 0x35, - 0x94, 0xe6, 0x25, 0x9e, 0x81, 0x2e, 0x6a, 0x70, 0x3d, 0x4f, 0x51, - ]); + + const invalidKey = (CURVE_ORDER_ECDSA + 1n).toString(16); + const invalidKeyBuffer = Buffer.from(invalidKey, "hex"); + const invalidPrivateKeyBytes = Uint8Array.from(invalidKeyBuffer); expect(() => { PublicKey.fromBytesECDSA(invalidPrivateKeyBytes); From 3171ba425ffe77550b94e0582a236268dd4e39ef Mon Sep 17 00:00:00 2001 From: Ivaylo Nikolov Date: Tue, 4 Feb 2025 16:00:34 +0200 Subject: [PATCH 3/3] chore: const order Signed-off-by: Ivaylo Nikolov --- test/unit/PublicKey.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/test/unit/PublicKey.js b/test/unit/PublicKey.js index 947097493..938f9f937 100644 --- a/test/unit/PublicKey.js +++ b/test/unit/PublicKey.js @@ -7,11 +7,6 @@ import { Transaction, } from "../../src/index.js"; -const CURVE_ORDER_ED25519 = - 7237005577332262213973186563042994240857116359379907606001950938285454250989n; -const CURVE_ORDER_ECDSA = - 115792089237316195423570985008687907852837564279074904382605163141518161494337n; - describe("PublicKey", function () { it("`verifyTransaction` works", async function () { const key1 = PrivateKey.generateED25519(); @@ -113,8 +108,8 @@ describe("PublicKey", function () { }); it("should throw error when creating ED25519 key from invalid bytes", function () { - // Test cases with invalid byte - + const CURVE_ORDER_ED25519 = + 7237005577332262213973186563042994240857116359379907606001950938285454250989n; const invalidKey = (CURVE_ORDER_ED25519 + 1n).toString(16); const invalidKeyBuffer = Buffer.from(invalidKey, "hex"); const invalidPrivateKeyBytes = Uint8Array.from(invalidKeyBuffer); @@ -125,8 +120,8 @@ describe("PublicKey", function () { }); it("should throw error when creating ECDSA key from invalid bytes", function () { - // Test cases with invalid byte - + const CURVE_ORDER_ECDSA = + 115792089237316195423570985008687907852837564279074904382605163141518161494337n; const invalidKey = (CURVE_ORDER_ECDSA + 1n).toString(16); const invalidKeyBuffer = Buffer.from(invalidKey, "hex"); const invalidPrivateKeyBytes = Uint8Array.from(invalidKeyBuffer);