[PATCH 3/6] rootfspostcommands.py: Cleanup subid backup files generated by shadow-utils

[PATCH 3/6] rootfspostcommands.py: Cleanup subid backup files generated by shadow-utils

[Andrei Gherzan]

From: Andrei Gherzan <andrei.gherzan@huawei.com>

When creating users, shadow-utils might create backup files for
subordinate ID files (subid, subgid). Make sure we clean them up
similarly to the other backup files shadow-utils creates.

Signed-off-by: Andrei Gherzan <andrei.gherzan@huawei.com>
---
 meta/lib/rootfspostcommands.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meta/lib/rootfspostcommands.py b/meta/lib/rootfspostcommands.py
index e344ae2efc..5386eea409 100644
--- a/meta/lib/rootfspostcommands.py
+++ b/meta/lib/rootfspostcommands.py
@@ -73,6 +73,8 @@ def remove_shadowutils_backup_files(sysconfdir):
             'gshadow',
             'passwd',
             'shadow',
+            'subgid',
+            'subuid',
         ):
         filepath = os.path.join(sysconfdir, filename)
         remove_shadowutils_backup_file(filepath)
-- 
2.25.1

[PATCH 4/6] selftest: Add module for testing rootfs postcommands

[Andrei Gherzan]

From: Andrei Gherzan <andrei.gherzan@huawei.com>

The initial implementation adds tests for 'tidy_shadowutils_files'.

Signed-off-by: Andrei Gherzan <andrei.gherzan@huawei.com>
---
 .../selftest/cases/rootfspostcommandstests.py | 97 +++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 meta/lib/oeqa/selftest/cases/rootfspostcommandstests.py

diff --git a/meta/lib/oeqa/selftest/cases/rootfspostcommandstests.py b/meta/lib/oeqa/selftest/cases/rootfspostcommandstests.py
new file mode 100644
index 0000000000..44e2c09a6f
--- /dev/null
+++ b/meta/lib/oeqa/selftest/cases/rootfspostcommandstests.py
@@ -0,0 +1,97 @@
+# SPDX-FileCopyrightText: Huawei Inc.
+#
+# SPDX-License-Identifier: MIT
+
+import os
+import oe
+import unittest
+from oeqa.selftest.case import OESelftestTestCase
+from oeqa.utils.commands import bitbake, get_bb_vars
+
+class ShadowUtilsTidyFiles(OESelftestTestCase):
+    """
+    Check if shadow image rootfs files are tidy.
+
+    The tests are focused on testing the functionality provided by the
+    'tidy_shadowutils_files' rootfs postprocess command (via
+    SORT_PASSWD_POSTPROCESS_COMMAND).
+    """
+
+    def sysconf_build(self):
+        """
+        Verify if shadow tidy files tests are to be run and if yes, build a
+        test image and return its sysconf rootfs path.
+        """
+
+        test_image = "core-image-minimal"
+
+        config = 'IMAGE_CLASSES += "extrausers"\n'
+        config += 'EXTRA_USERS_PARAMS = "groupadd -g 1000 oeqatester; "\n'
+        config += 'EXTRA_USERS_PARAMS += "useradd -p \'\' -u 1000 -N -g 1000 oeqatester; "\n'
+        self.write_config(config)
+
+        vars = get_bb_vars(("IMAGE_ROOTFS", "SORT_PASSWD_POSTPROCESS_COMMAND", "sysconfdir"),
+            test_image)
+        passwd_postprocess_cmd = vars["SORT_PASSWD_POSTPROCESS_COMMAND"]
+        self.assertIsNotNone(passwd_postprocess_cmd)
+        if (passwd_postprocess_cmd.strip() != 'tidy_shadowutils_files;'):
+            raise unittest.SkipTest("Testcase skipped as 'tidy_shadowutils_files' "
+                "rootfs post process command is not the set SORT_PASSWD_POSTPROCESS_COMMAND.")
+
+        rootfs = vars["IMAGE_ROOTFS"]
+        self.assertIsNotNone(rootfs)
+        sysconfdir = vars["sysconfdir"]
+        bitbake(test_image)
+        self.assertIsNotNone(sysconfdir)
+
+        return oe.path.join(rootfs, sysconfdir)
+
+    def test_shadowutils_backup_files(self):
+        """
+        Test that the rootfs doesn't include any known shadow backup files.
+        """
+
+        backup_files = (
+            'group-',
+            'gshadow-',
+            'passwd-',
+            'shadow-',
+            'subgid-',
+            'subuid-',
+        )
+
+        rootfs_sysconfdir = self.sysconf_build()
+        found = []
+        for backup_file in backup_files:
+            backup_filepath = oe.path.join(rootfs_sysconfdir, backup_file)
+            if os.path.exists(backup_filepath):
+                found.append(backup_file)
+        if (found):
+            raise Exception('The following shadow backup files were found in '
+                'the rootfs: %s' % found)
+
+    def test_shadowutils_sorted_files(self):
+        """
+        Test that the 'passwd' and the 'group' shadow utils files are ordered
+        by ID.
+        """
+
+        files = (
+            'passwd',
+            'group',
+        )
+
+        rootfs_sysconfdir = self.sysconf_build()
+        unsorted = []
+        for file in files:
+            filepath = oe.path.join(rootfs_sysconfdir, file)
+            with open(filepath, 'rb') as f:
+                ids = []
+                lines = f.readlines()
+                for line in lines:
+                    entries = line.split(b':')
+                    ids.append(int(entries[2]))
+            if (ids != sorted(ids)):
+                unsorted.append(file)
+        if (unsorted):
+            raise Exception("The following files were not sorted by ID as expected: %s" % unsorted)
-- 
2.25.1

[PATCH 5/6] rootfs-postcommands.bbclass: Follow function rename in rootfspostcommands.py

[Andrei Gherzan]

From: Andrei Gherzan <andrei.gherzan@huawei.com>

'shadow_sort' was renamed to 'tidy_shadowutils_files' in
rootfspostcommands.py so we reflect this in
SORT_PASSWD_POSTPROCESS_COMMAND. This also creates a deprecation
function for 'shadow_sort'.

Signed-off-by: Andrei Gherzan <andrei.gherzan@huawei.com>
---
 meta/classes-recipe/rootfs-postcommands.bbclass | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/meta/classes-recipe/rootfs-postcommands.bbclass b/meta/classes-recipe/rootfs-postcommands.bbclass
index bf1e992bb2..215e38e33d 100644
--- a/meta/classes-recipe/rootfs-postcommands.bbclass
+++ b/meta/classes-recipe/rootfs-postcommands.bbclass
@@ -63,7 +63,7 @@ inherit image-artifact-names
 # systemd_sysusers_create and set_user_group. Using :append is not
 # enough for that, set_user_group is added that way and would end
 # up running after us.
-SORT_PASSWD_POSTPROCESS_COMMAND ??= " sort_passwd; "
+SORT_PASSWD_POSTPROCESS_COMMAND ??= " tidy_shadowutils_files; "
 python () {
     d.appendVar('ROOTFS_POSTPROCESS_COMMAND', '${SORT_PASSWD_POSTPROCESS_COMMAND}')
     d.appendVar('ROOTFS_POSTPROCESS_COMMAND', 'rootfs_reproducible;')
@@ -221,9 +221,20 @@ serial_autologin_root () {
 	fi
 }
 
+python tidy_shadowutils_files () {
+    import rootfspostcommands
+    rootfspostcommands.tidy_shadowutils_files(d.expand('${IMAGE_ROOTFS}${sysconfdir}'))
+}
+
 python sort_passwd () {
+    """
+    Deprecated in the favour of tidy_shadowutils_files.
+    """
     import rootfspostcommands
-    rootfspostcommands.sort_passwd(d.expand('${IMAGE_ROOTFS}${sysconfdir}'))
+    bb.warn('[sort_passwd] You are using a deprecated function for '
+        'SORT_PASSWD_POSTPROCESS_COMMAND. The default one is now called '
+        '"tidy_shadowutils_files".')
+    rootfspostcommands.tidy_shadowutils_files(d.expand('${IMAGE_ROOTFS}${sysconfdir}'))
 }
 
 #
-- 
2.25.1

[PATCH 6/6] shadow: Avoid nss warning/error with musl

[Andrei Gherzan]

From: Andrei Gherzan <andrei.gherzan@huawei.com>

The libnss configuration file is only installed when glibc is used. The
inexistence of it on a musl-based rootfs, will make shadow complain
about it:

Failed opening /etc/nsswitch.conf

This is because shadow will try to use nsswich when dealing with
subordinate IDs and the message is just a warning as the tool will still
generate them correctly in subuid/subgid files.

We drop this log message for class native to avoid an error when rootfs
logs are checked ('Failed' will match the regex bitbake is using to
check for rootfs generation errors).

Signed-off-by: Andrei Gherzan <andrei.gherzan@huawei.com>
---
 ...f-message-when-not-in-place-eg.-musl.patch | 27 +++++++++++++++++++
 meta/recipes-extended/shadow/shadow.inc       |  2 ++
 2 files changed, 29 insertions(+)
 create mode 100644 meta/recipes-extended/shadow/files/0001-Drop-nsswitch.conf-message-when-not-in-place-eg.-musl.patch

diff --git a/meta/recipes-extended/shadow/files/0001-Drop-nsswitch.conf-message-when-not-in-place-eg.-musl.patch b/meta/recipes-extended/shadow/files/0001-Drop-nsswitch.conf-message-when-not-in-place-eg.-musl.patch
new file mode 100644
index 0000000000..aeb89ff6a0
--- /dev/null
+++ b/meta/recipes-extended/shadow/files/0001-Drop-nsswitch.conf-message-when-not-in-place-eg.-musl.patch
@@ -0,0 +1,27 @@
+From 11290e897a49adddee215833944a518443d9b0d6 Mon Sep 17 00:00:00 2001
+From: Andrei Gherzan <andrei.gherzan@huawei.com>
+Date: Wed, 24 Aug 2022 00:54:47 +0200
+Subject: [PATCH] Drop nsswitch.conf message when not in place - eg. musl
+
+Upstream-Status: Inappropriate [OE specific]
+Signed-off-by: Andrei Gherzan <andrei.gherzan@huawei.com>
+---
+ lib/nss.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/lib/nss.c b/lib/nss.c
+index 06fa48e..44245da 100644
+--- a/lib/nss.c
++++ b/lib/nss.c
+@@ -59,7 +59,7 @@ void nss_init(const char *nsswitch_path) {
+ 	//   subid:	files
+ 	nssfp = fopen(nsswitch_path, "r");
+ 	if (!nssfp) {
+-		fprintf(shadow_logfd, "Failed opening %s: %m\n", nsswitch_path);
++		//fprintf(shadow_logfd, "Failed opening %s: %m\n", nsswitch_path);
+ 		atomic_store(&nss_init_completed, true);
+ 		return;
+ 	}
+-- 
+2.25.1
+
diff --git a/meta/recipes-extended/shadow/shadow.inc b/meta/recipes-extended/shadow/shadow.inc
index b2f82e9ac7..414bf467ba 100644
--- a/meta/recipes-extended/shadow/shadow.inc
+++ b/meta/recipes-extended/shadow/shadow.inc
@@ -25,12 +25,14 @@ SRC_URI:append:class-target = " \
 SRC_URI:append:class-native = " \
            file://0001-Disable-use-of-syslog-for-sysroot.patch \
            file://commonio.c-fix-unexpected-open-failure-in-chroot-env.patch \
+           file://0001-Drop-nsswitch.conf-message-when-not-in-place-eg.-musl.patch \
            "
 SRC_URI:append:class-nativesdk = " \
            file://0001-Disable-use-of-syslog-for-sysroot.patch \
            "
 SRC_URI[sha256sum] = "9fdb73b5d2b44e8ba9fcee1b4493ac75dd5040bda35b9ac8b06570cd192e7ee3"
 
+
 # Additional Policy files for PAM
 PAM_SRC_URI = "file://pam.d/chfn \
                file://pam.d/chpasswd \
-- 
2.25.1

Re: [OE-core] [PATCH 6/6] shadow: Avoid nss warning/error with musl

[Alexander Kanavin]

I do not think it is inappropriate, as it does highlight an issue
which we need to inform upstream about. Can you submit it upstream
anyway please?

Alex

On Wed, 24 Aug 2022 at 01:57, Andrei Gherzan <andrei@gherzan.com> wrote:
>
> From: Andrei Gherzan <andrei.gherzan@huawei.com>
>
> The libnss configuration file is only installed when glibc is used. The
> inexistence of it on a musl-based rootfs, will make shadow complain
> about it:
>
> Failed opening /etc/nsswitch.conf
>
> This is because shadow will try to use nsswich when dealing with
> subordinate IDs and the message is just a warning as the tool will still
> generate them correctly in subuid/subgid files.
>
> We drop this log message for class native to avoid an error when rootfs
> logs are checked ('Failed' will match the regex bitbake is using to
> check for rootfs generation errors).
>
> Signed-off-by: Andrei Gherzan <andrei.gherzan@huawei.com>
> ---
>  ...f-message-when-not-in-place-eg.-musl.patch | 27 +++++++++++++++++++
>  meta/recipes-extended/shadow/shadow.inc       |  2 ++
>  2 files changed, 29 insertions(+)
>  create mode 100644 meta/recipes-extended/shadow/files/0001-Drop-nsswitch.conf-message-when-not-in-place-eg.-musl.patch
>
> diff --git a/meta/recipes-extended/shadow/files/0001-Drop-nsswitch.conf-message-when-not-in-place-eg.-musl.patch b/meta/recipes-extended/shadow/files/0001-Drop-nsswitch.conf-message-when-not-in-place-eg.-musl.patch
> new file mode 100644
> index 0000000000..aeb89ff6a0
> --- /dev/null
> +++ b/meta/recipes-extended/shadow/files/0001-Drop-nsswitch.conf-message-when-not-in-place-eg.-musl.patch
> @@ -0,0 +1,27 @@
> +From 11290e897a49adddee215833944a518443d9b0d6 Mon Sep 17 00:00:00 2001
> +From: Andrei Gherzan <andrei.gherzan@huawei.com>
> +Date: Wed, 24 Aug 2022 00:54:47 +0200
> +Subject: [PATCH] Drop nsswitch.conf message when not in place - eg. musl
> +
> +Upstream-Status: Inappropriate [OE specific]
> +Signed-off-by: Andrei Gherzan <andrei.gherzan@huawei.com>
> +---
> + lib/nss.c | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/lib/nss.c b/lib/nss.c
> +index 06fa48e..44245da 100644
> +--- a/lib/nss.c
> ++++ b/lib/nss.c
> +@@ -59,7 +59,7 @@ void nss_init(const char *nsswitch_path) {
> +       //   subid:     files
> +       nssfp = fopen(nsswitch_path, "r");
> +       if (!nssfp) {
> +-              fprintf(shadow_logfd, "Failed opening %s: %m\n", nsswitch_path);
> ++              //fprintf(shadow_logfd, "Failed opening %s: %m\n", nsswitch_path);
> +               atomic_store(&nss_init_completed, true);
> +               return;
> +       }
> +--
> +2.25.1
> +
> diff --git a/meta/recipes-extended/shadow/shadow.inc b/meta/recipes-extended/shadow/shadow.inc
> index b2f82e9ac7..414bf467ba 100644
> --- a/meta/recipes-extended/shadow/shadow.inc
> +++ b/meta/recipes-extended/shadow/shadow.inc
> @@ -25,12 +25,14 @@ SRC_URI:append:class-target = " \
>  SRC_URI:append:class-native = " \
>             file://0001-Disable-use-of-syslog-for-sysroot.patch \
>             file://commonio.c-fix-unexpected-open-failure-in-chroot-env.patch \
> +           file://0001-Drop-nsswitch.conf-message-when-not-in-place-eg.-musl.patch \
>             "
>  SRC_URI:append:class-nativesdk = " \
>             file://0001-Disable-use-of-syslog-for-sysroot.patch \
>             "
>  SRC_URI[sha256sum] = "9fdb73b5d2b44e8ba9fcee1b4493ac75dd5040bda35b9ac8b06570cd192e7ee3"
>
> +
>  # Additional Policy files for PAM
>  PAM_SRC_URI = "file://pam.d/chfn \
>                 file://pam.d/chpasswd \
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#169767): https://lists.openembedded.org/g/openembedded-core/message/169767
> Mute This Topic: https://lists.openembedded.org/mt/93217143/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Re: [OE-core] [PATCH 6/6] shadow: Avoid nss warning/error with musl

[Andrei Gherzan]

On Wed, 24 Aug 2022, at 08:36, Alexander Kanavin wrote:
> I do not think it is inappropriate, as it does highlight an issue
> which we need to inform upstream about. Can you submit it upstream
> anyway please?

Just to be sure we have the same understanding, could you detail the issue you are seeing?

Andrei

Re: [OE-core] [PATCH 6/6] shadow: Avoid nss warning/error with musl

[Alexander Kanavin]

The issue is described by you in the commit message? I think this is
good enough for upstream submission as is, as long as you edit out the
yocto specific references.

Alex

On Wed, 24 Aug 2022 at 10:16, Andrei Gherzan <andrei@gherzan.com> wrote:
>
> On Wed, 24 Aug 2022, at 08:36, Alexander Kanavin wrote:
> > I do not think it is inappropriate, as it does highlight an issue
> > which we need to inform upstream about. Can you submit it upstream
> > anyway please?
>
> Just to be sure we have the same understanding, could you detail the issue you are seeing?
>
> Andrei

Re: [OE-core] [PATCH 6/6] shadow: Avoid nss warning/error with musl

[Andrei Gherzan]

On Wed, 24 Aug 2022, at 09:21, Alexander Kanavin wrote:
> The issue is described by you in the commit message? I think this is
> good enough for upstream submission as is, as long as you edit out the
> yocto specific references.

RIght. I was more interested in how you see this issue per se. At the end of the day, it is not a bug, it is more of a warning/log message while the functionality is in place. What would you expect upstream to do?

Of course, there are a couple of options here. Compile macro for musl, change the log format for that specific message, etc. But it can also be treated as a dynamic check with an associated log message.

Andrei

Re: [OE-core] [PATCH 6/6] shadow: Avoid nss warning/error with musl

[Alexander Kanavin]

I trust the upstream will figure this out. My goal is to avoid adding
more patches that are declared unsuitable for upstream, and so have to
be rebased until the end of time by (usually) me.

Alex

On Wed, 24 Aug 2022 at 10:28, Andrei Gherzan <andrei@gherzan.com> wrote:
>
> On Wed, 24 Aug 2022, at 09:21, Alexander Kanavin wrote:
> > The issue is described by you in the commit message? I think this is
> > good enough for upstream submission as is, as long as you edit out the
> > yocto specific references.
>
> RIght. I was more interested in how you see this issue per se. At the end of the day, it is not a bug, it is more of a warning/log message while the functionality is in place. What would you expect upstream to do?
>
> Of course, there are a couple of options here. Compile macro for musl, change the log format for that specific message, etc. But it can also be treated as a dynamic check with an associated log message.
>
> Andrei

Re: [OE-core] [PATCH 6/6] shadow: Avoid nss warning/error with musl

[Andrei Gherzan]

On Wed, 24 Aug 2022, at 09:30, Alexander Kanavin wrote:
> I trust the upstream will figure this out. My goal is to avoid adding
> more patches that are declared unsuitable for upstream, and so have to
> be rebased until the end of time by (usually) me.

OK. I have created an upstream issue: https://github.com/shadow-maint/shadow/issues/557.

Andrei

Re: [OE-core] [PATCH 6/6] shadow: Avoid nss warning/error with musl

[Alexander Kanavin]

Thanks, can you also resend the patch with the link to the ticket included?

Alex

On Wed, 24 Aug 2022 at 10:49, Andrei Gherzan <andrei@gherzan.com> wrote:
>
> On Wed, 24 Aug 2022, at 09:30, Alexander Kanavin wrote:
> > I trust the upstream will figure this out. My goal is to avoid adding
> > more patches that are declared unsuitable for upstream, and so have to
> > be rebased until the end of time by (usually) me.
>
> OK. I have created an upstream issue: https://github.com/shadow-maint/shadow/issues/557.
>
> Andrei

Re: [OE-core] [PATCH 6/6] shadow: Avoid nss warning/error with musl

[Andrei Gherzan]

On Wed, 24 Aug 2022, at 09:56, Alexander Kanavin wrote:
> Thanks, can you also resend the patch with the link to the ticket included?

Sure. https://lists.openembedded.org/g/openembedded-core/message/169800

Andrei

Re: [OE-core] [PATCH 6/6] shadow: Avoid nss warning/error with musl

[Alexander Kanavin]

Almost there :) As the patch was not actually submitted, the correct
metadata is:

Upstream-Status: Inappropriate [issue reported at
https://github.com/shadow-maint/shadow/issues/557]

Alex

On Wed, 24 Aug 2022 at 11:37, Andrei Gherzan <andrei@gherzan.com> wrote:
>
> On Wed, 24 Aug 2022, at 09:56, Alexander Kanavin wrote:
> > Thanks, can you also resend the patch with the link to the ticket included?
>
> Sure. https://lists.openembedded.org/g/openembedded-core/message/169800
>
> Andrei

Re: [OE-core] [PATCH 6/6] shadow: Avoid nss warning/error with musl

[Andrei Gherzan]

On Wed, 24 Aug 2022, at 10:39, Alexander Kanavin wrote:
> Almost there :) As the patch was not actually submitted, the correct
> metadata is:
>
> Upstream-Status: Inappropriate [issue reported at
> https://github.com/shadow-maint/shadow/issues/557]

Now I know :)

https://lists.openembedded.org/g/openembedded-core/message/169804

Andrei