From: Stefan Sterz <s.sterz@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox v2 07/12] sys: crypt: use constant time comparison for password verification
Date: Wed, 6 Mar 2024 13:36:04 +0100 [thread overview]
Message-ID: <20240306123609.164021-8-s.sterz@proxmox.com> (raw)
In-Reply-To: <20240306123609.164021-1-s.sterz@proxmox.com>
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>
---
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
next prev parent reply other threads:[~2024-03-06 12:36 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-06 12:35 [pbs-devel] [PATCH proxmox{, -backup} v2 00/12] auth-api clean up and improvements Stefan Sterz
2024-03-06 12:35 ` [pbs-devel] [PATCH proxmox v2 01/12] auth-api: move signing into the private key Stefan Sterz
2024-03-06 12:35 ` [pbs-devel] [PATCH proxmox v2 02/12] auth-api: move to Ed25519 signatures Stefan Sterz
2024-03-06 12:36 ` [pbs-devel] [PATCH proxmox v2 03/12] auth-api: add ability to use hmac singing in keyring Stefan Sterz
2024-03-07 10:11 ` Max Carrara
2024-03-06 12:36 ` [pbs-devel] [PATCH proxmox v2 04/12] auth-api: use constant time comparison for csrf tokens Stefan Sterz
2024-03-07 10:17 ` Max Carrara
2024-03-06 12:36 ` [pbs-devel] [PATCH proxmox v2 05/12] auth-api: move to hmac signing " Stefan Sterz
2024-03-06 12:36 ` [pbs-devel] [PATCH proxmox v2 06/12] sys: crypt: move to yescrypt for password hashing Stefan Sterz
2024-03-06 12:36 ` Stefan Sterz [this message]
2024-03-06 12:36 ` [pbs-devel] [PATCH proxmox v2 08/12] auth-api: fix types `compilefail` test Stefan Sterz
2024-03-06 12:36 ` [pbs-devel] [PATCH proxmox-backup v2 09/12] auth: move to hmac keys for csrf tokens Stefan Sterz
2024-03-06 12:36 ` [pbs-devel] [PATCH proxmox-backup v2 10/12] auth: upgrade hashes on user log in Stefan Sterz
2024-03-06 12:36 ` [pbs-devel] [PATCH proxmox-backup v2 11/12] auth: move to auth-api's private and public keys when loading keys Stefan Sterz
2024-03-06 12:36 ` [pbs-devel] [PATCH proxmox-backup v2 12/12] auth: use auth-api when generating keys and generate ec keys Stefan Sterz
2024-03-07 10:12 ` [pbs-devel] [PATCH proxmox{, -backup} v2 00/12] auth-api clean up and improvements Max Carrara
2024-05-22 14:13 ` [pbs-devel] applied-series: " Wolfgang Bumiller
2024-05-24 8:45 ` Max Carrara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240306123609.164021-8-s.sterz@proxmox.com \
--to=s.sterz@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox