Add back support for verification of some invalid ECDSA sigs affected by old lib bug

At some point we used to generate invalid ECDSA sigs with the js (non-native) elliptic lib,
if the signature digest had leading zeros: https://github.com/openpgpjs/openpgpjs/pull/948 .

Brainpool curves are the most likely to have been affected by the bug, since they do not
have WebCrypto support (unlike NIST curves).
This commit reintroduces support on web to verify such invalid signatures
(support for this was previously built-in in the indutny-elliptic library).
It also expands the fix to work in Node.
This commit is contained in:
larabr 2023-10-06 17:21:50 +02:00
parent a9fae5ff12
commit 909d44f436

View File

@ -97,12 +97,25 @@ export async function sign(oid, hashAlgo, message, publicKey, privateKey, hashed
*/ */
export async function verify(oid, hashAlgo, signature, message, publicKey, hashed) { export async function verify(oid, hashAlgo, signature, message, publicKey, hashed) {
const curve = new CurveWithOID(oid); const curve = new CurveWithOID(oid);
// See https://github.com/openpgpjs/openpgpjs/pull/948.
// NB: the impact was more likely limited to Brainpool curves, since thanks
// to WebCrypto availability, NIST curve should not have been affected.
// Similarly, secp256k1 should have been used rarely enough.
// However, we implement the fix for all curves, since it's only needed in case of
// verification failure, which is unexpected, hence a minor slowdown is acceptable.
const tryFallbackVerificationForOldBug = () => (
hashed[0] === 0 ?
jsVerify(curve, signature, hashed.subarray(1), publicKey) :
false
);
if (message && !util.isStream(message)) { if (message && !util.isStream(message)) {
switch (curve.type) { switch (curve.type) {
case 'web': case 'web':
try { try {
// Need to await to make sure browser succeeds // Need to await to make sure browser succeeds
return await webVerify(curve, hashAlgo, signature, message, publicKey); const verified = await webVerify(curve, hashAlgo, signature, message, publicKey);
return verified || tryFallbackVerificationForOldBug();
} catch (err) { } catch (err) {
// We do not fallback if the error is related to key integrity // We do not fallback if the error is related to key integrity
// Unfortunately Safari does not support p521 and throws a DataError when using it // Unfortunately Safari does not support p521 and throws a DataError when using it
@ -113,14 +126,15 @@ export async function verify(oid, hashAlgo, signature, message, publicKey, hashe
util.printDebugError('Browser did not support verifying: ' + err.message); util.printDebugError('Browser did not support verifying: ' + err.message);
} }
break; break;
case 'node': case 'node': {
return nodeVerify(curve, hashAlgo, signature, message, publicKey); const verified = await nodeVerify(curve, hashAlgo, signature, message, publicKey);
return verified || tryFallbackVerificationForOldBug();
}
} }
} }
const nobleCurve = getNobleCurve(curve.name); const verified = jsVerify(curve, signature, hashed, publicKey);
// lowS: non-canonical sig: https://stackoverflow.com/questions/74338846/ecdsa-signature-verification-mismatch return verified || tryFallbackVerificationForOldBug();
return nobleCurve.verify(util.concatUint8Array([signature.r, signature.s]), hashed, publicKey, { lowS: false });
} }
/** /**
@ -164,6 +178,17 @@ export async function validateParams(oid, Q, d) {
// Helper functions // // Helper functions //
// // // //
////////////////////////// //////////////////////////
/**
* Fallback javascript implementation of ECDSA verification.
* To be used if no native implementation is available for the given curve/operation.
*/
function jsVerify(curve, signature, hashed, publicKey) {
const nobleCurve = getNobleCurve(curve.name);
// lowS: non-canonical sig: https://stackoverflow.com/questions/74338846/ecdsa-signature-verification-mismatch
return nobleCurve.verify(util.concatUint8Array([signature.r, signature.s]), hashed, publicKey, { lowS: false });
}
async function webSign(curve, hashAlgo, message, keyPair) { async function webSign(curve, hashAlgo, message, keyPair) {
const len = curve.payloadSize; const len = curve.payloadSize;
const jwk = privateToJWK(curve.payloadSize, webCurves[curve.name], keyPair.publicKey, keyPair.privateKey); const jwk = privateToJWK(curve.payloadSize, webCurves[curve.name], keyPair.publicKey, keyPair.privateKey);