Repository: dalek-cryptography/x25519-dalek (language: rust)

Issue:
Reexport rand_core
There are issues with conflicting versions of `rand_core` since x25519 does not use the latest version. It would be helpful if `rand_core` was exported to avoid conflicts


Discussion / hints:
Although, given that this crate already depends on `rand_core`, wouldn't there be more useful to have an additional feature flag, say `getrandom`, that just enables `OsRng` on `rand_core`, and then if this flag is set, to provide a wrapper say `new_from_os_rng` that just uses `new(rand_core::OsRng)`.

The current crate has a great API that makes it hard for misuse, and key generation should fall into this category.

I think a `getrandom` feature flag that activates `rand_core/getrandom` is a good idea.

A convenience method for generating a key using `OsRng` is a good idea as well.

Fix patch:
diff --git a/Cargo.toml b/Cargo.toml
index cce5567..021b6b8 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -53,6 +53,7 @@ harness = false
 
 [features]
 default = ["alloc", "precomputed-tables", "zeroize"]
+getrandom = ["rand_core/getrandom"]
 zeroize = ["dep:zeroize", "curve25519-dalek/zeroize"]
 serde = ["dep:serde", "curve25519-dalek/serde"]
 alloc = ["curve25519-dalek/alloc", "serde?/alloc", "zeroize?/alloc"]
diff --git a/README.md b/README.md
index 7145f06..6bc217d 100644
--- a/README.md
+++ b/README.md
@@ -28,23 +28,21 @@ up on modern public key cryptography and have learned a nifty trick called
 kittens will be able to secretly organise to find their mittens, and then spend
 the rest of the afternoon nomming some yummy pie!
 
-First, Alice uses `EphemeralSecret::new()` and then
+First, Alice uses `EphemeralSecret::random()` and then
 `PublicKey::from()` to produce her secret and public keys:
 
 ```rust
-use rand_core::OsRng;
 use x25519_dalek::{EphemeralSecret, PublicKey};
 
-let alice_secret = EphemeralSecret::new(OsRng);
+let alice_secret = EphemeralSecret::random();
 let alice_public = PublicKey::from(&alice_secret);
 ```
 
 Bob does the same:
 
 ```rust
-# use rand_core::OsRng;
 # use x25519_dalek::{EphemeralSecret, PublicKey};
-let bob_secret = EphemeralSecret::new(OsRng);
+let bob_secret = EphemeralSecret::random();
 let bob_public = PublicKey::from(&bob_secret);
 ```
 
diff --git a/benches/x25519.rs b/benches/x25519.rs
index dfcee4a..77c832d 100644
--- a/benches/x25519.rs
+++ b/benches/x25519.rs
@@ -19,12 +19,12 @@ use x25519_dalek::EphemeralSecret;
 use x25519_dalek::PublicKey;
 
 fn bench_diffie_hellman(c: &mut Criterion) {
-    let bob_secret = EphemeralSecret::new(OsRng);
+    let bob_secret = EphemeralSecret::random_from_rng(OsRng);
     let bob_public = PublicKey::from(&bob_secret);
 
     c.bench_function("diffie_hellman", move |b| {
         b.iter_with_setup(
-            || EphemeralSecret::new(OsRng),
+            || EphemeralSecret::random_from_rng(OsRng),
             |alice_secret| alice_secret.diffie_hellman(&bob_public),
         )
     });
diff --git a/src/lib.rs b/src/lib.rs
index 01369d0..7bcd8f4 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -50,14 +50,14 @@
 //! kittens will be able to secretly organise to find their mittens, and then spend
 //! the rest of the afternoon nomming some yummy pie!
 //!
-//! First, Alice uses `EphemeralSecret::new()` and then
+//! First, Alice uses `EphemeralSecret::random_from_rng` and then
 //! `PublicKey::from()` to produce her secret and public keys:
 //!
 //! ```rust
 //! use rand_core::OsRng;
 //! use x25519_dalek::{EphemeralSecret, PublicKey};
 //!
-//! let alice_secret = EphemeralSecret::new(OsRng);
+//! let alice_secret = EphemeralSecret::random_from_rng(OsRng);
 //! let alice_public = PublicKey::from(&alice_secret);
 //! ```
 //!
@@ -66,7 +66,7 @@
 //! ```rust
 //! # use rand_core::OsRng;
 //! # use x25519_dalek::{EphemeralSecret, PublicKey};
-//! let bob_secret = EphemeralSecret::new(OsRng);
+//! let bob_secret = EphemeralSecret::random_from_rng(OsRng);
 //! let bob_public = PublicKey::from(&bob_secret);
 //! ```
 //!
@@ -77,9 +77,9 @@
 //! ```rust
 //! # use rand_core::OsRng;
 //! # use x25519_dalek::{EphemeralSecret, PublicKey};
-//! # let alice_secret = EphemeralSecret::new(OsRng);
+//! # let alice_secret = EphemeralSecret::random_from_rng(OsRng);
 //! # let alice_public = PublicKey::from(&alice_secret);
-//! # let bob_secret = EphemeralSecret::new(OsRng);
+//! # let bob_secret = EphemeralSecret::random_from_rng(OsRng);
 //! # let bob_public = PublicKey::from(&bob_secret);
 //! let alice_shared_secret = alice_secret.diffie_hellman(&bob_public);
 //! ```
@@ -89,9 +89,9 @@
 //! ```rust
 //! # use rand_core::OsRng;
 //! # use x25519_dalek::{EphemeralSecret, PublicKey};
-//! # let alice_secret = EphemeralSecret::new(OsRng);
+//! # let alice_secret = EphemeralSecret::random_from_rng(OsRng);
 //! # let alice_public = PublicKey::from(&alice_secret);
-//! # let bob_secret = EphemeralSecret::new(OsRng);
+//! # let bob_secret = EphemeralSecret::random_from_rng(OsRng);
 //! # let bob_public = PublicKey::from(&bob_secret);
 //! let bob_shared_secret = bob_secret.diffie_hellman(&alice_public);
 //! ```
@@ -101,9 +101,9 @@
 //! ```rust
 //! # use rand_core::OsRng;
 //! # use x25519_dalek::{EphemeralSecret, PublicKey};
-//! # let alice_secret = EphemeralSecret::new(OsRng);
+//! # let alice_secret = EphemeralSecret::random_from_rng(OsRng);
 //! # let alice_public = PublicKey::from(&alice_secret);
-//! # let bob_secret = EphemeralSecret::new(OsRng);
+//! # let bob_secret = EphemeralSecret::random_from_rng(OsRng);
 //! # let bob_public = PublicKey::from(&bob_secret);
 //! # let alice_shared_secret = alice_secret.diffie_hellman(&bob_public);
 //! # let bob_shared_secret = bob_secret.diffie_hellman(&alice_public);
diff --git a/src/x25519.rs b/src/x25519.rs
index e95d528..7640805 100644
--- a/src/x25519.rs
+++ b/src/x25519.rs
@@ -63,8 +63,8 @@ impl PublicKey {
 /// This type is identical to the [`StaticSecret`] type, except that the
 /// [`EphemeralSecret::diffie_hellman`] method consumes and then wipes the secret key, and there
 /// are no serialization methods defined.  This means that [`EphemeralSecret`]s can only be
-/// generated from fresh randomness by [`EphemeralSecret::new`] and the compiler statically checks
-/// that the resulting secret is used at most once.
+/// generated from fresh randomness where the compiler statically checks that the resulting
+/// secret is used at most once.
 #[cfg_attr(feature = "zeroize", derive(Zeroize))]
 #[cfg_attr(feature = "zeroize", zeroize(drop))]
 pub struct EphemeralSecret(pub(crate) Scalar);
@@ -76,14 +76,29 @@ impl EphemeralSecret {
         SharedSecret(self.0 * their_public.0)
     }
 
-    /// Generate an x25519 [`EphemeralSecret`] key.
+    /// Generate a new [`EphemeralSecret`] with the supplied RNG.
+    #[deprecated(
+        since = "2.0.0",
+        note = "Renamed to `random_from_rng`. This will be removed in 2.1.0"
+    )]
     pub fn new<T: RngCore + CryptoRng>(mut csprng: T) -> Self {
+        Self::random_from_rng(&mut csprng)
+    }
+
+    /// Generate a new [`EphemeralSecret`] with the supplied RNG.
+    pub fn random_from_rng<T: RngCore + CryptoRng>(mut csprng: T) -> Self {
         let mut bytes = [0u8; 32];
 
         csprng.fill_bytes(&mut bytes);
 
         EphemeralSecret(Scalar::from_bits_clamped(bytes))
     }
+
+    /// Generate a new [`EphemeralSecret`].
+    #[cfg(feature = "getrandom")]
+    pub fn random() -> Self {
+        Self::random_from_rng(&mut rand_core::OsRng)
+    }
 }
 
 impl<'a> From<&'a EphemeralSecret> for PublicKey {
@@ -125,14 +140,29 @@ impl ReusableSecret {
         SharedSecret(self.0 * their_public.0)
     }
 
-    /// Generate a non-serializeable x25519 [`ReusableSecret`] key.
+    /// Generate a new [`ReusableSecret`] with the supplied RNG.
+    #[deprecated(
+        since = "2.0.0",
+        note = "Renamed to `random_from_rng`. This will be removed in 2.1.0."
+    )]
     pub fn new<T: RngCore + CryptoRng>(mut csprng: T) -> Self {
+        Self::random_from_rng(&mut csprng)
+    }
+
+    /// Generate a new [`ReusableSecret`] with the supplied RNG.
+    pub fn random_from_rng<T: RngCore + CryptoRng>(mut csprng: T) -> Self {
         let mut bytes = [0u8; 32];
 
         csprng.fill_bytes(&mut bytes);
 
         ReusableSecret(Scalar::from_bits_clamped(bytes))
     }
+
+    /// Generate a new [`ReusableSecret`].
+    #[cfg(feature = "getrandom")]
+    pub fn random() -> Self {
+        Self::random_from_rng(&mut rand_core::OsRng)
+    }
 }
 
 #[cfg(feature = "reusable_secrets")]
@@ -172,8 +202,17 @@ impl StaticSecret {
         SharedSecret(self.0 * their_public.0)
     }
 
-    /// Generate an x25519 key.
+    /// Generate a new [`StaticSecret`] with the supplied RNG.
+    #[deprecated(
+        since = "2.0.0",
+        note = "Renamed to `random_from_rng`. This will be removed in 2.1.0"
+    )]
     pub fn new<T: RngCore + CryptoRng>(mut csprng: T) -> Self {
+        Self::random_from_rng(&mut csprng)
+    }
+
+    /// Generate a new [`StaticSecret`] with the supplied RNG.
+    pub fn random_from_rng<T: RngCore + CryptoRng>(mut csprng: T) -> Self {
         let mut bytes = [0u8; 32];
 
         csprng.fill_bytes(&mut bytes);
@@ -181,6 +220,12 @@ impl StaticSecret {
         StaticSecret(Scalar::from_bits_clamped(bytes))
     }
 
+    /// Generate a new [`StaticSecret`].
+    #[cfg(feature = "getrandom")]
+    pub fn random() -> Self {
+        Self::random_from_rng(&mut rand_core::OsRng)
+    }
+
     /// Extract this key's bytes for serialization.
     pub fn to_bytes(&self) -> [u8; 32] {
         self.0.to_bytes()
@@ -276,11 +321,11 @@ impl SharedSecret {
 /// use x25519_dalek::PublicKey;
 ///
 /// // Generate Alice's key pair.
-/// let alice_secret = StaticSecret::new(&mut OsRng);
+/// let alice_secret = StaticSecret::random_from_rng(&mut OsRng);
 /// let alice_public = PublicKey::from(&alice_secret);
 ///
 /// // Generate Bob's key pair.
-/// let bob_secret = StaticSecret::new(&mut OsRng);
+/// let bob_secret = StaticSecret::random_from_rng(&mut OsRng);
 /// let bob_public = PublicKey::from(&bob_secret);
 ///
 /// // Alice and Bob should now exchange their public keys.


Test patch:
diff --git a/tests/x25519_tests.rs b/tests/x25519_tests.rs
index 21eeb43..280978d 100644
--- a/tests/x25519_tests.rs
+++ b/tests/x25519_tests.rs
@@ -180,3 +180,53 @@ fn rfc7748_ladder_test2() {
         ]
     );
 }
+
+mod rand_core {
+
+    use super::*;
+    use ::rand_core::OsRng;
+
+    #[test]
+    fn ephemeral_from_rng() {
+        #[allow(deprecated)]
+        EphemeralSecret::new(OsRng);
+        EphemeralSecret::random_from_rng(OsRng);
+    }
+
+    #[test]
+    #[cfg(feature = "reusable_secrets")]
+    fn reusable_from_rng() {
+        #[allow(deprecated)]
+        ReusableSecret::new(OsRng);
+        ReusableSecret::random_from_rng(OsRng);
+    }
+
+    #[test]
+    fn static_from_rng() {
+        #[allow(deprecated)]
+        StaticSecret::new(OsRng);
+        StaticSecret::random_from_rng(OsRng);
+    }
+}
+
+#[cfg(feature = "getrandom")]
+mod getrandom {
+
+    use super::*;
+
+    #[test]
+    fn ephemeral_random() {
+        EphemeralSecret::random();
+    }
+
+    #[test]
+    #[cfg(feature = "reusable_secrets")]
+    fn reusable_random() {
+        ReusableSecret::random();
+    }
+
+    #[test]
+    fn static_random() {
+        StaticSecret::random();
+    }
+}