zkcrypto / bls12_381 Goto Github PK

Feature request to implement impl Mul<Gt> for Gt

Currently we can only multiply Gt by a Scalar.

Similar to what is done here.

Cryptographic applications should be able to wipe sensitive data from memory before deallocating it. E.g. the elements of Fr can be used as secret BLS keys.

Without support from the bls12_381 crate itself, any attempt to clear such data necessarily depends on an implementation detail of bls12_381 that is not part of the public API, e.g. Fr not using any pointers internally.

The safest way to support this would be to add explicit methods for clearing, or to impl Zeroize for Fr. If the added zeroize dependency is a concern, it could be made optional, and put behind a feature flag.

Hi, shouldn't the zero point in SW Jacobian be any point of the form (t^2:t^3:0) with t <> 0 (conventionally represented with (1:1:0)), instead of (0:1:0), which is actually the zero point in SWProjective ?

Hi, I just started learning pairing crypto, and try to read the code, I am confused about the following code:

      let t1 = self.c2 * c1; 

The current serialization notes (https://github.com/zkcrypto/bls12_381/blob/main/src/notes/serialization.rs#L23-L25) state that for points with infinity bit set "the remaining bits of the group element's encoding should be set to zero."

Is this a should in the "SHOULD" sense of RFC 2119; a behavior that is preferential but can be ignored if useful? Or is it an informal statement of a strict requirement (aka a "MUST")? This crate seems to strictly require that if the infinity bit is set that all other bits are zeroed.

Hi,

I have a bunch of optimizations for faster G1/G2 scalar multiplications sitting in a fork at https://github.com/dfaranha/bls12_381

These provide a 2x speedup and include:

• Regular w-NAF recodings to reduce the number of point additions in comparison to double-and-always add
• GLV recoding for scalar decomposition, combined with interleaving to save half of the point doublings
• Moving to homogeneous projective coordinates for pairing computation to unify point arithmetic

Some technical details can be found at https://skillsmatter.com/skillscasts/17052-experimenting-with-faster-elliptic-curves-in-rust

I would like to know if there is interest in merging, so it makes sense to put time on preparing a proper pull request.

Thank you for your attention!

Zeroize is implemented for Fp12 but not for Gt - is this intentional or accidental?

https://eprint.iacr.org/2022/367 reports a 1.37x speedup in the computation of a full optimal ate pairing over BLS12-381. We should bring the relevant algorithms into this crate, assuming they can be implemented in constant time (which is one of the primary focuses of this crate).

We'd like to have Serde support for use implementing FROST for redjubjub (tracking issue: ZcashFoundation/redjubjub#21). I'd be happy to implement this (probably feature-gated behind a serde feature?) if it would fit with the library.

I cannot find clear_cofactor() method which was recently added when using crates.io... getting compiler messages.

G1 and G2 are prime-order subgroups, but they currently have several public APIs that don't apply to prime-order groups:

• G1Affine::is_torsion_free
• G1Affine::is_on_curve
• G1Projective::clear_cofactor
• G1Projective::is_on_curve
• Similarly for G2Affine and G2Projective.

The is_* APIs are all documented as "Always returns true unless the unchecked APIs were [mis]used". Meanwhile, clear_cofactor (added in #18) is defined as mapping elliptic curve points to elements of G_{1,2}, but the type it is defined on is already an element of G_{1,2}.

These APIs were implemented because of the G*Affine::from_[un]compressed_unchecked APIs, which can technically be used to load full-group elements, but doing so is explicitly breaking the requirements set by the *_unchecked APIs, so this shouldn't be a supported case.

We should make the above APIs private (all except G*Projective::is_on_curve are used internally), and then consider on a case-by-case basis how to handle full-group elements (which in general we do not want to expose or support).

Right now README.md calls the parameter for the curve z, but in various places in the code it is called x. Probably the code should be changed to match the README. (This shouldn't affect the API.)

Exponentiation between two scalars is currently difficult because exponentiation is implemented as fn pow(&self, by: &[u64; 4]) -> Self. Whilst Scalar::to_bytes converts to a little endian byte encoded integer, it is not directly usable for pow as it does not yield an array of [u64; 4].

Consider adding a variant of pow that directly accepts Scalars.

I'd like to upgrade sha2 in my codebase to 0.10, which is used for ExpandMsgXmd<H: Digest> here. It would be great if that could be upgraded when someone gets the chance.

I spent some time on an upgrade but then gave up since I am not at all familiar with the digest traits and how hash_to_curve is implemented here.

https://eprint.iacr.org/2015/1060.pdf

The https://docs.rs/bls12_381/0.7.0/bls12_381/struct.Gt.html has very little functionality exposed at the moment.
It implements the Group trait and a few helpers, however this is not enough for one of the ZK protocols we have.

What would suffice for us is a way to get the underlying Fp12 representation of Gt. For example,

impl AsRef<Fp12> for Gt {
   ...
}

would be perfectly adequate, or any other alternative you would prefer.

Is this something you are open to adding to the library? Without this we cannot upgrade from the old pairing crate that embedded the bls implementation to the recent one.

This is related to #10 although I need less than that.

If you try to run the tests for this lib, you'll see that you get:

error: package rayon-core v1.11.0 cannot be built because it requires rustc 1.59 or newer, while the currently active rustc version is 1.56.0

This is due to the fact that:

❯ cargo tree -i rayon-core       
rayon-core v1.11.0
└── rayon v1.7.0
    └── criterion v0.3.6
        [dev-dependencies]
        └── bls12_381 v0.8.0 

As you can see here, criterion depends on rayon_core with ^0.3. After rayon moved up MSRV without bumping minor version, everything broke.

We might want to either:

• Bump MSRV.
• Fix Cargo.lock (doesn't make any sense to me as this is a lib).
• Downgrade Criterion in hopes it uses a version of rayon compatible with rustc 1.56.0 or older.

@str4d let me know what you prefer and I'll push a PR.

Where can I find performance benchmarks? For instance, point doubling, point addition, inversion etc.

In the former implementation of pairing it was possible to use Fp12 directly to work with the intermediary results of miller loops, before doing the final accumulation. With the new API of MillerLoopResult and Gt this is impossible.

You can see an example of this here: https://github.com/filecoin-project/bls-signatures/blob/master/src/signature.rs#L158-L208, which I am trying to migrate to this crate.

I was trying to implement https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-04#section-2.9. The algorithm is:

   1.  R = signature_to_point(signature)
   2.  If R is INVALID, return INVALID
   3.  If signature_subgroup_check(R) is INVALID, return INVALID
   4.  C1 = 1 (the identity element in GT)
   5.  for i in 1, ..., n:
   6.      If KeyValidate(PK_i) is INVALID, return INVALID
   7.      xP = pubkey_to_point(PK_i)
   8.      Q = hash_to_point(message_i)
   9.      C1 = C1 * pairing(Q, xP)
   10. C2 = pairing(R, P)
   11. If C1 == C2, return VALID, else return INVALID

I'm stuck on line#9, the current code allows multiplying Gt * Scalar (if I'm reading the code correctly); is there a way to do Gt * Gt?

Thanks!
Specifically line#9;

We can just have from_raw as a const fn just like in the other field impl.

Rather than just having unit tests, I think it would be nice to have property-based testing for the library using proptest; this would allow much more detailed testing / structured fuzzing. Also, some existing unit tests could probably be replaced with property-based tests. Would you be interested in a PR that added property-based tests and refactored existing unit tests (as appropriate, obviously only the ones that are really just testing a property on a single example)?

Currently we have a #[derive(Default)] on Gt. This will return Gt(Fp12::default()) which is Gt(Fp12::zero()) - which is nonsensical. Gt::default() should actually return Gt::identity() which is Gt(Fp12::one()).

This is a bugfix, but as it materially affects the API, it should go into the next breaking change release.

Why do u state that the latest release is 0.1.1 while here, the release is 0.6.0?!

Could you show how to produce a key to be used in ordinary cryptography works like sign and verify,..?

The implementation of WnafGroup::recommended_wnaf_for_num_scalars re-uses the empirical recommendations from the old pairing::bls12_381 implementation. We should review them to ensure they make sense for this implementation.

zcash/librustzcash#245 (comment)

For G1Affine and G2Affine we have the to_uncompressed and from_uncompressed methods. I would like to serialize Gt as well.

I'll start work on a PR.

Does it fit with the goals of this crate to have architecture-specific backends? For instance, a specialized NEON backend for mobile or embedded devices, or an ADX backend for x86. This conflicts with some stated goals of the crate (no unsafe code), and would require some planning on how to abstract over the backend, so I wanted to check first. It is totally reasonable for the strategy to be to instead provide a pure-rust, all-safe-code implementation as is the case now.