fix: Mark the gossip keys from the message as verified, not the ones from the db (#5247)

This commit is contained in:
Hocuri
2024-02-06 10:22:38 +01:00
committed by GitHub
parent 693045b542
commit f1688d2b3f
4 changed files with 54 additions and 63 deletions

View File

@@ -82,9 +82,10 @@ pub(crate) struct MimeMessage {
/// If a message is not encrypted or the signature is not valid,
/// this set is empty.
pub signatures: HashSet<Fingerprint>,
/// The set of mail recipient addresses for which gossip headers were applied, regardless of
/// whether they modified any peerstates.
pub gossiped_addr: HashSet<String>,
/// The mail recipient addresses for which gossip headers were applied
/// and their respective gossiped keys,
/// regardless of whether they modified any peerstates.
pub gossiped_keys: HashMap<String, SignedPublicKey>,
/// True if the message is a forwarded message.
pub is_forwarded: bool,
@@ -282,7 +283,7 @@ impl MimeMessage {
// Memory location for a possible decrypted message.
let mut mail_raw = Vec::new();
let mut gossiped_addr = Default::default();
let mut gossiped_keys = Default::default();
let mut from_is_signed = false;
hop_info += "\n\n";
hop_info += &decryption_info.dkim_results.to_string();
@@ -322,7 +323,7 @@ impl MimeMessage {
// but only if the mail was correctly signed. Probably it's ok to not require
// encryption here, but let's follow the standard.
let gossip_headers = mail.headers.get_all_values("Autocrypt-Gossip");
gossiped_addr = update_gossip_peerstates(
gossiped_keys = update_gossip_peerstates(
context,
message_time,
&from.addr,
@@ -413,7 +414,7 @@ impl MimeMessage {
// only non-empty if it was a valid autocrypt message
signatures,
gossiped_addr,
gossiped_keys,
is_forwarded: false,
mdn_reports: Vec::new(),
is_system_message: SystemMessage::Unknown,
@@ -1728,9 +1729,9 @@ async fn update_gossip_peerstates(
from: &str,
recipients: &[SingleInfo],
gossip_headers: Vec<String>,
) -> Result<HashSet<String>> {
) -> Result<HashMap<String, SignedPublicKey>> {
// XXX split the parsing from the modification part
let mut gossiped_addr: HashSet<String> = Default::default();
let mut gossiped_keys: HashMap<String, SignedPublicKey> = Default::default();
for value in &gossip_headers {
let header = match value.parse::<Aheader>() {
@@ -1774,10 +1775,10 @@ async fn update_gossip_peerstates(
.handle_fingerprint_change(context, message_time)
.await?;
gossiped_addr.insert(header.addr.to_lowercase());
gossiped_keys.insert(header.addr.to_lowercase(), header.public_key);
}
Ok(gossiped_addr)
Ok(gossiped_keys)
}
/// Message Disposition Notification (RFC 8098)

View File

@@ -457,59 +457,44 @@ impl Peerstate {
Ok(backward_verified)
}
/// Set this peerstate to verified
/// Make sure to call `self.save_to_db` to save these changes
/// Set this peerstate to verified;
/// make sure to call `self.save_to_db` to save these changes.
///
/// Params:
/// verifier:
///
/// * key: The new verified key.
/// * fingerprint: Only set to verified if the key's fingerprint matches this.
/// * verifier:
/// The address which introduces the given contact.
/// If we are verifying the contact, use that contacts address.
pub fn set_verified(
&mut self,
which_key: PeerstateKeyType,
key: SignedPublicKey,
fingerprint: Fingerprint,
verifier: String,
) -> Result<()> {
match which_key {
PeerstateKeyType::PublicKey => {
if self.public_key_fingerprint.is_some()
&& self.public_key_fingerprint.as_ref().unwrap() == &fingerprint
{
self.verified_key = self.public_key.clone();
self.verified_key_fingerprint = Some(fingerprint);
self.verifier = Some(verifier);
Ok(())
} else {
Err(Error::msg(format!(
"{fingerprint} is not peer's public key fingerprint",
)))
}
}
PeerstateKeyType::GossipKey => {
if self.gossip_key_fingerprint.is_some()
&& self.gossip_key_fingerprint.as_ref().unwrap() == &fingerprint
{
self.verified_key = self.gossip_key.clone();
self.verified_key_fingerprint = Some(fingerprint);
self.verifier = Some(verifier);
Ok(())
} else {
Err(Error::msg(format!(
"{fingerprint} is not peer's gossip key fingerprint",
)))
}
}
if key.fingerprint() == fingerprint {
self.verified_key = Some(key);
self.verified_key_fingerprint = Some(fingerprint);
self.verifier = Some(verifier);
Ok(())
} else {
Err(Error::msg(format!(
"{fingerprint} is not peer's key fingerprint",
)))
}
}
/// Sets current gossiped key as the secondary verified key.
/// Sets the gossiped key as the secondary verified key.
///
/// If gossiped key is the same as the current verified key,
/// do nothing to avoid overwriting secondary verified key
/// which may be different.
pub fn set_secondary_verified_key_from_gossip(&mut self, verifier: String) {
if self.gossip_key_fingerprint != self.verified_key_fingerprint {
self.secondary_verified_key = self.gossip_key.clone();
self.secondary_verified_key_fingerprint = self.gossip_key_fingerprint.clone();
pub fn set_secondary_verified_key(&mut self, gossip_key: SignedPublicKey, verifier: String) {
let fingerprint = gossip_key.fingerprint();
if self.verified_key_fingerprint.as_ref() != Some(&fingerprint) {
self.secondary_verified_key = Some(gossip_key);
self.secondary_verified_key_fingerprint = Some(fingerprint);
self.secondary_verifier = Some(verifier);
}
}

View File

@@ -31,7 +31,7 @@ use crate::message::{
};
use crate::mimeparser::{parse_message_ids, AvatarAction, MimeMessage, SystemMessage};
use crate::param::{Param, Params};
use crate::peerstate::{Peerstate, PeerstateKeyType};
use crate::peerstate::Peerstate;
use crate::reaction::{set_msg_reaction, Reaction};
use crate::securejoin::{self, handle_securejoin_handshake, observe_securejoin_on_other_device};
use crate::simplify;
@@ -440,7 +440,7 @@ pub(crate) async fn receive_imf_inner(
&& mime_parser
.recipients
.iter()
.all(|recipient| mime_parser.gossiped_addr.contains(&recipient.addr))
.all(|recipient| mime_parser.gossiped_keys.contains_key(&recipient.addr))
{
info!(
context,
@@ -2572,7 +2572,7 @@ async fn mark_recipients_as_verified(
for (to_addr, is_verified) in rows {
// mark gossiped keys (if any) as verified
if mimeparser.gossiped_addr.contains(&to_addr.to_lowercase()) {
if let Some(gossiped_key) = mimeparser.gossiped_keys.get(&to_addr.to_lowercase()) {
if let Some(mut peerstate) = Peerstate::from_addr(context, &to_addr).await? {
// If we're here, we know the gossip key is verified.
//
@@ -2587,7 +2587,7 @@ async fn mark_recipients_as_verified(
if !is_verified {
info!(context, "{verifier_addr} has verified {to_addr}.");
if let Some(fp) = peerstate.gossip_key_fingerprint.clone() {
peerstate.set_verified(PeerstateKeyType::GossipKey, fp, verifier_addr)?;
peerstate.set_verified(gossiped_key.clone(), fp, verifier_addr)?;
peerstate.backward_verified_key_id =
Some(context.get_config_i64(Config::KeyId).await?).filter(|&id| id > 0);
peerstate.save_to_db(&context.sql).await?;
@@ -2609,7 +2609,7 @@ async fn mark_recipients_as_verified(
} else {
// The contact already has a verified key.
// Store gossiped key as the secondary verified key.
peerstate.set_secondary_verified_key_from_gossip(verifier_addr);
peerstate.set_secondary_verified_key(gossiped_key.clone(), verifier_addr);
peerstate.save_to_db(&context.sql).await?;
}
}

View File

@@ -18,7 +18,7 @@ use crate::key::{load_self_public_key, DcKey, Fingerprint};
use crate::message::{Message, Viewtype};
use crate::mimeparser::{MimeMessage, SystemMessage};
use crate::param::Param;
use crate::peerstate::{Peerstate, PeerstateKeyType};
use crate::peerstate::Peerstate;
use crate::qr::check_qr;
use crate::securejoin::bob::JoinerProgress;
use crate::stock_str;
@@ -229,11 +229,13 @@ async fn verify_sender_by_fingerprint(
.filter(|&fp| fp == fingerprint)
.is_some()
{
let verifier = contact.get_addr().to_owned();
peerstate.set_verified(PeerstateKeyType::PublicKey, fingerprint.clone(), verifier)?;
peerstate.prefer_encrypt = EncryptPreference::Mutual;
peerstate.save_to_db(&context.sql).await?;
return Ok(true);
if let Some(public_key) = &peerstate.public_key {
let verifier = contact.get_addr().to_owned();
peerstate.set_verified(public_key.clone(), fingerprint.clone(), verifier)?;
peerstate.prefer_encrypt = EncryptPreference::Mutual;
peerstate.save_to_db(&context.sql).await?;
return Ok(true);
}
}
}
@@ -599,7 +601,7 @@ pub(crate) async fn observe_securejoin_on_other_device(
.get_addr()
.to_lowercase();
if !mime_message.gossiped_addr.contains(&addr) {
let Some(key) = mime_message.gossiped_keys.get(&addr) else {
could_not_establish_secure_connection(
context,
contact_id,
@@ -612,7 +614,7 @@ pub(crate) async fn observe_securejoin_on_other_device(
)
.await?;
return Ok(HandshakeMessage::Ignore);
}
};
let Some(mut peerstate) = Peerstate::from_addr(context, &addr).await? else {
could_not_establish_secure_connection(
@@ -638,7 +640,7 @@ pub(crate) async fn observe_securejoin_on_other_device(
.await?;
return Ok(HandshakeMessage::Ignore);
};
peerstate.set_verified(PeerstateKeyType::GossipKey, fingerprint, addr)?;
peerstate.set_verified(key.clone(), fingerprint, addr)?;
peerstate.prefer_encrypt = EncryptPreference::Mutual;
peerstate.save_to_db(&context.sql).await?;
@@ -714,7 +716,10 @@ async fn mark_peer_as_verified(
let Some(ref mut peerstate) = Peerstate::from_fingerprint(context, &fingerprint).await? else {
return Ok(false);
};
peerstate.set_verified(PeerstateKeyType::PublicKey, fingerprint, verifier)?;
let Some(ref public_key) = peerstate.public_key else {
return Ok(false);
};
peerstate.set_verified(public_key.clone(), fingerprint, verifier)?;
peerstate.prefer_encrypt = EncryptPreference::Mutual;
if backward_verified {
peerstate.backward_verified_key_id =