Openembedded Core Discussions
 help / color / mirror / Atom feed
* [PATCH 0/3] rootfs-postcommands: replace the sysusers.d postcommand
@ 2023-06-15 11:43 Louis Rannou
  2023-06-15 11:43 ` [PATCH 1/3] rootfs-postcommands: change sysusers.d command Louis Rannou
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Louis Rannou @ 2023-06-15 11:43 UTC (permalink / raw)
  To: openembedded-core; +Cc: Louis Rannou, anuj.mittal

This is a suggestion to replace the management of sysusers.d in the build.

sysusers.d is a set of configuration files to declare system users and groups
supposed to be created at boot when they do not exist.

Until now, we have a rootfs post command that checks those configuration and
creates missing users and groups. This command is defective when a home
directory or a shell is specified. The actual parsing leads to incorrect
commands such as:

`useradd --shell /sbin/nologin --uid 0 --comment "Super User" /root --system
root`

Also, it appears there is more interest for a command that checks all required
users are correctly created before the rootfs is done.

Therefore, the first patch here replaces the command `systemd_create_users` by a
command `systemd_sysusers_check` that every users/groups declared in sysusers.d
configuration files already exist in `/etc/passwd` and `/etc/group` and check at
best if the properties match.

This reveals two misconfiguration:

WARNING: memfault-image-1.0-r0 do_rootfs: User root has been defined as (root, 0, 0, root, /home/root, /bin/sh) but sysusers.d expects it as (root, 0, -, Super User, /root, -)
WARNING: memfault-image-1.0-r0 do_rootfs: Group wheel has never been defined

1. As systemd supposes the rootfs should not be configurable, whereas it is possibl
ein yocto through the variable ROOT_HOME,, a second patch suggests to replace
the sysusers.d configuration file 'basic.conf' by ours.

2. The user wheel can be used for some superuser tasks such as consulting the
systemd journal or manage printers in cups. It can also be used for su and sudo
in replacement of the sudo group. It looks good to add this in the base-passwd
files. It is not upstreamable as the debian point of view is that the wheel
group is unset by default.

Signed-off-by: Louis Rannou <lrannou@baylibre.com>
---
Louis Rannou (3):
      rootfs-postcommands: change sysusers.d command
      systemd: replace the sysusers.d basic configuration
      base-passwd: add the wheel group

 meta/classes-recipe/rootfs-postcommands.bbclass    | 133 +++++++++++++++++----
 .../base-passwd/0007-Add-wheel-group.patch         |  20 ++++
 meta/recipes-core/base-passwd/base-passwd_3.6.1.bb |   1 +
 meta/recipes-core/systemd/systemd/basic.conf.in    |  40 +++++++
 meta/recipes-core/systemd/systemd_253.3.bb         |   5 +
 5 files changed, 175 insertions(+), 24 deletions(-)
---
base-commit: 8078a62739f08e60de98e194b9cd987d8c5b2e7b
change-id: 20230613-sysusersd-614778830079

Best regards,
-- 
Louis Rannou <lrannou@baylibre.com>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] rootfs-postcommands: change sysusers.d command
  2023-06-15 11:43 [PATCH 0/3] rootfs-postcommands: replace the sysusers.d postcommand Louis Rannou
@ 2023-06-15 11:43 ` Louis Rannou
  2023-06-15 11:43 ` [PATCH 2/3] systemd: replace the sysusers.d basic configuration Louis Rannou
  2023-06-15 11:43 ` [PATCH 3/3] base-passwd: add the wheel group Louis Rannou
  2 siblings, 0 replies; 10+ messages in thread
From: Louis Rannou @ 2023-06-15 11:43 UTC (permalink / raw)
  To: openembedded-core; +Cc: Louis Rannou, anuj.mittal

The configuration in sysusers.d used to be parsed to create users/groups at
build time instead at runtime. This was leading to several conflicts with
users/groups defined in base-passwd recipe and specific definitions in recipes
inheriting the useradd class. Some of those conflicts raised unseen errors in
the do_rootfs command's logs.

As an example, the root home directory is set by default to `/home/root` but
systemd expects it as `/root`.

The new command `systemd_sysusers_check` checks each configuration for
users/groups and compare their properties to what is actually defined in the
`/etc/passwd` and `/etc/group` of the target rootfs.

Signed-off-by: Louis Rannou <lrannou@baylibre.com>
---
 meta/classes-recipe/rootfs-postcommands.bbclass | 133 +++++++++++++++++++-----
 1 file changed, 109 insertions(+), 24 deletions(-)

diff --git a/meta/classes-recipe/rootfs-postcommands.bbclass b/meta/classes-recipe/rootfs-postcommands.bbclass
index 690fa976aa..652601b95f 100644
--- a/meta/classes-recipe/rootfs-postcommands.bbclass
+++ b/meta/classes-recipe/rootfs-postcommands.bbclass
@@ -43,7 +43,7 @@ ROOTFS_POSTUNINSTALL_COMMAND =+ "write_image_manifest ; "
 POSTINST_LOGFILE ?= "${localstatedir}/log/postinstall.log"
 # Set default target for systemd images
 SYSTEMD_DEFAULT_TARGET ?= '${@bb.utils.contains_any("IMAGE_FEATURES", [ "x11-base", "weston" ], "graphical.target", "multi-user.target", d)}'
-ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("DISTRO_FEATURES", "systemd", "set_systemd_default_target; systemd_create_users;", "", d)}'
+ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("DISTRO_FEATURES", "systemd", "set_systemd_default_target; systemd_sysusers_check;", "", d)}'
 
 ROOTFS_POSTPROCESS_COMMAND += 'empty_var_volatile;'
 
@@ -69,29 +69,114 @@ python () {
     d.appendVar('ROOTFS_POSTPROCESS_COMMAND', 'rootfs_reproducible;')
 }
 
-systemd_create_users () {
-	for conffile in ${IMAGE_ROOTFS}/usr/lib/sysusers.d/*.conf; do
-		[ -e $conffile ] || continue
-		grep -v "^#" $conffile | sed -e '/^$/d' | while read type name id comment; do
-		if [ "$type" = "u" ]; then
-			useradd_params="--shell /sbin/nologin"
-			[ "$id" != "-" ] && useradd_params="$useradd_params --uid $id"
-			[ "$comment" != "-" ] && useradd_params="$useradd_params --comment $comment"
-			useradd_params="$useradd_params --system $name"
-			eval useradd --root ${IMAGE_ROOTFS} $useradd_params || true
-		elif [ "$type" = "g" ]; then
-			groupadd_params=""
-			[ "$id" != "-" ] && groupadd_params="$groupadd_params --gid $id"
-			groupadd_params="$groupadd_params --system $name"
-			eval groupadd --root ${IMAGE_ROOTFS} $groupadd_params || true
-		elif [ "$type" = "m" ]; then
-			group=$id
-			eval groupadd --root ${IMAGE_ROOTFS} --system $group || true
-			eval useradd --root ${IMAGE_ROOTFS} --shell /sbin/nologin --system $name --no-user-group || true
-			eval usermod --root ${IMAGE_ROOTFS} -a -G $group $name
-		fi
-		done
-	done
+# Resolve the ID as described in the sysusers.d(5) manual: ID can be a numeric
+# uid, a couple uid:gid or uid:groupname or it is '-' meaning leaving it
+# automatic or it can be a path. In the latter, the uid/gid matches the
+# user/group owner of that file.
+def resolve_sysusers_id(d, sid):
+    # If the id is a path, the uid/gid matchs to the target's uid/gid in the
+    # rootfs.
+    if '/' in sid:
+        try:
+            osstat = os.stat(os.path.join(d.getVar('IMAGE_ROOTFS'), sid))
+        except FileNotFoundError:
+            bb.error('sysusers.d: file %s is required but it does not exist in the rootfs', sid)
+            return ('-', '-')
+        return (osstat.st_uid, osstat.st_gid)
+    # Else it is a uid:gid or uid:groupname syntax
+    if ':' in sid:
+        return sid.split(':')
+    else:
+        return (sid, '-')
+
+# Check a user exists in the rootfs password file and return its properties
+def check_user_exists(d, uname=None, uid=None):
+    with open(os.path.join(d.getVar('IMAGE_ROOTFS'), 'etc/passwd'), 'r') as pwfile:
+        for line in pwfile:
+            (name, _, u_id, gid, comment, homedir, ushell) = line.strip().split(':')
+            if uname == name or uid == u_id:
+                return (name, u_id, gid, comment or '-', homedir or '/', ushell or '-')
+    return None
+
+# Check a group exists in the rootfs group file and return its properties
+def check_group_exists(d, gname=None, gid=None):
+    with open(os.path.join(d.getVar('IMAGE_ROOTFS'), 'etc/group'), 'r') as gfile:
+        for line in gfile:
+            (name, _, g_id, _) = line.strip().split(':')
+            if name == gname or g_id == gid:
+                return (name, g_id)
+    return None
+
+def compare_users(user, e_user):
+    # user and e_user must not have None values. Unset values must be '-'.
+    (name, uid, gid, comment, homedir, ushell) = user
+    (e_name, e_uid, e_gid, e_comment, e_homedir, e_ushell) = e_user
+    # Ignore 'uid', 'gid' or 'comment' if they are not set
+    # Ignore 'shell' and 'ushell' if one is not set
+    return name == e_name \
+        and (uid == '-' or uid == e_uid) \
+        and (gid == '-' or gid == e_gid) \
+        and (comment == '-' or e_comment == '-' or comment.lower() == e_comment.lower()) \
+        and (homedir == '-' or e_homedir == '-' or homedir == e_homedir) \
+        and (ushell == '-' or e_ushell == '-' or ushell == e_ushell)
+
+# Open sysusers.d configuration files and parse each line to check the users and
+# groups are already defined in /etc/passwd and /etc/groups with similar
+# properties. Refer to the sysusers.d(5) manual for its syntax.
+python systemd_sysusers_check() {
+    import glob
+    import re
+
+    pattern_comment = r'(-|\"[^:\"]+\")'
+    pattern_word    = r'[^\s]+'
+    pattern_line   = r'(' + pattern_word + r')\s+(' + pattern_word + r')\s+(' + pattern_word + r')(\s+' \
+        + pattern_comment + r')?' + r'(\s+(' + pattern_word + r'))?' + r'(\s+(' + pattern_word + r'))?'
+
+    for conffile in glob.glob(os.path.join(d.getVar('IMAGE_ROOTFS'), 'usr/lib/sysusers.d/*.conf')):
+        with open(conffile, 'r') as f:
+            for line in f:
+                line = line.strip()
+                if not len(line) or line[0] == '#': continue
+                ret = re.fullmatch(pattern_line, line.strip())
+                if not ret: continue
+                (stype, sname, sid, _, scomment, _, shomedir, _, sshell) = ret.groups()
+                if stype == 'u':
+                    if sid:
+                        (suid, sgid) = resolve_sysusers_id(d, sid)
+                        if sgid.isalpha():
+                            sgid = check_group_exists(d, gname=sgid)
+                        elif sgid.isdigit():
+                            check_group_exists(d, gid=sgid)
+                        else:
+                            sgid = '-'
+                    else:
+                        suid = '-'
+                        sgid = '-'
+                    scomment = scomment.replace('"', '') if scomment else '-'
+                    shomedir = shomedir or '-'
+                    sshell = sshell or '-'
+                    e_user = check_user_exists(d, uname=sname)
+                    if not e_user:
+                        bb.warn('User %s has never been defined' % sname)
+                    elif not compare_users((sname, suid, sgid, scomment, shomedir, sshell), e_user):
+                        bb.warn('User %s has been defined as (%s) but sysusers.d expects it as (%s)'
+                                % (sname, ', '.join(e_user),
+                                ', '.join((sname, suid, sgid, scomment, shomedir, sshell))))
+                elif stype == 'g':
+                    gid = sid or '-'
+                    if '/' in gid:
+                        (_, gid) = resolve_sysusers_id(d, sid)
+                    e_group = check_group_exists(d, gname=sname)
+                    if not e_group:
+                        bb.warn('Group %s has never been defined' % sname)
+                    elif gid != '-':
+                        (_, e_gid) = e_group
+                        if gid != e_gid:
+                            bb.warn('Group %s has been defined with id (%s) but sysusers.d expects gid (%s)'
+                                    % (sname, e_gid, gid))
+                elif stype == 'm':
+                    check_user_exists(d, sname)
+                    check_group_exists(d, sid)
 }
 
 #

-- 
2.41.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] systemd: replace the sysusers.d basic configuration
  2023-06-15 11:43 [PATCH 0/3] rootfs-postcommands: replace the sysusers.d postcommand Louis Rannou
  2023-06-15 11:43 ` [PATCH 1/3] rootfs-postcommands: change sysusers.d command Louis Rannou
@ 2023-06-15 11:43 ` Louis Rannou
  2023-06-16 16:51   ` [OE-core] " Peter Kjellerstedt
  2023-06-15 11:43 ` [PATCH 3/3] base-passwd: add the wheel group Louis Rannou
  2 siblings, 1 reply; 10+ messages in thread
From: Louis Rannou @ 2023-06-15 11:43 UTC (permalink / raw)
  To: openembedded-core; +Cc: Louis Rannou, anuj.mittal

The default sysusers basic.conf.in file sets the root home directory to `/root`
and does not permit its configuration. Replace the file delivered by systemd so
the root home directory matches the `ROOT_HOME` variable.

Signed-off-by: Louis Rannou <lrannou@baylibre.com>
---
 meta/recipes-core/systemd/systemd/basic.conf.in | 40 +++++++++++++++++++++++++
 meta/recipes-core/systemd/systemd_253.3.bb      |  5 ++++
 2 files changed, 45 insertions(+)

diff --git a/meta/recipes-core/systemd/systemd/basic.conf.in b/meta/recipes-core/systemd/systemd/basic.conf.in
new file mode 100644
index 0000000000..fac288f7fa
--- /dev/null
+++ b/meta/recipes-core/systemd/systemd/basic.conf.in
@@ -0,0 +1,40 @@
+#  This file is part of systemd.
+#
+#  systemd is free software; you can redistribute it and/or modify it
+#  under the terms of the GNU Lesser General Public License as published by
+#  the Free Software Foundation; either version 2.1 of the License, or
+#  (at your option) any later version.
+
+# The superuser
+u root    0     "root" :ROOT_HOME:
+
+# The nobody user/group for NFS file systems
+g {{NOBODY_GROUP_NAME}} 65534       -            -
+u {{NOBODY_USER_NAME }} 65534:65534 "Nobody"     -
+
+# Administrator group: can *see* more than normal users
+g adm     {{ADM_GID    }}     -            -
+
+# Administrator group: can *do* more than normal users
+g wheel   {{WHEEL_GID  }}     -            -
+
+# Access to shared database of users on the system
+g utmp    {{UTMP_GID   }}     -            -
+
+# Physical and virtual hardware access groups
+g audio   {{AUDIO_GID  }}     -            -
+g cdrom   {{CDROM_GID  }}     -            -
+g dialout {{DIALOUT_GID}}     -            -
+g disk    {{DISK_GID   }}     -            -
+g input   {{INPUT_GID  }}     -            -
+g kmem    {{KMEM_GID   }}     -            -
+g kvm     {{KVM_GID    }}     -            -
+g lp      {{LP_GID     }}     -            -
+g render  {{RENDER_GID }}     -            -
+g sgx     {{SGX_GID    }}     -            -
+g tape    {{TAPE_GID   }}     -            -
+g tty     {{TTY_GID    }}     -            -
+g video   {{VIDEO_GID  }}     -            -
+
+# Default group for normal users
+g users   {{USERS_GID  }}     -            -
diff --git a/meta/recipes-core/systemd/systemd_253.3.bb b/meta/recipes-core/systemd/systemd_253.3.bb
index 45dc6ab5bb..87fbf6f785 100644
--- a/meta/recipes-core/systemd/systemd_253.3.bb
+++ b/meta/recipes-core/systemd/systemd_253.3.bb
@@ -17,6 +17,7 @@ REQUIRED_DISTRO_FEATURES = "systemd"
 SRC_URI += " \
            file://touchscreen.rules \
            file://00-create-volatile.conf \
+           file://basic.conf.in \
            ${@bb.utils.contains('PACKAGECONFIG', 'polkit_hostnamed_fallback', 'file://org.freedesktop.hostname1_no_polkit.conf', '', d)} \
            ${@bb.utils.contains('PACKAGECONFIG', 'polkit_hostnamed_fallback', 'file://00-hostnamed-network-user.conf', '', d)} \
            file://init \
@@ -252,6 +253,10 @@ EXTRA_OEMESON += "-Dkexec-path=${sbindir}/kexec \
 # The 60 seconds is watchdog's default vaule.
 WATCHDOG_TIMEOUT ??= "60"
 
+do_configure:prepend() {
+  sed s@:ROOT_HOME:@${ROOT_HOME}@g ${WORKDIR}/basic.conf.in > ${S}/sysusers.d/basic.conf.in
+}
+
 do_install() {
 	meson_do_install
 	install -d ${D}/${base_sbindir}

-- 
2.41.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] base-passwd: add the wheel group
  2023-06-15 11:43 [PATCH 0/3] rootfs-postcommands: replace the sysusers.d postcommand Louis Rannou
  2023-06-15 11:43 ` [PATCH 1/3] rootfs-postcommands: change sysusers.d command Louis Rannou
  2023-06-15 11:43 ` [PATCH 2/3] systemd: replace the sysusers.d basic configuration Louis Rannou
@ 2023-06-15 11:43 ` Louis Rannou
  2023-06-18  9:37   ` [OE-core] " Alexandre Belloni
  2 siblings, 1 reply; 10+ messages in thread
From: Louis Rannou @ 2023-06-15 11:43 UTC (permalink / raw)
  To: openembedded-core; +Cc: Louis Rannou, anuj.mittal

The wheel group is not declared while it can be used to access the systemd
journal and to configure printers in CUPS. It can also be used for su and sudo
permissions.

So far it was created later in the rootfs postcommand systemd_create_users.

Signed-off-by: Louis Rannou <lrannou@baylibre.com>
---
 .../base-passwd/0007-Add-wheel-group.patch           | 20 ++++++++++++++++++++
 meta/recipes-core/base-passwd/base-passwd_3.6.1.bb   |  1 +
 2 files changed, 21 insertions(+)

diff --git a/meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch b/meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch
new file mode 100644
index 0000000000..00eaec38a2
--- /dev/null
+++ b/meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch
@@ -0,0 +1,20 @@
+
+We need to have a wheel group which has some system privileges to consult the
+systemd journal or manage printers with cups.
+
+Upstream status says the group does not exist by default.
+
+Upstream-Status: Inappropriate [enable feature]
+
+Signed-off-by: Louis Rannou <lrannou@baylibre.com>
+Index: base-passwd-3.5.26/group.master
+===================================================================
+--- base-passwd-3.5.29.orig/group.master
++++ base-passwd-3.5.29/group.master
+@@ -38,5 +38,6 @@
+ staff:*:50:
+ games:*:60:
+ shutdown:*:70:
++wheel:*:80:
+ users:*:100:
+ nogroup:*:65534:
diff --git a/meta/recipes-core/base-passwd/base-passwd_3.6.1.bb b/meta/recipes-core/base-passwd/base-passwd_3.6.1.bb
index 853717176d..204016b3e7 100644
--- a/meta/recipes-core/base-passwd/base-passwd_3.6.1.bb
+++ b/meta/recipes-core/base-passwd/base-passwd_3.6.1.bb
@@ -12,6 +12,7 @@ SRC_URI = "https://launchpad.net/debian/+archive/primary/+files/${BPN}_${PV}.tar
            file://0004-Add-an-input-group-for-the-dev-input-devices.patch \
            file://0005-Add-kvm-group.patch \
            file://0006-Make-it-possible-to-configure-whether-to-use-SELinux.patch \
+           file://0007-Add-wheel-group.patch \
            "
 
 SRC_URI[sha256sum] = "6ff369be59d586ba63c0c5fcb00f75f9953fe49db88bc6c6428f2c92866f79af"

-- 
2.41.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* RE: [OE-core] [PATCH 2/3] systemd: replace the sysusers.d basic configuration
  2023-06-15 11:43 ` [PATCH 2/3] systemd: replace the sysusers.d basic configuration Louis Rannou
@ 2023-06-16 16:51   ` Peter Kjellerstedt
  2023-06-19 12:27     ` Louis Rannou
       [not found]     ` <5a438786-9f24-e30d-ed84-08bf48b7bfd4@baylibre.com>
  0 siblings, 2 replies; 10+ messages in thread
From: Peter Kjellerstedt @ 2023-06-16 16:51 UTC (permalink / raw)
  To: Louis Rannou, openembedded-core@lists.openembedded.org
  Cc: anuj.mittal@intel.com

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Louis Rannou
> Sent: den 15 juni 2023 13:44
> To: openembedded-core@lists.openembedded.org
> Cc: Louis Rannou <lrannou@baylibre.com>; anuj.mittal@intel.com
> Subject: [OE-core] [PATCH 2/3] systemd: replace the sysusers.d basic configuration
> 
> The default sysusers basic.conf.in file sets the root home directory to `/root`
> and does not permit its configuration. Replace the file delivered by systemd so
> the root home directory matches the `ROOT_HOME` variable.
> 
> Signed-off-by: Louis Rannou <lrannou@baylibre.com>
> ---
>  meta/recipes-core/systemd/systemd/basic.conf.in | 40 +++++++++++++++++++++++++
>  meta/recipes-core/systemd/systemd_253.3.bb      |  5 ++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/meta/recipes-core/systemd/systemd/basic.conf.in b/meta/recipes-core/systemd/systemd/basic.conf.in
> new file mode 100644
> index 0000000000..fac288f7fa
> --- /dev/null
> +++ b/meta/recipes-core/systemd/systemd/basic.conf.in
> @@ -0,0 +1,40 @@
> +#  This file is part of systemd.
> +#
> +#  systemd is free software; you can redistribute it and/or modify it
> +#  under the terms of the GNU Lesser General Public License as published by
> +#  the Free Software Foundation; either version 2.1 of the License, or
> +#  (at your option) any later version.
> +
> +# The superuser
> +u root    0     "root" :ROOT_HOME:
> +
> +# The nobody user/group for NFS file systems
> +g {{NOBODY_GROUP_NAME}} 65534       -            -
> +u {{NOBODY_USER_NAME }} 65534:65534 "Nobody"     -
> +
> +# Administrator group: can *see* more than normal users
> +g adm     {{ADM_GID    }}     -            -
> +
> +# Administrator group: can *do* more than normal users
> +g wheel   {{WHEEL_GID  }}     -            -
> +
> +# Access to shared database of users on the system
> +g utmp    {{UTMP_GID   }}     -            -
> +
> +# Physical and virtual hardware access groups
> +g audio   {{AUDIO_GID  }}     -            -
> +g cdrom   {{CDROM_GID  }}     -            -
> +g dialout {{DIALOUT_GID}}     -            -
> +g disk    {{DISK_GID   }}     -            -
> +g input   {{INPUT_GID  }}     -            -
> +g kmem    {{KMEM_GID   }}     -            -
> +g kvm     {{KVM_GID    }}     -            -
> +g lp      {{LP_GID     }}     -            -
> +g render  {{RENDER_GID }}     -            -
> +g sgx     {{SGX_GID    }}     -            -
> +g tape    {{TAPE_GID   }}     -            -
> +g tty     {{TTY_GID    }}     -            -
> +g video   {{VIDEO_GID  }}     -            -
> +
> +# Default group for normal users
> +g users   {{USERS_GID  }}     -            -
> diff --git a/meta/recipes-core/systemd/systemd_253.3.bb b/meta/recipes-core/systemd/systemd_253.3.bb
> index 45dc6ab5bb..87fbf6f785 100644
> --- a/meta/recipes-core/systemd/systemd_253.3.bb
> +++ b/meta/recipes-core/systemd/systemd_253.3.bb
> @@ -17,6 +17,7 @@ REQUIRED_DISTRO_FEATURES = "systemd"
>  SRC_URI += " \
>             file://touchscreen.rules \
>             file://00-create-volatile.conf \
> +           file://basic.conf.in \

Instead of including a modified copy of the basic.conf.in file 
from systemd, include a patch that modifies the file that systemd 
provides. Otherwise this becomes a maintenance problem where it 
is easy to miss changes that upstream does to the file, and also 
hard to know what you have changed.

>             ${@bb.utils.contains('PACKAGECONFIG', 'polkit_hostnamed_fallback', 'file://org.freedesktop.hostname1_no_polkit.conf', '', d)} \
>             ${@bb.utils.contains('PACKAGECONFIG', 'polkit_hostnamed_fallback', 'file://00-hostnamed-network-user.conf', '', d)} \
>             file://init \
> @@ -252,6 +253,10 @@ EXTRA_OEMESON += "-Dkexec-path=${sbindir}/kexec \
>  # The 60 seconds is watchdog's default vaule.
>  WATCHDOG_TIMEOUT ??= "60"
> 
> +do_configure:prepend() {
> +  sed s@:ROOT_HOME:@${ROOT_HOME}@g ${WORKDIR}/basic.conf.in > ${S}/sysusers.d/basic.conf.in

Please indent shell code using tabs, and change the command to:

	sed -i s@:ROOT_HOME:@${ROOT_HOME}@g ${S}/sysusers.d/basic.conf.in

once you have added the patch as per above. 

However, I am wondering if this has to be done during do_configure()? 
It might become confusing if you ever `devtool modify systemd` (which 
we do). Wouldn't it be better to do it on the installed file in the 
do_install() below instead?

> +}
> +
>  do_install() {
>  	meson_do_install
>  	install -d ${D}/${base_sbindir}
> 
> --
> 2.41.0

//Peter


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [OE-core] [PATCH 3/3] base-passwd: add the wheel group
  2023-06-15 11:43 ` [PATCH 3/3] base-passwd: add the wheel group Louis Rannou
@ 2023-06-18  9:37   ` Alexandre Belloni
  2023-06-19 12:28     ` Louis Rannou
  0 siblings, 1 reply; 10+ messages in thread
From: Alexandre Belloni @ 2023-06-18  9:37 UTC (permalink / raw)
  To: Louis Rannou; +Cc: openembedded-core, anuj.mittal

On 15/06/2023 13:43:55+0200, Louis Rannou wrote:
> The wheel group is not declared while it can be used to access the systemd
> journal and to configure printers in CUPS. It can also be used for su and sudo
> permissions.
> 
> So far it was created later in the rootfs postcommand systemd_create_users.
> 
> Signed-off-by: Louis Rannou <lrannou@baylibre.com>
> ---
>  .../base-passwd/0007-Add-wheel-group.patch           | 20 ++++++++++++++++++++
>  meta/recipes-core/base-passwd/base-passwd_3.6.1.bb   |  1 +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch b/meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch
> new file mode 100644
> index 0000000000..00eaec38a2
> --- /dev/null
> +++ b/meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch
> @@ -0,0 +1,20 @@
> +
> +We need to have a wheel group which has some system privileges to consult the
> +systemd journal or manage printers with cups.
> +
> +Upstream status says the group does not exist by default.

This should be rephrased because it causes:

Malformed Upstream-Status 'Upstream status' (meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch)
Unknown Upstream-Status value 'says' (meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch)
Patches missing Upstream-Status: 0 (0%)
Patches with malformed Upstream-Status: 1 (0%)


> +
> +Upstream-Status: Inappropriate [enable feature]
> +
> +Signed-off-by: Louis Rannou <lrannou@baylibre.com>
> +Index: base-passwd-3.5.26/group.master
> +===================================================================
> +--- base-passwd-3.5.29.orig/group.master
> ++++ base-passwd-3.5.29/group.master
> +@@ -38,5 +38,6 @@
> + staff:*:50:
> + games:*:60:
> + shutdown:*:70:
> ++wheel:*:80:
> + users:*:100:
> + nogroup:*:65534:
> diff --git a/meta/recipes-core/base-passwd/base-passwd_3.6.1.bb b/meta/recipes-core/base-passwd/base-passwd_3.6.1.bb
> index 853717176d..204016b3e7 100644
> --- a/meta/recipes-core/base-passwd/base-passwd_3.6.1.bb
> +++ b/meta/recipes-core/base-passwd/base-passwd_3.6.1.bb
> @@ -12,6 +12,7 @@ SRC_URI = "https://launchpad.net/debian/+archive/primary/+files/${BPN}_${PV}.tar
>             file://0004-Add-an-input-group-for-the-dev-input-devices.patch \
>             file://0005-Add-kvm-group.patch \
>             file://0006-Make-it-possible-to-configure-whether-to-use-SELinux.patch \
> +           file://0007-Add-wheel-group.patch \
>             "
>  
>  SRC_URI[sha256sum] = "6ff369be59d586ba63c0c5fcb00f75f9953fe49db88bc6c6428f2c92866f79af"
> 
> -- 
> 2.41.0
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#182849): https://lists.openembedded.org/g/openembedded-core/message/182849
> Mute This Topic: https://lists.openembedded.org/mt/99546759/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [OE-core] [PATCH 2/3] systemd: replace the sysusers.d basic configuration
  2023-06-16 16:51   ` [OE-core] " Peter Kjellerstedt
@ 2023-06-19 12:27     ` Louis Rannou
       [not found]     ` <5a438786-9f24-e30d-ed84-08bf48b7bfd4@baylibre.com>
  1 sibling, 0 replies; 10+ messages in thread
From: Louis Rannou @ 2023-06-19 12:27 UTC (permalink / raw)
  To: Peter Kjellerstedt, openembedded-core@lists.openembedded.org
  Cc: anuj.mittal@intel.com

Hello,

On 16/06/2023 18:51, Peter Kjellerstedt wrote:
>> -----Original Message-----
>> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Louis Rannou
>> Sent: den 15 juni 2023 13:44
>> To: openembedded-core@lists.openembedded.org
>> Cc: Louis Rannou <lrannou@baylibre.com>; anuj.mittal@intel.com
>> Subject: [OE-core] [PATCH 2/3] systemd: replace the sysusers.d basic configuration
>>
>> The default sysusers basic.conf.in file sets the root home directory to `/root`
>> and does not permit its configuration. Replace the file delivered by systemd so
>> the root home directory matches the `ROOT_HOME` variable.
>>
>> Signed-off-by: Louis Rannou <lrannou@baylibre.com>
>> ---
>>   meta/recipes-core/systemd/systemd/basic.conf.in | 40 +++++++++++++++++++++++++
>>   meta/recipes-core/systemd/systemd_253.3.bb      |  5 ++++
>>   2 files changed, 45 insertions(+)
>>
>> diff --git a/meta/recipes-core/systemd/systemd/basic.conf.in b/meta/recipes-core/systemd/systemd/basic.conf.in
>> new file mode 100644
>> index 0000000000..fac288f7fa
>> --- /dev/null
>> +++ b/meta/recipes-core/systemd/systemd/basic.conf.in
>> @@ -0,0 +1,40 @@
>> +#  This file is part of systemd.
>> +#
>> +#  systemd is free software; you can redistribute it and/or modify it
>> +#  under the terms of the GNU Lesser General Public License as published by
>> +#  the Free Software Foundation; either version 2.1 of the License, or
>> +#  (at your option) any later version.
>> +
>> +# The superuser
>> +u root    0     "root" :ROOT_HOME:
>> +
>> +# The nobody user/group for NFS file systems
>> +g {{NOBODY_GROUP_NAME}} 65534       -            -
>> +u {{NOBODY_USER_NAME }} 65534:65534 "Nobody"     -
>> +
>> +# Administrator group: can *see* more than normal users
>> +g adm     {{ADM_GID    }}     -            -
>> +
>> +# Administrator group: can *do* more than normal users
>> +g wheel   {{WHEEL_GID  }}     -            -
>> +
>> +# Access to shared database of users on the system
>> +g utmp    {{UTMP_GID   }}     -            -
>> +
>> +# Physical and virtual hardware access groups
>> +g audio   {{AUDIO_GID  }}     -            -
>> +g cdrom   {{CDROM_GID  }}     -            -
>> +g dialout {{DIALOUT_GID}}     -            -
>> +g disk    {{DISK_GID   }}     -            -
>> +g input   {{INPUT_GID  }}     -            -
>> +g kmem    {{KMEM_GID   }}     -            -
>> +g kvm     {{KVM_GID    }}     -            -
>> +g lp      {{LP_GID     }}     -            -
>> +g render  {{RENDER_GID }}     -            -
>> +g sgx     {{SGX_GID    }}     -            -
>> +g tape    {{TAPE_GID   }}     -            -
>> +g tty     {{TTY_GID    }}     -            -
>> +g video   {{VIDEO_GID  }}     -            -
>> +
>> +# Default group for normal users
>> +g users   {{USERS_GID  }}     -            -
>> diff --git a/meta/recipes-core/systemd/systemd_253.3.bb b/meta/recipes-core/systemd/systemd_253.3.bb
>> index 45dc6ab5bb..87fbf6f785 100644
>> --- a/meta/recipes-core/systemd/systemd_253.3.bb
>> +++ b/meta/recipes-core/systemd/systemd_253.3.bb
>> @@ -17,6 +17,7 @@ REQUIRED_DISTRO_FEATURES = "systemd"
>>   SRC_URI += " \
>>              file://touchscreen.rules \
>>              file://00-create-volatile.conf \
>> +           file://basic.conf.in \
> 
> Instead of including a modified copy of the basic.conf.in file
> from systemd, include a patch that modifies the file that systemd
> provides. Otherwise this becomes a maintenance problem where it
> is easy to miss changes that upstream does to the file, and also
> hard to know what you have changed.
> 
This came to my mind, but it seems to me it's a configuration and not a 
patch. Some distribution as debian generate their own. It could also be 
replaced by the user to set its own preferences. It is not supposed to 
change the default configuration, but to set the one we need.

>>              ${@bb.utils.contains('PACKAGECONFIG', 'polkit_hostnamed_fallback', 'file://org.freedesktop.hostname1_no_polkit.conf', '', d)} \
>>              ${@bb.utils.contains('PACKAGECONFIG', 'polkit_hostnamed_fallback', 'file://00-hostnamed-network-user.conf', '', d)} \
>>              file://init \
>> @@ -252,6 +253,10 @@ EXTRA_OEMESON += "-Dkexec-path=${sbindir}/kexec \
>>   # The 60 seconds is watchdog's default vaule.
>>   WATCHDOG_TIMEOUT ??= "60"
>>
>> +do_configure:prepend() {
>> +  sed s@:ROOT_HOME:@${ROOT_HOME}@g ${WORKDIR}/basic.conf.in > ${S}/sysusers.d/basic.conf.in
> 
> Please indent shell code using tabs, and change the command to:
> 
> 	sed -i s@:ROOT_HOME:@${ROOT_HOME}@g ${S}/sysusers.d/basic.conf.in
> 
> once you have added the patch as per above.
I understand '@' is easier to read than ':'. I have used the colon 
because its usage is very restricted in the sysusers.d file. But it's 
fine to me as this is supposed to be a path. Also, the input file and 
the output are not in the same directory. I am not sure about the "-i" 
option.

> 
> However, I am wondering if this has to be done during do_configure()?
> It might become confusing if you ever `devtool modify systemd` (which
> we do). Wouldn't it be better to do it on the installed file in the
> do_install() below instead?
> 
This file is parsed at systemd compilation. The file has to be correct 
before that.

>> +}
>> +
>>   do_install() {
>>   	meson_do_install
>>   	install -d ${D}/${base_sbindir}
>>
>> --
>> 2.41.0
> 
> //Peter
> 

Louis


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [OE-core] [PATCH 3/3] base-passwd: add the wheel group
  2023-06-18  9:37   ` [OE-core] " Alexandre Belloni
@ 2023-06-19 12:28     ` Louis Rannou
  0 siblings, 0 replies; 10+ messages in thread
From: Louis Rannou @ 2023-06-19 12:28 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: openembedded-core, anuj.mittal



On 18/06/2023 11:37, Alexandre Belloni wrote:
> On 15/06/2023 13:43:55+0200, Louis Rannou wrote:
>> The wheel group is not declared while it can be used to access the systemd
>> journal and to configure printers in CUPS. It can also be used for su and sudo
>> permissions.
>>
>> So far it was created later in the rootfs postcommand systemd_create_users.
>>
>> Signed-off-by: Louis Rannou <lrannou@baylibre.com>
>> ---
>>   .../base-passwd/0007-Add-wheel-group.patch           | 20 ++++++++++++++++++++
>>   meta/recipes-core/base-passwd/base-passwd_3.6.1.bb   |  1 +
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch b/meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch
>> new file mode 100644
>> index 0000000000..00eaec38a2
>> --- /dev/null
>> +++ b/meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch
>> @@ -0,0 +1,20 @@
>> +
>> +We need to have a wheel group which has some system privileges to consult the
>> +systemd journal or manage printers with cups.
>> +
>> +Upstream status says the group does not exist by default.
> 
> This should be rephrased because it causes:
> 
> Malformed Upstream-Status 'Upstream status' (meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch)
> Unknown Upstream-Status value 'says' (meta/recipes-core/base-passwd/base-passwd/0007-Add-wheel-group.patch)
> Patches missing Upstream-Status: 0 (0%)
> Patches with malformed Upstream-Status: 1 (0%

Sorry for that. I didn't expect it would parse any line starting with 
Upstream status...

> 
> 
>> +
>> +Upstream-Status: Inappropriate [enable feature]
>> +
>> +Signed-off-by: Louis Rannou <lrannou@baylibre.com>
>> +Index: base-passwd-3.5.26/group.master
>> +===================================================================
>> +--- base-passwd-3.5.29.orig/group.master
>> ++++ base-passwd-3.5.29/group.master
>> +@@ -38,5 +38,6 @@
>> + staff:*:50:
>> + games:*:60:
>> + shutdown:*:70:
>> ++wheel:*:80:
>> + users:*:100:
>> + nogroup:*:65534:
>> diff --git a/meta/recipes-core/base-passwd/base-passwd_3.6.1.bb b/meta/recipes-core/base-passwd/base-passwd_3.6.1.bb
>> index 853717176d..204016b3e7 100644
>> --- a/meta/recipes-core/base-passwd/base-passwd_3.6.1.bb
>> +++ b/meta/recipes-core/base-passwd/base-passwd_3.6.1.bb
>> @@ -12,6 +12,7 @@ SRC_URI = "https://launchpad.net/debian/+archive/primary/+files/${BPN}_${PV}.tar
>>              file://0004-Add-an-input-group-for-the-dev-input-devices.patch \
>>              file://0005-Add-kvm-group.patch \
>>              file://0006-Make-it-possible-to-configure-whether-to-use-SELinux.patch \
>> +           file://0007-Add-wheel-group.patch \
>>              "
>>   
>>   SRC_URI[sha256sum] = "6ff369be59d586ba63c0c5fcb00f75f9953fe49db88bc6c6428f2c92866f79af"
>>
>> -- 
>> 2.41.0
>>
> 
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#182849): https://lists.openembedded.org/g/openembedded-core/message/182849
>> Mute This Topic: https://lists.openembedded.org/mt/99546759/3617179
>> Group Owner: openembedded-core+owner@lists.openembedded.org
>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
> 
> 

Louis


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [OE-core] [PATCH 2/3] systemd: replace the sysusers.d basic configuration
       [not found]     ` <5a438786-9f24-e30d-ed84-08bf48b7bfd4@baylibre.com>
@ 2023-06-29 13:03       ` Louis Rannou
  2023-07-25 17:11         ` Peter Kjellerstedt
  0 siblings, 1 reply; 10+ messages in thread
From: Louis Rannou @ 2023-06-29 13:03 UTC (permalink / raw)
  To: Peter Kjellerstedt, openembedded-core@lists.openembedded.org
  Cc: anuj.mittal@intel.com

Hello, please find an answer to the last comment

On 19/06/2023 14:10, Louis Rannou wrote:
> Hello,
> 
> On 16/06/2023 18:51, Peter Kjellerstedt wrote:
>>> -----Original Message-----
>>> From: openembedded-core@lists.openembedded.org 
>>> <openembedded-core@lists.openembedded.org> On Behalf Of Louis Rannou
>>> Sent: den 15 juni 2023 13:44
>>> To: openembedded-core@lists.openembedded.org
>>> Cc: Louis Rannou <lrannou@baylibre.com>; anuj.mittal@intel.com
>>> Subject: [OE-core] [PATCH 2/3] systemd: replace the sysusers.d basic 
>>> configuration
>>>
>>> The default sysusers basic.conf.in file sets the root home directory 
>>> to `/root`
>>> and does not permit its configuration. Replace the file delivered by 
>>> systemd so
>>> the root home directory matches the `ROOT_HOME` variable.
>>>
>>> Signed-off-by: Louis Rannou <lrannou@baylibre.com>
>>> ---
>>>   meta/recipes-core/systemd/systemd/basic.conf.in | 40 
>>> +++++++++++++++++++++++++
>>>   meta/recipes-core/systemd/systemd_253.3.bb      |  5 ++++
>>>   2 files changed, 45 insertions(+)
>>>
>>> diff --git a/meta/recipes-core/systemd/systemd/basic.conf.in 
>>> b/meta/recipes-core/systemd/systemd/basic.conf.in
>>> new file mode 100644
>>> index 0000000000..fac288f7fa
>>> --- /dev/null
>>> +++ b/meta/recipes-core/systemd/systemd/basic.conf.in
>>> @@ -0,0 +1,40 @@
>>> +#  This file is part of systemd.
>>> +#
>>> +#  systemd is free software; you can redistribute it and/or modify it
>>> +#  under the terms of the GNU Lesser General Public License as 
>>> published by
>>> +#  the Free Software Foundation; either version 2.1 of the License, or
>>> +#  (at your option) any later version.
>>> +
>>> +# The superuser
>>> +u root    0     "root" :ROOT_HOME:
>>> +
>>> +# The nobody user/group for NFS file systems
>>> +g {{NOBODY_GROUP_NAME}} 65534       -            -
>>> +u {{NOBODY_USER_NAME }} 65534:65534 "Nobody"     -
>>> +
>>> +# Administrator group: can *see* more than normal users
>>> +g adm     {{ADM_GID    }}     -            -
>>> +
>>> +# Administrator group: can *do* more than normal users
>>> +g wheel   {{WHEEL_GID  }}     -            -
>>> +
>>> +# Access to shared database of users on the system
>>> +g utmp    {{UTMP_GID   }}     -            -
>>> +
>>> +# Physical and virtual hardware access groups
>>> +g audio   {{AUDIO_GID  }}     -            -
>>> +g cdrom   {{CDROM_GID  }}     -            -
>>> +g dialout {{DIALOUT_GID}}     -            -
>>> +g disk    {{DISK_GID   }}     -            -
>>> +g input   {{INPUT_GID  }}     -            -
>>> +g kmem    {{KMEM_GID   }}     -            -
>>> +g kvm     {{KVM_GID    }}     -            -
>>> +g lp      {{LP_GID     }}     -            -
>>> +g render  {{RENDER_GID }}     -            -
>>> +g sgx     {{SGX_GID    }}     -            -
>>> +g tape    {{TAPE_GID   }}     -            -
>>> +g tty     {{TTY_GID    }}     -            -
>>> +g video   {{VIDEO_GID  }}     -            -
>>> +
>>> +# Default group for normal users
>>> +g users   {{USERS_GID  }}     -            -
>>> diff --git a/meta/recipes-core/systemd/systemd_253.3.bb 
>>> b/meta/recipes-core/systemd/systemd_253.3.bb
>>> index 45dc6ab5bb..87fbf6f785 100644
>>> --- a/meta/recipes-core/systemd/systemd_253.3.bb
>>> +++ b/meta/recipes-core/systemd/systemd_253.3.bb
>>> @@ -17,6 +17,7 @@ REQUIRED_DISTRO_FEATURES = "systemd"
>>>   SRC_URI += " \
>>>              file://touchscreen.rules \
>>>              file://00-create-volatile.conf \
>>> +           file://basic.conf.in \
>>
>> Instead of including a modified copy of the basic.conf.in file
>> from systemd, include a patch that modifies the file that systemd
>> provides. Otherwise this becomes a maintenance problem where it
>> is easy to miss changes that upstream does to the file, and also
>> hard to know what you have changed.
>>
> This came to my mind, but it seems to me it's a configuration and not a 
> patch. Some distribution as debian generate their own.
> 
>>>              ${@bb.utils.contains('PACKAGECONFIG', 
>>> 'polkit_hostnamed_fallback', 
>>> 'file://org.freedesktop.hostname1_no_polkit.conf', '', d)} \
>>>              ${@bb.utils.contains('PACKAGECONFIG', 
>>> 'polkit_hostnamed_fallback', 'file://00-hostnamed-network-user.conf', 
>>> '', d)} \
>>>              file://init \
>>> @@ -252,6 +253,10 @@ EXTRA_OEMESON += "-Dkexec-path=${sbindir}/kexec \
>>>   # The 60 seconds is watchdog's default vaule.
>>>   WATCHDOG_TIMEOUT ??= "60"
>>>
>>> +do_configure:prepend() {
>>> +  sed s@:ROOT_HOME:@${ROOT_HOME}@g ${WORKDIR}/basic.conf.in > 
>>> ${S}/sysusers.d/basic.conf.in
>>
>> Please indent shell code using tabs, and change the command to:
>>
>>     sed -i s@:ROOT_HOME:@${ROOT_HOME}@g ${S}/sysusers.d/basic.conf.in
>>
>> once you have added the patch as per above.
That's incorrect as the source and destination are different. Perhaps I 
should call the initial file basic.conf.in.in if it's more clear.

>>
>> However, I am wondering if this has to be done during do_configure()?
>> It might become confusing if you ever `devtool modify systemd` (which
>> we do). Wouldn't it be better to do it on the installed file in the
>> do_install() below instead?
This file is used at compilation to produce the basic.conf file. So it's 
needed after the configuration. What would go wrong with devtool ?

>>
>>> +}
>>> +
>>>   do_install() {
>>>       meson_do_install
>>>       install -d ${D}/${base_sbindir}
>>>

Louis


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [OE-core] [PATCH 2/3] systemd: replace the sysusers.d basic configuration
  2023-06-29 13:03       ` Louis Rannou
@ 2023-07-25 17:11         ` Peter Kjellerstedt
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Kjellerstedt @ 2023-07-25 17:11 UTC (permalink / raw)
  To: Louis Rannou, openembedded-core@lists.openembedded.org
  Cc: anuj.mittal@intel.com

> -----Original Message-----
> From: Louis Rannou <lrannou@baylibre.com>
> Sent: den 29 juni 2023 15:04
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> core@lists.openembedded.org
> Cc: anuj.mittal@intel.com
> Subject: Re: [OE-core] [PATCH 2/3] systemd: replace the sysusers.d basic
> configuration
> 
> Hello, please find an answer to the last comment
> 
> On 19/06/2023 14:10, Louis Rannou wrote:
> > Hello,
> >
> > On 16/06/2023 18:51, Peter Kjellerstedt wrote:
> >>> -----Original Message-----
> >>> From: openembedded-core@lists.openembedded.org
> >>> <openembedded-core@lists.openembedded.org> On Behalf Of Louis Rannou
> >>> Sent: den 15 juni 2023 13:44
> >>> To: openembedded-core@lists.openembedded.org
> >>> Cc: Louis Rannou <lrannou@baylibre.com>; anuj.mittal@intel.com
> >>> Subject: [OE-core] [PATCH 2/3] systemd: replace the sysusers.d basic
> >>> configuration
> >>>
> >>> The default sysusers basic.conf.in file sets the root home directory
> >>> to `/root`
> >>> and does not permit its configuration. Replace the file delivered by
> >>> systemd so
> >>> the root home directory matches the `ROOT_HOME` variable.
> >>>
> >>> Signed-off-by: Louis Rannou <lrannou@baylibre.com>
> >>> ---
> >>>   meta/recipes-core/systemd/systemd/basic.conf.in | 40
> >>> +++++++++++++++++++++++++
> >>>   meta/recipes-core/systemd/systemd_253.3.bb      |  5 ++++
> >>>   2 files changed, 45 insertions(+)
> >>>
> >>> diff --git a/meta/recipes-core/systemd/systemd/basic.conf.in
> >>> b/meta/recipes-core/systemd/systemd/basic.conf.in
> >>> new file mode 100644
> >>> index 0000000000..fac288f7fa
> >>> --- /dev/null
> >>> +++ b/meta/recipes-core/systemd/systemd/basic.conf.in
> >>> @@ -0,0 +1,40 @@
> >>> +#  This file is part of systemd.
> >>> +#
> >>> +#  systemd is free software; you can redistribute it and/or modify it
> >>> +#  under the terms of the GNU Lesser General Public License as
> >>> published by
> >>> +#  the Free Software Foundation; either version 2.1 of the License,
> or
> >>> +#  (at your option) any later version.
> >>> +
> >>> +# The superuser
> >>> +u root    0     "root" :ROOT_HOME:
> >>> +
> >>> +# The nobody user/group for NFS file systems
> >>> +g {{NOBODY_GROUP_NAME}} 65534       -            -
> >>> +u {{NOBODY_USER_NAME }} 65534:65534 "Nobody"     -
> >>> +
> >>> +# Administrator group: can *see* more than normal users
> >>> +g adm     {{ADM_GID    }}     -            -
> >>> +
> >>> +# Administrator group: can *do* more than normal users
> >>> +g wheel   {{WHEEL_GID  }}     -            -
> >>> +
> >>> +# Access to shared database of users on the system
> >>> +g utmp    {{UTMP_GID   }}     -            -
> >>> +
> >>> +# Physical and virtual hardware access groups
> >>> +g audio   {{AUDIO_GID  }}     -            -
> >>> +g cdrom   {{CDROM_GID  }}     -            -
> >>> +g dialout {{DIALOUT_GID}}     -            -
> >>> +g disk    {{DISK_GID   }}     -            -
> >>> +g input   {{INPUT_GID  }}     -            -
> >>> +g kmem    {{KMEM_GID   }}     -            -
> >>> +g kvm     {{KVM_GID    }}     -            -
> >>> +g lp      {{LP_GID     }}     -            -
> >>> +g render  {{RENDER_GID }}     -            -
> >>> +g sgx     {{SGX_GID    }}     -            -
> >>> +g tape    {{TAPE_GID   }}     -            -
> >>> +g tty     {{TTY_GID    }}     -            -
> >>> +g video   {{VIDEO_GID  }}     -            -
> >>> +
> >>> +# Default group for normal users
> >>> +g users   {{USERS_GID  }}     -            -
> >>> diff --git a/meta/recipes-core/systemd/systemd_253.3.bb
> >>> b/meta/recipes-core/systemd/systemd_253.3.bb
> >>> index 45dc6ab5bb..87fbf6f785 100644
> >>> --- a/meta/recipes-core/systemd/systemd_253.3.bb
> >>> +++ b/meta/recipes-core/systemd/systemd_253.3.bb
> >>> @@ -17,6 +17,7 @@ REQUIRED_DISTRO_FEATURES = "systemd"
> >>>   SRC_URI += " \
> >>>              file://touchscreen.rules \
> >>>              file://00-create-volatile.conf \
> >>> +           file://basic.conf.in \
> >>
> >> Instead of including a modified copy of the basic.conf.in file
> >> from systemd, include a patch that modifies the file that systemd
> >> provides. Otherwise this becomes a maintenance problem where it
> >> is easy to miss changes that upstream does to the file, and also
> >> hard to know what you have changed.
> >>
> > This came to my mind, but it seems to me it's a configuration and not a
> > patch. Some distribution as debian generate their own.
> >
> >>>              ${@bb.utils.contains('PACKAGECONFIG', 'polkit_hostnamed_fallback', 'file://org.freedesktop.hostname1_no_polkit.conf', '', d)} \
> >>>              ${@bb.utils.contains('PACKAGECONFIG', 'polkit_hostnamed_fallback', 'file://00-hostnamed-network-user.conf', '', d)} \
> >>>              file://init \
> >>> @@ -252,6 +253,10 @@ EXTRA_OEMESON += "-Dkexec-path=${sbindir}/kexec \
> >>>   # The 60 seconds is watchdog's default vaule.
> >>>   WATCHDOG_TIMEOUT ??= "60"
> >>>
> >>> +do_configure:prepend() {
> >>> +  sed s@:ROOT_HOME:@${ROOT_HOME}@g ${WORKDIR}/basic.conf.in > ${S}/sysusers.d/basic.conf.in
> >>
> >> Please indent shell code using tabs, and change the command to:
> >>
> >>     sed -i s@:ROOT_HOME:@${ROOT_HOME}@g ${S}/sysusers.d/basic.conf.in
> >>
> >> once you have added the patch as per above.
> 
> That's incorrect as the source and destination are different. Perhaps I
> should call the initial file basic.conf.in.in if it's more clear.

Note the "once you have added the patch" part. I.e., once you have added a 
patch to modify the basic.conf.in that systemd provides, the above command 
should work. However, depending on whether systemd´s build system actually 
uses the path provided in the basic.conf.in file (which I doubt), a better 
way would be do run the sed above on the installed basic.conf file. I.e., 
use a patch to modify basic.conf.in to contain ":ROOT_HOME:" instead of 
what path systemd upstream provides, and then use sed in do_install:append 
to replace ":ROOT_HOME:" with "${ROOT_HOME}" in the installed basic.conf 
file.

> 
> >>
> >> However, I am wondering if this has to be done during do_configure()?
> >> It might become confusing if you ever `devtool modify systemd` (which
> >> we do). Wouldn't it be better to do it on the installed file in the
> >> do_install() below instead?
> This file is used at compilation to produce the basic.conf file. So it's
> needed after the configuration. What would go wrong with devtool ?

The change that sed makes would be seen as a change to the sources checked 
out by devtool, and thus you risk someone accidentally committing it when 
they make other changes to systemd using devtool modify.

> 
> >>
> >>> +}
> >>> +
> >>>   do_install() {
> >>>       meson_do_install
> >>>       install -d ${D}/${base_sbindir}
> >>>
> 
> Louis

//Peter


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-07-25 17:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-15 11:43 [PATCH 0/3] rootfs-postcommands: replace the sysusers.d postcommand Louis Rannou
2023-06-15 11:43 ` [PATCH 1/3] rootfs-postcommands: change sysusers.d command Louis Rannou
2023-06-15 11:43 ` [PATCH 2/3] systemd: replace the sysusers.d basic configuration Louis Rannou
2023-06-16 16:51   ` [OE-core] " Peter Kjellerstedt
2023-06-19 12:27     ` Louis Rannou
     [not found]     ` <5a438786-9f24-e30d-ed84-08bf48b7bfd4@baylibre.com>
2023-06-29 13:03       ` Louis Rannou
2023-07-25 17:11         ` Peter Kjellerstedt
2023-06-15 11:43 ` [PATCH 3/3] base-passwd: add the wheel group Louis Rannou
2023-06-18  9:37   ` [OE-core] " Alexandre Belloni
2023-06-19 12:28     ` Louis Rannou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox