fix(crypto): extend constant-time coverage to Schnorr proofs and signing rounds 3-5

[piotr-roslaniec]

Context

This is an additive finding for the existing CT-hardening stack (PRs #8 / #10 / #11 / #17) — submitted for review/consideration, not asking for immediate merge. It stacks on #17 (the head branch), in the same stacked-draft style #10 and #16 already use.

Upstream bnb-chain/tss-lib commit 3709c25 (folded into the BNB #328 series) hardened the constant-time path in two more spots that this fork's stack never picked up:

• crypto/schnorr/schnorr_proof.go — NewZKProofWithSession and NewZKVProofWithSession multiply the challenge c against the secret witnesses x, s, and l to build the proof responses t and u. Without CT, those math/big multiplications leak witness bits through timing.
• ecdsa/signing/round_3.go / round_4.go / round_5.go — thelta = k·γ, sigma = k·w, thetaInverse = θ⁻¹ mod q, and si = m·k + rx·σ are all secret-key multiplications and inverses on the ECDSA signing hot path.

PR #17's own file list (the d08dc73 "extend constant-time coverage" commit) covers Paillier, DLN, MtA, factor/mod proofs, ring-Pedersen keygen, and the existing MulCT/ExpCT/ModInverseCT primitives — but does not touch either of the two files above. Same gap exists in #8, #10, and #11. This PR closes that gap using the exact same pattern already established in common/constant_time.go (common.NewCTModInt(...).MulCT / .ModInverseCT, gated on IsConstantTimeEnabled()).

Why no ExpCTWithBitLen adaptation needed (vs. PR #10)

The new code calls only .MulCT(...) and .ModInverseCT(...) — never .ExpCT(...):

• MulCT takes no exponent at all (it's a constant-time modular multiplication, not an exponentiation), so PR fix(crypto): handle CT exponent bounds, zero values, and toggle changes #10's bounded-exponent-width hardening does not apply.
• ModInverseCT's internal exponent is the fixed, modulus-derived constant mod-2 (or phiN-1 via NewCTModIntWithPhi) — also a public, fixed-width value, not a caller-supplied variable-width secret. Confirmed by grep -n '\.ExpCT(' crypto/schnorr/schnorr_proof.go ecdsa/signing/round_3.go ecdsa/signing/round_4.go ecdsa/signing/round_5.go returning zero hits on this branch.

So this addition slots in directly under the existing IsConstantTimeEnabled() toggle without re-touching the exponent-bound logic PR #10 added.

Changes

• crypto/schnorr/schnorr_proof.go — wrap the c·x and c·s / c·l multiplications in NewZKProofWithSession / NewZKVProofWithSession with the CT branch.
• ecdsa/signing/round_3.go — wrap k·γ and k·w (thelta/sigma computation) with the CT branch.
• ecdsa/signing/round_4.go — wrap θ⁻¹ mod q with ModInverseCT.
• ecdsa/signing/round_5.go — wrap m·k and rx·σ (the si addend pair) with the CT branch.
• crypto/schnorr/constant_time_equiv_test.go — new equivalence test covering both NewZKProof and NewZKVProof: builds a non-CT baseline proof and a CT proof from the same witnesses and asserts both verify.
• common/constant_time.go — COVERAGE doc comment updated to list the Schnorr proof responses and ECDSA signing rounds 3-5 among the sites now covered. Default value (constantTimeEnabled = 1, set by PR Enable bounded bigmod operations by default #17) and PR Enable bounded bigmod operations by default #17's "limited coverage" warning are preserved verbatim.
• CHANGELOG.md — added a Breaking Change feat(crypto): opt-in constant-time path for secret-exponent modexps [DO NOT MERGE] #8 entry under the existing [Unreleased] heading describing the now-on-by-default CT framework plus this PR's Schnorr/signing-rounds extension, an Added entry for the CT API symbols, and removed two now-stale deferred-CT lines (the "optional constant-time framework ... deferred to a separate follow-up" bullet and "the optional constant-time work is not integrated" line). (An earlier revision of this branch introduced a dated [1.4.0] section header and removed the "has not yet published its own tagged release" line; both were reverted in commit 17e4771 after review feedback that this fork has no tags/releases yet — this description previously went stale relative to that revert.)

Verification on this branch (pushed)

$ gofmt -l .
(no output)

$ go build ./...
(no output)

$ go vet ./...
(no output)

$ go test ./...
ok    github.com/bnb-chain/tss-lib/common              28.469s
ok    github.com/bnb-chain/tss-lib/crypto              (cached)
ok    github.com/bnb-chain/tss-lib/crypto/ckd          (cached)
ok    github.com/bnb-chain/tss-lib/crypto/commitments  (cached)
ok    github.com/bnb-chain/tss-lib/crypto/dlnproof     23.557s
ok    github.com/bnb-chain/tss-lib/crypto/mta          286.977s
ok    github.com/bnb-chain/tss-lib/crypto/paillier     51.010s
ok    github.com/bnb-chain/tss-lib/crypto/schnorr      0.052s
ok    github.com/bnb-chain/tss-lib/crypto/vss          (cached)
ok    github.com/bnb-chain/tss-lib/ecdsa/keygen        340.405s
ok    github.com/bnb-chain/tss-lib/ecdsa/signing       186.710s
?     github.com/bnb-chain/tss-lib/test                [no test files]
ok    github.com/bnb-chain/tss-lib/tss                 (cached)

The new TestSchnorrProofCTVerifies / TestSchnorrVProofCTVerifies cases pass.

Benchmark (from the local pre-push run on this stack)

Constant-time modexp measured at parity with the standard math/big path on the same hardware:

go test ./common/... -bench BenchmarkExpCT -benchtime=2s
~2.73ms per op (CT,  n≈900)   vs   ~2.83ms per op (std,  n≈800)

The CPU-cost concern that originally motivated upstream's deferral did not materialize for this primitive.

Out of scope / NOT fixed here — for maintainer awareness only

Upstream's same 3709c25 commit also hardened crypto/mta/share_protocol.go's AliceEnd / AliceEndWC Paillier-decrypt paths — but with a different mechanism entirely: a sleep-based response-time normalizer (NewTimingProtection, ~200 ms target + jitter), not the bigmod constant-time path used everywhere else in common/constant_time.go. That primitive does not exist anywhere in this fork's lineage (master, both CT branches, or PRs #12–#17), and porting it would inject a fixed ~200 ms delay into every MtA share round — a real latency cost that needs its own sign-off rather than a mechanical extension of the existing CT pattern.

That gap is documented as a known, deliberately-deferred item in this PR's COVERAGE comment update to common/constant_time.go, not implemented in code. If/when a follow-up decides to port NewTimingProtection, it should land as its own PR with its own benchmark and a separate latency sign-off; the bigmod CT path doesn't subsume it.

[mswilkison]

Two P2 findings from the review.

Validation reported by the review: build and vet passed, and the Schnorr tests compiled; test binaries were not executed.

[mswilkison]

[P2] Explicitly set and restore CT mode in both equivalence tests

CT is enabled by default, so proofOff actually exercises the CT branch when either test runs alone. The deferred disable then leaves subsequent Schnorr tests—including the existing P-256 and session-binding cases—running with CT disabled. This makes coverage depend on test order and removes default-path coverage. Save the previous mode, explicitly disable CT before constructing the baseline, and restore the saved mode during cleanup in both tests.

[mswilkison]

[P2] Keep the untagged hardening stack under Unreleased

This repository currently has no tags or published GitHub releases, including v1.4.0. Moving the hardening stack into this dated release section therefore advertises security changes as released when consumers cannot obtain that version, and both new comparison links reference a nonexistent tag. Keep these entries under [Unreleased] until an actual release contains them.

[piotr-roslaniec]

Both P2 findings addressed in 17e4771.

Test CT mode. Confirmed the finding is exactly right: this branch sets constantTimeEnabled = 1 (common/constant_time.go:54), so proofOff was exercising the CT branch and the bare defer DisableConstantTimeOps() was leaving CT off for every later test in the package. Added a withCTMode(t, bool) helper that saves the ambient mode, sets the requested one, and restores the saved mode via t.Cleanup; the baseline now explicitly disables CT and asserts IsConstantTimeEnabled() == false so it cannot silently go vacuous again. Verified each CT test passes run alone (the original order-dependence case) and that the full package passes under -count=2 with the pre-existing P-256 and session-binding tests running after them.

Changelog. Also correct, and the [1.4.0] heading plus both comparison links were introduced by this PR, not inherited. Reverted to [Unreleased]; no 1.4.0 reference remains in the file. The changelog diff against this PR's base is now purely the new content entry, with no heading or link churn.

[piotr-roslaniec]

Merged into dev. #17 (including current dev) was merged into this branch first; the only conflict was CHANGELOG. Both #19 and #17 entries were retained, and the round-5 table now correctly says rx remains secret until round 10.

Verification:

• CI Test and Go fmt green on integrated head.
• Build, vet, tidy, format, and shuffled Schnorr tests green.
• CT-on and CT-off 11-party signing ceremonies both produce valid signatures.
• Round 3/4/5 equivalence tests pass, including inverse edge cases.
• Schnorr fixed-vs-random timing ratio was 0.985x; math/big control was 0.827x. Host noise prevents a formal dudect verdict, but the primitive check is consistent with constant-time behavior.
• Prior P2 findings fixed: equivalence tests explicitly set/restore CT mode and no longer depend on test order; all hardening remains under [Unreleased].

Residual limits are recorded, not hidden: surrounding big.Int add/reduce remains variable-time; round 5's rx reduction is value-dependent; secret-scalar EC point multiplication remains non-CT; the AliceEnd sleep normalizer was intentionally not ported because Decrypt itself is now CT.

Full report: agent-docs/pr-integration/23.md (local).