From 2393c3f3c08f451a04643fd9fed5027f491dc12d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Fri, 15 Nov 2024 18:38:16 +0100 Subject: [PATCH] Support SSH keys on desktop 2024.12 (#5187) * Support SSH keys on desktop 2024.12 * Document flags in .env.template * Validate key rotation contents --- .env.template | 2 + src/api/core/accounts.rs | 111 ++++++++++++++++++++++++++-------- src/api/core/ciphers.rs | 26 +++++++- src/api/core/organizations.rs | 18 +++--- src/auth.rs | 24 +++++++- src/config.rs | 11 +++- src/db/models/cipher.rs | 5 +- 7 files changed, 156 insertions(+), 41 deletions(-) diff --git a/.env.template b/.env.template index 075689e9..62ce5258 100644 --- a/.env.template +++ b/.env.template @@ -350,6 +350,8 @@ ## - "browser-fileless-import": Directly import credentials from other providers without a file. ## - "extension-refresh": Temporarily enable the new extension design until general availability (should be used with the beta Chrome extension) ## - "fido2-vault-credentials": Enable the use of FIDO2 security keys as second factor. +## - "ssh-key-vault-item": Enable the creation and use of SSH key vault items. (Needs clients >=2024.12.0) +## - "ssh-agent": Enable SSH agent support on Desktop. (Needs desktop >=2024.12.0) # EXPERIMENTAL_CLIENT_FEATURE_FLAGS=fido2-vault-credentials ## Require new device emails. When a user logs in an email is required to be sent. diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 1d3bcf37..87e44529 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use crate::db::DbPool; use chrono::Utc; use rocket::serde::json::Json; @@ -477,6 +479,60 @@ struct KeyData { private_key: String, } +fn validate_keydata( + data: &KeyData, + existing_ciphers: &[Cipher], + existing_folders: &[Folder], + existing_emergency_access: &[EmergencyAccess], + existing_user_orgs: &[UserOrganization], + existing_sends: &[Send], +) -> EmptyResult { + // Check that we're correctly rotating all the user's ciphers + let existing_cipher_ids = existing_ciphers.iter().map(|c| c.uuid.as_str()).collect::>(); + let provided_cipher_ids = data + .ciphers + .iter() + .filter(|c| c.organization_id.is_none()) + .filter_map(|c| c.id.as_deref()) + .collect::>(); + if !provided_cipher_ids.is_superset(&existing_cipher_ids) { + err!("All existing ciphers must be included in the rotation") + } + + // Check that we're correctly rotating all the user's folders + let existing_folder_ids = existing_folders.iter().map(|f| f.uuid.as_str()).collect::>(); + let provided_folder_ids = data.folders.iter().filter_map(|f| f.id.as_deref()).collect::>(); + if !provided_folder_ids.is_superset(&existing_folder_ids) { + err!("All existing folders must be included in the rotation") + } + + // Check that we're correctly rotating all the user's emergency access keys + let existing_emergency_access_ids = + existing_emergency_access.iter().map(|ea| ea.uuid.as_str()).collect::>(); + let provided_emergency_access_ids = + data.emergency_access_keys.iter().map(|ea| ea.id.as_str()).collect::>(); + if !provided_emergency_access_ids.is_superset(&existing_emergency_access_ids) { + err!("All existing emergency access keys must be included in the rotation") + } + + // Check that we're correctly rotating all the user's reset password keys + let existing_reset_password_ids = existing_user_orgs.iter().map(|uo| uo.org_uuid.as_str()).collect::>(); + let provided_reset_password_ids = + data.reset_password_keys.iter().map(|rp| rp.organization_id.as_str()).collect::>(); + if !provided_reset_password_ids.is_superset(&existing_reset_password_ids) { + err!("All existing reset password keys must be included in the rotation") + } + + // Check that we're correctly rotating all the user's sends + let existing_send_ids = existing_sends.iter().map(|s| s.uuid.as_str()).collect::>(); + let provided_send_ids = data.sends.iter().filter_map(|s| s.id.as_deref()).collect::>(); + if !provided_send_ids.is_superset(&existing_send_ids) { + err!("All existing sends must be included in the rotation") + } + + Ok(()) +} + #[post("/accounts/key", data = "")] async fn post_rotatekey(data: Json, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult { // TODO: See if we can wrap everything within a SQL Transaction. If something fails it should revert everything. @@ -494,20 +550,35 @@ async fn post_rotatekey(data: Json, headers: Headers, mut conn: DbConn, let user_uuid = &headers.user.uuid; + // TODO: Ideally we'd do everything after this point in a single transaction. + + let mut existing_ciphers = Cipher::find_owned_by_user(user_uuid, &mut conn).await; + let mut existing_folders = Folder::find_by_user(user_uuid, &mut conn).await; + let mut existing_emergency_access = EmergencyAccess::find_all_by_grantor_uuid(user_uuid, &mut conn).await; + let mut existing_user_orgs = UserOrganization::find_by_user(user_uuid, &mut conn).await; + // We only rotate the reset password key if it is set. + existing_user_orgs.retain(|uo| uo.reset_password_key.is_some()); + let mut existing_sends = Send::find_by_user(user_uuid, &mut conn).await; + + validate_keydata( + &data, + &existing_ciphers, + &existing_folders, + &existing_emergency_access, + &existing_user_orgs, + &existing_sends, + )?; + // Update folder data for folder_data in data.folders { // Skip `null` folder id entries. // See: https://github.com/bitwarden/clients/issues/8453 if let Some(folder_id) = folder_data.id { - let mut saved_folder = match Folder::find_by_uuid(&folder_id, &mut conn).await { + let saved_folder = match existing_folders.iter_mut().find(|f| f.uuid == folder_id) { Some(folder) => folder, None => err!("Folder doesn't exist"), }; - if &saved_folder.user_uuid != user_uuid { - err!("The folder is not owned by the user") - } - saved_folder.name = folder_data.name; saved_folder.save(&mut conn).await? } @@ -515,9 +586,8 @@ async fn post_rotatekey(data: Json, headers: Headers, mut conn: DbConn, // Update emergency access data for emergency_access_data in data.emergency_access_keys { - let mut saved_emergency_access = - match EmergencyAccess::find_by_uuid_and_grantor_uuid(&emergency_access_data.id, user_uuid, &mut conn).await - { + let saved_emergency_access = + match existing_emergency_access.iter_mut().find(|ea| ea.uuid == emergency_access_data.id) { Some(emergency_access) => emergency_access, None => err!("Emergency access doesn't exist or is not owned by the user"), }; @@ -528,13 +598,11 @@ async fn post_rotatekey(data: Json, headers: Headers, mut conn: DbConn, // Update reset password data for reset_password_data in data.reset_password_keys { - let mut user_org = - match UserOrganization::find_by_user_and_org(user_uuid, &reset_password_data.organization_id, &mut conn) - .await - { - Some(reset_password) => reset_password, - None => err!("Reset password doesn't exist"), - }; + let user_org = match existing_user_orgs.iter_mut().find(|uo| uo.org_uuid == reset_password_data.organization_id) + { + Some(reset_password) => reset_password, + None => err!("Reset password doesn't exist"), + }; user_org.reset_password_key = Some(reset_password_data.reset_password_key); user_org.save(&mut conn).await? @@ -542,12 +610,12 @@ async fn post_rotatekey(data: Json, headers: Headers, mut conn: DbConn, // Update send data for send_data in data.sends { - let mut send = match Send::find_by_uuid(send_data.id.as_ref().unwrap(), &mut conn).await { + let send = match existing_sends.iter_mut().find(|s| &s.uuid == send_data.id.as_ref().unwrap()) { Some(send) => send, None => err!("Send doesn't exist"), }; - update_send_from_data(&mut send, send_data, &headers, &mut conn, &nt, UpdateType::None).await?; + update_send_from_data(send, send_data, &headers, &mut conn, &nt, UpdateType::None).await?; } // Update cipher data @@ -555,20 +623,15 @@ async fn post_rotatekey(data: Json, headers: Headers, mut conn: DbConn, for cipher_data in data.ciphers { if cipher_data.organization_id.is_none() { - let mut saved_cipher = match Cipher::find_by_uuid(cipher_data.id.as_ref().unwrap(), &mut conn).await { + let saved_cipher = match existing_ciphers.iter_mut().find(|c| &c.uuid == cipher_data.id.as_ref().unwrap()) { Some(cipher) => cipher, None => err!("Cipher doesn't exist"), }; - if saved_cipher.user_uuid.as_ref().unwrap() != user_uuid { - err!("The cipher is not owned by the user") - } - // 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. // We force the users to logout after the user has been saved to try and prevent these issues. - update_cipher_from_data(&mut saved_cipher, cipher_data, &headers, None, &mut conn, &nt, UpdateType::None) - .await? + update_cipher_from_data(saved_cipher, cipher_data, &headers, None, &mut conn, &nt, UpdateType::None).await? } } diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 94bdc485..57d069b8 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -10,6 +10,7 @@ use rocket::{ }; use serde_json::Value; +use crate::auth::ClientVersion; use crate::util::NumberOrString; use crate::{ api::{self, core::log_event, EmptyResult, JsonResult, Notify, PasswordOrOtpData, UpdateType}, @@ -104,11 +105,27 @@ struct SyncData { } #[get("/sync?")] -async fn sync(data: SyncData, headers: Headers, mut conn: DbConn) -> Json { +async fn sync( + data: SyncData, + headers: Headers, + client_version: Option, + mut conn: DbConn, +) -> Json { let user_json = headers.user.to_json(&mut conn).await; // Get all ciphers which are visible by the user - let ciphers = Cipher::find_by_user_visible(&headers.user.uuid, &mut conn).await; + let mut ciphers = Cipher::find_by_user_visible(&headers.user.uuid, &mut conn).await; + + // Filter out SSH keys if the client version is less than 2024.12.0 + let show_ssh_keys = if let Some(client_version) = client_version { + let ver_match = semver::VersionReq::parse(">=2024.12.0").unwrap(); + ver_match.matches(&client_version.0) + } else { + false + }; + if !show_ssh_keys { + ciphers.retain(|c| c.atype != 5); + } let cipher_sync_data = CipherSyncData::new(&headers.user.uuid, CipherSyncType::User, &mut conn).await; @@ -216,7 +233,8 @@ pub struct CipherData { Login = 1, SecureNote = 2, Card = 3, - Identity = 4 + Identity = 4, + SshKey = 5 */ pub r#type: i32, pub name: String, @@ -228,6 +246,7 @@ pub struct CipherData { secure_note: Option, card: Option, identity: Option, + ssh_key: Option, favorite: Option, reprompt: Option, @@ -469,6 +488,7 @@ pub async fn update_cipher_from_data( 2 => data.secure_note, 3 => data.card, 4 => data.identity, + 5 => data.ssh_key, _ => err!("Invalid type"), }; diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 96ff9805..7ee6a089 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -9,7 +9,7 @@ use crate::{ core::{log_event, two_factor, CipherSyncData, CipherSyncType}, EmptyResult, JsonResult, Notify, PasswordOrOtpData, UpdateType, }, - auth::{decode_invite, AdminHeaders, Headers, ManagerHeaders, ManagerHeadersLoose, OwnerHeaders}, + auth::{decode_invite, AdminHeaders, ClientVersion, Headers, ManagerHeaders, ManagerHeadersLoose, OwnerHeaders}, db::{models::*, DbConn}, error::Error, mail, @@ -2999,18 +2999,20 @@ async fn put_reset_password_enrollment( // We need to convert all keys so they have the first character to be a lowercase. // Else the export will be just an empty JSON file. #[get("/organizations//export")] -async fn get_org_export(org_id: &str, headers: AdminHeaders, mut conn: DbConn) -> Json { - use semver::{Version, VersionReq}; - +async fn get_org_export( + org_id: &str, + headers: AdminHeaders, + client_version: Option, + mut conn: DbConn, +) -> Json { // Since version v2023.1.0 the format of the export is different. // Also, this endpoint was created since v2022.9.0. // Therefore, we will check for any version smaller then v2023.1.0 and return a different response. // If we can't determine the version, we will use the latest default v2023.1.0 and higher. // https://github.com/bitwarden/server/blob/9ca93381ce416454734418c3a9f99ab49747f1b6/src/Api/Controllers/OrganizationExportController.cs#L44 - let use_list_response_model = if let Some(client_version) = headers.client_version { - let ver_match = VersionReq::parse("<2023.1.0").unwrap(); - let client_version = Version::parse(&client_version).unwrap(); - ver_match.matches(&client_version) + let use_list_response_model = if let Some(client_version) = client_version { + let ver_match = semver::VersionReq::parse("<2023.1.0").unwrap(); + ver_match.matches(&client_version.0) } else { false }; diff --git a/src/auth.rs b/src/auth.rs index b1a743da..809ef9fd 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -615,7 +615,6 @@ pub struct AdminHeaders { pub device: Device, pub user: User, pub org_user_type: UserOrgType, - pub client_version: Option, pub ip: ClientIp, } @@ -625,14 +624,12 @@ impl<'r> FromRequest<'r> for AdminHeaders { async fn from_request(request: &'r Request<'_>) -> Outcome { let headers = try_outcome!(OrgHeaders::from_request(request).await); - let client_version = request.headers().get_one("Bitwarden-Client-Version").map(String::from); if headers.org_user_type >= UserOrgType::Admin { Outcome::Success(Self { host: headers.host, device: headers.device, user: headers.user, org_user_type: headers.org_user_type, - client_version, ip: headers.ip, }) } else { @@ -900,3 +897,24 @@ impl<'r> FromRequest<'r> for WsAccessTokenHeader { }) } } + +pub struct ClientVersion(pub semver::Version); + +#[rocket::async_trait] +impl<'r> FromRequest<'r> for ClientVersion { + type Error = &'static str; + + async fn from_request(request: &'r Request<'_>) -> Outcome { + let headers = request.headers(); + + let Some(version) = headers.get_one("Bitwarden-Client-Version") else { + err_handler!("No Bitwarden-Client-Version header provided") + }; + + let Ok(version) = semver::Version::parse(version) else { + err_handler!("Invalid Bitwarden-Client-Version header provided") + }; + + Outcome::Success(ClientVersion(version)) + } +} diff --git a/src/config.rs b/src/config.rs index 244499d0..e4e80927 100644 --- a/src/config.rs +++ b/src/config.rs @@ -811,8 +811,15 @@ fn validate_config(cfg: &ConfigItems) -> Result<(), Error> { } // TODO: deal with deprecated flags so they can be removed from this list, cf. #4263 - const KNOWN_FLAGS: &[&str] = - &["autofill-overlay", "autofill-v2", "browser-fileless-import", "extension-refresh", "fido2-vault-credentials"]; + const KNOWN_FLAGS: &[&str] = &[ + "autofill-overlay", + "autofill-v2", + "browser-fileless-import", + "extension-refresh", + "fido2-vault-credentials", + "ssh-key-vault-item", + "ssh-agent", + ]; let configured_flags = parse_experimental_client_feature_flags(&cfg.experimental_client_feature_flags); let invalid_flags: Vec<_> = configured_flags.keys().filter(|flag| !KNOWN_FLAGS.contains(&flag.as_str())).collect(); if !invalid_flags.is_empty() { diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index 9c568284..75d54df2 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -30,7 +30,8 @@ db_object! { Login = 1, SecureNote = 2, Card = 3, - Identity = 4 + Identity = 4, + SshKey = 5 */ pub atype: i32, pub name: String, @@ -319,6 +320,7 @@ impl Cipher { "secureNote": null, "card": null, "identity": null, + "sshKey": null, }); // These values are only needed for user/default syncs @@ -347,6 +349,7 @@ impl Cipher { 2 => "secureNote", 3 => "card", 4 => "identity", + 5 => "sshKey", _ => panic!("Wrong type"), };