diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 7c8d40ad05..9bfefb16d9 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -23,7 +23,7 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d122413f284cf2d62fb1b7db97e02edb8cda96d769b16e443a4f6195e35662b0" dependencies = [ - "crypto-common", + "crypto-common 0.1.7", "generic-array", ] @@ -261,13 +261,13 @@ dependencies = [ [[package]] name = "argon2" -version = "0.5.3" +version = "0.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3c3610892ee6e0cbce8ae2700349fcf8f98adb0dbfbee85aec3c9179d29cc072" +checksum = "134c52ddac6d63c576bef8168db10c83c49c26444ecbc68060fef078925a901c" dependencies = [ "base64ct", - "blake2", - "cpufeatures 0.2.17", + "blake2 0.11.0", + "cpufeatures 0.3.1", "password-hash", ] @@ -1824,6 +1824,15 @@ dependencies = [ "digest 0.10.7", ] +[[package]] +name = "blake2" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b5d4d889834ee8ecfc0f8426ad30faf7cdcb10f741a8e6d7224d95325479f6f" +dependencies = [ + "digest 0.11.3", +] + [[package]] name = "blake3" version = "1.8.7" @@ -1856,6 +1865,15 @@ dependencies = [ "generic-array", ] +[[package]] +name = "block-buffer" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2f6c7dbe95a6ed67ad9f18e57daf93a2f034c524b99fd2b76d18fdfeb6660aa" +dependencies = [ + "hybrid-array", +] + [[package]] name = "block-modes" version = "0.8.1" @@ -2390,7 +2408,7 @@ version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773f3b9af64447d2ce9850330c473515014aa235e6a783b02db81ff39e4a3dad" dependencies = [ - "crypto-common", + "crypto-common 0.1.7", "inout", ] @@ -2454,6 +2472,12 @@ dependencies = [ "cc", ] +[[package]] +name = "cmov" +version = "0.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c9ea0ac24bc397ab3c98583a3c9ba74fa56b09a4449bbe172b9b1ddb016027a" + [[package]] name = "cms" version = "0.2.3" @@ -2818,6 +2842,15 @@ dependencies = [ "typenum", ] +[[package]] +name = "crypto-common" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce6e4c961d6cd6c9a86db418387425e8bdeaf05b3c8bc1411e6dca4c252f1453" +dependencies = [ + "hybrid-array", +] + [[package]] name = "csv" version = "1.4.0" @@ -2848,6 +2881,15 @@ dependencies = [ "cipher 0.4.4", ] +[[package]] +name = "ctutils" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d5515a3834141de9eafb9717ad39eea8247b5674e6066c404e8c4b365d2a29e" +dependencies = [ + "cmov", +] + [[package]] name = "curl-sys" version = "0.4.90+curl-8.21.0" @@ -3413,7 +3455,7 @@ dependencies = [ "arrow", "arrow-buffer", "base64 0.22.1", - "blake2", + "blake2 0.10.6", "blake3", "chrono", "datafusion-common", @@ -4455,10 +4497,21 @@ checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" dependencies = [ "block-buffer 0.10.4", "const-oid", - "crypto-common", + "crypto-common 0.1.7", "subtle", ] +[[package]] +name = "digest" +version = "0.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1dd6dbb5841937940781866fa1281a1ff7bd3bf827091440879f9994983d5c2" +dependencies = [ + "block-buffer 0.12.1", + "crypto-common 0.2.2", + "ctutils", +] + [[package]] name = "dirs" version = "4.0.0" @@ -6139,6 +6192,15 @@ version = "2.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "15cdd26707701c53297e2fa6afb323d55fbc1d0810c3aec078ae3ef0424c3c15" +[[package]] +name = "hybrid-array" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "707114b52a152fa7bdb290cd7cd5912d9467273b6d74e21b8d81aca1f8533f6b" +dependencies = [ + "typenum", +] + [[package]] name = "hyper" version = "0.14.32" @@ -8924,13 +8986,12 @@ dependencies = [ [[package]] name = "password-hash" -version = "0.5.0" +version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "346f04948ba92c43e8469c1ee6736c7563d71012b17d40745260fe106aac2166" +checksum = "aab41826031698d6ffcd9cff78ef56ef998e39dc7e5067cdfebe373842d4723b" dependencies = [ - "base64ct", - "rand_core 0.6.4", - "subtle", + "getrandom 0.4.3", + "phc", ] [[package]] @@ -9074,6 +9135,17 @@ dependencies = [ "phf 0.11.3", ] +[[package]] +name = "phc" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44dc769b75f93afdddd8c7fa12d685292ddeff1e66f7f0f3a234cf1818afe892" +dependencies = [ + "base64ct", + "ctutils", + "getrandom 0.4.3", +] + [[package]] name = "phf" version = "0.11.3" @@ -14145,7 +14217,7 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fc1de2c688dc15305988b563c3854064043356019f97a4b46276fe734c4f07ea" dependencies = [ - "crypto-common", + "crypto-common 0.1.7", "subtle", ] diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 3f614c9388..86940805fc 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -477,10 +477,10 @@ rust-embed = { version = "^6", features = ["interpolate-folder-path"] } mime_guess = "^2" hex = "^0" sql-builder = "^3" -# Pinned: `^0` floats to 0.6, which moved `password_hash::SaltString`, put -# `rand_core` behind a feature and changed `hash_password`'s signature — -# users_ee.rs is written against 0.5 and does not compile otherwise. -argon2 = "0.5" +# Minor-pinned rather than the `^0` used elsewhere in this file: argon2's 0.x +# minors are API-breaking (0.6 moved `SaltString` into `phc`, put `rand_core` +# behind a feature and changed `hash_password`), so a float breaks the build. +argon2 = "0.6" quick_cache = "^0" rand = "=0.9.0" rand_core = { version = "^0", features = ["std"] } diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 5613e1173c..8e49c42d98 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -25d911019aebd3779bb9461a5faab4c20e2f9cf0 +58738c39ac41d57917bbd9400318704763d997f7 diff --git a/backend/windmill-api-integration-tests/tests/users.rs b/backend/windmill-api-integration-tests/tests/users.rs index fb64d9eb77..bec59ac45a 100644 --- a/backend/windmill-api-integration-tests/tests/users.rs +++ b/backend/windmill-api-integration-tests/tests/users.rs @@ -308,14 +308,17 @@ async fn test_user_endpoints(db: Pool) -> anyhow::Result<()> { let auth_base = format!("http://localhost:{port}/api/auth"); // --- login (will fail: password hash in fixture is fake) --- + // An unparseable stored hash must read as a failed login, not as a server error + // relaying the hash parser's message to an unauthenticated caller. let resp = client() .post(format!("{auth_base}/login")) .json(&json!({"email": "test@windmill.dev", "password": "wrong-password"})) .send() .await .unwrap(); - assert!( - resp.status() == 400 || resp.status() == 401 || resp.status() == 500, + assert_eq!( + resp.status(), + 400, "login: unexpected status {}", resp.status() ); @@ -804,12 +807,16 @@ async fn test_change_user_email_leaves_group_identities(db: Pool) -> a let server = ApiServer::start(db.clone()).await?; let global_base = format!("http://localhost:{}/api/users", server.addr.port()); - sqlx::query!("UPDATE password SET email = 'group-ops@windmill.dev' WHERE email = 'test2@windmill.dev'") - .execute(&db) - .await?; - sqlx::query!("UPDATE usr SET email = 'group-ops@windmill.dev' WHERE email = 'test2@windmill.dev'") - .execute(&db) - .await?; + sqlx::query!( + "UPDATE password SET email = 'group-ops@windmill.dev' WHERE email = 'test2@windmill.dev'" + ) + .execute(&db) + .await?; + sqlx::query!( + "UPDATE usr SET email = 'group-ops@windmill.dev' WHERE email = 'test2@windmill.dev'" + ) + .execute(&db) + .await?; sqlx::query!( "INSERT INTO group_(workspace_id, name, summary, extra_perms) VALUES ('test-workspace', 'ops', '', '{}')" ) diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index 8ea3bd0995..8daa414b46 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -19,7 +19,7 @@ use windmill_api_auth::ApiAuthed; pub use windmill_api_auth::Tokened; -use argon2::{Argon2, PasswordHash, PasswordVerifier}; +use argon2::{Argon2, PasswordVerifier}; use axum::{ extract::{Extension, Path, Query}, response::{IntoResponse, Response}, @@ -2680,10 +2680,8 @@ async fn login( .await?; if let Some((email, hash, super_admin)) = email_w_h { - let parsed_hash = - PasswordHash::new(&hash).map_err(|e| Error::internal_err(e.to_string()))?; if argon2 - .verify_password(password.as_bytes(), &parsed_hash) + .verify_password(password.as_bytes(), hash.as_str()) .is_err() { audit_log( @@ -3710,3 +3708,23 @@ async fn request_password_reset( } // NOTE: reset_password is in windmill-api (depends on users_oss::hash_password EE dispatch) + +#[cfg(test)] +mod tests { + use super::*; + + /// Stored hashes outlive the hashing crate: every instance still holds hashes minted by + /// older argon2 releases, and an upgrade that stopped reading them locks their users out. + #[test] + fn verifies_a_hash_minted_by_an_older_argon2() { + // The seeded admin hash from migration 20220508150023, m=4096,t=3,p=1. + let seeded = "$argon2id$v=19$m=4096,t=3,p=1$oLJo/lPn/gezXCuFOEyaNw$i0T2tCkw3xUFsrBIKZwr8jVNHlIfoxQe+HfDnLtd12I"; + + assert!(Argon2::default() + .verify_password(b"changeme", seeded) + .is_ok()); + assert!(Argon2::default() + .verify_password(b"not-the-password", seeded) + .is_err()); + } +}