Merge branch 'release/1.0.1'

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ed25519-dalek"
-version = "1.0.0"
+version = "1.0.1"
 edition = "2018"
 authors = ["isis lovecruft <[email protected]>"]
 readme = "README.md"
@@ -28,12 +28,14 @@ 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"] }
 
 [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"] }
@@ -47,11 +49,11 @@ 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", "rand/nightly"]
-serde = ["serde_crate", "ed25519/serde"]
+nightly = ["curve25519-dalek/nightly"]
+serde = ["serde_crate", "serde_bytes", "ed25519/serde"]
 batch = ["merlin", "rand"]
 # This feature enables deterministic batch verification.
 batch_deterministic = ["merlin", "rand", "rand_core"]

src/batch.rs

@@ -43,22 +43,32 @@ use crate::public::PublicKey;
 use crate::signature::InternalSignature;
 
 trait BatchTranscript {
-    fn append_hrams(&mut self, hrams: &Vec<Scalar>);
+    fn append_scalars(&mut self, scalars: &Vec<Scalar>);
     fn append_message_lengths(&mut self, message_lengths: &Vec<usize>);
 }
 
 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<Scalar>) {
-        for (i, hram) in hrams.iter().enumerate() {
-            // XXX add message length into transcript
+    fn append_scalars(&mut self, scalars: &Vec<Scalar>) {
+        for (i, scalar) in scalars.iter().enumerate() {
             self.append_u64(b"", i as u64);
-            self.append_message(b"hram", hram.as_bytes());
+            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<usize>) {
         for (i, len) in message_lengths.iter().enumerate() {
             self.append_u64(b"", i as u64);
@@ -121,6 +131,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,17 +250,20 @@ 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<usize> = messages.iter().map(|i| i.len()).collect();
+    let scalars: Vec<Scalar> = 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
     // purely deterministic in the case of compiling with the
     // "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());

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;
 
@@ -125,6 +123,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<R>(csprng: &mut R) -> Keypair
     where
         R: CryptoRng + RngCore,
@@ -427,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)
     }
 }
 
@@ -437,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<E>(self, bytes: &[u8]) -> Result<Keypair, E>
-            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<A>(self, mut seq: A) -> Result<Keypair, A::Error>
-            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 = <SerdeByteBuf>::deserialize(deserializer)?;
+        Keypair::from_bytes(bytes.as_ref()).map_err(SerdeError::custom)
     }
 }

src/lib.rs

@@ -176,16 +176,16 @@
 //! # 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.";
 //! # 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<u8> = serialize(&public_key, Infinite).unwrap();
-//! let encoded_signature: Vec<u8> = serialize(&signature, Infinite).unwrap();
+//! let encoded_public_key: Vec<u8> = serialize(&public_key).unwrap();
+//! let encoded_signature: Vec<u8> = 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<u8> = serialize(&public_key, Infinite).unwrap();
-//! # let encoded_signature: Vec<u8> = serialize(&signature, Infinite).unwrap();
+//! # let encoded_public_key: Vec<u8> = serialize(&public_key).unwrap();
+//! # let encoded_signature: Vec<u8> = serialize(&signature).unwrap();
 //! let decoded_public_key: PublicKey = deserialize(&encoded_public_key).unwrap();
 //! let decoded_signature: Signature = deserialize(&encoded_signature).unwrap();
 //!
@@ -235,6 +235,9 @@
 #![warn(future_incompatible)]
 #![deny(missing_docs)] // refuse to compile if documentation is missing
 
+#![cfg(not(test))]
+#![forbid(unsafe_code)]
+
 #[cfg(any(feature = "std", test))]
 #[macro_use]
 extern crate std;

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<E>(self, bytes: &[u8]) -> Result<PublicKey, E>
-            where
-                E: SerdeError,
-            {
-                PublicKey::from_bytes(bytes).or(Err(SerdeError::invalid_length(bytes.len(), &self)))
-            }
-        }
-        deserializer.deserialize_bytes(PublicKeyVisitor)
+        let bytes = <SerdeByteBuf>::deserialize(deserializer)?;
+        PublicKey::from_bytes(bytes.as_ref()).map_err(SerdeError::custom)
     }
 }