-
Notifications
You must be signed in to change notification settings - Fork 3
Switch PRNG to ChaCha20 #21
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
base: main
Are you sure you want to change the base?
Conversation
Hmm, this comment about the
While I think |
If I'm reading this correctly, I'm seeing that Linux's |
I've updated this PR in light of some changes in rand v0.9.0 that may or may not affect Phraze. Comments and advice welcome. The fundamental question remains similar though: Which of these two methods of creating an RNG should Phraze use to generate passphrases? let mut rng = rng();
// or
let mut rng = ChaCha20Rng::from_os_rng(); |
Phraze currently uses
thread_rng
from Rust's rand crate as its pseudo random number generator.I think
thread_rng
, "under the hood," uses a stream cipher called ChaCha12 -- here's as much proof I could find in the source code. From what I can tell, this is a good choice!Buttt I couldn't help but notice the same crate offers "ChaCha20", which is 20 rounds of the same algorithm. As the documentation for ChaCha20 notes:
Seeing as Phraze can afford to be a little slower in order to become more secure and more conservative, this PR explicitly uses 20 rounds (ChaCha20).
However! Here's a long discussion among rand crate stake holders arguing that 12 rounds is enough and that 20 is overkill!. But I still thought it'd be interesting to make this PR public and open and see if anyone has any thoughts.
Implementation
I think
is the method we want to use, given this description:
How this change would affect Phraze performance
Criterion benchmark
Using default
thread_rng
:Using ChaCha20 explicitly:
As expected, doing 20 rounds takes longer -- about 2x longer it turns out.
Hyperfine benchmark
Using Hyperfine to test the performance of the CLI (
cargo install --path=. --force && hyperfine -N -w 1000 -m 1000 'phraze'
), which we could argue is a more "real world" benchmark, shows a very minor change, in fact within Hyperfine's margin of error.Using default
thread_rng
:Using ChaCha20 explicitly:
This is almost certainly because the passphrase generation function itself makes up a relatively small amount of the total time it takes to run the command
phraze
.Open questions
What do other password generators use here? I can't tell from a glance at the KeePassXC codebase.
Pros of accepting this PR
Cons of accepting this PR
thread_rng
alias, we are relying on Phraze maintainers (me?) to keep up with latest best practice for which PRNG and how many rounds are suitable. In contrast, if we stuck with the alias and just kept upgrading the version of therand
crate that Phraze depends on, we'd probably always be using a good PRNG, with a "good enough" number of rounds.