| Message ID | 20180216181016.8747-1-chris.lesiak@licor.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] package/openssh: Add sysusers.d snippet | expand |
Chris, All, Sorry for thr huge delay in replying to this patch of your... On 2018-02-16 12:10 -0600, Chris Lesiak spake thusly: > Signed-off-by: Chris Lesiak <chris.lesiak@licor.com> > diff --git a/package/openssh/openssh.mk b/package/openssh/openssh.mk > index 6b7ac22c19..5d099ceb13 100644 > --- a/package/openssh/openssh.mk > +++ b/package/openssh/openssh.mk > @@ -60,12 +60,20 @@ else > OPENSSH_CONF_OPTS += --without-selinux > endif > > +ifeq ($(BR2_PACKAGE_SYSTEMD_SYSUSERS),y) > +define OPENSSH_INSTALL_SYSTEMD_SYSUSERS > + $(INSTALL) -m 0644 -D package/openssh/sshd_sysusers.conf \ > + $(TARGET_DIR)/usr/lib/sysusers.d/sshd.conf > +endef > +endif > + > define OPENSSH_INSTALL_INIT_SYSTEMD > $(INSTALL) -D -m 644 package/openssh/sshd.service \ > $(TARGET_DIR)/usr/lib/systemd/system/sshd.service > mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants > ln -fs ../../../../usr/lib/systemd/system/sshd.service \ > $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/sshd.service > + $(OPENSSH_INSTALL_SYSTEMD_SYSUSERS) > endef > > define OPENSSH_INSTALL_INIT_SYSV > diff --git a/package/openssh/sshd_sysusers.conf b/package/openssh/sshd_sysusers.conf > new file mode 100644 > index 0000000000..3ea46f65c6 > --- /dev/null > +++ b/package/openssh/sshd_sysusers.conf > @@ -0,0 +1,5 @@ > +# sysusers.d snippet for creating the sshd system user automatically > +# at boot on systemd-based systems that ship with an unpopulated > +# /etc. See sysusers.d(5) for details. No need for this boilerplate (which ends up being much bigger than the actual content...) > +u sshd - "Privilege-separated SSH" We've discussed this a bit with Thomas, and there is one thing that we did not like much, is that it is not integrated nicely in the existing users support in Buildroot. Shouldn't we have a generic mechanism, that takes all the FOO_USERS, and turns them into sysusers.d(%) entries? Maybe something like: define SYSTEMD_SYSUSERS mkdir -p $(TARGET_DIR)/usr/lib/sysusers.d/ echo "$(PACKAGES_USERS)" \ |while read user uid group gid passwd home shell groups comment; do printf "u %s %s %s\n" "${user}" "${uid}" "${comment}" done >$(TARGET_DIR)/usr/lib/sysusers.d/buildroot.conf # And similarly for groups... endef SYSTEMD_POST_TARGET_FINALIZE_HOOKS = SYSTEMD_SYSUSERS Regards, Yann E. MORIN.
Yann, Thanks for the review. On 12/16/18 7:45 AM, Yann E. MORIN wrote: > Chris, All, > > Sorry for thr huge delay in replying to this patch of your... > > On 2018-02-16 12:10 -0600, Chris Lesiak spake thusly: >> Signed-off-by: Chris Lesiak <chris.lesiak@licor.com> >> diff --git a/package/openssh/openssh.mk b/package/openssh/openssh.mk >> index 6b7ac22c19..5d099ceb13 100644 >> --- a/package/openssh/openssh.mk >> +++ b/package/openssh/openssh.mk >> @@ -60,12 +60,20 @@ else >> OPENSSH_CONF_OPTS += --without-selinux >> endif >> >> +ifeq ($(BR2_PACKAGE_SYSTEMD_SYSUSERS),y) >> +define OPENSSH_INSTALL_SYSTEMD_SYSUSERS >> + $(INSTALL) -m 0644 -D package/openssh/sshd_sysusers.conf \ >> + $(TARGET_DIR)/usr/lib/sysusers.d/sshd.conf >> +endef >> +endif >> + >> define OPENSSH_INSTALL_INIT_SYSTEMD >> $(INSTALL) -D -m 644 package/openssh/sshd.service \ >> $(TARGET_DIR)/usr/lib/systemd/system/sshd.service >> mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants >> ln -fs ../../../../usr/lib/systemd/system/sshd.service \ >> $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/sshd.service >> + $(OPENSSH_INSTALL_SYSTEMD_SYSUSERS) >> endef >> >> define OPENSSH_INSTALL_INIT_SYSV >> diff --git a/package/openssh/sshd_sysusers.conf b/package/openssh/sshd_sysusers.conf >> new file mode 100644 >> index 0000000000..3ea46f65c6 >> --- /dev/null >> +++ b/package/openssh/sshd_sysusers.conf >> @@ -0,0 +1,5 @@ >> +# sysusers.d snippet for creating the sshd system user automatically >> +# at boot on systemd-based systems that ship with an unpopulated >> +# /etc. See sysusers.d(5) for details. > No need for this boilerplate (which ends up being much bigger than the > actual content...) > >> +u sshd - "Privilege-separated SSH" > We've discussed this a bit with Thomas, and there is one thing that we > did not like much, is that it is not integrated nicely in the existing > users support in Buildroot. > > Shouldn't we have a generic mechanism, that takes all the FOO_USERS, and > turns them into sysusers.d(%) entries? Maybe something like: > > define SYSTEMD_SYSUSERS > mkdir -p $(TARGET_DIR)/usr/lib/sysusers.d/ > echo "$(PACKAGES_USERS)" \ > |while read user uid group gid passwd home shell groups comment; do > printf "u %s %s %s\n" "${user}" "${uid}" "${comment}" > done >$(TARGET_DIR)/usr/lib/sysusers.d/buildroot.conf > # And similarly for groups... > endef > SYSTEMD_POST_TARGET_FINALIZE_HOOKS = SYSTEMD_SYSUSERS > > Regards, > Yann E. MORIN. > That looks like a good idea, but I don't know how to handle upstream packages that already create sysusers.d drop-ins. Examples that I know of from my own build include: systemd - Creates basic.conf, systemd.conf, and systemd-remote.conf dbus - Creates dbus.conf Is there a reason (other than storage cost) to prefer a single buildroot.conf drop-in file instead of one per package? Sincerely, Chris Lesiak
Chris, All, On 2018-12-17 15:07 +0000, Chris Lesiak spake thusly: > On 12/16/18 7:45 AM, Yann E. MORIN wrote: > > On 2018-02-16 12:10 -0600, Chris Lesiak spake thusly: > >> Signed-off-by: Chris Lesiak <chris.lesiak@licor.com> [--SNIP--] > >> diff --git a/package/openssh/sshd_sysusers.conf b/package/openssh/sshd_sysusers.conf > >> new file mode 100644 > >> index 0000000000..3ea46f65c6 > >> --- /dev/null > >> +++ b/package/openssh/sshd_sysusers.conf [--SNIP--] > >> +u sshd - "Privilege-separated SSH" > > We've discussed this a bit with Thomas, and there is one thing that we > > did not like much, is that it is not integrated nicely in the existing > > users support in Buildroot. > > > > Shouldn't we have a generic mechanism, that takes all the FOO_USERS, and > > turns them into sysusers.d(%) entries? Maybe something like: > > > > define SYSTEMD_SYSUSERS > > mkdir -p $(TARGET_DIR)/usr/lib/sysusers.d/ > > echo "$(PACKAGES_USERS)" \ > > |while read user uid group gid passwd home shell groups comment; do > > printf "u %s %s %s\n" "${user}" "${uid}" "${comment}" > > done >$(TARGET_DIR)/usr/lib/sysusers.d/buildroot.conf > > # And similarly for groups... > > endef > > SYSTEMD_POST_TARGET_FINALIZE_HOOKS = SYSTEMD_SYSUSERS > > > > Regards, > > Yann E. MORIN. > > > That looks like a good idea, but I don't know how to handle upstream > packages that already create sysusers.d drop-ins. > > Examples that I know of from my own build include: > systemd - Creates basic.conf, systemd.conf, and systemd-remote.conf > dbus - Creates dbus.conf > > > Is there a reason (other than storage cost) to prefer a single > buildroot.conf drop-in file instead of one per package? Well, a file takes an inode, which takes some space, so that's that. But if one goes with systemd, then the number of inodes is probably irrelevant. And with the above, all users of all packages are in the PACKAGES_USERS variable, but there is no way to track them back to the corresponding packages. Currently, the set of users created by FOO_USERS and the set of users created by sysusers.d files is not consistent. Your proposed patch fixes it for openssh only, but: - the user definition is duplicated: one in the .mk, one in the sysusers.d file, so becomes a maintenacne burden (e.g. should we need to create anotehr user for it, for example) - other packages are left out in the cold. So, I'd like we find a solution so that the set of users installed in /etc/paswd and the set of users created by sysusers.d are identical. I don't have a good suggestion, though... :-/ Regards, Yann E. MORIN.
On 12/17/18 12:13 PM, Yann E. MORIN wrote: > [--SNIP--] > > Currently, the set of users created by FOO_USERS and the set of users > created by sysusers.d files is not consistent. Your proposed patch fixes > it for openssh only, but: > > - the user definition is duplicated: one in the .mk, one in the > sysusers.d file, so becomes a maintenacne burden (e.g. should we > need to create anotehr user for it, for example) > > - other packages are left out in the cold. > > So, I'd like we find a solution so that the set of users installed in > /etc/paswd and the set of users created by sysusers.d are identical. > > I don't have a good suggestion, though... :-/ > > Regards, > Yann E. MORIN. I haven't tried this yet, but maybe the systemd Dynamic Users feature is a better solution. See: http://0pointer.net/blog/dynamic-users-with-systemd.html The user name would still appear in two places (the make fragment and the service file), but it could eliminate the need for both the sysusers.d and tmpfiles.d snippets.
On 17/12/2018 19:13, Yann E. MORIN wrote: > Chris, All, > > On 2018-12-17 15:07 +0000, Chris Lesiak spake thusly: >> On 12/16/18 7:45 AM, Yann E. MORIN wrote: >>> On 2018-02-16 12:10 -0600, Chris Lesiak spake thusly: >>>> Signed-off-by: Chris Lesiak <chris.lesiak@licor.com> > [--SNIP--] >>>> diff --git a/package/openssh/sshd_sysusers.conf b/package/openssh/sshd_sysusers.conf >>>> new file mode 100644 >>>> index 0000000000..3ea46f65c6 >>>> --- /dev/null >>>> +++ b/package/openssh/sshd_sysusers.conf > [--SNIP--] >>>> +u sshd - "Privilege-separated SSH" >>> We've discussed this a bit with Thomas, and there is one thing that we >>> did not like much, is that it is not integrated nicely in the existing >>> users support in Buildroot. >>> >>> Shouldn't we have a generic mechanism, that takes all the FOO_USERS, and >>> turns them into sysusers.d(%) entries? Maybe something like: >>> >>> define SYSTEMD_SYSUSERS >>> mkdir -p $(TARGET_DIR)/usr/lib/sysusers.d/ >>> echo "$(PACKAGES_USERS)" \ >>> |while read user uid group gid passwd home shell groups comment; do >>> printf "u %s %s %s\n" "${user}" "${uid}" "${comment}" Obviously, we also want to add the comment, home and shell to the conf file. >>> done >$(TARGET_DIR)/usr/lib/sysusers.d/buildroot.conf >>> # And similarly for groups... >>> endef >>> SYSTEMD_POST_TARGET_FINALIZE_HOOKS = SYSTEMD_SYSUSERS >>> >>> Regards, >>> Yann E. MORIN. >>> >> That looks like a good idea, but I don't know how to handle upstream >> packages that already create sysusers.d drop-ins. >> >> Examples that I know of from my own build include: >> systemd - Creates basic.conf, systemd.conf, and systemd-remote.conf >> dbus - Creates dbus.conf >> >> >> Is there a reason (other than storage cost) to prefer a single >> buildroot.conf drop-in file instead of one per package? > > Well, a file takes an inode, which takes some space, so that's that. > But if one goes with systemd, then the number of inodes is probably > irrelevant. If we're making a single buildroot.conf file, what exactly does the sysusers.d approach bring over traditional passwd? The unly reason of existence of this feature (AFAICS) is to allow packages to create users by simply creating a file instead of editing passwd. Which brings me to my question to Chris: what was the purpose of this patch to begin with? Since OPENSSH_USERS is already set, the sshd user will already exist in /etc/passwd, so the sysusers.d directive will be ignored... Either that, or our mkusers script doesn't work correctly. The only reason why you'd want this is to support the creation of installable packages with Buildroot (which we officially don't support, but several people actually do that). I think this is a good reason, but it should be mentioned in the commit message. > And with the above, all users of all packages are in the PACKAGES_USERS > variable, but there is no way to track them back to the corresponding > packages. Anything is possible, with enough infra work :-) In this case, it could be something like updating the _USERS support in pkg-generic.mk to: ifneq ($$($(2)_USERS),) ifeq ($$(BR2_PACKAGE_SYSTEMD_SYSUSERS),y) $(2)_POST_INSTALL_TARGET_HOOKS += SYSTEMD_INSTALL_PKG_SYSUSERS_CONF else PACKAGES_USERS += $$($(2)_USERS)$$(sep) endif endif with define SYSTEMD_INSTALL_PKG_SYSUSERS_CONF mkdir -p $(TARGET_DIR)/usr/lib/sysusers.d/ echo "$($(PKG)_USERS)" \ | while read user uid group gid passwd home shell groups comment; do printf 'u %s %s "%s"\n' "${user}" "${uid}" "${comment}" "$home" "$shell" done >$(TARGET_DIR)/usr/lib/sysusers.d/$($(PKG)_NAME).conf # And similarly for groups... endef There's a slight complication though: what if the package does install a sysusers.d file? In that case, we don't want to create one from the Buildroot infra, but we still want to create a user in the non-systemd case... > > Currently, the set of users created by FOO_USERS and the set of users > created by sysusers.d files is not consistent. Your proposed patch fixes > it for openssh only, but: > > - the user definition is duplicated: one in the .mk, one in the > sysusers.d file, so becomes a maintenacne burden (e.g. should we > need to create anotehr user for it, for example) Well, no, as I wrote above: it makes no sense to add a user both to passwd and to sysusers.d, since the sysusers.d will just go ignored. > - other packages are left out in the cold. Absolutely, there is no point at all to do this just for sshd. Regards, Arnout > > So, I'd like we find a solution so that the set of users installed in > /etc/paswd and the set of users created by sysusers.d are identical. > > I don't have a good suggestion, though... :-/ > > Regards, > Yann E. MORIN. >
Hello, On Mon, 17 Dec 2018 23:59:04 +0100, Arnout Vandecappelle wrote: > Which brings me to my question to Chris: what was the purpose of this patch to > begin with? Since OPENSSH_USERS is already set, the sshd user will already exist > in /etc/passwd, so the sysusers.d directive will be ignored... Either that, or > our mkusers script doesn't work correctly. Stateless systems, where /etc and /var don't even "exist" in a persistent fashion, and are entirely populated at boot time. Best regards, Thomas
On 12/18/18 1:49 AM, Thomas Petazzoni wrote: > Hello, > > On Mon, 17 Dec 2018 23:59:04 +0100, Arnout Vandecappelle wrote: > >> Which brings me to my question to Chris: what was the purpose of this patch to >> begin with? Since OPENSSH_USERS is already set, the sshd user will already exist >> in /etc/passwd, so the sysusers.d directive will be ignored... Either that, or >> our mkusers script doesn't work correctly. > Stateless systems, where /etc and /var don't even "exist" in a > persistent fashion, and are entirely populated at boot time. > > Best regards, > > Thomas In my particular case, /etc is empty on first boot, but retains state after that. Even if I started with a populated /etc/passwd, when updating /usr (switching in a new version to update the the OS), I might have new services with associated users that I would like to have automatically added to /etc/passwd.
On 18/12/2018 15:14, Chris Lesiak wrote: > > On 12/18/18 1:49 AM, Thomas Petazzoni wrote: >> Hello, >> >> On Mon, 17 Dec 2018 23:59:04 +0100, Arnout Vandecappelle wrote: >> >>> Which brings me to my question to Chris: what was the purpose of this patch to >>> begin with? Since OPENSSH_USERS is already set, the sshd user will already exist >>> in /etc/passwd, so the sysusers.d directive will be ignored... Either that, or >>> our mkusers script doesn't work correctly. >> Stateless systems, where /etc and /var don't even "exist" in a >> persistent fashion, and are entirely populated at boot time. >> >> Best regards, >> >> Thomas > > > In my particular case, /etc is empty on first boot, but retains state > after that. Even if I started with a populated /etc/passwd, when > updating /usr (switching in a new version to update the the OS), I might > have new services with associated users that I would like to have > automatically added to /etc/passwd. That's a very good motivation! So you're generating a rootfs with empty /etc? Do you need any other changes to make that work? To support this use case, it would be nice if we could add an option to the system menu that enables it (this would depend on systemd of course), so that it can be used fo force packages to do whatever is needed to support empty /etc. And then we can add a post-build check that /etc is really empty at the end of the build, so that the autobuilders can detect problems. Regards, Arnout
On 12/18/18 8:32 AM, Arnout Vandecappelle wrote: > > On 18/12/2018 15:14, Chris Lesiak wrote: > >> In my particular case, /etc is empty on first boot, but retains state >> after that. Even if I started with a populated /etc/passwd, when >> updating /usr (switching in a new version to update the the OS), I might >> have new services with associated users that I would like to have >> automatically added to /etc/passwd. > That's a very good motivation! > > So you're generating a rootfs with empty /etc? Do you need any other changes to > make that work? To support this use case, it would be nice if we could add an > option to the system menu that enables it (this would depend on systemd of > course), so that it can be used fo force packages to do whatever is needed to > support empty /etc. And then we can add a post-build check that /etc is really > empty at the end of the build, so that the autobuilders can detect problems. > > Regards, > Arnout I am happy that you are interested in this use case. I'll try to describe more fully what I am doing and give a summary of the issues I had to tackle. My system has four partitions, with some space reserved before the partitions for an MBR, u-boot, and u-boot environment. The first two partitions are for two instances of the root filesystem. This allows me to replace the image on the one I'm not currently running, rewrite the MBR so as to swap the two, and then reboot. I manage that with package/fwup. The first entry in the MBR (not necessarily the one at the lowest block address) is always the default root; the second is the old root. The third partition is for /etc and the fourth for /var. Both /etc and /var are empty on first boot and automatically populated. I have a small custom initramfs that is stored in the /boot directory of the root filesystem. This directory also contains a u-boot script, the kernel zImage and one or more device trees. This initramfs has the following features: * The initramfs is permanently used (there is no pivot-root) as a volatile /. * From the root device, mounts /usr on /usr (read-only) * From the root device, mounts /boot on /boot (read-only) * Mounts the x-mount.etc device (an additional cmdline argument) on /etc (read-write) You see that the only two directories on the root filesystem image that I use are /usr and /boot. When I do an update, I get a completely new /usr and /boot with u-boot script, device tree files, kernel zImage, and user-space programs all updated together. Mounting of /var could have been done in the initramfs, but since it can wait until later, I let systemd mount it using a unit file similar to tmp.mount. Without this, I'd have a volatile /var which is sometimes desired; I actually prefer that but have a requirement for persistent logs. Although /etc starts out empty and is automatically populated, I've taken some steps to keep even this to a minimum. The less that is in /etc, the less the chance that it will contain something incompatible with a future version of /usr. Many packages require their configuration to be in /etc; I'd rather my default configurations could live in /usr/lib with the ability to override with a file in /etc. Most packages don't support this so I've had to use various work-arounds. For avahi, I store its configuration in /usr/share/factory/etc/avahi and have a tmpfiles.d snippet to create a link from /etc/avahi. For sshd, I store my default configuration in /usr/share/factory/etc/ssh/, but use the tmpfiles.d snippet to copy from /usr/share/factory/etc/ssh to /etc/ssh. Both the keys and config files are stored in this directory, so it must be writable. I'd rather the configuration lived in /usr/lib and only the keys were kept in /etc. Then there is configuring various service to run without ending up with a bunch of links in /etc/systed/system. Using a rootfs overlay, I have a /usr/lib/systemd/system/buildroot.target.wants directory containing symbolic links to all of the services that I want to run. I make default.target link to that directory. That is similar to what systemd does with sysinit.target.wants and builtin systemd services. This allows me to have an /etc/systemd/ containing only an empty /etc/systemd/system/. Note that I can still disable these services using "systemctl mask". I think some build infrastructure to make this easy to configure would be nice.
Arnout, All, On 2018-12-17 23:59 +0100, Arnout Vandecappelle spake thusly: > On 17/12/2018 19:13, Yann E. MORIN wrote: > > Chris, All, > > > > On 2018-12-17 15:07 +0000, Chris Lesiak spake thusly: > >> On 12/16/18 7:45 AM, Yann E. MORIN wrote: > >>> On 2018-02-16 12:10 -0600, Chris Lesiak spake thusly: > >>>> Signed-off-by: Chris Lesiak <chris.lesiak@licor.com> > > [--SNIP--] > >>>> diff --git a/package/openssh/sshd_sysusers.conf b/package/openssh/sshd_sysusers.conf > >>>> new file mode 100644 > >>>> index 0000000000..3ea46f65c6 > >>>> --- /dev/null > >>>> +++ b/package/openssh/sshd_sysusers.conf > > [--SNIP--] > >>>> +u sshd - "Privilege-separated SSH" > >>> We've discussed this a bit with Thomas, and there is one thing that we > >>> did not like much, is that it is not integrated nicely in the existing > >>> users support in Buildroot. > >>> > >>> Shouldn't we have a generic mechanism, that takes all the FOO_USERS, and > >>> turns them into sysusers.d(%) entries? Maybe something like: > >>> > >>> define SYSTEMD_SYSUSERS > >>> mkdir -p $(TARGET_DIR)/usr/lib/sysusers.d/ > >>> echo "$(PACKAGES_USERS)" \ > >>> |while read user uid group gid passwd home shell groups comment; do > >>> printf "u %s %s %s\n" "${user}" "${uid}" "${comment}" > Obviously, we also want to add the comment, home and shell to the conf file. No, because the format of the sysusers.d files do not allow for that, see the man page. [--SNIP the reason for it: stateles systems--] > > And with the above, all users of all packages are in the PACKAGES_USERS > > variable, but there is no way to track them back to the corresponding > > packages. > > Anything is possible, with enough infra work :-) > > In this case, it could be something like updating the _USERS support in > pkg-generic.mk to: > > ifneq ($$($(2)_USERS),) > ifeq ($$(BR2_PACKAGE_SYSTEMD_SYSUSERS),y) > $(2)_POST_INSTALL_TARGET_HOOKS += SYSTEMD_INSTALL_PKG_SYSUSERS_CONF > else > PACKAGES_USERS += $$($(2)_USERS)$$(sep) > endif > endif > > with > > define SYSTEMD_INSTALL_PKG_SYSUSERS_CONF > mkdir -p $(TARGET_DIR)/usr/lib/sysusers.d/ > echo "$($(PKG)_USERS)" \ > | while read user uid group gid passwd home shell groups comment; do > printf 'u %s %s "%s"\n' "${user}" "${uid}" "${comment}" "$home" "$shell" > done >$(TARGET_DIR)/usr/lib/sysusers.d/$($(PKG)_NAME).conf > # And similarly for groups... > endef Yeah, but is it worth it? Afterall, we only need the users to be created at runtime. Do we need to track in separate files what packages installed what user? We do not have that separation in /etc/paswd, mind you! ;-) > There's a slight complication though: what if the package does install a > sysusers.d file? In that case, we don't want to create one from the Buildroot > infra, but we still want to create a user in the non-systemd case... That's the tricky part. We would need some target-finalize hook to do all that job: - for each user in PACKAGES_USERS, create a sysusers.d file if there is none yet, - for each user in sysusers.d, create an entry in /etc/passwd if there is none yet. > > Currently, the set of users created by FOO_USERS and the set of users > > created by sysusers.d files is not consistent. Your proposed patch fixes > > it for openssh only, but: > > > > - the user definition is duplicated: one in the .mk, one in the > > sysusers.d file, so becomes a maintenacne burden (e.g. should we > > need to create anotehr user for it, for example) > > Well, no, as I wrote above: it makes no sense to add a user both to passwd and > to sysusers.d, since the sysusers.d will just go ignored. And as Thomas and Chris explained: it does make sense, and now you saw the Light, so all is fine! ;-) Regards, Yann E. MORIN.
On 2018-12-18 14:14 +0000, Chris Lesiak spake thusly: > On 12/18/18 1:49 AM, Thomas Petazzoni wrote: > > On Mon, 17 Dec 2018 23:59:04 +0100, Arnout Vandecappelle wrote: > >> Which brings me to my question to Chris: what was the purpose of this patch to > >> begin with? Since OPENSSH_USERS is already set, the sshd user will already exist > >> in /etc/passwd, so the sysusers.d directive will be ignored... Either that, or > >> our mkusers script doesn't work correctly. > > Stateless systems, where /etc and /var don't even "exist" in a > > persistent fashion, and are entirely populated at boot time. > In my particular case, /etc is empty on first boot, but retains state > after that. Even if I started with a populated /etc/passwd, when > updating /usr (switching in a new version to update the the OS), I might > have new services with associated users that I would like to have > automatically added to /etc/passwd. So it can also serve for reset-factory situations, too, I guess? A trigger that wipes the filesystem backing /etc and reboots, and then you are in factory-pristine configuartion. Neat. Regards, Yann E. MORIN.
Hi Chris, On 18/12/2018 18:03, Chris Lesiak wrote: > Then there is configuring various service to run without ending up with > a bunch of links in /etc/systed/system. Using a rootfs overlay, I have > a /usr/lib/systemd/system/buildroot.target.wants directory containing > symbolic links to all of the services that I want to run. I make > default.target link to that directory. That is similar to what systemd > does with sysinit.target.wants and builtin systemd services. This > allows me to have an /etc/systemd/ containing only an empty > /etc/systemd/system/. Note that I can still disable these services > using "systemctl mask". I think some build infrastructure to make this > easy to configure would be nice. So IIUC in your specific use case, you're using the sysusers file but not /etc/passwd as generated by Buildroot, right? Which is why it actually works for you? Because the patch as you posted it is wrong AFAIU. Since OPENSSH_USERS is still defined, you end up with both a sysusers entry and a passwd entry. In that case, the passwd entry has priority and the sysusers entry is ignored. So, there should be a condition around the definition of OPENSSH_USERS as well to avoid both of them to be included. We also discussed at the Buildroot developer meeting about what the best approach for this feature is. We saw three options by adding some infra to support sysusers. However, unless a couple of packages use it, we're not going to add infra for it. So, once you've added the condition around OPENSSH_USERS, your patch is fine. Care to update and resubmit? Thanks, and sorry for the long delay and discussion. Regards, Arnout
diff --git a/package/openssh/openssh.mk b/package/openssh/openssh.mk index 6b7ac22c19..5d099ceb13 100644 --- a/package/openssh/openssh.mk +++ b/package/openssh/openssh.mk @@ -60,12 +60,20 @@ else OPENSSH_CONF_OPTS += --without-selinux endif +ifeq ($(BR2_PACKAGE_SYSTEMD_SYSUSERS),y) +define OPENSSH_INSTALL_SYSTEMD_SYSUSERS + $(INSTALL) -m 0644 -D package/openssh/sshd_sysusers.conf \ + $(TARGET_DIR)/usr/lib/sysusers.d/sshd.conf +endef +endif + define OPENSSH_INSTALL_INIT_SYSTEMD $(INSTALL) -D -m 644 package/openssh/sshd.service \ $(TARGET_DIR)/usr/lib/systemd/system/sshd.service mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants ln -fs ../../../../usr/lib/systemd/system/sshd.service \ $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/sshd.service + $(OPENSSH_INSTALL_SYSTEMD_SYSUSERS) endef define OPENSSH_INSTALL_INIT_SYSV diff --git a/package/openssh/sshd_sysusers.conf b/package/openssh/sshd_sysusers.conf new file mode 100644 index 0000000000..3ea46f65c6 --- /dev/null +++ b/package/openssh/sshd_sysusers.conf @@ -0,0 +1,5 @@ +# sysusers.d snippet for creating the sshd system user automatically +# at boot on systemd-based systems that ship with an unpopulated +# /etc. See sysusers.d(5) for details. + +u sshd - "Privilege-separated SSH"
Signed-off-by: Chris Lesiak <chris.lesiak@licor.com> --- Changes v1 --> v2 - Rename the snippet file to sshd_sysusers.conf to make the use more apparent. --- package/openssh/openssh.mk | 8 ++++++++ package/openssh/sshd_sysusers.conf | 5 +++++ 2 files changed, 13 insertions(+) create mode 100644 package/openssh/sshd_sysusers.conf