Some minor things

.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

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
 

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."));
             }

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

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<UnlockedKeyring, Error> {
         self.unlock_inner(secret, false).await
     }

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<Path>,
@@ -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?

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)))]

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;

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:

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<PathBuf>,
 ) -> 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(())
     }
 }

python/src/lib.rs

@@ -1,4 +1,4 @@
-#![allow(unsafe_op_in_unsafe_fn)]
+#![forbid(unsafe_code)]
 
 use std::{collections::HashMap, sync::Arc};
 

server/src/capability.rs

@@ -1,3 +1,5 @@
+#![allow(unsafe_code)]
+
 use rustix::{
     mm::{MlockAllFlags, mlockall},
     process::{Gid, Uid, getgid, getuid},

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;

server/src/main.rs

@@ -1,3 +1,4 @@
+#![deny(unsafe_code)]
 mod capability;
 mod collection;
 mod error;

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 {

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,