From 3c76fa1492e8d88191544f1d9f2e06df1d73e383 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 29 Jul 2021 12:12:45 +0200 Subject: [PATCH] signatures: Fix verify_json and sign_json enforcing PDU size limits These functions are used for request signatures too. --- crates/ruma-serde/src/canonical_json.rs | 26 ++----------------- crates/ruma-serde/src/lib.rs | 2 +- crates/ruma-signatures/src/error.rs | 9 +++---- crates/ruma-signatures/src/functions.rs | 34 +++++++++++++++++++------ 4 files changed, 33 insertions(+), 38 deletions(-) diff --git a/crates/ruma-serde/src/canonical_json.rs b/crates/ruma-serde/src/canonical_json.rs index 83b0744e..513ccd0c 100644 --- a/crates/ruma-serde/src/canonical_json.rs +++ b/crates/ruma-serde/src/canonical_json.rs @@ -7,21 +7,6 @@ pub mod value; use value::Object as CanonicalJsonObject; -/// Returns a canonical JSON string according to Matrix specification. -/// -/// This function should be preferred over `serde_json::to_string` since it checks the size of the -/// canonical string. Matrix canonical JSON enforces a size limit of less than 65,535 when sending -/// PDU's for the server-server protocol. -pub fn to_string(val: &T) -> Result { - let s = serde_json::to_string(val).map_err(Error::SerDe)?; - - if s.as_bytes().len() > 65_535 { - Err(Error::JsonSize) - } else { - Ok(s) - } -} - /// The set of possible errors when serializing to canonical JSON. #[derive(Debug)] #[allow(clippy::exhaustive_enums)] @@ -29,9 +14,6 @@ pub enum Error { /// The numeric value failed conversion to js_int::Int. IntConvert, - /// The `CanonicalJsonValue` being serialized was larger than 65,535 bytes. - JsonSize, - /// An error occurred while serializing/deserializing. SerDe(JsonError), } @@ -40,7 +22,6 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Error::IntConvert => f.write_str("number found is not a valid `js_int::Int`"), - Error::JsonSize => f.write_str("JSON is larger than 65,535 byte max"), Error::SerDe(err) => write!(f, "serde Error: {}", err), } } @@ -67,10 +48,7 @@ mod tests { use js_int::int; use serde_json::{from_str as from_json_str, json, to_string as to_json_string}; - use super::{ - to_canonical_value, to_string as to_canonical_json_string, try_from_json_map, - value::CanonicalJsonValue, - }; + use super::{to_canonical_value, try_from_json_map, value::CanonicalJsonValue}; #[test] fn serialize_canon() { @@ -82,7 +60,7 @@ mod tests { .try_into() .unwrap(); - let ser = to_canonical_json_string(&json).unwrap(); + let ser = to_json_string(&json).unwrap(); let back = from_json_str::(&ser).unwrap(); assert_eq!(json, back); diff --git a/crates/ruma-serde/src/lib.rs b/crates/ruma-serde/src/lib.rs index c933523b..7b9fc68c 100644 --- a/crates/ruma-serde/src/lib.rs +++ b/crates/ruma-serde/src/lib.rs @@ -20,7 +20,7 @@ pub mod urlencoded; pub use buf::{json_to_buf, slice_to_buf}; pub use can_be_empty::{is_empty, CanBeEmpty}; pub use canonical_json::{ - to_canonical_value, to_string as to_canonical_json_string, try_from_json_map, + to_canonical_value, try_from_json_map, value::{CanonicalJsonValue, Object as CanonicalJsonObject}, Error as CanonicalJsonError, }; diff --git a/crates/ruma-signatures/src/error.rs b/crates/ruma-signatures/src/error.rs index 9304faef..c428a63b 100644 --- a/crates/ruma-signatures/src/error.rs +++ b/crates/ruma-signatures/src/error.rs @@ -24,6 +24,10 @@ pub enum Error { /// [`SplitError`] wrapper. #[error("Split error: {0}")] SplitError(#[from] SplitError), + + /// PDU was too large + #[error("PDU is larger than maximum of 65535 bytes")] + PduSize, } /// All errors related to JSON validation/parsing. @@ -69,11 +73,6 @@ pub enum JsonError { 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), diff --git a/crates/ruma-signatures/src/functions.rs b/crates/ruma-signatures/src/functions.rs index b5474c85..26598d70 100644 --- a/crates/ruma-signatures/src/functions.rs +++ b/crates/ruma-signatures/src/functions.rs @@ -10,8 +10,8 @@ use std::{ use base64::{decode_config, encode_config, Config, STANDARD_NO_PAD, URL_SAFE_NO_PAD}; 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 ruma_serde::{CanonicalJsonObject, CanonicalJsonValue}; +use serde_json::{from_str as from_json_str, to_string as to_json_string}; use sha2::Sha256; use crate::{ @@ -21,6 +21,8 @@ use crate::{ Error, JsonError, JsonType, ParseError, VerificationError, }; +const MAX_PDU_BYTES: usize = 65_535; + /// The fields that are allowed to remain in an event during redaction. static ALLOWED_KEYS: &[&str] = &[ "event_id", @@ -149,7 +151,7 @@ where let maybe_unsigned_entry = object.remove_entry("unsigned"); // Get the canonical JSON string. - let json = to_canonical_json_string(object).map_err(JsonError::CanonicalJson)?; + let json = to_json_string(object).map_err(JsonError::Serde)?; // Sign the canonical JSON string. let signature = key_pair.sign(json.as_bytes()); @@ -204,10 +206,14 @@ pub fn canonical_json(object: &CanonicalJsonObject) -> Result { /// Uses a set of public keys to verify a signed JSON object. /// +/// Unlike `content_hash` and `reference_hash`, this function does not report an error if the +/// canonical JSON is larger than 65535 bytes; this function may be used for requests that are +/// larger than just one PDU's maximum size. +/// /// # Parameters /// /// * public_key_map: A map from entity identifiers to a map from key identifiers to public keys. -/// Generally, entity identifiers are server names—the host/IP/port of a homeserver (e.g. +/// Generally, entity identifiers are server names — the host/IP/port of a homeserver (e.g. /// "example.com") for which a signature must be verified. Key identifiers for each server (e.g. /// "ed25519:1") then map to their respective public keys. /// * object: The JSON object that was signed. @@ -334,8 +340,16 @@ where /// # Parameters /// /// object: A JSON object to generate a content hash for. +/// +/// # Errors +/// +/// Returns an error if the event is too large. pub fn content_hash(object: &CanonicalJsonObject) -> Result { let json = canonical_json_with_fields_to_remove(object, CONTENT_HASH_FIELDS_TO_REMOVE)?; + if json.len() > MAX_PDU_BYTES { + return Err(Error::PduSize); + } + let hash = Sha256::digest(json.as_bytes()); Ok(encode_config(&hash, STANDARD_NO_PAD)) @@ -355,7 +369,7 @@ pub fn content_hash(object: &CanonicalJsonObject) -> Result { /// /// # Errors /// -/// Returns an error if redaction fails. +/// Returns an error if the event is too large or redaction fails. pub fn reference_hash( value: &CanonicalJsonObject, version: &RoomVersionId, @@ -364,6 +378,9 @@ pub fn reference_hash( let json = canonical_json_with_fields_to_remove(&redacted_value, REFERENCE_HASH_FIELDS_TO_REMOVE)?; + if json.len() > MAX_PDU_BYTES { + return Err(Error::PduSize); + } let hash = Sha256::digest(json.as_bytes()); @@ -671,8 +688,9 @@ struct SignatureAndPubkey<'a> { public_key: &'a String, } -/// Internal implementation detail of the canonical JSON algorithm. Allows customization of the -/// fields that will be removed before serializing. +/// 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], @@ -683,7 +701,7 @@ fn canonical_json_with_fields_to_remove( owned_object.remove(*field); } - to_canonical_json_string(&owned_object).map_err(|e| Error::Json(e.into())) + to_json_string(&owned_object).map_err(|e| Error::Json(e.into())) } /// Redacts an event using the rules specified in the Matrix client-server specification.