spellshift / realm

Realm is a cross platform Red Team engagement platform with a focus on automation and reliability.
https://docs.realm.pub/
GNU General Public License v3.0
414 stars 29 forks source link

[bug] `random.string` function does not use `rand_chacha::ChaCha20Rng` #774

Closed Cictrone closed 3 months ago

Cictrone commented 4 months ago

Describe the bug Although rand::thread_rng seems to be good for most use cases it has not undergone the same level of scrutiny as rand_chacha::ChaCha20Rng which is why we chose it for random.bool and random.int. As we are trying to allow for the random library to be cryptographically secure we should try to collapse to just rand_chacha::ChaCha20Rng.

Additional context https://rust-random.github.io/book/guide-gen.html#cryptographically-secure-pseudo-random-number-generator https://rust-random.github.io/rand/rand_chacha/struct.ChaCha20Rng.html

flemingcaleb commented 4 months ago

As mentioned on the MR, its my understanding of the documentation of StdRng here that it uses ChaCha20 internally and is cryptographically equivalent but with the added benefit of periodic reseeding. I'm happy to make the change if wanted, but I do think it's worth considering staying with the default configuration if it is equally secure. That way if there is a change in the future to address some issue we will automatically inherit the change once it hits the standard library instead of needing to make internal modifications.

Cictrone commented 4 months ago

I do think we want this change. ChaCha20Rng is a standard way to operate a CSPRNG and StdRng just does not make that guarantee/promise.