Closed progval closed 4 months ago
It also leaks internal ids. Is it a big deal?
We had a think about this with a few other staff and concluded that yes, that's a problem - even if we don't go for complete server hiding, the origin server of a msgid could disclose non-trivial behavioural information (e.g. suggesting that someone's currently on a mobile client instead of their normal one) which looks like it ought to be hidden by multi-connections.
We're going to go for uuid7 instead of reusing the internal ids; though we may revisit this decision later.
btw I started working on encrypting the internal ids, here is what I had:
commit 42f29fb23064d2735e05183efafc3df04ac6b3d4
Author: Val Lorentz <progval+git@progval.net>
Date: Fri Apr 19 13:59:25 2024 +0200
[WIP] encrypted message ids
diff --git a/Cargo.lock b/Cargo.lock
index 32c303d..d510b8d 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2126,6 +2126,7 @@ dependencies = [
"pretty_assertions",
"pwhash",
"rand",
+ "ring",
"rustls",
"rustls-pemfile",
"sable_macros",
diff --git a/sable_network/Cargo.toml b/sable_network/Cargo.toml
index 83c1083..74658f8 100644
--- a/sable_network/Cargo.toml
+++ b/sable_network/Cargo.toml
@@ -30,6 +30,7 @@ pwhash = "1"
tokio-rustls = "0.23"
rustls = "0.20"
rustls-pemfile = "0.2"
+ring = "0.16.0"
bitflags = "1.3"
itertools = "0.10"
futures = "0.3"
diff --git a/sable_network/src/network/network/message_state.rs b/sable_network/src/network/network/message_state.rs
index 7c31237..211a2de 100644
--- a/sable_network/src/network/network/message_state.rs
+++ b/sable_network/src/network/network/message_state.rs
@@ -10,6 +10,11 @@ impl Network {
) {
let message = state::Message {
id: target,
+ sealed_id: SealedMessageId::new(
+ self.config.message_keys.current_sealing_key_id,
+ self.config.message_keys.current_sealing_key,
+ target,
+ ),
source: details.source,
target: details.target,
ts: event.timestamp,
diff --git a/sable_network/src/network/state/message.rs b/sable_network/src/network/state/message.rs
index ed46675..3aefa9f 100644
--- a/sable_network/src/network/state/message.rs
+++ b/sable_network/src/network/state/message.rs
@@ -1,6 +1,75 @@
-use crate::prelude::*;
+use std::convert::TryInto;
+use ring::aead::chacha20_poly1305_openssh::{OpeningKey, SealingKey};
use serde::{Deserialize, Serialize};
+use thiserror::Error;
+
+use crate::prelude::*;
+
+#[derive(Error, Debug, Eq, PartialEq)]
+pub enum OpenMessageIdError {
+ #[error("Unknown key id {key_id}")]
+ UnknownKey { key_id: u32 },
+ #[error("Could not authenticate message id with key {key_id}")]
+ AuthenticationFailure { key_id: u32 },
+}
+
+pub struct MessageIdOpeningKey(pub OpeningKey);
+pub struct MessageIdSealingKey(pub SealingKey);
+
+// Safe to keep it always 0, see https://www.rfc-editor.org/rfc/rfc7539#section-2.4
+const SEQUENCE_NUMBER: u32 = 0;
+
+/// Encrypted and signed [`MessageId`]
+///
+/// Unlike [`MessageId`], this is suitable to be sent publicly without revealing information
+/// about the originating server (and epoch) of a message.
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct SealedMessageId {
+ key_id: u32,
+ nonce: [u8; 16],
+ ciphertext: [u8; 24],
+}
+
+impl SealedMessageId {
+ pub fn new(key_id: u32, key: MessageIdSealingKey, id: MessageId) -> SealedMessageId {
+ let mut buf = [0u8; 24];
+ buf[0..8].copy_from_slice(&id.server().local().to_le_bytes());
+ buf[8..16].copy_from_slice(&id.epoch().local().to_le_bytes());
+ buf[16..24].copy_from_slice(&id.local().to_le_bytes());
+ let mut nonce = [0u8; 16];
+
+ key.0.seal_in_place(SEQUENCE_NUMBER, &mut buf, &mut nonce);
+
+ SealedMessageId {
+ key_id,
+ nonce,
+ ciphertext: buf,
+ }
+ }
+
+ pub fn open(&self, keys: &[MessageIdOpeningKey]) -> Result<MessageId, OpenMessageIdError> {
+ let key_id = self.key_id;
+ let key: &MessageIdOpeningKey = keys
+ .get::<usize>(key_id.try_into().expect("key id overflowed usize"))
+ .ok_or(OpenMessageIdError::UnknownKey { key_id })?;
+
+ let mut buf = self.ciphertext.clone();
+ key.0
+ .open_in_place(SEQUENCE_NUMBER, &mut buf, &self.nonce)
+ .map_err(|_| OpenMessageIdError::AuthenticationFailure { key_id })?;
+
+ let server_id = i64::from_le_bytes(buf[0..8].try_into().unwrap());
+ let epoch_id = i64::from_le_bytes(buf[8..16].try_into().unwrap());
+ let local_id = i64::from_le_bytes(buf[16..24].try_into().unwrap());
+
+ Ok(MessageId::new(
+ ServerId::new(server_id),
+ EpochId::new(epoch_id),
+ local_id,
+ ))
+ }
+}
/// Message type - privmsg or notice
#[derive(Debug, Clone, Copy, Serialize, Deserialize)]
@@ -13,6 +82,7 @@ pub enum MessageType {
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Message {
pub id: MessageId,
+ pub sealed_id: SealedMessageId,
pub source: UserId,
pub target: ObjectId,
pub ts: i64,
This includes a minimal implementation of message-tags, in order to be able to send the msgid tag.
The tag looks like this:
msgid=AQAAAAAAAABDjhlmAAAAABAAAAAAAAAA
. I can probably make it more compact when values are small, but I don't think it's worth it. It also leaks internal ids. Is it a big deal?My main motivation for this is to be able to run irctest's chathistory tests, which depend on msgid to avoid complexity.