Browse Source

Security fixes for admin and sendmail

Because the Vaultwarden Admin Backend endpoints did not validated the Content-Type during a request, it was possible to update settings via CSRF. But, this was only possible if there was no `ADMIN_TOKEN` set at all. To make sure these environments are also safe I added the needed content-type checks at the functions.
This could cause some users who have scripts which uses cURL for example to adjust there commands to provide the correct headers.

By using a crafted favicon and having access to the Admin Backend an attacker could run custom commands on the host/container where Vaultwarden is running on. The main issue here is that we allowed the sendmail binary name/path to be changed. To mitigate this we removed this configuration item and only then `sendmail` binary as a name can be used.
This could cause some issues where the `sendmail` binary is not in the `$PATH` and thus not able to be started. In these cases the admins should make sure `$PATH` is set correctly or create a custom shell script or symlink at a location which is in the `$PATH`.

Added an extra security header and adjusted the CSP to be more strict by setting `default-src` to `none` and added the needed missing specific policies.

Also created a general email validation function which does some more checking to catch invalid email address not found by the email_address crate.

Signed-off-by: BlackDex <black.dex@gmail.com>
pull/5438/head
BlackDex 3 months ago
parent
commit
08fb368d5f
No known key found for this signature in database GPG Key ID: 58C80A2AA6C765E1
  1. 2
      .env.template
  2. 32
      src/api/admin.rs
  3. 30
      src/config.rs
  4. 5
      src/db/models/organization.rs
  5. 8
      src/db/models/user.rs
  6. 15
      src/mail.rs
  7. 5
      src/static/scripts/admin_diagnostics.js
  8. 23
      src/util.rs

2
.env.template

@ -534,8 +534,6 @@
# Whether to send mail via the `sendmail` command # Whether to send mail via the `sendmail` command
# USE_SENDMAIL=false # USE_SENDMAIL=false
# Which sendmail command to use. The one found in the $PATH is used if not specified.
# SENDMAIL_COMMAND="/path/to/sendmail"
## Defaults for SSL is "Plain" and "Login" and nothing for Non-SSL connections. ## Defaults for SSL is "Plain" and "Login" and nothing for Non-SSL connections.
## Possible values: ["Plain", "Login", "Xoauth2"]. ## Possible values: ["Plain", "Login", "Xoauth2"].

32
src/api/admin.rs

@ -171,7 +171,7 @@ struct LoginForm {
redirect: Option<String>, redirect: Option<String>,
} }
#[post("/", data = "<data>")] #[post("/", format = "application/x-www-form-urlencoded", data = "<data>")]
fn post_admin_login( fn post_admin_login(
data: Form<LoginForm>, data: Form<LoginForm>,
cookies: &CookieJar<'_>, cookies: &CookieJar<'_>,
@ -289,7 +289,7 @@ async fn get_user_or_404(user_id: &UserId, conn: &mut DbConn) -> ApiResult<User>
} }
} }
#[post("/invite", data = "<data>")] #[post("/invite", format = "application/json", data = "<data>")]
async fn invite_user(data: Json<InviteData>, _token: AdminToken, mut conn: DbConn) -> JsonResult { async fn invite_user(data: Json<InviteData>, _token: AdminToken, mut conn: DbConn) -> JsonResult {
let data: InviteData = data.into_inner(); let data: InviteData = data.into_inner();
if User::find_by_mail(&data.email, &mut conn).await.is_some() { if User::find_by_mail(&data.email, &mut conn).await.is_some() {
@ -315,7 +315,7 @@ async fn invite_user(data: Json<InviteData>, _token: AdminToken, mut conn: DbCon
Ok(Json(user.to_json(&mut conn).await)) Ok(Json(user.to_json(&mut conn).await))
} }
#[post("/test/smtp", data = "<data>")] #[post("/test/smtp", format = "application/json", data = "<data>")]
async fn test_smtp(data: Json<InviteData>, _token: AdminToken) -> EmptyResult { async fn test_smtp(data: Json<InviteData>, _token: AdminToken) -> EmptyResult {
let data: InviteData = data.into_inner(); let data: InviteData = data.into_inner();
@ -393,7 +393,7 @@ async fn get_user_json(user_id: UserId, _token: AdminToken, mut conn: DbConn) ->
Ok(Json(usr)) Ok(Json(usr))
} }
#[post("/users/<user_id>/delete")] #[post("/users/<user_id>/delete", format = "application/json")]
async fn delete_user(user_id: UserId, token: AdminToken, mut conn: DbConn) -> EmptyResult { async fn delete_user(user_id: UserId, token: AdminToken, mut conn: DbConn) -> EmptyResult {
let user = get_user_or_404(&user_id, &mut conn).await?; let user = get_user_or_404(&user_id, &mut conn).await?;
@ -417,7 +417,7 @@ async fn delete_user(user_id: UserId, token: AdminToken, mut conn: DbConn) -> Em
res res
} }
#[post("/users/<user_id>/deauth")] #[post("/users/<user_id>/deauth", format = "application/json")]
async fn deauth_user(user_id: UserId, _token: AdminToken, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult { async fn deauth_user(user_id: UserId, _token: AdminToken, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult {
let mut user = get_user_or_404(&user_id, &mut conn).await?; let mut user = get_user_or_404(&user_id, &mut conn).await?;
@ -438,7 +438,7 @@ async fn deauth_user(user_id: UserId, _token: AdminToken, mut conn: DbConn, nt:
user.save(&mut conn).await user.save(&mut conn).await
} }
#[post("/users/<user_id>/disable")] #[post("/users/<user_id>/disable", format = "application/json")]
async fn disable_user(user_id: UserId, _token: AdminToken, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult { async fn disable_user(user_id: UserId, _token: AdminToken, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult {
let mut user = get_user_or_404(&user_id, &mut conn).await?; let mut user = get_user_or_404(&user_id, &mut conn).await?;
Device::delete_all_by_user(&user.uuid, &mut conn).await?; Device::delete_all_by_user(&user.uuid, &mut conn).await?;
@ -452,7 +452,7 @@ async fn disable_user(user_id: UserId, _token: AdminToken, mut conn: DbConn, nt:
save_result save_result
} }
#[post("/users/<user_id>/enable")] #[post("/users/<user_id>/enable", format = "application/json")]
async fn enable_user(user_id: UserId, _token: AdminToken, mut conn: DbConn) -> EmptyResult { async fn enable_user(user_id: UserId, _token: AdminToken, mut conn: DbConn) -> EmptyResult {
let mut user = get_user_or_404(&user_id, &mut conn).await?; let mut user = get_user_or_404(&user_id, &mut conn).await?;
user.enabled = true; user.enabled = true;
@ -460,7 +460,7 @@ async fn enable_user(user_id: UserId, _token: AdminToken, mut conn: DbConn) -> E
user.save(&mut conn).await user.save(&mut conn).await
} }
#[post("/users/<user_id>/remove-2fa")] #[post("/users/<user_id>/remove-2fa", format = "application/json")]
async fn remove_2fa(user_id: UserId, token: AdminToken, mut conn: DbConn) -> EmptyResult { async fn remove_2fa(user_id: UserId, token: AdminToken, mut conn: DbConn) -> EmptyResult {
let mut user = get_user_or_404(&user_id, &mut conn).await?; let mut user = get_user_or_404(&user_id, &mut conn).await?;
TwoFactor::delete_all_by_user(&user.uuid, &mut conn).await?; TwoFactor::delete_all_by_user(&user.uuid, &mut conn).await?;
@ -469,7 +469,7 @@ async fn remove_2fa(user_id: UserId, token: AdminToken, mut conn: DbConn) -> Emp
user.save(&mut conn).await user.save(&mut conn).await
} }
#[post("/users/<user_id>/invite/resend")] #[post("/users/<user_id>/invite/resend", format = "application/json")]
async fn resend_user_invite(user_id: UserId, _token: AdminToken, mut conn: DbConn) -> EmptyResult { async fn resend_user_invite(user_id: UserId, _token: AdminToken, mut conn: DbConn) -> EmptyResult {
if let Some(user) = User::find_by_uuid(&user_id, &mut conn).await { if let Some(user) = User::find_by_uuid(&user_id, &mut conn).await {
//TODO: replace this with user.status check when it will be available (PR#3397) //TODO: replace this with user.status check when it will be available (PR#3397)
@ -496,7 +496,7 @@ struct MembershipTypeData {
org_uuid: OrganizationId, org_uuid: OrganizationId,
} }
#[post("/users/org_type", data = "<data>")] #[post("/users/org_type", format = "application/json", data = "<data>")]
async fn update_membership_type(data: Json<MembershipTypeData>, token: AdminToken, mut conn: DbConn) -> EmptyResult { async fn update_membership_type(data: Json<MembershipTypeData>, token: AdminToken, mut conn: DbConn) -> EmptyResult {
let data: MembershipTypeData = data.into_inner(); let data: MembershipTypeData = data.into_inner();
@ -550,7 +550,7 @@ async fn update_membership_type(data: Json<MembershipTypeData>, token: AdminToke
member_to_edit.save(&mut conn).await member_to_edit.save(&mut conn).await
} }
#[post("/users/update_revision")] #[post("/users/update_revision", format = "application/json")]
async fn update_revision_users(_token: AdminToken, mut conn: DbConn) -> EmptyResult { async fn update_revision_users(_token: AdminToken, mut conn: DbConn) -> EmptyResult {
User::update_all_revisions(&mut conn).await User::update_all_revisions(&mut conn).await
} }
@ -575,7 +575,7 @@ async fn organizations_overview(_token: AdminToken, mut conn: DbConn) -> ApiResu
Ok(Html(text)) Ok(Html(text))
} }
#[post("/organizations/<org_id>/delete")] #[post("/organizations/<org_id>/delete", format = "application/json")]
async fn delete_organization(org_id: OrganizationId, _token: AdminToken, mut conn: DbConn) -> EmptyResult { async fn delete_organization(org_id: OrganizationId, _token: AdminToken, mut conn: DbConn) -> EmptyResult {
let org = Organization::find_by_uuid(&org_id, &mut conn).await.map_res("Organization doesn't exist")?; let org = Organization::find_by_uuid(&org_id, &mut conn).await.map_res("Organization doesn't exist")?;
org.delete(&mut conn).await org.delete(&mut conn).await
@ -733,7 +733,7 @@ async fn diagnostics(_token: AdminToken, ip_header: IpHeader, mut conn: DbConn)
Ok(Html(text)) Ok(Html(text))
} }
#[get("/diagnostics/config")] #[get("/diagnostics/config", format = "application/json")]
fn get_diagnostics_config(_token: AdminToken) -> Json<Value> { fn get_diagnostics_config(_token: AdminToken) -> Json<Value> {
let support_json = CONFIG.get_support_json(); let support_json = CONFIG.get_support_json();
Json(support_json) Json(support_json)
@ -744,7 +744,7 @@ fn get_diagnostics_http(code: u16, _token: AdminToken) -> EmptyResult {
err_code!(format!("Testing error {code} response"), code); err_code!(format!("Testing error {code} response"), code);
} }
#[post("/config", data = "<data>")] #[post("/config", format = "application/json", data = "<data>")]
fn post_config(data: Json<ConfigBuilder>, _token: AdminToken) -> EmptyResult { fn post_config(data: Json<ConfigBuilder>, _token: AdminToken) -> EmptyResult {
let data: ConfigBuilder = data.into_inner(); let data: ConfigBuilder = data.into_inner();
if let Err(e) = CONFIG.update_config(data) { if let Err(e) = CONFIG.update_config(data) {
@ -753,7 +753,7 @@ fn post_config(data: Json<ConfigBuilder>, _token: AdminToken) -> EmptyResult {
Ok(()) Ok(())
} }
#[post("/config/delete")] #[post("/config/delete", format = "application/json")]
fn delete_config(_token: AdminToken) -> EmptyResult { fn delete_config(_token: AdminToken) -> EmptyResult {
if let Err(e) = CONFIG.delete_user_config() { if let Err(e) = CONFIG.delete_user_config() {
err!(format!("Unable to delete config: {e:?}")) err!(format!("Unable to delete config: {e:?}"))
@ -761,7 +761,7 @@ fn delete_config(_token: AdminToken) -> EmptyResult {
Ok(()) Ok(())
} }
#[post("/config/backup_db")] #[post("/config/backup_db", format = "application/json")]
async fn backup_db(_token: AdminToken, mut conn: DbConn) -> ApiResult<String> { async fn backup_db(_token: AdminToken, mut conn: DbConn) -> ApiResult<String> {
if *CAN_BACKUP { if *CAN_BACKUP {
match backup_database(&mut conn).await { match backup_database(&mut conn).await {

30
src/config.rs

@ -1,8 +1,10 @@
use std::env::consts::EXE_SUFFIX; use std::{
use std::process::exit; env::consts::EXE_SUFFIX,
use std::sync::{ process::exit,
sync::{
atomic::{AtomicBool, Ordering}, atomic::{AtomicBool, Ordering},
RwLock, RwLock,
},
}; };
use job_scheduler_ng::Schedule; use job_scheduler_ng::Schedule;
@ -12,7 +14,7 @@ use reqwest::Url;
use crate::{ use crate::{
db::DbConnType, db::DbConnType,
error::Error, error::Error,
util::{get_env, get_env_bool, get_web_vault_version, parse_experimental_client_feature_flags}, util::{get_env, get_env_bool, get_web_vault_version, is_valid_email, parse_experimental_client_feature_flags},
}; };
static CONFIG_FILE: Lazy<String> = Lazy::new(|| { static CONFIG_FILE: Lazy<String> = Lazy::new(|| {
@ -676,8 +678,6 @@ make_config! {
_enable_smtp: bool, true, def, true; _enable_smtp: bool, true, def, true;
/// Use Sendmail |> Whether to send mail via the `sendmail` command /// Use Sendmail |> Whether to send mail via the `sendmail` command
use_sendmail: bool, true, def, false; use_sendmail: bool, true, def, false;
/// Sendmail Command |> Which sendmail command to use. The one found in the $PATH is used if not specified.
sendmail_command: String, true, option;
/// Host /// Host
smtp_host: String, true, option; smtp_host: String, true, option;
/// DEPRECATED smtp_ssl |> DEPRECATED - Please use SMTP_SECURITY /// DEPRECATED smtp_ssl |> DEPRECATED - Please use SMTP_SECURITY
@ -890,16 +890,12 @@ fn validate_config(cfg: &ConfigItems) -> Result<(), Error> {
} }
if cfg.use_sendmail { if cfg.use_sendmail {
let command = cfg.sendmail_command.clone().unwrap_or_else(|| format!("sendmail{EXE_SUFFIX}")); let command = format!("sendmail{EXE_SUFFIX}");
let mut path = std::path::PathBuf::from(&command);
if !path.is_absolute() { // Check if we can find the sendmail command to execute
match which::which(&command) { let Ok(path) = which::which(&command) else {
Ok(result) => path = result, err!(format!("sendmail command {command} not found in $PATH"))
Err(_) => err!(format!("sendmail command {command:?} not found in $PATH")), };
}
}
match path.metadata() { match path.metadata() {
Err(err) if err.kind() == std::io::ErrorKind::NotFound => { Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
@ -932,8 +928,8 @@ fn validate_config(cfg: &ConfigItems) -> Result<(), Error> {
} }
} }
if (cfg.smtp_host.is_some() || cfg.use_sendmail) && !cfg.smtp_from.contains('@') { if !is_valid_email(&cfg.smtp_from) {
err!("SMTP_FROM does not contain a mandatory @ sign") err!(format!("SMTP_FROM '{}' is not a valid email address", cfg.smtp_from))
} }
if cfg._enable_email_2fa && cfg.email_token_size < 6 { if cfg._enable_email_2fa && cfg.email_token_size < 6 {

5
src/db/models/organization.rs

@ -152,6 +152,7 @@ impl PartialOrd<MembershipType> for i32 {
/// Local methods /// Local methods
impl Organization { impl Organization {
pub fn new(name: String, billing_email: String, private_key: Option<String>, public_key: Option<String>) -> Self { pub fn new(name: String, billing_email: String, private_key: Option<String>, public_key: Option<String>) -> Self {
let billing_email = billing_email.to_lowercase();
Self { Self {
uuid: OrganizationId(crate::util::get_uuid()), uuid: OrganizationId(crate::util::get_uuid()),
name, name,
@ -307,8 +308,8 @@ use crate::error::MapResult;
/// Database methods /// Database methods
impl Organization { impl Organization {
pub async fn save(&self, conn: &mut DbConn) -> EmptyResult { pub async fn save(&self, conn: &mut DbConn) -> EmptyResult {
if !email_address::EmailAddress::is_valid(self.billing_email.trim()) { if !crate::util::is_valid_email(&self.billing_email) {
err!(format!("BillingEmail {} is not a valid email address", self.billing_email.trim())) err!(format!("BillingEmail {} is not a valid email address", self.billing_email))
} }
for member in Membership::find_by_org(&self.uuid, conn).await.iter() { for member in Membership::find_by_org(&self.uuid, conn).await.iter() {

8
src/db/models/user.rs

@ -267,8 +267,8 @@ impl User {
} }
pub async fn save(&mut self, conn: &mut DbConn) -> EmptyResult { pub async fn save(&mut self, conn: &mut DbConn) -> EmptyResult {
if self.email.trim().is_empty() { if !crate::util::is_valid_email(&self.email) {
err!("User email can't be empty") err!(format!("User email {} is not a valid email address", self.email))
} }
self.updated_at = Utc::now().naive_utc(); self.updated_at = Utc::now().naive_utc();
@ -408,8 +408,8 @@ impl Invitation {
} }
pub async fn save(&self, conn: &mut DbConn) -> EmptyResult { pub async fn save(&self, conn: &mut DbConn) -> EmptyResult {
if self.email.trim().is_empty() { if !crate::util::is_valid_email(&self.email) {
err!("Invitation email can't be empty") err!(format!("Invitation email {} is not a valid email address", self.email))
} }
db_run! {conn: db_run! {conn:

15
src/mail.rs

@ -1,7 +1,6 @@
use std::str::FromStr;
use chrono::NaiveDateTime; use chrono::NaiveDateTime;
use percent_encoding::{percent_encode, NON_ALPHANUMERIC}; use percent_encoding::{percent_encode, NON_ALPHANUMERIC};
use std::{env::consts::EXE_SUFFIX, str::FromStr};
use lettre::{ use lettre::{
message::{Attachment, Body, Mailbox, Message, MultiPart, SinglePart}, message::{Attachment, Body, Mailbox, Message, MultiPart, SinglePart},
@ -23,11 +22,7 @@ use crate::{
}; };
fn sendmail_transport() -> AsyncSendmailTransport<Tokio1Executor> { fn sendmail_transport() -> AsyncSendmailTransport<Tokio1Executor> {
if let Some(command) = CONFIG.sendmail_command() { AsyncSendmailTransport::new_with_command(format!("sendmail{EXE_SUFFIX}"))
AsyncSendmailTransport::new_with_command(command)
} else {
AsyncSendmailTransport::new()
}
} }
fn smtp_transport() -> AsyncSmtpTransport<Tokio1Executor> { fn smtp_transport() -> AsyncSmtpTransport<Tokio1Executor> {
@ -595,13 +590,13 @@ async fn send_with_selected_transport(email: Message) -> EmptyResult {
// Match some common errors and make them more user friendly // Match some common errors and make them more user friendly
Err(e) => { Err(e) => {
if e.is_client() { if e.is_client() {
debug!("Sendmail client error: {:#?}", e); debug!("Sendmail client error: {:?}", e);
err!(format!("Sendmail client error: {e}")); err!(format!("Sendmail client error: {e}"));
} else if e.is_response() { } else if e.is_response() {
debug!("Sendmail response error: {:#?}", e); debug!("Sendmail response error: {:?}", e);
err!(format!("Sendmail response error: {e}")); err!(format!("Sendmail response error: {e}"));
} else { } else {
debug!("Sendmail error: {:#?}", e); debug!("Sendmail error: {:?}", e);
err!(format!("Sendmail error: {e}")); err!(format!("Sendmail error: {e}"));
} }
} }

5
src/static/scripts/admin_diagnostics.js

@ -236,8 +236,11 @@ function checkSecurityHeaders(headers, omit) {
"referrer-policy": ["same-origin"], "referrer-policy": ["same-origin"],
"x-xss-protection": ["0"], "x-xss-protection": ["0"],
"x-robots-tag": ["noindex", "nofollow"], "x-robots-tag": ["noindex", "nofollow"],
"cross-origin-resource-policy": ["same-origin"],
"content-security-policy": [ "content-security-policy": [
"default-src 'self'", "default-src 'none'",
"font-src 'self'",
"manifest-src 'self'",
"base-uri 'self'", "base-uri 'self'",
"form-action 'self'", "form-action 'self'",
"object-src 'self' blob:", "object-src 'self' blob:",

23
src/util.rs

@ -55,6 +55,8 @@ impl Fairing for AppHeaders {
res.set_raw_header("Referrer-Policy", "same-origin"); res.set_raw_header("Referrer-Policy", "same-origin");
res.set_raw_header("X-Content-Type-Options", "nosniff"); res.set_raw_header("X-Content-Type-Options", "nosniff");
res.set_raw_header("X-Robots-Tag", "noindex, nofollow"); res.set_raw_header("X-Robots-Tag", "noindex, nofollow");
res.set_raw_header("Cross-Origin-Resource-Policy", "same-origin");
// Obsolete in modern browsers, unsafe (XS-Leak), and largely replaced by CSP // Obsolete in modern browsers, unsafe (XS-Leak), and largely replaced by CSP
res.set_raw_header("X-XSS-Protection", "0"); res.set_raw_header("X-XSS-Protection", "0");
@ -74,7 +76,9 @@ impl Fairing for AppHeaders {
// # Mail Relay: https://bitwarden.com/blog/add-privacy-and-security-using-email-aliases-with-bitwarden/ // # Mail Relay: https://bitwarden.com/blog/add-privacy-and-security-using-email-aliases-with-bitwarden/
// app.simplelogin.io, app.addy.io, api.fastmail.com, quack.duckduckgo.com // app.simplelogin.io, app.addy.io, api.fastmail.com, quack.duckduckgo.com
let csp = format!( let csp = format!(
"default-src 'self'; \ "default-src 'none'; \
font-src 'self'; \
manifest-src 'self'; \
base-uri 'self'; \ base-uri 'self'; \
form-action 'self'; \ form-action 'self'; \
object-src 'self' blob:; \ object-src 'self' blob:; \
@ -461,6 +465,23 @@ pub fn parse_date(date: &str) -> NaiveDateTime {
DateTime::parse_from_rfc3339(date).unwrap().naive_utc() DateTime::parse_from_rfc3339(date).unwrap().naive_utc()
} }
/// Returns true or false if an email address is valid or not
///
/// Some extra checks instead of only using email_address
/// This prevents from weird email formats still excepted but in the end invalid
pub fn is_valid_email(email: &str) -> bool {
let Ok(email) = email_address::EmailAddress::from_str(email) else {
return false;
};
let Ok(email_url) = url::Url::parse(&format!("https://{}", email.domain())) else {
return false;
};
if email_url.path().ne("/") || email_url.domain().is_none() || email_url.query().is_some() {
return false;
}
true
}
// //
// Deployment environment methods // Deployment environment methods
// //

Loading…
Cancel
Save