20240716 check mkdir (#2906)

unix_integration/resolver/src/bin/kanidm_unixd_tasks.rs

@@ -13,7 +13,7 @@
 use std::ffi::CString;
 use std::os::unix::ffi::OsStrExt;
 use std::os::unix::fs::symlink;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::process::ExitCode;
 use std::time::Duration;
 use std::{fs, io};
@@ -88,31 +88,43 @@ fn chown(path: &Path, gid: u32) -> Result<(), String> {
 
 fn create_home_directory(
     info: &HomeDirectoryInfo,
-    home_prefix: &str,
+    home_prefix_path: &Path,
-    home_mount_prefix: &Option<String>,
+    home_mount_prefix_path: Option<&PathBuf>,
     use_etc_skel: bool,
     use_selinux: bool,
 ) -> Result<(), String> {
-    // Final sanity check to prevent certain classes of attacks.
+    // Final sanity check to prevent certain classes of attacks. This should *never*
+    // be possible, but we assert this to be sure.
     let name = info.name.trim_start_matches('.').replace(['/', '\\'], "");
 
-    let home_prefix_path = Path::new(home_prefix);
+    let home_mount_prefix_path_is_set = home_mount_prefix_path.is_some();
-    let home_mount_prefix_path = Path::new(home_mount_prefix.as_deref().unwrap_or(home_prefix));
+
+    let home_prefix_path = home_prefix_path
+        .canonicalize()
+        .map_err(|e| format!("{:?}", e))?;
+
+    let home_mount_prefix_path = home_mount_prefix_path
+        .unwrap_or_else(|| &home_prefix_path)
+        .canonicalize()
+        .map_err(|e| format!("{:?}", e))?;
 
     // Does our home_prefix actually exist?
-    if !home_prefix_path.exists() || !home_prefix_path.is_dir() {
+    if !home_prefix_path.exists() || !home_prefix_path.is_dir() || !home_prefix_path.is_absolute() {
-        return Err("Invalid home_prefix from configuration".to_string());
+        return Err("Invalid home_prefix from configuration - home_prefix path must exist, must be a directory, and must be absolute (not relative)".to_string());
     }
 
-    if !home_mount_prefix_path.exists() || !home_mount_prefix_path.is_dir() {
+    if !home_mount_prefix_path.exists()
-        return Err("Invalid home_mount_prefix from configuration".to_string());
+        || !home_mount_prefix_path.is_dir()
+        || !home_mount_prefix_path.is_absolute()
+    {
+        return Err("Invalid home_mount_prefix from configuration - home_prefix path must exist, must be a directory, and must be absolute (not relative)".to_string());
     }
 
     // Actually process the request here.
-    let hd_path_raw = format!("{}{}", home_prefix, name);
+    let hd_path = Path::join(&home_prefix_path, &name);
-    let hd_path = Path::new(&hd_path_raw);
 
-    // Assert the resulting named home path is consistent and correct.
+    // Assert the resulting named home path is consistent and correct. This is to ensure that
+    // the complete home path is not a path traversal outside of the home_prefixes.
     if let Some(pp) = hd_path.parent() {
         if pp != home_prefix_path {
             return Err("Invalid home directory name - not within home_prefix".to_string());
@@ -121,6 +133,19 @@ fn create_home_directory(
         return Err("Invalid/Corrupt home directory path - no prefix found".to_string());
     }
 
+    let hd_mount_path = Path::join(&home_mount_prefix_path, &name);
+
+    if let Some(pp) = hd_mount_path.parent() {
+        if pp != home_mount_prefix_path {
+            return Err(
+                "Invalid home directory name - not within home_prefix/home_mount_prefix"
+                    .to_string(),
+            );
+        }
+    } else {
+        return Err("Invalid/Corrupt home directory path - no prefix found".to_string());
+    }
+
     // Get a handle to the SELinux labeling interface
     debug!(?use_selinux, "selinux for home dir labeling");
     #[cfg(all(target_family = "unix", feature = "selinux"))]
@@ -130,8 +155,8 @@ fn create_home_directory(
         selinux_util::SelinuxLabeler::new_noop()
     };
 
-    // Does the home directory exist?
+    // Does the home directory exist? This is checking the *true* home mount storage.
-    if !hd_path.exists() {
+    if !hd_mount_path.exists() {
         // Set the SELinux security context for file creation
         #[cfg(all(target_family = "unix", feature = "selinux"))]
         labeler.do_setfscreatecon_for_path()?;
@@ -140,20 +165,20 @@ fn create_home_directory(
         let before = unsafe { umask(0o0027) };
 
         // Create the dir
-        if let Err(e) = fs::create_dir_all(hd_path) {
+        if let Err(e) = fs::create_dir_all(&hd_mount_path) {
             let _ = unsafe { umask(before) };
             return Err(format!("{:?}", e));
         }
         let _ = unsafe { umask(before) };
 
-        chown(hd_path, info.gid)?;
+        chown(&hd_mount_path, info.gid)?;
 
         // Copy in structure from /etc/skel/ if present
         let skel_dir = Path::new("/etc/skel/");
         if use_etc_skel && skel_dir.exists() {
             info!("preparing homedir using /etc/skel");
             for entry in WalkDir::new(skel_dir).into_iter().filter_map(|e| e.ok()) {
-                let dest = &hd_path.join(
+                let dest = &hd_mount_path.join(
                     entry
                         .path()
                         .strip_prefix(skel_dir)
@@ -178,7 +203,7 @@ fn create_home_directory(
 
                 // Create equivalence rule in the SELinux policy
                 #[cfg(all(target_family = "unix", feature = "selinux"))]
-                labeler.setup_equivalence_rule(&hd_path_raw)?;
+                labeler.setup_equivalence_rule(&hd_mount_path)?;
             }
         }
     }
@@ -187,28 +212,34 @@ fn create_home_directory(
     #[cfg(all(target_family = "unix", feature = "selinux"))]
     labeler.set_default_context_for_fs_objects()?;
 
-    let name_rel_path = Path::new(&name);
+    // If there is a mount prefix we use it, otherwise we use a relative path
-    // Does the aliases exist
+    // within the same directory.
+    let name_rel_path = if home_mount_prefix_path_is_set {
+        // Use the absolute path.
+        hd_mount_path.as_ref()
+    } else {
+        Path::new(&name)
+    };
+
+    // Do the aliases exist?
     for alias in info.aliases.iter() {
         // Sanity check the alias.
         // let alias = alias.replace(".", "").replace("/", "").replace("\\", "");
         let alias = alias.trim_start_matches('.').replace(['/', '\\'], "");
-        let alias_path_raw = format!("{}{}", home_prefix, alias);
-        let alias_path = Path::new(&alias_path_raw);
 
-        // Assert the resulting alias path is consistent and correct.
+        let alias_path = Path::join(&home_prefix_path, &alias);
+
+        // Assert the resulting alias path is consistent and correct within the home_prefix.
         if let Some(pp) = alias_path.parent() {
-            if pp != home_mount_prefix_path {
+            if pp != home_prefix_path {
-                return Err(
+                return Err("Invalid home directory alias - not within home_prefix".to_string());
-                    "Invalid home directory alias - not within home_mount_prefix".to_string(),
-                );
             }
         } else {
             return Err("Invalid/Corrupt alias directory path - no prefix found".to_string());
         }
 
         if alias_path.exists() {
-            let attr = match fs::symlink_metadata(alias_path) {
+            let attr = match fs::symlink_metadata(&alias_path) {
                 Ok(a) => a,
                 Err(e) => {
                     return Err(format!("{:?}", e));
@@ -217,12 +248,19 @@ fn create_home_directory(
 
             if attr.file_type().is_symlink() {
                 // Probably need to update it.
-                if let Err(e) = fs::remove_file(alias_path) {
+                if let Err(e) = fs::remove_file(&alias_path) {
                     return Err(format!("{:?}", e));
                 }
-                if let Err(e) = symlink(name_rel_path, alias_path) {
+                if let Err(e) = symlink(name_rel_path, &alias_path) {
                     return Err(format!("{:?}", e));
                 }
+            } else {
+                warn!(
+                    ?alias_path,
+                    ?alias,
+                    ?name,
+                    "home directory alias is not a symlink, unable to update"
+                );
             }
         } else {
             // Does not exist. Create.
@@ -244,8 +282,8 @@ async fn handle_tasks(stream: UnixStream, cfg: &KanidmUnixdConfig) {
 
                 let resp = match create_home_directory(
                     &info,
-                    &cfg.home_prefix,
+                    cfg.home_prefix.as_ref(),
-                    &cfg.home_mount_prefix,
+                    cfg.home_mount_prefix.as_ref(),
                     cfg.use_etc_skel,
                     cfg.selinux,
                 ) {

unix_integration/resolver/src/resolver.rs

@@ -4,7 +4,7 @@ use std::collections::BTreeSet;
 use std::fmt::Display;
 use std::num::NonZeroUsize;
 use std::ops::{Add, DerefMut, Sub};
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::string::ToString;
 use std::time::{Duration, SystemTime};
 
@@ -77,7 +77,7 @@ pub struct Resolver {
     pam_allow_groups: BTreeSet<String>,
     timeout_seconds: u64,
     default_shell: String,
-    home_prefix: String,
+    home_prefix: PathBuf,
     home_attr: HomeAttr,
     home_alias: Option<HomeAttr>,
     uid_attr_map: UidAttr,
@@ -107,7 +107,7 @@ impl Resolver {
         timeout_seconds: u64,
         pam_allow_groups: Vec<String>,
         default_shell: String,
-        home_prefix: String,
+        home_prefix: PathBuf,
         home_attr: HomeAttr,
         home_alias: Option<HomeAttr>,
         uid_attr_map: UidAttr,
@@ -748,7 +748,10 @@ impl Resolver {
 
     #[inline(always)]
     fn token_abs_homedirectory(&self, token: &UserToken) -> String {
-        format!("{}{}", self.home_prefix, self.token_homedirectory(token))
+        self.home_prefix
+            .join(self.token_homedirectory(token))
+            .to_string_lossy()
+            .to_string()
     }
 
     #[inline(always)]

unix_integration/resolver/src/selinux_util.rs

@@ -103,14 +103,14 @@ impl SelinuxLabeler {
         }
     }
 
-    pub fn setup_equivalence_rule(&self, path: &str) -> Result<(), String> {
+    pub fn setup_equivalence_rule<P: AsRef<Path>>(&self, path: P) -> Result<(), String> {
         match &self {
             SelinuxLabeler::None => Ok(()),
             SelinuxLabeler::Enabled {
                 labeler: _,
                 sel_lookup_path_raw,
             } => Command::new("semanage")
-                .args(["fcontext", "-ae", sel_lookup_path_raw, &path])
+                .args(["fcontext", "-ae", sel_lookup_path_raw, path.as_ref()])
                 .spawn()
                 .map(|_| ())
                 .map_err(|_| "Failed creating SELinux policy equivalence rule".to_string()),

unix_integration/resolver/src/unix_config.rs

@@ -2,7 +2,7 @@ use std::env;
 use std::fmt::{Display, Formatter};
 use std::fs::File;
 use std::io::{ErrorKind, Read};
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 #[cfg(all(target_family = "unix", feature = "selinux"))]
 use crate::selinux_util;
@@ -70,8 +70,8 @@ pub struct KanidmUnixdConfig {
     pub unix_sock_timeout: u64,
     pub pam_allowed_login_groups: Vec<String>,
     pub default_shell: String,
-    pub home_prefix: String,
+    pub home_prefix: PathBuf,
-    pub home_mount_prefix: Option<String>,
+    pub home_mount_prefix: Option<PathBuf>,
     pub home_attr: HomeAttr,
     pub home_alias: Option<HomeAttr>,
     pub use_etc_skel: bool,
@@ -105,9 +105,9 @@ impl Display for KanidmUnixdConfig {
             self.pam_allowed_login_groups
         )?;
         writeln!(f, "default_shell: {}", self.default_shell)?;
-        writeln!(f, "home_prefix: {}", self.home_prefix)?;
+        writeln!(f, "home_prefix: {:?}", self.home_prefix)?;
         match self.home_mount_prefix.as_deref() {
-            Some(val) => writeln!(f, "home_mount_prefix: {}", val)?,
+            Some(val) => writeln!(f, "home_mount_prefix: {:?}", val)?,
             None => writeln!(f, "home_mount_prefix: unset")?,
         }
         writeln!(f, "home_attr: {}", self.home_attr)?;
@@ -152,7 +152,7 @@ impl KanidmUnixdConfig {
             cache_timeout: DEFAULT_CACHE_TIMEOUT,
             pam_allowed_login_groups: Vec::new(),
             default_shell: DEFAULT_SHELL.to_string(),
-            home_prefix: DEFAULT_HOME_PREFIX.to_string(),
+            home_prefix: DEFAULT_HOME_PREFIX.into(),
             home_mount_prefix: None,
             home_attr: DEFAULT_HOME_ATTR,
             home_alias: DEFAULT_HOME_ALIAS,
@@ -228,8 +228,11 @@ impl KanidmUnixdConfig {
                 .pam_allowed_login_groups
                 .unwrap_or(self.pam_allowed_login_groups),
             default_shell: config.default_shell.unwrap_or(self.default_shell),
-            home_prefix: config.home_prefix.unwrap_or(self.home_prefix),
+            home_prefix: config
-            home_mount_prefix: config.home_mount_prefix,
+                .home_prefix
+                .map(|p| p.into())
+                .unwrap_or(self.home_prefix.clone()),
+            home_mount_prefix: config.home_mount_prefix.map(|p| p.into()),
             home_attr: config
                 .home_attr
                 .and_then(|v| match v.as_str() {

unix_integration/resolver/tests/cache_layer_test.rs

@@ -131,7 +131,7 @@ async fn setup_test(fix_fn: Fixture) -> (Resolver, KanidmClient) {
         300,
         vec!["allowed_group".to_string()],
         DEFAULT_SHELL.to_string(),
-        DEFAULT_HOME_PREFIX.to_string(),
+        DEFAULT_HOME_PREFIX.into(),
         DEFAULT_HOME_ATTR,
         DEFAULT_HOME_ALIAS,
         DEFAULT_UID_ATTR_MAP,