Skip to content

BBS: expose the signature header parameter (closes #2)#3

Merged
moisesja merged 3 commits into
mainfrom
feat/bbs-header-param
Jun 13, 2026
Merged

BBS: expose the signature header parameter (closes #2)#3
moisesja merged 3 commits into
mainfrom
feat/bbs-header-param

Conversation

@moisesja

Copy link
Copy Markdown
Owner

Summary

Closes #2. IBbsCryptoProvider hardcoded the BBS signature header to empty at the FFI boundary, so a consumer could not bind application data into a BBS signature/proof. This blocks the W3C bbs-2023 cryptosuite from binding its mandatory-disclosure group — the only thing that stops a malicious holder from dropping an issuer-mandated claim and still passing VerifyProof.

This PR adds an optional header parameter to all four BBS operations and threads it into the already-present FFI header arguments.

Key finding: the native zkryptium-ffi layer already accepts and consumes the header (verified in native/zkryptium-ffi/src/lib.rs and its Rust tests, which round-trip non-empty headers). So this is a managed-layer-only change — no Rust rebuild, no native redistribution.

Changes

  • ICryptoProvider.cs / DefaultBbsCryptoProvider.cs — add ReadOnlySpan<byte> header = default (last param) to Sign, Verify, DeriveProof, VerifyProof, threaded into the FFI header_ptr/len slot. Default-empty preserves prior behavior.
  • Rename noncepresentationHeader on DeriveProof/VerifyProof — it is the BBS presentation header (ph), a value distinct from the signature header (issuer-fixed at sign time vs holder-chosen at derive time). Source-compatible for all in-repo (positional) callers.
  • PublicAPI.Unshipped.txt — record the signature changes (8 removed, 8 added).
  • BbsHeaderTests.cs — 8 new NativeFFI tests: header round-trip; mismatch fails; a header-bound signature fails under the default empty header; the proof commits the header; header and presentationHeader are independently committed; default-header back-compat.
  • Docs — PRD FR-5 (change item + acceptance criterion), BBS sample section 8 (demonstrates header binding, stays exit-0 in both native-present and native-absent modes), README + FFI README notes.

API change

The nonce → presentationHeader rename and the added optional header are source-compatible for positional callers (the only kind in-repo). A downstream caller using the named argument nonce: would need to update. Acceptable at 1.0.0-preview.1.

Verification

  • dotnet build NetCrypto.sln -c Release -warnaserror → clean
  • Native-present test leg (--filter "Category!=BbsAbsent") → 703/703 passed
  • BBS sample → exits 0
  • API coverage tool → OK
  • Adversarial review (per AGENTS.md): an independent agent ran 24 exploit attempts — header-strip/mandatory-drop, header↔ph aliasing, empty-vs-1-byte boundary, FFI mid-buffer-slice marshalling, asymmetric argument-order (to catch a header/ph slot swap), and 1 MB / embedded-null headers — no exploit found; the header is committed by both Verify and the derived proof.

Follow-up (out of scope)

This only surfaces the header. To close dataproofs-dotnet's tracked dependency, a NetCrypto prerelease must ship so it can pin the new version and implement the conformant bbsHeader = SHA-256(proofHash) ‖ SHA-256(mandatoryHash) binding.

🤖 Generated with Claude Code

moisesja and others added 2 commits June 13, 2026 10:02
`IBbsCryptoProvider` hardcoded the BBS signature `header` to empty at the FFI
boundary, so a consumer could not bind application data into a BBS signature/
proof. This blocks the W3C `bbs-2023` cryptosuite from binding its mandatory-
disclosure group, the only thing that stops a holder from dropping an
issuer-mandated claim and still passing `VerifyProof`.

Add an optional `ReadOnlySpan<byte> header = default` to Sign/Verify/
DeriveProof/VerifyProof and thread it into the already-present FFI header
arguments. The native zkryptium-ffi layer already accepts and consumes the
header (verified in lib.rs and its Rust tests), so no Rust change is required.

Rename the existing `nonce` parameter on DeriveProof/VerifyProof to
`presentationHeader`: it is the BBS presentation header (`ph`), distinct from
the signature `header`. Default-empty preserves prior behavior for callers
that omit it.

- ICryptoProvider / DefaultBbsCryptoProvider: header param + ph rename + docs
- PublicAPI.Unshipped.txt: record the signature changes
- BbsHeaderTests: 8 tests proving the header is committed by Verify and the
  derived proof, independent of ph, with default-empty back-compat
- PRD FR-5, BBS sample (section 8), README + FFI README updated

Verified: solution builds with -warnaserror; native-present test leg 703/703;
BBS sample exits 0; API coverage OK.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ships the BBS `header` parameter (issue #2) so downstream consumers
(dataproofs-dotnet) can pin a release exposing the binding.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copy link
Copy Markdown
Owner Author

Automated review — looks good ✅

Reviewed the title, description, and full diff. This is a clean, well-scoped change that surfaces the BBS signature header without touching the native layer.

What I verified

  • FFI mapping is correct. Cross-checked the four managed call sites against the ZkryptiumNative P/Invoke signatures: bbs_sign/bbs_verify take header_ptr/header_len immediately before the messages, and bbs_proof_gen/bbs_proof_verify take header then ph — which is exactly the order DefaultBbsCryptoProvider now passes (header, header.Length then presentationHeader, presentationHeader.Length). No header↔ph slot swap.
  • Default-empty preserves behavior. = default is declared identically on both the interface and the sealed impl, so there's no interface-vs-concrete default divergence, and omitting the arg reproduces the prior ReadOnlySpan<byte>.Empty, 0 calls.
  • Tests are present and meaningful. The 8 new NativeFFI tests cover the security-relevant properties: header round-trip, header-mismatch → false, a header-bound signature failing under the default empty header, the proof committing the header, header/ph independence, and default-header back-compat.
  • Docs/PRD kept in sync (FR-5 change item + acceptance criterion, sample section 8, README + FFI README), as the contributor guide requires.

Notes (non-blocking)

  • The nonce → presentationHeader rename is source-breaking only for named-argument callers; the description correctly notes there are none in-repo and that this is acceptable at 1.0.0-preview.x. Tracked properly in PublicAPI.Unshipped.txt.
  • CI: the no-native (BBS-absent) leg passed; the ubuntu/windows/macos build-test jobs (which run the new native header tests) were still in progress at review time — worth a glance before merge.

Nice work threading the security rationale (mandatory-disclosure binding) through the PRD and the adversarial-review note. No changes requested.


Generated by Claude Code

@moisesja moisesja merged commit 82746d5 into main Jun 13, 2026
4 checks passed
@moisesja moisesja deleted the feat/bbs-header-param branch June 13, 2026 15:04
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.

BBS: expose the BBS header parameter on IBbsCryptoProvider (Sign/Verify/DeriveProof/VerifyProof)

1 participant