From b5b295e414a0ee859750b9800cd912a7568530e5 Mon Sep 17 00:00:00 2001 From: Without Boats Date: Tue, 5 Dec 2017 18:13:43 -0800 Subject: [PATCH 1/3] Use a custom error type instead of &'static str. Advantages of a custom error type: - It can be more easily integrated into other error types by clients; they can implement From for their error types, or they can use a library like failure. - It is a zero-sized type, which can enable some representational optimizations. - It can be easier and more stable to test for. --- src/ed25519.rs | 86 +++++++++++++++++++++++++++++++++----------------- src/lib.rs | 6 ++-- 2 files changed, 60 insertions(+), 32 deletions(-) diff --git a/src/ed25519.rs b/src/ed25519.rs index 6dd4099..b790d91 100644 --- a/src/ed25519.rs +++ b/src/ed25519.rs @@ -10,7 +10,7 @@ //! A Rust implementation of ed25519 EdDSA key generation, signing, and //! verification. -use core::fmt::Debug; +use core::fmt::{self, Debug, Display}; #[cfg(feature = "std")] use rand::Rng; @@ -123,10 +123,8 @@ impl Signature { /// Construct a `Signature` from a slice of bytes. #[inline] - pub fn from_bytes(bytes: &[u8]) -> Result { - if bytes.len() != SIGNATURE_LENGTH { - return Err("Wrong length of bytes for signature! Need 64 bytes.") - } + pub fn from_bytes(bytes: &[u8]) -> Result { + check_bytes_len(bytes, SIGNATURE_LENGTH)?; let lower: &[u8; 32] = array_ref!(bytes, 0, 32); let upper: &[u8; 32] = array_ref!(bytes, 32, 32); @@ -199,8 +197,9 @@ impl SecretKey { /// # /// use ed25519_dalek::SecretKey; /// use ed25519_dalek::SECRET_KEY_LENGTH; + /// use ed25519_dalek::FromBytesError; /// - /// # fn doctest() -> Result { + /// # fn doctest() -> Result { /// let secret_key_bytes: [u8; SECRET_KEY_LENGTH] = [ /// 157, 097, 177, 157, 239, 253, 090, 096, /// 186, 132, 074, 244, 146, 236, 044, 196, @@ -221,12 +220,11 @@ impl SecretKey { /// # Returns /// /// A `Result` whose okay value is an EdDSA `SecretKey` or whose error value - /// is an `&'static str` describing the error that occurred. + /// is an `FromBytesError` describing the error that occurred. #[inline] - pub fn from_bytes(bytes: &[u8]) -> Result { - if bytes.len() != SECRET_KEY_LENGTH { - return Err("Wrong length of bytes for creating secret key!"); - } + pub fn from_bytes(bytes: &[u8]) -> Result { + check_bytes_len(bytes, SECRET_KEY_LENGTH)?; + Ok(SecretKey(*array_ref!(bytes, 0, SECRET_KEY_LENGTH))) } @@ -441,7 +439,7 @@ impl ExpandedSecretKey { /// # Returns /// /// A `Result` whose okay value is an EdDSA `ExpandedSecretKey` or whose - /// error value is an `&'static str` describing the error that occurred. + /// error value is an `FromBytesError` describing the error that occurred. /// /// # Examples /// @@ -452,9 +450,10 @@ impl ExpandedSecretKey { /// # /// use rand::{Rng, OsRng}; /// use ed25519_dalek::{SecretKey, ExpandedSecretKey}; + /// use ed25519_dalek::FromBytesError; /// /// # #[cfg(feature = "sha2")] - /// # fn do_test() -> Result { + /// # fn do_test() -> Result { /// # /// let mut csprng: OsRng = OsRng::new().unwrap(); /// let secret_key: SecretKey = SecretKey::generate(&mut csprng); @@ -475,10 +474,9 @@ impl ExpandedSecretKey { /// # fn main() {} /// ``` #[inline] - pub fn from_bytes(bytes: &[u8]) -> Result { - if bytes.len() != 64 { - return Err("Wrong length of bytes for creating expanded secret key!"); - } + pub fn from_bytes(bytes: &[u8]) -> Result { + check_bytes_len(bytes, 64)?; + Ok(ExpandedSecretKey{ key: Scalar(*array_ref!(bytes, 0, 32)), nonce: *array_ref!(bytes, 32, 32), }) } @@ -622,8 +620,9 @@ impl PublicKey { /// # /// use ed25519_dalek::PublicKey; /// use ed25519_dalek::PUBLIC_KEY_LENGTH; + /// use ed25519_dalek::FromBytesError; /// - /// # fn doctest() -> Result { + /// # fn doctest() -> Result { /// let public_key_bytes: [u8; PUBLIC_KEY_LENGTH] = [ /// 215, 90, 152, 1, 130, 177, 10, 183, 213, 75, 254, 211, 201, 100, 7, 58, /// 14, 225, 114, 243, 218, 166, 35, 37, 175, 2, 26, 104, 247, 7, 81, 26]; @@ -641,12 +640,11 @@ impl PublicKey { /// # Returns /// /// A `Result` whose okay value is an EdDSA `PublicKey` or whose error value - /// is an `&'static str` describing the error that occurred. + /// is an `FromBytesError` describing the error that occurred. #[inline] - pub fn from_bytes(bytes: &[u8]) -> Result { - if bytes.len() != PUBLIC_KEY_LENGTH { - return Err("Wrong length of bytes for creating public key!"); - } + pub fn from_bytes(bytes: &[u8]) -> Result { + check_bytes_len(bytes, PUBLIC_KEY_LENGTH)?; + Ok(PublicKey(CompressedEdwardsY(*array_ref!(bytes, 0, 32)))) } @@ -796,11 +794,10 @@ impl Keypair { /// # Returns /// /// A `Result` whose okay value is an EdDSA `Keypair` or whose error value - /// is an `&'static str` describing the error that occurred. - pub fn from_bytes<'a>(bytes: &'a [u8]) -> Result { - if bytes.len() != KEYPAIR_LENGTH { - return Err("Wrong length of bytes for creating keypair!"); - } + /// is an `FromBytesError` describing the error that occurred. + pub fn from_bytes<'a>(bytes: &'a [u8]) -> Result { + check_bytes_len(bytes, KEYPAIR_LENGTH)?; + let secret = SecretKey::from_bytes(&bytes[..SECRET_KEY_LENGTH])?; let public = PublicKey::from_bytes(&bytes[SECRET_KEY_LENGTH..])?; @@ -896,6 +893,37 @@ impl<'d> Deserialize<'d> for Keypair { } } +/// An error which occurred when using the `from_bytes` constructor. +/// +/// This error will be returned if the byte slice given was not the correct +/// length for constructing that kind of object. +#[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)] +pub struct FromBytesError { + _private: (), +} + +impl Display for FromBytesError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "wrong length of bytes when constructing ed25519 object") + } +} + +#[cfg(feature = "std")] +impl ::std::error::Error for FromBytesError { + fn description(&self) -> &str { + "wrong length of bytes when constructing ed25519 object" + } +} + +#[inline(always)] +fn check_bytes_len(bytes: &[u8], len: usize) -> Result<(), FromBytesError> { + if bytes.len() != len { + Err(FromBytesError { _private: () }) + } else { + Ok(()) + } +} + #[cfg(test)] mod test { use std::io::BufReader; @@ -1032,7 +1060,7 @@ mod test { #[test] fn public_key_from_bytes() { // Make another function so that we can test the ? operator. - fn do_the_test() -> Result { + fn do_the_test() -> Result { let public_key_bytes: [u8; PUBLIC_KEY_LENGTH] = [ 215, 090, 152, 001, 130, 177, 010, 183, 213, 075, 254, 211, 201, 100, 007, 058, diff --git a/src/lib.rs b/src/lib.rs index 78ec572..d862d23 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -143,9 +143,9 @@ //! # extern crate ed25519_dalek; //! # use rand::{Rng, OsRng}; //! # use sha2::Sha512; -//! # use ed25519_dalek::{Keypair, Signature, PublicKey, SecretKey}; +//! # use ed25519_dalek::{Keypair, Signature, PublicKey, SecretKey, FromBytesError}; //! # use ed25519_dalek::{PUBLIC_KEY_LENGTH, SECRET_KEY_LENGTH, KEYPAIR_LENGTH, SIGNATURE_LENGTH}; -//! # fn do_test() -> Result<(SecretKey, PublicKey, Keypair, Signature), &'static str> { +//! # fn do_test() -> Result<(SecretKey, PublicKey, Keypair, Signature), FromBytesError> { //! # let mut cspring: OsRng = OsRng::new().unwrap(); //! # let keypair_orig: Keypair = Keypair::generate::(&mut cspring); //! # let message: &[u8] = "This is a test of the tsunami alert system.".as_bytes(); @@ -267,7 +267,7 @@ extern crate subtle; #[cfg(feature = "std")] extern crate rand; -#[cfg(test)] +#[cfg(any(feature = "std", test))] #[macro_use] extern crate std; From 96246506c07dd9ee1017ed7a222258f1c33d6ca7 Mon Sep 17 00:00:00 2001 From: Without Boats Date: Tue, 5 Dec 2017 18:18:34 -0800 Subject: [PATCH 2/3] This is a breaking change. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 5402d2a..1de19d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ed25519-dalek" -version = "0.5.0" +version = "0.6.0" authors = ["Isis Lovecruft "] readme = "README.md" license = "BSD-3-Clause" From 6c1acaca7c40877af5eca3c2eb191821baf0ab45 Mon Sep 17 00:00:00 2001 From: Without Boats Date: Wed, 6 Dec 2017 15:25:12 -0800 Subject: [PATCH 3/3] Use failure instead of std::error::Error. failure is no_std compatible, whereas std::error::Error is not. --- Cargo.toml | 6 +++++- src/ed25519.rs | 7 +------ src/lib.rs | 1 + 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1de19d7..c64887b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,10 @@ optional = true version = "^0.6" optional = true +[dependencies.failure] +version = "^0.1.1" +default-features = false + [dev-dependencies] hex = "0.2" sha2 = "^0.6" @@ -52,7 +56,7 @@ bincode = "^0.9" [features] default = ["std"] -std = ["rand", "curve25519-dalek/std"] +std = ["rand", "curve25519-dalek/std", "failure/std"] bench = [] nightly = ["curve25519-dalek/nightly"] asm = ["sha2/asm"] diff --git a/src/ed25519.rs b/src/ed25519.rs index b790d91..da8c2cb 100644 --- a/src/ed25519.rs +++ b/src/ed25519.rs @@ -908,12 +908,7 @@ impl Display for FromBytesError { } } -#[cfg(feature = "std")] -impl ::std::error::Error for FromBytesError { - fn description(&self) -> &str { - "wrong length of bytes when constructing ed25519 object" - } -} +impl ::failure::Fail for FromBytesError { } #[inline(always)] fn check_bytes_len(bytes: &[u8], len: usize) -> Result<(), FromBytesError> { diff --git a/src/lib.rs b/src/lib.rs index d862d23..4d20d9e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -263,6 +263,7 @@ extern crate curve25519_dalek; extern crate generic_array; extern crate digest; extern crate subtle; +extern crate failure; #[cfg(feature = "std")] extern crate rand;