Upgrade ed25519-dalek to 2.0

Co-authored-by: Kévin Commaille <zecakeh@tedomum.fr>

crates/ruma-signatures/CHANGELOG.md

@@ -1,5 +1,11 @@
 # [unreleased]
 
+Breaking changes:
+
+- Update `ed25519-dalek` crate
+  - `Ed25519KeyPair::generate()` returns a `Zeroizing<Vec<u8>>` on success
+  - `Ed25519KeyPair::public_key()` returns an array instead of a slice
+
 Bug fixes:
 
 - Ignore keys with unknown algorithms in `verify_events`

crates/ruma-signatures/Cargo.toml

@@ -21,10 +21,9 @@ unstable-exhaustive-types = []
 
 [dependencies]
 base64 = { workspace = true }
-ed25519-dalek = "1.0.1"
+ed25519-dalek = { version = "2.0.0", features = ["pkcs8", "rand_core"] }
-pkcs8 = { version = "0.9.0", features = ["alloc"] }
+pkcs8 = { version = "0.10.0", features = ["alloc"] }
-# because dalek uses an older version of rand_core
+rand = { version = "0.8.5", features = ["getrandom"] }
-rand = { version = "0.7", features = ["getrandom"] }
 ruma-common = { workspace = true, features = ["canonical-json"] }
 serde_json = { workspace = true }
 sha2 = "0.10.6"

crates/ruma-signatures/src/error.rs

@@ -22,9 +22,9 @@ pub enum Error {
     #[error("Parse error: {0}")]
     Parse(#[from] ParseError),
 
-    /// Wrapper for [`pkcs8::der::Error`].
+    /// Wrapper for [`pkcs8::Error`].
     #[error("DER Parse error: {0}")]
-    DerParse(pkcs8::der::Error),
+    DerParse(pkcs8::Error),
 
     /// The signature's ID does not have exactly two components separated by a colon.
     #[error("malformed signature ID: expected exactly 2 segment separated by a colon, found {0}")]
@@ -204,8 +204,8 @@ pub enum ParseError {
     },
 
     /// For when [`ed25519_dalek`] cannot parse a secret/private key.
-    #[error("Could not parse secret key: {0}")]
+    #[error("Could not parse secret key")]
-    SecretKey(#[source] ed25519_dalek::SignatureError),
+    SecretKey,
 
     /// For when [`ed25519_dalek`] cannot parse a public key.
     #[error("Could not parse public key: {0}")]

crates/ruma-signatures/src/functions.rs

@@ -983,7 +983,7 @@ mod tests {
         let mut public_key_map = PublicKeyMap::new();
         let mut sender_key_map = PublicKeySet::new();
         let newly_generated_key_pair = generate_key_pair("2");
-        let encoded_public_key = Base64::new(newly_generated_key_pair.public_key().to_owned());
+        let encoded_public_key = Base64::new(newly_generated_key_pair.public_key().to_vec());
         let version = ServerSigningKeyId::from_parts(
             SigningKeyAlgorithm::Ed25519,
             key_pair_sender.version().try_into().unwrap(),
@@ -1159,7 +1159,7 @@ mod tests {
 
     fn add_key_to_map(public_key_map: &mut PublicKeyMap, name: &str, pair: &Ed25519KeyPair) {
         let sender_key_map = public_key_map.entry(name.to_owned()).or_default();
-        let encoded_public_key = Base64::new(pair.public_key().to_owned());
+        let encoded_public_key = Base64::new(pair.public_key().to_vec());
         let version = ServerSigningKeyId::from_parts(
             SigningKeyAlgorithm::Ed25519,
             pair.version().try_into().unwrap(),
@@ -1174,7 +1174,7 @@ mod tests {
         pair: &Ed25519KeyPair,
     ) {
         let sender_key_map = public_key_map.entry(name.to_owned()).or_default();
-        let encoded_public_key = Base64::new(pair.public_key().to_owned());
+        let encoded_public_key = Base64::new(pair.public_key().to_vec());
         let version = ServerSigningKeyId::from_parts(
             SigningKeyAlgorithm::from("an-unknown-algorithm"),
             pair.version().try_into().unwrap(),

crates/ruma-signatures/src/keys.rs

@@ -5,8 +5,10 @@ use std::{
     fmt::{Debug, Formatter, Result as FmtResult},
 };
 
-use ed25519_dalek::{ExpandedSecretKey, PublicKey, SecretKey};
+use ed25519_dalek::{pkcs8::ALGORITHM_OID, SecretKey, Signer, SigningKey, PUBLIC_KEY_LENGTH};
-use pkcs8::{AlgorithmIdentifier, ObjectIdentifier, PrivateKeyInfo};
+use pkcs8::{
+    der::zeroize::Zeroizing, DecodePrivateKey, EncodePrivateKey, ObjectIdentifier, PrivateKeyInfo,
+};
 use ruma_common::serde::Base64;
 
 use crate::{signatures::Signature, Algorithm, Error, ParseError};
@@ -24,14 +26,9 @@ pub trait KeyPair: Sized {
     fn sign(&self, message: &[u8]) -> Signature;
 }
 
-const ED25519_OID: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.3.101.112");
-
 /// An Ed25519 key pair.
 pub struct Ed25519KeyPair {
-    extended_privkey: ExpandedSecretKey,
+    signing_key: SigningKey,
-
-    pubkey: PublicKey,
-
     /// The specific name of the key pair.
     version: String,
 }
@@ -44,31 +41,26 @@ impl Ed25519KeyPair {
         pubkey: Option<&[u8]>,
         version: String,
     ) -> Result<Self, Error> {
-        if oid != ED25519_OID {
+        if oid != ALGORITHM_OID {
-            return Err(ParseError::Oid { expected: ED25519_OID, found: oid }.into());
+            return Err(ParseError::Oid { expected: ALGORITHM_OID, found: oid }.into());
         }
 
-        let secret_key = SecretKey::from_bytes(Self::correct_privkey_from_octolet(privkey))
+        let secret_key = Self::correct_privkey_from_octolet(privkey)?;
-            .map_err(ParseError::SecretKey)?;
+        let signing_key = SigningKey::from_bytes(secret_key);
-
-        let derived_pubkey = PublicKey::from(&secret_key);
 
         if let Some(oak_key) = pubkey {
             // If the document had a public key, we're verifying it.
+            let verifying_key = signing_key.verifying_key();
 
-            if oak_key != derived_pubkey.as_bytes() {
+            if oak_key != verifying_key.as_bytes() {
                 return Err(ParseError::derived_vs_parsed_mismatch(
                     oak_key,
-                    derived_pubkey.as_bytes().to_vec(),
+                    verifying_key.as_bytes().to_vec(),
                 ));
             }
         }
 
-        Ok(Self {
+        Ok(Self { signing_key, version })
-            extended_privkey: ExpandedSecretKey::from(&secret_key),
-            pubkey: derived_pubkey,
-            version,
-        })
     }
 
     /// Initializes a new key pair.
@@ -90,33 +82,28 @@ impl Ed25519KeyPair {
     /// generated from the private key. This is a fallback and extra validation against
     /// corruption or
     pub fn from_der(document: &[u8], version: String) -> Result<Self, Error> {
-        use pkcs8::der::Decode;
-
         #[cfg(feature = "ring-compat")]
         use self::compat::CompatibleDocument;
 
-        #[cfg(feature = "ring-compat")]
+        let signing_key;
-        let backing: Vec<u8>;
-        let oak;
 
         #[cfg(feature = "ring-compat")]
         {
-            oak = match CompatibleDocument::from_bytes(document) {
+            signing_key = match CompatibleDocument::from_bytes(document) {
                 CompatibleDocument::WellFormed(bytes) => {
-                    PrivateKeyInfo::from_der(bytes).map_err(Error::DerParse)?
+                    SigningKey::from_pkcs8_der(bytes).map_err(Error::DerParse)?
                 }
                 CompatibleDocument::CleanedFromRing(vec) => {
-                    backing = vec;
+                    SigningKey::from_pkcs8_der(&vec).map_err(Error::DerParse)?
-
-                    PrivateKeyInfo::from_der(&backing).map_err(Error::DerParse)?
                 }
             }
         }
         #[cfg(not(feature = "ring-compat"))]
         {
-            oak = PrivateKeyInfo::from_der(document).map_err(Error::DerParse)?;
+            signing_key = SigningKey::from_pkcs8_der(document).map_err(Error::DerParse)?;
         }
-        Self::from_pkcs8_oak(oak, version)
+
+        Ok(Self { signing_key, version })
     }
 
     /// Constructs a key pair from [`pkcs8::PrivateKeyInfo`].
@@ -133,11 +120,11 @@ impl Ed25519KeyPair {
     /// so convert it if it is wrongly formatted.
     ///
     /// See [RFC 8310 10.3](https://datatracker.ietf.org/doc/html/rfc8410#section-10.3) for more details
-    fn correct_privkey_from_octolet(key: &[u8]) -> &[u8] {
+    fn correct_privkey_from_octolet(key: &[u8]) -> Result<&SecretKey, ParseError> {
         if key.len() == 34 && key[..2] == [0x04, 0x20] {
-            &key[2..]
+            Ok(key[2..].try_into().unwrap())
         } else {
-            key
+            key.try_into().map_err(|_| ParseError::SecretKey)
         }
     }
 
@@ -150,25 +137,9 @@ impl Ed25519KeyPair {
     /// # Errors
     ///
     /// Returns an error if the generation failed.
-    pub fn generate() -> Result<Vec<u8>, Error> {
+    pub fn generate() -> Result<Zeroizing<Vec<u8>>, Error> {
-        use pkcs8::der::Encode;
+        let signing_key = SigningKey::generate(&mut rand::rngs::OsRng);
-
+        Ok(signing_key.to_pkcs8_der().map_err(Error::DerParse)?.to_bytes())
-        let secret = SecretKey::generate(&mut rand::rngs::OsRng);
-
-        let public = PublicKey::from(&secret);
-
-        // Convert into nested OCTAL STRING
-        // Per: https://datatracker.ietf.org/doc/html/rfc8410#section-10.3
-        let mut private: Vec<u8> = vec![0x04, 0x20];
-        private.extend_from_slice(secret.as_bytes());
-
-        let pkinfo = PrivateKeyInfo {
-            algorithm: AlgorithmIdentifier { oid: ED25519_OID, parameters: None },
-            private_key: private.as_ref(),
-            public_key: Some(public.as_bytes()),
-        };
-
-        pkinfo.to_vec().map_err(Error::DerParse)
     }
 
     /// Returns the version string for this keypair.
@@ -177,8 +148,8 @@ impl Ed25519KeyPair {
     }
 
     /// Returns the public key.
-    pub fn public_key(&self) -> &[u8] {
+    pub fn public_key(&self) -> [u8; PUBLIC_KEY_LENGTH] {
-        self.pubkey.as_ref()
+        self.signing_key.verifying_key().to_bytes()
     }
 }
 
@@ -186,7 +157,7 @@ impl KeyPair for Ed25519KeyPair {
     fn sign(&self, message: &[u8]) -> Signature {
         Signature {
             algorithm: Algorithm::Ed25519,
-            signature: self.extended_privkey.sign(message, &self.pubkey).as_ref().to_vec(),
+            signature: self.signing_key.sign(message).to_bytes().to_vec(),
             version: self.version.clone(),
         }
     }
@@ -196,7 +167,7 @@ impl Debug for Ed25519KeyPair {
     fn fmt(&self, formatter: &mut Formatter<'_>) -> FmtResult {
         formatter
             .debug_struct("Ed25519KeyPair")
-            .field("public_key", &self.pubkey.as_bytes())
+            .field("verifying_key", &self.signing_key.verifying_key().as_bytes())
             .field("version", &self.version)
             .finish()
     }
@@ -242,7 +213,7 @@ mod tests {
     fn well_formed_key() {
         let keypair = Ed25519KeyPair::from_der(WELL_FORMED_DOC, "".to_owned()).unwrap();
 
-        assert_eq!(keypair.pubkey.as_bytes(), WELL_FORMED_PUBKEY);
+        assert_eq!(keypair.public_key(), WELL_FORMED_PUBKEY);
     }
 
     #[cfg(feature = "ring-compat")]
@@ -269,7 +240,7 @@ mod tests {
         fn ring_key() {
             let keypair = Ed25519KeyPair::from_der(RING_DOC, "".to_owned()).unwrap();
 
-            assert_eq!(keypair.pubkey.as_bytes(), RING_PUBKEY);
+            assert_eq!(keypair.public_key(), RING_PUBKEY);
         }
     }
 }

crates/ruma-signatures/src/verification.rs

@@ -1,6 +1,6 @@
 //! Verification of digital signatures.
 
-use ed25519_dalek::{PublicKey, Verifier as _};
+use ed25519_dalek::{Verifier as _, VerifyingKey};
 
 use crate::{Error, ParseError, VerificationError};
 
@@ -32,11 +32,15 @@ impl Verifier for Ed25519Verifier {
         signature: &[u8],
         message: &[u8],
     ) -> Result<(), Error> {
-        PublicKey::from_bytes(public_key)
+        VerifyingKey::from_bytes(
-            .map_err(ParseError::PublicKey)?
+            public_key
-            .verify(message, &signature.try_into().map_err(ParseError::Signature)?)
+                .try_into()
-            .map_err(VerificationError::Signature)
+                .map_err(|_| ParseError::PublicKey(ed25519_dalek::SignatureError::new()))?,
-            .map_err(Error::from)
+        )
+        .map_err(ParseError::PublicKey)?
+        .verify(message, &signature.try_into().map_err(ParseError::Signature)?)
+        .map_err(VerificationError::Signature)
+        .map_err(Error::from)
     }
 }
 

crates/ruma-signatures/tests/tests.rs

@@ -7,7 +7,7 @@ static PKCS8_ED25519_DER: &[u8] = include_bytes!("./keys/ed25519.der");
 
 fn add_key_to_map(public_key_map: &mut PublicKeyMap, name: &str, pair: &Ed25519KeyPair) {
     let sender_key_map = public_key_map.entry(name.to_owned()).or_default();
-    let encoded_public_key = Base64::new(pair.public_key().to_owned());
+    let encoded_public_key = Base64::new(pair.public_key().to_vec());
     let version = ServerSigningKeyId::from_parts(
         SigningKeyAlgorithm::Ed25519,
         pair.version().try_into().unwrap(),