Skip to content

fix: harden BN254 G2 SRS ingress#22858

Merged
suyash67 merged 4 commits into
merge-train/barretenbergfrom
sb/srs-fixes
May 1, 2026
Merged

fix: harden BN254 G2 SRS ingress#22858
suyash67 merged 4 commits into
merge-train/barretenbergfrom
sb/srs-fixes

Conversation

@suyash67

Copy link
Copy Markdown
Contributor

Summary

Tightens the BN254 G2 SRS load path with four layered defenses, addressing an audit finding around G2 input validation.

  • add subgroup check on g2 points — implements affine_element::is_in_prime_subgroup() (textbook [r]·P == O check) and applies it at every G2 entry point. Catches G2 inputs that lie in a cofactor subgroup of E'(Fq2) but happen to be on-curve.
  • pin SHA-256 of g2 SRS bytes and verify on disk load — bakes the canonical Aztec [x]_2 bytes (BN254_G2_ELEMENT_BYTES) and their SHA-256 (BN254_G2_ELEMENT_SHA256) into the binary as constants, mirroring the existing G1 chunk-hash mechanism. get_bn254_g2_data now hash-compares the on-disk payload before deserialization, so a tampered bn254_g2.dat is rejected before its bytes can reach any verifier. Also implements the previously declaration-only get_bn254_g2_data.
  • reject g2 = point at infinity — explicit gate against [x]_2 = O. Infinity is in every subgroup (so the subgroup check passes) and e(−W, O) = 1 collapses the KZG pairing equation. Strictly redundant with the hash pin, but kept as defense-in-depth so any future loosening of the hash gate (e.g. a "custom SRS" flag) does not silently reopen the issue.
  • reject off-curve points in is_in_prime_subgroup — guards the primitive itself: the Weierstrass group law is unsound for off-curve coordinates, which means the [r]·P trick can return a false positive on a point satisfying y² = x³ + b' for some b' ≠ b with a prime-r factor in its order. One squaring up front closes that gap and protects every existing and future caller.

Threat model

The hash pin promotes the on-disk SRS bytes to the same trust level as the binary itself. Concretely, the attack vectors closed:

  • MITM on the plain-HTTP CRS download from crs.aztec-cdn.foundation
  • Compromise of the CDN bucket (R2/S3 key leak, BGP hijack)
  • Local cache tampering by a same-user-but-not-root process (Docker volume mounts, sibling processes, backup tooling)

None of these require the attacker to patch the verifier binary itself.

Test plan

  • srs_tests — all CrsFactory.* pass, including new Bn254HardcodedG2IsInPrimeSubgroup, Bn254G2HashMatchesPinnedBytes, Bn254G2DataLoadsAndVerifies, Bn254G2CorruptionDetected, Bn254G2InfinityRejected
  • ecc_tests — full suite (843 tests) passes, including new g2.IsInPrimeSubgroupAcceptsSubgroupPoints, g2.IsInPrimeSubgroupRejectsCofactorPoint, g2.IsInPrimeSubgroupRejectsOffCurvePoint (regression test for the off-curve guard) and the typed IsInPrimeSubgroupAcceptsSubgroupPoints
  • Compiles clean with ninja srs_tests ecc_tests

@suyash67 suyash67 changed the title fix(srs): harden BN254 G2 SRS ingress fix: harden BN254 G2 SRS ingress Apr 29, 2026
const std::string& fallback_url);

g2::affine_element get_bn254_g2_data(const std::filesystem::path& path, bool allow_download = true);
g2::affine_element get_bn254_g2_data(const std::filesystem::path& path);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question for @ludamad: this function was unimplemented before. I've implemented it to check if g2 point:

  • is not a point at infinity
  • is in the prime-ordered subgroup
  • the sha256 matches with the canonical $[x]_2$

Do we need to keep the boolean allow_download to allow g2 data to be downloaded if all of the above checks fail (like we do in g1 data)?

@ledwards2225 ledwards2225 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I think pairing this with my PR validating the actual trusted setup data would allow a paranoid user to confirm that everything is kosher but in practice this is probably the more useful line of defense

@suyash67 suyash67 merged commit f75749a into merge-train/barretenberg May 1, 2026
15 checks passed
@suyash67 suyash67 deleted the sb/srs-fixes branch May 1, 2026 21:06
danielntmd pushed a commit to danielntmd/aztec-packages that referenced this pull request May 6, 2026
BEGIN_COMMIT_OVERRIDE
fix(ci): default S3_BUILD_CACHE_AWS_PARAMS in cache_s3_transfer{,_to}
(AztecProtocol#22898)
chore: low-hanging chonk prover fixes from profiling (AztecProtocol#22855)
chore: fuse N `add_scaled` into one `parallel_for` (AztecProtocol#22893)
feat: Delayed merge implementation (AztecProtocol#22775)
chore: numeric audit response (AztecProtocol#22856)
fix: harden BN254 G2 SRS ingress (AztecProtocol#22858)
fix: remove unused hash_challenge variable in batch_merge.test.cpp
(AztecProtocol#22906)
fix(bbup): remove jq dependency (AztecProtocol#22912)
chore: fix g2 test failing on merge-train (AztecProtocol#22920)
fix(ci): error on disabled-cache in CI hash calculation (AztecProtocol#22904)
END_COMMIT_OVERRIDE
suyash67 added a commit that referenced this pull request May 14, 2026
## Summary

Addresses
[findings](https://cantina.xyz/code/3dc4ffe5-40f6-4d08-926c-b17315a5bedb/findings)
from the ecc/groups audit.

## Commits

- **finding 1** — extend `batch_affine_double_impl` slope formula to
include `a` for curves with `a ≠ 0`
- **finding 2** — ~reject non-canonical x-coordinate encodings in
`affine_element::from_compressed`~ (Opened its own
[PR](#22908))
- **finding 3** — route ECDSA / Schnorr secret-nonce scalar mul through
a new constant-time `element::mul_const_time` (Montgomery ladder + Coron
blinding)
- **finding 4** — handle `rhs = ∞` in mixed-coordinate `operator+=`
- **finding 5** — zero `y` (and `z`) in `self_set_inf` so the infinity
representation is canonical
- **finding 6** — fix `pairing.FinalExponentiation` reference helper to
iterate the full 256-bit exponent
- **finding 7** — extend the G1/G2 SRS defenses from #22858 across the
bb.js boundary: in `SrsInitSrs::execute`, SHA-256-verify every
fully-present compressed-G1 chunk against `BN254_G1_CHUNK_HASHES` before
decompression, pin `g1_points[0]` to the canonical generator and
`g1_points[1]` to `τ·G` after parsing, and SHA-256-pin the 128-byte G2
input. The subgroup check, on-disk G2 hash pin, infinity rejection, and
`is_in_prime_subgroup` implementation itself already landed in #22858.
- **finding 8** — make `field::is_zero` constant-time

## Test plan

- `ecc_tests` — pass (audit-related filter: 110 pass, 8 skipped, 0
failures)
- `srs_tests` (`CrsFactory.*`) — pass
- `bbapi_tests` — pass (30/30)
- New regression tests in `affine_element.test.cpp`, `element.test.cpp`,
`pairing.test.cpp`, `prime_field.test.cpp`
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