Skip to content

Commit

Permalink
Merge pull request #396 from himmelblau-idm/dmulder/correct_tasks_gro…
Browse files Browse the repository at this point in the history
…up_gid

Ensure tasks daemon creates files w/ correct gid
  • Loading branch information
dmulder authored Feb 26, 2025
2 parents 7999db9 + 7ac56ee commit 95f9aff
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 17 deletions.
15 changes: 12 additions & 3 deletions src/common/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,10 @@ where
}
}

pub async fn get_user_ccaches(&self, account_id: Id) -> Option<(uid_t, Vec<u8>, Vec<u8>)> {
pub async fn get_user_ccaches(
&self,
account_id: Id,
) -> Option<(uid_t, uid_t, Vec<u8>, Vec<u8>)> {
let token = match self.get_usertoken(account_id.clone()).await {
Ok(Some(token)) => token,
_ => {
Expand All @@ -674,7 +677,12 @@ where

drop(hsm_lock);

Some((token.gidnumber, cloud_ccache, ad_ccache))
Some((
token.gidnumber,
token.real_gidnumber.unwrap_or(token.gidnumber),
cloud_ccache,
ad_ccache,
))
}

pub async fn get_user_prt_cookie(&self, account_id: Id) -> Option<String> {
Expand Down Expand Up @@ -1361,7 +1369,8 @@ where
) -> Result<Option<HomeDirectoryInfo>, ()> {
let token = self.get_usertoken(Id::Name(account_id.to_string())).await?;
Ok(token.as_ref().map(|tok| HomeDirectoryInfo {
gid: tok.gidnumber,
uid: tok.gidnumber,
gid: tok.real_gidnumber.unwrap_or(tok.gidnumber),
name: self.token_homedirectory_attr(tok),
aliases: self
.token_homedirectory_alias(tok)
Expand Down
7 changes: 5 additions & 2 deletions src/common/src/unix_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ impl From<PamAuthResponse> for ClientResponse {

#[derive(Serialize, Deserialize, Debug, Clone)]
pub struct HomeDirectoryInfo {
pub uid: u32,
pub gid: u32,
pub name: String,
pub aliases: Vec<String>,
Expand All @@ -150,7 +151,7 @@ pub enum TaskRequest {
HomeDirectory(HomeDirectoryInfo),
LocalGroups(String),
LogonScript(String, String),
KerberosCCache(uid_t, Vec<u8>, Vec<u8>),
KerberosCCache(uid_t, uid_t, Vec<u8>, Vec<u8>),
LoadProfilePhoto(String, String),
}

Expand All @@ -161,7 +162,9 @@ impl TaskRequest {
TaskRequest::HomeDirectory(info) => format!("HomeDirectory({:?})", info),
TaskRequest::LocalGroups(groups) => format!("LocalGroups({})", groups),
TaskRequest::LogonScript(account_id, _) => format!("LogonScript({}, ...)", account_id),
TaskRequest::KerberosCCache(uid, _, _) => format!("KerberosCCache({}, ...)", uid),
TaskRequest::KerberosCCache(uid, gid, _, _) => {
format!("KerberosCCache({}, {}, ...)", uid, gid)
}
TaskRequest::LoadProfilePhoto(account_id, _) => {
format!("LoadProfilePhoto({}, ...)", account_id)
}
Expand Down
3 changes: 2 additions & 1 deletion src/daemon/src/daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ async fn handle_client(
}

// Initialize the user Kerberos ccache
if let Some((uid, cloud_ccache, ad_ccache)) =
if let Some((uid, gid, cloud_ccache, ad_ccache)) =
cachelayer
.get_user_ccaches(Id::Name(
account_id.to_string(),
Expand All @@ -426,6 +426,7 @@ async fn handle_client(
(
TaskRequest::KerberosCCache(
uid,
gid,
cloud_ccache,
ad_ccache,
),
Expand Down
22 changes: 11 additions & 11 deletions src/daemon/src/tasks_daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ impl TaskCodec {
}
}

fn chown(path: &Path, gid: u32) -> Result<(), String> {
fn chown(path: &Path, uid: u32, gid: u32) -> Result<(), String> {
let path_os = CString::new(path.as_os_str().as_bytes())
.map_err(|_| "Unable to create c-string".to_string())?;

// Change the owner to the gid - remember, himmelblau ONLY has gid's, the uid is implied.
if unsafe { lchown(path_os.as_ptr(), gid, gid) } != 0 {
if unsafe { lchown(path_os.as_ptr(), uid, gid) } != 0 {
return Err("Unable to set ownership".to_string());
}
Ok(())
Expand Down Expand Up @@ -138,7 +138,7 @@ fn create_home_directory(
debug!(?use_selinux, "selinux for home dir labeling");
#[cfg(all(target_family = "unix", feature = "selinux"))]
let labeler = if use_selinux {
selinux_util::SelinuxLabeler::new(info.gid, home_prefix)?
selinux_util::SelinuxLabeler::new(info.uid, home_prefix)?
} else {
selinux_util::SelinuxLabeler::new_noop()
};
Expand All @@ -159,7 +159,7 @@ fn create_home_directory(
}
let _ = unsafe { umask(before) };

chown(hd_path, info.gid)?;
chown(hd_path, info.uid, info.gid)?;

// Copy in structure from /etc/skel/ if present
let skel_dir = Path::new("/etc/skel/");
Expand Down Expand Up @@ -187,7 +187,7 @@ fn create_home_directory(
} else {
fs::copy(entry.path(), dest).map_err(|e| e.to_string())?;
}
chown(dest, info.gid)?;
chown(dest, info.uid, info.gid)?;

// Create equivalence rule in the SELinux policy
#[cfg(all(target_family = "unix", feature = "selinux"))]
Expand Down Expand Up @@ -303,7 +303,7 @@ fn execute_user_script(account_id: &str, script: &str, access_token: &str) -> i3
}
}

fn write_bytes_to_file(bytes: &[u8], filename: &Path, owner: uid_t) -> i32 {
fn write_bytes_to_file(bytes: &[u8], filename: &Path, uid: uid_t, gid: uid_t) -> i32 {
let mut file = match OpenOptions::new()
.create(true)
.truncate(true)
Expand All @@ -318,7 +318,7 @@ fn write_bytes_to_file(bytes: &[u8], filename: &Path, owner: uid_t) -> i32 {
return 2;
}

if chown(filename, owner).is_err() {
if chown(filename, uid, gid).is_err() {
return 3;
}

Expand Down Expand Up @@ -379,7 +379,7 @@ async fn handle_tasks(stream: UnixStream, cfg: &HimmelblauConfig) {
return;
}
}
Some(Ok(TaskRequest::KerberosCCache(uid, cloud_ccache, ad_ccache))) => {
Some(Ok(TaskRequest::KerberosCCache(uid, gid, cloud_ccache, ad_ccache))) => {
debug!("Received task -> KerberosCCache({}, ...)", uid);
let ccache_dir_str = format!("{}{}", DEFAULT_CCACHE_DIR, uid);
let ccache_dir = Path::new(&ccache_dir_str);
Expand All @@ -395,7 +395,7 @@ async fn handle_tasks(stream: UnixStream, cfg: &HimmelblauConfig) {
}
};
let primary_name = ccache_dir.join("primary");
let _ = write_bytes_to_file(b"tkt\n", &primary_name, uid);
let _ = write_bytes_to_file(b"tkt\n", &primary_name, uid, gid);

let cloud_ret = if !cloud_ccache.is_empty() {
// The cloud_tkt is the primary only if the on-prem isn't
Expand All @@ -406,7 +406,7 @@ async fn handle_tasks(stream: UnixStream, cfg: &HimmelblauConfig) {
"tkt"
};
let cloud_ccache_name = ccache_dir.join(name);
write_bytes_to_file(&cloud_ccache, &cloud_ccache_name, uid) * 10
write_bytes_to_file(&cloud_ccache, &cloud_ccache_name, uid, gid) * 10
} else {
0
};
Expand All @@ -415,7 +415,7 @@ async fn handle_tasks(stream: UnixStream, cfg: &HimmelblauConfig) {
// If the on-prem ad_tkt exists, it overrides the primary
let name = "tkt";
let ad_ccache_name = ccache_dir.join(name);
write_bytes_to_file(&ad_ccache, &ad_ccache_name, uid) * 100
write_bytes_to_file(&ad_ccache, &ad_ccache_name, uid, gid) * 100
} else {
0
};
Expand Down

0 comments on commit 95f9aff

Please sign in to comment.