Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 27 additions & 18 deletions BNB_HARDENING_INTEGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,32 @@
- BNB upstream head compared: `3f677ff761fcf692edb0243a5d812930844d879a`
- Common ancestor: `afbe264b44b63155a864dbe0171040c66e442963`
- Goal: port applicable security and correctness hardening without replacing Threshold's Paillier/NTilde remediation or weakening `ModProof`/`FactorProof`.
- Follow-up cleanup: this fork now retains only the ECDSA keygen/signing protocol surface used by Threshold/tBTC. EdDSA and resharing protocol packages, generated messages, and fixtures were removed to reduce unused attack surface.

## Compatibility Notice

This is a protocol/wire compatibility break for proof transcripts. Proofs whose Fiat-Shamir challenges now use tagged hashing or session context will not verify across mixed old/new versions, even where the Go API remains source-compatible through variadic arguments. Operators should roll this out as a coordinated protocol upgrade rather than mixing parties from before and after this PR in the same keygen, signing, or resharing ceremony.
This is a protocol/wire compatibility break for proof transcripts. Proofs whose Fiat-Shamir challenges now use tagged hashing or session context will not verify across mixed old/new versions, even where the Go API remains source-compatible through variadic arguments. Operators should roll this out as a coordinated protocol upgrade rather than mixing parties from before and after this PR in the same keygen or signing ceremony.

## Ported Or Manually Adapted

- `3d95e54` / PR `#252`, ECDSA protocol security updates: manually adapted tagged challenges, MtA/range-proof validation, session-context plumbing, and proof boundary checks while preserving Threshold's existing Paillier/NTilde proof model.
- `1a14f3a` / PR `#256`, ECDSA proof session byte: manually adapted proof-session APIs for DLN, Schnorr, MtA, Paillier mod proof, and factor proof. Public callers remain source-compatible through variadic session parameters, but generated proof transcripts are not wire-compatible with old versions.
- `ff989bf` / PR `#257`, tagged hash encoding: ported length-delimited tagged hashing as `common.SHA512_256i_TAGGED`.
- `f3aad28` / PR `#276`, nil `String()` panic: ported `BaseParty.String()` nil-round guard.
- `409542e` / PR `#282`, round update correctness: ported the `round.ok` accumulation fix for all non-terminal ECDSA/EdDSA keygen, signing, and resharing rounds, plus the resharing party-0 broadcast nil guard.
- `9acd90b`, `2f294cf`, `6b92e7d`, `c0de534` / PR `#284`, leading-zero message signing: manually adapted for ECDSA and EdDSA with variadic `fullBytesLen` parameters that are now required at runtime and bounded to the curve order byte length. EdDSA now also hashes the full-length message bytes in round 3.
- `409542e` / PR `#282`, round update correctness: ported the `round.ok` accumulation fix for all non-terminal ECDSA keygen and signing rounds.
- `9acd90b`, `2f294cf`, `6b92e7d`, `c0de534` / PR `#284`, leading-zero message signing: manually adapted for ECDSA with a variadic `fullBytesLen` parameter that is now required at runtime and bounded to the curve order byte length.
- `843de68` / PR `#291`, VSS threshold-size validation: ported `len(vs) == threshold+1` verification and added test coverage.
- `5d01446` / PR `#289`, range-proof update: ported MtA range-proof GCD, interval, lower-bound, non-one, and tagged challenge checks.
- `4878da5` / PR `#324`, VSS reconstruction fix: ported `threshold+1` reconstruction requirement and updated ECDSA/EdDSA keygen fixture tests.
- `4878da5` / PR `#324`, VSS reconstruction fix: ported `threshold+1` reconstruction requirement and updated ECDSA keygen fixture tests.
- `b59ed36`, session context for DLN and MtA proofs: manually adapted with optional session contexts and focused replay/session-mismatch tests.
- `fc38979`, GG20 SSID uniqueness: ported `tss.Parameters.SessionNonce` / `SetSessionNonce`, added `SetSessionNonceBytes` (requires session IDs of at least 16 bytes), and wired ECDSA/EdDSA keygen/signing plus ECDSA resharing SSID derivation. Keygen, ECDSA resharing, and signing now require a positive `SetSessionNonce` and fail closed if it is not set — the previous zero fallback (keygen/resharing) and SHA512_256(messageBytes) fallback (signing) caused two ceremonies with otherwise-identical inputs to derive the same SSID, breaking the session-binding property the proofs rely on.
- `fc38979`, GG20 SSID uniqueness: ported `tss.Parameters.SessionNonce` / `SetSessionNonce`, added `SetSessionNonceBytes` (requires session IDs of at least 16 bytes), and wired ECDSA keygen/signing SSID derivation. Keygen and signing now require a positive `SetSessionNonce` and fail closed if it is not set — the previous zero fallback (keygen) and SHA512_256(messageBytes) fallback (signing) caused two ceremonies with otherwise-identical inputs to derive the same SSID, breaking the session-binding property the proofs rely on.
- `685c2af`, canonical EC coordinates: ported rejection of EC coordinates outside `[0, P)`.
- `5d0d0f3`, EdDSA nil-pointer fix: ported by checking `NewECPoint` errors before `EightInvEight()`.
- Post-review cleanup: party-specific proof contexts now append fixed-width uint64 party indexes so party 0 does not collapse to the bare SSID. The earlier signing default that derived an SSID nonce from full message bytes has been removed in favour of the fail-closed requirement above; the helpers that produced it (`messageBytes`, `messageSessionNonce`) are gone with it.

## Already Covered Or Superseded

- `c84c096` / PR `#323`, modproof checker: Threshold's Paillier proof implementation already had the key Jacobi/non-prime validation from its GHSA-h24c-6p6p-m3vx remediation. Threshold's stronger `ModProof` coverage for both Paillier `N` and `NTilde` was kept.
- `e0e4299`, EdDSA keygen error aggregation: current Threshold code already had the corrected behavior.
- `e0e4299`, EdDSA keygen error aggregation: no longer applicable after EdDSA package removal.
- `0629cff`, `773b6af`, `b7b73a0`, `27922e0`, `4c83ace`, `c8136c9`, `8a87b22`, `f67a429`, `002397d`, `28d0622`, `bddf60d`: style, comment, logging, gofmt, minor optimization, or merge-only changes with no security behavior to port.

## Skipped
Expand All @@ -46,19 +46,29 @@ This is a protocol/wire compatibility break for proof transcripts. Proofs whose
## Semantic Differences From BNB

- Threshold's Paillier/NTilde `ModProof` and `FactorProof` remediation was retained. No BNB no-proof escape hatches were introduced.
- Session parameters were added as variadic arguments to preserve existing public call sites. This is API source-compatible for callers, but not wire-compatible for proof transcripts; callers must set `Parameters.SessionNonce()` before starting keygen, signing, or ECDSA resharing.
- ECDSA/EdDSA signing constructors still accept `fullBytesLen` as a variadic argument for source compatibility, but exactly one positive value is required at runtime so all signers agree on message byte width before the protocol starts.
- Keygen, signing, and ECDSA resharing SSIDs use `Parameters.SessionNonce()`. Callers must provide a unique agreed nonce, for example via `SetSessionNonceBytes`, for every ceremony.
- ECDSA resharing now broadcasts the locally-derived SSID in `DGRound1Message` so the new committee can reject old-committee broadcasts whose SSID differs from the local protocol context.
- Session parameters were added as variadic arguments to preserve existing public call sites. This is API source-compatible for callers, but not wire-compatible for proof transcripts; callers must set `Parameters.SessionNonce()` before starting keygen or signing.
- The ECDSA signing constructor still accepts `fullBytesLen` as a variadic argument for source compatibility, but exactly one positive value is required at runtime so all signers agree on message byte width before the protocol starts.
- Keygen and signing SSIDs use `Parameters.SessionNonce()`. Callers must provide a unique agreed nonce, for example via `SetSessionNonceBytes`, for every ceremony.
- `common.RejectionSample` keeps BNB's function name for porting clarity, but this implementation is modular reduction rather than a looping rejection sampler.
- Constant-time operations are not included and remain a residual follow-up.
- EdDSA and resharing protocol packages are intentionally removed in this fork because Threshold/tBTC does not use them.

## Removed Public Surface

The following public packages and symbols were removed in this fork. Downstream consumers upgrading from upstream BNB tss-lib or from an earlier Threshold fork can grep against this list to confirm they do not depend on the removed surface. Audit of `keep-core-security` confirmed it imports only ECDSA keygen/signing plus the shared `common`, `crypto`, and `tss` packages.

- Packages: `github.com/bnb-chain/tss-lib/eddsa/keygen`, `eddsa/signing`, `eddsa/resharing`, `ecdsa/resharing`.
- `tss.Ed25519` curve-name constant and the registered `Edwards()` curve.
- `tss.ReSharingParameters` and `tss.NewReSharingParameters`.
- `crypto.ECPoint.EightInvEight()` and the unexported `eight` / `eightInv` package-level constants.
- Proto definitions: `protob/ecdsa-resharing.proto`, `protob/eddsa-keygen.proto`, `protob/eddsa-signing.proto`, `protob/eddsa-resharing.proto`.

The legacy `IsToOldCommittee` / `IsToOldAndNewCommittees` fields on `tss.Message` and `MessageWrapper` are intentionally retained; this fork never sets them to `true`, but the wrapper field numbers stay reserved for proto wire-layout stability.

## Tests

- `go test ./crypto/... ./ecdsa/keygen ./ecdsa/signing ./eddsa/signing` passed.
- `go test ./eddsa/keygen ./eddsa/resharing` passed after updating EdDSA VSS threshold tests and resharing nil guard.
- `go test ./ecdsa/resharing` passed after the analogous resharing guard.
- `go test ./common ./crypto/paillier ./crypto/mta ./ecdsa/keygen ./ecdsa/resharing ./ecdsa/signing ./eddsa/keygen ./eddsa/signing ./eddsa/resharing` passed after review fixes.
- `go test ./crypto/... ./ecdsa/keygen ./ecdsa/signing` passed.
- `go test ./common ./crypto/paillier ./crypto/mta ./ecdsa/keygen ./ecdsa/signing` passed after review fixes.
- `go test ./...` passed.
- `go vet ./...` passed.

Expand All @@ -71,10 +81,9 @@ Added or updated focused tests cover:
- ProofBob and ProofBobWC session mismatch/replay failures.
- VSS `threshold+1` verification/reconstruction behavior.
- Non-canonical EC coordinate rejection.
- ECDSA and EdDSA leading-zero message signing.
- ECDSA leading-zero message signing.

## Residual Risks

- Applications must call `SetSessionNonce` or `SetSessionNonceBytes` before keygen, signing, and ECDSA resharing; those protocols now fail closed without it.
- Applications must call `SetSessionNonce` or `SetSessionNonceBytes` before keygen and signing; those protocols now fail closed without it.
- The optional constant-time upstream work is not integrated.
- EdDSA resharing has no SSID-bound proof transcript in this port.
3 changes: 1 addition & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ all: protob test

protob:
@echo "--> Building Protocol Buffers"
@for protocol in message signature ecdsa-keygen ecdsa-signing ecdsa-resharing eddsa-keygen eddsa-signing eddsa-resharing; do \
@for protocol in message signature ecdsa-keygen ecdsa-signing; do \
echo "Generating $$protocol.pb.go" ; \
protoc --go_out=. ./protob/$$protocol.proto ; \
done
Expand Down Expand Up @@ -43,4 +43,3 @@ pre_commit: build test
# # unless there is a reason not to.
# # https://www.gnu.org/software/make/manual/html_node/Phony-Targets.html
.PHONY: protob build test_unit test_unit_race test

35 changes: 7 additions & 28 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,17 @@ Permissively MIT Licensed.
Note! This is a library for developers. You may find a TSS tool that you can use with the Binance Chain CLI [here](https://docs.binance.org/tss.html).

## Introduction
This is an implementation of multi-party {t,n}-threshold ECDSA (Elliptic Curve Digital Signature Algorithm) based on Gennaro and Goldfeder CCS 2018 [1] and EdDSA (Edwards-curve Digital Signature Algorithm) following a similar approach.
This Threshold-maintained fork is an implementation of multi-party {t,n}-threshold ECDSA (Elliptic Curve Digital Signature Algorithm) based on Gennaro and Goldfeder CCS 2018 [1].

This library includes three protocols:
This fork includes the protocols used by Threshold/tBTC:

* Key Generation for creating secret shares with no trusted dealer ("keygen").
* Signing for using the secret shares to generate a signature ("signing").
* Dynamic Groups to change the group of participants while keeping the secret ("resharing").

⚠️ Do not miss [these important notes](#how-to-use-this-securely) on implementing this library securely

## Rationale
ECDSA is used extensively for crypto-currencies such as Bitcoin, Ethereum (secp256k1 curve), NEO (NIST P-256 curve) and many more.

EdDSA is used extensively for crypto-currencies such as Cardano, Aeternity, Stellar Lumens and many more.
ECDSA is used extensively for crypto-currencies such as Bitcoin, Ethereum (secp256k1 curve), NEO (NIST P-256 curve) and many more.

For such currencies this technique may be used to create crypto wallets where multiple parties must collaborate to sign transactions. See [MultiSig Use Cases](https://en.bitcoin.it/wiki/Multisignature#Multisignature_Applications)

Expand All @@ -39,7 +36,7 @@ There is also a performance bonus in that blockchain nodes may check the validit
## Usage
You should start by creating an instance of a `LocalParty` and giving it the arguments that it needs.

The `LocalParty` that you use should be from the `keygen`, `signing` or `resharing` package depending on what you want to do.
The `LocalParty` that you use should be from the `keygen` or `signing` package depending on what you want to do.

### Setup
```go
Expand All @@ -57,11 +54,8 @@ parties := tss.SortPartyIDs(getParticipantPartyIDs())
thisParty := tss.NewPartyID(id, moniker, uniqueKey)
ctx := tss.NewPeerContext(parties)

// Select an elliptic curve
// use ECDSA
// Select an elliptic curve.
curve := tss.S256()
// or use EdDSA
// curve := tss.Edwards()

params := tss.NewParameters(curve, ctx, thisParty, len(parties), threshold)

Expand Down Expand Up @@ -97,21 +91,6 @@ go func() {
}()
```

### Re-Sharing
Use the `resharing.LocalParty` to re-distribute the secret shares. The save data received through the `endCh` should overwrite the existing key data in storage, or write new data if the party is receiving a new share.

Please note that `ReSharingParameters` is used to give this Party more context about the re-sharing that should be carried out.

```go
party := resharing.NewLocalParty(params, ourKeyData, outCh, endCh)
go func() {
err := party.Start()
// handle err ...
}()
```

⚠️ During re-sharing the key data may be modified during the rounds. Do not ever overwrite any data saved on disk until the final struct has been received through the `end` channel.

## Messaging
In these examples the `outCh` will collect outgoing messages from the party and the `endCh` will receive save data or a signature when the protocol is complete.

Expand Down Expand Up @@ -145,7 +124,7 @@ The transport for messaging is left to the application layer and is not provided

When you build a transport, it should offer a broadcast channel as well as point-to-point channels connecting every pair of parties. Your transport should also employ suitable end-to-end encryption (TLS with an [AEAD cipher](https://en.wikipedia.org/wiki/Authenticated_encryption#Authenticated_encryption_with_associated_data_(AEAD)) is recommended) between parties to ensure that a party can only read the messages sent to it.

Within your transport, each message should be wrapped with a **session ID** that is unique to a single run of the keygen, signing or re-sharing rounds. This session ID should be agreed upon out-of-band and known only by the participating parties before the rounds begin. Upon receiving any message, your program should make sure that the received session ID matches the one that was agreed upon at the start.
Within your transport, each message should be wrapped with a **session ID** that is unique to a single run of the keygen or signing rounds. This session ID should be agreed upon out-of-band and known only by the participating parties before the rounds begin. Upon receiving any message, your program should make sure that the received session ID matches the one that was agreed upon at the start.

The same session ID should be bound into the protocol parameters before constructing local parties:

Expand All @@ -154,7 +133,7 @@ params := tss.NewParameters(curve, ctx, thisParty, len(parties), threshold)
params.SetSessionNonceBytes([]byte(sessionID))
```

All parties in the run must use the same high-entropy session ID of at least 16 bytes, and it must be unique to the ceremony. Keygen, signing, and ECDSA re-sharing fail closed if no session nonce is set; reusing a session ID across otherwise identical ceremonies reintroduces transcript-splicing risk.
All parties in the run must use the same high-entropy session ID of at least 16 bytes, and it must be unique to the ceremony. Keygen and signing fail closed if no session nonce is set; reusing a session ID across otherwise identical ceremonies reintroduces transcript-splicing risk.

Additionally, there should be a mechanism in your transport to allow for "reliable broadcasts", meaning parties can broadcast a message to other parties such that it's guaranteed that each one receives the same message. There are several examples of algorithms online that do this by sharing and comparing hashes of received messages.

Expand Down
15 changes: 0 additions & 15 deletions crypto/ecpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import (
"fmt"
"math/big"

"github.com/decred/dcrd/dcrec/edwards/v2"

"github.com/bnb-chain/tss-lib/tss"
)

Expand All @@ -27,11 +25,6 @@ type ECPoint struct {
coords [2]*big.Int
}

var (
eight = big.NewInt(8)
eightInv = new(big.Int).ModInverse(eight, edwards.Edwards().Params().N)
)

// Creates a new ECPoint and checks that the given coordinates are on the elliptic curve.
func NewECPoint(curve elliptic.Curve, X, Y *big.Int) (*ECPoint, error) {
if !isOnCurve(curve, X, Y) {
Expand Down Expand Up @@ -109,14 +102,6 @@ func (p *ECPoint) ValidateBasic() bool {
return p != nil && p.coords[0] != nil && p.coords[1] != nil && p.IsOnCurve()
}

func (p *ECPoint) EightInvEight() *ECPoint {
q := p.ScalarMult(eight)
if q == nil {
return nil
}
return q.ScalarMult(eightInv)
}

func ScalarBaseMult(curve elliptic.Curve, k *big.Int) *ECPoint {
if curve == nil || k == nil {
return nil
Expand Down
16 changes: 6 additions & 10 deletions crypto/ecpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
package crypto_test

import (
"crypto/elliptic"
"encoding/hex"
"encoding/json"
"math/big"
"reflect"
"testing"

"github.com/btcsuite/btcd/btcec"
"github.com/decred/dcrd/dcrec/edwards/v2"
"github.com/stretchr/testify/assert"

. "github.com/bnb-chain/tss-lib/crypto"
Expand Down Expand Up @@ -163,16 +163,12 @@ func TestS256EcpointJsonSerialization(t *testing.T) {
assert.True(t, reflect.TypeOf(point.Curve()) == reflect.TypeOf(umpoint.Curve()))
}

func TestEdwardsEcpointJsonSerialization(t *testing.T) {
ec := edwards.Edwards()
tss.RegisterCurve("ed25519", ec)
func TestP256EcpointJsonSerialization(t *testing.T) {
ec := elliptic.P256()
tss.RegisterCurve("p256", ec)

pubKeyBytes, err := hex.DecodeString("ae1e5bf5f3d6bf58b5c222088671fcbe78b437e28fae944c793897b26091f249")
assert.NoError(t, err)
pbk, err := edwards.ParsePubKey(pubKeyBytes)
assert.NoError(t, err)

point, err := NewECPoint(ec, pbk.X, pbk.Y)
x, y := ec.ScalarBaseMult(big.NewInt(1).Bytes())
point, err := NewECPoint(ec, x, y)
assert.NoError(t, err)
bz, err := json.Marshal(point)
assert.NoError(t, err)
Expand Down
3 changes: 2 additions & 1 deletion crypto/mta/share_protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package mta

import (
"context"
"crypto/elliptic"
"math/big"
"testing"
"time"
Expand Down Expand Up @@ -138,7 +139,7 @@ func TestShareProtocolWC(t *testing.T) {
badV.V = big.NewInt(0)
assert.False(t, badV.Verify(tss.EC(), pk, NTildei, h1i, h2i, cA, cB, gBPoint), "V equal to zero must fail")

wrongCurveX := crypto.NewECPointNoCurveCheck(tss.Edwards(), gBPoint.X(), gBPoint.Y())
wrongCurveX := crypto.NewECPointNoCurveCheck(elliptic.P256(), gBPoint.X(), gBPoint.Y())
assert.False(t, pfB.Verify(tss.EC(), pk, NTildei, h1i, h2i, cA, cB, wrongCurveX), "X on a different curve must fail")

alpha, err := AliceEndWC(tss.EC(), pk, pfB, gBPoint, cA, cB, NTildei, h1i, h2i, sk)
Expand Down
Loading