Pull request for branch yem-package-create-user

Packages that install daemons may need those daemons to run as a non-root, or an otherwise non-system (eg. 'daemon'), user.

Add infrastructure for packages to create users, by declaring the FOO_USERS variable that conatin a makedev-syntax-like description of the user(s) to add.

Signed-off-by "Yann E. MORIN" <[email protected]>

---

docs/manual/makedev-syntax.txt +4 -0

+
+The syntax for adding a user is a space-separated list of fields, one
+user per-line; the fields are:
+

docs/manual/makedev-syntax.txt +19 -0

+- +home+ is the desired home directory for the user. If set to '-', no
+  home directory will be created, and the user's home will be +/+.
+  Explicitly setting +home+ to +/+ is not allowed.
+- +shell+ is the desired shell for the user. If set to +-+, then
+  +/bin/false+ is set as the user's shell.
+- +groups+ is the comma-separated list of additional groups the user
+  should be part of. If set to +-+, then the user will be a member of
+  no additional group.
+- +comment+ is an almost-free-form text.
+
+There are a few restrictions on the content of each field:
+
+* except for +comment+, all fields are mandatory.
+* except for +comment+, fields may not contain spaces.
+* no field may contain a column (+:+).
+
+If +home+ is not +-+, then the home directory, and all files below,
+will belong to the user and its main group.
+

docs/manual/makedev-syntax.txt +5 -0

+- clear-text +password+ is: +blabla+, will be crypt(3)-encrypted, but login is disabled.
+- +home+ is: +/home/foo+
+- +shell+ is: +/bin/sh+
+- +foo+ is also a member of +groups+: +alpha+ and +bravo+
+- +comment+ is: +Foo user+

support/scripts/mkusers new file +12 -0

@@ -0,0 +1,348 @@
+#!/bin/bash
+set -e
+myname="${0##*/}"
+
+#----------------------------------------------------------------------------
+# Configurable items
+MIN_UID=1000
+MAX_UID=1999
+MIN_GID=1000
+MAX_GID=1999
+# No more is configurable below this point
+#----------------------------------------------------------------------------

2013/1/1 Yann E. MORIN <[email protected]>:

Quoted 6 lines

Packages that install daemons may need those daemons to run as a non-root, or an otherwise non-system (eg. 'daemon'), user.

Add infrastructure for packages to create users, by declaring the FOO_USERS variable that conatin a makedev-syntax-like description of the user(s) to add.

docs/manual/makedev-syntax.txt

Quoted 4 lines

docs/manual/makedev-syntax.txt +0 -0

index 27517b3..fffdac9 100644

You could add this in a new file...
Why should makedev-syntax.txt contain syntax details for makedev and mkusers?

docs/manual/makedev-syntax.txt and then for device files corresponding to the partitions of · line 85

Quoted 4 lines

+- +password+ is the crypt(3)-encrypted password. If prefixed with +=+,

docs/manual/makedev-syntax.txt +3 -0

+  then it is interpreted as clear-text, and will be cypt-encoded. If
+  prefixed with +!+, then login is disabled. If set to +*+, then login
+  is not allowed.

Multiple prefix is allowed/supported; it could be explicitly mentioned. Is the prefix order important?

So, the clear-text password itself should not starts with any prefix character?

Quoted 1 line · On Tuesday 01 January 2013 Samuel Martin wrote:

2013/1/1 Yann E. MORIN <[email protected]>:

Quoted 8 lines

docs/manual/makedev-syntax.txt and then for device files corresponding to the partitions of · line 85

Quoted 4 lines

+- +password+ is the crypt(3)-encrypted password. If prefixed with +=+,

docs/manual/makedev-syntax.txt +3 -0

+  then it is interpreted as clear-text, and will be cypt-encoded. If
+  prefixed with +!+, then login is disabled. If set to +*+, then login
+  is not allowed.

Multiple prefix is allowed/supported; it could be explicitly mentioned. Is the prefix order important?

So, the clear-text password itself should not starts with any prefix character?

So, valid combinations are:

    *           no password, login not allowed
    XXXX        crypt-encoded password
    !XXXX       crypt-encoded password, login disabled
    =1234       clear-text password
    !=1234      clear-text password, login disabled

+set -e

support/scripts/mkusers new file +11 -0

+}
+
+#----------------------------------------------------------------------------
+get_ugid() {
+    local username="${1}"
+
+    grep -r -E "${username}:" "${PASSWD}" |cut -d: -f4
+}
+
+#----------------------------------------------------------------------------
+get_gid() {

support/scripts/mkusers new file +33 -0

+}
+
+#----------------------------------------------------------------------------
+get_group() {
+    local gid="${1}"
+
+    sed -r -e '/^([^:]+):[^:]+:'"${gid}"':/!d; s//\1/;' "${GROUP}"
+}
+
+#----------------------------------------------------------------------------
+get_ugroup() {
+    local username="${1}"
+    local ugid
+
+    ugid="$( get_ugid "${username}" )"
+    if [ -n "${ugid}" ]; then
+        get_group "${ugid}"
+    fi
+}
+
+#----------------------------------------------------------------------------
+# Sanity-check the new user/group:
+#   - check the gid is not already used for another group
+#   - check the group does not already exist with another gid
+#   - check the user does not already exist with another gid
+#   - check the uid is not already used for another user
+#   - check the user does not already exist with another uid
+#   - check the user does not already exist in another group
+check_user_validity() {
+    local username="${1}"
+    local uid="${2}"
+    local group="${3}"
+    local gid="${4}"

docs/manual/adding-packages-generic.txt information is (assuming the package name is +libfoo+) : · line 263

Quoted 2 lines · Yann E. MORIN

docs/manual/adding-packages-generic.txt +2 -0

+* +LIBFOO_USERS+ lists the users to create for this package, if it installs
+  a daemon you want to run with a specific user. The syntax is similar in

"if it installs a program you want to run as a specific user"

that is, s/daemon/program/ and s/with/as/

docs/manual/makedev-syntax.txt and then for device files corresponding to the partitions of · line 81

Quoted 4 lines

+- +group+ is the desired name for the user's main group.
+- +gid+ is the desired GID for the user's main group. It must be unique,

docs/manual/makedev-syntax.txt +2 -0

+  and not +0+. If set to +-1+, then a unique GID will be computed by
+  buildroot.

I think this is saying it creates groups as well as users. If so, the documentation should say so.

Also, how do you specifiy that you do not want a group created - such as when you want a new user in an existing system group. I would assume something like

foo -1 daemon -1 ...

to create user "foo" in the system "daemon" group, but the documentation implies a new GID will be allocated.

docs/manual/makedev-syntax.txt and then for device files corresponding to the partitions of · line 85

Quoted 4 lines

+- +password+ is the crypt(3)-encrypted password. If prefixed with +=+,

docs/manual/makedev-syntax.txt +3 -0

+  then it is interpreted as clear-text, and will be cypt-encoded. If
+  prefixed with +!+, then login is disabled. If set to +*+, then login
+  is not allowed.

What is the status of the support of other encryption algorithms? e.g. $6$salt$enc for SHA-512 encrypted passwords? This will depend on settings in BuildRoot itself, but it would be worth making a reference here since it is relevant to a user providing encrypted user passwords.

Quoted 1 line

+set -e

....

support/scripts/mkusers line 17

Quoted 3 lines

support/scripts/mkusers new file +3 -0

+USERS_TABLE="${1}"
+TARGET_DIR="${2}"
+shift 2

This will fail under 'set -e' if there are less than two parameters, but fail without any useful diagnostics. An error/usage message here would be useful.

Quoted 5 lines

support/scripts/mkusers new file +5 -0

+#----------------------------------------------------------------------------
+error() {
+    local fmt="${1}"
+    shift
+

support/scripts/mkusers line 32

Quoted 2 lines

support/scripts/mkusers new file +2 -0

+    printf "%s: " "${myname}" >&2
+    printf "${fmt}" "${@}" >&2

I think you should put a \n in the format string here. Every call to this function (via fail) includes a \n, so it seems redundant require each caller to put a \n on the string. In fact, the last call to fail does not have a \n in the string which it should, so this will also fix what would probably become a common error.

Quoted 5 lines

support/scripts/mkusers new file +5 -0

+}
+fail() {
+    error "$@"
+    exit 1
+}

support/scripts/mkusers line 43

Quoted 6 lines

support/scripts/mkusers new file +6 -0

+
+#----------------------------------------------------------------------------
+get_uid() {
+    local username="${1}"
+
+    grep -r -E "${username}:" "${PASSWD}" |cut -d: -f3

Why grep -r?

Why grep -E? You are not using any features of extended regular expressions.

An argument could be made that you should be using grep -F.

You should also anchor ${username}

support/scripts/mkusers line 64

Quoted 4 lines

support/scripts/mkusers new file +4 -0

+get_username() {
+    local uid="${1}"
+
+    sed -r -e '/^([^:]+):[^:]+:'"${uid}"':.*/!d; s//\1/;' "${PASSWD}"

Is awk available in the host toolchain?

Quoted 11 lines

support/scripts/mkusers new file +11 -0

+check_user_validity() {
+    local username="${1}"
+    local uid="${2}"
+    local group="${3}"
+    local gid="${4}"
+    local _uid _ugid _gid _username _group _ugroup
+
+    _group="$( get_group "${gid}" )"
+    _gid="$( get_gid "${group}" )"
+    _ugid="$( get_ugid "${username}" )"
+    _username="$( get_username "${uid}" )"

support/scripts/mkusers line 107

Quoted 4 lines

support/scripts/mkusers new file +4 -0

+    _uid="$( get_uid "${username}" )"
+    _ugroup="$( get_ugroup "${username}" )"
+
+    if [ ${gid} -ge 0 ]; then

Elsewhere in this script you check uid/gid against -1, not >= 0. This should be changed to "${gid} -eq -1" to be consistent with that and the documentation.

support/scripts/mkusers line 272

Quoted 5 lines

support/scripts/mkusers new file +5 -0

+    case "${home}" in
+        -)  _home="/";;
+        /)  fail "home can not be explicitly '/'\n";;
+        /*) _home="${home}";;
+        *)  fail "home must be an absolute path";;

This is where you missed the \n in the error message.

There is also another spelling mistake somewhere :-) I noticed three when I first skimmed this patch; two have already been pointed out; and I did not see the third when I went through to comment.

Cam, All,

Quoted 8 lines · On Wednesday 02 January 2013 Cam Hutchison wrote:

docs/manual/adding-packages-generic.txt information is (assuming the package name is +libfoo+) : · line 263

Quoted 2 lines · Yann E. MORIN

docs/manual/adding-packages-generic.txt +2 -0

+* +LIBFOO_USERS+ lists the users to create for this package, if it installs
+  a daemon you want to run with a specific user. The syntax is similar in

"if it installs a program you want to run as a specific user"

that is, s/daemon/program/ and s/with/as/

Well, I would like to emphasise that this is primarily for running programs as daemons (ie. started by startup scripts). It does not really make sense to run program as a specific user when logged in, especially for embedded systems, where logging in a seldom done.

What about:

.... if it installs a daemon program you want to run as ...

Quoted 8 lines

support/scripts/mkusers line 43

Quoted 5 lines

support/scripts/mkusers new file +5 -0

+#----------------------------------------------------------------------------
+get_uid() {
+    local username="${1}"
+
+    grep -r -E "${username}:" "${PASSWD}" |cut -d: -f3

An argument could be made that you should be using grep -F.

I don't know (ie. I don't usualy use) this switch, so I am not confident in using it here. If plain 'grep' does the job, lets just use that.

Quoted 1 line

You should also anchor ${username}

Yes, indeed.

Another +1 for awk here - processing record-oriented files is what awk is good at, but I can understand not wanting to introduce that dependency.

Quoted 3 lines · Cam Hutchison

Another +1 for awk here - processing record-oriented files is what awk is good at, but I can understand not wanting to introduce that dependency.

We already depend on awk:
http://git.buildroot.net/buildroot/tree/support/dependencies/dependencies.sh#n133

+* no field may contain a column (+:+).

docs/manual/makedev-syntax.txt and then for device files corresponding to the partitions of · line 66

Quoted 2 lines · Yann E. MORIN

docs/manual/makedev-syntax.txt +2 -0

+The syntax for adding a user is a space-separated list of fields, one
+user per-line; the fields are:

per line - no hyphen here.

Quoted 1 line

+* no field may contain a column (+:+).

Found it - this is the third spelling mistake I referred to earlier. s/column/colon/