All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.