From c62979a45b347cb5f33ff9a76edf2b40f15f30fa Mon Sep 17 00:00:00 2001
From: Chris <anzhengchao@gmail.com>
Date: Tue, 29 Sep 2026 10:13:58 +0800
Subject: [PATCH] fix(connect): secure new offline state roots (#8218)

---
 rustfs/src/connect/offline/key_store.rs | 112 +++++++++++++++++++++++-
 1 file changed, 110 insertions(+), 2 deletions(-)

diff --git a/rustfs/src/connect/offline/key_store.rs b/rustfs/src/connect/offline/key_store.rs
index 1f67f7b7e..654a334fb 100644
--- a/rustfs/src/connect/offline/key_store.rs
+++ b/rustfs/src/connect/offline/key_store.rs
@@ -28,6 +28,11 @@
 //! key-store abstraction that would have to describe both lifecycles.
 
 use std::path::{Path, PathBuf};
+#[cfg(unix)]
+use std::{
+    fs, io,
+    os::unix::fs::{DirBuilderExt as _, MetadataExt as _, PermissionsExt as _},
+};
 
 use super::super::identity::DeviceIdentity;
 use super::super::identity_store::{IdentityStore, StoreError};
@@ -39,13 +44,18 @@ const OFFLINE_DIRECTORY: &str = "offline";
 /// The offline enrolment key of one deployment.
 #[derive(Clone, Debug)]
 pub struct OfflineKeyStore {
+    #[cfg(unix)]
+    state_root: PathBuf,
     inner: IdentityStore,
 }
 
 impl OfflineKeyStore {
     pub fn new(directory: impl AsRef<Path>) -> Self {
+        let state_root = directory.as_ref().to_path_buf();
         Self {
-            inner: IdentityStore::new(directory.as_ref().join(OFFLINE_DIRECTORY)),
+            inner: IdentityStore::new(state_root.join(OFFLINE_DIRECTORY)),
+            #[cfg(unix)]
+            state_root,
         }
     }
 
@@ -65,8 +75,106 @@ impl OfflineKeyStore {
     /// A second enrolment attempt returns the original key rather than minting a
     /// replacement: the operator may already be carrying a response for it, and
     /// two keys would mean the response and the device disagree about which one
-    /// Connect pinned.
+    /// Connect pinned. The state root must also satisfy the service IPC's
+    /// owner-only 0700 boundary; an existing unsafe root is never repaired here.
     pub fn load_or_create(&self) -> Result<DeviceIdentity, StoreError> {
+        #[cfg(unix)]
+        ensure_private_state_root(&self.state_root)?;
         self.inner.load_or_create()
     }
 }
+
+#[cfg(unix)]
+fn ensure_private_state_root(path: &Path) -> Result<(), StoreError> {
+    let mut builder = fs::DirBuilder::new();
+    builder.recursive(true).mode(0o700);
+    builder.create(path).map_err(|source| StoreError::Io {
+        path: path.to_path_buf(),
+        source,
+    })?;
+
+    let metadata = fs::symlink_metadata(path).map_err(|source| StoreError::Io {
+        path: path.to_path_buf(),
+        source,
+    })?;
+    if metadata.file_type().is_symlink()
+        || !metadata.is_dir()
+        || metadata.uid() != process_uid()
+        || metadata.permissions().mode() & 0o7777 != 0o700
+    {
+        return Err(StoreError::Io {
+            path: path.to_path_buf(),
+            source: io::Error::new(
+                io::ErrorKind::PermissionDenied,
+                "offline state root must be a real owner-owned directory with mode 0700",
+            ),
+        });
+    }
+    Ok(())
+}
+
+#[cfg(unix)]
+#[allow(unsafe_code)]
+fn process_uid() -> u32 {
+    // SAFETY: geteuid has no pointer arguments or caller preconditions.
+    unsafe { libc::geteuid() }
+}
+
+#[cfg(all(test, unix))]
+mod tests {
+    use std::{
+        fs,
+        os::unix::fs::{PermissionsExt as _, symlink},
+    };
+
+    use super::OfflineKeyStore;
+
+    #[test]
+    fn first_enrollment_creates_private_state_root() {
+        let temporary = tempfile::tempdir().unwrap();
+        let state = temporary.path().join("connect");
+        let store = OfflineKeyStore::new(&state);
+
+        store.load_or_create().unwrap();
+
+        assert_eq!(fs::metadata(&state).unwrap().permissions().mode() & 0o7777, 0o700);
+        assert_eq!(fs::metadata(store.key_path()).unwrap().permissions().mode() & 0o7777, 0o600);
+    }
+
+    #[test]
+    fn existing_public_state_root_is_rejected_without_creating_or_replacing_a_key() {
+        let temporary = tempfile::tempdir().unwrap();
+        let state = temporary.path().join("connect");
+        fs::create_dir(&state).unwrap();
+        fs::set_permissions(&state, fs::Permissions::from_mode(0o755)).unwrap();
+        let store = OfflineKeyStore::new(&state);
+
+        let error = store.load_or_create().err().expect("public state root must fail closed");
+        assert!(error.to_string().contains("mode 0700"));
+        assert!(!store.key_path().exists());
+        assert_eq!(fs::metadata(&state).unwrap().permissions().mode() & 0o7777, 0o755);
+
+        fs::set_permissions(&state, fs::Permissions::from_mode(0o700)).unwrap();
+        store.load_or_create().unwrap();
+        let original = fs::read(store.key_path()).unwrap();
+        fs::set_permissions(&state, fs::Permissions::from_mode(0o755)).unwrap();
+
+        assert!(store.load_or_create().is_err());
+        assert_eq!(fs::read(store.key_path()).unwrap(), original);
+        assert_eq!(fs::metadata(&state).unwrap().permissions().mode() & 0o7777, 0o755);
+    }
+
+    #[test]
+    fn symbolic_state_root_is_rejected_before_key_generation() {
+        let temporary = tempfile::tempdir().unwrap();
+        let target = temporary.path().join("target");
+        fs::create_dir(&target).unwrap();
+        fs::set_permissions(&target, fs::Permissions::from_mode(0o700)).unwrap();
+        let state = temporary.path().join("connect");
+        symlink(&target, &state).unwrap();
+        let store = OfflineKeyStore::new(&state);
+
+        assert!(store.load_or_create().is_err());
+        assert!(!target.join("offline/device.key").exists());
+    }
+}