zcash / librustzcash Goto Github PK
View Code? Open in Web Editor NEWRust-language assets for Zcash
License: Other
Rust-language assets for Zcash
License: Other
In current librustzcash, FFI wrapper code is mixed with cryptographic code. An example short enough to show in full is:
Lines 239 to 250 in 93e26d1
I recommend instead that the FFI wrapper functions be strictly separated from cryptographic code, something like this (uncompiled):
#[no_mangle]
pub extern "system" fn librustzcash_ask_to_ak(
ask: *const [c_uchar; 32],
result: *mut [c_uchar; 32],
) {
let ask = unsafe { &*ask };
let ak = ask_to_ak(ask);
let result = unsafe { &mut *result };
ak.write(&mut result[..]).expect("length is 32 bytes");
}
fn ask_to_ask(ask: &[u8; 32]) -> edwards::Point<Bls12, PrimeOrder> {
fixed_scalar_mult(ask, FixedGenerators::SpendingKeyGenerator);
}
In this example the cryptographic code is trivial, but there are many functions in librustzcash, such as librustzcash_sapling_check_spend, that currently have extensive, tricky, security-critical cryptographic code that is peppered with unsafe blocks for argument handling.
The advantages of separating out the FFI wrappers as above are:
I'm a rust n00b so...
How do I compile and install the lib?
Right now zcash_client_sqlite
(and lightwalletd
) doesn't gracefully handle reorgs. We need to make changes to handle those reorgs more gracefully. Relates to Electric-Coin-Company/zcash-android-wallet-sdk#23.
The bvk
field in the SaplingProvingContext
and SaplingVerifyingContext
structs does not actually contain bvk
. It actually tracks the sum of the spend cv
minus the sum of the output cv
, which is then used at the end along with valueBalance
to calculate bvk
. We should rename it (e.g. to cv_sum
) and document it to make this clearer.
Once #41 is completed, the librustzcash
crate should just be an FFI wrapper around a set of pure-Rust APIs. Currently, we test (some of) this FFI indirectly via testing of zcashd
. We should instead have FFI tests in the librustzcash
crate.
The simple way would be to duplicate tests from the pure-Rust crates, and modify them to use the FFI methods. This would probably be rather hard to maintain. It would be great to figure out a strategy to reduce test duplication, while keeping the individual tests readable. Or we might decide that doing so isn't feasible, and just decide to duplicate test logic, in which case this is an easy issue to close 😄
The zeroize
crate provides:
safe†, portable access to cross-platform intrinsics for securely zeroing memory which are specifically documented as guaranteeing they won't be "optimized away".
We should use it as appropriate to put fixed limits on the persistence of secret material in-memory.
We should address this once we reach the point in #41 of cleaning up crate APIs.
The Sapling logic for proving and verifying was refactored into the zcash_proofs
crate in #30. We should add tests!
The tests themselves should be pretty straightforward. However, we can't assume that the test runner has the mainnet Sapling parameters, so we probably want to generate fake parameters on-the-fly (if this is not too complex or slow to do in a test).
I'm trying to do a JNR wrapper of the library, and i can't find unit tests for methods like: librustzcash_sapling_check_spend
, librustzcash_sapling_binding_sig
, etc. It is in your future plans to add them?
Line 610 in 93e26d1
As I argue here https://chat.zcashcommunity.com/channel/zcashd-team?msg=KrsprfCZBfa83ksJz
it might be better to check that rk
has prime order.
The deployed circuits for Sapling (and hybrid Sprout) are not going to change now that Sapling has activated, so there is no need for callers to specify the expected hash of the files. (Doing so was useful during the testing phase, before the final parameters were generated via the MPC.)
There are official Zcash binaries which seem to implies there are official librustzcash binaries but they are statically linked.
Compilers versions and flags play a great role in security for rust. Please let testing the library itself !
We can do this in parallel with reading, just like I do in powersoftau/phase2. See HashReader
.
Hi,
I am trying to use edwards::Point
from my own library.
I am using extern crate sapling_crypto;
and extern crate pairing;
I run the following:
use sapling_crypto::jubjub::{edwards, fs::Fs, PrimeOrder, Unknown, JubjubBls12, JubjubEngine };
use pairing::bls12_381::Bls12;
pub type PK = edwards::Point<Bls12,Unknown>;
#[derive(Clone)]
pub struct JubjubPoint {
purpose: &'static str,
ge: PK,
}
and get the error that the trait 'elliptic::curves::sapling_crypto::jubjub::JubjubEngine' is not implemented for 'elliptic::curves::pairing::bls12_381::Bls12'
I also tried to import all relevant traits but I get the same error.
When I try this code as part of the tests in redjubjub.rs I get no error.
Apparently I need to import the code from jubjub.rs
that implements the jubjubEngine
trait for Bls12
but I am not sure how to do it.
Thanks!
I'm getting a test failure when running cargo test uint32_shr
. Is this normal/expected, or do I have something odd in my configuration?
Here's the tail end of the traceback:
7: core::panicking::panic_fmt
at libcore/panicking.rs:77
8: core::panicking::panic
at libcore/panicking.rs:52
9: sapling_crypto::circuit::uint32::test::test_uint32_shr
at sapling-crypto/src/circuit/uint32.rs:631
10: sapling_crypto::__test::TESTS::{{closure}}
at sapling-crypto/src/circuit/uint32.rs:624
11: core::ops::function::FnOnce::call_once
at /checkout/src/libcore/ops/function.rs:223
12: <F as alloc::boxed::FnBox<A>>::call_box
at libtest/lib.rs:1451
at /checkout/src/libcore/ops/function.rs:223
at /checkout/src/liballoc/boxed.rs:642
13: __rust_maybe_catch_panic
at libpanic_unwind/lib.rs:105
Some specs:
We should do this once the current crop of PRs is through, before the next phase of the refactor. We should then enforce consistent formatting in Travis CI.
Goals:
Strategy:
Different components will be built and cleaned up independently and then brought together as git subtrees in this librustzcash repository as code matures.
Primitive crates (not Zcash specific, generally useful):
zkcrypto/jubjub
implements the Jubjub elliptic curve, the scalar field Fr, and the base field Fq, as defined in the Zcash protocol specification.zkcrypto/ff
implements traits for finite fields. We will not be bringing in ff_derive
or any other macros, to reduce dependency trees. jubjub
will bring this in as a dependency.zkcrypto/group
implements traits and generic group tools such as multi-exponentiation and FFTs. (TODO: multi-threaded variants of these algorithms; enabled by crate feature, or built outside this crate by exposing primitives?)zkcrypto/bellman
implements circuit traits and primitive structures, as well as basic gadget implementations such as booleans, number abstractions, etc. Unlike now, bellman will be generic using ff
and won't be pairing-specific.zkcrypto/pairing
implements basic traits for pairing-friendly elliptic curve constructions.zkcrypto/bls12_381
implements BLS12-381, which brings in jubjub
as a dependency (to use its Fq implementation).zkcrypto/groth16
implements the Groth16 proving system as used in Zcash. This brings in pairing
and bellman
as dependencies.Zcash-specific crates:
zcash_primitives
contains implementations of crypto components in Zcash, such as keys and their derivations (ZIP32), addresses, notes, and any other structures or algorithms specific to Zcash, such as transactions.zcash_proofs
contains an implementation of the prover and verifier for Sapling proofs. This brings in bls12-381
, groth16
as dependencies and contains the implementation of the Spend/Output circuits.zcash_client_backend
contains tools for maintaining a Zcash wallet (something which can send and receive payments, maintain keys, etc.)librustzcash
is a C FFI for consensus-rule verification of Sapling transactions, interacting with the prover from C++, etc.Task list (as of 2019-08-14):
zip32
crate into zcash_primitives
.
sapling-crypto
across bellman
, zcash_primitives
, and zcash_proofs
.
jubjub
crate.jubjub
into workspace.
ff
traits to match jubjub
API.
impl ff::* for jubjub
group
traits to match jubjub
API.
bellman
into group
.impl group::* for jubjub
bls12_381
crate.bls12_381
into workspace.impl ff::* for bls12_381
impl group::* for bls12_381
pairing
traits to match bls12_381
API.
impl pairing::* for bls12_381
zcash_primitives::jubjub
with the new crate.
pairing::bls12_381
with the new crate.
groth16
from bellman
.
From #84.
This is mainly a technicality; 64 bytes should be sufficient for security and the resulting alpha will be adequately uniform. However, it was specified as 640 bits, i.e. 80 bytes in the protocol spec (RedDSA.GenRandom in section 5.4.6). Either the spec or the implementation should be changed; I would prefer the implementation.
librustzcash/librustzcash/src/rustzcash.rs
Lines 392 to 408 in 041671f
According to #41 there is a stated goal to
2. Separate components so that hardware wallets, light wallets etc. can use lightweight, portable implementations of the pieces that they need.
and this issue aligns directly with that goal. Currently all 3 of these parameter files are needed to init librustzcash:
3.8M sapling-output.params
46M sapling-spend.params
692M sprout-groth16.params
There now exist multiple Zcash source code forks (at least Hush and Pirate) that no longer use Sprout param files. Recently I was able to make it so Hush does not download 900MB of sprout files, since the new Hush mainnet has never had a Sprout transaction. It was a completely wasteful download: wasteful of user time+bandwidth and also costly in terms of Amazon CloudFront costs.
I looked into the Rust code to see if it was possible to initialize librustzcash without sprout-groth16.params, but it seems baked in deep. Is the librustzcash willing to support this use case or does it fall outside your support realm?
What I am looking for are versions oflibrustzcash_init_zksnark_params
that do not take sprout-groth16 file as parameter, or perhaps will not fail if null values are given for sprout-groth16:
https://github.com/zcash/librustzcash/blob/master/librustzcash/src/rustzcash.rs#L123
If you can make your library more flexible, I imagine many thousands of dollars per year can be saved in Amazon Cloudfront costs, by not having every full node of various other coins downloading files that they don't actually need.
rand crate version is now 0.6.5
while the projects in this workspace are using rand = "0.4"
.
Specifically Rand
trait is deprecated
TestConstraintSystem
is a useful module for testing circuit code, that currently lives inside the sapling-crypto
crate alongside the Sprout and Sapling circuits. It should be moved somewhere that is obviously part of the toolkit for circuit implementers.
This will probably be a new crate that depends on bellman
(given that @ebfull wants bellman
to be flexible enough that such things don't need to be declared or maintained by bellman
itself).
Part of #41.
e.g. #92 (comment)
There are strategies we can use for this, such as creating a "last raised error" FFI function.
We currently hand-craft the librustzcash.h
file for the librustzcash
crate. We could instead use cbindgen
to generate it automatically. We should look into what additional dependencies this would require zcashd
to depend on.
Hello,
I'm trying to write a test similar to the one written in mimc.rs
which does the following steps for the mimc hash function: generate parameters, create proof and then verify it. I want to do this for the blake2s hash function using the blake2s circuit already implemented in the sapling-crypto crate. The problem I am having is that verification is failing and I don't know what I'm doing wrong. Here are the major components of the code
/// This is our demo circuit for proving knowledge of the
/// preimage of a Blake2s hash invocation.
struct Blake2sDemo<E: Engine> {
preimage: Vec<Boolean>,
phantom: PhantomData<E>
}
impl<E: Engine> Circuit<E> for Blake2sDemo<E> {
fn synthesize<CS: ConstraintSystem<E>>(
self,
cs: &mut CS
) -> Result<(), SynthesisError>
{
// Compute blake2s circuit
let image = blake2s::blake2s(
cs.namespace(|| "computation of image"),
&self.preimage,
CRH_IVK_PERSONALIZATION
)?;
// makes the image a public input to the circuit
multipack::pack_into_inputs(cs.namespace(|| "pack nullifier"), &image);
Ok(())
}
}
fn test_blake2s() {
let rng = &mut thread_rng();
// Create parameters for our circuit
let mut rng2 = XorShiftRng::from_seed([0x5dbe6259, 0x8d313d76, 0x3237db17, 0xe5bc0654]);
let input_len = 8;
let data: Vec<u8> = (0..input_len).map(|_| rng2.gen()).collect();
let params = {
let c = Blake2sDemo {
preimage: convert_to_boolean_vec(&data),
phantom: PhantomData
};
generate_random_parameters::<Bls12, _, _>(c, rng).unwrap()
};
// Prepare the verification key (for proof verification)
let pvk = prepare_verifying_key(¶ms.vk);
const SAMPLES: u32 = 1;
let mut proof_vec = vec![];
for _ in 0..SAMPLES {
// Generate a random preimage and compute the image
// TODO: understand if msg has to have a multiple of 8 bytes
let mut rng2 = XorShiftRng::from_seed([0x5dbe6259, 0x8d313d76, 0x3237db17, 0xe5bc0654]);
let input_len = 8;
let mut h = Blake2s::with_params(32, &[], &[], CRH_IVK_PERSONALIZATION);
let data: Vec<u8> = (0..input_len).map(|_| rng2.gen()).collect();
h.update(&data);
let hash_result = h.finalize();
let preimage = data;
let image = hash_result.as_ref().to_vec();
proof_vec.truncate(0);
// Create an instance of our circuit (with the witness)
{
let c = Blake2sDemo {
preimage: convert_to_boolean_vec(&preimage),
phantom: PhantomData
};
// Create a groth16 proof with our parameters.
let proof = create_random_proof(c, ¶ms, rng).unwrap();
proof.write(&mut proof_vec).unwrap();
}
total_proving += start.elapsed();
let start = Instant::now();
let proof = Proof::read(&proof_vec[..]).unwrap();
let mut public_input = [Fr::zero(); 2];
// Add the blake2s hash output as public input to circuit through multiscalar packing
{
let image = multipack::bytes_to_bits_le(&image);
println!("num bits of black256: {}", image.len());
let image = multipack::compute_multipacking::<Bls12>(&image);
assert_eq!(image.len(), 2);
public_input[0] = image[0];
public_input[1] = image[1];
println!("pub input 0: {:?}", public_input[0]);
println!("pub input 1: {:?}", public_input[1]);
}
// Check the proof: THIS FAILS
assert!(verify_proof(
&pvk,
&proof,
&public_input
).unwrap());
}
}
I've spent many days debugging ensuring things like I'm passing the correct preimage to the circuit and hash function. What am I missing? Is parameter generation supposed to be done differently?
My overall goal is to strip out components of the full sapling circuit to build a smaller circuit and this test was my first step towards that. Any help would be appreciated.
I might not be understanding the code correctly, but where are we enforcing that the x-coordinate is correctly selected from the lookup table, when doing a 3-bit lookup with conditional negation (e.g. used in pedersen_hash
)?
librustzcash/sapling-crypto/src/circuit/lookup.rs
Lines 174 to 192 in 06da3b9
I can only see the y-lookup being enforced. Does enforcement happen in add_bool_with_coeff
(I couldn't find any cs.enforce
in that method)?
Or do we not need to verify the x-lookup for some reason?
This will take a point P
and a secret sk
and compute [sk] [8] P
or fail if P
is not valid.
There are several projects that use pairing and would like to contribute to it. They will probably start collaborating on a fork, but I think everyone would also be happy to contribute upstream instead.
Will the librustzcash changes be merged back into the pairing repo eventually, or is that deprecated? Will the new version be published on crates.io?
Would you be open to merging the functionality mentioned in that issue? (Most of the pairing users' feature requests seem to overlap a lot.)
Or is it better for POA, Filecoin, MaidSafe and others to work on a separate fork for now?
Most Windows systems when using ANSI APIs, and some Unix systems, do not use UTF-8 encoding for filesystem paths. It's also common for usernames to have non-ASCII characters. In that case parameter loading will fail here:
librustzcash/librustzcash/src/rustzcash.rs
Lines 144 to 157 in 041671f
The comment is correct; we should not be using CStr
, since CStr::to_str
assumes UTF-8 (see doc just above here).
Strictly speaking the Right Way to do it on Windows is to use "wide" (UTF-16) filesystem APIs. I don't know how well supported that is in Rust, and for now I'd recommend sticking to ANSI APIs and just passing the bytes through as-is.
Hi,
I want to use the crate Jubjub in your workspace in my Rust project.
Do you know what I should specify in my Cargo.toml?
With #12 merged, these wrappers are now trivial and thus redundant.
If we want to retain the slight cleanliness improvement (two lines into one) from read_le()
, we should move it to Fs::from_le()
.
It is currently possible to construct an invalid PaymentAddress
, and then pass it to functions that assume a valid address (e.g. #92 (comment)). For example, the specification requires that pk_d
is of prime order (which excludes the identity), but the type of pk_d
only enforces that it is in the prime-order subgroup (which permits the identity).
Instead of requiring all APIs to contain parameter fields, we can leverage the Rust features system to have this handled at build time. The will be a feature on the zcash_client_backend
crate and possibly on other crates depending on what makes sense.
I looked at all of zcash_client_backend
, zcash_client_sqlite
, and some of zcash_primitives
in revision ae63116. I was only focused on stuff related to the mobile wallet. I'm posting here instead of in @str4d's fork for greater visibility.
tl;dr: No major issues, mostly just stuff that hasn't been implemented yet, side-channel attack ideas, and a suggestion to adjust the threat model so that light wallets can recover from briefly connecting to a compromised lightwalletd.
Design Comments
lightwalletd
is sending correct data to the light client. In practice this won't be perfectly true. I think the threat model should be adjusted so that a light wallet must be able to recover from having once connected to a malicious lightwalletd. This seems likely to happen, e.g. when a popular lightwalletd server gets compromised temporarily. Right now it's possible for a bad lightwalletd to permanently DoS a light wallet, probably in a couple ways. One is that the highest-numbered block in the cache is taken as authoritative in validate_combined_chain
, so a bad lightwalletd can send a very-high-numbered fake block which won't be fixed by syncing with a good lightwalletd. A malicious lightwalletd could also corrupt past transaction data by sending fake transaction whose txid
already exists in the DB and then stmt_update_tx
updates its block number to the wrong value. Another way to permanently mess up a light wallet is to omit any spending transactions it sent (i.e. don't send it back to the wallet when it gets mined); the wallet will think the transaction expired and mark that balance as spendable again, and any subsequent spends of the same notes will fail.zcash_client_backend
scan_block
, so an attacker observing this through local side-channels could potentially learn which nullifiers belong to the user.scan_block_from_bytes
looks unused.encoding.rs
(there is already a test for encoding/decoding addresses).zcash_client_sqlite
rewind_to_height
doesn't seem to "rewind" any notes that were discovered (and put into the received_notes
table) in blocks that got rewound, so the user might be left thinking they have balance that they don't.validate_combined_chain
used? I can't find its use in the Rust crates or the SDK. (I was looking for it because I was concerned about threading/TOCTTOU bugs.)send_to_address
, there's a non-constant-time comparison of the viewing keys inside an SQL query. This potentially makes it easier to steal the keys through side-channels.zcash_primitives
parse_note_plaintext_minus_memo
extracts the values from the unauthenticated plaintext. There are a couple ways this function can return None
, and if an adversary can distinguish between them through side-channels they can use it as a decryption oracle. I think you could get both rcm
and v
by flipping bits in the ciphertext in a binary-search-like manner until you find a value that's one off from the maximums they are compared against (MAX_MONEY
and MODULUS
respectively). The comparison of the note commitment is also not constant time so it may be possible to learn the computed commitment value based on how long it takes the comparison to fail.spending_key
and the ZIP32 functions it calls don't enforce ZIP32's requirement that the seed "MUST be at least 32 bytes."Stuff I didn't look at
See https://trac.torproject.org/projects/tor/ticket/27199 , and the upstream Rust bug rust-lang/rust#52652 (in particular my comment here).
We currently do set panic = 'abort'
in the release profile of librustzcash, which is why this is not a security bug (I believe) for current Zcash as built by default. I think (but I could be wrong) that to avoid the undefined behaviour, it only needs to be set for the crate that is directly being called by the FFI. We may want to set it for other crates under the librustzcash project anyway.
We do this for other stuff, and I think this should be no exception. It avoids me having to check hashes in PRs like this.
Hi,
When I was testing librustzcash.a with a c++ testcase, I encounter the following error:
./librustzcash.a(compiler_builtins-d0572f7a936161bf.compiler_builtins.2h648oya-cgu.0.rcgu.o): In function `compiler_builtins::int::sdiv::rust_i128_div':
/rustc/9fda7c2237db910e41d6a712e9a2139b352e558b//src/rustc/compiler_builtins_shim/../../libcompiler_builtins/src/macros.rs:293: multiple definition of `__divti3'
/usr/bin/../lib/gcc/x86_64-linux-gnu/7.2.0/libgcc.a(_divdi3.o):(.text+0x0): first defined here
It seems a redefinition of some function with libgcc ?
Or the way I link the static lib is incorrect?
Looking forward to your reply.
Thanks!
This function will take diversifier d
, scalar esk
, compute g_d = GH(d)
or fail if d
is invalid, and produce [esk] g_d
as its result.
In the protocol specification, when verifying proof, nullifier is split into 2 parts as follows.
In check_spend function,
librustzcash/zcash_proofs/src/sapling/verifier.rs
Lines 93 to 102 in 4255b44
librustzcash/sapling-crypto/src/circuit/multipack.rs
Lines 56 to 79 in 4255b44
E::Fr::CAPACITY is 254 rather than 251. It means nf_{0..253}^{old} and nf_{254,255}^{old} are used in public_input[5] and public_input[6], respectively. In fact, it is reasonable that CAPACITY is 254, since the rs is 255-bit.
Is it possible for pedersen_hash
to return the point at infinity? If so how do we ensure that such results don't lead to hash collisions?
Let's assume we have a scalar s
that spans two segments (let's assume s
includes the 6 personalisation bits). Further lets1 = s[0..63]
and s2 = [63..-1]
be the two segments and G1, G2
two prime order base points of the jubjub curve.
If I understand the code correctly, we compute pedersen_hash(s) = s1 * G1 + s2 * G2
It seems possible that this result could be the point at infinity.
In that case, if we e.g. shift s1
and s2
by one bit each, that is s1' = s1 << 1
and s2' = s2 << 1
and calculate pedersen_hash(s')
, where s' = concat(bin(s1), bin(s2))
we get:
pedersen_hash(s') = (s1 << 1) * G1 + (s2 << 1) * G2
= 2*s1*G1 + 2*s2*G2
= 2*(s1*G1 + s2*G2)
= 2 * (INF) = INF = pedersen_hash(s)
which would be a collision, wouldn't it?
I'm likely missing some basic understanding of why this is not possible or what we are doing to avoid this case, and would appreciate some insight.
Thanks.
I'm a rust n00b so...
Any known examples for creating proofs and nullifiers from C/C++?
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.