From 030eff547f918e99e17d0d9e81692cb244208d0c Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Fri, 13 Jul 2018 12:39:45 -0700 Subject: [PATCH] Make `verify` return a `Result`. This also simplifies the verification logic. Because the verification check happens in variable time, we don't need to do a constant-time eq check at the end, so we can drop the `subtle` dependency entirely. The `DecodingError` type becomes `SignatureError` and is also used to signal failing verifications. --- Cargo.toml | 8 +-- README.md | 9 +-- src/ed25519.rs | 172 ++++++++++++++++++------------------------------- src/errors.rs | 36 +++++------ src/lib.rs | 14 ++-- 5 files changed, 86 insertions(+), 153 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b22e552..3da42c9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,10 +19,6 @@ travis-ci = { repository = "dalek-cryptography/ed25519-dalek", branch = "master" version = "0.18" default-features = false -[dependencies.subtle] -version = "0.6" -default-features = false - [dependencies.rand] version = "0.5" default-features = false @@ -59,8 +55,8 @@ harness = false [features] default = ["std", "u64_backend"] # We don't add "rand/std" here because it would enable a bunch of Fuchsia dependencies. -std = ["subtle/std", "curve25519-dalek/std"] -nightly = ["curve25519-dalek/nightly", "subtle/nightly", "rand/nightly"] +std = ["curve25519-dalek/std"] +nightly = ["curve25519-dalek/nightly", "rand/nightly"] asm = ["sha2/asm"] yolocrypto = ["curve25519-dalek/yolocrypto"] u64_backend = ["curve25519-dalek/u64_backend"] diff --git a/README.md b/README.md index 342e395..037eeb6 100644 --- a/README.md +++ b/README.md @@ -55,12 +55,9 @@ nanoseconds to cycles per second on a 2591 Mhz CPU, that's 237660 cycles for verification and 102523 for signing, which for signing is competitive with optimised assembly versions. -Additionally, if you're on the Rust nightly channel, be sure to build with -`cargo build --features="nightly"` which enables more secure compiler -optimisation protections in the -[subtle](https://github.com/dalek-cryptography/subtle) crate. Additionally, if -you're using a CSPRNG from the `rand` crate, the `nightly` feature will enable -`u128`/`i128` features there, resulting in potentially faster performance. +Additionally, if you're using a CSPRNG from the `rand` crate, the `nightly` +feature will enable `u128`/`i128` features there, resulting in potentially +faster performance. Additionally, thanks to Rust, this implementation has both type and memory safety. It's also easily readable by a much larger set of people than those who diff --git a/src/ed25519.rs b/src/ed25519.rs index 13e2133..3bd43ac 100644 --- a/src/ed25519.rs +++ b/src/ed25519.rs @@ -36,9 +36,7 @@ use curve25519_dalek::edwards::CompressedEdwardsY; use curve25519_dalek::edwards::EdwardsPoint; use curve25519_dalek::scalar::Scalar; -use subtle::ConstantTimeEq; - -use errors::DecodingError; +use errors::SignatureError; use errors::InternalError; /// The length of a curve25519 EdDSA `Signature`, in bytes. @@ -132,9 +130,9 @@ impl Signature { /// Construct a `Signature` from a slice of bytes. #[inline] - pub fn from_bytes(bytes: &[u8]) -> Result { + pub fn from_bytes(bytes: &[u8]) -> Result { if bytes.len() != SIGNATURE_LENGTH { - return Err(DecodingError(InternalError::BytesLengthError{ + return Err(SignatureError(InternalError::BytesLengthError{ name: "Signature", length: SIGNATURE_LENGTH })); } let mut lower: [u8; 32] = [0u8; 32]; @@ -144,7 +142,7 @@ impl Signature { upper.copy_from_slice(&bytes[32..]); if upper[31] & 224 != 0 { - return Err(DecodingError(InternalError::ScalarFormatError)); + return Err(SignatureError(InternalError::ScalarFormatError)); } Ok(Signature{ r: CompressedEdwardsY(lower), s: Scalar::from_bits(upper) }) @@ -215,9 +213,9 @@ impl SecretKey { /// # /// use ed25519_dalek::SecretKey; /// use ed25519_dalek::SECRET_KEY_LENGTH; - /// use ed25519_dalek::DecodingError; + /// use ed25519_dalek::SignatureError; /// - /// # 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, @@ -238,11 +236,11 @@ impl SecretKey { /// # Returns /// /// A `Result` whose okay value is an EdDSA `SecretKey` or whose error value - /// is an `DecodingError` wrapping the internal error that occurred. + /// is an `SignatureError` wrapping the internal error that occurred. #[inline] - pub fn from_bytes(bytes: &[u8]) -> Result { + pub fn from_bytes(bytes: &[u8]) -> Result { if bytes.len() != SECRET_KEY_LENGTH { - return Err(DecodingError(InternalError::BytesLengthError{ + return Err(SignatureError(InternalError::BytesLengthError{ name: "SecretKey", length: SECRET_KEY_LENGTH })); } let mut bits: [u8; 32] = [0u8; 32]; @@ -469,7 +467,7 @@ impl ExpandedSecretKey { /// # Returns /// /// A `Result` whose okay value is an EdDSA `ExpandedSecretKey` or whose - /// error value is an `DecodingError` describing the error that occurred. + /// error value is an `SignatureError` describing the error that occurred. /// /// # Examples /// @@ -479,11 +477,11 @@ impl ExpandedSecretKey { /// # extern crate ed25519_dalek; /// # /// # #[cfg(all(feature = "sha2", feature = "std"))] - /// # fn do_test() -> Result { + /// # fn do_test() -> Result { /// # /// use rand::{Rng, OsRng}; /// use ed25519_dalek::{SecretKey, ExpandedSecretKey}; - /// use ed25519_dalek::DecodingError; + /// use ed25519_dalek::SignatureError; /// /// let mut csprng: OsRng = OsRng::new().unwrap(); /// let secret_key: SecretKey = SecretKey::generate(&mut csprng); @@ -504,9 +502,9 @@ impl ExpandedSecretKey { /// # fn main() { } /// ``` #[inline] - pub fn from_bytes(bytes: &[u8]) -> Result { + pub fn from_bytes(bytes: &[u8]) -> Result { if bytes.len() != EXPANDED_SECRET_KEY_LENGTH { - return Err(DecodingError(InternalError::BytesLengthError{ + return Err(SignatureError(InternalError::BytesLengthError{ name: "ExpandedSecretKey", length: EXPANDED_SECRET_KEY_LENGTH })); } let mut lower: [u8; 32] = [0u8; 32]; @@ -746,9 +744,9 @@ impl PublicKey { /// # /// use ed25519_dalek::PublicKey; /// use ed25519_dalek::PUBLIC_KEY_LENGTH; - /// use ed25519_dalek::DecodingError; + /// use ed25519_dalek::SignatureError; /// - /// # 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]; @@ -766,11 +764,11 @@ impl PublicKey { /// # Returns /// /// A `Result` whose okay value is an EdDSA `PublicKey` or whose error value - /// is an `DecodingError` describing the error that occurred. + /// is an `SignatureError` describing the error that occurred. #[inline] - pub fn from_bytes(bytes: &[u8]) -> Result { + pub fn from_bytes(bytes: &[u8]) -> Result { if bytes.len() != PUBLIC_KEY_LENGTH { - return Err(DecodingError(InternalError::BytesLengthError{ + return Err(SignatureError(InternalError::BytesLengthError{ name: "PublicKey", length: PUBLIC_KEY_LENGTH })); } let mut bits: [u8; 32] = [0u8; 32]; @@ -779,12 +777,6 @@ impl PublicKey { Ok(PublicKey(CompressedEdwardsY(bits))) } - /// Convert this public key to its underlying extended twisted Edwards coordinate. - #[inline] - fn decompress(&self) -> Option { - self.0.decompress() - } - /// Derive this public key from its corresponding `SecretKey`. #[allow(unused_assignments)] pub fn from_secret(secret_key: &SecretKey) -> PublicKey @@ -812,36 +804,27 @@ impl PublicKey { /// /// # Return /// - /// Returns true if the signature was successfully verified, and - /// false otherwise. - pub fn verify(&self, message: &[u8], signature: &Signature) -> bool + /// Returns `Ok(())` if the signature is valid, and `Err` otherwise. + #[allow(non_snake_case)] + pub fn verify(&self, message: &[u8], signature: &Signature) -> Result<(), SignatureError> where D: Digest + Default { - let mut h: D = D::default(); - let mut a: EdwardsPoint; - let ao: Option; - let mut digest: [u8; 64] = [0u8; 64]; - - ao = self.decompress(); - - if ao.is_some() { - a = ao.unwrap(); - } else { - return false; - } - a = -(&a); + let A = self.0.decompress() + .ok_or_else(|| SignatureError(InternalError::PointDecompressionError))?; + let mut h = D::default(); h.input(signature.r.as_bytes()); h.input(self.as_bytes()); h.input(&message); + let k = Scalar::from_hash(h); - digest.copy_from_slice(h.fixed_result().as_slice()); + let R = EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(-A), &signature.s); - let digest_reduced: Scalar = Scalar::from_bytes_mod_order_wide(&digest); - let r: EdwardsPoint = EdwardsPoint::vartime_double_scalar_mul_basepoint(&digest_reduced, - &a, &signature.s); - - (signature.r.as_bytes()).ct_eq(r.compress().as_bytes()).unwrap_u8() == 1 + if R.compress() == signature.r { + Ok(()) + } else { + Err(SignatureError(InternalError::VerifyError)) + } } /// Verify a `signature` on a `prehashed_message` using the Ed25519ph algorithm. @@ -862,43 +845,36 @@ impl PublicKey { /// `Keypair` on the `prehashed_message`. /// /// [rfc8032]: https://tools.ietf.org/html/rfc8032#section-5.1 + #[allow(non_snake_case)] pub fn verify_prehashed(&self, prehashed_message: D, context: Option<&[u8]>, - signature: &Signature) -> bool + signature: &Signature) -> Result<(), SignatureError> where D: Digest + Default { - let mut h: D = D::default(); - let mut hash: [u8; 64] = [0u8; 64]; - - let mut a: EdwardsPoint = match self.decompress() { - Some(x) => x, - None => return false, - }; - a = -(&a); - - let ctx: &[u8] = match context { - Some(x) => x, - None => b"", // By default, the context is an empty string. - }; + let ctx = context.unwrap_or(b""); debug_assert!(ctx.len() <= 255, "The context must not be longer than 255 octets."); - let ctx_len: u8 = ctx.len() as u8; + let A = self.0.decompress() + .ok_or_else(|| SignatureError(InternalError::PointDecompressionError))?; + let mut h = D::default(); h.input(b"SigEd25519 no Ed25519 collisions"); h.input(&[1]); // Ed25519ph - h.input(&[ctx_len]); + h.input(&[ctx.len() as u8]); h.input(ctx); h.input(signature.r.as_bytes()); h.input(self.as_bytes()); h.input(prehashed_message.fixed_result().as_slice()); - hash.copy_from_slice(h.fixed_result().as_slice()); + let k = Scalar::from_hash(h); - let digest_reduced: Scalar = Scalar::from_bytes_mod_order_wide(&hash); - let r: EdwardsPoint = EdwardsPoint::vartime_double_scalar_mul_basepoint(&digest_reduced, - &a, &signature.s); + let R = EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(-A), &signature.s); - (signature.r.as_bytes()).ct_eq(r.compress().as_bytes()).unwrap_u8() == 1 + if R.compress() == signature.r { + Ok(()) + } else { + Err(SignatureError(InternalError::VerifyError)) + } } } @@ -976,10 +952,10 @@ impl Keypair { /// # Returns /// /// A `Result` whose okay value is an EdDSA `Keypair` or whose error value - /// is an `DecodingError` describing the error that occurred. - pub fn from_bytes<'a>(bytes: &'a [u8]) -> Result { + /// is an `SignatureError` describing the error that occurred. + pub fn from_bytes<'a>(bytes: &'a [u8]) -> Result { if bytes.len() != KEYPAIR_LENGTH { - return Err(DecodingError(InternalError::BytesLengthError{ + return Err(SignatureError(InternalError::BytesLengthError{ name: "Keypair", length: KEYPAIR_LENGTH})); } let secret = SecretKey::from_bytes(&bytes[..SECRET_KEY_LENGTH])?; @@ -1145,7 +1121,7 @@ impl Keypair { } /// Verify a signature on a message with this keypair's public key. - pub fn verify(&self, message: &[u8], signature: &Signature) -> bool + pub fn verify(&self, message: &[u8], signature: &Signature) -> Result<(), SignatureError> where D: Digest + Default { self.public.verify::(message, signature) } @@ -1210,7 +1186,7 @@ impl Keypair { pub fn verify_prehashed(&self, prehashed_message: D, context: Option<&[u8]>, - signature: &Signature) -> bool + signature: &Signature) -> Result<(), SignatureError> where D: Digest + Default { self.public.verify_prehashed::(prehashed_message, context, signature) @@ -1261,7 +1237,6 @@ mod test { use std::fs::File; use std::string::String; use std::vec::Vec; - use curve25519_dalek::edwards::EdwardsPoint; use rand::ChaChaRng; use rand::SeedableRng; use hex::FromHex; @@ -1295,32 +1270,7 @@ mod test { 063, 120, 126, 100, 092, 059, 050, 011, ]; #[test] - fn unmarshal_marshal() { // TestUnmarshalMarshal - let mut csprng: ChaChaRng; - let mut keypair: Keypair; - let mut x: Option; - let a: EdwardsPoint; - let public: PublicKey; - - csprng = ChaChaRng::from_seed([0u8; 32]); - - // from_bytes() fails if vx²-u=0 and vx²+u=0 - loop { - keypair = Keypair::generate::(&mut csprng); - x = keypair.public.decompress(); - - if x.is_some() { - a = x.unwrap(); - break; - } - } - public = PublicKey(a.compress()); - - assert!(keypair.public.0 == public.0); - } - - #[test] - fn ed25519_sign_verify() { // TestSignVerify + fn sign_verify() { // TestSignVerify let mut csprng: ChaChaRng; let keypair: Keypair; let good_sig: Signature; @@ -1334,11 +1284,11 @@ mod test { good_sig = keypair.sign::(&good); bad_sig = keypair.sign::(&bad); - assert!(keypair.verify::(&good, &good_sig) == true, + assert!(keypair.verify::(&good, &good_sig).is_ok(), "Verification of a valid signature failed!"); - assert!(keypair.verify::(&good, &bad_sig) == false, + assert!(keypair.verify::(&good, &bad_sig).is_err(), "Verification of a signature on a different message passed!"); - assert!(keypair.verify::(&bad, &good_sig) == false, + assert!(keypair.verify::(&bad, &good_sig).is_err(), "Verification of a signature on a different message passed!"); } @@ -1383,7 +1333,7 @@ mod test { let sig2: Signature = keypair.sign::(&msg_bytes); assert!(sig1 == sig2, "Signature bytes not equal on line {}", lineno); - assert!(keypair.verify::(&msg_bytes, &sig2), + assert!(keypair.verify::(&msg_bytes, &sig2).is_ok(), "Signature verification failed on line {}", lineno); } } @@ -1417,7 +1367,7 @@ mod test { assert!(sig1 == sig2, "Original signature from test vectors doesn't equal signature produced:\ \noriginal:\n{:?}\nproduced:\n{:?}", sig1, sig2); - assert!(keypair.verify_prehashed(prehash_for_verifying, None, &sig2), + assert!(keypair.verify_prehashed(prehash_for_verifying, None, &sig2).is_ok(), "Could not verify ed25519ph signature!"); } @@ -1451,18 +1401,18 @@ mod test { good_sig = keypair.sign_prehashed::(prehashed_good1, Some(context)); bad_sig = keypair.sign_prehashed::(prehashed_bad1, Some(context)); - assert!(keypair.verify_prehashed::(prehashed_good2, Some(context), &good_sig) == true, + assert!(keypair.verify_prehashed::(prehashed_good2, Some(context), &good_sig).is_ok(), "Verification of a valid signature failed!"); - assert!(keypair.verify_prehashed::(prehashed_good3, Some(context), &bad_sig) == false, + assert!(keypair.verify_prehashed::(prehashed_good3, Some(context), &bad_sig).is_err(), "Verification of a signature on a different message passed!"); - assert!(keypair.verify_prehashed::(prehashed_bad2, Some(context), &good_sig) == false, + assert!(keypair.verify_prehashed::(prehashed_bad2, Some(context), &good_sig).is_err(), "Verification of a signature on a different message passed!"); } #[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/errors.rs b/src/errors.rs index 57968dd..bf568a6 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -20,7 +20,6 @@ use core::fmt::Display; /// need to pay any attention to these. #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] pub (crate) enum InternalError { - #[allow(dead_code)] PointDecompressionError, ScalarFormatError, /// An error in the length of bytes handed to a constructor. @@ -29,55 +28,52 @@ pub (crate) enum InternalError { /// returning the error, and the `length` in bytes which its constructor /// expects. BytesLengthError{ name: &'static str, length: usize }, + /// The verification equation wasn't satisfied + VerifyError, } impl Display for InternalError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { InternalError::PointDecompressionError - => write!(f, "Cannot decompress extended twisted edwards point"), + => write!(f, "Cannot decompress Edwards point"), InternalError::ScalarFormatError => write!(f, "Cannot use scalar with high-bit set"), InternalError::BytesLengthError{ name: n, length: l} => write!(f, "{} must be {} bytes in length", n, l), + InternalError::VerifyError + => write!(f, "Verification equation was not satisfied"), } } } impl ::failure::Fail for InternalError {} -/// Errors which may occur in the `from_bytes()` constructors of `PublicKey`, -/// `SecretKey`, `ExpandedSecretKey`, `Keypair`, and `Signature`. -/// -/// There was an internal problem due to parsing the `Signature`. +/// Errors which may occur while processing signatures and keypairs. /// /// This error may arise due to: /// +/// * Being given bytes with a length different to what was expected. +/// /// * A problem decompressing `r`, a curve point, in the `Signature`, or the /// curve point for a `PublicKey`. +/// /// * A problem with the format of `s`, a scalar, in the `Signature`. This /// is only raised if the high-bit of the scalar was set. (Scalars must /// only be constructed from 255-bit integers.) -/// * Being given bytes with a length different to what was expected. +/// +/// * Failure of a signature to satisfy the verification equation. #[derive(Clone, Copy, Eq, PartialEq, Hash, Debug)] -pub struct DecodingError(pub (crate) InternalError); +pub struct SignatureError(pub (crate) InternalError); -impl Display for DecodingError { +impl Display for SignatureError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self.0 { - InternalError::PointDecompressionError => write!(f, "{}", self.0), - InternalError::ScalarFormatError => write!(f, "{}", self.0), - InternalError::BytesLengthError{ name: _, length: _ } => write!(f, "{}", self.0), - } + write!(f, "{}", self.0) } } -impl ::failure::Fail for DecodingError { +impl ::failure::Fail for SignatureError { fn cause(&self) -> Option<&::failure::Fail> { - match self.0 { - InternalError::PointDecompressionError => Some(&self.0), - InternalError::ScalarFormatError => Some(&self.0), - InternalError::BytesLengthError{ name: _, length: _} => Some(&self.0), - } + Some(&self.0) } } diff --git a/src/lib.rs b/src/lib.rs index d516597..b74999a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -78,9 +78,7 @@ //! # let keypair: Keypair = Keypair::generate::(&mut csprng); //! # let message: &[u8] = "This is a test of the tsunami alert system.".as_bytes(); //! # let signature: Signature = keypair.sign::(message); -//! let verified: bool = keypair.verify::(message, &signature); -//! -//! assert!(verified); +//! assert!(keypair.verify::(message, &signature).is_ok()); //! # } //! ``` //! @@ -105,9 +103,7 @@ //! # let signature: Signature = keypair.sign::(message); //! //! let public_key: PublicKey = keypair.public; -//! let verified: bool = public_key.verify::(message, &signature); -//! -//! assert!(verified); +//! assert!(public_key.verify::(message, &signature).is_ok()); //! # } //! ``` //! @@ -133,7 +129,6 @@ //! # let message: &[u8] = "This is a test of the tsunami alert system.".as_bytes(); //! # let signature: Signature = keypair.sign::(message); //! # let public_key: PublicKey = keypair.public; -//! # let verified: bool = public_key.verify::(message, &signature); //! //! let public_key_bytes: [u8; PUBLIC_KEY_LENGTH] = public_key.to_bytes(); //! let secret_key_bytes: [u8; SECRET_KEY_LENGTH] = keypair.secret.to_bytes(); @@ -150,9 +145,9 @@ //! # extern crate ed25519_dalek; //! # use rand::{Rng, ChaChaRng, SeedableRng}; //! # use sha2::Sha512; -//! # use ed25519_dalek::{Keypair, Signature, PublicKey, SecretKey, DecodingError}; +//! # use ed25519_dalek::{Keypair, Signature, PublicKey, SecretKey, SignatureError}; //! # use ed25519_dalek::{PUBLIC_KEY_LENGTH, SECRET_KEY_LENGTH, KEYPAIR_LENGTH, SIGNATURE_LENGTH}; -//! # fn do_test() -> Result<(SecretKey, PublicKey, Keypair, Signature), DecodingError> { +//! # fn do_test() -> Result<(SecretKey, PublicKey, Keypair, Signature), SignatureError> { //! # let mut csprng: ChaChaRng = ChaChaRng::from_seed([0u8; 32]); //! # let keypair_orig: Keypair = Keypair::generate::(&mut csprng); //! # let message: &[u8] = "This is a test of the tsunami alert system.".as_bytes(); @@ -267,7 +262,6 @@ extern crate curve25519_dalek; extern crate generic_array; extern crate digest; -extern crate subtle; extern crate failure; extern crate rand;