From 5955eddc7d6e344e8f18ad6823e3bd8ac3fb84b7 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 27 Jul 2018 13:18:21 -0700 Subject: [PATCH 1/5] ADR: Fix malleability problems in Secp256k1 signatures Previously you could not assume that your transaction hash would appear on chain. --- .../architecture/adr-014-secp-malleability.md | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 docs/architecture/adr-014-secp-malleability.md diff --git a/docs/architecture/adr-014-secp-malleability.md b/docs/architecture/adr-014-secp-malleability.md new file mode 100644 index 00000000..ee196f48 --- /dev/null +++ b/docs/architecture/adr-014-secp-malleability.md @@ -0,0 +1,53 @@ +# ADR 014: Secp256k1 Signature Malleability + +## Context + +Secp256k1 has two layers of malleability. +The signer has a random nonce, and thus can produce many different valid signatures. +This ADR is not concerned with that. +The second layer of malleability basically allows one who is given a signature +to produce exactly one more valid signature for the same message from the same public key. +(They don't even have to know the message!) +The math behind this will be explained in the subsequent section. + +Note that in many downstream applications, signatures will appear in a transaction, and therefore in the tx hash. +This means that if someone broadcasts a transaction with secp256k1 signature, the signature can be altered into the other form by anyone in the p2p network. +Thus the tx hash will change, and this altered tx hash may be committed instead. +This breaks the assumption that you can broadcast a valid transaction and just wait for its hash to be included on chain. +You may not even know to increment your sequence number for example. +Removing this second layer of signature malleability concerns could ease downstream development. + +### ECDSA context + +Secp256k1 is ECDSA over a particular curve. +The signature is of the form `(r, s)`, where `s` is an elliptic curve group element. +However `(r, -s)` is also another valid solution. +Note that anyone can negate a group element, and therefore can get this second signature. + +## Decision + +We can just distinguish a canonical form for the ECDSA signatures. +Then we require that all ECDSA signatures be in the canonical form between the two. + +The canonical form is rather easy to define and check. +It would just be the smaller of the two y coordinates for the given x coordinate, defined lexicographically. +Example of other systems using this: https://github.com/zkcrypto/pairing/tree/master/src/bls12_381#serialization. + +## Proposed Implementation + +Fork https://github.com/btcsuite/btcd, and just update the [parse sig method](https://github.com/btcsuite/btcd/blob/master/btcec/signature.go#195) and serialize functions to enforce our canonical form. + +## Status + +Proposed. + +## Consequences + +### Positive +* Lets us maintain the ability to expect a tx hash to appear in the blockchain. + +### Negative +* More work in all future implementations (Though this is a very simple check) +* Requires us to maintain another fork + +### Neutral From af2894c0f80176759e9ac0631696a9d7799cc22d Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 27 Jul 2018 19:27:25 -0700 Subject: [PATCH 2/5] (squash this) improve grammar. --- docs/architecture/adr-014-secp-malleability.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/architecture/adr-014-secp-malleability.md b/docs/architecture/adr-014-secp-malleability.md index ee196f48..b84a4785 100644 --- a/docs/architecture/adr-014-secp-malleability.md +++ b/docs/architecture/adr-014-secp-malleability.md @@ -14,7 +14,9 @@ Note that in many downstream applications, signatures will appear in a transacti This means that if someone broadcasts a transaction with secp256k1 signature, the signature can be altered into the other form by anyone in the p2p network. Thus the tx hash will change, and this altered tx hash may be committed instead. This breaks the assumption that you can broadcast a valid transaction and just wait for its hash to be included on chain. -You may not even know to increment your sequence number for example. +One example is if you are broadcasting a tx in cosmos, +and you wait for it to appear on chain before incrementing your sequence number. +You may never increment your sequence number if a different tx hash got committed. Removing this second layer of signature malleability concerns could ease downstream development. ### ECDSA context @@ -27,9 +29,9 @@ Note that anyone can negate a group element, and therefore can get this second s ## Decision We can just distinguish a canonical form for the ECDSA signatures. -Then we require that all ECDSA signatures be in the canonical form between the two. +Then we require that all ECDSA signatures be in the form which we defined as canonical. -The canonical form is rather easy to define and check. +A canonical form is rather easy to define and check. It would just be the smaller of the two y coordinates for the given x coordinate, defined lexicographically. Example of other systems using this: https://github.com/zkcrypto/pairing/tree/master/src/bls12_381#serialization. From 3d5d254932bd53808f64410b3dcc067ede4840f2 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Sat, 28 Jul 2018 20:40:21 -0700 Subject: [PATCH 3/5] (squash this) Mixed up field element and curve element. Idea still stands. --- docs/architecture/adr-014-secp-malleability.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/architecture/adr-014-secp-malleability.md b/docs/architecture/adr-014-secp-malleability.md index b84a4785..f0db5743 100644 --- a/docs/architecture/adr-014-secp-malleability.md +++ b/docs/architecture/adr-014-secp-malleability.md @@ -22,7 +22,8 @@ Removing this second layer of signature malleability concerns could ease downstr ### ECDSA context Secp256k1 is ECDSA over a particular curve. -The signature is of the form `(r, s)`, where `s` is an elliptic curve group element. +The signature is of the form `(r, s)`, where `s` is a field element. +(The particular field is the `Z_n`, where the elliptic curve has order `n`) However `(r, -s)` is also another valid solution. Note that anyone can negate a group element, and therefore can get this second signature. @@ -30,10 +31,13 @@ Note that anyone can negate a group element, and therefore can get this second s We can just distinguish a canonical form for the ECDSA signatures. Then we require that all ECDSA signatures be in the form which we defined as canonical. +We reject signatures in non-canonical form. A canonical form is rather easy to define and check. -It would just be the smaller of the two y coordinates for the given x coordinate, defined lexicographically. -Example of other systems using this: https://github.com/zkcrypto/pairing/tree/master/src/bls12_381#serialization. +It would just be the smaller of the two values for `s`, defined lexicographically. +This is a simple check, instead of checking if `s < n`, instead check `s <= (n - 1)/2`. +An example of another cryptosystem using this +is the parity definition here https://github.com/zkcrypto/pairing/pull/30#issuecomment-372910663. ## Proposed Implementation From 87f09adeec3c2c589d34a2902557fe4a92860a04 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Thu, 2 Aug 2018 23:27:16 -0700 Subject: [PATCH 4/5] (Squash this) Be more explicit about the exact encoding of the secp signature --- docs/architecture/adr-015-crypto-encoding.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/architecture/adr-015-crypto-encoding.md b/docs/architecture/adr-015-crypto-encoding.md index 98341127..67cce95f 100644 --- a/docs/architecture/adr-015-crypto-encoding.md +++ b/docs/architecture/adr-015-crypto-encoding.md @@ -59,8 +59,8 @@ Use the canonical representation for signatures. #### Secp256k1 There isn't a clear canonical representation here. Signatures have two elements `r,s`. -We should encode these bytes as `r || s`, where `r` and `s` are both exactly -32 bytes long. +These bytes are encoded as `r || s`, where `r` and `s` are both exactly +32 bytes long, encoded big-endian. This is basically Ethereum's encoding, but without the leading recovery bit. ## Status From 66914925404a91b9620c41f5b6c3720baa923534 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 3 Aug 2018 17:49:46 -0700 Subject: [PATCH 5/5] (squash this) indicate what Ethereum does --- docs/architecture/adr-014-secp-malleability.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/architecture/adr-014-secp-malleability.md b/docs/architecture/adr-014-secp-malleability.md index f0db5743..3351056c 100644 --- a/docs/architecture/adr-014-secp-malleability.md +++ b/docs/architecture/adr-014-secp-malleability.md @@ -39,6 +39,8 @@ This is a simple check, instead of checking if `s < n`, instead check `s <= (n - An example of another cryptosystem using this is the parity definition here https://github.com/zkcrypto/pairing/pull/30#issuecomment-372910663. +This is the same solution Ethereum has chosen for solving secp malleability. + ## Proposed Implementation Fork https://github.com/btcsuite/btcd, and just update the [parse sig method](https://github.com/btcsuite/btcd/blob/master/btcec/signature.go#195) and serialize functions to enforce our canonical form.