Ignore system users for UPG synthesiseation (#3297)

Our unix resolver would attempt the right thing to synthesise
user private groups on linux as these are an important security
boundary. However, it turns out that almost every distro has
botched their default system user accounts, and many are
installed with numeric-only UPGs that don't resolve. In the
case that later the user does attempt to fix that, because we
synthesised as UPG for the system account, the user trying to
add the UPG would now fail. In some cases this could cause
system updates to be prevented from installing.

This change limits UPG synth to user accounts only (uid > 1000)
which is the common uid boundary on unix-like platforms.
This commit is contained in:
Firstyear 2024-12-17 13:08:17 +10:00 committed by GitHub
parent 7e9c33ab03
commit eba8dff23a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -8,6 +8,9 @@ use kanidm_unix_common::unix_passwd::{CryptPw, EtcGroup, EtcShadow, EtcUser};
use kanidm_unix_common::unix_proto::PamAuthRequest; use kanidm_unix_common::unix_proto::PamAuthRequest;
use kanidm_unix_common::unix_proto::{NssGroup, NssUser}; use kanidm_unix_common::unix_proto::{NssGroup, NssUser};
// The minimum GID that Kanidm will consider for creating a UPG
const SYSTEM_GID_BOUNDARY: u32 = 1000;
pub struct SystemProviderInternal { pub struct SystemProviderInternal {
users: HashMap<Id, Arc<EtcUser>>, users: HashMap<Id, Arc<EtcUser>>,
user_list: Vec<Arc<EtcUser>>, user_list: Vec<Arc<EtcUser>>,
@ -223,22 +226,22 @@ impl SystemProvider {
let uid = Id::Gid(user.uid); let uid = Id::Gid(user.uid);
let gid = Id::Gid(user.gid); let gid = Id::Gid(user.gid);
if user.uid != user.gid {
error!(name = %user.name, uid = %user.uid, gid = %user.gid, "user uid and gid are not the same, this may be a security risk!");
}
// Security checks. // Security checks.
if let Some(group) = system_ids_txn.groups.get(&gid) { if user.uid != user.gid {
warn!(name = %user.name, uid = %user.uid, gid = %user.gid, "user uid and gid are not the same, this may be a security risk!");
} else if let Some(group) = system_ids_txn.groups.get(&gid) {
if group.name != user.name { if group.name != user.name {
error!(name = %user.name, uid = %user.uid, gid = %user.gid, "user private group does not appear to have the same name as the user, this may be a security risk!"); warn!(name = %user.name, uid = %user.uid, gid = %user.gid, "user private group does not appear to have the same name as the user, this may be a security risk!");
} }
if !(group.members.is_empty() if !(group.members.is_empty()
|| (group.members.len() == 1 && group.members.first() == Some(&user.name))) || (group.members.len() == 1 && group.members.first() == Some(&user.name)))
{ {
error!(name = %user.name, uid = %user.uid, gid = %user.gid, members = ?group.members, "user private group must not have members, THIS IS A SECURITY RISK!"); warn!(name = %user.name, uid = %user.uid, gid = %user.gid, members = ?group.members, "user private group must not have members, THIS IS A SECURITY RISK!");
} }
} else if user.uid < SYSTEM_GID_BOUNDARY {
warn!(name = %user.name, uid = %user.uid, gid = %user.gid, "user private group is not present on system, ignoring as this is a system account.");
} else { } else {
info!(name = %user.name, uid = %user.uid, gid = %user.gid, "user private group is not present on system, synthesising it"); info!(name = %user.name, uid = %user.uid, gid = %user.gid, "user private group is not present on system, synthesising it.");
let group = Arc::new(EtcGroup { let group = Arc::new(EtcGroup {
name: user.name.clone(), name: user.name.clone(),
password: String::new(), password: String::new(),