From 8ca3be99e9d1585dbe187e33e10bc5f405009fac Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 9 Dec 2019 22:39:57 +0000 Subject: [PATCH 01/22] Switch to using zeroize rather than clear_on_drop. --- Cargo.toml | 6 +++--- src/ed25519.rs | 25 +------------------------ src/lib.rs | 2 +- src/secret.rs | 51 +++++++++++++++++++++++++++++++------------------- 4 files changed, 37 insertions(+), 47 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c9d77a6..fc87a11 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,13 +22,13 @@ travis-ci = { repository = "dalek-cryptography/ed25519-dalek", branch = "master" features = ["nightly", "batch"] [dependencies] -clear_on_drop = { version = "0.2" } curve25519-dalek = { version = "2", default-features = false } merlin = { version = "1", default-features = false, optional = true, git = "https://github.com/isislovecruft/merlin", branch = "develop" } rand = { version = "0.7", default-features = false, optional = true } rand_core = { version = "0.5", default-features = false, optional = true } serde = { version = "1.0", optional = true } sha2 = { version = "0.8", default-features = false } +zeroize = { version = "1", default-features = false, features = ["zeroize_derive"] } [dev-dependencies] hex = "^0.4" @@ -46,8 +46,8 @@ harness = false [features] default = ["std", "u64_backend"] std = ["curve25519-dalek/std", "sha2/std", "rand/std"] -alloc = ["curve25519-dalek/alloc", "rand/alloc"] -nightly = ["curve25519-dalek/nightly", "clear_on_drop/nightly", "rand/nightly"] +alloc = ["curve25519-dalek/alloc", "rand/alloc", "zeroize/alloc"] +nightly = ["curve25519-dalek/nightly", "rand/nightly"] batch = ["merlin", "rand"] # This feature enables deterministic batch verification. batch_deterministic = ["merlin", "rand", "rand_core"] diff --git a/src/ed25519.rs b/src/ed25519.rs index 7c55a65..dd150bf 100644 --- a/src/ed25519.rs +++ b/src/ed25519.rs @@ -34,7 +34,7 @@ pub use crate::secret::*; pub use crate::signature::*; /// An ed25519 keypair. -#[derive(Debug, Default)] // we derive Default in order to use the clear() method in Drop +#[derive(Debug)] pub struct Keypair { /// The secret half of this keypair. pub secret: SecretKey, @@ -444,26 +444,3 @@ impl<'d> Deserialize<'d> for Keypair { deserializer.deserialize_bytes(KeypairVisitor) } } - -#[cfg(test)] -mod test { - use super::*; - - use clear_on_drop::clear::Clear; - - #[test] - fn keypair_clear_on_drop() { - let mut keypair: Keypair = Keypair::from_bytes(&[1u8; KEYPAIR_LENGTH][..]).unwrap(); - - keypair.clear(); - - fn as_bytes(x: &T) -> &[u8] { - use std::mem; - use std::slice; - - unsafe { slice::from_raw_parts(x as *const T as *const u8, mem::size_of_val(x)) } - } - - assert!(!as_bytes(&keypair).contains(&0x15)); - } -} diff --git a/src/lib.rs b/src/lib.rs index 32aff4e..bee0f7c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -239,7 +239,6 @@ extern crate std; #[cfg(all(feature = "alloc", not(feature = "std")))] extern crate alloc; -extern crate clear_on_drop; extern crate curve25519_dalek; #[cfg(all(any(feature = "batch", feature = "batch_deterministic"), any(feature = "std", feature = "alloc")))] extern crate merlin; @@ -248,6 +247,7 @@ extern crate rand; #[cfg(feature = "serde")] extern crate serde; extern crate sha2; +extern crate zeroize; #[cfg(all(any(feature = "batch", feature = "batch_deterministic"), any(feature = "std", feature = "alloc")))] mod batch; diff --git a/src/secret.rs b/src/secret.rs index 0c54275..f1e751d 100644 --- a/src/secret.rs +++ b/src/secret.rs @@ -11,8 +11,6 @@ use core::fmt::Debug; -use clear_on_drop::clear::Clear; - use curve25519_dalek::constants; use curve25519_dalek::digest::generic_array::typenum::U64; use curve25519_dalek::digest::Digest; @@ -32,13 +30,19 @@ use serde::{Deserialize, Serialize}; #[cfg(feature = "serde")] use serde::{Deserializer, Serializer}; +use zeroize::Zeroize; + use crate::constants::*; use crate::errors::*; use crate::public::*; use crate::signature::*; /// An EdDSA secret key. -#[derive(Default)] // we derive Default in order to use the clear() method in Drop +/// +/// Instances of this secret are automatically overwritten with zeroes when they +/// fall out of scope. +#[derive(Zeroize)] +#[zeroize(drop)] // Overwrite secret key material with null bytes when it goes out of scope. pub struct SecretKey(pub(crate) [u8; SECRET_KEY_LENGTH]); impl Debug for SecretKey { @@ -47,13 +51,6 @@ impl Debug for SecretKey { } } -/// Overwrite secret key material with null bytes when it goes out of scope. -impl Drop for SecretKey { - fn drop(&mut self) { - self.0.clear(); - } -} - impl AsRef<[u8]> for SecretKey { fn as_ref(&self) -> &[u8] { self.as_bytes() @@ -223,6 +220,9 @@ impl<'d> Deserialize<'d> for SecretKey { /// upper half is used a sort of half-baked, ill-designed² pseudo-domain-separation /// "nonce"-like thing, which is used during signature production by /// concatenating it with the message to be signed before the message is hashed. +/// +/// Instances of this secret are automatically overwritten with zeroes when they +/// fall out of scope. // // ¹ This results in a slight bias towards non-uniformity at one spectrum of // the range of valid keys. Oh well: not my idea; not my problem. @@ -250,20 +250,13 @@ impl<'d> Deserialize<'d> for SecretKey { // same signature scheme, and which both fail in exactly the same way. For a // better-designed, Schnorr-based signature scheme, see Trevor Perrin's work on // "generalised EdDSA" and "VXEdDSA". -#[derive(Default)] // we derive Default in order to use the clear() method in Drop +#[derive(Zeroize)] +#[zeroize(drop)] // Overwrite secret key material with null bytes when it goes out of scope. pub struct ExpandedSecretKey { pub(crate) key: Scalar, pub(crate) nonce: [u8; 32], } -/// Overwrite secret key material with null bytes when it goes out of scope. -impl Drop for ExpandedSecretKey { - fn drop(&mut self) { - self.key.clear(); - self.nonce.clear(); - } -} - impl<'a> From<&'a SecretKey> for ExpandedSecretKey { /// Construct an `ExpandedSecretKey` from a `SecretKey`. /// @@ -554,3 +547,23 @@ impl<'d> Deserialize<'d> for ExpandedSecretKey { deserializer.deserialize_bytes(ExpandedSecretKeyVisitor) } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn secret_key_zeroize_on_drop() { + let secret_ptr: *const u8; + + { // scope for the secret to ensure it's been dropped + let secret = SecretKey::from_bytes(&[0x15u8; 32][..]).unwrap(); + + secret_ptr = secret.0.as_ptr(); + } + + let memory: &[u8] = unsafe { ::std::slice::from_raw_parts(secret_ptr, 32) }; + + assert!(!memory.contains(&0x15)); + } +} From 0a191a86f63e553dd33212313587b2ec3815f3cc Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Tue, 10 Dec 2019 13:30:56 -0800 Subject: [PATCH 02/22] Use `default-features = false` with `serde` It doesn't appear to me that ed25519-dalek crate needs any of the std-related features of serde. But it turns them on anyways because it doesn't put `default-features = false`. This breaks no_std builds. Otherwise I think we could use 1.0.0-pre3 in mobilecoin. I'm going to test this revision in our build and see if I'm right. I don't think this is a breaking change from dalek's point of view. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index c9d77a6..ba80153 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,7 @@ curve25519-dalek = { version = "2", default-features = false } merlin = { version = "1", default-features = false, optional = true, git = "https://github.com/isislovecruft/merlin", branch = "develop" } rand = { version = "0.7", default-features = false, optional = true } rand_core = { version = "0.5", default-features = false, optional = true } -serde = { version = "1.0", optional = true } +serde = { version = "1.0", default-features = false, optional = true } sha2 = { version = "0.8", default-features = false } [dev-dependencies] From 9363690191b1c3798bfeca8a6c36f70cb7f311f7 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Wed, 11 Dec 2019 22:48:33 +0000 Subject: [PATCH 03/22] Fix outdated docstring for verify_batch(). --- src/batch.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/batch.rs b/src/batch.rs index a778934..90d28ef 100644 --- a/src/batch.rs +++ b/src/batch.rs @@ -111,7 +111,6 @@ fn zero_rng() -> ZeroRng { /// * `messages` is a slice of byte slices, one per signed message. /// * `signatures` is a slice of `Signature`s. /// * `public_keys` is a slice of `PublicKey`s. -/// * `csprng` is an implementation of `Rng + CryptoRng`. /// /// # Returns /// @@ -195,7 +194,6 @@ pub fn verify_batch( .map(|_| Scalar::from(prng.gen::())) .collect(); - // Compute the basepoint coefficient, ∑ s[i]z[i] (mod l) let B_coefficient: Scalar = signatures .iter() From 8a2e9af9d6c71f7d12be90e13223d585dd5df15e Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Wed, 11 Dec 2019 23:02:44 +0000 Subject: [PATCH 04/22] Fix breakage on builds with the rand crate disabled. * CLOSES https://github.com/dalek-cryptography/ed25519-dalek/issues/108 * THANKS TO @tarcieri --- src/ed25519.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ed25519.rs b/src/ed25519.rs index dd150bf..076d30c 100644 --- a/src/ed25519.rs +++ b/src/ed25519.rs @@ -11,6 +11,7 @@ use core::default::Default; +#[cfg(feature = "rand")] use rand::{CryptoRng, RngCore}; #[cfg(feature = "serde")] From 3a9101933b38acf4136925f89c2cee290d30445b Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Wed, 11 Dec 2019 23:14:27 +0000 Subject: [PATCH 05/22] Enable serde/std if std is enabled. * FIXES part of https://github.com/dalek-cryptography/ed25519-dalek/pull/107 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 4fb9d9d..2931ff5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,7 +45,7 @@ harness = false [features] default = ["std", "u64_backend"] -std = ["curve25519-dalek/std", "sha2/std", "rand/std"] +std = ["curve25519-dalek/std", "serde/std", "sha2/std", "rand/std"] alloc = ["curve25519-dalek/alloc", "rand/alloc", "zeroize/alloc"] nightly = ["curve25519-dalek/nightly", "rand/nightly"] batch = ["merlin", "rand"] From 3d9d11dcdf64a27b8c2fa7f5cdb0fcbff415d0d6 Mon Sep 17 00:00:00 2001 From: James Munns Date: Mon, 13 Jan 2020 00:59:08 +0100 Subject: [PATCH 06/22] Add additional visitor methods for deserialization --- src/ed25519.rs | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/src/ed25519.rs b/src/ed25519.rs index 7c55a65..e08a185 100644 --- a/src/ed25519.rs +++ b/src/ed25519.rs @@ -18,6 +18,8 @@ use serde::de::Error as SerdeError; #[cfg(feature = "serde")] use serde::de::Visitor; #[cfg(feature = "serde")] +use serde::de::SeqAccess; +#[cfg(feature = "serde")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; pub use sha2::Sha512; @@ -431,15 +433,47 @@ impl<'d> Deserialize<'d> for Keypair { where E: SerdeError, { + if bytes.len() != KEYPAIR_LENGTH { + return Err(SerdeError::invalid_length(bytes.len(), &self)); + } + let secret_key = SecretKey::from_bytes(&bytes[..SECRET_KEY_LENGTH]); let public_key = PublicKey::from_bytes(&bytes[SECRET_KEY_LENGTH..]); - if secret_key.is_ok() && public_key.is_ok() { - Ok(Keypair{ secret: secret_key.unwrap(), public: public_key.unwrap() }) + if let (Ok(secret), Ok(public)) = (secret_key, public_key) { + Ok(Keypair{ secret, public }) } else { Err(SerdeError::invalid_length(bytes.len(), &self)) } } + + fn visit_seq(self, mut seq: A) -> Result + where + A: SeqAccess<'d> + { + if let Some(len) = seq.size_hint() { + if len != KEYPAIR_LENGTH { + return Err(SerdeError::invalid_length(len, &self)); + } + } + + // TODO: We could do this with `MaybeUninit` to avoid unnecessary initialization costs + let mut bytes: [u8; KEYPAIR_LENGTH] = [0u8; KEYPAIR_LENGTH]; + + for i in 0..KEYPAIR_LENGTH { + bytes[i] = seq.next_element()?.ok_or_else(|| SerdeError::invalid_length(i, &self))?; + } + + let secret_key = SecretKey::from_bytes(&bytes[..SECRET_KEY_LENGTH]); + let public_key = PublicKey::from_bytes(&bytes[SECRET_KEY_LENGTH..]); + + if let (Ok(secret), Ok(public)) = (secret_key, public_key) { + Ok(Keypair{ secret, public }) + } else { + Err(SerdeError::invalid_length(bytes.len(), &self)) + } + } + } deserializer.deserialize_bytes(KeypairVisitor) } From dedbb9b96a1bacec5a181fae32ce601f640eff99 Mon Sep 17 00:00:00 2001 From: NikVolf Date: Sat, 22 Feb 2020 15:10:21 +0300 Subject: [PATCH 07/22] fix alloc feature --- src/batch.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/batch.rs b/src/batch.rs index a778934..7f5838f 100644 --- a/src/batch.rs +++ b/src/batch.rs @@ -9,9 +9,11 @@ //! Batch signature verification. +#[cfg(feature = "alloc")] +extern crate alloc; #[cfg(feature = "alloc")] use alloc::vec::Vec; -#[cfg(feature = "std")] +#[cfg(all(not(feature = "alloc"), feature = "std"))] use std::vec::Vec; use core::iter::once; From aa38c6419d7859d0fa95f561e8a72cb623bec167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Thu, 16 Apr 2020 20:48:49 -0700 Subject: [PATCH 08/22] Updates the merlin dependency to ^2 and the correct repo This fixes the "batch" feature, see #126. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index c9d77a6..5f79970 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ features = ["nightly", "batch"] [dependencies] clear_on_drop = { version = "0.2" } curve25519-dalek = { version = "2", default-features = false } -merlin = { version = "1", default-features = false, optional = true, git = "https://github.com/isislovecruft/merlin", branch = "develop" } +merlin = { version = "2", default-features = false, optional = true } rand = { version = "0.7", default-features = false, optional = true } rand_core = { version = "0.5", default-features = false, optional = true } serde = { version = "1.0", optional = true } From 6e0667d4298fdf9cb0d3c3cd65c39ba52f0ec702 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Tue, 17 Mar 2020 10:25:33 -0700 Subject: [PATCH 09/22] Use `ed25519` + `signature` interop crates The `signature` crate provides `Signer` and `Verifier` traits generic over signature types: https://github.com/RustCrypto/traits/tree/master/signature There's presently an open call to stabilize the parts of its API needed by Ed25519 signatures and release a 1.0 version: https://github.com/RustCrypto/traits/issues/78 The `ed25519` crate, based on the `signature` crate, provides an `ed25519::Signature` type which can be shared across multiple Ed25519 crates (e.g. it is also used by the `yubihsm` crate): https://github.com/RustCrypto/signatures/tree/master/ed25519 This commit integrates the `ed25519::Signature` type, and changes the existing `sign` and `verify` methods (where applicable) to use the `Signer` and `Verifier` traits from the `signature` crate. Additionally, it replaces `SignatureError` with the `signature` crate's error type. This has the drawback of requiring the `Signer` and/or `Verifier` traits are in scope in order to create and/or verify signatures, but with the benefit of supporting interoperability with other Ed25519 crates which also make use of these traits. --- Cargo.toml | 6 ++- src/batch.rs | 20 +++++--- src/errors.rs | 22 ++++---- src/{ed25519.rs => keypair.rs} | 51 ++++++++++--------- src/lib.rs | 47 +++++++++++------- src/public.rs | 91 +++++++++++++++++++--------------- src/secret.rs | 16 +++--- src/signature.rs | 67 +++++++------------------ tests/ed25519.rs | 6 ++- 9 files changed, 169 insertions(+), 157 deletions(-) rename src/{ed25519.rs => keypair.rs} (94%) diff --git a/Cargo.toml b/Cargo.toml index 2931ff5..23196e9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,10 +23,11 @@ features = ["nightly", "batch"] [dependencies] curve25519-dalek = { version = "2", default-features = false } +ed25519 = { version = "1", default-features = false } merlin = { version = "1", default-features = false, optional = true, git = "https://github.com/isislovecruft/merlin", branch = "develop" } rand = { version = "0.7", default-features = false, optional = true } rand_core = { version = "0.5", default-features = false, optional = true } -serde = { version = "1.0", default-features = false, optional = true } +serde_crate = { package = "serde", version = "1.0", default-features = false, optional = true } sha2 = { version = "0.8", default-features = false } zeroize = { version = "1", default-features = false, features = ["zeroize_derive"] } @@ -45,9 +46,10 @@ harness = false [features] default = ["std", "u64_backend"] -std = ["curve25519-dalek/std", "serde/std", "sha2/std", "rand/std"] +std = ["curve25519-dalek/std", "ed25519/std", "serde_crate/std", "sha2/std", "rand/std"] alloc = ["curve25519-dalek/alloc", "rand/alloc", "zeroize/alloc"] nightly = ["curve25519-dalek/nightly", "rand/nightly"] +serde = ["serde_crate", "ed25519/serde"] batch = ["merlin", "rand"] # This feature enables deterministic batch verification. batch_deterministic = ["merlin", "rand", "rand_core"] diff --git a/src/batch.rs b/src/batch.rs index 90d28ef..d7816eb 100644 --- a/src/batch.rs +++ b/src/batch.rs @@ -14,6 +14,7 @@ use alloc::vec::Vec; #[cfg(feature = "std")] use std::vec::Vec; +use core::convert::TryFrom; use core::iter::once; use curve25519_dalek::constants; @@ -37,7 +38,7 @@ use sha2::Sha512; use crate::errors::InternalError; use crate::errors::SignatureError; use crate::public::PublicKey; -use crate::signature::Signature; +use crate::signature::InternalSignature; trait BatchTranscript { fn append_hrams(&mut self, hrams: &Vec); @@ -127,6 +128,7 @@ fn zero_rng() -> ZeroRng { /// use ed25519_dalek::verify_batch; /// use ed25519_dalek::Keypair; /// use ed25519_dalek::PublicKey; +/// use ed25519_dalek::Signer; /// use ed25519_dalek::Signature; /// use rand::rngs::OsRng; /// @@ -147,7 +149,7 @@ fn zero_rng() -> ZeroRng { #[allow(non_snake_case)] pub fn verify_batch( messages: &[&[u8]], - signatures: &[Signature], + signatures: &[ed25519::Signature], public_keys: &[PublicKey], ) -> Result<(), SignatureError> { @@ -155,13 +157,19 @@ pub fn verify_batch( if signatures.len() != messages.len() || signatures.len() != public_keys.len() || public_keys.len() != messages.len() { - return Err(SignatureError(InternalError::ArrayLengthError{ + return Err(InternalError::ArrayLengthError{ name_a: "signatures", length_a: signatures.len(), name_b: "messages", length_b: messages.len(), name_c: "public_keys", length_c: public_keys.len(), - })); + }.into()); } + // Convert all signatures to `InternalSignature` + let signatures = signatures + .iter() + .map(InternalSignature::try_from) + .collect::, _>>()?; + // Compute H(R || A || M) for each (signature, public_key, message) triplet let hrams: Vec = (0..signatures.len()).map(|i| { let mut h: Sha512 = Sha512::default(); @@ -213,11 +221,11 @@ pub fn verify_batch( let id = EdwardsPoint::optional_multiscalar_mul( once(-B_coefficient).chain(zs.iter().cloned()).chain(zhrams), B.chain(Rs).chain(As), - ).ok_or_else(|| SignatureError(InternalError::VerifyError))?; + ).ok_or(InternalError::VerifyError)?; if id.is_identity() { Ok(()) } else { - Err(SignatureError(InternalError::VerifyError)) + Err(InternalError::VerifyError.into()) } } diff --git a/src/errors.rs b/src/errors.rs index 1d14759..108ca1f 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -80,18 +80,16 @@ impl Error for InternalError { } /// only be constructed from 255-bit integers.) /// /// * Failure of a signature to satisfy the verification equation. -#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug)] -pub struct SignatureError(pub(crate) InternalError); +pub type SignatureError = ed25519::signature::Error; -impl Display for SignatureError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) - } -} - -#[cfg(feature = "std")] -impl Error for SignatureError { - fn source(&self) -> Option<&(dyn Error + 'static)> { - Some(&self.0) +impl From for SignatureError { + #[cfg(not(feature = "std"))] + fn from(_err: InternalError) -> SignatureError { + SignatureError::new() + } + + #[cfg(feature = "std")] + fn from(err: InternalError) -> SignatureError { + SignatureError::from_source(err) } } diff --git a/src/ed25519.rs b/src/keypair.rs similarity index 94% rename from src/ed25519.rs rename to src/keypair.rs index 076d30c..4fd63df 100644 --- a/src/ed25519.rs +++ b/src/keypair.rs @@ -9,8 +9,6 @@ //! ed25519 keypairs. -use core::default::Default; - #[cfg(feature = "rand")] use rand::{CryptoRng, RngCore}; @@ -26,13 +24,12 @@ pub use sha2::Sha512; use curve25519_dalek::digest::generic_array::typenum::U64; pub use curve25519_dalek::digest::Digest; -#[cfg(all(feature = "batch", any(feature = "std", feature = "alloc")))] -pub use crate::batch::*; -pub use crate::constants::*; -pub use crate::errors::*; -pub use crate::public::*; -pub use crate::secret::*; -pub use crate::signature::*; +use ed25519::signature::{Signer, Verifier}; + +use crate::constants::*; +use crate::errors::*; +use crate::public::*; +use crate::secret::*; /// An ed25519 keypair. #[derive(Debug)] @@ -82,10 +79,10 @@ impl Keypair { /// is an `SignatureError` describing the error that occurred. pub fn from_bytes<'a>(bytes: &'a [u8]) -> Result { if bytes.len() != KEYPAIR_LENGTH { - return Err(SignatureError(InternalError::BytesLengthError { + return Err(InternalError::BytesLengthError { name: "Keypair", length: KEYPAIR_LENGTH, - })); + }.into()); } let secret = SecretKey::from_bytes(&bytes[..SECRET_KEY_LENGTH])?; let public = PublicKey::from_bytes(&bytes[SECRET_KEY_LENGTH..])?; @@ -136,13 +133,6 @@ impl Keypair { Keypair{ public: pk, secret: sk } } - /// Sign a message with this keypair's secret key. - pub fn sign(&self, message: &[u8]) -> Signature { - let expanded: ExpandedSecretKey = (&self.secret).into(); - - expanded.sign(&message, &self.public) - } - /// Sign a `prehashed_message` with this `Keypair` using the /// Ed25519ph algorithm defined in [RFC8032 §5.1][rfc8032]. /// @@ -241,20 +231,20 @@ impl Keypair { &self, prehashed_message: D, context: Option<&[u8]>, - ) -> Signature + ) -> ed25519::Signature where D: Digest, { let expanded: ExpandedSecretKey = (&self.secret).into(); // xxx thanks i hate this - expanded.sign_prehashed(prehashed_message, &self.public, context) + expanded.sign_prehashed(prehashed_message, &self.public, context).into() } /// Verify a signature on a message with this keypair's public key. pub fn verify( &self, message: &[u8], - signature: &Signature + signature: &ed25519::Signature ) -> Result<(), SignatureError> { self.public.verify(message, signature) @@ -320,7 +310,7 @@ impl Keypair { &self, prehashed_message: D, context: Option<&[u8]>, - signature: &Signature, + signature: &ed25519::Signature, ) -> Result<(), SignatureError> where D: Digest, @@ -394,13 +384,28 @@ impl Keypair { pub fn verify_strict( &self, message: &[u8], - signature: &Signature, + signature: &ed25519::Signature, ) -> Result<(), SignatureError> { self.public.verify_strict(message, signature) } } +impl Signer for Keypair { + /// Sign a message with this keypair's secret key. + fn try_sign(&self, message: &[u8]) -> Result { + let expanded: ExpandedSecretKey = (&self.secret).into(); + Ok(expanded.sign(&message, &self.public).into()) + } +} + +impl Verifier for Keypair { + /// Verify a signature on a message with this keypair's public key. + fn verify(&self, message: &[u8], signature: &ed25519::Signature) -> Result<(), SignatureError> { + self.public.verify(message, signature) + } +} + #[cfg(feature = "serde")] impl Serialize for Keypair { fn serialize(&self, serializer: S) -> Result diff --git a/src/lib.rs b/src/lib.rs index bee0f7c..22cd7e9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,9 +44,9 @@ //! # fn main() { //! # use rand::rngs::OsRng; //! # use ed25519_dalek::Keypair; -//! # use ed25519_dalek::Signature; //! # let mut csprng = OsRng{}; //! # let keypair: Keypair = Keypair::generate(&mut csprng); +//! use ed25519_dalek::{Signature, Signer}; //! let message: &[u8] = b"This is a test of the tsunami alert system."; //! let signature: Signature = keypair.sign(message); //! # } @@ -60,12 +60,12 @@ //! # extern crate ed25519_dalek; //! # fn main() { //! # use rand::rngs::OsRng; -//! # use ed25519_dalek::Keypair; -//! # use ed25519_dalek::Signature; +//! # use ed25519_dalek::{Keypair, Signature, Signer}; //! # let mut csprng = OsRng{}; //! # let keypair: Keypair = Keypair::generate(&mut csprng); //! # let message: &[u8] = b"This is a test of the tsunami alert system."; //! # let signature: Signature = keypair.sign(message); +//! use ed25519_dalek::Verifier; //! assert!(keypair.verify(message, &signature).is_ok()); //! # } //! ``` @@ -80,7 +80,8 @@ //! # use rand::rngs::OsRng; //! # use ed25519_dalek::Keypair; //! # use ed25519_dalek::Signature; -//! use ed25519_dalek::PublicKey; +//! # use ed25519_dalek::Signer; +//! use ed25519_dalek::{PublicKey, Verifier}; //! # let mut csprng = OsRng{}; //! # let keypair: Keypair = Keypair::generate(&mut csprng); //! # let message: &[u8] = b"This is a test of the tsunami alert system."; @@ -104,7 +105,7 @@ //! # extern crate ed25519_dalek; //! # fn main() { //! # use rand::rngs::OsRng; -//! # use ed25519_dalek::{Keypair, Signature, PublicKey}; +//! # use ed25519_dalek::{Keypair, Signature, Signer, PublicKey}; //! use ed25519_dalek::{PUBLIC_KEY_LENGTH, SECRET_KEY_LENGTH, KEYPAIR_LENGTH, SIGNATURE_LENGTH}; //! # let mut csprng = OsRng{}; //! # let keypair: Keypair = Keypair::generate(&mut csprng); @@ -124,8 +125,9 @@ //! ``` //! # extern crate rand; //! # extern crate ed25519_dalek; +//! # use std::convert::TryFrom; //! # use rand::rngs::OsRng; -//! # use ed25519_dalek::{Keypair, Signature, PublicKey, SecretKey, SignatureError}; +//! # use ed25519_dalek::{Keypair, Signature, Signer, PublicKey, SecretKey, SignatureError}; //! # use ed25519_dalek::{PUBLIC_KEY_LENGTH, SECRET_KEY_LENGTH, KEYPAIR_LENGTH, SIGNATURE_LENGTH}; //! # fn do_test() -> Result<(SecretKey, PublicKey, Keypair, Signature), SignatureError> { //! # let mut csprng = OsRng{}; @@ -140,7 +142,7 @@ //! let public_key: PublicKey = PublicKey::from_bytes(&public_key_bytes)?; //! let secret_key: SecretKey = SecretKey::from_bytes(&secret_key_bytes)?; //! let keypair: Keypair = Keypair::from_bytes(&keypair_bytes)?; -//! let signature: Signature = Signature::from_bytes(&signature_bytes)?; +//! let signature: Signature = Signature::try_from(&signature_bytes[..])?; //! # //! # Ok((secret_key, public_key, keypair, signature)) //! # } @@ -166,14 +168,14 @@ //! # extern crate rand; //! # extern crate ed25519_dalek; //! # #[cfg(feature = "serde")] -//! extern crate serde; +//! # extern crate serde_crate as serde; //! # #[cfg(feature = "serde")] -//! extern crate bincode; +//! # extern crate bincode; //! //! # #[cfg(feature = "serde")] //! # fn main() { //! # use rand::rngs::OsRng; -//! # use ed25519_dalek::{Keypair, Signature, PublicKey}; +//! # use ed25519_dalek::{Keypair, Signature, Signer, Verifier, PublicKey}; //! use bincode::{serialize, Infinite}; //! # let mut csprng = OsRng{}; //! # let keypair: Keypair = Keypair::generate(&mut csprng); @@ -196,16 +198,16 @@ //! # extern crate rand; //! # extern crate ed25519_dalek; //! # #[cfg(feature = "serde")] -//! # extern crate serde; +//! # extern crate serde_crate as serde; //! # #[cfg(feature = "serde")] //! # extern crate bincode; //! # //! # #[cfg(feature = "serde")] //! # fn main() { //! # use rand::rngs::OsRng; -//! # use ed25519_dalek::{Keypair, Signature, PublicKey}; +//! # use ed25519_dalek::{Keypair, Signature, Signer, Verifier, PublicKey}; //! # use bincode::{serialize, Infinite}; -//! use bincode::{deserialize}; +//! use bincode::deserialize; //! //! # let mut csprng = OsRng{}; //! # let keypair: Keypair = Keypair::generate(&mut csprng); @@ -237,6 +239,8 @@ #[macro_use] extern crate std; +pub extern crate ed25519; + #[cfg(all(feature = "alloc", not(feature = "std")))] extern crate alloc; extern crate curve25519_dalek; @@ -245,20 +249,29 @@ extern crate merlin; #[cfg(any(feature = "batch", feature = "std", feature = "alloc", test))] extern crate rand; #[cfg(feature = "serde")] -extern crate serde; +extern crate serde_crate as serde; extern crate sha2; extern crate zeroize; #[cfg(all(any(feature = "batch", feature = "batch_deterministic"), any(feature = "std", feature = "alloc")))] mod batch; mod constants; -mod ed25519; +mod keypair; mod errors; mod public; mod secret; mod signature; -// Export everything public in ed25519. -pub use crate::ed25519::*; +pub use curve25519_dalek::digest::Digest; + #[cfg(all(any(feature = "batch", feature = "batch_deterministic"), any(feature = "std", feature = "alloc")))] pub use crate::batch::*; +pub use crate::constants::*; +pub use crate::errors::*; +pub use crate::keypair::*; +pub use crate::public::*; +pub use crate::secret::*; + +// Re-export the `Signer` and `Verifier` traits from the `signature` crate +pub use ed25519::signature::{Signer, Verifier}; +pub use ed25519::Signature; diff --git a/src/public.rs b/src/public.rs index f901fcf..0fb4188 100644 --- a/src/public.rs +++ b/src/public.rs @@ -9,6 +9,7 @@ //! ed25519 public keys. +use core::convert::TryFrom; use core::fmt::Debug; use curve25519_dalek::constants; @@ -18,6 +19,8 @@ use curve25519_dalek::edwards::CompressedEdwardsY; use curve25519_dalek::edwards::EdwardsPoint; use curve25519_dalek::scalar::Scalar; +use ed25519::signature::Verifier; + pub use sha2::Sha512; #[cfg(feature = "serde")] @@ -127,10 +130,10 @@ impl PublicKey { #[inline] pub fn from_bytes(bytes: &[u8]) -> Result { if bytes.len() != PUBLIC_KEY_LENGTH { - return Err(SignatureError(InternalError::BytesLengthError { + return Err(InternalError::BytesLengthError { name: "PublicKey", length: PUBLIC_KEY_LENGTH, - })); + }.into()); } let mut bits: [u8; 32] = [0u8; 32]; bits.copy_from_slice(&bytes[..32]); @@ -138,7 +141,7 @@ impl PublicKey { let compressed = CompressedEdwardsY(bits); let point = compressed .decompress() - .ok_or(SignatureError(InternalError::PointDecompressionError))?; + .ok_or(InternalError::PointDecompressionError)?; Ok(PublicKey(compressed, point)) } @@ -159,37 +162,6 @@ impl PublicKey { PublicKey(compressed, point) } - /// Verify a signature on a message with this keypair's public key. - /// - /// # Return - /// - /// Returns `Ok(())` if the signature is valid, and `Err` otherwise. - #[allow(non_snake_case)] - pub fn verify( - &self, - message: &[u8], - signature: &Signature - ) -> Result<(), SignatureError> - { - let mut h: Sha512 = Sha512::new(); - let R: EdwardsPoint; - let k: Scalar; - let minus_A: EdwardsPoint = -self.1; - - h.input(signature.R.as_bytes()); - h.input(self.as_bytes()); - h.input(&message); - - k = Scalar::from_hash(h); - R = EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(minus_A), &signature.s); - - if R.compress() == signature.R { - Ok(()) - } else { - Err(SignatureError(InternalError::VerifyError)) - } - } - /// Verify a `signature` on a `prehashed_message` using the Ed25519ph algorithm. /// /// # Inputs @@ -213,11 +185,13 @@ impl PublicKey { &self, prehashed_message: D, context: Option<&[u8]>, - signature: &Signature, + signature: &ed25519::Signature, ) -> Result<(), SignatureError> where D: Digest, { + let signature = InternalSignature::try_from(signature)?; + let mut h: Sha512 = Sha512::default(); let R: EdwardsPoint; let k: Scalar; @@ -241,7 +215,7 @@ impl PublicKey { if R.compress() == signature.R { Ok(()) } else { - Err(SignatureError(InternalError::VerifyError)) + Err(InternalError::VerifyError.into()) } } @@ -311,9 +285,11 @@ impl PublicKey { pub fn verify_strict( &self, message: &[u8], - signature: &Signature, + signature: &ed25519::Signature, ) -> Result<(), SignatureError> { + let signature = InternalSignature::try_from(signature)?; + let mut h: Sha512 = Sha512::new(); let R: EdwardsPoint; let k: Scalar; @@ -321,13 +297,13 @@ impl PublicKey { let signature_R: EdwardsPoint; match signature.R.decompress() { - None => return Err(SignatureError(InternalError::VerifyError)), + None => return Err(InternalError::VerifyError.into()), Some(x) => signature_R = x, } // Logical OR is fine here as we're not trying to be constant time. if signature_R.is_small_order() || self.1.is_small_order() { - return Err(SignatureError(InternalError::VerifyError)); + return Err(InternalError::VerifyError.into()); } h.input(signature.R.as_bytes()); @@ -340,7 +316,42 @@ impl PublicKey { if R == signature_R { Ok(()) } else { - Err(SignatureError(InternalError::VerifyError)) + Err(InternalError::VerifyError.into()) + } + } +} + +impl Verifier for PublicKey { + /// Verify a signature on a message with this keypair's public key. + /// + /// # Return + /// + /// Returns `Ok(())` if the signature is valid, and `Err` otherwise. + #[allow(non_snake_case)] + fn verify( + &self, + message: &[u8], + signature: &ed25519::Signature + ) -> Result<(), SignatureError> + { + let signature = InternalSignature::try_from(signature)?; + + let mut h: Sha512 = Sha512::new(); + let R: EdwardsPoint; + let k: Scalar; + let minus_A: EdwardsPoint = -self.1; + + h.input(signature.R.as_bytes()); + h.input(self.as_bytes()); + h.input(&message); + + k = Scalar::from_hash(h); + R = EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(minus_A), &signature.s); + + if R.compress() == signature.R { + Ok(()) + } else { + Err(InternalError::VerifyError.into()) } } } diff --git a/src/secret.rs b/src/secret.rs index f1e751d..5066569 100644 --- a/src/secret.rs +++ b/src/secret.rs @@ -106,10 +106,10 @@ impl SecretKey { #[inline] pub fn from_bytes(bytes: &[u8]) -> Result { if bytes.len() != SECRET_KEY_LENGTH { - return Err(SignatureError(InternalError::BytesLengthError { + return Err(InternalError::BytesLengthError { name: "SecretKey", length: SECRET_KEY_LENGTH, - })); + }.into()); } let mut bits: [u8; 32] = [0u8; 32]; bits.copy_from_slice(&bytes[..32]); @@ -383,10 +383,10 @@ impl ExpandedSecretKey { #[inline] pub fn from_bytes(bytes: &[u8]) -> Result { if bytes.len() != EXPANDED_SECRET_KEY_LENGTH { - return Err(SignatureError(InternalError::BytesLengthError { + return Err(InternalError::BytesLengthError { name: "ExpandedSecretKey", length: EXPANDED_SECRET_KEY_LENGTH, - })); + }.into()); } let mut lower: [u8; 32] = [0u8; 32]; let mut upper: [u8; 32] = [0u8; 32]; @@ -402,7 +402,7 @@ impl ExpandedSecretKey { /// Sign a message with this `ExpandedSecretKey`. #[allow(non_snake_case)] - pub fn sign(&self, message: &[u8], public_key: &PublicKey) -> Signature { + pub fn sign(&self, message: &[u8], public_key: &PublicKey) -> ed25519::Signature { let mut h: Sha512 = Sha512::new(); let R: CompressedEdwardsY; let r: Scalar; @@ -423,7 +423,7 @@ impl ExpandedSecretKey { k = Scalar::from_hash(h); s = &(&k * &self.key) + &r; - Signature { R, s } + InternalSignature { R, s }.into() } /// Sign a `prehashed_message` with this `ExpandedSecretKey` using the @@ -450,7 +450,7 @@ impl ExpandedSecretKey { prehashed_message: D, public_key: &PublicKey, context: Option<&'a [u8]>, - ) -> Signature + ) -> ed25519::Signature where D: Digest, { @@ -505,7 +505,7 @@ impl ExpandedSecretKey { k = Scalar::from_hash(h); s = &(&k * &self.key) + &r; - Signature { R, s } + InternalSignature { R, s }.into() } } diff --git a/src/signature.rs b/src/signature.rs index 59da225..c01e754 100644 --- a/src/signature.rs +++ b/src/signature.rs @@ -9,19 +9,12 @@ //! An ed25519 signature. +use core::convert::TryFrom; use core::fmt::Debug; use curve25519_dalek::edwards::CompressedEdwardsY; use curve25519_dalek::scalar::Scalar; - -#[cfg(feature = "serde")] -use serde::de::Error as SerdeError; -#[cfg(feature = "serde")] -use serde::de::Visitor; -#[cfg(feature = "serde")] -use serde::{Deserialize, Serialize}; -#[cfg(feature = "serde")] -use serde::{Deserializer, Serializer}; +use ed25519::signature::Signature as _; use crate::constants::*; use crate::errors::*; @@ -35,7 +28,7 @@ use crate::errors::*; /// been signed. #[allow(non_snake_case)] #[derive(Copy, Eq, PartialEq)] -pub struct Signature { +pub(crate) struct InternalSignature { /// `R` is an `EdwardsPoint`, formed by using an hash function with /// 512-bits output to produce the digest of: /// @@ -59,13 +52,13 @@ pub struct Signature { pub(crate) s: Scalar, } -impl Clone for Signature { +impl Clone for InternalSignature { fn clone(&self) -> Self { *self } } -impl Debug for Signature { +impl Debug for InternalSignature { fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { write!(f, "Signature( R: {:?}, s: {:?} )", &self.R, &self.s) } @@ -103,12 +96,12 @@ fn check_scalar(bytes: [u8; 32]) -> Result { } match Scalar::from_canonical_bytes(bytes) { - None => return Err(SignatureError(InternalError::ScalarFormatError)), + None => return Err(InternalError::ScalarFormatError.into()), Some(x) => return Ok(x), }; } -impl Signature { +impl InternalSignature { /// Convert this `Signature` to a byte array. #[inline] pub fn to_bytes(&self) -> [u8; SIGNATURE_LENGTH] { @@ -170,12 +163,12 @@ impl Signature { /// only checking the most significant three bits. (See also the /// documentation for `PublicKey.verify_strict`.) #[inline] - pub fn from_bytes(bytes: &[u8]) -> Result { + pub fn from_bytes(bytes: &[u8]) -> Result { if bytes.len() != SIGNATURE_LENGTH { - return Err(SignatureError(InternalError::BytesLengthError { + return Err(InternalError::BytesLengthError { name: "Signature", length: SIGNATURE_LENGTH, - })); + }.into()); } let mut lower: [u8; 32] = [0u8; 32]; let mut upper: [u8; 32] = [0u8; 32]; @@ -190,45 +183,23 @@ impl Signature { Err(x) => return Err(x), } - Ok(Signature { + Ok(InternalSignature { R: CompressedEdwardsY(lower), s: s, }) } } -#[cfg(feature = "serde")] -impl Serialize for Signature { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - serializer.serialize_bytes(&self.to_bytes()[..]) +impl TryFrom<&ed25519::Signature> for InternalSignature { + type Error = SignatureError; + + fn try_from(sig: &ed25519::Signature) -> Result { + InternalSignature::from_bytes(sig.as_bytes()) } } -#[cfg(feature = "serde")] -impl<'d> Deserialize<'d> for Signature { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'d>, - { - struct SignatureVisitor; - - impl<'d> Visitor<'d> for SignatureVisitor { - type Value = Signature; - - fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { - formatter.write_str("An ed25519 signature as 64 bytes, as specified in RFC8032.") - } - - fn visit_bytes(self, bytes: &[u8]) -> Result - where - E: SerdeError, - { - Signature::from_bytes(bytes).or(Err(SerdeError::invalid_length(bytes.len(), &self))) - } - } - deserializer.deserialize_bytes(SignatureVisitor) +impl From for ed25519::Signature { + fn from(sig: InternalSignature) -> ed25519::Signature { + ed25519::Signature::from_bytes(&sig.to_bytes()).unwrap() } } diff --git a/tests/ed25519.rs b/tests/ed25519.rs index 88a24df..2d42997 100644 --- a/tests/ed25519.rs +++ b/tests/ed25519.rs @@ -24,6 +24,8 @@ use sha2::Sha512; #[cfg(test)] mod vectors { + use ed25519::signature::Signature as _; + use std::io::BufReader; use std::io::BufRead; use std::fs::File; @@ -219,6 +221,8 @@ mod serialisation { use self::bincode::{serialize, serialized_size, deserialize, Infinite}; + use ed25519::signature::Signature as _; + static PUBLIC_KEY_BYTES: [u8; PUBLIC_KEY_LENGTH] = [ 130, 039, 155, 015, 062, 076, 188, 063, 124, 122, 026, 251, 233, 253, 225, 220, @@ -281,7 +285,7 @@ mod serialisation { #[test] fn serialize_signature_size() { let signature: Signature = Signature::from_bytes(&SIGNATURE_BYTES).unwrap(); - assert_eq!(serialized_size(&signature) as usize, 72); // These sizes are specific to bincode==1.0.1 + assert_eq!(serialized_size(&signature) as usize, 64); // These sizes are specific to bincode==1.0.1 } #[test] From 9e247c493c1b24727233e117dcd341a8cae13ea1 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 30 Jun 2020 22:40:24 +0000 Subject: [PATCH 10/22] Add additional tests for keypair (de)serialisation. --- Cargo.toml | 3 ++- tests/ed25519.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 2931ff5..2264509 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,7 @@ curve25519-dalek = { version = "2", default-features = false } merlin = { version = "1", default-features = false, optional = true, git = "https://github.com/isislovecruft/merlin", branch = "develop" } rand = { version = "0.7", default-features = false, optional = true } rand_core = { version = "0.5", default-features = false, optional = true } -serde = { version = "1.0", default-features = false, optional = true } +serde = { version = "1.0", default-features = false, optional = true, features = ["derive"] } sha2 = { version = "0.8", default-features = false } zeroize = { version = "1", default-features = false, features = ["zeroize_derive"] } @@ -35,6 +35,7 @@ hex = "^0.4" bincode = "^0.9" criterion = "0.3" rand = "0.7" +toml = "0.5" [[bench]] name = "ed25519_benchmarks" diff --git a/tests/ed25519.rs b/tests/ed25519.rs index 88a24df..3a480ad 100644 --- a/tests/ed25519.rs +++ b/tests/ed25519.rs @@ -15,6 +15,10 @@ extern crate ed25519_dalek; extern crate hex; extern crate sha2; extern crate rand; +#[cfg(all(test, feature = "serde"))] +extern crate serde; +#[cfg(all(test, feature = "serde"))] +extern crate toml; use ed25519_dalek::*; @@ -213,11 +217,19 @@ mod integrations { } } +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Serialize, Deserialize)] +struct Demo { + keypair: Keypair +} + #[cfg(all(test, feature = "serde"))] mod serialisation { use super::*; use self::bincode::{serialize, serialized_size, deserialize, Infinite}; + use self::toml; static PUBLIC_KEY_BYTES: [u8; PUBLIC_KEY_LENGTH] = [ 130, 039, 155, 015, 062, 076, 188, 063, @@ -242,6 +254,16 @@ mod serialisation { 216, 085, 134, 144, 129, 149, 041, 081, 063, 120, 126, 100, 092, 059, 050, 011, ]; + static KEYPAIR_BYTES: [u8; KEYPAIR_LENGTH] = [ + 239, 085, 017, 235, 167, 103, 034, 062, + 007, 010, 032, 146, 113, 039, 096, 174, + 003, 219, 232, 166, 240, 121, 167, 013, + 098, 238, 122, 116, 193, 114, 215, 213, + 175, 181, 075, 166, 224, 164, 140, 146, + 053, 120, 010, 037, 104, 094, 136, 225, + 249, 102, 171, 160, 097, 132, 015, 071, + 035, 056, 000, 074, 130, 168, 225, 071, ]; + #[test] fn serialize_deserialize_signature() { let signature: Signature = Signature::from_bytes(&SIGNATURE_BYTES).unwrap(); @@ -272,6 +294,28 @@ mod serialisation { } } + #[test] + fn serialize_deserialize_keypair_bincode() { + let keypair = Keypair::from_bytes(&KEYPAIR_BYTES).unwrap(); + let encoded_keypair: Vec = serialize(&keypair, Infinite).unwrap(); + let decoded_keypair: Keypair = deserialize(&encoded_keypair).unwrap(); + + for i in 0..64 { + assert_eq!(KEYPAIR_BYTES[i], decoded_keypair.to_bytes()[i]); + } + } + + #[test] + fn serialize_deserialize_keypair_toml() { + let demo = Demo { keypair: Keypair::from_bytes(&KEYPAIR_BYTES).unwrap() }; + + println!("\n\nWrite to toml"); + let demo_toml = toml::to_string(&demo).unwrap(); + println!("{}", demo_toml); + let demo_toml_rebuild: Result = toml::from_str(&demo_toml); + println!("{:?}", demo_toml_rebuild); + } + #[test] fn serialize_public_key_size() { let public_key: PublicKey = PublicKey::from_bytes(&PUBLIC_KEY_BYTES).unwrap(); From 3a9435df9457467344349c8f96a5a3b9a3fa94c5 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 13 Jul 2020 23:16:30 +0000 Subject: [PATCH 11/22] Fixup serde and ed25519 trait errors in tests/benches. --- Cargo.toml | 3 ++- benches/ed25519_benchmarks.rs | 1 + tests/ed25519.rs | 4 +++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 23196e9..eb6b73c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ features = ["nightly", "batch"] [dependencies] curve25519-dalek = { version = "2", default-features = false } ed25519 = { version = "1", default-features = false } -merlin = { version = "1", default-features = false, optional = true, git = "https://github.com/isislovecruft/merlin", branch = "develop" } +merlin = { version = "2", default-features = false, optional = true } rand = { version = "0.7", default-features = false, optional = true } rand_core = { version = "0.5", default-features = false, optional = true } serde_crate = { package = "serde", version = "1.0", default-features = false, optional = true } @@ -36,6 +36,7 @@ hex = "^0.4" bincode = "^0.9" criterion = "0.3" rand = "0.7" +serde_crate = { package = "serde", version = "1.0", features = ["derive"] } [[bench]] name = "ed25519_benchmarks" diff --git a/benches/ed25519_benchmarks.rs b/benches/ed25519_benchmarks.rs index 0af2812..45dce35 100644 --- a/benches/ed25519_benchmarks.rs +++ b/benches/ed25519_benchmarks.rs @@ -20,6 +20,7 @@ mod ed25519_benches { use ed25519_dalek::Keypair; use ed25519_dalek::PublicKey; use ed25519_dalek::Signature; + use ed25519_dalek::Signer; use ed25519_dalek::verify_batch; use rand::thread_rng; use rand::prelude::ThreadRng; diff --git a/tests/ed25519.rs b/tests/ed25519.rs index 8904a69..2b4d419 100644 --- a/tests/ed25519.rs +++ b/tests/ed25519.rs @@ -16,7 +16,7 @@ extern crate hex; extern crate sha2; extern crate rand; #[cfg(all(test, feature = "serde"))] -extern crate serde; +extern crate serde_crate; #[cfg(all(test, feature = "serde"))] extern crate toml; @@ -219,8 +219,10 @@ mod integrations { } } +#[cfg(all(test, feature = "serde"))] use serde::{Deserialize, Serialize}; +#[cfg(all(test, feature = "serde"))] #[derive(Debug, Serialize, Deserialize)] struct Demo { keypair: Keypair From f1d8576f12577fbc52c6192928e02361963fb524 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 13 Jul 2020 23:19:55 +0000 Subject: [PATCH 12/22] Impl std::error::Error for SignatureError. --- src/errors.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/errors.rs b/src/errors.rs index 108ca1f..08f04de 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -93,3 +93,6 @@ impl From for SignatureError { SignatureError::from_source(err) } } + +#[cfg(feature = "std")] +impl Error for SignatureError { } From 989c5e4c18d4d36c5ac849c462caa333934200c2 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Jul 2020 00:25:40 +0000 Subject: [PATCH 13/22] Fix ed25519ph context length error handling in sign_prehashed(). RFC8032 specifies that the context cannot be greater than 255 octets, but in the previous implementation in ed25519-dalek, this error would only be caught by a debug_assert. This changes the sign_prehashed() function to return a Result so that the error can be handled at runtime and the library no longer allows misuse by creating signatures that other libraries cannot handle. --- src/errors.rs | 4 ++++ src/secret.rs | 12 ++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index 08f04de..5a9182e 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -41,6 +41,8 @@ pub(crate) enum InternalError { ArrayLengthError{ name_a: &'static str, length_a: usize, name_b: &'static str, length_b: usize, name_c: &'static str, length_c: usize, }, + /// An ed25519ph signature can only take up to 255 octets of context. + PrehashedContextLengthError, } impl Display for InternalError { @@ -59,6 +61,8 @@ impl Display for InternalError { name_c: nc, length_c: lc, } => write!(f, "Arrays must be the same length: {} has length {}, {} has length {}, {} has length {}.", na, la, nb, lb, nc, lc), + InternalError::PrehashedContextError + => write!(f, "An ed25519ph signature can only take up to 255 octets of context"), } } } diff --git a/src/secret.rs b/src/secret.rs index 3579e2b..b305eed 100644 --- a/src/secret.rs +++ b/src/secret.rs @@ -441,7 +441,9 @@ impl ExpandedSecretKey { /// /// # Returns /// - /// An Ed25519ph [`Signature`] on the `prehashed_message`. + /// A `Result` whose `Ok` value is an Ed25519ph [`Signature`] on the + /// `prehashed_message` if the context was 255 bytes or less, otherwise + /// a `SignatureError`. /// /// [rfc8032]: https://tools.ietf.org/html/rfc8032#section-5.1 #[allow(non_snake_case)] @@ -450,7 +452,7 @@ impl ExpandedSecretKey { prehashed_message: D, public_key: &PublicKey, context: Option<&'a [u8]>, - ) -> ed25519::Signature + ) -> Result where D: Digest, { @@ -463,7 +465,9 @@ impl ExpandedSecretKey { let ctx: &[u8] = context.unwrap_or(b""); // By default, the context is an empty string. - debug_assert!(ctx.len() <= 255, "The context must not be longer than 255 octets."); + if ctx.len() > 255 { + return Err(SignatureError(InternalError::PrehashedContextError)); + } let ctx_len: u8 = ctx.len() as u8; @@ -505,7 +509,7 @@ impl ExpandedSecretKey { k = Scalar::from_hash(h); s = &(&k * &self.key) + &r; - InternalSignature { R, s }.into() + Ok(InternalSignature { R, s }.into()) } } From 97787d37160dac90a4ba3844ad611d4481c115de Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Jul 2020 00:37:51 +0000 Subject: [PATCH 14/22] Remove impl of std::error::Error for SignatureError. We're now aliasing SignatureError to the error type from the signature crate. --- src/errors.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index 08f04de..108ca1f 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -93,6 +93,3 @@ impl From for SignatureError { SignatureError::from_source(err) } } - -#[cfg(feature = "std")] -impl Error for SignatureError { } From 980ed6445fc698d40c1df39499f1cacb78138fe5 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Jul 2020 22:23:31 +0000 Subject: [PATCH 15/22] Add missing toml dev-dependency. --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index eb6b73c..1f16de7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ bincode = "^0.9" criterion = "0.3" rand = "0.7" serde_crate = { package = "serde", version = "1.0", features = ["derive"] } +toml = { version = "0.5" } [[bench]] name = "ed25519_benchmarks" From e7a88c2c7fac48bc23110f006ec516e6b24c64dc Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Jul 2020 23:58:35 +0000 Subject: [PATCH 16/22] Try compiling tests using serde_crate instead. --- tests/ed25519.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ed25519.rs b/tests/ed25519.rs index 2b4d419..b987f88 100644 --- a/tests/ed25519.rs +++ b/tests/ed25519.rs @@ -220,7 +220,7 @@ mod integrations { } #[cfg(all(test, feature = "serde"))] -use serde::{Deserialize, Serialize}; +use serde_crate::{Deserialize, Serialize}; #[cfg(all(test, feature = "serde"))] #[derive(Debug, Serialize, Deserialize)] From b8f36d48d8ac1a36b224e9daf741df36fc15455b Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Wed, 15 Jul 2020 17:39:23 +0000 Subject: [PATCH 17/22] Fix proc_macro crate name resolution for serde integration tests. --- tests/ed25519.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/ed25519.rs b/tests/ed25519.rs index b987f88..732b29d 100644 --- a/tests/ed25519.rs +++ b/tests/ed25519.rs @@ -219,11 +219,9 @@ mod integrations { } } +#[serde(crate = "serde_crate")] #[cfg(all(test, feature = "serde"))] -use serde_crate::{Deserialize, Serialize}; - -#[cfg(all(test, feature = "serde"))] -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, serde_crate::Serialize, serde_crate::Deserialize)] struct Demo { keypair: Keypair } From 69004599c54fc0d152c6b0a0eec2aafa4bcd3bf0 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Thu, 16 Jul 2020 21:49:14 +0000 Subject: [PATCH 18/22] Fix misnamed error type. --- src/secret.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/secret.rs b/src/secret.rs index b305eed..03af823 100644 --- a/src/secret.rs +++ b/src/secret.rs @@ -466,7 +466,7 @@ impl ExpandedSecretKey { let ctx: &[u8] = context.unwrap_or(b""); // By default, the context is an empty string. if ctx.len() > 255 { - return Err(SignatureError(InternalError::PrehashedContextError)); + return Err(SignatureError(InternalError::PrehashedContextLengthError)); } let ctx_len: u8 = ctx.len() as u8; From 7243d7151d204637ed226242b42348ad44b43f9b Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Thu, 16 Jul 2020 22:19:40 +0000 Subject: [PATCH 19/22] Fix handling of external error types. --- src/errors.rs | 5 ++++- src/keypair.rs | 27 ++++++++++++++++++++------- src/secret.rs | 2 +- tests/ed25519.rs | 6 +++--- 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index 8a35093..3194737 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -45,6 +45,9 @@ pub(crate) enum InternalError { PrehashedContextLengthError, } +unsafe impl Send for InternalError {} +unsafe impl Sync for InternalError {} + impl Display for InternalError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self { @@ -61,7 +64,7 @@ impl Display for InternalError { name_c: nc, length_c: lc, } => write!(f, "Arrays must be the same length: {} has length {}, {} has length {}, {} has length {}.", na, la, nb, lb, nc, lc), - InternalError::PrehashedContextError + InternalError::PrehashedContextLengthError => write!(f, "An ed25519ph signature can only take up to 255 octets of context"), } } diff --git a/src/keypair.rs b/src/keypair.rs index ae242d1..e4f2a4f 100644 --- a/src/keypair.rs +++ b/src/keypair.rs @@ -207,11 +207,11 @@ impl Keypair { /// # use ed25519_dalek::Digest; /// # use ed25519_dalek::Keypair; /// # use ed25519_dalek::Signature; + /// # use ed25519_dalek::SignatureError; /// # use ed25519_dalek::Sha512; /// # use rand::rngs::OsRng; /// # - /// # #[cfg(feature = "std")] - /// # fn main() { + /// # fn do_test() -> Result { /// # let mut csprng = OsRng{}; /// # let keypair: Keypair = Keypair::generate(&mut csprng); /// # let message: &[u8] = b"All I want is to pet all of the dogs."; @@ -220,7 +220,13 @@ impl Keypair { /// # /// let context: &[u8] = b"Ed25519DalekSignPrehashedDoctest"; /// - /// let sig: Signature = keypair.sign_prehashed(prehashed, Some(context)); + /// let sig: Signature = keypair.sign_prehashed(prehashed, Some(context))?; + /// # + /// # Ok(sig) + /// # } + /// # #[cfg(feature = "std")] + /// # fn main() { + /// # do_test(); /// # } /// # /// # #[cfg(not(feature = "std"))] @@ -233,7 +239,7 @@ impl Keypair { &self, prehashed_message: D, context: Option<&[u8]>, - ) -> ed25519::Signature + ) -> Result where D: Digest, { @@ -278,11 +284,11 @@ impl Keypair { /// use ed25519_dalek::Digest; /// use ed25519_dalek::Keypair; /// use ed25519_dalek::Signature; + /// use ed25519_dalek::SignatureError; /// use ed25519_dalek::Sha512; /// use rand::rngs::OsRng; /// - /// # #[cfg(feature = "std")] - /// # fn main() { + /// # fn do_test() -> Result<(), SignatureError> { /// let mut csprng = OsRng{}; /// let keypair: Keypair = Keypair::generate(&mut csprng); /// let message: &[u8] = b"All I want is to pet all of the dogs."; @@ -292,7 +298,7 @@ impl Keypair { /// /// let context: &[u8] = b"Ed25519DalekSignPrehashedDoctest"; /// - /// let sig: Signature = keypair.sign_prehashed(prehashed, Some(context)); + /// let sig: Signature = keypair.sign_prehashed(prehashed, Some(context))?; /// /// // The sha2::Sha512 struct doesn't implement Copy, so we'll have to create a new one: /// let mut prehashed_again: Sha512 = Sha512::default(); @@ -301,6 +307,13 @@ impl Keypair { /// let verified = keypair.public.verify_prehashed(prehashed_again, Some(context), &sig); /// /// assert!(verified.is_ok()); + /// + /// # verified + /// # } + /// # + /// # #[cfg(feature = "std")] + /// # fn main() { + /// # do_test(); /// # } /// # /// # #[cfg(not(feature = "std"))] diff --git a/src/secret.rs b/src/secret.rs index 03af823..ca57062 100644 --- a/src/secret.rs +++ b/src/secret.rs @@ -466,7 +466,7 @@ impl ExpandedSecretKey { let ctx: &[u8] = context.unwrap_or(b""); // By default, the context is an empty string. if ctx.len() > 255 { - return Err(SignatureError(InternalError::PrehashedContextLengthError)); + return Err(SignatureError::from_source(InternalError::PrehashedContextLengthError)); } let ctx_len: u8 = ctx.len() as u8; diff --git a/tests/ed25519.rs b/tests/ed25519.rs index 732b29d..b0e206f 100644 --- a/tests/ed25519.rs +++ b/tests/ed25519.rs @@ -104,7 +104,7 @@ mod vectors { prehash_for_signing.input(&msg_bytes[..]); prehash_for_verifying.input(&msg_bytes[..]); - let sig2: Signature = keypair.sign_prehashed(prehash_for_signing, None); + let sig2: Signature = keypair.sign_prehashed(prehash_for_signing, None).unwrap(); assert!(sig1 == sig2, "Original signature from test vectors doesn't equal signature produced:\ @@ -169,8 +169,8 @@ mod integrations { let context: &[u8] = b"testing testing 1 2 3"; keypair = Keypair::generate(&mut csprng); - good_sig = keypair.sign_prehashed(prehashed_good1, Some(context)); - bad_sig = keypair.sign_prehashed(prehashed_bad1, Some(context)); + good_sig = keypair.sign_prehashed(prehashed_good1, Some(context)).unwrap(); + bad_sig = keypair.sign_prehashed(prehashed_bad1, Some(context)).unwrap(); assert!(keypair.verify_prehashed(prehashed_good2, Some(context), &good_sig).is_ok(), "Verification of a valid signature failed!"); From d3a5b3bd8143a89132df34e1e7a6b184e3e41cb3 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Thu, 16 Jul 2020 23:02:27 +0000 Subject: [PATCH 20/22] Remove unsafe trait impls. --- src/errors.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index 3194737..b66fae0 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -45,9 +45,6 @@ pub(crate) enum InternalError { PrehashedContextLengthError, } -unsafe impl Send for InternalError {} -unsafe impl Sync for InternalError {} - impl Display for InternalError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self { From 5458ebef880d878e5bb8e08183d69286f6a69c75 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Thu, 16 Jul 2020 23:18:00 +0000 Subject: [PATCH 21/22] Fix no_std issue with new error types. --- src/secret.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/secret.rs b/src/secret.rs index ca57062..e1fa2c4 100644 --- a/src/secret.rs +++ b/src/secret.rs @@ -466,7 +466,7 @@ impl ExpandedSecretKey { let ctx: &[u8] = context.unwrap_or(b""); // By default, the context is an empty string. if ctx.len() > 255 { - return Err(SignatureError::from_source(InternalError::PrehashedContextLengthError)); + return Err(SignatureError::from(InternalError::PrehashedContextLengthError)); } let ctx_len: u8 = ctx.len() as u8; From 5f22d899a0fb680c77af7051d967e62d0f0859ee Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Thu, 16 Jul 2020 23:25:09 +0000 Subject: [PATCH 22/22] Bump ed25519-dalek version to 1.0.0-pre.4. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 1f16de7..d1bf7ad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ed25519-dalek" -version = "1.0.0-pre.3" +version = "1.0.0-pre.4" edition = "2018" authors = ["isis lovecruft "] readme = "README.md"