diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 67b315603..3093b6a20 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -166,6 +166,7 @@ Pedro Schreiber Foxy_null Arbyste Vasll +laalsaas ******************** The text of the 3 clause BSD license follows: diff --git a/Cargo.lock b/Cargo.lock index 68788cca7..0d6711817 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -132,6 +132,7 @@ dependencies = [ "num_cpus", "num_enum", "once_cell", + "pbkdf2 0.12.2", "percent-encoding-iri", "phf 0.11.2", "pin-project", @@ -3699,6 +3700,17 @@ dependencies = [ "subtle", ] +[[package]] +name = "password-hash" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "346f04948ba92c43e8469c1ee6736c7563d71012b17d40745260fe106aac2166" +dependencies = [ + "base64ct", + "rand_core 0.6.4", + "subtle", +] + [[package]] name = "paste" version = "1.0.14" @@ -3719,7 +3731,19 @@ checksum = "83a0692ec44e4cf1ef28ca317f14f8f07da2d95ec3fa01f86e4467b725e60917" dependencies = [ "digest", "hmac", - "password-hash", + "password-hash 0.4.2", + "sha2", +] + +[[package]] +name = "pbkdf2" +version = "0.12.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8ed6a7761f76e3b9f92dfb0a60a6a6477c61024b775147ff0973a02653abaf2" +dependencies = [ + "digest", + "hmac", + "password-hash 0.5.0", "sha2", ] @@ -6881,7 +6905,7 @@ dependencies = [ "crossbeam-utils", "flate2", "hmac", - "pbkdf2", + "pbkdf2 0.11.0", "sha1", "time", "zstd 0.11.2+zstd.1.5.2", diff --git a/Cargo.toml b/Cargo.toml index 5040c89c4..197797ea8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -96,6 +96,7 @@ num-format = "0.4.4" num_cpus = "1.16.0" num_enum = "0.7.2" once_cell = "1.19.0" +pbkdf2 = { version = "0.12", features = ["simple"] } phf = { version = "0.11.2", features = ["macros"] } pin-project = "1.1.4" plist = "1.5.1" diff --git a/cargo/licenses.json b/cargo/licenses.json index bbe1f32e2..22a762032 100644 --- a/cargo/licenses.json +++ b/cargo/licenses.json @@ -2510,6 +2510,15 @@ "license_file": null, "description": "Traits which describe the functionality of password hashing algorithms, as well as a `no_std`-friendly implementation of the PHC string format (a well-defined subset of the Modular Crypt Format a.k.a. MCF)" }, + { + "name": "password-hash", + "version": "0.5.0", + "authors": "RustCrypto Developers", + "repository": "https://github.com/RustCrypto/traits/tree/master/password-hash", + "license": "Apache-2.0 OR MIT", + "license_file": null, + "description": "Traits which describe the functionality of password hashing algorithms, as well as a `no_std`-friendly implementation of the PHC string format (a well-defined subset of the Modular Crypt Format a.k.a. MCF)" + }, { "name": "paste", "version": "1.0.14", @@ -2528,6 +2537,15 @@ "license_file": null, "description": "Generic implementation of PBKDF2" }, + { + "name": "pbkdf2", + "version": "0.12.2", + "authors": "RustCrypto Developers", + "repository": "https://github.com/RustCrypto/password-hashes/tree/master/pbkdf2", + "license": "Apache-2.0 OR MIT", + "license_file": null, + "description": "Generic implementation of PBKDF2" + }, { "name": "percent-encoding", "version": "2.3.1", diff --git a/rslib/Cargo.toml b/rslib/Cargo.toml index f7b3af147..f6bb48724 100644 --- a/rslib/Cargo.toml +++ b/rslib/Cargo.toml @@ -73,6 +73,7 @@ nom.workspace = true num_cpus.workspace = true num_enum.workspace = true once_cell.workspace = true +pbkdf2.workspace = true percent-encoding-iri.workspace = true phf.workspace = true pin-project.workspace = true diff --git a/rslib/src/sync/http_server/mod.rs b/rslib/src/sync/http_server/mod.rs index 1d98c381c..1b4c0ee80 100644 --- a/rslib/src/sync/http_server/mod.rs +++ b/rslib/src/sync/http_server/mod.rs @@ -22,6 +22,11 @@ use anki_io::create_dir_all; use axum::extract::DefaultBodyLimit; use axum::Router; use axum_client_ip::SecureClientIpSource; +use pbkdf2::password_hash::PasswordHash; +use pbkdf2::password_hash::PasswordHasher; +use pbkdf2::password_hash::PasswordVerifier; +use pbkdf2::password_hash::SaltString; +use pbkdf2::Pbkdf2; use snafu::whatever; use snafu::OptionExt; use snafu::ResultExt; @@ -91,9 +96,26 @@ impl SimpleServerInner { match std::env::var(&envvar) { Ok(val) => { let hkey = derive_hkey(&val); - let (name, _) = val.split_once(':').with_whatever_context(|| { - format!("{envvar} should be in 'username:password' format.") - })?; + let (name, pwhash) = { + let (name, password) = val.split_once(':').with_whatever_context(|| { + format!("{envvar} should be in 'username:password' format.") + })?; + if std::env::var("PASSWORDS_HASHED").is_ok() { + (name, password.to_string()) + } else { + ( + name, + // Plain text passwords provided; hash them with a fixed salt. + Pbkdf2 + .hash_password( + password.as_bytes(), + &SaltString::from_b64("tonuvYGpksNFQBlEmm3lxg").unwrap(), + ) + .expect("couldn't hash password") + .to_string(), + ) + } + }; let folder = base_folder.join(name); create_dir_all(&folder).whatever_context("creating SYNC_BASE")?; let media = @@ -102,6 +124,7 @@ impl SimpleServerInner { hkey, User { name: name.into(), + password_hash: pwhash, col: None, sync_state: None, media, @@ -150,11 +173,47 @@ impl SimpleServer { request: HostKeyRequest, ) -> HttpResult> { let state = self.state.lock().unwrap(); - let key = derive_hkey(&format!("{}:{}", request.username, request.password)); - if state.users.contains_key(&key) { - SyncResponse::try_from_obj(HostKeyResponse { key }) - } else { - None.or_forbidden("invalid user/pass in get_host_key") + + // This control structure might seem a bit crude, + // its goal is to prevent a timing attack from gaining + // information about whether a specific user exists. + let user = { + // This inner block returns Ok(hkey,user) if a user with corresponding + // name is found and Err(user) with a random user if it isn't found. + // The user is needed to verify against a random hash, + // before returning an Error. + let mut result: Result<(String, &User), &User> = + Err(state.users.iter().next().unwrap().1); + for (hkey, user) in state.users.iter() { + if user.name == request.username { + result = Ok((hkey.to_string(), user)); + } + } + result + }; + + match user { + Ok((key, user)) => { + // Verify password + let pwhash = + &PasswordHash::new(&user.password_hash).expect("couldn't parse password hash"); + if Pbkdf2 + .verify_password(request.password.as_bytes(), pwhash) + .is_ok() + { + SyncResponse::try_from_obj(HostKeyResponse { key }) + } else { + None.or_forbidden("invalid user/pass in get_host_key") + } + } + Err(user) => { + // Verify random password, in order to ensure constant-timedness, + // then return an error + let pwhash = + &PasswordHash::new(&user.password_hash).expect("couldn't parse password hash"); + let _ = Pbkdf2.verify_password(request.password.as_bytes(), pwhash); + None.or_forbidden("invalid user/pass in get_host_key") + } } } diff --git a/rslib/src/sync/http_server/user.rs b/rslib/src/sync/http_server/user.rs index df7ae7596..72dcc7197 100644 --- a/rslib/src/sync/http_server/user.rs +++ b/rslib/src/sync/http_server/user.rs @@ -15,6 +15,7 @@ use crate::sync::http_server::media_manager::ServerMediaManager; pub(in crate::sync) struct User { pub name: String, + pub password_hash: String, pub col: Option, pub sync_state: Option, pub media: ServerMediaManager,