From 504f1a36ab3b9320cda81052c2961028c1de2700 Mon Sep 17 00:00:00 2001 From: Scott Lamb Date: Wed, 27 Oct 2021 11:38:56 -0700 Subject: [PATCH] switch from libpasta to just scrypt This drops several older dependencies and reduces final binary size (text section by ~200KiB, unstripped binary by ~12MiB) I'll have to manually add new hash formats, and I won't ever be able to take advantage of libpasta's (currently unused) facility to wrap hashes, but I think it's worth it. libpasta isn't well-maintained. --- server/Cargo.lock | 220 +++---------------------------------------- server/Cargo.toml | 1 + server/db/Cargo.toml | 2 +- server/db/auth.rs | 132 ++++++-------------------- server/db/schema.sql | 5 +- 5 files changed, 48 insertions(+), 312 deletions(-) diff --git a/server/Cargo.lock b/server/Cargo.lock index ec08834..e58e910 100644 --- a/server/Cargo.lock +++ b/server/Cargo.lock @@ -37,31 +37,12 @@ dependencies = [ "winapi", ] -[[package]] -name = "argon2rs" -version = "0.2.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3f67b0b6a86dae6e67ff4ca2b6201396074996379fba2b92ff649126f37cb392" -dependencies = [ - "blake2-rfc", - "scoped_threadpool", -] - [[package]] name = "arrayref" version = "0.3.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a4c527152e37cf757a3f78aae5a06fbeefdb07ccc535c980a3208ee3060dd544" -[[package]] -name = "arrayvec" -version = "0.4.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cd9fd44efafa8690358b7408d253adf110036b88f55672a933f01d616ad9b1b9" -dependencies = [ - "nodrop", -] - [[package]] name = "arrayvec" version = "0.7.1" @@ -108,20 +89,9 @@ checksum = "904dfeac50f3cdaba28fc6f57fdcddb75f49ed61346676a78c4ffe55877802fd" [[package]] name = "base64ct" -version = "1.1.1" +version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6b4d9b1225d28d360ec6a231d65af1fd99a2a095154c8040689617290569c5c" - -[[package]] -name = "bcrypt" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4d0faafe9e089674fc3efdb311ff5253d445c79d85d1d28bd3ace76d45e7164" -dependencies = [ - "base64", - "blowfish", - "getrandom", -] +checksum = "8a32fd6af2b5827bce66c29053ba0e7c42b9dcab01835835058558c10851a46b" [[package]] name = "bitflags" @@ -138,16 +108,6 @@ dependencies = [ "cfg-if", ] -[[package]] -name = "blake2-rfc" -version = "0.2.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d6d530bdd2d52966a6d03b7a964add7ae1a288d25214066fd4b600f0f796400" -dependencies = [ - "arrayvec 0.4.12", - "constant_time_eq", -] - [[package]] name = "blake3" version = "1.1.0" @@ -155,7 +115,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2607a74355ce2e252d0c483b2d8a348e1bba36036e786ccc2dcd777213c86ffd" dependencies = [ "arrayref", - "arrayvec 0.7.1", + "arrayvec", "cc", "cfg-if", "constant_time_eq", @@ -171,17 +131,6 @@ dependencies = [ "generic-array", ] -[[package]] -name = "blowfish" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32fa6a061124e37baba002e496d203e23ba3d7b73750be82dbfbc92913048a5b" -dependencies = [ - "byteorder", - "cipher 0.2.5", - "opaque-debug", -] - [[package]] name = "bstr" version = "0.2.17" @@ -236,15 +185,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "cipher" -version = "0.2.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "12f8e7987cbd042a63249497f41aed09f8e65add917ea6566effbc56578d6801" -dependencies = [ - "generic-array", -] - [[package]] name = "cipher" version = "0.3.0" @@ -414,12 +354,6 @@ dependencies = [ "syn", ] -[[package]] -name = "data-encoding" -version = "2.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3ee2393c4a91429dffb4bedf19f4d6abf27d8a732c8ce4980305d782e5426d57" - [[package]] name = "diff" version = "0.1.12" @@ -435,12 +369,6 @@ dependencies = [ "generic-array", ] -[[package]] -name = "dtoa" -version = "0.4.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "56899898ce76aaf4a0f24d914c97ea6ed976d42fec6ad33fcbb0a1103e07b2b0" - [[package]] name = "either" version = "1.6.1" @@ -497,25 +425,6 @@ dependencies = [ "syn", ] -[[package]] -name = "error-chain" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ff511d5dc435d703f4971bc399647c9bc38e20cb41452e3b9feb4765419ed3f3" -dependencies = [ - "backtrace", -] - -[[package]] -name = "error-chain" -version = "0.12.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d2f06b9cac1506ece98fe3231e3cc9c4410ec3d5b1f24ae1c8946f0742cdefc" -dependencies = [ - "backtrace", - "version_check", -] - [[package]] name = "failure" version = "0.1.8" @@ -962,27 +871,6 @@ version = "0.2.105" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "869d572136620d55835903746bcb5cdc54cb2851fd0aeec53220b4bb65ef3013" -[[package]] -name = "libpasta" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cafc9e4bd8d9be7c73e85751c09d8ada2d973473d0af7423ee037bd82efebc2e" -dependencies = [ - "argon2rs", - "bcrypt", - "data-encoding", - "error-chain 0.12.4", - "lazy_static", - "log", - "num-traits", - "ring", - "rpassword", - "scrypt", - "serde", - "serde_mcf", - "serde_yaml", -] - [[package]] name = "libsqlite3-sys" version = "0.23.1" @@ -994,12 +882,6 @@ dependencies = [ "vcpkg", ] -[[package]] -name = "linked-hash-map" -version = "0.5.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7fb9b38af92608140b86b693604b9ffcc5824240a484d1ecd4795bacb2fe88f3" - [[package]] name = "lock_api" version = "0.4.5" @@ -1134,7 +1016,6 @@ dependencies = [ "itertools", "lazy_static", "libc", - "libpasta", "log", "moonfire-base", "mylog", @@ -1147,6 +1028,7 @@ dependencies = [ "protobuf-codegen-pure", "ring", "rusqlite", + "scrypt", "serde", "serde_json", "smallvec", @@ -1200,6 +1082,7 @@ dependencies = [ "nom", "num-rational", "parking_lot", + "password-hash", "protobuf", "reffers", "reqwest", @@ -1271,12 +1154,6 @@ dependencies = [ "memoffset", ] -[[package]] -name = "nodrop" -version = "0.1.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72ef4a56884ca558e5ddb05a1d1e7e1bfd9a68d9ed024c21704cc98872dae1bb" - [[package]] name = "nom" version = "7.0.0" @@ -1438,9 +1315,9 @@ dependencies = [ [[package]] name = "password-hash" -version = "0.2.3" +version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77e0b28ace46c5a396546bcf443bf422b57049617433d8854227352a4a9b24e7" +checksum = "1d791538a6dcc1e7cb7fe6f6b58aca40e7f79403c45b2bc274008b5e647af1d8" dependencies = [ "base64ct", "rand_core", @@ -1449,9 +1326,9 @@ dependencies = [ [[package]] name = "pbkdf2" -version = "0.8.0" +version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d95f5254224e617595d2cc3cc73ff0a5eaf2637519e25f03388154e9378b6ffa" +checksum = "f05894bce6a1ba4be299d0c5f29563e08af2bc18bb7d48313113bed71e904739" dependencies = [ "crypto-mac", ] @@ -1766,16 +1643,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "rpassword" -version = "5.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ffc936cf8a7ea60c58f030fd36a612a48f440610214dc54bc36431f9ea0c3efb" -dependencies = [ - "libc", - "winapi", -] - [[package]] name = "rtp-rs" version = "0.6.0" @@ -1827,15 +1694,9 @@ version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ecbd2eb639fd7cab5804a0837fe373cc2172d15437e804c054a9fb885cb923b0" dependencies = [ - "cipher 0.3.0", + "cipher", ] -[[package]] -name = "scoped_threadpool" -version = "0.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d51f5df5af43ab3f1360b429fa5e0152ac5ce8c0bd6485cae490332e96846a8" - [[package]] name = "scopeguard" version = "1.1.0" @@ -1844,11 +1705,10 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" [[package]] name = "scrypt" -version = "0.7.0" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "879588d8f90906e73302547e20fffefdd240eb3e0e744e142321f5d49dea0518" +checksum = "9f2cc535b6997b0c755bf9344e71ca0e1be070d07ff792f1fcd31e7b90e07d5f" dependencies = [ - "base64ct", "hmac", "password-hash", "pbkdf2", @@ -1875,15 +1735,6 @@ dependencies = [ "serde_derive", ] -[[package]] -name = "serde_bytes" -version = "0.10.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "defbb8a83d7f34cc8380751eeb892b825944222888aff18996ea7901f24aec88" -dependencies = [ - "serde", -] - [[package]] name = "serde_derive" version = "1.0.130" @@ -1901,28 +1752,11 @@ version = "1.0.68" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0f690853975602e1bfe1ccbf50504d67174e3bcf340f23b5ea9992e0587a52d8" dependencies = [ - "indexmap", "itoa", "ryu", "serde", ] -[[package]] -name = "serde_mcf" -version = "0.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a8e71f1e2ddef2fb0451a9fcf2d975d05aac7fb6e452de01da13bff1b4c46cd" -dependencies = [ - "data-encoding", - "error-chain 0.11.0", - "lazy_static", - "serde", - "serde_bytes", - "serde_derive", - "serde_json", - "toml", -] - [[package]] name = "serde_urlencoded" version = "0.7.0" @@ -1935,18 +1769,6 @@ dependencies = [ "serde", ] -[[package]] -name = "serde_yaml" -version = "0.8.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d8c608a35705a5d3cdc9fbe403147647ff34b921f8e833e49306df898f9b20af" -dependencies = [ - "dtoa", - "indexmap", - "serde", - "yaml-rust", -] - [[package]] name = "sha-1" version = "0.9.8" @@ -2240,15 +2062,6 @@ dependencies = [ "tokio", ] -[[package]] -name = "toml" -version = "0.4.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "758664fc71a3a69038656bee8b6be6477d2a6c315a6b81f7081f591bffa4111f" -dependencies = [ - "serde", -] - [[package]] name = "tower-service" version = "0.3.1" @@ -2533,12 +2346,3 @@ name = "xi-unicode" version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a67300977d3dc3f8034dae89778f502b6ba20b269527b3223ba59c0cf393bb8a" - -[[package]] -name = "yaml-rust" -version = "0.4.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "56c1936c4cc7a1c9ab21a1ebb602eb942ba868cbd44a99cb7cdc5892335e1c85" -dependencies = [ - "linked-hash-map", -] diff --git a/server/Cargo.toml b/server/Cargo.toml index aafc36f..600e2bf 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -44,6 +44,7 @@ mylog = { git = "https://github.com/scottlamb/mylog" } nix = "0.23.0" nom = "7.0.0" parking_lot = { version = "0.11.1", features = [] } +password-hash = "0.3.2" protobuf = "3.0.0-alpha.1" reffers = "0.6.0" retina = "0.3.3" diff --git a/server/db/Cargo.toml b/server/db/Cargo.toml index d50032c..8abd07d 100644 --- a/server/db/Cargo.toml +++ b/server/db/Cargo.toml @@ -26,7 +26,6 @@ h264-reader = "0.5.0" hashlink = "0.7.0" lazy_static = "1.0" libc = "0.2" -libpasta = "0.1.2" log = "0.4" mylog = { git = "https://github.com/scottlamb/mylog" } nix = "0.23.0" @@ -37,6 +36,7 @@ pretty-hex = "0.2.1" protobuf = "3.0.0-alpha.1" ring = "0.16.2" rusqlite = "0.26.1" +scrypt = "0.8.0" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" #similar = "2.1.0" diff --git a/server/db/auth.rs b/server/db/auth.rs index b24b1b1..0e145e1 100644 --- a/server/db/auth.rs +++ b/server/db/auth.rs @@ -6,8 +6,8 @@ use crate::json::UserConfig; use crate::schema::Permissions; -use base::{bail_t, format_err_t, strutil, ErrorKind, ResultExt}; -use failure::{bail, format_err, Error}; +use base::{bail_t, format_err_t, strutil, ErrorKind, ResultExt as _}; +use failure::{bail, format_err, Error, Fail, ResultExt as _}; use fnv::FnvHashMap; use lazy_static::lazy_static; use log::info; @@ -15,24 +15,21 @@ use parking_lot::Mutex; use protobuf::Message; use ring::rand::{SecureRandom, SystemRandom}; use rusqlite::{named_params, params, Connection, Transaction}; +use scrypt::password_hash::{PasswordHash, PasswordHasher, PasswordVerifier, SaltString}; use std::collections::BTreeMap; use std::fmt; use std::net::IpAddr; use std::str::FromStr; -use std::sync::Arc; lazy_static! { - static ref PASTA_CONFIG: Mutex> = - Mutex::new(Arc::new(libpasta::Config::default())); + static ref PARAMS: Mutex = Mutex::new(scrypt::Params::recommended()); } -/// For testing only: use a fast but insecure libpasta config. -/// See also . +/// For testing only: use fast but insecure hashes. /// Call via `testutil::init()`. pub(crate) fn set_test_config() { - *PASTA_CONFIG.lock() = Arc::new(libpasta::Config::with_primitive( - libpasta::primitives::Bcrypt::new(2), - )); + let params = scrypt::Params::new(8, 8, 1).unwrap(); + *PARAMS.lock() = params; } #[derive(Debug)] @@ -94,8 +91,12 @@ impl UserChange { } pub fn set_password(&mut self, pwd: String) { - let c = Arc::clone(&PASTA_CONFIG.lock()); - self.set_password_hash = Some(Some(c.hash_password(&pwd))); + let salt = SaltString::generate(&mut scrypt::password_hash::rand_core::OsRng); + let params = PARAMS.lock().clone(); + let hash = scrypt::Scrypt + .hash_password_customized(pwd.as_bytes(), None, None, params, &salt) + .unwrap(); + self.set_password_hash = Some(Some(hash.to_string())); } pub fn clear_password(&mut self) { @@ -525,25 +526,26 @@ impl State { if u.config.disabled { bail!("user {:?} is disabled", username); } - let new_hash = { - let hash = match u.password_hash.as_ref() { - None => bail!("no password set for user {:?}", username), - Some(h) => h, - }; - let c = Arc::clone(&PASTA_CONFIG.lock()); - match c.verify_password_update_hash(hash, &password) { - libpasta::HashUpdate::Failed => { - u.dirty = true; - u.password_failure_count += 1; - bail!("incorrect password for user {:?}", username); - } - libpasta::HashUpdate::Verified(new_pwd) => new_pwd, + let hash = u + .password_hash + .as_ref() + .ok_or_else(|| format_err!("no password set for user {:?}", username))?; + let hash = PasswordHash::new(hash) + .with_context(|_| format!("bad stored password hash for user {:?}", username))?; + match scrypt::Scrypt.verify_password(password.as_bytes(), &hash) { + Ok(()) => {} + Err(scrypt::password_hash::errors::Error::Password) => { + u.dirty = true; + u.password_failure_count += 1; + bail!("incorrect password for user {:?}", username); + } + Err(e) => { + return Err(e + .context(format!("unable to verify password for user {:?}", username)) + .into()); } - }; - if let Some(h) = new_hash { - u.password_hash = Some(h); - u.dirty = true; } + let password_id = u.password_id; State::make_session_int( &self.rand, @@ -1039,78 +1041,6 @@ mod tests { ); } - #[test] - fn upgrade_hash() { - // This hash is generated with cost=1 vs the cost=2 of PASTA_CONFIG. - let insecure_hash = libpasta::Config::with_primitive(libpasta::primitives::Bcrypt::new(1)) - .hash_password("hunter2"); - testutil::init(); - let mut conn = Connection::open_in_memory().unwrap(); - db::init(&mut conn).unwrap(); - let mut state = State::init(&conn).unwrap(); - let mut change = UserChange::add_user("slamb".to_owned()); - - // hunter2, in insecure MD5. - change.set_password_hash = Some(Some(insecure_hash.clone())); - let uid = { - let u = state.apply(&conn, change).unwrap(); - assert_eq!(&insecure_hash, u.password_hash.as_ref().unwrap()); - u.id - }; - - let req = Request { - when_sec: Some(42), - addr: Some(::std::net::IpAddr::V4(::std::net::Ipv4Addr::new( - 127, 0, 0, 1, - ))), - user_agent: Some(b"some ua".to_vec()), - }; - state - .login_by_password( - &conn, - req.clone(), - "slamb", - "hunter2".to_owned(), - Some(b"nvr.example.com".to_vec()), - 0, - ) - .unwrap(); - let new_hash = { - // Password should have been automatically upgraded. - let u = state.users_by_id().get(&uid).unwrap(); - assert!(u.dirty); - assert_ne!(u.password_hash.as_ref().unwrap(), &insecure_hash); - u.password_hash.as_ref().unwrap().clone() - }; - - { - let tx = conn.transaction().unwrap(); - state.flush(&tx).unwrap(); - tx.commit().unwrap(); - } - - // On reload, the new hash should still be visible. - drop(state); - let mut state = State::init(&conn).unwrap(); - { - let u = state.users_by_id().get(&uid).unwrap(); - assert!(!u.dirty); - assert_eq!(u.password_hash.as_ref().unwrap(), &new_hash); - } - - // Login should still work. - state - .login_by_password( - &conn, - req.clone(), - "slamb", - "hunter2".to_owned(), - Some(b"nvr.example.com".to_vec()), - 0, - ) - .unwrap(); - } - #[test] fn disable() { testutil::init(); diff --git a/server/db/schema.sql b/server/db/schema.sql index 9a34ab5..6fc43e4 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -292,8 +292,9 @@ create table user ( -- A json.UserConfig. config text, - -- If set, a hash for password authentication, as generated by - -- `libpasta::hash_password`. This is separate from config for two reasons: + -- If set, a hash for password authentication, which currently must be + -- in PHC format using the scrypt algorithm. This is separate from config for + -- two reasons: -- * It should never be sent over the wire, because password hashes are -- almost as sensitive as passwords themselves. Keeping it separate avoids -- complicating the protocol for retrieving the config and updating it