diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index f6eed1166..256366631 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -106,11 +106,11 @@ jobs: with: components: clippy - name: Clippy workspace (default features) - run: cargo clippy --workspace --tests -- -D warnings + run: cargo clippy --workspace --all-targets -- -D warnings - name: Clippy client (tracing / async-std / native crypto) run: cargo clippy -p oo7 --no-default-features --features tracing,async-std,native_crypto,schema -- -D warnings - name: Clippy client (tracing / tokio / OpenSSL) - run: cargo clippy -p oo7 --no-default-features --features tracing,tokio,openssl_crypto,schema --tests -- -D warnings + run: cargo clippy -p oo7 --no-default-features --features tracing,tokio,openssl_crypto,schema --all-targets -- -D warnings meson: name: Meson diff --git a/README.md b/README.md index b46966ab3..8a90a768d 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,8 @@ The repository consists of the following projects: - [pam](./pam/): PAM integration for the server implementation - [portal](./portal/): [org.freedesktop.impl.portal.Secret](https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.impl.portal.Secret.html) implementation - [server](./server/): [org.freedesktop.secrets](https://specifications.freedesktop.org/secret-service-spec/latest/) server implementation +- [kwallet/cli](./kwallet/cli): a simple CLI to read KWallet `.kwl` files +- [kwallet/parser](./kwallet//parser): Parse Kwallet `.kwl` files, used for automatic migration on the server side. ## Hacking on oo7 services diff --git a/cli/src/main.rs b/cli/src/main.rs index 63a7e8e1b..2f0b8ee30 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_code)] + use std::{ collections::HashMap, fmt, @@ -376,9 +378,13 @@ impl Commands { }; let keyring = match (path, secret) { - (Some(path), Some(secret)) => unsafe { - Keyring::File(oo7::file::UnlockedKeyring::load_unchecked(path, secret).await?) - }, + (Some(path), Some(secret)) => { + #[allow(unsafe_code)] + unsafe { + Keyring::File(oo7::file::UnlockedKeyring::load_unchecked(path, secret).await?) + } + } + (Some(_), None) => { return Err(Error::new("A keyring requires a secret.")); } diff --git a/client/examples/file_tracing.rs b/client/examples/file_tracing.rs index 6cf252002..b8581517d 100644 --- a/client/examples/file_tracing.rs +++ b/client/examples/file_tracing.rs @@ -294,7 +294,7 @@ async fn test_search_performance() -> oo7::Result<()> { let start = Instant::now(); let items = keyring.items().await?; let all_items_time = start.elapsed(); - let valid_items = items.iter().count(); + let valid_items = items.len(); info!( "Get all items: {:?} (found {} valid items)", all_items_time, valid_items diff --git a/client/src/file/locked_keyring.rs b/client/src/file/locked_keyring.rs index 6bbab6bea..d2466312b 100644 --- a/client/src/file/locked_keyring.rs +++ b/client/src/file/locked_keyring.rs @@ -77,8 +77,11 @@ impl LockedKeyring { /// /// # Safety /// - /// The method doesn't validate that the secret can decrypt all the items in - /// the keyring. + /// This method skips validation and doesn't verify that the secret can + /// decrypt all items in the keyring. Use only for recovery scenarios where + /// you need to access a partially corrupted keyring. The keyring may + /// contain items that cannot be decrypted with the provided secret. + #[allow(unsafe_code)] pub async unsafe fn unlock_unchecked(self, secret: Secret) -> Result { self.unlock_inner(secret, false).await } diff --git a/client/src/file/unlocked_keyring.rs b/client/src/file/unlocked_keyring.rs index 60640378e..5bfbfdedd 100644 --- a/client/src/file/unlocked_keyring.rs +++ b/client/src/file/unlocked_keyring.rs @@ -49,7 +49,7 @@ impl UnlockedKeyring { Self::load_inner(path, secret, true).await } - /// Load from a keyring file. + /// Load from a keyring file without validating the secret. /// /// # Arguments /// @@ -58,12 +58,12 @@ impl UnlockedKeyring { /// /// # Safety /// - /// The secret is not validated to be the correct one to decrypt the keyring - /// items. Allowing the API user to write new items with a different - /// secret on top of previously added items with a different secret. - /// - /// As it is not a supported behaviour, this API is mostly meant for - /// recovering broken keyrings. + /// This method skips validation and doesn't verify that the secret can + /// decrypt all items in the keyring. Use only for recovery scenarios where + /// you need to access a partially corrupted keyring. The keyring may + /// contain items that cannot be decrypted with the provided secret, and + /// writing new items may use a different secret than existing items. + #[allow(unsafe_code)] #[cfg_attr(feature = "tracing", tracing::instrument(skip(secret), fields(path = ?path.as_ref())))] pub async unsafe fn load_unchecked( path: impl AsRef, @@ -83,6 +83,7 @@ impl UnlockedKeyring { if validate_items { LockedKeyring::load(path).await?.unlock(secret).await } else { + #[allow(unsafe_code)] unsafe { LockedKeyring::load(path) .await? diff --git a/client/src/lib.rs b/client/src/lib.rs index 320d165b7..a252e19fa 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -1,6 +1,7 @@ #![cfg_attr(docsrs, feature(doc_cfg))] #![deny(rustdoc::broken_intra_doc_links)] #![doc = include_str!("../README.md")] +#![deny(unsafe_code)] #[cfg(all(all(feature = "tokio", feature = "async-std"), not(doc)))] compile_error!("You can't enable both async-std & tokio features at once"); #[cfg(all(not(feature = "tokio"), not(feature = "async-std"), not(doc)))] diff --git a/kwallet/cli/src/main.rs b/kwallet/cli/src/main.rs index 2b2b984b2..f982f4b0e 100644 --- a/kwallet/cli/src/main.rs +++ b/kwallet/cli/src/main.rs @@ -1,3 +1,5 @@ +#![forbid(unsafe_code)] + use std::{collections::HashMap, io::Write, path::PathBuf, process::ExitCode}; use clap::Parser; diff --git a/kwallet/parser/src/lib.rs b/kwallet/parser/src/lib.rs index eaeddbfbf..d38e0ce72 100644 --- a/kwallet/parser/src/lib.rs +++ b/kwallet/parser/src/lib.rs @@ -1,3 +1,4 @@ +#![forbid(unsafe_code)] //! Read-only parser for KWallet file format //! //! This crate provides parsing support for KWallet wallet files: diff --git a/pam/src/socket.rs b/pam/src/socket.rs index 7468f8dd1..7a0149c3d 100644 --- a/pam/src/socket.rs +++ b/pam/src/socket.rs @@ -54,7 +54,7 @@ pub fn send_secret_to_daemon( tracing::debug!("Already running as target user (UID={uid}, GID={gid})",); let runtime = tokio::runtime::Runtime::new().map_err(SocketError::Connect)?; return runtime - .block_on(async { send_secret_to_daemon_async(message, uid, auto_start).await }); + .block_on(async { send_secret_to_daemon_async(message, uid, auto_start, None).await }); } // Need to fork and switch credentials @@ -92,8 +92,9 @@ pub fn send_secret_to_daemon( } }; - let result = runtime - .block_on(async { send_secret_to_daemon_async(message, uid, auto_start).await }); + let result = runtime.block_on(async { + send_secret_to_daemon_async(message, uid, auto_start, None).await + }); match result { Ok(_) => unsafe { libc::_exit(0) }, @@ -189,10 +190,11 @@ async fn send_secret_to_daemon_async( message: PamMessage, uid: u32, auto_start: bool, + socket_path: Option, ) -> Result<(), SocketError> { - let socket_path = std::env::var("OO7_PAM_SOCKET") - .map(PathBuf::from) - .unwrap_or_else(|_| PathBuf::from(format!("/run/user/{uid}/oo7-pam.sock"))); + let socket_path = socket_path + .or_else(|| std::env::var("OO7_PAM_SOCKET").ok().map(PathBuf::from)) + .unwrap_or_else(|| PathBuf::from(format!("/run/user/{uid}/oo7-pam.sock"))); tracing::debug!("Connecting to daemon socket at: {}", socket_path.display()); @@ -284,13 +286,9 @@ mod tests { let temp_dir = tempfile::tempdir()?; let socket_path = temp_dir.path().join("test.sock"); - // Set the environment variable to use our test socket - unsafe { - std::env::set_var("OO7_PAM_SOCKET", socket_path.to_str().unwrap()); - } - + let socket_path_clone = socket_path.clone(); let server = tokio::spawn(async move { - let listener = UnixListener::bind(&socket_path).unwrap(); + let listener = UnixListener::bind(&socket_path_clone).unwrap(); let (mut stream, _) = listener.accept().await.unwrap(); let mut length_bytes = [0u8; 4]; @@ -309,15 +307,11 @@ mod tests { let message = PamMessage::unlock("testuser".to_string(), b"testpassword".to_vec()); - let result = send_secret_to_daemon_async(message, 1000, false).await; + let result = send_secret_to_daemon_async(message, 1000, false, Some(socket_path)).await; assert!(result.is_ok()); server.await?; - unsafe { - std::env::remove_var("OO7_PAM_SOCKET"); - } - Ok(()) } } diff --git a/python/src/lib.rs b/python/src/lib.rs index 6465d0b43..95f9978b1 100644 --- a/python/src/lib.rs +++ b/python/src/lib.rs @@ -1,4 +1,4 @@ -#![allow(unsafe_op_in_unsafe_fn)] +#![forbid(unsafe_code)] use std::{collections::HashMap, sync::Arc}; diff --git a/server/src/capability.rs b/server/src/capability.rs index 697be7779..3f7e68ea7 100644 --- a/server/src/capability.rs +++ b/server/src/capability.rs @@ -1,3 +1,5 @@ +#![allow(unsafe_code)] + use rustix::{ mm::{MlockAllFlags, mlockall}, process::{Gid, Uid, getgid, getuid}, diff --git a/server/src/lib.rs b/server/src/lib.rs index 43c857042..768c5cfb9 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -1,6 +1,7 @@ // Library interface for oo7_server // Only expose test utilities when the test-util feature is enabled #![allow(unused)] +#![deny(unsafe_code)] pub(crate) mod collection; pub(crate) mod error; diff --git a/server/src/main.rs b/server/src/main.rs index 9d6fec7eb..b373cd115 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -1,3 +1,4 @@ +#![deny(unsafe_code)] mod capability; mod collection; mod error; diff --git a/server/src/pam_listener/mod.rs b/server/src/pam_listener/mod.rs index ef959a5b2..0e5adce3b 100644 --- a/server/src/pam_listener/mod.rs +++ b/server/src/pam_listener/mod.rs @@ -53,7 +53,7 @@ pub struct PamListener { impl PamListener { pub fn new(service: Service) -> Self { - let uid = unsafe { libc::getuid() }; + let uid = rustix::process::getuid().as_raw(); let socket_path = service .pam_socket .clone() @@ -108,7 +108,7 @@ impl PamListener { // 1. Root (UID 0) as PAM modules run as root during authentication // 2. Same UID as us let peer_cred = stream.peer_cred()?; - let our_uid = unsafe { libc::getuid() }; + let our_uid = rustix::process::getuid().as_raw(); let peer_uid = peer_cred.uid(); if peer_uid != 0 && peer_uid != our_uid { diff --git a/server/src/service/mod.rs b/server/src/service/mod.rs index 08de7aa21..30c9eee10 100644 --- a/server/src/service/mod.rs +++ b/server/src/service/mod.rs @@ -580,6 +580,7 @@ impl Service { } #[cfg(any(test, feature = "test-util"))] + #[allow(dead_code)] pub async fn run_with_connection( connection: zbus::Connection, data_dir: std::path::PathBuf,