From cc50f8b8dca7b7a232a1ef09c7ebd9258cc48336 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 24 May 2021 22:38:51 +0200 Subject: [PATCH] Convert `ruma-signatures` error story into `thiserror` --- crates/ruma-signatures/Cargo.toml | 3 +- crates/ruma-signatures/src/error.rs | 262 +++++++++++++++++++++ crates/ruma-signatures/src/functions.rs | 144 ++++++----- crates/ruma-signatures/src/keys.rs | 22 +- crates/ruma-signatures/src/lib.rs | 77 +----- crates/ruma-signatures/src/signatures.rs | 18 +- crates/ruma-signatures/src/verification.rs | 14 +- 7 files changed, 363 insertions(+), 177 deletions(-) create mode 100644 crates/ruma-signatures/src/error.rs diff --git a/crates/ruma-signatures/Cargo.toml b/crates/ruma-signatures/Cargo.toml index 7ba8e17e..3adb0d40 100644 --- a/crates/ruma-signatures/Cargo.toml +++ b/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" diff --git a/crates/ruma-signatures/src/error.rs b/crates/ruma-signatures/src/error.rs new file mode 100644 index 00000000..0a2908d4 --- /dev/null +++ b/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>(target: T, of_type: JsonType) -> Error { + Self::NotOfType { target: target.into(), of_type }.into() + } + + pub(crate) fn not_multiples_of_type>(target: T, of_type: JsonType) -> Error { + Self::NotMultiplesOfType { target: target.into(), of_type }.into() + } + + pub(crate) fn field_missing_from_object>(target: T) -> Error { + Self::JsonFieldMissingFromObject(target.into()).into() + } + + pub(crate) fn key_missing, T2: Into, T3: Into>( + 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>(target: T) -> Error { + Self::SignatureNotFound(target.into()).into() + } + + pub(crate) fn public_key_not_found>(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, + /// The derived key. + derived_key: Vec, + }, + + /// 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>, D: Into>>( + parsed: P, + derived: D, + ) -> Error { + Self::DerivedPublicKeyDoesNotMatchParsedKey { + parsed_key: parsed.into(), + derived_key: derived.into(), + } + .into() + } + + pub(crate) fn base64, T2: Into>( + 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), +} diff --git a/crates/ruma-signatures/src/functions.rs b/crates/ruma-signatures/src/functions.rs index eb67480e..262f44cd 100644 --- a/crates/ruma-signatures/src/functions.rs +++ b/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")), - None => return Err(Error::new("JSON object must contain a `signatures` field")), + Some(_) => return Err(JsonError::not_of_type("signatures", JsonType::Object)), + 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!( - "no keys for signature in public_key_map for `{}`", - entity_id - ))) + return Err(JsonError::key_missing("public_key_map", "public_keys", &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 - .get(key_id) - .ok_or_else(|| Error::new("no key for signature in public_key_map"))?; + let public_key = public_keys.get(key_id).ok_or_else(|| { + JsonError::key_missing( + 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")), - None => return Err(Error::new("JSON object must contain a `signatures` field.")), + Some(_) => return Err(JsonError::not_of_type("signatures", JsonType::Object)), + 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")), - None => { - return Err(Error::new(format!("no signatures found for entity `{}`", entity_id))) + Some(_) => { + return Err(JsonError::not_multiples_of_type("signature sets", JsonType::Object)) } + None => return Err(VerificationError::signature_not_found(entity_id)), }; - let mut maybe_signature = None; - let mut maybe_public_key = None; + let mut maybe_signature_and_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_public_key = Some(public_key); + maybe_signature_and_public_key = Some(SignatureAndPubkey { signature, public_key }); break; } } - let signature = match maybe_signature { - Some(CanonicalJsonValue::String(signature)) => signature, - Some(_) => return Err(Error::new("signature must be a string")), - None => { - return Err(Error::new("event is not signed with any of the given public keys")) - } + let signature_and_pubkey = match maybe_signature_and_public_key { + Some(value) => value, + None => return Err(VerificationError::UnknownPublicKeysForSignature.into()), }; - let public_key = match maybe_public_key { - Some(public_key) => public_key, - None => { - return Err(Error::new("event is not signed with any of the given public keys")) - } + let signature = match signature_and_pubkey.signature { + CanonicalJsonValue::String(signature) => signature, + _ => return Err(JsonError::not_of_type("signature", JsonType::String)), }; + 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) - .map_err(|_| Error::new("could not parse user id"))?; + let 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(|| { - Error::new("Event id should have a server name for the given room version") - })? + .ok_or_else(|| ParseError::from_event_id_by_room_version(&event_id, version))? .to_owned(); servers_to_check.insert(server_name); } _ => { - return Err(Error::new( - "Expected to find a string `event_id` for the given room version", - )) + return Err(JsonError::field_missing_from_object("event_id")); } }, _ => (), @@ -792,7 +793,7 @@ fn servers_to_check_signatures( fn is_third_party_invite(object: &CanonicalJsonObject) -> Result { 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; - assert!(error_msg.contains("missing public keys for server")); + let error_msg = verification_result.err().unwrap(); + 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; - assert!(error_msg.contains("Could not verify signature")); + let error_msg = verification_result.err().unwrap(); + 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 { diff --git a/crates/ruma-signatures/src/keys.rs b/crates/ruma-signatures/src/keys.rs index 3c7756b2..a7183709 100644 --- a/crates/ruma-signatures/src/keys.rs +++ b/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 { if oid != ED25519_OID { - return Err(Error::new(format!( - "Algorithm OID does not match ed25519, expected {}, found {}", - ED25519_OID, oid - ))); + return Err(ParseError::Oid { expected: ED25519_OID, found: oid }.into()); } 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!( - "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(), + return Err(ParseError::derived_vs_parsed_mismatch( 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 { - 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. diff --git a/crates/ruma-signatures/src/lib.rs b/crates/ruma-signatures/src/lib.rs index a0b9c88d..f5b5aff9 100644 --- a/crates/ruma-signatures/src/lib.rs +++ b/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(message: T) -> Self - where - T: Into, - { - 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 for Error { - fn from(error: base64::DecodeError) -> Self { - Self::new(error.to_string()) - } -} - -impl From for Error { - fn from(error: serde_json::Error) -> Self { - Self::new(error.to_string()) - } -} - -impl From 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())) diff --git a/crates/ruma-signatures/src/signatures.rs b/crates/ruma-signatures/src/signatures.rs index a599915a..f8926645 100644 --- a/crates/ruma-signatures/src/signatures.rs +++ b/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 { - let (algorithm, version) = split_id(id).map_err(|split_error| match split_error { - 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)) - } - })?; + let (algorithm, version) = split_id(id)?; Ok(Self { algorithm, signature: bytes.to_vec(), version }) } diff --git a/crates/ruma-signatures/src/verification.rs b/crates/ruma-signatures/src/verification.rs index 432ac7ad..13851acd 100644 --- a/crates/ruma-signatures/src/verification.rs +++ b/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)))? - .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))) + .map_err(ParseError::PublicKey)? + .verify(message, &signature.try_into().map_err(ParseError::Signature)?) + .map_err(VerificationError::Signature) + .map_err(Error::from) } }