crypto: Add the master key to the SAS MAC if we trust it

This commit is contained in:
Damir Jelić 2021-08-12 14:13:22 +02:00
parent 356506060c
commit 680f77beb9
7 changed files with 95 additions and 13 deletions

View file

@ -666,6 +666,13 @@ impl ReadOnlyUserIdentities {
} }
} }
pub(crate) fn into_own(self) -> Option<ReadOnlyOwnUserIdentity> {
match self {
ReadOnlyUserIdentities::Own(i) => Some(i),
_ => None,
}
}
/// Destructure the enum into an `UserIdentity` if it's of the correct /// Destructure the enum into an `UserIdentity` if it's of the correct
/// type. /// type.
pub fn other(&self) -> Option<&ReadOnlyUserIdentity> { pub fn other(&self) -> Option<&ReadOnlyUserIdentity> {

View file

@ -128,12 +128,15 @@ impl VerificationMachine {
device: ReadOnlyDevice, device: ReadOnlyDevice,
) -> Result<(Sas, OutgoingVerificationRequest), CryptoStoreError> { ) -> Result<(Sas, OutgoingVerificationRequest), CryptoStoreError> {
let identity = self.store.get_user_identity(device.user_id()).await?; let identity = self.store.get_user_identity(device.user_id()).await?;
let own_identity =
self.store.get_user_identity(self.own_user_id()).await?.and_then(|i| i.into_own());
let private_identity = self.private_identity.lock().await.clone(); let private_identity = self.private_identity.lock().await.clone();
let (sas, content) = Sas::start( let (sas, content) = Sas::start(
private_identity, private_identity,
device.clone(), device.clone(),
self.store.clone(), self.store.clone(),
own_identity,
identity, identity,
None, None,
true, true,
@ -411,6 +414,11 @@ impl VerificationMachine {
self.store.get_device(event.sender(), c.from_device()).await? self.store.get_device(event.sender(), c.from_device()).await?
{ {
let private_identity = self.private_identity.lock().await.clone(); let private_identity = self.private_identity.lock().await.clone();
let own_identity = self
.store
.get_user_identity(self.own_user_id())
.await?
.and_then(|i| i.into_own());
let identity = self.store.get_user_identity(event.sender()).await?; let identity = self.store.get_user_identity(event.sender()).await?;
match Sas::from_start_event( match Sas::from_start_event(
@ -419,6 +427,7 @@ impl VerificationMachine {
self.store.clone(), self.store.clone(),
private_identity, private_identity,
device, device,
own_identity,
identity, identity,
None, None,
false, false,
@ -559,6 +568,7 @@ mod test {
bob_store, bob_store,
None, None,
None, None,
None,
true, true,
None, None,
); );

View file

@ -46,8 +46,8 @@ use super::{
}; };
use crate::{ use crate::{
olm::{PrivateCrossSigningIdentity, ReadOnlyAccount}, olm::{PrivateCrossSigningIdentity, ReadOnlyAccount},
CryptoStoreError, OutgoingVerificationRequest, ReadOnlyDevice, ReadOnlyUserIdentities, CryptoStoreError, OutgoingVerificationRequest, ReadOnlyDevice, ReadOnlyOwnUserIdentity,
RoomMessageRequest, Sas, ToDeviceRequest, ReadOnlyUserIdentities, RoomMessageRequest, Sas, ToDeviceRequest,
}; };
const SUPPORTED_METHODS: &[VerificationMethod] = &[ const SUPPORTED_METHODS: &[VerificationMethod] = &[
@ -920,6 +920,7 @@ impl RequestState<Ready> {
&self, &self,
content: &StartContent<'a>, content: &StartContent<'a>,
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
own_identity: Option<ReadOnlyOwnUserIdentity>,
other_identity: Option<ReadOnlyUserIdentities>, other_identity: Option<ReadOnlyUserIdentities>,
we_started: bool, we_started: bool,
request_handle: RequestHandle, request_handle: RequestHandle,
@ -930,6 +931,7 @@ impl RequestState<Ready> {
self.store.clone(), self.store.clone(),
self.private_cross_signing_identity.clone(), self.private_cross_signing_identity.clone(),
other_device, other_device,
own_identity,
other_identity, other_identity,
Some(request_handle), Some(request_handle),
we_started, we_started,
@ -1100,12 +1102,18 @@ impl RequestState<Ready> {
}; };
let identity = self.store.get_user_identity(sender).await?; let identity = self.store.get_user_identity(sender).await?;
let own_identity = self
.store
.get_user_identity(self.store.account.user_id())
.await?
.and_then(|i| i.into_own());
match content.method() { match content.method() {
StartMethod::SasV1(_) => { StartMethod::SasV1(_) => {
match self.to_started_sas( match self.to_started_sas(
content, content,
device.clone(), device.clone(),
own_identity,
identity, identity,
we_started, we_started,
request_handle, request_handle,
@ -1169,6 +1177,11 @@ impl RequestState<Ready> {
// TODO signal why starting the sas flow doesn't work? // TODO signal why starting the sas flow doesn't work?
let other_identity = store.get_user_identity(&self.other_user_id).await?; let other_identity = store.get_user_identity(&self.other_user_id).await?;
let own_identity = self
.store
.get_user_identity(self.store.account.user_id())
.await?
.and_then(|i| i.into_own());
let device = if let Some(device) = let device = if let Some(device) =
self.store.get_device(&self.other_user_id, &self.state.other_device_id).await? self.store.get_device(&self.other_user_id, &self.state.other_device_id).await?
@ -1190,6 +1203,7 @@ impl RequestState<Ready> {
private_identity, private_identity,
device, device,
store, store,
own_identity,
other_identity, other_identity,
Some(t.to_owned()), Some(t.to_owned()),
we_started, we_started,
@ -1204,6 +1218,7 @@ impl RequestState<Ready> {
private_identity, private_identity,
device, device,
store, store,
own_identity,
other_identity, other_identity,
we_started, we_started,
request_handle, request_handle,

View file

@ -34,12 +34,13 @@ use crate::{
identities::{ReadOnlyDevice, ReadOnlyUserIdentities}, identities::{ReadOnlyDevice, ReadOnlyUserIdentities},
utilities::encode, utilities::encode,
verification::event_enums::{MacContent, StartContent}, verification::event_enums::{MacContent, StartContent},
ReadOnlyAccount, ReadOnlyAccount, ReadOnlyOwnUserIdentity,
}; };
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub struct SasIds { pub struct SasIds {
pub account: ReadOnlyAccount, pub account: ReadOnlyAccount,
pub own_identity: Option<ReadOnlyOwnUserIdentity>,
pub other_device: ReadOnlyDevice, pub other_device: ReadOnlyDevice,
pub other_identity: Option<ReadOnlyUserIdentities>, pub other_identity: Option<ReadOnlyUserIdentities>,
} }
@ -304,6 +305,20 @@ pub fn get_mac_content(sas: &OlmSas, ids: &SasIds, flow_id: &FlowId) -> Outgoing
sas.calculate_mac(key, &format!("{}{}", info, key_id)).expect("Can't calculate SAS MAC"), sas.calculate_mac(key, &format!("{}{}", info, key_id)).expect("Can't calculate SAS MAC"),
); );
if let Some(own_identity) = &ids.own_identity {
if own_identity.is_verified() {
if let Some(key) = own_identity.master_key().get_first_key() {
let key_id = format!("{}:{}", DeviceKeyAlgorithm::Ed25519, &key);
let calculated_mac = sas
.calculate_mac(key, &format!("{}{}", info, &key_id))
.expect("Can't calculate SAS Master key MAC");
mac.insert(key_id, calculated_mac);
}
}
}
// TODO Add the cross signing master key here if we trust/have it. // TODO Add the cross signing master key here if we trust/have it.
let mut keys = mac.keys().cloned().collect::<Vec<String>>(); let mut keys = mac.keys().cloned().collect::<Vec<String>>();

View file

@ -34,7 +34,7 @@ use crate::{
event_enums::{AnyVerificationContent, OutgoingContent, OwnedAcceptContent, StartContent}, event_enums::{AnyVerificationContent, OutgoingContent, OwnedAcceptContent, StartContent},
Cancelled, Done, Cancelled, Done,
}, },
ReadOnlyAccount, ReadOnlyAccount, ReadOnlyOwnUserIdentity,
}; };
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
@ -55,10 +55,17 @@ impl InnerSas {
pub fn start( pub fn start(
account: ReadOnlyAccount, account: ReadOnlyAccount,
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
own_identity: Option<ReadOnlyOwnUserIdentity>,
other_identity: Option<ReadOnlyUserIdentities>, other_identity: Option<ReadOnlyUserIdentities>,
transaction_id: Option<String>, transaction_id: Option<String>,
) -> (InnerSas, OutgoingContent) { ) -> (InnerSas, OutgoingContent) {
let sas = SasState::<Created>::new(account, other_device, other_identity, transaction_id); let sas = SasState::<Created>::new(
account,
other_device,
own_identity,
other_identity,
transaction_id,
);
let content = sas.as_content(); let content = sas.as_content();
(InnerSas::Created(sas), content.into()) (InnerSas::Created(sas), content.into())
} }
@ -131,6 +138,7 @@ impl InnerSas {
room_id: RoomId, room_id: RoomId,
account: ReadOnlyAccount, account: ReadOnlyAccount,
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
own_identity: Option<ReadOnlyOwnUserIdentity>,
other_identity: Option<ReadOnlyUserIdentities>, other_identity: Option<ReadOnlyUserIdentities>,
) -> (InnerSas, OutgoingContent) { ) -> (InnerSas, OutgoingContent) {
let sas = SasState::<Created>::new_in_room( let sas = SasState::<Created>::new_in_room(
@ -138,6 +146,7 @@ impl InnerSas {
event_id, event_id,
account, account,
other_device, other_device,
own_identity,
other_identity, other_identity,
); );
let content = sas.as_content(); let content = sas.as_content();
@ -149,12 +158,14 @@ impl InnerSas {
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
flow_id: FlowId, flow_id: FlowId,
content: &StartContent, content: &StartContent,
own_identity: Option<ReadOnlyOwnUserIdentity>,
other_identity: Option<ReadOnlyUserIdentities>, other_identity: Option<ReadOnlyUserIdentities>,
started_from_request: bool, started_from_request: bool,
) -> Result<InnerSas, OutgoingContent> { ) -> Result<InnerSas, OutgoingContent> {
match SasState::<Started>::from_start_event( match SasState::<Started>::from_start_event(
account, account,
other_device, other_device,
own_identity,
other_identity, other_identity,
flow_id, flow_id,
content, content,

View file

@ -42,7 +42,7 @@ use crate::{
olm::PrivateCrossSigningIdentity, olm::PrivateCrossSigningIdentity,
requests::{OutgoingVerificationRequest, RoomMessageRequest}, requests::{OutgoingVerificationRequest, RoomMessageRequest},
store::CryptoStoreError, store::CryptoStoreError,
ReadOnlyAccount, ToDeviceRequest, ReadOnlyAccount, ReadOnlyOwnUserIdentity, ToDeviceRequest,
}; };
/// Short authentication string object. /// Short authentication string object.
@ -183,10 +183,12 @@ impl Sas {
/// ///
/// Returns the new `Sas` object and a `StartEventContent` that needs to be /// Returns the new `Sas` object and a `StartEventContent` that needs to be
/// sent out through the server to the other device. /// sent out through the server to the other device.
#[allow(clippy::too_many_arguments)]
pub(crate) fn start( pub(crate) fn start(
private_identity: PrivateCrossSigningIdentity, private_identity: PrivateCrossSigningIdentity,
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
store: VerificationStore, store: VerificationStore,
own_identity: Option<ReadOnlyOwnUserIdentity>,
other_identity: Option<ReadOnlyUserIdentities>, other_identity: Option<ReadOnlyUserIdentities>,
transaction_id: Option<String>, transaction_id: Option<String>,
we_started: bool, we_started: bool,
@ -195,6 +197,7 @@ impl Sas {
let (inner, content) = InnerSas::start( let (inner, content) = InnerSas::start(
store.account.clone(), store.account.clone(),
other_device.clone(), other_device.clone(),
own_identity,
other_identity.clone(), other_identity.clone(),
transaction_id, transaction_id,
); );
@ -230,6 +233,7 @@ impl Sas {
private_identity: PrivateCrossSigningIdentity, private_identity: PrivateCrossSigningIdentity,
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
store: VerificationStore, store: VerificationStore,
own_identity: Option<ReadOnlyOwnUserIdentity>,
other_identity: Option<ReadOnlyUserIdentities>, other_identity: Option<ReadOnlyUserIdentities>,
we_started: bool, we_started: bool,
request_handle: RequestHandle, request_handle: RequestHandle,
@ -239,6 +243,7 @@ impl Sas {
room_id, room_id,
store.account.clone(), store.account.clone(),
other_device.clone(), other_device.clone(),
own_identity,
other_identity.clone(), other_identity.clone(),
); );
@ -273,6 +278,7 @@ impl Sas {
store: VerificationStore, store: VerificationStore,
private_identity: PrivateCrossSigningIdentity, private_identity: PrivateCrossSigningIdentity,
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
own_identity: Option<ReadOnlyOwnUserIdentity>,
other_identity: Option<ReadOnlyUserIdentities>, other_identity: Option<ReadOnlyUserIdentities>,
request_handle: Option<RequestHandle>, request_handle: Option<RequestHandle>,
we_started: bool, we_started: bool,
@ -282,6 +288,7 @@ impl Sas {
other_device.clone(), other_device.clone(),
flow_id, flow_id,
content, content,
own_identity,
other_identity.clone(), other_identity.clone(),
request_handle.is_some(), request_handle.is_some(),
)?; )?;
@ -601,6 +608,7 @@ mod test {
alice_store, alice_store,
None, None,
None, None,
None,
true, true,
None, None,
); );
@ -616,6 +624,7 @@ mod test {
alice_device, alice_device,
None, None,
None, None,
None,
false, false,
) )
.unwrap(); .unwrap();

View file

@ -60,7 +60,7 @@ use crate::{
}, },
Cancelled, Done, FlowId, Cancelled, Done, FlowId,
}, },
ReadOnlyAccount, ReadOnlyAccount, ReadOnlyOwnUserIdentity,
}; };
const KEY_AGREEMENT_PROTOCOLS: &[KeyAgreementProtocol] = const KEY_AGREEMENT_PROTOCOLS: &[KeyAgreementProtocol] =
@ -362,13 +362,21 @@ impl SasState<Created> {
pub fn new( pub fn new(
account: ReadOnlyAccount, account: ReadOnlyAccount,
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
own_identity: Option<ReadOnlyOwnUserIdentity>,
other_identity: Option<ReadOnlyUserIdentities>, other_identity: Option<ReadOnlyUserIdentities>,
transaction_id: Option<String>, transaction_id: Option<String>,
) -> SasState<Created> { ) -> SasState<Created> {
let started_from_request = transaction_id.is_some(); let started_from_request = transaction_id.is_some();
let flow_id = let flow_id =
FlowId::ToDevice(transaction_id.unwrap_or_else(|| Uuid::new_v4().to_string())); FlowId::ToDevice(transaction_id.unwrap_or_else(|| Uuid::new_v4().to_string()));
Self::new_helper(flow_id, account, other_device, other_identity, started_from_request) Self::new_helper(
flow_id,
account,
other_device,
own_identity,
other_identity,
started_from_request,
)
} }
/// Create a new SAS in-room verification flow. /// Create a new SAS in-room verification flow.
@ -388,22 +396,24 @@ impl SasState<Created> {
event_id: EventId, event_id: EventId,
account: ReadOnlyAccount, account: ReadOnlyAccount,
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
own_identity: Option<ReadOnlyOwnUserIdentity>,
other_identity: Option<ReadOnlyUserIdentities>, other_identity: Option<ReadOnlyUserIdentities>,
) -> SasState<Created> { ) -> SasState<Created> {
let flow_id = FlowId::InRoom(room_id, event_id); let flow_id = FlowId::InRoom(room_id, event_id);
Self::new_helper(flow_id, account, other_device, other_identity, false) Self::new_helper(flow_id, account, other_device, own_identity, other_identity, false)
} }
fn new_helper( fn new_helper(
flow_id: FlowId, flow_id: FlowId,
account: ReadOnlyAccount, account: ReadOnlyAccount,
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
own_identity: Option<ReadOnlyOwnUserIdentity>,
other_identity: Option<ReadOnlyUserIdentities>, other_identity: Option<ReadOnlyUserIdentities>,
started_from_request: bool, started_from_request: bool,
) -> SasState<Created> { ) -> SasState<Created> {
SasState { SasState {
inner: Arc::new(Mutex::new(OlmSas::new())), inner: Arc::new(Mutex::new(OlmSas::new())),
ids: SasIds { account, other_device, other_identity }, ids: SasIds { account, other_device, other_identity, own_identity },
verification_flow_id: flow_id.into(), verification_flow_id: flow_id.into(),
creation_time: Arc::new(Instant::now()), creation_time: Arc::new(Instant::now()),
@ -497,6 +507,7 @@ impl SasState<Started> {
pub fn from_start_event( pub fn from_start_event(
account: ReadOnlyAccount, account: ReadOnlyAccount,
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
own_identity: Option<ReadOnlyOwnUserIdentity>,
other_identity: Option<ReadOnlyUserIdentities>, other_identity: Option<ReadOnlyUserIdentities>,
flow_id: FlowId, flow_id: FlowId,
content: &StartContent, content: &StartContent,
@ -514,6 +525,7 @@ impl SasState<Started> {
ids: SasIds { ids: SasIds {
account: account.clone(), account: account.clone(),
other_device: other_device.clone(), other_device: other_device.clone(),
own_identity: own_identity.clone(),
other_identity: other_identity.clone(), other_identity: other_identity.clone(),
}, },
@ -536,7 +548,7 @@ impl SasState<Started> {
Ok(SasState { Ok(SasState {
inner: Arc::new(Mutex::new(sas)), inner: Arc::new(Mutex::new(sas)),
ids: SasIds { account, other_device, other_identity }, ids: SasIds { account, other_device, other_identity, own_identity },
creation_time: Arc::new(Instant::now()), creation_time: Arc::new(Instant::now()),
last_event_time: Arc::new(Instant::now()), last_event_time: Arc::new(Instant::now()),
@ -1158,7 +1170,7 @@ mod test {
let bob = ReadOnlyAccount::new(&bob_id(), &bob_device_id()); let bob = ReadOnlyAccount::new(&bob_id(), &bob_device_id());
let bob_device = ReadOnlyDevice::from_account(&bob).await; let bob_device = ReadOnlyDevice::from_account(&bob).await;
let alice_sas = SasState::<Created>::new(alice.clone(), bob_device, None, None); let alice_sas = SasState::<Created>::new(alice.clone(), bob_device, None, None, None);
let start_content = alice_sas.as_content(); let start_content = alice_sas.as_content();
let flow_id = start_content.flow_id(); let flow_id = start_content.flow_id();
@ -1167,6 +1179,7 @@ mod test {
bob.clone(), bob.clone(),
alice_device, alice_device,
None, None,
None,
flow_id, flow_id,
&start_content.as_start_content(), &start_content.as_start_content(),
false, false,
@ -1339,7 +1352,7 @@ mod test {
let bob = ReadOnlyAccount::new(&bob_id(), &bob_device_id()); let bob = ReadOnlyAccount::new(&bob_id(), &bob_device_id());
let bob_device = ReadOnlyDevice::from_account(&bob).await; let bob_device = ReadOnlyDevice::from_account(&bob).await;
let alice_sas = SasState::<Created>::new(alice.clone(), bob_device, None, None); let alice_sas = SasState::<Created>::new(alice.clone(), bob_device, None, None, None);
let mut start_content = alice_sas.as_content(); let mut start_content = alice_sas.as_content();
let method = start_content.method_mut(); let method = start_content.method_mut();
@ -1358,6 +1371,7 @@ mod test {
bob.clone(), bob.clone(),
alice_device.clone(), alice_device.clone(),
None, None,
None,
flow_id, flow_id,
&content, &content,
false, false,
@ -1379,6 +1393,7 @@ mod test {
bob.clone(), bob.clone(),
alice_device, alice_device,
None, None,
None,
flow_id.into(), flow_id.into(),
&content, &content,
false, false,