Improved HTTP client (#4740)

* Improved HTTP client

* Change config compat to use auto, rename blacklist

* Fix wrong doc references

.env.template

@@ -320,15 +320,15 @@
 ## The default is 10 seconds, but this could be to low on slower network connections
 # ICON_DOWNLOAD_TIMEOUT=10
 
-## Icon blacklist Regex
-## Any domains or IPs that match this regex won't be fetched by the icon service.
+## Block HTTP domains/IPs by Regex
+## Any domains or IPs that match this regex won't be fetched by the internal HTTP client.
 ## Useful to hide other servers in the local network. Check the WIKI for more details
 ## NOTE: Always enclose this regex withing single quotes!
-# ICON_BLACKLIST_REGEX='^(192\.168\.0\.[0-9]+|192\.168\.1\.[0-9]+)$'
+# HTTP_REQUEST_BLOCK_REGEX='^(192\.168\.0\.[0-9]+|192\.168\.1\.[0-9]+)$'
 
-## Any IP which is not defined as a global IP will be blacklisted.
+## Enabling this will cause the internal HTTP client to refuse to connect to any non global IP address.
 ## Useful to secure your internal environment: See https://en.wikipedia.org/wiki/Reserved_IP_addresses for a list of IPs which it will block
-# ICON_BLACKLIST_NON_GLOBAL_IPS=true
+# HTTP_REQUEST_BLOCK_NON_GLOBAL_IPS=true
 
 ## Client Settings
 ## Enable experimental feature flags for clients.

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<T: DeserializeOwned>(url: &str) -> Result<T, Error> {
-    let json_api = get_reqwest_client();
-
-    Ok(json_api.get(url).send().await?.error_for_status()?.json::<T>().await?)
+    Ok(make_http_request(Method::GET, url)?.send().await?.error_for_status()?.json::<T>().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,
     }

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<Route> {
@@ -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 {

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)

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<Client> = 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<Redirect> {
         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<u8>)> {
         );
     }
 
+    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<Option<(String, Regex)>> = 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<u8>, String)> {
     let path = format!("{}/{}.png", CONFIG.icon_cache_folder(), domain);
 
@@ -195,9 +181,9 @@ async fn get_icon(domain: &str) -> Option<(Vec<u8>, 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<IconUrlResult, Error> {
 
     // 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(_) => {

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

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<String> {
         ("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(&params)
         .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<String>) -> 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)

src/config.rs

@@ -146,6 +146,12 @@ macro_rules! make_config {
                 config.signups_domains_whitelist = config.signups_domains_whitelist.trim().to_lowercase();
                 config.org_creation_users = config.org_creation_users.trim().to_lowercase();
 
+
+                // Copy the values from the deprecated flags to the new ones
+                if config.http_request_block_regex.is_none() {
+                    config.http_request_block_regex = config.icon_blacklist_regex.clone();
+                }
+
                 config
             }
         }
@@ -531,12 +537,18 @@ make_config! {
         icon_cache_negttl:      u64,    true,   def,    259_200;
         /// Icon download timeout |> Number of seconds when to stop attempting to download an icon.
         icon_download_timeout:  u64,    true,   def,    10;
-        /// Icon blacklist Regex |> Any domains or IPs that match this regex won't be fetched by the icon service.
+
+        /// [Deprecated] Icon blacklist Regex |> Use `http_request_block_regex` instead
+        icon_blacklist_regex:   String, false,   option;
+        /// [Deprecated] Icon blacklist non global IPs |> Use `http_request_block_non_global_ips` instead
+        icon_blacklist_non_global_ips:  bool,   false,   def, true;
+
+        /// Block HTTP domains/IPs by Regex |> Any domains or IPs that match this regex won't be fetched by the internal HTTP client.
         /// Useful to hide other servers in the local network. Check the WIKI for more details
-        icon_blacklist_regex:   String, true,   option;
-        /// Icon blacklist non global IPs |> Any IP which is not defined as a global IP will be blacklisted.
+        http_request_block_regex:   String, true,   option;
+        /// Block non global IPs |> Enabling this will cause the internal HTTP client to refuse to connect to any non global IP address.
         /// Useful to secure your internal environment: See https://en.wikipedia.org/wiki/Reserved_IP_addresses for a list of IPs which it will block
-        icon_blacklist_non_global_ips:  bool,   true,   def,    true;
+        http_request_block_non_global_ips:  bool,   true,   auto, |c| c.icon_blacklist_non_global_ips;
 
         /// Disable Two-Factor remember |> Enabling this would force the users to use a second factor to login every time.
         /// Note that the checkbox would still be present, but ignored.
@@ -899,12 +911,12 @@ fn validate_config(cfg: &ConfigItems) -> Result<(), Error> {
         err!("To use email 2FA as automatic fallback, email 2fa has to be enabled!");
     }
 
-    // Check if the icon blacklist regex is valid
-    if let Some(ref r) = cfg.icon_blacklist_regex {
+    // Check if the HTTP request block regex is valid
+    if let Some(ref r) = cfg.http_request_block_regex {
         let validate_regex = regex::Regex::new(r);
         match validate_regex {
             Ok(_) => (),
-            Err(e) => err!(format!("`ICON_BLACKLIST_REGEX` is invalid: {e:#?}")),
+            Err(e) => err!(format!("`HTTP_REQUEST_BLOCK_REGEX` is invalid: {e:#?}")),
         }
     }
 

src/error.rs

@@ -2,6 +2,7 @@
 // Error generator macro
 //
 use crate::db::models::EventType;
+use crate::http_client::CustomHttpClientError;
 use std::error::Error as StdError;
 
 macro_rules! make_error {
@@ -68,6 +69,10 @@ make_error! {
     Empty(Empty):     _no_source, _serialize,
     // Used to represent err! calls
     Simple(String):  _no_source,  _api_error,
+
+    // Used in our custom http client to handle non-global IPs and blocked domains
+    CustomHttpClient(CustomHttpClientError): _has_source, _api_error,
+
     // Used for special return values, like 2FA errors
     Json(Value):     _no_source,  _serialize,
     Db(DieselErr):   _has_source, _api_error,