signatures: Replace ring with ed25519-dalek

crates/ruma-signatures/Cargo.toml

@@ -19,9 +19,13 @@ compat = ["tracing"]
 
 [dependencies]
 base64 = "0.13.0"
-ring = "0.16.19"
+ed25519-dalek = "1.0.1"
+pkcs8 = { git = "https://github.com/RustCrypto/utils", rev = "51e7c9d734e4d3c5279ba1c181c65b1bd77bcad0", features = ["alloc"] }
+# because dalek uses an older version of rand_core
+rand = { version = "0.7", features = ["getrandom"] }
 ruma-identifiers = { version = "0.19.2", path = "../ruma-identifiers" }
 ruma-serde = { version = "0.4.0", path = "../ruma-serde" }
 serde_json = "1.0.60"
+sha2 = "0.9.5"
 tracing = { version = "0.1.25", optional = true }
 untrusted = "0.7.1"

crates/ruma-signatures/src/functions.rs

@@ -8,10 +8,11 @@ use std::{
 };
 
 use base64::{decode_config, encode_config, Config, STANDARD_NO_PAD, URL_SAFE_NO_PAD};
-use ring::digest::{digest, SHA256};
+use ed25519_dalek::Digest;
 use ruma_identifiers::{EventId, RoomVersionId, ServerNameBox, UserId};
 use ruma_serde::{to_canonical_json_string, CanonicalJsonObject, CanonicalJsonValue};
 use serde_json::from_str as from_json_str;
+use sha2::Sha256;
 
 use crate::{
     keys::{KeyPair, PublicKeyMap},
@@ -108,7 +109,7 @@ static REFERENCE_HASH_FIELDS_TO_REMOVE: &[&str] = &["age_ts", "signatures", "uns
 /// let document = base64::decode_config(&PKCS8, base64::STANDARD_NO_PAD).unwrap();
 ///
 /// // Create an Ed25519 key pair.
-/// let key_pair = ruma_signatures::Ed25519KeyPair::new(
+/// let key_pair = ruma_signatures::Ed25519KeyPair::from_der(
 ///     &document,
 ///     "1".into(), // The "version" of the key.
 /// ).unwrap();
@@ -332,7 +333,7 @@ where
 /// object: A JSON object to generate a content hash for.
 pub fn content_hash(object: &CanonicalJsonObject) -> String {
     let json = canonical_json_with_fields_to_remove(object, CONTENT_HASH_FIELDS_TO_REMOVE);
-    let hash = digest(&SHA256, json.as_bytes());
+    let hash = Sha256::digest(json.as_bytes());
 
     encode_config(&hash, STANDARD_NO_PAD)
 }
@@ -361,7 +362,7 @@ pub fn reference_hash(
     let json =
         canonical_json_with_fields_to_remove(&redacted_value, REFERENCE_HASH_FIELDS_TO_REMOVE);
 
-    let hash = digest(&SHA256, json.as_bytes());
+    let hash = Sha256::digest(json.as_bytes());
 
     Ok(encode_config(
         &hash,
@@ -411,7 +412,7 @@ pub fn reference_hash(
 /// let document = base64::decode_config(&PKCS8, base64::STANDARD_NO_PAD).unwrap();
 ///
 /// // Create an Ed25519 key pair.
-/// let key_pair = Ed25519KeyPair::new(
+/// let key_pair = Ed25519KeyPair::from_der(
 ///     &document,
 ///     "1".into(), // The "version" of the key.
 /// ).unwrap();
@@ -1046,12 +1047,13 @@ mod tests {
 
         assert!(verification_result.is_err());
         let error_msg = verification_result.err().unwrap().message;
-        assert!(error_msg.contains("signature verification failed"));
+        assert!(error_msg.contains("Could not verify signature"));
     }
 
     fn generate_key_pair() -> Ed25519KeyPair {
         let key_content = Ed25519KeyPair::generate().unwrap();
-        Ed25519KeyPair::new(&key_content, "1".to_owned()).unwrap()
+        Ed25519KeyPair::from_der(&key_content, "1".to_owned())
+            .unwrap_or_else(|_| panic!("{:?}", &key_content))
     }
 
     fn add_key_to_map(public_key_map: &mut PublicKeyMap, name: &str, pair: &Ed25519KeyPair) {

crates/ruma-signatures/src/keys.rs

@@ -5,7 +5,12 @@ use std::{
     fmt::{Debug, Formatter, Result as FmtResult},
 };
 
-use ring::signature::{Ed25519KeyPair as RingEd25519KeyPair, KeyPair as _};
+use ed25519_dalek::{ExpandedSecretKey, PublicKey, SecretKey};
+
+use pkcs8::{
+    der::{Decodable, Encodable},
+    AlgorithmIdentifier, ObjectIdentifier, OneAsymmetricKey, PrivateKeyInfo,
+};
 
 use crate::{signatures::Signature, Algorithm, Error};
 
@@ -19,21 +24,65 @@ pub trait KeyPair: Sized {
     fn sign(&self, message: &[u8]) -> Signature;
 }
 
+pub const ED25519_OID: ObjectIdentifier = ObjectIdentifier::new("1.3.101.112");
+
 /// An Ed25519 key pair.
 pub struct Ed25519KeyPair {
-    /// Ring's Keypair type
+    extended_privkey: ExpandedSecretKey,
-    keypair: RingEd25519KeyPair,
 
-    /// The version of the key pair.
+    pubkey: PublicKey,
+
+    /// The specific name of the key pair.
     version: String,
 }
 
 impl Ed25519KeyPair {
+    /// Create a key pair from its constituent parts.
+    pub fn new(
+        oid: ObjectIdentifier,
+        privkey: &[u8],
+        pubkey: Option<&[u8]>,
+        version: String,
+    ) -> Result<Self, Error> {
+        if oid != ED25519_OID {
+            return Err(Error::new(format!(
+                "Algorithm OID does not match ed25519, expected {}, found {}",
+                ED25519_OID, oid
+            )));
+        }
+
+        let secret_key = SecretKey::from_bytes(Self::correct_privkey_from_octolet(privkey))
+            .map_err(|e| Error::new(e.to_string()))?;
+
+        let derived_pubkey = PublicKey::from(&secret_key);
+
+        if let Some(oak_key) = pubkey {
+            // If the document had a public key, we're verifying it.
+
+            if oak_key != derived_pubkey.as_bytes() {
+                return Err(Error::new(format!(
+                    "PKCS#8 Document public key does not match public key derived from private key; derived: {:X?} (len {}), parsed: {:X?} (len {})",
+                    &derived_pubkey.as_bytes(),
+                    derived_pubkey.as_bytes().len(),
+                    oak_key,
+                    oak_key.len()
+                )));
+            }
+        }
+
+        Ok(Self {
+            extended_privkey: ExpandedSecretKey::from(&secret_key),
+            pubkey: derived_pubkey,
+            version,
+        })
+    }
+
     /// Initializes a new key pair.
     ///
     /// # Parameters
     ///
-    /// * document: PKCS8-formatted bytes containing the private & public keys.
+    /// * document: PKCS#8 v1/v2 DER-formatted document containing the private (and optionally
+    ///   public) key.
     /// * version: The "version" of the key used for this signature. Versions are used as an
     ///   identifier to distinguish signatures generated from different keys but using the same
     ///   algorithm on the same homeserver.
@@ -42,27 +91,64 @@ impl Ed25519KeyPair {
     ///
     /// Returns an error if the public and private keys provided are invalid for the implementing
     /// algorithm.
-    pub fn new(document: &[u8], version: String) -> Result<Self, Error> {
+    ///
-        let keypair = RingEd25519KeyPair::from_pkcs8(document)
+    /// Returns an error when the PKCS#8 document had a public key, but it doesn't match the one
-            .map_err(|error| Error::new(error.to_string()))?;
+    /// 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> {
+        let oak = OneAsymmetricKey::from_der(document).map_err(|e| Error::new(e.to_string()))?;
 
-        Ok(Self { keypair, version })
+        Self::from_pkcs8_oak(oak, version)
+    }
+
+    /// Constructs a key pair from [`pkcs8::OneAsymmetricKey`].
+    pub fn from_pkcs8_oak(oak: OneAsymmetricKey<'_>, version: String) -> Result<Self, Error> {
+        Self::new(oak.algorithm.oid, oak.private_key, oak.public_key, version)
+    }
+
+    /// Constructs a key pair from [`pkcs8::PrivateKeyInfo`].
+    pub fn from_pkcs8_pki(oak: PrivateKeyInfo<'_>, version: String) -> Result<Self, Error> {
+        Self::new(oak.algorithm.oid, oak.private_key, None, version)
+    }
+
+    /// PKCS#8's "private key" is not yet actually the entire key,
+    /// 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] {
+        if key.len() == 34 && key[..2] == [0x04, 0x20] {
+            &key[2..]
+        } else {
+            key
+        }
     }
 
     /// Generates a new key pair.
     ///
     /// # Returns
     ///
-    /// Returns a Vec<u8> representing a pkcs8-encoded private/public keypair
+    /// Returns a Vec<u8> representing a DER-encoded PKCS#8 v2 document (with public key)
     ///
     /// # Errors
     ///
     /// Returns an error if the generation failed.
     pub fn generate() -> Result<Vec<u8>, Error> {
-        let document = RingEd25519KeyPair::generate_pkcs8(&ring::rand::SystemRandom::new())
+        let secret = SecretKey::generate(&mut rand::rngs::OsRng);
-            .map_err(|e| Error::new(e.to_string()))?;
 
-        Ok(document.as_ref().to_vec())
+        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 oak = OneAsymmetricKey {
+            algorithm: AlgorithmIdentifier { oid: ED25519_OID, parameters: None },
+            private_key: private.as_ref(),
+            public_key: Some(public.as_bytes()),
+        };
+
+        oak.to_vec().map_err(|e| Error::new(e.to_string()))
     }
 
     /// Returns the version string for this keypair.
@@ -72,7 +158,7 @@ impl Ed25519KeyPair {
 
     /// Returns the public key.
     pub fn public_key(&self) -> &[u8] {
-        self.keypair.public_key().as_ref()
+        self.pubkey.as_ref()
     }
 }
 
@@ -80,7 +166,7 @@ impl KeyPair for Ed25519KeyPair {
     fn sign(&self, message: &[u8]) -> Signature {
         Signature {
             algorithm: Algorithm::Ed25519,
-            signature: self.keypair.sign(message).as_ref().to_vec(),
+            signature: self.extended_privkey.sign(message, &self.pubkey).as_ref().to_vec(),
             version: self.version.clone(),
         }
     }
@@ -90,7 +176,7 @@ impl Debug for Ed25519KeyPair {
     fn fmt(&self, formatter: &mut Formatter<'_>) -> FmtResult {
         formatter
             .debug_struct("Ed25519KeyPair")
-            .field("public_key", &self.keypair.public_key())
+            .field("public_key", &self.pubkey.as_bytes())
             .field("version", &self.version)
             .finish()
     }
@@ -110,8 +196,30 @@ pub type PublicKeySet = BTreeMap<String, String>;
 mod tests {
     use super::Ed25519KeyPair;
 
+    const RING_DOC: &[u8] = &[
+        0x30, 0x53, 0x02, 0x01, 0x01, 0x30, 0x05, 0x06, 0x03, 0x2B, 0x65, 0x70, 0x04, 0x22, 0x04,
+        0x20, 0x61, 0x9E, 0xD8, 0x25, 0xA6, 0x1D, 0x32, 0x29, 0xD7, 0xD8, 0x22, 0x03, 0xC6, 0x0E,
+        0x37, 0x48, 0xE9, 0xC9, 0x11, 0x96, 0x3B, 0x03, 0x15, 0x94, 0x19, 0x3A, 0x86, 0xEC, 0xE6,
+        0x2D, 0x73, 0xC0, 0xA1, 0x23, 0x03, 0x21, 0x00, 0x3D, 0xA6, 0xC8, 0xD1, 0x76, 0x2F, 0xD6,
+        0x49, 0xB8, 0x4F, 0xF6, 0xC6, 0x1D, 0x04, 0xEA, 0x4A, 0x70, 0xA8, 0xC9, 0xF0, 0x8F, 0x96,
+        0x7F, 0x6B, 0xD7, 0xDA, 0xE5, 0x2E, 0x88, 0x8D, 0xBA, 0x3E,
+    ];
+
+    const RING_PUBKEY: &[u8] = &[
+        0x3D, 0xA6, 0xC8, 0xD1, 0x76, 0x2F, 0xD6, 0x49, 0xB8, 0x4F, 0xF6, 0xC6, 0x1D, 0x04, 0xEA,
+        0x4A, 0x70, 0xA8, 0xC9, 0xF0, 0x8F, 0x96, 0x7F, 0x6B, 0xD7, 0xDA, 0xE5, 0x2E, 0x88, 0x8D,
+        0xBA, 0x3E,
+    ];
+
     #[test]
     fn generate_key() {
         Ed25519KeyPair::generate().unwrap();
     }
+
+    #[test]
+    fn ring_key() {
+        let keypair = Ed25519KeyPair::from_der(RING_DOC, "".to_string()).unwrap();
+
+        assert_eq!(keypair.pubkey.as_bytes(), RING_PUBKEY);
+    }
 }

crates/ruma-signatures/src/lib.rs

@@ -171,7 +171,7 @@ mod tests {
     use std::collections::BTreeMap;
 
     use base64::{decode_config, encode_config, STANDARD_NO_PAD};
-    use ring::signature::{Ed25519KeyPair as RingEd25519KeyPair, KeyPair as _};
+    use pkcs8::{der::Decodable, OneAsymmetricKey};
     use ruma_identifiers::RoomVersionId;
     use serde_json::{from_str as from_json_str, to_string as to_json_string};
 
@@ -187,9 +187,10 @@ mod tests {
     /// Convenience method for getting the public key as a string
     fn public_key_string() -> String {
         encode_config(
-            &RingEd25519KeyPair::from_pkcs8(&decode_config(PKCS8, STANDARD_NO_PAD).unwrap())
+            &OneAsymmetricKey::from_der(&decode_config(PKCS8, STANDARD_NO_PAD).unwrap())
                 .unwrap()
-                .public_key(),
+                .public_key
+                .unwrap(),
             STANDARD_NO_PAD,
         )
     }
@@ -291,7 +292,7 @@ mod tests {
 
     #[test]
     fn sign_empty_json() {
-        let key_pair = Ed25519KeyPair::new(
+        let key_pair = Ed25519KeyPair::from_der(
             decode_config(&PKCS8, STANDARD_NO_PAD).unwrap().as_slice(),
             "1".into(),
         )
@@ -322,7 +323,7 @@ mod tests {
 
     #[test]
     fn sign_minimal_json() {
-        let key_pair = Ed25519KeyPair::new(
+        let key_pair = Ed25519KeyPair::from_der(
             decode_config(&PKCS8, STANDARD_NO_PAD).unwrap().as_slice(),
             "1".into(),
         )
@@ -382,7 +383,7 @@ mod tests {
 
     #[test]
     fn sign_minimal_event() {
-        let key_pair = Ed25519KeyPair::new(
+        let key_pair = Ed25519KeyPair::from_der(
             decode_config(&PKCS8, STANDARD_NO_PAD).unwrap().as_slice(),
             "1".into(),
         )
@@ -416,7 +417,7 @@ mod tests {
 
     #[test]
     fn sign_redacted_event() {
-        let key_pair = Ed25519KeyPair::new(
+        let key_pair = Ed25519KeyPair::from_der(
             decode_config(&PKCS8, STANDARD_NO_PAD).unwrap().as_slice(),
             "1".into(),
         )

crates/ruma-signatures/src/verification.rs

@@ -1,7 +1,8 @@
 //! Verification of digital signatures.
 
-use ring::signature::{VerificationAlgorithm, ED25519};
+use std::convert::TryInto;
-use untrusted::Input;
+
+use ed25519_dalek::{PublicKey, Verifier as _};
 
 use crate::Error;
 
@@ -33,9 +34,15 @@ impl Verifier for Ed25519Verifier {
         signature: &[u8],
         message: &[u8],
     ) -> Result<(), Error> {
-        ED25519
+        PublicKey::from_bytes(public_key)
-            .verify(Input::from(public_key), Input::from(message), Input::from(signature))
+            .map_err(|e| Error::new(format!("Could not parse public key: {:?}", e)))?
-            .map_err(|_| Error::new("signature verification failed"))
+            .verify(
+                message,
+                &signature
+                    .try_into()
+                    .map_err(|e| Error::new(format!("Could not parse signature: {:?}", e)))?,
+            )
+            .map_err(|e| Error::new(format!("Could not verify signature: {:?}", e)))
     }
 }