diff --git a/Cargo.lock b/Cargo.lock index a899b71a5..c46219f89 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4734,7 +4734,7 @@ dependencies = [ [[package]] name = "ruma" version = "0.9.4" -source = "git+https://github.com/ruma/ruma?rev=a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4#a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4" +source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" dependencies = [ "assign", "js_int", @@ -4750,8 +4750,9 @@ dependencies = [ [[package]] name = "ruma-client-api" version = "0.17.4" -source = "git+https://github.com/ruma/ruma?rev=a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4#a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4" +source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" dependencies = [ + "as_variant", "assign", "bytes", "http", @@ -4768,7 +4769,7 @@ dependencies = [ [[package]] name = "ruma-common" version = "0.12.1" -source = "git+https://github.com/ruma/ruma?rev=a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4#a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4" +source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" dependencies = [ "as_variant", "base64 0.21.5", @@ -4798,7 +4799,7 @@ dependencies = [ [[package]] name = "ruma-events" version = "0.27.11" -source = "git+https://github.com/ruma/ruma?rev=a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4#a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4" +source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" dependencies = [ "as_variant", "indexmap 2.1.0", @@ -4822,7 +4823,7 @@ dependencies = [ [[package]] name = "ruma-federation-api" version = "0.8.0" -source = "git+https://github.com/ruma/ruma?rev=a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4#a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4" +source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" dependencies = [ "js_int", "ruma-common", @@ -4834,7 +4835,7 @@ dependencies = [ [[package]] name = "ruma-html" version = "0.1.0" -source = "git+https://github.com/ruma/ruma?rev=a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4#a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4" +source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" dependencies = [ "as_variant", "html5ever", @@ -4846,7 +4847,7 @@ dependencies = [ [[package]] name = "ruma-identifiers-validation" version = "0.9.3" -source = "git+https://github.com/ruma/ruma?rev=a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4#a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4" +source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" dependencies = [ "js_int", "thiserror", @@ -4855,7 +4856,7 @@ dependencies = [ [[package]] name = "ruma-macros" version = "0.12.0" -source = "git+https://github.com/ruma/ruma?rev=a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4#a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4" +source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" dependencies = [ "once_cell", "proc-macro-crate 2.0.1", @@ -4870,7 +4871,7 @@ dependencies = [ [[package]] name = "ruma-push-gateway-api" version = "0.8.0" -source = "git+https://github.com/ruma/ruma?rev=a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4#a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4" +source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" dependencies = [ "js_int", "ruma-common", diff --git a/Cargo.toml b/Cargo.toml index db231d420..bff1526c8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,8 +36,8 @@ futures-executor = "0.3.21" futures-util = { version = "0.3.26", default-features = false, features = ["alloc"] } http = "0.2.6" itertools = "0.12.0" -ruma = { git = "https://github.com/ruma/ruma", rev = "a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4", features = ["client-api-c", "compat-upload-signatures", "compat-user-id", "compat-arbitrary-length-ids", "unstable-msc3401"] } -ruma-common = { git = "https://github.com/ruma/ruma", rev = "a7e1d7fa49d93f4d9cbc369aa28378a89b3dc9e4"} +ruma = { git = "https://github.com/ruma/ruma", rev = "684ffc789877355bd25269451b2356817c17cc3f", features = ["client-api-c", "compat-upload-signatures", "compat-user-id", "compat-arbitrary-length-ids", "unstable-msc3401"] } +ruma-common = { git = "https://github.com/ruma/ruma", rev = "684ffc789877355bd25269451b2356817c17cc3f"} once_cell = "1.16.0" rand = "0.8.5" serde = "1.0.151" diff --git a/crates/matrix-sdk-crypto/src/secret_storage.rs b/crates/matrix-sdk-crypto/src/secret_storage.rs index 9bb195f88..bf94f559f 100644 --- a/crates/matrix-sdk-crypto/src/secret_storage.rs +++ b/crates/matrix-sdk-crypto/src/secret_storage.rs @@ -245,29 +245,40 @@ impl SecretStorageKey { fn check_zero_message(&self) -> Result<(), DecodeError> { match &self.storage_key_info.algorithm { SecretStorageEncryptionAlgorithm::V1AesHmacSha2(properties) => { - if properties.iv.as_bytes().len() != IV_SIZE { - Err(DecodeError::IvLength(IV_SIZE, properties.iv.as_bytes().len())) - } else { - let mut iv_array = [0u8; 16]; - iv_array.copy_from_slice(properties.iv.as_bytes()); - - // I'm not particularly convinced that this couldn't have been done simpler. Why - // do we need to reproduce the ciphertext? Couldn't we just generate the MAC tag - // using the `ZERO_MESSAGE`? + let (Some(iv), Some(mac)) = (&properties.iv, &properties.mac) else { + // The IV and/or MAC are missing from the account data + // content. As the [spec] says, we have to assume that the + // key is valid. // - // If someone is reading this and is designing a new secret encryption - // algorithm, please consider the above suggestion. - let key = AesHmacSha2Key::from_secret_storage_key(&self.secret_key, ""); - let ciphertext = key.apply_keystream(Self::ZERO_MESSAGE.to_vec(), &iv_array); - let expected_mac = HmacSha256Mac::from_slice(properties.mac.as_bytes()) - .ok_or_else(|| { - DecodeError::MacLength(MAC_SIZE, properties.mac.as_bytes().len()) - })?; + // [spec]: https://spec.matrix.org/unstable/client-server-api/#msecret_storagev1aes-hmac-sha2 + return Ok(()); + }; - key.verify_mac(&ciphertext, expected_mac.as_bytes())?; + let iv = iv.as_bytes(); + let iv_length = iv.len(); - Ok(()) + if iv_length != IV_SIZE { + return Err(DecodeError::IvLength(IV_SIZE, iv_length)); } + + let mut iv_array = [0u8; 16]; + iv_array.copy_from_slice(iv); + + // I'm not particularly convinced that this couldn't have been done simpler. + // Why do we need to reproduce the ciphertext? + // Couldn't we just generate the MAC tag + // using the `ZERO_MESSAGE`? + // + // If someone is reading this and is designing a new secret encryption + // algorithm, please consider the above suggestion. + let key = AesHmacSha2Key::from_secret_storage_key(&self.secret_key, ""); + let ciphertext = key.apply_keystream(Self::ZERO_MESSAGE.to_vec(), &iv_array); + let expected_mac = HmacSha256Mac::from_slice(mac.as_bytes()) + .ok_or_else(|| DecodeError::MacLength(MAC_SIZE, mac.as_bytes().len()))?; + + key.verify_mac(&ciphertext, expected_mac.as_bytes())?; + + Ok(()) } custom => Err(DecodeError::UnsupportedAlgorithm(custom.algorithm().to_owned())), } @@ -283,7 +294,7 @@ impl SecretStorageKey { SecretStorageKeyEventContent::new( key_id, SecretStorageEncryptionAlgorithm::V1AesHmacSha2( - SecretStorageV1AesHmacSha2Properties::new(iv, mac), + SecretStorageV1AesHmacSha2Properties::new(Some(iv), Some(mac)), ), ) } @@ -862,8 +873,8 @@ mod test { "bmur2d9ypPUH1msSwCxQOJkuKRmJI55e".to_owned(), SecretStorageEncryptionAlgorithm::V1AesHmacSha2( SecretStorageV1AesHmacSha2Properties::new( - Base64::new(vec![0u8; 14]), - Base64::new(vec![0u8; 32]), + Some(Base64::new(vec![0u8; 14])), + Some(Base64::new(vec![0u8; 32])), ), ), ); @@ -878,8 +889,8 @@ mod test { "bmur2d9ypPUH1msSwCxQOJkuKRmJI55e".to_owned(), SecretStorageEncryptionAlgorithm::V1AesHmacSha2( SecretStorageV1AesHmacSha2Properties::new( - Base64::new(vec![0u8; 16]), - Base64::new(vec![0u8; 10]), + Some(Base64::new(vec![0u8; 16])), + Some(Base64::new(vec![0u8; 10])), ), ), );