Trusted proxy support, unauthenticated rate limit & other fixes (#7472)

* Trusted proxies, unauthenticated rate limits and various fixes

* Fix get_groups_data

* Fix get_groups_data when not using full_access

* Fmt

* Fix org import

* deduplicate send validation
This commit is contained in:
Daniel García
2026-07-24 18:27:32 +02:00
committed by GitHub
parent a6a88e7929
commit 46ae59eaf4
21 changed files with 415 additions and 98 deletions
+12 -6
View File
@@ -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,
@@ -1193,7 +1193,9 @@ struct DeleteRecoverData {
}
#[post("/accounts/delete-recover", data = "<data>")]
async fn post_delete_recover(data: Json<DeleteRecoverData>, conn: DbConn) -> EmptyResult {
async fn post_delete_recover(data: Json<DeleteRecoverData>, ip: ClientIp, conn: DbConn) -> EmptyResult {
crate::ratelimit::check_limit_unauthenticated(&ip.ip)?;
let data: DeleteRecoverData = data.into_inner();
if CONFIG.mail_enabled() {
@@ -1266,9 +1268,11 @@ struct PasswordHintData {
}
#[post("/accounts/password-hint", data = "<data>")]
async fn password_hint(data: Json<PasswordHintData>, conn: DbConn) -> EmptyResult {
async fn password_hint(data: Json<PasswordHintData>, 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.");
}
@@ -1513,7 +1517,9 @@ async fn put_device_token(device_id: DeviceId, data: Json<PushToken>, headers: H
}
#[put("/devices/identifier/<device_id>/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
@@ -1535,8 +1541,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/<device_id>/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")]
+11 -2
View File
@@ -450,7 +450,9 @@ pub async fn update_cipher_from_data(
match Membership::find_confirmed_by_user_and_org(&headers.user.uuid, &org_id, conn).await {
None => err!("You don't have permission to add item to organization"),
Some(member) => {
if shared_to_collections.is_some()
// 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.
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
{
@@ -629,7 +631,7 @@ async fn post_ciphers_import(data: Json<ImportData>, 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 +1045,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 {
+7 -1
View File
@@ -182,7 +182,10 @@ async fn post_events_collect(data: Json<Vec<EventCollection>>, 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<Vec<EventCollection>>, 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(
+47 -9
View File
@@ -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<Option<CollectionId>> =
Collection::find_by_organization(&org_id, &conn).await.into_iter().map(|c| Some(c.uuid)).collect();
let existing_collections: HashMap<CollectionId, Collection> =
Collection::find_by_organization(&org_id, &conn).await.into_iter().map(|c| (c.uuid.clone(), c)).collect();
let mut collections: Vec<CollectionId> = 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.
@@ -1853,6 +1871,8 @@ async fn post_org_import(
for mut cipher_data in data.ciphers {
// Always clear folder_id's via an organization import
cipher_data.folder_id = None;
// Replace the client-provided, unvalidated organizationId with the real target org
cipher_data.organization_id = Some(org_id.clone());
let mut cipher = Cipher::new(cipher_data.r#type, cipher_data.name.clone());
update_cipher_from_data(
&mut cipher,
@@ -1870,8 +1890,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 +2462,23 @@ async fn get_groups_data(
if org_id != headers.membership.org_uuid {
err!("Organization not found", "Organization id's do not match");
}
// The details view (group→collection/user mappings) needs full org access; the plain list only
// needs manage access to a collection, so a manager of a collection (directly or via a group)
// can load it to assign groups.
let has_full_access = headers.membership.has_full_access()
|| (CONFIG.org_groups_enabled()
&& GroupUser::has_full_access_by_member(&org_id, &headers.membership.uuid, &conn).await);
let allowed = if details {
has_full_access
} else {
has_full_access
|| Collection::has_manageable_collection_by_user(&org_id, &headers.membership.user_uuid, &conn).await
};
if !allowed {
err_code!("Resource not found.", "User does not have access", rocket::http::Status::NotFound.code);
}
let groups: Vec<Value> = if CONFIG.org_groups_enabled() {
let groups = Group::find_by_organization(&org_id, &conn).await;
let mut groups_json = Vec::with_capacity(groups.len());
+9 -2
View File
@@ -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<OrgImportData>, 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?;
}
+21 -28
View File
@@ -453,6 +453,9 @@ async fn post_access(headers: SendHeaders, conn: DbConn, nt: Notify<'_>) -> Json
let Some(send) = Send::find_by_uuid(&headers.send_id, &conn).await else {
err_code!(SEND_INACCESSIBLE_MSG, 404)
};
if !send.is_accessible() {
err_code!(SEND_INACCESSIBLE_MSG, 404)
}
process_access(send, conn, nt).await
}
@@ -471,6 +474,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)
};
@@ -481,17 +486,7 @@ async fn post_access_legacy(
err_code!(SEND_INACCESSIBLE_MSG, 404);
}
if let Some(expiration) = send.expiration_date
&& Utc::now().naive_utc() >= expiration
{
err_code!(SEND_INACCESSIBLE_MSG, 404)
}
if Utc::now().naive_utc() >= send.deletion_date {
err_code!(SEND_INACCESSIBLE_MSG, 404)
}
if send.disabled {
if !send.is_accessible() {
err_code!(SEND_INACCESSIBLE_MSG, 404)
}
@@ -505,11 +500,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
}
@@ -537,6 +534,9 @@ async fn post_access_file(
let Some(send) = Send::find_by_uuid(&headers.send_id, &conn).await else {
err_code!(SEND_INACCESSIBLE_MSG, 404)
};
if !send.is_accessible() {
err_code!(SEND_INACCESSIBLE_MSG, 404)
}
process_access_file(send, file_id, host, conn, nt).await
}
@@ -548,8 +548,11 @@ async fn post_access_file_legacy(
data: Json<SendAccessData>,
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)
};
@@ -560,17 +563,7 @@ async fn post_access_file_legacy(
err_code!(SEND_INACCESSIBLE_MSG, 404)
}
if let Some(expiration) = send.expiration_date
&& Utc::now().naive_utc() >= expiration
{
err_code!(SEND_INACCESSIBLE_MSG, 404)
}
if Utc::now().naive_utc() >= send.deletion_date {
err_code!(SEND_INACCESSIBLE_MSG, 404)
}
if send.disabled {
if !send.is_accessible() {
err_code!(SEND_INACCESSIBLE_MSG, 404)
}
@@ -582,9 +575,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
}
+16
View File
@@ -405,6 +405,22 @@ async fn get_page(url: &str) -> Result<Response, Error> {
}
async fn get_page_with_referer(url: &str, referer: &str) -> Result<Response, Error> {
// 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);
+4
View File
@@ -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 = "<data>")]
async fn register_verification_email(
data: Json<RegisterVerificationData>,
ip: ClientIp,
conn: DbConn,
) -> ApiResult<RegisterVerificationResponse> {
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
+60 -10
View File
@@ -33,9 +33,14 @@ pub static WS_USERS: LazyLock<Arc<WebSocketUsers>> = LazyLock::new(|| {
pub static WS_ANONYMOUS_SUBSCRIPTIONS: LazyLock<Arc<AnonymousWebSocketSubscriptions>> = 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<bool> = LazyLock::new(|| !CONFIG.enable_websocket() && !CONFIG.push_enabled());
pub fn routes() -> Vec<Route> {
@@ -82,14 +87,21 @@ impl Drop for WSEntryMapGuard {
struct WSAnonymousEntryMapGuard {
subscriptions: Arc<AnonymousWebSocketSubscriptions>,
token: String,
entry_uuid: uuid::Uuid,
addr: IpAddr,
}
impl WSAnonymousEntryMapGuard {
fn new(subscriptions: Arc<AnonymousWebSocketSubscriptions>, token: String, addr: IpAddr) -> Self {
fn new(
subscriptions: Arc<AnonymousWebSocketSubscriptions>,
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::<Message>(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<dashmap::DashMap<String, Sender<Message>>>,
map: Arc<dashmap::DashMap<String, Vec<UserSenders>>>,
connections: Arc<dashmap::DashMap<IpAddr, u32>>,
}
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}");
}
}
}