Skip to content

hardening(frost): zeroize DKG FFI secret buffers (parity with Sign path)#4128

Merged
mswilkison merged 1 commit into
feat/frost-schnorr-migration-scaffoldfrom
hardening/zeroize-dkg-ffi-buffers
Jul 2, 2026
Merged

hardening(frost): zeroize DKG FFI secret buffers (parity with Sign path)#4128
mswilkison merged 1 commit into
feat/frost-schnorr-migration-scaffoldfrom
hardening/zeroize-dkg-ffi-buffers

Conversation

@mswilkison

Copy link
Copy Markdown
Contributor

Finding

The native tbtc-signer Sign path scrubs its Go-heap FFI transport buffers that carry secret material — defer zeroBytes(...) on the request payload, response payload, and nonces slices (native_frost_engine_tbtc_signer_registration_frost_native.go). The DKG path in the same file did not, leaving long-term share / DKG secret material resident in the Go heap after use. This is a memory-hygiene inconsistency: DKG secrets (private polynomial coefficients, per-recipient secret shares, and the final long-term signing share) linger in reclaimable-but-unwiped Go heap buffers, whereas the equivalent Sign secrets are wiped.

Sites zeroed

Each DKG engine method marshals a Go-heap JSON request, hands a copy to Rust via C.CBytes, and receives the response as a fresh Go slice via C.GoBytes. Only the genuinely secret-bearing Go-side buffers are wiped (public-only buffers are left untouched):

Method Buffer Secret it carries
Part1 (~L718) response round-1 secret package (private polynomial coefficients, "must never be broadcast")
Part2 (~L737) request round-1 secret package
Part2 (~L746) response round-2 secret package + per-recipient round-2 secret shares
Part3 (~L768) request round-2 secret package + received round-2 secret shares
Part3 (~L777) response final key package (long-term signing share)
RunDKGWithSeed (~L686) request DKG seed that deterministically reconstructs the group secret

Left untouched because they carry no secret: RunDKG request/response (participant public keys + metadata), RunDKGWithSeed response (public metadata only), Part1 request (participant id + signer counts).

How it mirrors the Sign path

The Sign path uses the package-local zeroBytes(data []byte) helper (native_frost_engine_frost_native.go:59) via defer:

  • GenerateNoncesAndCommitments: defer zeroBytes(responsePayload) (response carries one-time nonces).
  • Sign: defer zeroBytes(noncesData) and defer zeroBytes(requestPayload).

This change reuses the same helper and the same defer placement (right after a secret request is built / a secret response is received), so a mid-function or error return still wipes. No new/divergent mechanism is introduced.

cgo-safety reasoning

  • callBuildTaggedTBTCSignerOperation already C.CBytes-copies the request to the C heap and, on defer, zeroBytes+C.frees that C copy. The Go-side request slice is a separate json.Marshal allocation, so zeroing it after the call returns neither races the C side nor risks a double-free.
  • The response is a C.GoBytes copy; the C-side response buffer is freed separately by tbtc_signer_free_buffer. Wiping the Go copy is independent and safe.
  • The deferred zeroBytes runs after the decoder evaluates the return value, and the decoders return freshly hex-decoded copies (independent of the transport buffer), so wiping never corrupts the returned secret. Identical ordering to the existing GenerateNoncesAndCommitments.

Validation

  • gofmt -l clean on the touched file.
  • go vet -tags "frost_native frost_tbtc_signer" ./pkg/frost/signing/ clean.
  • go build -tags "frost_native frost_tbtc_signer" ./pkg/frost/... succeeds (cgo path uses runtime dlopen, so it compiles the touched file without the Rust lib present).
  • go build -tags "frost_roast_retry" ./pkg/frost/... succeeds (non-cgo compile check).
  • DKG/RunDKG/Sign/Nonces unit tests pass.

🤖 Generated with Claude Code

The native tbtc-signer Sign path scrubs its Go-heap FFI transport buffers
that carry secret material (via `defer zeroBytes(...)` on the request/
response/nonces slices), but the DKG path did not, leaving long-term
share and DKG secret material resident in the Go heap after use. This
closes that DKG<->Sign zeroization inconsistency.

The DKG engine methods build a Go-heap request payload (JSON), hand a C
copy to the Rust FFI via C.CBytes, and receive the response as a fresh
Go slice via C.GoBytes. callBuildTaggedTBTCSignerOperation already scrubs
and frees the C-heap request copy, and the Rust side frees the C response
buffer, but the Go-side request/response slices were never wiped. Mirror
the Sign path exactly by deferring zeroBytes on the secret-bearing Go
buffers, so a mid-function or error return still wipes:

- Part1: response (round-1 secret package / private polynomial coeffs).
- Part2: request (round-1 secret package) and response (round-2 secret
  package + per-recipient round-2 secret shares).
- Part3: request (round-2 secret package + received secret shares) and
  response (final key package / long-term signing share).
- RunDKGWithSeed: request (DKG seed that deterministically reconstructs
  the group secret); its response is public metadata only.

Public-only buffers are left untouched (RunDKG request/response, Part1
request). The defers run after the decoders evaluate the return value,
and the decoders return freshly hex-decoded copies, so wiping the
transport buffers never corrupts the returned secrets. cgo-safe: the
Go slices are independent of the C copies, so zeroing them after the
call returns neither races the C side nor risks a double-free.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d13930c8-9c87-4910-8cab-f9c4a6cc0199

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hardening/zeroize-dkg-ffi-buffers

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mswilkison mswilkison merged commit 275b508 into feat/frost-schnorr-migration-scaffold Jul 2, 2026
17 checks passed
@mswilkison mswilkison deleted the hardening/zeroize-dkg-ffi-buffers branch July 2, 2026 16:39
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.

1 participant