Some fixes

[fjarri]

• remove a *Box::new hack
• functional style hash digest calculation
• more secure handling of signing_key_bytes when generating hashes

TODO:

• the derivatives of signing key bytes (Hmac, ChaCha20Rng) should be zeroized too - but currently they don't implement the trait. Need to create PRs for the corresponding crates. Probably there are other non-obvious objects that should be zeroized, need to review.
• some remaining explicit .zeroize() calls need to be replaced with zeroized-on-drop containers. The problem is that secrecy seemingly does not support exposing a mutable reference to a secret which we need e.g. in ceygen(). Trying to find a way to work around that.
• should we use https://docs.rs/secrets instead of secrecy? The problem here is that it depends on an external libsodium, but provides better zeroization guarantees.

[ok-john]

the derivatives of signing key bytes (Hmac, ChaCha20Rng) should be zeroized too - but currently they don't implement the trait. Need to create PRs for the corresponding crates. Probably there are other non-obvious objects that should be zeroized, need to review.

Curious why we use ChaCha20Rng instead of the Linux Kernel CSPRNG? The Kernel CSPRNG uses ChaCha20 and implements fast key erasure, this can be accessed through getrandom crate. Perhaps one down-side of using the system CSPRNG is that non-linux users won't get the same security guarantees.

The problem is that secrecy seemingly does not support exposing a mutable reference to a secret which we need e.g. in ceygen(). Trying to find a way to work around that.

Have you seen SecretBytesMut? It wraps a BytesMut (ref: this example).

[fjarri]

Have you seen SecretBytesMut? It wraps a BytesMut (ref: this example).

I have, but as I said the problem is that Secret (and SecretBytesMut is an alias for Secret<BytesMut>) does not provide a method to expose a mutable reference to the secret, so you can't actually mutate the BytesMut.

[fjarri]

Curious why we use ChaCha20Rng instead of the Linux Kernel CSPRNG?

Would it work on Mac? Would it work in WASM and Windows (and do we need it to)? (I don't know the answers to these questions myself, just some things to consider)

[ok-john]

I have, but as I said the problem is that Secret (and SecretBytesMut is an alias for Secret<BytesMut>) does not provide a method to expose a mutable reference to the secret, so you can't actually mutate the BytesMut.

Hmm... I just played around with this for 30 minutes, and you're right - well, the naming is certainly misleading 😅

Would it work on Mac? Would it work in WASM and Windows (and do we need it to)? (I don't know the answers to these questions myself, just some things to consider)

Mac: ✅
WASM: ✅ (Uses the browsers Crypto.getRandomValues)
Windows: ✅ (with a note that windows uses BCryptGenRandom which I have little insight into the security properties of.)
WASI: (Unclear, uses some function random_get which is nondescript in its documentation on the source of entropy).

See the full layout here.

EDIT:

and do we need it to?

That's a good question - I'm not sure. Let's sync up on this with the team. It would be good to clearly specify the target operating system(s)/environments we plan to use for ceygen.

[fjarri]

One thing I don't follow is whether getrandom actually provides a seeded RNG that we need here? Am I not seeing something?

Also this question is probably worth a separate issue filed.

[ok-john]

One thing I don't follow is whether getrandom actually provides a seeded RNG that we need here? Am I not seeing something?

So it depends on the underlying operating system {CS,}PRNG, why is it important that we seed the CSPRNG?

Most CSPRNGs (like the Linux Kernel) offer a function like get_random_bytes(void *buf, size_t len), since from a user API perspective users just want the CSPRNG to "give me cryptographically secure random bytes" and let the CSPRNG implementation handle seeding, among the other things, that make a CSPRNG secure.

[fjarri]

why is it important that we seed the CSPRNG?

It was important to the authors of tofn. @thor314 may provide a better insight, but I imagine it was for the reproducibility of shares. If seeding is not important, then certainly we have a much wider range of options here.

[thor314]

why is it important that we seed the CSPRNG?

It was important to the authors of tofn. @thor314 may provide a better insight, but I imagine it was for the reproducibility of shares. If seeding is not important, then certainly we have a much wider range of options here.

Agree that reproducibility of shares seems the most likely reason.