From bb82d616de9f4d267e9fe4f68104d4279010812f Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Tue, 4 Aug 2020 11:58:07 -0700 Subject: [PATCH 01/12] Make `use rand::...` gated on `cfg(feature = "rand")` This is no longer actively breaking our no_std build, but I think it's still technically a minor bug, and further case of issue #108 --- src/secret.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/secret.rs b/src/secret.rs index e1fa2c4..b09ff64 100644 --- a/src/secret.rs +++ b/src/secret.rs @@ -17,6 +17,7 @@ use curve25519_dalek::digest::Digest; use curve25519_dalek::edwards::CompressedEdwardsY; use curve25519_dalek::scalar::Scalar; +#[cfg(feature = "rand")] use rand::{CryptoRng, RngCore}; use sha2::Sha512; From da959c041d62ec4237c604a3b3e55a08e0a0df25 Mon Sep 17 00:00:00 2001 From: Ivan Temchenko <35359595i@gmail.com> Date: Mon, 24 Aug 2020 16:33:04 +0200 Subject: [PATCH 02/12] check_scalar bug fix for legacy_compatibility feature --- src/signature.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/signature.rs b/src/signature.rs index c01e754..880a78b 100644 --- a/src/signature.rs +++ b/src/signature.rs @@ -74,7 +74,7 @@ fn check_scalar(bytes: [u8; 32]) -> Result { // This is compatible with ed25519-donna and libsodium when // -DED25519_COMPAT is NOT specified. if bytes[31] & 224 != 0 { - return Err(SignatureError(InternalError::ScalarFormatError)); + return Err(InternalError::ScalarFormatError.into()); } Ok(Scalar::from_bits(bytes)) From 57a5473cb0b6024d250674d1308a6f07802b4bd0 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 21 Sep 2020 22:04:18 +0000 Subject: [PATCH 03/12] Fix and document malleability issue in deterministic batch_verify(). Thank you to @real_or_random and @jonasnick for initially pointing it out and ensuing discussion. --- src/batch.rs | 81 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 74 insertions(+), 7 deletions(-) diff --git a/src/batch.rs b/src/batch.rs index 4d15589..6a4a7c6 100644 --- a/src/batch.rs +++ b/src/batch.rs @@ -43,19 +43,24 @@ use crate::public::PublicKey; use crate::signature::InternalSignature; trait BatchTranscript { - fn append_hrams(&mut self, hrams: &Vec); + fn append_scalars(&mut self, scalars: &Vec); fn append_message_lengths(&mut self, message_lengths: &Vec); } impl BatchTranscript for Transcript { - /// Add all the computed `H(R||A||M)`s to the protocol transcript. + /// Append some `scalars` to this batch verification sigma protocol transcript. + /// + /// For ed25519 batch verification, we include the following as scalars: + /// + /// * All of the computed `H(R||A||M)`s to the protocol transcript, and + /// * All of the `s` components of each signature. /// /// Each is also prefixed with their index in the vector. - fn append_hrams(&mut self, hrams: &Vec) { - for (i, hram) in hrams.iter().enumerate() { + fn append_scalars(&mut self, scalars: &Vec) { + for (i, scalar) in scalars.iter().enumerate() { // XXX add message length into transcript self.append_u64(b"", i as u64); - self.append_message(b"hram", hram.as_bytes()); + self.append_message(b"hram", scalar.as_bytes()); } } @@ -121,6 +126,65 @@ fn zero_rng() -> ZeroRng { /// `SignatureError` containing a description of the internal error which /// occured. /// +/// # Notes on Nonce Generation & Malleability +/// +/// ## On Synthetic Nonces +/// +/// This library defaults to using what is called "synthetic" nonces, which +/// means that a mixture of deterministic (per any unique set of inputs to this +/// function) data and system randomness is used to seed the CSPRNG for nonce +/// generation. For more of the background theory on why many cryptographers +/// currently believe this to be superior to either purely deterministic +/// generation or purely relying on the system's randomness, see [this section +/// of the Merlin design](https://merlin.cool/transcript/rng.html) by Henry de +/// Valence, isis lovecruft, and Oleg Andreev, as well as Trevor Perrin's +/// [designs for generalised +/// EdDSA](https://moderncrypto.org/mail-archive/curves/2017/000925.html). +/// +/// ## On Deterministic Nonces +/// +/// In order to be ammenable to protocols which require stricter third-party +/// auditability trails, such as in some financial cryptographic settings, this +/// library also supports a `--features=batch_deterministic` setting, where the +/// nonces for batch signature verification are derived purely from the inputs +/// to this function themselves. +/// +/// **This is not recommended for use unless you have several cryptographers on +/// staff who can advise you in its usage and all the horrible, terrible, +/// awful ways it can go horribly, terribly, awfully wrong.** +/// +/// In any sigma protocol it is wise to include as much context pertaining +/// to the public state in the protocol as possible, to avoid malleability +/// attacks where an adversary alters publics in an algebraic manner that +/// manages to satisfy the equations for the protocol in question. +/// +/// For ed25519 batch verification (both with synthetic and deterministic nonce +/// generation), we include the following as scalars in the protocol transcript: +/// +/// * All of the computed `H(R||A||M)`s to the protocol transcript, and +/// * All of the `s` components of each signature. +/// +/// Each is also prefixed with their index in the vector. +/// +/// The former, while not quite as elegant as adding the `R`s, `A`s, and +/// `M`s separately, saves us a bit of context hashing since the +/// `H(R||A||M)`s need to be computed for the verification equation anyway. +/// +/// The latter prevents a malleability attack only found in deterministic batch +/// signature verification (i.e. only when compiling `ed25519-dalek` with +/// `--features batch_deterministic`) wherein an adversary, without access +/// to the signing key(s), can take any valid signature, `(s,R)`, and swap +/// `s` with `s' = -z1`. This doesn't contitute a signature forgery, merely +/// a vulnerability, as the resulting signature will not pass single +/// signature verification. (Thanks to Github users @real_or_random and +/// @jonasnick for pointing out this malleability issue.) +/// +/// For an additional way in which signatures can be made to probablistically +/// falsely "pass" the synthethic batch verification equation *for the same +/// inputs*, but *only some crafted inputs* will pass the deterministic batch +/// single, and neither of these will ever pass single signature verification, +/// see the documentation for [`PublicKey.validate()`]. +/// /// # Examples /// /// ``` @@ -181,8 +245,10 @@ pub fn verify_batch( Scalar::from_hash(h) }).collect(); - // Collect the message lengths to add into the transcript. + // Collect the message lengths and the scalar portions of the signatures, + // and add them into the transcript. let message_lengths: Vec = messages.iter().map(|i| i.len()).collect(); + let scalars: Vec = signatures.iter().map(|i| i.s).collect(); // Build a PRNG based on a transcript of the H(R || A || M)s seen thus far. // This provides synthethic randomness in the default configuration, and @@ -190,8 +256,9 @@ pub fn verify_batch( // "batch_deterministic" feature. let mut transcript: Transcript = Transcript::new(b"ed25519 batch verification"); - transcript.append_hrams(&hrams); + transcript.append_scalars(&hrams); transcript.append_message_lengths(&message_lengths); + transcript.append_scalars(&scalars); #[cfg(all(feature = "batch", not(feature = "batch_deterministic")))] let mut prng = transcript.build_rng().finalize(&mut thread_rng()); From a02190adf3a835a49165877997bed61cae9415fa Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 21 Sep 2020 22:05:59 +0000 Subject: [PATCH 04/12] Document that we include the message lengths in the transcript. --- src/batch.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/batch.rs b/src/batch.rs index 6a4a7c6..3a4b8e9 100644 --- a/src/batch.rs +++ b/src/batch.rs @@ -58,12 +58,17 @@ impl BatchTranscript for Transcript { /// Each is also prefixed with their index in the vector. fn append_scalars(&mut self, scalars: &Vec) { for (i, scalar) in scalars.iter().enumerate() { - // XXX add message length into transcript self.append_u64(b"", i as u64); self.append_message(b"hram", scalar.as_bytes()); } } + /// Append the lengths of the messages into the transcript. + /// + /// This is done out of an (potential over-)abundance of caution, to guard + /// against the unlikely event of collisions. However, a nicer way to do + /// this would be to append the message length before the message, but this + /// is messy w.r.t. the calculations of the `H(R||A||M)`s above. fn append_message_lengths(&mut self, message_lengths: &Vec) { for (i, len) in message_lengths.iter().enumerate() { self.append_u64(b"", i as u64); From 5d7bc29ba2ff725be9a198c8f3ab4ff9c6d0985a Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 21 Sep 2020 23:25:15 +0000 Subject: [PATCH 05/12] Workaround for rand crate "nightly" feature breakage. Cf. https://github.com/rust-random/rand/issues/1047 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index ecc1bd3..ade7645 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,7 +50,7 @@ harness = false default = ["std", "u64_backend"] 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"] +nightly = ["curve25519-dalek/nightly"] serde = ["serde_crate", "ed25519/serde"] batch = ["merlin", "rand"] # This feature enables deterministic batch verification. From 660964203651a816a08d0a3cf77a55e3f1ff3e7d Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 21 Sep 2020 23:48:57 +0000 Subject: [PATCH 06/12] Enable rand crate by default. See https://github.com/dalek-cryptography/ed25519-dalek/pull/139. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index ade7645..37287e0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,7 +47,7 @@ harness = false # required-features = ["batch"] [features] -default = ["std", "u64_backend"] +default = ["std", "rand", "u64_backend"] 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"] From b5a15bf4518ca0e6ec19a068241877d448a78573 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 21 Sep 2020 23:52:21 +0000 Subject: [PATCH 07/12] Feature gate key generation on the "rand" dependency. See https://github.com/dalek-cryptography/ed25519-dalek/pull/139. --- src/keypair.rs | 1 + src/secret.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/keypair.rs b/src/keypair.rs index f4024a1..c12f751 100644 --- a/src/keypair.rs +++ b/src/keypair.rs @@ -125,6 +125,7 @@ impl Keypair { /// The standard hash function used for most ed25519 libraries is SHA-512, /// which is available with `use sha2::Sha512` as in the example above. /// Other suitable hash functions include Keccak-512 and Blake2b-512. + #[cfg(feature = "rand")] pub fn generate(csprng: &mut R) -> Keypair where R: CryptoRng + RngCore, diff --git a/src/secret.rs b/src/secret.rs index 1d421b6..64c9d1f 100644 --- a/src/secret.rs +++ b/src/secret.rs @@ -165,6 +165,7 @@ impl SecretKey { /// # Input /// /// A CSPRNG with a `fill_bytes()` method, e.g. `rand::OsRng` + #[cfg(feature = "rand")] pub fn generate(csprng: &mut T) -> SecretKey where T: CryptoRng + RngCore, From 008c9680f669f54129f785be4b39dec1ce2119eb Mon Sep 17 00:00:00 2001 From: Cheng XU Date: Fri, 7 Aug 2020 13:39:57 -0700 Subject: [PATCH 08/12] Update tests for serde * Upgrade bincode to 1.0 * Add more serde tests including json serialization. --- Cargo.toml | 3 +- src/lib.rs | 12 ++--- tests/ed25519.rs | 114 ++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 102 insertions(+), 27 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 37287e0..35592f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,8 @@ zeroize = { version = "1", default-features = false, features = ["zeroize_derive [dev-dependencies] hex = "^0.4" -bincode = "^0.9" +bincode = "1.0" +serde_json = "1.0" criterion = "0.3" rand = "0.7" serde_crate = { package = "serde", version = "1.0", features = ["derive"] } diff --git a/src/lib.rs b/src/lib.rs index 22cd7e9..26c161e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -176,7 +176,7 @@ //! # fn main() { //! # use rand::rngs::OsRng; //! # use ed25519_dalek::{Keypair, Signature, Signer, Verifier, PublicKey}; -//! use bincode::{serialize, Infinite}; +//! use bincode::serialize; //! # let mut csprng = OsRng{}; //! # let keypair: Keypair = Keypair::generate(&mut csprng); //! # let message: &[u8] = b"This is a test of the tsunami alert system."; @@ -184,8 +184,8 @@ //! # let public_key: PublicKey = keypair.public; //! # let verified: bool = public_key.verify(message, &signature).is_ok(); //! -//! let encoded_public_key: Vec = serialize(&public_key, Infinite).unwrap(); -//! let encoded_signature: Vec = serialize(&signature, Infinite).unwrap(); +//! let encoded_public_key: Vec = serialize(&public_key).unwrap(); +//! let encoded_signature: Vec = serialize(&signature).unwrap(); //! # } //! # #[cfg(not(feature = "serde"))] //! # fn main() {} @@ -206,7 +206,7 @@ //! # fn main() { //! # use rand::rngs::OsRng; //! # use ed25519_dalek::{Keypair, Signature, Signer, Verifier, PublicKey}; -//! # use bincode::{serialize, Infinite}; +//! # use bincode::serialize; //! use bincode::deserialize; //! //! # let mut csprng = OsRng{}; @@ -215,8 +215,8 @@ //! # let signature: Signature = keypair.sign(message); //! # let public_key: PublicKey = keypair.public; //! # let verified: bool = public_key.verify(message, &signature).is_ok(); -//! # let encoded_public_key: Vec = serialize(&public_key, Infinite).unwrap(); -//! # let encoded_signature: Vec = serialize(&signature, Infinite).unwrap(); +//! # let encoded_public_key: Vec = serialize(&public_key).unwrap(); +//! # let encoded_signature: Vec = serialize(&signature).unwrap(); //! let decoded_public_key: PublicKey = deserialize(&encoded_public_key).unwrap(); //! let decoded_signature: Signature = deserialize(&encoded_signature).unwrap(); //! diff --git a/tests/ed25519.rs b/tests/ed25519.rs index 4ed2a8b..696e287 100644 --- a/tests/ed25519.rs +++ b/tests/ed25519.rs @@ -230,11 +230,11 @@ struct Demo { mod serialisation { use super::*; - use self::bincode::{serialize, serialized_size, deserialize, Infinite}; - use self::toml; - use ed25519::signature::Signature as _; + // The size for bincode to serialize the length of a byte array. + static BINCODE_INT_LENGTH: usize = 8; + static PUBLIC_KEY_BYTES: [u8; PUBLIC_KEY_LENGTH] = [ 130, 039, 155, 015, 062, 076, 188, 063, 124, 122, 026, 251, 233, 253, 225, 220, @@ -269,42 +269,104 @@ mod serialisation { 035, 056, 000, 074, 130, 168, 225, 071, ]; #[test] - fn serialize_deserialize_signature() { + fn serialize_deserialize_signature_bincode() { + let signature: Signature = Signature::from_bytes(&SIGNATURE_BYTES).unwrap(); + let encoded_signature: Vec = bincode::serialize(&signature).unwrap(); + let decoded_signature: Signature = bincode::deserialize(&encoded_signature).unwrap(); + + assert_eq!(signature, decoded_signature); + } + + #[test] + fn serialize_deserialize_signature_json() { let signature: Signature = Signature::from_bytes(&SIGNATURE_BYTES).unwrap(); - let encoded_signature: Vec = serialize(&signature, Infinite).unwrap(); - let decoded_signature: Signature = deserialize(&encoded_signature).unwrap(); + let encoded_signature = serde_json::to_string(&signature).unwrap(); + let decoded_signature: Signature = serde_json::from_str(&encoded_signature).unwrap(); assert_eq!(signature, decoded_signature); } #[test] - fn serialize_deserialize_public_key() { + fn serialize_deserialize_public_key_bincode() { let public_key: PublicKey = PublicKey::from_bytes(&PUBLIC_KEY_BYTES).unwrap(); - let encoded_public_key: Vec = serialize(&public_key, Infinite).unwrap(); - let decoded_public_key: PublicKey = deserialize(&encoded_public_key).unwrap(); + let encoded_public_key: Vec = bincode::serialize(&public_key).unwrap(); + let decoded_public_key: PublicKey = bincode::deserialize(&encoded_public_key).unwrap(); - assert_eq!(&PUBLIC_KEY_BYTES[..], &encoded_public_key[encoded_public_key.len() - 32..]); + assert_eq!(&PUBLIC_KEY_BYTES[..], &encoded_public_key[encoded_public_key.len() - PUBLIC_KEY_LENGTH..]); assert_eq!(public_key, decoded_public_key); } #[test] - fn serialize_deserialize_secret_key() { + fn serialize_deserialize_public_key_json() { + let public_key: PublicKey = PublicKey::from_bytes(&PUBLIC_KEY_BYTES).unwrap(); + let encoded_public_key = serde_json::to_string(&public_key).unwrap(); + let decoded_public_key: PublicKey = serde_json::from_str(&encoded_public_key).unwrap(); + + assert_eq!(public_key, decoded_public_key); + } + + #[test] + fn serialize_deserialize_secret_key_bincode() { let secret_key: SecretKey = SecretKey::from_bytes(&SECRET_KEY_BYTES).unwrap(); - let encoded_secret_key: Vec = serialize(&secret_key, Infinite).unwrap(); - let decoded_secret_key: SecretKey = deserialize(&encoded_secret_key).unwrap(); + let encoded_secret_key: Vec = bincode::serialize(&secret_key).unwrap(); + let decoded_secret_key: SecretKey = bincode::deserialize(&encoded_secret_key).unwrap(); - for i in 0..32 { + for i in 0..SECRET_KEY_LENGTH { assert_eq!(SECRET_KEY_BYTES[i], decoded_secret_key.as_bytes()[i]); } } + #[test] + fn serialize_deserialize_secret_key_json() { + let secret_key: SecretKey = SecretKey::from_bytes(&SECRET_KEY_BYTES).unwrap(); + let encoded_secret_key = serde_json::to_string(&secret_key).unwrap(); + let decoded_secret_key: SecretKey = serde_json::from_str(&encoded_secret_key).unwrap(); + + for i in 0..SECRET_KEY_LENGTH { + assert_eq!(SECRET_KEY_BYTES[i], decoded_secret_key.as_bytes()[i]); + } + } + + #[test] + fn serialize_deserialize_expanded_secret_key_bincode() { + let expanded_secret_key = ExpandedSecretKey::from(&SecretKey::from_bytes(&SECRET_KEY_BYTES).unwrap()); + let encoded_expanded_secret_key: Vec = bincode::serialize(&expanded_secret_key).unwrap(); + let decoded_expanded_secret_key: ExpandedSecretKey = bincode::deserialize(&encoded_expanded_secret_key).unwrap(); + + for i in 0..EXPANDED_SECRET_KEY_LENGTH { + assert_eq!(expanded_secret_key.to_bytes()[i], decoded_expanded_secret_key.to_bytes()[i]); + } + } + + #[test] + fn serialize_deserialize_expanded_secret_key_json() { + let expanded_secret_key = ExpandedSecretKey::from(&SecretKey::from_bytes(&SECRET_KEY_BYTES).unwrap()); + let encoded_expanded_secret_key = serde_json::to_string(&expanded_secret_key).unwrap(); + let decoded_expanded_secret_key: ExpandedSecretKey = serde_json::from_str(&encoded_expanded_secret_key).unwrap(); + + for i in 0..EXPANDED_SECRET_KEY_LENGTH { + assert_eq!(expanded_secret_key.to_bytes()[i], decoded_expanded_secret_key.to_bytes()[i]); + } + } + #[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(); + let encoded_keypair: Vec = bincode::serialize(&keypair).unwrap(); + let decoded_keypair: Keypair = bincode::deserialize(&encoded_keypair).unwrap(); + + for i in 0..KEYPAIR_LENGTH { + assert_eq!(KEYPAIR_BYTES[i], decoded_keypair.to_bytes()[i]); + } + } + + #[test] + fn serialize_deserialize_keypair_json() { + let keypair = Keypair::from_bytes(&KEYPAIR_BYTES).unwrap(); + let encoded_keypair = serde_json::to_string(&keypair).unwrap(); + let decoded_keypair: Keypair = serde_json::from_str(&encoded_keypair).unwrap(); - for i in 0..64 { + for i in 0..KEYPAIR_LENGTH { assert_eq!(KEYPAIR_BYTES[i], decoded_keypair.to_bytes()[i]); } } @@ -323,18 +385,30 @@ mod serialisation { #[test] fn serialize_public_key_size() { let public_key: PublicKey = PublicKey::from_bytes(&PUBLIC_KEY_BYTES).unwrap(); - assert_eq!(serialized_size(&public_key) as usize, 40); // These sizes are specific to bincode==1.0.1 + assert_eq!(bincode::serialized_size(&public_key).unwrap() as usize, BINCODE_INT_LENGTH + PUBLIC_KEY_LENGTH); } #[test] fn serialize_signature_size() { let signature: Signature = Signature::from_bytes(&SIGNATURE_BYTES).unwrap(); - assert_eq!(serialized_size(&signature) as usize, 64); // These sizes are specific to bincode==1.0.1 + assert_eq!(bincode::serialized_size(&signature).unwrap() as usize, SIGNATURE_LENGTH); } #[test] fn serialize_secret_key_size() { let secret_key: SecretKey = SecretKey::from_bytes(&SECRET_KEY_BYTES).unwrap(); - assert_eq!(serialized_size(&secret_key) as usize, 40); // These sizes are specific to bincode==1.0.1 + assert_eq!(bincode::serialized_size(&secret_key).unwrap() as usize, BINCODE_INT_LENGTH + SECRET_KEY_LENGTH); + } + + #[test] + fn serialize_expanded_secret_key_size() { + let expanded_secret_key = ExpandedSecretKey::from(&SecretKey::from_bytes(&SECRET_KEY_BYTES).unwrap()); + assert_eq!(bincode::serialized_size(&expanded_secret_key).unwrap() as usize, BINCODE_INT_LENGTH + EXPANDED_SECRET_KEY_LENGTH); + } + + #[test] + fn serialize_keypair_size() { + let keypair = Keypair::from_bytes(&KEYPAIR_BYTES).unwrap(); + assert_eq!(bincode::serialized_size(&keypair).unwrap() as usize, BINCODE_INT_LENGTH + KEYPAIR_LENGTH); } } From 69eccda4449b564aff57a776c6a0ed51fce01123 Mon Sep 17 00:00:00 2001 From: Cheng XU Date: Mon, 21 Sep 2020 18:26:16 -0700 Subject: [PATCH 09/12] Fix serde implementation for serde_json We use the [serde_bytes](https://github.com/serde-rs/bytes) crate for serialization implementations, which simplifies codes and fixes issues for serde_json. --- Cargo.toml | 3 ++- src/keypair.rs | 69 +++++--------------------------------------------- src/public.rs | 29 ++++----------------- src/secret.rs | 52 +++++++------------------------------ 4 files changed, 22 insertions(+), 131 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 35592f6..d154b14 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,7 @@ 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 } +serde_bytes = { version = "0.11", optional = true } sha2 = { version = "0.9", default-features = false } zeroize = { version = "1", default-features = false, features = ["zeroize_derive"] } @@ -52,7 +53,7 @@ default = ["std", "rand", "u64_backend"] 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"] -serde = ["serde_crate", "ed25519/serde"] +serde = ["serde_crate", "serde_bytes", "ed25519/serde"] batch = ["merlin", "rand"] # This feature enables deterministic batch verification. batch_deterministic = ["merlin", "rand", "rand_core"] diff --git a/src/keypair.rs b/src/keypair.rs index c12f751..55af2df 100644 --- a/src/keypair.rs +++ b/src/keypair.rs @@ -15,11 +15,9 @@ use rand::{CryptoRng, RngCore}; #[cfg(feature = "serde")] 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}; +#[cfg(feature = "serde")] +use serde_bytes::{Bytes as SerdeBytes, ByteBuf as SerdeByteBuf}; pub use sha2::Sha512; @@ -428,7 +426,8 @@ impl Serialize for Keypair { where S: Serializer, { - serializer.serialize_bytes(&self.to_bytes()[..]) + let bytes = &self.to_bytes()[..]; + SerdeBytes::new(bytes).serialize(serializer) } } @@ -438,63 +437,7 @@ impl<'d> Deserialize<'d> for Keypair { where D: Deserializer<'d>, { - struct KeypairVisitor; - - impl<'d> Visitor<'d> for KeypairVisitor { - type Value = Keypair; - - fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { - formatter.write_str("An ed25519 keypair, 64 bytes in total where the secret key is \ - the first 32 bytes and is in unexpanded form, and the second \ - 32 bytes is a compressed point for a public key.") - } - - fn visit_bytes(self, bytes: &[u8]) -> Result - 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 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) + let bytes = ::deserialize(deserializer)?; + Keypair::from_bytes(bytes.as_ref()).map_err(SerdeError::custom) } } diff --git a/src/public.rs b/src/public.rs index 170390d..342adf6 100644 --- a/src/public.rs +++ b/src/public.rs @@ -26,11 +26,9 @@ pub use sha2::Sha512; #[cfg(feature = "serde")] use serde::de::Error as SerdeError; #[cfg(feature = "serde")] -use serde::de::Visitor; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[cfg(feature = "serde")] -use serde::{Deserialize, Serialize}; -#[cfg(feature = "serde")] -use serde::{Deserializer, Serializer}; +use serde_bytes::{Bytes as SerdeBytes, ByteBuf as SerdeByteBuf}; use crate::constants::*; use crate::errors::*; @@ -362,7 +360,7 @@ impl Serialize for PublicKey { where S: Serializer, { - serializer.serialize_bytes(self.as_bytes()) + SerdeBytes::new(self.as_bytes()).serialize(serializer) } } @@ -372,24 +370,7 @@ impl<'d> Deserialize<'d> for PublicKey { where D: Deserializer<'d>, { - struct PublicKeyVisitor; - - impl<'d> Visitor<'d> for PublicKeyVisitor { - type Value = PublicKey; - - fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { - formatter.write_str( - "An ed25519 public key as a 32-byte compressed point, as specified in RFC8032", - ) - } - - fn visit_bytes(self, bytes: &[u8]) -> Result - where - E: SerdeError, - { - PublicKey::from_bytes(bytes).or(Err(SerdeError::invalid_length(bytes.len(), &self))) - } - } - deserializer.deserialize_bytes(PublicKeyVisitor) + let bytes = ::deserialize(deserializer)?; + PublicKey::from_bytes(bytes.as_ref()).map_err(SerdeError::custom) } } diff --git a/src/secret.rs b/src/secret.rs index 64c9d1f..2ca3a12 100644 --- a/src/secret.rs +++ b/src/secret.rs @@ -25,11 +25,9 @@ use sha2::Sha512; #[cfg(feature = "serde")] use serde::de::Error as SerdeError; #[cfg(feature = "serde")] -use serde::de::Visitor; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[cfg(feature = "serde")] -use serde::{Deserialize, Serialize}; -#[cfg(feature = "serde")] -use serde::{Deserializer, Serializer}; +use serde_bytes::{Bytes as SerdeBytes, ByteBuf as SerdeByteBuf}; use zeroize::Zeroize; @@ -184,7 +182,7 @@ impl Serialize for SecretKey { where S: Serializer, { - serializer.serialize_bytes(self.as_bytes()) + SerdeBytes::new(self.as_bytes()).serialize(serializer) } } @@ -194,23 +192,8 @@ impl<'d> Deserialize<'d> for SecretKey { where D: Deserializer<'d>, { - struct SecretKeyVisitor; - - impl<'d> Visitor<'d> for SecretKeyVisitor { - type Value = SecretKey; - - fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { - formatter.write_str("An ed25519 secret key as 32 bytes, as specified in RFC8032.") - } - - fn visit_bytes(self, bytes: &[u8]) -> Result - where - E: SerdeError, - { - SecretKey::from_bytes(bytes).or(Err(SerdeError::invalid_length(bytes.len(), &self))) - } - } - deserializer.deserialize_bytes(SecretKeyVisitor) + let bytes = ::deserialize(deserializer)?; + SecretKey::from_bytes(bytes.as_ref()).map_err(SerdeError::custom) } } @@ -521,7 +504,8 @@ impl Serialize for ExpandedSecretKey { where S: Serializer, { - serializer.serialize_bytes(&self.to_bytes()[..]) + let bytes = &self.to_bytes()[..]; + SerdeBytes::new(bytes).serialize(serializer) } } @@ -531,26 +515,8 @@ impl<'d> Deserialize<'d> for ExpandedSecretKey { where D: Deserializer<'d>, { - struct ExpandedSecretKeyVisitor; - - impl<'d> Visitor<'d> for ExpandedSecretKeyVisitor { - type Value = ExpandedSecretKey; - - fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { - formatter.write_str( - "An ed25519 expanded secret key as 64 bytes, as specified in RFC8032.", - ) - } - - fn visit_bytes(self, bytes: &[u8]) -> Result - where - E: SerdeError, - { - ExpandedSecretKey::from_bytes(bytes) - .or(Err(SerdeError::invalid_length(bytes.len(), &self))) - } - } - deserializer.deserialize_bytes(ExpandedSecretKeyVisitor) + let bytes = ::deserialize(deserializer)?; + ExpandedSecretKey::from_bytes(bytes.as_ref()).map_err(SerdeError::custom) } } From d6ff6de2cff8af36fef8933dd81dc5100121dee3 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 22 Sep 2020 01:36:49 +0000 Subject: [PATCH 10/12] Add #![forbid(unsafe_code)]. CLOSES https://github.com/dalek-cryptography/ed25519-dalek/issues/144 --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index 22cd7e9..8f586ec 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -234,6 +234,7 @@ #![no_std] #![warn(future_incompatible)] #![deny(missing_docs)] // refuse to compile if documentation is missing +#![forbid(unsafe_code)] #[cfg(any(feature = "std", test))] #[macro_use] From 8c15bce61d157e55148b1056f4726870b4d528f3 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 22 Sep 2020 01:54:44 +0000 Subject: [PATCH 11/12] Actually, we use unsafe{} in one test. --- src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 29c8c1c..88dfc93 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -234,6 +234,8 @@ #![no_std] #![warn(future_incompatible)] #![deny(missing_docs)] // refuse to compile if documentation is missing + +#![cfg(not(test))] #![forbid(unsafe_code)] #[cfg(any(feature = "std", test))] From 1042cb60a07cdaacb59ca209716b69f444460f8f Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 22 Sep 2020 01:56:35 +0000 Subject: [PATCH 12/12] Bump ed25519-dalek version to 1.0.1. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index d154b14..94d9f96 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ed25519-dalek" -version = "1.0.0" +version = "1.0.1" edition = "2018" authors = ["isis lovecruft "] readme = "README.md"