Convert ruma-signatures error story into thiserror

crates/ruma-signatures/Cargo.toml

@@ -20,12 +20,13 @@ compat = ["tracing"]
 [dependencies]
 base64 = "0.13.0"
 ed25519-dalek = "1.0.1"
-pkcs8 = { git = "https://github.com/RustCrypto/utils", rev = "51e7c9d734e4d3c5279ba1c181c65b1bd77bcad0", features = ["alloc"] }
+pkcs8 = { git = "https://github.com/RustCrypto/utils", rev = "51e7c9d734e4d3c5279ba1c181c65b1bd77bcad0", features = ["alloc", "std"] }
 # 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"
+thiserror = "1.0.23"
 tracing = { version = "0.1.25", optional = true }
 untrusted = "0.7.1"

crates/ruma-signatures/src/error.rs

@@ -0,0 +1,262 @@
+use ruma_identifiers::{EventId, RoomVersionId, ServerNameBox};
+use thiserror::Error;
+
+/// `ruma-signature`'s error type, wraps a number of other error types.
+#[derive(Debug, Error)]
+#[non_exhaustive]
+pub enum Error {
+    /// [`JsonError`] wrapper.
+    #[error("JSON error: {0}")]
+    Json(#[from] JsonError),
+
+    /// [`VerificationError`] wrapper.
+    #[error("Verification error: {0}")]
+    Verification(#[from] VerificationError),
+
+    /// [`ParseError`] wrapper.
+    #[error("Parse error: {0}")]
+    Parse(#[from] ParseError),
+
+    /// Wrapper for [`pkcs8::der::Error`].
+    #[error("DER Parse error: {0}")]
+    DerParse(pkcs8::der::Error),
+
+    /// [`SplitError`] wrapper.
+    #[error("Split error: {0}")]
+    SplitError(#[from] SplitError),
+}
+
+/// All errors related to JSON validation/parsing.
+#[derive(Debug, Error)]
+#[non_exhaustive]
+pub enum JsonError {
+    /// Signals that `target` is not of type `of_type` ([`JsonType`]).
+    #[error("Value in {target:?} must be a JSON {of_type:?}")]
+    NotOfType {
+        /// An arbitrary "target" that doesn't have the required type.
+        target: String,
+        /// The JSON type the target was expected to be.
+        of_type: JsonType,
+    },
+
+    /// Like [`JsonError::NotOfType`], only called when the `target` is a multiple;
+    /// array, set, etc.
+    #[error("Values in {target:?} must be JSON {of_type:?}s")]
+    NotMultiplesOfType {
+        /// An arbitrary "target" where
+        /// each or one of it's elements doesn't have the required type.
+        target: String,
+        /// The JSON type the element was expected to be.
+        of_type: JsonType,
+    },
+
+    /// Signals that a specific field is missing from a JSON object.
+    #[error("JSON object must contain the field {0:?}")]
+    JsonFieldMissingFromObject(String),
+
+    /// Signals a key missing from a JSON object.
+    ///
+    /// Note that this is different from [`JsonError::JsonFieldMissingFromObject`],
+    /// this error talks about an expected identifying key (`"ed25519:abcd"`)
+    /// missing from a target, where the key has a specific "type"/name.
+    #[error("JSON object {for_target:?} does not have {type_of} key {with_key:?}")]
+    JsonKeyMissing {
+        /// The target from which the key is missing.
+        for_target: String,
+        /// The kind of thing the key indicates.
+        type_of: String,
+        /// The key that is missing.
+        with_key: String,
+    },
+
+    /// A derivative error from [`ruma_serde::CanonicalJsonError`],
+    /// captured here.
+    #[error("Canonical JSON error: {0}")]
+    CanonicalJson(#[from] ruma_serde::CanonicalJsonError),
+
+    /// A more generic JSON error from [`serde_json`].
+    #[error(transparent)]
+    Serde(#[from] serde_json::Error),
+}
+
+// TODO: make macro for this
+impl JsonError {
+    pub(crate) fn not_of_type<T: Into<String>>(target: T, of_type: JsonType) -> Error {
+        Self::NotOfType { target: target.into(), of_type }.into()
+    }
+
+    pub(crate) fn not_multiples_of_type<T: Into<String>>(target: T, of_type: JsonType) -> Error {
+        Self::NotMultiplesOfType { target: target.into(), of_type }.into()
+    }
+
+    pub(crate) fn field_missing_from_object<T: Into<String>>(target: T) -> Error {
+        Self::JsonFieldMissingFromObject(target.into()).into()
+    }
+
+    pub(crate) fn key_missing<T1: Into<String>, T2: Into<String>, T3: Into<String>>(
+        for_target: T1,
+        type_of: T2,
+        with_key: T3,
+    ) -> Error {
+        Self::JsonKeyMissing {
+            for_target: for_target.into(),
+            type_of: type_of.into(),
+            with_key: with_key.into(),
+        }
+        .into()
+    }
+}
+
+/// A JSON type enum for [`JsonError`] variants.
+#[derive(Debug)]
+pub enum JsonType {
+    /// A JSON Object.
+    Object,
+    /// A JSON String.
+    String,
+    /// A JSON Integer.
+    Integer,
+    /// A JSON Array.
+    Array,
+    /// A JSON Boolean.
+    Boolean,
+    /// JSON Null.
+    Null,
+}
+
+/// Errors relating to verification of events and signatures.
+#[derive(Debug, Error)]
+#[non_exhaustive]
+pub enum VerificationError {
+    /// For when a signature cannot be found for a `target`.
+    #[error("Could not find signatures for {0:?}")]
+    SignatureNotFound(ServerNameBox),
+
+    /// For when a public key cannot be found for a `target`.
+    #[error("Could not find public key for {0:?}")]
+    PublicKeyNotFound(ServerNameBox),
+
+    /// For when no public key matches the signature given.
+    #[error("Not signed with any of the given public keys")]
+    UnknownPublicKeysForSignature,
+
+    /// For when [`ed25519_dalek`] cannot verify a signature.
+    #[error("Could not verify signature: {0}")]
+    Signature(#[source] ed25519_dalek::SignatureError),
+}
+
+impl VerificationError {
+    pub(crate) fn signature_not_found<T: Into<ServerNameBox>>(target: T) -> Error {
+        Self::SignatureNotFound(target.into()).into()
+    }
+
+    pub(crate) fn public_key_not_found<T: Into<ServerNameBox>>(target: T) -> Error {
+        Self::PublicKeyNotFound(target.into()).into()
+    }
+}
+
+/// Errors relating to parsing of all sorts.
+#[derive(Debug, Error)]
+#[non_exhaustive]
+pub enum ParseError {
+    /// For user ID parsing errors.
+    #[error("Could not parse User ID: {0}")]
+    UserId(#[source] ruma_identifiers::Error),
+    /// For event ID parsing errors.
+    #[error("Could not parse Event ID: {0}")]
+    EventId(#[source] ruma_identifiers::Error),
+
+    /// For when an event ID, coupled with a specific room version, doesn't have a server name
+    /// embedded.
+    #[error("Event Id {0:?} should have a server name for the given room version {1:?}")]
+    ServerNameFromEventIdByRoomVersion(EventId, RoomVersionId),
+
+    /// For when the extracted/"parsed" public key from a PKCS#8 v2 document doesn't match the
+    /// public key derived from it's private key.
+    #[error("PKCS#8 Document public key does not match public key derived from private key; derived: {0:X?} (len {}), parsed: {1:X?} (len {})", .derived_key.len(), .parsed_key.len())]
+    DerivedPublicKeyDoesNotMatchParsedKey {
+        /// The parsed key.
+        parsed_key: Vec<u8>,
+        /// The derived key.
+        derived_key: Vec<u8>,
+    },
+
+    /// For when the ASN.1 Object Identifier on a PKCS#8 document doesn't match the expected one.
+    ///
+    /// e.g. the document describes a RSA key, while an ed25519 key was expected.
+    #[error("Algorithm OID does not match ed25519, expected {expected}, found {found}")]
+    Oid {
+        /// The expected OID.
+        expected: pkcs8::ObjectIdentifier,
+        /// The OID that was found instead.
+        found: pkcs8::ObjectIdentifier,
+    },
+
+    /// For when [`ed25519_dalek`] cannot parse a secret/private key.
+    #[error("Could not parse secret key: {0}")]
+    SecretKey(#[source] ed25519_dalek::SignatureError),
+
+    /// For when [`ed25519_dalek`] cannot parse a public key.
+    #[error("Could not parse public key: {0}")]
+    PublicKey(#[source] ed25519_dalek::SignatureError),
+
+    /// For when [`ed25519_dalek`] cannot parse a signature.
+    #[error("Could not parse signature: {0}")]
+    Signature(#[source] ed25519_dalek::SignatureError),
+
+    /// For when parsing base64 gives an error.
+    #[error("Could not parse {of_type} base64 string {string:?}: {source}")]
+    Base64 {
+        /// The "type"/name of the base64 string
+        of_type: String,
+        /// The string itself.
+        string: String,
+        /// The originating error.
+        #[source]
+        source: base64::DecodeError,
+    },
+}
+
+impl ParseError {
+    pub(crate) fn from_event_id_by_room_version(
+        event_id: &EventId,
+        room_version: &RoomVersionId,
+    ) -> Error {
+        Self::ServerNameFromEventIdByRoomVersion(event_id.clone(), room_version.clone()).into()
+    }
+
+    pub(crate) fn derived_vs_parsed_mismatch<P: Into<Vec<u8>>, D: Into<Vec<u8>>>(
+        parsed: P,
+        derived: D,
+    ) -> Error {
+        Self::DerivedPublicKeyDoesNotMatchParsedKey {
+            parsed_key: parsed.into(),
+            derived_key: derived.into(),
+        }
+        .into()
+    }
+
+    pub(crate) fn base64<T1: Into<String>, T2: Into<String>>(
+        of_type: T1,
+        string: T2,
+        source: base64::DecodeError,
+    ) -> Error {
+        Self::Base64 { of_type: of_type.into(), string: string.into(), source }.into()
+    }
+}
+
+/// An error when trying to extract the algorithm and version from a key identifier.
+#[derive(Error, Debug)]
+pub enum SplitError {
+    /// 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}")]
+    InvalidLength(usize),
+
+    /// The signature's ID contains invalid characters in its version.
+    #[error("malformed signature ID: expected version to contain only characters in the character set `[a-zA-Z0-9_]`, found `{0}`")]
+    InvalidVersion(String),
+
+    /// The signature uses an unknown algorithm.
+    #[error("unknown algorithm: {0}")]
+    UnknownAlgorithm(String),
+}

crates/ruma-signatures/src/functions.rs

@@ -18,7 +18,7 @@ use crate::{
     keys::{KeyPair, PublicKeyMap},
     split_id,
     verification::{Ed25519Verifier, Verified, Verifier},
-    Error,
+    Error, JsonError, JsonType, ParseError, VerificationError,
 };
 
 /// The fields that are allowed to remain in an event during redaction.
@@ -142,14 +142,14 @@ where
 {
     let (signatures_key, mut signature_map) = match object.remove_entry("signatures") {
         Some((key, CanonicalJsonValue::Object(signatures))) => (Cow::Owned(key), signatures),
-        Some(_) => return Err(Error::new("field `signatures` must be a JSON object")),
+        Some(_) => return Err(JsonError::not_of_type("signatures", JsonType::Object)),
         None => (Cow::Borrowed("signatures"), BTreeMap::new()),
     };
 
     let maybe_unsigned_entry = object.remove_entry("unsigned");
 
     // Get the canonical JSON string.
-    let json = to_canonical_json_string(object)?;
+    let json = to_canonical_json_string(object).map_err(JsonError::CanonicalJson)?;
 
     // Sign the canonical JSON string.
     let signature = key_pair.sign(json.as_bytes());
@@ -161,7 +161,7 @@ where
 
     let signature_set = match signature_set {
         CanonicalJsonValue::Object(obj) => obj,
-        _ => return Err(Error::new("fields in `signatures` must be objects")),
+        _ => return Err(JsonError::not_multiples_of_type("signatures", JsonType::Object)),
     };
 
     signature_set.insert(signature.id(), CanonicalJsonValue::String(signature.base64()));
@@ -249,40 +249,43 @@ pub fn verify_json(
 ) -> Result<(), Error> {
     let signature_map = match object.get("signatures") {
         Some(CanonicalJsonValue::Object(signatures)) => signatures.clone(),
-        Some(_) => return Err(Error::new("field `signatures` must be a JSON object")),
+        Some(_) => return Err(JsonError::not_of_type("signatures", JsonType::Object)),
-        None => return Err(Error::new("JSON object must contain a `signatures` field")),
+        None => return Err(JsonError::field_missing_from_object("signatures")),
     };
 
     for (entity_id, signature_set) in signature_map {
         let signature_set = match signature_set {
             CanonicalJsonValue::Object(set) => set,
-            _ => return Err(Error::new("signature sets must be JSON objects")),
+            _ => return Err(JsonError::not_multiples_of_type("signature sets", JsonType::Object)),
         };
 
         let public_keys = match public_key_map.get(&entity_id) {
             Some(keys) => keys,
             None => {
-                return Err(Error::new(format!(
+                return Err(JsonError::key_missing("public_key_map", "public_keys", &entity_id))
-                    "no keys for signature in public_key_map for `{}`",
-                    entity_id
-                )))
             }
         };
 
         for (key_id, signature) in &signature_set {
             let signature = match signature {
                 CanonicalJsonValue::String(s) => s,
-                _ => return Err(Error::new("signature must be a string")),
+                _ => return Err(JsonError::not_of_type("signature", JsonType::String)),
             };
 
-            let public_key = public_keys
+            let public_key = public_keys.get(key_id).ok_or_else(|| {
-                .get(key_id)
+                JsonError::key_missing(
-                .ok_or_else(|| Error::new("no key for signature in public_key_map"))?;
+                    format!("public_keys of {}", &entity_id),
+                    "signature",
+                    key_id,
+                )
+            })?;
 
             let verify = |config: Config| {
-                let signature_bytes = decode_config(signature, config)?;
+                let signature_bytes = decode_config(signature, config)
+                    .map_err(|e| ParseError::base64("signature", signature, e))?;
 
-                let public_key_bytes = decode_config(&public_key, config)?;
+                let public_key_bytes = decode_config(&public_key, config)
+                    .map_err(|e| ParseError::base64("public key", public_key, e))?;
 
                 verify_json_with(&Ed25519Verifier, &public_key_bytes, &signature_bytes, object)
             };
@@ -488,7 +491,7 @@ where
         CanonicalJsonValue::Object(hashes) => {
             hashes.insert("sha256".into(), CanonicalJsonValue::String(hash))
         }
-        _ => return Err(Error::new("field `hashes` must be a JSON object")),
+        _ => return Err(JsonError::not_of_type("hashes", JsonType::Object)),
     };
 
     let mut redacted = redact(object, version)?;
@@ -579,39 +582,38 @@ pub fn verify_event(
             CanonicalJsonValue::Object(hashes) => match hashes.get("sha256") {
                 Some(hash_value) => match hash_value {
                     CanonicalJsonValue::String(hash) => hash,
-                    _ => return Err(Error::new("sha256 hash must be a JSON string")),
+                    _ => return Err(JsonError::not_of_type("sha256 hash", JsonType::String)),
                 },
-                None => return Err(Error::new("field `hashes` must be a JSON object")),
+                None => return Err(JsonError::not_of_type("hashes", JsonType::Object)),
             },
-            _ => return Err(Error::new("event missing sha256 hash")),
+            _ => return Err(JsonError::field_missing_from_object("sha256")),
         },
-        None => return Err(Error::new("field `hashes` must be present")),
+        None => return Err(JsonError::field_missing_from_object("hashes")),
     };
 
     let signature_map = match object.get("signatures") {
         Some(CanonicalJsonValue::Object(signatures)) => signatures,
-        Some(_) => return Err(Error::new("field `signatures` must be a JSON object")),
+        Some(_) => return Err(JsonError::not_of_type("signatures", JsonType::Object)),
-        None => return Err(Error::new("JSON object must contain a `signatures` field.")),
+        None => return Err(JsonError::field_missing_from_object("signatures")),
     };
 
     let servers_to_check = servers_to_check_signatures(object, version)?;
-    let canonical_json = from_json_str(&canonical_json(&redacted))?;
+    let canonical_json = from_json_str(&canonical_json(&redacted)).map_err(JsonError::from)?;
 
     for entity_id in servers_to_check {
         let signature_set = match signature_map.get(entity_id.as_str()) {
             Some(CanonicalJsonValue::Object(set)) => set,
-            Some(_) => return Err(Error::new("signatures sets must be JSON objects")),
+            Some(_) => {
-            None => {
+                return Err(JsonError::not_multiples_of_type("signature sets", JsonType::Object))
-                return Err(Error::new(format!("no signatures found for entity `{}`", entity_id)))
             }
+            None => return Err(VerificationError::signature_not_found(entity_id)),
         };
 
-        let mut maybe_signature = None;
+        let mut maybe_signature_and_public_key = None;
-        let mut maybe_public_key = None;
 
         let public_keys = public_key_map
             .get(entity_id.as_str())
-            .ok_or_else(|| Error::new(format!("missing public keys for server {}", entity_id)))?;
+            .ok_or_else(|| VerificationError::public_key_not_found(entity_id))?;
 
         for (key_id, public_key) in public_keys {
             // Since only ed25519 is supported right now, we don't actually need to check what the
@@ -621,32 +623,30 @@ pub fn verify_event(
             }
 
             if let Some(signature) = signature_set.get(key_id) {
-                maybe_signature = Some(signature);
+                maybe_signature_and_public_key = Some(SignatureAndPubkey { signature, public_key });
-                maybe_public_key = Some(public_key);
 
                 break;
             }
         }
 
-        let signature = match maybe_signature {
+        let signature_and_pubkey = match maybe_signature_and_public_key {
-            Some(CanonicalJsonValue::String(signature)) => signature,
+            Some(value) => value,
-            Some(_) => return Err(Error::new("signature must be a string")),
+            None => return Err(VerificationError::UnknownPublicKeysForSignature.into()),
-            None => {
-                return Err(Error::new("event is not signed with any of the given public keys"))
-            }
         };
 
-        let public_key = match maybe_public_key {
+        let signature = match signature_and_pubkey.signature {
-            Some(public_key) => public_key,
+            CanonicalJsonValue::String(signature) => signature,
-            None => {
+            _ => return Err(JsonError::not_of_type("signature", JsonType::String)),
-                return Err(Error::new("event is not signed with any of the given public keys"))
-            }
         };
 
+        let public_key = signature_and_pubkey.public_key;
+
         let verify = |config: Config| {
-            let signature_bytes = decode_config(signature, config)?;
+            let signature_bytes = decode_config(signature, config)
+                .map_err(|e| ParseError::base64("signature", signature, e))?;
 
-            let public_key_bytes = decode_config(&public_key, config)?;
+            let public_key_bytes = decode_config(&public_key, config)
+                .map_err(|e| ParseError::base64("public key", public_key, e))?;
 
             verify_json_with(&Ed25519Verifier, &public_key_bytes, &signature_bytes, &canonical_json)
         };
@@ -666,6 +666,11 @@ pub fn verify_event(
     }
 }
 
+struct SignatureAndPubkey<'a> {
+    signature: &'a CanonicalJsonValue,
+    public_key: &'a String,
+}
+
 /// Internal implementation detail of the canonical JSON algorithm. Allows customization of the
 /// fields that will be removed before serializing.
 fn canonical_json_with_fields_to_remove(object: &CanonicalJsonObject, fields: &[&str]) -> String {
@@ -707,18 +712,18 @@ pub fn redact(
 
     let event_type_value = match event.get("type") {
         Some(event_type_value) => event_type_value,
-        None => return Err(Error::new("field `type` in JSON value must be present")),
+        None => return Err(JsonError::field_missing_from_object("type")),
     };
 
     let allowed_content_keys = match event_type_value {
         CanonicalJsonValue::String(event_type) => allowed_content_keys_for(event_type, version),
-        _ => return Err(Error::new("field `type` in JSON value must be a JSON string")),
+        _ => return Err(JsonError::not_of_type("type", JsonType::String)),
     };
 
     if let Some(content_value) = event.get_mut("content") {
         let content = match content_value {
             CanonicalJsonValue::Object(map) => map,
-            _ => return Err(Error::new("field `content` in JSON value must be a JSON object")),
+            _ => return Err(JsonError::not_of_type("content", JsonType::Object)),
         };
 
         let mut old_content = mem::take(content);
@@ -752,12 +757,12 @@ fn servers_to_check_signatures(
     if !is_third_party_invite(object)? {
         match object.get("sender") {
             Some(CanonicalJsonValue::String(raw_sender)) => {
-                let user_id = UserId::from_str(raw_sender)
+                let user_id =
-                    .map_err(|_| Error::new("could not parse user id"))?;
+                    UserId::from_str(raw_sender).map_err(|e| Error::from(ParseError::UserId(e)))?;
 
                 servers_to_check.insert(user_id.server_name().to_owned());
             }
-            _ => return Err(Error::new("field `sender` must be a JSON string")),
+            _ => return Err(JsonError::not_of_type("sender", JsonType::String)),
         };
     }
 
@@ -765,21 +770,17 @@ fn servers_to_check_signatures(
         RoomVersionId::Version1 | RoomVersionId::Version2 => match object.get("event_id") {
             Some(CanonicalJsonValue::String(raw_event_id)) => {
                 let event_id = EventId::from_str(raw_event_id)
-                    .map_err(|_| Error::new("could not parse event id"))?;
+                    .map_err(|e| Error::from(ParseError::EventId(e)))?;
 
                 let server_name = event_id
                     .server_name()
-                    .ok_or_else(|| {
+                    .ok_or_else(|| ParseError::from_event_id_by_room_version(&event_id, version))?
-                        Error::new("Event id should have a server name for the given room version")
-                    })?
                     .to_owned();
 
                 servers_to_check.insert(server_name);
             }
             _ => {
-                return Err(Error::new(
+                return Err(JsonError::field_missing_from_object("event_id"));
-                    "Expected to find a string `event_id` for the given room version",
-                ))
             }
         },
         _ => (),
@@ -792,7 +793,7 @@ fn servers_to_check_signatures(
 fn is_third_party_invite(object: &CanonicalJsonObject) -> Result<bool, Error> {
     match object.get("type") {
         Some(CanonicalJsonValue::String(raw_type)) => Ok(raw_type == "m.room.third_party_invite"),
-        _ => Err(Error::new("field `type` must be a JSON string")),
+        _ => Err(JsonError::not_of_type("type", JsonType::String)),
     }
 }
 
@@ -844,7 +845,10 @@ mod tests {
     use serde_json::json;
 
     use super::canonical_json;
-    use crate::{sign_json, verify_event, Ed25519KeyPair, PublicKeyMap, PublicKeySet, Verified};
+    use crate::{
+        sign_json, verify_event, Ed25519KeyPair, Error, PublicKeyMap, PublicKeySet,
+        VerificationError, Verified,
+    };
 
     #[test]
     fn canonical_json_complex() {
@@ -1000,8 +1004,12 @@ mod tests {
             verify_event(&public_key_map, &signed_event, &RoomVersionId::Version6);
 
         assert!(verification_result.is_err());
-        let error_msg = verification_result.err().unwrap().message;
+        let error_msg = verification_result.err().unwrap();
-        assert!(error_msg.contains("missing public keys for server"));
+        if let Error::Verification(VerificationError::PublicKeyNotFound(entity)) = error_msg {
+            assert_eq!(entity, "domain-sender");
+        } else {
+            panic!("Error was not VerificationError::UnknownPublicKeysForEvent: {:?}", error_msg);
+        };
     }
 
     #[test]
@@ -1046,8 +1054,14 @@ mod tests {
             verify_event(&public_key_map, &signed_event, &RoomVersionId::Version6);
 
         assert!(verification_result.is_err());
-        let error_msg = verification_result.err().unwrap().message;
+        let error_msg = verification_result.err().unwrap();
-        assert!(error_msg.contains("Could not verify signature"));
+        if let Error::Verification(VerificationError::Signature(error)) = error_msg {
+            // dalek doesn't expose InternalError :(
+            // https://github.com/dalek-cryptography/ed25519-dalek/issues/174
+            assert!(format!("{:?}", error).contains("Some(Verification equation was not satisfied)"))
+        } else {
+            panic!("Error was not VerificationError::Signature: {:?}", error_msg);
+        };
     }
 
     fn generate_key_pair() -> Ed25519KeyPair {

crates/ruma-signatures/src/keys.rs

@@ -12,7 +12,7 @@ use pkcs8::{
     AlgorithmIdentifier, ObjectIdentifier, OneAsymmetricKey, PrivateKeyInfo,
 };
 
-use crate::{signatures::Signature, Algorithm, Error};
+use crate::{signatures::Signature, Algorithm, Error, ParseError};
 
 /// A cryptographic key pair for digitally signing data.
 pub trait KeyPair: Sized {
@@ -45,14 +45,11 @@ impl Ed25519KeyPair {
         version: String,
     ) -> Result<Self, Error> {
         if oid != ED25519_OID {
-            return Err(Error::new(format!(
+            return Err(ParseError::Oid { expected: ED25519_OID, found: oid }.into());
-                "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()))?;
+            .map_err(ParseError::SecretKey)?;
 
         let derived_pubkey = PublicKey::from(&secret_key);
 
@@ -60,13 +57,10 @@ impl Ed25519KeyPair {
             // If the document had a public key, we're verifying it.
 
             if oak_key != derived_pubkey.as_bytes() {
-                return Err(Error::new(format!(
+                return Err(ParseError::derived_vs_parsed_mismatch(
-                    "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()
+                    derived_pubkey.as_bytes().to_vec(),
-                )));
+                ));
             }
         }
 
@@ -96,7 +90,7 @@ 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> {
-        let oak = OneAsymmetricKey::from_der(document).map_err(|e| Error::new(e.to_string()))?;
+        let oak = OneAsymmetricKey::from_der(document).map_err(Error::DerParse)?;
 
         Self::from_pkcs8_oak(oak, version)
     }
@@ -148,7 +142,7 @@ impl Ed25519KeyPair {
             public_key: Some(public.as_bytes()),
         };
 
-        oak.to_vec().map_err(|e| Error::new(e.to_string()))
+        oak.to_vec().map_err(Error::DerParse)
     }
 
     /// Returns the version string for this keypair.

crates/ruma-signatures/src/lib.rs

@@ -44,13 +44,9 @@
 
 #![warn(missing_docs)]
 
-use std::{
-    error::Error as StdError,
-    fmt::{Display, Formatter, Result as FmtResult},
-};
-
 use ruma_serde::{AsRefStr, DisplayAsRefStr};
 
+pub use error::{Error, JsonError, JsonType, ParseError, SplitError, VerificationError};
 pub use functions::{
     canonical_json, content_hash, hash_and_sign_event, redact, reference_hash, sign_json,
     verify_event, verify_json,
@@ -60,62 +56,12 @@ pub use ruma_serde::{CanonicalJsonError, CanonicalJsonObject, CanonicalJsonValue
 pub use signatures::Signature;
 pub use verification::Verified;
 
+mod error;
 mod functions;
 mod keys;
 mod signatures;
 mod verification;
 
-/// An error produced when ruma-signatures operations fail.
-#[derive(Clone, Debug, PartialEq)]
-pub struct Error {
-    /// A human-readable description of the error.
-    message: String,
-}
-
-impl Error {
-    /// Creates a new error.
-    ///
-    /// # Parameters
-    ///
-    /// * message: The error message.
-    pub(crate) fn new<T>(message: T) -> Self
-    where
-        T: Into<String>,
-    {
-        Self { message: message.into() }
-    }
-}
-
-impl StdError for Error {
-    fn description(&self) -> &str {
-        &self.message
-    }
-}
-
-impl Display for Error {
-    fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
-        write!(f, "{}", self.message)
-    }
-}
-
-impl From<base64::DecodeError> for Error {
-    fn from(error: base64::DecodeError) -> Self {
-        Self::new(error.to_string())
-    }
-}
-
-impl From<serde_json::Error> for Error {
-    fn from(error: serde_json::Error) -> Self {
-        Self::new(error.to_string())
-    }
-}
-
-impl From<CanonicalJsonError> for Error {
-    fn from(error: CanonicalJsonError) -> Self {
-        Self::new(error.to_string())
-    }
-}
-
 /// The algorithm used for signing data.
 #[derive(Clone, Debug, Eq, Hash, PartialEq, AsRefStr, DisplayAsRefStr)]
 #[ruma_enum(rename_all = "snake_case")]
@@ -124,21 +70,8 @@ pub enum Algorithm {
     Ed25519,
 }
 
-/// An error when trying to extract the algorithm and version from a key identifier.
-#[derive(Clone, Debug, PartialEq)]
-enum SplitError<'a> {
-    /// The signature's ID does not have exactly two components separated by a colon.
-    InvalidLength(usize),
-
-    /// The signature's ID contains invalid characters in its version.
-    InvalidVersion(&'a str),
-
-    /// The signature uses an unknown algorithm.
-    UnknownAlgorithm(&'a str),
-}
-
 /// Extract the algorithm and version from a key identifier.
-fn split_id(id: &str) -> Result<(Algorithm, String), SplitError<'_>> {
+fn split_id(id: &str) -> Result<(Algorithm, String), SplitError> {
     /// The length of a valid signature ID.
     const SIGNATURE_ID_LENGTH: usize = 2;
 
@@ -153,14 +86,14 @@ fn split_id(id: &str) -> Result<(Algorithm, String), SplitError<'_>> {
     let version = signature_id[1];
 
     if !version.bytes().all(|ch| ch.is_ascii_alphanumeric() || ch == b'_') {
-        return Err(SplitError::InvalidVersion(version));
+        return Err(SplitError::InvalidVersion(version.into()));
     }
 
     let algorithm_input = signature_id[0];
 
     let algorithm = match algorithm_input {
         "ed25519" => Algorithm::Ed25519,
-        algorithm => return Err(SplitError::UnknownAlgorithm(algorithm)),
+        algorithm => return Err(SplitError::UnknownAlgorithm(algorithm.into())),
     };
 
     Ok((algorithm, signature_id[1].to_owned()))

crates/ruma-signatures/src/signatures.rs

@@ -2,7 +2,7 @@
 
 use base64::{encode_config, STANDARD_NO_PAD};
 
-use crate::{split_id, Algorithm, Error, SplitError};
+use crate::{split_id, Algorithm, Error};
 
 /// A digital signature.
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]
@@ -42,21 +42,7 @@ impl Signature {
     /// * The key ID is malformed.
     /// * The key ID contains a version with invalid characters.
     pub fn new(id: &str, bytes: &[u8]) -> Result<Self, Error> {
-        let (algorithm, version) = split_id(id).map_err(|split_error| match split_error {
+        let (algorithm, version) = split_id(id)?;
-            SplitError::InvalidLength(length) => Error::new(format!(
-                "malformed signature ID: expected exactly \
-                 2 segment separated by a colon, found {}",
-                length
-            )),
-            SplitError::InvalidVersion(version) => Error::new(format!(
-                "malformed signature ID: expected version to contain only \
-                 characters in the character set `[a-zA-Z0-9_]`, found `{}`",
-                version
-            )),
-            SplitError::UnknownAlgorithm(algorithm) => {
-                Error::new(format!("unknown algorithm: {}", algorithm))
-            }
-        })?;
 
         Ok(Self { algorithm, signature: bytes.to_vec(), version })
     }

crates/ruma-signatures/src/verification.rs

@@ -4,7 +4,7 @@ use std::convert::TryInto;
 
 use ed25519_dalek::{PublicKey, Verifier as _};
 
-use crate::Error;
+use crate::{Error, ParseError, VerificationError};
 
 /// A digital signature verifier.
 pub trait Verifier {
@@ -35,14 +35,10 @@ impl Verifier for Ed25519Verifier {
         message: &[u8],
     ) -> Result<(), Error> {
         PublicKey::from_bytes(public_key)
-            .map_err(|e| Error::new(format!("Could not parse public key: {:?}", e)))?
+            .map_err(ParseError::PublicKey)?
-            .verify(
+            .verify(message, &signature.try_into().map_err(ParseError::Signature)?)
-                message,
+            .map_err(VerificationError::Signature)
-                &signature
+            .map_err(Error::from)
-                    .try_into()
-                    .map_err(|e| Error::new(format!("Could not parse signature: {:?}", e)))?,
-            )
-            .map_err(|e| Error::new(format!("Could not verify signature: {:?}", e)))
     }
 }