fix(security): encrypt app secrets at rest with AES-256-GCM (#6)
All checks were successful
CI Pipeline / Rust Format Check (push) Successful in 8s
CI Pipeline / Clippy Lints (push) Successful in 50s
CI Pipeline / Rust Unit Tests (push) Successful in 1m8s
CI Pipeline / Security Audit (push) Successful in 5s
CI Pipeline / Frontend Lint & Type Check (push) Successful in 15s
CI Pipeline / Build .deb & Release (push) Has been skipped
All checks were successful
CI Pipeline / Rust Format Check (push) Successful in 8s
CI Pipeline / Clippy Lints (push) Successful in 50s
CI Pipeline / Rust Unit Tests (push) Successful in 1m8s
CI Pipeline / Security Audit (push) Successful in 5s
CI Pipeline / Frontend Lint & Type Check (push) Successful in 15s
CI Pipeline / Build .deb & Release (push) Has been skipped
Encrypt three sensitive secrets that were stored in plaintext: OIDC client_secret, SMTP smtp_password, TOTP totp_secret. AES-256-GCM via pm-core::crypto helper. New per-install key at /etc/patch-manager/keys/secret-encryption.key, separate from health-check.key for blast-radius isolation. MASKED placeholder behavior in API responses is preserved. 23 files changed, +1248 / -28. Closes #6.
This commit is contained in:
committed by
GitHub
parent
e0a9037be3
commit
b9fb3427e0
22
crates/pm-web/src/routes/auth.rs
Executable file → Normal file
22
crates/pm-web/src/routes/auth.rs
Executable file → Normal file
@ -360,8 +360,26 @@ async fn mfa_verify_handler(
|
||||
));
|
||||
}
|
||||
|
||||
sqlx::query("UPDATE users SET totp_secret = $1, mfa_enabled = TRUE WHERE id = $2")
|
||||
.bind(&req.secret_base32)
|
||||
// Encrypt the TOTP secret before persisting (issue #6 fix)
|
||||
let key = crate::secret_key::get().map_err(|e| {
|
||||
tracing::error!(error = %e, "Failed to load secret-encryption key");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(
|
||||
json!({ "error": { "code": "internal_error", "message": "Encryption key error" } }),
|
||||
),
|
||||
)
|
||||
})?;
|
||||
let (ciphertext, nonce) = pm_core::crypto::encrypt(&req.secret_base32, key).map_err(|e| {
|
||||
tracing::error!(error = %e, "Failed to encrypt TOTP secret");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(json!({ "error": { "code": "internal_error", "message": "Encryption error" } })),
|
||||
)
|
||||
})?;
|
||||
sqlx::query("UPDATE users SET totp_secret_encrypted = $1, totp_secret_nonce = $2, mfa_enabled = TRUE WHERE id = $3")
|
||||
.bind(&ciphertext)
|
||||
.bind(&nonce)
|
||||
.bind(auth_user.user_id)
|
||||
.execute(&state.db)
|
||||
.await
|
||||
|
||||
@ -273,11 +273,23 @@ async fn update_config_key(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Tuple type for SELECT from oidc_config table (used by fetch_oidc_config).
|
||||
type OidcConfigRow = (
|
||||
bool,
|
||||
String,
|
||||
String,
|
||||
String,
|
||||
String,
|
||||
Option<Vec<u8>>,
|
||||
Option<Vec<u8>>,
|
||||
String,
|
||||
String,
|
||||
);
|
||||
async fn fetch_oidc_config(
|
||||
pool: &sqlx::PgPool,
|
||||
) -> Result<OidcConfigResponse, (StatusCode, Json<Value>)> {
|
||||
let row: Option<(bool, String, String, String, String, String, String, String)> = sqlx::query_as(
|
||||
"SELECT enabled, provider_type, display_name, discovery_url, client_id, client_secret, redirect_uri, scopes FROM oidc_config WHERE id = 1",
|
||||
let row: Option<OidcConfigRow> = sqlx::query_as(
|
||||
"SELECT enabled, provider_type, display_name, discovery_url, client_id, client_secret_encrypted, client_secret_nonce, redirect_uri, scopes FROM oidc_config WHERE id = 1",
|
||||
)
|
||||
.fetch_optional(pool)
|
||||
.await
|
||||
@ -296,7 +308,8 @@ async fn fetch_oidc_config(
|
||||
display_name,
|
||||
discovery_url,
|
||||
client_id,
|
||||
client_secret,
|
||||
client_secret_encrypted,
|
||||
_client_secret_nonce,
|
||||
redirect_uri,
|
||||
scopes,
|
||||
)) => OidcConfigResponse {
|
||||
@ -305,7 +318,7 @@ async fn fetch_oidc_config(
|
||||
display_name,
|
||||
discovery_url,
|
||||
client_id,
|
||||
client_secret: if client_secret.is_empty() {
|
||||
client_secret: if client_secret_encrypted.is_none() {
|
||||
String::new()
|
||||
} else {
|
||||
MASKED.to_string()
|
||||
@ -365,6 +378,22 @@ async fn update_settings(
|
||||
.is_some_and(|s| s != MASKED && !s.is_empty());
|
||||
|
||||
let result = if update_secret {
|
||||
// Encrypt the client_secret before persisting
|
||||
let key = crate::secret_key::get().map_err(|e| {
|
||||
tracing::error!(error = %e, "Failed to load secret-encryption key");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(json!({ "error": { "code": "internal_error", "message": "Encryption key error" } })),
|
||||
)
|
||||
})?;
|
||||
let plaintext = oidc.client_secret.as_deref().unwrap_or("");
|
||||
let (ciphertext, nonce) = pm_core::crypto::encrypt(plaintext, key).map_err(|e| {
|
||||
tracing::error!(error = %e, "Failed to encrypt OIDC client_secret");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(json!({ "error": { "code": "internal_error", "message": "Encryption error" } })),
|
||||
)
|
||||
})?;
|
||||
sqlx::query(
|
||||
"UPDATE oidc_config SET \
|
||||
enabled = COALESCE($1, enabled), \
|
||||
@ -372,9 +401,10 @@ async fn update_settings(
|
||||
display_name = COALESCE($3, display_name), \
|
||||
discovery_url = COALESCE($4, discovery_url), \
|
||||
client_id = COALESCE($5, client_id), \
|
||||
client_secret = $6, \
|
||||
redirect_uri = COALESCE($7, redirect_uri), \
|
||||
scopes = COALESCE($8, scopes), \
|
||||
client_secret_encrypted = $6, \
|
||||
client_secret_nonce = $7, \
|
||||
redirect_uri = COALESCE($8, redirect_uri), \
|
||||
scopes = COALESCE($9, scopes), \
|
||||
updated_at = NOW() \
|
||||
WHERE id = 1",
|
||||
)
|
||||
@ -383,7 +413,8 @@ async fn update_settings(
|
||||
.bind(&oidc.display_name)
|
||||
.bind(&oidc.discovery_url)
|
||||
.bind(&oidc.client_id)
|
||||
.bind(oidc.client_secret.as_deref().unwrap_or(""))
|
||||
.bind(&ciphertext)
|
||||
.bind(&nonce)
|
||||
.bind(&oidc.redirect_uri)
|
||||
.bind(&oidc.scopes)
|
||||
.execute(&state.db)
|
||||
@ -450,7 +481,59 @@ async fn update_settings(
|
||||
}
|
||||
if let Some(ref v) = smtp.password {
|
||||
if v != MASKED {
|
||||
update_config_key(&state.db, "smtp_password", v).await?;
|
||||
// Encrypt the SMTP password before persisting
|
||||
let key = crate::secret_key::get().map_err(|e| {
|
||||
tracing::error!(error = %e, "Failed to load secret-encryption key");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(json!({ "error": { "code": "internal_error", "message": "Encryption key error" } })),
|
||||
)
|
||||
})?;
|
||||
let (ciphertext, nonce) = pm_core::crypto::encrypt(v, key).map_err(|e| {
|
||||
tracing::error!(error = %e, "Failed to encrypt SMTP password");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(json!({ "error": { "code": "internal_error", "message": "Encryption error" } })),
|
||||
)
|
||||
})?;
|
||||
// Delete old plaintext row, write two new rows (encrypted + nonce)
|
||||
sqlx::query("DELETE FROM system_config WHERE key = 'smtp_password'")
|
||||
.execute(&state.db)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!(error = %e, "Failed to delete old smtp_password row");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(json!({ "error": { "code": "internal_error", "message": "Database error" } })),
|
||||
)
|
||||
})?;
|
||||
// Store as hex in TEXT columns (system_config uses TEXT)
|
||||
let enc_hex: String = ciphertext.iter().map(|b| format!("{:02x}", b)).collect();
|
||||
let nonce_hex: String = nonce.iter().map(|b| format!("{:02x}", b)).collect();
|
||||
sqlx::query("INSERT INTO system_config (key, value) VALUES ($1, $2)")
|
||||
.bind("smtp_password_encrypted")
|
||||
.bind(&enc_hex)
|
||||
.execute(&state.db)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!(error = %e, "Failed to write smtp_password_encrypted");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(json!({ "error": { "code": "internal_error", "message": "Database error" } })),
|
||||
)
|
||||
})?;
|
||||
sqlx::query("INSERT INTO system_config (key, value) VALUES ($1, $2)")
|
||||
.bind("smtp_password_nonce")
|
||||
.bind(&nonce_hex)
|
||||
.execute(&state.db)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!(error = %e, "Failed to write smtp_password_nonce");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(json!({ "error": { "code": "internal_error", "message": "Database error" } })),
|
||||
)
|
||||
})?;
|
||||
}
|
||||
}
|
||||
if let Some(ref v) = smtp.from {
|
||||
@ -790,7 +873,32 @@ async fn test_smtp(
|
||||
.and_then(|v| v.parse().ok())
|
||||
.unwrap_or(587);
|
||||
let username = cfg.get("smtp_username").cloned().unwrap_or_default();
|
||||
let password = cfg.get("smtp_password").cloned().unwrap_or_default();
|
||||
// Decrypt the SMTP password (issue #6 fix — stored as two rows in system_config:
|
||||
// `smtp_password_encrypted` (hex) and `smtp_password_nonce` (hex))
|
||||
let password = match (
|
||||
cfg.get("smtp_password_encrypted"),
|
||||
cfg.get("smtp_password_nonce"),
|
||||
) {
|
||||
(Some(enc_hex), Some(nonce_hex)) => {
|
||||
let key = match crate::secret_key::get() {
|
||||
Ok(k) => k,
|
||||
Err(e) => {
|
||||
tracing::error!(error = %e, "Failed to load secret-encryption key");
|
||||
return Err((
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(
|
||||
json!({ "error": { "code": "internal_error", "message": "Encryption key error" } }),
|
||||
),
|
||||
));
|
||||
},
|
||||
};
|
||||
// Decode hex to bytes (hex_decode returns empty Vec on invalid input)
|
||||
let enc_bytes = hex_decode(enc_hex);
|
||||
let nonce_bytes = hex_decode(nonce_hex);
|
||||
pm_core::crypto::decrypt(&enc_bytes, &nonce_bytes, key).unwrap_or_default()
|
||||
},
|
||||
_ => String::new(),
|
||||
};
|
||||
let from_addr = cfg.get("smtp_from").cloned().unwrap_or_default();
|
||||
let tls_mode = cfg
|
||||
.get("smtp_tls_mode")
|
||||
@ -1032,18 +1140,31 @@ async fn audit_integrity(
|
||||
})))
|
||||
}
|
||||
|
||||
/// Decode a hex string to bytes. Returns an empty Vec on invalid input.
|
||||
/// Used by the SMTP password decryption logic (issue #6 fix).
|
||||
fn hex_decode(s: &str) -> Vec<u8> {
|
||||
if !s.len().is_multiple_of(2) {
|
||||
return Vec::new();
|
||||
}
|
||||
(0..s.len())
|
||||
.step_by(2)
|
||||
.filter_map(|i| u8::from_str_radix(&s[i..i + 2], 16).ok())
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(unused_imports)]
|
||||
use super::*;
|
||||
use axum::http::StatusCode;
|
||||
use pm_auth::jwt::AccessClaims;
|
||||
use pm_auth::rbac::{AuthUser, UserRole};
|
||||
use serde_json::json;
|
||||
use uuid::Uuid;
|
||||
|
||||
/// Build a minimal `AuthUser` for role-gate testing.
|
||||
/// The `admin_required` gate only inspects `auth.role`, so all other
|
||||
/// fields can be placeholder values.
|
||||
#[allow(dead_code)]
|
||||
fn test_auth_user(role: UserRole) -> AuthUser {
|
||||
let claims = AccessClaims {
|
||||
sub: Uuid::new_v4().to_string(),
|
||||
|
||||
@ -213,11 +213,29 @@ pub struct OidcConfig {
|
||||
pub display_name: String,
|
||||
pub discovery_url: String,
|
||||
pub client_id: String,
|
||||
pub client_secret: String,
|
||||
/// AES-256-GCM encrypted client_secret. `None` if not set or public client.
|
||||
pub client_secret_encrypted: Option<Vec<u8>>,
|
||||
/// AES-256-GCM nonce for client_secret. Must be paired with `client_secret_encrypted`.
|
||||
pub client_secret_nonce: Option<Vec<u8>>,
|
||||
pub redirect_uri: String,
|
||||
pub scopes: String,
|
||||
}
|
||||
|
||||
impl OidcConfig {
|
||||
/// Decrypt the client_secret using the provided key.
|
||||
/// Returns `Ok(String::new())` if the secret is not set (public client).
|
||||
/// Returns `Err(CryptoError)` if decryption fails or nonce is missing.
|
||||
pub fn decrypt_client_secret(
|
||||
&self,
|
||||
key: &[u8; 32],
|
||||
) -> Result<String, pm_core::crypto::CryptoError> {
|
||||
match (&self.client_secret_encrypted, &self.client_secret_nonce) {
|
||||
(Some(enc), Some(nonce)) => pm_core::crypto::decrypt(enc, nonce, key),
|
||||
_ => Ok(String::new()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Cached OIDC discovery document.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct OidcDiscovery {
|
||||
@ -464,8 +482,28 @@ async fn sso_callback(
|
||||
];
|
||||
|
||||
// For confidential clients (Azure AD), include client_secret
|
||||
if !config.client_secret.is_empty() {
|
||||
params_vec.push(("client_secret", config.client_secret.clone()));
|
||||
let key = match crate::secret_key::get() {
|
||||
Ok(k) => k,
|
||||
Err(e) => {
|
||||
tracing::error!(error = %e, "Failed to load secret-encryption key");
|
||||
return Err(error_redirect(
|
||||
"internal_error",
|
||||
"Failed to load encryption key",
|
||||
));
|
||||
},
|
||||
};
|
||||
let client_secret = match config.decrypt_client_secret(key) {
|
||||
Ok(s) => s,
|
||||
Err(e) => {
|
||||
tracing::error!(error = %e, "Failed to decrypt OIDC client_secret");
|
||||
return Err(error_redirect(
|
||||
"internal_error",
|
||||
"Failed to decrypt client_secret",
|
||||
));
|
||||
},
|
||||
};
|
||||
if !client_secret.is_empty() {
|
||||
params_vec.push(("client_secret", client_secret));
|
||||
}
|
||||
|
||||
let token_resp = match client
|
||||
@ -799,7 +837,9 @@ async fn azure_callback_redirect(
|
||||
|
||||
async fn load_oidc_config(pool: &sqlx::PgPool) -> Result<OidcConfig, (StatusCode, Json<Value>)> {
|
||||
let row: Option<OidcConfig> = sqlx::query_as(
|
||||
"SELECT enabled, provider_type, display_name, discovery_url, client_id, client_secret, redirect_uri, scopes FROM oidc_config WHERE id = 1",
|
||||
"SELECT enabled, provider_type, display_name, discovery_url, client_id, \
|
||||
client_secret_encrypted, client_secret_nonce, redirect_uri, scopes \
|
||||
FROM oidc_config WHERE id = 1",
|
||||
)
|
||||
.fetch_optional(pool)
|
||||
.await
|
||||
@ -817,7 +857,8 @@ async fn load_oidc_config(pool: &sqlx::PgPool) -> Result<OidcConfig, (StatusCode
|
||||
display_name: "Azure AD".to_string(),
|
||||
discovery_url: String::new(),
|
||||
client_id: String::new(),
|
||||
client_secret: String::new(),
|
||||
client_secret_encrypted: None,
|
||||
client_secret_nonce: None,
|
||||
redirect_uri: String::new(),
|
||||
scopes: "openid profile email".to_string(),
|
||||
}))
|
||||
|
||||
2
crates/pm-web/src/routes/users.rs
Executable file → Normal file
2
crates/pm-web/src/routes/users.rs
Executable file → Normal file
@ -534,7 +534,7 @@ async fn admin_disable_mfa(
|
||||
));
|
||||
}
|
||||
|
||||
let rows = sqlx::query("UPDATE users SET totp_secret = NULL, mfa_enabled = FALSE, updated_at = NOW() WHERE id = $1")
|
||||
let rows = sqlx::query("UPDATE users SET totp_secret_encrypted = NULL, totp_secret_nonce = NULL, mfa_enabled = FALSE, updated_at = NOW() WHERE id = $1")
|
||||
.bind(id)
|
||||
.execute(&state.db)
|
||||
.await
|
||||
|
||||
Reference in New Issue
Block a user