From 94b107b42b722aff9518f64ad603ce01665b25f3 Mon Sep 17 00:00:00 2001 From: June Clementine Strawberry Date: Thu, 3 Apr 2025 16:08:02 -0400 Subject: [PATCH] add some debug logging and misc cleanup to keys/signatures/upload Signed-off-by: June Clementine Strawberry --- Cargo.lock | 22 +++++----- Cargo.toml | 2 +- src/api/client/keys.rs | 95 ++++++++++++++++++++++++++-------------- src/service/users/mod.rs | 18 +++++--- 4 files changed, 86 insertions(+), 51 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8918a631..0753f81d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3654,7 +3654,7 @@ dependencies = [ [[package]] name = "ruma" version = "0.10.1" -source = "git+https://github.com/girlbossceo/ruwuma?rev=0701341a2fd5a6ea74beada18d5974cc401a4fc1#0701341a2fd5a6ea74beada18d5974cc401a4fc1" +source = "git+https://github.com/girlbossceo/ruwuma?rev=edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef#edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef" dependencies = [ "assign", "js_int", @@ -3674,7 +3674,7 @@ dependencies = [ [[package]] name = "ruma-appservice-api" version = "0.10.0" -source = "git+https://github.com/girlbossceo/ruwuma?rev=0701341a2fd5a6ea74beada18d5974cc401a4fc1#0701341a2fd5a6ea74beada18d5974cc401a4fc1" +source = "git+https://github.com/girlbossceo/ruwuma?rev=edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef#edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef" dependencies = [ "js_int", "ruma-common", @@ -3686,7 +3686,7 @@ dependencies = [ [[package]] name = "ruma-client-api" version = "0.18.0" -source = "git+https://github.com/girlbossceo/ruwuma?rev=0701341a2fd5a6ea74beada18d5974cc401a4fc1#0701341a2fd5a6ea74beada18d5974cc401a4fc1" +source = "git+https://github.com/girlbossceo/ruwuma?rev=edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef#edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef" dependencies = [ "as_variant", "assign", @@ -3709,7 +3709,7 @@ dependencies = [ [[package]] name = "ruma-common" version = "0.13.0" -source = "git+https://github.com/girlbossceo/ruwuma?rev=0701341a2fd5a6ea74beada18d5974cc401a4fc1#0701341a2fd5a6ea74beada18d5974cc401a4fc1" +source = "git+https://github.com/girlbossceo/ruwuma?rev=edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef#edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef" dependencies = [ "as_variant", "base64 0.22.1", @@ -3741,7 +3741,7 @@ dependencies = [ [[package]] name = "ruma-events" version = "0.28.1" -source = "git+https://github.com/girlbossceo/ruwuma?rev=0701341a2fd5a6ea74beada18d5974cc401a4fc1#0701341a2fd5a6ea74beada18d5974cc401a4fc1" +source = "git+https://github.com/girlbossceo/ruwuma?rev=edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef#edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef" dependencies = [ "as_variant", "indexmap 2.8.0", @@ -3766,7 +3766,7 @@ dependencies = [ [[package]] name = "ruma-federation-api" version = "0.9.0" -source = "git+https://github.com/girlbossceo/ruwuma?rev=0701341a2fd5a6ea74beada18d5974cc401a4fc1#0701341a2fd5a6ea74beada18d5974cc401a4fc1" +source = "git+https://github.com/girlbossceo/ruwuma?rev=edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef#edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef" dependencies = [ "bytes", "headers", @@ -3788,7 +3788,7 @@ dependencies = [ [[package]] name = "ruma-identifiers-validation" version = "0.9.5" -source = "git+https://github.com/girlbossceo/ruwuma?rev=0701341a2fd5a6ea74beada18d5974cc401a4fc1#0701341a2fd5a6ea74beada18d5974cc401a4fc1" +source = "git+https://github.com/girlbossceo/ruwuma?rev=edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef#edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef" dependencies = [ "js_int", "thiserror 2.0.12", @@ -3797,7 +3797,7 @@ dependencies = [ [[package]] name = "ruma-identity-service-api" version = "0.9.0" -source = "git+https://github.com/girlbossceo/ruwuma?rev=0701341a2fd5a6ea74beada18d5974cc401a4fc1#0701341a2fd5a6ea74beada18d5974cc401a4fc1" +source = "git+https://github.com/girlbossceo/ruwuma?rev=edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef#edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef" dependencies = [ "js_int", "ruma-common", @@ -3807,7 +3807,7 @@ dependencies = [ [[package]] name = "ruma-macros" version = "0.13.0" -source = "git+https://github.com/girlbossceo/ruwuma?rev=0701341a2fd5a6ea74beada18d5974cc401a4fc1#0701341a2fd5a6ea74beada18d5974cc401a4fc1" +source = "git+https://github.com/girlbossceo/ruwuma?rev=edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef#edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef" dependencies = [ "cfg-if", "proc-macro-crate", @@ -3822,7 +3822,7 @@ dependencies = [ [[package]] name = "ruma-push-gateway-api" version = "0.9.0" -source = "git+https://github.com/girlbossceo/ruwuma?rev=0701341a2fd5a6ea74beada18d5974cc401a4fc1#0701341a2fd5a6ea74beada18d5974cc401a4fc1" +source = "git+https://github.com/girlbossceo/ruwuma?rev=edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef#edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef" dependencies = [ "js_int", "ruma-common", @@ -3834,7 +3834,7 @@ dependencies = [ [[package]] name = "ruma-signatures" version = "0.15.0" -source = "git+https://github.com/girlbossceo/ruwuma?rev=0701341a2fd5a6ea74beada18d5974cc401a4fc1#0701341a2fd5a6ea74beada18d5974cc401a4fc1" +source = "git+https://github.com/girlbossceo/ruwuma?rev=edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef#edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef" dependencies = [ "base64 0.22.1", "ed25519-dalek", diff --git a/Cargo.toml b/Cargo.toml index bf7ec2bb..a44fc0f0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -350,7 +350,7 @@ version = "0.1.2" [workspace.dependencies.ruma] git = "https://github.com/girlbossceo/ruwuma" #branch = "conduwuit-changes" -rev = "0701341a2fd5a6ea74beada18d5974cc401a4fc1" +rev = "edbdc79e560d01d9e4a76f7421e70ea4fd4c54ef" features = [ "compat", "rand", diff --git a/src/api/client/keys.rs b/src/api/client/keys.rs index f50d7afa..f6224343 100644 --- a/src/api/client/keys.rs +++ b/src/api/client/keys.rs @@ -9,7 +9,8 @@ use ruma::{ client::{ error::ErrorKind, keys::{ - claim_keys, get_key_changes, get_keys, upload_keys, upload_signatures, + claim_keys, get_key_changes, get_keys, upload_keys, + upload_signatures::{self, v3::Failure}, upload_signing_keys, }, uiaa::{AuthFlow, AuthType, UiaaInfo}, @@ -308,53 +309,81 @@ async fn check_for_new_keys( /// # `POST /_matrix/client/r0/keys/signatures/upload` /// /// Uploads end-to-end key signatures from the sender user. +/// +/// TODO: clean this timo-code up more. tried to improve it a bit to stop +/// exploding the entire request on bad sigs, but needs way more work. pub(crate) async fn upload_signatures_route( State(services): State, body: Ruma, ) -> Result { - let sender_user = body.sender_user.as_ref().expect("user is authenticated"); + use upload_signatures::v3::FailureErrorCode::*; + + if body.signed_keys.is_empty() { + debug!("Empty signed_keys sent in key signature upload"); + return Ok(upload_signatures::v3::Response::new()); + } + + let sender_user = body.sender_user(); + let mut failures: BTreeMap> = BTreeMap::new(); + let mut failure_reasons: BTreeMap = BTreeMap::new(); + let failure = Failure { + errcode: InvalidSignature, + error: String::new(), + }; for (user_id, keys) in &body.signed_keys { for (key_id, key) in keys { - let key = serde_json::to_value(key) - .map_err(|_| Error::BadRequest(ErrorKind::InvalidParam, "Invalid key JSON"))?; + let Ok(key) = serde_json::to_value(key) + .inspect_err(|e| debug_warn!(?key_id, "Invalid \"key\" JSON: {e}")) + else { + let mut failure = failure.clone(); + failure.error = String::from("Invalid \"key\" JSON"); + failure_reasons.insert(key_id.to_owned(), failure); + continue; + }; - for signature in key - .get("signatures") - .ok_or(Error::BadRequest(ErrorKind::InvalidParam, "Missing signatures field."))? - .get(sender_user.to_string()) - .ok_or(Error::BadRequest( - ErrorKind::InvalidParam, - "Invalid user in signatures field.", - ))? - .as_object() - .ok_or(Error::BadRequest(ErrorKind::InvalidParam, "Invalid signature."))? - .clone() - { - // Signature validation? - let signature = ( - signature.0, - signature - .1 - .as_str() - .ok_or(Error::BadRequest( - ErrorKind::InvalidParam, - "Invalid signature value.", - ))? - .to_owned(), - ); + let Some(signatures) = key.get("signatures") else { + let mut failure = failure.clone(); + failure.error = String::from("Missing \"signatures\" field"); + failure_reasons.insert(key_id.to_owned(), failure); + continue; + }; - services + let Some(sender_user_val) = signatures.get(sender_user.to_string()) else { + let mut failure = failure.clone(); + failure.error = String::from("Invalid user in signatures field"); + failure_reasons.insert(key_id.to_owned(), failure); + continue; + }; + + let Some(sender_user_object) = sender_user_val.as_object() else { + let mut failure = failure.clone(); + failure.error = String::from("signatures field is not a JSON object"); + failure_reasons.insert(key_id.to_owned(), failure); + continue; + }; + + for (signature, val) in sender_user_object.clone() { + let signature = (signature, val.to_string()); + + if let Err(e) = services .users .sign_key(user_id, key_id, signature, sender_user) - .await?; + .await + .inspect_err(|e| debug_warn!("{e}")) + { + let mut failure = failure.clone(); + failure.error = format!("Error signing key: {e}"); + failure_reasons.insert(key_id.to_owned(), failure); + continue; + } } } + + failures.insert(user_id.to_owned(), failure_reasons.clone()); } - Ok(upload_signatures::v3::Response { - failures: BTreeMap::new(), // TODO: integrate - }) + Ok(upload_signatures::v3::Response { failures }) } /// # `POST /_matrix/client/r0/keys/changes` diff --git a/src/service/users/mod.rs b/src/service/users/mod.rs index 87a8b93b..1eb289fc 100644 --- a/src/service/users/mod.rs +++ b/src/service/users/mod.rs @@ -593,7 +593,7 @@ impl Service { key_id: &str, signature: (String, String), sender_id: &UserId, - ) -> Result<()> { + ) -> Result { let key = (target_id, key_id); let mut cross_signing_key: serde_json::Value = self @@ -601,21 +601,27 @@ impl Service { .keyid_key .qry(&key) .await - .map_err(|_| err!(Request(InvalidParam("Tried to sign nonexistent key."))))? + .map_err(|_| err!(Request(InvalidParam("Tried to sign nonexistent key"))))? .deserialized() - .map_err(|e| err!(Database("key in keyid_key is invalid. {e:?}")))?; + .map_err(|e| err!(Database(debug_warn!("key in keyid_key is invalid: {e:?}"))))?; let signatures = cross_signing_key .get_mut("signatures") - .ok_or_else(|| err!(Database("key in keyid_key has no signatures field.")))? + .ok_or_else(|| { + err!(Database(debug_warn!("key in keyid_key has no signatures field"))) + })? .as_object_mut() - .ok_or_else(|| err!(Database("key in keyid_key has invalid signatures field.")))? + .ok_or_else(|| { + err!(Database(debug_warn!("key in keyid_key has invalid signatures field."))) + })? .entry(sender_id.to_string()) .or_insert_with(|| serde_json::Map::new().into()); signatures .as_object_mut() - .ok_or_else(|| err!(Database("signatures in keyid_key for a user is invalid.")))? + .ok_or_else(|| { + err!(Database(debug_warn!("signatures in keyid_key for a user is invalid."))) + })? .insert(signature.0, signature.1.into()); let key = (target_id, key_id);