Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support SSH keys on desktop 2024.12 #5187

Merged
merged 5 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .env.template
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
111 changes: 87 additions & 24 deletions src/api/core/accounts.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::HashSet;

use crate::db::DbPool;
use chrono::Utc;
use rocket::serde::json::Json;
Expand Down Expand Up @@ -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::<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>")]
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.
Expand All @@ -494,30 +550,44 @@ async fn post_rotatekey(data: Json<KeyData>, 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?
}
}

// 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"),
};
Expand All @@ -528,47 +598,40 @@ async fn post_rotatekey(data: Json<KeyData>, 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?
}

// 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
use super::ciphers::update_cipher_from_data;

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?
}
}

Expand Down
26 changes: 23 additions & 3 deletions src/api/core/ciphers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -104,11 +105,27 @@ struct SyncData {
}

#[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;

// 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;

Expand Down Expand Up @@ -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,
Expand All @@ -228,6 +246,7 @@ pub struct CipherData {
secure_note: Option<Value>,
card: Option<Value>,
identity: Option<Value>,
ssh_key: Option<Value>,

favorite: Option<bool>,
reprompt: Option<i32>,
Expand Down Expand Up @@ -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"),
};

Expand Down
18 changes: 10 additions & 8 deletions src/api/core/organizations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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/<org_id>/export")]
async fn get_org_export(org_id: &str, headers: AdminHeaders, mut conn: DbConn) -> Json<Value> {
use semver::{Version, VersionReq};

async fn get_org_export(
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.
// 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
};
Expand Down
24 changes: 21 additions & 3 deletions src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,6 @@ pub struct AdminHeaders {
pub device: Device,
pub user: User,
pub org_user_type: UserOrgType,
pub client_version: Option<String>,
pub ip: ClientIp,
}

Expand All @@ -625,14 +624,12 @@ impl<'r> FromRequest<'r> for AdminHeaders {

async fn from_request(request: &'r Request<'_>) -> Outcome<Self, Self::Error> {
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 {
Expand Down Expand Up @@ -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")
dani-garcia marked this conversation as resolved.
Show resolved Hide resolved
};

Outcome::Success(ClientVersion(version))
}
}
11 changes: 9 additions & 2 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
dani-garcia marked this conversation as resolved.
Show resolved Hide resolved
"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() {
Expand Down
5 changes: 4 additions & 1 deletion src/db/models/cipher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ db_object! {
Login = 1,
SecureNote = 2,
Card = 3,
Identity = 4
Identity = 4,
SshKey = 5
*/
pub atype: i32,
pub name: String,
Expand Down Expand Up @@ -319,6 +320,7 @@ impl Cipher {
"secureNote": null,
"card": null,
"identity": null,
"sshKey": null,
});

// These values are only needed for user/default syncs
Expand Down Expand Up @@ -347,6 +349,7 @@ impl Cipher {
2 => "secureNote",
3 => "card",
4 => "identity",
5 => "sshKey",
_ => panic!("Wrong type"),
};

Expand Down
Loading