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 <s.sterz@proxmox.com>
This commit is contained in:
Stefan Sterz 2024-03-06 13:36:04 +01:00 committed by Wolfgang Bumiller
parent f82bb2fc2b
commit eef12f91a1
2 changed files with 9 additions and 2 deletions

View File

@ -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 = []

View File

@ -155,9 +155,15 @@ pub fn encrypt_pw(password: &str) -> Result<String, Error> {
/// 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(())
}