From 1136240d9079ff181cc80705b3b383400b762241 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Sun, 7 Jan 2018 11:34:53 +0000 Subject: [PATCH 1/5] Add new error types from dhardy/master branch Design: https://github.com/dhardy/rand/issues/10 --- src/error.rs | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 4 ++ 2 files changed, 143 insertions(+) create mode 100644 src/error.rs diff --git a/src/error.rs b/src/error.rs new file mode 100644 index 00000000000..620541a99ef --- /dev/null +++ b/src/error.rs @@ -0,0 +1,139 @@ +// Copyright 2017-2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! Error types + +use core::fmt; + +#[cfg(feature="std")] +use std::error::Error as stdError; + +/// Error kind which can be matched over. +#[derive(PartialEq, Eq, Debug, Copy, Clone)] +pub enum ErrorKind { + /// Permanent failure: likely not recoverable without user action. + Unavailable, + /// Temporary failure: recommended to retry a few times, but may also be + /// irrecoverable. + Transient, + /// Not ready yet: recommended to try again a little later. + NotReady, + /// Uncategorised error + Other, + #[doc(hidden)] + __Nonexhaustive, +} + +impl ErrorKind { + /// True if this kind of error may resolve itself on retry. + /// + /// See also `should_wait()`. + pub fn should_retry(self) -> bool { + match self { + ErrorKind::Transient | ErrorKind::NotReady => true, + _ => false, + } + } + + /// True if we should retry but wait before retrying + /// + /// This implies `should_retry()` is true. + pub fn should_wait(self) -> bool { + self == ErrorKind::NotReady + } + + /// A description of this error kind + pub fn description(self) -> &'static str { + match self { + ErrorKind::Unavailable => "permanent failure or unavailable", + ErrorKind::Transient => "transient failure", + ErrorKind::NotReady => "not ready yet", + ErrorKind::Other => "uncategorised", + ErrorKind::__Nonexhaustive => unreachable!(), + } + } +} + +/// Error type of random number generators +/// +/// This is a relatively simple error type, designed for compatibility with and +/// without the Rust `std` library. It embeds a "kind" code, a message (static +/// string only), and an optional chained cause (`std` only). +#[derive(Debug)] +pub struct Error { + kind: ErrorKind, + msg: &'static str, + #[cfg(feature="std")] + cause: Option>, +} + +impl Error { + /// Create a new instance, with specified kind and a message. + pub fn new(kind: ErrorKind, msg: &'static str) -> Self { + #[cfg(feature="std")] { + Self { kind, msg, cause: None } + } + #[cfg(not(feature="std"))] { + Self { kind, msg } + } + } + + /// Create a new instance, with specified kind, message, and a + /// chained cause. + /// + /// Note: `stdError` is an alias for `std::error::Error`. + /// + /// If not targetting `std` (i.e. `no_std`), this function is replaced by + /// another with the same prototype, except that there are no bounds on the + /// type `E` (because both `Box` and `stdError` are unavailable), and the + /// `cause` is ignored. + #[cfg(feature="std")] + pub fn with_cause(kind: ErrorKind, msg: &'static str, cause: E) -> Self + where E: Into> + { + Self { kind, msg, cause: Some(cause.into()) } + } + + /// Create a new instance, with specified kind, message, and a + /// chained cause. + /// + /// In `no_std` mode the *cause* is ignored. + #[cfg(not(feature="std"))] + pub fn with_cause(kind: ErrorKind, msg: &'static str, _cause: E) -> Self { + Self { kind, msg } + } + + /// Get the error kind + pub fn kind(&self) -> ErrorKind { + self.kind + } + + /// Get the error message + pub fn msg(&self) -> &'static str { + self.msg + } +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "RNG error [{}]: {}", self.kind.description(), self.msg()) + } +} + +#[cfg(feature="std")] +impl stdError for Error { + fn description(&self) -> &str { + self.msg + } + + fn cause(&self) -> Option<&stdError> { + self.cause.as_ref().map(|e| e.as_ref() as &stdError) + } +} diff --git a/src/lib.rs b/src/lib.rs index ead940781ae..79f0566a10e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -265,6 +265,9 @@ pub use isaac::{IsaacRng, Isaac64Rng}; pub use chacha::ChaChaRng; pub use prng::XorShiftRng; +// error types +pub use error::{ErrorKind, Error}; + // local use declarations #[cfg(target_pointer_width = "32")] use prng::IsaacRng as IsaacWordRng; @@ -294,6 +297,7 @@ pub mod isaac { } // private modules +mod error; mod rand_impls; mod prng; From a68cb1476561fee3c0ce2b58ace62093687247a0 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Mon, 8 Jan 2018 15:35:17 +0000 Subject: [PATCH 2/5] Fix for Rust 1.15 --- src/error.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/error.rs b/src/error.rs index 620541a99ef..f1eb27ef780 100644 --- a/src/error.rs +++ b/src/error.rs @@ -78,10 +78,10 @@ impl Error { /// Create a new instance, with specified kind and a message. pub fn new(kind: ErrorKind, msg: &'static str) -> Self { #[cfg(feature="std")] { - Self { kind, msg, cause: None } + Error { kind: kind, msg: msg, cause: None } } #[cfg(not(feature="std"))] { - Self { kind, msg } + Error { kind: kind, msg: msg } } } @@ -98,7 +98,7 @@ impl Error { pub fn with_cause(kind: ErrorKind, msg: &'static str, cause: E) -> Self where E: Into> { - Self { kind, msg, cause: Some(cause.into()) } + Error { kind: kind, msg: msg, cause: Some(cause.into()) } } /// Create a new instance, with specified kind, message, and a @@ -107,7 +107,7 @@ impl Error { /// In `no_std` mode the *cause* is ignored. #[cfg(not(feature="std"))] pub fn with_cause(kind: ErrorKind, msg: &'static str, _cause: E) -> Self { - Self { kind, msg } + Error { kind: kind, msg: msg } } /// Get the error kind From 230b2258dbd99ff8bd991008c972d923d4b5d10c Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Mon, 8 Jan 2018 11:18:10 +0000 Subject: [PATCH 3/5] Add Rng::try_fill_bytes; revise error handling in ReadRng --- src/lib.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ src/os.rs | 6 +++++- src/read.rs | 41 ++++++++++++++++------------------------- src/reseeding.rs | 8 +++++++- 4 files changed, 76 insertions(+), 27 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 79f0566a10e..fb272dbe70a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -453,6 +453,22 @@ pub trait Rng { impls::fill_bytes_via_u64(self, dest) } + /// Fill `dest` entirely with random data. + /// + /// This is the only method which allows an RNG to report errors while + /// generating random data; other methods either handle the error + /// internally or panic. This method is + /// the intended way to use external (true) RNGs, like `OsRng`. Its main + /// use-cases are to generate keys and to seed (infallible) PRNGs. + /// + /// Other than error handling, this method is identical to [`fill_bytes`], and + /// has a default implementation simply wrapping [`fill_bytes`]. + /// + /// [`fill_bytes`]: trait.Rng.html#method.fill_bytes + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { + Ok(self.fill_bytes(dest)) + } + /// Return a random value of a `Rand` type. /// /// # Example @@ -607,48 +623,68 @@ pub trait Rng { } impl<'a, R: ?Sized> Rng for &'a mut R where R: Rng { + #[inline] fn next_u32(&mut self) -> u32 { (**self).next_u32() } + #[inline] fn next_u64(&mut self) -> u64 { (**self).next_u64() } + #[inline] fn next_f32(&mut self) -> f32 { (**self).next_f32() } + #[inline] fn next_f64(&mut self) -> f64 { (**self).next_f64() } + #[inline] fn fill_bytes(&mut self, dest: &mut [u8]) { (**self).fill_bytes(dest) } + + #[inline] + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { + (**self).try_fill_bytes(dest) + } } #[cfg(feature="std")] impl Rng for Box where R: Rng { + #[inline] fn next_u32(&mut self) -> u32 { (**self).next_u32() } + #[inline] fn next_u64(&mut self) -> u64 { (**self).next_u64() } + #[inline] fn next_f32(&mut self) -> f32 { (**self).next_f32() } + #[inline] fn next_f64(&mut self) -> f64 { (**self).next_f64() } + #[inline] fn fill_bytes(&mut self, dest: &mut [u8]) { (**self).fill_bytes(dest) } + + #[inline] + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { + (**self).try_fill_bytes(dest) + } } /// Iterator which will generate a stream of random items. @@ -811,6 +847,11 @@ impl Rng for StdRng { fn fill_bytes(&mut self, dest: &mut [u8]) { self.rng.fill_bytes(dest) } + + #[inline] + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { + self.rng.try_fill_bytes(dest) + } } impl<'a> SeedableRng<&'a [usize]> for StdRng { @@ -895,10 +936,12 @@ pub fn thread_rng() -> ThreadRng { #[cfg(feature="std")] impl Rng for ThreadRng { + #[inline] fn next_u32(&mut self) -> u32 { self.rng.borrow_mut().next_u32() } + #[inline] fn next_u64(&mut self) -> u64 { self.rng.borrow_mut().next_u64() } @@ -907,6 +950,11 @@ impl Rng for ThreadRng { fn fill_bytes(&mut self, bytes: &mut [u8]) { self.rng.borrow_mut().fill_bytes(bytes) } + + #[inline] + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { + self.rng.borrow_mut().try_fill_bytes(dest) + } } /// Generates a random value using the thread-local random number generator. diff --git a/src/os.rs b/src/os.rs index 3526e85f2e0..f49f2a70835 100644 --- a/src/os.rs +++ b/src/os.rs @@ -12,7 +12,7 @@ //! generators. use std::{io, mem, fmt}; -use Rng; +use {Rng, Error}; /// A random number generator that retrieves randomness straight from /// the operating system. Platform sources: @@ -45,6 +45,10 @@ impl Rng for OsRng { fn next_u32(&mut self) -> u32 { self.0.next_u32() } fn next_u64(&mut self) -> u64 { self.0.next_u64() } fn fill_bytes(&mut self, v: &mut [u8]) { self.0.fill_bytes(v) } + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { + // TODO: error handling per variant + Ok(self.0.fill_bytes(dest)) + } } impl fmt::Debug for OsRng { diff --git a/src/read.rs b/src/read.rs index 7a20a52ec2a..0590d6fe35e 100644 --- a/src/read.rs +++ b/src/read.rs @@ -10,9 +10,10 @@ //! A wrapper around any Read to treat it as an RNG. -use std::io::{self, Read}; -use std::mem; -use Rng; +use std::io::Read; + +use {Rng, Error, ErrorKind, impls}; + /// An RNG that reads random bytes straight from a `Read`. This will /// work best with an infinite reader, but this is not required. @@ -46,34 +47,24 @@ impl ReadRng { impl Rng for ReadRng { fn next_u32(&mut self) -> u32 { - // This is designed for speed: reading a LE integer on a LE - // platform just involves blitting the bytes into the memory - // of the u32, similarly for BE on BE; avoiding byteswapping. - let mut buf = [0; 4]; - fill(&mut self.reader, &mut buf).unwrap(); - unsafe { *(buf.as_ptr() as *const u32) } + impls::next_u32_via_fill(self) } + fn next_u64(&mut self) -> u64 { - // see above for explanation. - let mut buf = [0; 8]; - fill(&mut self.reader, &mut buf).unwrap(); - unsafe { *(buf.as_ptr() as *const u64) } + impls::next_u64_via_fill(self) } - fn fill_bytes(&mut self, v: &mut [u8]) { - if v.len() == 0 { return } - fill(&mut self.reader, v).unwrap(); + + fn fill_bytes(&mut self, dest: &mut [u8]) { + self.try_fill_bytes(dest).unwrap(); } -} -fn fill(r: &mut Read, mut buf: &mut [u8]) -> io::Result<()> { - while buf.len() > 0 { - match try!(r.read(buf)) { - 0 => return Err(io::Error::new(io::ErrorKind::Other, - "end of file reached")), - n => buf = &mut mem::replace(&mut buf, &mut [])[n..], - } + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { + if dest.len() == 0 { return Ok(()); } + // Use `std::io::read_exact`, which retries on `ErrorKind::Interrupted`. + self.reader.read_exact(dest).map_err(|err| { + Error::with_cause(ErrorKind::Unavailable, "ReadRng: read error", err) + }) } - Ok(()) } #[cfg(test)] diff --git a/src/reseeding.rs b/src/reseeding.rs index 93bc32523fb..77cf0cbd437 100644 --- a/src/reseeding.rs +++ b/src/reseeding.rs @@ -13,7 +13,7 @@ use core::default::Default; -use {Rng, SeedableRng}; +use {Rng, SeedableRng, Error}; /// How many bytes of entropy the underling RNG is allowed to generate /// before it is reseeded @@ -76,6 +76,12 @@ impl> Rng for ReseedingRng { self.bytes_generated += dest.len() as u64; self.rng.fill_bytes(dest) } + + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { + self.reseed_if_necessary(); + self.bytes_generated += dest.len() as u64; + self.rng.try_fill_bytes(dest) + } } impl, Rsdr: Reseeder + Default> From fed7943c45d73343a4094ea8cc9f339362e011c5 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Mon, 8 Jan 2018 12:35:37 +0000 Subject: [PATCH 4/5] OsRng: merge in changes from dhardy/master: better error handling, -nacl This removes support for the NaCl target, which appears to be dead. --- src/impls.rs | 36 +++++ src/lib.rs | 3 +- src/os.rs | 365 +++++++++++++++++++-------------------------------- 3 files changed, 170 insertions(+), 234 deletions(-) diff --git a/src/impls.rs b/src/impls.rs index 34c17786c1f..6e005965391 100644 --- a/src/impls.rs +++ b/src/impls.rs @@ -167,4 +167,40 @@ pub fn next_u64_via_fill(rng: &mut R) -> u64 { impl_uint_from_fill!(rng, u64, 8) } +/// Implement `fill_bytes` via `try_fill` with implicit error handling. +pub fn fill_via_try_fill(rng: &mut R, dest: &mut [u8]) { + const WAIT_DUR_MS: u32 = 100; + const MAX_WAIT: u32 = (1 * 60 * 1000) / WAIT_DUR_MS; + const TRANSIENT_STEP: u32 = MAX_WAIT / 8; + let mut err_count = 0; + + loop { + if let Err(e) = rng.try_fill_bytes(dest) { + if e.kind().should_retry() { + if err_count > MAX_WAIT { + // TODO: log details & cause? + panic!("Too many RNG errors or timeout; last error: {}", e.msg()); + } + + if e.kind().should_wait() { + #[cfg(feature="std")]{ + let dur = ::std::time::Duration::from_millis(WAIT_DUR_MS as u64); + ::std::thread::sleep(dur); + } + err_count += 1; + } else { + err_count += TRANSIENT_STEP; + } + + continue; + } + + // TODO: log details & cause? + panic!("Fatal RNG error: {}", e.msg()); + } + + break; + } +} + // TODO: implement tests for the above diff --git a/src/lib.rs b/src/lib.rs index fb272dbe70a..75978e5512d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -253,7 +253,6 @@ use core::marker; use core::mem; #[cfg(feature="std")] use std::cell::RefCell; -#[cfg(feature="std")] use std::io; #[cfg(feature="std")] use std::rc::Rc; // external rngs @@ -817,7 +816,7 @@ impl StdRng { /// Reading the randomness from the OS may fail, and any error is /// propagated via the `io::Result` return value. #[cfg(feature="std")] - pub fn new() -> io::Result { + pub fn new() -> Result { match OsRng::new() { Ok(mut r) => Ok(StdRng { rng: r.gen() }), Err(e1) => { diff --git a/src/os.rs b/src/os.rs index f49f2a70835..5e0c28f3a21 100644 --- a/src/os.rs +++ b/src/os.rs @@ -11,11 +11,15 @@ //! Interfaces to the operating system provided random number //! generators. -use std::{io, mem, fmt}; -use {Rng, Error}; +use std::fmt; +use std::io::Read; + +use {Rng, Error, ErrorKind, impls}; /// A random number generator that retrieves randomness straight from -/// the operating system. Platform sources: +/// the operating system. +/// +/// Platform sources: /// /// - Unix-like systems (Linux, Android, Mac OSX): read directly from /// `/dev/urandom`, or from `getrandom(2)` system call if available. @@ -24,7 +28,6 @@ use {Rng, Error}; /// - Windows: calls `RtlGenRandom`, exported from `advapi32.dll` as /// `SystemFunction036`. /// - iOS: calls SecRandomCopyBytes as /dev/(u)random is sandboxed. -/// - PNaCl: calls into the `nacl-irt-random-0.1` IRT interface. /// /// This usually does not block. On some systems (e.g. FreeBSD, OpenBSD, /// Max OS X, and modern Linux) this may block very early in the init @@ -32,41 +35,53 @@ use {Rng, Error}; /// /// [1] See for a more /// in-depth discussion. +#[allow(unused)] // not used by all targets pub struct OsRng(imp::OsRng); +impl fmt::Debug for OsRng { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.0.fmt(f) + } +} + impl OsRng { /// Create a new `OsRng`. - pub fn new() -> io::Result { + pub fn new() -> Result { imp::OsRng::new().map(OsRng) } } impl Rng for OsRng { - fn next_u32(&mut self) -> u32 { self.0.next_u32() } - fn next_u64(&mut self) -> u64 { self.0.next_u64() } - fn fill_bytes(&mut self, v: &mut [u8]) { self.0.fill_bytes(v) } - fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { - // TODO: error handling per variant - Ok(self.0.fill_bytes(dest)) + fn next_u32(&mut self) -> u32 { + impls::next_u32_via_fill(self) } -} -impl fmt::Debug for OsRng { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "OsRng {{}}") + fn next_u64(&mut self) -> u64 { + impls::next_u64_via_fill(self) } -} -fn next_u32(fill_buf: &mut FnMut(&mut [u8])) -> u32 { - let mut buf: [u8; 4] = [0; 4]; - fill_buf(&mut buf); - unsafe { mem::transmute::<[u8; 4], u32>(buf) } + fn fill_bytes(&mut self, dest: &mut [u8]) { + impls::fill_via_try_fill(self, dest) + } + + fn try_fill_bytes(&mut self, v: &mut [u8]) -> Result<(), Error> { + self.0.try_fill_bytes(v) + } } -fn next_u64(fill_buf: &mut FnMut(&mut [u8])) -> u64 { - let mut buf: [u8; 8] = [0; 8]; - fill_buf(&mut buf); - unsafe { mem::transmute::<[u8; 8], u64>(buf) } +// Specialisation of `ReadRng` for our purposes +#[derive(Debug)] +struct ReadRng (R); + +impl ReadRng { + #[allow(unused)] // not used by all targets + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { + if dest.len() == 0 { return Ok(()); } + // Use `std::io::read_exact`, which retries on `ErrorKind::Interrupted`. + self.0.read_exact(dest).map_err(|err| { + Error::with_cause(ErrorKind::Unavailable, "error reading random device", err) + }) + } } #[cfg(all(unix, not(target_os = "ios"), @@ -78,13 +93,12 @@ fn next_u64(fill_buf: &mut FnMut(&mut [u8])) -> u64 { mod imp { extern crate libc; - use super::{next_u32, next_u64}; use self::OsRngInner::*; + use super::ReadRng; + use {Error, ErrorKind}; use std::io; use std::fs::File; - use Rng; - use read::ReadRng; #[cfg(all(target_os = "linux", any(target_arch = "x86_64", @@ -107,9 +121,11 @@ mod imp { const NR_GETRANDOM: libc::c_long = 278; #[cfg(target_arch = "powerpc")] const NR_GETRANDOM: libc::c_long = 384; + + const GRND_NONBLOCK: libc::c_uint = 0x0001; unsafe { - syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(), 0) + syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(), GRND_NONBLOCK) } } @@ -121,22 +137,33 @@ mod imp { target_arch = "powerpc"))))] fn getrandom(_buf: &mut [u8]) -> libc::c_long { -1 } - fn getrandom_fill_bytes(v: &mut [u8]) { + fn getrandom_try_fill(v: &mut [u8]) -> Result<(), Error> { let mut read = 0; let len = v.len(); while read < len { let result = getrandom(&mut v[read..]); if result == -1 { let err = io::Error::last_os_error(); - if err.kind() == io::ErrorKind::Interrupted { - continue + let kind = err.kind(); + if kind == io::ErrorKind::Interrupted { + continue; + } else if kind == io::ErrorKind::WouldBlock { + // Potentially this would waste bytes, but since we use + // /dev/urandom blocking only happens if not initialised. + // Also, wasting the bytes in v doesn't matter very much. + return Err(Error::new(ErrorKind::NotReady, "getrandom not ready")); } else { - panic!("unexpected getrandom error: {}", err); + return Err(Error::with_cause( + ErrorKind::Unavailable, + "unexpected getrandom error", + err, + )); } } else { read += result as usize; } } + Ok(()) } #[cfg(all(target_os = "linux", @@ -175,45 +202,35 @@ mod imp { target_arch = "powerpc"))))] fn is_getrandom_available() -> bool { false } + #[derive(Debug)] pub struct OsRng { inner: OsRngInner, } + #[derive(Debug)] enum OsRngInner { OsGetrandomRng, OsReadRng(ReadRng), } impl OsRng { - pub fn new() -> io::Result { + pub fn new() -> Result { if is_getrandom_available() { return Ok(OsRng { inner: OsGetrandomRng }); } - let reader = try!(File::open("/dev/urandom")); - let reader_rng = ReadRng::new(reader); + let reader = File::open("/dev/urandom").map_err(|err| { + Error::with_cause(ErrorKind::Unavailable, "error opening random device", err) + })?; + let reader_rng = ReadRng(reader); Ok(OsRng { inner: OsReadRng(reader_rng) }) } - } - - impl Rng for OsRng { - fn next_u32(&mut self) -> u32 { + + pub fn try_fill_bytes(&mut self, v: &mut [u8]) -> Result<(), Error> { match self.inner { - OsGetrandomRng => next_u32(&mut getrandom_fill_bytes), - OsReadRng(ref mut rng) => rng.next_u32(), - } - } - fn next_u64(&mut self) -> u64 { - match self.inner { - OsGetrandomRng => next_u64(&mut getrandom_fill_bytes), - OsReadRng(ref mut rng) => rng.next_u64(), - } - } - fn fill_bytes(&mut self, v: &mut [u8]) { - match self.inner { - OsGetrandomRng => getrandom_fill_bytes(v), - OsReadRng(ref mut rng) => rng.fill_bytes(v) + OsGetrandomRng => getrandom_try_fill(v), + OsReadRng(ref mut rng) => rng.try_fill_bytes(v) } } } @@ -223,10 +240,9 @@ mod imp { mod imp { extern crate libc; - use super::{next_u32, next_u64}; - + use {Error, ErrorKind}; + use std::io; - use Rng; use self::libc::{c_int, size_t}; #[derive(Debug)] @@ -244,24 +260,20 @@ mod imp { } impl OsRng { - pub fn new() -> io::Result { + pub fn new() -> Result { Ok(OsRng) } - } - - impl Rng for OsRng { - fn next_u32(&mut self) -> u32 { - next_u32(&mut |v| self.fill_bytes(v)) - } - fn next_u64(&mut self) -> u64 { - next_u64(&mut |v| self.fill_bytes(v)) - } - fn fill_bytes(&mut self, v: &mut [u8]) { + pub fn try_fill_bytes(&mut self, v: &mut [u8]) -> Result<(), Error> { let ret = unsafe { SecRandomCopyBytes(kSecRandomDefault, v.len() as size_t, v.as_mut_ptr()) }; if ret == -1 { - panic!("couldn't generate random bytes: {}", io::Error::last_os_error()); + Err(Error::with_cause( + ErrorKind::Unavailable, + "couldn't generate random bytes", + io::Error::last_os_error())) + } else { + Ok(()) } } } @@ -271,28 +283,18 @@ mod imp { mod imp { extern crate libc; - use std::{io, ptr}; - use Rng; - - use super::{next_u32, next_u64}; + use {Error, ErrorKind}; + + use std::ptr; #[derive(Debug)] pub struct OsRng; impl OsRng { - pub fn new() -> io::Result { + pub fn new() -> Result { Ok(OsRng) } - } - - impl Rng for OsRng { - fn next_u32(&mut self) -> u32 { - next_u32(&mut |v| self.fill_bytes(v)) - } - fn next_u64(&mut self) -> u64 { - next_u64(&mut |v| self.fill_bytes(v)) - } - fn fill_bytes(&mut self, v: &mut [u8]) { + pub fn try_fill_bytes(&mut self, v: &mut [u8]) -> Result<(), Error> { let mib = [libc::CTL_KERN, libc::KERN_ARND]; // kern.arandom permits a maximum buffer size of 256 bytes for s in v.chunks_mut(256) { @@ -303,10 +305,12 @@ mod imp { ptr::null(), 0) }; if ret == -1 || s_len != s.len() { - panic!("kern.arandom sysctl failed! (returned {}, s.len() {}, oldlenp {})", - ret, s.len(), s_len); + return Err(Error::new( + ErrorKind::Unavailable, + "kern.arandom sysctl failed")); } } + Ok(()) } } } @@ -315,48 +319,42 @@ mod imp { mod imp { extern crate libc; + use {Error, ErrorKind}; + use std::io; - use Rng; - - use super::{next_u32, next_u64}; #[derive(Debug)] pub struct OsRng; impl OsRng { - pub fn new() -> io::Result { + pub fn new() -> Result { Ok(OsRng) } - } - - impl Rng for OsRng { - fn next_u32(&mut self) -> u32 { - next_u32(&mut |v| self.fill_bytes(v)) - } - fn next_u64(&mut self) -> u64 { - next_u64(&mut |v| self.fill_bytes(v)) - } - fn fill_bytes(&mut self, v: &mut [u8]) { + pub fn try_fill_bytes(&mut self, v: &mut [u8]) -> Result<(), Error> { // getentropy(2) permits a maximum buffer size of 256 bytes for s in v.chunks_mut(256) { let ret = unsafe { libc::getentropy(s.as_mut_ptr() as *mut libc::c_void, s.len()) }; if ret == -1 { - let err = io::Error::last_os_error(); - panic!("getentropy failed: {}", err); + return Err(Error::with_cause( + ErrorKind::Unavailable, + "getentropy failed", + io::Error::last_os_error())); } } + Ok(()) } } } #[cfg(target_os = "redox")] mod imp { + use {Error, ErrorKind}; + use std::io; use std::fs::File; - use Rng; - use read::ReadRng; + use super::ReadRng; #[derive(Debug)] pub struct OsRng { @@ -364,23 +362,16 @@ mod imp { } impl OsRng { - pub fn new() -> io::Result { - let reader = try!(File::open("rand:")); - let reader_rng = ReadRng::new(reader); + pub fn new() -> Result { + let reader = File::open("rand:").map_err(|err| { + Error::with_cause(ErrorKind::Unavailable, "error opening random device", err) + })?; + let reader_rng = ReadRng(reader); Ok(OsRng { inner: reader_rng }) } - } - - impl Rng for OsRng { - fn next_u32(&mut self) -> u32 { - self.inner.next_u32() - } - fn next_u64(&mut self) -> u64 { - self.inner.next_u64() - } - fn fill_bytes(&mut self, v: &mut [u8]) { - self.inner.fill_bytes(v) + pub fn try_fill_bytes(&mut self, v: &mut [u8]) -> Result<(), Error> { + self.inner.try_fill_bytes(v) } } } @@ -389,37 +380,33 @@ mod imp { mod imp { extern crate fuchsia_zircon; + use {Error, ErrorKind}; + use std::io; - use Rng; - - use super::{next_u32, next_u64}; #[derive(Debug)] pub struct OsRng; impl OsRng { - pub fn new() -> io::Result { + pub fn new() -> Result { Ok(OsRng) } - } - - impl Rng for OsRng { - fn next_u32(&mut self) -> u32 { - next_u32(&mut |v| self.fill_bytes(v)) - } - fn next_u64(&mut self) -> u64 { - next_u64(&mut |v| self.fill_bytes(v)) - } - fn fill_bytes(&mut self, v: &mut [u8]) { + pub fn try_fill_bytes(&mut self, v: &mut [u8]) -> Result<(), Error> { for s in v.chunks_mut(fuchsia_zircon::sys::ZX_CPRNG_DRAW_MAX_LEN) { let mut filled = 0; while filled < s.len() { match fuchsia_zircon::cprng_draw(&mut s[filled..]) { Ok(actual) => filled += actual, - Err(e) => panic!("cprng_draw failed: {:?}", e), + Err(e) => { + return Err(Error::with_cause( + ErrorKind::Unavailable, + "cprng_draw failed", + e)); + } }; } } + Ok(()) } } } @@ -427,11 +414,10 @@ mod imp { #[cfg(windows)] mod imp { extern crate winapi; - + + use {Error, ErrorKind}; + use std::io; - use Rng; - - use super::{next_u32, next_u64}; use self::winapi::shared::minwindef::ULONG; use self::winapi::um::ntsecapi::RtlGenRandom; @@ -441,19 +427,10 @@ mod imp { pub struct OsRng; impl OsRng { - pub fn new() -> io::Result { + pub fn new() -> Result { Ok(OsRng) } - } - - impl Rng for OsRng { - fn next_u32(&mut self) -> u32 { - next_u32(&mut |v| self.fill_bytes(v)) - } - fn next_u64(&mut self) -> u64 { - next_u64(&mut |v| self.fill_bytes(v)) - } - fn fill_bytes(&mut self, v: &mut [u8]) { + pub fn try_fill_bytes(&mut self, v: &mut [u8]) -> Result<(), Error> { // RtlGenRandom takes an ULONG (u32) for the length so we need to // split up the buffer. for slice in v.chunks_mut(::max_value() as usize) { @@ -461,86 +438,13 @@ mod imp { RtlGenRandom(slice.as_mut_ptr() as PVOID, slice.len() as ULONG) }; if ret == 0 { - panic!("couldn't generate random bytes: {}", - io::Error::last_os_error()); + return Err(Error::with_cause( + ErrorKind::Unavailable, + "couldn't generate random bytes", + io::Error::last_os_error())); } } - } - } -} - -#[cfg(target_os = "nacl")] -mod imp { - extern crate libc; - - use std::io; - use std::mem; - use Rng; - - use super::{next_u32, next_u64}; - - #[derive(Debug)] - pub struct OsRng(extern fn(dest: *mut libc::c_void, - bytes: libc::size_t, - read: *mut libc::size_t) -> libc::c_int); - - extern { - fn nacl_interface_query(name: *const libc::c_char, - table: *mut libc::c_void, - table_size: libc::size_t) -> libc::size_t; - } - - const INTERFACE: &'static [u8] = b"nacl-irt-random-0.1\0"; - - #[repr(C)] - struct NaClIRTRandom { - get_random_bytes: Option libc::c_int>, - } - - impl OsRng { - pub fn new() -> io::Result { - let mut iface = NaClIRTRandom { - get_random_bytes: None, - }; - let result = unsafe { - nacl_interface_query(INTERFACE.as_ptr() as *const _, - mem::transmute(&mut iface), - mem::size_of::() as libc::size_t) - }; - if result != 0 { - assert!(iface.get_random_bytes.is_some()); - let result = OsRng(iface.get_random_bytes.take().unwrap()); - Ok(result) - } else { - let error = io::ErrorKind::NotFound; - let error = io::Error::new(error, "IRT random interface missing"); - Err(error) - } - } - } - - impl Rng for OsRng { - fn next_u32(&mut self) -> u32 { - next_u32(&mut |v| self.fill_bytes(v)) - } - fn next_u64(&mut self) -> u64 { - next_u64(&mut |v| self.fill_bytes(v)) - } - fn fill_bytes(&mut self, v: &mut [u8]) { - let mut read = 0; - loop { - let mut r: libc::size_t = 0; - let len = v.len(); - let error = (self.0)(v[read..].as_mut_ptr() as *mut _, - (len - read) as libc::size_t, - &mut r as *mut _); - assert!(error == 0, "`get_random_bytes` failed!"); - read += r as usize; - - if read >= v.len() { break; } - } + Ok(()) } } } @@ -554,14 +458,11 @@ mod imp { pub struct OsRng; impl OsRng { - pub fn new() -> io::Result { - Err(io::Error::new(io::ErrorKind::Other, "Not supported")) + pub fn new() -> Result { + Err(Error::new(ErrorKind::Unavailable, "not supported on WASM")) } - } - - impl Rng for OsRng { - fn next_u32(&mut self) -> u32 { - panic!("Not supported") + pub fn try_fill_bytes(&mut self, v: &mut [u8]) -> Result<(), Error> { + Err(Error::new(ErrorKind::Unavailable, "not supported on WASM")) } } } From c9bfa3a1d34f2957b16a3c0ed2cf8d8bc82cbf10 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Tue, 9 Jan 2018 12:05:53 +0000 Subject: [PATCH 5/5] Inline fill_via_try_fill implicit error handling, and revise --- src/impls.rs | 36 ------------------------------------ src/os.rs | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 37 deletions(-) diff --git a/src/impls.rs b/src/impls.rs index 6e005965391..34c17786c1f 100644 --- a/src/impls.rs +++ b/src/impls.rs @@ -167,40 +167,4 @@ pub fn next_u64_via_fill(rng: &mut R) -> u64 { impl_uint_from_fill!(rng, u64, 8) } -/// Implement `fill_bytes` via `try_fill` with implicit error handling. -pub fn fill_via_try_fill(rng: &mut R, dest: &mut [u8]) { - const WAIT_DUR_MS: u32 = 100; - const MAX_WAIT: u32 = (1 * 60 * 1000) / WAIT_DUR_MS; - const TRANSIENT_STEP: u32 = MAX_WAIT / 8; - let mut err_count = 0; - - loop { - if let Err(e) = rng.try_fill_bytes(dest) { - if e.kind().should_retry() { - if err_count > MAX_WAIT { - // TODO: log details & cause? - panic!("Too many RNG errors or timeout; last error: {}", e.msg()); - } - - if e.kind().should_wait() { - #[cfg(feature="std")]{ - let dur = ::std::time::Duration::from_millis(WAIT_DUR_MS as u64); - ::std::thread::sleep(dur); - } - err_count += 1; - } else { - err_count += TRANSIENT_STEP; - } - - continue; - } - - // TODO: log details & cause? - panic!("Fatal RNG error: {}", e.msg()); - } - - break; - } -} - // TODO: implement tests for the above diff --git a/src/os.rs b/src/os.rs index 5e0c28f3a21..248e980acb9 100644 --- a/src/os.rs +++ b/src/os.rs @@ -61,7 +61,38 @@ impl Rng for OsRng { } fn fill_bytes(&mut self, dest: &mut [u8]) { - impls::fill_via_try_fill(self, dest) + // We cannot return Err(..), so we try to handle before panicking. + const WAIT_DUR_MS: u32 = 100; // retry every 100ms + const MAX_WAIT: u32 = (10 * 1000) / WAIT_DUR_MS; // max 10s + const TRANSIENT_STEP: u32 = MAX_WAIT / 8; + let mut err_count = 0; + + loop { + if let Err(e) = self.try_fill_bytes(dest) { + // TODO: add logging to explain why we wait and the full cause + if e.kind().should_retry() { + if err_count > MAX_WAIT { + panic!("Too many RNG errors or timeout; last error: {}", e.msg()); + } + + if e.kind().should_wait() { + #[cfg(feature="std")]{ + let dur = ::std::time::Duration::from_millis(WAIT_DUR_MS as u64); + ::std::thread::sleep(dur); + } + err_count += 1; + } else { + err_count += TRANSIENT_STEP; + } + + continue; + } + + panic!("Fatal RNG error: {}", e.msg()); + } + + break; + } } fn try_fill_bytes(&mut self, v: &mut [u8]) -> Result<(), Error> {