Support SSH keys on desktop 2024.12 (#5187)

* Support SSH keys on desktop 2024.12

* Document flags in .env.template

* Validate key rotation contents
This commit is contained in:
Daniel García 2024-11-15 18:38:16 +01:00 committed by GitHub
parent 0d16b38a68
commit 2393c3f3c0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 156 additions and 41 deletions

View file

@ -350,6 +350,8 @@
## - "browser-fileless-import": Directly import credentials from other providers without a file. ## - "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) ## - "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. ## - "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 # EXPERIMENTAL_CLIENT_FEATURE_FLAGS=fido2-vault-credentials
## Require new device emails. When a user logs in an email is required to be sent. ## Require new device emails. When a user logs in an email is required to be sent.

View file

@ -1,3 +1,5 @@
use std::collections::HashSet;
use crate::db::DbPool; use crate::db::DbPool;
use chrono::Utc; use chrono::Utc;
use rocket::serde::json::Json; use rocket::serde::json::Json;
@ -477,6 +479,60 @@ struct KeyData {
private_key: String, 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::<HashSet<_>>();
let provided_cipher_ids = data
.ciphers
.iter()
.filter(|c| c.organization_id.is_none())
.filter_map(|c| c.id.as_deref())
.collect::<HashSet<_>>();
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::<HashSet<_>>();
let provided_folder_ids = data.folders.iter().filter_map(|f| f.id.as_deref()).collect::<HashSet<_>>();
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::<HashSet<_>>();
let provided_emergency_access_ids =
data.emergency_access_keys.iter().map(|ea| ea.id.as_str()).collect::<HashSet<_>>();
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::<HashSet<_>>();
let provided_reset_password_ids =
data.reset_password_keys.iter().map(|rp| rp.organization_id.as_str()).collect::<HashSet<_>>();
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::<HashSet<_>>();
let provided_send_ids = data.sends.iter().filter_map(|s| s.id.as_deref()).collect::<HashSet<_>>();
if !provided_send_ids.is_superset(&existing_send_ids) {
err!("All existing sends must be included in the rotation")
}
Ok(())
}
#[post("/accounts/key", data = "<data>")] #[post("/accounts/key", data = "<data>")]
async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult { async fn post_rotatekey(data: Json<KeyData>, 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. // 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<KeyData>, headers: Headers, mut conn: DbConn,
let user_uuid = &headers.user.uuid; 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 // Update folder data
for folder_data in data.folders { for folder_data in data.folders {
// Skip `null` folder id entries. // Skip `null` folder id entries.
// See: https://github.com/bitwarden/clients/issues/8453 // See: https://github.com/bitwarden/clients/issues/8453
if let Some(folder_id) = folder_data.id { 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, Some(folder) => folder,
None => err!("Folder doesn't exist"), 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.name = folder_data.name;
saved_folder.save(&mut conn).await? saved_folder.save(&mut conn).await?
} }
@ -515,9 +586,8 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn,
// Update emergency access data // Update emergency access data
for emergency_access_data in data.emergency_access_keys { for emergency_access_data in data.emergency_access_keys {
let mut saved_emergency_access = let saved_emergency_access =
match EmergencyAccess::find_by_uuid_and_grantor_uuid(&emergency_access_data.id, user_uuid, &mut conn).await match existing_emergency_access.iter_mut().find(|ea| ea.uuid == emergency_access_data.id) {
{
Some(emergency_access) => emergency_access, Some(emergency_access) => emergency_access,
None => err!("Emergency access doesn't exist or is not owned by the user"), None => err!("Emergency access doesn't exist or is not owned by the user"),
}; };
@ -528,13 +598,11 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn,
// Update reset password data // Update reset password data
for reset_password_data in data.reset_password_keys { for reset_password_data in data.reset_password_keys {
let mut user_org = let user_org = match existing_user_orgs.iter_mut().find(|uo| uo.org_uuid == reset_password_data.organization_id)
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"),
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.reset_password_key = Some(reset_password_data.reset_password_key);
user_org.save(&mut conn).await? user_org.save(&mut conn).await?
@ -542,12 +610,12 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn,
// Update send data // Update send data
for send_data in data.sends { 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, Some(send) => send,
None => err!("Send doesn't exist"), 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 // Update cipher data
@ -555,20 +623,15 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn,
for cipher_data in data.ciphers { for cipher_data in data.ciphers {
if cipher_data.organization_id.is_none() { 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, Some(cipher) => cipher,
None => err!("Cipher doesn't exist"), 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 // 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. // 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. // 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) update_cipher_from_data(saved_cipher, cipher_data, &headers, None, &mut conn, &nt, UpdateType::None).await?
.await?
} }
} }

View file

@ -10,6 +10,7 @@ use rocket::{
}; };
use serde_json::Value; use serde_json::Value;
use crate::auth::ClientVersion;
use crate::util::NumberOrString; use crate::util::NumberOrString;
use crate::{ use crate::{
api::{self, core::log_event, EmptyResult, JsonResult, Notify, PasswordOrOtpData, UpdateType}, api::{self, core::log_event, EmptyResult, JsonResult, Notify, PasswordOrOtpData, UpdateType},
@ -104,11 +105,27 @@ struct SyncData {
} }
#[get("/sync?<data..>")] #[get("/sync?<data..>")]
async fn sync(data: SyncData, headers: Headers, mut conn: DbConn) -> Json<Value> { async fn sync(
data: SyncData,
headers: Headers,
client_version: Option<ClientVersion>,
mut conn: DbConn,
) -> Json<Value> {
let user_json = headers.user.to_json(&mut conn).await; let user_json = headers.user.to_json(&mut conn).await;
// Get all ciphers which are visible by the user // 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; let cipher_sync_data = CipherSyncData::new(&headers.user.uuid, CipherSyncType::User, &mut conn).await;
@ -216,7 +233,8 @@ pub struct CipherData {
Login = 1, Login = 1,
SecureNote = 2, SecureNote = 2,
Card = 3, Card = 3,
Identity = 4 Identity = 4,
SshKey = 5
*/ */
pub r#type: i32, pub r#type: i32,
pub name: String, pub name: String,
@ -228,6 +246,7 @@ pub struct CipherData {
secure_note: Option<Value>, secure_note: Option<Value>,
card: Option<Value>, card: Option<Value>,
identity: Option<Value>, identity: Option<Value>,
ssh_key: Option<Value>,
favorite: Option<bool>, favorite: Option<bool>,
reprompt: Option<i32>, reprompt: Option<i32>,
@ -469,6 +488,7 @@ pub async fn update_cipher_from_data(
2 => data.secure_note, 2 => data.secure_note,
3 => data.card, 3 => data.card,
4 => data.identity, 4 => data.identity,
5 => data.ssh_key,
_ => err!("Invalid type"), _ => err!("Invalid type"),
}; };

View file

@ -9,7 +9,7 @@ use crate::{
core::{log_event, two_factor, CipherSyncData, CipherSyncType}, core::{log_event, two_factor, CipherSyncData, CipherSyncType},
EmptyResult, JsonResult, Notify, PasswordOrOtpData, UpdateType, 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}, db::{models::*, DbConn},
error::Error, error::Error,
mail, 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. // 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. // Else the export will be just an empty JSON file.
#[get("/organizations/<org_id>/export")] #[get("/organizations/<org_id>/export")]
async fn get_org_export(org_id: &str, headers: AdminHeaders, mut conn: DbConn) -> Json<Value> { async fn get_org_export(
use semver::{Version, VersionReq}; org_id: &str,
headers: AdminHeaders,
client_version: Option<ClientVersion>,
mut conn: DbConn,
) -> Json<Value> {
// Since version v2023.1.0 the format of the export is different. // Since version v2023.1.0 the format of the export is different.
// Also, this endpoint was created since v2022.9.0. // 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. // 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. // 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 // 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 use_list_response_model = if let Some(client_version) = client_version {
let ver_match = VersionReq::parse("<2023.1.0").unwrap(); let ver_match = semver::VersionReq::parse("<2023.1.0").unwrap();
let client_version = Version::parse(&client_version).unwrap(); ver_match.matches(&client_version.0)
ver_match.matches(&client_version)
} else { } else {
false false
}; };

View file

@ -615,7 +615,6 @@ pub struct AdminHeaders {
pub device: Device, pub device: Device,
pub user: User, pub user: User,
pub org_user_type: UserOrgType, pub org_user_type: UserOrgType,
pub client_version: Option<String>,
pub ip: ClientIp, pub ip: ClientIp,
} }
@ -625,14 +624,12 @@ impl<'r> FromRequest<'r> for AdminHeaders {
async fn from_request(request: &'r Request<'_>) -> Outcome<Self, Self::Error> { async fn from_request(request: &'r Request<'_>) -> Outcome<Self, Self::Error> {
let headers = try_outcome!(OrgHeaders::from_request(request).await); 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 { if headers.org_user_type >= UserOrgType::Admin {
Outcome::Success(Self { Outcome::Success(Self {
host: headers.host, host: headers.host,
device: headers.device, device: headers.device,
user: headers.user, user: headers.user,
org_user_type: headers.org_user_type, org_user_type: headers.org_user_type,
client_version,
ip: headers.ip, ip: headers.ip,
}) })
} else { } 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<Self, Self::Error> {
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))
}
}

View file

@ -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 // TODO: deal with deprecated flags so they can be removed from this list, cf. #4263
const KNOWN_FLAGS: &[&str] = const KNOWN_FLAGS: &[&str] = &[
&["autofill-overlay", "autofill-v2", "browser-fileless-import", "extension-refresh", "fido2-vault-credentials"]; "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 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(); let invalid_flags: Vec<_> = configured_flags.keys().filter(|flag| !KNOWN_FLAGS.contains(&flag.as_str())).collect();
if !invalid_flags.is_empty() { if !invalid_flags.is_empty() {

View file

@ -30,7 +30,8 @@ db_object! {
Login = 1, Login = 1,
SecureNote = 2, SecureNote = 2,
Card = 3, Card = 3,
Identity = 4 Identity = 4,
SshKey = 5
*/ */
pub atype: i32, pub atype: i32,
pub name: String, pub name: String,
@ -319,6 +320,7 @@ impl Cipher {
"secureNote": null, "secureNote": null,
"card": null, "card": null,
"identity": null, "identity": null,
"sshKey": null,
}); });
// These values are only needed for user/default syncs // These values are only needed for user/default syncs
@ -347,6 +349,7 @@ impl Cipher {
2 => "secureNote", 2 => "secureNote",
3 => "card", 3 => "card",
4 => "identity", 4 => "identity",
5 => "sshKey",
_ => panic!("Wrong type"), _ => panic!("Wrong type"),
}; };