Skip to content

Enable bounded bigmod operations by default - #17

Merged
piotr-roslaniec merged 2 commits into
devfrom
codex/ct-defaults-and-bounds
Sep 29, 2026
Merged

piotr-roslaniec merged 2 commits into
devfrom
codex/ct-defaults-and-bounds

Conversation

@mswilkison

@mswilkison mswilkison commented Sep 14, 2026 •

Copy link
Copy Markdown

Depends on #11 and is stacked on constant-time-hardening-backport. This carries the public corrections from #10 onto the current backport and enables the covered bigmod operations by default, following upstream's public default-on change.

Selectively adapts public commits 3169e56d59bf022817dad838ca81e62b02dd08e9 and cf15377f96bf8f3d522c6de7a626f4a13e6e3649, and upstream 44a95b248096396b4ef8a611bed789948be46267.

Encode exponents at a fixed public width, including zero, reject overflow instead of silently widening the operation, and use explicit public bounds where proof exponents exceed the arithmetic modulus width. Snapshot the mode across multi-step operations. Decryption uses its existing Carmichael exponent for inversion, preserving support for keys without the optional PhiN field.

Coverage remains limited to the documented exponentiation paths: surrounding conversions, reductions, and other math/big operations remain variable-time. The explicit opt-out API remains available. Kept as a draft for application performance and rollout review.

Validation: focused default-mode, zero, width, inverse, mode-toggle, unequal-modulus, and nil-PhiN compatibility tests passed in common, crypto/paillier, and crypto/mta. Repository-wide go vet, formatting, diff checks, independent patch review, and correction verification passed. The repository-wide test run is in progress. Existing 2048-bit exponentiation benchmarks ran successfully; these are primitive measurements on one machine and do not establish application throughput or side-channel resistance.

Full test CI and formatting CI were dispatched for this branch because the automatic PR trigger only covers master.

Port the zero-exponent path, public exponent bounds, and toggle snapshot
corrections from public PR #10 commits
3169e56 and
cf15377. Adapt the default-on behavior
from upstream PR bnb-chain#332 commit 44a95b2
on top of public PR #11 at 68686af.

Keep the existing exported method and toggle signatures, add an explicit
public exponent bound, and snapshot the mode across each multi-step
operation. Use LambdaN for decryption's inverse so keys without PhiN
remain supported. Restore prior toggle state in tests and select disabled
baselines explicitly. Document the limited coverage of the bigmod path.
@piotr-roslaniec

Copy link
Copy Markdown

Heads up — a separate review pass turned up one more CT gap that none of #8 / #10 / #11 / this PR cover: upstream bnb-chain/tss-lib's 3709c25 also hardens crypto/schnorr/schnorr_proof.go (witness multiplications in NewZKProofWithSession / NewZKVProofWithSession) and ecdsa/signing/round_3.go / round_4.go / round_5.go (k·γ, k·w, θ⁻¹, m·k, rx·σ). I cherry-picked the port onto a new stacked draft — #23 (ct-hardening-schnorr-signing-coverage, base codex/ct-defaults-and-bounds) — using the exact same IsConstantTimeEnabled() / NewCTModInt(...).MulCT / .ModInverseCT pattern. gofmt -l ., go build ./..., go vet ./..., and go test ./... all clean there; new Schnorr equivalence test (crypto/schnorr/constant_time_equiv_test.go) passes.

Separately, the same upstream commit also touches crypto/mta/share_protocol.go's AliceEnd / AliceEndWC — but via a different mechanism (sleep-based NewTimingProtection, ~200 ms + jitter) that doesn't exist in this fork and isn't a mechanical extension of the bigmod CT pattern. I deliberately did not port that, but I did add a COVERAGE comment in common/constant_time.go recording it as a known, intentionally-deferred gap so it doesn't get lost — see #23 for that doc update.

piotr-roslaniec added a commit that referenced this pull request Sep 18, 2026
…_init.go

Two spots still described CT-enablement as happening via a package init()
in common/constant_time_init.go. That file was removed before this branch
was ever pushed (superseded by PR #17's direct default-value change,
constantTimeEnabled = 1 in common/constant_time.go) -- these two CHANGELOG
lines were never updated to match and got carried forward by the cherry-pick
onto this branch. Correct the mechanism description in both places.
@mswilkison
mswilkison marked this pull request as ready for review September 18, 2026 14:37
piotr-roslaniec added a commit that referenced this pull request Sep 21, 2026
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)
@piotr-roslaniec
piotr-roslaniec changed the base branch from constant-time-hardening-backport to dev September 29, 2026 09:10
piotr-roslaniec added a commit that referenced this pull request Sep 29, 2026
Keep the #19 and #17 changelog entries and correct the round-5 rx secrecy
wording identified during review.
piotr-roslaniec added a commit that referenced this pull request Sep 29, 2026
Resolve crypto/paillier/paillier.go Proof(): keep this branch's len(xs) !=
iters guard together with dev's #11/#17-compatible constant-time branches
(ExpCT/Exp behind common.IsConstantTimeEnabled()). The inverse M = N^-1 mod
PhiN stays on math/big (PhiN is even; bigmod requires an odd modulus).
crypto/mta/proofs.go and range_proof.go auto-merged: this branch's
beta/random-sampler nil guards and current CT exponent-width call sites are
preserved; dev dependency versions win.
@piotr-roslaniec
piotr-roslaniec merged commit 815a743 into dev Sep 29, 2026
2 checks passed
@piotr-roslaniec

Copy link
Copy Markdown

Merged into dev after merging the current dev head into this branch (b1400ad). Dependency/toolchain choices from #20/#21 were retained.

Verification on the integrated head:

  • CI: Test and Go fmt green.
  • Local: go mod tidy/go mod verify, build, vet, format, and full go test ./... green (496 s).
  • Independent CT equivalence: >150 assertions, 0 failures; 11-party default-on signing succeeds; concurrent toggle test is race-clean.
  • Timing controls detected the expected math/big leak (Exp median delta 633.6%, Decrypt 968.9%); CT paths were flat in the same harness (0.03% and 0.09%).
  • Benchmark impact: Exp2048 -16.96%, DLN proof -11.21%, but Paillier Decrypt +447.97% (about 5.5x) due to constant-time inversion. This is a consumer-visible cost keep-core must budget for before its tss-lib bump.
  • Overflow panics are not peer-reachable in shipped protocol paths; every in-tree caller supplies a sufficient public bound.

#23 is the required stacked follow-up for Schnorr and signing-round coverage. One missing #10 regression (TestShareProtocolUnequalWidthsCTEquivalence) will land in the CT test-only follow-up PR.

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

piotr-roslaniec added a commit that referenced this pull request Sep 29, 2026
Merge advanced origin/dev (includes #17/#23 CT hardening) into PR #14.

Conflict choices:
- No textual conflicts; clean automatic merge.
- Go toolchain/deps and CT changes kept from origin/dev (#17 defaults/bounds,
  #23 CT schnorr-signing coverage).
- Preserved PR #14 changes on top of dev: BaseParty fatal-error latch
  (abort/abortedWith) and keygen unmarshalVSSCommitment part-count guard
  in ecdsa/keygen/round_3.go.
piotr-roslaniec added a commit that referenced this pull request Sep 29, 2026
Absorb PR #17 (ct-defaults-and-bounds) and #23 (ct-hardening-schnorr-signing-coverage). Preserve this branch's safe-prime cancellation (b547dd9) and save-data ownership changes (0f88232); dev wins on Go toolchain and dependency versions.
piotr-roslaniec added a commit that referenced this pull request Sep 29, 2026
Second integration pass: origin/dev advanced to 0efe90c (merged
#17 ct-defaults-and-bounds and #23 ct-hardening-schnorr-signing-coverage
after the first merge at 39e7d87).

Auto-merged cleanly; key choices:
- crypto/mta/proofs.go: disjoint regions survived both sides —
  #12 decoder guards (ProofBobWC 12-part arity, Bytes nil-receiver
  panics) plus #17/#23 CT changes on the same file.
- ecdsa/signing/round_5.go: #12 R == nil guard retained below the
  #23 CT-context hunks; no conflict.
- go.mod/go.sum: origin/dev dependency set retained.
piotr-roslaniec added a commit that referenced this pull request Sep 29, 2026
Absorb #17/#23 constant-time defaults/bounds and signing-coverage changes from
advanced origin/dev. Preserve this branch's nil/sampling guards in
crypto/paillier/paillier.go and crypto/mta/proofs.go/range_proof.go; dev wins
on Go toolchain and dependency versions.
piotr-roslaniec added a commit that referenced this pull request Sep 29, 2026
Absorb PR #12 (codex/input-guards) on top of #17/#23. Preserve this
branch's safe-prime cancellation and save-data ownership changes; dev
wins on Go toolchain and dependency versions.
piotr-roslaniec added a commit that referenced this pull request Sep 29, 2026
Absorb #12 input guards on top of #17/#23. Preserve this branch's
nil/sampling guards in crypto/paillier/paillier.go and crypto/mta;
dev wins on Go toolchain and dependency versions.
piotr-roslaniec added a commit that referenced this pull request Sep 29, 2026
Bring PR #9 current with dev after #17/#23/#12/#13/#15/#14/#26 landed.

Conflicts resolved:
- CHANGELOG.md: retain dev's PR #17/#23 composing-PR entry alongside PR #9;
  keep the rollout-only historical Bob compatibility (8ae2cf8) risk text.
- crypto/mta/proofs.go: preserve #9 legacy vs security-v2 tau/gamma sampling
  branches; keep dev's #15 nil sampling guard on beta. P1 8ae2cf8 opt-in
  historical Bob compatibility (default tight maxT1, opt-in (q+1)*N bound)
  unchanged.
- crypto/schnorr/schnorr_proof.go: combine #9 legacy/v2 challenge API split
  with dev's #23 constant-time MulCT branch for t = a + c*x.
- ecdsa/keygen/round_3.go: keep #14 unmarshalVSSCommitment part-count guard
  and re-apply #9 mode-conditional round.proofContext(j) for FactorVerify.
- ecdsa/signing/round_ct_wiring_test.go: select ProtocolModeSecurityV2 +
  nonce for the CT wiring test that constructs local signing parties.

Auto-merged and preserved: dev toolchain (Go 1.25.7 / 1.26.8) and deps in
go.mod/go.sum, #17 CT default-on behavior and fixed public-width ExpCTWithBitLen
calls, #12 arity/input guards, #15 nil/bounds guards, #14 keygen changes, and
dev workflows/tests. No transcript mode or CT path was dropped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants