[Buildroot] [PATCH] package/openssh: Add sysusers.d snippet

[Buildroot] [PATCH] package/openssh: Add sysusers.d snippet

[Chris Lesiak]

Signed-off-by: Chris Lesiak <chris.lesiak@licor.com>
---
 package/openssh/openssh.mk | 8 ++++++++
 package/openssh/sshd.conf  | 5 +++++
 2 files changed, 13 insertions(+)
 create mode 100644 package/openssh/sshd.conf

diff --git a/package/openssh/openssh.mk b/package/openssh/openssh.mk
index 6b7ac22c19..fc82300ab7 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.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.conf b/package/openssh/sshd.conf
new file mode 100644
index 0000000000..3ea46f65c6
--- /dev/null
+++ b/package/openssh/sshd.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"
-- 
2.13.6

[Buildroot] [PATCH] package/openssh: Add sysusers.d snippet

[Samuel Martin]

Chris, all,

On Wed, Jan 24, 2018 at 12:22 AM, Chris Lesiak <chris.lesiak@licor.com> wrote:
> Signed-off-by: Chris Lesiak <chris.lesiak@licor.com>
> ---
>  package/openssh/openssh.mk | 8 ++++++++
>  package/openssh/sshd.conf  | 5 +++++
>  2 files changed, 13 insertions(+)
>  create mode 100644 package/openssh/sshd.conf
>
> diff --git a/package/openssh/openssh.mk b/package/openssh/openssh.mk
> index 6b7ac22c19..fc82300ab7 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.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.conf b/package/openssh/sshd.conf
> new file mode 100644
> index 0000000000..3ea46f65c6
> --- /dev/null
> +++ b/package/openssh/sshd.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"

Reviewed-by: Samuel Martin <s.martin49@gmail.com>

Regards,

-- 
Samuel

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[Chris Lesiak]

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

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"
-- 
2.14.3

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[Yann E. MORIN]

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 E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[Chris Lesiak]

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 Lesiak
Principal Design Engineer, Software
LI-COR Biosciences
4647 Superior Street
Lincoln, NE 68504 USA
chris.lesiak at licor.com

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[Yann E. MORIN]

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[Chris Lesiak]

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.

-- 
Chris Lesiak
Principal Design Engineer, Software
LI-COR Biosciences
4647 Superior Street
Lincoln, NE 68504 USA
chris.lesiak at licor.com

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[Arnout Vandecappelle]

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.
> 

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[Chris Lesiak]

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.

-- 
Chris Lesiak
Principal Design Engineer, Software
LI-COR Biosciences
4647 Superior Street
Lincoln, NE 68504 USA
chris.lesiak at licor.com

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[Arnout Vandecappelle]

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

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[Chris Lesiak]

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.

-- 
Chris Lesiak
Principal Design Engineer, Software
LI-COR Biosciences
4647 Superior Street
Lincoln, NE 68504 USA
chris.lesiak at licor.com

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[Arnout Vandecappelle]

 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

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2] package/openssh: Add sysusers.d snippet

[Yann E. MORIN]

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'