Add error handling to ReseedingRng
#32
Conversation
Instead of panicking when reseeding failed, we now return an error via `try_fill`. For all other methods we ignore the failed reseed, and try again after the next amount of `generation_threshold` bytes is consumed. If an error occurs, the cause chain is: `ErrorKind::Other` > `ReseedingError` > error from `Rsdr`
| self.bytes_generated += dest.len() as u64; | ||
| self.rng.fill_bytes(dest); | ||
| } | ||
|
|
||
| fn try_fill(&mut self, dest: &mut [u8]) -> Result<(), Error> { | ||
| self.reseed_if_necessary(); | ||
| self.reseed_if_necessary()?; |
There was a problem hiding this comment.
I'm not keen on this difference of error handling between functions. Either the ReseedingRng should always allow continuation when reseeding fails (in which case this function should either silently continue or report ErrorKind::Transient and have some extra logic not to try reseeding too soon after a failure), or ReseedingRng should always fail if reseeding fails (with panic if necessary).
I'm not sure which; it could even be configurable but probably unnecessary. Perhaps allowing continuation after failure is the best option (users shouldn't rely on reseeding for security in any case).
There was a problem hiding this comment.
Good point, we can return an error here and fill dest. Then it is up to the calling code to decide what to do.
| /// An error that can occur when reseeding failed. | ||
| #[cfg(feature="std")] | ||
| #[derive(Debug)] | ||
| pub struct ReseedingError { |
There was a problem hiding this comment.
I'm not convinced we need another error type (see #10).
There was a problem hiding this comment.
An alternative is to add a ReseedingFailed variant to the error kinds in rand_core.
It is just that I first explored this route (should have made a better PR message that said so).
| @@ -109,9 +155,9 @@ impl<S, R: SeedableRng<S>, Rsdr: Reseeder<R>> SeedableRng<(Rsdr, S)> for | |||
| } | |||
|
|
|||
| /// Something that can be used to reseed an RNG via `ReseedingRng`. | |||
| pub trait Reseeder<R: ?Sized>: Debug { | |||
| pub trait Reseeder<R: ?Sized> { | |||
There was a problem hiding this comment.
I think this API guideline was appropriate. Besides I see no functional need for the Debug bound here.
|
A variation of this is now included in #30. |
|
Feel free to close this once anything that could be useful here is part of #30. |
This is based on #30.
Instead of panicking when reseeding failed, we now return an error via
try_fill. For all other methods we ignore the failed reseed, and try again after the next amount ofgeneration_thresholdbytes is consumed.If an error occurs, the cause chain is:
ErrorKind::Other>ReseedingError> error fromRsdrI made some effort to return the error cause for
std, and a static string "reseeding failed" inno_stdmode.