Re: [PATCH] sshd-regen-keys: Improve service, make more robust

[Henning Schild <henning.schild@siemens.com>]

Am Fri, 26 Mar 2021 10:44:39 +0100
schrieb Harald Seiler <hws@denx.de>:

> Hi,
> 
> On Fri, 2021-03-26 at 09:11 +0100, Henning Schild wrote:
> > Switch to using "/usr/bin/ssh-keygen -A" instead of
> > dpkg-reconfigure. With this we would generate new host keys every
> > time the service starts and no keys exist. Removing the keys from
> > openssh-server in a postinst makes it complete so that we really
> > only generate on the first boot.
> > 
> > This is easier to handle that reusing the debian package hooks for
> > key generation.  
> 
> Yes, this is a _much_ more robust solution, I agree.  The debian hooks
> were a mess to deal with and we had so many edge cases over time that
> not relying on them here is a much better choice.  This also means
> the package would now work on a target where dpkg was removed for
> size constraints.

Thanks for the positive review.

@Jan did you get around testing this for your use-case?

Henning

> > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > ---
> >  .../sshd-regen-keys/files/postinst            |  2 ++
> >  .../files/sshd-regen-keys.service             |  4 +---
> >  .../sshd-regen-keys/files/sshd-regen-keys.sh  | 20
> > ------------------- .../sshd-regen-keys/sshd-regen-keys_0.3.bb    |
> > 17 ---------------- .../sshd-regen-keys/sshd-regen-keys_0.4.bb    |
> > 14 +++++++++++++ 5 files changed, 17 insertions(+), 40 deletions(-)
> >  delete mode 100644
> > meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.sh
> > delete mode 100644
> > meta/recipes-support/sshd-regen-keys/sshd-regen-keys_0.3.bb create
> > mode 100644
> > meta/recipes-support/sshd-regen-keys/sshd-regen-keys_0.4.bb
> > 
> > diff --git a/meta/recipes-support/sshd-regen-keys/files/postinst
> > b/meta/recipes-support/sshd-regen-keys/files/postinst index
> > ae722a7349a2..1c9b03e3e040 100644 ---
> > a/meta/recipes-support/sshd-regen-keys/files/postinst +++
> > b/meta/recipes-support/sshd-regen-keys/files/postinst @@ -1,4 +1,6
> > @@ #!/bin/sh
> >  set -e
> >  
> > 
> > +rm /etc/ssh/ssh_host_*_key*
> > +  
> 
> Just to make sure, this will always run after the openssh-server
> postinst which initially generates the keys?
> 
> >  systemctl enable sshd-regen-keys.service
> > diff --git
> > a/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.service
> > b/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.service
> > index f50d34c820d8..af98d5e9e966 100644 ---
> > a/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.service
> > +++
> > b/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.service
> > @@ -9,9 +9,7 @@ ConditionPathIsReadWrite=/etc [Service]
> > Type=oneshot RemainAfterExit=yes
> > -Environment=DEBIAN_FRONTEND=noninteractive
> > -ExecStart=/usr/sbin/sshd-regen-keys.sh
> > -ExecStartPost=-/bin/systemctl disable sshd-regen-keys.service
> > +ExecStart=/usr/bin/ssh-keygen -A StandardOutput=syslog
> >  StandardError=syslog  
> 
> This is also much cleaner because it no longer relies on the "self
> disabling service hack".  Much preferred!  Not sure if worth it,
> because ssh-keygen already ignores existing keys, but maybe we could
> add some
> 
>   ConditionPathExists=|!/etc/ssh/ssh_host_ecdsa_key
>   ConditionPathExists=|!/etc/ssh/ssh_host_ed25519_key
>   ConditionPathExists=|!/etc/ssh/ssh_host_rsa_key
> 
> (== systemd will skip the unit if all keys are present).  This would
> also hide the service in the startup log when all keys exist where it
> would otherwise show up unconditionally.
> 
> > diff --git
> > a/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.sh
> > b/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.sh
> > deleted file mode 100644 index 910d879ba51f..000000000000 ---
> > a/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.sh +++
> > /dev/null @@ -1,20 +0,0 @@
> > -#!/usr/bin/env sh
> > -
> > -echo -n "SSH server is "
> > -if systemctl is-enabled ssh; then
> > -    SSHD_ENABLED="true"
> > -    systemctl disable --no-reload ssh
> > -fi
> > -
> > -echo "Removing keys ..."
> > -rm -v /etc/ssh/ssh_host_*_key*
> > -
> > -echo "Regenerating keys ..."
> > -dpkg-reconfigure openssh-server
> > -
> > -if test -n $SSHD_ENABLED; then
> > -    echo "Reenabling ssh server ..."
> > -    systemctl enable --no-reload ssh
> > -fi
> > -
> > -sync
> > diff --git
> > a/meta/recipes-support/sshd-regen-keys/sshd-regen-keys_0.3.bb
> > b/meta/recipes-support/sshd-regen-keys/sshd-regen-keys_0.3.bb
> > deleted file mode 100644 index 6f12414239a3..000000000000 ---
> > a/meta/recipes-support/sshd-regen-keys/sshd-regen-keys_0.3.bb +++
> > /dev/null @@ -1,17 +0,0 @@
> > -# This software is a part of ISAR.
> > -inherit dpkg-raw
> > -
> > -DESCRIPTION = "Systemd service to regenerate sshd keys"
> > -MAINTAINER = "isar-users <isar-users@googlegroups.com>"
> > -DEBIAN_DEPENDS = "openssh-server, systemd"
> > -
> > -SRC_URI = "file://postinst \
> > -           file://sshd-regen-keys.service \
> > -           file://sshd-regen-keys.sh"
> > -
> > -do_install[cleandirs] = "${D}/lib/systemd/system \
> > -                         ${D}/usr/sbin"
> > -do_install() {
> > -    install -v -m 644 "${WORKDIR}/sshd-regen-keys.service"
> > "${D}/lib/systemd/system/sshd-regen-keys.service"
> > -    install -v -m 755 "${WORKDIR}/sshd-regen-keys.sh"
> > "${D}/usr/sbin/sshd-regen-keys.sh" -}
> > diff --git
> > a/meta/recipes-support/sshd-regen-keys/sshd-regen-keys_0.4.bb
> > b/meta/recipes-support/sshd-regen-keys/sshd-regen-keys_0.4.bb new
> > file mode 100644 index 000000000000..8b1cd8d4aba0 --- /dev/null
> > +++ b/meta/recipes-support/sshd-regen-keys/sshd-regen-keys_0.4.bb
> > @@ -0,0 +1,14 @@
> > +# This software is a part of ISAR.
> > +inherit dpkg-raw
> > +
> > +DESCRIPTION = "Systemd service to regenerate sshd keys"
> > +MAINTAINER = "isar-users <isar-users@googlegroups.com>"
> > +DEBIAN_DEPENDS = "openssh-server, systemd"
> > +
> > +SRC_URI = "file://postinst \
> > +           file://sshd-regen-keys.service"
> > +
> > +do_install() {
> > +    install -m 0755 "${D}/lib/systemd/system"
> > +    install -m 0644 "${WORKDIR}/sshd-regen-keys.service"
> > "${D}/lib/systemd/system/sshd-regen-keys.service" +}
> > -- 
> > 2.26.3  
> 
> Otherwise:
> 
> Reviewed-by: Harald Seiler <hws@denx.de>
>