From 0957d9dc737d93d53bff2c2a41af1805ed40f15d Mon Sep 17 00:00:00 2001 From: maclane Date: Thu, 21 May 2026 11:46:16 -0500 Subject: [PATCH 1/6] Address residual review items from BNB hardening stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Doc + small defensive-code follow-ups to the PR #2/#4/#5 review thread. None of these are security-blocking; they close out the cosmetic and pre-existing items that did not warrant their own PR earlier. - common/hash_utils.go: strengthen RejectionSample doc so the name does not mislead future readers — the function is modular reduction, not true rejection sampling, and the bias bound depends on q vs the hash width. - crypto/paillier/factor_proof.go: document that the tagged and legacy FactorChallenge paths emit different challenge distributions (positive-only vs signed), and note that FactorVerify's CmpAbs bounds exist to accommodate the legacy signed encoding. - tss/params.go: expand SetSessionNonceBytes docstring with the collision/uniqueness/entropy guidance reviewers asked for. - ecdsa/{keygen,signing}/rounds.go: document the round-1-capture invariant for getSSID so future refactors do not silently drift the hashed round.number domain separator. - crypto/ecpoint.go: flag SetCurve's in-place mutation in the doc comment; the chained-call style is a footgun on shared points. - crypto/vss/feldman_vss.go: reject nil shares, nil/zero share IDs, and duplicate IDs in ReConstruct before the Lagrange loop, so malformed inputs return an error instead of panicking through ModInverse(0). Add focused test coverage for each rejection path. Co-Authored-By: Claude Opus 4.7 (1M context) --- common/hash_utils.go | 14 ++++-- crypto/ecpoint.go | 5 ++ crypto/paillier/factor_proof.go | 13 ++++++ crypto/vss/feldman_vss.go | 30 ++++++++++-- crypto/vss/feldman_vss_test.go | 82 +++++++++++++++++++++++++++++++++ ecdsa/keygen/rounds.go | 7 +++ ecdsa/signing/rounds.go | 7 +++ tss/params.go | 27 +++++++---- 8 files changed, 167 insertions(+), 18 deletions(-) diff --git a/common/hash_utils.go b/common/hash_utils.go index 616a1ffd..e0587d89 100644 --- a/common/hash_utils.go +++ b/common/hash_utils.go @@ -22,9 +22,17 @@ func LiterallyJustMod(q *big.Int, eHash *big.Int) *big.Int { // e' = eHash } // RejectionSample preserves the upstream challenge-reduction function name. -// This implementation reduces the hash modulo q rather than looping with fresh -// hash material, so callers must only use it where modular-reduction bias is -// acceptable for the proof challenge. +// THIS IS NOT TRUE REJECTION SAMPLING: it reduces the hash modulo q rather +// than looping with fresh hash material until the candidate falls in [0, q). +// +// The bias of `eHash mod q` is ≤ q / 2^eHash.BitLen(). For SHA512_256-derived +// hashes (256 bits) and curve orders q close to 2^256 (secp256k1, curve25519), +// the bias is ≤ 2^-128 — negligible for Fiat-Shamir challenges. For q +// significantly smaller than 2^eHash.BitLen() (e.g., q = 2^256 reduced from a +// 256-bit hash) the bias is exactly 0. For q that is NOT close to a power of +// 2 (or for any application requiring an unbiased uniform sample over [0, q)), +// callers must not use this function — implement true rejection sampling at +// the call site instead. func RejectionSample(q *big.Int, eHash *big.Int) *big.Int { return LiterallyJustMod(q, eHash) } diff --git a/crypto/ecpoint.go b/crypto/ecpoint.go index 6e16c9d0..7cedaba7 100644 --- a/crypto/ecpoint.go +++ b/crypto/ecpoint.go @@ -90,6 +90,11 @@ func (p *ECPoint) Equals(p2 *ECPoint) bool { return p.X().Cmp(p2.X()) == 0 && p.Y().Cmp(p2.Y()) == 0 } +// SetCurve mutates the receiver's curve field in place and returns the same +// pointer. The chained-call style (`p.SetCurve(ec).ScalarMult(k)`) reads as +// fluent but is a footgun when p is shared — every alias observes the new +// curve. Callers that need to ensure the curve without mutating a shared point +// should construct a fresh ECPoint via NewECPoint instead. func (p *ECPoint) SetCurve(curve elliptic.Curve) *ECPoint { if p == nil { return nil diff --git a/crypto/paillier/factor_proof.go b/crypto/paillier/factor_proof.go index 5ec95e1a..d4de4a2c 100644 --- a/crypto/paillier/factor_proof.go +++ b/crypto/paillier/factor_proof.go @@ -177,6 +177,19 @@ func (pf FactorProof) FactorVerify(pkN, N, s, t *big.Int, session ...[]byte) (bo return true, nil } +// FactorChallenge derives the Fiat-Shamir challenge for the no-small-factor +// proof. The two paths emit different challenge distributions; both are +// internally consistent because prover and verifier always traverse the same +// branch for a given proof, but the absolute-value bounds on Z1/Z2/W1/W2 in +// FactorVerify (which use CmpAbs) accommodate the signed legacy challenge. +// +// - Tagged path (session != nil): e ∈ [0, 2^256), derived via +// SHA512_256i_TAGGED + modular reduction. Honest responses Z1/Z2 = a+e*p, +// b+e*q are positive. +// - Legacy path (session absent): e ∈ [-(2^256-1), 2^256), derived via +// HashToN(2q-1, …) − (q-1). Honest responses can be negative; this is the +// historical Threshold encoding, kept for wire-compat with non-session +// callers. func FactorChallenge(N, s, t, pkN, P, Q, A, B, T, sigma *big.Int, session ...[]byte) *big.Int { q := big.NewInt(1) q = q.Lsh(q, 256) // q = 2^256 diff --git a/crypto/vss/feldman_vss.go b/crypto/vss/feldman_vss.go index 736e8e4e..2fc17123 100644 --- a/crypto/vss/feldman_vss.go +++ b/crypto/vss/feldman_vss.go @@ -134,14 +134,34 @@ func (shares Shares) ReConstruct(ec elliptic.Curve) (secret *big.Int, err error) if len(shares) == 0 { return nil, ErrNumSharesBelowThreshold } - if shares != nil && shares[0].Threshold+1 > len(shares) { + if shares[0] == nil { + return nil, errors.New("vss reconstruct: nil share") + } + if shares[0].Threshold+1 > len(shares) { return nil, ErrNumSharesBelowThreshold } - modN := common.ModInt(ec.Params().N) - - // x coords - xs := make([]*big.Int, 0) + q := ec.Params().N + modN := common.ModInt(q) + + // x coords. Reject zero or duplicate share IDs (mod q) up front: a zero + // ID encodes the secret directly, and two equal IDs make the Lagrange + // denominator xs[j]-share.ID zero, which would otherwise propagate into + // ModInverse(0) → nil → nil-deref in the interpolation loop below. + xs := make([]*big.Int, 0, len(shares)) + seen := make(map[string]struct{}, len(shares)) for _, share := range shares { + if share == nil || share.ID == nil || share.Share == nil { + return nil, errors.New("vss reconstruct: nil share or share field") + } + id := new(big.Int).Mod(share.ID, q) + if id.Sign() == 0 { + return nil, errors.New("vss reconstruct: share ID is zero mod q") + } + key := id.String() + if _, dup := seen[key]; dup { + return nil, fmt.Errorf("vss reconstruct: duplicate share ID %s", key) + } + seen[key] = struct{}{} xs = append(xs, share.ID) } diff --git a/crypto/vss/feldman_vss_test.go b/crypto/vss/feldman_vss_test.go index 8192891d..cfcbe030 100644 --- a/crypto/vss/feldman_vss_test.go +++ b/crypto/vss/feldman_vss_test.go @@ -169,3 +169,85 @@ func TestReconstruct(t *testing.T) { assert.NotZero(t, secret4) assert.Zero(t, secret.Cmp(secret4)) } + +// TestReconstructRejectsMalformedShares pins ReConstruct's input validation: +// nil share, nil ID, nil Share, zero-mod-q ID, and duplicate IDs must all be +// rejected up front instead of propagating into ModInverse(0) → nil-deref in +// the Lagrange interpolation loop. +func TestReconstructRejectsMalformedShares(t *testing.T) { + num, threshold := 5, 3 + q := tss.EC().Params().N + + secret := common.GetRandomPositiveInt(q) + ids := make([]*big.Int, 0, num) + for i := 0; i < num; i++ { + ids = append(ids, common.GetRandomPositiveInt(q)) + } + _, shares, err := Create(tss.EC(), threshold, secret, ids) + assert.NoError(t, err) + + cases := []struct { + name string + mutate func(Shares) Shares + }{ + { + name: "nil share entry", + mutate: func(in Shares) Shares { + out := append(Shares(nil), in...) + out[1] = nil + return out + }, + }, + { + name: "nil ID", + mutate: func(in Shares) Shares { + out := append(Shares(nil), in...) + bad := *in[1] + bad.ID = nil + out[1] = &bad + return out + }, + }, + { + name: "nil Share", + mutate: func(in Shares) Shares { + out := append(Shares(nil), in...) + bad := *in[1] + bad.Share = nil + out[1] = &bad + return out + }, + }, + { + name: "zero ID mod q", + mutate: func(in Shares) Shares { + out := append(Shares(nil), in...) + bad := *in[1] + bad.ID = new(big.Int).Set(q) // q mod q == 0 + out[1] = &bad + return out + }, + }, + { + name: "duplicate ID", + mutate: func(in Shares) Shares { + out := append(Shares(nil), in...) + dup := *in[0] + dup.ID = new(big.Int).Set(in[1].ID) + out[0] = &dup + return out + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + mutated := tc.mutate(shares[:threshold+1]) + assert.NotPanics(t, func() { + got, err := mutated.ReConstruct(tss.EC()) + assert.Error(t, err) + assert.Nil(t, got) + }) + }) + } +} diff --git a/ecdsa/keygen/rounds.go b/ecdsa/keygen/rounds.go index 9d9c1858..85c0a24e 100644 --- a/ecdsa/keygen/rounds.go +++ b/ecdsa/keygen/rounds.go @@ -98,6 +98,13 @@ func (round *base) resetOK() { } } +// getSSID derives the session-binding identifier for keygen. +// +// Callers must invoke this exactly once, in round 1, and store the result in +// round.temp.ssid for the rest of the protocol — round.number is hashed in +// here as a domain separator, so calling it from a later round would produce a +// different SSID that no peer would agree with. The current call site is +// round1.Start; if you move it, make sure round.number is still 1 at the call. func (round *base) getSSID() []byte { ssidList := []*big.Int{ round.EC().Params().P, diff --git a/ecdsa/signing/rounds.go b/ecdsa/signing/rounds.go index 82f7d6b6..a5856689 100644 --- a/ecdsa/signing/rounds.go +++ b/ecdsa/signing/rounds.go @@ -125,6 +125,13 @@ func (round *base) resetOK() { } } +// getSSID derives the session-binding identifier for signing. +// +// Callers must invoke this exactly once, in round 1, and store the result in +// round.temp.ssid for the rest of the protocol — round.number is hashed in +// here as a domain separator, so calling it from a later round would produce a +// different SSID that no peer would agree with. The current call site is +// round1.Start; if you move it, make sure round.number is still 1 at the call. func (round *base) getSSID() ([]byte, error) { ssidList := []*big.Int{ round.EC().Params().P, diff --git a/tss/params.go b/tss/params.go index 0ee4bc7c..094ef307 100644 --- a/tss/params.go +++ b/tss/params.go @@ -109,17 +109,24 @@ func (params *Parameters) SetSessionNonce(nonce *big.Int) { } // SetSessionNonceBytes hashes an application-level session ID into the -// per-session nonce. All parties must call it with the same high-entropy -// session ID before constructing local parties for a protocol run. It panics if -// the session ID is shorter than 16 bytes. +// per-session nonce. All parties must call it with the same session ID before +// constructing local parties for a protocol run. // -// The 16-byte minimum is a floor that catches obvious misuse (empty input, a -// short ASCII tag); it is not a sufficient condition. Callers must supply at -// least 128 bits of true randomness. A counter, timestamp, or other -// low-entropy 16-byte value passes the length check but defeats the -// session-binding property that the proofs rely on. Prefer a freshly drawn -// random session ID from a CSPRNG, or a high-entropy ceremony identifier -// negotiated out of band. +// The session ID must be: +// +// - At least 16 bytes (enforced by panic). 16 bytes = 128 bits is the +// birthday-bound minimum below which random collisions become plausible. +// - Unique per ceremony. Reusing the same session ID across two distinct +// ceremonies on the same inputs reintroduces the transcript-splicing risk +// that the session-binding contract is meant to prevent. +// - Drawn from a high-entropy source for collision resistance. The bytes are +// hashed through SHA512_256 to a 256-bit nonce; if the application uses +// structured IDs (timestamps, counters, slot numbers), prefer concatenating +// them with a per-ceremony random seed so two ceremonies cannot collide +// under the hash. +// +// Callers that already have an unpredictable big.Int (e.g., a draw from a +// CSPRNG) can pass it through SetSessionNonce directly instead. func (params *Parameters) SetSessionNonceBytes(sessionID []byte) { if len(sessionID) < 16 { panic("tss: session ID must be at least 16 bytes") From 6a1e6e9e7b68e356d52ada77dad7b7803d5934a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Sat, 23 May 2026 13:24:35 +0000 Subject: [PATCH 2/6] Correct RejectionSample bias docstring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous text stated the bias bound as `q / 2^eHash.BitLen()`, which evaluates to ≈ 1 for secp256k1 and cannot support the docstring's own ≈ 2^-128 conclusion. The "q significantly smaller than 2^eHash.BitLen() (e.g., q = 2^256)" example was also internally inconsistent (q = 2^256 is not smaller than 2^256). Rewrite the paragraph to: - State the safe regime as a property of q ("close to 2^k from below") rather than via a loose formula or call-site enumeration. - Drop the unused curve25519 reference whose conclusion is correct but not derivable from the simple `r/M` bound. - Cross-reference HashToN / HashToNTagged for the large-modulus regime that they were introduced to address. Documentation-only change. RejectionSample's behavior is unchanged (LiterallyJustMod under the hood), so no computed challenge value moves. --- common/hash_utils.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/common/hash_utils.go b/common/hash_utils.go index e0587d89..329a8464 100644 --- a/common/hash_utils.go +++ b/common/hash_utils.go @@ -21,18 +21,19 @@ func LiterallyJustMod(q *big.Int, eHash *big.Int) *big.Int { // e' = eHash return e } -// RejectionSample preserves the upstream challenge-reduction function name. -// THIS IS NOT TRUE REJECTION SAMPLING: it reduces the hash modulo q rather -// than looping with fresh hash material until the candidate falls in [0, q). +// RejectionSample reduces `eHash` modulo q. The name preserves the upstream +// challenge-derivation function name; this is NOT true rejection sampling +// (no loop with fresh hash material until a candidate falls in [0, q)). // -// The bias of `eHash mod q` is ≤ q / 2^eHash.BitLen(). For SHA512_256-derived -// hashes (256 bits) and curve orders q close to 2^256 (secp256k1, curve25519), -// the bias is ≤ 2^-128 — negligible for Fiat-Shamir challenges. For q -// significantly smaller than 2^eHash.BitLen() (e.g., q = 2^256 reduced from a -// 256-bit hash) the bias is exactly 0. For q that is NOT close to a power of -// 2 (or for any application requiring an unbiased uniform sample over [0, q)), -// callers must not use this function — implement true rejection sampling at -// the call site instead. +// Safe only when q is close to 2^k from below, where k is the hash output +// width in bits (k = 256 for SHA512_256). For secp256k1 the bias is ≈ 2^-128 +// — negligible for Fiat-Shamir challenges. +// +// For q that is NOT close to a power of 2, or for moduli larger than 2^k +// (e.g. Paillier N ≈ 2^2048), use HashToN / HashToNTagged: those absorb +// ≥ k + 256 bits of entropy before reduction and bound bias at ≤ 2^-256 +// regardless of q. For applications requiring an unbiased uniform sample +// over [0, q), implement true rejection sampling at the call site. func RejectionSample(q *big.Int, eHash *big.Int) *big.Int { return LiterallyJustMod(q, eHash) } From 64a2e5c0c6e3d56c802499506369c505e826f73c Mon Sep 17 00:00:00 2001 From: maclane Date: Thu, 28 May 2026 12:58:17 -0500 Subject: [PATCH 3/6] Port BNB hardening follow-ups --- common/hash_utils.go | 10 ++++- common/int.go | 4 ++ common/random.go | 4 +- common/random_test.go | 6 +++ common/validation.go | 38 ++++++++++++++++++ crypto/commitments/commitment.go | 8 ++++ crypto/dlnproof/proof.go | 30 +++++++-------- crypto/ecpoint.go | 12 +++++- crypto/mta/proofs.go | 50 ++++++++++++++---------- crypto/mta/range_proof.go | 46 +++++++++++++++++----- crypto/paillier/factor_proof.go | 25 ++++++++---- crypto/paillier/mod_proof.go | 56 ++++++++++++++++++++------- crypto/paillier/paillier.go | 24 ++++++++++-- crypto/schnorr/schnorr_proof.go | 41 +++++++++++++++----- crypto/vss/feldman_vss.go | 28 +++++++++++--- ecdsa/keygen/local_party.go | 21 +++++++++- ecdsa/keygen/local_party_test.go | 4 +- ecdsa/signing/local_party.go | 39 ++++++++++++++++++- ecdsa/signing/local_party_test.go | 4 +- ecdsa/signing/prepare.go | 19 ++++----- ecdsa/signing/round_1.go | 5 ++- tss/message.go | 15 ++++++++ tss/params.go | 33 ++++++++++++++++ tss/params_test.go | 64 +++++++++++++++++++++++++++---- tss/party_id.go | 7 +++- 25 files changed, 481 insertions(+), 112 deletions(-) create mode 100644 common/validation.go diff --git a/common/hash_utils.go b/common/hash_utils.go index 329a8464..2df8290f 100644 --- a/common/hash_utils.go +++ b/common/hash_utils.go @@ -34,10 +34,18 @@ func LiterallyJustMod(q *big.Int, eHash *big.Int) *big.Int { // e' = eHash // ≥ k + 256 bits of entropy before reduction and bound bias at ≤ 2^-256 // regardless of q. For applications requiring an unbiased uniform sample // over [0, q), implement true rejection sampling at the call site. -func RejectionSample(q *big.Int, eHash *big.Int) *big.Int { +func ModReduceHash(q *big.Int, eHash *big.Int) *big.Int { return LiterallyJustMod(q, eHash) } +// RejectionSample is kept for compatibility with older callers. +// +// Deprecated: use ModReduceHash. This function is modular reduction, not true +// rejection sampling. +func RejectionSample(q *big.Int, eHash *big.Int) *big.Int { + return ModReduceHash(q, eHash) +} + // Return a big.Int between 0 and N func HashToN(N *big.Int, in ...*big.Int) *big.Int { bitCnt := N.BitLen() diff --git a/common/int.go b/common/int.go index ad487c51..e7c01604 100644 --- a/common/int.go +++ b/common/int.go @@ -105,6 +105,10 @@ func IsInInterval(b *big.Int, bound *big.Int) bool { return b != nil && bound != nil && b.Cmp(bound) < 0 && b.Cmp(zero) >= 0 } +func IsInIntervalPositive(b *big.Int, bound *big.Int) bool { + return b != nil && bound != nil && b.Cmp(bound) < 0 && b.Sign() > 0 +} + func AppendBigIntToBytesSlice(commonBytes []byte, appended *big.Int) []byte { resultBytes := make([]byte, len(commonBytes), len(commonBytes)+len(appended.Bytes())) copy(resultBytes, commonBytes) diff --git a/common/random.go b/common/random.go index 6a247d29..acda0451 100644 --- a/common/random.go +++ b/common/random.go @@ -36,13 +36,13 @@ func MustGetRandomInt(bits int) *big.Int { } func GetRandomPositiveInt(lessThan *big.Int) *big.Int { - if lessThan == nil || zero.Cmp(lessThan) != -1 { + if lessThan == nil || lessThan.Cmp(one) <= 0 { return nil } var try *big.Int for { try = MustGetRandomInt(lessThan.BitLen()) - if try.Cmp(lessThan) < 0 && try.Cmp(zero) >= 0 { + if try.Cmp(lessThan) < 0 && try.Sign() > 0 { break } } diff --git a/common/random_test.go b/common/random_test.go index 4a9eb3c8..c1d51c8d 100644 --- a/common/random_test.go +++ b/common/random_test.go @@ -31,6 +31,12 @@ func TestGetRandomPositiveInt(t *testing.T) { assert.True(t, rndPos.Cmp(big.NewInt(0)) == 1, "rand int should be positive") } +func TestGetRandomPositiveIntRejectsNoPositiveRange(t *testing.T) { + assert.Nil(t, common.GetRandomPositiveInt(nil)) + assert.Nil(t, common.GetRandomPositiveInt(big.NewInt(0))) + assert.Nil(t, common.GetRandomPositiveInt(big.NewInt(1))) +} + func TestGetRandomPositiveRelativelyPrimeInt(t *testing.T) { rnd := common.MustGetRandomInt(randomIntBitLen) rndPosRP := common.GetRandomPositiveRelativelyPrimeInt(rnd) diff --git a/common/validation.go b/common/validation.go new file mode 100644 index 00000000..3cf09e14 --- /dev/null +++ b/common/validation.go @@ -0,0 +1,38 @@ +// Copyright © 2019 Binance +// +// This file is part of Binance. The full Binance copyright notice, including +// terms governing use, modification, and redistribution, is contained in the +// file LICENSE at the root of the source code distribution tree. + +package common + +import "math/big" + +const primalityRounds = 30 + +func IsUsableUnknownOrderModulus(N *big.Int, minBitLen int) bool { + return N != nil && + N.Sign() == 1 && + N.Bit(0) == 1 && + N.BitLen() >= minBitLen && + !N.ProbablyPrime(primalityRounds) +} + +func IsCanonicalGenerator(N, v *big.Int) bool { + return N != nil && + N.Sign() == 1 && + v != nil && + v.Cmp(one) > 0 && + v.Cmp(N) < 0 && + IsNumberInMultiplicativeGroup(N, v) +} + +func IsCanonicalPaillierCiphertext(c, N *big.Int) bool { + if c == nil || N == nil || N.Sign() != 1 { + return false + } + NSquared := new(big.Int).Mul(N, N) + return c.Sign() > 0 && + c.Cmp(NSquared) < 0 && + new(big.Int).GCD(nil, nil, c, N).Cmp(one) == 0 +} diff --git a/crypto/commitments/commitment.go b/crypto/commitments/commitment.go index b58a3ced..501a576c 100644 --- a/crypto/commitments/commitment.go +++ b/crypto/commitments/commitment.go @@ -53,10 +53,18 @@ func NewHashDeCommitmentFromBytes(marshalled [][]byte) HashDeCommitment { } func (cmt *HashCommitDecommit) Verify() bool { + if cmt == nil { + return false + } C, D := cmt.C, cmt.D if C == nil || D == nil { return false } + for _, part := range D { + if part == nil { + return false + } + } hash := common.SHA512_256i(D...) return hash.Cmp(C) == 0 } diff --git a/crypto/dlnproof/proof.go b/crypto/dlnproof/proof.go index bf9b8aad..14215214 100644 --- a/crypto/dlnproof/proof.go +++ b/crypto/dlnproof/proof.go @@ -18,7 +18,15 @@ import ( "github.com/bnb-chain/tss-lib/common" ) -const Iterations = 128 +const ( + Iterations = 128 + verifyMinModulusBitLen = 2048 + fsDomainTagDLNProof = "tss-lib.threshold.dlnproof" +) + +func fsSessionDLNProof(session []byte) []byte { + return append([]byte(fsDomainTagDLNProof+"|"), session...) +} type ( Proof struct { @@ -42,7 +50,7 @@ func NewDLNProof(h1, h2, x, p, q, N *big.Int, session ...[]byte) *Proof { alpha[i] = modN.Exp(h1, a[i]) } msg := append([]*big.Int{h1, h2, N}, alpha[:]...) - c := common.SHA512_256i_TAGGED(Session, msg...) + c := common.SHA512_256i_TAGGED(fsSessionDLNProof(Session), msg...) t := [Iterations]*big.Int{} cIBI := new(big.Int) for i := range t { @@ -58,22 +66,14 @@ func (p *Proof) Verify(h1, h2, N *big.Int, session ...[]byte) bool { if p == nil { return false } - if h1 == nil || h2 == nil || N == nil || N.Sign() != 1 { + if !common.IsUsableUnknownOrderModulus(N, verifyMinModulusBitLen) { return false } modN := common.ModInt(N) - h1_ := new(big.Int).Mod(h1, N) - if h1_.Cmp(one) != 1 || h1_.Cmp(N) != -1 { - return false - } - h2_ := new(big.Int).Mod(h2, N) - if h2_.Cmp(one) != 1 || h2_.Cmp(N) != -1 { - return false - } - if h1_.Cmp(h2_) == 0 { + if !common.IsCanonicalGenerator(N, h1) || !common.IsCanonicalGenerator(N, h2) { return false } - if !common.Coprime(h1_, N) || !common.Coprime(h2_, N) { + if h1.Cmp(h2) == 0 { return false } for i := range p.T { @@ -82,12 +82,12 @@ func (p *Proof) Verify(h1, h2, N *big.Int, session ...[]byte) bool { } } for i := range p.Alpha { - if p.Alpha[i] == nil || p.Alpha[i].Cmp(one) <= 0 || p.Alpha[i].Cmp(N) >= 0 { + if !common.IsCanonicalGenerator(N, p.Alpha[i]) { return false } } msg := append([]*big.Int{h1, h2, N}, p.Alpha[:]...) - c := common.SHA512_256i_TAGGED(Session, msg...) + c := common.SHA512_256i_TAGGED(fsSessionDLNProof(Session), msg...) cIBI := new(big.Int) for i := 0; i < Iterations; i++ { cI := c.Bit(i) diff --git a/crypto/ecpoint.go b/crypto/ecpoint.go index 7cedaba7..8761f86c 100644 --- a/crypto/ecpoint.go +++ b/crypto/ecpoint.go @@ -104,7 +104,17 @@ func (p *ECPoint) SetCurve(curve elliptic.Curve) *ECPoint { } func (p *ECPoint) ValidateBasic() bool { - return p != nil && p.coords[0] != nil && p.coords[1] != nil && p.IsOnCurve() + return p != nil && p.coords[0] != nil && p.coords[1] != nil && p.IsOnCurve() && !p.IsIdentity() +} + +func (p *ECPoint) IsIdentity() bool { + if p == nil || p.coords[0] == nil || p.coords[1] == nil { + return false + } + if p.coords[0].Sign() != 0 { + return false + } + return p.coords[1].Sign() == 0 || p.coords[1].Cmp(big.NewInt(1)) == 0 } func ScalarBaseMult(curve elliptic.Curve, k *big.Int) *ECPoint { diff --git a/crypto/mta/proofs.go b/crypto/mta/proofs.go index 0128a9f0..0cd40498 100644 --- a/crypto/mta/proofs.go +++ b/crypto/mta/proofs.go @@ -38,7 +38,7 @@ type ( // an absent `X` generates the proof without the X consistency check X = g^x func ProveBobWC(ec elliptic.Curve, pk *paillier.PublicKey, NTilde, h1, h2, c1, c2, x, y, r *big.Int, X *crypto.ECPoint, session ...[]byte) (*ProofBobWC, error) { Session := optionalProofSession(session) - if pk == nil || NTilde == nil || h1 == nil || h2 == nil || c1 == nil || c2 == nil || x == nil || y == nil || r == nil { + if ec == nil || pk == nil || NTilde == nil || h1 == nil || h2 == nil || c1 == nil || c2 == nil || x == nil || y == nil || r == nil { return nil, errors.New("ProveBob() received a nil argument") } @@ -103,11 +103,11 @@ func ProveBobWC(ec elliptic.Curve, pk *paillier.PublicKey, NTilde, h1, h2, c1, c var eHash *big.Int // X is nil if called by ProveBob (Bob's proof "without check") if X == nil { - eHash = common.SHA512_256i_TAGGED(Session, append(pk.AsInts(), NTilde, h1, h2, c1, c2, z, zPrm, t, v, w)...) + eHash = common.SHA512_256i_TAGGED(fsSessionBob(Session), append(pk.AsInts(), NTilde, h1, h2, c1, c2, z, zPrm, t, v, w)...) } else { - eHash = common.SHA512_256i_TAGGED(Session, append(pk.AsInts(), NTilde, h1, h2, X.X(), X.Y(), c1, c2, u.X(), u.Y(), z, zPrm, t, v, w)...) + eHash = common.SHA512_256i_TAGGED(fsSessionBobWC(Session), append(pk.AsInts(), NTilde, h1, h2, X.X(), X.Y(), c1, c2, u.X(), u.Y(), z, zPrm, t, v, w)...) } - e = common.RejectionSample(q, eHash) + e = common.ModReduceHash(q, eHash) } // 13. @@ -203,6 +203,16 @@ func (pf *ProofBobWC) Verify(ec elliptic.Curve, pk *paillier.PublicKey, NTilde, } else if !pf.ProofBob.ValidateBasic() { return false } + if !common.IsUsableUnknownOrderModulus(pk.N, verifyMinModulusBitLen) || + !common.IsUsableUnknownOrderModulus(NTilde, verifyMinModulusBitLen) { + return false + } + if !common.IsCanonicalGenerator(NTilde, h1) || !common.IsCanonicalGenerator(NTilde, h2) || h1.Cmp(h2) == 0 { + return false + } + if !common.IsCanonicalPaillierCiphertext(c1, pk.N) || !common.IsCanonicalPaillierCiphertext(c2, pk.N) { + return false + } q := ec.Params().N q3 := new(big.Int).Mul(q, q) @@ -215,22 +225,22 @@ func (pf *ProofBobWC) Verify(ec elliptic.Curve, pk *paillier.PublicKey, NTilde, maxS2 := new(big.Int).Lsh(q3NTilde, 1) maxT2 := new(big.Int).Set(maxS2) - if !common.IsInInterval(pf.Z, NTilde) { + if !common.IsInIntervalPositive(pf.Z, NTilde) { return false } - if !common.IsInInterval(pf.ZPrm, NTilde) { + if !common.IsInIntervalPositive(pf.ZPrm, NTilde) { return false } - if !common.IsInInterval(pf.T, NTilde) { + if !common.IsInIntervalPositive(pf.T, NTilde) { return false } - if !common.IsInInterval(pf.V, pk.NSquare()) { + if !common.IsInIntervalPositive(pf.V, pk.NSquare()) { return false } - if !common.IsInInterval(pf.W, NTilde) { + if !common.IsInIntervalPositive(pf.W, NTilde) { return false } - if !common.IsInInterval(pf.S, pk.N) { + if !common.IsInIntervalPositive(pf.S, pk.N) { return false } if new(big.Int).GCD(nil, nil, pf.Z, NTilde).Cmp(one) != 0 { @@ -255,12 +265,6 @@ func (pf *ProofBobWC) Verify(ec elliptic.Curve, pk *paillier.PublicKey, NTilde, if gcd.GCD(nil, nil, pf.S, pk.N).Cmp(one) != 0 { return false } - if pf.V.Cmp(zero) == 0 { - return false - } - if gcd.GCD(nil, nil, pf.V, pk.N).Cmp(one) != 0 { - return false - } if pf.S1.Cmp(q) == -1 { return false } @@ -294,14 +298,20 @@ func (pf *ProofBobWC) Verify(ec elliptic.Curve, pk *paillier.PublicKey, NTilde, var eHash *big.Int // X is nil if called on a ProveBob (Bob's proof "without check") if X == nil { - eHash = common.SHA512_256i_TAGGED(Session, append(pk.AsInts(), NTilde, h1, h2, c1, c2, pf.Z, pf.ZPrm, pf.T, pf.V, pf.W)...) + eHash = common.SHA512_256i_TAGGED(fsSessionBob(Session), append(pk.AsInts(), NTilde, h1, h2, c1, c2, pf.Z, pf.ZPrm, pf.T, pf.V, pf.W)...) } else { if !X.ValidateBasic() || !tss.SameCurve(ec, X.Curve()) { return false } - eHash = common.SHA512_256i_TAGGED(Session, append(pk.AsInts(), NTilde, h1, h2, X.X(), X.Y(), c1, c2, pf.U.X(), pf.U.Y(), pf.Z, pf.ZPrm, pf.T, pf.V, pf.W)...) + if !pf.U.ValidateBasic() || !tss.SameCurve(ec, pf.U.Curve()) { + return false + } + eHash = common.SHA512_256i_TAGGED(fsSessionBobWC(Session), append(pk.AsInts(), NTilde, h1, h2, X.X(), X.Y(), c1, c2, pf.U.X(), pf.U.Y(), pf.Z, pf.ZPrm, pf.T, pf.V, pf.W)...) } - e = common.RejectionSample(q, eHash) + e = common.ModReduceHash(q, eHash) + } + if e.Sign() == 0 { + return false } var left, right *big.Int // for the following conditionals @@ -315,7 +325,7 @@ func (pf *ProofBobWC) Verify(ec elliptic.Curve, pk *paillier.PublicKey, NTilde, return false } xEU, err := xE.Add(pf.U) - if err != nil || xEU == nil || !gS1.Equals(xEU) { + if err != nil || xEU == nil || gS1 == nil || !gS1.Equals(xEU) { return false } } diff --git a/crypto/mta/range_proof.go b/crypto/mta/range_proof.go index 2c5c7fd0..6598787c 100644 --- a/crypto/mta/range_proof.go +++ b/crypto/mta/range_proof.go @@ -18,8 +18,24 @@ import ( const ( RangeProofAliceBytesParts = 6 + verifyMinModulusBitLen = 2048 + fsDomainTagRangeAlice = "tss-lib.threshold.mta.range-alice" + fsDomainTagBob = "tss-lib.threshold.mta.bob" + fsDomainTagBobWC = "tss-lib.threshold.mta.bob-wc" ) +func fsSessionRangeAlice(session []byte) []byte { + return append([]byte(fsDomainTagRangeAlice+"|"), session...) +} + +func fsSessionBob(session []byte) []byte { + return append([]byte(fsDomainTagBob+"|"), session...) +} + +func fsSessionBobWC(session []byte) []byte { + return append([]byte(fsDomainTagBobWC+"|"), session...) +} + var ( zero = big.NewInt(0) one = big.NewInt(1) @@ -34,7 +50,7 @@ type ( // ProveRangeAlice implements Alice's range proof used in the MtA and MtAwc protocols from GG18Spec (9) Fig. 9. func ProveRangeAlice(ec elliptic.Curve, pk *paillier.PublicKey, c, NTilde, h1, h2, m, r *big.Int, session ...[]byte) (*RangeProofAlice, error) { Session := optionalProofSession(session) - if pk == nil || NTilde == nil || h1 == nil || h2 == nil || c == nil || m == nil || r == nil { + if ec == nil || pk == nil || NTilde == nil || h1 == nil || h2 == nil || c == nil || m == nil || r == nil { return nil, errors.New("ProveRangeAlice constructor received nil value(s)") } @@ -70,8 +86,8 @@ func ProveRangeAlice(ec elliptic.Curve, pk *paillier.PublicKey, c, NTilde, h1, h w = modNTilde.Mul(w, modNTilde.Exp(h2, gamma)) // 8-9. e' - eHash := common.SHA512_256i_TAGGED(Session, append(pk.AsInts(), NTilde, h1, h2, c, z, u, w)...) - e := common.RejectionSample(q, eHash) + eHash := common.SHA512_256i_TAGGED(fsSessionRangeAlice(Session), append(pk.AsInts(), NTilde, h1, h2, c, z, u, w)...) + e := common.ModReduceHash(q, eHash) modN := common.ModInt(pk.N) s := modN.Exp(r, e) @@ -109,7 +125,14 @@ func (pf *RangeProofAlice) Verify(ec elliptic.Curve, pk *paillier.PublicKey, NTi NTilde == nil || h1 == nil || h2 == nil || c == nil { return false } - if new(big.Int).GCD(nil, nil, c, pk.N).Cmp(one) != 0 { + if !common.IsUsableUnknownOrderModulus(pk.N, verifyMinModulusBitLen) || + !common.IsUsableUnknownOrderModulus(NTilde, verifyMinModulusBitLen) { + return false + } + if !common.IsCanonicalGenerator(NTilde, h1) || !common.IsCanonicalGenerator(NTilde, h2) || h1.Cmp(h2) == 0 { + return false + } + if !common.IsCanonicalPaillierCiphertext(c, pk.N) { return false } @@ -121,16 +144,16 @@ func (pf *RangeProofAlice) Verify(ec elliptic.Curve, pk *paillier.PublicKey, NTi q3NTilde := new(big.Int).Mul(q3, NTilde) maxS2 := new(big.Int).Lsh(q3NTilde, 1) - if !common.IsInInterval(pf.Z, NTilde) { + if !common.IsInIntervalPositive(pf.Z, NTilde) { return false } - if !common.IsInInterval(pf.U, pk.NSquare()) { + if !common.IsInIntervalPositive(pf.U, pk.NSquare()) { return false } - if !common.IsInInterval(pf.W, NTilde) { + if !common.IsInIntervalPositive(pf.W, NTilde) { return false } - if !common.IsInInterval(pf.S, pk.N) { + if !common.IsInIntervalPositive(pf.S, pk.N) { return false } if new(big.Int).GCD(nil, nil, pf.Z, NTilde).Cmp(one) != 0 { @@ -173,8 +196,11 @@ func (pf *RangeProofAlice) Verify(ec elliptic.Curve, pk *paillier.PublicKey, NTi } // 1-2. e' - eHash := common.SHA512_256i_TAGGED(Session, append(pk.AsInts(), NTilde, h1, h2, c, pf.Z, pf.U, pf.W)...) - e := common.RejectionSample(q, eHash) + eHash := common.SHA512_256i_TAGGED(fsSessionRangeAlice(Session), append(pk.AsInts(), NTilde, h1, h2, c, pf.Z, pf.U, pf.W)...) + e := common.ModReduceHash(q, eHash) + if e.Sign() == 0 { + return false + } var products *big.Int // for the following conditionals minusE := new(big.Int).Sub(zero, e) diff --git a/crypto/paillier/factor_proof.go b/crypto/paillier/factor_proof.go index d4de4a2c..57cc8b1e 100644 --- a/crypto/paillier/factor_proof.go +++ b/crypto/paillier/factor_proof.go @@ -8,10 +8,15 @@ import ( ) const ( - PARAM_E = 512 // 2 * secp256k1 element bit length - PARAM_L = 256 // 1 * secp256k1 element bit length + PARAM_E = 512 // 2 * secp256k1 element bit length + PARAM_L = 256 // 1 * secp256k1 element bit length + fsDomainTagFactorProof = "tss-lib.threshold.factorproof" ) +func fsSessionFactorProof(session []byte) []byte { + return append([]byte(fsDomainTagFactorProof+"|"), session...) +} + type ( FactorProof struct { // Commitment @@ -89,8 +94,11 @@ func (pf FactorProof) FactorVerify(pkN, N, s, t *big.Int, session ...[]byte) (bo if common.AnyIsNil(pf.P, pf.Q, pf.A, pf.B, pf.T, pf.Sigma, pf.Z1, pf.Z2, pf.W1, pf.W2, pf.V) { return false, fmt.Errorf("fac proof verify: nil bigint present in proof") } - if N.Sign() <= 0 { - return false, fmt.Errorf("fac proof verify: invalid modulus %x", N) + if !common.IsUsableUnknownOrderModulus(pkN, verifyMinModulusBitLen) { + return false, fmt.Errorf("fac proof verify: invalid Paillier modulus %x", pkN) + } + if !common.IsUsableUnknownOrderModulus(N, verifyMinModulusBitLen) { + return false, fmt.Errorf("fac proof verify: invalid auxiliary modulus %x", N) } for name, base := range map[string]*big.Int{ "s": s, @@ -101,7 +109,7 @@ func (pf FactorProof) FactorVerify(pkN, N, s, t *big.Int, session ...[]byte) (bo "B": pf.B, "T": pf.T, } { - if !common.IsInInterval(base, N) || !common.Coprime(base, N) { + if !common.IsCanonicalGenerator(N, base) { return false, fmt.Errorf("fac proof verify: base %s = %x is not invertible modulo N", name, base) } } @@ -149,6 +157,9 @@ func (pf FactorProof) FactorVerify(pkN, N, s, t *big.Int, session ...[]byte) (bo } e := FactorChallenge(N, s, t, pkN, pf.P, pf.Q, pf.A, pf.B, pf.T, pf.Sigma, session...) + if e.Sign() == 0 { + return false, fmt.Errorf("fac proof verify: Fiat-Shamir challenge is zero") + } modN := common.ModInt(N) @@ -200,8 +211,8 @@ func FactorChallenge(N, s, t, pkN, P, Q, A, B, T, sigma *big.Int, session ...[]b if len(session[0]) == 0 { panic("paillier: factor proof session tag must be non-empty") } - eHash := common.SHA512_256i_TAGGED(session[0], N, s, t, pkN, P, Q, A, B, T, sigma) - return common.RejectionSample(q, eHash) + eHash := common.SHA512_256i_TAGGED(fsSessionFactorProof(session[0]), N, s, t, pkN, P, Q, A, B, T, sigma) + return common.ModReduceHash(q, eHash) } // 2. Verifier replies with e <- +-q diff --git a/crypto/paillier/mod_proof.go b/crypto/paillier/mod_proof.go index f39c48c3..5608d042 100644 --- a/crypto/paillier/mod_proof.go +++ b/crypto/paillier/mod_proof.go @@ -1,6 +1,7 @@ package paillier import ( + "encoding/binary" "fmt" "math/big" @@ -8,9 +9,14 @@ import ( ) const ( - PARAM_M = 80 // ZKP iterations + PARAM_M = 80 // ZKP iterations + fsDomainTagModProof = "tss-lib.threshold.modproof" ) +func fsSessionModProof(session []byte) []byte { + return append([]byte(fsDomainTagModProof+"|"), session...) +} + type ( ModProof struct { W *big.Int @@ -72,15 +78,8 @@ func (pf ModProof) ModVerify(N *big.Int, session ...[]byte) (bool, error) { return false, fmt.Errorf("mod proof verify: nil inputs in proof") } - rem2 := new(big.Int).Mod(N, big.NewInt(2)) - odd := rem2.Int64() == 1 - - if !odd { - return false, fmt.Errorf("mod proof verify: modulus %d is even", N) - } - - if N.ProbablyPrime(30) { - return false, fmt.Errorf("mod proof verify: modulus %d seems prime", N) + if !common.IsUsableUnknownOrderModulus(N, verifyMinModulusBitLen) { + return false, fmt.Errorf("mod proof verify: invalid modulus %d", N) } if !common.Gt(pf.W, zero) || !common.Lt(pf.W, N) { @@ -100,6 +99,12 @@ func (pf ModProof) ModVerify(N *big.Int, session ...[]byte) (bool, error) { if !common.Gt(pf.Z[i], zero) || !common.Lt(pf.Z[i], N) { return false, fmt.Errorf("mod proof verify: z_%d must be in [1, N), got %d", i, pf.Z[i]) } + if new(big.Int).GCD(nil, nil, pf.X[i], N).Cmp(one) != 0 { + return false, fmt.Errorf("mod proof verify: x_%d is not a unit modulo N", i) + } + if new(big.Int).GCD(nil, nil, pf.Z[i], N).Cmp(one) != 0 { + return false, fmt.Errorf("mod proof verify: z_%d is not a unit modulo N", i) + } ziN := new(big.Int).Exp(pf.Z[i], N, N) @@ -126,8 +131,8 @@ func (pf ModProof) ModVerify(N *big.Int, session ...[]byte) (bool, error) { // Standard Fiat-Shamir transform. // -// The session-tagged path uses HashToNTagged to derive each y_i with at least -// N.BitLen() + 256 bits of entropy before reducing mod N. Reducing a single +// The session-tagged path uses expand-then-reject sampling to derive each y_i +// uniformly in [0, N). Reducing a single // 256-bit SHA512_256i_TAGGED output mod ~2^2048 would emit challenges in // [0, 2^256) instead of [0, N), giving the session-tagged path a strictly // weaker challenge distribution than the HashToN path it shares the @@ -146,12 +151,37 @@ func ModChallenge(N, w *big.Int, session ...[]byte) [PARAM_M]*big.Int { panic("paillier: mod proof session tag must be non-empty") } inputs := append([]*big.Int{w, N}, y[:i]...) - y[i] = common.HashToNTagged(session[0], N, inputs...) + y[i] = sampleYModN(fsSessionModProof(session[0]), N, inputs...) } return y } +func sampleYModN(tag []byte, N *big.Int, inputs ...*big.Int) *big.Int { + seedInt := common.SHA512_256i_TAGGED(tag, inputs...) + seed := seedInt.FillBytes(make([]byte, 32)) + byteLen := (N.BitLen() + 7) / 8 + blocks := (byteLen + 31) / 32 + excessBits := uint(byteLen*8 - N.BitLen()) + + for counter := uint32(0); ; counter++ { + counterBytes := make([]byte, 4) + binary.BigEndian.PutUint32(counterBytes, counter) + out := make([]byte, 0, blocks*32) + for blockIdx := 0; blockIdx < blocks; blockIdx++ { + out = append(out, common.SHA512_256(seed, counterBytes, []byte{byte(blockIdx)})...) + } + out = out[:byteLen] + if excessBits > 0 { + out[0] &= byte(0xff >> excessBits) + } + candidate := new(big.Int).SetBytes(out) + if candidate.Cmp(N) < 0 { + return candidate + } + } +} + // Determine values a_i and b_i so that a valid x_i exists, // and return a_i, b_i and x_i. func defineXi(w, y_i, p, q, N, phiN *big.Int) (bool, bool, *big.Int) { diff --git a/crypto/paillier/paillier.go b/crypto/paillier/paillier.go index d5cdb258..6651e86c 100644 --- a/crypto/paillier/paillier.go +++ b/crypto/paillier/paillier.go @@ -31,9 +31,10 @@ import ( ) const ( - ProofIters = 13 - verifyPrimesUntil = 1000 // Verify uses primes <1000 - pQBitLenDifference = 3 // >1020-bit P-Q + ProofIters = 13 + verifyPrimesUntil = 1000 // Verify uses primes <1000 + pQBitLenDifference = 3 // >1020-bit P-Q + verifyMinModulusBitLen = 2048 ) type ( @@ -207,7 +208,24 @@ func (privateKey *PrivateKey) Proof(k *big.Int, ecdsaPub *crypto2.ECPoint) Proof } func (pf Proof) Verify(pkN, k *big.Int, ecdsaPub *crypto2.ECPoint) (bool, error) { + if pkN == nil || k == nil || ecdsaPub == nil || !ecdsaPub.ValidateBasic() { + return false, nil + } + if k.Sign() < 0 { + return false, nil + } + if !common.IsUsableUnknownOrderModulus(pkN, verifyMinModulusBitLen) { + return false, nil + } iters := ProofIters + for i := 0; i < iters; i++ { + if pf[i] == nil || pf[i].Sign() != 1 || pf[i].Cmp(pkN) != -1 { + return false, nil + } + if new(big.Int).GCD(nil, nil, pf[i], pkN).Cmp(one) != 0 { + return false, nil + } + } pch, xch := make(chan bool, 1), make(chan []*big.Int, 1) // buffered to allow early exit prms := primes.Until(verifyPrimesUntil).List() // uses cache primed in init() go func(ch chan<- bool) { diff --git a/crypto/schnorr/schnorr_proof.go b/crypto/schnorr/schnorr_proof.go index d5a2ccd8..799e8eda 100644 --- a/crypto/schnorr/schnorr_proof.go +++ b/crypto/schnorr/schnorr_proof.go @@ -12,6 +12,7 @@ import ( "github.com/bnb-chain/tss-lib/common" "github.com/bnb-chain/tss-lib/crypto" + "github.com/bnb-chain/tss-lib/tss" ) type ( @@ -26,6 +27,19 @@ type ( } ) +const ( + fsDomainTagZK = "tss-lib.threshold.schnorr.zk" + fsDomainTagZKV = "tss-lib.threshold.schnorr.zkv" +) + +func fsSessionZK(session []byte) []byte { + return append([]byte(fsDomainTagZK+"|"), session...) +} + +func fsSessionZKV(session []byte) []byte { + return append([]byte(fsDomainTagZKV+"|"), session...) +} + // NewZKProof constructs a new Schnorr ZK proof of knowledge of the discrete logarithm (GG18Spec Fig. 16) func NewZKProof(x *big.Int, X *crypto.ECPoint) (*ZKProof, error) { return NewZKProofWithSession(nil, x, X) @@ -45,8 +59,8 @@ func NewZKProofWithSession(session []byte, x *big.Int, X *crypto.ECPoint) (*ZKPr a := common.GetRandomPositiveInt(q) alpha := crypto.ScalarBaseMult(ec, a) - cHash := common.SHA512_256i_TAGGED(session, X.X(), X.Y(), g.X(), g.Y(), alpha.X(), alpha.Y()) - c := common.RejectionSample(q, cHash) + cHash := common.SHA512_256i_TAGGED(fsSessionZK(session), X.X(), X.Y(), g.X(), g.Y(), alpha.X(), alpha.Y()) + c := common.ModReduceHash(q, cHash) t := new(big.Int).Mul(c, x) t = common.ModInt(q).Add(a, t) @@ -64,6 +78,9 @@ func (pf *ZKProof) VerifyWithSession(session []byte, X *crypto.ECPoint) bool { if pf == nil || !pf.ValidateBasic() || X == nil || !X.ValidateBasic() { return false } + if !tss.SameCurve(X.Curve(), pf.Alpha.Curve()) { + return false + } ec := X.Curve() ecParams := ec.Params() q := ecParams.N @@ -72,8 +89,8 @@ func (pf *ZKProof) VerifyWithSession(session []byte, X *crypto.ECPoint) bool { } g := crypto.NewECPointNoCurveCheck(ec, ecParams.Gx, ecParams.Gy) - cHash := common.SHA512_256i_TAGGED(session, X.X(), X.Y(), g.X(), g.Y(), pf.Alpha.X(), pf.Alpha.Y()) - c := common.RejectionSample(q, cHash) + cHash := common.SHA512_256i_TAGGED(fsSessionZK(session), X.X(), X.Y(), g.X(), g.Y(), pf.Alpha.X(), pf.Alpha.Y()) + c := common.ModReduceHash(q, cHash) if c.Sign() == 0 { return false } @@ -115,8 +132,8 @@ func NewZKVProofWithSession(session []byte, V, R *crypto.ECPoint, s, l *big.Int) bG := crypto.ScalarBaseMult(ec, b) alpha, _ := aR.Add(bG) // already on the curve. - cHash := common.SHA512_256i_TAGGED(session, V.X(), V.Y(), R.X(), R.Y(), g.X(), g.Y(), alpha.X(), alpha.Y()) - c := common.RejectionSample(q, cHash) + cHash := common.SHA512_256i_TAGGED(fsSessionZKV(session), V.X(), V.Y(), R.X(), R.Y(), g.X(), g.Y(), alpha.X(), alpha.Y()) + c := common.ModReduceHash(q, cHash) modQ := common.ModInt(q) t := modQ.Add(a, new(big.Int).Mul(c, s)) @@ -136,6 +153,9 @@ func (pf *ZKVProof) VerifyWithSession(session []byte, V, R *crypto.ECPoint) bool V == nil || R == nil || !V.ValidateBasic() || !R.ValidateBasic() { return false } + if !tss.SameCurve(V.Curve(), R.Curve()) || !tss.SameCurve(V.Curve(), pf.Alpha.Curve()) { + return false + } ec := V.Curve() ecParams := ec.Params() q := ecParams.N @@ -144,8 +164,8 @@ func (pf *ZKVProof) VerifyWithSession(session []byte, V, R *crypto.ECPoint) bool } g := crypto.NewECPointNoCurveCheck(ec, ecParams.Gx, ecParams.Gy) - cHash := common.SHA512_256i_TAGGED(session, V.X(), V.Y(), R.X(), R.Y(), g.X(), g.Y(), pf.Alpha.X(), pf.Alpha.Y()) - c := common.RejectionSample(q, cHash) + cHash := common.SHA512_256i_TAGGED(fsSessionZKV(session), V.X(), V.Y(), R.X(), R.Y(), g.X(), g.Y(), pf.Alpha.X(), pf.Alpha.Y()) + c := common.ModReduceHash(q, cHash) if c.Sign() == 0 { return false } @@ -155,7 +175,10 @@ func (pf *ZKVProof) VerifyWithSession(session []byte, V, R *crypto.ECPoint) bool if tR == nil || uG == nil { return false } - tRuG, _ := tR.Add(uG) // already on the curve. + tRuG, err := tR.Add(uG) + if err != nil { + return false + } Vc := V.ScalarMult(c) if Vc == nil { diff --git a/crypto/vss/feldman_vss.go b/crypto/vss/feldman_vss.go index 2fc17123..7242a9df 100644 --- a/crypto/vss/feldman_vss.go +++ b/crypto/vss/feldman_vss.go @@ -18,6 +18,7 @@ import ( "github.com/bnb-chain/tss-lib/common" "github.com/bnb-chain/tss-lib/crypto" + "github.com/bnb-chain/tss-lib/tss" ) type ( @@ -59,6 +60,9 @@ func CheckIndexes(ec elliptic.Curve, indexes []*big.Int) ([]*big.Int, error) { // Returns a new array of secret shares created by Shamir's Secret Sharing Algorithm, // requiring a minimum number of shares to recreate, of length shares, from the input secret func Create(ec elliptic.Curve, threshold int, secret *big.Int, indexes []*big.Int) (Vs, Shares, error) { + if ec == nil { + return nil, nil, fmt.Errorf("vss ec == nil") + } if secret == nil || indexes == nil { return nil, nil, fmt.Errorf("vss secret or indexes == nil: %v %v", secret, indexes) } @@ -72,7 +76,7 @@ func Create(ec elliptic.Curve, threshold int, secret *big.Int, indexes []*big.In } num := len(indexes) - if num < threshold { + if num < threshold+1 { return nil, nil, ErrNumSharesBelowThreshold } @@ -104,21 +108,21 @@ func (share *Share) Verify(ec elliptic.Curve, threshold int, vs Vs) bool { var err error modQ := common.ModInt(q) v, t := vs[0], one // YRO : we need to have our accumulator outside of the loop - if v == nil || !v.SetCurve(ec).ValidateBasic() { + if v == nil || !tss.SameCurve(v.Curve(), ec) || !v.ValidateBasic() { return false } for j := 1; j <= threshold; j++ { - if vs[j] == nil || !vs[j].SetCurve(ec).ValidateBasic() { + if vs[j] == nil || !tss.SameCurve(vs[j].Curve(), ec) || !vs[j].ValidateBasic() { return false } // t = k_i^j t = modQ.Mul(t, share.ID) // v = v * v_j^t - vjt := vs[j].SetCurve(ec).ScalarMult(t) + vjt := vs[j].ScalarMult(t) if vjt == nil { return false } - v, err = v.SetCurve(ec).Add(vjt) + v, err = v.Add(vjt) if err != nil { return false } @@ -131,13 +135,25 @@ func (share *Share) Verify(ec elliptic.Curve, threshold int, vs Vs) bool { } func (shares Shares) ReConstruct(ec elliptic.Curve) (secret *big.Int, err error) { + if ec == nil { + return nil, errors.New("vss reconstruct: ec is nil") + } if len(shares) == 0 { return nil, ErrNumSharesBelowThreshold } if shares[0] == nil { return nil, errors.New("vss reconstruct: nil share") } - if shares[0].Threshold+1 > len(shares) { + threshold := shares[0].Threshold + for idx, share := range shares { + if share == nil || share.ID == nil || share.Share == nil { + return nil, fmt.Errorf("vss reconstruct: nil share or share field at index %d", idx) + } + if share.Threshold != threshold { + return nil, fmt.Errorf("vss reconstruct: share %d has threshold %d, want %d", idx, share.Threshold, threshold) + } + } + if threshold+1 > len(shares) { return nil, ErrNumSharesBelowThreshold } q := ec.Params().N diff --git a/ecdsa/keygen/local_party.go b/ecdsa/keygen/local_party.go index a5b8e968..d287301b 100644 --- a/ecdsa/keygen/local_party.go +++ b/ecdsa/keygen/local_party.go @@ -135,15 +135,34 @@ func (p *LocalParty) StoreMessage(msg tss.ParsedMessage) (bool, *tss.Error) { fromPIdx := msg.GetFrom().Index // switch/case is necessary to store any messages beyond current round - // this does not handle message replays. we expect the caller to apply replay and spoofing protection. + // Identical redelivery is idempotent; content-different replacement from + // a peer is rejected so commit-reveal state cannot be silently overwritten. + isDup := fromPIdx != p.PartyID().Index + dupErr := func() (bool, *tss.Error) { + return false, p.WrapError( + fmt.Errorf("duplicate %T from party %d", msg.Content(), fromPIdx), + msg.GetFrom()) + } switch msg.Content().(type) { case *KGRound1Message: + if isDup && p.temp.kgRound1Messages[fromPIdx] != nil && !tss.IsSameMessage(p.temp.kgRound1Messages[fromPIdx], msg) { + return dupErr() + } p.temp.kgRound1Messages[fromPIdx] = msg case *KGRound2Message1: + if isDup && p.temp.kgRound2Message1s[fromPIdx] != nil && !tss.IsSameMessage(p.temp.kgRound2Message1s[fromPIdx], msg) { + return dupErr() + } p.temp.kgRound2Message1s[fromPIdx] = msg case *KGRound2Message2: + if isDup && p.temp.kgRound2Message2s[fromPIdx] != nil && !tss.IsSameMessage(p.temp.kgRound2Message2s[fromPIdx], msg) { + return dupErr() + } p.temp.kgRound2Message2s[fromPIdx] = msg case *KGRound3Message: + if isDup && p.temp.kgRound3Messages[fromPIdx] != nil && !tss.IsSameMessage(p.temp.kgRound3Messages[fromPIdx], msg) { + return dupErr() + } p.temp.kgRound3Messages[fromPIdx] = msg default: // unrecognised message, just ignore! common.Logger.Warningf("unrecognised message ignored: %v", msg) diff --git a/ecdsa/keygen/local_party_test.go b/ecdsa/keygen/local_party_test.go index 8d8fb4ea..ca25ec87 100644 --- a/ecdsa/keygen/local_party_test.go +++ b/ecdsa/keygen/local_party_test.go @@ -97,7 +97,7 @@ func TestKeygen_Start_RequiresSessionNonce(t *testing.T) { func TestStartRound1Paillier(t *testing.T) { setUp("debug") - pIDs := tss.GenerateTestPartyIDs(1) + pIDs := tss.GenerateTestPartyIDs(2) p2pCtx := tss.NewPeerContext(pIDs) threshold := 1 params := tss.NewParameters(tss.EC(), p2pCtx, pIDs[0], len(pIDs), threshold) @@ -138,7 +138,7 @@ func TestStartRound1Paillier(t *testing.T) { func TestFinishAndSaveH1H2(t *testing.T) { setUp("debug") - pIDs := tss.GenerateTestPartyIDs(1) + pIDs := tss.GenerateTestPartyIDs(2) p2pCtx := tss.NewPeerContext(pIDs) threshold := 1 params := tss.NewParameters(tss.EC(), p2pCtx, pIDs[0], len(pIDs), threshold) diff --git a/ecdsa/signing/local_party.go b/ecdsa/signing/local_party.go index 9fe9f274..f1fa736f 100644 --- a/ecdsa/signing/local_party.go +++ b/ecdsa/signing/local_party.go @@ -235,27 +235,64 @@ func (p *LocalParty) StoreMessage(msg tss.ParsedMessage) (bool, *tss.Error) { fromPIdx := msg.GetFrom().Index // switch/case is necessary to store any messages beyond current round - // this does not handle message replays. we expect the caller to apply replay and spoofing protection. + // Identical redelivery is idempotent; content-different replacement from + // a peer is rejected so commit-reveal state cannot be silently overwritten. + isDup := fromPIdx != p.PartyID().Index + dupErr := func() (bool, *tss.Error) { + return false, p.WrapError( + fmt.Errorf("duplicate %T from party %d", msg.Content(), fromPIdx), + msg.GetFrom()) + } switch msg.Content().(type) { case *SignRound1Message1: + if isDup && p.temp.signRound1Message1s[fromPIdx] != nil && !tss.IsSameMessage(p.temp.signRound1Message1s[fromPIdx], msg) { + return dupErr() + } p.temp.signRound1Message1s[fromPIdx] = msg case *SignRound1Message2: + if isDup && p.temp.signRound1Message2s[fromPIdx] != nil && !tss.IsSameMessage(p.temp.signRound1Message2s[fromPIdx], msg) { + return dupErr() + } p.temp.signRound1Message2s[fromPIdx] = msg case *SignRound2Message: + if isDup && p.temp.signRound2Messages[fromPIdx] != nil && !tss.IsSameMessage(p.temp.signRound2Messages[fromPIdx], msg) { + return dupErr() + } p.temp.signRound2Messages[fromPIdx] = msg case *SignRound3Message: + if isDup && p.temp.signRound3Messages[fromPIdx] != nil && !tss.IsSameMessage(p.temp.signRound3Messages[fromPIdx], msg) { + return dupErr() + } p.temp.signRound3Messages[fromPIdx] = msg case *SignRound4Message: + if isDup && p.temp.signRound4Messages[fromPIdx] != nil && !tss.IsSameMessage(p.temp.signRound4Messages[fromPIdx], msg) { + return dupErr() + } p.temp.signRound4Messages[fromPIdx] = msg case *SignRound5Message: + if isDup && p.temp.signRound5Messages[fromPIdx] != nil && !tss.IsSameMessage(p.temp.signRound5Messages[fromPIdx], msg) { + return dupErr() + } p.temp.signRound5Messages[fromPIdx] = msg case *SignRound6Message: + if isDup && p.temp.signRound6Messages[fromPIdx] != nil && !tss.IsSameMessage(p.temp.signRound6Messages[fromPIdx], msg) { + return dupErr() + } p.temp.signRound6Messages[fromPIdx] = msg case *SignRound7Message: + if isDup && p.temp.signRound7Messages[fromPIdx] != nil && !tss.IsSameMessage(p.temp.signRound7Messages[fromPIdx], msg) { + return dupErr() + } p.temp.signRound7Messages[fromPIdx] = msg case *SignRound8Message: + if isDup && p.temp.signRound8Messages[fromPIdx] != nil && !tss.IsSameMessage(p.temp.signRound8Messages[fromPIdx], msg) { + return dupErr() + } p.temp.signRound8Messages[fromPIdx] = msg case *SignRound9Message: + if isDup && p.temp.signRound9Messages[fromPIdx] != nil && !tss.IsSameMessage(p.temp.signRound9Messages[fromPIdx], msg) { + return dupErr() + } p.temp.signRound9Messages[fromPIdx] = msg default: // unrecognised message, just ignore! common.Logger.Warningf("unrecognised message ignored: %v", msg) diff --git a/ecdsa/signing/local_party_test.go b/ecdsa/signing/local_party_test.go index 306accf6..ead6380b 100644 --- a/ecdsa/signing/local_party_test.go +++ b/ecdsa/signing/local_party_test.go @@ -347,8 +347,8 @@ func TestNewLocalPartyWithKDD_FullBytesLen_Required(t *testing.T) { } func TestNewLocalPartyWithKDD_FullBytesLen_TooWide(t *testing.T) { - pIDs := tss.GenerateTestPartyIDs(1) - params := tss.NewParameters(tss.S256(), tss.NewPeerContext(pIDs), pIDs[0], 1, 0) + pIDs := tss.GenerateTestPartyIDs(2) + params := tss.NewParameters(tss.S256(), tss.NewPeerContext(pIDs), pIDs[0], len(pIDs), 1) defer func() { r := recover() if r == nil { diff --git a/ecdsa/signing/prepare.go b/ecdsa/signing/prepare.go index 60f107bf..7e818f34 100644 --- a/ecdsa/signing/prepare.go +++ b/ecdsa/signing/prepare.go @@ -16,18 +16,19 @@ import ( ) // PrepareForSigning(), GG18Spec (11) Fig. 14 -func PrepareForSigning(ec elliptic.Curve, i, pax int, xi *big.Int, ks []*big.Int, bigXs []*crypto.ECPoint) (wi *big.Int, bigWs []*crypto.ECPoint) { +func PrepareForSigning(ec elliptic.Curve, i, pax int, xi *big.Int, ks []*big.Int, bigXs []*crypto.ECPoint) (wi *big.Int, bigWs []*crypto.ECPoint, err error) { modQ := common.ModInt(ec.Params().N) if len(ks) != len(bigXs) { - panic(fmt.Errorf("PrepareForSigning: len(ks) != len(bigXs) (%d != %d)", len(ks), len(bigXs))) + return nil, nil, fmt.Errorf("PrepareForSigning: len(ks) != len(bigXs) (%d != %d)", len(ks), len(bigXs)) } if len(ks) != pax { - panic(fmt.Errorf("PrepareForSigning: len(ks) != pax (%d != %d)", len(ks), pax)) + return nil, nil, fmt.Errorf("PrepareForSigning: len(ks) != pax (%d != %d)", len(ks), pax) } if len(ks) <= i { - panic(fmt.Errorf("PrepareForSigning: len(ks) <= i (%d <= %d)", len(ks), i)) + return nil, nil, fmt.Errorf("PrepareForSigning: len(ks) <= i (%d <= %d)", len(ks), i) } + q := ec.Params().N // 2-4. wi = xi for j := 0; j < pax; j++ { @@ -36,8 +37,8 @@ func PrepareForSigning(ec elliptic.Curve, i, pax int, xi *big.Int, ks []*big.Int } ksj := ks[j] ksi := ks[i] - if ksj.Cmp(ksi) == 0 { - panic(fmt.Errorf("index of two parties are equal")) + if new(big.Int).Mod(ksj, q).Cmp(new(big.Int).Mod(ksi, q)) == 0 { + return nil, nil, fmt.Errorf("PrepareForSigning: party keys at indices %d and %d collide mod q", j, i) } // big.Int Div is calculated as: a/b = a * modInv(b,q) coef := modQ.Mul(ks[j], modQ.ModInverse(new(big.Int).Sub(ksj, ksi))) @@ -54,8 +55,8 @@ func PrepareForSigning(ec elliptic.Curve, i, pax int, xi *big.Int, ks []*big.Int } ksc := ks[c] ksj := ks[j] - if ksj.Cmp(ksc) == 0 { - panic(fmt.Errorf("index of two parties are equal")) + if new(big.Int).Mod(ksj, q).Cmp(new(big.Int).Mod(ksc, q)) == 0 { + return nil, nil, fmt.Errorf("PrepareForSigning: party keys at indices %d and %d collide mod q", j, c) } // big.Int Div is calculated as: a/b = a * modInv(b,q) iota := modQ.Mul(ksc, modQ.ModInverse(new(big.Int).Sub(ksc, ksj))) @@ -63,5 +64,5 @@ func PrepareForSigning(ec elliptic.Curve, i, pax int, xi *big.Int, ks []*big.Int } bigWs[j] = bigWj } - return + return wi, bigWs, nil } diff --git a/ecdsa/signing/round_1.go b/ecdsa/signing/round_1.go index f4f938d4..1b935530 100644 --- a/ecdsa/signing/round_1.go +++ b/ecdsa/signing/round_1.go @@ -152,7 +152,10 @@ func (round *round1) prepare() error { if round.Threshold()+1 > len(ks) { return fmt.Errorf("t+1=%d is not satisfied by the key count of %d", round.Threshold()+1, len(ks)) } - wi, bigWs := PrepareForSigning(round.Params().EC(), i, len(ks), xi, ks, bigXs) + wi, bigWs, err := PrepareForSigning(round.Params().EC(), i, len(ks), xi, ks, bigXs) + if err != nil { + return err + } round.temp.w = wi round.temp.bigWs = bigWs diff --git a/tss/message.go b/tss/message.go index 9aa8e59e..ca99a418 100644 --- a/tss/message.go +++ b/tss/message.go @@ -7,6 +7,7 @@ package tss import ( + "bytes" "fmt" "google.golang.org/protobuf/proto" @@ -167,3 +168,17 @@ func (mm *MessageImpl) String() string { } return fmt.Sprintf("Type: %s, From: %s, To: %s%s", mm.Type(), mm.From.String(), toStr, extraStr) } + +func IsSameMessage(lhs, rhs ParsedMessage) bool { + if lhs == nil || rhs == nil { + return lhs == rhs + } + lhsBytes, _, lhsErr := lhs.WireBytes() + rhsBytes, _, rhsErr := rhs.WireBytes() + if lhsErr != nil || rhsErr != nil { + return false + } + return lhs.Type() == rhs.Type() && + lhs.IsBroadcast() == rhs.IsBroadcast() && + bytes.Equal(lhsBytes, rhsBytes) +} diff --git a/tss/params.go b/tss/params.go index 094ef307..2245a056 100644 --- a/tss/params.go +++ b/tss/params.go @@ -8,6 +8,7 @@ package tss import ( "crypto/elliptic" + "fmt" "math/big" "runtime" "time" @@ -37,6 +38,16 @@ const ( // Exported, used in `tss` client func NewParameters(ec elliptic.Curve, ctx *PeerContext, partyID *PartyID, partyCount, threshold int) *Parameters { + if partyCount < 2 { + panic("tss: party count must be at least 2") + } + if threshold < 1 { + panic("tss: threshold must be at least 1") + } + if threshold >= partyCount { + panic("tss: threshold must be less than party count") + } + assertDistinctIDsModQ(ec, ctx) return &Parameters{ ec: ec, parties: ctx, @@ -48,6 +59,28 @@ func NewParameters(ec elliptic.Curve, ctx *PeerContext, partyID *PartyID, partyC } } +func assertDistinctIDsModQ(ec elliptic.Curve, ctx *PeerContext) { + if ec == nil || ctx == nil { + return + } + q := ec.Params().N + seen := make(map[string]*PartyID, len(ctx.IDs())) + for _, partyID := range ctx.IDs() { + if partyID == nil || partyID.Key == nil { + continue + } + residue := new(big.Int).Mod(partyID.KeyInt(), q) + if residue.Sign() == 0 { + panic(fmt.Errorf("tss: party %s has key congruent to 0 mod q", partyID)) + } + key := residue.Text(16) + if previous, exists := seen[key]; exists { + panic(fmt.Errorf("tss: party keys for %s and %s collide mod q", previous, partyID)) + } + seen[key] = partyID + } +} + func (params *Parameters) EC() elliptic.Curve { return params.ec } diff --git a/tss/params_test.go b/tss/params_test.go index 358fce16..c9fed7a1 100644 --- a/tss/params_test.go +++ b/tss/params_test.go @@ -16,8 +16,8 @@ import ( ) func TestSetSessionNonceCopiesInput(t *testing.T) { - pIDs := GenerateTestPartyIDs(1) - params := NewParameters(S256(), NewPeerContext(pIDs), pIDs[0], 1, 0) + pIDs := GenerateTestPartyIDs(2) + params := NewParameters(S256(), NewPeerContext(pIDs), pIDs[0], len(pIDs), 1) nonce := big.NewInt(42) params.SetSessionNonce(nonce) @@ -27,8 +27,8 @@ func TestSetSessionNonceCopiesInput(t *testing.T) { } func TestSetSessionNonceBytesHashesSessionID(t *testing.T) { - pIDs := GenerateTestPartyIDs(1) - params := NewParameters(S256(), NewPeerContext(pIDs), pIDs[0], 1, 0) + pIDs := GenerateTestPartyIDs(2) + params := NewParameters(S256(), NewPeerContext(pIDs), pIDs[0], len(pIDs), 1) sessionID := []byte("session-1-with-128-bits") params.SetSessionNonceBytes(sessionID) @@ -38,8 +38,8 @@ func TestSetSessionNonceBytesHashesSessionID(t *testing.T) { } func TestSetSessionNonceBytesRejectsShortSessionID(t *testing.T) { - pIDs := GenerateTestPartyIDs(1) - params := NewParameters(S256(), NewPeerContext(pIDs), pIDs[0], 1, 0) + pIDs := GenerateTestPartyIDs(2) + params := NewParameters(S256(), NewPeerContext(pIDs), pIDs[0], len(pIDs), 1) assert.Panics(t, func() { params.SetSessionNonceBytes(nil) @@ -53,8 +53,8 @@ func TestSetSessionNonceBytesRejectsShortSessionID(t *testing.T) { } func TestSetSessionNonceRejectsNonPositiveNonce(t *testing.T) { - pIDs := GenerateTestPartyIDs(1) - params := NewParameters(S256(), NewPeerContext(pIDs), pIDs[0], 1, 0) + pIDs := GenerateTestPartyIDs(2) + params := NewParameters(S256(), NewPeerContext(pIDs), pIDs[0], len(pIDs), 1) assert.Panics(t, func() { params.SetSessionNonce(nil) @@ -66,3 +66,51 @@ func TestSetSessionNonceRejectsNonPositiveNonce(t *testing.T) { params.SetSessionNonce(big.NewInt(-1)) }) } + +func TestNewParametersRejectsInvalidThresholdBounds(t *testing.T) { + pIDs := GenerateTestPartyIDs(2) + ctx := NewPeerContext(pIDs) + + assert.Panics(t, func() { + NewParameters(S256(), ctx, pIDs[0], 1, 1) + }) + assert.Panics(t, func() { + NewParameters(S256(), ctx, pIDs[0], len(pIDs), 0) + }) + assert.Panics(t, func() { + NewParameters(S256(), ctx, pIDs[0], len(pIDs), len(pIDs)) + }) +} + +func TestNewParametersRejectsPartyIDCollisionsModQ(t *testing.T) { + q := S256().Params().N + pIDs := SortPartyIDs(UnSortedPartyIDs{ + NewPartyID("p0", "p0", big.NewInt(1)), + NewPartyID("p1", "p1", new(big.Int).Add(q, big.NewInt(1))), + }) + + assert.Panics(t, func() { + NewParameters(S256(), NewPeerContext(pIDs), pIDs[0], len(pIDs), 1) + }) +} + +func TestNewParametersRejectsZeroResiduePartyID(t *testing.T) { + q := S256().Params().N + pIDs := SortPartyIDs(UnSortedPartyIDs{ + NewPartyID("p0", "p0", q), + NewPartyID("p1", "p1", big.NewInt(1)), + }) + + assert.Panics(t, func() { + NewParameters(S256(), NewPeerContext(pIDs), pIDs[0], len(pIDs), 1) + }) +} + +func TestSortPartyIDsRejectsDuplicateKeys(t *testing.T) { + assert.Panics(t, func() { + SortPartyIDs(UnSortedPartyIDs{ + NewPartyID("p0", "p0", big.NewInt(1)), + NewPartyID("p1", "p1", big.NewInt(1)), + }) + }) +} diff --git a/tss/party_id.go b/tss/party_id.go index 614f8590..16d7a869 100644 --- a/tss/party_id.go +++ b/tss/party_id.go @@ -66,6 +66,11 @@ func SortPartyIDs(ids UnSortedPartyIDs, startAt ...int) SortedPartyIDs { sorted = append(sorted, id) } sort.Sort(sorted) + for i := 1; i < len(sorted); i++ { + if sorted[i-1].KeyInt().Cmp(sorted[i].KeyInt()) == 0 { + panic(fmt.Errorf("SortPartyIDs: duplicate party key %s", sorted[i].KeyInt())) + } + } // assign party indexes for i, id := range sorted { frm := 0 @@ -140,7 +145,7 @@ func (spids SortedPartyIDs) Len() int { } func (spids SortedPartyIDs) Less(a, b int) bool { - return spids[a].KeyInt().Cmp(spids[b].KeyInt()) <= 0 + return spids[a].KeyInt().Cmp(spids[b].KeyInt()) < 0 } func (spids SortedPartyIDs) Swap(a, b int) { From 296642a12b64cb80e72c2b00449f1b16e9d6ffa7 Mon Sep 17 00:00:00 2001 From: maclane Date: Thu, 28 May 2026 14:27:45 -0500 Subject: [PATCH 4/6] Address hardening review findings --- common/random.go | 19 ++++++++++- common/random_test.go | 6 ++++ crypto/ecpoint.go | 25 ++++++++++++++ crypto/mta/proofs.go | 5 ++- crypto/paillier/mod_proof.go | 4 ++- crypto/paillier/mod_proof_test.go | 15 +++++++++ crypto/schnorr/schnorr_proof.go | 5 ++- crypto/schnorr/schnorr_proof_test.go | 28 ++++++++++++++++ crypto/vss/feldman_vss.go | 5 ++- crypto/vss/feldman_vss_test.go | 18 ++++++++++ ecdsa/keygen/local_party_test.go | 40 +++++++++++++++++++++++ ecdsa/signing/local_party_test.go | 49 ++++++++++++++++++++++++++++ ecdsa/signing/prepare.go | 4 +++ tss/message.go | 7 +++- 14 files changed, 218 insertions(+), 12 deletions(-) diff --git a/common/random.go b/common/random.go index acda0451..81d07ff4 100644 --- a/common/random.go +++ b/common/random.go @@ -49,13 +49,30 @@ func GetRandomPositiveInt(lessThan *big.Int) *big.Int { return try } +func getRandomNonNegativeInt(lessThan *big.Int) *big.Int { + if lessThan == nil || lessThan.Sign() <= 0 { + return nil + } + var try *big.Int + for { + try = MustGetRandomInt(lessThan.BitLen()) + if try.Cmp(lessThan) < 0 { + break + } + } + return try +} + // Sample an integer in range (-limit, limit) func GetRandomInt(limit *big.Int) *big.Int { + if limit == nil || limit.Sign() <= 0 { + return nil + } limitMinus1 := new(big.Int).Sub(limit, big.NewInt(1)) limitDoubleMinus1 := new(big.Int).Add(limit, limitMinus1) // get an integer in [0, 2*limit-1) and subtract limit-1 // to get an integer in [-limit+1, limit-1] - i := GetRandomPositiveInt(limitDoubleMinus1) + i := getRandomNonNegativeInt(limitDoubleMinus1) i = i.Sub(i, limitMinus1) return i } diff --git a/common/random_test.go b/common/random_test.go index c1d51c8d..8c18e37e 100644 --- a/common/random_test.go +++ b/common/random_test.go @@ -37,6 +37,12 @@ func TestGetRandomPositiveIntRejectsNoPositiveRange(t *testing.T) { assert.Nil(t, common.GetRandomPositiveInt(big.NewInt(1))) } +func TestGetRandomIntPreservesZeroInclusiveRange(t *testing.T) { + assert.Nil(t, common.GetRandomInt(nil)) + assert.Nil(t, common.GetRandomInt(big.NewInt(0))) + assert.Zero(t, common.GetRandomInt(big.NewInt(1)).Sign()) +} + func TestGetRandomPositiveRelativelyPrimeInt(t *testing.T) { rnd := common.MustGetRandomInt(randomIntBitLen) rndPosRP := common.GetRandomPositiveRelativelyPrimeInt(rnd) diff --git a/crypto/ecpoint.go b/crypto/ecpoint.go index 8761f86c..6d2852da 100644 --- a/crypto/ecpoint.go +++ b/crypto/ecpoint.go @@ -83,6 +83,29 @@ func (p *ECPoint) Curve() elliptic.Curve { return p.curve } +func SameCurve(lhs, rhs elliptic.Curve) bool { + if lhs == nil || rhs == nil { + return false + } + lParams, rParams := lhs.Params(), rhs.Params() + if lParams == nil || rParams == nil { + return false + } + return sameBigInt(lParams.P, rParams.P) && + sameBigInt(lParams.N, rParams.N) && + sameBigInt(lParams.B, rParams.B) && + sameBigInt(lParams.Gx, rParams.Gx) && + sameBigInt(lParams.Gy, rParams.Gy) && + lParams.BitSize == rParams.BitSize +} + +func sameBigInt(lhs, rhs *big.Int) bool { + if lhs == nil || rhs == nil { + return lhs == rhs + } + return lhs.Cmp(rhs) == 0 +} + func (p *ECPoint) Equals(p2 *ECPoint) bool { if p == nil || p2 == nil { return false @@ -111,6 +134,8 @@ func (p *ECPoint) IsIdentity() bool { if p == nil || p.coords[0] == nil || p.coords[1] == nil { return false } + // The supported curves encode usable affine points away from x=0; reject + // common identity-like encodings before arithmetic reaches curve methods. if p.coords[0].Sign() != 0 { return false } diff --git a/crypto/mta/proofs.go b/crypto/mta/proofs.go index 0cd40498..9c36e25d 100644 --- a/crypto/mta/proofs.go +++ b/crypto/mta/proofs.go @@ -15,7 +15,6 @@ import ( "github.com/bnb-chain/tss-lib/common" "github.com/bnb-chain/tss-lib/crypto" "github.com/bnb-chain/tss-lib/crypto/paillier" - "github.com/bnb-chain/tss-lib/tss" ) const ( @@ -300,10 +299,10 @@ func (pf *ProofBobWC) Verify(ec elliptic.Curve, pk *paillier.PublicKey, NTilde, if X == nil { eHash = common.SHA512_256i_TAGGED(fsSessionBob(Session), append(pk.AsInts(), NTilde, h1, h2, c1, c2, pf.Z, pf.ZPrm, pf.T, pf.V, pf.W)...) } else { - if !X.ValidateBasic() || !tss.SameCurve(ec, X.Curve()) { + if !X.ValidateBasic() || !crypto.SameCurve(ec, X.Curve()) { return false } - if !pf.U.ValidateBasic() || !tss.SameCurve(ec, pf.U.Curve()) { + if !pf.U.ValidateBasic() || !crypto.SameCurve(ec, pf.U.Curve()) { return false } eHash = common.SHA512_256i_TAGGED(fsSessionBobWC(Session), append(pk.AsInts(), NTilde, h1, h2, X.X(), X.Y(), c1, c2, pf.U.X(), pf.U.Y(), pf.Z, pf.ZPrm, pf.T, pf.V, pf.W)...) diff --git a/crypto/paillier/mod_proof.go b/crypto/paillier/mod_proof.go index 5608d042..de76adcb 100644 --- a/crypto/paillier/mod_proof.go +++ b/crypto/paillier/mod_proof.go @@ -169,7 +169,9 @@ func sampleYModN(tag []byte, N *big.Int, inputs ...*big.Int) *big.Int { binary.BigEndian.PutUint32(counterBytes, counter) out := make([]byte, 0, blocks*32) for blockIdx := 0; blockIdx < blocks; blockIdx++ { - out = append(out, common.SHA512_256(seed, counterBytes, []byte{byte(blockIdx)})...) + blockIdxBytes := make([]byte, 4) + binary.BigEndian.PutUint32(blockIdxBytes, uint32(blockIdx)) + out = append(out, common.SHA512_256(seed, counterBytes, blockIdxBytes)...) } out = out[:byteLen] if excessBits > 0 { diff --git a/crypto/paillier/mod_proof_test.go b/crypto/paillier/mod_proof_test.go index 4596c58a..fff4467f 100644 --- a/crypto/paillier/mod_proof_test.go +++ b/crypto/paillier/mod_proof_test.go @@ -101,6 +101,21 @@ func TestModChallenge_SessionPath_ChainsPreviousChallenges(t *testing.T) { } } +func TestSampleYModNDeterministicAndSupportsManyBlocks(t *testing.T) { + N := new(big.Int).Lsh(one, 32*257*8) + N.Sub(N, one) + tag := []byte("sample-y-large-modulus-test") + inputs := []*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(3)} + + y1 := sampleYModN(tag, N, inputs...) + y2 := sampleYModN(tag, N, inputs...) + + assert.Equal(t, y1, y2) + assert.True(t, y1.Sign() >= 0) + assert.True(t, y1.Cmp(N) < 0) + assert.True(t, N.BitLen() > 32*256*8) +} + func TestModProofVerifyFail(t *testing.T) { modSetUp(t) proof := privateKey.ModProof() diff --git a/crypto/schnorr/schnorr_proof.go b/crypto/schnorr/schnorr_proof.go index 799e8eda..c85b6400 100644 --- a/crypto/schnorr/schnorr_proof.go +++ b/crypto/schnorr/schnorr_proof.go @@ -12,7 +12,6 @@ import ( "github.com/bnb-chain/tss-lib/common" "github.com/bnb-chain/tss-lib/crypto" - "github.com/bnb-chain/tss-lib/tss" ) type ( @@ -78,7 +77,7 @@ func (pf *ZKProof) VerifyWithSession(session []byte, X *crypto.ECPoint) bool { if pf == nil || !pf.ValidateBasic() || X == nil || !X.ValidateBasic() { return false } - if !tss.SameCurve(X.Curve(), pf.Alpha.Curve()) { + if !crypto.SameCurve(X.Curve(), pf.Alpha.Curve()) { return false } ec := X.Curve() @@ -153,7 +152,7 @@ func (pf *ZKVProof) VerifyWithSession(session []byte, V, R *crypto.ECPoint) bool V == nil || R == nil || !V.ValidateBasic() || !R.ValidateBasic() { return false } - if !tss.SameCurve(V.Curve(), R.Curve()) || !tss.SameCurve(V.Curve(), pf.Alpha.Curve()) { + if !crypto.SameCurve(V.Curve(), R.Curve()) || !crypto.SameCurve(V.Curve(), pf.Alpha.Curve()) { return false } ec := V.Curve() diff --git a/crypto/schnorr/schnorr_proof_test.go b/crypto/schnorr/schnorr_proof_test.go index e5040e7d..d97b3fe6 100644 --- a/crypto/schnorr/schnorr_proof_test.go +++ b/crypto/schnorr/schnorr_proof_test.go @@ -7,6 +7,7 @@ package schnorr_test import ( + "crypto/elliptic" "math/big" "testing" @@ -41,6 +42,17 @@ func TestSchnorrProofVerify(t *testing.T) { assert.True(t, res, "verify result must be true") } +func TestSchnorrProofVerifyAllowsUnregisteredCurve(t *testing.T) { + ec := elliptic.P256() + q := ec.Params().N + u := common.GetRandomPositiveInt(q) + X := crypto.ScalarBaseMult(ec, u) + + proof, err := NewZKProof(u, X) + assert.NoError(t, err) + assert.True(t, proof.Verify(X), "ZK proof must verify on an unregistered curve") +} + func TestSchnorrProofVerifySessionBinding(t *testing.T) { q := tss.EC().Params().N u := common.GetRandomPositiveInt(q) @@ -99,6 +111,22 @@ func TestSchnorrVProofVerify(t *testing.T) { assert.True(t, res, "verify result must be true") } +func TestSchnorrVProofVerifyAllowsUnregisteredCurve(t *testing.T) { + ec := elliptic.P256() + q := ec.Params().N + k := common.GetRandomPositiveInt(q) + s := common.GetRandomPositiveInt(q) + l := common.GetRandomPositiveInt(q) + R := crypto.ScalarBaseMult(ec, k) + Rs := R.ScalarMult(s) + lG := crypto.ScalarBaseMult(ec, l) + V, _ := Rs.Add(lG) + + proof, err := NewZKVProof(V, R, s, l) + assert.NoError(t, err) + assert.True(t, proof.Verify(V, R), "ZKV proof must verify on an unregistered curve") +} + func TestSchnorrVProofVerifyRejectsZeroScalars(t *testing.T) { q := tss.EC().Params().N k := common.GetRandomPositiveInt(q) diff --git a/crypto/vss/feldman_vss.go b/crypto/vss/feldman_vss.go index 7242a9df..2d225c34 100644 --- a/crypto/vss/feldman_vss.go +++ b/crypto/vss/feldman_vss.go @@ -18,7 +18,6 @@ import ( "github.com/bnb-chain/tss-lib/common" "github.com/bnb-chain/tss-lib/crypto" - "github.com/bnb-chain/tss-lib/tss" ) type ( @@ -108,11 +107,11 @@ func (share *Share) Verify(ec elliptic.Curve, threshold int, vs Vs) bool { var err error modQ := common.ModInt(q) v, t := vs[0], one // YRO : we need to have our accumulator outside of the loop - if v == nil || !tss.SameCurve(v.Curve(), ec) || !v.ValidateBasic() { + if v == nil || !crypto.SameCurve(v.Curve(), ec) || !v.ValidateBasic() { return false } for j := 1; j <= threshold; j++ { - if vs[j] == nil || !tss.SameCurve(vs[j].Curve(), ec) || !vs[j].ValidateBasic() { + if vs[j] == nil || !crypto.SameCurve(vs[j].Curve(), ec) || !vs[j].ValidateBasic() { return false } // t = k_i^j diff --git a/crypto/vss/feldman_vss_test.go b/crypto/vss/feldman_vss_test.go index cfcbe030..fda85365 100644 --- a/crypto/vss/feldman_vss_test.go +++ b/crypto/vss/feldman_vss_test.go @@ -7,6 +7,7 @@ package vss_test import ( + "crypto/elliptic" "math/big" "testing" @@ -90,6 +91,23 @@ func TestVerify(t *testing.T) { assert.False(t, shares[0].Verify(tss.EC(), threshold, vs[:threshold])) } +func TestVerifyAllowsUnregisteredCurve(t *testing.T) { + ec := elliptic.P256() + num, threshold := 5, 3 + secret := common.GetRandomPositiveInt(ec.Params().N) + + ids := make([]*big.Int, 0, num) + for i := 1; i <= num; i++ { + ids = append(ids, big.NewInt(int64(i))) + } + + vs, shares, err := Create(ec, threshold, secret, ids) + assert.NoError(t, err) + for i := 0; i < num; i++ { + assert.True(t, shares[i].Verify(ec, threshold, vs)) + } +} + func TestVerifyRejectsMalformedShare(t *testing.T) { num, threshold := 5, 3 diff --git a/ecdsa/keygen/local_party_test.go b/ecdsa/keygen/local_party_test.go index ca25ec87..0dd661a5 100644 --- a/ecdsa/keygen/local_party_test.go +++ b/ecdsa/keygen/local_party_test.go @@ -58,6 +58,46 @@ func testKeygenSSID(pIDs tss.SortedPartyIDs, sessionID []byte) []byte { return round.getSSID() } +func TestStoreMessageRejectsContentDifferentReplay(t *testing.T) { + pIDs := tss.GenerateTestPartyIDs(2) + params := tss.NewParameters(tss.S256(), tss.NewPeerContext(pIDs), pIDs[0], len(pIDs), 1) + lp := NewLocalParty(params, nil, nil).(*LocalParty) + + msg1 := NewKGRound2Message2(pIDs[1], []*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(3)}) + ok, err := lp.StoreMessage(msg1) + assert.True(t, ok) + assert.Nil(t, err) + + redelivery := NewKGRound2Message2(pIDs[1], []*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(3)}) + assert.True(t, tss.IsSameMessage(msg1, redelivery)) + ok, err = lp.StoreMessage(redelivery) + assert.True(t, ok) + assert.Nil(t, err) + + replacement := NewKGRound2Message2(pIDs[1], []*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(4)}) + assert.False(t, tss.IsSameMessage(msg1, replacement)) + ok, err = lp.StoreMessage(replacement) + assert.False(t, ok) + assert.Error(t, err) + assert.Contains(t, err.Error(), "duplicate") +} + +func TestStoreMessageAllowsSelfReplacement(t *testing.T) { + pIDs := tss.GenerateTestPartyIDs(2) + params := tss.NewParameters(tss.S256(), tss.NewPeerContext(pIDs), pIDs[0], len(pIDs), 1) + lp := NewLocalParty(params, nil, nil).(*LocalParty) + + msg1 := NewKGRound2Message2(pIDs[0], []*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(3)}) + replacement := NewKGRound2Message2(pIDs[0], []*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(4)}) + + ok, err := lp.StoreMessage(msg1) + assert.True(t, ok) + assert.Nil(t, err) + ok, err = lp.StoreMessage(replacement) + assert.True(t, ok) + assert.Nil(t, err) +} + func setUp(level string) { if err := log.SetLogLevel("tss-lib", level); err != nil { panic(err) diff --git a/ecdsa/signing/local_party_test.go b/ecdsa/signing/local_party_test.go index ead6380b..54ca896a 100644 --- a/ecdsa/signing/local_party_test.go +++ b/ecdsa/signing/local_party_test.go @@ -37,6 +37,55 @@ func setUp(level string) { } } +func TestStoreMessageRejectsContentDifferentReplay(t *testing.T) { + lp, pIDs := newStoreMessageTestParty(t) + + msg1 := NewSignRound3Message(pIDs[1], big.NewInt(1)) + ok, err := lp.StoreMessage(msg1) + assert.True(t, ok) + assert.Nil(t, err) + + redelivery := NewSignRound3Message(pIDs[1], big.NewInt(1)) + assert.True(t, tss.IsSameMessage(msg1, redelivery)) + ok, err = lp.StoreMessage(redelivery) + assert.True(t, ok) + assert.Nil(t, err) + + replacement := NewSignRound3Message(pIDs[1], big.NewInt(2)) + assert.False(t, tss.IsSameMessage(msg1, replacement)) + ok, err = lp.StoreMessage(replacement) + assert.False(t, ok) + assert.Error(t, err) + assert.Contains(t, err.Error(), "duplicate") +} + +func TestStoreMessageAllowsSelfReplacement(t *testing.T) { + lp, pIDs := newStoreMessageTestParty(t) + + msg1 := NewSignRound3Message(pIDs[0], big.NewInt(1)) + replacement := NewSignRound3Message(pIDs[0], big.NewInt(2)) + + ok, err := lp.StoreMessage(msg1) + assert.True(t, ok) + assert.Nil(t, err) + ok, err = lp.StoreMessage(replacement) + assert.True(t, ok) + assert.Nil(t, err) +} + +func newStoreMessageTestParty(t *testing.T) (*LocalParty, tss.SortedPartyIDs) { + t.Helper() + + pIDs := tss.GenerateTestPartyIDs(2) + params := tss.NewParameters(tss.S256(), tss.NewPeerContext(pIDs), pIDs[0], len(pIDs), 1) + keys := keygen.NewLocalPartySaveData(len(pIDs)) + for i, id := range pIDs { + keys.Ks[i] = id.KeyInt() + } + lp := NewLocalParty(big.NewInt(1), params, keys, nil, nil, 32).(*LocalParty) + return lp, pIDs +} + func TestE2EConcurrent(t *testing.T) { setUp("info") threshold := testThreshold diff --git a/ecdsa/signing/prepare.go b/ecdsa/signing/prepare.go index 7e818f34..17fcbbaf 100644 --- a/ecdsa/signing/prepare.go +++ b/ecdsa/signing/prepare.go @@ -40,6 +40,8 @@ func PrepareForSigning(ec elliptic.Curve, i, pax int, xi *big.Int, ks []*big.Int if new(big.Int).Mod(ksj, q).Cmp(new(big.Int).Mod(ksi, q)) == 0 { return nil, nil, fmt.Errorf("PrepareForSigning: party keys at indices %d and %d collide mod q", j, i) } + // The denominator is inverted modulo q, so equality has to be checked + // modulo q even when the raw key encodings differ. // big.Int Div is calculated as: a/b = a * modInv(b,q) coef := modQ.Mul(ks[j], modQ.ModInverse(new(big.Int).Sub(ksj, ksi))) wi = modQ.Mul(wi, coef) @@ -58,6 +60,8 @@ func PrepareForSigning(ec elliptic.Curve, i, pax int, xi *big.Int, ks []*big.Int if new(big.Int).Mod(ksj, q).Cmp(new(big.Int).Mod(ksc, q)) == 0 { return nil, nil, fmt.Errorf("PrepareForSigning: party keys at indices %d and %d collide mod q", j, c) } + // The denominator is inverted modulo q, so equality has to be checked + // modulo q even when the raw key encodings differ. // big.Int Div is calculated as: a/b = a * modInv(b,q) iota := modQ.Mul(ksc, modQ.ModInverse(new(big.Int).Sub(ksc, ksj))) bigWj = bigWj.ScalarMult(iota) diff --git a/tss/message.go b/tss/message.go index ca99a418..80a88686 100644 --- a/tss/message.go +++ b/tss/message.go @@ -83,6 +83,11 @@ var ( func NewMessageWrapper(routing MessageRouting, content MessageContent) *MessageWrapper { // marshal the content to the ProtoBuf Any type any, _ := anypb.New(content) + if any != nil { + if bz, err := (proto.MarshalOptions{Deterministic: true}).Marshal(content); err == nil { + any.Value = bz + } + } // convert given PartyIDs to the wire format var to []*MessageWrapper_PartyID if routing.To != nil { @@ -138,7 +143,7 @@ func (mm *MessageImpl) IsToOldAndNewCommittees() bool { } func (mm *MessageImpl) WireBytes() ([]byte, *MessageRouting, error) { - bz, err := proto.Marshal(mm.wire.Message) + bz, err := proto.MarshalOptions{Deterministic: true}.Marshal(mm.wire.Message) if err != nil { return nil, nil, err } From 8994c3e8cc95a0951897d871c0fd0e1c5f640937 Mon Sep 17 00:00:00 2001 From: maclane Date: Thu, 28 May 2026 14:44:35 -0500 Subject: [PATCH 5/6] Polish hardening review follow-ups --- crypto/ecpoint.go | 3 +++ crypto/paillier/mod_proof_test.go | 2 ++ ecdsa/keygen/local_party.go | 2 +- ecdsa/keygen/local_party_test.go | 3 ++- ecdsa/signing/local_party.go | 2 +- ecdsa/signing/local_party_test.go | 3 ++- tss/message.go | 13 ++++++++----- tss/party.go | 2 ++ 8 files changed, 21 insertions(+), 9 deletions(-) diff --git a/crypto/ecpoint.go b/crypto/ecpoint.go index 6d2852da..2d1e9bcf 100644 --- a/crypto/ecpoint.go +++ b/crypto/ecpoint.go @@ -83,6 +83,9 @@ func (p *ECPoint) Curve() elliptic.Curve { return p.curve } +// SameCurve compares curve domain parameters, not implementation identity. It +// intentionally accepts distinct elliptic.Curve implementations with identical +// parameters; callers must still trust the curve implementation they pass in. func SameCurve(lhs, rhs elliptic.Curve) bool { if lhs == nil || rhs == nil { return false diff --git a/crypto/paillier/mod_proof_test.go b/crypto/paillier/mod_proof_test.go index fff4467f..f25b5713 100644 --- a/crypto/paillier/mod_proof_test.go +++ b/crypto/paillier/mod_proof_test.go @@ -102,6 +102,8 @@ func TestModChallenge_SessionPath_ChainsPreviousChallenges(t *testing.T) { } func TestSampleYModNDeterministicAndSupportsManyBlocks(t *testing.T) { + // Force 257 SHA512_256 blocks, exceeding the former uint8 block-index + // capacity that would have collided block 256 with block 0. N := new(big.Int).Lsh(one, 32*257*8) N.Sub(N, one) tag := []byte("sample-y-large-modulus-test") diff --git a/ecdsa/keygen/local_party.go b/ecdsa/keygen/local_party.go index d287301b..f20c5003 100644 --- a/ecdsa/keygen/local_party.go +++ b/ecdsa/keygen/local_party.go @@ -140,7 +140,7 @@ func (p *LocalParty) StoreMessage(msg tss.ParsedMessage) (bool, *tss.Error) { isDup := fromPIdx != p.PartyID().Index dupErr := func() (bool, *tss.Error) { return false, p.WrapError( - fmt.Errorf("duplicate %T from party %d", msg.Content(), fromPIdx), + fmt.Errorf("%w: %T from party %d", tss.ErrDuplicateMessage, msg.Content(), fromPIdx), msg.GetFrom()) } switch msg.Content().(type) { diff --git a/ecdsa/keygen/local_party_test.go b/ecdsa/keygen/local_party_test.go index 0dd661a5..137edd4e 100644 --- a/ecdsa/keygen/local_party_test.go +++ b/ecdsa/keygen/local_party_test.go @@ -10,6 +10,7 @@ import ( "crypto/ecdsa" "crypto/rand" "encoding/json" + "errors" "fmt" "math/big" "os" @@ -79,7 +80,7 @@ func TestStoreMessageRejectsContentDifferentReplay(t *testing.T) { ok, err = lp.StoreMessage(replacement) assert.False(t, ok) assert.Error(t, err) - assert.Contains(t, err.Error(), "duplicate") + assert.True(t, errors.Is(err, tss.ErrDuplicateMessage)) } func TestStoreMessageAllowsSelfReplacement(t *testing.T) { diff --git a/ecdsa/signing/local_party.go b/ecdsa/signing/local_party.go index f1fa736f..8fafe9b4 100644 --- a/ecdsa/signing/local_party.go +++ b/ecdsa/signing/local_party.go @@ -240,7 +240,7 @@ func (p *LocalParty) StoreMessage(msg tss.ParsedMessage) (bool, *tss.Error) { isDup := fromPIdx != p.PartyID().Index dupErr := func() (bool, *tss.Error) { return false, p.WrapError( - fmt.Errorf("duplicate %T from party %d", msg.Content(), fromPIdx), + fmt.Errorf("%w: %T from party %d", tss.ErrDuplicateMessage, msg.Content(), fromPIdx), msg.GetFrom()) } switch msg.Content().(type) { diff --git a/ecdsa/signing/local_party_test.go b/ecdsa/signing/local_party_test.go index 54ca896a..6ecd0d91 100644 --- a/ecdsa/signing/local_party_test.go +++ b/ecdsa/signing/local_party_test.go @@ -9,6 +9,7 @@ package signing import ( "crypto/ecdsa" "encoding/hex" + "errors" "fmt" "math/big" "runtime" @@ -56,7 +57,7 @@ func TestStoreMessageRejectsContentDifferentReplay(t *testing.T) { ok, err = lp.StoreMessage(replacement) assert.False(t, ok) assert.Error(t, err) - assert.Contains(t, err.Error(), "duplicate") + assert.True(t, errors.Is(err, tss.ErrDuplicateMessage)) } func TestStoreMessageAllowsSelfReplacement(t *testing.T) { diff --git a/tss/message.go b/tss/message.go index 80a88686..39ab8be4 100644 --- a/tss/message.go +++ b/tss/message.go @@ -82,12 +82,15 @@ var ( // NewMessageWrapper constructs a MessageWrapper from routing metadata and content func NewMessageWrapper(routing MessageRouting, content MessageContent) *MessageWrapper { // marshal the content to the ProtoBuf Any type - any, _ := anypb.New(content) - if any != nil { - if bz, err := (proto.MarshalOptions{Deterministic: true}).Marshal(content); err == nil { - any.Value = bz - } + any, err := anypb.New(content) + if err != nil { + panic(fmt.Errorf("NewMessageWrapper: marshal content into Any: %w", err)) + } + bz, err := (proto.MarshalOptions{Deterministic: true}).Marshal(content) + if err != nil { + panic(fmt.Errorf("NewMessageWrapper: deterministic marshal content: %w", err)) } + any.Value = bz // convert given PartyIDs to the wire format var to []*MessageWrapper_PartyID if routing.To != nil { diff --git a/tss/party.go b/tss/party.go index f7e3d246..5aa73533 100644 --- a/tss/party.go +++ b/tss/party.go @@ -14,6 +14,8 @@ import ( "github.com/bnb-chain/tss-lib/common" ) +var ErrDuplicateMessage = errors.New("duplicate message") + type Party interface { Start() *Error // The main entry point when updating a party's state from the wire. From 03554f768bb354b0b099d556339bc41191423ab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Mon, 15 Jun 2026 09:38:25 +0000 Subject: [PATCH 6/6] docs(changelog): document PR #6 BNB hardening follow-ups Record PR #6's changes: per-proof-system Fiat-Shamir domain tags (further transcript change), the PrepareForSigning error-return source break, and stricter NewParameters/SortPartyIDs validation as breaking changes; plus shared input validators, VSS reconstruction checks, idempotent message redelivery, and review-follow-up correctness fixes as non-breaking hardening. Add PR #6 to the composing-PRs list and update the source-compatibility note for PrepareForSigning. --- CHANGELOG.md | 62 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d267c0f..6b380d59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ belongs to PR #2 (the base BNB hardening integration) unless it is tagged with a - **PR #2** — base BNB hardening integration. - **PR #4** — BNB #332 tBTC-relevant hardening backport (stacked on PR #2). - **PR #5** — removal of EdDSA and ECDSA resharing protocols (stacked on PR #4). +- **PR #6** — remaining BNB cryptographic hardening follow-ups (stacked on PR #5). ### ⚠️ Compatibility — read before upgrading @@ -120,13 +121,48 @@ Two new caller obligations are enforced at runtime (see Breaking Changes 1 and 2 so in practice this is active on the protocol path. - **Migration:** Covered by the coordinated upgrade in Breaking Change 1/3. -> Every session / `fullBytesLen` parameter was added as a trailing variadic argument, so the -> hardening itself changed no exported signatures (verified by diffing exported signatures -> between base and HEAD); those breaks are runtime/wire, not compile-time. The source/compile -> breaks in this set come from PR #5's protocol removal, which deleted the exported +#### 5. Per-proof-system Fiat-Shamir domain tags (PR #6) +- **What:** DLN, Schnorr, MtA, and Paillier challenges now prepend a per-proof-system domain + tag (e.g. `dlnproof|`, `zk|`, `zkv|`, via `fsDomainTag*` / `fsSession*`) to the session + before tagged hashing. This further changes every proof transcript relative to Breaking + Change 3. +- **Break type:** Wire/protocol — compounds Breaking Change 3; still a single coordinated + upgrade (a PR #6 node and a PR #2–#5 node will not cross-verify). +- **Motivation:** Distinct domain separation per proof system, so a challenge from one proof + type can never be reused in another. +- **Provenance:** `BNB #252` / `BNB #256` domain-tag design; PR #6. +- **Migration:** Covered by the coordinated upgrade in Breaking Change 1/3. + +#### 6. `ecdsa/signing.PrepareForSigning` returns an error (PR #6) +- **What:** the exported signature changed from `(wi, bigWs)` to `(wi, bigWs, err)`; it now + validates its inputs and returns an error instead of proceeding on malformed data + (`ecdsa/signing/prepare.go`). +- **Break type:** Source/compile — downstream callers must handle the third return value. +- **Motivation:** Surface invalid signing-preparation inputs instead of producing corrupt + signing state. +- **Provenance:** BNB hardening follow-ups; PR #6. (A code search found no current + `threshold-network/keep-core` callers.) +- **Migration:** Update call sites to handle the returned `error`. + +#### 7. Stricter `tss.NewParameters` and `SortPartyIDs` validation (PR #6) +- **What:** `NewParameters` now panics on a party count below 2, a threshold outside + `[1, partyCount)`, a `PartyID` key congruent to 0 mod q, or two `PartyID`s colliding mod q; + `SortPartyIDs` panics on duplicate raw party keys (`tss/params.go`, `tss/party_id.go`). +- **Break type:** Runtime — rejects previously-accepted but invalid/degenerate party sets. + Honest setups with ≥2 distinct, non-colliding parties and a valid threshold are unaffected. +- **Motivation:** Fail fast on malformed party sets that would otherwise corrupt VSS or the + protocol. +- **Provenance:** `threshold-original` / BNB hardening; PR #6. +- **Migration:** Ensure ceremonies use ≥2 distinct parties, a threshold in `[1, partyCount)`, + and non-colliding keys (normal configurations already satisfy this). + +> Source/compile breaks in this set: `ecdsa/signing.PrepareForSigning` gained an `error` return +> (Breaking Change 6, PR #6), and PR #5's protocol removal deleted the exported > `tss.ReSharingParameters` / `tss.NewReSharingParameters`, `crypto.ECPoint.EightInvEight`, and -> `ecdsa/resharing.NewDGRound1Message` API (see Removed). Downstream code using EdDSA, ECDSA -> resharing, or those symbols must adapt. +> `ecdsa/resharing.NewDGRound1Message` API (see Removed). Otherwise every session / +> `fullBytesLen` parameter was added as a trailing variadic argument, so all remaining call +> sites compile unchanged; those breaks are runtime/wire. Verified by diffing exported +> signatures between base and HEAD. ### Removed @@ -227,6 +263,20 @@ rejecting input that an honest caller would previously have produced. - **ECDSA signing round-4 nil theta-inverse guard (PR #4):** a non-invertible theta (`ModInverse` returning nil) is rejected with a clean error instead of propagating nil (`ecdsa/signing/round_4.go`). _Provenance: `BNB #332`, PR #4._ +- **Shared cryptographic input validators (PR #6):** `common/validation.go` adds reusable + canonical checks for unknown-order moduli, generators, and Paillier ciphertexts, wired into + the proof verifiers and round handlers. _Provenance: `BNB #252`/`BNB #332`, PR #6._ +- **VSS reconstruction input validation (PR #6):** `feldman_vss` rejects malformed + reconstruction inputs and out-of-bound parameters before use (`crypto/vss/feldman_vss.go`). + _Provenance: `BNB #332`, PR #6._ +- **Idempotent message redelivery (PR #6):** keygen/signing message storage treats an + identical redelivery from a party as a no-op while rejecting a content-different replay, + preventing duplicate-message state corruption (`tss/message.go`). _Provenance: `threshold-original`, PR #6._ +- **Review follow-up correctness fixes (PR #6):** Schnorr verification accepts unregistered + generic curves; `common.GetRandomInt`'s zero-inclusive range is corrected; message wire + bytes are made deterministic; large-modulus `sampleYModN` block indexing is fixed; and + canonical-generator checks were added in `crypto/commitments` and `crypto/paillier`. + _Provenance: `BNB #332` + `threshold-original`, PR #6._ ### Added