[pbs-devel] [PATCH proxmox 06/12] sys: crypt: use constant time comparison for password verification

[Stefan Sterz]

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 at proxmox.com>
---
 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 5ddbe21..1a44702 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 fa10911..3313f66 100644
--- a/proxmox-sys/src/crypt.rs
+++ b/proxmox-sys/src/crypt.rs
@@ -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(())
 }
 
-- 
2.39.2