From 403f35b571ae2abb8e1df118bfa543e35805a52f Mon Sep 17 00:00:00 2001 From: BlackDex Date: Sun, 4 Jul 2021 23:02:56 +0200 Subject: [PATCH] Added web-vault v2.21.x support + some misc fixes - The new web-vault v2.21.0+ has support for Master Password Reset. For this to work it generates a public/private key-pair which needs to be stored in the database. Currently the Master Password Reset is not fixed, but there are endpoints which are needed even if we do not support this feature (yet). This PR fixes those endpoints, and stores the keys already in the database. - There was an issue when you want to do a key-rotate when you change your password, it also called an Emergency Access endpoint, which we do not yet support. Because this endpoint failed to reply correctly produced some errors, and also prevent the user from being forced to logout. This resolves #1826 by adding at least that endpoint. Because of that extra endpoint check to Emergency Access is done using an old user stamp, i also modified the stamp exception to allow multiple rocket routes to be called, and added an expiration timestamp to it. During these tests i stumbled upon an issue that after my key-change was done, it triggered the websockets to try and reload my ciphers, because they were updated. This shouldn't happen when rotating they keys, since all access should be invalided. Now there will be no websocket notification for this, which also prevents error toasts. - Increased Send Size limit to 500MB (with a litle overhead) As a side note, i tested these changes on both v2.20.4 and v2.21.1 web-vault versions, all keeps working. --- .../down.sql | 0 .../up.sql | 5 +++ .../down.sql | 0 .../up.sql | 5 +++ .../down.sql | 0 .../up.sql | 5 +++ src/api/core/accounts.rs | 10 +++-- src/api/core/emergency_access.rs | 24 +++++++++++ src/api/core/mod.rs | 2 + src/api/core/organizations.rs | 43 ++++++++++++++++++- src/api/core/sends.rs | 8 ++-- src/auth.rs | 15 ++++++- src/db/models/organization.rs | 27 ++++++++---- src/db/models/user.rs | 27 ++++++------ src/db/schemas/mysql/schema.rs | 2 + src/db/schemas/postgresql/schema.rs | 2 + src/db/schemas/sqlite/schema.rs | 2 + src/error.rs | 3 ++ 18 files changed, 147 insertions(+), 33 deletions(-) create mode 100644 migrations/mysql/2021-07-01-203140_add_password_reset_keys/down.sql create mode 100644 migrations/mysql/2021-07-01-203140_add_password_reset_keys/up.sql create mode 100644 migrations/postgresql/2021-07-01-203140_add_password_reset_keys/down.sql create mode 100644 migrations/postgresql/2021-07-01-203140_add_password_reset_keys/up.sql create mode 100644 migrations/sqlite/2021-07-01-203140_add_password_reset_keys/down.sql create mode 100644 migrations/sqlite/2021-07-01-203140_add_password_reset_keys/up.sql create mode 100644 src/api/core/emergency_access.rs diff --git a/migrations/mysql/2021-07-01-203140_add_password_reset_keys/down.sql b/migrations/mysql/2021-07-01-203140_add_password_reset_keys/down.sql new file mode 100644 index 00000000..e69de29b diff --git a/migrations/mysql/2021-07-01-203140_add_password_reset_keys/up.sql b/migrations/mysql/2021-07-01-203140_add_password_reset_keys/up.sql new file mode 100644 index 00000000..2e83860c --- /dev/null +++ b/migrations/mysql/2021-07-01-203140_add_password_reset_keys/up.sql @@ -0,0 +1,5 @@ +ALTER TABLE organizations + ADD COLUMN private_key TEXT; + +ALTER TABLE organizations + ADD COLUMN public_key TEXT; diff --git a/migrations/postgresql/2021-07-01-203140_add_password_reset_keys/down.sql b/migrations/postgresql/2021-07-01-203140_add_password_reset_keys/down.sql new file mode 100644 index 00000000..e69de29b diff --git a/migrations/postgresql/2021-07-01-203140_add_password_reset_keys/up.sql b/migrations/postgresql/2021-07-01-203140_add_password_reset_keys/up.sql new file mode 100644 index 00000000..2e83860c --- /dev/null +++ b/migrations/postgresql/2021-07-01-203140_add_password_reset_keys/up.sql @@ -0,0 +1,5 @@ +ALTER TABLE organizations + ADD COLUMN private_key TEXT; + +ALTER TABLE organizations + ADD COLUMN public_key TEXT; diff --git a/migrations/sqlite/2021-07-01-203140_add_password_reset_keys/down.sql b/migrations/sqlite/2021-07-01-203140_add_password_reset_keys/down.sql new file mode 100644 index 00000000..e69de29b diff --git a/migrations/sqlite/2021-07-01-203140_add_password_reset_keys/up.sql b/migrations/sqlite/2021-07-01-203140_add_password_reset_keys/up.sql new file mode 100644 index 00000000..2e83860c --- /dev/null +++ b/migrations/sqlite/2021-07-01-203140_add_password_reset_keys/up.sql @@ -0,0 +1,5 @@ +ALTER TABLE organizations + ADD COLUMN private_key TEXT; + +ALTER TABLE organizations + ADD COLUMN public_key TEXT; diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 3888075b..8cf62f45 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -231,7 +231,10 @@ fn post_password(data: JsonUpcase, headers: Headers, conn: DbCon err!("Invalid password") } - user.set_password(&data.NewMasterPasswordHash, Some("post_rotatekey")); + user.set_password( + &data.NewMasterPasswordHash, + Some(vec![String::from("post_rotatekey"), String::from("get_contacts")]), + ); user.akey = data.Key; user.save(&conn) } @@ -320,7 +323,9 @@ fn post_rotatekey(data: JsonUpcase, headers: Headers, conn: DbConn, nt: err!("The cipher is not owned by the user") } - update_cipher_from_data(&mut saved_cipher, cipher_data, &headers, false, &conn, &nt, UpdateType::CipherUpdate)? + // Prevent triggering cipher updates via WebSockets by settings UpdateType::None + // The user sessions are invalidated because all the ciphers were re-encrypted and thus triggering an update could cause issues. + update_cipher_from_data(&mut saved_cipher, cipher_data, &headers, false, &conn, &nt, UpdateType::None)? } // Update user data @@ -329,7 +334,6 @@ fn post_rotatekey(data: JsonUpcase, headers: Headers, conn: DbConn, nt: user.akey = data.Key; user.private_key = Some(data.PrivateKey); user.reset_security_stamp(); - user.reset_stamp_exception(); user.save(&conn) } diff --git a/src/api/core/emergency_access.rs b/src/api/core/emergency_access.rs new file mode 100644 index 00000000..60ebdf10 --- /dev/null +++ b/src/api/core/emergency_access.rs @@ -0,0 +1,24 @@ +use rocket::Route; +use rocket_contrib::json::Json; + +use crate::{api::JsonResult, auth::Headers, db::DbConn}; + +pub fn routes() -> Vec { + routes![get_contacts,] +} + +/// This endpoint is expected to return at least something. +/// If we return an error message that will trigger error toasts for the user. +/// To prevent this we just return an empty json result with no Data. +/// When this feature is going to be implemented it also needs to return this empty Data +/// instead of throwing an error/4XX unless it really is an error. +#[get("/emergency-access/trusted")] +fn get_contacts(_headers: Headers, _conn: DbConn) -> JsonResult { + debug!("Emergency access is not supported."); + + Ok(Json(json!({ + "Data": [], + "Object": "list", + "ContinuationToken": null + }))) +} diff --git a/src/api/core/mod.rs b/src/api/core/mod.rs index e1c1fa0d..ea682963 100644 --- a/src/api/core/mod.rs +++ b/src/api/core/mod.rs @@ -1,5 +1,6 @@ mod accounts; mod ciphers; +mod emergency_access; mod folders; mod organizations; mod sends; @@ -15,6 +16,7 @@ pub fn routes() -> Vec { let mut routes = Vec::new(); routes.append(&mut accounts::routes()); routes.append(&mut ciphers::routes()); + routes.append(&mut emergency_access::routes()); routes.append(&mut folders::routes()); routes.append(&mut organizations::routes()); routes.append(&mut two_factor::routes()); diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 5c1ccedb..f0488128 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -51,6 +51,7 @@ pub fn routes() -> Vec { get_plans, get_plans_tax_rates, import, + post_org_keys, ] } @@ -61,6 +62,7 @@ struct OrgData { CollectionName: String, Key: String, Name: String, + Keys: Option, #[serde(rename = "PlanType")] _PlanType: NumberOrString, // Ignored, always use the same plan } @@ -78,6 +80,13 @@ struct NewCollectionData { Name: String, } +#[derive(Deserialize)] +#[allow(non_snake_case)] +struct OrgKeyData { + EncryptedPrivateKey: String, + PublicKey: String, +} + #[post("/organizations", data = "")] fn create_organization(headers: Headers, data: JsonUpcase, conn: DbConn) -> JsonResult { if !CONFIG.is_org_creation_allowed(&headers.user.email) { @@ -85,8 +94,14 @@ fn create_organization(headers: Headers, data: JsonUpcase, conn: DbConn } let data: OrgData = data.into_inner().data; + let (private_key, public_key) = if data.Keys.is_some() { + let keys: OrgKeyData = data.Keys.unwrap(); + (Some(keys.EncryptedPrivateKey), Some(keys.PublicKey)) + } else { + (None, None) + }; - let org = Organization::new(data.Name, data.BillingEmail); + let org = Organization::new(data.Name, data.BillingEmail, private_key, public_key); let mut user_org = UserOrganization::new(headers.user.uuid, org.uuid.clone()); let collection = Collection::new(org.uuid.clone(), data.CollectionName); @@ -468,6 +483,32 @@ fn get_org_users(org_id: String, _headers: ManagerHeadersLoose, conn: DbConn) -> })) } +#[post("/organizations//keys", data = "")] +fn post_org_keys(org_id: String, data: JsonUpcase, _headers: AdminHeaders, conn: DbConn) -> JsonResult { + let data: OrgKeyData = data.into_inner().data; + + let mut org = match Organization::find_by_uuid(&org_id, &conn) { + Some(organization) => { + if organization.private_key.is_some() && organization.public_key.is_some() { + err!("Organization Keys already exist") + } + organization + } + None => err!("Can't find organization details"), + }; + + org.private_key = Some(data.EncryptedPrivateKey); + org.public_key = Some(data.PublicKey); + + org.save(&conn)?; + + Ok(Json(json!({ + "Object": "organizationKeys", + "PublicKey": org.public_key, + "PrivateKey": org.private_key, + }))) +} + #[derive(Deserialize)] #[allow(non_snake_case)] struct CollectionData { diff --git a/src/api/core/sends.rs b/src/api/core/sends.rs index 13cd300e..39133431 100644 --- a/src/api/core/sends.rs +++ b/src/api/core/sends.rs @@ -165,8 +165,8 @@ fn post_send_file(data: Data, content_type: &ContentType, headers: Headers, conn let data = serde_json::from_str::>(&buf)?; enforce_disable_hide_email_policy(&data.data, &headers, &conn)?; - // Get the file length and add an extra 10% to avoid issues - const SIZE_110_MB: u64 = 115_343_360; + // Get the file length and add an extra 5% to avoid issues + const SIZE_525_MB: u64 = 550_502_400; let size_limit = match CONFIG.user_attachment_limit() { Some(0) => err!("File uploads are disabled"), @@ -175,9 +175,9 @@ fn post_send_file(data: Data, content_type: &ContentType, headers: Headers, conn if left <= 0 { err!("Attachment size limit reached! Delete some files to open space") } - std::cmp::Ord::max(left as u64, SIZE_110_MB) + std::cmp::Ord::max(left as u64, SIZE_525_MB) } - None => SIZE_110_MB, + None => SIZE_525_MB, }; // Create the Send diff --git a/src/auth.rs b/src/auth.rs index 1329cdc3..694f6af9 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -325,8 +325,19 @@ impl<'a, 'r> FromRequest<'a, 'r> for Headers { _ => err_handler!("Error getting current route for stamp exception"), }; - // Check if both match, if not this route is not allowed with the current security stamp. - if stamp_exception.route != current_route { + // Check if the stamp exception has expired first. + // Then, check if the current route matches any of the allowed routes. + // After that check the stamp in exception matches the one in the claims. + if Utc::now().naive_utc().timestamp() > stamp_exception.expire { + // If the stamp exception has been expired remove it from the database. + // This prevents checking this stamp exception for new requests. + let mut user = user; + user.reset_stamp_exception(); + if let Err(e) = user.save(&conn) { + error!("Error updating user: {:#?}", e); + } + err_handler!("Stamp exception is expired") + } else if !stamp_exception.routes.contains(¤t_route.to_string()) { err_handler!("Invalid security stamp: Current route and exception route do not match") } else if stamp_exception.security_stamp != claims.sstamp { err_handler!("Invalid security stamp for matched stamp exception") diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index e0c87977..f628d95f 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -12,6 +12,8 @@ db_object! { pub uuid: String, pub name: String, pub billing_email: String, + pub private_key: Option, + pub public_key: Option, } #[derive(Identifiable, Queryable, Insertable, AsChangeset)] @@ -122,12 +124,13 @@ impl PartialOrd for i32 { /// Local methods impl Organization { - pub fn new(name: String, billing_email: String) -> Self { + pub fn new(name: String, billing_email: String, private_key: Option, public_key: Option) -> Self { Self { uuid: crate::util::get_uuid(), - name, billing_email, + private_key, + public_key, } } @@ -140,14 +143,16 @@ impl Organization { "MaxCollections": 10, // The value doesn't matter, we don't check server-side "MaxStorageGb": 10, // The value doesn't matter, we don't check server-side "Use2fa": true, - "UseDirectory": false, - "UseEvents": false, - "UseGroups": false, + "UseDirectory": false, // Is supported, but this value isn't checked anywhere (yet) + "UseEvents": false, // not supported by us + "UseGroups": false, // not supported by us "UseTotp": true, "UsePolicies": true, "UseSso": false, // We do not support SSO "SelfHost": true, "UseApi": false, // not supported by us + "HasPublicAndPrivateKeys": self.private_key.is_some() && self.public_key.is_some(), + "ResetPasswordEnrolled": false, // not supported by us "BusinessName": null, "BusinessAddress1": null, @@ -269,13 +274,15 @@ impl UserOrganization { "UsersGetPremium": true, "Use2fa": true, - "UseDirectory": false, - "UseEvents": false, - "UseGroups": false, + "UseDirectory": false, // Is supported, but this value isn't checked anywhere (yet) + "UseEvents": false, // not supported by us + "UseGroups": false, // not supported by us "UseTotp": true, "UsePolicies": true, "UseApi": false, // not supported by us "SelfHost": true, + "HasPublicAndPrivateKeys": org.private_key.is_some() && org.public_key.is_some(), + "ResetPasswordEnrolled": false, // not supported by us "SsoBound": false, // We do not support SSO "UseSso": false, // We do not support SSO // TODO: Add support for Business Portal @@ -293,10 +300,12 @@ impl UserOrganization { // "AccessReports": false, // "ManageAllCollections": false, // "ManageAssignedCollections": false, + // "ManageCiphers": false, // "ManageGroups": false, // "ManagePolicies": false, + // "ManageResetPassword": false, // "ManageSso": false, - // "ManageUsers": false + // "ManageUsers": false, // }, "MaxStorageGb": 10, // The value doesn't matter, we don't check server-side diff --git a/src/db/models/user.rs b/src/db/models/user.rs index 5b2e1131..b370d356 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -1,4 +1,4 @@ -use chrono::{NaiveDateTime, Utc}; +use chrono::{Duration, NaiveDateTime, Utc}; use serde_json::Value; use crate::crypto; @@ -63,8 +63,9 @@ enum UserStatus { #[derive(Serialize, Deserialize)] pub struct UserStampException { - pub route: String, + pub routes: Vec, pub security_stamp: String, + pub expire: i64, } /// Local methods @@ -135,9 +136,11 @@ impl User { /// # Arguments /// /// * `password` - A str which contains a hashed version of the users master password. - /// * `allow_next_route` - A Option<&str> with the function name of the next allowed (rocket) route. + /// * `allow_next_route` - A Option> with the function names of the next allowed (rocket) routes. + /// These routes are able to use the previous stamp id for the next 2 minutes. + /// After these 2 minutes this stamp will expire. /// - pub fn set_password(&mut self, password: &str, allow_next_route: Option<&str>) { + pub fn set_password(&mut self, password: &str, allow_next_route: Option>) { self.password_hash = crypto::hash_password(password.as_bytes(), &self.salt, self.password_iterations as u32); if let Some(route) = allow_next_route { @@ -154,24 +157,20 @@ impl User { /// Set the stamp_exception to only allow a subsequent request matching a specific route using the current security-stamp. /// /// # Arguments - /// * `route_exception` - A str with the function name of the next allowed (rocket) route. + /// * `route_exception` - A Vec with the function names of the next allowed (rocket) routes. + /// These routes are able to use the previous stamp id for the next 2 minutes. + /// After these 2 minutes this stamp will expire. /// - /// ### Future - /// In the future it could be posible that we need more of these exception routes. - /// In that case we could use an Vec and add multiple exceptions. - pub fn set_stamp_exception(&mut self, route_exception: &str) { + pub fn set_stamp_exception(&mut self, route_exception: Vec) { let stamp_exception = UserStampException { - route: route_exception.to_string(), + routes: route_exception, security_stamp: self.security_stamp.to_string(), + expire: (Utc::now().naive_utc() + Duration::minutes(2)).timestamp(), }; self.stamp_exception = Some(serde_json::to_string(&stamp_exception).unwrap_or_default()); } /// Resets the stamp_exception to prevent re-use of the previous security-stamp - /// - /// ### Future - /// In the future it could be posible that we need more of these exception routes. - /// In that case we could use an Vec and add multiple exceptions. pub fn reset_stamp_exception(&mut self) { self.stamp_exception = None; } diff --git a/src/db/schemas/mysql/schema.rs b/src/db/schemas/mysql/schema.rs index ad1ddbc1..149d2267 100644 --- a/src/db/schemas/mysql/schema.rs +++ b/src/db/schemas/mysql/schema.rs @@ -100,6 +100,8 @@ table! { uuid -> Text, name -> Text, billing_email -> Text, + private_key -> Nullable, + public_key -> Nullable, } } diff --git a/src/db/schemas/postgresql/schema.rs b/src/db/schemas/postgresql/schema.rs index 8964da89..8feb2eb2 100644 --- a/src/db/schemas/postgresql/schema.rs +++ b/src/db/schemas/postgresql/schema.rs @@ -100,6 +100,8 @@ table! { uuid -> Text, name -> Text, billing_email -> Text, + private_key -> Nullable, + public_key -> Nullable, } } diff --git a/src/db/schemas/sqlite/schema.rs b/src/db/schemas/sqlite/schema.rs index 8964da89..8feb2eb2 100644 --- a/src/db/schemas/sqlite/schema.rs +++ b/src/db/schemas/sqlite/schema.rs @@ -100,6 +100,8 @@ table! { uuid -> Text, name -> Text, billing_email -> Text, + private_key -> Nullable, + public_key -> Nullable, } } diff --git a/src/error.rs b/src/error.rs index f441f55f..0378d4ad 100644 --- a/src/error.rs +++ b/src/error.rs @@ -174,6 +174,9 @@ fn _api_error(_: &impl std::any::Any, msg: &str) -> String { "Message": msg, "Object": "error" }, + "ExceptionMessage": null, + "ExceptionStackTrace": null, + "InnerExceptionMessage": null, "Object": "error" }); _serialize(&json, "")