Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature flag to hide PRG. #272

Merged
merged 1 commit into from
Jul 29, 2022
Merged

Conversation

simon-friedberger
Copy link
Contributor

@simon-friedberger simon-friedberger commented Jul 25, 2022

The motivation for this PR is to remove the dependencies on the crates aes, ctr and cmac so a user of the crate can rely on an existing crypto library. This should be dable by implementing analogues of PrgAes128 and SeedStreamAes128 and constructing, e.g. Prio3Aes128Count = Prio3<Count<Field64>, PrgAes128Alt, 16>;

This would fix the remainder of #241.

src/vdaf/prio3_instances.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/vdaf/poplar1.rs Outdated Show resolved Hide resolved
src/vdaf/prio3.rs Outdated Show resolved Hide resolved
src/vdaf/prio3.rs Outdated Show resolved Hide resolved
src/vdaf/prio3_instances.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/vdaf/poplar1.rs Outdated Show resolved Hide resolved
@simon-friedberger
Copy link
Contributor Author

I've created a little copy & paste PoC to see if this actually works: https://github.com/simon-friedberger/priouser

It did require a lot of copy pasting of things for testing. Does anybody have an idea how that could be improved?

Copy link
Collaborator

@cjpatton cjpatton left a comment

Choose a reason for hiding this comment

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

Apart from deciding what to call the new feature, this is looking good. Just a few minor comments.

Cargo.toml Outdated Show resolved Hide resolved
src/vdaf/prio3.rs Outdated Show resolved Hide resolved
src/vdaf/prio3.rs Outdated Show resolved Hide resolved
src/vdaf/prio3.rs Show resolved Hide resolved
@tgeoghegan
Copy link
Contributor

I've created a little copy & paste PoC to see if this actually works: https://github.com/simon-friedberger/priouser

It did require a lot of copy pasting of things for testing. Does anybody have an idea how that could be improved?

Are you referring to struct TPrio3<M> and struct TPrio3Prep? I think we could take a change to expose items like vdaf::{run_vdaf, run_vdaf_prepare} or parts of mod prio3_test under feature test-vector (or test-util if you rename it) to make it easier to develop VDAFs or specific bits of VDAFs outside of crate prio. I think that could land in a subsequent PR, though.

@simon-friedberger
Copy link
Contributor Author

Yes, I think exposing prio3_test.rs under test-util would probably be helpful.

Copy link
Collaborator

@cjpatton cjpatton left a comment

Choose a reason for hiding this comment

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

LGTM, I'm ready to approve. First, I think it would be useful to add some context to the commit message. Plus, we should edit the top comment of this PR to point to the issue.

This allows removing the dependencies on several cryptographic libraries
like aes, ctr and cipher. This option is helpful for users who want to
use a different cryptographic library and implement their own version
of the PRG based on that.
@tgeoghegan
Copy link
Contributor

Looks great, thank you for this change @simon-friedberger!

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.

None yet

3 participants