[kirkstone][PATCH 0/4] Add sub dir for passwd files

[kirkstone][PATCH 0/4] Add sub dir for passwd files

[Joakim Tjernlund]

These patches adds the possibility to store passwd/shadow files
in a sub dir, like /etc/pwdb
In a RO Root FS one can bind mount a writeable dir on /etc/pwdb
to support password changes etc.


Joakim Tjernlund (4):
  [meta classes] sed -i destroys symlinks
  base-passwd: Add PW_SUBDIR
  pseudo: Add PW_SUBDIR
  shadow: Add PW_SUBDIR

 meta/classes/rootfs-postcommands.bbclass      |  4 +-
 meta/classes/useradd_base.bbclass             |  2 +-
 .../base-passwd/base-passwd_3.5.29.bb         | 24 +++--
 meta/recipes-devtools/pseudo/pseudo.inc       | 11 ++-
 .../0001-Define-SUBUID_FILE-SUBGID_FILE.patch | 92 +++++++++++++++++++
 meta/recipes-extended/shadow/shadow.inc       | 30 +++++-
 6 files changed, 145 insertions(+), 18 deletions(-)
 create mode 100644 meta/recipes-extended/shadow/files/0001-Define-SUBUID_FILE-SUBGID_FILE.patch

-- 
2.41.0

[kirkstone][PATCH 1/4] [meta classes] sed -i destroys symlinks

[Joakim Tjernlund]

If /etc/passwd is a symlink, sed -i on same file will replace the
symlink with a new file. Prevent that by adding --follow-symlinks
option to sed

Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
---
 meta/classes/rootfs-postcommands.bbclass | 4 ++--
 meta/classes/useradd_base.bbclass        | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/meta/classes/rootfs-postcommands.bbclass b/meta/classes/rootfs-postcommands.bbclass
index 5c0b3ec37c..993262dd0c 100644
--- a/meta/classes/rootfs-postcommands.bbclass
+++ b/meta/classes/rootfs-postcommands.bbclass
@@ -148,10 +148,10 @@ read_only_rootfs_hook () {
 #
 zap_empty_root_password () {
 	if [ -e ${IMAGE_ROOTFS}/etc/shadow ]; then
-		sed -i 's%^root::%root:*:%' ${IMAGE_ROOTFS}/etc/shadow
+		sed --follow-symlinks -i 's%^root::%root:*:%' ${IMAGE_ROOTFS}/etc/shadow
         fi
 	if [ -e ${IMAGE_ROOTFS}/etc/passwd ]; then
-		sed -i 's%^root::%root:*:%' ${IMAGE_ROOTFS}/etc/passwd
+		sed --follow-symlinks -i 's%^root::%root:*:%' ${IMAGE_ROOTFS}/etc/passwd
 	fi
 }
 
diff --git a/meta/classes/useradd_base.bbclass b/meta/classes/useradd_base.bbclass
index 7f5b9b7219..da47311e84 100644
--- a/meta/classes/useradd_base.bbclass
+++ b/meta/classes/useradd_base.bbclass
@@ -154,7 +154,7 @@ perform_passwd_expire () {
 	local username=`echo "$opts" | awk '{ print $NF }'`
 	local user_exists="`grep "^$username:" $rootdir/etc/passwd || true`"
 	if test "x$user_exists" != "x"; then
-		eval flock -x $rootdir${sysconfdir} -c \"$PSEUDO sed -i \''s/^\('$username':[^:]*\):[^:]*:/\1:0:/'\' $rootdir/etc/shadow \" || true
+		eval flock -x $rootdir${sysconfdir} -c \"$PSEUDO sed --follow-symlinks -i \''s/^\('$username':[^:]*\):[^:]*:/\1:0:/'\' $rootdir/etc/shadow \" || true
 		local passwd_lastchanged="`grep "^$username:" $rootdir/etc/shadow | cut -d: -f3`"
 		if test "x$passwd_lastchanged" != "x0"; then
 			bbfatal "${PN}: passwd --expire operation did not succeed."
-- 
2.41.0

[kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR

[Joakim Tjernlund]

Add support for creating passwd files in a /etc subdir
Set PW_SUBIR to pwdb to get passwd  files in /etc/pwdb

Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
---
 .../base-passwd/base-passwd_3.5.29.bb         | 24 ++++++++++++-------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
index ef7792ae49..e453be0763 100644
--- a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
+++ b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
@@ -20,6 +20,9 @@ SRC_URI = "https://launchpad.net/debian/+archive/primary/+files/${BPN}_${PV}.tar
 SRC_URI[md5sum] = "6beccac48083fe8ae5048acd062e5421"
 SRC_URI[sha256sum] = "f0b66388b2c8e49c15692439d2bee63bcdd4bbbf7a782c7f64accc55986b6a36"
 
+#Set PW_SUBDIR to pwdb to get passwd  files in /etc/pwdb
+PW_SUBDIR ?= ""
+
 # the package is taken from launchpad; that source is static and goes stale
 # so we check the latest upstream from a directory that does get updated
 UPSTREAM_CHECK_URI = "${DEBIAN_MIRROR}/main/b/base-passwd/"
@@ -50,10 +53,11 @@ basepasswd_sysroot_postinst() {
 #!/bin/sh
 
 # Install passwd.master and group.master to sysconfdir
-install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}
+install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}
 for i in passwd group; do
 	install -p -m 644 ${STAGING_DIR_TARGET}${datadir}/base-passwd/\$i.master \
-		${STAGING_DIR_TARGET}${sysconfdir}/\$i
+		${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}/\$i
+	[ -n "${PW_SUBDIR}" ] && ln -fs ${PW_SUBDIR}/\$i ${STAGING_DIR_TARGET}${sysconfdir}/\$i
 done
 
 # Run any useradd postinsts
@@ -89,15 +93,19 @@ python populate_packages:prepend() {
     f.close()
 
     preinst = """#!/bin/sh
-mkdir -p $D${sysconfdir}
-if [ ! -e $D${sysconfdir}/passwd ]; then
-\tcat << 'EOF' > $D${sysconfdir}/passwd
+mkdir -p $D${sysconfdir}/${PW_SUBDIR}
+if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/passwd ]; then
+\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/passwd
 """ + passwd + """EOF
 fi
-if [ ! -e $D${sysconfdir}/group ]; then
-\tcat << 'EOF' > $D${sysconfdir}/group
+if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/group ]; then
+\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/group
 """ + group + """EOF
 fi
+if [ -n "${PW_SUBDIR}" ]; then
+ln -fs ${PW_SUBDIR}/passwd $D${sysconfdir}/passwd
+ln -fs ${PW_SUBDIR}/group $D${sysconfdir}/group
+fi
 """
     d.setVar(d.expand('pkg_preinst:${PN}'), preinst)
 }
@@ -114,5 +122,5 @@ pkg_postinst:${PN}-update () {
 if [ -n "$D" ]; then
 	exit 0
 fi
-${sbindir}/update-passwd
+${sbindir}/update-passwd -P /etc/${PW_SUBDIR}/passwd -S /etc/${PW_SUBDIR}/shadow -G /etc/${PW_SUBDIR}/group
 }
-- 
2.41.0

[kirkstone][PATCH 3/4] pseudo: Add PW_SUBDIR

[Joakim Tjernlund]

Add support for creating passwd files in a /etc subdir
Set PW_SUBIR to pwdb to get passwd files in /etc/pwdb

Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
---
 meta/recipes-devtools/pseudo/pseudo.inc | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-devtools/pseudo/pseudo.inc b/meta/recipes-devtools/pseudo/pseudo.inc
index 7e09b6d58c..7ba2e2261c 100644
--- a/meta/recipes-devtools/pseudo/pseudo.inc
+++ b/meta/recipes-devtools/pseudo/pseudo.inc
@@ -10,6 +10,9 @@ SECTION = "base"
 LICENSE = "LGPL-2.1-only"
 DEPENDS = "sqlite3 attr"
 
+#Set PW_SUBDIR to pwdb to get passwd files in /etc/pwdb
+PW_SUBDIR ?= ""
+
 FILES:${PN} = "${prefix}/lib/pseudo/lib*/libpseudo.so ${bindir}/* ${localstatedir}/pseudo ${prefix}/var/pseudo"
 INSANE_SKIP:${PN} += "libdir"
 INSANE_SKIP:${PN}-dbg += "libdir"
@@ -131,10 +134,12 @@ do_install () {
 
 do_install:append:class-native () {
 	chrpath ${D}${bindir}/pseudo -r `chrpath ${D}${bindir}/pseudo | cut -d = -f 2 | sed s/XORIGIN/\\$ORIGIN/`
-	install -d ${D}${sysconfdir}
+	install -d ${D}${sysconfdir}/${PW_SUBDIR}
 	# The fallback files should never be modified
-	install -m 444 ${WORKDIR}/fallback-passwd ${D}${sysconfdir}/passwd
-	install -m 444 ${WORKDIR}/fallback-group ${D}${sysconfdir}/group
+	install -m 444 ${WORKDIR}/fallback-passwd ${D}${sysconfdir}/${PW_SUBDIR}/passwd
+        [ -n "${PW_SUBDIR}" ] && ln -fs ${PW_SUBDIR}/passwd ${D}${sysconfdir}/passwd
+	install -m 444 ${WORKDIR}/fallback-group ${D}${sysconfdir}/${PW_SUBDIR}/group
+        [ -n "${PW_SUBDIR}" ] && ln -fs ${PW_SUBDIR}/group ${D}${sysconfdir}/group
 
 	# Two native/nativesdk entries below are the same
 	# If necessary install for the alternative machine arch.  This is only
-- 
2.41.0

[kirkstone][PATCH 4/4] shadow: Add PW_SUBDIR

[Joakim Tjernlund]

Add support for creating passwd files in a /etc subdir
Set PW_SUBIR to pwdb to get passwd files in /etc/pwdb

Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
---
 .../0001-Define-SUBUID_FILE-SUBGID_FILE.patch | 92 +++++++++++++++++++
 meta/recipes-extended/shadow/shadow.inc       | 30 +++++-
 2 files changed, 118 insertions(+), 4 deletions(-)
 create mode 100644 meta/recipes-extended/shadow/files/0001-Define-SUBUID_FILE-SUBGID_FILE.patch

diff --git a/meta/recipes-extended/shadow/files/0001-Define-SUBUID_FILE-SUBGID_FILE.patch b/meta/recipes-extended/shadow/files/0001-Define-SUBUID_FILE-SUBGID_FILE.patch
new file mode 100644
index 0000000000..9f85159e97
--- /dev/null
+++ b/meta/recipes-extended/shadow/files/0001-Define-SUBUID_FILE-SUBGID_FILE.patch
@@ -0,0 +1,92 @@
+From f605fb315faef7ddcad70d638f3b3aa16ea98fc0 Mon Sep 17 00:00:00 2001
+From: Joakim Tjernlund <joakim.tjernlund@infinera.com>
+Date: Thu, 2 Nov 2023 00:27:10 +0100
+Subject: [PATCH] Define SUBUID_FILE/SUBGID_FILE
+
+Upstream-Status: Backport, https://github.com/shadow-maint/shadow/commit/ee3a79c6952f8ca649c286c7f76639d9d1dedaad
+
+These where hard coded, make them definable like SHADOW_FILE
+
+Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
+---
+ lib/defines.h         | 8 ++++++++
+ lib/subordinateio.c   | 6 +++---
+ libmisc/prefix_flag.c | 8 ++++----
+ 3 files changed, 15 insertions(+), 7 deletions(-)
+
+diff --git a/lib/defines.h b/lib/defines.h
+index fc1521c..27b220f 100644
+--- a/lib/defines.h
++++ b/lib/defines.h
+@@ -312,6 +312,14 @@ char *strchr (), *strrchr (), *strtok ();
+ #define SHADOW_FILE "/etc/shadow"
+ #endif
+ 
++#ifndef SUBUID_FILE
++#define SUBUID_FILE "/etc/subuid"
++#endif
++
++#ifndef SUBGID_FILE
++#define SUBGID_FILE "/etc/subgid"
++#endif
++
+ #ifdef SHADOWGRP
+ #ifndef SGROUP_FILE
+ #define SGROUP_FILE "/etc/gshadow"
+diff --git a/lib/subordinateio.c b/lib/subordinateio.c
+index 9ca70b8..9ddc5e1 100644
+--- a/lib/subordinateio.c
++++ b/lib/subordinateio.c
+@@ -206,7 +206,7 @@ static const struct subordinate_range *find_range(struct commonio_db *db,
+         /*
+          * We only do special handling for these two files
+          */
+-        if ((0 != strcmp(db->filename, "/etc/subuid")) && (0 != strcmp(db->filename, "/etc/subgid")))
++        if ((0 != strcmp(db->filename, SUBUID_FILE)) && (0 != strcmp(db->filename, SUBGID_FILE)))
+                 return NULL;
+ 
+         /*
+@@ -554,7 +554,7 @@ static int remove_range (struct commonio_db *db,
+ }
+ 
+ static struct commonio_db subordinate_uid_db = {
+-	"/etc/subuid",		/* filename */
++	SUBUID_FILE,		/* filename */
+ 	&subordinate_ops,	/* ops */
+ 	NULL,			/* fp */
+ #ifdef WITH_SELINUX
+@@ -650,7 +650,7 @@ uid_t sub_uid_find_free_range(uid_t min, uid_t max, unsigned long count)
+ }
+ 
+ static struct commonio_db subordinate_gid_db = {
+-	"/etc/subgid",		/* filename */
++	SUBGID_FILE,		/* filename */
+ 	&subordinate_ops,	/* ops */
+ 	NULL,			/* fp */
+ #ifdef WITH_SELINUX
+diff --git a/libmisc/prefix_flag.c b/libmisc/prefix_flag.c
+index d4dfbc2..0e7dfa7 100644
+--- a/libmisc/prefix_flag.c
++++ b/libmisc/prefix_flag.c
+@@ -120,14 +120,14 @@ extern const char* process_prefix_flag (const char* short_opt, int argc, char **
+ 		spw_setdbname(spw_db_file);
+ 
+ #ifdef ENABLE_SUBIDS
+-		len = strlen(prefix) + strlen("/etc/subuid") + 2;
++		len = strlen(prefix) + strlen(SUBUID_FILE) + 2;
+ 		suid_db_file = xmalloc(len);
+-		snprintf(suid_db_file, len, "%s/%s", prefix, "/etc/subuid");
++		snprintf(suid_db_file, len, "%s/%s", prefix, SUBUID_FILE);
+ 		sub_uid_setdbname(suid_db_file);
+ 
+-		len = strlen(prefix) + strlen("/etc/subgid") + 2;
++		len = strlen(prefix) + strlen(SUBGID_FILE) + 2;
+ 		sgid_db_file = xmalloc(len);
+-		snprintf(sgid_db_file, len, "%s/%s", prefix, "/etc/subgid");
++		snprintf(sgid_db_file, len, "%s/%s", prefix, SUBGID_FILE);
+ 		sub_gid_setdbname(sgid_db_file);
+ #endif
+ 
+-- 
+2.41.0
+
diff --git a/meta/recipes-extended/shadow/shadow.inc b/meta/recipes-extended/shadow/shadow.inc
index 3c1dd2f98e..bcb9b09a49 100644
--- a/meta/recipes-extended/shadow/shadow.inc
+++ b/meta/recipes-extended/shadow/shadow.inc
@@ -18,6 +18,7 @@ SRC_URI = "https://github.com/shadow-maint/shadow/releases/download/v${PV}/${BP}
            file://useradd \
            file://CVE-2023-29383.patch \
            file://0001-Overhaul-valid_field.patch \
+           file://0001-Define-SUBUID_FILE-SUBGID_FILE.patch \
            "
 
 SRC_URI:append:class-target = " \
@@ -46,6 +47,21 @@ PAM_SRC_URI = "file://pam.d/chfn \
                file://pam.d/passwd \
                file://pam.d/su"
 
+#Set PW_SUBDIR to pwdb to get passwd files in /etc/pwdb
+PW_SUBDIR ?= ""
+PWPRE = "/etc/${PW_SUBDIR}"
+CFLAGS:append = ' -DPASSWD_FILE=\\"${PWPRE}/passwd\\"'
+CFLAGS:append = ' -DSHADOW_FILE=\\"${PWPRE}/shadow\\"'
+CFLAGS:append = ' -DGROUP_FILE=\\"${PWPRE}/group\\"'
+CFLAGS:append = ' -DSGROUP_FILE=\\"${PWPRE}/gshadow\\"'
+CFLAGS:append = ' -DSUBUID_FILE=\\"${PWPRE}/subuid\\"'
+CFLAGS:append = ' -DSUBGID_FILE=\\"${PWPRE}/subgid\\"'
+
+#shadow has it own impl. that uses whatever dir passwd files are in
+do_configure:prepend () {
+    sed -i -e 's/lckpwdf//' ${S}/configure.ac
+}
+
 inherit autotools gettext
 
 export CONFIG_SHELL="/bin/sh"
@@ -157,9 +173,9 @@ do_install:append() {
 	# usermod requires the subuid/subgid files to be in place before being
 	# able to use the -v/-V flags otherwise it fails:
 	# usermod: /etc/subuid does not exist, you cannot use the flags -v or -V
-	install -d ${D}${sysconfdir}
-	touch ${D}${sysconfdir}/subuid
-	touch ${D}${sysconfdir}/subgid
+	install -d ${D}${sysconfdir}/${PW_SUBDIR}
+	touch ${D}${sysconfdir}/${PW_SUBDIR}/subuid
+	touch ${D}${sysconfdir}/${PW_SUBDIR}/subgid
 }
 
 PACKAGES =+ "${PN}-base"
@@ -193,12 +209,18 @@ ALTERNATIVE_LINK_NAME[su] = "${base_bindir}/su"
 
 PACKAGE_WRITE_DEPS += "shadow-native"
 pkg_postinst:${PN}:class-target () {
+	install -d $D${sysconfdir}/${PW_SUBDIR}
 	if [ "x$D" != "x" ]; then
 	  rootarg="--root $D"
 	else
 	  rootarg=""
 	fi
-
+	if [ -n "${PW_SUBDIR}" ]; then
+	    ln -fs ${PW_SUBDIR}/subuid $D${sysconfdir}/subuid
+	    ln -fs ${PW_SUBDIR}/subgid $D${sysconfdir}/subgid
+	    ln -fs ${PW_SUBDIR}/shadow $D${sysconfdir}/shadow
+	    ln -fs ${PW_SUBDIR}/gshadow $D${sysconfdir}/gshadow
+	fi
 	pwconv $rootarg || exit 1
 	grpconv $rootarg || exit 1
 }
-- 
2.41.0

RE: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Joakim Tjernlund via lists.openembedded.org
> Sent: den 24 november 2023 15:11
> To: openembedded-core@lists.openembedded.org
> Cc: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> Subject: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR
> 
> Add support for creating passwd files in a /etc subdir
> Set PW_SUBIR to pwdb to get passwd  files in /etc/pwdb
> 
> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> ---
>  .../base-passwd/base-passwd_3.5.29.bb         | 24 ++++++++++++-------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> index ef7792ae49..e453be0763 100644
> --- a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> +++ b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> @@ -20,6 +20,9 @@ SRC_URI = "https://launchpad.net/debian/+archive/primary/+files/${BPN}_${PV}.tar
>  SRC_URI[md5sum] = "6beccac48083fe8ae5048acd062e5421"
>  SRC_URI[sha256sum] = "f0b66388b2c8e49c15692439d2bee63bcdd4bbbf7a782c7f64accc55986b6a36"
> 
> +#Set PW_SUBDIR to pwdb to get passwd  files in /etc/pwdb
> +PW_SUBDIR ?= ""
> +

Rather than defining a subdirectory, I would recommend defining the full 
path, e.g.:

PW_DIR ?= "${sysconfdir}"

This avoids generating a lot of "//" in the middle of paths for the majority 
of us who do not use a subdirectory for the password files.

>  # the package is taken from launchpad; that source is static and goes stale
>  # so we check the latest upstream from a directory that does get updated
>  UPSTREAM_CHECK_URI = "${DEBIAN_MIRROR}/main/b/base-passwd/"
> @@ -50,10 +53,11 @@ basepasswd_sysroot_postinst() {
>  #!/bin/sh
> 
>  # Install passwd.master and group.master to sysconfdir
> -install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}
> +install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}
>  for i in passwd group; do
>  	install -p -m 644 ${STAGING_DIR_TARGET}${datadir}/base-passwd/\$i.master \
> -		${STAGING_DIR_TARGET}${sysconfdir}/\$i
> +		${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}/\$i
> +	[ -n "${PW_SUBDIR}" ] && ln -fs ${PW_SUBDIR}/\$i ${STAGING_DIR_TARGET}${sysconfdir}/\$i

I generally recommended to use `[ ! ... ] || ...` instead of `[ ... ] && ...`:

	[ -z "${PW_SUBDIR}" ] || ln -fs ${PW_SUBDIR}/\$i ${STAGING_DIR_TARGET}${sysconfdir}/\$i

Or, assuming my recommendation above is followed: 

	[ "${PW_DIR}" = "${sysconfdir}" ] ||
		ln -fsr ${STAGING_DIR_TARGET}${PW_DIR}/\$i ${STAGING_DIR_TARGET}${sysconfdir}/\$i

The reason is that the return status ($?) of `[ ... ] && ...` is 1 if the 
test fails, while it is 0 for `[ ! ... ] || ...` when the test succeeds.

>  done
> 
>  # Run any useradd postinsts
> @@ -89,15 +93,19 @@ python populate_packages:prepend() {
>      f.close()
> 
>      preinst = """#!/bin/sh
> -mkdir -p $D${sysconfdir}
> -if [ ! -e $D${sysconfdir}/passwd ]; then
> -\tcat << 'EOF' > $D${sysconfdir}/passwd
> +mkdir -p $D${sysconfdir}/${PW_SUBDIR}
> +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/passwd ]; then
> +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/passwd
>  """ + passwd + """EOF
>  fi
> -if [ ! -e $D${sysconfdir}/group ]; then
> -\tcat << 'EOF' > $D${sysconfdir}/group
> +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/group ]; then
> +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/group
>  """ + group + """EOF
>  fi
> +if [ -n "${PW_SUBDIR}" ]; then
> +ln -fs ${PW_SUBDIR}/passwd $D${sysconfdir}/passwd
> +ln -fs ${PW_SUBDIR}/group $D${sysconfdir}/group

Use \t to indent the above two lines like the code before.

> +fi
>  """
>      d.setVar(d.expand('pkg_preinst:${PN}'), preinst)
>  }
> @@ -114,5 +122,5 @@ pkg_postinst:${PN}-update () {
>  if [ -n "$D" ]; then
>  	exit 0
>  fi
> -${sbindir}/update-passwd
> +${sbindir}/update-passwd -P /etc/${PW_SUBDIR}/passwd -S /etc/${PW_SUBDIR}/shadow -G /etc/${PW_SUBDIR}/group

Replace /etc with ${sysconfdir}.

>  }
> --
> 2.41.0

//Peter

RE: [OE-core] [kirkstone][PATCH 3/4] pseudo: Add PW_SUBDIR

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Joakim Tjernlund via lists.openembedded.org
> Sent: den 24 november 2023 15:11
> To: openembedded-core@lists.openembedded.org
> Cc: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> Subject: [OE-core] [kirkstone][PATCH 3/4] pseudo: Add PW_SUBDIR
> 
> Add support for creating passwd files in a /etc subdir
> Set PW_SUBIR to pwdb to get passwd files in /etc/pwdb
> 
> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> ---
>  meta/recipes-devtools/pseudo/pseudo.inc | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/meta/recipes-devtools/pseudo/pseudo.inc b/meta/recipes-devtools/pseudo/pseudo.inc
> index 7e09b6d58c..7ba2e2261c 100644
> --- a/meta/recipes-devtools/pseudo/pseudo.inc
> +++ b/meta/recipes-devtools/pseudo/pseudo.inc
> @@ -10,6 +10,9 @@ SECTION = "base"
>  LICENSE = "LGPL-2.1-only"
>  DEPENDS = "sqlite3 attr"
> 
> +#Set PW_SUBDIR to pwdb to get passwd files in /etc/pwdb
> +PW_SUBDIR ?= ""
> +
>  FILES:${PN} = "${prefix}/lib/pseudo/lib*/libpseudo.so ${bindir}/* ${localstatedir}/pseudo ${prefix}/var/pseudo"
>  INSANE_SKIP:${PN} += "libdir"
>  INSANE_SKIP:${PN}-dbg += "libdir"
> @@ -131,10 +134,12 @@ do_install () {
> 
>  do_install:append:class-native () {
>  	chrpath ${D}${bindir}/pseudo -r `chrpath ${D}${bindir}/pseudo | cut -d = -f 2 | sed s/XORIGIN/\\$ORIGIN/`
> -	install -d ${D}${sysconfdir}
> +	install -d ${D}${sysconfdir}/${PW_SUBDIR}
>  	# The fallback files should never be modified
> -	install -m 444 ${WORKDIR}/fallback-passwd ${D}${sysconfdir}/passwd
> -	install -m 444 ${WORKDIR}/fallback-group ${D}${sysconfdir}/group
> +	install -m 444 ${WORKDIR}/fallback-passwd ${D}${sysconfdir}/${PW_SUBDIR}/passwd
> +        [ -n "${PW_SUBDIR}" ] && ln -fs ${PW_SUBDIR}/passwd ${D}${sysconfdir}/passwd

Inconsistent indentation (spaces instead of tab). And here too I 
recommend to use || instead of &&.

> +	install -m 444 ${WORKDIR}/fallback-group ${D}${sysconfdir}/${PW_SUBDIR}/group
> +        [ -n "${PW_SUBDIR}" ] && ln -fs ${PW_SUBDIR}/group ${D}${sysconfdir}/group

Inconsistent indentation (spaces instead of tab).

> 
>  	# Two native/nativesdk entries below are the same
>  	# If necessary install for the alternative machine arch.  This is only
> --
> 2.41.0

//Peter

RE: [OE-core] [kirkstone][PATCH 4/4] shadow: Add PW_SUBDIR

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Joakim Tjernlund via lists.openembedded.org
> Sent: den 24 november 2023 15:11
> To: openembedded-core@lists.openembedded.org
> Cc: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> Subject: [OE-core] [kirkstone][PATCH 4/4] shadow: Add PW_SUBDIR
> 
> Add support for creating passwd files in a /etc subdir
> Set PW_SUBIR to pwdb to get passwd files in /etc/pwdb
> 
> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> ---
>  .../0001-Define-SUBUID_FILE-SUBGID_FILE.patch | 92 +++++++++++++++++++
>  meta/recipes-extended/shadow/shadow.inc       | 30 +++++-
>  2 files changed, 118 insertions(+), 4 deletions(-)
>  create mode 100644 meta/recipes-extended/shadow/files/0001-Define-SUBUID_FILE-SUBGID_FILE.patch
> 
> diff --git a/meta/recipes-extended/shadow/files/0001-Define-SUBUID_FILE-SUBGID_FILE.patch b/meta/recipes-extended/shadow/files/0001-Define-SUBUID_FILE-SUBGID_FILE.patch
> new file mode 100644
> index 0000000000..9f85159e97
> --- /dev/null
> +++ b/meta/recipes-extended/shadow/files/0001-Define-SUBUID_FILE-SUBGID_FILE.patch
> @@ -0,0 +1,92 @@
> +From f605fb315faef7ddcad70d638f3b3aa16ea98fc0 Mon Sep 17 00:00:00 2001
> +From: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> +Date: Thu, 2 Nov 2023 00:27:10 +0100
> +Subject: [PATCH] Define SUBUID_FILE/SUBGID_FILE
> +
> +Upstream-Status: Backport, https://github.com/shadow-maint/shadow/commit/ee3a79c6952f8ca649c286c7f76639d9d1dedaad

Follow the recommended syntax:

Upstream-Status: Backport [https://github.com/shadow-maint/shadow/commit/ee3a79c6952f8ca649c286c7f76639d9d1dedaad]

> +
> +These where hard coded, make them definable like SHADOW_FILE
> +
> +Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> +---

[cut]

> diff --git a/meta/recipes-extended/shadow/shadow.inc b/meta/recipes-extended/shadow/shadow.inc
> index 3c1dd2f98e..bcb9b09a49 100644
> --- a/meta/recipes-extended/shadow/shadow.inc
> +++ b/meta/recipes-extended/shadow/shadow.inc
> @@ -18,6 +18,7 @@ SRC_URI = "https://github.com/shadow-maint/shadow/releases/download/v${PV}/${BP}
>             file://useradd \
>             file://CVE-2023-29383.patch \
>             file://0001-Overhaul-valid_field.patch \
> +           file://0001-Define-SUBUID_FILE-SUBGID_FILE.patch \
>             "
> 
>  SRC_URI:append:class-target = " \
> @@ -46,6 +47,21 @@ PAM_SRC_URI = "file://pam.d/chfn \
>                 file://pam.d/passwd \
>                 file://pam.d/su"
> 
> +#Set PW_SUBDIR to pwdb to get passwd files in /etc/pwdb
> +PW_SUBDIR ?= ""
> +PWPRE = "/etc/${PW_SUBDIR}"

Use ${sysconfdir} instead of /etc.

> +CFLAGS:append = ' -DPASSWD_FILE=\\"${PWPRE}/passwd\\"'
> +CFLAGS:append = ' -DSHADOW_FILE=\\"${PWPRE}/shadow\\"'
> +CFLAGS:append = ' -DGROUP_FILE=\\"${PWPRE}/group\\"'
> +CFLAGS:append = ' -DSGROUP_FILE=\\"${PWPRE}/gshadow\\"'
> +CFLAGS:append = ' -DSUBUID_FILE=\\"${PWPRE}/subuid\\"'
> +CFLAGS:append = ' -DSUBGID_FILE=\\"${PWPRE}/subgid\\"'

Use `CFLAGS += ...` instead of `CFLAGS:append = ...`:

CFLAGS += ' \
    -DPASSWD_FILE=\\"${PWPRE}/passwd\\" \
    -DSHADOW_FILE=\\"${PWPRE}/shadow\\" \
    -DGROUP_FILE=\\"${PWPRE}/group\\" \
    -DSGROUP_FILE=\\"${PWPRE}/gshadow\\" \
    -DSUBUID_FILE=\\"${PWPRE}/subuid\\" \
    -DSUBGID_FILE=\\"${PWPRE}/subgid\\" \
'

> +
> +#shadow has it own impl. that uses whatever dir passwd files are in
> +do_configure:prepend () {
> +    sed -i -e 's/lckpwdf//' ${S}/configure.ac

Use tabs to indent shell code. However, this should really be a patch...

> +}
> +
>  inherit autotools gettext
> 
>  export CONFIG_SHELL="/bin/sh"
> @@ -157,9 +173,9 @@ do_install:append() {
>  	# usermod requires the subuid/subgid files to be in place before being
>  	# able to use the -v/-V flags otherwise it fails:
>  	# usermod: /etc/subuid does not exist, you cannot use the flags -v or -V
> -	install -d ${D}${sysconfdir}
> -	touch ${D}${sysconfdir}/subuid
> -	touch ${D}${sysconfdir}/subgid
> +	install -d ${D}${sysconfdir}/${PW_SUBDIR}
> +	touch ${D}${sysconfdir}/${PW_SUBDIR}/subuid
> +	touch ${D}${sysconfdir}/${PW_SUBDIR}/subgid
>  }
> 
>  PACKAGES =+ "${PN}-base"
> @@ -193,12 +209,18 @@ ALTERNATIVE_LINK_NAME[su] = "${base_bindir}/su"
> 
>  PACKAGE_WRITE_DEPS += "shadow-native"
>  pkg_postinst:${PN}:class-target () {
> +	install -d $D${sysconfdir}/${PW_SUBDIR}
>  	if [ "x$D" != "x" ]; then
>  	  rootarg="--root $D"
>  	else
>  	  rootarg=""
>  	fi
> -
> +	if [ -n "${PW_SUBDIR}" ]; then
> +	    ln -fs ${PW_SUBDIR}/subuid $D${sysconfdir}/subuid
> +	    ln -fs ${PW_SUBDIR}/subgid $D${sysconfdir}/subgid
> +	    ln -fs ${PW_SUBDIR}/shadow $D${sysconfdir}/shadow
> +	    ln -fs ${PW_SUBDIR}/gshadow $D${sysconfdir}/gshadow

Inconsistent indentation (mix of tabs and spaces). This also applies to 
the old code above, but with a different number of spaces...

> +	fi
>  	pwconv $rootarg || exit 1
>  	grpconv $rootarg || exit 1
>  }
> --
> 2.41.0

//Peter

Re: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR

[Joakim Tjernlund]

Hi Peter :)

All good comments, will fix accordingly. Not sure how PW_DIR ?= "${sysconfdir}" will work though.

How do you envision one should set PW_DIR in distro .conf or layer.conf?
Just PW_DIR = "/etc/pwdb" or PW_DIR = "${sysconfdir}/pwdb" ?

 Jocke

On Sun, 2023-11-26 at 21:21 +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Joakim Tjernlund via lists.openembedded.org
> > Sent: den 24 november 2023 15:11
> > To: openembedded-core@lists.openembedded.org
> > Cc: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > Subject: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR
> >
> > Add support for creating passwd files in a /etc subdir
> > Set PW_SUBIR to pwdb to get passwd  files in /etc/pwdb
> >
> > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > ---
> >  .../base-passwd/base-passwd_3.5.29.bb         | 24 ++++++++++++-------
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > index ef7792ae49..e453be0763 100644
> > --- a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > +++ b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > @@ -20,6 +20,9 @@ SRC_URI = "https://launchpad.net/debian/+archive/primary/+files/${BPN}_${PV}.tar
> >  SRC_URI[md5sum] = "6beccac48083fe8ae5048acd062e5421"
> >  SRC_URI[sha256sum] = "f0b66388b2c8e49c15692439d2bee63bcdd4bbbf7a782c7f64accc55986b6a36"
> >
> > +#Set PW_SUBDIR to pwdb to get passwd  files in /etc/pwdb
> > +PW_SUBDIR ?= ""
> > +
>
> Rather than defining a subdirectory, I would recommend defining the full
> path, e.g.:
>
> PW_DIR ?= "${sysconfdir}"
>
> This avoids generating a lot of "//" in the middle of paths for the majority
> of us who do not use a subdirectory for the password files.
>
> >  # the package is taken from launchpad; that source is static and goes stale
> >  # so we check the latest upstream from a directory that does get updated
> >  UPSTREAM_CHECK_URI = "${DEBIAN_MIRROR}/main/b/base-passwd/"
> > @@ -50,10 +53,11 @@ basepasswd_sysroot_postinst() {
> >  #!/bin/sh
> >
> >  # Install passwd.master and group.master to sysconfdir
> > -install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}
> > +install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}
> >  for i in passwd group; do
> >     install -p -m 644 ${STAGING_DIR_TARGET}${datadir}/base-passwd/\$i.master \
> > -           ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > +           ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}/\$i
> > +   [ -n "${PW_SUBDIR}" ] && ln -fs ${PW_SUBDIR}/\$i ${STAGING_DIR_TARGET}${sysconfdir}/\$i
>
> I generally recommended to use `[ ! ... ] || ...` instead of `[ ... ] && ...`:
>
>       [ -z "${PW_SUBDIR}" ] || ln -fs ${PW_SUBDIR}/\$i ${STAGING_DIR_TARGET}${sysconfdir}/\$i
>
> Or, assuming my recommendation above is followed:
>
>       [ "${PW_DIR}" = "${sysconfdir}" ] ||
>               ln -fsr ${STAGING_DIR_TARGET}${PW_DIR}/\$i ${STAGING_DIR_TARGET}${sysconfdir}/\$i
>
> The reason is that the return status ($?) of `[ ... ] && ...` is 1 if the
> test fails, while it is 0 for `[ ! ... ] || ...` when the test succeeds.
>
> >  done
> >
> >  # Run any useradd postinsts
> > @@ -89,15 +93,19 @@ python populate_packages:prepend() {
> >      f.close()
> >
> >      preinst = """#!/bin/sh
> > -mkdir -p $D${sysconfdir}
> > -if [ ! -e $D${sysconfdir}/passwd ]; then
> > -\tcat << 'EOF' > $D${sysconfdir}/passwd
> > +mkdir -p $D${sysconfdir}/${PW_SUBDIR}
> > +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/passwd ]; then
> > +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/passwd
> >  """ + passwd + """EOF
> >  fi
> > -if [ ! -e $D${sysconfdir}/group ]; then
> > -\tcat << 'EOF' > $D${sysconfdir}/group
> > +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/group ]; then
> > +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/group
> >  """ + group + """EOF
> >  fi
> > +if [ -n "${PW_SUBDIR}" ]; then
> > +ln -fs ${PW_SUBDIR}/passwd $D${sysconfdir}/passwd
> > +ln -fs ${PW_SUBDIR}/group $D${sysconfdir}/group
>
> Use \t to indent the above two lines like the code before.
>
> > +fi
> >  """
> >      d.setVar(d.expand('pkg_preinst:${PN}'), preinst)
> >  }
> > @@ -114,5 +122,5 @@ pkg_postinst:${PN}-update () {
> >  if [ -n "$D" ]; then
> >     exit 0
> >  fi
> > -${sbindir}/update-passwd
> > +${sbindir}/update-passwd -P /etc/${PW_SUBDIR}/passwd -S /etc/${PW_SUBDIR}/shadow -G /etc/${PW_SUBDIR}/group
>
> Replace /etc with ${sysconfdir}.
>
> >  }
> > --
> > 2.41.0
>
> //Peter
>

Re: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR

[Steve Sakoman]

On Wed, Nov 29, 2023 at 1:11 AM Joakim Tjernlund via
lists.openembedded.org
<Joakim.Tjernlund=infinera.com@lists.openembedded.org> wrote:
>
> Hi Peter :)
>
> All good comments, will fix accordingly. Not sure how PW_DIR ?= "${sysconfdir}" will work though.

One additional comment: a change like this should be submitted for the
master branch, it can't go into a stable branch first.

Steve

> How do you envision one should set PW_DIR in distro .conf or layer.conf?
> Just PW_DIR = "/etc/pwdb" or PW_DIR = "${sysconfdir}/pwdb" ?
>
>  Jocke
>
> On Sun, 2023-11-26 at 21:21 +0000, Peter Kjellerstedt wrote:
> > > -----Original Message-----
> > > From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Joakim Tjernlund via lists.openembedded.org
> > > Sent: den 24 november 2023 15:11
> > > To: openembedded-core@lists.openembedded.org
> > > Cc: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > Subject: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR
> > >
> > > Add support for creating passwd files in a /etc subdir
> > > Set PW_SUBIR to pwdb to get passwd  files in /etc/pwdb
> > >
> > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > ---
> > >  .../base-passwd/base-passwd_3.5.29.bb         | 24 ++++++++++++-------
> > >  1 file changed, 16 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > index ef7792ae49..e453be0763 100644
> > > --- a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > +++ b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > @@ -20,6 +20,9 @@ SRC_URI = "https://launchpad.net/debian/+archive/primary/+files/${BPN}_${PV}.tar
> > >  SRC_URI[md5sum] = "6beccac48083fe8ae5048acd062e5421"
> > >  SRC_URI[sha256sum] = "f0b66388b2c8e49c15692439d2bee63bcdd4bbbf7a782c7f64accc55986b6a36"
> > >
> > > +#Set PW_SUBDIR to pwdb to get passwd  files in /etc/pwdb
> > > +PW_SUBDIR ?= ""
> > > +
> >
> > Rather than defining a subdirectory, I would recommend defining the full
> > path, e.g.:
> >
> > PW_DIR ?= "${sysconfdir}"
> >
> > This avoids generating a lot of "//" in the middle of paths for the majority
> > of us who do not use a subdirectory for the password files.
> >
> > >  # the package is taken from launchpad; that source is static and goes stale
> > >  # so we check the latest upstream from a directory that does get updated
> > >  UPSTREAM_CHECK_URI = "${DEBIAN_MIRROR}/main/b/base-passwd/"
> > > @@ -50,10 +53,11 @@ basepasswd_sysroot_postinst() {
> > >  #!/bin/sh
> > >
> > >  # Install passwd.master and group.master to sysconfdir
> > > -install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}
> > > +install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}
> > >  for i in passwd group; do
> > >     install -p -m 644 ${STAGING_DIR_TARGET}${datadir}/base-passwd/\$i.master \
> > > -           ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > > +           ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}/\$i
> > > +   [ -n "${PW_SUBDIR}" ] && ln -fs ${PW_SUBDIR}/\$i ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> >
> > I generally recommended to use `[ ! ... ] || ...` instead of `[ ... ] && ...`:
> >
> >       [ -z "${PW_SUBDIR}" ] || ln -fs ${PW_SUBDIR}/\$i ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> >
> > Or, assuming my recommendation above is followed:
> >
> >       [ "${PW_DIR}" = "${sysconfdir}" ] ||
> >               ln -fsr ${STAGING_DIR_TARGET}${PW_DIR}/\$i ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> >
> > The reason is that the return status ($?) of `[ ... ] && ...` is 1 if the
> > test fails, while it is 0 for `[ ! ... ] || ...` when the test succeeds.
> >
> > >  done
> > >
> > >  # Run any useradd postinsts
> > > @@ -89,15 +93,19 @@ python populate_packages:prepend() {
> > >      f.close()
> > >
> > >      preinst = """#!/bin/sh
> > > -mkdir -p $D${sysconfdir}
> > > -if [ ! -e $D${sysconfdir}/passwd ]; then
> > > -\tcat << 'EOF' > $D${sysconfdir}/passwd
> > > +mkdir -p $D${sysconfdir}/${PW_SUBDIR}
> > > +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/passwd ]; then
> > > +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/passwd
> > >  """ + passwd + """EOF
> > >  fi
> > > -if [ ! -e $D${sysconfdir}/group ]; then
> > > -\tcat << 'EOF' > $D${sysconfdir}/group
> > > +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/group ]; then
> > > +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/group
> > >  """ + group + """EOF
> > >  fi
> > > +if [ -n "${PW_SUBDIR}" ]; then
> > > +ln -fs ${PW_SUBDIR}/passwd $D${sysconfdir}/passwd
> > > +ln -fs ${PW_SUBDIR}/group $D${sysconfdir}/group
> >
> > Use \t to indent the above two lines like the code before.
> >
> > > +fi
> > >  """
> > >      d.setVar(d.expand('pkg_preinst:${PN}'), preinst)
> > >  }
> > > @@ -114,5 +122,5 @@ pkg_postinst:${PN}-update () {
> > >  if [ -n "$D" ]; then
> > >     exit 0
> > >  fi
> > > -${sbindir}/update-passwd
> > > +${sbindir}/update-passwd -P /etc/${PW_SUBDIR}/passwd -S /etc/${PW_SUBDIR}/shadow -G /etc/${PW_SUBDIR}/group
> >
> > Replace /etc with ${sysconfdir}.
> >
> > >  }
> > > --
> > > 2.41.0
> >
> > //Peter
> >
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#191437): https://lists.openembedded.org/g/openembedded-core/message/191437
> Mute This Topic: https://lists.openembedded.org/mt/102780967/3620601
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [steve@sakoman.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Re: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR

[Richard Purdie]

On Wed, 2023-11-29 at 07:17 -1000, Steve Sakoman wrote:
> On Wed, Nov 29, 2023 at 1:11 AM Joakim Tjernlund via
> lists.openembedded.org
> <Joakim.Tjernlund=infinera.com@lists.openembedded.org> wrote:
> > 
> > Hi Peter :)
> > 
> > All good comments, will fix accordingly. Not sure how PW_DIR ?= "${sysconfdir}" will work though.
> 
> One additional comment: a change like this should be submitted for the
> master branch, it can't go into a stable branch first.

Being realistic, this is a feature not a bugfix so it isn't really
appropriate for kirkstone in general.

I do agree with Ross that the approach isn't really what we'd want in
master either since we'd have to keep adding variables for each file
people wanted to change. As such I'm unlikely to accept these patches
for master as there are other ways to handle this.

Cheers,

Richard

Re: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR

[Joakim Tjernlund]

On Wed, 2023-11-29 at 21:37 +0000, Richard Purdie wrote:
> On Wed, 2023-11-29 at 07:17 -1000, Steve Sakoman wrote:
> > On Wed, Nov 29, 2023 at 1:11 AM Joakim Tjernlund via
> > lists.openembedded.org
> > <Joakim.Tjernlund=infinera.com@lists.openembedded.org> wrote:
> > > 
> > > Hi Peter :)
> > > 
> > > All good comments, will fix accordingly. Not sure how PW_DIR ?= "${sysconfdir}" will work though.
> > 
> > One additional comment: a change like this should be submitted for the
> > master branch, it can't go into a stable branch first.
> 
> Being realistic, this is a feature not a bugfix so it isn't really
> appropriate for kirkstone in general.

That is OK, I can rebase against master.
> 
> I do agree with Ross that the approach isn't really what we'd want in
> master either since we'd have to keep adding variables for each file
> people wanted to change. As such I'm unlikely to accept these patches
> for master as there are other ways to handle this.

What other ways? I have explored several and this was the only thing that worked.
I don't consider overlayfs over all of /etc an alternative.

 Cheers 
          Jocke

RE: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR

[Peter Kjellerstedt]

Since I've seen Richard's reply and his reluctance to merge this, 
this is mostly technical.

I would use either PW_DIR = "${sysconfdir}/pwdb" or PW_DIR:append = "/pwdb". 
Using "/etc" (and other hardcoded paths) should be avoided wherever 
possible.

//Peter

> -----Original Message-----
> From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> Sent: den 29 november 2023 12:11
> To: openembedded-core@lists.openembedded.org; Peter Kjellerstedt
> <peter.kjellerstedt@axis.com>
> Subject: Re: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR
> 
> Hi Peter :)
> 
> All good comments, will fix accordingly. Not sure how PW_DIR ?=
> "${sysconfdir}" will work though.
> 
> How do you envision one should set PW_DIR in distro .conf or layer.conf?
> Just PW_DIR = "/etc/pwdb" or PW_DIR = "${sysconfdir}/pwdb" ?
> 
>  Jocke
> 
> On Sun, 2023-11-26 at 21:21 +0000, Peter Kjellerstedt wrote:
> > > -----Original Message-----
> > > From: openembedded-core@lists.openembedded.org <openembedded-
> core@lists.openembedded.org> On Behalf Of Joakim Tjernlund via
> lists.openembedded.org
> > > Sent: den 24 november 2023 15:11
> > > To: openembedded-core@lists.openembedded.org
> > > Cc: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > Subject: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR
> > >
> > > Add support for creating passwd files in a /etc subdir
> > > Set PW_SUBIR to pwdb to get passwd  files in /etc/pwdb
> > >
> > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > ---
> > >  .../base-passwd/base-passwd_3.5.29.bb         | 24 ++++++++++++------
> -
> > >  1 file changed, 16 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > index ef7792ae49..e453be0763 100644
> > > --- a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > +++ b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > @@ -20,6 +20,9 @@ SRC_URI =
> "https://launchpad.net/debian/+archive/primary/+files/${BPN}_${PV}.tar
> > >  SRC_URI[md5sum] = "6beccac48083fe8ae5048acd062e5421"
> > >  SRC_URI[sha256sum] =
> "f0b66388b2c8e49c15692439d2bee63bcdd4bbbf7a782c7f64accc55986b6a36"
> > >
> > > +#Set PW_SUBDIR to pwdb to get passwd  files in /etc/pwdb
> > > +PW_SUBDIR ?= ""
> > > +
> >
> > Rather than defining a subdirectory, I would recommend defining the full
> > path, e.g.:
> >
> > PW_DIR ?= "${sysconfdir}"
> >
> > This avoids generating a lot of "//" in the middle of paths for the
> majority
> > of us who do not use a subdirectory for the password files.
> >
> > >  # the package is taken from launchpad; that source is static and goes
> stale
> > >  # so we check the latest upstream from a directory that does get
> updated
> > >  UPSTREAM_CHECK_URI = "${DEBIAN_MIRROR}/main/b/base-passwd/"
> > > @@ -50,10 +53,11 @@ basepasswd_sysroot_postinst() {
> > >  #!/bin/sh
> > >
> > >  # Install passwd.master and group.master to sysconfdir
> > > -install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}
> > > +install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}
> > >  for i in passwd group; do
> > >     install -p -m 644 ${STAGING_DIR_TARGET}${datadir}/base-
> passwd/\$i.master \
> > > -           ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > > +           ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}/\$i
> > > +   [ -n "${PW_SUBDIR}" ] && ln -fs ${PW_SUBDIR}/\$i
> ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> >
> > I generally recommended to use `[ ! ... ] || ...` instead of `[ ... ] &&
> ...`:
> >
> >       [ -z "${PW_SUBDIR}" ] || ln -fs ${PW_SUBDIR}/\$i
> ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> >
> > Or, assuming my recommendation above is followed:
> >
> >       [ "${PW_DIR}" = "${sysconfdir}" ] ||
> >               ln -fsr ${STAGING_DIR_TARGET}${PW_DIR}/\$i
> ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> >
> > The reason is that the return status ($?) of `[ ... ] && ...` is 1 if
> the
> > test fails, while it is 0 for `[ ! ... ] || ...` when the test succeeds.
> >
> > >  done
> > >
> > >  # Run any useradd postinsts
> > > @@ -89,15 +93,19 @@ python populate_packages:prepend() {
> > >      f.close()
> > >
> > >      preinst = """#!/bin/sh
> > > -mkdir -p $D${sysconfdir}
> > > -if [ ! -e $D${sysconfdir}/passwd ]; then
> > > -\tcat << 'EOF' > $D${sysconfdir}/passwd
> > > +mkdir -p $D${sysconfdir}/${PW_SUBDIR}
> > > +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/passwd ]; then
> > > +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/passwd
> > >  """ + passwd + """EOF
> > >  fi
> > > -if [ ! -e $D${sysconfdir}/group ]; then
> > > -\tcat << 'EOF' > $D${sysconfdir}/group
> > > +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/group ]; then
> > > +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/group
> > >  """ + group + """EOF
> > >  fi
> > > +if [ -n "${PW_SUBDIR}" ]; then
> > > +ln -fs ${PW_SUBDIR}/passwd $D${sysconfdir}/passwd
> > > +ln -fs ${PW_SUBDIR}/group $D${sysconfdir}/group
> >
> > Use \t to indent the above two lines like the code before.
> >
> > > +fi
> > >  """
> > >      d.setVar(d.expand('pkg_preinst:${PN}'), preinst)
> > >  }
> > > @@ -114,5 +122,5 @@ pkg_postinst:${PN}-update () {
> > >  if [ -n "$D" ]; then
> > >     exit 0
> > >  fi
> > > -${sbindir}/update-passwd
> > > +${sbindir}/update-passwd -P /etc/${PW_SUBDIR}/passwd -S
> /etc/${PW_SUBDIR}/shadow -G /etc/${PW_SUBDIR}/group
> >
> > Replace /etc with ${sysconfdir}.
> >
> > >  }
> > > --
> > > 2.41.0
> >
> > //Peter
> >

Re: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR

[Joakim Tjernlund]

On Wed, 2023-12-06 at 20:21 +0000, Peter Kjellerstedt wrote:
> Since I've seen Richard's reply and his reluctance to merge this,
> this is mostly technical.

I know but I don't understand what "batter ways" to do this is, I have tested symlinks/--prefix/--root and bind mounting the passwd/shadow
files and none of then work. Using overlaysfs is an inferior solution to me that makes the whole /etc writeable, may break when RFS underneath is
upgraded and I am unsure how resilient overlayfs is in case  of power failure. What else is there ?

>
> I would use either PW_DIR = "${sysconfdir}/pwdb" or PW_DIR:append = "/pwdb".
> Using "/etc" (and other hardcoded paths) should be avoided wherever
> possible.

Thanks, will tru there out.

 Jocke

>
> //Peter
>
> > -----Original Message-----
> > From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> > Sent: den 29 november 2023 12:11
> > To: openembedded-core@lists.openembedded.org; Peter Kjellerstedt
> > <peter.kjellerstedt@axis.com>
> > Subject: Re: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR
> >
> > Hi Peter :)
> >
> > All good comments, will fix accordingly. Not sure how PW_DIR ?=
> > "${sysconfdir}" will work though.
> >
> > How do you envision one should set PW_DIR in distro .conf or layer.conf?
> > Just PW_DIR = "/etc/pwdb" or PW_DIR = "${sysconfdir}/pwdb" ?
> >
> >  Jocke
> >
> > On Sun, 2023-11-26 at 21:21 +0000, Peter Kjellerstedt wrote:
> > > > -----Original Message-----
> > > > From: openembedded-core@lists.openembedded.org <openembedded-
> > core@lists.openembedded.org> On Behalf Of Joakim Tjernlund via
> > lists.openembedded.org
> > > > Sent: den 24 november 2023 15:11
> > > > To: openembedded-core@lists.openembedded.org
> > > > Cc: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > > Subject: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR
> > > >
> > > > Add support for creating passwd files in a /etc subdir
> > > > Set PW_SUBIR to pwdb to get passwd  files in /etc/pwdb
> > > >
> > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > > ---
> > > >  .../base-passwd/base-passwd_3.5.29.bb         | 24 ++++++++++++------
> > -
> > > >  1 file changed, 16 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > > index ef7792ae49..e453be0763 100644
> > > > --- a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > > +++ b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > > @@ -20,6 +20,9 @@ SRC_URI =
> > "https://launchpad.net/debian/+archive/primary/+files/${BPN}_${PV}.tar
> > > >  SRC_URI[md5sum] = "6beccac48083fe8ae5048acd062e5421"
> > > >  SRC_URI[sha256sum] =
> > "f0b66388b2c8e49c15692439d2bee63bcdd4bbbf7a782c7f64accc55986b6a36"
> > > >
> > > > +#Set PW_SUBDIR to pwdb to get passwd  files in /etc/pwdb
> > > > +PW_SUBDIR ?= ""
> > > > +
> > >
> > > Rather than defining a subdirectory, I would recommend defining the full
> > > path, e.g.:
> > >
> > > PW_DIR ?= "${sysconfdir}"
> > >
> > > This avoids generating a lot of "//" in the middle of paths for the
> > majority
> > > of us who do not use a subdirectory for the password files.
> > >
> > > >  # the package is taken from launchpad; that source is static and goes
> > stale
> > > >  # so we check the latest upstream from a directory that does get
> > updated
> > > >  UPSTREAM_CHECK_URI = "${DEBIAN_MIRROR}/main/b/base-passwd/"
> > > > @@ -50,10 +53,11 @@ basepasswd_sysroot_postinst() {
> > > >  #!/bin/sh
> > > >
> > > >  # Install passwd.master and group.master to sysconfdir
> > > > -install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}
> > > > +install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}
> > > >  for i in passwd group; do
> > > >     install -p -m 644 ${STAGING_DIR_TARGET}${datadir}/base-
> > passwd/\$i.master \
> > > > -           ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > > > +           ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}/\$i
> > > > +   [ -n "${PW_SUBDIR}" ] && ln -fs ${PW_SUBDIR}/\$i
> > ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > >
> > > I generally recommended to use `[ ! ... ] || ...` instead of `[ ... ] &&
> > ...`:
> > >
> > >       [ -z "${PW_SUBDIR}" ] || ln -fs ${PW_SUBDIR}/\$i
> > ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > >
> > > Or, assuming my recommendation above is followed:
> > >
> > >       [ "${PW_DIR}" = "${sysconfdir}" ] ||
> > >               ln -fsr ${STAGING_DIR_TARGET}${PW_DIR}/\$i
> > ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > >
> > > The reason is that the return status ($?) of `[ ... ] && ...` is 1 if
> > the
> > > test fails, while it is 0 for `[ ! ... ] || ...` when the test succeeds.
> > >
> > > >  done
> > > >
> > > >  # Run any useradd postinsts
> > > > @@ -89,15 +93,19 @@ python populate_packages:prepend() {
> > > >      f.close()
> > > >
> > > >      preinst = """#!/bin/sh
> > > > -mkdir -p $D${sysconfdir}
> > > > -if [ ! -e $D${sysconfdir}/passwd ]; then
> > > > -\tcat << 'EOF' > $D${sysconfdir}/passwd
> > > > +mkdir -p $D${sysconfdir}/${PW_SUBDIR}
> > > > +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/passwd ]; then
> > > > +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/passwd
> > > >  """ + passwd + """EOF
> > > >  fi
> > > > -if [ ! -e $D${sysconfdir}/group ]; then
> > > > -\tcat << 'EOF' > $D${sysconfdir}/group
> > > > +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/group ]; then
> > > > +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/group
> > > >  """ + group + """EOF
> > > >  fi
> > > > +if [ -n "${PW_SUBDIR}" ]; then
> > > > +ln -fs ${PW_SUBDIR}/passwd $D${sysconfdir}/passwd
> > > > +ln -fs ${PW_SUBDIR}/group $D${sysconfdir}/group
> > >
> > > Use \t to indent the above two lines like the code before.
> > >
> > > > +fi
> > > >  """
> > > >      d.setVar(d.expand('pkg_preinst:${PN}'), preinst)
> > > >  }
> > > > @@ -114,5 +122,5 @@ pkg_postinst:${PN}-update () {
> > > >  if [ -n "$D" ]; then
> > > >     exit 0
> > > >  fi
> > > > -${sbindir}/update-passwd
> > > > +${sbindir}/update-passwd -P /etc/${PW_SUBDIR}/passwd -S
> > /etc/${PW_SUBDIR}/shadow -G /etc/${PW_SUBDIR}/group
> > >
> > > Replace /etc with ${sysconfdir}.
> > >
> > > >  }
> > > > --
> > > > 2.41.0
> > >
> > > //Peter
> > >
>

RE: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR

[Peter Kjellerstedt]

> -----Original Message-----
> From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> Sent: den 7 december 2023 09:48
> To: openembedded-core@lists.openembedded.org; Peter Kjellerstedt
> <peter.kjellerstedt@axis.com>
> Subject: Re: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR
> 
> On Wed, 2023-12-06 at 20:21 +0000, Peter Kjellerstedt wrote:
> > Since I've seen Richard's reply and his reluctance to merge this,
> > this is mostly technical.
> 
> I know but I don't understand what "batter ways" to do this is, I have
> tested symlinks/--prefix/--root and bind mounting the passwd/shadow
> files and none of then work. Using overlaysfs is an inferior solution to
> me that makes the whole /etc writeable, may break when RFS underneath is
> upgraded and I am unsure how resilient overlayfs is in case  of power
> failure. What else is there ?

For what it is worth, we use overlayfs on /etc in all of our products, 
and AFAIK have not had any problems with it. Our product upgrade solution 
is of course aware of the fact and takes care when upgrading to migrate 
all relevant changes in a controlled way.

//Peter

> >
> > I would use either PW_DIR = "${sysconfdir}/pwdb" or PW_DIR:append = "/pwdb".
> > Using "/etc" (and other hardcoded paths) should be avoided wherever
> > possible.
> 
> Thanks, will tru there out.
> 
>  Jocke
> 
> >
> > //Peter
> >
> > > -----Original Message-----
> > > From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> > > Sent: den 29 november 2023 12:11
> > > To: openembedded-core@lists.openembedded.org; Peter Kjellerstedt
> > > <peter.kjellerstedt@axis.com>
> > > Subject: Re: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add
> PW_SUBDIR
> > >
> > > Hi Peter :)
> > >
> > > All good comments, will fix accordingly. Not sure how PW_DIR ?=
> > > "${sysconfdir}" will work though.
> > >
> > > How do you envision one should set PW_DIR in distro .conf or
> layer.conf?
> > > Just PW_DIR = "/etc/pwdb" or PW_DIR = "${sysconfdir}/pwdb" ?
> > >
> > >  Jocke
> > >
> > > On Sun, 2023-11-26 at 21:21 +0000, Peter Kjellerstedt wrote:
> > > > > -----Original Message-----
> > > > > From: openembedded-core@lists.openembedded.org <openembedded-
> > > core@lists.openembedded.org> On Behalf Of Joakim Tjernlund via
> > > lists.openembedded.org
> > > > > Sent: den 24 november 2023 15:11
> > > > > To: openembedded-core@lists.openembedded.org
> > > > > Cc: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > > > Subject: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add
> PW_SUBDIR
> > > > >
> > > > > Add support for creating passwd files in a /etc subdir
> > > > > Set PW_SUBIR to pwdb to get passwd  files in /etc/pwdb
> > > > >
> > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > > > ---
> > > > >  .../base-passwd/base-passwd_3.5.29.bb         | 24 ++++++++++++--
> ----
> > > -
> > > > >  1 file changed, 16 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > > > index ef7792ae49..e453be0763 100644
> > > > > --- a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > > > +++ b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > > > @@ -20,6 +20,9 @@ SRC_URI =
> > > "https://launchpad.net/debian/+archive/primary/+files/${BPN}_${PV}.tar
> > > > >  SRC_URI[md5sum] = "6beccac48083fe8ae5048acd062e5421"
> > > > >  SRC_URI[sha256sum] =
> > > "f0b66388b2c8e49c15692439d2bee63bcdd4bbbf7a782c7f64accc55986b6a36"
> > > > >
> > > > > +#Set PW_SUBDIR to pwdb to get passwd  files in /etc/pwdb
> > > > > +PW_SUBDIR ?= ""
> > > > > +
> > > >
> > > > Rather than defining a subdirectory, I would recommend defining the
> full
> > > > path, e.g.:
> > > >
> > > > PW_DIR ?= "${sysconfdir}"
> > > >
> > > > This avoids generating a lot of "//" in the middle of paths for the
> > > majority
> > > > of us who do not use a subdirectory for the password files.
> > > >
> > > > >  # the package is taken from launchpad; that source is static and
> goes
> > > stale
> > > > >  # so we check the latest upstream from a directory that does get
> > > updated
> > > > >  UPSTREAM_CHECK_URI = "${DEBIAN_MIRROR}/main/b/base-passwd/"
> > > > > @@ -50,10 +53,11 @@ basepasswd_sysroot_postinst() {
> > > > >  #!/bin/sh
> > > > >
> > > > >  # Install passwd.master and group.master to sysconfdir
> > > > > -install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}
> > > > > +install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}
> > > > >  for i in passwd group; do
> > > > >     install -p -m 644 ${STAGING_DIR_TARGET}${datadir}/base-
> > > passwd/\$i.master \
> > > > > -           ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > > > > +           ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}/\$i
> > > > > +   [ -n "${PW_SUBDIR}" ] && ln -fs ${PW_SUBDIR}/\$i
> > > ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > > >
> > > > I generally recommended to use `[ ! ... ] || ...` instead of `[ ...
> ] &&
> > > ...`:
> > > >
> > > >       [ -z "${PW_SUBDIR}" ] || ln -fs ${PW_SUBDIR}/\$i
> > > ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > > >
> > > > Or, assuming my recommendation above is followed:
> > > >
> > > >       [ "${PW_DIR}" = "${sysconfdir}" ] ||
> > > >               ln -fsr ${STAGING_DIR_TARGET}${PW_DIR}/\$i
> > > ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > > >
> > > > The reason is that the return status ($?) of `[ ... ] && ...` is 1
> if
> > > the
> > > > test fails, while it is 0 for `[ ! ... ] || ...` when the test
> succeeds.
> > > >
> > > > >  done
> > > > >
> > > > >  # Run any useradd postinsts
> > > > > @@ -89,15 +93,19 @@ python populate_packages:prepend() {
> > > > >      f.close()
> > > > >
> > > > >      preinst = """#!/bin/sh
> > > > > -mkdir -p $D${sysconfdir}
> > > > > -if [ ! -e $D${sysconfdir}/passwd ]; then
> > > > > -\tcat << 'EOF' > $D${sysconfdir}/passwd
> > > > > +mkdir -p $D${sysconfdir}/${PW_SUBDIR}
> > > > > +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/passwd ]; then
> > > > > +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/passwd
> > > > >  """ + passwd + """EOF
> > > > >  fi
> > > > > -if [ ! -e $D${sysconfdir}/group ]; then
> > > > > -\tcat << 'EOF' > $D${sysconfdir}/group
> > > > > +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/group ]; then
> > > > > +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/group
> > > > >  """ + group + """EOF
> > > > >  fi
> > > > > +if [ -n "${PW_SUBDIR}" ]; then
> > > > > +ln -fs ${PW_SUBDIR}/passwd $D${sysconfdir}/passwd
> > > > > +ln -fs ${PW_SUBDIR}/group $D${sysconfdir}/group
> > > >
> > > > Use \t to indent the above two lines like the code before.
> > > >
> > > > > +fi
> > > > >  """
> > > > >      d.setVar(d.expand('pkg_preinst:${PN}'), preinst)
> > > > >  }
> > > > > @@ -114,5 +122,5 @@ pkg_postinst:${PN}-update () {
> > > > >  if [ -n "$D" ]; then
> > > > >     exit 0
> > > > >  fi
> > > > > -${sbindir}/update-passwd
> > > > > +${sbindir}/update-passwd -P /etc/${PW_SUBDIR}/passwd -S
> > > /etc/${PW_SUBDIR}/shadow -G /etc/${PW_SUBDIR}/group
> > > >
> > > > Replace /etc with ${sysconfdir}.
> > > >
> > > > >  }
> > > > > --
> > > > > 2.41.0
> > > >
> > > > //Peter
> > > >
> >

Re: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR

[Joakim Tjernlund]

On Thu, 2023-12-07 at 15:05 +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> > Sent: den 7 december 2023 09:48
> > To: openembedded-core@lists.openembedded.org; Peter Kjellerstedt
> > <peter.kjellerstedt@axis.com>
> > Subject: Re: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add PW_SUBDIR
> >
> > On Wed, 2023-12-06 at 20:21 +0000, Peter Kjellerstedt wrote:
> > > Since I've seen Richard's reply and his reluctance to merge this,
> > > this is mostly technical.
> >
> > I know but I don't understand what "batter ways" to do this is, I have
> > tested symlinks/--prefix/--root and bind mounting the passwd/shadow
> > files and none of then work. Using overlaysfs is an inferior solution to
> > me that makes the whole /etc writeable, may break when RFS underneath is
> > upgraded and I am unsure how resilient overlayfs is in case  of power
> > failure. What else is there ?
>
> For what it is worth, we use overlayfs on /etc in all of our products,
> and AFAIK have not had any problems with it. Our product upgrade solution
> is of course aware of the fact and takes care when upgrading to migrate
> all relevant changes in a controlled way.

Thanks, this indicates that overlayfs is somewhat resilient against power cuts.
My other concerns are still valid I think, exposing /etc as RW can be a security risk
and having to take special care when updating RO RFS underneath overlayfs.
Is that really better than my proposal ?

  //Jocke

>
> //Peter
>
> > >
> > > I would use either PW_DIR = "${sysconfdir}/pwdb" or PW_DIR:append = "/pwdb".
> > > Using "/etc" (and other hardcoded paths) should be avoided wherever
> > > possible.
> >
> > Thanks, will tru there out.
> >
> >  Jocke
> >
> > >
> > > //Peter
> > >
> > > > -----Original Message-----
> > > > From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> > > > Sent: den 29 november 2023 12:11
> > > > To: openembedded-core@lists.openembedded.org; Peter Kjellerstedt
> > > > <peter.kjellerstedt@axis.com>
> > > > Subject: Re: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add
> > PW_SUBDIR
> > > >
> > > > Hi Peter :)
> > > >
> > > > All good comments, will fix accordingly. Not sure how PW_DIR ?=
> > > > "${sysconfdir}" will work though.
> > > >
> > > > How do you envision one should set PW_DIR in distro .conf or
> > layer.conf?
> > > > Just PW_DIR = "/etc/pwdb" or PW_DIR = "${sysconfdir}/pwdb" ?
> > > >
> > > >  Jocke
> > > >
> > > > On Sun, 2023-11-26 at 21:21 +0000, Peter Kjellerstedt wrote:
> > > > > > -----Original Message-----
> > > > > > From: openembedded-core@lists.openembedded.org <openembedded-
> > > > core@lists.openembedded.org> On Behalf Of Joakim Tjernlund via
> > > > lists.openembedded.org
> > > > > > Sent: den 24 november 2023 15:11
> > > > > > To: openembedded-core@lists.openembedded.org
> > > > > > Cc: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > > > > Subject: [OE-core] [kirkstone][PATCH 2/4] base-passwd: Add
> > PW_SUBDIR
> > > > > >
> > > > > > Add support for creating passwd files in a /etc subdir
> > > > > > Set PW_SUBIR to pwdb to get passwd  files in /etc/pwdb
> > > > > >
> > > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > > > > ---
> > > > > >  .../base-passwd/base-passwd_3.5.29.bb         | 24 ++++++++++++--
> > ----
> > > > -
> > > > > >  1 file changed, 16 insertions(+), 8 deletions(-)
> > > > > >
> > > > > > diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > > b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > > > > index ef7792ae49..e453be0763 100644
> > > > > > --- a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > > > > +++ b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> > > > > > @@ -20,6 +20,9 @@ SRC_URI =
> > > > "https://launchpad.net/debian/+archive/primary/+files/${BPN}_${PV}.tar
> > > > > >  SRC_URI[md5sum] = "6beccac48083fe8ae5048acd062e5421"
> > > > > >  SRC_URI[sha256sum] =
> > > > "f0b66388b2c8e49c15692439d2bee63bcdd4bbbf7a782c7f64accc55986b6a36"
> > > > > >
> > > > > > +#Set PW_SUBDIR to pwdb to get passwd  files in /etc/pwdb
> > > > > > +PW_SUBDIR ?= ""
> > > > > > +
> > > > >
> > > > > Rather than defining a subdirectory, I would recommend defining the
> > full
> > > > > path, e.g.:
> > > > >
> > > > > PW_DIR ?= "${sysconfdir}"
> > > > >
> > > > > This avoids generating a lot of "//" in the middle of paths for the
> > > > majority
> > > > > of us who do not use a subdirectory for the password files.
> > > > >
> > > > > >  # the package is taken from launchpad; that source is static and
> > goes
> > > > stale
> > > > > >  # so we check the latest upstream from a directory that does get
> > > > updated
> > > > > >  UPSTREAM_CHECK_URI = "${DEBIAN_MIRROR}/main/b/base-passwd/"
> > > > > > @@ -50,10 +53,11 @@ basepasswd_sysroot_postinst() {
> > > > > >  #!/bin/sh
> > > > > >
> > > > > >  # Install passwd.master and group.master to sysconfdir
> > > > > > -install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}
> > > > > > +install -d -m 755 ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}
> > > > > >  for i in passwd group; do
> > > > > >     install -p -m 644 ${STAGING_DIR_TARGET}${datadir}/base-
> > > > passwd/\$i.master \
> > > > > > -           ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > > > > > +           ${STAGING_DIR_TARGET}${sysconfdir}/${PW_SUBDIR}/\$i
> > > > > > +   [ -n "${PW_SUBDIR}" ] && ln -fs ${PW_SUBDIR}/\$i
> > > > ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > > > >
> > > > > I generally recommended to use `[ ! ... ] || ...` instead of `[ ...
> > ] &&
> > > > ...`:
> > > > >
> > > > >       [ -z "${PW_SUBDIR}" ] || ln -fs ${PW_SUBDIR}/\$i
> > > > ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > > > >
> > > > > Or, assuming my recommendation above is followed:
> > > > >
> > > > >       [ "${PW_DIR}" = "${sysconfdir}" ] ||
> > > > >               ln -fsr ${STAGING_DIR_TARGET}${PW_DIR}/\$i
> > > > ${STAGING_DIR_TARGET}${sysconfdir}/\$i
> > > > >
> > > > > The reason is that the return status ($?) of `[ ... ] && ...` is 1
> > if
> > > > the
> > > > > test fails, while it is 0 for `[ ! ... ] || ...` when the test
> > succeeds.
> > > > >
> > > > > >  done
> > > > > >
> > > > > >  # Run any useradd postinsts
> > > > > > @@ -89,15 +93,19 @@ python populate_packages:prepend() {
> > > > > >      f.close()
> > > > > >
> > > > > >      preinst = """#!/bin/sh
> > > > > > -mkdir -p $D${sysconfdir}
> > > > > > -if [ ! -e $D${sysconfdir}/passwd ]; then
> > > > > > -\tcat << 'EOF' > $D${sysconfdir}/passwd
> > > > > > +mkdir -p $D${sysconfdir}/${PW_SUBDIR}
> > > > > > +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/passwd ]; then
> > > > > > +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/passwd
> > > > > >  """ + passwd + """EOF
> > > > > >  fi
> > > > > > -if [ ! -e $D${sysconfdir}/group ]; then
> > > > > > -\tcat << 'EOF' > $D${sysconfdir}/group
> > > > > > +if [ ! -e $D${sysconfdir}/${PW_SUBDIR}/group ]; then
> > > > > > +\tcat << 'EOF' > $D${sysconfdir}/${PW_SUBDIR}/group
> > > > > >  """ + group + """EOF
> > > > > >  fi
> > > > > > +if [ -n "${PW_SUBDIR}" ]; then
> > > > > > +ln -fs ${PW_SUBDIR}/passwd $D${sysconfdir}/passwd
> > > > > > +ln -fs ${PW_SUBDIR}/group $D${sysconfdir}/group
> > > > >
> > > > > Use \t to indent the above two lines like the code before.
> > > > >
> > > > > > +fi
> > > > > >  """
> > > > > >      d.setVar(d.expand('pkg_preinst:${PN}'), preinst)
> > > > > >  }
> > > > > > @@ -114,5 +122,5 @@ pkg_postinst:${PN}-update () {
> > > > > >  if [ -n "$D" ]; then
> > > > > >     exit 0
> > > > > >  fi
> > > > > > -${sbindir}/update-passwd
> > > > > > +${sbindir}/update-passwd -P /etc/${PW_SUBDIR}/passwd -S
> > > > /etc/${PW_SUBDIR}/shadow -G /etc/${PW_SUBDIR}/group
> > > > >
> > > > > Replace /etc with ${sysconfdir}.
> > > > >
> > > > > >  }
> > > > > > --
> > > > > > 2.41.0
> > > > >
> > > > > //Peter
> > > > >
> > >
>