From 558171a4fd9cae33e905fd0db23a71fd8c1e8e8f Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Tue, 4 Aug 2026 11:56:48 +0200 Subject: [PATCH 1/3] Prevent feature unification from disabling ring proof blinding --- .github/workflows/ci.yml | 17 +++++++-- CHANGELOG.md | 23 +++++++++++- Cargo.toml | 22 ++++++----- README.md | 2 +- src/lib.rs | 22 +++++++++-- src/ring/bandersnatch.rs | 80 +++++++++++++++++++++++++++++++++++----- src/ring/mod.rs | 11 +++++- 7 files changed, 146 insertions(+), 31 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d28d7fd..5210f96 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,6 +7,11 @@ on: env: CARGO_TERM_COLOR: always + # `--all-features` is rejected at compile time: `std` (production prover) + # and `insecure-deterministic-no-std-prover` (deterministic no_std prover) are + # mutually exclusive. Every prover job picks one configuration explicitly. + PRODUCTION_FEATURES: mock,builder-params + DETERMINISTIC_FEATURES: insecure-deterministic-no-std-prover,builder-params,mock jobs: fmt: @@ -30,7 +35,8 @@ jobs: ~/.cargo/git target key: clippy-${{ hashFiles('Cargo.lock') }} - - run: cargo clippy --all-features --tests -- -D warnings + - run: cargo clippy --features ${{ env.PRODUCTION_FEATURES }} --tests -- -D warnings + - run: cargo clippy --no-default-features --features ${{ env.DETERMINISTIC_FEATURES }} --tests -- -D warnings build: name: Build @@ -44,7 +50,9 @@ jobs: ~/.cargo/git target key: build-${{ hashFiles('Cargo.lock') }} - - run: cargo build --all-features + - run: cargo build --features ${{ env.PRODUCTION_FEATURES }} + # `std` + `insecure-deterministic-no-std-prover` must be rejected at compile time. + - run: "! cargo build --all-features" no-std: name: Build no-std (wasm32) @@ -60,6 +68,7 @@ jobs: target key: wasm-${{ hashFiles('Cargo.lock') }} - run: cargo build --target wasm32-unknown-unknown --no-default-features + - run: cargo build --target wasm32-unknown-unknown --no-default-features --features ${{ env.DETERMINISTIC_FEATURES }} test: name: Tests @@ -73,5 +82,5 @@ jobs: ~/.cargo/git target key: test-${{ hashFiles('Cargo.lock') }} - - run: cargo test --release --all-features - + - run: cargo test --release --features ${{ env.PRODUCTION_FEATURES }} + - run: cargo test --release --no-default-features --features ${{ env.DETERMINISTIC_FEATURES }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 7800e03..41890a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,10 +52,31 @@ All changes are relative to 0.2.0, the last published version. a reusable `MockMembers` newtype in the `mock` module. - **`secret-split` feature**: side-channel resistant secret scalar multiplication, bundled into `std` (the only place a production prover runs). -- **`insecure-deterministic-prover` feature**: deterministic, non-zero-knowledge prover +- **`insecure-deterministic-no-std-prover` feature**: deterministic, non-zero-knowledge prover for `no_std` test environments. Enabling `prover` on `no_std` without it is now a compile-time error, since the ring prover has no system RNG there and would panic. +### Security +- **Feature unification can no longer disable the ring proof's zero-knowledge blinding** + (SRLabs audit finding; [ring-proof#93](https://github.com/paritytech/ring-proof/pull/93), + [ark-vrf#97](https://github.com/davxy/ark-vrf/pull/97)) + - `insecure-deterministic-no-std-prover` no longer enables `ark-vrf/test-vectors` (removed + upstream): the deterministic prover is selected by building this crate's prover + ring context via the new runtime `RingContext::new_without_blinding`, so other + `ark-vrf` users in the same build are unaffected + - Combining `insecure-deterministic-no-std-prover` with `std` (the production proving + configuration) is now a compile-time error, mirroring the existing no_std guard + - CI builds and tests both supported prover configurations explicitly instead of + `--all-features`; a regression test pins blinding on (production) and off + (deterministic prover) + +### Changed +- **arkworks dependencies bumped to 0.6** (required by the upstream changes above). + Note: since arkworks 0.6 the identity is represented as `(0, 0)` for short + Weierstrass curves, so all-zero uncompressed G1 encodings (e.g. in + `MembersCommitment`) decode as valid identity points instead of being rejected; + such commitments still fail proof verification cleanly + ### Fixed - **Validate curve points on decode to prevent panics** ([#44](https://github.com/paritytech/verifiable/pull/44)) - ark-serialize validation is enabled when decoding types that may come from untrusted sources diff --git a/Cargo.toml b/Cargo.toml index 6b3c441..d01eaac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,9 +13,11 @@ exclude = ["src/ring/data/bls12-381/zcash-srs-2-16-uncompressed.bin"] bounded-collections = { version = "0.3.2", default-features = false, features = ["scale-codec"] } parity-scale-codec = { version = "3.7.4", default-features = false, features = ["derive","max-encoded-len"] } scale-info = { version = "2.11", default-features = false, features = ["derive"] } -ark-serialize = { version = "0.5", default-features = false, features = ["derive"] } -ark-scale = { version = "0.0.13", default-features = false } -ark-vrf = { version = "0.5.1", default-features = false, features = ["bandersnatch", "ring"] } +ark-serialize = { version = "0.6", default-features = false, features = ["derive"] } +# TODO: switch back to a crates.io version once 0.0.14 (arkworks 0.6) is released +ark-scale = { git = "https://github.com/paritytech/ark-scale", rev = "93532caff34ae2af87270a5fafdbd55f69988fdd", default-features = false } +# TODO: switch back to a crates.io version once `RingContext::new_without_blinding` is released +ark-vrf = { git = "https://github.com/davxy/ark-vrf", rev = "0c4a3ef1426fe190fa59da77cf24598b1c9f051e", default-features = false, features = ["bandersnatch", "ring"] } spin = { version = "0.9", default-features = false, features = ["once"] } smallvec = { version = "1", default-features = false } sha2 = { version = "0.10", default-features = false, optional = true } @@ -57,14 +59,14 @@ prover = [] # test prover does not need it. secret-split = ["ark-vrf/secret-split"] # Deterministic, NON-ZERO-KNOWLEDGE prover for `no_std`/test environments. -# DO NOT USE IN PRODUCTION: this enables `ark-vrf/test-vectors`, which zeroes the -# ring proof's blinding rows instead of sampling randomness. The resulting proofs -# verify normally but are trivially deanonymizable by a passive observer (the ring -# member index is recoverable from public data). Exists only so the prover can run -# in `no_std` without a system RNG, for testing. -insecure-deterministic-prover = [ +# DO NOT USE IN PRODUCTION: the prover's ring context is built without column +# blinding, so proofs verify normally but are trivially deanonymizable by a +# passive observer (the ring member index is recoverable from public data). +# Exists only so the prover can run in `no_std` without a system RNG, for +# testing. Rejected at compile time when combined with `std`. Only affects +# provers built by this crate; other `ark-vrf` users in the build are untouched. +insecure-deterministic-no-std-prover = [ "prover", - "ark-vrf/test-vectors", ] # Exposes the `mock` module with a non-cryptographic `Mock` implementation # of `GenerateVerifiable`. Intended as a test double for downstream crates diff --git a/README.md b/README.md index f398021..771acaf 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ supporting up to 16127 members. | `prover` | Proof generation (`open`, `create`, `create_multi_context`) | | `secret-split` | Side-channel-resistant secret scalar multiplication (masks the secret before EC multiplication). Requires a system RNG, so it is only enabled under `std` | | `builder-params` | Includes precomputed ring builder params for building ring commitments | -| `insecure-deterministic-prover` | **Insecure, testing only.** Deterministic `no_std` prover whose proofs are trivially deanonymizable (non-zero-knowledge). Never enable for production | +| `insecure-deterministic-no-std-prover` | **Insecure, testing only.** Deterministic `no_std` prover whose proofs are trivially deanonymizable (non-zero-knowledge). Rejected at compile time when combined with `std` | | `mock` | Exposes the `mock` module with a non-cryptographic `Mock` implementation for tests | For verifier-only builds (e.g. on-chain), disable default features. diff --git a/src/lib.rs b/src/lib.rs index a16f4f3..3915e82 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,22 +1,36 @@ -#![cfg_attr(not(feature = "std"), no_std)] +// Unit tests use `std` unconditionally, so keep it linked for test builds of the +// no_std configurations (e.g. `insecure-deterministic-no-std-prover` without `std`). +#![cfg_attr(all(not(feature = "std"), not(test)), no_std)] // A production prover needs a system RNG for the ring proof's zero-knowledge // blinding (sourced via `getrandom_or_panic` deep in the proving stack). That is // only wired up when `std` is enabled; the only no_std prover is the deterministic, -// non-zero-knowledge one selected by `insecure-deterministic-prover` (test use +// non-zero-knowledge one selected by `insecure-deterministic-no-std-prover` (test use // only). Any other no_std build with `prover` compiles but panics at proof // generation, so reject it here instead. #[cfg(all( feature = "prover", not(feature = "std"), - not(feature = "insecure-deterministic-prover") + not(feature = "insecure-deterministic-no-std-prover") ))] compile_error!( - "`prover` on a no_std target requires the `insecure-deterministic-prover` feature \ + "`prover` on a no_std target requires the `insecure-deterministic-no-std-prover` feature \ (deterministic, NON-ZERO-KNOWLEDGE, testing only). Without it the ring prover has no \ system RNG and panics during proof generation. Use `std` for production proving." ); +// Cargo features are additive across the build graph: any crate enabling +// `insecure-deterministic-no-std-prover` would silently switch every other user of this +// crate in the same build to the non-zero-knowledge prover. Its proofs verify +// normally but leak the ring member index, so a `std` build (the production +// proving configuration) must never carry it. +#[cfg(all(feature = "std", feature = "insecure-deterministic-no-std-prover"))] +compile_error!( + "`insecure-deterministic-no-std-prover` is a no_std testing prover and must not be combined \ + with `std`: it disables the ring proof's zero-knowledge blinding, producing proofs \ + that verify normally but are trivially deanonymizable." +); + extern crate alloc; use alloc::vec::Vec; diff --git a/src/ring/bandersnatch.rs b/src/ring/bandersnatch.rs index 92352ec..21256dc 100644 --- a/src/ring/bandersnatch.rs +++ b/src/ring/bandersnatch.rs @@ -303,6 +303,35 @@ mod tests { .unwrap(); assert_eq!(proof_multi.len(), ring_signature_size::(3)); } + + // The ring proof's zero-knowledge blinding must be active in the production + // configuration: repeating a proof over identical inputs must give different + // bytes (fresh randomness each time). A deterministic proof is a function of + // the witness, so the ring member index becomes recoverable from public data + // while the proof still verifies (SRLabs finding #710). With + // `insecure-deterministic-no-std-prover` blinding is intentionally off and the same + // inputs must reproduce the exact same proof. The compile-time feature guards + // cannot see an upstream regression (e.g. ring-proof disabling blinding + // again); this test does. + #[test] + fn ring_proof_blinding_regression() { + let domain_size = RingDomainSize::Domain11; + let secret = BandersnatchVrfVerifiable::new_secret([42; 32]); + let member = BandersnatchVrfVerifiable::member_from_secret(&secret); + let prove = || { + let commitment = + BandersnatchVrfVerifiable::open(domain_size, &member, [member].into_iter()) + .unwrap(); + BandersnatchVrfVerifiable::create(commitment, &secret, b"ctx", b"msg") + .unwrap() + .0 + }; + let (first, second) = (prove(), prove()); + #[cfg(not(feature = "insecure-deterministic-no-std-prover"))] + assert_ne!(first, second); + #[cfg(feature = "insecure-deterministic-no-std-prover")] + assert_eq!(first, second); + } } #[cfg(test)] @@ -1130,12 +1159,40 @@ mod builder_tests { } }); - // Regression: decoding a bogus (all-zero) members commitment must fail rather than - // producing an invalid object that panics during verification. + // Regression: decoding a bogus members commitment must fail rather than + // producing an invalid object that panics during verification. Since + // arkworks 0.6 the all-zero encoding no longer works as the bogus witness: + // the identity is represented as (0, 0), so all-zero bytes decode to a + // commitment of identity points, a valid group element. Use a not-on-curve + // point instead, and check that the identity commitment, while decodable, + // fails verification cleanly. #[test] fn decode_bogus_commitment_fails() { + // x = 1, y = 0: not on the curve (y^2 = 0 != x^3 + 4). + let mut bad_bytes = vec![0u8; BandersnatchSha512Ell2::MEMBERS_COMMITMENT_SIZE]; + bad_bytes[0] = 1; + assert!(MembersCommitment::decode(&mut &bad_bytes[..]).is_err()); + let zero_bytes = vec![0u8; BandersnatchSha512Ell2::MEMBERS_COMMITMENT_SIZE]; - assert!(MembersCommitment::decode(&mut &zero_bytes[..]).is_err()); + let identity_members = MembersCommitment::decode(&mut &zero_bytes[..]).unwrap(); + + let domain_size = RingDomainSize::Domain11; + let secret = BandersnatchVrfVerifiable::new_secret([0; 32]); + let member = BandersnatchVrfVerifiable::member_from_secret(&secret); + let commitment = + BandersnatchVrfVerifiable::open(domain_size, &member, [member].into_iter()).unwrap(); + let (proof, _) = + BandersnatchVrfVerifiable::create(commitment, &secret, b"ctx", b"msg").unwrap(); + assert_eq!( + BandersnatchVrfVerifiable::validate( + domain_size, + &proof, + &identity_members, + b"ctx", + b"msg" + ), + Err(crate::Error::VerificationFailed), + ); } // The `DecodeUnchecked::decode_unchecked` entry point shares the wire format @@ -1163,13 +1220,15 @@ mod builder_tests { fn decode_unchecked_skips_validation() { use crate::ring::DecodeUnchecked; - let zero_bytes = vec![0u8; BandersnatchSha512Ell2::MEMBERS_COMMITMENT_SIZE]; + // x = 1, y = 0: not on the curve (y^2 = 0 != x^3 + 4). + let mut bad_bytes = vec![0u8; BandersnatchSha512Ell2::MEMBERS_COMMITMENT_SIZE]; + bad_bytes[0] = 1; // Validated path still rejects (sanity). - assert!(MembersCommitment::decode(&mut &zero_bytes[..]).is_err()); + assert!(MembersCommitment::decode(&mut &bad_bytes[..]).is_err()); // Unchecked path accepts the same bytes. - assert!(MembersCommitment::decode_unchecked(&mut &zero_bytes[..]).is_ok()); + assert!(MembersCommitment::decode_unchecked(&mut &bad_bytes[..]).is_ok()); } // A proof with trailing garbage bytes must not be accepted, otherwise the same @@ -1331,11 +1390,12 @@ mod builder_tests { // A ring proof that verifies under the attacker setup (the forgery vehicle), // assembled exactly as `create_multi_context` would, but with the attacker's - // prover key. + // prover key. Built without blinding: it needs system randomness, unavailable + // in the no_std deterministic configuration, and is irrelevant to the + // property under test. let prover_key = attacker_setup.prover_key(&pks).unwrap(); - let ring_prover = attacker_setup - .ring_context() - .ring_prover(prover_key, prover_idx); + let ring_prover = ark_vrf::ring::RingContext::::new_without_blinding(ring_size) + .into_ring_prover(prover_key, prover_idx); let input_msg = [::VRF_INPUT_DOMAIN, context].concat(); let input = ark_vrf::Input::::new(&input_msg).unwrap(); let output = secrets[prover_idx].output(input); diff --git a/src/ring/mod.rs b/src/ring/mod.rs index c5a3880..d2d0b0c 100644 --- a/src/ring/mod.rs +++ b/src/ring/mod.rs @@ -139,7 +139,16 @@ pub fn make_ring_setup( let pcs_params = ark_vrf::ring::PcsParams::::deserialize_uncompressed_unchecked(data).unwrap(); let ring_size = domain_size.max_ring_size::(); - ark_vrf::ring::RingSetup::::from_pcs_params(ring_size, pcs_params).unwrap() + let setup = ark_vrf::ring::RingSetup::::from_pcs_params(ring_size, pcs_params).unwrap(); + // Column blinding needs a system RNG, unavailable in no_std. Provers built + // from this context produce deterministic, NON-ZERO-KNOWLEDGE proofs, still + // valid for verifiers using the regular blinding-enabled context. + #[cfg(feature = "insecure-deterministic-no-std-prover")] + let setup = ark_vrf::ring::RingSetup { + ring_ctx: RingContext::new_without_blinding(ring_size), + ..setup + }; + setup } /// Get ring builder params for the given domain size. From bccf5dad051d0b010adf06b81c2ea638249f7e60 Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Wed, 12 Aug 2026 18:35:48 +0200 Subject: [PATCH 2/3] Bump ark-scale version and ark-vrf rev --- Cargo.toml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d01eaac..f9fd0b7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,10 +14,9 @@ bounded-collections = { version = "0.3.2", default-features = false, features = parity-scale-codec = { version = "3.7.4", default-features = false, features = ["derive","max-encoded-len"] } scale-info = { version = "2.11", default-features = false, features = ["derive"] } ark-serialize = { version = "0.6", default-features = false, features = ["derive"] } -# TODO: switch back to a crates.io version once 0.0.14 (arkworks 0.6) is released -ark-scale = { git = "https://github.com/paritytech/ark-scale", rev = "93532caff34ae2af87270a5fafdbd55f69988fdd", default-features = false } +ark-scale = { version = "0.0.14", default-features = false } # TODO: switch back to a crates.io version once `RingContext::new_without_blinding` is released -ark-vrf = { git = "https://github.com/davxy/ark-vrf", rev = "0c4a3ef1426fe190fa59da77cf24598b1c9f051e", default-features = false, features = ["bandersnatch", "ring"] } +ark-vrf = { git = "https://github.com/davxy/ark-vrf", rev = "b2eb42bb9f533d5d7607c6960dd0627a37f84f6a", default-features = false, features = ["bandersnatch", "ring"] } spin = { version = "0.9", default-features = false, features = ["once"] } smallvec = { version = "1", default-features = false } sha2 = { version = "0.10", default-features = false, optional = true } From 5a92eb37b8ea0107e3be07de9b44bf40966c2c60 Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Wed, 12 Aug 2026 18:48:16 +0200 Subject: [PATCH 3/3] Bump ark-vrf version to 0.5.2 --- Cargo.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f9fd0b7..c9bc038 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,8 +15,7 @@ parity-scale-codec = { version = "3.7.4", default-features = false, features = [ scale-info = { version = "2.11", default-features = false, features = ["derive"] } ark-serialize = { version = "0.6", default-features = false, features = ["derive"] } ark-scale = { version = "0.0.14", default-features = false } -# TODO: switch back to a crates.io version once `RingContext::new_without_blinding` is released -ark-vrf = { git = "https://github.com/davxy/ark-vrf", rev = "b2eb42bb9f533d5d7607c6960dd0627a37f84f6a", default-features = false, features = ["bandersnatch", "ring"] } +ark-vrf = { version = "0.5.2", default-features = false, features = ["bandersnatch", "ring"] } spin = { version = "0.9", default-features = false, features = ["once"] } smallvec = { version = "1", default-features = false } sha2 = { version = "0.10", default-features = false, optional = true }