diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d267c0fc..6b380d59d 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 diff --git a/common/hash_utils.go b/common/hash_utils.go index 616a1ffd4..2df8290f7 100644 --- a/common/hash_utils.go +++ b/common/hash_utils.go @@ -21,14 +21,31 @@ func LiterallyJustMod(q *big.Int, eHash *big.Int) *big.Int { // e' = eHash return e } -// 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. -func RejectionSample(q *big.Int, eHash *big.Int) *big.Int { +// 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)). +// +// 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 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 ad487c517..e7c016046 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 6a247d296..81d07ff43 100644 --- a/common/random.go +++ b/common/random.go @@ -36,13 +36,27 @@ 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 + } + } + 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 } } @@ -51,11 +65,14 @@ func GetRandomPositiveInt(lessThan *big.Int) *big.Int { // 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 4a9eb3c8b..8c18e37ec 100644 --- a/common/random_test.go +++ b/common/random_test.go @@ -31,6 +31,18 @@ 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 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/common/validation.go b/common/validation.go new file mode 100644 index 000000000..3cf09e145 --- /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 b58a3ced9..501a576c6 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 bf9b8aad9..142152144 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 6e16c9d08..2d1e9bcf7 100644 --- a/crypto/ecpoint.go +++ b/crypto/ecpoint.go @@ -83,6 +83,32 @@ 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 + } + 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 @@ -90,6 +116,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 @@ -99,7 +130,19 @@ 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 + } + // 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 + } + 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 0128a9f0f..9c36e25d3 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 ( @@ -38,7 +37,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 +102,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 +202,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 +224,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 +264,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 +297,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()) { + if !X.ValidateBasic() || !crypto.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() || !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)...) } - 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 +324,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 2c5c7fd06..6598787cd 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 5ec95e1ac..57cc8b1e8 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) @@ -177,6 +188,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 @@ -187,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 f39c48c3f..de76adcbc 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,39 @@ 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++ { + 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 { + 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/mod_proof_test.go b/crypto/paillier/mod_proof_test.go index 4596c58af..f25b57130 100644 --- a/crypto/paillier/mod_proof_test.go +++ b/crypto/paillier/mod_proof_test.go @@ -101,6 +101,23 @@ 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") + 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/paillier/paillier.go b/crypto/paillier/paillier.go index d5cdb2582..6651e86c6 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 d5a2ccd84..c85b64009 100644 --- a/crypto/schnorr/schnorr_proof.go +++ b/crypto/schnorr/schnorr_proof.go @@ -26,6 +26,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 +58,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 +77,9 @@ func (pf *ZKProof) VerifyWithSession(session []byte, X *crypto.ECPoint) bool { if pf == nil || !pf.ValidateBasic() || X == nil || !X.ValidateBasic() { return false } + if !crypto.SameCurve(X.Curve(), pf.Alpha.Curve()) { + return false + } ec := X.Curve() ecParams := ec.Params() q := ecParams.N @@ -72,8 +88,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 +131,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 +152,9 @@ func (pf *ZKVProof) VerifyWithSession(session []byte, V, R *crypto.ECPoint) bool V == nil || R == nil || !V.ValidateBasic() || !R.ValidateBasic() { return false } + if !crypto.SameCurve(V.Curve(), R.Curve()) || !crypto.SameCurve(V.Curve(), pf.Alpha.Curve()) { + return false + } ec := V.Curve() ecParams := ec.Params() q := ecParams.N @@ -144,8 +163,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 +174,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/schnorr/schnorr_proof_test.go b/crypto/schnorr/schnorr_proof_test.go index e5040e7d0..d97b3fe6a 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 736e8e4eb..2d225c34a 100644 --- a/crypto/vss/feldman_vss.go +++ b/crypto/vss/feldman_vss.go @@ -59,6 +59,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 +75,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 +107,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 || !crypto.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 || !crypto.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,17 +134,49 @@ 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 != nil && shares[0].Threshold+1 > len(shares) { + if shares[0] == nil { + return nil, errors.New("vss reconstruct: nil share") + } + 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 } - 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 8192891d7..fda85365d 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 @@ -169,3 +187,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/local_party.go b/ecdsa/keygen/local_party.go index a5b8e9689..f20c50031 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("%w: %T from party %d", tss.ErrDuplicateMessage, 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 8d8fb4ea2..137edd4e2 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" @@ -58,6 +59,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.True(t, errors.Is(err, tss.ErrDuplicateMessage)) +} + +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) @@ -97,7 +138,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 +179,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/keygen/rounds.go b/ecdsa/keygen/rounds.go index 9d9c18583..85c0a24ed 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/local_party.go b/ecdsa/signing/local_party.go index 9fe9f2741..8fafe9b4f 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("%w: %T from party %d", tss.ErrDuplicateMessage, 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 306accf65..6ecd0d919 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" @@ -37,6 +38,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.True(t, errors.Is(err, tss.ErrDuplicateMessage)) +} + +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 @@ -347,8 +397,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 60f107bf4..17fcbbafb 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,9 +37,11 @@ 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) } + // 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) @@ -54,14 +57,16 @@ 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) } + // 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) } bigWs[j] = bigWj } - return + return wi, bigWs, nil } diff --git a/ecdsa/signing/round_1.go b/ecdsa/signing/round_1.go index f4f938d4e..1b9355306 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/ecdsa/signing/rounds.go b/ecdsa/signing/rounds.go index 82f7d6b63..a58566898 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/message.go b/tss/message.go index 9aa8e59e4..39ab8be4b 100644 --- a/tss/message.go +++ b/tss/message.go @@ -7,6 +7,7 @@ package tss import ( + "bytes" "fmt" "google.golang.org/protobuf/proto" @@ -81,7 +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) + 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 { @@ -137,7 +146,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 } @@ -167,3 +176,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 0ee4bc7cd..2245a056f 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 } @@ -109,17 +142,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 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. // -// 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. +// 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") diff --git a/tss/params_test.go b/tss/params_test.go index 358fce167..c9fed7a1b 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.go b/tss/party.go index f7e3d2463..5aa735334 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. diff --git a/tss/party_id.go b/tss/party_id.go index 614f85904..16d7a8693 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) {