Skip to content

Commit f135d5e

Browse files
committed
fix: authenticate signatureR in DKLS DSG round 4 messages
1 parent 866deca commit f135d5e

File tree

11 files changed

+351
-128
lines changed

11 files changed

+351
-128
lines changed

modules/abstract-lightning/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
]
4040
},
4141
"dependencies": {
42-
"@bitgo/public-types": "5.76.1",
42+
"@bitgo/public-types": "5.92.0",
4343
"@bitgo/sdk-core": "^36.40.0",
4444
"@bitgo/statics": "^58.35.0",
4545
"@bitgo/utxo-lib": "^11.22.0",

modules/bitgo/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@
138138
"superagent": "^9.0.1"
139139
},
140140
"devDependencies": {
141-
"@bitgo/public-types": "5.76.1",
141+
"@bitgo/public-types": "5.92.0",
142142
"@bitgo/sdk-opensslbytes": "^2.1.0",
143143
"@bitgo/sdk-test": "^9.1.38",
144144
"@openpgp/web-stream-tools": "0.0.14",

modules/express/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
"superagent": "^9.0.1"
6161
},
6262
"devDependencies": {
63-
"@bitgo/public-types": "5.76.1",
63+
"@bitgo/public-types": "5.92.0",
6464
"@bitgo/sdk-lib-mpc": "^10.10.2",
6565
"@bitgo/sdk-test": "^9.1.38",
6666
"@types/argparse": "^1.0.36",

modules/express/test/unit/clientRoutes/externalSign.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,16 +1046,22 @@ async function signBitgoMPCv2Round3(
10461046
userGPGPubKey: string
10471047
): Promise<{ userMsg4: MPCv2SignatureShareRound3Input }> {
10481048
const parsedSignatureShare = JSON.parse(userShare.share) as MPCv2SignatureShareRound3Input;
1049+
const msg4 = parsedSignatureShare.data.msg4;
1050+
const signatureRAuthMessage =
1051+
msg4.signatureR && msg4.signatureRSignature
1052+
? { message: msg4.signatureR, signature: msg4.signatureRSignature }
1053+
: undefined;
10491054
const serializedMessages = await DklsComms.decryptAndVerifyIncomingMessages(
10501055
{
10511056
p2pMessages: [],
10521057
broadcastMessages: [
10531058
{
1054-
from: parsedSignatureShare.data.msg4.from,
1059+
from: msg4.from,
10551060
payload: {
1056-
message: parsedSignatureShare.data.msg4.message,
1057-
signature: parsedSignatureShare.data.msg4.signature,
1061+
message: msg4.message,
1062+
signature: msg4.signature,
10581063
},
1064+
signatureR: signatureRAuthMessage,
10591065
},
10601066
],
10611067
},

modules/sdk-coin-flrp/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
"@bitgo/sdk-test": "^9.1.38"
4848
},
4949
"dependencies": {
50-
"@bitgo/public-types": "5.76.1",
50+
"@bitgo/public-types": "5.92.0",
5151
"@bitgo/sdk-core": "^36.40.0",
5252
"@bitgo/secp256k1": "^1.11.0",
5353
"@bitgo/statics": "^58.35.0",

modules/sdk-coin-sol/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
},
5858
"dependencies": {
5959
"@bitgo/logger": "^1.4.0",
60-
"@bitgo/public-types": "5.76.1",
60+
"@bitgo/public-types": "5.92.0",
6161
"@bitgo/sdk-core": "^36.40.0",
6262
"@bitgo/sdk-lib-mpc": "^10.10.2",
6363
"@bitgo/statics": "^58.35.0",

modules/sdk-core/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
]
4141
},
4242
"dependencies": {
43-
"@bitgo/public-types": "5.76.1",
43+
"@bitgo/public-types": "5.92.0",
4444
"@bitgo/sdk-lib-mpc": "^10.10.2",
4545
"@bitgo/secp256k1": "^1.11.0",
4646
"@bitgo/sjcl": "^1.1.0",

modules/sdk-core/src/bitgo/tss/ecdsa/ecdsaMPCv2.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,17 +126,22 @@ export async function getSignatureShareRoundThree(
126126
[getUserPartyGpgKey(userGpgKey, partyId)]
127127
);
128128
assert(MPCv2PartyFromStringOrNumber.is(userToBitGoEncryptedMsg4.broadcastMessages[0].from));
129-
if (!userToBitGoEncryptedMsg4.broadcastMessages[0].signatureR?.message) {
129+
const signatureR = userToBitGoEncryptedMsg4.broadcastMessages[0].signatureR;
130+
if (!signatureR?.message) {
130131
throw Error('signatureR should be defined');
131132
}
133+
if (!signatureR.signature) {
134+
throw Error('signatureR signature should be defined');
135+
}
132136
const share: MPCv2SignatureShareRound3Input = {
133137
type: 'round3Input',
134138
data: {
135139
msg4: {
136140
from: userToBitGoEncryptedMsg4.broadcastMessages[0].from,
137141
message: userToBitGoEncryptedMsg4.broadcastMessages[0].payload.message,
138142
signature: userToBitGoEncryptedMsg4.broadcastMessages[0].payload.signature,
139-
signatureR: userToBitGoEncryptedMsg4.broadcastMessages[0].signatureR.message,
143+
signatureR: signatureR.message,
144+
signatureRSignature: signatureR.signature,
140145
},
141146
},
142147
};

modules/sdk-lib-mpc/src/tss/ecdsa-dkls/commsLayer.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,15 @@ export async function decryptAndVerifyIncomingMessages(
188188
if (!(await verifySignedData(m.payload, pubGpgKey.gpgKey))) {
189189
throw Error(`Failed to authenticate broadcast message from party: ${m.from}`);
190190
}
191+
if (m.signatureR !== undefined) {
192+
if (!(await verifySignedData(m.signatureR, pubGpgKey.gpgKey))) {
193+
throw Error(`Failed to authenticate signatureR in broadcast message from party: ${m.from}`);
194+
}
195+
}
191196
return {
192197
from: m.from,
193198
payload: m.payload.message,
199+
signatureR: m.signatureR?.message,
194200
};
195201
})
196202
),
@@ -233,15 +239,16 @@ export async function encryptAndAuthOutgoingMessages(
233239
if (!prvGpgKey) {
234240
throw Error(`No private key provided for sender with ID: ${m.from}`);
235241
}
242+
const [signedPayload, signedSignatureR] = await Promise.all([
243+
detachSignData(Buffer.from(m.payload, 'base64'), prvGpgKey.gpgKey),
244+
m.signatureR
245+
? detachSignData(Buffer.from(m.signatureR, 'base64'), prvGpgKey.gpgKey)
246+
: Promise.resolve(undefined),
247+
]);
236248
return {
237249
from: m.from,
238-
payload: await detachSignData(Buffer.from(m.payload, 'base64'), prvGpgKey.gpgKey),
239-
signatureR: m.signatureR
240-
? {
241-
message: m.signatureR,
242-
signature: '',
243-
}
244-
: undefined,
250+
payload: signedPayload,
251+
signatureR: signedSignatureR,
245252
};
246253
})
247254
),

modules/sdk-lib-mpc/test/unit/tss/ecdsa/dklsComms.ts

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import {
22
decryptAndVerifySignedData,
33
encryptAndDetachSignData,
4+
decryptAndVerifyIncomingMessages,
5+
encryptAndAuthOutgoingMessages,
46
verifySignedData,
57
SIGNATURE_DATE_TOLERANCE_MS,
68
} from '../../../../src/tss/ecdsa-dkls/commsLayer';
@@ -165,3 +167,184 @@ describe('DKLS Communication Layer', function () {
165167
});
166168
});
167169
});
170+
171+
describe('DKLS encryptAndAuthOutgoingMessages / decryptAndVerifyIncomingMessages', function () {
172+
let partyAKey: { publicKey: string; privateKey: string };
173+
let partyBKey: { publicKey: string; privateKey: string };
174+
let tampererKey: { publicKey: string; privateKey: string };
175+
176+
before(async function () {
177+
openpgp.config.rejectCurves = new Set();
178+
partyAKey = await openpgp.generateKey({
179+
userIDs: [{ name: 'partyA', email: 'a@test.com' }],
180+
curve: 'secp256k1',
181+
});
182+
partyBKey = await openpgp.generateKey({
183+
userIDs: [{ name: 'partyB', email: 'b@test.com' }],
184+
curve: 'secp256k1',
185+
});
186+
tampererKey = await openpgp.generateKey({
187+
userIDs: [{ name: 'tamperer', email: 'evil@test.com' }],
188+
curve: 'secp256k1',
189+
});
190+
});
191+
192+
const partyAId = 0;
193+
const partyBId = 2;
194+
195+
it('should sign signatureR and verify it on receive', async function () {
196+
const payload = Buffer.from('deadbeef', 'hex').toString('base64');
197+
const signatureRBytes = Buffer.from('cafebabe', 'hex').toString('base64');
198+
199+
const encrypted = await encryptAndAuthOutgoingMessages(
200+
{
201+
p2pMessages: [],
202+
broadcastMessages: [{ from: partyAId, payload, signatureR: signatureRBytes }],
203+
},
204+
[{ partyId: partyBId, gpgKey: partyBKey.publicKey }],
205+
[{ partyId: partyAId, gpgKey: partyAKey.privateKey }]
206+
);
207+
208+
const broadcastMsg = encrypted.broadcastMessages[0];
209+
broadcastMsg.signatureR!.message.should.equal(signatureRBytes);
210+
broadcastMsg.signatureR!.signature.should.not.equal('');
211+
212+
const decrypted = await decryptAndVerifyIncomingMessages(
213+
{
214+
p2pMessages: [],
215+
broadcastMessages: [broadcastMsg],
216+
},
217+
[{ partyId: partyAId, gpgKey: partyAKey.publicKey }],
218+
[{ partyId: partyBId, gpgKey: partyBKey.privateKey }]
219+
);
220+
221+
decrypted.broadcastMessages[0].signatureR!.should.equal(signatureRBytes);
222+
});
223+
224+
it('should reject a tampered signatureR on receive', async function () {
225+
const payload = Buffer.from('deadbeef', 'hex').toString('base64');
226+
const signatureRBytes = Buffer.from('cafebabe', 'hex').toString('base64');
227+
228+
const encrypted = await encryptAndAuthOutgoingMessages(
229+
{
230+
p2pMessages: [],
231+
broadcastMessages: [{ from: partyAId, payload, signatureR: signatureRBytes }],
232+
},
233+
[{ partyId: partyBId, gpgKey: partyBKey.publicKey }],
234+
[{ partyId: partyAId, gpgKey: partyAKey.privateKey }]
235+
);
236+
237+
const tampered = {
238+
...encrypted.broadcastMessages[0],
239+
signatureR: {
240+
message: Buffer.from('00000000', 'hex').toString('base64'),
241+
signature: encrypted.broadcastMessages[0].signatureR!.signature,
242+
},
243+
};
244+
245+
await decryptAndVerifyIncomingMessages(
246+
{ p2pMessages: [], broadcastMessages: [tampered] },
247+
[{ partyId: partyAId, gpgKey: partyAKey.publicKey }],
248+
[{ partyId: partyBId, gpgKey: partyBKey.privateKey }]
249+
).should.be.rejectedWith(`Failed to authenticate signatureR in broadcast message from party: ${partyAId}`);
250+
});
251+
252+
it('should reject a signatureR signed by a different key', async function () {
253+
const payload = Buffer.from('deadbeef', 'hex').toString('base64');
254+
const signatureRBytes = Buffer.from('cafebabe', 'hex').toString('base64');
255+
256+
const encrypted = await encryptAndAuthOutgoingMessages(
257+
{
258+
p2pMessages: [],
259+
broadcastMessages: [{ from: partyAId, payload, signatureR: signatureRBytes }],
260+
},
261+
[{ partyId: partyBId, gpgKey: partyBKey.publicKey }],
262+
[{ partyId: partyAId, gpgKey: tampererKey.privateKey }]
263+
);
264+
265+
await decryptAndVerifyIncomingMessages(
266+
{ p2pMessages: [], broadcastMessages: [encrypted.broadcastMessages[0]] },
267+
[{ partyId: partyAId, gpgKey: partyAKey.publicKey }],
268+
[{ partyId: partyBId, gpgKey: partyBKey.privateKey }]
269+
).should.be.rejectedWith(`Failed to authenticate broadcast message from party: ${partyAId}`);
270+
});
271+
272+
it('should handle broadcast messages without signatureR unchanged', async function () {
273+
const payload = Buffer.from('deadbeef', 'hex').toString('base64');
274+
275+
const encrypted = await encryptAndAuthOutgoingMessages(
276+
{
277+
p2pMessages: [],
278+
broadcastMessages: [{ from: partyAId, payload }],
279+
},
280+
[{ partyId: partyBId, gpgKey: partyBKey.publicKey }],
281+
[{ partyId: partyAId, gpgKey: partyAKey.privateKey }]
282+
);
283+
284+
(encrypted.broadcastMessages[0].signatureR === undefined).should.equal(true);
285+
286+
const decrypted = await decryptAndVerifyIncomingMessages(
287+
{ p2pMessages: [], broadcastMessages: [encrypted.broadcastMessages[0]] },
288+
[{ partyId: partyAId, gpgKey: partyAKey.publicKey }],
289+
[{ partyId: partyBId, gpgKey: partyBKey.privateKey }]
290+
);
291+
292+
(decrypted.broadcastMessages[0].signatureR === undefined).should.equal(true);
293+
});
294+
295+
it('should skip signatureR verification when signatureR field is omitted (soft downgrade)', async function () {
296+
const payload = Buffer.from('deadbeef', 'hex').toString('base64');
297+
const signatureRBytes = Buffer.from('cafebabe', 'hex').toString('base64');
298+
299+
const encrypted = await encryptAndAuthOutgoingMessages(
300+
{
301+
p2pMessages: [],
302+
broadcastMessages: [{ from: partyAId, payload, signatureR: signatureRBytes }],
303+
},
304+
[{ partyId: partyBId, gpgKey: partyBKey.publicKey }],
305+
[{ partyId: partyAId, gpgKey: partyAKey.privateKey }]
306+
);
307+
308+
const broadcastWithoutAuthR = {
309+
from: encrypted.broadcastMessages[0].from,
310+
payload: encrypted.broadcastMessages[0].payload,
311+
};
312+
313+
const decrypted = await decryptAndVerifyIncomingMessages(
314+
{ p2pMessages: [], broadcastMessages: [broadcastWithoutAuthR] },
315+
[{ partyId: partyAId, gpgKey: partyAKey.publicKey }],
316+
[{ partyId: partyBId, gpgKey: partyBKey.privateKey }]
317+
);
318+
319+
(decrypted.broadcastMessages[0].signatureR === undefined).should.equal(true);
320+
});
321+
322+
it('should reject when signatureR message is present but signature is empty string', async function () {
323+
const payload = Buffer.from('deadbeef', 'hex').toString('base64');
324+
const signatureRBytes = Buffer.from('cafebabe', 'hex').toString('base64');
325+
326+
const encrypted = await encryptAndAuthOutgoingMessages(
327+
{
328+
p2pMessages: [],
329+
broadcastMessages: [{ from: partyAId, payload, signatureR: signatureRBytes }],
330+
},
331+
[{ partyId: partyBId, gpgKey: partyBKey.publicKey }],
332+
[{ partyId: partyAId, gpgKey: partyAKey.privateKey }]
333+
);
334+
335+
const strippedSig = {
336+
...encrypted.broadcastMessages[0],
337+
signatureR: {
338+
message: encrypted.broadcastMessages[0].signatureR!.message,
339+
signature: '',
340+
},
341+
};
342+
343+
// Empty armor: OpenPGP fails parsing before verifySignedData returns false.
344+
await decryptAndVerifyIncomingMessages(
345+
{ p2pMessages: [], broadcastMessages: [strippedSig] },
346+
[{ partyId: partyAId, gpgKey: partyAKey.publicKey }],
347+
[{ partyId: partyBId, gpgKey: partyBKey.privateKey }]
348+
).should.be.rejected();
349+
});
350+
});

0 commit comments

Comments
 (0)