Skip to content

enabling zeroize for field elements#24

Closed
zhenfeizhang wants to merge 2 commits into
zkcrypto:masterfrom
algorand:upstream
Closed

enabling zeroize for field elements#24
zhenfeizhang wants to merge 2 commits into
zkcrypto:masterfrom
algorand:upstream

Conversation

@zhenfeizhang
Copy link
Copy Markdown

This PR enables zeroize for field elements -- allows for better memory safety for upper layer libs such as pairing or bls signatures. Would love to see this functionality merged to the main repo.

Replacing PR #22

Copy link
Copy Markdown
Member

@str4d str4d left a comment

Choose a reason for hiding this comment

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

This needs to be rebased.

Comment thread Cargo.toml
byteorder = "1"
ff_derive = { version = "0.4.0", path = "ff_derive", optional = true }
rand_core = "0.5"
zeroize = {version = "1.1", features = ["zeroize_derive"]}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm still not sure that we want to have a required dependency on zeroize for the ff crate, especially as Zeroize is not part of any bound on the Field or PrimeField traits. I think I would prefer that we make this crate optional, and instead of using zeroize_derive we manually add an impl Zeroize for #repr if the zeroize feature is enabled.

This would require adding a zeroize feature flag to ff_derive, and then here:

[dependencies]
...
zeroize_crate = { package = "zeroize", version = "1.1", optional = true }

[features]
...
zeroize = ["ff_derive/zeroize", "zeroize_crate"]

(Crates that use namespaced-features can't be published, so we can't have a feature with the same name as a crate.)

cc @tarcieri.

Copy link
Copy Markdown
Contributor

@tarcieri tarcieri Sep 4, 2020

Choose a reason for hiding this comment

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

We make zeroize optional in most of the RustCrypto crates, FWIW.

I think it may be possible to avoid adding a zeroize feature to ff_derive, but I'd have to double check what I have in mind: amely there's already a zeroize_derive crate, so it might be possible to use that rather than modifying ff_derive to have custom zeroize support?

Comment thread src/lib.rs
pub use ff_derive::*;

#[macro_use]
extern crate zeroize;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could then be removed.

@ebfull ebfull closed this Sep 8, 2020
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.

4 participants