Merge branch 'poljar/borrow-signature-check-fix'

This commit is contained in:
Damir Jelić
2022-05-30 12:54:35 +02:00
8 changed files with 52 additions and 77 deletions
+3 -3
View File
@@ -119,7 +119,7 @@ impl BackupMachine {
/// [`/room_keys/version`]: https://spec.matrix.org/unstable/client-server-api/#get_matrixclientv3room_keysversion
pub async fn verify_backup(
&self,
mut serialized_auth_data: Value,
serialized_auth_data: Value,
) -> Result<bool, CryptoStoreError> {
#[derive(Debug, Serialize, Deserialize)]
struct AuthData {
@@ -138,7 +138,7 @@ impl BackupMachine {
for device_key_id in signatures.keys() {
if device_key_id.algorithm() == DeviceKeyAlgorithm::Ed25519 {
if device_key_id.device_id() == self.account.device_id() {
let result = self.account.is_signed(&mut serialized_auth_data);
let result = self.account.is_signed(serialized_auth_data.clone());
trace!(?result, "Checking auth data signature of our own device");
@@ -158,7 +158,7 @@ impl BackupMachine {
if let Some(device) = device {
if device.verified()
&& device.is_signed_by_device(&mut serialized_auth_data).is_ok()
&& device.is_signed_by_device(serialized_auth_data.clone()).is_ok()
{
return Ok(true);
}
@@ -35,7 +35,7 @@ use ruma::{
OwnedDeviceKeyId, UserId,
};
use serde::{Deserialize, Deserializer, Serialize, Serializer};
use serde_json::{json, Value};
use serde_json::Value;
use tracing::warn;
use vodozemac::{Curve25519PublicKey, Ed25519PublicKey};
@@ -559,7 +559,7 @@ impl ReadOnlyDevice {
Ok(())
}
pub(crate) fn is_signed_by_device(&self, json: &mut Value) -> Result<(), SignatureError> {
pub(crate) fn is_signed_by_device(&self, json: Value) -> Result<(), SignatureError> {
let key = self.ed25519_key().ok_or(SignatureError::MissingSigningKey)?;
key.verify_json(
@@ -577,15 +577,16 @@ impl ReadOnlyDevice {
&self,
device_keys: &DeviceKeys,
) -> Result<(), SignatureError> {
let mut device_keys = serde_json::to_value(device_keys)?;
self.is_signed_by_device(&mut device_keys)
let device_keys = serde_json::to_value(device_keys)?;
self.is_signed_by_device(device_keys)
}
pub(crate) fn verify_one_time_key(
&self,
one_time_key: &SignedKey,
) -> Result<(), SignatureError> {
self.is_signed_by_device(&mut json!(&one_time_key))
let one_time_key = serde_json::to_value(one_time_key)?;
self.is_signed_by_device(one_time_key)
}
/// Mark the device as deleted.
@@ -456,12 +456,7 @@ impl MasterPubkey {
}
if let SigningKey::Ed25519(key) = key {
key.verify_json(
&self.0.user_id,
key_id,
&mut to_value(subkey.cross_signing_key())
.map_err(|_| SignatureError::NotAnObject)?,
)
key.verify_json(&self.0.user_id, key_id, to_value(subkey.cross_signing_key())?)
} else {
Err(SignatureError::UnsupportedAlgorithm)
}
@@ -506,11 +501,7 @@ impl UserSigningPubkey {
// TODO check that the usage is OK.
if let SigningKey::Ed25519(key) = key {
key.verify_json(
&self.0.user_id,
key_id.as_str().try_into()?,
&mut to_value(&master_key.0).map_err(|_| SignatureError::NotAnObject)?,
)
key.verify_json(&self.0.user_id, key_id.as_str().try_into()?, to_value(&master_key.0)?)
} else {
Err(SignatureError::UnsupportedAlgorithm)
}
@@ -541,10 +532,10 @@ impl SelfSigningPubkey {
let (key_id, key) = self.0.keys.iter().next().ok_or(SignatureError::MissingSigningKey)?;
// TODO check that the usage is OK.
let mut device = to_value(device_keys)?;
let device = to_value(device_keys)?;
if let SigningKey::Ed25519(key) = key {
key.verify_json(&self.0.user_id, key_id.as_str().try_into()?, &mut device)
key.verify_json(&self.0.user_id, key_id.as_str().try_into()?, device)
} else {
Err(SignatureError::UnsupportedAlgorithm)
}
+5 -5
View File
@@ -1705,7 +1705,7 @@ pub(crate) mod tests {
let ret = ed25519_key.verify_json(
&machine.user_id,
&DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, machine.device_id()),
&mut json!(&mut device_keys),
json!(&mut device_keys),
);
assert!(ret.is_ok());
}
@@ -1738,7 +1738,7 @@ pub(crate) mod tests {
let ret = key.verify_json(
&machine.user_id,
&DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, machine.device_id()),
&mut json!(&mut device_keys),
json!(&mut device_keys),
);
assert!(ret.is_err());
}
@@ -1758,7 +1758,7 @@ pub(crate) mod tests {
.verify_json(
&machine.user_id,
&DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, machine.device_id()),
&mut json!(&mut one_time_key),
json!(&mut one_time_key),
)
.expect("One-time key has been signed successfully");
}
@@ -1776,14 +1776,14 @@ pub(crate) mod tests {
let ret = ed25519_key.verify_json(
&machine.user_id,
&DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, machine.device_id()),
&mut json!(&mut request.one_time_keys.values_mut().next()),
json!(&mut request.one_time_keys.values_mut().next()),
);
assert!(ret.is_ok());
let ret = ed25519_key.verify_json(
&machine.user_id,
&DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, machine.device_id()),
&mut json!(&mut request.device_keys.unwrap()),
json!(&mut request.device_keys.unwrap()),
);
assert!(ret.is_ok());
+1 -1
View File
@@ -672,7 +672,7 @@ impl ReadOnlyAccount {
/// Check that the given json value is signed by this account.
#[cfg(feature = "backups_v1")]
pub fn is_signed(&self, json: &mut Value) -> Result<(), SignatureError> {
pub fn is_signed(&self, json: Value) -> Result<(), SignatureError> {
use crate::olm::utility::VerifyJson;
let signing_key = self.identity_keys.ed25519;
@@ -682,13 +682,12 @@ mod tests {
let signatures =
BTreeMap::from([(user_id, BTreeMap::from([(key_id.clone(), signature.to_base64())]))]);
let mut json = json!({
let json = json!({
"hello": "world",
"signatures": signatures,
});
assert!(signing.verify_json(user_id, &key_id, &mut json).is_ok());
assert!(signing.verify_json(user_id, &key_id, json).is_ok());
}
#[test]
@@ -318,7 +318,7 @@ impl Signing {
&self,
user_id: &ruma::UserId,
key_id: &DeviceKeyId,
message: &mut Value,
message: Value,
) -> Result<(), SignatureError> {
use crate::olm::VerifyJson;
self.public_key.verify_json(user_id, key_id, message)
+30 -46
View File
@@ -15,27 +15,28 @@
use std::convert::TryInto;
use ruma::{serde::CanonicalJsonValue, DeviceKeyAlgorithm, DeviceKeyId, UserId};
use serde::Deserialize;
use serde_json::Value;
use vodozemac::{olm::Account, Ed25519SecretKey, Ed25519Signature};
use crate::error::SignatureError;
use crate::{error::SignatureError, types::Signatures};
fn to_signable_json(mut value: Value) -> Result<String, SignatureError> {
let json_object = value.as_object_mut().ok_or(SignatureError::NotAnObject)?;
let _ = json_object.remove("signatures");
let _ = json_object.remove("unsigned");
let canonical_json: CanonicalJsonValue = value.try_into()?;
Ok(canonical_json.to_string())
}
pub trait SignJson {
fn sign_json(&self, value: Value) -> Result<Ed25519Signature, SignatureError>;
fn to_signable_json(mut value: Value) -> Result<String, SignatureError> {
let json_object = value.as_object_mut().ok_or(SignatureError::NotAnObject)?;
let _ = json_object.remove("signatures");
let _ = json_object.remove("unsigned");
let canonical_json: CanonicalJsonValue = value.try_into()?;
Ok(canonical_json.to_string())
}
}
impl SignJson for Account {
fn sign_json(&self, value: Value) -> Result<Ed25519Signature, SignatureError> {
let serialized = Self::to_signable_json(value)?;
let serialized = to_signable_json(value)?;
Ok(self.sign(serialized.as_ref()))
}
@@ -43,7 +44,7 @@ impl SignJson for Account {
impl SignJson for Ed25519SecretKey {
fn sign_json(&self, value: Value) -> Result<Ed25519Signature, SignatureError> {
let serialized = Self::to_signable_json(value)?;
let serialized = to_signable_json(value)?;
Ok(self.sign(serialized.as_ref()))
}
@@ -69,7 +70,7 @@ pub trait VerifyJson {
&self,
user_id: &UserId,
key_id: &DeviceKeyId,
json: &mut Value,
json: Value,
) -> Result<(), SignatureError>;
}
@@ -78,44 +79,27 @@ impl VerifyJson for vodozemac::Ed25519PublicKey {
&self,
user_id: &UserId,
key_id: &DeviceKeyId,
json: &mut Value,
json: Value,
) -> Result<(), SignatureError> {
#[derive(Debug, Deserialize)]
struct SignedJson {
#[serde(default)]
signatures: Signatures,
}
if key_id.algorithm() != DeviceKeyAlgorithm::Ed25519 {
return Err(SignatureError::UnsupportedAlgorithm);
}
let json_object = json.as_object_mut().ok_or(SignatureError::NotAnObject)?;
let unsigned = json_object.remove("unsigned");
let signatures = json_object.remove("signatures");
let signed_json: SignedJson = serde_json::from_value(json.clone())?;
let canonicalized = to_signable_json(json)?;
let canonical_json: CanonicalJsonValue =
json.clone().try_into().map_err(|_| SignatureError::NotAnObject)?;
let canonical_json: String = canonical_json.to_string();
let signatures = signatures.ok_or(SignatureError::NoSignatureFound)?;
let signature_object = signatures.as_object().ok_or(SignatureError::NoSignatureFound)?;
let signature =
signature_object.get(user_id.as_str()).ok_or(SignatureError::NoSignatureFound)?;
let signature =
signature.get(key_id.to_string()).ok_or(SignatureError::NoSignatureFound)?;
let signature = signature.as_str().ok_or(SignatureError::NoSignatureFound)?;
let signature = vodozemac::Ed25519Signature::from_base64(signature)?;
let ret = self
.verify(canonical_json.as_bytes(), &signature)
.map_err(SignatureError::VerificationError);
let json_object = json.as_object_mut().ok_or(SignatureError::NotAnObject)?;
if let Some(u) = unsigned {
json_object.insert("unsigned".to_owned(), u);
if let Some(signature) = signed_json.signatures.get_signature(user_id, key_id) {
self.verify(canonicalized.as_bytes(), &signature)
.map_err(SignatureError::VerificationError)
} else {
Err(SignatureError::NoSignatureFound)
}
json_object.insert("signatures".to_owned(), signatures);
ret
}
}
@@ -129,7 +113,7 @@ mod tests {
#[test]
fn signature_test() {
let mut device_keys = json!({
let device_keys = json!({
"device_id": "GBEWHQOYGS",
"algorithms": [
"m.olm.v1.curve25519-aes-sha2",
@@ -159,7 +143,7 @@ mod tests {
.verify_json(
user_id!("@example:localhost"),
&DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, device_id!("GBEWHQOYGS")),
&mut device_keys,
device_keys,
)
.expect("Can't verify device keys");
}