CodeReview: rename verification_latch to previously verified

This commit is contained in:
Valere
2024-08-05 13:51:30 +02:00
parent d81b12f389
commit 2e410e5d94
4 changed files with 59 additions and 56 deletions

View File

@@ -533,7 +533,7 @@ impl IdentityManager {
own_user_identity.is_identity_signed(&identity).is_ok()
});
if is_verified {
identity.mark_as_verified_once();
identity.mark_as_previously_verified();
}
Ok(identity.into())
@@ -2065,8 +2065,8 @@ pub(crate) mod tests {
// Set up a machine do initial own key query and import cross-signing secret to
// make the current session verified.
async fn common_verification_latch_machine_setup() -> OlmMachine {
use test_json::keys_query_sets::VerificationLatchTestData as DataSet;
async fn common_verified_identity_changes_machine_setup() -> OlmMachine {
use test_json::keys_query_sets::PreviouslyVerifiedTestData as DataSet;
let machine = OlmMachine::new(DataSet::own_id(), device_id!("LOCAL")).await;
@@ -2086,9 +2086,9 @@ pub(crate) mod tests {
}
#[async_test]
async fn test_manager_verified_latch_setup_on_new_identities() {
use test_json::keys_query_sets::VerificationLatchTestData as DataSet;
use test_json::keys_query_sets::PreviouslyVerifiedTestData as DataSet;
let machine = common_verification_latch_machine_setup().await;
let machine = common_verified_identity_changes_machine_setup().await;
// ######
// First test: Assert that the latch is properly set on new identities
@@ -2105,7 +2105,7 @@ pub(crate) mod tests {
let bob_identity =
machine.get_identity(DataSet::bob_id(), None).await.unwrap().unwrap().other().unwrap();
// The verified latch should be true
assert!(bob_identity.is_verified_latch_set());
assert!(bob_identity.was_previously_verified());
// And bob is verified
assert!(bob_identity.is_verified());
@@ -2121,7 +2121,7 @@ pub(crate) mod tests {
// Bob is not verified anymore
assert!(!bob_identity.is_verified());
// The verified latch should still be true
assert!(bob_identity.is_verified_latch_set());
assert!(bob_identity.was_previously_verified());
// Bob device_2 is self-signed even if there is this verification latch
// violation
let bob_device = machine
@@ -2136,15 +2136,15 @@ pub(crate) mod tests {
bob_identity.pin_current_master_key().await.unwrap();
assert!(!bob_identity.has_pin_violation());
let has_latch_violation =
bob_identity.is_verified_latch_set() && !bob_identity.is_verified();
bob_identity.was_previously_verified() && !bob_identity.is_verified();
assert!(has_latch_violation);
}
#[async_test]
async fn test_manager_verified_latch_setup_on_updated_identities() {
use test_json::keys_query_sets::VerificationLatchTestData as DataSet;
async fn test_manager_verified_identity_changes_setup_on_updated_identities() {
use test_json::keys_query_sets::PreviouslyVerifiedTestData as DataSet;
let machine = common_verification_latch_machine_setup().await;
let machine = common_verified_identity_changes_machine_setup().await;
// ######
// Get the Carol identity for the first time
@@ -2163,10 +2163,10 @@ pub(crate) mod tests {
// The identity is not verified
assert!(!carol_identity.is_verified());
// The verified latch is off
assert!(!carol_identity.is_verified_latch_set());
assert!(!carol_identity.was_previously_verified());
// Carol is verified, likely from another session. Ensure the latch is updated
// when the key query response is processes
// when the key query response is processed
let keys_query = DataSet::carol_keys_query_response_signed();
let txn_id = TransactionId::new();
machine.mark_request_as_sent(&txn_id, &keys_query).await.unwrap();
@@ -2180,7 +2180,7 @@ pub(crate) mod tests {
.unwrap();
assert!(carol_identity.is_verified());
// This should have updated the latch
assert!(carol_identity.is_verified_latch_set());
assert!(carol_identity.was_previously_verified());
// It is the same identity, it's just signed now so no pin violation
assert!(!carol_identity.has_pin_violation());
}
@@ -2188,8 +2188,8 @@ pub(crate) mod tests {
// Set up a machine do initial own key query.
// The cross signing secrets are not yet uploaded.
// Then query keys for carol and bob (both signed by own identity)
async fn common_verification_latch_own_trust_change_machine_setup() -> OlmMachine {
use test_json::keys_query_sets::VerificationLatchTestData as DataSet;
async fn common_verified_identity_changes_own_trust_change_machine_setup() -> OlmMachine {
use test_json::keys_query_sets::PreviouslyVerifiedTestData as DataSet;
// Start on a non-verified session
let machine = OlmMachine::new(DataSet::own_id(), device_id!("LOCAL")).await;
@@ -2222,9 +2222,9 @@ pub(crate) mod tests {
}
#[async_test]
async fn test_manager_verified_latch_setup_on_own_identity_trust_change() {
use test_json::keys_query_sets::VerificationLatchTestData as DataSet;
let machine = common_verification_latch_own_trust_change_machine_setup().await;
async fn test_manager_verified_identity_changes_setup_on_own_identity_trust_change() {
use test_json::keys_query_sets::PreviouslyVerifiedTestData as DataSet;
let machine = common_verified_identity_changes_own_trust_change_machine_setup().await;
let own_identity =
machine.get_identity(DataSet::own_id(), None).await.unwrap().unwrap().own().unwrap();
@@ -2233,7 +2233,7 @@ pub(crate) mod tests {
machine.get_identity(DataSet::bob_id(), None).await.unwrap().unwrap().other().unwrap();
// Carol is verified by our identity but our own identity is not yet trusted
assert!(own_identity.is_identity_signed(&bob_identity).is_ok());
assert!(!bob_identity.is_verified_latch_set());
assert!(!bob_identity.was_previously_verified());
let carol_identity = machine
.get_identity(DataSet::carol_id(), None)
@@ -2244,7 +2244,7 @@ pub(crate) mod tests {
.unwrap();
// Carol is verified by our identity but our own identity is not yet trusted
assert!(own_identity.is_identity_signed(&carol_identity).is_ok());
assert!(!carol_identity.is_verified_latch_set());
assert!(!carol_identity.was_previously_verified());
// Marking our own identity as trusted should update the existing identities
let _ = own_identity.verify().await;
@@ -2262,19 +2262,19 @@ pub(crate) mod tests {
.unwrap();
assert!(carol_identity.is_verified());
// The latch should be set now
assert!(carol_identity.is_verified_latch_set());
assert!(carol_identity.was_previously_verified());
let bob_identity =
machine.get_identity(DataSet::bob_id(), None).await.unwrap().unwrap().other().unwrap();
assert!(bob_identity.is_verified());
// The latch should be set now
assert!(bob_identity.is_verified_latch_set());
assert!(bob_identity.was_previously_verified());
}
#[async_test]
async fn test_manager_verified_latch_setup_on_import_secrets() {
use test_json::keys_query_sets::VerificationLatchTestData as DataSet;
let machine = common_verification_latch_own_trust_change_machine_setup().await;
async fn test_manager_verified_identity_change_setup_on_import_secrets() {
use test_json::keys_query_sets::PreviouslyVerifiedTestData as DataSet;
let machine = common_verified_identity_changes_own_trust_change_machine_setup().await;
let own_identity =
machine.get_identity(DataSet::own_id(), None).await.unwrap().unwrap().own().unwrap();
@@ -2283,7 +2283,7 @@ pub(crate) mod tests {
machine.get_identity(DataSet::bob_id(), None).await.unwrap().unwrap().other().unwrap();
// Carol is verified by our identity but our own identity is not yet trusted
assert!(own_identity.is_identity_signed(&bob_identity).is_ok());
assert!(!bob_identity.is_verified_latch_set());
assert!(!bob_identity.was_previously_verified());
let carol_identity = machine
.get_identity(DataSet::carol_id(), None)
@@ -2294,7 +2294,7 @@ pub(crate) mod tests {
.unwrap();
// Carol is verified by our identity but our own identity is not yet trusted
assert!(own_identity.is_identity_signed(&carol_identity).is_ok());
assert!(!carol_identity.is_verified_latch_set());
assert!(!carol_identity.was_previously_verified());
// Marking our own identity as trusted should update the existing identities
machine
@@ -2319,12 +2319,12 @@ pub(crate) mod tests {
.unwrap();
assert!(carol_identity.is_verified());
// The latch should be set now
assert!(carol_identity.is_verified_latch_set());
assert!(carol_identity.was_previously_verified());
let bob_identity =
machine.get_identity(DataSet::bob_id(), None).await.unwrap().unwrap().other().unwrap();
assert!(bob_identity.is_verified());
// The latch should be set now
assert!(bob_identity.is_verified_latch_set());
assert!(bob_identity.was_previously_verified());
}
}

View File

@@ -427,9 +427,10 @@ impl UserIdentityData {
/// identity changes (for example, in case of key material or device loss).
///
/// As soon as the cryptographic identity is verified (i.e. signed by our own
/// trusted identity), a latch is set to remember it (`verified_latch`). Future
/// interactions will expect this user to stay verified, in case of violation
/// the user should be notified with a blocking warning.
/// trusted identity), a flag is set to remember it (`previously_verified`).
/// Future interactions will expect this user to stay verified, in case of
/// violation the user should be notified with a blocking warning when sending a
/// message.
#[derive(Debug, Clone, Deserialize, Serialize)]
#[serde(try_from = "OtherUserIdentityDataSerializer", into = "OtherUserIdentityDataSerializer")]
pub struct OtherUserIdentityData {
@@ -437,10 +438,10 @@ pub struct OtherUserIdentityData {
pub(crate) master_key: Arc<MasterPubkey>,
self_signing_key: Arc<SelfSigningPubkey>,
pinned_master_key: Arc<RwLock<MasterPubkey>>,
/// This tracks whether we've ever verified this user with any identity at
/// This tracks whether we've verified this user with any identity at
/// any point in time on this device. To use it in the future to detect
/// cases where the user has become unverified for any reason.
verified_latch: Arc<AtomicBool>,
previously_verified: Arc<AtomicBool>,
}
/// Intermediate struct to help serialize OtherUserIdentityData and support
@@ -476,7 +477,7 @@ struct OtherUserIdentityDataSerializerV2 {
master_key: MasterPubkey,
self_signing_key: SelfSigningPubkey,
pinned_master_key: MasterPubkey,
verified_latch: bool,
previously_verified: bool,
}
impl TryFrom<OtherUserIdentityDataSerializer> for OtherUserIdentityData {
@@ -494,7 +495,7 @@ impl TryFrom<OtherUserIdentityDataSerializer> for OtherUserIdentityData {
self_signing_key: Arc::new(v0.self_signing_key),
// We migrate by pinning the current master key
pinned_master_key: Arc::new(RwLock::new(v0.master_key)),
verified_latch: Arc::new(false.into()),
previously_verified: Arc::new(false.into()),
})
}
Some(v) if v == "1" => {
@@ -506,7 +507,7 @@ impl TryFrom<OtherUserIdentityDataSerializer> for OtherUserIdentityData {
pinned_master_key: Arc::new(RwLock::new(v1.pinned_master_key)),
// Put it to false. There will be a migration to mark all users as dirty, so we
// will receive an update for the identity that will correctly set up the value.
verified_latch: Arc::new(false.into()),
previously_verified: Arc::new(false.into()),
})
}
Some(v) if v == "2" => {
@@ -516,7 +517,7 @@ impl TryFrom<OtherUserIdentityDataSerializer> for OtherUserIdentityData {
master_key: Arc::new(v2.master_key.clone()),
self_signing_key: Arc::new(v2.self_signing_key),
pinned_master_key: Arc::new(RwLock::new(v2.pinned_master_key)),
verified_latch: Arc::new(v2.verified_latch.into()),
previously_verified: Arc::new(v2.previously_verified.into()),
})
}
_ => Err(serde::de::Error::custom(format!("Unsupported Version {:?}", value.version))),
@@ -531,7 +532,7 @@ impl From<OtherUserIdentityData> for OtherUserIdentityDataSerializer {
master_key: value.master_key().to_owned(),
self_signing_key: value.self_signing_key().to_owned(),
pinned_master_key: value.pinned_master_key.read().unwrap().clone(),
verified_latch: value.verified_latch.load(Ordering::SeqCst),
previously_verified: value.previously_verified.load(Ordering::SeqCst),
};
OtherUserIdentityDataSerializer {
version: Some("2".to_owned()),
@@ -583,7 +584,7 @@ impl OtherUserIdentityData {
master_key: master_key.clone().into(),
self_signing_key: self_signing_key.into(),
pinned_master_key: RwLock::new(master_key).into(),
verified_latch: Arc::new(false.into()),
previously_verified: Arc::new(false.into()),
})
}
@@ -598,7 +599,7 @@ impl OtherUserIdentityData {
master_key: Arc::new(master_key.clone()),
self_signing_key,
pinned_master_key: Arc::new(RwLock::new(master_key.clone())),
verified_latch: Arc::new(false.into()),
previously_verified: Arc::new(false.into()),
}
}
@@ -624,17 +625,17 @@ impl OtherUserIdentityData {
}
/// Remember that this identity used to be verified at some point.
pub(crate) fn mark_as_verified_once(&self) {
self.verified_latch.store(true, Ordering::SeqCst)
pub(crate) fn mark_as_previously_verified(&self) {
self.previously_verified.store(true, Ordering::SeqCst)
}
/// True if we ever verified this identity (with any own identity, at any
/// True if we verified this identity (with any own identity, at any
/// point).
///
/// To pass this latch back to false, one must call
/// [`OtherUserIdentityData::withdraw_verification()`].
pub fn is_verified_latch_set(&self) -> bool {
self.verified_latch.load(Ordering::SeqCst)
pub fn was_previously_verified(&self) -> bool {
self.previously_verified.load(Ordering::SeqCst)
}
/// Remove the requirement for this identity to be verified.
@@ -643,7 +644,7 @@ impl OtherUserIdentityData {
/// reported to the user. In order to remove this notice users have to
/// verify again or to withdraw the verification requirement.
pub fn withdraw_verification(&self) {
self.verified_latch.store(false, Ordering::SeqCst)
self.previously_verified.store(false, Ordering::SeqCst)
}
/// Returns true if the identity has changed since we last pinned it.
@@ -700,7 +701,9 @@ impl OtherUserIdentityData {
master_key: master_key.clone().into(),
self_signing_key: self_signing_key.into(),
pinned_master_key: RwLock::new(pinned_master_key).into(),
verified_latch: Arc::new((self.is_verified_latch_set() || updated_is_verified).into()),
previously_verified: Arc::new(
(self.was_previously_verified() || updated_is_verified).into(),
),
};
let changed = new != *self;

View File

@@ -151,12 +151,12 @@ impl CryptoStoreWrapper {
// identities.
if let Some(own_identity_after) = maybe_own_identity.as_ref() {
// Only do this if our identity is passing from not verified to verified,
// the verified_latch can only go this way.
// the previously_verified can only change in that case.
if !own_identity_was_verified_before_change && own_identity_after.is_verified() {
debug!("Own identity is now verified, check all known identities for verification status changes");
// We need to review all the other identities to see if they are verified now
// and mark them as such
self.check_all_identities_and_update_verified_latch_if_needed(
self.check_all_identities_and_update_was_previousy_verified_flag_if_needed(
own_identity_after,
)
.await?;
@@ -169,7 +169,7 @@ impl CryptoStoreWrapper {
Ok(())
}
async fn check_all_identities_and_update_verified_latch_if_needed(
async fn check_all_identities_and_update_was_previousy_verified_flag_if_needed(
&self,
own_identity_after: &OwnUserIdentityData,
) -> Result<(), CryptoStoreError> {
@@ -183,11 +183,11 @@ impl CryptoStoreWrapper {
.as_ref()
.and_then(|i| i.other())
{
if !other_identity.is_verified_latch_set()
if !other_identity.was_previously_verified()
&& own_identity_after.is_identity_signed(other_identity).is_ok()
{
trace!(?tracked_user.user_id, "Marking set verified_latch to true.");
other_identity.mark_as_verified_once();
other_identity.mark_as_previously_verified();
updated_identities.push(other_identity.clone().into());
}
}

View File

@@ -668,10 +668,10 @@ impl IdentityChangeDataSet {
}
}
pub struct VerificationLatchTestData {}
pub struct PreviouslyVerifiedTestData {}
#[allow(dead_code)]
impl VerificationLatchTestData {
impl PreviouslyVerifiedTestData {
pub const MASTER_KEY_PRIVATE_EXPORT: &'static str =
"bSa0nVTocZArMzL7OLmeFUIVF4ycp64rrkVMgqOYg6Y";
pub const SELF_SIGNING_KEY_PRIVATE_EXPORT: &'static str =