[pbs-devel] [PATCH proxmox-backup v2 09/12] auth: move to hmac keys for csrf tokens
Stefan Sterz
s.sterz at proxmox.com
Wed Mar 6 13:36:06 CET 2024
previously we used a self-rolled implementation for csrf tokens. while
it's unlikely to cause issues in reality, as csrf tokens are only
valid for a given tickets lifetime, there are still theoretical
attacks on our implementation. so move all of this code into the
proxmox-auth-api crate and use hmac instead.
this change should not impact existing installations for now, as this
falls back to the old implementation if a key is already present. hmac
keys will only be used for new installations and if users manually
remove the old key and
Signed-off-by: Stefan Sterz <s.sterz at proxmox.com>
---
src/auth.rs | 10 ++---
src/auth_helpers.rs | 97 ++++++++++-----------------------------------
2 files changed, 26 insertions(+), 81 deletions(-)
diff --git a/src/auth.rs b/src/auth.rs
index ec0bc41f..c89314f5 100644
--- a/src/auth.rs
+++ b/src/auth.rs
@@ -15,7 +15,7 @@ use serde_json::json;
use proxmox_auth_api::api::{Authenticator, LockedTfaConfig};
use proxmox_auth_api::ticket::{Empty, Ticket};
use proxmox_auth_api::types::Authid;
-use proxmox_auth_api::Keyring;
+use proxmox_auth_api::{HMACKey, Keyring};
use proxmox_ldap::{Config, Connection, ConnectionMode};
use proxmox_tfa::api::{OpenUserChallengeData, TfaConfig};
@@ -267,7 +267,7 @@ pub fn setup_auth_context(use_private_key: bool) {
AUTH_CONTEXT
.set(PbsAuthContext {
keyring,
- csrf_secret: crate::auth_helpers::csrf_secret().to_vec(),
+ csrf_secret: crate::auth_helpers::csrf_secret(),
})
.map_err(drop)
.expect("auth context setup twice");
@@ -285,7 +285,7 @@ pub(crate) fn public_auth_keyring() -> &'static Keyring {
struct PbsAuthContext {
keyring: &'static Keyring,
- csrf_secret: Vec<u8>,
+ csrf_secret: &'static HMACKey,
}
impl proxmox_auth_api::api::AuthContext for PbsAuthContext {
@@ -327,8 +327,8 @@ impl proxmox_auth_api::api::AuthContext for PbsAuthContext {
}
/// CSRF prevention token secret data.
- fn csrf_secret(&self) -> &[u8] {
- &self.csrf_secret
+ fn csrf_secret(&self) -> &'static HMACKey {
+ self.csrf_secret
}
/// Verify a token secret.
diff --git a/src/auth_helpers.rs b/src/auth_helpers.rs
index c2eaaef1..1a483d84 100644
--- a/src/auth_helpers.rs
+++ b/src/auth_helpers.rs
@@ -1,81 +1,20 @@
use std::path::PathBuf;
+use std::sync::OnceLock;
-use anyhow::{bail, format_err, Error};
+use anyhow::Error;
use lazy_static::lazy_static;
use openssl::pkey::{PKey, Private, Public};
use openssl::rsa::Rsa;
-use openssl::sha;
use pbs_config::BackupLockGuard;
-use proxmox_lang::try_block;
+use proxmox_auth_api::HMACKey;
use proxmox_sys::fs::{file_get_contents, replace_file, CreateOptions};
-use pbs_api_types::Userid;
use pbs_buildcfg::configdir;
use serde_json::json;
pub use crate::auth::setup_auth_context;
-
-fn compute_csrf_secret_digest(timestamp: i64, secret: &[u8], userid: &Userid) -> String {
- let mut hasher = sha::Sha256::new();
- let data = format!("{:08X}:{}:", timestamp, userid);
- hasher.update(data.as_bytes());
- hasher.update(secret);
-
- base64::encode_config(hasher.finish(), base64::STANDARD_NO_PAD)
-}
-
-pub fn assemble_csrf_prevention_token(secret: &[u8], userid: &Userid) -> String {
- let epoch = proxmox_time::epoch_i64();
-
- let digest = compute_csrf_secret_digest(epoch, secret, userid);
-
- format!("{:08X}:{}", epoch, digest)
-}
-
-pub fn verify_csrf_prevention_token(
- secret: &[u8],
- userid: &Userid,
- token: &str,
- min_age: i64,
- max_age: i64,
-) -> Result<i64, Error> {
- use std::collections::VecDeque;
-
- let mut parts: VecDeque<&str> = token.split(':').collect();
-
- try_block!({
- if parts.len() != 2 {
- bail!("format error - wrong number of parts.");
- }
-
- let timestamp = parts.pop_front().unwrap();
- let sig = parts.pop_front().unwrap();
-
- let ttime = i64::from_str_radix(timestamp, 16)
- .map_err(|err| format_err!("timestamp format error - {}", err))?;
-
- let digest = compute_csrf_secret_digest(ttime, secret, userid);
-
- if digest != sig {
- bail!("invalid signature.");
- }
-
- let now = proxmox_time::epoch_i64();
-
- let age = now - ttime;
- if age < min_age {
- bail!("timestamp newer than expected.");
- }
-
- if age > max_age {
- bail!("timestamp too old.");
- }
-
- Ok(age)
- })
- .map_err(|err| format_err!("invalid csrf token - {}", err))
-}
+pub use proxmox_auth_api::api::assemble_csrf_prevention_token;
pub fn generate_csrf_key() -> Result<(), Error> {
let path = PathBuf::from(configdir!("/csrf.key"));
@@ -84,17 +23,14 @@ pub fn generate_csrf_key() -> Result<(), Error> {
return Ok(());
}
- let rsa = Rsa::generate(2048).unwrap();
-
- let pem = rsa.private_key_to_pem()?;
+ let key = HMACKey::generate()?.to_base64()?;
use nix::sys::stat::Mode;
-
let backup_user = pbs_config::backup_user()?;
replace_file(
&path,
- &pem,
+ key.as_bytes(),
CreateOptions::new()
.perm(Mode::from_bits_truncate(0o0640))
.owner(nix::unistd::ROOT)
@@ -145,12 +81,21 @@ pub fn generate_auth_key() -> Result<(), Error> {
Ok(())
}
-pub fn csrf_secret() -> &'static [u8] {
- lazy_static! {
- static ref SECRET: Vec<u8> = file_get_contents(configdir!("/csrf.key")).unwrap();
- }
-
- &SECRET
+pub fn csrf_secret() -> &'static HMACKey {
+ static SECRET: OnceLock<HMACKey> = OnceLock::new();
+
+ SECRET.get_or_init(|| {
+ let bytes = file_get_contents(configdir!("/csrf.key")).unwrap();
+ std::str::from_utf8(&bytes)
+ .map_err(anyhow::Error::new)
+ .and_then(HMACKey::from_base64)
+ // legacy fall back to load legacy csrf secrets
+ // TODO: remove once we move away from legacy token verification
+ .unwrap_or_else(|_| {
+ let key_as_b64 = base64::encode_config(bytes, base64::STANDARD_NO_PAD);
+ HMACKey::from_base64(&key_as_b64).unwrap()
+ })
+ })
}
fn load_public_auth_key() -> Result<PKey<Public>, Error> {
--
2.39.2
More information about the pbs-devel
mailing list