From 8ee5d51bd47279d5b23c409744fab6614af0e918 Mon Sep 17 00:00:00 2001 From: Jeremy Lin Date: Sat, 10 Jul 2021 01:20:37 -0700 Subject: [PATCH 1/2] Disable `show_password_hint` by default A setting that provides unauthenticated access to potentially sensitive data shouldn't be enabled by default. --- .env.template | 6 ++++-- src/config.rs | 7 ++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.env.template b/.env.template index 530a6a01..1662080e 100644 --- a/.env.template +++ b/.env.template @@ -210,8 +210,10 @@ ## The change only applies when the password is changed # PASSWORD_ITERATIONS=100000 -## Whether password hint should be sent into the error response when the client request it -# SHOW_PASSWORD_HINT=true +## Controls whether a password hint should be shown directly in the web page if +## SMTP service is not configured. Not recommended for publicly-accessible instances +## as this provides unauthenticated access to potentially sensitive data. +# SHOW_PASSWORD_HINT=false ## Domain settings ## The domain must match the address from where you access the server diff --git a/src/config.rs b/src/config.rs index 6b4fce59..347b0c5e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -388,9 +388,10 @@ make_config! { /// Password iterations |> Number of server-side passwords hashing iterations. /// The changes only apply when a user changes their password. Not recommended to lower the value password_iterations: i32, true, def, 100_000; - /// Show password hints |> Controls if the password hint should be shown directly in the web page. - /// Otherwise, if email is disabled, there is no way to see the password hint - show_password_hint: bool, true, def, true; + /// Show password hint |> Controls whether a password hint should be shown directly in the web page + /// if SMTP service is not configured. Not recommended for publicly-accessible instances as this + /// provides unauthenticated access to potentially sensitive data. + show_password_hint: bool, true, def, false; /// Admin page token |> The token used to authenticate in this very same page. Changing it here won't deauthorize the current session admin_token: Pass, true, option; From 88bea44dd81c6fc9755d42d9bee2533db8765c2a Mon Sep 17 00:00:00 2001 From: Jeremy Lin Date: Sat, 10 Jul 2021 01:21:27 -0700 Subject: [PATCH 2/2] Prevent user enumeration via password hints When `show_password_hint` is enabled but mail is not configured, the previous implementation returned a differentiable response for non-existent email addresses. Even if mail is enabled, there is a timing side channel since mail is sent synchronously. Add a randomized sleep to mitigate this somewhat. --- src/api/core/accounts.rs | 53 ++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 3888075b..b0c17cfe 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -576,24 +576,45 @@ struct PasswordHintData { #[post("/accounts/password-hint", data = "")] fn password_hint(data: JsonUpcase, conn: DbConn) -> EmptyResult { - let data: PasswordHintData = data.into_inner().data; - - let hint = match User::find_by_mail(&data.Email, &conn) { - Some(user) => user.password_hint, - None => return Ok(()), - }; - - if CONFIG.mail_enabled() { - mail::send_password_hint(&data.Email, hint)?; - } else if CONFIG.show_password_hint() { - if let Some(hint) = hint { - err!(format!("Your password hint is: {}", &hint)); - } else { - err!("Sorry, you have no password hint..."); - } + if !CONFIG.mail_enabled() && !CONFIG.show_password_hint() { + err!("This server is not configured to provide password hints."); } - Ok(()) + const NO_HINT: &str = "Sorry, you have no password hint..."; + + let data: PasswordHintData = data.into_inner().data; + let email = &data.Email; + + match User::find_by_mail(email, &conn) { + None => { + // To prevent user enumeration, act as if the user exists. + if CONFIG.mail_enabled() { + // There is still a timing side channel here in that the code + // paths that send mail take noticeably longer than ones that + // don't. Add a randomized sleep to mitigate this somewhat. + use rand::{thread_rng, Rng}; + let mut rng = thread_rng(); + let base = 1000; + let delta: i32 = 100; + let sleep_ms = (base + rng.gen_range(-delta..=delta)) as u64; + std::thread::sleep(std::time::Duration::from_millis(sleep_ms)); + Ok(()) + } else { + err!(NO_HINT); + } + } + Some(user) => { + let hint: Option = user.password_hint; + if CONFIG.mail_enabled() { + mail::send_password_hint(email, hint)?; + Ok(()) + } else if let Some(hint) = hint { + err!(format!("Your password hint is: {}", hint)); + } else { + err!(NO_HINT); + } + } + } } #[derive(Deserialize)]