Skip to content

Commit

Permalink
docs: Document vulnerable proof of possession
Browse files Browse the repository at this point in the history
  • Loading branch information
pmerkleplant committed Oct 2, 2024
1 parent 9115b55 commit 0ef985b
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 2 deletions.
7 changes: 5 additions & 2 deletions docs/Schnorr.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,10 @@ Let the aggregated public key be:
```

Note that this aggregation scheme is vulnerable to rogue-key attacks[^musig2-paper]!
In order to prevent such attacks, it **MUST** be verified that participating
public keys own the corresponding private key.
In order to prevent such attacks a separate public key validation step, called a
proof of possession, must be performed. This proof of possession can be
implemented via an ECDSA signature, however, the message signed **MUST** be
derived from the respective public key[^bls-proof-of-possession].

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
Expand Down Expand Up @@ -172,5 +174,6 @@ N = Qr * Pₓ⁻¹ | Qr = [(
- [Analysis of Bitcoin Improvement Proposal 340](https://courses.csail.mit.edu/6.857/2020/projects/4-Elbahrawy-Lovejoy-Ouyang-Perez.pdf)

[^musig2-paper]:[MuSig2 Paper](https://eprint.iacr.org/2020/1261.pdf)
[^bls-proof-of-possession]:[BLSBLS Signatures](https://www.ietf.org/archive/id/draft-irtf-cfrg-bls-signature-05.html#name-proof-of-possession)
[^baby-step-giant-step-wikipedia]:[Baby-step giant-step Wikipedia](https://en.wikipedia.org/wiki/Baby-step_giant-step)
[^vitalik-ethresearch-post]:[Vitalik's ethresearch post](https://ethresear.ch/t/you-can-kinda-abuse-ecrecover-to-do-ecmul-in-secp256k1-today/2384)
8 changes: 8 additions & 0 deletions docs/Scribe.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ A feed's identifier is defined as the highest order byte of the feed's address a

Feeds _must_ prove the integrity of their public key by proving the ownership of the corresponding private key. The `lift()` function therefore expects an ECDSA signed message, for more info see [`IScribe.feedRegistrationMessage()`](../src/IScribe.sol).

> [!WARNING]
>
> The proof of possession implemented in Scribe is insufficient to defend against rogue-key attacks. In order to sufficiently verify a public key the message being signed MUST be derived from the public key itself.
>
> In order to keep Scribe backwards compatible the extended proof of possession is implemented in the external [ValidatorRegistry](https://github.com/chronicleprotocol/validator-registry) contract.
>
> For more info, see [audits/[email protected]_2.pdf](../audits/[email protected]_2.pdf).
If public key's would not be verified, the Schnorr signature verification would be vulnerable to rogue-key attacks. For more info, see [`docs/Schnorr.md`](./Schnorr.md#key-aggregation-for-multisignatures).

## Chainlink Compatibility
Expand Down
8 changes: 8 additions & 0 deletions src/IScribe.sol
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ interface IScribe is IChronicle {
/// @dev Only callable by auth'ed address.
/// @dev The message expected to be signed by `ecdsaData` is defined via
/// `feedRegistrationMessage()(bytes32)`.
/// @custom:security The lift function's proof of possession is vulnerable
/// to rogue-key attacks. Additional verification MUST be
/// performed before lifting to ensure a feed's public key
/// validity.
/// @param pubKey The public key of the feed.
/// @param ecdsaData ECDSA signed message by the feed's public key.
/// @return feedId The id of the newly lifted feed.
Expand All @@ -199,6 +203,10 @@ interface IScribe is IChronicle {
/// @dev Only callable by auth'ed address.
/// @dev The message expected to be signed by `ecdsaDatas` is defined via
/// `feedRegistrationMessage()(bytes32)`.
/// @custom:security The lift function's proof of possession is vulnerable
/// to rogue-key attacks. Additional verification MUST be
/// performed before lifting to ensure a feed's public key
/// validity.
/// @param pubKeys The public keys of the feeds.
/// @param ecdsaDatas ECDSA signed message by the feeds' public keys.
/// @return List of feed ids of the newly lifted feeds.
Expand Down

0 comments on commit 0ef985b

Please sign in to comment.