From 035f694d2f94df5203bec6c0af951f78fcc888c2 Mon Sep 17 00:00:00 2001 From: Daniel García Date: Fri, 12 Jul 2024 22:33:11 +0200 Subject: Improved HTTP client (#4740) * Improved HTTP client * Change config compat to use auto, rename blacklist * Fix wrong doc references --- src/api/admin.rs | 17 ++++++++------- src/api/core/mod.rs | 8 +++---- src/api/core/two_factor/duo.rs | 7 ++---- src/api/icons.rs | 48 +++++++++++++++--------------------------- src/api/mod.rs | 2 +- src/api/push.rs | 27 +++++++++++++++--------- 6 files changed, 50 insertions(+), 59 deletions(-) (limited to 'src/api') diff --git a/src/api/admin.rs b/src/api/admin.rs index 58a056b6..1ea9aa59 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -1,4 +1,5 @@ use once_cell::sync::Lazy; +use reqwest::Method; use serde::de::DeserializeOwned; use serde_json::Value; use std::env; @@ -21,10 +22,10 @@ use crate::{ config::ConfigBuilder, db::{backup_database, get_sql_server_version, models::*, DbConn, DbConnType}, error::{Error, MapResult}, + http_client::make_http_request, mail, util::{ - container_base_image, format_naive_datetime_local, get_display_size, get_reqwest_client, - is_running_in_container, NumberOrString, + container_base_image, format_naive_datetime_local, get_display_size, is_running_in_container, NumberOrString, }, CONFIG, VERSION, }; @@ -594,15 +595,15 @@ struct TimeApi { } async fn get_json_api(url: &str) -> Result { - let json_api = get_reqwest_client(); - - Ok(json_api.get(url).send().await?.error_for_status()?.json::().await?) + Ok(make_http_request(Method::GET, url)?.send().await?.error_for_status()?.json::().await?) } async fn has_http_access() -> bool { - let http_access = get_reqwest_client(); - - match http_access.head("https://github.com/dani-garcia/vaultwarden").send().await { + let req = match make_http_request(Method::HEAD, "https://github.com/dani-garcia/vaultwarden") { + Ok(r) => r, + Err(_) => return false, + }; + match req.send().await { Ok(r) => r.status().is_success(), _ => false, } diff --git a/src/api/core/mod.rs b/src/api/core/mod.rs index 9da0e886..41bd4d6b 100644 --- a/src/api/core/mod.rs +++ b/src/api/core/mod.rs @@ -12,6 +12,7 @@ pub use accounts::purge_auth_requests; pub use ciphers::{purge_trashed_ciphers, CipherData, CipherSyncData, CipherSyncType}; pub use emergency_access::{emergency_notification_reminder_job, emergency_request_timeout_job}; pub use events::{event_cleanup_job, log_event, log_user_event}; +use reqwest::Method; pub use sends::purge_sends; pub fn routes() -> Vec { @@ -53,7 +54,8 @@ use crate::{ auth::Headers, db::DbConn, error::Error, - util::{get_reqwest_client, parse_experimental_client_feature_flags}, + http_client::make_http_request, + util::parse_experimental_client_feature_flags, }; #[derive(Debug, Serialize, Deserialize)] @@ -139,9 +141,7 @@ async fn hibp_breach(username: &str) -> JsonResult { ); if let Some(api_key) = crate::CONFIG.hibp_api_key() { - let hibp_client = get_reqwest_client(); - - let res = hibp_client.get(&url).header("hibp-api-key", api_key).send().await?; + let res = make_http_request(Method::GET, &url)?.header("hibp-api-key", api_key).send().await?; // If we get a 404, return a 404, it means no breached accounts if res.status() == 404 { diff --git a/src/api/core/two_factor/duo.rs b/src/api/core/two_factor/duo.rs index c5bfa9e5..8554999c 100644 --- a/src/api/core/two_factor/duo.rs +++ b/src/api/core/two_factor/duo.rs @@ -15,7 +15,7 @@ use crate::{ DbConn, }, error::MapResult, - util::get_reqwest_client, + http_client::make_http_request, CONFIG, }; @@ -210,10 +210,7 @@ async fn duo_api_request(method: &str, path: &str, params: &str, data: &DuoData) let m = Method::from_str(method).unwrap_or_default(); - let client = get_reqwest_client(); - - client - .request(m, &url) + make_http_request(m, &url)? .basic_auth(username, Some(password)) .header(header::USER_AGENT, "vaultwarden:Duo/1.0 (Rust)") .header(header::DATE, date) diff --git a/src/api/icons.rs b/src/api/icons.rs index 94fab3f8..83f3e9e9 100644 --- a/src/api/icons.rs +++ b/src/api/icons.rs @@ -1,6 +1,6 @@ use std::{ net::IpAddr, - sync::{Arc, Mutex}, + sync::Arc, time::{Duration, SystemTime}, }; @@ -22,7 +22,8 @@ use html5gum::{Emitter, HtmlString, InfallibleTokenizer, Readable, StringReader, use crate::{ error::Error, - util::{get_reqwest_client_builder, Cached, CustomDnsResolver, CustomResolverError}, + http_client::{get_reqwest_client_builder, should_block_address, CustomHttpClientError}, + util::Cached, CONFIG, }; @@ -53,7 +54,6 @@ static CLIENT: Lazy = Lazy::new(|| { .timeout(icon_download_timeout) .pool_max_idle_per_host(5) // Configure the Hyper Pool to only have max 5 idle connections .pool_idle_timeout(pool_idle_timeout) // Configure the Hyper Pool to timeout after 10 seconds - .dns_resolver(CustomDnsResolver::instance()) .default_headers(default_headers.clone()) .build() .expect("Failed to build client") @@ -69,7 +69,8 @@ fn icon_external(domain: &str) -> Option { return None; } - if is_domain_blacklisted(domain) { + if should_block_address(domain) { + warn!("Blocked address: {}", domain); return None; } @@ -99,6 +100,15 @@ async fn icon_internal(domain: &str) -> Cached<(ContentType, Vec)> { ); } + if should_block_address(domain) { + warn!("Blocked address: {}", domain); + return Cached::ttl( + (ContentType::new("image", "png"), FALLBACK_ICON.to_vec()), + CONFIG.icon_cache_negttl(), + true, + ); + } + match get_icon(domain).await { Some((icon, icon_type)) => { Cached::ttl((ContentType::new("image", icon_type), icon), CONFIG.icon_cache_ttl(), true) @@ -144,30 +154,6 @@ fn is_valid_domain(domain: &str) -> bool { true } -pub fn is_domain_blacklisted(domain: &str) -> bool { - let Some(config_blacklist) = CONFIG.icon_blacklist_regex() else { - return false; - }; - - // Compiled domain blacklist - static COMPILED_BLACKLIST: Mutex> = Mutex::new(None); - let mut guard = COMPILED_BLACKLIST.lock().unwrap(); - - // If the stored regex is up to date, use it - if let Some((value, regex)) = &*guard { - if value == &config_blacklist { - return regex.is_match(domain); - } - } - - // If we don't have a regex stored, or it's not up to date, recreate it - let regex = Regex::new(&config_blacklist).unwrap(); - let is_match = regex.is_match(domain); - *guard = Some((config_blacklist, regex)); - - is_match -} - async fn get_icon(domain: &str) -> Option<(Vec, String)> { let path = format!("{}/{}.png", CONFIG.icon_cache_folder(), domain); @@ -195,9 +181,9 @@ async fn get_icon(domain: &str) -> Option<(Vec, String)> { Some((icon.to_vec(), icon_type.unwrap_or("x-icon").to_string())) } Err(e) => { - // If this error comes from the custom resolver, this means this is a blacklisted domain + // If this error comes from the custom resolver, this means this is a blocked domain // or non global IP, don't save the miss file in this case to avoid leaking it - if let Some(error) = CustomResolverError::downcast_ref(&e) { + if let Some(error) = CustomHttpClientError::downcast_ref(&e) { warn!("{error}"); return None; } @@ -353,7 +339,7 @@ async fn get_icon_url(domain: &str) -> Result { // First check the domain as given during the request for HTTPS. let resp = match get_page(&ssldomain).await { - Err(e) if CustomResolverError::downcast_ref(&e).is_none() => { + Err(e) if CustomHttpClientError::downcast_ref(&e).is_none() => { // If we get an error that is not caused by the blacklist, we retry with HTTP match get_page(&httpdomain).await { mut sub_resp @ Err(_) => { diff --git a/src/api/mod.rs b/src/api/mod.rs index d5281bda..27a3775f 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -20,7 +20,7 @@ pub use crate::api::{ core::two_factor::send_incomplete_2fa_notifications, core::{emergency_notification_reminder_job, emergency_request_timeout_job}, core::{event_cleanup_job, events_routes as core_events_routes}, - icons::{is_domain_blacklisted, routes as icons_routes}, + icons::routes as icons_routes, identity::routes as identity_routes, notifications::routes as notifications_routes, notifications::{AnonymousNotify, Notify, UpdateType, WS_ANONYMOUS_SUBSCRIPTIONS, WS_USERS}, diff --git a/src/api/push.rs b/src/api/push.rs index 607fb7ea..eaf304f9 100644 --- a/src/api/push.rs +++ b/src/api/push.rs @@ -1,11 +1,14 @@ -use reqwest::header::{ACCEPT, AUTHORIZATION, CONTENT_TYPE}; +use reqwest::{ + header::{ACCEPT, AUTHORIZATION, CONTENT_TYPE}, + Method, +}; use serde_json::Value; use tokio::sync::RwLock; use crate::{ api::{ApiResult, EmptyResult, UpdateType}, db::models::{Cipher, Device, Folder, Send, User}, - util::get_reqwest_client, + http_client::make_http_request, CONFIG, }; @@ -50,8 +53,7 @@ async fn get_auth_push_token() -> ApiResult { ("client_secret", &client_secret), ]; - let res = match get_reqwest_client() - .post(&format!("{}/connect/token", CONFIG.push_identity_uri())) + let res = match make_http_request(Method::POST, &format!("{}/connect/token", CONFIG.push_identity_uri()))? .form(¶ms) .send() .await @@ -104,8 +106,7 @@ pub async fn register_push_device(device: &mut Device, conn: &mut crate::db::DbC let auth_push_token = get_auth_push_token().await?; let auth_header = format!("Bearer {}", &auth_push_token); - if let Err(e) = get_reqwest_client() - .post(CONFIG.push_relay_uri() + "/push/register") + if let Err(e) = make_http_request(Method::POST, &(CONFIG.push_relay_uri() + "/push/register"))? .header(CONTENT_TYPE, "application/json") .header(ACCEPT, "application/json") .header(AUTHORIZATION, auth_header) @@ -132,8 +133,7 @@ pub async fn unregister_push_device(push_uuid: Option) -> EmptyResult { let auth_header = format!("Bearer {}", &auth_push_token); - match get_reqwest_client() - .delete(CONFIG.push_relay_uri() + "/push/" + &push_uuid.unwrap()) + match make_http_request(Method::DELETE, &(CONFIG.push_relay_uri() + "/push/" + &push_uuid.unwrap()))? .header(AUTHORIZATION, auth_header) .send() .await @@ -266,8 +266,15 @@ async fn send_to_push_relay(notification_data: Value) { let auth_header = format!("Bearer {}", &auth_push_token); - if let Err(e) = get_reqwest_client() - .post(CONFIG.push_relay_uri() + "/push/send") + let req = match make_http_request(Method::POST, &(CONFIG.push_relay_uri() + "/push/send")) { + Ok(r) => r, + Err(e) => { + error!("An error occurred while sending a send update to the push relay: {}", e); + return; + } + }; + + if let Err(e) = req .header(ACCEPT, "application/json") .header(CONTENT_TYPE, "application/json") .header(AUTHORIZATION, &auth_header) -- cgit v1.2.3