diff --git a/Cargo.lock b/Cargo.lock index 64cb50c94..c61b1a337 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1364,6 +1364,7 @@ dependencies = [ "openssl", "rpassword", "serde", + "serde_repr", "sha2", "tokio", "tokio-stream", @@ -1379,6 +1380,7 @@ version = "0.6.0" dependencies = [ "libc", "serde", + "serde_repr", "tempfile", "tokio", "tracing", diff --git a/pam/Cargo.toml b/pam/Cargo.toml index 8e5e83f79..703d722c4 100644 --- a/pam/Cargo.toml +++ b/pam/Cargo.toml @@ -17,6 +17,7 @@ crate-type = ["cdylib"] libc = "0.2" tokio = { workspace = true, features = ["rt", "rt-multi-thread", "net", "io-util", "sync", "time"] } serde = { workspace = true } +serde_repr = "0.1" zvariant = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true, features = ["env-filter"] } diff --git a/pam/README.md b/pam/README.md index a923dd7ab..41ecba0bf 100644 --- a/pam/README.md +++ b/pam/README.md @@ -59,14 +59,25 @@ auth required pam_unix.so auth optional pam_oo7.so account required pam_unix.so password required pam_unix.so +password optional pam_oo7.so session required pam_unix.so session optional pam_oo7.so auto_start session optional pam_systemd.so ``` -**Important**: The module must be added to both `auth` and `session` stacks: -- `auth` stack: Captures and stashes the password -- `session` stack: Retrieves the password and sends it to the daemon +**Important**: The module should be added to three PAM stacks: +- `auth` stack: Captures and stashes the password during authentication +- `session` stack: Retrieves the stashed password and sends it to the daemon for keyring unlocking +- `password` stack: Intercepts password changes and updates the keyring password to match + +#### Password Change Support + +When added to the `password` stack, the module will automatically update your keyring passwords when you change your user password (e.g., using `passwd` command). This ensures your keyrings remain accessible after password changes. + +The module intercepts the password change operation: +1. Captures both the old and new passwords +2. Sends them to the daemon +3. The daemon validates the old password and re-encrypts all matching keyrings with the new password ## Configuration diff --git a/pam/src/ffi.rs b/pam/src/ffi.rs index aab7178b8..8cc44523c 100644 --- a/pam/src/ffi.rs +++ b/pam/src/ffi.rs @@ -4,9 +4,15 @@ use std::os::raw::{c_char, c_int, c_void}; pub const PAM_SUCCESS: c_int = 0; pub const PAM_SYSTEM_ERR: c_int = 4; pub const PAM_AUTHTOK_RECOVER_ERR: c_int = 20; +pub const PAM_IGNORE: c_int = 25; // PAM item types pub const PAM_AUTHTOK: c_int = 6; +pub const PAM_OLDAUTHTOK: c_int = 7; + +// PAM chauthtok flags +pub const PAM_PRELIM_CHECK: c_int = 0x1; +pub const PAM_UPDATE_AUTHTOK: c_int = 0x2; // Opaque PAM handle type #[repr(C)] diff --git a/pam/src/lib.rs b/pam/src/lib.rs index 971f9e463..cc47cee3d 100644 --- a/pam/src/lib.rs +++ b/pam/src/lib.rs @@ -15,7 +15,10 @@ use std::{ use zeroize::Zeroizing; use crate::{ - ffi::{PAM_AUTHTOK, PAM_AUTHTOK_RECOVER_ERR, PAM_SUCCESS, PAM_SYSTEM_ERR, pam_handle_t}, + ffi::{ + PAM_AUTHTOK, PAM_AUTHTOK_RECOVER_ERR, PAM_IGNORE, PAM_OLDAUTHTOK, PAM_PRELIM_CHECK, + PAM_SUCCESS, PAM_SYSTEM_ERR, PAM_UPDATE_AUTHTOK, pam_handle_t, + }, protocol::PamMessage, socket::send_secret_to_daemon, }; @@ -42,18 +45,30 @@ unsafe fn get_user(pamh: *mut pam_handle_t) -> Result { /// Get the authentication token from PAM unsafe fn get_auth_token(pamh: *mut pam_handle_t) -> Result>, c_int> { + unsafe { get_auth_token_internal(pamh, PAM_AUTHTOK, "PAM_AUTHTOK") } +} + +/// Get the old authentication token from PAM (for password changes) +unsafe fn get_old_auth_token(pamh: *mut pam_handle_t) -> Result>, c_int> { + unsafe { get_auth_token_internal(pamh, PAM_OLDAUTHTOK, "PAM_OLDAUTHTOK") } +} + +unsafe fn get_auth_token_internal( + pamh: *mut pam_handle_t, + item_type: c_int, + item_name: &str, +) -> Result>, c_int> { let mut authtok_ptr: *const std::os::raw::c_void = std::ptr::null(); - // Use pam_get_item to get PAM_AUTHTOK (just like pam_gnome_keyring does) - let ret = unsafe { ffi::pam_get_item(pamh, PAM_AUTHTOK, &mut authtok_ptr) }; + let ret = unsafe { ffi::pam_get_item(pamh, item_type, &mut authtok_ptr) }; if ret != PAM_SUCCESS { - tracing::debug!("pam_get_item returned error: {}", ret); + tracing::debug!("pam_get_item({}) returned error: {}", item_name, ret); return Err(ret); } if authtok_ptr.is_null() { - tracing::debug!("PAM_AUTHTOK is null (password not available)"); + tracing::debug!("{} is null (password not available)", item_name); return Err(PAM_SYSTEM_ERR); } @@ -62,7 +77,8 @@ unsafe fn get_auth_token(pamh: *mut pam_handle_t) -> Result>, let password_bytes = password_cstr.to_bytes().to_vec(); tracing::debug!( - "Captured auth token of length {} bytes", + "Captured {} of length {} bytes", + item_name, password_bytes.len() ); @@ -258,10 +274,7 @@ pub unsafe extern "C" fn pam_sm_open_session( } }; - let message = PamMessage { - username: username.clone(), - secret: password.to_vec(), - }; + let message = PamMessage::unlock(username.clone(), password.to_vec()); // Send the secret to the oo7 daemon std::thread::spawn( @@ -294,11 +307,100 @@ pub extern "C" fn pam_sm_close_session( /// PAM password change entry point #[unsafe(no_mangle)] -pub extern "C" fn pam_sm_chauthtok( - _pamh: *mut pam_handle_t, - _flags: c_int, +#[allow(clippy::missing_safety_doc)] +pub unsafe extern "C" fn pam_sm_chauthtok( + pamh: *mut pam_handle_t, + flags: c_int, _argc: c_int, _argv: *mut *const c_char, ) -> c_int { - PAM_SUCCESS + if let Ok(layer) = tracing_journald::layer() { + use tracing_subscriber::layer::SubscriberExt; + let subscriber = tracing_subscriber::registry().with(layer); + let _ = tracing::subscriber::set_global_default(subscriber); + } + + if flags & PAM_PRELIM_CHECK != 0 { + tracing::debug!("PAM_PRELIM_CHECK phase for password change"); + return PAM_IGNORE; + } + + if flags & PAM_UPDATE_AUTHTOK != 0 { + tracing::debug!("PAM_UPDATE_AUTHTOK phase for password change"); + + let username = match unsafe { get_user(pamh) } { + Ok(user) => user, + Err(ret) => { + tracing::error!("Failed to get username during password change"); + return ret; + } + }; + + let user_uid = match get_user_uid(&username) { + Some(uid) => uid, + None => { + tracing::error!("Failed to get UID for user: {}", username); + return PAM_SYSTEM_ERR; + } + }; + + let old_password = match unsafe { get_old_auth_token(pamh) } { + Ok(pass) => pass, + Err(_) => { + tracing::warn!( + "No old password available for user {}, cannot update keyring password", + username + ); + return PAM_SUCCESS; + } + }; + + let new_password = match unsafe { get_auth_token(pamh) } { + Ok(pass) => pass, + Err(_) => { + tracing::warn!( + "No new password available for user {}, cannot update keyring password", + username + ); + return PAM_SUCCESS; + } + }; + + if old_password.is_empty() || new_password.is_empty() { + tracing::debug!("Old or new password is empty, skipping keyring password change"); + return PAM_SUCCESS; + } + + tracing::info!( + "Password change for user {}: old={} bytes, new={} bytes", + username, + old_password.len(), + new_password.len() + ); + + let message = PamMessage::change_password( + username.clone(), + old_password.to_vec(), + new_password.to_vec(), + ); + + std::thread::spawn( + move || match send_secret_to_daemon(message, user_uid, false) { + Ok(_) => { + tracing::info!( + "Successfully sent password change request to oo7 daemon for user: {}", + username + ); + } + Err(e) => { + tracing::error!("Failed to send password change to oo7 daemon: {}", e); + } + }, + ); + + return PAM_SUCCESS; + } + + tracing::warn!("pam_sm_chauthtok called with unknown flags: {}", flags); + PAM_IGNORE } diff --git a/pam/src/protocol.rs b/pam/src/protocol.rs index 21af38d59..638088516 100644 --- a/pam/src/protocol.rs +++ b/pam/src/protocol.rs @@ -1,14 +1,45 @@ use serde::{Deserialize, Serialize}; +use serde_repr::{Deserialize_repr, Serialize_repr}; use zeroize::{Zeroize, ZeroizeOnDrop}; use zvariant::{Type, serialized::Context, to_bytes}; +#[derive(Debug, Clone, Copy, Serialize_repr, Deserialize_repr, Type, PartialEq, Eq)] +#[repr(u8)] +pub enum PamOperation { + Unlock = 0, + ChangePassword = 1, +} + #[derive(Debug, Serialize, Deserialize, Type, Zeroize, ZeroizeOnDrop)] pub struct PamMessage { + #[zeroize(skip)] + operation: PamOperation, pub username: String, - pub secret: Vec, + pub old_secret: Vec, + pub new_secret: Vec, } impl PamMessage { + /// Create an unlock message + pub fn unlock(username: String, secret: Vec) -> Self { + Self { + operation: PamOperation::Unlock, + username, + old_secret: Vec::new(), + new_secret: secret, + } + } + + /// Create a password change message + pub fn change_password(username: String, old_secret: Vec, new_secret: Vec) -> Self { + Self { + operation: PamOperation::ChangePassword, + username, + old_secret, + new_secret, + } + } + pub fn to_bytes(&self) -> Result, zvariant::Error> { let ctxt = Context::new_dbus(zvariant::LE, 0); to_bytes(ctxt, self).map(|data| data.to_vec()) diff --git a/pam/src/socket.rs b/pam/src/socket.rs index e319a58c2..63378ec3d 100644 --- a/pam/src/socket.rs +++ b/pam/src/socket.rs @@ -156,11 +156,7 @@ async fn send_secret_to_daemon_async( tracing::debug!("Connected to daemon socket"); - tracing::debug!( - "Sending secret of length {} bytes for user {}", - message.secret.len(), - message.username - ); + tracing::debug!("Sending message for user {}", message.username); let message_bytes = Zeroizing::new(message.to_bytes().map_err(SocketError::Serialize)?); let length = message_bytes.len() as u32; @@ -212,15 +208,12 @@ mod tests { let message = PamMessage::from_bytes(&message_bytes).unwrap(); assert_eq!(message.username, "testuser"); - assert_eq!(message.secret, b"testpassword"); + assert_eq!(message.new_secret, b"testpassword"); }); tokio::time::sleep(Duration::from_millis(100)).await; - let message = PamMessage { - username: "testuser".to_string(), - secret: b"testpassword".to_vec(), - }; + let message = PamMessage::unlock("testuser".to_string(), b"testpassword".to_vec()); let result = send_secret_to_daemon_async(message, 1000, false).await; assert!(result.is_ok()); diff --git a/server/Cargo.toml b/server/Cargo.toml index 6f6749327..0e00a3444 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -25,6 +25,7 @@ openssl = { version = "0.10", optional = true } oo7 = { workspace = true, features = ["tokio"] } rpassword = "7.4" serde.workspace = true +serde_repr = "0.1" sha2 = { version = "0.10", optional = true } tokio = { workspace = true, features = ["full"] } tokio-stream = "0.1" diff --git a/server/src/collection.rs b/server/src/collection.rs index a42bb7b3c..054092365 100644 --- a/server/src/collection.rs +++ b/server/src/collection.rs @@ -404,14 +404,27 @@ impl Collection { .duration_since(SystemTime::UNIX_EPOCH) .unwrap(); + let sanitized_label = label + .chars() + .map(|c| { + if c.is_alphanumeric() || c == '_' { + c + } else { + '_' + } + }) + .collect::(); + Self { items: Default::default(), label: Arc::new(Mutex::new(label.to_owned())), modified: Arc::new(Mutex::new(created)), alias: Arc::new(Mutex::new(alias.to_owned())), item_index: Arc::new(RwLock::new(0)), - path: OwnedObjectPath::try_from(format!("/org/freedesktop/secrets/collection/{label}")) - .unwrap(), + path: OwnedObjectPath::try_from(format!( + "/org/freedesktop/secrets/collection/{sanitized_label}" + )) + .expect("Sanitized label should always produce valid object path"), created, service, keyring: Arc::new(RwLock::new(Some(keyring))), @@ -492,9 +505,22 @@ impl Collection { custom_service_error("Cannot unlock collection without a secret") })?; - let unlocked = locked_kr.unlock(secret).await.map_err(|err| { - custom_service_error(&format!("Failed to unlock keyring: {err}")) - })?; + let keyring_path = locked_kr.path().map(|p| p.to_path_buf()); + + let unlocked = match locked_kr.unlock(secret).await { + Ok(unlocked) => unlocked, + Err(err) => { + // Reload the locked keyring from disk before returning error + if let Some(path) = keyring_path { + if let Ok(reloaded) = oo7::file::LockedKeyring::load(&path).await { + *keyring_guard = Some(Keyring::Locked(reloaded)); + } + } + return Err(custom_service_error(&format!( + "Failed to unlock keyring: {err}" + ))); + } + }; let items = self.items.lock().await; for item in items.iter() { diff --git a/server/src/pam_listener.rs b/server/src/pam_listener.rs index b90497cce..e4587f3ff 100644 --- a/server/src/pam_listener.rs +++ b/server/src/pam_listener.rs @@ -4,6 +4,7 @@ use std::{os::unix::fs::PermissionsExt, path::PathBuf, sync::Arc}; use oo7::Secret; use serde::{Deserialize, Serialize}; +use serde_repr::{Deserialize_repr, Serialize_repr}; use tokio::{ io::AsyncReadExt, net::{UnixListener, UnixStream}, @@ -17,10 +18,20 @@ use zeroize::{Zeroize, ZeroizeOnDrop}; use crate::{Service, error::Error}; +#[derive(Debug, Clone, Copy, Serialize_repr, Deserialize_repr, Type, PartialEq, Eq)] +#[repr(u8)] +enum PamOperation { + Unlock = 0, + ChangePassword = 1, +} + #[derive(Debug, Serialize, Deserialize, Type, Zeroize, ZeroizeOnDrop)] struct PamMessage { + #[zeroize(skip)] + operation: PamOperation, username: String, - secret: Vec, + old_secret: Vec, + new_secret: Vec, } impl PamMessage { @@ -105,29 +116,74 @@ impl PamListener { let message = PamMessage::from_bytes(&message_bytes) .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e.to_string()))?; - tracing::info!("Received PAM authentication for user: {}", message.username); - tracing::debug!("Received secret of length {} bytes", message.secret.len()); + match message.operation { + PamOperation::Unlock => { + tracing::info!("Received unlock request for user: {}", message.username); + tracing::debug!( + "Received secret of length {} bytes", + message.new_secret.len() + ); - let secret = Secret::from(message.secret.to_vec()); - self.user_secrets - .write() - .await - .insert(message.username.clone(), secret.clone()); + let secret = Secret::from(message.new_secret.to_vec()); + self.user_secrets + .write() + .await + .insert(message.username.clone(), secret.clone()); - // Try to unlock the default collection with this secret - match self.try_unlock_collections(&secret).await { - Ok(_) => { + match self.try_unlock_collections(&secret).await { + Ok(_) => { + tracing::info!( + "Successfully unlocked collections for user: {}", + message.username + ); + } + Err(e) => { + tracing::warn!( + "Failed to unlock collections for user {}: {}", + message.username, + e + ); + } + } + } + PamOperation::ChangePassword => { tracing::info!( - "Successfully unlocked collections for user: {}", + "Received password change request for user: {}", message.username ); - } - Err(e) => { - tracing::warn!( - "Failed to unlock collections for user {}: {}", - message.username, - e + tracing::debug!( + "Old secret: {} bytes, new secret: {} bytes", + message.old_secret.len(), + message.new_secret.len() ); + + let old_secret = Secret::from(message.old_secret.to_vec()); + let new_secret = Secret::from(message.new_secret.to_vec()); + + match self + .change_collection_passwords(&old_secret, &new_secret) + .await + { + Ok(changed_count) => { + tracing::info!( + "Successfully changed password for {} collection(s) for user: {}", + changed_count, + message.username + ); + // Update stored secret + self.user_secrets + .write() + .await + .insert(message.username.clone(), new_secret); + } + Err(e) => { + tracing::error!( + "Failed to change password for user {}: {}", + message.username, + e + ); + } + } } } @@ -158,6 +214,110 @@ impl PamListener { Ok(()) } + + /// Change password for all collections that match the old password + async fn change_collection_passwords( + &self, + old_secret: &Secret, + new_secret: &Secret, + ) -> Result { + let collections = self.service.collections.lock().await; + let mut changed_count = 0; + + for (path, collection) in collections.iter() { + // Skip session collection (it's temporary and doesn't persist) + if collection.alias().await == oo7::dbus::Service::SESSION_COLLECTION { + tracing::debug!("Skipping session collection: {}", path); + continue; + } + + // Get the keyring from the collection + let keyring_guard = collection.keyring.read().await; + let Some(keyring) = keyring_guard.as_ref() else { + tracing::debug!("Collection {} has no keyring", path); + continue; + }; + + // Track if we unlocked the collection (so we can re-lock it after) + let was_locked = matches!(keyring, oo7::file::Keyring::Locked(_)); + + // Check if the keyring is locked and unlock if needed + if was_locked { + // Try to unlock with old password first + tracing::debug!( + "Collection {} is locked, attempting to unlock with old password", + path + ); + drop(keyring_guard); + + if let Err(e) = collection.set_locked(false, Some(old_secret.clone())).await { + tracing::warn!( + "Failed to unlock collection {} with old password: {}", + path, + e + ); + continue; + } + } else { + drop(keyring_guard); + } + + // Re-acquire the lock to get the unlocked keyring + let keyring_guard = collection.keyring.read().await; + let Some(oo7::file::Keyring::Unlocked(uk)) = keyring_guard.as_ref() else { + tracing::warn!("Collection {} is not unlocked", path); + continue; + }; + + // Validate that the old password can decrypt items in the keyring + let can_decrypt = match uk.validate_secret(old_secret).await { + Ok(valid) => valid, + Err(e) => { + tracing::warn!( + "Failed to validate old password for collection {}: {}", + path, + e + ); + continue; + } + }; + + if !can_decrypt { + tracing::debug!( + "Old password does not match keyring {} password, skipping", + path + ); + continue; + } + + // Change the keyring password + match uk.change_secret(new_secret.clone()).await { + Ok(_) => { + tracing::info!("Successfully changed password for collection: {}", path); + changed_count += 1; + + // Re-lock the collection if it was locked before we unlocked it + drop(keyring_guard); + if was_locked { + if let Err(e) = collection.set_locked(true, None).await { + tracing::warn!( + "Failed to re-lock collection {} after password change: {}", + path, + e + ); + } else { + tracing::debug!("Re-locked collection: {}", path); + } + } + } + Err(e) => { + tracing::error!("Failed to change password for collection {}: {}", path, e); + } + } + } + + Ok(changed_count) + } } impl Drop for PamListener {