| Message ID | 20200113153516.486106-1-nolange79@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] support/scripts/mkusers: allow option for system uid/gid | expand |
Hello Norbert, On Mon, 13 Jan 2020 16:35:13 +0100 Norbert Lange <nolange79@gmail.com> wrote: > Extend the mkusers script to allow -2 for uid/gid. > This value will take an identifier from the system range. > > Signed-off-by: Norbert Lange <nolange79@gmail.com> Sorry for the long delay in getting back to you. We had an earlier proposal from Stephan Henningsen doing pretty much the same: https://patchwork.ozlabs.org/project/buildroot/patch/20191023211313.6758-1-stephan+buildroot@asklandd.dk/ The argument of Stephan was pretty much beauty/consistency with what "most systems do" without much other arguments. However, based on your PATCH 2/3 and a reading of https://systemd.io/UIDS-GIDS/ it seems like systemd somehow cares about this system vs. normal user difference. Could you give some details about the *why* you did this change? Indeed, your commit log doesn't explain anything about the *why*. Also, could you compare your changes to mkusers with the ones proposed by Stephan? The ones proposed by Stephan looked quite a bit more complicated. Another (minor) question is: if we're going to go to this route of separating system and normal users, wouldn't it make sense to have -1 identify system users, and -2 identify normal users? Indeed the vast majority (all?) Buildroot packages probably want to create system users, and they already use -1. Best regards, Thomas
Thomas Petazzoni <thomas.petazzoni@bootlin.com> schrieb am Di., 15. Sep. 2020, 22:47: > Hello Norbert, > > On Mon, 13 Jan 2020 16:35:13 +0100 > Norbert Lange <nolange79@gmail.com> wrote: > > > Extend the mkusers script to allow -2 for uid/gid. > > This value will take an identifier from the system range. > > > > Signed-off-by: Norbert Lange <nolange79@gmail.com> > > Sorry for the long delay in getting back to you. We had an earlier > proposal from Stephan Henningsen doing pretty much the same: > > > https://patchwork.ozlabs.org/project/buildroot/patch/20191023211313.6758-1-stephan+buildroot@asklandd.dk/ > > The argument of Stephan was pretty much beauty/consistency with what > "most systems do" without much other arguments. > > However, based on your PATCH 2/3 and a reading of > https://systemd.io/UIDS-GIDS/ it seems like systemd somehow cares about > this system vs. normal user difference. > > Could you give some details about the *why* you did this change? > Indeed, your commit log doesn't explain anything about the *why*. > > Also, could you compare your changes to mkusers with the ones proposed > by Stephan? The ones proposed by Stephan looked quite a bit more > complicated. > > Another (minor) question is: if we're going to go to this route of > separating system and normal users, wouldn't it make sense to have -1 > identify system users, and -2 identify normal users? Indeed the vast > majority (all?) Buildroot packages probably want to create system > users, and they already use -1. > > Best regards, > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com Yeah, this was touched upon (both points), see [1]. System users exist as concept on all distros an observable effects are for ex journald spawning a separate logging stream. So no, it's not just cosmetics, and yes I'd make system user the default. Norbert [1] http://lists.busybox.net/pipermail/buildroot/2020-February/273558.html > >
I'll give a bit more detail about the way journald handles system/user UID.
there might be other places where systemd treats them differently, but
that's the only
one I know from memory.
journald collects all logs on the system
* from the daemons running around
* from the kernel/audit system
* from containers
* from user sessions.
When a user logs in, a "per user" instance of systemd is spawned that
can starts daemons for that user (ssh-agent, pulseaudio, colord...
daemons that make sense at the user level but not at the system level)
To ease the handling of permissions and reading those files, journald does
not store all logs in a single file, but in one file per user. Journald uses
ACL to allow each user to have access to his logs through normal unix
permissions instead of relying on some sort of SUID mechanism.
However, it would be a bad idea to have a separate journal file for system
users, since system users are part of the system and the log they produce
are really only for the administrator to see. So those logs are stored with
the
system logs in the machine's main log file.
The separation between the two types of users uses the UID1000 split
A quick grep in systemd yields a couple of other usages
* systemd-coredumps will tweak access right to allow non-system users to
read the core-dumps they generate
* When closing a session, logind may clean up IPC for non-system user.
The explanation is a bit complex so i'll just copy/paste the comment in
the code
/* Clean SysV + POSIX IPC objects, but only if this is not a system
user. Background: in many setups cronjobs
* are run in full PAM and thus logind sessions, even if the code
run doesn't belong to actual users but to
* system components. Since enable RemoveIPC= globally for all
users, we need to be a bit careful with such
* cases, as we shouldn't accidentally remove a system service's
IPC objects while it is running, just because
* a cronjob running as the same user just finished. Hence: exclude
system users generally from IPC clean-up,
* and do it only for normal users. */
* there is a unit condition calle ConditionUser= (and AssertUser=) that
allow to limit a unit file to only be allowed for a certain user. This
condition
can take a user name, a UID or the magic value "@system" to be allowed
to any system user
Regards
Jeremy
Hello, On Tue, 15 Sep 2020 23:29:24 +0200 Norbert Lange <nolange79@gmail.com> wrote: > Yeah, this was touched upon (both points), see [1]. > > System users exist as concept on all distros an observable effects are for > ex journald spawning a separate logging stream. Right, thanks for the additional feedback. > So no, it's not just cosmetics, and yes I'd make system user the default. Agreed. Did you compare your implementation with the one proposed by Stephan earlier ? Best regards, Thomas
diff --git a/support/scripts/mkusers b/support/scripts/mkusers index d00ba33823..1bf1336e48 100755 --- a/support/scripts/mkusers +++ b/support/scripts/mkusers @@ -8,6 +8,12 @@ MIN_UID=1000 MAX_UID=1999 MIN_GID=1000 MAX_GID=1999 +# use names from /etc/adduser.conf +FIRST_SYSTEM_UID=100 +LAST_SYSTEM_UID=999 +FIRST_SYSTEM_GID=100 +LAST_SYSTEM_GID=999 + # No more is configurable below this point #---------------------------------------------------------------------------- @@ -136,9 +142,9 @@ check_user_validity() { fail "invalid username '%s\n'" "${username}" fi - if [ ${gid} -lt -1 -o ${gid} -eq 0 ]; then + if [ ${gid} -lt -2 -o ${gid} -eq 0 ]; then fail "invalid gid '%d' for '%s'\n" ${gid} "${username}" - elif [ ${gid} -ne -1 ]; then + elif [ ${gid} -gt -1 ]; then # check the gid is not already used for another group if [ -n "${_group}" -a "${_group}" != "${group}" ]; then fail "gid '%d' for '%s' is already used by group '%s'\n" \ @@ -162,9 +168,9 @@ check_user_validity() { fi fi - if [ ${uid} -lt -1 -o ${uid} -eq 0 ]; then + if [ ${uid} -lt -2 -o ${uid} -eq 0 ]; then fail "invalid uid '%d' for '%s'\n" ${uid} "${username}" - elif [ ${uid} -ne -1 ]; then + elif [ ${uid} -gt -1 ]; then # check the uid is not already used for another user if [ -n "${_username}" -a "${_username}" != "${username}" ]; then fail "uid '%d' for '%s' already used by user '%s'\n" \ @@ -198,16 +204,18 @@ check_user_validity() { # - not already used by a group generate_gid() { local group="${1}" + local mingid="${2:-$MIN_UID}" + local maxgid="${3:-$MAX_UID}" local gid gid="$( get_gid "${group}" )" if [ -z "${gid}" ]; then - for(( gid=MIN_GID; gid<=MAX_GID; gid++ )); do + for(( gid=mingid; gid<=maxgid; gid++ )); do if [ -z "$( get_group "${gid}" )" ]; then break fi done - if [ ${gid} -gt ${MAX_GID} ]; then + if [ ${gid} -gt ${maxgid} ]; then fail "can not allocate a GID for group '%s'\n" "${group}" fi fi @@ -222,8 +230,13 @@ add_one_group() { local members # Generate a new GID if needed - if [ ${gid} -eq -1 ]; then - gid="$( generate_gid "${group}" )" + if [ ${gid} -le -1 ]; then + if [ ${gid} -eq -1 ]; then + gid="$( generate_gid "${group}" )" + else + gid="$( generate_gid "${group}" $FIRST_SYSTEM_GID $LAST_SYSTEM_GID )" + + fi fi members=$(get_members "$group") @@ -247,16 +260,19 @@ add_one_group() { # - not already used by a user generate_uid() { local username="${1}" + local minuid="${2:-$MIN_UID}" + local maxuid="${3:-$MAX_UID}" + local uid uid="$( get_uid "${username}" )" if [ -z "${uid}" ]; then - for(( uid=MIN_UID; uid<=MAX_UID; uid++ )); do + for(( uid=minuid; uid<=maxuid; uid++ )); do if [ -z "$( get_username "${uid}" )" ]; then break fi done - if [ ${uid} -gt ${MAX_UID} ]; then + if [ ${uid} -gt ${maxuid} ]; then fail "can not allocate a UID for user '%s'\n" "${username}" fi fi @@ -307,8 +323,13 @@ add_one_user() { check_user_validity "${username}" "${uid}" "${group}" "${gid}" # Generate a new UID if needed - if [ ${uid} -eq -1 ]; then - uid="$( generate_uid "${username}" )" + if [ ${uid} -le -1 ]; then + if [ ${uid} -eq -1 ]; then + uid="$( generate_uid "${username}" )" + else + uid="$( generate_uid "${username}" $FIRST_SYSTEM_UID $LAST_SYSTEM_UID )" + + fi fi # Remove any previous instance of this user @@ -399,7 +420,7 @@ main() { # Then, create all the main groups which gid *is* automatic for line in "${ENTRIES[@]}"; do read username uid group gid passwd home shell groups comment <<<"${line}" - [ ${gid} -eq -1 ] || continue # Non-automatic gid + [ ${gid} -le -1 ] || continue # Non-automatic gid add_one_group "${group}" "${gid}" done @@ -433,7 +454,7 @@ main() { for line in "${ENTRIES[@]}"; do read username uid group gid passwd home shell groups comment <<<"${line}" [ "${username}" != "-" ] || continue # Magic string to skip user creation - [ ${uid} -eq -1 ] || continue # Non-automatic uid + [ ${uid} -le -1 ] || continue # Non-automatic uid add_one_user "${username}" "${uid}" "${group}" "${gid}" "${passwd}" \ "${home}" "${shell}" "${groups}" "${comment}" done
Extend the mkusers script to allow -2 for uid/gid. This value will take an identifier from the system range. Signed-off-by: Norbert Lange <nolange79@gmail.com> --- support/scripts/mkusers | 49 +++++++++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 14 deletions(-)