hardening(tbtc/signer): bind full transcript into round nonces; gate transitional signing out of production - #4028
Merged
mswilkison merged 3 commits intoJun 11, 2026
Conversation
…transitional signing out of production The transitional StartSignRound/FinalizeSignRound flow derives round-1 nonces deterministically. Its nonce-reuse safety previously rested on two indirect properties: (1) every transcript-affecting input staying bound into the seed via the round_id derivation schema, and (2) consumed-round registry integrity on durable state, which rollback/restore/replication can silently violate. Remove the failure class instead of guarding it: - Introduce RoundNonceBinding with a documented invariant: every value entering the FROST binding factor, challenge, Lagrange set, or key material selection feeds the nonce seed directly. The seed now also binds the group verifying key, the Taproot tweak root, and the canonical signing-participant set (domain bumped to round-nonce-v2). Nonce safety no longer depends on round_id schema evolution or on registry integrity: any transcript variation yields a fresh nonce, so state rollback can only repeat identical transcripts (yielding the identical signature), never the same nonce under a new challenge. - Gate the deterministic-nonce entry points out of the production profile. Dealer DKG was already production-blocked, but persisted state created under a development profile could be carried into a production-profile process and signed with. StartSignRound and FinalizeSignRound now reject with transitional_deterministic_signing_disabled_in_production; production signing is the interactive FROST path with OS-random nonces only. A per-boot RAM-only salt was considered and rejected: the transitional flow has every member independently derive all participants' commitments with no round-1 exchange, so cross-machine determinism is load-bearing; a per-machine salt would break every multi-member bootstrap session. Mirror note: port back to the tBTC monorepo signer alongside the next extraction sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The widened round-nonce-v2 binding mixes encodings: the participants set serializes big-endian while participant_identifier keeps the v1 little-endian encoding. Harmless (fixed-width parts, length-framed by deterministic_seed) but part of the derived value -- note that any encoding change requires a new seed domain, never an in-place edit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes a completeness gap in the v2 RoundNonceBinding found in third-pass review (P1). The seed bound only the *group* verifying key, not the individual verifying shares. In the transitional flow every member re-derives ALL participants' round-1 commitments from the held key packages, so each other participant's verifying share enters the commitment list -> this member's binding factor and challenge. Two key packages can share a group verifying key while differing in a non-target share (any threshold t>=3 admits two polynomials with identical f(0) and target share but a different non-target share). Consequence under the old binding: a rolled-back/restored/cloned state (exactly #4028's threat model) could present an identical nonce seed under a *different* challenge -> the same member signs two different challenges with one deterministic nonce -> share extraction. The in-process run_dkg SessionConflict guard does not cover this, by design: nonce safety must not depend on registry integrity, since durable state can be rolled back or replicated. The production hard-gate still blocks this transitional flow in production, so the exposure is confined to the dealer-DKG dev/staging path; the interactive production path draws from OS randomness and is unaffected. Fix: bind the full serialized PublicKeyPackage (group key AND every verifying share); domain round-nonce-v2 -> round-nonce-v3. Regression: deterministic_round_nonce_and_commitment_binds_full_transcript now includes a variant with the baseline group key but a non-target verifying share swapped; it produces an identical seed (and asserts an identical group key) under the old binding, a different commitment under the new one. Full signer suite 246 pass; clippy/rustfmt clean. Mirror note: v3 domain + the widened binding port back to the tBTC monorepo signer with the next extraction sync, alongside the rest of #4028. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mswilkison
merged commit Jun 11, 2026
d47f009
into
extraction/frost-signer-mirror-2026-05-26
19 checks passed
mswilkison
added a commit
that referenced
this pull request
Jun 12, 2026
…submodules (#4036) Post-merge follow-up #2 from the June 2026 review stack (#4028–#4035). `engine.rs` was deferred-split to avoid conflicting with the open stack; with the stack merged it had grown to 18,248 lines (absorbing four merges plus the round-nonce-v3 fix), and every new PR contends for the same file. This lands the split before anything new piles onto the monolith. ## What this is A **pure code move** — no behavior change, no API change, no test-path change. | Module | Lines | Contents | |---|---|---| | `state` | 434 | in-memory engine/session state, state-file lock, registry capacity guards | | `persistence` | 1,421 | encrypted state envelope, key providers/commands, corruption recovery, persisted↔live conversions | | `config` | 392 | the `TBTC_SIGNER_*` env surface: const names, defaults, parsers, profile detection | | `policy` | 633 | admission, signing-policy firewall, rate limiting, auto-quarantine config | | `provenance` | 353 | runtime provenance attestation gate | | `telemetry` | 313 | hardening latency trackers + metrics | | `lifecycle` | 468 | canary rollout, refresh cadence/shares, emergency rekey, quarantine status | | `audit` | 376 | transcript audit, blame-proof verification, differential fuzzing | | `codec` | 430 | hex/struct codecs, Go↔frost identifier conversions | | `frost_ops` | 303 | stateless `dkg_part1..3`, nonces, signing package, share, aggregate | | `nonce` | 99 | **`RoundNonceBinding` + deterministic round-nonce derivation (round-nonce-v3), isolated for audit** | | `roast` | 1,003 | RFC-21 attempt machinery: request fingerprints, round/attempt ids, attempt-context + transition-evidence validation | | `dkg` | 257 | `run_dkg` flow + transitional-dealer production gates | | `signing` | 970 | `start_sign_round` / `finalize_sign_round` flows, bootstrap synthetic contributions | | `transaction` | 227 | taproot tx building | | `testsupport` | 88 | cfg(test) cross-module helpers (`lock_test_state`, `reset_for_tests`, …) | | `tests` | 10,558 | the former inline `mod tests`, moved **verbatim** | ## Design decisions - **`engine::tests::*` paths are preserved.** `mod tests` moved as a single child module (`engine/tests.rs`), so `scripts/run_phase5_chaos_suite.sh`'s five `cargo test … -- --exact` filters and every `engine::tests::<name>` reference in the phase docs remain valid. Splitting tests further would force rewriting those contracts — left as an explicit team decision. - **Visibility:** formerly-private items are now `pub(crate)`; each submodule opens with `use super::*;` against glob re-exports in `mod.rs`. Since `lib.rs` keeps `mod engine;` **private**, the crate-external surface is byte-identical. Per-module visibility tightening can happen incrementally later. - **`config.rs` deliberately concentrates the env surface** — it pre-stages follow-up #3 (move `TBTC_SIGNER_*` env vars into an init-time FFI config struct) as a mostly-one-file change. - **Only semantic edit in the whole diff:** the `include_str!("../testdata/coordinator_seed_vectors.json")` in the tests gains one `../` because the file now sits one directory deeper. ## Verification - `cargo fmt --check` ✅, `cargo clippy --all-targets -- -D warnings` ✅ - Full suite: **223 passed + 1 ignored / 24 / 1 — counts identical to the pre-split HEAD** (verified by stashing the split and re-running on d47f009) - `cargo test formal_verification_` ✅ (5/5); all five chaos-suite `--exact` paths ✅ - `testdata/` untouched — seed vectors and shuffle corpus remain byte-identical - Review aid: `git diff d47f009 --color-moved=zebra --color-moved-ws=ignore-all-space` renders nearly the entire diff as moved lines; `git blame -C -C` follows history across the split. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
mswilkison
added a commit
that referenced
this pull request
Jun 12, 2026
…ig (#4037) Post-merge follow-up **#3** from the June 2026 review stack (#4028–#4035; #4036 landed the engine split that stages this change): move the `TBTC_SIGNER_*` env-var surface into an init-time FFI config struct, shrinking the ops/audit surface from ~40 scattered `std::env::var` reads to one explicit, validated installation at startup. ## What this adds New FFI entry `frost_tbtc_init_signer_config(request_ptr, request_len)` taking a typed JSON `InitSignerConfigRequest` (40 optional fields; field name = lowercased `TBTC_SIGNER_*` suffix). The host installs it once at startup. ## Semantics - **Wholesale source of truth.** Once installed, the environment is *not consulted* for any covered knob; an unset field means the built-in default. No per-knob mixing of config and env — split-brain configs can't exist. - **Fail-closed init.** `deny_unknown_fields` rejects typo'd knobs; enforcement-gated policy combinations (admission, signing-policy firewall, auto-quarantine) are validated at install by running the same loaders the runtime gates use, with rollback on rejection — a misconfigured signer fails at startup, not at first signing. - **Idempotent re-init** for an identical request (fingerprint match); conflicting re-init rejected. - **Secrets never ride the config FFI.** `TBTC_SIGNER_STATE_ENCRYPTION_KEY_HEX` is read exclusively from the dedicated env/command key-provider channel even when a config is installed (the one deliberate `std::env::var` left outside the chokepoint, commented at the read). - **Transitional compatibility.** With no config installed, `engine::signer_env_var` falls through to the process environment — existing hosts and the entire pre-existing test suite run unchanged; non-development profiles log a one-time warning suggesting the init FFI. ## Why parity is safe by construction The typed request converts to the same canonical strings the existing env parsers consume (`"true"`/`"false"`, decimal ints, comma-joined identifier lists), and every existing clamp/warn/reject path runs unchanged on identical inputs. The diff swaps `std::env::var(X)` → `signer_env_var(X)` at 31 sites and changes nothing else about how values are interpreted. Also deletes `lib.rs`'s duplicated profile/truthy parsing in favor of the engine's single implementation. Thanks to #4036, this lands as one new ~400-line module (`engine/init_config.rs`) plus one-line touches across `config/lifecycle/persistence/policy/provenance/state` — not an 18k-line-file churn. `engine/config.rs` remains the single home of the env-name constants. ## Verification - `cargo fmt --check` ✅; `cargo clippy --all-targets -- -D warnings` ✅ - Full suite **235 passed + 1 ignored / 24 / 1** — all 224 pre-existing tests pass unchanged (env-fallback parity), plus 11 new tests: config-over-env precedence, wholesale env-ignoring for unset fields, idempotent/conflicting re-init, invalid-profile rejection, install rollback on incomplete firewall policy, complete-admission-policy validation, secret-stays-on-env-channel, production-profile-forces-strict via config, `reset_for_tests` clearing, `deny_unknown_fields`, list/bool canonicalization, and an FFI round-trip - `--features bench-restart-hook` builds; chaos suite (5/5 `--exact` paths) and `formal_verification_` filter pass - `include/frost_tbtc.h` gains the symbol; README documents the contract ## Notes for reviewers - Knobs the runtime warn-and-defaults on (e.g. out-of-range timeouts) keep that behavior under config values — init validation only rejects what the runtime gates would reject. Tightening init further is possible later without breaking the contract. - Go-host adoption is a follow-up: this is additive ABI; nothing changes for hosts until they call the new entry. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
mswilkison
added a commit
that referenced
this pull request
Jun 12, 2026
…ence (#4040) Post-merge follow-up **#4** from the June 2026 review stack (#4028–#4035): replace `json.Marshal` as the canonical signed-bytes encoding for evidence snapshots/bundles — explicitly scheduled to land **before Phase 7 wiring ossifies the format**. (Items 2 and 3 landed as #4036/#4037 on the mirror branch; this is the Go-side sibling on the scaffold branch.) ## Why now The RFC-21 Layer B evidence signatures were computed over canonical JSON. That byte stability is a Go-implementation accident — field-order-stable `encoding/json` output — not a portable contract. The moment Phase 7 wires evidence verification into the Rust signer (or any second implementation appears), every verifier would need to replicate Go's exact JSON emission. No persisted or cross-component evidence exists yet, so the format can still change for free. ## Design: sign what you transmit, verify what you received New `pkg/frost/roast/gen/pb/evidence.proto`: - A snapshot travels as `SignedLocalEvidenceSnapshot{body, operator_signature}` where `body` is the serialized `LocalEvidenceSnapshotBody` — the operator signs those exact bytes. - A transition message travels as `SignedTransitionMessage{body, coordinator_signature}` whose `TransitionMessageBody` embeds every member's signed snapshot envelope **verbatim** (`repeated bytes signed_snapshots`) — the coordinator attests to the exact signed snapshots it assembled, in order. - Producers marshal a body **exactly once**, at signing time, and cache it; parsed messages retain received body/envelope bytes verbatim; verification always runs over exact received bytes. **Nothing in the evidence chain is ever re-encoded**, so signature validity never depends on any serializer's canonical form — across protobuf library versions or across languages. This deliberately sidesteps protobuf's own caveat that deterministic serialization is not canonical across implementations. - `Marshal` of a received message returns the received envelope verbatim — evidence bytes survive re-broadcast, including wire-legal but non-canonical encodings (pinned by a handcrafted reversed-field-order test). - `CanonicalSnapshotBytes`/`CanonicalBundleBytes` → `SignableBytes()` accessors; the coordinator's first-write-wins conflict check now compares exact signed bytes. ## Tests - Existing suite migrated off JSON fixtures: test-only encode helpers bypass production signing so every structural-rejection path (zero sender, bad hash length, unsorted/duplicate entries, oversize caps, bundle ordering/hash-binding) is still exercised at the wire level. - New `wire_test.go` pins the format's core properties: byte-preservation through unmarshal→re-marshal, verbatim snapshot-envelope embedding inside bundle bodies, producer-signed bytes == receiver-verified bytes, non-canonical-encoding survival, tampered-body verification failure. - `go build ./...`, `go vet`, `gofmt` clean; frost + tbtc package tests green. Generated with protoc 33.4 / protoc-gen-go v1.36.3 (matches the go.mod protobuf runtime v1.36.3). ## Docs RFC-21 "Evidence message format" decision rewritten: signed-body protobuf envelopes, with the retirement rationale for canonical JSON recorded. ## Notes for reviewers - The in-memory model types (`LocalEvidenceSnapshot`, `TransitionMessage`) are unchanged apart from two unexported byte caches; all call sites kept their shapes. - Immutability contract: evidence fields must not be mutated after `SignableBytes()` is first computed (documented on the cache fields); the aggregation flow already treats snapshots as immutable post-receipt. - Phase 7 cross-language note: the Rust signer will verify operator/coordinator signatures over `body` bytes and parse them with any protobuf implementation — no canonicalization requirements transfer. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #4005 (base:
extraction/frost-signer-mirror-2026-05-26). Implements item 1 of the review feedback on the FROST/ROAST stack (nonce rollback safety).Problem
The transitional
StartSignRound/FinalizeSignRoundflow derives round-1 nonces deterministically fromH("round-nonce", signing_share, session_id, round_id, message, participant_id). Nonce-reuse safety therefore rested on two indirect properties:round_idschema integrity — the participants set, Taproot tweak, and attempt context were bound only throughderive_round_id. Any future change to that derivation (or an encoding collision in it) could let two different FROST transcripts share a nonce seed, which is the share-extraction condition.What changed
1. Total direct binding (
RoundNonceBinding, domainround-nonce-v2). The nonce seed now directly binds every value that enters the FROST binding factor, challenge, Lagrange interpolation set, or key-material selection: group verifying key, Taproot merkle root, canonical signing-participant set — in addition to the existing signing share, session, round, message, and participant id. The struct carries a documented invariant so the next transcript input added to this flow gets added to the seed in the same change. Consequence: a rolled-back or cloned state can only ever repeat an identical transcript (producing the identical signature — no new information), never the same nonce under a different challenge. Nonce safety no longer depends onround_idschema or registry integrity at all.2. Production hard-gate on the deterministic-nonce entry points. Dealer DKG was already blocked in production, but that gate only fires at session creation: persisted state created under a development profile could be carried into a production-profile process and signed with.
StartSignRound/FinalizeSignRoundnow reject in the production profile with reasontransitional_deterministic_signing_disabled_in_production, making the OS-random interactive FROST path the only production signing path regardless of how on-disk state was created.Why not the per-boot RAM-only salt suggested in the review
Exploration showed the transitional flow has every member independently derive all participants' commitments with zero round-1 exchange (members exchange only signature shares), so cross-machine determinism is load-bearing: a per-machine salt would break every multi-member bootstrap session. The two changes above implement the same goal — rollback costs liveness, never keys — within the flow's actual architecture: (1) removes the nonce-reuse class structurally, (2) removes production exposure structurally. The stateless interactive path (
GenerateNoncesAndCommitments) already draws from OS randomness and holds nonces only in caller RAM.Tests
deterministic_round_nonce_and_commitment_binds_full_transcript: identical binding re-derives identical commitments; each of 7 binding inputs (message, tweak root, participants set, group key, session, round, participant) independently changes the commitment.start_sign_round_rejects_transitional_signing_in_production_profile/finalize_sign_round_rejects_transitional_signing_in_production_profile: the state-smuggling scenario — dev-created dealer session, production-profile process — rejects at both entry points, even with the strict-mode env flag explicitly disabled.production_profile_forces_roast_strict_mode_without_env_flagrepurposed to assert the strict-mode forcing at the helper level (the FFI-level path it previously exercised is now unreachable in production by design).Notes for the mirror
round-nonce→round-nonce-v2) changes derived commitments for identical inputs; a mixed-version fleet cannot co-sign transitional rounds mid-rollout. Dev/staging-only flow, so the cost is a failed attempt until the fleet converges.