tss-lib: v3 channel-free round functions, EdDSA, security fixes#9
tss-lib: v3 channel-free round functions, EdDSA, security fixes#9marcopeereboom wants to merge 45 commits intomainfrom
Conversation
| @@ -0,0 +1,48 @@ | |||
| # Copyright (c) 2024 Hemi Labs, Inc. | |||
… audit fixes 112 fixes across 20 audit waves (63 Opus 4.6 agents) covering: - Domain-separated SSID with ceremony context (ceremonyType, attemptNumber, CeremonyID) - ValidateBasic() on all incoming wire messages (nil/empty/range checks) - Xi share zeroing after keygen/reshare round completion - ProofCallback support for ZK witness extraction (keygen, reshare, signing) - Canonical protobuf re-encoding for deterministic message hashing - Low-S normalization on all ECDSA signatures - PartyID.Key set to compressed ECDSA pubkey (33 bytes) - Defense-in-depth range checks on crypto primitives (Paillier, VSS, MtA, Schnorr, DLN, FacProof, ModProof) - ReceiverID field in P2P message routing - Comprehensive fork test suite (~40 new test files) All 17 Go test packages pass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add ceremonyID []byte to Parameters with getter/setter. Include it in all 6 getSSID() implementations (ECDSA + EdDSA: keygen, signing, resharing). The nonce field is the attempt counter for retries of the same logical ceremony. CeremonyID distinguishes concurrent ceremonies that share the same parties, threshold, and curve. Both fields are now in the SSID hash, covering both dimensions of uniqueness. The field is optional: callers that do not set CeremonyID get the same SSID as before (backward compatible).
Pure function API for ECDSA keygen: Round1, Round2, Round3, Round4. Each round takes explicit state + inbound messages and returns outbound messages. No channels, no goroutines in the caller, no recursive BaseUpdate. KeygenState is opaque — passed between rounds without caller modification. RoundOutput carries outbound messages and, on Round1, the VSS polynomial for SNARK witness extraction. On Round4, the final LocalPartySaveData. All crypto is identical to the channel-based Start() methods. The existing NewLocalParty channel API is untouched. Test: 3-party keygen via round functions. Verifies ECDSAPub agreement, Feldman invariant (Xi*G == BigXj[i]), and Lagrange interpolation recovery of the private key.
Pure function API for ECDSA signing: SignRound1 through SignRound9 plus SignFinalize. Same pattern as keygen: each round takes explicit state + inbound messages, returns outbound messages. SigningState is opaque. SignRoundOutput carries messages and, after Finalize, the verified SignatureData. All MtA exchanges (BobMid, BobMidWC, AliceEnd, AliceEndWC), ReceiverID checks, Schnorr proofs, commitment verification, and low-S normalization match the channel-based implementation. Also adds ExportR2P2PSelf/ExportR2BcastSelf to KeygenState so the signing test can build the keygen message matrix. Test: 3-party keygen + sign via round functions. All parties produce identical (r,s) verified by ecdsa.Verify.
Pure function API for ECDSA resharing: ReshareRound1 through ReshareRound5. Handles dual-committee (old/new) roles: old committee produces VSS shares and commitments, new committee verifies and saves new key shares. ReshareState is opaque. ReshareRoundOutput carries messages and, after Round5, the new LocalPartySaveData (new committee only). Old committee's Xi is zeroed in Round5. All parameter validation (Paillier, NTilde, Pedersen, DLN/Mod/ FacProof), ReceiverID checks, VSS verification, and SNARK witness extraction match the channel-based implementation. Test: 3-party keygen → reshare(3→3 disjoint) → sign with new keys. ECDSAPub preserved across reshare, signature verified.
SignRound1 now calls BuildLocalSaveDataSubset automatically when len(key.Ks) > PartyCount(). This handles the common case where keygen produces shares for N parties but signing uses a threshold+1 subset. Without this, callers had to manually subset the key data before calling SignRound1, matching the channel-based API's requirement that was easy to miss. Test: keygen(5) → sign with 2-of-5 subset → ecdsa.Verify passes.
Bump module path from tss-lib/v2 to tss-lib/v3. Delete all
channel-based ceremony code:
- ecdsa/{keygen,signing,resharing}: local_party.go, rounds.go,
round_*.go, finalize.go and their tests
- tss/party.go (Party/BaseParty/BaseUpdate/BaseStart interfaces)
- tss/round.go (Round interface)
- eddsa/ directory (future SOW, channel-based)
- test/ directory (old fixture infrastructure)
- Old test files referencing deleted fixtures (test_utils.go,
save_data_test.go, dln_verifier_test.go, keygen_benchmark_test.go,
xi_zeroing_test.go, share_protocol_test.go, mta_fork_test.go,
range_proof_test.go)
Shared types extracted into types.go per package:
- localTempData, localMessageStore (from deleted local_party.go)
- TaskName constant (from deleted rounds.go)
- paillierBitsLen constant (from deleted rounds.go)
- padToLengthBytesInPlace (from deleted finalize.go)
Production code (round_fn.go, round_state.go, round_fn_test.go),
messages.go, save_data.go, prepare.go, dln_verifier.go, and all
crypto/ packages are unchanged except v2→v3 import paths.
154 files changed, 330 insertions(+), 15909 deletions(-)
EdDSA (Ed25519) threshold keygen (3 rounds), signing (3 rounds + finalize), and resharing (5 rounds) using the v3 channel-free round function API. Resharing supports overlapping committees. Simpler than ECDSA: no Paillier pre-parameters, no MtA protocol. Uses Feldman VSS + Schnorr proofs for keygen, additive nonce aggregation for signing. Fix: cached Edwards() singleton in tss/curve.go — edwards.Edwards() allocates a new instance per call, but ECPoint.Add() compares curve pointers for identity. All Edwards points must share one instance. Fix: added oldIndex()/newIndex() helpers to both ECDSA and EdDSA resharing round_fn.go — Pi.Index refers to one committee sort position but gets used to index into the other committee message arrays. Overlapping committees require lookup by KeyInt. Renamed FORK_CHANGES.md to SECURITY_FIXES.md with complete rewrite for clarity. Updated README.md with keygen/sign/reshare docs for both ECDSA and EdDSA including overlapping committee usage. Updated ECDSA example_test.go with overlapping resharing.
Error-path and coverage tests for every crypto primitive and all three EdDSA/ECDSA ceremony round functions. Negative tests exercise nil arguments, out-of-range values, wrong keys, bad commitments, corrupted shares, truncated serialization, wrong session IDs, and malformed proofs. Coverage tests hit ValidateBasic for all 31 message types, MtA full protocol (with and without witness check), ProofBob/ProofBobWC bytes round-trips, DLN proof unmarshal errors, modproof ValidateBasic nil-field paths, facproof/schnorr wrong-input rejection, VSS create/verify/reconstruct error paths, ECPoint Gob/JSON encode/decode error paths, Paillier HomoMult/HomoAdd/ Decrypt range checks, and CKD DeriveChildKeyFromHierarchy. Coverage: 66.1% -> 76.2% total. Every package now above 74%. Per-package improvements: crypto/mta 0%->76.8%, crypto/commitments 83.9%->96.8%, crypto/vss 91.5%->95.7%, crypto/ecpoint 83.0%->91.8%, crypto/paillier 83.6%->91.0%, eddsa/* 85-89%->90-91%, ecdsa/* 68-78%->74-82%. 19 new test files, 2058 lines.
Sync with heminetwork to avoid overwriting the binary when running make deps in both repos.
ValidateSaveData was at 5.9% — every branch now exercised: nil Xi/ShareID/ECDSAPub, n < 2, array mismatch, nil per-party fields, off-curve BigXj, ShareID not in Ks, zero Xi, Feldman VSS failure. Also covers ValidateWithProof nil Alpha, P==Q, bad NTilde, and BuildLocalSaveDataSubset success path. CKD: add NewExtendedKeyFromString bad length, bad checksum, and DeriveChildKey to exercise paddedBytes short-src path.
golangci-lint v2.11.3 flags elliptic.Curve.Add/ScalarMult/etc as SA1019 deprecated. tss-lib requires raw curve arithmetic that crypto/ecdh does not expose — suppress in .golangci.yaml. go vet requires both //go:build and // +build when go.mod declares go 1.16. Add missing // +build tssexamples to ecdsa/example_test.go and eddsa/example_test.go.
db049c5 to
03972e0
Compare
joshuasing
left a comment
There was a problem hiding this comment.
Blocking until reviewed
ClaytonNorthey92
left a comment
There was a problem hiding this comment.
hey @marcopeereboom , at the time of writing this, I made this change f2c4733, and all of the tests still pass. I think this requires a larger discussion better done via slack or in a meeting, as I would expect something to break by changing that function. and it may be indicative of larger issues. I'll reach out out-of-band 👍
actually @marcopeereboom I am realizing that my change is in the |
tss/common/hash.go
Outdated
| @@ -20,6 +20,7 @@ const ( | |||
| // SHA-512/256 is protected against length extension attacks and is more performant than SHA-256 on 64-bit architectures. | |||
| // https://en.wikipedia.org/wiki/Template:Comparison_of_SHA_functions | |||
| func SHA512_256(in ...[]byte) []byte { | |||
| return []byte("lolz no.") | |||
There was a problem hiding this comment.
can we just delete tss altogether? it doesn't seem to be tested well, and tss-lib is a replacement?
| generatorCtx, cancelGeneratorCtx := context.WithCancel(ctx) | ||
| defer cancelGeneratorCtx() | ||
|
|
||
| for i := 0; i < concurrency; i++ { |
There was a problem hiding this comment.
this is still using goroutines and channels, did we want to remove all of this?
| gopkg.in/yaml.v3 v3.0.1 // indirect | ||
| ) | ||
|
|
||
| replace github.com/agl/ed25519 => github.com/binance-chain/edwards25519 v0.0.0-20200305024217-f36fc4b53d43 |
There was a problem hiding this comment.
I think we need to look at replacing github.com/agl/ed25519 (and Binance's version) with https://github.com/FiloSottile/edwards25519
The v2→v3 module rename (ca2f494) deleted mta_fork_test.go, leaving all 14 [FORK] security checks in the MtA proof verifiers with zero negative test coverage. Restore the 10 deleted tests (ported to v3/external package) and add 10 new tests covering checks that were never tested: zero S/V, non- coprime S/V, small Paillier N, non-coprime c2, oversized S2/T2, and ProofBobWC-specific guards (s1ModQ=0, e=0). 14 of 14 [FORK] MtA checks now have negative tests. Also add -tags tssexamples to CI so the ECDSA/EdDSA lifecycle tests (keygen→sign→reshare→sign) run automatically. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add 7 new test files and expand 2 existing files (33 → 123 tests) covering parameter validation, proof verification, and error paths that previously had no dedicated negative test coverage. - DlnProofVerifier: 13 unit tests (valid/invalid proofs, nil proof, session binding, semaphore bounds) - Round2: 21 negative tests for all parameter validation checks (bit length, even, prime, perfect-square, equality, coprimality, duplicates, cross-collision H1/H2, oversized N/NTilde) - Round3: 8 negative tests (decommitment, ModProof, FacProof, ReceiverID, VSS share, context cancellation) - Round4: 4 tests (corrupted Paillier proof with culprit check, nil proof, context cancellation, honest sanity) - Round1: 6 tests (stale pre-params, context cancellation, SSID nonce and CeremonyID binding) - SSID: 7 unit tests for getSSID domain separation - PreParams: 10 edge-case tests for GeneratePreParams - ValidateWithProof/Validate: 16 new tests covering all branches - ValidateBasic: expanded to cover every return-false path - Constructor field tests for all 4 message types Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nalize Add 7 new test files (1,976 lines) covering error paths and ValidateBasic boundaries that previously had no dedicated negative test coverage (8 → 41 test functions). - Round1: 3 tests (negative/oversized message hash, key count mismatch) - Round2: 3 tests (ReceiverID mismatch, nil RangeProofAlice, context cancellation) - Round3: 4 tests (ReceiverID mismatch, nil ProofBob/ProofBobWC, context cancellation) - Round4: 1 test (zero theta via crafted sum) - Round5: 4 tests (bad commitment, nil/bad ZKProof, off-curve decommitment point) - Round7: 3 tests (bad decommitment, bad Schnorr proof, bad VVerify proof) - Round9: 1 test (bad decommitment) - SignFinalize: 4 tests (negative si, oversized si, zero sum S, corrupted si causing verification failure) - ValidateBasic: 10 table-driven tests covering all 53 return-false paths across all 9 message types - Shared test infrastructure with deep-clone helpers for all message types and incremental fixture builders for each round Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SHA512_256 provides security (binding, soundness) not correctness. A stub returning a constant still produces valid keys and signatures because both prover and verifier compute the same broken Fiat-Shamir challenge — the protocol is internally consistent even with garbage hashes. The existing tests verified correctness (ecdsa.Verify), not security properties. Pin SHA512_256, SHA512_256i, SHA512_256iOne to exact digests so any replacement is caught immediately. Add collision and domain-separation checks. Add commitment binding/hiding tests that break when the hash is replaced with a constant.
Remove unnecessary HashCommitment type conversion in messages_test.go — HashCommitment is *big.Int. Add missing "errors" import in round4_negative_test.go — isError() uses errors.As.
…d1–5 Add 8 new test files (3,631 lines) covering error paths, ValidateBasic boundaries, SSID domain separation, and message constructors that previously had no dedicated test coverage (3 → 63 test functions). Also merge paillierNMap/nTildeMap into a unified modulusMap in both keygen and resharing to catch cross-party N/NTilde collisions, and fix a comment on DGRound4Message1 (new→new, not new→old). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The keygen/signing/resharing negative tests each generate fresh Paillier pre-params (safe prime generation), which takes 100-500s per test on CI's 2-vCPU runners. The cumulative runtime of 43+ tests now exceeds 60 minutes, causing a panic timeout. Also add a 150-minute job-level timeout as a safety net. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Every negative test that receives a *tss.Error now verifies the correct culprit party is identified, not just the error message string. This catches potential off-by-one bugs in the concurrent verification goroutines (keygen R2, signing R2/R3, resharing R4). Changes: - keygen: update expectRound2Error + requireRound3Error helpers (29 tests) - signing: add requireCulprit helper (13 tests) - resharing: add requireCulprit helper, update expectRound4ParamError (31 tests) Tests producing raw errors (context.Canceled, errors.New) are correctly excluded — only *tss.Error paths are checked. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix misleading comment on isError helper (said "errors.As" but uses direct type assertion). Change TestRound4RejectsAllNilPaillierProof to use the tolerant "culprit is among" pattern instead of strict len==1, avoiding a theoretical race where a second goroutine could also report a culprit before gcancel() propagates. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add 19 new SSID tests covering previously untested inputs: - signing: new ssid_test.go (12 tests) — determinism, CeremonyID, round numbers, nonce, message hash sensitivity (critical for cross-message replay prevention), BigXj/NTildej/H1j/H2j sensitivity, nil-vs-non-nil message, output length - keygen: threshold and party count sensitivity (2 tests) - resharing: BigXj, NTildej, H1j, H2j individual mutation (4 tests) plus cloneResharingSaveData helper Closes the signing SSID gap (zero tests → 12) and brings keygen variable-input coverage to 100%. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add 7 tests for previously untested SSID inputs: - keygen: curve parameter sensitivity (S256 vs Edwards25519) - signing: party keys, threshold, party count sensitivity - resharing: old party keys, new party keys, party count sensitivity All variable inputs to all three SSID functions now have dedicated sensitivity tests. Coverage: keygen 100%, signing 100%, resharing 100%. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rewrites the tss-lib API from channel-based state machines to pure round functions, adds EdDSA support, and hardens the cryptographic primitives.
What changed
Architecture (v3 API). Every ceremony (keygen, signing, resharing) is now a sequence of pure functions: each round takes explicit state + inbound messages and returns outbound messages. No channels, no goroutines, no Party/Round/BaseUpdate interfaces. The caller owns the event loop and transport. Protobuf is removed — all message types are plain Go structs; serialization is the caller's responsibility.
EdDSA (Ed25519). Threshold keygen (3 rounds), signing (3 rounds + finalize), and resharing (5 rounds). Simpler than ECDSA: no Paillier pre-parameters, no MtA. Uses Feldman VSS + Schnorr proofs for keygen, additive nonce aggregation for signing.
Overlapping committee resharing. Both ECDSA and EdDSA resharing support parties that serve on both old and new committees simultaneously. oldIndex()/newIndex() helpers resolve the dual-committee index ambiguity that caused OOB panics with the naive Pi.Index approach. Callers must use separate *PartyID copies per committee since SortPartyIDs mutates .Index in place.
Security fixes. 76 audit fixes in production code across 24 files (from 260 annotations in the v2 base across 88 files — the difference is eliminated attack surface from deleted channel code). All annotated with [FORK] comments. Organized by attack class in SECURITY_FIXES.md: cross-ceremony replay, message redirection, message bounds, identity point/zero share, secret zeroing, modulus validation, ZK proof hardening, VSS hardening, signing protocol, EdDSA-specific.
Edwards singleton. edwards.Edwards() allocates a new curve instance per call, but ECPoint.Add() compares curve pointers for identity. Cached as a package-level singleton in tss/curve.go.
CLI demos
What was deleted
All channel-based ceremony code (local_party.go, rounds.go, round_*.go, finalize.go), Party/Round interfaces, protobuf definitions and generated code, old test fixtures and fixture infrastructure. 190 files removed in the v3 module commit.