* [RFC] incorrect parsing of sysusers.d in rootfs generation @ 2023-06-05 15:55 Louis Rannou 2023-06-07 15:16 ` Louis Rannou 0 siblings, 1 reply; 7+ messages in thread From: Louis Rannou @ 2023-06-05 15:55 UTC (permalink / raw) To: tgamblin, openembedded-core Hello, I have found an issue in the rootfs routine. The rootfs-postcommands.bbclass has a funtion systemd_create_users that reads /etc/sysusers.d/*.conf files and parses lines as 'type name id comment'. However, the sysusers.d manual says, those lines can be 'type name id comment home_dir shell'. If a home directory of shell is defined, they are considered as part of the comment, and we run incorrect commands such as the one below : useradd --shell /sbin/nologin --uid 0 --comment "Super User" /root --system root To fix that, we require a stronger parsing. Several options look possible to me, but I am not sure which one is preferred. 1. sed with a regular expression that returns something that still needs parsing 2. awk with a step by step script that returns something that still needs to be parsed 3. use python and regexp module Also I don't know if the parsing should completely check the sysusers syntax as said in the manual (first field is [urgm], second is alphanum_-, etc.). In my opinion it should not as this will be made by the useradd command. Do you think it worth to add some testing about that ? I am not sure how to do that. Regards, Louis Rannou ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] incorrect parsing of sysusers.d in rootfs generation 2023-06-05 15:55 [RFC] incorrect parsing of sysusers.d in rootfs generation Louis Rannou @ 2023-06-07 15:16 ` Louis Rannou 2023-06-08 9:19 ` [OE-core] " Richard Purdie 0 siblings, 1 reply; 7+ messages in thread From: Louis Rannou @ 2023-06-07 15:16 UTC (permalink / raw) To: tgamblin, openembedded-core Hello again, a python solution could be one below. Also, I found that most of users/groups defined there are redundant as they already exist (such as root). I guess they are defined from base-passwd. I am not sure which recipe (base-passwd or systemd) should have the precedence on this. If it's base-passwd, perhaps this postcommand should check first if the user does already exist. Regards, Louis --- meta/classes/rootfs-postcommands.bbclass | 69 ++++++++++++++++-------- 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/meta/classes/rootfs-postcommands.bbclass b/meta/classes/rootfs-postcommands.bbclass index 5c0b3ec37c..1741919918 100644 --- a/meta/classes/rootfs-postcommands.bbclass +++ b/meta/classes/rootfs-postcommands.bbclass @@ -61,29 +61,52 @@ python () { d.appendVar('ROOTFS_POSTPROCESS_COMMAND', 'rootfs_reproducible;') } -systemd_create_users () { - for conffile in ${IMAGE_ROOTFS}/usr/lib/sysusers.d/*.conf; do - [ -e $conffile ] || continue - grep -v "^#" $conffile | sed -e '/^$/d' | while read type name id comment; do - if [ "$type" = "u" ]; then - useradd_params="--shell /sbin/nologin" - [ "$id" != "-" ] && useradd_params="$useradd_params --uid $id" - [ "$comment" != "-" ] && useradd_params="$useradd_params --comment $comment" - useradd_params="$useradd_params --system $name" - eval useradd --root ${IMAGE_ROOTFS} $useradd_params || true - elif [ "$type" = "g" ]; then - groupadd_params="" - [ "$id" != "-" ] && groupadd_params="$groupadd_params --gid $id" - groupadd_params="$groupadd_params --system $name" - eval groupadd --root ${IMAGE_ROOTFS} $groupadd_params || true - elif [ "$type" = "m" ]; then - group=$id - eval groupadd --root ${IMAGE_ROOTFS} --system $group || true - eval useradd --root ${IMAGE_ROOTFS} --shell /sbin/nologin --system $name --no-user-group || true - eval usermod --root ${IMAGE_ROOTFS} -a -G $group $name - fi - done - done +python systemd_create_users() { + import glob + import re + import subprocess + + pattern_comment = r'(-|\"[^:\"]+\")' + pattern_word = r'[^\s]+' + pattern_line = r'(' + pattern_word + r')\s+(' + pattern_word + r')\s+(' + pattern_word + r')(\s+' \ + + pattern_comment + r')?' + r'(\s+(' + pattern_word + r'))?' + r'(\s+(' + pattern_word + r'))?' + + IMAGE_ROOTFS = d.getVar('IMAGE_ROOTFS') + + for conffile in glob.glob(os.path.join(IMAGE_ROOTFS, 'usr/lib/sysusers.d/*.conf')): + with open(conffile, 'r') as f: + for line in f: + line = line.strip() + if not len(line) or line[0] == '#': continue + ret = re.fullmatch(pattern_line, line.strip()) + if not ret: continue + (stype, sname, sid, _, scomment, _, shomedir, _, sshell) = ret.groups() + if stype == 'u': + useradd_command = ['useradd'] + if sid != '-': + useradd_command.extend(['--uid', sid]) + if scomment and scomment != '-': + useradd_command.extend(['--comment', scomment]) + if shomedir and shomedir != '-': + useradd_command.extend(['--root', IMAGE_ROOTFS + shomedir]) + else: + useradd_command.extend(['--root', IMAGE_ROOTFS]) + if sshell and sshell != '-': + useradd_command.extend(['--shell', sshell]) + else: + useradd_command.extend(['--shell', '/sbin/nologin']) + useradd_command.extend(['--system', sname]) + subprocess.run(useradd_command) + elif stype == 'g': + groupadd_command = ['groupadd'] + if sid != '-': + groupadd_command.extend(['--gid', sid]) + groupadd_command.extend(['--system', sname]) + subprocess.run(groupadd_command) + elif stype == 'm': + subprocess.run(['groupadd', '--root', IMAGE_ROOTFS, '--system', sid]) + subprocess.run(['useradd', '--root', IMAGE_ROOTFS, '--shell', '/sbin/nologin', '--system', name, 'no-user-group']) + subprocess.run(['usermod', '-a', '-G', sid, sname]) } # On 05/06/2023 17:55, Louis Rannou wrote:> Hello, > > I have found an issue in the rootfs routine. The > rootfs-postcommands.bbclass has a funtion systemd_create_users that > reads /etc/sysusers.d/*.conf files and parses lines as 'type name id > comment'. > > However, the sysusers.d manual says, those lines can be 'type name id > comment home_dir shell'. If a home directory of shell is defined, they > are considered as part of the comment, and we run incorrect commands > such as the one below : > > useradd --shell /sbin/nologin --uid 0 --comment "Super User" /root > --system root > > To fix that, we require a stronger parsing. Several options look > possible to me, but I am not sure which one is preferred. > > 1. sed with a regular expression that returns something that still needs > parsing > 2. awk with a step by step script that returns something that still > needs to be parsed > 3. use python and regexp module > > Also I don't know if the parsing should completely check the sysusers > syntax as said in the manual (first field is [urgm], second is > alphanum_-, etc.). In my opinion it should not as this will be made by > the useradd command. > > Do you think it worth to add some testing about that ? I am not sure how > to do that. > > Regards, > Louis Rannou ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [OE-core] [RFC] incorrect parsing of sysusers.d in rootfs generation 2023-06-07 15:16 ` Louis Rannou @ 2023-06-08 9:19 ` Richard Purdie 2023-06-08 10:28 ` Louis Rannou 0 siblings, 1 reply; 7+ messages in thread From: Richard Purdie @ 2023-06-08 9:19 UTC (permalink / raw) To: Louis Rannou, tgamblin, openembedded-core On Wed, 2023-06-07 at 17:16 +0200, Louis Rannou wrote: > Hello again, > > a python solution could be one below. > > Also, I found that most of users/groups defined there are redundant as > they already exist (such as root). I guess they are defined from > base-passwd. I am not sure which recipe (base-passwd or systemd) should > have the precedence on this. If it's base-passwd, perhaps this > postcommand should check first if the user does already exist. I'd say base-passwd should likely be the winner but we should probably error if there is a conflict between what sysusers.d and base-passwd are defining. Cheers, Richard ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [OE-core] [RFC] incorrect parsing of sysusers.d in rootfs generation 2023-06-08 9:19 ` [OE-core] " Richard Purdie @ 2023-06-08 10:28 ` Louis Rannou 2023-06-08 10:36 ` Richard Purdie 0 siblings, 1 reply; 7+ messages in thread From: Louis Rannou @ 2023-06-08 10:28 UTC (permalink / raw) To: Richard Purdie, tgamblin, openembedded-core Hello, On 08/06/2023 11:19, Richard Purdie wrote: > On Wed, 2023-06-07 at 17:16 +0200, Louis Rannou wrote: >> Hello again, >> >> a python solution could be one below. >> >> Also, I found that most of users/groups defined there are redundant as >> they already exist (such as root). I guess they are defined from >> base-passwd. I am not sure which recipe (base-passwd or systemd) should >> have the precedence on this. If it's base-passwd, perhaps this >> postcommand should check first if the user does already exist. > > I'd say base-passwd should likely be the winner but we should probably > error if there is a conflict between what sysusers.d and base-passwd > are defining. There will be some conflict. As an example the root home directory in sysusers.d is /root. Some others users/groups defined in sysusers.d files are already created in recipes with the useradd class (such as systemd-resolved, systemd-network who are defined in the systemd recipe). In the end, almost all users/groups in sysusers.d/* already exist. Perhaps it's a wrong way to parse this file to add users. In my opinion, we should only parse this file to check users/groups are already created and raise an error if one is missing. Louis ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [OE-core] [RFC] incorrect parsing of sysusers.d in rootfs generation 2023-06-08 10:28 ` Louis Rannou @ 2023-06-08 10:36 ` Richard Purdie 2023-06-08 11:56 ` Louis Rannou 0 siblings, 1 reply; 7+ messages in thread From: Richard Purdie @ 2023-06-08 10:36 UTC (permalink / raw) To: Louis Rannou, tgamblin, openembedded-core On Thu, 2023-06-08 at 12:28 +0200, Louis Rannou wrote: > Hello, > > On 08/06/2023 11:19, Richard Purdie wrote: > > On Wed, 2023-06-07 at 17:16 +0200, Louis Rannou wrote: > > > Hello again, > > > > > > a python solution could be one below. > > > > > > Also, I found that most of users/groups defined there are redundant as > > > they already exist (such as root). I guess they are defined from > > > base-passwd. I am not sure which recipe (base-passwd or systemd) should > > > have the precedence on this. If it's base-passwd, perhaps this > > > postcommand should check first if the user does already exist. > > > > I'd say base-passwd should likely be the winner but we should probably > > error if there is a conflict between what sysusers.d and base-passwd > > are defining. > There will be some conflict. As an example the root home directory in > sysusers.d is /root. > > Some others users/groups defined in sysusers.d files are already created > in recipes with the useradd class (such as systemd-resolved, > systemd-network who are defined in the systemd recipe). > > In the end, almost all users/groups in sysusers.d/* already exist. > Perhaps it's a wrong way to parse this file to add users. In my opinion, > we should only parse this file to check users/groups are already created > and raise an error if one is missing. Perhaps lets start there. The fact different bits of systemd are configured with different home directories is a problem we should really fix though and ultimately that probably should be an error too. Cheers, Richard ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [OE-core] [RFC] incorrect parsing of sysusers.d in rootfs generation 2023-06-08 10:36 ` Richard Purdie @ 2023-06-08 11:56 ` Louis Rannou 2023-06-09 11:06 ` Louis Rannou 0 siblings, 1 reply; 7+ messages in thread From: Louis Rannou @ 2023-06-08 11:56 UTC (permalink / raw) To: Richard Purdie, tgamblin, openembedded-core On 08/06/2023 12:36, Richard Purdie wrote: > On Thu, 2023-06-08 at 12:28 +0200, Louis Rannou wrote: >> Hello, >> >> On 08/06/2023 11:19, Richard Purdie wrote: >>> On Wed, 2023-06-07 at 17:16 +0200, Louis Rannou wrote: >>>> Hello again, >>>> >>>> a python solution could be one below. >>>> >>>> Also, I found that most of users/groups defined there are redundant as >>>> they already exist (such as root). I guess they are defined from >>>> base-passwd. I am not sure which recipe (base-passwd or systemd) should >>>> have the precedence on this. If it's base-passwd, perhaps this >>>> postcommand should check first if the user does already exist. >>> >>> I'd say base-passwd should likely be the winner but we should probably >>> error if there is a conflict between what sysusers.d and base-passwd >>> are defining. >> There will be some conflict. As an example the root home directory in >> sysusers.d is /root. >> >> Some others users/groups defined in sysusers.d files are already created >> in recipes with the useradd class (such as systemd-resolved, >> systemd-network who are defined in the systemd recipe). >> >> In the end, almost all users/groups in sysusers.d/* already exist. >> Perhaps it's a wrong way to parse this file to add users. In my opinion, >> we should only parse this file to check users/groups are already created >> and raise an error if one is missing. > > Perhaps lets start there. The fact different bits of systemd are > configured with different home directories is a problem we should > really fix though and ultimately that probably should be an error too. Concerning the root home directory, I asked a question to systemd https://github.com/systemd/systemd/issues/27959 and LP answered he doesn't want to support a customizable root home directory. Should we patch systemd to match our configuration, or should we change our configuration to match systemd's will... Louis ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [OE-core] [RFC] incorrect parsing of sysusers.d in rootfs generation 2023-06-08 11:56 ` Louis Rannou @ 2023-06-09 11:06 ` Louis Rannou 0 siblings, 0 replies; 7+ messages in thread From: Louis Rannou @ 2023-06-09 11:06 UTC (permalink / raw) To: Richard Purdie, tgamblin, openembedded-core; +Cc: Chen Qi Hello, below is a counter proposition including some work in the systemd recipe, On 08/06/2023 13:56, Louis Rannou wrote: > > > On 08/06/2023 12:36, Richard Purdie wrote: >> On Thu, 2023-06-08 at 12:28 +0200, Louis Rannou wrote: >>> Hello, >>> >>> On 08/06/2023 11:19, Richard Purdie wrote: >>>> On Wed, 2023-06-07 at 17:16 +0200, Louis Rannou wrote: >>>>> Hello again, >>>>> >>>>> a python solution could be one below. >>>>> >>>>> Also, I found that most of users/groups defined there are redundant as >>>>> they already exist (such as root). I guess they are defined from >>>>> base-passwd. I am not sure which recipe (base-passwd or systemd) >>>>> should >>>>> have the precedence on this. If it's base-passwd, perhaps this >>>>> postcommand should check first if the user does already exist. >>>> >>>> I'd say base-passwd should likely be the winner but we should probably >>>> error if there is a conflict between what sysusers.d and base-passwd >>>> are defining. >>> There will be some conflict. As an example the root home directory in >>> sysusers.d is /root. >>> >>> Some others users/groups defined in sysusers.d files are already created >>> in recipes with the useradd class (such as systemd-resolved, >>> systemd-network who are defined in the systemd recipe). >>> >>> In the end, almost all users/groups in sysusers.d/* already exist. >>> Perhaps it's a wrong way to parse this file to add users. In my opinion, >>> we should only parse this file to check users/groups are already created >>> and raise an error if one is missing. >> >> Perhaps lets start there. The fact different bits of systemd are >> configured with different home directories is a problem we should >> really fix though and ultimately that probably should be an error too. > > Concerning the root home directory, I asked a question to systemd > https://github.com/systemd/systemd/issues/27959 and LP answered he > doesn't want to support a customizable root home directory. > > Should we patch systemd to match our configuration, or should we change > our configuration to match systemd's will... > > Louis The more I look at this and at #9497 (which gives the reason of this command), the more I think the problem was not correctly handled. sysusers.d is made to create missing users at runtime with the systemd-sysusers service. First of all, that means there is a wrong runtime configuration as long as /etc/{passwd, gpasswd, shadow, gshadow} do not match sysusers.d/*.conf I think it is fine to check during the build that sysusers.d/*.conf do match configuration for users/groups created during build, but we should not create users and groups at this stage because missing users will be created at runtime. I think sysusers/*.conf should be provided with the systemd recipe so they match our configuration (such as ROOT_HOME for the root directory). If you are ok with this, I may suggest a patch. Regards, Louis ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-06-09 11:06 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-06-05 15:55 [RFC] incorrect parsing of sysusers.d in rootfs generation Louis Rannou 2023-06-07 15:16 ` Louis Rannou 2023-06-08 9:19 ` [OE-core] " Richard Purdie 2023-06-08 10:28 ` Louis Rannou 2023-06-08 10:36 ` Richard Purdie 2023-06-08 11:56 ` Louis Rannou 2023-06-09 11:06 ` Louis Rannou
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).