From 0daaa9b175f0a254d86f60ee1eb5d0a1f6260b4b Mon Sep 17 00:00:00 2001 From: Stefan Melmuk Date: Thu, 30 Mar 2023 23:52:10 +0200 Subject: [PATCH 1/2] always return KdfMemory and KdfParallelism the client will ignore the value of theses fields in case of `PBKDF2` (whether they are unset or left from trying out `Argon2id` as KDF). with `Argon2id` those fields should never be `null` but always in a valid state. if they are `null` (how would that even happen?) the client still assumes default values for `Argon2id` (i.e. m=64 and p=4) and if they are set to something else login will fail anyway. --- src/api/core/accounts.rs | 9 +++------ src/api/core/emergency_access.rs | 12 +++--------- src/api/identity.rs | 31 ++++++++----------------------- 3 files changed, 14 insertions(+), 38 deletions(-) diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 332fda77..3e1185ad 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -803,16 +803,13 @@ pub async fn _prelogin(data: JsonUpcase, mut conn: DbConn) -> Json None => (User::CLIENT_KDF_TYPE_DEFAULT, User::CLIENT_KDF_ITER_DEFAULT, None, None), }; - let mut result = json!({ + let result = json!({ "Kdf": kdf_type, "KdfIterations": kdf_iter, + "KdfMemory": kdf_mem, + "KdfParallelism": kdf_para, }); - if kdf_type == UserKdfType::Argon2id as i32 { - result["KdfMemory"] = Value::Number(kdf_mem.expect("Argon2 memory parameter is required.").into()); - result["KdfParallelism"] = Value::Number(kdf_para.expect("Argon2 parallelism parameter is required.").into()); - } - Json(result) } diff --git a/src/api/core/emergency_access.rs b/src/api/core/emergency_access.rs index dd0617e2..68309714 100644 --- a/src/api/core/emergency_access.rs +++ b/src/api/core/emergency_access.rs @@ -628,21 +628,15 @@ async fn takeover_emergency_access(emer_id: String, headers: Headers, mut conn: None => err!("Grantor user not found."), }; - let mut result = json!({ + let result = json!({ "Kdf": grantor_user.client_kdf_type, "KdfIterations": grantor_user.client_kdf_iter, + "KdfMemory": grantor_user.client_kdf_memory, + "KdfParallelism": grantor_user.client_kdf_parallelism, "KeyEncrypted": &emergency_access.key_encrypted, "Object": "emergencyAccessTakeover", }); - if grantor_user.client_kdf_type == UserKdfType::Argon2id as i32 { - result["KdfMemory"] = - Value::Number(grantor_user.client_kdf_memory.expect("Argon2 memory parameter is required.").into()); - result["KdfParallelism"] = Value::Number( - grantor_user.client_kdf_parallelism.expect("Argon2 parallelism parameter is required.").into(), - ); - } - Ok(Json(result)) } diff --git a/src/api/identity.rs b/src/api/identity.rs index 5115a534..cefee692 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -107,7 +107,7 @@ async fn _refresh_login(data: ConnectData, conn: &mut DbConn) -> JsonResult { let (access_token, expires_in) = device.refresh_tokens(&user, orgs, scope_vec); device.save(conn).await?; - let mut result = json!({ + let result = json!({ "access_token": access_token, "expires_in": expires_in, "token_type": "Bearer", @@ -117,18 +117,13 @@ async fn _refresh_login(data: ConnectData, conn: &mut DbConn) -> JsonResult { "Kdf": user.client_kdf_type, "KdfIterations": user.client_kdf_iter, + "KdfMemory": user.client_kdf_memory, + "KdfParallelism": user.client_kdf_parallelism, "ResetMasterPassword": false, // TODO: according to official server seems something like: user.password_hash.is_empty(), but would need testing "scope": scope, "unofficialServer": true, }); - if user.client_kdf_type == UserKdfType::Argon2id as i32 { - result["KdfMemory"] = - Value::Number(user.client_kdf_memory.expect("Argon2 memory parameter is required.").into()); - result["KdfParallelism"] = - Value::Number(user.client_kdf_parallelism.expect("Argon2 parallelism parameter is required.").into()); - } - Ok(Json(result)) } @@ -260,6 +255,8 @@ async fn _password_login( "Kdf": user.client_kdf_type, "KdfIterations": user.client_kdf_iter, + "KdfMemory": user.client_kdf_memory, + "KdfParallelism": user.client_kdf_parallelism, "ResetMasterPassword": false,// TODO: Same as above "scope": scope, "unofficialServer": true, @@ -269,13 +266,6 @@ async fn _password_login( result["TwoFactorToken"] = Value::String(token); } - if user.client_kdf_type == UserKdfType::Argon2id as i32 { - result["KdfMemory"] = - Value::Number(user.client_kdf_memory.expect("Argon2 memory parameter is required.").into()); - result["KdfParallelism"] = - Value::Number(user.client_kdf_parallelism.expect("Argon2 parallelism parameter is required.").into()); - } - info!("User {} logged in successfully. IP: {}", username, ip.ip); Ok(Json(result)) } @@ -360,7 +350,7 @@ async fn _api_key_login( // Note: No refresh_token is returned. The CLI just repeats the // client_credentials login flow when the existing token expires. - let mut result = json!({ + let result = json!({ "access_token": access_token, "expires_in": expires_in, "token_type": "Bearer", @@ -369,18 +359,13 @@ async fn _api_key_login( "Kdf": user.client_kdf_type, "KdfIterations": user.client_kdf_iter, + "KdfMemory": user.client_kdf_memory, + "KdfParallelism": user.client_kdf_parallelism, "ResetMasterPassword": false, // TODO: Same as above "scope": scope, "unofficialServer": true, }); - if user.client_kdf_type == UserKdfType::Argon2id as i32 { - result["KdfMemory"] = - Value::Number(user.client_kdf_memory.expect("Argon2 memory parameter is required.").into()); - result["KdfParallelism"] = - Value::Number(user.client_kdf_parallelism.expect("Argon2 parallelism parameter is required.").into()); - } - Ok(Json(result)) } From 39a5f2dbe8554c7cdea1b594c4a41d37597fc430 Mon Sep 17 00:00:00 2001 From: Stefan Melmuk Date: Fri, 31 Mar 2023 07:29:12 +0200 Subject: [PATCH 2/2] clear kdf memory and parallelism with pbkdf2 when changing back from argon2id to PBKDF2 the unused parameters should be set to 0. also fix small bug in _register --- src/api/core/accounts.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 3e1185ad..b8e2c79a 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -169,8 +169,8 @@ pub async fn _register(data: JsonUpcase, mut conn: DbConn) -> Json user.client_kdf_iter = client_kdf_iter; } - user.client_kdf_parallelism = data.KdfMemory; - user.client_kdf_memory = data.KdfParallelism; + user.client_kdf_memory = data.KdfMemory; + user.client_kdf_parallelism = data.KdfParallelism; user.set_password(&data.MasterPasswordHash, Some(data.Key), true, None); user.password_hint = password_hint; @@ -389,6 +389,9 @@ async fn post_kdf(data: JsonUpcase, headers: Headers, mut conn: D } else { err!("Argon2 parallelism parameter is required.") } + } else { + user.client_kdf_memory = None; + user.client_kdf_parallelism = None; } user.client_kdf_iter = data.KdfIterations; user.client_kdf_type = data.Kdf;