Repository navigation
feat(crypto): opt-in constant-time path for secret-exponent modexps [DO NOT MERGE] - #8
piotr-roslaniec wants to merge 2 commits into
Conversation
Port the upstream constant-time framework (common/constant_time.go, built on filippo.io/bigmod) and wire it, behind a default-off runtime toggle, into the secret-exponent modular exponentiations in Paillier Decrypt/Proof and the DLN, factor, and Paillier-Blum modulus proofs. Mitigates the timing side-channel gap (CVE-2023-26557 class) flagged in the threshold-ECDSA variant analysis. - common: EnableConstantTimeOps / DisableConstantTimeOps / IsConstantTimeEnabled toggle + CTModInt (ExpCT / ModInverseCT / MulCT) over filippo.io/bigmod. - Wired ops use odd moduli only; phi(N)-even inverses stay on math/big. - Default off: zero behavior/perf change unless EnableConstantTimeOps() is called. - Bump go directive 1.16 -> 1.23 (filippo.io/bigmod floor). - Tests: framework equivalence, per-proof byte-identical/verifies equivalence, and full constant-time keygen + signing end-to-end.
Close the secret-exponent coverage gap and fix correctness/robustness issues in the opt-in constant-time path: - Harden the remaining secret-exponent modexps: MtA ProveBobWC (h1^x, h1^y), ProveRangeAlice (h1^m), the ring-Pedersen trapdoor setup in keygen (h1i^alpha), and Paillier Encrypt (gamma^m) / HomoMult (c1^m). - Pad the exponent to a fixed width in ExpCT so its running time no longer leaks the secret exponent's magnitude. - ModInverseCT returns nil for non-coprime inputs (matching math/big.ModInverse) instead of a silent wrong value. - reduceToPaddedBytes reduces unconditionally (no secret-dependent branch). - Assert odd modulus at CTModInt construction (fail at build, not at Exp). - Remove the unused TimingProtection / ConstantTimeCompare / Mod() API (the jitter helper also discarded a rand error -> nil-deref panic path). - Document the hardening coverage scope in the package doc. - Add equivalence/regression tests for every new site.
|
Update on blocking consideration 1: the Go >=1.23 compiler requirement is already satisfied on keep-core's keep-core PR #4312 goes further: it raises the module minimum to Go 1.25.7 and aligns the toolchain, CI, and Docker builders on Go 1.26.8. That PR is currently open and unmerged, but the minimum-version requirement here does not depend on it landing. The remaining consumer work is integrating the updated tss-lib revision, explicitly enabling constant-time operations where intended, and validating that integration. The keep-core PR retains the historical tss-lib pin, so its passing CI does not exercise this PR's constant-time implementation. The loop-variable semantics change from tss-lib's own The minimum keep-core compiler-version requirement in this PR's blocker list can be marked satisfied for |
Addresses review findings on PR #23's constant-time hardening entry: - CHANGELOG.md: tag Breaking Change #8 and the Added CT-symbols entry with PR #17/#23, extend the Composing PRs list (F2) - CHANGELOG.md: soften 'closing the timing side-channel' framing to scope it to the operations covered, cross-reference the mta AliceEnd/AliceEndWC gap instead of implying full closure (F6) - CHANGELOG.md: note the coverage broadening from secret-exponent-only to secret-operand operations (F7) - CHANGELOG.md: restore a 'Not ported / deferred' bullet for the mta gap so the dangling '(see below)' cross-reference resolves again (F8) - CHANGELOG.md: fix the benchmark command to actually run both BenchmarkExpCT and BenchmarkExpStandard, correct the mismatched sample-count claim (F10) - common/constant_time.go: note in the reduceToPaddedBytes NOTE that round_5's rx (a field-prime coordinate) is the one operand reduced into group-order space, and why that's still safe (F3) - common/constant_time_test.go: add BenchmarkMulCT/BenchmarkModInverseCT on a 256-bit-class modulus so the CHANGELOG's performance claim can cite the operations this PR's stack actually added, not just the 2048-bit ExpCT benchmark (F5)
Addresses the project go-rand-v2 rule on new code. crypto/schnorr/ constant_time_equiv_test.go used math/rand.NewSource for its deterministic rand.Reader override; the sibling paillier/mta/dlnproof files pre-date the rule and stay on legacy math/rand, but new code on this fork uses math/rand/v2 (see PR #4243 for the established v2+PCG+fillRandom pattern). math/rand/v2 dropped the Read method on *Rand, so the v2 *Rand does not implement io.Reader and cannot substitute for crypto/rand.Reader directly. This commit adds a small seededReader type that adapts a v2 PCG to io.Reader (8-byte Little-Endian chunks, matching the byte granularity of the legacy generator the test was written against) and uses it via newSeededReader(1, 1). The two proof generations in each test reseed the same (1, 1) tuple so both CT and non-CT runs draw identical bytes from offset zero, preserving the bit-exact equivalence asserts. Behaviour, determinism, and the test's catching-power property (bit-exact Alpha/T/U identity between CT and non-CT under matched entropy) are unchanged. CHANGELOG notes the refactor under Breaking Change #8.
|
Closing as superseded by #11 (merged into Hunk-by-hunk check against The branch |
2 participants
Summary
Closes the timing side-channel / constant-time gap (CVE-2023-26557 class) identified in the
threshold-ECDSA variant analysis. Ports the upstream BNB
tss-libconstant-time framework(
common/constant_time.go, built onfilippo.io/bigmod—the same constant-time bignum library used inside Go's
crypto/rsa/crypto/ecdsa) and wires it,behind a default-off runtime toggle, into the secret-exponent modular exponentiations.
math/bigis variable-time and leaks information about secret exponents through execution time;this provides a constant-time alternative for the operations that touch long-lived secrets.
What is wired (all behind
common.IsConstantTimeEnabled())crypto/paillier/paillier.goDecryptc^λ,Γ^λ mod N²;ModInverse(Lg, N)ExpCT;NewCTModIntWithPhi.ModInverseCTLambdaN,PhiNcrypto/paillier/paillier.goProofxs[i]^M mod NExpCTM = N⁻¹ mod φcrypto/dlnproof/proof.goNewDLNProofh1^a mod N;c·x mod pqExpCT;MulCTxcrypto/paillier/factor_proof.goFactorProofs^p,s^q mod NTildeExpCTp,qcrypto/paillier/mod_proof.goModProof+ QR helpersy^invN,x^e,x^((p-1)/2)ExpCTφ,p,qEvery modulus fed to bigmod is odd; the two φ(N)-even modular inverses (
M,invN) stay onmath/bigexactly as upstream documents. Eachelsebranch preserves the original code verbatim.Default-off / opt-in
constantTimeEnableddefaults to0. With it off (the default), behavior and performance areidentical to before. A consumer enables it once at startup:
The gap only closes once the consumer opts in. keep-core integration note: call
EnableConstantTimeOps()at startup, and note the Go-version requirement below.Verification
go build ./...,go vet(changed pkgs), fullcommon/crypto/tsssuites,ecdsa/keygen,ecdsa/signingall pass.the deterministic ops (
Decrypt,Proof, QR helpers), verifies for the randomized proofs;full CT-on keygen E2E (all parties agree on the public key) and CT-on signing E2E
(valid ECDSA signature). Every CT test asserts
IsConstantTimeEnabled()so it cannot passvacuously, and the QR helper test covers an explicit non-residue.
Blocking considerations (why "do not merge" yet)
filippo.io/bigmod). keep-core pins this forkand would need to build with Go ≥1.23. The bump also crosses Go 1.22 loop-variable
semantics — broader than just the bigmod floor. This coordination must be agreed before merge.
filippo.io/bigmod v0.1.0.isQuadResidueModPrimeCT-wrapping goes beyond upstream (no analog) and is partial —only the exponentiation is constant-time;
NewCTModInt(p)'s reduction of the secret prime isnot. Consistent with upstream's own
MulCT mod pMulQ, but a future upstream re-sync shows adelta here. Conscious choice, flagged for reviewer agreement.
Verifyis always non-CT over public data) but is not directly tested.Out of scope (separate follow-up)
The secp256k1 EC scalar-mult side channel (2019
btcecNAF →btcec/v2) — large dependencymigration, rated low-exploitability in the variant analysis, and not part of "the constant_time.go
issue."