LFDT-Lockness / paillier-zk

Zero-knoledge proofs of some paillier cryptosystem properties for use in CGGMP21
Apache License 2.0
1 stars 1 forks source link

+-x into 2x #6

Closed maurges closed 1 year ago

maurges commented 1 year ago

In the initial implementation I forgot that the notation for y <- +-a means that y <- [0; 2a]. This patch changes the generation functions to account for that

While making this change, I also refactored tests and added a new category of tests: check that the probablity to reject a correct proof is similar to the one in paper. This helps to show that the change above didn't break security (at least completely)

survived commented 1 year ago

I see you added a lot of docs. Is it a right time to add #[forbid(missing_docs)] to the top of lib.rs or not yet?

maurges commented 1 year ago

Found another case of non-determinism that slipped me by, and fixed it. Should be completely ready now

maurges commented 1 year ago

Oh god, and now CI fails with that exact test, what happened

maurges commented 1 year ago

I'm not sure what happened there with CI. I debugged it locally, and the proofs are computed deterministically

survived commented 1 year ago

I recommend you to use rand_dev in tests rather than OsRng. It prints seed to stdout when constructed, so you can locally reproduce whatever issue you have on CI (assuming tests are 100% deterministic with fixed randomness source)

maurges commented 1 year ago

Owners: survived

=)) Good crate though, will use it

The problem with determinism here is that it's easy to accidentally use functions from unknown_order that use OsRng implicitly.

survived commented 1 year ago

it's easy to accidentally use functions from unknown_order that use OsRng

Should we then add a script that greps BigNumber::prime and others bad functions and fails CI if it found any of them in the project?

survived commented 1 year ago

So still unresolved things in this PR

If you don't want to resolve any of them within this PR, please just open an issue so we could keep track of them.

maurges commented 1 year ago

I think all those 4 changes are out of scope of this MR at least, yes. Let me make the issues for them

maurges commented 1 year ago

See #9, #10, #11

survived commented 1 year ago

Great, thanks!