chris/ruwuma
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

signatures: Prevent accepting events without at least one valid signature

Browse Source 
`verify_event` has been changed recently to be more aligned with the
spec. In the previous algorithm, it returned an error if the event was
not signed by at least one of the required entities.

The new algortihm is iterating over all the signtures for the required
entities *and* skipping unknown algorithms. If an event is signed only
by unkonwn algorithms, the event would be accepted and not verifications
will happen. In order to prevent that, we check that, at least, a single
key is checked. This covers the 2nd point in: https://spec.matrix.org/v1.6/appendices/#checking-for-a-signature

A few more test cases were added:

- The event is properly signed, but key map contains a key with an
  unknown algorithm. Under this circumstances, the event should be
  allowed.
- An event signed by multiple keys for an entity, should verify all
  those signatures
- An event signed by a single key with an unkown algorithm by the
  required entity, should fail
This commit is contained in:
gnieto 2023-03-02 10:34:57 +01:00 committed by GitHub
parent 87237bf100
commit 7565588be7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 184 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
crates/ruma-signatures/CHANGELOG.md
View File

@ -1,5 +1,9 @@
# [unreleased]
Bug fixes:
- Ignore keys with unknown algorithms in `verify_events`
# 0.13.1
No changes for this version

192
crates/ruma-signatures/src/functions.rs
View File

@ -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);
}
}