* [Buildroot] [PATCH 1/2] support/scripts/mkusers: allow option for system uid/gid
@ 2022-01-14 10:12 Norbert Lange
2022-01-14 10:12 ` [Buildroot] [PATCH 2/2] mkusers: change default from normal to system user Norbert Lange
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Norbert Lange @ 2022-01-14 10:12 UTC (permalink / raw)
To: buildroot; +Cc: Norbert Lange
Some software decides based on uid/gid whether a user is a
system or normal/human user, with differnt behaviour for those
flavors (example journald [2]).
So adding logic to create system-users is necessary, we take
the now common ranges from [1].
This extends the mkusers script to allow -2 for uid/gid,
this argument will take an identifier from the system range.
System/user ranges are added as variables, and the argument
for user/system uid was added as variable aswell.
Thus some magic constants could be removed, some further
occurences of -1 were replaced with equivalent logic.
[1] - https://systemd.io/UIDS-GIDS/
[2] - https://www.freedesktop.org/software/systemd/man/journald.conf.html
Signed-off-by: Norbert Lange <nolange79@gmail.com>
---
support/scripts/mkusers | 57 +++++++++++++++++++++++++++++------------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/support/scripts/mkusers b/support/scripts/mkusers
index d00ba33823..9d8295e8a3 100755
--- a/support/scripts/mkusers
+++ b/support/scripts/mkusers
@@ -8,6 +8,15 @@ 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
+# argument to automatically crease system/user id
+AUTO_SYSTEM_ID=-2
+AUTO_USER_ID=-1
+
# No more is configurable below this point
#----------------------------------------------------------------------------
@@ -136,9 +145,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} -ge 0 ]; 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 +171,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} -ge 0 ]; 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 +207,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 +233,12 @@ add_one_group() {
local members
# Generate a new GID if needed
- if [ ${gid} -eq -1 ]; then
- gid="$( generate_gid "${group}" )"
+ if [ ${gid} -lt 0 ]; then
+ if [ ${gid} -eq ${AUTO_USER_ID} ]; then
+ gid="$( generate_gid "${group}" )"
+ else
+ gid="$( generate_gid "${group}" $FIRST_SYSTEM_GID $LAST_SYSTEM_GID )"
+ fi
fi
members=$(get_members "$group")
@@ -247,16 +262,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 +325,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} -lt 0 ]; then
+ if [ ${uid} -eq ${AUTO_USER_ID} ]; 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
@@ -384,8 +407,8 @@ main() {
ENTRIES+=( "${line}" )
done < <( sed -r -e 's/#.*//; /^[[:space:]]*$/d;' "${USERS_TABLE}" )
- # We first create groups whose gid is not -1, and then we create groups
- # whose gid is -1 (automatic), so that, if a group is defined both with
+ # We first create groups whose gid is positive, and then we create groups
+ # whose gid is automatic, so that, if a group is defined both with
# a specified gid and an automatic gid, we ensure the specified gid is
# used, rather than a different automatic gid is computed.
@@ -399,7 +422,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} -lt 0 ] || continue # Non-automatic gid
add_one_group "${group}" "${gid}"
done
@@ -410,7 +433,7 @@ main() {
read username uid group gid passwd home shell groups comment <<<"${line}"
if [ "${groups}" != "-" ]; then
for g in ${groups//,/ }; do
- add_one_group "${g}" -1
+ add_one_group "${g}" ${AUTO_USER_ID}
done
fi
done
@@ -433,7 +456,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} -lt 0 ] || continue # Non-automatic uid
add_one_user "${username}" "${uid}" "${group}" "${gid}" "${passwd}" \
"${home}" "${shell}" "${groups}" "${comment}"
done
--
2.34.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 2/2] mkusers: change default from normal to system user
2022-01-14 10:12 [Buildroot] [PATCH 1/2] support/scripts/mkusers: allow option for system uid/gid Norbert Lange
@ 2022-01-14 10:12 ` Norbert Lange
2022-01-16 12:27 ` Arnout Vandecappelle
2022-02-05 22:14 ` Arnout Vandecappelle
2022-01-16 12:25 ` [Buildroot] [PATCH 1/2] support/scripts/mkusers: allow option for system uid/gid Arnout Vandecappelle
2022-02-05 22:13 ` Arnout Vandecappelle
2 siblings, 2 replies; 7+ messages in thread
From: Norbert Lange @ 2022-01-14 10:12 UTC (permalink / raw)
To: buildroot; +Cc: Norbert Lange, Thomas De Schampheleire
for all packages, this is the fitting default,
but the fallout will be user provided tables.
Also update the docs with the chagned behaviour.
Signed-off-by: Norbert Lange <nolange79@gmail.com>
---
docs/manual/makeusers-syntax.txt | 11 +++++++----
support/scripts/mkusers | 6 +++---
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/docs/manual/makeusers-syntax.txt b/docs/manual/makeusers-syntax.txt
index 467e596230..3d1013f447 100644
--- a/docs/manual/makeusers-syntax.txt
+++ b/docs/manual/makeusers-syntax.txt
@@ -20,13 +20,16 @@ Where:
It can not be +root+, and must be unique. If set to +-+, then just a
group will be created.
- +uid+ is the desired UID for the user. It must be unique, and not
- +0+. If set to +-1+, then a unique UID will be computed by Buildroot
- in the range [1000...1999]
+ +0+. If set to +-1+ or +-2+, then a unique UID will be computed by
+ Buildroot, with +-1+ denoting a system UID from [100...999] and +-2+
+ denoting a user UID from [1000...1999].
- +group+ is the desired name for the user's main group. It can not
be +root+. If the group does not exist, it will be created.
- +gid+ is the desired GID for the user's main group. It must be unique,
- and not +0+. If set to +-1+, and the group does not already exist, then
- a unique GID will be computed by Buildroot in the range [1000..1999]
+ and not +0+. If set to +-1+ or +-2+, and the group does not already
+ exist, then a unique GID will be computed by Buildroot, with +-1+
+ denoting a system GID from [100...999] and +-2+ denoting a user GID
+ from [1000...1999].
- +password+ is the crypt(3)-encoded password. If prefixed with +!+,
then login is disabled. If prefixed with +=+, then it is interpreted
as clear-text, and will be crypt-encoded (using MD5). If prefixed with
diff --git a/support/scripts/mkusers b/support/scripts/mkusers
index 9d8295e8a3..f7a3180e30 100755
--- a/support/scripts/mkusers
+++ b/support/scripts/mkusers
@@ -14,8 +14,8 @@ LAST_SYSTEM_UID=999
FIRST_SYSTEM_GID=100
LAST_SYSTEM_GID=999
# argument to automatically crease system/user id
-AUTO_SYSTEM_ID=-2
-AUTO_USER_ID=-1
+AUTO_SYSTEM_ID=-1
+AUTO_USER_ID=-2
# No more is configurable below this point
#----------------------------------------------------------------------------
@@ -433,7 +433,7 @@ main() {
read username uid group gid passwd home shell groups comment <<<"${line}"
if [ "${groups}" != "-" ]; then
for g in ${groups//,/ }; do
- add_one_group "${g}" ${AUTO_USER_ID}
+ add_one_group "${g}" ${AUTO_SYSTEM_ID}
done
fi
done
--
2.34.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Buildroot] [PATCH 1/2] support/scripts/mkusers: allow option for system uid/gid
2022-01-14 10:12 [Buildroot] [PATCH 1/2] support/scripts/mkusers: allow option for system uid/gid Norbert Lange
2022-01-14 10:12 ` [Buildroot] [PATCH 2/2] mkusers: change default from normal to system user Norbert Lange
@ 2022-01-16 12:25 ` Arnout Vandecappelle
2022-01-17 9:34 ` Norbert Lange
2022-02-05 22:13 ` Arnout Vandecappelle
2 siblings, 1 reply; 7+ messages in thread
From: Arnout Vandecappelle @ 2022-01-16 12:25 UTC (permalink / raw)
To: Norbert Lange, buildroot
On 14/01/2022 11:12, Norbert Lange wrote:
> Some software decides based on uid/gid whether a user is a
> system or normal/human user, with differnt behaviour for those
> flavors (example journald [2]).
Thank you for the short yet sufficient summary!
>
> So adding logic to create system-users is necessary, we take
> the now common ranges from [1].
>
> This extends the mkusers script to allow -2 for uid/gid,
> this argument will take an identifier from the system range.
>
> System/user ranges are added as variables, and the argument
> for user/system uid was added as variable aswell.
> Thus some magic constants could be removed, some further
> occurences of -1 were replaced with equivalent logic.
>
> [1] - https://systemd.io/UIDS-GIDS/
> [2] - https://www.freedesktop.org/software/systemd/man/journald.conf.html
>
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
You forgot to mention that this is v2. v1 is here [1].
I think the only change is in the commit message.
> ---
> support/scripts/mkusers | 57 +++++++++++++++++++++++++++++------------
> 1 file changed, 40 insertions(+), 17 deletions(-)
>
> diff --git a/support/scripts/mkusers b/support/scripts/mkusers
> index d00ba33823..9d8295e8a3 100755
> --- a/support/scripts/mkusers
> +++ b/support/scripts/mkusers
> @@ -8,6 +8,15 @@ 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
> +# argument to automatically crease system/user id
> +AUTO_SYSTEM_ID=-2
Wouldn't it be better to use -1 for system users (= the usual case) and add -2
for normal users? System users are the usual, after all, and all current in-tree
uses of -1 are definitely system users.
I though that someone had already made this remark, but I can't find any trace
of it.
> +AUTO_USER_ID=-1
> +
[snip]
> @@ -222,8 +233,12 @@ add_one_group() {
> local members
>
> # Generate a new GID if needed
> - if [ ${gid} -eq -1 ]; then
> - gid="$( generate_gid "${group}" )"
> + if [ ${gid} -lt 0 ]; then
> + if [ ${gid} -eq ${AUTO_USER_ID} ]; then
> + gid="$( generate_gid "${group}" )"
(nitpick) For symmetry, I'd always pass the first and last as argument here.
> + else
> + gid="$( generate_gid "${group}" $FIRST_SYSTEM_GID $LAST_SYSTEM_GID )"
> + fi
> fi
(nitpick) I'd structure the entire block above as follows:
if [ ${gid} -eq ${AUTO_USER_ID} ]; then
elif [ ${gid} -eq ${AUTO_SYSTEM_ID} ]; then
fi
That's one level less deep, and more explicit about the system-id branch.
Both remarks also apply to the uid case of course.
>
> members=$(get_members "$group")
> @@ -247,16 +262,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}"
So here I'd remove the defaults and instead always pass the arguments.
Regards,
Arnout
[1]
https://patchwork.ozlabs.org/project/buildroot/list/?series=152843&state=%2A&archive=both
> +
> 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 +325,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} -lt 0 ]; then
> + if [ ${uid} -eq ${AUTO_USER_ID} ]; 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
> @@ -384,8 +407,8 @@ main() {
> ENTRIES+=( "${line}" )
> done < <( sed -r -e 's/#.*//; /^[[:space:]]*$/d;' "${USERS_TABLE}" )
>
> - # We first create groups whose gid is not -1, and then we create groups
> - # whose gid is -1 (automatic), so that, if a group is defined both with
> + # We first create groups whose gid is positive, and then we create groups
> + # whose gid is automatic, so that, if a group is defined both with
> # a specified gid and an automatic gid, we ensure the specified gid is
> # used, rather than a different automatic gid is computed.
>
> @@ -399,7 +422,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} -lt 0 ] || continue # Non-automatic gid
> add_one_group "${group}" "${gid}"
> done
>
> @@ -410,7 +433,7 @@ main() {
> read username uid group gid passwd home shell groups comment <<<"${line}"
> if [ "${groups}" != "-" ]; then
> for g in ${groups//,/ }; do
> - add_one_group "${g}" -1
> + add_one_group "${g}" ${AUTO_USER_ID}
> done
> fi
> done
> @@ -433,7 +456,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} -lt 0 ] || continue # Non-automatic uid
> add_one_user "${username}" "${uid}" "${group}" "${gid}" "${passwd}" \
> "${home}" "${shell}" "${groups}" "${comment}"
> done
>
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Buildroot] [PATCH 2/2] mkusers: change default from normal to system user
2022-01-14 10:12 ` [Buildroot] [PATCH 2/2] mkusers: change default from normal to system user Norbert Lange
@ 2022-01-16 12:27 ` Arnout Vandecappelle
2022-02-05 22:14 ` Arnout Vandecappelle
1 sibling, 0 replies; 7+ messages in thread
From: Arnout Vandecappelle @ 2022-01-16 12:27 UTC (permalink / raw)
To: Norbert Lange, buildroot; +Cc: Thomas De Schampheleire
On 14/01/2022 11:12, Norbert Lange wrote:
> for all packages, this is the fitting default,
Ah, this is where I saw the remark! :-) I should have read the entire series
before commenting.
This patch should be squashed with the preceding one.
Since my only remarks are nitpicks, I'm going to merge it as is (after
squashing) - but later. So I'll already give it my
Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> but the fallout will be user provided tables.
>
> Also update the docs with the chagned behaviour.
>
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
> docs/manual/makeusers-syntax.txt | 11 +++++++----
> support/scripts/mkusers | 6 +++---
> 2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/docs/manual/makeusers-syntax.txt b/docs/manual/makeusers-syntax.txt
> index 467e596230..3d1013f447 100644
> --- a/docs/manual/makeusers-syntax.txt
> +++ b/docs/manual/makeusers-syntax.txt
> @@ -20,13 +20,16 @@ Where:
> It can not be +root+, and must be unique. If set to +-+, then just a
> group will be created.
> - +uid+ is the desired UID for the user. It must be unique, and not
> - +0+. If set to +-1+, then a unique UID will be computed by Buildroot
> - in the range [1000...1999]
> + +0+. If set to +-1+ or +-2+, then a unique UID will be computed by
> + Buildroot, with +-1+ denoting a system UID from [100...999] and +-2+
> + denoting a user UID from [1000...1999].
> - +group+ is the desired name for the user's main group. It can not
> be +root+. If the group does not exist, it will be created.
> - +gid+ is the desired GID for the user's main group. It must be unique,
> - and not +0+. If set to +-1+, and the group does not already exist, then
> - a unique GID will be computed by Buildroot in the range [1000..1999]
> + and not +0+. If set to +-1+ or +-2+, and the group does not already
> + exist, then a unique GID will be computed by Buildroot, with +-1+
> + denoting a system GID from [100...999] and +-2+ denoting a user GID
> + from [1000...1999].
> - +password+ is the crypt(3)-encoded password. If prefixed with +!+,
> then login is disabled. If prefixed with +=+, then it is interpreted
> as clear-text, and will be crypt-encoded (using MD5). If prefixed with
> diff --git a/support/scripts/mkusers b/support/scripts/mkusers
> index 9d8295e8a3..f7a3180e30 100755
> --- a/support/scripts/mkusers
> +++ b/support/scripts/mkusers
> @@ -14,8 +14,8 @@ LAST_SYSTEM_UID=999
> FIRST_SYSTEM_GID=100
> LAST_SYSTEM_GID=999
> # argument to automatically crease system/user id
> -AUTO_SYSTEM_ID=-2
> -AUTO_USER_ID=-1
> +AUTO_SYSTEM_ID=-1
> +AUTO_USER_ID=-2
>
> # No more is configurable below this point
> #----------------------------------------------------------------------------
> @@ -433,7 +433,7 @@ main() {
> read username uid group gid passwd home shell groups comment <<<"${line}"
> if [ "${groups}" != "-" ]; then
> for g in ${groups//,/ }; do
> - add_one_group "${g}" ${AUTO_USER_ID}
> + add_one_group "${g}" ${AUTO_SYSTEM_ID}
> done
> fi
> done
>
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Buildroot] [PATCH 1/2] support/scripts/mkusers: allow option for system uid/gid
2022-01-16 12:25 ` [Buildroot] [PATCH 1/2] support/scripts/mkusers: allow option for system uid/gid Arnout Vandecappelle
@ 2022-01-17 9:34 ` Norbert Lange
0 siblings, 0 replies; 7+ messages in thread
From: Norbert Lange @ 2022-01-17 9:34 UTC (permalink / raw)
To: Arnout Vandecappelle; +Cc: buildroot
Am So., 16. Jan. 2022 um 13:25 Uhr schrieb Arnout Vandecappelle
<arnout@mind.be>:
>
>
>
> On 14/01/2022 11:12, Norbert Lange wrote:
> > Some software decides based on uid/gid whether a user is a
> > system or normal/human user, with differnt behaviour for those
> > flavors (example journald [2]).
>
> Thank you for the short yet sufficient summary!
>
> >
> > So adding logic to create system-users is necessary, we take
> > the now common ranges from [1].
> >
> > This extends the mkusers script to allow -2 for uid/gid,
> > this argument will take an identifier from the system range.
> >
> > System/user ranges are added as variables, and the argument
> > for user/system uid was added as variable aswell.
> > Thus some magic constants could be removed, some further
> > occurences of -1 were replaced with equivalent logic.
> >
> > [1] - https://systemd.io/UIDS-GIDS/
> > [2] - https://www.freedesktop.org/software/systemd/man/journald.conf.html
> >
> > Signed-off-by: Norbert Lange <nolange79@gmail.com>
>
> You forgot to mention that this is v2. v1 is here [1].
I did not forget it, just did not find references I pushed
the patch before.
>
> I think the only change is in the commit message.
Nah, its a bit more.
>
> > ---
> > support/scripts/mkusers | 57 +++++++++++++++++++++++++++++------------
> > 1 file changed, 40 insertions(+), 17 deletions(-)
> >
> > diff --git a/support/scripts/mkusers b/support/scripts/mkusers
> > index d00ba33823..9d8295e8a3 100755
> > --- a/support/scripts/mkusers
> > +++ b/support/scripts/mkusers
> > @@ -8,6 +8,15 @@ 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
> > +# argument to automatically crease system/user id
> > +AUTO_SYSTEM_ID=-2
>
> Wouldn't it be better to use -1 for system users (= the usual case) and add -2
> for normal users? System users are the usual, after all, and all current in-tree
> uses of -1 are definitely system users.
>
> I though that someone had already made this remark, but I can't find any trace
> of it.
>
> > +AUTO_USER_ID=-1
> > +
>
> [snip]
> > @@ -222,8 +233,12 @@ add_one_group() {
> > local members
> >
> > # Generate a new GID if needed
> > - if [ ${gid} -eq -1 ]; then
> > - gid="$( generate_gid "${group}" )"
> > + if [ ${gid} -lt 0 ]; then
> > + if [ ${gid} -eq ${AUTO_USER_ID} ]; then
> > + gid="$( generate_gid "${group}" )"
>
> (nitpick) For symmetry, I'd always pass the first and last as argument here.
>
> > + else
> > + gid="$( generate_gid "${group}" $FIRST_SYSTEM_GID $LAST_SYSTEM_GID )"
> > + fi
> > fi
>
> (nitpick) I'd structure the entire block above as follows:
>
> if [ ${gid} -eq ${AUTO_USER_ID} ]; then
> elif [ ${gid} -eq ${AUTO_SYSTEM_ID} ]; then
> fi
>
> That's one level less deep, and more explicit about the system-id branch.
>
> Both remarks also apply to the uid case of course.
>
> >
> > members=$(get_members "$group")
> > @@ -247,16 +262,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}"
>
> So here I'd remove the defaults and instead always pass the arguments.
>
>
>
> Regards,
> Arnout
>
> [1]
> https://patchwork.ozlabs.org/project/buildroot/list/?series=152843&state=%2A&archive=both
>
>
> > +
> > 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 +325,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} -lt 0 ]; then
> > + if [ ${uid} -eq ${AUTO_USER_ID} ]; 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
> > @@ -384,8 +407,8 @@ main() {
> > ENTRIES+=( "${line}" )
> > done < <( sed -r -e 's/#.*//; /^[[:space:]]*$/d;' "${USERS_TABLE}" )
> >
> > - # We first create groups whose gid is not -1, and then we create groups
> > - # whose gid is -1 (automatic), so that, if a group is defined both with
> > + # We first create groups whose gid is positive, and then we create groups
> > + # whose gid is automatic, so that, if a group is defined both with
> > # a specified gid and an automatic gid, we ensure the specified gid is
> > # used, rather than a different automatic gid is computed.
> >
> > @@ -399,7 +422,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} -lt 0 ] || continue # Non-automatic gid
> > add_one_group "${group}" "${gid}"
> > done
> >
> > @@ -410,7 +433,7 @@ main() {
> > read username uid group gid passwd home shell groups comment <<<"${line}"
> > if [ "${groups}" != "-" ]; then
> > for g in ${groups//,/ }; do
> > - add_one_group "${g}" -1
> > + add_one_group "${g}" ${AUTO_USER_ID}
> > done
> > fi
> > done
> > @@ -433,7 +456,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} -lt 0 ] || continue # Non-automatic uid
> > add_one_user "${username}" "${uid}" "${group}" "${gid}" "${passwd}" \
> > "${home}" "${shell}" "${groups}" "${comment}"
> > done
> >
Norbert
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Buildroot] [PATCH 1/2] support/scripts/mkusers: allow option for system uid/gid
2022-01-14 10:12 [Buildroot] [PATCH 1/2] support/scripts/mkusers: allow option for system uid/gid Norbert Lange
2022-01-14 10:12 ` [Buildroot] [PATCH 2/2] mkusers: change default from normal to system user Norbert Lange
2022-01-16 12:25 ` [Buildroot] [PATCH 1/2] support/scripts/mkusers: allow option for system uid/gid Arnout Vandecappelle
@ 2022-02-05 22:13 ` Arnout Vandecappelle
2 siblings, 0 replies; 7+ messages in thread
From: Arnout Vandecappelle @ 2022-02-05 22:13 UTC (permalink / raw)
To: Norbert Lange, buildroot
On 14/01/2022 11:12, Norbert Lange wrote:
> Some software decides based on uid/gid whether a user is a
> system or normal/human user, with differnt behaviour for those
> flavors (example journald [2]).
>
> So adding logic to create system-users is necessary, we take
> the now common ranges from [1].
>
> This extends the mkusers script to allow -2 for uid/gid,
> this argument will take an identifier from the system range.
>
> System/user ranges are added as variables, and the argument
> for user/system uid was added as variable aswell.
> Thus some magic constants could be removed, some further
> occurences of -1 were replaced with equivalent logic.
>
> [1] - https://systemd.io/UIDS-GIDS/
> [2] - https://www.freedesktop.org/software/systemd/man/journald.conf.html
>
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
I applied both to master, after squashing and making the modifications I
mentioned in my previous review. In addition...
> ---
> support/scripts/mkusers | 57 +++++++++++++++++++++++++++++------------
> 1 file changed, 40 insertions(+), 17 deletions(-)
>
> diff --git a/support/scripts/mkusers b/support/scripts/mkusers
> index d00ba33823..9d8295e8a3 100755
> --- a/support/scripts/mkusers
> +++ b/support/scripts/mkusers
> @@ -8,6 +8,15 @@ MIN_UID=1000
> MAX_UID=1999
For consistency wiht LAST_SYSTEM_UID, I renamed this one to LAST_USER_UID (and
same for the other 3 of course).
Regards,
Arnout
> 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
> +# argument to automatically crease system/user id
> +AUTO_SYSTEM_ID=-2
> +AUTO_USER_ID=-1
> +
> # No more is configurable below this point
> #----------------------------------------------------------------------------
>
> @@ -136,9 +145,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} -ge 0 ]; 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 +171,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} -ge 0 ]; 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 +207,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 +233,12 @@ add_one_group() {
> local members
>
> # Generate a new GID if needed
> - if [ ${gid} -eq -1 ]; then
> - gid="$( generate_gid "${group}" )"
> + if [ ${gid} -lt 0 ]; then
> + if [ ${gid} -eq ${AUTO_USER_ID} ]; then
> + gid="$( generate_gid "${group}" )"
> + else
> + gid="$( generate_gid "${group}" $FIRST_SYSTEM_GID $LAST_SYSTEM_GID )"
> + fi
> fi
>
> members=$(get_members "$group")
> @@ -247,16 +262,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 +325,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} -lt 0 ]; then
> + if [ ${uid} -eq ${AUTO_USER_ID} ]; 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
> @@ -384,8 +407,8 @@ main() {
> ENTRIES+=( "${line}" )
> done < <( sed -r -e 's/#.*//; /^[[:space:]]*$/d;' "${USERS_TABLE}" )
>
> - # We first create groups whose gid is not -1, and then we create groups
> - # whose gid is -1 (automatic), so that, if a group is defined both with
> + # We first create groups whose gid is positive, and then we create groups
> + # whose gid is automatic, so that, if a group is defined both with
> # a specified gid and an automatic gid, we ensure the specified gid is
> # used, rather than a different automatic gid is computed.
>
> @@ -399,7 +422,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} -lt 0 ] || continue # Non-automatic gid
> add_one_group "${group}" "${gid}"
> done
>
> @@ -410,7 +433,7 @@ main() {
> read username uid group gid passwd home shell groups comment <<<"${line}"
> if [ "${groups}" != "-" ]; then
> for g in ${groups//,/ }; do
> - add_one_group "${g}" -1
> + add_one_group "${g}" ${AUTO_USER_ID}
> done
> fi
> done
> @@ -433,7 +456,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} -lt 0 ] || continue # Non-automatic uid
> add_one_user "${username}" "${uid}" "${group}" "${gid}" "${passwd}" \
> "${home}" "${shell}" "${groups}" "${comment}"
> done
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Buildroot] [PATCH 2/2] mkusers: change default from normal to system user
2022-01-14 10:12 ` [Buildroot] [PATCH 2/2] mkusers: change default from normal to system user Norbert Lange
2022-01-16 12:27 ` Arnout Vandecappelle
@ 2022-02-05 22:14 ` Arnout Vandecappelle
1 sibling, 0 replies; 7+ messages in thread
From: Arnout Vandecappelle @ 2022-02-05 22:14 UTC (permalink / raw)
To: Norbert Lange, buildroot; +Cc: Thomas De Schampheleire
On 14/01/2022 11:12, Norbert Lange wrote:
> for all packages, this is the fitting default,
> but the fallout will be user provided tables.
>
> Also update the docs with the chagned behaviour.
>
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
> docs/manual/makeusers-syntax.txt | 11 +++++++----
> support/scripts/mkusers | 6 +++---
> 2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/docs/manual/makeusers-syntax.txt b/docs/manual/makeusers-syntax.txt
> index 467e596230..3d1013f447 100644
> --- a/docs/manual/makeusers-syntax.txt
> +++ b/docs/manual/makeusers-syntax.txt
> @@ -20,13 +20,16 @@ Where:
> It can not be +root+, and must be unique. If set to +-+, then just a
> group will be created.
> - +uid+ is the desired UID for the user. It must be unique, and not
> - +0+. If set to +-1+, then a unique UID will be computed by Buildroot
> - in the range [1000...1999]
> + +0+. If set to +-1+ or +-2+, then a unique UID will be computed by
> + Buildroot, with +-1+ denoting a system UID from [100...999] and +-2+
> + denoting a user UID from [1000...1999].
> - +group+ is the desired name for the user's main group. It can not
> be +root+. If the group does not exist, it will be created.
> - +gid+ is the desired GID for the user's main group. It must be unique,
> - and not +0+. If set to +-1+, and the group does not already exist, then
> - a unique GID will be computed by Buildroot in the range [1000..1999]
> + and not +0+. If set to +-1+ or +-2+, and the group does not already
> + exist, then a unique GID will be computed by Buildroot, with +-1+
> + denoting a system GID from [100...999] and +-2+ denoting a user GID
> + from [1000...1999].
> - +password+ is the crypt(3)-encoded password. If prefixed with +!+,
> then login is disabled. If prefixed with +=+, then it is interpreted
> as clear-text, and will be crypt-encoded (using MD5). If prefixed with
> diff --git a/support/scripts/mkusers b/support/scripts/mkusers
> index 9d8295e8a3..f7a3180e30 100755
> --- a/support/scripts/mkusers
> +++ b/support/scripts/mkusers
> @@ -14,8 +14,8 @@ LAST_SYSTEM_UID=999
> FIRST_SYSTEM_GID=100
> LAST_SYSTEM_GID=999
> # argument to automatically crease system/user id
> -AUTO_SYSTEM_ID=-2
> -AUTO_USER_ID=-1
> +AUTO_SYSTEM_ID=-1
> +AUTO_USER_ID=-2
>
> # No more is configurable below this point
> #----------------------------------------------------------------------------
> @@ -433,7 +433,7 @@ main() {
> read username uid group gid passwd home shell groups comment <<<"${line}"
> if [ "${groups}" != "-" ]; then
> for g in ${groups//,/ }; do
> - add_one_group "${g}" ${AUTO_USER_ID}
> + add_one_group "${g}" ${AUTO_SYSTEM_ID}
I changed this to decide on user or system ID based on the uid:
if [ ${uid} -le 0 ]; then
auto_id=${uid}
elif [ ${uid} -le ${LAST_SYSTEM_UID} ]; then
auto_id=${AUTO_SYSTEM_ID}
else
auto_id=${AUTO_USER_ID}
fi
for g in ${groups//,/ }; do
add_one_group "${g}" ${auto_id}
done
It seems to work well.
Regards,
Arnout
> done
> fi
> done
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-02-05 22:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-14 10:12 [Buildroot] [PATCH 1/2] support/scripts/mkusers: allow option for system uid/gid Norbert Lange
2022-01-14 10:12 ` [Buildroot] [PATCH 2/2] mkusers: change default from normal to system user Norbert Lange
2022-01-16 12:27 ` Arnout Vandecappelle
2022-02-05 22:14 ` Arnout Vandecappelle
2022-01-16 12:25 ` [Buildroot] [PATCH 1/2] support/scripts/mkusers: allow option for system uid/gid Arnout Vandecappelle
2022-01-17 9:34 ` Norbert Lange
2022-02-05 22:13 ` Arnout Vandecappelle
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.