diff --git a/.env.template b/.env.template index 0d922774..fd7c2fd2 100644 --- a/.env.template +++ b/.env.template @@ -324,6 +324,14 @@ ## Set to the string "none" (without quotes), to disable any headers and just use the remote IP # IP_HEADER=X-Real-IP +## Which addresses the header above is accepted from, defaults to "local". +## Anyone able to reach Vaultwarden can set the header, and the client IP is used for the login and +## admin rate limits, so it is only trusted when the request comes from a proxy listed here. +## "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. +# IP_HEADER_TRUSTED_PROXIES=local + ## 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 `{}`, @@ -461,6 +469,13 @@ ## Note that this applies to both the login and the 2FA, so it's recommended to allow a burst size of at least 2. # LOGIN_RATELIMIT_MAX_BURST=10 +## Number of seconds, on average, between requests from the same IP address to one of the rate limited +## unauthenticated endpoints, like the password hint, the account recovery mails or accessing a Send. +# UNAUTHENTICATED_RATELIMIT_SECONDS=60 +## Allow a burst of requests of up to this size, while maintaining the average indicated by `UNAUTHENTICATED_RATELIMIT_SECONDS`. +## This budget is shared between all of those endpoints, so it is more lenient than the login one. +# UNAUTHENTICATED_RATELIMIT_MAX_BURST=50 + ## BETA FEATURE: Groups ## Controls whether group support is enabled for organizations ## This setting applies to organizations. diff --git a/Cargo.lock b/Cargo.lock index 0715098c..cb72d16f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5922,6 +5922,7 @@ dependencies = [ "hickory-resolver", "html5gum", "http 1.4.2", + "ipnet", "job_scheduler_ng", "jsonwebtoken", "lettre", diff --git a/Cargo.toml b/Cargo.toml index 909570e7..2a3ca2de 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -230,6 +230,9 @@ pico-args = "0.5.0" pastey = "0.2.3" governor = "0.10.4" +# CIDR parsing for the trusted proxies of the client IP header +ipnet = "2.12.0" + # OIDC for SSO openidconnect = { version = "4.0.1", default-features = false } moka = { version = "0.12.15", features = ["future"] } diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 623edf24..d04a2de1 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -15,7 +15,7 @@ use crate::{ core::{accept_org_invite, log_user_event, two_factor::email}, master_password_policy, register_push_device, unregister_push_device, }, - auth::{ClientHeaders, Headers, decode_delete, decode_invite, decode_verify_email}, + auth::{ClientHeaders, ClientIp, Headers, decode_delete, decode_invite, decode_verify_email}, crypto, db::{ DbConn, DbPool, @@ -1197,7 +1197,9 @@ struct DeleteRecoverData { } #[post("/accounts/delete-recover", data = "")] -async fn post_delete_recover(data: Json, conn: DbConn) -> EmptyResult { +async fn post_delete_recover(data: Json, ip: ClientIp, conn: DbConn) -> EmptyResult { + crate::ratelimit::check_limit_unauthenticated(&ip.ip)?; + let data: DeleteRecoverData = data.into_inner(); if CONFIG.mail_enabled() { @@ -1270,9 +1272,11 @@ struct PasswordHintData { } #[post("/accounts/password-hint", data = "")] -async fn password_hint(data: Json, conn: DbConn) -> EmptyResult { +async fn password_hint(data: Json, ip: ClientIp, conn: DbConn) -> EmptyResult { const NO_HINT: &str = "Sorry, you have no password hint..."; + crate::ratelimit::check_limit_unauthenticated(&ip.ip)?; + if !CONFIG.password_hints_allowed() || (!CONFIG.mail_enabled() && !CONFIG.show_password_hint()) { err!("This server is not configured to provide password hints."); } @@ -1510,7 +1514,9 @@ async fn put_device_token(device_id: DeviceId, data: Json, headers: H } #[put("/devices/identifier//clear-token")] -async fn put_clear_device_token(device_id: DeviceId, conn: DbConn) -> EmptyResult { +async fn put_clear_device_token(device_id: DeviceId, ip: ClientIp, conn: DbConn) -> EmptyResult { + crate::ratelimit::check_limit_unauthenticated(&ip.ip)?; + // This only clears push token // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Api/Controllers/DevicesController.cs#L215 // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Core/Services/Implementations/DeviceService.cs#L37 @@ -1532,8 +1538,8 @@ async fn put_clear_device_token(device_id: DeviceId, conn: DbConn) -> EmptyResul // On upstream server, both PUT and POST are declared. Implementing the POST method in case it would be useful somewhere #[post("/devices/identifier//clear-token")] -async fn post_clear_device_token(device_id: DeviceId, conn: DbConn) -> EmptyResult { - put_clear_device_token(device_id, conn).await +async fn post_clear_device_token(device_id: DeviceId, ip: ClientIp, conn: DbConn) -> EmptyResult { + put_clear_device_token(device_id, ip, conn).await } #[get("/tasks")] diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 14e9f72f..b416173e 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -629,7 +629,7 @@ async fn post_ciphers_import(data: Json, headers: Headers, conn: DbC // Read and create the ciphers for (index, mut cipher_data) in data.ciphers.into_iter().enumerate() { - let folder_id = relations_map.get(&index).map(|i| folders[*i].clone()); + let folder_id = relations_map.get(&index).and_then(|i| folders.get(*i).cloned()); cipher_data.folder_id = folder_id; let mut cipher = Cipher::new(cipher_data.r#type, cipher_data.name.clone()); @@ -1043,6 +1043,13 @@ async fn share_cipher_by_uuid( err!("Cipher doesn't exist") }; + // `update_cipher_from_data()` rejects this too, but only after the collections below were + // already linked. There are no transactions, so that would leave the cipher linked to a + // collection of another organization. + if cipher.organization_uuid.is_some() && cipher.organization_uuid != data.cipher.organization_id { + err!("Organization mismatch. Please resync the client before updating the cipher") + } + let mut shared_to_collections = vec![]; if let Some(organization_id) = &data.cipher.organization_id { diff --git a/src/api/core/events.rs b/src/api/core/events.rs index 698a890f..5518fa3c 100644 --- a/src/api/core/events.rs +++ b/src/api/core/events.rs @@ -182,7 +182,10 @@ async fn post_events_collect(data: Json>, headers: Headers, .await; } 1600..=1699 => { - if let Some(org_id) = &event.organization_id { + // 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() + { log_event_impl( event.r#type, org_id, @@ -197,8 +200,11 @@ async fn post_events_collect(data: Json>, headers: Headers, } } _ => { + // 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 && let Some(cipher) = Cipher::find_by_uuid(cipher_uuid, &conn).await + && cipher.is_accessible_to_user(&headers.user.uuid, &conn).await && let Some(org_id) = cipher.organization_uuid { log_event_impl( diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index c7e79aed..09946658 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -577,6 +577,13 @@ async fn post_bulk_access_collections( err!("Can't find organization details") } + // The collections and members are checked below, the groups only here. + let org_groups = Group::find_by_organization(&org_id, &conn).await; + let org_group_ids: HashSet<&GroupId> = org_groups.iter().map(|g| &g.uuid).collect(); + if let Some(g) = data.groups.iter().find(|g| !org_group_ids.contains(&g.id)) { + err!("Invalid group", format!("Group {} does not belong to organization {}!", g.id, org_id)) + } + for col_id in data.collection_ids { let Some(collection) = Collection::find_by_uuid_and_org(&col_id, &org_id, &conn).await else { err!("Collection not found") @@ -946,6 +953,11 @@ async fn get_members( if org_id != headers.membership.org_uuid { err!("Organization not found", "Organization id's do not match"); } + + if !headers.membership.has_full_access() { + err_code!("Resource not found.", "User does not have full access", rocket::http::Status::NotFound.code); + } + let mut users_json = Vec::new(); for u in Membership::find_by_org(&org_id, &conn).await { users_json.push( @@ -1167,6 +1179,9 @@ async fn send_invite( } for group_id in &data.groups { + if Group::find_by_uuid_and_org(group_id, &org_id, &conn).await.is_none() { + err!("Group not found in Organization") + } let mut group_entry = GroupUser::new(group_id.clone(), new_member.uuid.clone()); group_entry.save(&conn).await?; } @@ -1614,6 +1629,9 @@ async fn edit_member( GroupUser::delete_all_by_member(&member_to_edit.uuid, &conn).await?; for group_id in data.groups.iter().flatten() { + if Group::find_by_uuid_and_org(group_id, &org_id, &conn).await.is_none() { + err!("Group not found in Organization") + } let mut group_entry = GroupUser::new(group_id.clone(), member_to_edit.uuid.clone()); group_entry.save(&conn).await?; } @@ -1813,19 +1831,19 @@ async fn post_org_import( // TODO: See if we can optimize the whole cipher adding/importing and prevent duplicate code and checks. Cipher::validate_cipher_data(&data.ciphers)?; - let existing_collections: HashSet> = - Collection::find_by_organization(&org_id, &conn).await.into_iter().map(|c| Some(c.uuid)).collect(); + let existing_collections: HashMap = + Collection::find_by_organization(&org_id, &conn).await.into_iter().map(|c| (c.uuid.clone(), c)).collect(); let mut collections: Vec = Vec::with_capacity(data.collections.len()); for col in data.collections { - let collection_uuid = if existing_collections.contains(&col.id) { - let col_id = col.id.unwrap(); - // When not an Owner or Admin, check if the member is allowed to access the collection. + let existing = col.id.as_ref().and_then(|col_id| existing_collections.get(col_id)); + let collection_uuid = if let Some(collection) = existing { + // When not an Owner or Admin, check if the member is allowed to write to the collection. if headers.membership.atype < MembershipType::Admin - && !Collection::can_access_collection(&headers.membership, &col_id, &conn).await + && !collection.is_writable_by_user(&headers.membership.user_uuid, &conn).await { err!(Compact, "The current user isn't allowed to manage this collection") } - col_id + collection.uuid.clone() } else { // We do not allow users or managers which can not manage all collections to create new collections // If there is any collection other than an existing import collection, abort the import. @@ -1870,8 +1888,9 @@ async fn post_org_import( // Assign the collections for (cipher_index, col_index) in relations { - let cipher_id = &ciphers[cipher_index]; - let col_id = &collections[col_index]; + let (Some(cipher_id), Some(col_id)) = (ciphers.get(cipher_index), collections.get(col_index)) else { + err!(Compact, "Invalid collection relationship") + }; CollectionCipher::save(cipher_id, col_id, &conn).await?; } @@ -2441,6 +2460,11 @@ async fn get_groups_data( if org_id != headers.membership.org_uuid { err!("Organization not found", "Organization id's do not match"); } + + if !headers.membership.has_full_access() { + err_code!("Resource not found.", "User does not have full access", rocket::http::Status::NotFound.code); + } + let groups: Vec = if CONFIG.org_groups_enabled() { let groups = Group::find_by_organization(&org_id, &conn).await; let mut groups_json = Vec::with_capacity(groups.len()); diff --git a/src/api/core/public.rs b/src/api/core/public.rs index 33189e78..3db25df9 100644 --- a/src/api/core/public.rs +++ b/src/api/core/public.rs @@ -14,7 +14,7 @@ use crate::{ db::{ DbConn, models::{ - Group, GroupUser, Invitation, Membership, MembershipStatus, MembershipType, Organization, + Group, GroupUser, Invitation, Membership, MembershipStatus, MembershipType, OrgPolicy, Organization, OrganizationApiKey, OrganizationId, User, }, }, @@ -84,8 +84,15 @@ 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 restored = member.restore(); + let mut restored = 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. + if restored && let Err(e) = OrgPolicy::check_user_allowed(&member, "restore", &conn).await { + warn!("Not restoring {}: {e:?}", user_data.email); + member.revoke(); + restored = false; + } if restored || ext_modified { member.save(&conn).await?; } diff --git a/src/api/core/sends.rs b/src/api/core/sends.rs index fb3ee48f..36cdb1c3 100644 --- a/src/api/core/sends.rs +++ b/src/api/core/sends.rs @@ -471,6 +471,8 @@ async fn post_access_legacy( ip: ClientIp, nt: Notify<'_>, ) -> JsonResult { + crate::ratelimit::check_limit_unauthenticated(&ip.ip)?; + let Some(mut send) = Send::find_by_access_id(access_id, &conn).await else { err_code!(SEND_INACCESSIBLE_MSG, 404) }; @@ -505,11 +507,13 @@ async fn post_access_legacy( // Files are incremented during the download if send.atype == SendType::Text as i32 { - send.access_count += 1; + if !send.register_access(&conn).await? { + err_code!(SEND_INACCESSIBLE_MSG, 404) + } + } else { + send.save(&conn).await?; } - send.save(&conn).await?; - process_access(send, conn, nt).await } @@ -548,8 +552,11 @@ async fn post_access_file_legacy( data: Json, host: Host, conn: DbConn, + ip: ClientIp, nt: Notify<'_>, ) -> JsonResult { + crate::ratelimit::check_limit_unauthenticated(&ip.ip)?; + let Some(mut send) = Send::find_by_uuid(&send_id, &conn).await else { err_code!(SEND_INACCESSIBLE_MSG, 404) }; @@ -582,9 +589,9 @@ async fn post_access_file_legacy( } } - send.access_count += 1; - - send.save(&conn).await?; + if !send.register_access(&conn).await? { + err_code!(SEND_INACCESSIBLE_MSG, 404) + } process_access_file(send, file_id, host, conn, nt).await } diff --git a/src/api/icons.rs b/src/api/icons.rs index 81191e38..8f3e730b 100644 --- a/src/api/icons.rs +++ b/src/api/icons.rs @@ -405,6 +405,22 @@ async fn get_page(url: &str) -> Result { } async fn get_page_with_referer(url: &str, referer: &str) -> Result { + // The resolver only sees hosts needing name resolution, so IP-literal hrefs from + // attacker-controlled HTML never reach `post_resolve()`. Check them here. + let Ok(parsed_url) = url::Url::parse(url) else { + err_silent!("Invalid URL", url) + }; + + if !matches!(parsed_url.scheme(), "http" | "https") { + err_silent!("Invalid scheme", url) + } + + let Some(host) = parsed_url.host() else { + err_silent!("Invalid host", url) + }; + + should_block_host(&host)?; + let mut client = CLIENT.get(url); if !referer.is_empty() { client = client.header("Referer", referer); diff --git a/src/api/identity.rs b/src/api/identity.rs index 1597698f..9212ed8d 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -109,6 +109,7 @@ async fn login( } "authorization_code" => err!("SSO sign-in is not available"), "send_access" => { + crate::ratelimit::check_limit_unauthenticated(&client_header.ip.ip)?; check_is_some(data.client_id.as_ref(), "client_id cannot be blank")?; check_is_some(data.send_id.as_ref(), "send_id cannot be blank")?; @@ -1055,8 +1056,11 @@ enum RegisterVerificationResponse { #[post("/accounts/register/send-verification-email", data = "")] async fn register_verification_email( data: Json, + ip: ClientIp, conn: DbConn, ) -> ApiResult { + crate::ratelimit::check_limit_unauthenticated(&ip.ip)?; + let data = data.into_inner(); // the registration can only continue if signup is allowed or there exists an invitation diff --git a/src/api/notifications.rs b/src/api/notifications.rs index 80067433..8bfcd518 100644 --- a/src/api/notifications.rs +++ b/src/api/notifications.rs @@ -33,9 +33,14 @@ pub static WS_USERS: LazyLock> = LazyLock::new(|| { pub static WS_ANONYMOUS_SUBSCRIPTIONS: LazyLock> = LazyLock::new(|| { Arc::new(AnonymousWebSocketSubscriptions { map: Arc::new(dashmap::DashMap::new()), + connections: Arc::new(dashmap::DashMap::new()), }) }); +/// The anonymous hub needs no authentication, so bound how much a single client can hold open. +/// One connection is needed per pending login request, several at once are only expected behind NAT. +const MAX_ANONYMOUS_CONNECTIONS_PER_IP: u32 = 25; + static NOTIFICATIONS_DISABLED: LazyLock = LazyLock::new(|| !CONFIG.enable_websocket() && !CONFIG.push_enabled()); pub fn routes() -> Vec { @@ -82,14 +87,21 @@ impl Drop for WSEntryMapGuard { struct WSAnonymousEntryMapGuard { subscriptions: Arc, token: String, + entry_uuid: uuid::Uuid, addr: IpAddr, } impl WSAnonymousEntryMapGuard { - fn new(subscriptions: Arc, token: String, addr: IpAddr) -> Self { + fn new( + subscriptions: Arc, + token: String, + entry_uuid: uuid::Uuid, + addr: IpAddr, + ) -> Self { Self { subscriptions, token, + entry_uuid, addr, } } @@ -98,7 +110,11 @@ impl WSAnonymousEntryMapGuard { impl Drop for WSAnonymousEntryMapGuard { fn drop(&mut self) { info!("Closing WS connection from {}", self.addr); - self.subscriptions.map.remove(&self.token); + if let Some(mut entry) = self.subscriptions.map.get_mut(&self.token) { + entry.retain(|(uuid, _)| uuid != &self.entry_uuid); + } + self.subscriptions.map.remove_if(&self.token, |_, senders| senders.is_empty()); + self.subscriptions.release(self.addr); } } @@ -194,12 +210,19 @@ fn anonymous_websockets_hub<'r>(ws: WebSocket, token: String, ip: ClientIp) -> R let (mut rx, guard) = { let subscriptions = Arc::clone(&WS_ANONYMOUS_SUBSCRIPTIONS); - // Add a channel to send messages to this client to the map + if !subscriptions.try_reserve(ip.ip) { + err_code!("Too many connections", 429) + } + + // Add a channel to send messages to this client to the map. + // Clients reconnect with the same token while a login request is still pending, so keep + // every subscriber instead of replacing, otherwise the older one takes the newer one down. let (tx, rx) = tokio::sync::mpsc::channel::(100); - subscriptions.map.insert(token.clone(), tx); + let entry_uuid = uuid::Uuid::new_v4(); + subscriptions.map.entry(token.clone()).or_default().push((entry_uuid, tx)); // Once the guard goes out of scope, the connection will have been closed and the entry will be deleted from the map - (rx, WSAnonymousEntryMapGuard::new(subscriptions, token, ip.ip)) + (rx, WSAnonymousEntryMapGuard::new(subscriptions, token, entry_uuid, ip.ip)) }; Ok({ @@ -534,15 +557,42 @@ impl WebSocketUsers { #[derive(Clone)] pub struct AnonymousWebSocketSubscriptions { - map: Arc>>, + map: Arc>>, + connections: Arc>, } impl AnonymousWebSocketSubscriptions { + /// Takes a connection slot for this address, returns false when it already reached the limit. + fn try_reserve(&self, addr: IpAddr) -> bool { + let mut count = self.connections.entry(addr).or_insert(0); + if *count >= MAX_ANONYMOUS_CONNECTIONS_PER_IP { + return false; + } + *count += 1; + true + } + + /// Releases a slot taken by `try_reserve`. + fn release(&self, addr: IpAddr) { + let empty = if let Some(mut count) = self.connections.get_mut(&addr) { + *count = count.saturating_sub(1); + *count == 0 + } else { + false + }; + // Only remove once the guard above is dropped, otherwise this deadlocks. + if empty { + self.connections.remove_if(&addr, |_, count| *count == 0); + } + } + async fn send_update(&self, token: &str, data: &[u8]) { - if let Some(sender) = self.map.get(token).map(|v| v.clone()) - && let Err(e) = sender.send(Message::binary(data)).await - { - error!("Error sending WS update {e}"); + // Clone the senders so the map isn't kept locked while sending. + let senders = self.map.get(token).map(|v| v.clone()).unwrap_or_default(); + for (_, sender) in senders { + if let Err(e) = sender.send(Message::binary(data)).await { + error!("Error sending WS update {e}"); + } } } diff --git a/src/auth.rs b/src/auth.rs index 88a59b4b..9015de43 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -10,6 +10,7 @@ use std::{ }; use chrono::{DateTime, TimeDelta, Utc}; +use ipnet::IpNet; use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header, errors::ErrorKind}; use num_traits::FromPrimitive; use openssl::rsa::Rsa; @@ -1054,12 +1055,44 @@ pub struct ClientIp { pub ip: IpAddr, } +/// Parses a single entry of `ip_header_trusted_proxies`, which can be a CIDR range or a plain IP. +pub fn parse_trusted_proxy(entry: &str) -> Option { + let entry = entry.trim(); + match entry.parse::() { + Ok(net) => Some(net), + // Without a prefix length it is a single address, which is a valid way to write this. + Err(_) => entry.parse::().ok().map(IpNet::from), + } +} + +/// The client IP header can be set by anyone able to reach us, so only accept it from a proxy we trust. +fn ip_header_is_trusted(remote: Option) -> bool { + let trusted = CONFIG.ip_header_trusted_proxies(); + let trusted = trusted.trim(); + if trusted.eq_ignore_ascii_case("all") { + return true; + } + + let Some(remote) = remote else { + return false; + }; + // A dual stack listener reports IPv4 clients as IPv4-mapped IPv6, which `is_global()` reports as + // non global. That is what we want when blocking outgoing requests, but here it would trust them. + let remote = remote.to_canonical(); + if trusted.eq_ignore_ascii_case("local") { + return !crate::util::is_global(remote); + } + trusted.split(',').filter_map(parse_trusted_proxy).any(|net| net.contains(&remote)) +} + #[rocket::async_trait] impl<'r> FromRequest<'r> for ClientIp { type Error = (); async fn from_request(req: &'r Request<'_>) -> Outcome { - let ip = if CONFIG._ip_header_enabled() { + 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], @@ -1070,10 +1103,13 @@ impl<'r> FromRequest<'r> for ClientIp { .ok() }) } else { + if CONFIG._ip_header_enabled() && req.headers().get_one(&CONFIG.ip_header()).is_some() { + debug!("Ignoring the '{}' header, {remote:?} is not a trusted proxy", CONFIG.ip_header()); + } None }; - let ip = ip.or_else(|| req.remote().map(|r| r.ip())).unwrap_or_else(|| "0.0.0.0".parse().unwrap()); + let ip = ip.or(remote).unwrap_or_else(|| "0.0.0.0".parse().unwrap()); Outcome::Success(ClientIp { ip, @@ -1268,20 +1304,8 @@ pub async fn refresh_tokens( ) -> ApiResult<(Device, AuthTokens)> { let refresh_claims = match decode_refresh(refresh_token) { Err(err) => { - error!("Failed to decode {} refresh_token: {refresh_token}: {err:?}", ip.ip); - //err_silent!(format!("Impossible to read refresh_token: {}", err.message())) - - // If the token failed to decode, it was probably one of the old style tokens that was just a Base64 string. - // We can generate a claim for them for backwards compatibility. Note that the password refresh claims don't - // check expiration or issuer, so they're not included here. - RefreshJwtClaims { - nbf: 0, - exp: 0, - iss: String::new(), - sub: AuthMethod::Password, - device_token: refresh_token.into(), - token: None, - } + error!("Failed to decode refresh_token from {}: {err:?}", ip.ip); + err_silent!("Invalid refresh token") } Ok(claims) => claims, }; diff --git a/src/auth/send.rs b/src/auth/send.rs index 84500b6a..00ede0ed 100644 --- a/src/auth/send.rs +++ b/src/auth/send.rs @@ -113,8 +113,9 @@ impl SendTokens { } } - send.access_count += 1; - send.save(conn).await?; + if !send.register_access(conn).await? { + return Self::invalid_error(&format!("Send {send_id}, max access reached"), "send_id_invalid", true); + } Ok(Self { access_claims: generate_send_access_claims(&send_id), diff --git a/src/config.rs b/src/config.rs index 49281b6c..c4457478 100644 --- a/src/config.rs +++ b/src/config.rs @@ -666,6 +666,12 @@ make_config! { ip_header: String, true, def, "X-Real-IP".to_owned(); /// Internal IP header property, used to avoid recomputing each time _ip_header_enabled: bool, false, generated, |c| &c.ip_header.trim().to_lowercase() != "none"; + /// Trusted proxies |> Which addresses the client IP header is accepted from. Requests from any + /// 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. + 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 `{}`, /// which is replaced with the domain. For example: `https://icon.example.com/domain/{}`. @@ -768,6 +774,11 @@ make_config! { /// Max burst size for login requests |> Allow a burst of requests of up to this size, while maintaining the average indicated by `login_ratelimit_seconds`. Note that this applies to both the login and the 2FA, so it's recommended to allow a burst size of at least 2 login_ratelimit_max_burst: u32, false, def, 10; + /// Seconds between unauthenticated requests |> Number of seconds, on average, between requests from the same IP address to any of the rate limited unauthenticated endpoints + unauthenticated_ratelimit_seconds: u64, false, def, 60; + /// Max burst size for unauthenticated requests |> Allow a burst of requests of up to this size, while maintaining the average indicated by `unauthenticated_ratelimit_seconds`. This is shared between several endpoints, so it needs to be more lenient than the login one + unauthenticated_ratelimit_max_burst: u32, false, def, 50; + /// Seconds between admin login requests |> Number of seconds, on average, between admin requests from the same IP address before rate limiting kicks in admin_ratelimit_seconds: u64, false, def, 300; /// Max burst size for admin login requests |> Allow a burst of requests of up to this size, while maintaining the average indicated by `admin_ratelimit_seconds` @@ -942,6 +953,18 @@ fn validate_config(cfg: &ConfigItems, on_update: bool) -> Result<(), Error> { } } + let trusted_proxies = cfg.ip_header_trusted_proxies.trim(); + if !trusted_proxies.eq_ignore_ascii_case("all") && !trusted_proxies.eq_ignore_ascii_case("local") { + for entry in trusted_proxies.split(',').filter(|e| !e.trim().is_empty()) { + if crate::auth::parse_trusted_proxy(entry).is_none() { + err!(format!( + "Invalid IP_HEADER_TRUSTED_PROXIES entry `{}`, expected an IP or CIDR range", + entry.trim() + )); + } + } + } + if cfg.password_iterations < 100_000 { err!("PASSWORD_ITERATIONS should be at least 100000 or higher. The default is 600000!"); } diff --git a/src/db/models/group.rs b/src/db/models/group.rs index 820d3700..37037de6 100644 --- a/src/db/models/group.rs +++ b/src/db/models/group.rs @@ -271,7 +271,9 @@ impl Group { groups::table .inner_join(groups_users::table.on(groups_users::groups_uuid.eq(groups::uuid))) .inner_join( - users_organizations::table.on(users_organizations::uuid.eq(groups_users::users_organizations_uuid)), + users_organizations::table.on(users_organizations::uuid + .eq(groups_users::users_organizations_uuid) + .and(users_organizations::org_uuid.eq(groups::organizations_uuid))), ) .filter(users_organizations::user_uuid.eq(user_uuid)) .filter(groups::organizations_uuid.eq(org_uuid)) diff --git a/src/db/models/send.rs b/src/db/models/send.rs index 48159e8a..616479ed 100644 --- a/src/db/models/send.rs +++ b/src/db/models/send.rs @@ -231,6 +231,37 @@ impl Send { } } + /// Registers an access, incrementing `access_count` only while below `max_access_count`. + /// Returns false when the limit was already reached. The check and the increment are a single + /// statement, otherwise concurrent accesses can both pass the check and exceed the limit. + pub async fn register_access(&mut self, conn: &DbConn) -> Result { + self.update_users_revision(conn).await; + + let revision_date = Utc::now().naive_utc(); + let uuid = self.uuid.clone(); + let updated = conn + .run(move |conn| { + diesel::update(sends::table) + .filter(sends::uuid.eq(uuid)) + .filter( + sends::max_access_count + .is_null() + .or(sends::access_count.nullable().lt(sends::max_access_count)), + ) + .set((sends::access_count.eq(sends::access_count + 1), sends::revision_date.eq(revision_date))) + .execute(conn) + }) + .await?; + + if updated == 0 { + return Ok(false); + } + + self.access_count += 1; + self.revision_date = revision_date; + Ok(true) + } + pub async fn delete(&self, conn: &DbConn) -> EmptyResult { self.update_users_revision(conn).await; diff --git a/src/http_client.rs b/src/http_client.rs index 205b1cc3..0831d990 100644 --- a/src/http_client.rs +++ b/src/http_client.rs @@ -174,6 +174,27 @@ pub enum CustomHttpClientError { } impl CustomHttpClientError { + /// Attach the domain that resolved to this address, which `should_block_host()` can't know. + fn with_domain(self, name: &str) -> Self { + match self { + Self::NonGlobalIp { + ip, + .. + } => Self::NonGlobalIp { + domain: Some(name.to_owned()), + ip, + }, + Self::Blocked { + domain, + } => Self::Blocked { + domain: format!("{name} ({domain})"), + }, + other @ Self::Invalid { + .. + } => other, + } + } + pub fn downcast_ref(e: &dyn std::error::Error) -> Option<&Self> { let mut source = e.source(); @@ -285,14 +306,12 @@ fn pre_resolve(name: &str, enforce_block: bool) -> Result<(), CustomHttpClientEr } fn post_resolve(name: &str, ip: IpAddr) -> Result<(), CustomHttpClientError> { - if should_block_ip(ip) { - Err(CustomHttpClientError::NonGlobalIp { - domain: Some(name.to_owned()), - ip, - }) - } else { - Ok(()) - } + let host: Host<&str> = match ip { + IpAddr::V4(ip) => Host::Ipv4(ip), + IpAddr::V6(ip) => Host::Ipv6(ip), + }; + + should_block_host(&host).map_err(|e| e.with_domain(name)) } impl Resolve for CustomDns { diff --git a/src/ratelimit.rs b/src/ratelimit.rs index 2b422924..70217957 100644 --- a/src/ratelimit.rs +++ b/src/ratelimit.rs @@ -18,6 +18,24 @@ static LIMITER_ADMIN: LazyLock = LazyLock::new(|| { RateLimiter::keyed(Quota::with_period(seconds).expect("Non-zero admin ratelimit seconds").allow_burst(burst)) }); +static LIMITER_UNAUTHENTICATED: LazyLock = LazyLock::new(|| { + let seconds = Duration::from_secs(CONFIG.unauthenticated_ratelimit_seconds()); + let burst = NonZeroU32::new(CONFIG.unauthenticated_ratelimit_max_burst()) + .expect("Non-zero unauthenticated ratelimit burst"); + RateLimiter::keyed( + Quota::with_period(seconds).expect("Non-zero unauthenticated ratelimit seconds").allow_burst(burst), + ) +}); + +pub fn check_limit_unauthenticated(ip: &IpAddr) -> Result<(), Error> { + match LIMITER_UNAUTHENTICATED.check_key(ip) { + Ok(()) => Ok(()), + Err(_e) => { + err_code!("Too many requests", 429); + } + } +} + pub fn check_limit_login(ip: &IpAddr) -> Result<(), Error> { match LIMITER_LOGIN.check_key(ip) { Ok(()) => Ok(()), diff --git a/src/sso_client.rs b/src/sso_client.rs index ff39b0b0..bc766586 100644 --- a/src/sso_client.rs +++ b/src/sso_client.rs @@ -95,7 +95,9 @@ impl<'c> AsyncHttpClient<'c> for OidcHttpClient { } let body = response.bytes().await.map_err(Box::new)?; - debug!("Response body {}", String::from_utf8_lossy(&body)); + if CONFIG.sso_debug_tokens() { + debug!("Response body {}", String::from_utf8_lossy(&body)); + } builder.body(body.to_vec()).map_err(HttpClientError::Http) }) }