| Message ID | 20251204083208.1046464-2-Qi.Chen@windriver.com |
|---|---|
| State | New |
| Headers | show |
| Series | [V5,1/2] rootfs-postcommands.bbclass: fix echo + '\n' in 'no password' banner | expand |
On Thu, 2025-12-04 at 08:32 +0000, Qi.Chen@windriver.com wrote: > From: Chen Qi <Qi.Chen@windriver.com> > > It's possible that users use EXTRA_USERS_PARAMS to set password > for root or explicitly expire root password. So we need to check > these two cases to ensure the 'no password' banner is not misleading. > > As an example, below are configurations to make an image requiring > setting a root password on first boot, but without having to first enter > a static initial password: > > In conf/toolcfg.cfg: > OE_FRAGMENTS += "distro/poky core/yocto/root-login-with-empty-password > In local.conf: > INHERIT += "extrausers" > EXTRA_USERS_PARAMS += " passwd-expire root;" > > Checking and adding such a banner is ensured to run as last steps of > ROOTFS_POSTPROCESS_COMMAND, regardless of IMAGE_FEATURES. In particualr, > we want to ensure that the function runs after set_user_group function > from extrausers.bbclass. So unlike other commands in this bbclass using > the '+=', this function uses ':append'. > > Besides, adding such banner is only meaningful when base-passwd and > baes-files are installed. In case of container image, they might not > be installed (e.g., container-test-image). So add extra checking for it. > With the above logic, we avoid breaking the following oe-selftest test case: > containerimage.ContainerImageTests.test_expected_files > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com> > --- > meta/classes-recipe/rootfs-postcommands.bbclass | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/meta/classes-recipe/rootfs-postcommands.bbclass b/meta/classes-recipe/rootfs-postcommands.bbclass > index f4fbc4c57e..8d7e5e7652 100644 > --- a/meta/classes-recipe/rootfs-postcommands.bbclass > +++ b/meta/classes-recipe/rootfs-postcommands.bbclass > @@ -5,7 +5,7 @@ > # > > # Zap the root password if empty-root-password feature is not enabled > -ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("IMAGE_FEATURES", "empty-root-password", "add_empty_root_password_note", "zap_empty_root_password ",d)}' > +ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("IMAGE_FEATURES", "empty-root-password", "", "zap_empty_root_password ",d)}' > > # Allow dropbear/openssh to accept logins from accounts with an empty password string if allow-empty-password is enabled > ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("IMAGE_FEATURES", "allow-empty-password", "ssh_allow_empty_password ", "",d)}' > @@ -64,6 +64,10 @@ ROOTFS_POSTPROCESS_COMMAND += '${SORT_PASSWD_POSTPROCESS_COMMAND}' > # > ROOTFS_POSTPROCESS_COMMAND += 'rootfs_reproducible' > > +# Check and add 'no root password' banner. > +# This needs to done at the end of ROOTFS_POSTPROCESS_COMMAND, thus using :append. > +ROOTFS_POSTPROCESS_COMMAND:append = " add_empty_root_password_note" We should use += instead of :append in all cases unless the semantics of append are actually needed. My understanding is that we just need to ensure that add_empty_root_password_note appears after zap_empty_root_password in ROOTFS_POSTPROCESS_COMMAND, and we don't need to use append to do that. It would also be better to keep the root password related modifications of ROOTFS_POSTPROCESS_COMMAND together, so please move these lines up so they are immediately after the zap_empty_root_password change above. Thanks,
Hi Paul, The :append is necessary. See the commit message. The function must run *after* the set_user_group function invoked by extrausers.bbclass. This is not related to zap_empty_root_password function. This functions checks the final status of root password and does modification on /etc/issues. Regards, Qi -----Original Message----- From: Paul Barker <paul@pbarker.dev> Sent: Thursday, December 11, 2025 9:51 PM To: Chen, Qi <Qi.Chen@windriver.com>; openembedded-core@lists.openembedded.org Cc: alex@linutronix.de Subject: Re: [OE-core][PATCH V5 2/2] rootfs-postcommands.bbclass: fix adding 'no password' banner On Thu, 2025-12-04 at 08:32 +0000, Qi.Chen@windriver.com wrote: > From: Chen Qi <Qi.Chen@windriver.com> > > It's possible that users use EXTRA_USERS_PARAMS to set password for > root or explicitly expire root password. So we need to check these two > cases to ensure the 'no password' banner is not misleading. > > As an example, below are configurations to make an image requiring > setting a root password on first boot, but without having to first > enter a static initial password: > > In conf/toolcfg.cfg: > OE_FRAGMENTS += "distro/poky core/yocto/root-login-with-empty-password > In local.conf: > INHERIT += "extrausers" > EXTRA_USERS_PARAMS += " passwd-expire root;" > > Checking and adding such a banner is ensured to run as last steps of > ROOTFS_POSTPROCESS_COMMAND, regardless of IMAGE_FEATURES. In > particualr, we want to ensure that the function runs after > set_user_group function from extrausers.bbclass. So unlike other > commands in this bbclass using the '+=', this function uses ':append'. > > Besides, adding such banner is only meaningful when base-passwd and > baes-files are installed. In case of container image, they might not > be installed (e.g., container-test-image). So add extra checking for it. > With the above logic, we avoid breaking the following oe-selftest test case: > containerimage.ContainerImageTests.test_expected_files > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com> > --- > meta/classes-recipe/rootfs-postcommands.bbclass | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/meta/classes-recipe/rootfs-postcommands.bbclass > b/meta/classes-recipe/rootfs-postcommands.bbclass > index f4fbc4c57e..8d7e5e7652 100644 > --- a/meta/classes-recipe/rootfs-postcommands.bbclass > +++ b/meta/classes-recipe/rootfs-postcommands.bbclass > @@ -5,7 +5,7 @@ > # > > # Zap the root password if empty-root-password feature is not enabled > -ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("IMAGE_FEATURES", "empty-root-password", "add_empty_root_password_note", "zap_empty_root_password ",d)}' > +ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("IMAGE_FEATURES", "empty-root-password", "", "zap_empty_root_password ",d)}' > > # Allow dropbear/openssh to accept logins from accounts with an empty > password string if allow-empty-password is enabled ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("IMAGE_FEATURES", "allow-empty-password", "ssh_allow_empty_password ", "",d)}' > @@ -64,6 +64,10 @@ ROOTFS_POSTPROCESS_COMMAND += '${SORT_PASSWD_POSTPROCESS_COMMAND}' > # > ROOTFS_POSTPROCESS_COMMAND += 'rootfs_reproducible' > > +# Check and add 'no root password' banner. > +# This needs to done at the end of ROOTFS_POSTPROCESS_COMMAND, thus using :append. > +ROOTFS_POSTPROCESS_COMMAND:append = " add_empty_root_password_note" We should use += instead of :append in all cases unless the semantics of append are actually needed. My understanding is that we just need to ensure that add_empty_root_password_note appears after zap_empty_root_password in ROOTFS_POSTPROCESS_COMMAND, and we don't need to use append to do that. It would also be better to keep the root password related modifications of ROOTFS_POSTPROCESS_COMMAND together, so please move these lines up so they are immediately after the zap_empty_root_password change above. Thanks, -- Paul Barker
Ping Randy told me that this one was hold because the :append concern. I think I've stated the reason clearly. The function is not changing /etc/passwd and /etc/shadow. It's just checking them and changing /etc/issues. Anyway, if anyone has some other approach. Please let me know. Regards, Qi -----Original Message----- From: Chen, Qi Sent: Friday, December 12, 2025 9:13 AM To: 'Paul Barker' <paul@pbarker.dev>; openembedded-core@lists.openembedded.org Cc: alex@linutronix.de Subject: RE: [OE-core][PATCH V5 2/2] rootfs-postcommands.bbclass: fix adding 'no password' banner Hi Paul, The :append is necessary. See the commit message. The function must run *after* the set_user_group function invoked by extrausers.bbclass. This is not related to zap_empty_root_password function. This functions checks the final status of root password and does modification on /etc/issues. Regards, Qi -----Original Message----- From: Paul Barker <paul@pbarker.dev> Sent: Thursday, December 11, 2025 9:51 PM To: Chen, Qi <Qi.Chen@windriver.com>; openembedded-core@lists.openembedded.org Cc: alex@linutronix.de Subject: Re: [OE-core][PATCH V5 2/2] rootfs-postcommands.bbclass: fix adding 'no password' banner On Thu, 2025-12-04 at 08:32 +0000, Qi.Chen@windriver.com wrote: > From: Chen Qi <Qi.Chen@windriver.com> > > It's possible that users use EXTRA_USERS_PARAMS to set password for > root or explicitly expire root password. So we need to check these two > cases to ensure the 'no password' banner is not misleading. > > As an example, below are configurations to make an image requiring > setting a root password on first boot, but without having to first > enter a static initial password: > > In conf/toolcfg.cfg: > OE_FRAGMENTS += "distro/poky core/yocto/root-login-with-empty-password > In local.conf: > INHERIT += "extrausers" > EXTRA_USERS_PARAMS += " passwd-expire root;" > > Checking and adding such a banner is ensured to run as last steps of > ROOTFS_POSTPROCESS_COMMAND, regardless of IMAGE_FEATURES. In > particualr, we want to ensure that the function runs after > set_user_group function from extrausers.bbclass. So unlike other > commands in this bbclass using the '+=', this function uses ':append'. > > Besides, adding such banner is only meaningful when base-passwd and > baes-files are installed. In case of container image, they might not > be installed (e.g., container-test-image). So add extra checking for it. > With the above logic, we avoid breaking the following oe-selftest test case: > containerimage.ContainerImageTests.test_expected_files > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com> > --- > meta/classes-recipe/rootfs-postcommands.bbclass | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/meta/classes-recipe/rootfs-postcommands.bbclass > b/meta/classes-recipe/rootfs-postcommands.bbclass > index f4fbc4c57e..8d7e5e7652 100644 > --- a/meta/classes-recipe/rootfs-postcommands.bbclass > +++ b/meta/classes-recipe/rootfs-postcommands.bbclass > @@ -5,7 +5,7 @@ > # > > # Zap the root password if empty-root-password feature is not enabled > -ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("IMAGE_FEATURES", "empty-root-password", "add_empty_root_password_note", "zap_empty_root_password ",d)}' > +ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("IMAGE_FEATURES", "empty-root-password", "", "zap_empty_root_password ",d)}' > > # Allow dropbear/openssh to accept logins from accounts with an empty > password string if allow-empty-password is enabled ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("IMAGE_FEATURES", "allow-empty-password", "ssh_allow_empty_password ", "",d)}' > @@ -64,6 +64,10 @@ ROOTFS_POSTPROCESS_COMMAND += '${SORT_PASSWD_POSTPROCESS_COMMAND}' > # > ROOTFS_POSTPROCESS_COMMAND += 'rootfs_reproducible' > > +# Check and add 'no root password' banner. > +# This needs to done at the end of ROOTFS_POSTPROCESS_COMMAND, thus using :append. > +ROOTFS_POSTPROCESS_COMMAND:append = " add_empty_root_password_note" We should use += instead of :append in all cases unless the semantics of append are actually needed. My understanding is that we just need to ensure that add_empty_root_password_note appears after zap_empty_root_password in ROOTFS_POSTPROCESS_COMMAND, and we don't need to use append to do that. It would also be better to keep the root password related modifications of ROOTFS_POSTPROCESS_COMMAND together, so please move these lines up so they are immediately after the zap_empty_root_password change above. Thanks, -- Paul Barker
On Tue, 2025-12-16 at 02:13 +0000, Chen, Qi wrote: > Ping > > Randy told me that this one was hold because the :append concern. > I think I've stated the reason clearly. > The function is not changing /etc/passwd and /etc/shadow. It's just checking them and changing /etc/issues. > > Anyway, if anyone has some other approach. Please let me know. Hi Qi, We discussed this on the patch review call yesterday. I understand the reasons for the :append here and the interaction with the extrausers bbclass. However, this still leaves us with concerns - we don't want recipes chasing each other using :append as the ordering gets very difficult to reason about. The extrausers bbclass also uses :append, would the ordering be correct if set_user_group and add_empty_root_password_note were both added using += instead? If so, let's patch extrausers.bbclass instead of using an append here. Best regards,
On 12/16/25 21:23, Paul Barker wrote: > On Tue, 2025-12-16 at 02:13 +0000, Chen, Qi wrote: >> Ping >> >> Randy told me that this one was hold because the :append concern. >> I think I've stated the reason clearly. >> The function is not changing /etc/passwd and /etc/shadow. It's just checking them and changing /etc/issues. >> >> Anyway, if anyone has some other approach. Please let me know. > Hi Qi, > > We discussed this on the patch review call yesterday. > > I understand the reasons for the :append here and the interaction with > the extrausers bbclass. However, this still leaves us with concerns - we > don't want recipes chasing each other using :append as the ordering gets > very difficult to reason about. > > The extrausers bbclass also uses :append, would the ordering be correct > if set_user_group and add_empty_root_password_note were both added using > += instead? If so, let's patch extrausers.bbclass instead of using an > append here. Hi Paul, Thanks for the suggestion. I checked the history. I found that the 'append' in extrausers.bbclass was first introduced by me. And I cannot recall why I used 'append' at that time. After changing to use '+=', things continue to work. And the ordering is correct. I'll send out V6. Regards, Qi > > Best regards, >
diff --git a/meta/classes-recipe/rootfs-postcommands.bbclass b/meta/classes-recipe/rootfs-postcommands.bbclass index f4fbc4c57e..8d7e5e7652 100644 --- a/meta/classes-recipe/rootfs-postcommands.bbclass +++ b/meta/classes-recipe/rootfs-postcommands.bbclass @@ -5,7 +5,7 @@ # # Zap the root password if empty-root-password feature is not enabled -ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("IMAGE_FEATURES", "empty-root-password", "add_empty_root_password_note", "zap_empty_root_password ",d)}' +ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("IMAGE_FEATURES", "empty-root-password", "", "zap_empty_root_password ",d)}' # Allow dropbear/openssh to accept logins from accounts with an empty password string if allow-empty-password is enabled ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains("IMAGE_FEATURES", "allow-empty-password", "ssh_allow_empty_password ", "",d)}' @@ -64,6 +64,10 @@ ROOTFS_POSTPROCESS_COMMAND += '${SORT_PASSWD_POSTPROCESS_COMMAND}' # ROOTFS_POSTPROCESS_COMMAND += 'rootfs_reproducible' +# Check and add 'no root password' banner. +# This needs to done at the end of ROOTFS_POSTPROCESS_COMMAND, thus using :append. +ROOTFS_POSTPROCESS_COMMAND:append = " add_empty_root_password_note" + # 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 @@ -259,8 +263,14 @@ zap_empty_root_password () { # This function adds a note to the login banner that the system is configured for root logins without password # add_empty_root_password_note () { - echo "Type 'root' to login with superuser privileges (no password will be asked)." >> ${IMAGE_ROOTFS}/etc/issue - echo "" >> ${IMAGE_ROOTFS}/etc/issue + if [ -e ${IMAGE_ROOTFS}/etc/shadow -a -e ${IMAGE_ROOTFS}/etc/issue ]; then + rootpw="`grep '^root:' ${IMAGE_ROOTFS}/etc/shadow | cut -d':' -f2`" + rootpw_lastchanged="`grep "^root:" ${IMAGE_ROOTFS}/etc/shadow | cut -d: -f3`" + if [ -z "$rootpw" -a "$rootpw_lastchanged" != "0" ]; then + echo "Type 'root' to login with superuser privileges (no password will be asked)." >> ${IMAGE_ROOTFS}/etc/issue + echo "" >> ${IMAGE_ROOTFS}/etc/issue + fi + fi } #