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 William Brown
parent caa8b2d7a6
commit 6b0c8be718

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::{NssGroup, NssUser};
// The minimum GID that Kanidm will consider for creating a UPG
const SYSTEM_GID_BOUNDARY: u32 = 1000;
pub struct SystemProviderInternal {
users: HashMap<Id, Arc<EtcUser>>,
user_list: Vec<Arc<EtcUser>>,
@ -223,22 +226,22 @@ impl SystemProvider {
let uid = Id::Gid(user.uid);
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.
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 {
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()
|| (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 {
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 {
name: user.name.clone(),
password: String::new(),