Tolerate slightly malformed base64

* add in fixes

* cargo fmt and newline

* sort dependencies
This commit is contained in:
Jonathan de Jong 2021-05-24 22:31:36 +02:00 committed by GitHub
parent 087a2b9012
commit 42bbb81bd2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 75 additions and 7 deletions

View File

@ -17,4 +17,8 @@ ring = "0.16.19"
ruma-identifiers = { version = "0.19.1", path = "../ruma-identifiers" } ruma-identifiers = { version = "0.19.1", path = "../ruma-identifiers" }
ruma-serde = { version = "0.4.0", path = "../ruma-serde" } ruma-serde = { version = "0.4.0", path = "../ruma-serde" }
serde_json = "1.0.60" serde_json = "1.0.60"
tracing = { version = "0.1.25", optional = true }
untrusted = "0.7.1" untrusted = "0.7.1"
[features]
compat = ["tracing"]

View File

@ -7,7 +7,7 @@ use std::{
str::FromStr, str::FromStr,
}; };
use base64::{decode_config, encode_config, STANDARD_NO_PAD, URL_SAFE_NO_PAD}; use base64::{decode_config, encode_config, Config, STANDARD_NO_PAD, URL_SAFE_NO_PAD};
use ring::digest::{digest, SHA256}; use ring::digest::{digest, SHA256};
use ruma_identifiers::{EventId, RoomVersionId, ServerNameBox, UserId}; use ruma_identifiers::{EventId, RoomVersionId, ServerNameBox, UserId};
use ruma_serde::{to_canonical_json_string, CanonicalJsonObject, CanonicalJsonValue}; use ruma_serde::{to_canonical_json_string, CanonicalJsonObject, CanonicalJsonValue};
@ -278,11 +278,18 @@ pub fn verify_json(
.get(key_id) .get(key_id)
.ok_or_else(|| Error::new("no key for signature in public_key_map"))?; .ok_or_else(|| Error::new("no key for signature in public_key_map"))?;
let signature_bytes = decode_config(signature, STANDARD_NO_PAD)?; let verify = |config: Config| {
let signature_bytes = decode_config(signature, config)?;
let public_key_bytes = decode_config(&public_key, STANDARD_NO_PAD)?; let public_key_bytes = decode_config(&public_key, config)?;
verify_json_with(&Ed25519Verifier, &public_key_bytes, &signature_bytes, object)?; verify_json_with(&Ed25519Verifier, &public_key_bytes, &signature_bytes, object)
};
#[cfg(feature = "compat")]
also_try_forgiving_base64(STANDARD_NO_PAD, verify)?;
#[cfg(not(feature = "compat"))]
verify(STANDARD_NO_PAD)?;
} }
} }
@ -635,11 +642,18 @@ pub fn verify_event(
} }
}; };
let signature_bytes = decode_config(signature, STANDARD_NO_PAD)?; let verify = |config: Config| {
let signature_bytes = decode_config(signature, config)?;
let public_key_bytes = decode_config(&public_key, STANDARD_NO_PAD)?; let public_key_bytes = decode_config(&public_key, config)?;
verify_json_with(&Ed25519Verifier, &public_key_bytes, &signature_bytes, &canonical_json)?; verify_json_with(&Ed25519Verifier, &public_key_bytes, &signature_bytes, &canonical_json)
};
#[cfg(feature = "compat")]
also_try_forgiving_base64(STANDARD_NO_PAD, verify)?;
#[cfg(not(feature = "compat"))]
verify(STANDARD_NO_PAD)?;
} }
let calculated_hash = content_hash(object); let calculated_hash = content_hash(object);
@ -781,6 +795,41 @@ fn is_third_party_invite(object: &CanonicalJsonObject) -> Result<bool, Error> {
} }
} }
#[cfg(feature = "compat")]
// see https://github.com/ruma/ruma/issues/591
// synapse allows this, so we must allow it too.
// shouldn't lose data, but when it does, it'll make verification fail instead.
pub(crate) fn also_try_forgiving_base64<T, E>(
config: Config,
twice: impl Fn(Config) -> Result<T, E>,
) -> Result<T, E>
where
E: std::fmt::Display,
{
use tracing::{debug, warn};
let first_try = match twice(config) {
Ok(t) => return Ok(t),
Err(e) => e,
};
let adjusted = config.decode_allow_trailing_bits(true);
match twice(adjusted) {
Ok(t) => {
warn!(
"Usage of base64 config only worked after allowing trailing bits, first error: {}",
first_try
);
Ok(t)
}
Err(e) => {
debug!("Second error when trying to allow trailing bits: {}", e);
Err(first_try)
}
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use std::{ use std::{
@ -864,6 +913,20 @@ mod tests {
assert!(matches!(verification, Verified::Signatures)); assert!(matches!(verification, Verified::Signatures));
} }
#[cfg(feature = "compat")]
#[test]
fn fallback_invalid_base64() {
use base64::{decode_config, Config};
const SLIGHTLY_MALFORMED_BASE64: &str = "3UmJnEIzUr2xWyaUnJg5fXwRybwG5FVC6GqMHverEUn0ztuIsvVxX89JXX2pvdTsOBbLQx+4TVL02l4Cp5wPCm";
let verify = |config: Config| decode_config(SLIGHTLY_MALFORMED_BASE64, config);
assert!(verify(STANDARD_NO_PAD).is_err());
assert!(super::also_try_forgiving_base64(STANDARD_NO_PAD, verify).is_ok());
}
#[test] #[test]
fn verify_event_check_signatures_for_both_sender_and_event_id() { fn verify_event_check_signatures_for_both_sender_and_event_id() {
let key_pair_sender = generate_key_pair(); let key_pair_sender = generate_key_pair();

View File

@ -113,6 +113,7 @@ compat = [
"ruma-events/compat", "ruma-events/compat",
"ruma-identifiers/compat", "ruma-identifiers/compat",
"ruma-client-api/compat", "ruma-client-api/compat",
"ruma-signatures/compat",
] ]
# Helper features that aren't exactly part of the spec but could be helpful # Helper features that aren't exactly part of the spec but could be helpful