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

Use PBKDF2 in the password checker example #910

Merged
merged 2 commits into from
Oct 14, 2023

Conversation

nategraf
Copy link
Contributor

This PR changes the password checker example to use PBKDF2 SHA-256 in the password checker, using the RustCrypto crates.

I had the thought that we actually can simply run PBKDF2 in the guest, and furthermore could actually prove a PBKDF2 derivation with reasonably secure iteration counts (e.g. 500,000) with Bonsai.
Now, this would be likely never be worth doing, but it's possible and showing how easily we can important and use existing crates, which would be huge pain to implement in another zk system, is a boon to this exmaple.

On the other hand, this PR brings in Cargo patching and the crypto accelerated forks.
We may wish to avoid adding that cogantive overhead.
This PR took me 15 minutes to put together and I am happy to throw it away. :)

Copy link
Member

@tzerrell tzerrell left a comment

Choose a reason for hiding this comment

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

Ooo, interesting... This seems like a good idea (it makes the example more "real" and as you say shows off the value of importing & using existing crates).

In thinking about whether we want this, I'm wondering whether we want password checker to be more of a cookbook or showcase (in the taxonomy of https://github.com/risc0/rfcs/pull/29) -- I think there could be an argument for either. As a showcase, I'm pretty heavily in favor of including these changes -- the benefits of showing a more complex workload are brought to the forefront, and if it's trickier for users to reproduce, that's not so big a deal with a showcase.

As a cookbook, I'm a bit less certain. It reduces the clarity of purpose of the example (although it kinda lacks clarity of purpose as-is) and not everyone will want every part of it. That said, for the specific thing we're demonstrating, it shows people techniques they will need to use to be following best practices. So I still think including this is good, but I'm a bit more split.

@3lkn do you have thoughts on where password checker would fall between showcase/cookbook and whether to include these changes in the cookbook case?

Copy link
Member

@tzerrell tzerrell left a comment

Choose a reason for hiding this comment

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

I like this and no one else has spoken up to object or request changes.

@vercel
Copy link

vercel bot commented Oct 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 13, 2023 9:16pm

@nategraf nategraf added this pull request to the merge queue Oct 13, 2023
Merged via the queue into main with commit d557b32 Oct 14, 2023
19 checks passed
@nategraf nategraf deleted the victor/password-checker-pbkdf2 branch October 14, 2023 00:37
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

2 participants