From ef1d659d30f349922244c931c0fec7f043e857f3 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 2 Aug 2025 18:04:45 +0200 Subject: [PATCH 1/3] fix(crypto): improve handling of a same share key being passed more than once --- packages/crypto/src/shamir.ts | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/crypto/src/shamir.ts b/packages/crypto/src/shamir.ts index bbd9427..026bfdc 100644 --- a/packages/crypto/src/shamir.ts +++ b/packages/crypto/src/shamir.ts @@ -189,17 +189,36 @@ export function split( */ export function reconstruct( parts: BufferSource[], - neededParts: u8 = parts.length, + neededParts: u8 = undefined, ): Uint8Array { - if (parts.length < neededParts) - throw new Error("Not enough parts to reconstruct key"); const bytes = parts[0].byteLength - 1; const result = new Uint8Array(bytes); const dataViews = parts.map(part => ArrayBuffer.isView(part) ? new DataView(part.buffer, part.byteOffset, bytes + 1) : new DataView(part), - ); + ).filter((shareView, i, shares) => { + const x = shareView.getUint8(bytes); + for (let j = 0; j < i; j++) { + const otherShareView = shares[j]; + if (x !== otherShareView.getUint8(bytes)) continue; + for (let k = 0; k < bytes; i++) { + if (shareView.getUint8(k) !== otherShareView.getUint8(k)) { + throw new Error("There are conflicting key shares", { cause: [ + parts[j], + parts[i], + ] }); + } + } + return false; + } + return true; + }); + if (neededParts == null) { + neededParts = dataViews.length; + } else if (dataViews.length < neededParts) { + throw new Error("Not enough parts to reconstruct key"); + } for (let i = 0; i < bytes; i++) { result[i] = reconstructByte( Array.from({ length: neededParts }, (_, j) => { From 634a687f4792548df34cf31e3cd83d7c63f0e511 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 12 Aug 2025 15:25:40 +0200 Subject: [PATCH 2/3] fixup! --- packages/crypto/src/shamir.ts | 2 +- packages/crypto/test/shamir.test.ts | 41 ++++++++++++++++++++++++----- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/packages/crypto/src/shamir.ts b/packages/crypto/src/shamir.ts index 026bfdc..2291dcb 100644 --- a/packages/crypto/src/shamir.ts +++ b/packages/crypto/src/shamir.ts @@ -202,7 +202,7 @@ export function reconstruct( for (let j = 0; j < i; j++) { const otherShareView = shares[j]; if (x !== otherShareView.getUint8(bytes)) continue; - for (let k = 0; k < bytes; i++) { + for (let k = 0; k < bytes; k++) { if (shareView.getUint8(k) !== otherShareView.getUint8(k)) { throw new Error("There are conflicting key shares", { cause: [ parts[j], diff --git a/packages/crypto/test/shamir.test.ts b/packages/crypto/test/shamir.test.ts index 79f9f8f..1c3b588 100644 --- a/packages/crypto/test/shamir.test.ts +++ b/packages/crypto/test/shamir.test.ts @@ -13,6 +13,39 @@ it("should reconstruct a key", () => { ); }); +describe("should handle a key part being passed twice", () => { + it("fail when there are not enough key parts", () => { + assert.notStrictEqual( + Buffer.from(shamir.reconstruct([ + Buffer.from([0xef, 0x05, 0x70, 0x4a, 0xf2, 0xb2, 0xcd, 0x02]), + Buffer.from([0x62, 0x1e, 0x41, 0x63, 0xfa, 0x5e, 0x0b, 0x1c]), + Buffer.from([0xef, 0x05, 0x70, 0x4a, 0xf2, 0xb2, 0xcd, 0x02]), + ])).toString("hex"), + Buffer.from("caritat").toString("hex"), + ); + }); + it("throw if the are incompatible key parts", () => { + assert.throws( + () => shamir.reconstruct([ + Buffer.from([0xef, 0x05, 0x70, 0x4a, 0xf2, 0xb2, 0xcd, 0x02]), + Buffer.from([0x62, 0x1e, 0x41, 0x63, 0xfa, 0x5e, 0x0b, 0x1c]), + Buffer.from([0xc4, 0xc8, 0x3c, 0x22, 0x53, 0x05, 0x62, 0x02]), + ]), /There are conflicting key shares/); + }); + it("succeed when there are enough key parts", () => { + assert.strictEqual( + Buffer.from(shamir.reconstruct([ + Buffer.from([0xef, 0x05, 0x70, 0x4a, 0xf2, 0xb2, 0xcd, 0x02]), + Buffer.from([0x62, 0x1e, 0x41, 0x63, 0xfa, 0x5e, 0x0b, 0x1c]), + Buffer.from([0xef, 0x05, 0x70, 0x4a, 0xf2, 0xb2, 0xcd, 0x02]), + Buffer.from([0xc4, 0xc8, 0x3c, 0x22, 0x53, 0x05, 0x62, 0x0a]), + Buffer.from([0xef, 0x05, 0x70, 0x4a, 0xf2, 0xb2, 0xcd, 0x02]), + ])).toString("hex"), + Buffer.from("caritat").toString("hex"), + ); + }); +}); + const key = crypto.getRandomValues(new Uint8Array(256)); describe("should reconstruct single byte with enough shareholders", () => { @@ -93,12 +126,8 @@ it("should fail reconstruct key from not enough shareholders", () => { }); it("should fail reconstruct key with duplicate shareholders", () => { - assert.throws( - () => { - shamir.reconstruct([parts[1], parts[5], parts[1]]); - }, - { message: "Div/0" }, - ); + const reconstructed = shamir.reconstruct([parts[1], parts[5], parts[1]]); + assert.notDeepStrictEqual(reconstructed, key); }); it("should still reconstruct key with too many shareholders", () => { From 2b72b4490181b34c54b6026d64985befb7581932 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 12 Aug 2025 15:32:24 +0200 Subject: [PATCH 3/3] fixup! fixup! --- packages/crypto/test/shamir.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/crypto/test/shamir.test.ts b/packages/crypto/test/shamir.test.ts index 1c3b588..6c4d539 100644 --- a/packages/crypto/test/shamir.test.ts +++ b/packages/crypto/test/shamir.test.ts @@ -126,8 +126,12 @@ it("should fail reconstruct key from not enough shareholders", () => { }); it("should fail reconstruct key with duplicate shareholders", () => { - const reconstructed = shamir.reconstruct([parts[1], parts[5], parts[1]]); - assert.notDeepStrictEqual(reconstructed, key); + assert.throws( + () => { + shamir.reconstruct([parts[1], parts[5], parts[1]], 3); + }, + { message: "Not enough parts to reconstruct key" }, + ); }); it("should still reconstruct key with too many shareholders", () => {