diff --git a/.env.template b/.env.template index 43ed914f..f34ea249 100644 --- a/.env.template +++ b/.env.template @@ -338,6 +338,8 @@ ## "local" accepts it from any non global address, which covers a reverse proxy running on the same ## host or container network. Use "all" to accept it from anywhere, or list the addresses of your ## proxy as IPs and CIDR ranges if it connects from a public address. +## For a list header like X-Forwarded-For, the client IP is the rightmost address that isn't one of +## these proxies, so with several proxies in a row (a CDN in front of nginx, for example) list all of them. # IP_HEADER_TRUSTED_PROXIES=local ## Icon service diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 930107c9..b3499a93 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -42,6 +42,7 @@ pub fn routes() -> Vec { post_profile, put_avatar, get_public_keys, + get_keys, post_keys, post_password, post_set_password, @@ -568,6 +569,24 @@ async fn get_public_keys(user_id: UserId, _headers: Headers, conn: DbConn) -> Js }))) } +#[get("/accounts/keys")] +fn get_keys(headers: Headers) -> JsonResult { + let user = headers.user; + + // The SDK reads a 404 as the user having no key pair yet + if user.private_key.is_none() || user.public_key.is_none() { + err_code!("User has no key pair", Status::NotFound.code) + } + + Ok(Json(json!({ + "key": (!user.akey.is_empty()).then_some(&user.akey), + "publicKey": user.public_key, + "privateKey": user.private_key, + "accountKeys": user.account_keys_json(), + "object": "keys", + }))) +} + #[post("/accounts/keys", data = "")] async fn post_keys(data: Json, headers: Headers, conn: DbConn) -> JsonResult { let data: KeysData = data.into_inner(); @@ -994,7 +1013,8 @@ async fn post_rotatekey(data: Json, headers: Headers, conn: DbConn, nt: // Update cipher data for cipher_data in data.account_data.ciphers { if cipher_data.organization_id.is_none() { - let Some(saved_cipher) = existing_ciphers.iter_mut().find(|c| &c.uuid == cipher_data.id.as_ref().unwrap()) + let Some(saved_cipher) = + cipher_data.id.as_ref().and_then(|id| existing_ciphers.iter_mut().find(|c| &c.uuid == id)) else { err!("Cipher doesn't exist") }; @@ -1360,14 +1380,33 @@ pub struct PreloginData { email: String, } +/// Like upstream, an unknown email gets KDF settings picked from a list of common ones by a keyed hash of the email, +/// so they stay the same between requests. The salt stays null, as for existing users. +fn unknown_email_kdf(email: &str) -> (i32, i32, Option, Option) { + const PBKDF2: i32 = UserKdfType::Pbkdf2 as i32; + const ARGON2ID: i32 = UserKdfType::Argon2id as i32; + // Same list as upstream, the default is in it twice to give it more weight + const KDF_SETTINGS: [(i32, i32, Option, Option); 6] = [ + (User::CLIENT_KDF_TYPE_DEFAULT, User::CLIENT_KDF_ITER_DEFAULT, None, None), + (User::CLIENT_KDF_TYPE_DEFAULT, User::CLIENT_KDF_ITER_DEFAULT, None, None), + (PBKDF2, 100_000, None, None), + (PBKDF2, 5_000, None, None), + (ARGON2ID, 3, Some(64), Some(4)), + (ARGON2ID, 6, Some(32), Some(4)), + ]; + KDF_SETTINGS[crate::auth::prelogin_kdf_index(email, KDF_SETTINGS.len())] +} + pub async fn prelogin(data: Json, ip: ClientIp, conn: DbConn) -> JsonResult { crate::ratelimit::check_limit_unauthenticated(&ip.ip)?; let data: PreloginData = data.into_inner(); + // Normalized once, so the lookup and the settings for an unknown email use the same value + let email = data.email.trim().to_lowercase(); - let (kdf_type, kdf_iter, kdf_mem, kdf_para) = match User::find_by_mail(&data.email, &conn).await { + let (kdf_type, kdf_iter, kdf_mem, kdf_para) = match User::find_by_mail(&email, &conn).await { Some(user) => (user.client_kdf_type, user.client_kdf_iter, user.client_kdf_memory, user.client_kdf_parallelism), - None => (User::CLIENT_KDF_TYPE_DEFAULT, User::CLIENT_KDF_ITER_DEFAULT, None, None), + None => unknown_email_kdf(&email), }; Ok(Json(json!({ diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 5010bd92..46766896 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -29,7 +29,7 @@ use crate::{ util::{NumberOrString, deser_opt_nonempty_str, save_temp_file}, }; -use super::folders::FolderData; +use super::{folders::FolderData, sends::SIZE_525_MB}; pub fn routes() -> Vec { // Note that many routes have an `admin` variant; this seems to be @@ -441,10 +441,10 @@ pub async fn update_cipher_from_data( // We do not mind which data is in it, the keep our model more flexible when there are upstream changes. // But, we at least know we do not need to store and return this specific key. fn clean_cipher_data(mut json_data: Value) -> Value { - if json_data.is_array() { - json_data.as_array_mut().unwrap().iter_mut().for_each(|ref mut f| { - f.as_object_mut().unwrap().remove("response"); - }); + if let Some(items) = json_data.as_array_mut() { + for item in items.iter_mut().filter_map(Value::as_object_mut) { + item.remove("response"); + } } json_data } @@ -486,9 +486,10 @@ pub async fn update_cipher_from_data( Some(member) => { // A non-empty list of collections implies the caller already validated the user's write // access to them, so we can move the cipher into the organization on that basis. + // Write access to the cipher itself only counts when it already belongs to this organization. if shared_to_collections.as_ref().is_some_and(|cols| !cols.is_empty()) || member.has_full_access() - || cipher.is_write_accessible_to_user(&headers.user.uuid, conn).await + || (!transfer_cipher && cipher.is_write_accessible_to_user(&headers.user.uuid, conn).await) { cipher.organization_uuid = Some(org_id); // After some discussion in PR #1329 re-added the user_uuid = None again. @@ -552,7 +553,9 @@ pub async fn update_cipher_from_data( let type_data = if let Some(mut data) = type_data_opt { // Remove the 'Response' key from the base object. - data.as_object_mut().unwrap().remove("response"); + if let Some(data_obj) = data.as_object_mut() { + data_obj.remove("response"); + } // Remove the 'Response' key from every Uri. if data["uris"].is_array() { data["uris"] = clean_cipher_data(data["uris"].clone()); @@ -1182,6 +1185,10 @@ async fn post_attachment_v2( if file_size < 0 { err!("Attachment size can't be negative") } + // Matches the "file" upload limit in main.rs, nothing larger can be uploaded + if file_size > SIZE_525_MB { + err!("Max file size is 525 MB.") + } let attachment_id = crypto::generate_attachment_id(); let attachment = Attachment::new(attachment_id.clone(), cipher.uuid.clone(), data.file_name, file_size, Some(data.key)); diff --git a/src/api/core/emergency_access.rs b/src/api/core/emergency_access.rs index 2eb95502..6afc683c 100644 --- a/src/api/core/emergency_access.rs +++ b/src/api/core/emergency_access.rs @@ -225,7 +225,7 @@ async fn send_invite(data: Json, headers: Headers, co let (grantee_user, new_user) = match User::find_by_mail(&email, &conn).await { None => { if !CONFIG.invitations_allowed() { - err!(format!("Grantee user does not exist: {email}")) + err!(format!("Grantee user does not exist: {}", email.escape_debug())) } if !CONFIG.is_email_domain_allowed(&email) { diff --git a/src/api/core/events.rs b/src/api/core/events.rs index a5b5b6b1..80906e40 100644 --- a/src/api/core/events.rs +++ b/src/api/core/events.rs @@ -6,12 +6,13 @@ use serde_json::Value; use crate::{ CONFIG, - api::{EmptyResult, JsonResult}, + api::{ApiResult, EmptyResult, JsonResult}, auth::{AdminHeaders, Headers}, db::{ DbConn, DbPool, models::{Cipher, CipherId, Event, EventType, Membership, MembershipId, OrganizationId, UserId}, }, + error::MapResult, util::parse_date, }; @@ -29,6 +30,14 @@ struct EventRange { continuation_token: Option, } +impl EventRange { + fn date_range(&self) -> ApiResult<(NaiveDateTime, NaiveDateTime)> { + let start_date = parse_date(&self.start).map_res("Invalid start date")?; + let end_date = parse_date(self.continuation_token.as_ref().unwrap_or(&self.end)).map_res("Invalid end date")?; + Ok((start_date, end_date)) + } +} + // Upstream: https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Api/AdminConsole/Controllers/EventsController.cs#L87 #[get("/organizations//events?")] async fn get_org_events(org_id: OrganizationId, data: EventRange, headers: AdminHeaders, conn: DbConn) -> JsonResult { @@ -39,12 +48,7 @@ async fn get_org_events(org_id: OrganizationId, data: EventRange, headers: Admin // Return an empty vec when we org events are disabled. // This prevents client errors let events_json: Vec = if CONFIG.org_events_enabled() { - let start_date = parse_date(&data.start); - let end_date = if let Some(before_date) = &data.continuation_token { - parse_date(before_date) - } else { - parse_date(&data.end) - }; + let (start_date, end_date) = data.date_range()?; Event::find_by_organization_uuid(&org_id, &start_date, &end_date, &conn) .await @@ -69,12 +73,7 @@ async fn get_cipher_events(cipher_id: CipherId, data: EventRange, headers: Heade let events_json: Vec = if CONFIG.org_events_enabled() && Membership::user_has_ge_admin_access_to_cipher(&headers.user.uuid, &cipher_id, &conn).await { - let start_date = parse_date(&data.start); - let end_date = if let Some(before_date) = &data.continuation_token { - parse_date(before_date) - } else { - parse_date(&data.end) - }; + let (start_date, end_date) = data.date_range()?; Event::find_by_cipher_uuid(&cipher_id, &start_date, &end_date, &conn).await.iter().map(Event::to_json).collect() } else { @@ -102,12 +101,7 @@ async fn get_user_events( // Return an empty vec when we org events are disabled. // This prevents client errors let events_json: Vec = if CONFIG.org_events_enabled() { - let start_date = parse_date(&data.start); - let end_date = if let Some(before_date) = &data.continuation_token { - parse_date(before_date) - } else { - parse_date(&data.end) - }; + let (start_date, end_date) = data.date_range()?; Event::find_by_org_and_member(&org_id, &member_id, &start_date, &end_date, &conn) .await @@ -168,9 +162,10 @@ async fn post_events_collect(data: Json>, headers: Headers, } for event in data.iter() { - let event_date = parse_date(&event.date); + let event_date = parse_date(&event.date).map_res("Invalid event date")?; + // Only the event types upstream accepts from clients, the rest are written server-side. match event.r#type { - 1000..=1099 => { + t if t == EventType::UserClientExportedVault as i32 => { log_user_event_impl( event.r#type, &headers.user.uuid, @@ -181,7 +176,7 @@ async fn post_events_collect(data: Json>, headers: Headers, ) .await; } - 1600..=1699 => { + t if t == EventType::OrganizationClientExportedVault as i32 => { // Only allow logging events for an organization the user is actually a member of. if let Some(org_id) = &event.organization_id && Membership::find_confirmed_by_user_and_org(&headers.user.uuid, org_id, &conn).await.is_some() @@ -219,7 +214,8 @@ async fn post_events_collect(data: Json>, headers: Headers, .await; } } - _ => { + // Client-side cipher events (viewed, copied, toggled, autofilled). + 1107..=1114 | 1117 | 1119..=1132 => { // The cipher determines the organization the event is logged to, so make sure the // user can actually access it instead of trusting the provided cipher uuid. if let Some(cipher_uuid) = &event.cipher_id @@ -240,6 +236,7 @@ async fn post_events_collect(data: Json>, headers: Headers, .await; } } + _ => {} } } Ok(()) diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index f64b8c53..6464f774 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -1028,7 +1028,7 @@ async fn send_invite( let user = match User::find_by_mail(email, &conn).await { None => { if !CONFIG.invitations_allowed() { - err!(format!("User does not exist: {email}")) + err!(format!("User does not exist: {}", email.escape_debug())) } if !CONFIG.is_email_domain_allowed(email) { @@ -1046,7 +1046,7 @@ async fn send_invite( } Some(user) => { if Membership::find_by_user_and_org(&user.uuid, &org_id, &conn).await.is_some() { - err!(format!("User already in organization: {email}")) + err!(format!("User already in organization: {}", email.escape_debug())) } if !CONFIG.mail_enabled() { @@ -1321,7 +1321,9 @@ async fn bulk_confirm_invite( match data.keys { Some(keys) => { for invite in keys { - let member_id = invite.id.unwrap(); + let Some(member_id) = invite.id else { + err!("Member id is required") + }; let user_key = invite.key.unwrap_or_default(); let err_msg = match confirm_invite_impl(&org_id, &member_id, &user_key, &headers, &conn, &nt).await { Ok(()) => String::new(), diff --git a/src/api/core/public.rs b/src/api/core/public.rs index 3db25df9..fbb1f855 100644 --- a/src/api/core/public.rs +++ b/src/api/core/public.rs @@ -58,25 +58,13 @@ async fn ldap_import(data: Json, token: PublicToken, conn: DbConn let org_id = token.0; let data = data.into_inner(); + // Like upstream, the import never removes Owners, so it doesn't revoke or restore them either. for user_data in &data.members { let mut user_created: bool = false; if user_data.deleted { // If user is marked for deletion and it exists, revoke it if let Some(mut member) = Membership::find_by_email_and_org(&user_data.email, &org_id, &conn).await { - // Only revoke a user if it is not the last confirmed owner - let revoked = if member.atype == MembershipType::Owner - && member.status == MembershipStatus::Confirmed as i32 - { - if Membership::count_confirmed_by_org_and_type(&org_id, MembershipType::Owner, &conn).await <= 1 { - warn!("Can't revoke the last owner"); - false - } else { - member.revoke() - } - } else { - member.revoke() - }; - + let revoked = member.atype != MembershipType::Owner && member.revoke(); let ext_modified = member.set_external_id(Some(user_data.external_id.clone())); if revoked || ext_modified { member.save(&conn).await?; @@ -84,7 +72,7 @@ async fn ldap_import(data: Json, token: PublicToken, conn: DbConn } // If user is part of the organization, restore it } else if let Some(mut member) = Membership::find_by_email_and_org(&user_data.email, &org_id, &conn).await { - let mut restored = member.restore(); + let mut restored = member.atype != MembershipType::Owner && member.restore(); let ext_modified = member.set_external_id(Some(user_data.external_id.clone())); // Enforce org policies as every other restore path does. // If the user is not allowed, we revoke again and continue so the external_id is still updated. @@ -180,14 +168,8 @@ async fn ldap_import(data: Json, token: PublicToken, conn: DbConn for member in Membership::find_by_org(&org_id, &conn).await { if let Some(ref user_external_id) = member.external_id && !sync_members.contains(user_external_id) + && member.atype != MembershipType::Owner { - if member.atype == MembershipType::Owner && member.status == MembershipStatus::Confirmed as i32 { - // Removing owner, check that there is at least one other confirmed owner - if Membership::count_confirmed_by_org_and_type(&org_id, MembershipType::Owner, &conn).await <= 1 { - warn!("Can't delete the last owner"); - continue; - } - } member.delete(&conn).await?; } } diff --git a/src/api/core/sends.rs b/src/api/core/sends.rs index bfff4e96..2a604b74 100644 --- a/src/api/core/sends.rs +++ b/src/api/core/sends.rs @@ -23,7 +23,7 @@ use crate::{ const SEND_INACCESSIBLE_MSG: &str = "Send does not exist or is no longer available"; static ANON_PUSH_DEVICE: LazyLock = LazyLock::new(|| { - let dt = crate::util::parse_date("1970-01-01T00:00:00.000000Z"); + let dt = DateTime::UNIX_EPOCH.naive_utc(); Device { uuid: String::from("00000000-0000-0000-0000-000000000000").into(), created_at: dt, @@ -39,7 +39,7 @@ static ANON_PUSH_DEVICE: LazyLock = LazyLock::new(|| { }); // The max file size allowed by Bitwarden clients and add an extra 5% to avoid issues -const SIZE_525_MB: i64 = 550_502_400; +pub(crate) const SIZE_525_MB: i64 = 550_502_400; pub fn routes() -> Vec { routes![ @@ -328,8 +328,8 @@ async fn post_send_file_v2_data( "Send file name does not match.", format!( "Expected file name '{}' got '{}'", - send_data.fileName, - raw_file_name.dangerous_unsafe_unsanitized_raw() + send_data.fileName.escape_debug(), + raw_file_name.dangerous_unsafe_unsanitized_raw().as_str().escape_debug() ) ), _ => err!("Send file name does not match or is not provided."), diff --git a/src/api/core/two_factor/yubikey.rs b/src/api/core/two_factor/yubikey.rs index eb3d6dfd..54f0adb5 100644 --- a/src/api/core/two_factor/yubikey.rs +++ b/src/api/core/two_factor/yubikey.rs @@ -217,7 +217,9 @@ pub async fn validate_yubikey_login(response: &str, twofactor_data: &str) -> Emp } let yubikey_metadata: YubikeyMetadata = serde_json::from_str(twofactor_data).expect("Can't parse Yubikey Metadata"); - let response_id = &response[..12]; + let Some(response_id) = response.get(..12) else { + err!("Invalid Yubikey OTP"); + }; if !yubikey_metadata.keys.contains(&response_id.to_owned()) { err!("Given Yubikey is not registered"); diff --git a/src/api/icons.rs b/src/api/icons.rs index 8f3e730b..05027959 100644 --- a/src/api/icons.rs +++ b/src/api/icons.rs @@ -85,7 +85,7 @@ static ICON_SIZE_REGEX: LazyLock = LazyLock::new(|| Regex::new(r"(?x)(\d+ #[get("//icon.png")] fn icon_external(host: &str) -> Cached> { let Ok(host) = get_valid_host(host) else { - warn!("Invalid host: {host}"); + warn!("Invalid host: {}", host.escape_debug()); return Cached::ttl(None, CONFIG.icon_cache_negttl(), true); }; @@ -113,7 +113,7 @@ async fn icon_internal(host: &str) -> Cached<(ContentType, Vec)> { const FALLBACK_ICON: &[u8] = include_bytes!("../static/images/fallback-icon.png"); let Ok(host) = get_valid_host(host) else { - warn!("Invalid host: {host}"); + warn!("Invalid host: {}", host.escape_debug()); return Cached::ttl( (ContentType::new("image", "png"), FALLBACK_ICON.to_vec()), CONFIG.icon_cache_negttl(), diff --git a/src/api/identity.rs b/src/api/identity.rs index 702a0d0e..ac05386a 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -20,7 +20,7 @@ use crate::{ authenticator, duo_oidc, email, enforce_2fa_policy, is_twofactor_provider_usable, webauthn, yubikey, }, }, - master_password_policy, + identity_master_password_policy, push::register_push_device, }, auth, @@ -83,10 +83,6 @@ async fn login(data: Form, client_header: ClientHeaders, conn: DbCo check_is_some(data.client_secret.as_ref(), "client_secret cannot be blank")?; check_is_some(data.scope.as_ref(), "scope cannot be blank")?; - check_is_some(data.device_identifier.as_ref(), "device_identifier cannot be blank")?; - check_is_some(data.device_name.as_ref(), "device_name cannot be blank")?; - check_is_some(data.device_type.as_ref(), "device_type cannot be blank")?; - api_key_login(data, &mut user_id, &conn, &client_header.ip).await } "authorization_code" if CONFIG.sso_enabled() => { @@ -408,6 +404,7 @@ async fn password_login(data: ConnectData, user_id: &mut Option, conn: & if auth_request.user_uuid != user.uuid || !auth_request.approved.unwrap_or(false) + || auth_request.authentication_date.is_some() || request_expired || ip.ip.to_string() != auth_request.request_ip || !auth_request.check_access_code(password) @@ -484,6 +481,19 @@ async fn password_login(data: ConnectData, user_id: &mut Option, conn: & let twofactor_token = twofactor_auth(&mut user, &data, &mut device, ip, conn).await?; + // Like upstream, an auth request can be used for one login only. Marked after 2FA so a 2FA prompt doesn't use it up. + if let Some(ref auth_request_id) = data.auth_request + && !AuthRequest::set_authentication_date(auth_request_id, conn).await? + { + err!( + "Username or access code is incorrect. Try again", + format!("IP: {}. Username: {log_username}.", ip.ip), + ErrorEvent { + event: EventType::UserFailedLogIn, + } + ) + } + let auth_tokens = auth::AuthTokens::new(&device, &user, AuthMethod::Password, data.client_id); authenticated_response(&user, &mut device, auth_tokens, twofactor_token, conn, ip).await @@ -521,7 +531,7 @@ async fn authenticated_response( // Save to update `device.updated_at` to track usage and toggle new status device.save(true, conn).await?; - let master_password_policy = master_password_policy(user, conn).await; + let master_password_policy = identity_master_password_policy(user, conn).await; let has_master_password = !user.password_hash.is_empty(); let master_password_unlock = if has_master_password { @@ -594,7 +604,14 @@ async fn api_key_login(data: ConnectData, user_id: &mut Option, conn: &D // Validate scope match data.scope.as_ref() { - Some(scope) if scope == &AuthMethod::UserApiKey.scope() => user_api_key_login(data, user_id, conn, ip).await, + Some(scope) if scope == &AuthMethod::UserApiKey.scope() => { + // Like upstream, only the user API key logs in a device + check_is_some(data.device_identifier.as_ref(), "device_identifier cannot be blank")?; + check_is_some(data.device_name.as_ref(), "device_name cannot be blank")?; + check_is_some(data.device_type.as_ref(), "device_type cannot be blank")?; + + user_api_key_login(data, user_id, conn, ip).await + } Some(scope) if scope == &AuthMethod::OrgApiKey.scope() => organization_api_key_login(data, conn, ip).await, _ => err!("Scope not supported"), } @@ -811,12 +828,12 @@ async fn twofactor_auth( if ![TwoFactorType::Remember as i32, TwoFactorType::RecoveryCode as i32].contains(&selected_id) && !twofactor_ids.contains(&selected_id) { - err_json!(json_err_twofactor(&twofactor_ids, &user.uuid, data, conn).await?, "Invalid two factor provider") + err_json!(json_err_twofactor(&twofactor_ids, user, data, conn).await?, "Invalid two factor provider") } // Like upstream, a blank token counts as not provided let Some(twofactor_code) = data.two_factor_token.as_deref().filter(|t| !t.trim().is_empty()) else { - err_json!(json_err_twofactor(&twofactor_ids, &user.uuid, data, conn).await?, "2FA token not provided") + err_json!(json_err_twofactor(&twofactor_ids, user, data, conn).await?, "2FA token not provided") }; let selected_twofactor = twofactors.into_iter().find(|tf| tf.atype == selected_id && tf.enabled); @@ -859,16 +876,22 @@ async fn twofactor_auth( device.save(true, conn).await?; } err_json!( - json_err_twofactor(&twofactor_ids, &user.uuid, data, conn).await?, + json_err_twofactor(&twofactor_ids, user, data, conn).await?, "2FA Remember token not provided or expired" ) } } } Some(TwoFactorType::RecoveryCode) => { - // Check if recovery code is correct - if !user.check_valid_recovery_code(twofactor_code) { - err!("Recovery code is incorrect. Try again.") + // Like upstream, spaces and case don't matter + let recovery_code = twofactor_code.replace(' ', "").trim().to_lowercase(); + if !user.check_valid_recovery_code(&recovery_code) { + err!( + "Two-step token is invalid. Try again.", + ErrorEvent { + event: EventType::UserFailedLogIn2fa + } + ) } // Remove all twofactors from the user @@ -883,6 +906,13 @@ async fn twofactor_auth( log_user_event(EventType::UserRecovered2fa as i32, &user.uuid, device.atype, &ip.ip, conn).await; + if CONFIG.mail_enabled() + && let Err(e) = + mail::send_recover_twofactor(&user.email, &ip.ip.to_string(), &Utc::now().naive_utc()).await + { + error!("Error sending two-step login recovered email: {e:#?}"); + } + // Remove the recovery code, not needed without twofactors user.totp_recover = None; user.save(conn).await?; @@ -910,20 +940,14 @@ fn selected_data(tf: Option) -> ApiResult { tf.map(|t| t.data).map_res("Two factor doesn't exist") } -async fn json_err_twofactor( - providers: &[i32], - user_id: &UserId, - data: &ConnectData, - conn: &DbConn, -) -> ApiResult { +async fn json_err_twofactor(providers: &[i32], user: &User, data: &ConnectData, conn: &DbConn) -> ApiResult { + let user_id = &user.uuid; let mut result = json!({ "error" : "invalid_grant", "error_description" : "Two factor required.", "TwoFactorProviders" : providers.iter().map(ToString::to_string).collect::>(), "TwoFactorProviders2" : {}, // { "0" : null } - "MasterPasswordPolicy": { - "Object": "masterPasswordPolicy" - } + "MasterPasswordPolicy": identity_master_password_policy(user, conn).await, }); for provider in providers { @@ -936,14 +960,8 @@ async fn json_err_twofactor( } Some(TwoFactorType::Duo) => { - let email = if let Some(u) = User::find_by_uuid(user_id, conn).await { - u.email - } else { - err!("User does not exist") - }; - let auth_url = duo_oidc::get_duo_auth_url( - &email, + &user.email, data.client_id.as_ref().unwrap(), data.device_identifier.as_ref().unwrap(), conn, @@ -1212,7 +1230,7 @@ async fn oidcsignin_redirect( let state = sso::decode_state(&base64_state)?; let Some(mut sso_auth) = SsoAuth::find(&state, conn).await else { - err!(format!("Cannot retrieve sso_auth for {state}")) + err!(format!("Cannot retrieve sso_auth for {}", state.escape_debug())) }; // Browser-binding check @@ -1221,7 +1239,7 @@ async fn oidcsignin_redirect( let provided_hash = cookie_value.as_deref().map(|v| crypto::sha256_hex(v.as_bytes())); match (sso_auth.binding_hash.as_deref(), provided_hash.as_deref()) { (Some(expected), Some(actual)) if crypto::ct_eq(expected, actual) => {} - _ => err!(format!("SSO session binding mismatch for {state}")), + _ => err!(format!("SSO session binding mismatch for {}", state.escape_debug())), } cookies .remove(Cookie::build(SSO_BINDING_COOKIE).path(format!("{}/identity/connect/", CONFIG.domain_path())).build()); diff --git a/src/api/mod.rs b/src/api/mod.rs index 9a79ce95..1b3b5a7b 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -124,3 +124,21 @@ async fn master_password_policy(user: &User, conn: &DbConn) -> Value { mpp_json["Object"] = json!("masterPasswordPolicy"); mpp_json } + +// Identity responses use PascalCase keys like upstream, some clients (Android) only read those +async fn identity_master_password_policy(user: &User, conn: &DbConn) -> Value { + match master_password_policy(user, conn).await { + Value::Object(policy) => Value::Object( + policy + .into_iter() + .map(|(mut key, value)| { + if let Some(first) = key.get_mut(..1) { + first.make_ascii_uppercase(); + } + (key, value) + }) + .collect(), + ), + policy => policy, + } +} diff --git a/src/auth.rs b/src/auth.rs index 39ce2732..cad9b298 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -14,6 +14,7 @@ use ipnet::IpNet; use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header, errors::ErrorKind}; use num_traits::FromPrimitive; use openssl::rsa::Rsa; +use ring::hmac; use serde::{de::DeserializeOwned, ser::Serialize}; use rocket::{ @@ -65,6 +66,7 @@ static JWT_2FA_REMEMBER_ISSUER: LazyLock = LazyLock::new(|| format!("{}| static PRIVATE_RSA_KEY: OnceLock = OnceLock::new(); static PUBLIC_RSA_KEY: OnceLock = OnceLock::new(); +static PRELOGIN_KDF_KEY: OnceLock = OnceLock::new(); pub async fn initialize_keys() -> Result<(), Error> { use std::io::Error as IoError; @@ -99,9 +101,22 @@ pub async fn initialize_keys() -> Result<(), Error> { if PUBLIC_RSA_KEY.set(dec).is_err() { err!("PUBLIC_RSA_KEY must only be initialized once") } + + // Derived from the RSA key, so it stays the same across restarts without storing another secret + let prelogin_kdf_key = hmac::sign(&hmac::Key::new(hmac::HMAC_SHA256, &priv_key_buffer), b"prelogin-kdf"); + if PRELOGIN_KDF_KEY.set(hmac::Key::new(hmac::HMAC_SHA256, prelogin_kdf_key.as_ref())).is_err() { + err!("PRELOGIN_KDF_KEY must only be initialized once") + } Ok(()) } +/// Returns an index in `0..len` derived from the email, the same for every request +pub fn prelogin_kdf_index(email: &str, len: usize) -> usize { + let tag = hmac::sign(PRELOGIN_KDF_KEY.wait(), email.as_bytes()); + let tag = tag.as_ref(); + usize::from(u16::from_be_bytes([tag[0], tag[1]])) % len +} + pub fn encode_jwt(claims: &T) -> String { match jsonwebtoken::encode(&JWT_HEADER, claims, PRIVATE_RSA_KEY.wait()) { Ok(token) => token, @@ -422,7 +437,7 @@ pub fn generate_file_download_claims(cipher_id: CipherId, file_id: AttachmentId) let time_now = Utc::now(); FileDownloadClaims { nbf: time_now.timestamp(), - exp: (time_now + TimeDelta::try_minutes(5).unwrap()).timestamp(), + exp: (time_now + TimeDelta::try_minutes(1).unwrap()).timestamp(), iss: JWT_FILE_DOWNLOAD_ISSUER.to_string(), sub: cipher_id, file_id, @@ -1091,6 +1106,31 @@ fn ip_header_is_trusted(remote: Option) -> bool { trusted.split(',').filter_map(parse_trusted_proxy).any(|net| net.contains(&remote)) } +/// Each proxy appends the address it received the request from to a list header (`X-Forwarded-For: client, proxy1`). +/// Like ASP.NET's ForwardedHeaders, walk the entries of every occurrence of the header from the right and take the +/// first one that isn't a trusted proxy. If they all are, the leftmost one is the client. +fn client_ip_from_header(req: &Request<'_>, header: &str, is_trusted: impl Fn(IpAddr) -> bool) -> Option { + let values: Vec<&str> = req.headers().get(header).collect(); + client_ip_from_header_values(&values, header, is_trusted) +} + +fn client_ip_from_header_values(values: &[&str], header: &str, is_trusted: impl Fn(IpAddr) -> bool) -> Option { + let entries: Vec<&str> = values.iter().flat_map(|value| value.split(',')).map(str::trim).collect(); + + let mut client_ip = None; + for entry in entries.into_iter().rev() { + let Ok(ip) = entry.parse() else { + warn!("'{header}' header is malformed: {entry}"); + break; + }; + client_ip = Some(ip); + if !is_trusted(ip) { + break; + } + } + client_ip +} + #[rocket::async_trait] impl<'r> FromRequest<'r> for ClientIp { type Error = (); @@ -1099,15 +1139,7 @@ impl<'r> FromRequest<'r> for ClientIp { let remote = req.remote().map(|r| r.ip()); let ip = if CONFIG._ip_header_enabled() && ip_header_is_trusted(remote) { - req.headers().get_one(&CONFIG.ip_header()).and_then(|ip| { - match ip.find(',') { - Some(idx) => &ip[..idx], - None => ip, - } - .parse() - .map_err(|_| warn!("'{}' header is malformed: {ip}", CONFIG.ip_header())) - .ok() - }) + client_ip_from_header(req, &CONFIG.ip_header(), |ip| ip_header_is_trusted(Some(ip))) } else { if CONFIG._ip_header_enabled() && req.headers().get_one(&CONFIG.ip_header()).is_some() { // Log the canonical IP, which is what the user filter will need to match against @@ -1226,7 +1258,7 @@ impl AuthMethod { match scope { None => err!("Missing scope"), Some(scope) if scope == &method_scope => Ok(method_scope), - Some(scope) => err!(format!("Scope ({scope}) not supported")), + Some(scope) => err!(format!("Scope ({}) not supported", scope.escape_debug())), } } } @@ -1345,3 +1377,52 @@ pub async fn refresh_tokens( Ok((device, auth_tokens)) } + +#[cfg(test)] +mod tests { + use super::*; + + fn client_ip(values: &[&str]) -> Option { + let is_trusted = |ip: IpAddr| match ip { + IpAddr::V4(v4) => v4.is_private() || v4.is_loopback(), + IpAddr::V6(v6) => v6.is_loopback(), + }; + client_ip_from_header_values(values, "X-Forwarded-For", is_trusted) + } + + fn ip(s: &str) -> Option { + Some(s.parse().unwrap()) + } + + #[test] + fn ip_header_single_value() { + assert_eq!(client_ip(&["203.0.113.5"]), ip("203.0.113.5")); + assert_eq!(client_ip(&["10.0.0.7"]), ip("10.0.0.7")); + assert_eq!(client_ip(&["2001:db8::1"]), ip("2001:db8::1")); + } + + #[test] + fn ip_header_uses_rightmost_untrusted_entry() { + // Entries already in the request come before the one the proxy appended + assert_eq!(client_ip(&["198.51.100.1, 203.0.113.5"]), ip("203.0.113.5")); + // Trusted proxies on the right are skipped + assert_eq!(client_ip(&["198.51.100.1, 203.0.113.5, 10.0.0.2, 127.0.0.1"]), ip("203.0.113.5")); + // All trusted, so the leftmost entry is the client + assert_eq!(client_ip(&["192.168.1.10, 10.0.0.2"]), ip("192.168.1.10")); + } + + #[test] + fn ip_header_reads_every_occurrence() { + assert_eq!(client_ip(&["198.51.100.1", "203.0.113.5"]), ip("203.0.113.5")); + assert_eq!(client_ip(&["198.51.100.1", "203.0.113.5, 10.0.0.2"]), ip("203.0.113.5")); + } + + #[test] + fn ip_header_malformed_entries() { + assert_eq!(client_ip(&[]), None); + assert_eq!(client_ip(&[""]), None); + assert_eq!(client_ip(&["203.0.113.5, not-an-ip"]), None); + // Stops at the malformed entry and keeps the closest proxy + assert_eq!(client_ip(&["not-an-ip, 10.0.0.2"]), ip("10.0.0.2")); + } +} diff --git a/src/config.rs b/src/config.rs index b03e006c..7d35461c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -675,7 +675,8 @@ make_config! { /// other address use the remote IP instead, so a client can't spoof the header. /// Either the string "local" (the default, any non-global address, which covers a reverse proxy /// running on the same host or container network), the string "all" to accept it from anywhere, - /// or a comma separated list of IPs and CIDR ranges. + /// or a comma separated list of IPs and CIDR ranges. For a list header like X-Forwarded-For, the + /// client IP is the rightmost address that isn't a trusted proxy, so list every proxy in the chain. ip_header_trusted_proxies: String, true, def, "local".to_owned(); /// Icon service |> The predefined icon services are: internal, bitwarden, duckduckgo, google. /// To specify a custom icon service, set a URL template with exactly one instance of `{}`, @@ -1726,6 +1727,7 @@ where reg!("email/protected_action", ".html"); reg!("email/pw_hint_none", ".html"); reg!("email/pw_hint_some", ".html"); + reg!("email/recover_twofactor", ".html"); reg!("email/register_verify_email", ".html"); reg!("email/send_2fa_removed_from_org", ".html"); reg!("email/send_emergency_access_invite", ".html"); diff --git a/src/db/models/attachment.rs b/src/db/models/attachment.rs index 0536dde5..659648d0 100644 --- a/src/db/models/attachment.rs +++ b/src/db/models/attachment.rs @@ -61,7 +61,7 @@ impl Attachment { let token = encode_jwt(&generate_file_download_claims(self.cipher_uuid.clone(), self.id.clone())); Ok(format!("{host}/attachments/{}/{}?token={token}", self.cipher_uuid, self.id)) } else { - Ok(operator.presign_read(&self.get_file_path(), Duration::from_mins(5)).await?.uri().to_string()) + Ok(operator.presign_read(&self.get_file_path(), Duration::from_mins(1)).await?.uri().to_string()) } } diff --git a/src/db/models/auth_request.rs b/src/db/models/auth_request.rs index cc4b60fd..b06c9e56 100644 --- a/src/db/models/auth_request.rs +++ b/src/db/models/auth_request.rs @@ -4,7 +4,7 @@ use diesel::prelude::*; use serde_json::Value; use crate::{ - api::EmptyResult, + api::{ApiResult, EmptyResult}, crypto::ct_eq, db::{DbConn, schema::auth_requests}, error::MapResult, @@ -166,6 +166,21 @@ impl AuthRequest { .await } + /// Marks the request as used to log in. Returns false when it already was, so it only works once. + pub async fn set_authentication_date(uuid: &AuthRequestId, conn: &DbConn) -> ApiResult { + let now = Utc::now().naive_utc(); + conn.run(move |conn| { + diesel::update(auth_requests::table) + .filter(auth_requests::uuid.eq(uuid)) + .filter(auth_requests::authentication_date.is_null()) + .set(auth_requests::authentication_date.eq(now)) + .execute(conn) + .map(|updated| updated == 1) + .map_res("Error updating auth request") + }) + .await + } + pub fn check_access_code(&self, access_code: &str) -> bool { ct_eq(&self.access_code, access_code) } diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 9816c579..72684782 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -342,7 +342,7 @@ impl OrganizationApiKey { impl Organization { pub async fn save(&self, conn: &DbConn) -> EmptyResult { if !crate::util::is_valid_email(&self.billing_email) { - err!(format!("BillingEmail {} is not a valid email address", self.billing_email)) + err!(format!("BillingEmail {} is not a valid email address", self.billing_email.escape_debug())) } for member in &Membership::find_by_org(&self.uuid, conn).await { diff --git a/src/db/models/user.rs b/src/db/models/user.rs index b4f507f4..ead01373 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -261,6 +261,25 @@ impl User { !CONFIG.mail_enabled() || self.verified_at.is_some() } + /// The `accountKeys` object (upstream's `PrivateKeysResponseModel`), null without a key pair + pub fn account_keys_json(&self) -> Value { + if self.private_key.is_some() { + json!({ + "publicKeyEncryptionKeyPair": { + "wrappedPrivateKey": self.private_key, + "publicKey": self.public_key, + "signedPublicKey": null, + "object": "publicKeyEncryptionKeyPair", + }, + "securityState": null, + "signatureKeyPair": null, + "object": "privateKeys" + }) + } else { + Value::Null + } + } + pub async fn to_json(&self, conn: &DbConn) -> Value { let mut orgs_json = Vec::new(); for c in Membership::find_confirmed_by_user(&self.uuid, conn).await { @@ -281,21 +300,7 @@ impl User { UserStatus::Enabled }; - let account_keys = if self.private_key.is_some() { - json!({ - "publicKeyEncryptionKeyPair": { - "wrappedPrivateKey": self.private_key, - "publicKey": self.public_key, - "signedPublicKey": null, - "object": "publicKeyEncryptionKeyPair", - }, - "securityState": null, - "signatureKeyPair": null, - "object": "privateKeys" - }) - } else { - Value::Null - }; + let account_keys = self.account_keys_json(); json!({ "_status": status as i32, @@ -325,7 +330,7 @@ impl User { pub async fn save(&mut self, conn: &DbConn) -> EmptyResult { if !crate::util::is_valid_email(&self.email) { - err!(format!("User email {} is not a valid email address", self.email)) + err!(format!("User email {} is not a valid email address", self.email.escape_debug())) } self.updated_at = Utc::now().naive_utc(); @@ -473,7 +478,7 @@ impl Invitation { pub async fn save(&self, conn: &DbConn) -> EmptyResult { if !crate::util::is_valid_email(&self.email) { - err!(format!("Invitation email {} is not a valid email address", self.email)) + err!(format!("Invitation email {} is not a valid email address", self.email.escape_debug())) } db_run! { conn: diff --git a/src/mail.rs b/src/mail.rs index b20f2853..2efb8d70 100644 --- a/src/mail.rs +++ b/src/mail.rs @@ -531,6 +531,21 @@ pub async fn send_new_device_logged_in(address: &str, ip: &str, dt: &NaiveDateTi send_email(address, &subject, body_html, body_text).await } +pub async fn send_recover_twofactor(address: &str, ip: &str, dt: &NaiveDateTime) -> EmptyResult { + let fmt = "%A, %B %_d, %Y at %r %Z"; + let (subject, body_html, body_text) = get_text( + "email/recover_twofactor", + json!({ + "url": CONFIG.domain(), + "img_src": CONFIG._smtp_img_src(), + "ip": ip, + "datetime": crate::util::format_naive_datetime_local(dt, fmt), + }), + )?; + + send_email(address, &subject, body_html, body_text).await +} + pub async fn send_incomplete_2fa_login( address: &str, ip: &str, diff --git a/src/sso.rs b/src/sso.rs index 01fbd906..6b575a3e 100644 --- a/src/sso.rs +++ b/src/sso.rs @@ -2,7 +2,6 @@ use std::{sync::LazyLock, time::Duration}; use chrono::Utc; use derive_more::{AsRef, Deref, Display, From, Into}; -use regex::Regex; use url::Url; use crate::{ @@ -175,16 +174,16 @@ pub fn decode_state(base64_state: &str) -> ApiResult { if let Ok(valid) = String::from_utf8(vec) { OIDCState(valid) } else { - err!(format!("Invalid utf8 chars in {base64_state} after base64 decoding")) + err!(format!("Invalid utf8 chars in {} after base64 decoding", base64_state.escape_debug())) } } else { - err!(format!("Failed to decode {base64_state} using base64")) + err!(format!("Failed to decode {} using base64", base64_state.escape_debug())) }; Ok(state) } -// redirect_uri from: https://github.com/bitwarden/server/blob/main/src/Identity/IdentityServer/ApiClient.cs +// redirect_uri from: https://github.com/bitwarden/server/blob/main/src/Identity/IdentityServer/StaticClientStore.cs pub async fn authorize_url( state: OIDCState, client_challenge: OIDCCodeChallenge, @@ -193,20 +192,21 @@ pub async fn authorize_url( binding_hash: Option, conn: DbConn, ) -> ApiResult { + // The desktop app uses the localhost callback when it can't register the custom scheme (AppImage, dev builds) + let localhost_redirect_uri = + (8065..=8070).map(|port| format!("http://localhost:{port}")).find(|uri| uri == raw_redirect_uri); + let redirect_uri = match client_id { "web" | "browser" => format!("{}/sso-connector.html", CONFIG.domain()), - "desktop" | "mobile" => "bitwarden://sso-callback".to_owned(), + "desktop" => localhost_redirect_uri.unwrap_or_else(|| "bitwarden://sso-callback".to_owned()), + "mobile" => "bitwarden://sso-callback".to_owned(), "cli" => { - let port_regex = Regex::new(r"^http://localhost:([0-9]{4})$").unwrap(); - if let Some(port) = - port_regex.captures(raw_redirect_uri).and_then(|captures| captures.get(1).map(|c| c.as_str())) - { - format!("http://localhost:{port}") - } else { - err!("Failed to extract port number") - } + let Some(uri) = localhost_redirect_uri else { + err!(format!("Unsupported redirect uri {}", raw_redirect_uri.escape_debug())) + }; + uri } - _ => err!(format!("Unsupported client {client_id}")), + _ => err!(format!("Unsupported client {}", client_id.escape_debug())), }; let (auth_url, sso_auth) = Client::authorize_url(state, client_challenge, redirect_uri, binding_hash).await?; @@ -242,7 +242,8 @@ impl OIDCIdentifier { // During the 2FA flow we will // - retrieve the user information and then only discover he needs 2FA. -// - second time we will rely on `SsoAuth.auth_response` since the `code` has already been exchanged. +// - second time we will rely on `SsoAuth.auth_response` since the `code` has already been exchanged, +// after checking the PKCE verifier again like upstream does on every redemption. // The `SsoAuth` will ensure that the user is authorized only once. pub async fn exchange_code( code: &OIDCCode, @@ -256,6 +257,7 @@ pub async fn exchange_code( }; if let Some(authenticated_user) = sso_auth.auth_response.clone() { + Client::check_client_verifier(&client_verifier, &sso_auth)?; return Ok((sso_auth, authenticated_user)); } diff --git a/src/sso_client.rs b/src/sso_client.rs index bc766586..a5dcaa83 100644 --- a/src/sso_client.rs +++ b/src/sso_client.rs @@ -212,6 +212,16 @@ impl Client { Ok((auth_url, SsoAuth::new(state, client_challenge, nonce.secret().clone(), redirect_uri, binding_hash))) } + // Check the client's PKCE verifier against the challenge it sent when starting the flow + pub fn check_client_verifier(client_verifier: &OIDCCodeVerifier, sso_auth: &SsoAuth) -> EmptyResult { + let verifier = PkceCodeVerifier::new(client_verifier.to_string()); + let challenge = PkceCodeChallenge::from_code_verifier_sha256(&verifier); + if challenge.as_str() != &*sso_auth.client_challenge { + err!("PKCE client challenge failed") + } + Ok(()) + } + pub async fn exchange_code( &self, code: OIDCCode, @@ -234,15 +244,10 @@ impl Client { let mut exchange = self.core_client.exchange_code(oidc_code); - let verifier = PkceCodeVerifier::new(client_verifier.into()); if CONFIG.sso_pkce() { - exchange = exchange.set_pkce_verifier(verifier); + exchange = exchange.set_pkce_verifier(PkceCodeVerifier::new(client_verifier.into())); } else { - let challenge = PkceCodeChallenge::from_code_verifier_sha256(&verifier); - if challenge.as_str() != String::from(sso_auth.client_challenge.clone()) { - err!("PKCE client challenge failed") - // Might need to notify admin ? how ? - } + Self::check_client_verifier(&client_verifier, sso_auth)?; } match exchange.request_async(&self.http_client).await { diff --git a/src/static/templates/email/recover_twofactor.hbs b/src/static/templates/email/recover_twofactor.hbs new file mode 100644 index 00000000..92e4330a --- /dev/null +++ b/src/static/templates/email/recover_twofactor.hbs @@ -0,0 +1,9 @@ +Two-Step Login Recovered From {{ip}} + +Two-step login on your account has been disabled by using the account's recovery code. + +* Date: {{datetime}} +* IP Address: {{ip}} + +If you did not perform this action, you should immediately take steps to secure your account. +{{> email/email_footer_text }} diff --git a/src/static/templates/email/recover_twofactor.html.hbs b/src/static/templates/email/recover_twofactor.html.hbs new file mode 100644 index 00000000..847cbae0 --- /dev/null +++ b/src/static/templates/email/recover_twofactor.html.hbs @@ -0,0 +1,26 @@ +Two-Step Login Recovered From {{ip}} + +{{> email/email_header }} + + + + + + + + + + + + + +
+ Two-step login on your account has been disabled by using the account's recovery code. +
+ Date: {{datetime}} +
+ IP Address: {{ip}} +
+ If you did not perform this action, you should immediately take steps to secure your account. +
+{{> email/email_footer }} diff --git a/src/util.rs b/src/util.rs index 2a5678bc..9d319542 100644 --- a/src/util.rs +++ b/src/util.rs @@ -380,7 +380,7 @@ pub fn get_display_size(size: i64) -> String { let mut unit_counter = 0; loop { - if size > 1024. { + if size > 1024. && unit_counter < UNITS.len() - 1 { size /= 1024.; unit_counter += 1; } else { @@ -522,8 +522,8 @@ pub fn format_datetime_http(dt: &DateTime) -> String { expiry_time.to_rfc2822().replace("+0000", "GMT") } -pub fn parse_date(date: &str) -> NaiveDateTime { - DateTime::parse_from_rfc3339(date).unwrap().naive_utc() +pub fn parse_date(date: &str) -> Option { + DateTime::parse_from_rfc3339(date).ok().map(|dt| dt.naive_utc()) } /// Returns true or false if an email address is valid or not