diff --git a/crates/ruma-signatures/CHANGELOG.md b/crates/ruma-signatures/CHANGELOG.md index 2ff569c9..c0a12a53 100644 --- a/crates/ruma-signatures/CHANGELOG.md +++ b/crates/ruma-signatures/CHANGELOG.md @@ -1,5 +1,9 @@ # [unreleased] +Bug fixes: + +- Ignore keys with unknown algorithms in `verify_events` + # 0.13.1 No changes for this version diff --git a/crates/ruma-signatures/src/functions.rs b/crates/ruma-signatures/src/functions.rs index f1bd805c..96ef8e13 100644 --- a/crates/ruma-signatures/src/functions.rs +++ b/crates/ruma-signatures/src/functions.rs @@ -582,6 +582,7 @@ pub fn verify_event( .get(entity_id.as_str()) .ok_or_else(|| VerificationError::public_key_not_found(entity_id))?; + let mut checked = false; for (key_id, signature) in signature_set { // Since only ed25519 is supported right now, we don't actually need to check what the // algorithm is. If it split successfully, it's ed25519. @@ -608,6 +609,11 @@ pub fn verify_event( signature.as_bytes(), &canonical_json, )?; + checked = true; + } + + if !checked { + return Err(VerificationError::UnknownPublicKeysForSignature.into()); } } @@ -799,8 +805,8 @@ mod tests { #[test] fn verify_event_check_signatures_for_both_sender_and_event_id() { - let key_pair_sender = generate_key_pair(); - let key_pair_event = generate_key_pair(); + let key_pair_sender = generate_key_pair("1"); + let key_pair_event = generate_key_pair("2"); let mut signed_event = serde_json::from_str( r#"{ "event_id": "$event_id:domain-event", @@ -837,8 +843,8 @@ mod tests { #[test] fn verify_event_check_signatures_for_authorized_user() { - let key_pair_sender = generate_key_pair(); - let key_pair_authorized = generate_key_pair(); + let key_pair_sender = generate_key_pair("1"); + let key_pair_authorized = generate_key_pair("2"); let mut signed_event = serde_json::from_str( r#"{ "event_id": "$event_id:domain-event", @@ -875,7 +881,7 @@ mod tests { #[test] fn verification_fails_if_missing_signatures_for_authorized_user() { - let key_pair_sender = generate_key_pair(); + let key_pair_sender = generate_key_pair("1"); let mut signed_event = serde_json::from_str( r#"{ "event_id": "$event_id:domain-event", @@ -913,7 +919,7 @@ mod tests { #[test] fn verification_fails_if_required_keys_are_not_given() { - let key_pair_sender = generate_key_pair(); + let key_pair_sender = generate_key_pair("1"); let mut signed_event = serde_json::from_str( r#"{ @@ -950,7 +956,7 @@ mod tests { #[test] fn verify_event_fails_if_public_key_is_invalid() { - let key_pair_sender = generate_key_pair(); + let key_pair_sender = generate_key_pair("1"); let mut signed_event = serde_json::from_str( r#"{ @@ -976,7 +982,7 @@ mod tests { let mut public_key_map = PublicKeyMap::new(); let mut sender_key_map = PublicKeySet::new(); - let newly_generated_key_pair = generate_key_pair(); + let newly_generated_key_pair = generate_key_pair("2"); let encoded_public_key = Base64::new(newly_generated_key_pair.public_key().to_owned()); let version = ServerSigningKeyId::from_parts( SigningKeyAlgorithm::Ed25519, @@ -996,14 +1002,163 @@ mod tests { assert!(format!("{error:?}").contains("Some(Verification equation was not satisfied)")); } - fn generate_key_pair() -> Ed25519KeyPair { + #[test] + fn verify_event_check_signatures_for_sender_is_allowed_with_unknown_algorithms_in_key_map() { + let key_pair_sender = generate_key_pair("1"); + let mut signed_event = serde_json::from_str( + r#"{ + "auth_events": [], + "content": {}, + "depth": 3, + "hashes": { + "sha256": "5jM4wQpv6lnBo7CLIghJuHdW+s2CMBJPUOGOC89ncos" + }, + "origin": "domain", + "origin_server_ts": 1000000, + "prev_events": [], + "room_id": "!x:domain", + "sender": "@name:domain-sender", + "type": "X", + "unsigned": { + "age_ts": 1000000 + } + }"#, + ) + .unwrap(); + sign_json("domain-sender", &key_pair_sender, &mut signed_event).unwrap(); + + let mut public_key_map = BTreeMap::new(); + add_key_to_map(&mut public_key_map, "domain-sender", &key_pair_sender); + add_invalid_key_to_map(&mut public_key_map, "domain-sender", &generate_key_pair("2")); + + let verification = + verify_event(&public_key_map, &signed_event, &RoomVersionId::V6).unwrap(); + + assert_eq!(verification, Verified::Signatures); + } + + #[test] + fn verify_event_fails_with_missing_key_when_event_is_signed_multiple_times_by_same_entity() { + let key_pair_sender = generate_key_pair("1"); + let secondary_key_pair_sender = generate_key_pair("2"); + let mut signed_event = serde_json::from_str( + r#"{ + "auth_events": [], + "content": {}, + "depth": 3, + "hashes": { + "sha256": "5jM4wQpv6lnBo7CLIghJuHdW+s2CMBJPUOGOC89ncos" + }, + "origin": "domain", + "origin_server_ts": 1000000, + "prev_events": [], + "room_id": "!x:domain", + "sender": "@name:domain-sender", + "type": "X", + "unsigned": { + "age_ts": 1000000 + } + }"#, + ) + .unwrap(); + sign_json("domain-sender", &key_pair_sender, &mut signed_event).unwrap(); + sign_json("domain-sender", &secondary_key_pair_sender, &mut signed_event).unwrap(); + + let mut public_key_map = BTreeMap::new(); + add_key_to_map(&mut public_key_map, "domain-sender", &key_pair_sender); + + let verification_result = verify_event(&public_key_map, &signed_event, &RoomVersionId::V6); + + assert_matches!( + verification_result, + Err(Error::Verification(VerificationError::UnknownPublicKeysForSignature)) + ); + } + + #[test] + fn verify_event_checks_all_signatures_from_sender_entity() { + let key_pair_sender = generate_key_pair("1"); + let secondary_key_pair_sender = generate_key_pair("2"); + let mut signed_event = serde_json::from_str( + r#"{ + "auth_events": [], + "content": {}, + "depth": 3, + "hashes": { + "sha256": "5jM4wQpv6lnBo7CLIghJuHdW+s2CMBJPUOGOC89ncos" + }, + "origin": "domain", + "origin_server_ts": 1000000, + "prev_events": [], + "room_id": "!x:domain", + "sender": "@name:domain-sender", + "type": "X", + "unsigned": { + "age_ts": 1000000 + } + }"#, + ) + .unwrap(); + sign_json("domain-sender", &key_pair_sender, &mut signed_event).unwrap(); + sign_json("domain-sender", &secondary_key_pair_sender, &mut signed_event).unwrap(); + + let mut public_key_map = BTreeMap::new(); + add_key_to_map(&mut public_key_map, "domain-sender", &key_pair_sender); + add_key_to_map(&mut public_key_map, "domain-sender", &secondary_key_pair_sender); + + let verification = + verify_event(&public_key_map, &signed_event, &RoomVersionId::V6).unwrap(); + + assert_eq!(verification, Verified::Signatures); + } + + #[test] + fn verify_event_with_single_key_with_unknown_algorithm_should_not_accept_event() { + let key_pair_sender = generate_key_pair("1"); + let signed_event = serde_json::from_str( + r#"{ + "auth_events": [], + "content": {}, + "depth": 3, + "hashes": { + "sha256": "5jM4wQpv6lnBo7CLIghJuHdW+s2CMBJPUOGOC89ncos" + }, + "origin": "domain", + "origin_server_ts": 1000000, + "prev_events": [], + "room_id": "!x:domain", + "sender": "@name:domain-sender", + "type": "X", + "unsigned": { + "age_ts": 1000000 + }, + "signatures": { + "domain-sender": { + "an-unknown-algorithm:1": "pE5UT/4JiY7YZDtZDOsEaxc0wblurdoYqNQx4bCXORA3vLFOGOK10Q/xXVLPWWgIKo15LNvWwWd/2YjmdPvYCg" + } + } + }"#, + ) + .unwrap(); + + let mut public_key_map = BTreeMap::new(); + add_invalid_key_to_map(&mut public_key_map, "domain-sender", &key_pair_sender); + + let verification_result = verify_event(&public_key_map, &signed_event, &RoomVersionId::V6); + assert_matches!( + verification_result, + Err(Error::Verification(VerificationError::UnknownPublicKeysForSignature)) + ); + } + + fn generate_key_pair(name: &str) -> Ed25519KeyPair { let key_content = Ed25519KeyPair::generate().unwrap(); - Ed25519KeyPair::from_der(&key_content, "1".to_owned()) + Ed25519KeyPair::from_der(&key_content, name.to_owned()) .unwrap_or_else(|_| panic!("{:?}", &key_content)) } fn add_key_to_map(public_key_map: &mut PublicKeyMap, name: &str, pair: &Ed25519KeyPair) { - let mut sender_key_map = PublicKeySet::new(); + let sender_key_map = public_key_map.entry(name.to_owned()).or_default(); let encoded_public_key = Base64::new(pair.public_key().to_owned()); let version = ServerSigningKeyId::from_parts( SigningKeyAlgorithm::Ed25519, @@ -1011,7 +1166,20 @@ mod tests { ); sender_key_map.insert(version.to_string(), encoded_public_key); + } - public_key_map.insert(name.to_owned(), sender_key_map); + fn add_invalid_key_to_map( + public_key_map: &mut PublicKeyMap, + name: &str, + pair: &Ed25519KeyPair, + ) { + let sender_key_map = public_key_map.entry(name.to_owned()).or_default(); + let encoded_public_key = Base64::new(pair.public_key().to_owned()); + let version = ServerSigningKeyId::from_parts( + SigningKeyAlgorithm::from("an-unknown-algorithm"), + pair.version().try_into().unwrap(), + ); + + sender_key_map.insert(version.to_string(), encoded_public_key); } }