From 601a2b9ffdf53f84eafe489218746e8be17d4d7a Mon Sep 17 00:00:00 2001 From: larabr Date: Mon, 27 Nov 2023 13:08:59 +0100 Subject: [PATCH] Add EdDSA signature verification step to detect and avoid outputting faulty signatures (#11) EdDSA is known to be vulnerable to fault attacks which can lead to secret key extraction if two signatures over the same data can be collected. Randomly occurring bitflips in specific parts of the computation might in principle result in vulnerable faulty signatures being generated, hence we add the option to verify the signatures before outputting them. This commit also adds the `checkEdDSAFaultySignatures` flag to the global config to be able to control the deployment of the eddsa check. Support is limited to the global config object as the affected functions currently don't take a config input, and we don't need to selectively enable the option anyway, so we limit the scope of the changes. --- openpgp.d.ts | 2 +- src/config/config.js | 9 ++++++++- src/crypto/public_key/elliptic/eddsa.js | 15 +++++++++++++++ src/crypto/public_key/elliptic/eddsa_legacy.js | 15 +++++++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/openpgp.d.ts b/openpgp.d.ts index 5b4e567ce..3baca2221 100644 --- a/openpgp.d.ts +++ b/openpgp.d.ts @@ -345,7 +345,7 @@ interface Config { rejectPublicKeyAlgorithms: Set; rejectCurves: Set; } -export var config: Config; +export var config: Config & { checkEdDSAFaultySignatures: boolean }; // option only supported if set at the global openpgp.config level // PartialConfig has the same properties as Config, but declared as optional. // This interface is relevant for top-level functions, which accept a subset of configuration options diff --git a/src/config/config.js b/src/config/config.js index a5b60896f..dcf68d728 100644 --- a/src/config/config.js +++ b/src/config/config.js @@ -286,5 +286,12 @@ export default { * @memberof module:config * @property {Set} rejectCurves {@link module:enums.curve} */ - rejectCurves: new Set([enums.curve.secp256k1]) + rejectCurves: new Set([enums.curve.secp256k1]), + /** + * Whether to validate generated EdDSA signatures before returning them, to ensure they are not faulty signatures. + * This check will make signing 2-3 times slower. + * Faulty signatures may be generated (in principle) if random bitflips occur at specific points in the signature + * computation, and could be used to recover the signer's secret key given a second signature over the same data. + */ + checkEdDSAFaultySignatures: true }; diff --git a/src/crypto/public_key/elliptic/eddsa.js b/src/crypto/public_key/elliptic/eddsa.js index e38cbb96f..c7acf0d6b 100644 --- a/src/crypto/public_key/elliptic/eddsa.js +++ b/src/crypto/public_key/elliptic/eddsa.js @@ -27,6 +27,7 @@ import util from '../../../util'; import enums from '../../../enums'; import hash from '../../hash'; import { getRandomBytes } from '../../random'; +import defaultConfig from '../../../config'; nacl.hash = bytes => new Uint8Array(sha512().update(bytes).digest()); @@ -68,6 +69,20 @@ export async function sign(algo, hashAlgo, message, publicKey, privateKey, hashe case enums.publicKey.ed25519: { const secretKey = util.concatUint8Array([privateKey, publicKey]); const signature = nacl.sign.detached(hashed, secretKey); + if (defaultConfig.checkEdDSAFaultySignatures && !nacl.sign.detached.verify(hashed, signature, publicKey)) { + /** + * Detect faulty signatures caused by random bitflips during `crypto_sign` which could lead to private key extraction + * if two signatures over the same message are obtained. + * See https://github.com/jedisct1/libsodium/issues/170. + * If the input data is not deterministic, e.g. thanks to the random salt in v6 OpenPGP signatures (not yet implemented), + * then the generated signature is always safe, and the verification step is skipped. + * Otherwise, we need to verify the generated to ensure that no bitflip occured: + * - in M between the computation of `r` and `h`. + * - in the public key before computing `h` + * The verification step is almost 2-3 times as slow as signing, but it's faster than re-signing + re-deriving the public key for separate checks. + */ + throw new Error('Transient signing failure'); + } return { RS: signature }; } case enums.publicKey.ed448: diff --git a/src/crypto/public_key/elliptic/eddsa_legacy.js b/src/crypto/public_key/elliptic/eddsa_legacy.js index 63929ea7c..65b5f7fce 100644 --- a/src/crypto/public_key/elliptic/eddsa_legacy.js +++ b/src/crypto/public_key/elliptic/eddsa_legacy.js @@ -27,6 +27,7 @@ import nacl from '@openpgp/tweetnacl/nacl-fast-light'; import util from '../../../util'; import enums from '../../../enums'; import hash from '../../hash'; +import defaultConfig from '../../../config'; nacl.hash = bytes => new Uint8Array(sha512().update(bytes).digest()); @@ -51,6 +52,20 @@ export async function sign(oid, hashAlgo, message, publicKey, privateKey, hashed } const secretKey = util.concatUint8Array([privateKey, publicKey.subarray(1)]); const signature = nacl.sign.detached(hashed, secretKey); + if (defaultConfig.checkEdDSAFaultySignatures && !nacl.sign.detached.verify(hashed, signature, publicKey.subarray(1))) { + /** + * Detect faulty signatures caused by random bitflips during `crypto_sign` which could lead to private key extraction + * if two signatures over the same message are obtained. + * See https://github.com/jedisct1/libsodium/issues/170. + * If the input data is not deterministic, e.g. thanks to the random salt in v6 OpenPGP signatures (not yet implemented), + * then the generated signature is always safe, and the verification step is skipped. + * Otherwise, we need to verify the generated to ensure that no bitflip occured: + * - in M between the computation of `r` and `h`. + * - in the public key before computing `h` + * The verification step is almost 2-3 times as slow as signing, but it's faster than re-signing + re-deriving the public key for separate checks. + */ + throw new Error('Transient signing failure'); + } // EdDSA signature params are returned in little-endian format return { r: signature.subarray(0, 32),