From eef12f91a15dd13d6b73478f9e8fc5f0b7a1d1f3 Mon Sep 17 00:00:00 2001 From: Stefan Sterz Date: Wed, 6 Mar 2024 13:36:04 +0100 Subject: [PATCH] sys: crypt: use constant time comparison for password verification by using `openssl::memcmp::eq()` we can avoid potential timing side channels as its runtime only depends on the length of the arrays, not the contents. this requires the two arrays to have the same length, but that should be a given since the hashes should always have the same length. Signed-off-by: Stefan Sterz --- proxmox-sys/Cargo.toml | 3 ++- proxmox-sys/src/crypt.rs | 8 +++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/proxmox-sys/Cargo.toml b/proxmox-sys/Cargo.toml index a4176e13..8f038ff2 100644 --- a/proxmox-sys/Cargo.toml +++ b/proxmox-sys/Cargo.toml @@ -16,6 +16,7 @@ lazy_static.workspace = true libc.workspace = true log.workspace = true nix.workspace = true +openssl = { workspace = true, optional = true } regex.workspace = true serde_json.workspace = true serde = { workspace = true, features = [ "derive" ] } @@ -29,5 +30,5 @@ proxmox-time.workspace = true default = [] logrotate = ["dep:zstd"] acl = [] -crypt = [] +crypt = ["dep:openssl"] timer = [] diff --git a/proxmox-sys/src/crypt.rs b/proxmox-sys/src/crypt.rs index fa109113..3313f668 100644 --- a/proxmox-sys/src/crypt.rs +++ b/proxmox-sys/src/crypt.rs @@ -155,9 +155,15 @@ pub fn encrypt_pw(password: &str) -> Result { /// Verify if an encrypted password matches pub fn verify_crypt_pw(password: &str, enc_password: &str) -> Result<(), Error> { let verify = crypt(password.as_bytes(), enc_password.as_bytes())?; - if verify != enc_password { + + // `openssl::memcmp::eq()`'s runtime does not depend on the content of the arrays only the + // length, this makes it harder to exploit timing side-channels. + if verify.len() != enc_password.len() + || !openssl::memcmp::eq(verify.as_bytes(), enc_password.as_bytes()) + { bail!("invalid credentials"); } + Ok(()) }