Integrate BNB tss-lib hardening

.github/workflows/gofmt.yml

@@ -1,6 +1,9 @@
 name: Go-fmt
 on:
   push:
+    branches:
+    - master
+    - release/*
   pull_request:
     branches:
     - master

.github/workflows/test.yml

@@ -1,6 +1,9 @@
 name: Go Test
 on:
   push:
+    branches:
+    - master
+    - release/*
   pull_request:
     branches:
     - master

BNB_HARDENING_INTEGRATION.md

@@ -0,0 +1,80 @@
+# BNB Hardening Integration Report
+
+## Scope
+
+- Threshold base: `2e712689cfbeefede15f95a0ec7112227d86f702`
+- BNB upstream head compared: `3f677ff761fcf692edb0243a5d812930844d879a`
+- Common ancestor: `afbe264b44b63155a864dbe0171040c66e442963`
+- Goal: port applicable security and correctness hardening without replacing Threshold's Paillier/NTilde remediation or weakening `ModProof`/`FactorProof`.
+
+## Compatibility Notice
+
+This is a protocol/wire compatibility break for proof transcripts. Proofs whose Fiat-Shamir challenges now use tagged hashing or session context will not verify across mixed old/new versions, even where the Go API remains source-compatible through variadic arguments. Operators should roll this out as a coordinated protocol upgrade rather than mixing parties from before and after this PR in the same keygen, signing, or resharing ceremony.
+
+## Ported Or Manually Adapted
+
+- `3d95e54` / PR `#252`, ECDSA protocol security updates: manually adapted tagged challenges, MtA/range-proof validation, session-context plumbing, and proof boundary checks while preserving Threshold's existing Paillier/NTilde proof model.
+- `1a14f3a` / PR `#256`, ECDSA proof session byte: manually adapted proof-session APIs for DLN, Schnorr, MtA, Paillier mod proof, and factor proof. Public callers remain source-compatible through variadic session parameters, but generated proof transcripts are not wire-compatible with old versions.
+- `ff989bf` / PR `#257`, tagged hash encoding: ported length-delimited tagged hashing as `common.SHA512_256i_TAGGED`.
+- `f3aad28` / PR `#276`, nil `String()` panic: ported `BaseParty.String()` nil-round guard.
+- `409542e` / PR `#282`, round update correctness: ported the `round.ok` accumulation fix for all non-terminal ECDSA/EdDSA keygen, signing, and resharing rounds, plus the resharing party-0 broadcast nil guard.
+- `9acd90b`, `2f294cf`, `6b92e7d`, `c0de534` / PR `#284`, leading-zero message signing: manually adapted for ECDSA and EdDSA with variadic `fullBytesLen` parameters that are now required at runtime and bounded to the curve order byte length. EdDSA now also hashes the full-length message bytes in round 3.
+- `843de68` / PR `#291`, VSS threshold-size validation: ported `len(vs) == threshold+1` verification and added test coverage.
+- `5d01446` / PR `#289`, range-proof update: ported MtA range-proof GCD, interval, lower-bound, non-one, and tagged challenge checks.
+- `4878da5` / PR `#324`, VSS reconstruction fix: ported `threshold+1` reconstruction requirement and updated ECDSA/EdDSA keygen fixture tests.
+- `b59ed36`, session context for DLN and MtA proofs: manually adapted with optional session contexts and focused replay/session-mismatch tests.
+- `fc38979`, GG20 SSID uniqueness: ported `tss.Parameters.SessionNonce` / `SetSessionNonce`, added `SetSessionNonceBytes` (requires session IDs of at least 16 bytes), and wired ECDSA/EdDSA keygen/signing plus ECDSA resharing SSID derivation. Keygen, ECDSA resharing, and signing now require a positive `SetSessionNonce` and fail closed if it is not set — the previous zero fallback (keygen/resharing) and SHA512_256(messageBytes) fallback (signing) caused two ceremonies with otherwise-identical inputs to derive the same SSID, breaking the session-binding property the proofs rely on.
+- `685c2af`, canonical EC coordinates: ported rejection of EC coordinates outside `[0, P)`.
+- `5d0d0f3`, EdDSA nil-pointer fix: ported by checking `NewECPoint` errors before `EightInvEight()`.
+- Post-review cleanup: party-specific proof contexts now append fixed-width uint64 party indexes so party 0 does not collapse to the bare SSID. The earlier signing default that derived an SSID nonce from full message bytes has been removed in favour of the fail-closed requirement above; the helpers that produced it (`messageBytes`, `messageSessionNonce`) are gone with it.
+
+## Already Covered Or Superseded
+
+- `c84c096` / PR `#323`, modproof checker: Threshold's Paillier proof implementation already had the key Jacobi/non-prime validation from its GHSA-h24c-6p6p-m3vx remediation. Threshold's stronger `ModProof` coverage for both Paillier `N` and `NTilde` was kept.
+- `e0e4299`, EdDSA keygen error aggregation: current Threshold code already had the corrected behavior.
+- `0629cff`, `773b6af`, `b7b73a0`, `27922e0`, `4c83ace`, `c8136c9`, `8a87b22`, `f67a429`, `002397d`, `28d0622`, `bddf60d`: style, comment, logging, gofmt, minor optimization, or merge-only changes with no security behavior to port.
+
+## Skipped
+
+- `faf1884`, `c23246e`: module path bumps to `/v2` and `/v3`; skipped to preserve Threshold compatibility.
+- `fbb0ef7`: changes `SignatureData` channels to pointers; skipped as public API churn not required for the hardening.
+- `b8d526d`, `8abf1d5`, `6c233c6`, `87f7e12`: dependency and random-source API churn; skipped except where existing Threshold APIs already supported the needed behavior.
+- `7113b68`, `d0325a1`, `dca2ac4`: repository metadata, CI, or security-report housekeeping.
+- `3709c25`, `7a10240`, `0735081`, `3f677ff` / PR `#328`: broad optional constant-time framework. Not ported in this pass because it adds a new dependency, broad Paillier/MtA rewrites, and is default-disabled upstream. Treat as a separate follow-up security project with benchmarking and side-channel review.
+- Merge-only commits `b15a0cf`, `c76a1a5`, `b79b349`, `d6e2aa9`, `ba5b734`: no direct changes to port beyond their constituent commits above.
+
+## Semantic Differences From BNB
+
+- Threshold's Paillier/NTilde `ModProof` and `FactorProof` remediation was retained. No BNB no-proof escape hatches were introduced.
+- Session parameters were added as variadic arguments to preserve existing public call sites. This is API source-compatible for callers, but not wire-compatible for proof transcripts; callers must set `Parameters.SessionNonce()` before starting keygen, signing, or ECDSA resharing.
+- ECDSA/EdDSA signing constructors still accept `fullBytesLen` as a variadic argument for source compatibility, but exactly one positive value is required at runtime so all signers agree on message byte width before the protocol starts.
+- Keygen, signing, and ECDSA resharing SSIDs use `Parameters.SessionNonce()`. Callers must provide a unique agreed nonce, for example via `SetSessionNonceBytes`, for every ceremony.
+- ECDSA resharing now broadcasts the locally-derived SSID in `DGRound1Message` so the new committee can reject old-committee broadcasts whose SSID differs from the local protocol context.
+- `common.RejectionSample` keeps BNB's function name for porting clarity, but this implementation is modular reduction rather than a looping rejection sampler.
+- Constant-time operations are not included and remain a residual follow-up.
+
+## Tests
+
+- `go test ./crypto/... ./ecdsa/keygen ./ecdsa/signing ./eddsa/signing` passed.
+- `go test ./eddsa/keygen ./eddsa/resharing` passed after updating EdDSA VSS threshold tests and resharing nil guard.
+- `go test ./ecdsa/resharing` passed after the analogous resharing guard.
+- `go test ./common ./crypto/paillier ./crypto/mta ./ecdsa/keygen ./ecdsa/resharing ./ecdsa/signing ./eddsa/keygen ./eddsa/signing ./eddsa/resharing` passed after review fixes.
+- `go test ./...` passed.
+- `go vet ./...` passed.
+
+Added or updated focused tests cover:
+
+- DLN, Schnorr, Paillier mod proof, factor proof, and MtA range proof session mismatch/replay failures.
+- Non-invertible malformed factor-proof bases returning errors instead of panicking.
+- MtA range-proof malformed ciphertext and proof-value boundary failures.
+- ProofBobWC malformed lower-bound, zero-value, and curve-mismatch failures.
+- ProofBob and ProofBobWC session mismatch/replay failures.
+- VSS `threshold+1` verification/reconstruction behavior.
+- Non-canonical EC coordinate rejection.
+- ECDSA and EdDSA leading-zero message signing.
+
+## Residual Risks
+
+- Applications must call `SetSessionNonce` or `SetSessionNonceBytes` before keygen, signing, and ECDSA resharing; those protocols now fail closed without it.
+- The optional constant-time upstream work is not integrated.
+- EdDSA resharing has no SSID-bound proof transcript in this port.

README.md

@@ -89,7 +89,8 @@ Use the `signing.LocalParty` for signing and provide it with a `message` to sign
 Please note that `t+1` signers are required to sign a message and for optimal usage no more than this should be involved. Each signer should have the same view of who the `t+1` signers are.
 
 ```go
-party := signing.NewLocalParty(message, params, ourKeyData, outCh, endCh)
+fullBytesLen := (params.EC().Params().N.BitLen() + 7) / 8
+party := signing.NewLocalParty(message, params, ourKeyData, outCh, endCh, fullBytesLen)
 go func() {
     err := party.Start()
     // handle err ...
@@ -146,6 +147,15 @@ When you build a transport, it should offer a broadcast channel as well as point
 
 Within your transport, each message should be wrapped with a **session ID** that is unique to a single run of the keygen, signing or re-sharing rounds. This session ID should be agreed upon out-of-band and known only by the participating parties before the rounds begin. Upon receiving any message, your program should make sure that the received session ID matches the one that was agreed upon at the start.
 
+The same session ID should be bound into the protocol parameters before constructing local parties:
+
+```go
+params := tss.NewParameters(curve, ctx, thisParty, len(parties), threshold)
+params.SetSessionNonceBytes([]byte(sessionID))
+```
+
+All parties in the run must use the same high-entropy session ID of at least 16 bytes, and it must be unique to the ceremony. Keygen, signing, and ECDSA re-sharing fail closed if no session nonce is set; reusing a session ID across otherwise identical ceremonies reintroduces transcript-splicing risk.
+
 Additionally, there should be a mechanism in your transport to allow for "reliable broadcasts", meaning parties can broadcast a message to other parties such that it's guaranteed that each one receives the same message. There are several examples of algorithms online that do this by sharing and comparing hashes of received messages.
 
 Timeouts and errors should be handled by your application. The method `WaitingFor` may be called on a `Party` to get the set of other parties that it is still waiting for messages from. You may also get the set of culprit parties that caused an error from a `*tss.Error`.
@@ -155,4 +165,3 @@ A full review of this library was carried out by Kudelski Security and their fin
 
 ## References
 \[1\] https://eprint.iacr.org/2019/114.pdf
-

common/hash.go

@@ -93,6 +93,48 @@ func SHA512_256i(in ...*big.Int) *big.Int {
 	return new(big.Int).SetBytes(state.Sum(nil))
 }
 
+// SHA512_256i_TAGGED is a domain-separated variant of SHA512_256i. The tag is
+// hashed and prepended twice.
+func SHA512_256i_TAGGED(tag []byte, in ...*big.Int) *big.Int {
+	tagBz := SHA512_256(tag)
+	state := crypto.SHA512_256.New()
+	if _, err := state.Write(tagBz); err != nil {
+		panic("SHA512_256i_TAGGED Write(tag) failed: " + err.Error())
+	}
+	if _, err := state.Write(tagBz); err != nil {
+		panic("SHA512_256i_TAGGED Write(tag) failed: " + err.Error())
+	}
+
+	inLen := len(in)
+	bzSize := 0
+	inLenBz := make([]byte, 64/8)
+	binary.LittleEndian.PutUint64(inLenBz, uint64(inLen))
+	ptrs := make([][]byte, inLen)
+	for i, n := range in {
+		if n == nil {
+			ptrs[i] = zero.Bytes()
+		} else {
+			ptrs[i] = n.Bytes()
+		}
+		bzSize += len(ptrs[i])
+	}
+
+	dataCap := len(inLenBz) + bzSize + inLen + (inLen * 8)
+	data := make([]byte, 0, dataCap)
+	data = append(data, inLenBz...)
+	for i := range in {
+		data = append(data, ptrs[i]...)
+		data = append(data, hashInputDelimiter)
+		dataLen := make([]byte, 8)
+		binary.LittleEndian.PutUint64(dataLen, uint64(len(ptrs[i])))
+		data = append(data, dataLen...)
+	}
+	if _, err := state.Write(data); err != nil {
+		panic("SHA512_256i_TAGGED Write(data) failed: " + err.Error())
+	}
+	return new(big.Int).SetBytes(state.Sum(nil))
+}
+
 func SHA512_256iOne(in *big.Int) *big.Int {
 	var data []byte
 	state := crypto.SHA512_256.New()

common/hash_test.go

@@ -0,0 +1,60 @@
+// 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_test
+
+import (
+	"math/big"
+	"testing"
+
+	"github.com/bnb-chain/tss-lib/common"
+)
+
+func TestSHA512_256iTaggedDomainSeparation(t *testing.T) {
+	in := []*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(3)}
+
+	tagA := common.SHA512_256i_TAGGED([]byte("tag-a"), in...)
+	tagAAgain := common.SHA512_256i_TAGGED([]byte("tag-a"), in...)
+	tagB := common.SHA512_256i_TAGGED([]byte("tag-b"), in...)
+
+	if tagA.Cmp(tagAAgain) != 0 {
+		t.Fatal("same tag and inputs must hash deterministically")
+	}
+	if tagA.Cmp(tagB) == 0 {
+		t.Fatal("different tags must produce different hashes")
+	}
+}
+
+func TestSHA512_256iTaggedLengthDelimitsInputs(t *testing.T) {
+	left := common.SHA512_256i_TAGGED([]byte("tag"), big.NewInt(1), big.NewInt(0x0203))
+	right := common.SHA512_256i_TAGGED([]byte("tag"), big.NewInt(0x0102), big.NewInt(3))
+
+	if left.Cmp(right) == 0 {
+		t.Fatal("tagged hash must length-delimit adjacent inputs")
+	}
+}
+
+func TestSHA512_256iTaggedNilAndEmptyTagMatch(t *testing.T) {
+	nilTag := common.SHA512_256i_TAGGED(nil, big.NewInt(1))
+	emptyTag := common.SHA512_256i_TAGGED([]byte{}, big.NewInt(1))
+
+	if nilTag.Cmp(emptyTag) != 0 {
+		t.Fatal("nil and empty tags should preserve the legacy untagged domain")
+	}
+}
+
+func TestHashToNTaggedUsesFullModulusWidth(t *testing.T) {
+	N := new(big.Int).Lsh(big.NewInt(1), 2048)
+	N.Sub(N, big.NewInt(159))
+
+	got := common.HashToNTagged([]byte("large-modulus-tag"), N, big.NewInt(1), big.NewInt(2))
+	if got.Sign() < 0 || got.Cmp(N) >= 0 {
+		t.Fatal("HashToNTagged must return a value in [0, N)")
+	}
+	if got.BitLen() <= 256 {
+		t.Fatalf("HashToNTagged appears truncated to one hash block: bitlen=%d", got.BitLen())
+	}
+}

common/hash_utils.go

@@ -21,6 +21,14 @@ 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 {
+	return LiterallyJustMod(q, eHash)
+}
+
 // Return a big.Int between 0 and N
 func HashToN(N *big.Int, in ...*big.Int) *big.Int {
 	bitCnt := N.BitLen()
@@ -43,3 +51,30 @@ func HashToN(N *big.Int, in ...*big.Int) *big.Int {
 	// thus it is safe to use Mod
 	return LiterallyJustMod(N, dest)
 }
+
+// HashToNTagged is the tagged-hash analogue of HashToN. It produces a value in
+// [0, N) by concatenating ((N.BitLen()/256) + 2) blocks of SHA512_256i_TAGGED
+// — one per block-index counter — and reducing modulo N. The total entropy
+// before reduction is at least N.BitLen() + 256 bits, so the modular reduction
+// has the same bias budget as HashToN (≤ 2^-256).
+//
+// Use this for Fiat-Shamir challenges over large moduli (e.g. Paillier N ≈ 2^2048)
+// when the derivation must be domain-separated by a session tag. Reducing a
+// single 256-bit SHA512_256i_TAGGED output modulo N would emit challenges in
+// [0, 2^256) instead of [0, N).
+func HashToNTagged(tag []byte, N *big.Int, in ...*big.Int) *big.Int {
+	bitCnt := N.BitLen()
+	blockCnt := (bitCnt / 256) + 2
+
+	dest := big.NewInt(0)
+	tmp := make([]*big.Int, 1, 1+len(in))
+	tmp = append(tmp, in...)
+
+	for i := 0; i < blockCnt; i++ {
+		tmp[0] = big.NewInt(int64(i))
+		dest.Lsh(dest, 256)
+		dest.Or(dest, SHA512_256i_TAGGED(tag, tmp...))
+	}
+
+	return LiterallyJustMod(N, dest)
+}

common/hash_utils_test.go

@@ -60,3 +60,18 @@ func TestLiterallyJustMod(t *testing.T) {
 		})
 	}
 }
+
+func TestRejectionSampleReducesModuloQ(t *testing.T) {
+	q := big.NewInt(101)
+	eHash := big.NewInt(12345)
+
+	got := common.RejectionSample(q, new(big.Int).Set(eHash))
+	want := new(big.Int).Mod(eHash, q)
+
+	if got.Cmp(want) != 0 {
+		t.Fatalf("RejectionSample() = %v, want %v", got, want)
+	}
+	if got.Sign() < 0 || got.Cmp(q) >= 0 {
+		t.Fatal("RejectionSample must return a value in [0, q)")
+	}
+}

common/int.go

@@ -7,6 +7,7 @@
 package common
 
 import (
+	"encoding/binary"
 	"math/big"
 )
 
@@ -100,6 +101,24 @@ func (mi *modInt) i() *big.Int {
 	return (*big.Int)(mi)
 }
 
+func IsInInterval(b *big.Int, bound *big.Int) bool {
+	return b != nil && bound != nil && b.Cmp(bound) < 0 && b.Cmp(zero) >= 0
+}
+
+func AppendBigIntToBytesSlice(commonBytes []byte, appended *big.Int) []byte {
+	resultBytes := make([]byte, len(commonBytes), len(commonBytes)+len(appended.Bytes()))
+	copy(resultBytes, commonBytes)
+	return append(resultBytes, appended.Bytes()...)
+}
+
+func AppendUint64ToBytesSlice(commonBytes []byte, appended uint64) []byte {
+	resultBytes := make([]byte, len(commonBytes), len(commonBytes)+8)
+	copy(resultBytes, commonBytes)
+	idxBytes := make([]byte, 8)
+	binary.BigEndian.PutUint64(idxBytes, appended)
+	return append(resultBytes, idxBytes...)
+}
+
 // Marshal the given bigint into bytes.
 // with the sign stored in the first byte and the absolute value in the rest.
 // `nil` or 0 is stored as the byte 0x00.

common/int_test.go

@@ -144,3 +144,37 @@ func TestAnyIsNil(t *testing.T) {
 	assert.True(common.AnyIsNil(nil, big.NewInt(2)))
 	assert.False(common.AnyIsNil(big.NewInt(1), big.NewInt(2)))
 }
+
+// TestAppendUint64ToBytesSlice_PartyContextSeparation pins the invariant that
+// per-party Fiat-Shamir context derivation depends on: appending a party index
+// (including 0) must always produce a value distinct from the bare SSID, and
+// distinct party indices must produce distinct contexts. If this invariant
+// regresses (e.g. via a future "skip leading zeros" optimization), party 0's
+// proof transcripts would collapse back to the untagged SSID.
+func TestAppendUint64ToBytesSlice_PartyContextSeparation(t *testing.T) {
+	assert := assert.New(t)
+
+	ssid := []byte{0xDE, 0xAD, 0xBE, 0xEF}
+
+	ctx0 := common.AppendUint64ToBytesSlice(ssid, 0)
+	ctx1 := common.AppendUint64ToBytesSlice(ssid, 1)
+	ctx256 := common.AppendUint64ToBytesSlice(ssid, 256)
+
+	assert.NotEqual(ssid, ctx0, "party-0 context must not collapse to bare SSID")
+	assert.NotEqual(ctx0, ctx1, "party-0 and party-1 contexts must differ")
+	assert.NotEqual(ctx0, ctx256, "party-0 and party-256 contexts must differ")
+	assert.NotEqual(ctx1, ctx256, "party-1 and party-256 contexts must differ")
+
+	assert.Equal(len(ssid)+8, len(ctx0), "appended index must be a fixed 8 bytes")
+	assert.Equal(len(ssid)+8, len(ctx1), "appended index must be a fixed 8 bytes")
+	assert.Equal(ssid, ctx0[:len(ssid)], "SSID prefix must be preserved")
+
+	expectedCtx0 := append(append([]byte{}, ssid...), 0, 0, 0, 0, 0, 0, 0, 0)
+	assert.Equal(expectedCtx0, ctx0, "party-0 must append 8 zero bytes (big-endian uint64)")
+
+	// nil and empty SSID must still yield distinct, non-collapsing contexts.
+	emptyCtx0 := common.AppendUint64ToBytesSlice(nil, 0)
+	emptyCtx1 := common.AppendUint64ToBytesSlice(nil, 1)
+	assert.Equal(8, len(emptyCtx0), "empty SSID + index 0 must still produce 8 bytes")
+	assert.NotEqual(emptyCtx0, emptyCtx1, "indices must differ even with empty SSID")
+}