From 3d25d46dc5b14c506692ea8a82151b6e4f39fafd Mon Sep 17 00:00:00 2001 From: Moritz Bitsch Date: Wed, 20 Oct 2021 06:20:34 +0200 Subject: [PATCH 1/5] Use simple BTreeMap to store uiaa requests some uiaa requests contain plaintext passwords which should never be persisted to disk. Currently there is no cleanup implemented (you have to restart conduit) --- src/database.rs | 3 +-- src/database/uiaa.rs | 16 +++++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/database.rs b/src/database.rs index 84ca68dc..83b0fd5e 100644 --- a/src/database.rs +++ b/src/database.rs @@ -250,8 +250,7 @@ impl Database { }, uiaa: uiaa::Uiaa { userdevicesessionid_uiaainfo: builder.open_tree("userdevicesessionid_uiaainfo")?, - userdevicesessionid_uiaarequest: builder - .open_tree("userdevicesessionid_uiaarequest")?, + userdevicesessionid_uiaarequest: RwLock::new(BTreeMap::new()), }, rooms: rooms::Rooms { edus: rooms::RoomEdus { diff --git a/src/database/uiaa.rs b/src/database/uiaa.rs index 1c0fb566..2ecca93d 100644 --- a/src/database/uiaa.rs +++ b/src/database/uiaa.rs @@ -1,4 +1,6 @@ use std::sync::Arc; +use std::sync::RwLock; +use std::collections::BTreeMap; use crate::{client_server::SESSION_ID_LENGTH, utils, Error, Result}; use ruma::{ @@ -18,7 +20,7 @@ use super::abstraction::Tree; pub struct Uiaa { pub(super) userdevicesessionid_uiaainfo: Arc, // User-interactive authentication - pub(super) userdevicesessionid_uiaarequest: Arc, // UiaaRequest = canonical json value + pub(super) userdevicesessionid_uiaarequest: RwLock, Vec>>, // UiaaRequest = canonical json value } impl Uiaa { @@ -153,10 +155,10 @@ impl Uiaa { userdevicesessionid.push(0xff); userdevicesessionid.extend_from_slice(session.as_bytes()); - self.userdevicesessionid_uiaarequest.insert( - &userdevicesessionid, - &serde_json::to_vec(request).expect("json value to vec always works"), - )?; + self.userdevicesessionid_uiaarequest.write().unwrap().insert( + userdevicesessionid, + serde_json::to_vec(request).expect("json value to vec always works"), + ); Ok(()) } @@ -173,8 +175,8 @@ impl Uiaa { userdevicesessionid.push(0xff); userdevicesessionid.extend_from_slice(session.as_bytes()); - self.userdevicesessionid_uiaarequest - .get(&userdevicesessionid)? + self.userdevicesessionid_uiaarequest.read().unwrap() + .get(&userdevicesessionid) .map(|bytes| { serde_json::from_str::( &utils::string_from_bytes(&bytes) From fe8cfe05569e667b03ee855a2463964a5a029661 Mon Sep 17 00:00:00 2001 From: Moritz Bitsch Date: Tue, 14 Dec 2021 17:55:28 +0100 Subject: [PATCH 2/5] Add database migration to remove stored passwords uiaarequests can contain plaintext passwords, which were stored on disk --- src/database.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/database.rs b/src/database.rs index 83b0fd5e..8b29b221 100644 --- a/src/database.rs +++ b/src/database.rs @@ -754,6 +754,15 @@ impl Database { println!("Migration: 9 -> 10 finished"); } + + if db.globals.database_version()? < 11 { + db._db + .open_tree("userdevicesessionid_uiaarequest")? + .clear()?; + db.globals.bump_database_version(11)?; + + println!("Migration: 10 -> 11 finished"); + } } let guard = db.read().await; From 0725b69abb7453df534a764947b6015ffe8293c4 Mon Sep 17 00:00:00 2001 From: Moritz Bitsch Date: Sat, 18 Dec 2021 18:46:38 +0100 Subject: [PATCH 3/5] Clean up userdevicesessionid_uiaarequest BTreeMap There is no need to encode or decode anything as we are not saving to disk --- src/database/uiaa.rs | 52 ++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/src/database/uiaa.rs b/src/database/uiaa.rs index 2ecca93d..461a3e27 100644 --- a/src/database/uiaa.rs +++ b/src/database/uiaa.rs @@ -1,6 +1,6 @@ +use std::collections::BTreeMap; use std::sync::Arc; use std::sync::RwLock; -use std::collections::BTreeMap; use crate::{client_server::SESSION_ID_LENGTH, utils, Error, Result}; use ruma::{ @@ -20,7 +20,8 @@ use super::abstraction::Tree; pub struct Uiaa { pub(super) userdevicesessionid_uiaainfo: Arc, // User-interactive authentication - pub(super) userdevicesessionid_uiaarequest: RwLock, Vec>>, // UiaaRequest = canonical json value + pub(super) userdevicesessionid_uiaarequest: + RwLock>, } impl Uiaa { @@ -149,16 +150,17 @@ impl Uiaa { session: &str, request: &CanonicalJsonValue, ) -> Result<()> { - let mut userdevicesessionid = user_id.as_bytes().to_vec(); - userdevicesessionid.push(0xff); - userdevicesessionid.extend_from_slice(device_id.as_bytes()); - userdevicesessionid.push(0xff); - userdevicesessionid.extend_from_slice(session.as_bytes()); - - self.userdevicesessionid_uiaarequest.write().unwrap().insert( - userdevicesessionid, - serde_json::to_vec(request).expect("json value to vec always works"), - ); + self.userdevicesessionid_uiaarequest + .write() + .unwrap() + .insert( + ( + user_id.to_owned(), + device_id.to_string(), + session.to_string(), + ), + request.to_owned(), + ); Ok(()) } @@ -169,22 +171,16 @@ impl Uiaa { device_id: &DeviceId, session: &str, ) -> Result> { - let mut userdevicesessionid = user_id.as_bytes().to_vec(); - userdevicesessionid.push(0xff); - userdevicesessionid.extend_from_slice(device_id.as_bytes()); - userdevicesessionid.push(0xff); - userdevicesessionid.extend_from_slice(session.as_bytes()); - - self.userdevicesessionid_uiaarequest.read().unwrap() - .get(&userdevicesessionid) - .map(|bytes| { - serde_json::from_str::( - &utils::string_from_bytes(&bytes) - .map_err(|_| Error::bad_database("Invalid uiaa request bytes in db."))?, - ) - .map_err(|_| Error::bad_database("Invalid uiaa request in db.")) - }) - .transpose() + Ok(self + .userdevicesessionid_uiaarequest + .read() + .unwrap() + .get(&( + user_id.to_owned(), + device_id.to_string(), + session.to_string(), + )) + .map(|j| j.to_owned())) } fn update_uiaa_session( From 720a54b3bb74301eaf08f54edd163995bf5ef7fa Mon Sep 17 00:00:00 2001 From: Moritz Bitsch Date: Sat, 18 Dec 2021 19:05:18 +0100 Subject: [PATCH 4/5] Use String to store UserId for uiaa request Fixes compilation error after ruma upgrade --- src/database/uiaa.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/database/uiaa.rs b/src/database/uiaa.rs index 461a3e27..6a5f7a33 100644 --- a/src/database/uiaa.rs +++ b/src/database/uiaa.rs @@ -21,7 +21,7 @@ use super::abstraction::Tree; pub struct Uiaa { pub(super) userdevicesessionid_uiaainfo: Arc, // User-interactive authentication pub(super) userdevicesessionid_uiaarequest: - RwLock>, + RwLock>, } impl Uiaa { @@ -155,7 +155,7 @@ impl Uiaa { .unwrap() .insert( ( - user_id.to_owned(), + user_id.to_string(), device_id.to_string(), session.to_string(), ), @@ -176,7 +176,7 @@ impl Uiaa { .read() .unwrap() .get(&( - user_id.to_owned(), + user_id.to_string(), device_id.to_string(), session.to_string(), )) From c4a438460e0537e465f5b93514fd05b66a03ad37 Mon Sep 17 00:00:00 2001 From: Moritz Bitsch Date: Wed, 22 Dec 2021 19:26:23 +0100 Subject: [PATCH 5/5] Use Box to store UserID and DeviceID Userid and DeviceID are of unknown size, use Box to be able to store them into the userdevicesessionid_uiaarequest BTreeMap --- src/database/uiaa.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/database/uiaa.rs b/src/database/uiaa.rs index 6a5f7a33..772dab9e 100644 --- a/src/database/uiaa.rs +++ b/src/database/uiaa.rs @@ -21,7 +21,7 @@ use super::abstraction::Tree; pub struct Uiaa { pub(super) userdevicesessionid_uiaainfo: Arc, // User-interactive authentication pub(super) userdevicesessionid_uiaarequest: - RwLock>, + RwLock, Box, String), CanonicalJsonValue>>, } impl Uiaa { @@ -155,8 +155,8 @@ impl Uiaa { .unwrap() .insert( ( - user_id.to_string(), - device_id.to_string(), + user_id.to_owned(), + device_id.to_owned(), session.to_string(), ), request.to_owned(), @@ -176,8 +176,8 @@ impl Uiaa { .read() .unwrap() .get(&( - user_id.to_string(), - device_id.to_string(), + user_id.to_owned(), + device_id.to_owned(), session.to_string(), )) .map(|j| j.to_owned()))