From 0ad5020bd65cefcfd43c97b50a9ebc9ce4020548 Mon Sep 17 00:00:00 2001 From: pascal Date: Mon, 30 Sep 2024 15:17:37 +0200 Subject: [PATCH 1/2] script: Removes PublicKeyVerifier script --- docs/Schnorr.md | 10 -- script/PublicKeyVerifier.s.sol | 147 ------------------------ script/libs/LibPublicKeyVerifier.sol | 163 --------------------------- 3 files changed, 320 deletions(-) delete mode 100644 script/PublicKeyVerifier.s.sol delete mode 100644 script/libs/LibPublicKeyVerifier.sol diff --git a/docs/Schnorr.md b/docs/Schnorr.md index 550a37e..7f6ada4 100644 --- a/docs/Schnorr.md +++ b/docs/Schnorr.md @@ -88,16 +88,6 @@ Note that this aggregation scheme is vulnerable to rogue-key attacks[^musig2-pap In order to prevent such attacks, it **MUST** be verified that participating public keys own the corresponding private key. -Note further that this aggregation scheme is vulnerable to public keys with -linear relationships. A set of public keys `A` leaking the sum of their private -keys would allow the creation of a second set of public keys `B` with -`aggPubKey(A) = aggPubKey(B)`. This would make signatures created by set `A` -indistinguishable from signatures created by set `B`. -In order to prevent such issues, it **MUST** be verified that no two distinct -sets of public keys derive to the same aggregated public key. Note that -cryptographically sound created random private keys have a negligible -probability of having a linear relationship. - ## Other Security Considerations diff --git a/script/PublicKeyVerifier.s.sol b/script/PublicKeyVerifier.s.sol deleted file mode 100644 index e926497..0000000 --- a/script/PublicKeyVerifier.s.sol +++ /dev/null @@ -1,147 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; - -import {Script} from "forge-std/Script.sol"; -import {StdStyle} from "forge-std/StdStyle.sol"; -import {console2} from "forge-std/console2.sol"; - -import {LibSecp256k1} from "src/libs/LibSecp256k1.sol"; - -import {LibSecp256k1Extended} from "./libs/LibSecp256k1Extended.sol"; -import { - LibPublicKeyVerifier, LibSignerSet -} from "./libs/LibPublicKeyVerifier.sol"; - -/** - * @title PublicKeyVerifier - * - * @dev This script verifies that no subset of a given set of public keys - * compute to the same aggregated public key. - * - * The script must _never_ fail for a set of lifted feeds! - * - * @dev Usage: - * 1. Add the set of lifted feeds' public keys to `_pubKeys()` - * 2. Add the new feeds' public key to `_pubKeys()` - * 3. Run script via: - * - * ```bash - * $ forge script script/PublicKeyVerifier.s.sol:PublicKeyVerifierScript \ - * --memory-limit 10000000000 - * ``` - * - * Note that the script has a runtime and memory consumption of - * ω(2^#pubKeys). If the script fails with "EvmError: MemoryLimitOOG", - * increase the memory limit. - * - * @dev Why? - * - * Scribe uses a simple, but efficient, key aggregation scheme that is - * vulnerable to rogue key attacks (see docs/Schnorr.md#key-aggregation-for-multisignatures). - * While this attack vector is mitigated via proving a feed has ownership - * of their claimed private key, a similar attack is possible if feeds - * collude or create their private keys non-cryptographically secure. - * - * Let a set of feeds A have an aggregated public of aggPubKey(A) and leak - * the sum of their private keys. - * - * This would allow a distinct set of feeds B to be created with an - * aggregated public key of aggPubKey(B) = aggPubKey(A) by choosing private - * keys with the same sum as set A's. - * - * Having two distinct sets of feeds' public keys with the same aggregated - * public key makes it impossible to determine which set participated in a - * Schnorr signature. - * - * Note that this issue only occurs, with non-negligible probability, if - * at least one feed does not create their private key cryptographically - * sound. - * - * This script verifies that no two distinct subsets of public keys derive - * to the same aggregated public key. - */ -contract PublicKeyVerifierScript is Script { - using LibSecp256k1 for LibSecp256k1.Point; - using LibSecp256k1Extended for uint; - using LibPublicKeyVerifier for LibPublicKeyVerifier.PublicKeyVerifier; - - LibPublicKeyVerifier.PublicKeyVerifier verifier; - - function _pubKeys() internal pure returns (LibSecp256k1.Point[] memory) { - LibSecp256k1.Point[] memory pubKeys; - pubKeys = new LibSecp256k1.Point[](1_000); - uint ctr; - - // Use private keys during testing to see linear relationships easier. - //pubKeys[ctr++] = (uint(2).derivePublicKey()); - //pubKeys[ctr++] = (uint(3).derivePublicKey()); - //pubKeys[ctr++] = (uint(4).derivePublicKey()); - //pubKeys[ctr++] = (uint(5).derivePublicKey()); - // -> Fails because 2 + 3 = 5 - - // @todo Add pubKey to be lifted. - pubKeys[ctr++] = LibSecp256k1.Point({ - x: 0x0000000000000000000000000000000000000000000000000000000000000001, - y: 0x0000000000000000000000000000000000000000000000000000000000000000 - }); - - // @todo Add already lifted pubKeys. - pubKeys[ctr++] = LibSecp256k1.Point({ - x: 0x0000000000000000000000000000000000000000000000000000000000000000, - y: 0x0000000000000000000000000000000000000000000000000000000000000000 - }); - // ... - - // Set length of array to public keys actually added. - assembly ("memory-safe") { - mstore(pubKeys, ctr) - } - - return pubKeys; - } - - function run() public { - bool ok; - LibSecp256k1.Point[] memory set1; - LibSecp256k1.Point[] memory set2; - (ok, set1, set2) = verifier.verifyPublicKeys(_pubKeys()); - - if (!ok) { - console2.log( - StdStyle.red( - "[FAIL] Different sets of public keys compute to same aggregated public key" - ) - ); - console2.log("Set1 = {"); - for (uint j; j < set1.length; j++) { - console2.log(string.concat(" "), _pubKeyToString(set1[j])); - } - console2.log("}"); - console2.log("Set2 = {"); - for (uint j; j < set2.length; j++) { - console2.log(string.concat(" "), _pubKeyToString(set2[j])); - } - console2.log("}"); - - revert(); // Fail to have non-zero exit code - } else { - console2.log( - StdStyle.green( - "[PASS] No sets of public keys with same aggregated public key found" - ) - ); - } - } - - function _pubKeyToString(LibSecp256k1.Point memory pubKey) - internal - pure - returns (string memory) - { - string memory result = "pubKey("; - result = string.concat( - result, "x: ", vm.toString(pubKey.x), ", y: ", vm.toString(pubKey.y) - ); - return string.concat(result, ")"); - } -} diff --git a/script/libs/LibPublicKeyVerifier.sol b/script/libs/LibPublicKeyVerifier.sol deleted file mode 100644 index 98f50d3..0000000 --- a/script/libs/LibPublicKeyVerifier.sol +++ /dev/null @@ -1,163 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; - -import {Vm} from "forge-std/Vm.sol"; - -import {IScribe} from "src/IScribe.sol"; - -import {LibSecp256k1} from "src/libs/LibSecp256k1.sol"; - -import {LibSchnorrExtended} from "./LibSchnorrExtended.sol"; -import {LibSecp256k1Extended} from "./LibSecp256k1Extended.sol"; - -/** - * @title LibPublicKeyVerifier - * - * @dev Solidity library to verify a set of public key does not break - * assumptions in Scribe's Schnorr signature scheme. - * - * @dev Note that the `verifyPublicKeys` function has a runtime of - * ω(2^#pubKeys). Consider running it with a high memory limit to not run - * into out-of-gas errors. - */ -library LibPublicKeyVerifier { - using LibSecp256k1 for LibSecp256k1.Point; - using LibSecp256k1Extended for uint; - using LibSchnorrExtended for LibSecp256k1.Point[]; - using LibSignerSet for LibSignerSet.SignerSetConstructor; - - struct PublicKeyVerifier { - LibSignerSet.SignerSetConstructor signerSetConstructor; - mapping(address => LibSignerSet.SignerSet) lookupTable; - } - - /// @dev Verifies that no subset of a given set of public keys compute to - /// the same aggregated public key. If two distinct sets of public keys - /// with same aggregated public key is found, the two sets are returned. - /// - /// @return True if public keys have no linear relationship, false - /// otherwise. - /// @return Set of public keys - /// @return Set of public keys - function verifyPublicKeys( - PublicKeyVerifier storage self, - LibSecp256k1.Point[] memory pubKeys - ) - internal - returns (bool, LibSecp256k1.Point[] memory, LibSecp256k1.Point[] memory) - { - // Generate all possible signer sets (subsets) from set of public keys. - LibSignerSet.SignerSet[] memory signerSets; - signerSets = _generateSignerSets(self.signerSetConstructor, pubKeys); - - for (uint i; i < signerSets.length; i++) { - // Continue if empty set. - if (signerSets[i].pubKeys.length == 0) { - continue; - } - - // Compute aggregated pubKey and derive address. - LibSecp256k1.Point memory aggPubKey; - aggPubKey = signerSets[i].pubKeys.aggregatePublicKeys(); - address addr = aggPubKey.toAddress(); - - // Fail if address (i.e. aggPubKey) already known. - if (self.lookupTable[addr].pubKeys.length != 0) { - LibSecp256k1.Point[] memory set1; - set1 = self.lookupTable[addr].pubKeys; - - LibSecp256k1.Point[] memory set2; - set2 = signerSets[i].pubKeys; - - // Set 1 and set 2 compute to same aggregated public key. - // Their public keys therefore have a linear relationship. - return (false, set1, set2); - } - - // Otherwise add address (i.e. aggPubKey) to lookupTable. - // - // Note that copying memory structs with array fields to storage is - // not supported by solc if --via-ir is not used. To support non - // --via-ir compilation, the elements are copied manually. - // - // self.lookupTable[addr] = signerSets[i]; - for (uint j; j < signerSets[i].pubKeys.length; j++) { - self.lookupTable[addr].pubKeys.push(signerSets[i].pubKeys[j]); - } - } - - return (true, new LibSecp256k1.Point[](0), new LibSecp256k1.Point[](0)); - } - - function _generateSignerSets( - LibSignerSet.SignerSetConstructor storage signerSetConstructor, - LibSecp256k1.Point[] memory pubKeys - ) private returns (LibSignerSet.SignerSet[] memory) { - // The total number of subsets of pubKeys is 2^pubKeys.length. - uint totalSignerSets = 1 << pubKeys.length; - - LibSignerSet.SignerSet[] memory signerSets; - signerSets = new LibSignerSet.SignerSet[](totalSignerSets); - - for (uint i; i < totalSignerSets; i++) { - for (uint j; j < pubKeys.length; j++) { - if (i & (1 << j) != 0) { - signerSetConstructor.add(pubKeys[j]); - } - } - - // Add signerSet to signersSets. - signerSets[i] = signerSetConstructor.finalize(); - - // Reset signerSetConstructor. - signerSetConstructor.reset(); - } - - return signerSets; - } -} - -library LibSignerSet { - using LibSecp256k1 for LibSecp256k1.Point; - - struct SignerSet { - LibSecp256k1.Point[] pubKeys; - } - - // Note that constructor type is needed because types containing mappings cannot - // be loaded into memory. - struct SignerSetConstructor { - SignerSet signerSet; - mapping(address => bool) saved; - } - - function add( - SignerSetConstructor storage self, - LibSecp256k1.Point memory pubKey - ) internal { - address addr = pubKey.toAddress(); - if (!self.saved[addr]) { - self.signerSet.pubKeys.push(pubKey); - self.saved[addr] = true; - } - } - - function finalize(SignerSetConstructor storage self) - internal - view - returns (SignerSet memory) - { - return self.signerSet; - } - - function reset(SignerSetConstructor storage self) internal { - while (self.signerSet.pubKeys.length != 0) { - LibSecp256k1.Point[] storage pubKeys = self.signerSet.pubKeys; - - address addr = pubKeys[pubKeys.length - 1].toAddress(); - - self.saved[addr] = false; - self.signerSet.pubKeys.pop(); - } - } -} From 9edf1f5915d955fdfc75d442c10d1033a4873d89 Mon Sep 17 00:00:00 2001 From: pascal Date: Tue, 1 Oct 2024 10:22:29 +0200 Subject: [PATCH 2/2] docs: Update Schnorr spec regarding linear relationship check --- docs/Schnorr.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/Schnorr.md b/docs/Schnorr.md index 7f6ada4..1bf24eb 100644 --- a/docs/Schnorr.md +++ b/docs/Schnorr.md @@ -88,6 +88,14 @@ Note that this aggregation scheme is vulnerable to rogue-key attacks[^musig2-pap In order to prevent such attacks, it **MUST** be verified that participating public keys own the corresponding private key. +Note further that this aggregation scheme is vulnerable to public keys with +linear relationships. A set of public keys `A` leaking the sum of their private +keys would allow the creation of a second set of public keys `B` with +`aggPubKey(A) = aggPubKey(B)`. This would make signatures created by set `A` +indistinguishable from signatures created by set `B`. +However, this specification assumes that participants do not share private key +material leading to negligible probability for such cases to happen. + ## Other Security Considerations