Re: [PATCH] openssh: Atomically generate host keys

Joshua Watt
2017-05-22T13:08:21Z

Could I get a review of this? I'm somewhat new to submitting patches,
so if I'm missing something I probably don't realize it, any feedback
is appreciated.

Thanks,
Joshua Watt

On Sat, May 6, 2017 at 8:33 PM, Joshua Watt <jpewhacker@gmail.com> wrote:
> Generating the host keys atomically prevents power interruptions during
> the first boot from leaving the key files incomplete, which often
> prevents users from being able to ssh into the device.
>
> Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
> ---
> meta/recipes-connectivity/openssh/openssh/init | 21 ++++------------
> .../openssh/openssh/sshd-check-key | 28 ++++++++++++++++++++++
> .../openssh/openssh/sshdgenkeys.service | 16 ++++---------
> meta/recipes-connectivity/openssh/openssh_7.4p1.bb | 8 +++++++
> 4 files changed, 44 insertions(+), 29 deletions(-)
> create mode 100644 meta/recipes-connectivity/openssh/openssh/sshd-check-key
>
> diff --git a/meta/recipes-connectivity/openssh/openssh/init b/meta/recipes-connectivity/openssh/openssh/init
> index 1f63725..22124a9 100644
> --- a/meta/recipes-connectivity/openssh/openssh/init
> +++ b/meta/recipes-connectivity/openssh/openssh/init
> @@ -45,23 +45,10 @@ check_config() {
> }
>
> check_keys() {
> - # create keys if necessary
> - if [ ! -f $HOST_KEY_RSA ]; then
> - echo " generating ssh RSA key..."
> - ssh-keygen -q -f $HOST_KEY_RSA -N '' -t rsa
> - fi
> - if [ ! -f $HOST_KEY_ECDSA ]; then
> - echo " generating ssh ECDSA key..."
> - ssh-keygen -q -f $HOST_KEY_ECDSA -N '' -t ecdsa
> - fi
> - if [ ! -f $HOST_KEY_DSA ]; then
> - echo " generating ssh DSA key..."
> - ssh-keygen -q -f $HOST_KEY_DSA -N '' -t dsa
> - fi
> - if [ ! -f $HOST_KEY_ED25519 ]; then
> - echo " generating ssh ED25519 key..."
> - ssh-keygen -q -f $HOST_KEY_ED25519 -N '' -t ed25519
> - fi
> + @LIBEXECDIR@/sshd-check-key $HOST_KEY_RSA rsa
> + @LIBEXECDIR@/sshd-check-key $HOST_KEY_ECDSA ecdsa
> + @LIBEXECDIR@/sshd-check-key $HOST_KEY_DSA dsa
> + @LIBEXECDIR@/sshd-check-key $HOST_KEY_ED25519 ed25519
> }
>
> export PATH="${PATH:+$PATH:}/usr/sbin:/sbin"
> diff --git a/meta/recipes-connectivity/openssh/openssh/sshd-check-key b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
> new file mode 100644
> index 0000000..3495d98
> --- /dev/null
> +++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
> @@ -0,0 +1,28 @@
> +#! /bin/sh
> +set -e
> +
> +NAME="$1"
> +TYPE="$2"
> +
> +if [ -z "$NAME" ] || [ -z "$TYPE" ]; then
> + echo "Usage: $0 NAME TYPE"
> + exit 1;
> +fi
> +
> +if [ ! -f "$NAME" ]; then
> + echo " generating ssh $TYPE key..."
> + ssh-keygen -q -f "${NAME}.tmp" -N '' -t $TYPE
> +
> + # Sync to ensure data is written to temp file before renaming
> + sync
> +
> + # Move (Atomically rename) files
> + # Rename the .pub file first, since the check that triggers a
> + # key generation is based on the private file.
> + mv -f "${NAME}.tmp.pub" "${NAME}.pub"
> + sync
> +
> + mv -f "${NAME}.tmp" "${NAME}"
> + sync
> +fi
> +
> diff --git a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> index 148e6ad..af56404 100644
> --- a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> +++ b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> @@ -1,22 +1,14 @@
> [Unit]
> Description=OpenSSH Key Generation
> RequiresMountsFor=/var /run
> -ConditionPathExists=!/var/run/ssh/ssh_host_rsa_key
> -ConditionPathExists=!/var/run/ssh/ssh_host_dsa_key
> -ConditionPathExists=!/var/run/ssh/ssh_host_ecdsa_key
> -ConditionPathExists=!/var/run/ssh/ssh_host_ed25519_key
> -ConditionPathExists=!/etc/ssh/ssh_host_rsa_key
> -ConditionPathExists=!/etc/ssh/ssh_host_dsa_key
> -ConditionPathExists=!/etc/ssh/ssh_host_ecdsa_key
> -ConditionPathExists=!/etc/ssh/ssh_host_ed25519_key
>
> [Service]
> Environment="SYSCONFDIR=/etc/ssh"
> EnvironmentFile=-/etc/default/ssh
> ExecStart=@BASE_BINDIR@/mkdir -p $SYSCONFDIR
> -ExecStart=@BINDIR@/ssh-keygen -q -f ${SYSCONFDIR}/ssh_host_rsa_key -N '' -t rsa
> -ExecStart=@BINDIR@/ssh-keygen -q -f ${SYSCONFDIR}/ssh_host_dsa_key -N '' -t dsa
> -ExecStart=@BINDIR@/ssh-keygen -q -f ${SYSCONFDIR}/ssh_host_ecdsa_key -N '' -t ecdsa
> -ExecStart=@BINDIR@/ssh-keygen -q -f ${SYSCONFDIR}/ssh_host_ed25519_key -N '' -t ed25519
> +ExecStart=@LIBEXECDIR@/sshd-check-key ${SYSCONFDIR}/ssh_host_rsa_key rsa
> +ExecStart=@LIBEXECDIR@/sshd-check-key ${SYSCONFDIR}/ssh_host_dsa_key dsa
> +ExecStart=@LIBEXECDIR@/sshd-check-key ${SYSCONFDIR}/ssh_host_ecdsa_key ecdsa
> +ExecStart=@LIBEXECDIR@/sshd-check-key ${SYSCONFDIR}/ssh_host_ed25519_key ed25519
> Type=oneshot
> RemainAfterExit=yes
> diff --git a/meta/recipes-connectivity/openssh/openssh_7.4p1.bb b/meta/recipes-connectivity/openssh/openssh_7.4p1.bb
> index c8093d4..ccd7a02 100644
> --- a/meta/recipes-connectivity/openssh/openssh_7.4p1.bb
> +++ b/meta/recipes-connectivity/openssh/openssh_7.4p1.bb
> @@ -25,6 +25,7 @@ SRC_URI = "http://ftp.openbsd.org/pub/OpenBSD/OpenSSH/portable/openssh-${PV}.tar
> file://openssh-7.1p1-conditional-compile-des-in-cipher.patch \
> file://openssh-7.1p1-conditional-compile-des-in-pkcs11.patch \
> file://fix-potential-signed-overflow-in-pointer-arithmatic.patch \
> + file://sshd-check-key \
> "
>
> PAM_SRC_URI = "file://sshd"
> @@ -124,7 +125,14 @@ do_install_append () {
> sed -i -e 's,@BASE_BINDIR@,${base_bindir},g' \
> -e 's,@SBINDIR@,${sbindir},g' \
> -e 's,@BINDIR@,${bindir},g' \
> + -e 's,@LIBEXECDIR@,${libexecdir}/${BPN},g' \
> ${D}${systemd_unitdir}/system/sshd.socket ${D}${systemd_unitdir}/system/*.service
> +
> + sed -i -e 's,@LIBEXECDIR@,${libexecdir}/${BPN},g' \
> + ${D}${sysconfdir}/init.d/sshd
> +
> + install -d 644 ${D}${libexecdir}/${BPN}
> + install -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}
> }
>
> do_install_ptest () {
> --
> 2.9.3
>