[PATCH] openssh: Atomically generate host keys

[PATCH] openssh: Atomically generate host keys

[Joshua Watt]

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

(No subject)

[Joshua Watt]

No need to create a directory name "644"

[PATCH v2] openssh: Atomically generate host keys

[Joshua Watt]

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..ad27342 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 ${D}${libexecdir}/${BPN}
+	install -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}
 }
 
 do_install_ptest () {
-- 
2.9.3

Re: [PATCH] openssh: Atomically generate host keys

[Joshua Watt]

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
>

Re: [PATCH] openssh: Atomically generate host keys

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 1915 bytes --]

On 7 May 2017 at 02:33, Joshua Watt <jpewhacker@gmail.com> wrote:

> +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
>
>
All of these syncs seem quite enthusiastic, are they really needed?
Writing the file to a temporary name and then mving it to the real name
should result in either no file or a complete file in the event of power
loss, surely?


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

Can you not continue to use ConditionPathExists to only run this unit if it
needs to run?  You can prepend the argument with | to make them logical OR
instead of logical AND, if I'm reading this documentation correctly.

Ross

[-- Attachment #2: Type: text/html, Size: 2766 bytes --]

Re: [PATCH] openssh: Atomically generate host keys

[Joshua Watt]

On Tue, May 23, 2017 at 9:37 AM, Burton, Ross <ross.burton@intel.com> wrote:
>
> On 7 May 2017 at 02:33, Joshua Watt <jpewhacker@gmail.com> wrote:
>>
>> +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
>>
>
> All of these syncs seem quite enthusiastic, are they really needed?  Writing
> the file to a temporary name and then mving it to the real name should
> result in either no file or a complete file in the event of power loss,
> surely?

My understanding (and observation) of most journal file systems is
that only metadata (i.e. directory entries and such) are journaled in
typical usage. The first sync is necessary in this case to ensure that
the actual file data gets put on the disk before we rename the files,
otherwise in the event of interruption journaled rename might get
replayed but have garbage data. The second one is more of a "force
operation order" sync to make sure the public file is written before
the private one, as a reordering would cause problems. The last sync
is the most optional, but I've seen it take minutes for disk to sync
contents if no one calls "sync", so it is very possible that all our
work of regenerating keys would be for naught if power is interrupted
in the meantime.

I think some of these syncs can be removed. Namely, the first and
third one. The second one needs to be there, but can server double
duty of syncing data to disk and enforcing the order between the
public and private rename. It does mean we could get a state where the
public key exists but is garbage, but this should be OK because the
private key won't exist and it would be regenerated.

The third sync can be removed and I can put one final sync at the end
after all keys are generated to ensure we won't go through all the
effort of regenerating the (last) key again in the event of
interruption shortly after, unless you would prefer I didn't.

>
>>
>> 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
>
>
> Can you not continue to use ConditionPathExists to only run this unit if it
> needs to run?  You can prepend the argument with | to make them logical OR
> instead of logical AND, if I'm reading this documentation correctly.

I will try that. I wasn't aware that was an option since systemd conf
files are somewhat new to me.

>
> Ross

Re: [PATCH] openssh: Atomically generate host keys

[Randy Witt]

On 05/23/2017 08:29 AM, Joshua Watt wrote:
> On Tue, May 23, 2017 at 9:37 AM, Burton, Ross <ross.burton@intel.com> wrote:
>>
>> On 7 May 2017 at 02:33, Joshua Watt <jpewhacker@gmail.com> wrote:
>>>
>>> +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
>>>
>>
>> All of these syncs seem quite enthusiastic, are they really needed?  Writing
>> the file to a temporary name and then mving it to the real name should
>> result in either no file or a complete file in the event of power loss,
>> surely?
> 
> My understanding (and observation) of most journal file systems is
> that only metadata (i.e. directory entries and such) are journaled in
> typical usage. The first sync is necessary in this case to ensure that
> the actual file data gets put on the disk before we rename the files,
> otherwise in the event of interruption journaled rename might get
> replayed but have garbage data. The second one is more of a "force
> operation order" sync to make sure the public file is written before
> the private one, as a reordering would cause problems. The last sync
> is the most optional, but I've seen it take minutes for disk to sync
> contents if no one calls "sync", so it is very possible that all our
> work of regenerating keys would be for naught if power is interrupted
> in the meantime.
> 
> I think some of these syncs can be removed. Namely, the first and
> third one. The second one needs to be there, but can server double
> duty of syncing data to disk and enforcing the order between the
> public and private rename. It does mean we could get a state where the
> public key exists but is garbage, but this should be OK because the
> private key won't exist and it would be regenerated.
> 
> The third sync can be removed and I can put one final sync at the end
> after all keys are generated to ensure we won't go through all the
> effort of regenerating the (last) key again in the event of
> interruption shortly after, unless you would prefer I didn't.
> 

The typical convention for this is,

1. Update file data.
2. sync file
3. sync containing directory
4. mv file to new location
5. sync directory containing new file (although I've seen this step left out before)

https://lwn.net/Articles/457672/ is a good example which is linked from 
https://lwn.net/Articles/457667/

But one of the important parts vs your code, is also that the example is only 
calling sync on the files/directory needed, vs calling "sync" with no arguments 
which is going to cause all data in all filesystem caches to be flushed.

>>
>>>
>>> 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
>>
>>
>> Can you not continue to use ConditionPathExists to only run this unit if it
>> needs to run?  You can prepend the argument with | to make them logical OR
>> instead of logical AND, if I'm reading this documentation correctly.
> 
> I will try that. I wasn't aware that was an option since systemd conf
> files are somewhat new to me.
> 
>>
>> Ross

Re: [PATCH] openssh: Atomically generate host keys

[Joshua Watt]

On Tue, May 23, 2017 at 12:23 PM, Randy Witt
<randy.e.witt@linux.intel.com> wrote:
> On 05/23/2017 08:29 AM, Joshua Watt wrote:
>>
>> On Tue, May 23, 2017 at 9:37 AM, Burton, Ross <ross.burton@intel.com>
>> wrote:
>>>
>>>
>>> On 7 May 2017 at 02:33, Joshua Watt <jpewhacker@gmail.com> wrote:
>>>>
>>>>
>>>> +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
>>>>
>>>
>>> All of these syncs seem quite enthusiastic, are they really needed?
>>> Writing
>>> the file to a temporary name and then mving it to the real name should
>>> result in either no file or a complete file in the event of power loss,
>>> surely?
>>
>>
>> My understanding (and observation) of most journal file systems is
>> that only metadata (i.e. directory entries and such) are journaled in
>> typical usage. The first sync is necessary in this case to ensure that
>> the actual file data gets put on the disk before we rename the files,
>> otherwise in the event of interruption journaled rename might get
>> replayed but have garbage data. The second one is more of a "force
>> operation order" sync to make sure the public file is written before
>> the private one, as a reordering would cause problems. The last sync
>> is the most optional, but I've seen it take minutes for disk to sync
>> contents if no one calls "sync", so it is very possible that all our
>> work of regenerating keys would be for naught if power is interrupted
>> in the meantime.
>>
>> I think some of these syncs can be removed. Namely, the first and
>> third one. The second one needs to be there, but can server double
>> duty of syncing data to disk and enforcing the order between the
>> public and private rename. It does mean we could get a state where the
>> public key exists but is garbage, but this should be OK because the
>> private key won't exist and it would be regenerated.
>>
>> The third sync can be removed and I can put one final sync at the end
>> after all keys are generated to ensure we won't go through all the
>> effort of regenerating the (last) key again in the event of
>> interruption shortly after, unless you would prefer I didn't.
>>
>
> The typical convention for this is,
>
> 1. Update file data.
> 2. sync file
> 3. sync containing directory
> 4. mv file to new location
> 5. sync directory containing new file (although I've seen this step left out
> before)
>
> https://lwn.net/Articles/457672/ is a good example which is linked from
> https://lwn.net/Articles/457667/
>
> But one of the important parts vs your code, is also that the example is
> only calling sync on the files/directory needed, vs calling "sync" with no
> arguments which is going to cause all data in all filesystem caches to be
> flushed.

Ah, OK. That makes sense, I will update sync to specify the
files/directory explicitly.

>
>
>>>
>>>>
>>>> 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
>>>
>>>
>>>
>>> Can you not continue to use ConditionPathExists to only run this unit if
>>> it
>>> needs to run?  You can prepend the argument with | to make them logical
>>> OR
>>> instead of logical AND, if I'm reading this documentation correctly.
>>
>>
>> I will try that. I wasn't aware that was an option since systemd conf
>> files are somewhat new to me.
>>
>>>
>>> Ross
>
>

Re: [PATCH] openssh: Atomically generate host keys

[Joshua Watt]

On Tue, May 23, 2017 at 12:56 PM, Joshua Watt <jpewhacker@gmail.com> wrote:
> On Tue, May 23, 2017 at 12:23 PM, Randy Witt
> <randy.e.witt@linux.intel.com> wrote:
>> On 05/23/2017 08:29 AM, Joshua Watt wrote:
>>>
>>> On Tue, May 23, 2017 at 9:37 AM, Burton, Ross <ross.burton@intel.com>
>>> wrote:
>>>>
>>>>
>>>> On 7 May 2017 at 02:33, Joshua Watt <jpewhacker@gmail.com> wrote:
>>>>>
>>>>>
>>>>> +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
>>>>>
>>>>
>>>> All of these syncs seem quite enthusiastic, are they really needed?
>>>> Writing
>>>> the file to a temporary name and then mving it to the real name should
>>>> result in either no file or a complete file in the event of power loss,
>>>> surely?
>>>
>>>
>>> My understanding (and observation) of most journal file systems is
>>> that only metadata (i.e. directory entries and such) are journaled in
>>> typical usage. The first sync is necessary in this case to ensure that
>>> the actual file data gets put on the disk before we rename the files,
>>> otherwise in the event of interruption journaled rename might get
>>> replayed but have garbage data. The second one is more of a "force
>>> operation order" sync to make sure the public file is written before
>>> the private one, as a reordering would cause problems. The last sync
>>> is the most optional, but I've seen it take minutes for disk to sync
>>> contents if no one calls "sync", so it is very possible that all our
>>> work of regenerating keys would be for naught if power is interrupted
>>> in the meantime.
>>>
>>> I think some of these syncs can be removed. Namely, the first and
>>> third one. The second one needs to be there, but can server double
>>> duty of syncing data to disk and enforcing the order between the
>>> public and private rename. It does mean we could get a state where the
>>> public key exists but is garbage, but this should be OK because the
>>> private key won't exist and it would be regenerated.
>>>
>>> The third sync can be removed and I can put one final sync at the end
>>> after all keys are generated to ensure we won't go through all the
>>> effort of regenerating the (last) key again in the event of
>>> interruption shortly after, unless you would prefer I didn't.
>>>
>>
>> The typical convention for this is,
>>
>> 1. Update file data.
>> 2. sync file
>> 3. sync containing directory
>> 4. mv file to new location
>> 5. sync directory containing new file (although I've seen this step left out
>> before)
>>
>> https://lwn.net/Articles/457672/ is a good example which is linked from
>> https://lwn.net/Articles/457667/
>>
>> But one of the important parts vs your code, is also that the example is
>> only calling sync on the files/directory needed, vs calling "sync" with no
>> arguments which is going to cause all data in all filesystem caches to be
>> flushed.
>
> Ah, OK. That makes sense, I will update sync to specify the
> files/directory explicitly.

FWIW, I did some tests on the sync behavior:

It appears that older versions of the sync command ignore any
arguments and just call sync(). From Ubuntu 14.04:
$ strace sync foo
...
write(2, "sync: ", 6sync: )                   = 6
write(2, "ignoring all arguments", 22)  = 22
write(2, "\n", 1)                       = 1
sync()                                  = 0
...

The same is true for the (default) busybox version of sync on master
(again verified with strace), but it doesn't complain nosily about it.

I looked at the coreutils code, and sync was changed to respect
arguments in January of 2015
(8b2bf5295f353016d4f5e6a2317d55b6a8e7fd00) and only call fsync() on
the provided arguments (if any are provided).

Either way, adding the arguments to the sync call in my patch won't
hurt because it is forward compatible, even though it won't be
optimally efficient currently because the sync command currently
simply calls sync()

>
>>
>>
>>>>
>>>>>
>>>>> 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
>>>>
>>>>
>>>>
>>>> Can you not continue to use ConditionPathExists to only run this unit if
>>>> it
>>>> needs to run?  You can prepend the argument with | to make them logical
>>>> OR
>>>> instead of logical AND, if I'm reading this documentation correctly.
>>>
>>>
>>> I will try that. I wasn't aware that was an option since systemd conf
>>> files are somewhat new to me.
>>>
>>>>
>>>> Ross
>>
>>

Re: [PATCH] openssh: Atomically generate host keys

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org
> [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
> Joshua Watt
> Sent: den 23 maj 2017 21:57
> To: Randy Witt <randy.e.witt@linux.intel.com>
> Cc: OE-core <openembedded-core@lists.openembedded.org>
> Subject: Re: [OE-core] [PATCH] openssh: Atomically generate host keys
> 
> On Tue, May 23, 2017 at 12:56 PM, Joshua Watt <jpewhacker@gmail.com>
> wrote:
> > On Tue, May 23, 2017 at 12:23 PM, Randy Witt
> > <randy.e.witt@linux.intel.com> wrote:
> >> On 05/23/2017 08:29 AM, Joshua Watt wrote:
> >>>
> >>> On Tue, May 23, 2017 at 9:37 AM, Burton, Ross
> <ross.burton@intel.com>
> >>> wrote:
> >>>>
> >>>>
> >>>> On 7 May 2017 at 02:33, Joshua Watt <jpewhacker@gmail.com> wrote:
> >>>>>
> >>>>>
> >>>>> +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
> >>>>>
> >>>>
> >>>> All of these syncs seem quite enthusiastic, are they really
> needed?
> >>>> Writing
> >>>> the file to a temporary name and then mving it to the real name
> should
> >>>> result in either no file or a complete file in the event of power
> loss,
> >>>> surely?
> >>>
> >>>
> >>> My understanding (and observation) of most journal file systems is
> >>> that only metadata (i.e. directory entries and such) are journaled
> in
> >>> typical usage. The first sync is necessary in this case to ensure
> that
> >>> the actual file data gets put on the disk before we rename the
> files,
> >>> otherwise in the event of interruption journaled rename might get
> >>> replayed but have garbage data. The second one is more of a "force
> >>> operation order" sync to make sure the public file is written
> before
> >>> the private one, as a reordering would cause problems. The last
> sync
> >>> is the most optional, but I've seen it take minutes for disk to
> sync
> >>> contents if no one calls "sync", so it is very possible that all
> our
> >>> work of regenerating keys would be for naught if power is
> interrupted
> >>> in the meantime.
> >>>
> >>> I think some of these syncs can be removed. Namely, the first and
> >>> third one. The second one needs to be there, but can server double
> >>> duty of syncing data to disk and enforcing the order between the
> >>> public and private rename. It does mean we could get a state where
> the
> >>> public key exists but is garbage, but this should be OK because the
> >>> private key won't exist and it would be regenerated.
> >>>
> >>> The third sync can be removed and I can put one final sync at the
> end
> >>> after all keys are generated to ensure we won't go through all the
> >>> effort of regenerating the (last) key again in the event of
> >>> interruption shortly after, unless you would prefer I didn't.
> >>>
> >>
> >> The typical convention for this is,
> >>
> >> 1. Update file data.
> >> 2. sync file
> >> 3. sync containing directory
> >> 4. mv file to new location
> >> 5. sync directory containing new file (although I've seen this step
> left out
> >> before)
> >>
> >> https://lwn.net/Articles/457672/ is a good example which is linked
> from
> >> https://lwn.net/Articles/457667/
> >>
> >> But one of the important parts vs your code, is also that the
> example is
> >> only calling sync on the files/directory needed, vs calling "sync"
> with no
> >> arguments which is going to cause all data in all filesystem caches
> to be
> >> flushed.
> >
> > Ah, OK. That makes sense, I will update sync to specify the
> > files/directory explicitly.
> 
> FWIW, I did some tests on the sync behavior:
> 
> It appears that older versions of the sync command ignore any
> arguments and just call sync(). From Ubuntu 14.04:
> $ strace sync foo
> ...
> write(2, "sync: ", 6sync: )                   = 6
> write(2, "ignoring all arguments", 22)  = 22
> write(2, "\n", 1)                       = 1
> sync()                                  = 0
> ...
> 
> The same is true for the (default) busybox version of sync on master
> (again verified with strace), but it doesn't complain nosily about it.

Busybox has a separate fsync applet to do file synchronization, but it 
is not enabled in the default OE-core configuration...

> I looked at the coreutils code, and sync was changed to respect
> arguments in January of 2015
> (8b2bf5295f353016d4f5e6a2317d55b6a8e7fd00) and only call fsync() on
> the provided arguments (if any are provided).
> 
> Either way, adding the arguments to the sync call in my patch won't
> hurt because it is forward compatible, even though it won't be
> optimally efficient currently because the sync command currently
> simply calls sync()

//Peter

Re: [PATCH] openssh: Atomically generate host keys

[Joshua Watt]

On Wed, May 24, 2017 at 5:03 AM, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
>> -----Original Message-----
>> From: openembedded-core-bounces@lists.openembedded.org
>> [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
>> Joshua Watt
>> Sent: den 23 maj 2017 21:57
>> To: Randy Witt <randy.e.witt@linux.intel.com>
>> Cc: OE-core <openembedded-core@lists.openembedded.org>
>> Subject: Re: [OE-core] [PATCH] openssh: Atomically generate host keys
>>
>> On Tue, May 23, 2017 at 12:56 PM, Joshua Watt <jpewhacker@gmail.com>
>> wrote:
>> > On Tue, May 23, 2017 at 12:23 PM, Randy Witt
>> > <randy.e.witt@linux.intel.com> wrote:
>> >> On 05/23/2017 08:29 AM, Joshua Watt wrote:
>> >>>
>> >>> On Tue, May 23, 2017 at 9:37 AM, Burton, Ross
>> <ross.burton@intel.com>
>> >>> wrote:
>> >>>>
>> >>>>
>> >>>> On 7 May 2017 at 02:33, Joshua Watt <jpewhacker@gmail.com> wrote:
>> >>>>>
>> >>>>>
>> >>>>> +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
>> >>>>>
>> >>>>
>> >>>> All of these syncs seem quite enthusiastic, are they really
>> needed?
>> >>>> Writing
>> >>>> the file to a temporary name and then mving it to the real name
>> should
>> >>>> result in either no file or a complete file in the event of power
>> loss,
>> >>>> surely?
>> >>>
>> >>>
>> >>> My understanding (and observation) of most journal file systems is
>> >>> that only metadata (i.e. directory entries and such) are journaled
>> in
>> >>> typical usage. The first sync is necessary in this case to ensure
>> that
>> >>> the actual file data gets put on the disk before we rename the
>> files,
>> >>> otherwise in the event of interruption journaled rename might get
>> >>> replayed but have garbage data. The second one is more of a "force
>> >>> operation order" sync to make sure the public file is written
>> before
>> >>> the private one, as a reordering would cause problems. The last
>> sync
>> >>> is the most optional, but I've seen it take minutes for disk to
>> sync
>> >>> contents if no one calls "sync", so it is very possible that all
>> our
>> >>> work of regenerating keys would be for naught if power is
>> interrupted
>> >>> in the meantime.
>> >>>
>> >>> I think some of these syncs can be removed. Namely, the first and
>> >>> third one. The second one needs to be there, but can server double
>> >>> duty of syncing data to disk and enforcing the order between the
>> >>> public and private rename. It does mean we could get a state where
>> the
>> >>> public key exists but is garbage, but this should be OK because the
>> >>> private key won't exist and it would be regenerated.
>> >>>
>> >>> The third sync can be removed and I can put one final sync at the
>> end
>> >>> after all keys are generated to ensure we won't go through all the
>> >>> effort of regenerating the (last) key again in the event of
>> >>> interruption shortly after, unless you would prefer I didn't.
>> >>>
>> >>
>> >> The typical convention for this is,
>> >>
>> >> 1. Update file data.
>> >> 2. sync file
>> >> 3. sync containing directory
>> >> 4. mv file to new location
>> >> 5. sync directory containing new file (although I've seen this step
>> left out
>> >> before)
>> >>
>> >> https://lwn.net/Articles/457672/ is a good example which is linked
>> from
>> >> https://lwn.net/Articles/457667/
>> >>
>> >> But one of the important parts vs your code, is also that the
>> example is
>> >> only calling sync on the files/directory needed, vs calling "sync"
>> with no
>> >> arguments which is going to cause all data in all filesystem caches
>> to be
>> >> flushed.
>> >
>> > Ah, OK. That makes sense, I will update sync to specify the
>> > files/directory explicitly.
>>
>> FWIW, I did some tests on the sync behavior:
>>
>> It appears that older versions of the sync command ignore any
>> arguments and just call sync(). From Ubuntu 14.04:
>> $ strace sync foo
>> ...
>> write(2, "sync: ", 6sync: )                   = 6
>> write(2, "ignoring all arguments", 22)  = 22
>> write(2, "\n", 1)                       = 1
>> sync()                                  = 0
>> ...
>>
>> The same is true for the (default) busybox version of sync on master
>> (again verified with strace), but it doesn't complain nosily about it.
>
> Busybox has a separate fsync applet to do file synchronization, but it
> is not enabled in the default OE-core configuration...

Ok. I think for best compatibility I'll just use "sync", as that will
always be safe, just not always optimally efficient. I think this is
OK, since key generation either only occurs once (r/w rootfs), or once
per boot (r/o rootfs).

>
>> I looked at the coreutils code, and sync was changed to respect
>> arguments in January of 2015
>> (8b2bf5295f353016d4f5e6a2317d55b6a8e7fd00) and only call fsync() on
>> the provided arguments (if any are provided).
>>
>> Either way, adding the arguments to the sync call in my patch won't
>> hurt because it is forward compatible, even though it won't be
>> optimally efficient currently because the sync command currently
>> simply calls sync()
>
> //Peter
>

[meta-oe][PATCH v3] openssh: Atomically generate host keys

[Joshua Watt]

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.
---
 meta/recipes-connectivity/openssh/openssh/init     | 21 +++----------
 .../openssh/openssh/sshd-check-key                 | 36 ++++++++++++++++++++++
 .../openssh/openssh/sshdgenkeys.service            | 24 +++++++--------
 meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  8 +++++
 4 files changed, 60 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..d2613af
--- /dev/null
+++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
@@ -0,0 +1,36 @@
+#! /bin/sh
+set -e
+
+NAME="$1"
+TYPE="$2"
+
+if [ -z "$NAME" ] || [ -z "$TYPE" ]; then
+    echo "Usage: $0 NAME TYPE"
+    exit 1;
+fi
+
+DIR="$(dirname "$NAME")"
+
+if [ ! -f "$NAME" ]; then
+    echo "  generating ssh $TYPE key..."
+    ssh-keygen -q -f "${NAME}.tmp" -N '' -t $TYPE
+
+    # Move (Atomically rename) files
+    mv -f "${NAME}.tmp.pub" "${NAME}.pub"
+
+    # This sync does double duty: Ensuring that the data in the temporary
+    # private key file is on disk before the rename, and ensuring that the
+    # public key rename is completed before the private key rename, since we
+    # switch on the existence of the private key to trigger key generation.
+    # This does mean it is possible for the public key to exist, but be garbage
+    # but this is OK because in that case the private key won't exist and the
+    # keys will be regenerated.
+    #
+    # In the event that sync understands arguments that limit what it tries to
+    # fsync(), we provided them. If it does not, it will simply call sync()
+    # which is just as well
+    sync "${NAME}.pub" "$DIR" "${NAME}.tmp"
+
+    mv -f "${NAME}.tmp" "${NAME}"
+fi
+
diff --git a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
index 148e6ad..5d08b53 100644
--- a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
+++ b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
@@ -1,22 +1,22 @@
 [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
+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.5p1.bb b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
index 5b96745..ede8823 100644
--- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
+++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.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 ${D}${libexecdir}/${BPN}
+	install -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}
 }
 
 do_install_ptest () {
-- 
2.9.3

✗ patchtest: failure for openssh: Atomically generate host keys (rev3)

[Patchwork]

== Series Details ==

Series: openssh: Atomically generate host keys (rev3)
Revision: 3
URL   : https://patchwork.openembedded.org/series/6626/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Patch            [meta-oe,v3] openssh: Atomically generate host keys
 Issue             Patch is missing Signed-off-by [test_signed_off_by_presence] 
  Suggested fix    Sign off the patch (either manually or with "git commit --amend -s")



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Re: [meta-oe][PATCH v3] openssh: Atomically generate host keys

[Ian Arkver]

On 25/05/17 03:17, Joshua Watt 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.
> ---
>   meta/recipes-connectivity/openssh/openssh/init     | 21 +++----------
>   .../openssh/openssh/sshd-check-key                 | 36 ++++++++++++++++++++++
>   .../openssh/openssh/sshdgenkeys.service            | 24 +++++++--------
>   meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  8 +++++
>   4 files changed, 60 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..d2613af
> --- /dev/null
> +++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
> @@ -0,0 +1,36 @@
> +#! /bin/sh
> +set -e
> +
> +NAME="$1"
> +TYPE="$2"
> +
> +if [ -z "$NAME" ] || [ -z "$TYPE" ]; then
> +    echo "Usage: $0 NAME TYPE"
> +    exit 1;
> +fi
> +
> +DIR="$(dirname "$NAME")"
> +
> +if [ ! -f "$NAME" ]; then
> +    echo "  generating ssh $TYPE key..."
> +    ssh-keygen -q -f "${NAME}.tmp" -N '' -t $TYPE
> +
> +    # Move (Atomically rename) files
> +    mv -f "${NAME}.tmp.pub" "${NAME}.pub"
> +
> +    # This sync does double duty: Ensuring that the data in the temporary
> +    # private key file is on disk before the rename, and ensuring that the
> +    # public key rename is completed before the private key rename, since we
> +    # switch on the existence of the private key to trigger key generation.
> +    # This does mean it is possible for the public key to exist, but be garbage
> +    # but this is OK because in that case the private key won't exist and the
> +    # keys will be regenerated.
> +    #
> +    # In the event that sync understands arguments that limit what it tries to
> +    # fsync(), we provided them. If it does not, it will simply call sync()
> +    # which is just as well
> +    sync "${NAME}.pub" "$DIR" "${NAME}.tmp"
> +
> +    mv -f "${NAME}.tmp" "${NAME}"

You previously mentioned moving the third, most optional sync to a 
single sync at the end, but I don't see it at all now. Should there be 
another sync "$DIR" somewhere in the init script or service file?

Regards,
Ian

> +fi
> +
> diff --git a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> index 148e6ad..5d08b53 100644
> --- a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> +++ b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> @@ -1,22 +1,22 @@
>   [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
> +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.5p1.bb b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> index 5b96745..ede8823 100644
> --- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> +++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.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 ${D}${libexecdir}/${BPN}
> +	install -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}
>   }
>   
>   do_install_ptest () {
> 

[meta-oe][PATCH v4] openssh: Atomically generate host keys

[Joshua Watt]

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.
---
 meta/recipes-connectivity/openssh/openssh/init     | 22 +++----------
 .../openssh/openssh/sshd-check-key                 | 36 ++++++++++++++++++++++
 .../openssh/openssh/sshdgenkeys.service            | 25 +++++++--------
 meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  9 ++++++
 4 files changed, 63 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..e02c479 100644
--- a/meta/recipes-connectivity/openssh/openssh/init
+++ b/meta/recipes-connectivity/openssh/openssh/init
@@ -45,23 +45,11 @@ 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
+    @BASE_BINDIR@/sync
 }
 
 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..d2613af
--- /dev/null
+++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
@@ -0,0 +1,36 @@
+#! /bin/sh
+set -e
+
+NAME="$1"
+TYPE="$2"
+
+if [ -z "$NAME" ] || [ -z "$TYPE" ]; then
+    echo "Usage: $0 NAME TYPE"
+    exit 1;
+fi
+
+DIR="$(dirname "$NAME")"
+
+if [ ! -f "$NAME" ]; then
+    echo "  generating ssh $TYPE key..."
+    ssh-keygen -q -f "${NAME}.tmp" -N '' -t $TYPE
+
+    # Move (Atomically rename) files
+    mv -f "${NAME}.tmp.pub" "${NAME}.pub"
+
+    # This sync does double duty: Ensuring that the data in the temporary
+    # private key file is on disk before the rename, and ensuring that the
+    # public key rename is completed before the private key rename, since we
+    # switch on the existence of the private key to trigger key generation.
+    # This does mean it is possible for the public key to exist, but be garbage
+    # but this is OK because in that case the private key won't exist and the
+    # keys will be regenerated.
+    #
+    # In the event that sync understands arguments that limit what it tries to
+    # fsync(), we provided them. If it does not, it will simply call sync()
+    # which is just as well
+    sync "${NAME}.pub" "$DIR" "${NAME}.tmp"
+
+    mv -f "${NAME}.tmp" "${NAME}"
+fi
+
diff --git a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
index 148e6ad..23fd351 100644
--- a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
+++ b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
@@ -1,22 +1,23 @@
 [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
+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
+ExecStart=@BASE_BINDIR@/sync
 Type=oneshot
 RemainAfterExit=yes
diff --git a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
index 5b96745..c1fcda4 100644
--- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
+++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.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,15 @@ 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' \
+		-e 's,@BASE_BINDIR@,${base_bindir},g' \
+		${D}${sysconfdir}/init.d/sshd
+
+	install -d ${D}${libexecdir}/${BPN}
+	install -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}
 }
 
 do_install_ptest () {
-- 
2.9.3

[meta-oe][PATCH v5] openssh: Atomically generate host keys

[Joshua Watt]

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     | 22 +++----------
 .../openssh/openssh/sshd-check-key                 | 36 ++++++++++++++++++++++
 .../openssh/openssh/sshdgenkeys.service            | 25 +++++++--------
 meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  9 ++++++
 4 files changed, 63 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..e02c479 100644
--- a/meta/recipes-connectivity/openssh/openssh/init
+++ b/meta/recipes-connectivity/openssh/openssh/init
@@ -45,23 +45,11 @@ 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
+    @BASE_BINDIR@/sync
 }
 
 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..d2613af
--- /dev/null
+++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
@@ -0,0 +1,36 @@
+#! /bin/sh
+set -e
+
+NAME="$1"
+TYPE="$2"
+
+if [ -z "$NAME" ] || [ -z "$TYPE" ]; then
+    echo "Usage: $0 NAME TYPE"
+    exit 1;
+fi
+
+DIR="$(dirname "$NAME")"
+
+if [ ! -f "$NAME" ]; then
+    echo "  generating ssh $TYPE key..."
+    ssh-keygen -q -f "${NAME}.tmp" -N '' -t $TYPE
+
+    # Move (Atomically rename) files
+    mv -f "${NAME}.tmp.pub" "${NAME}.pub"
+
+    # This sync does double duty: Ensuring that the data in the temporary
+    # private key file is on disk before the rename, and ensuring that the
+    # public key rename is completed before the private key rename, since we
+    # switch on the existence of the private key to trigger key generation.
+    # This does mean it is possible for the public key to exist, but be garbage
+    # but this is OK because in that case the private key won't exist and the
+    # keys will be regenerated.
+    #
+    # In the event that sync understands arguments that limit what it tries to
+    # fsync(), we provided them. If it does not, it will simply call sync()
+    # which is just as well
+    sync "${NAME}.pub" "$DIR" "${NAME}.tmp"
+
+    mv -f "${NAME}.tmp" "${NAME}"
+fi
+
diff --git a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
index 148e6ad..23fd351 100644
--- a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
+++ b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
@@ -1,22 +1,23 @@
 [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
+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
+ExecStart=@BASE_BINDIR@/sync
 Type=oneshot
 RemainAfterExit=yes
diff --git a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
index 5b96745..c1fcda4 100644
--- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
+++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.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,15 @@ 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' \
+		-e 's,@BASE_BINDIR@,${base_bindir},g' \
+		${D}${sysconfdir}/init.d/sshd
+
+	install -d ${D}${libexecdir}/${BPN}
+	install -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}
 }
 
 do_install_ptest () {
-- 
2.9.3

✗ patchtest: failure for openssh: Atomically generate host keys (rev4)

[Patchwork]

== Series Details ==

Series: openssh: Atomically generate host keys (rev4)
Revision: 4
URL   : https://patchwork.openembedded.org/series/6626/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Patch            [meta-oe,v4] openssh: Atomically generate host keys
 Issue             Patch is missing Signed-off-by [test_signed_off_by_presence] 
  Suggested fix    Sign off the patch (either manually or with "git commit --amend -s")



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Re: [meta-oe][PATCH v5] openssh: Atomically generate host keys

[Joshua Watt]

On Fri, May 26, 2017 at 8:33 AM, Leonardo Sandoval
<leonardo.sandoval.gonzalez@linux.intel.com> wrote:
> Just a minor comment: there is no need to include any prefix besides
> [PATCH vN]. Also, patches either go to oe-core or meta-oe, so it makes
> no sense to send the patch to the oe-core and using the [meta-oe] as
> prefix.

Ya, sorry. I was following these directions and didn't read closely enough:
https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded

>
>
> Leo
>
>
>
> On Thu, 2017-05-25 at 20:54 -0500, Joshua Watt 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.
>
>

Re: [meta-oe][PATCH v5] openssh: Atomically generate host keys

[Leonardo Sandoval]

Just a minor comment: there is no need to include any prefix besides
[PATCH vN]. Also, patches either go to oe-core or meta-oe, so it makes
no sense to send the patch to the oe-core and using the [meta-oe] as
prefix.


Leo



On Thu, 2017-05-25 at 20:54 -0500, Joshua Watt 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.

Re: [meta-oe][PATCH v4] openssh: Atomically generate host keys

[Andre McCurdy]

 On Thu, May 25, 2017 at 6:52 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.
> ---
>  meta/recipes-connectivity/openssh/openssh/init     | 22 +++----------
>  .../openssh/openssh/sshd-check-key                 | 36 ++++++++++++++++++++++
>  .../openssh/openssh/sshdgenkeys.service            | 25 +++++++--------
>  meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  9 ++++++
>  4 files changed, 63 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..e02c479 100644
> --- a/meta/recipes-connectivity/openssh/openssh/init
> +++ b/meta/recipes-connectivity/openssh/openssh/init
> @@ -45,23 +45,11 @@ 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
> +    @BASE_BINDIR@/sync
>  }
>
>  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..d2613af
> --- /dev/null
> +++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
> @@ -0,0 +1,36 @@
> +#! /bin/sh
> +set -e

Is this appropriate for an init script? Aborting on unforeseen errors
may be worse than continuing.

> +NAME="$1"
> +TYPE="$2"
> +
> +if [ -z "$NAME" ] || [ -z "$TYPE" ]; then
> +    echo "Usage: $0 NAME TYPE"
> +    exit 1;

Remove the stray ";"

> +fi
> +
> +DIR="$(dirname "$NAME")"

This could be moved down so dirname is only called if DIR is going to be used.

> +if [ ! -f "$NAME" ]; then
> +    echo "  generating ssh $TYPE key..."
> +    ssh-keygen -q -f "${NAME}.tmp" -N '' -t $TYPE
> +
> +    # Move (Atomically rename) files
> +    mv -f "${NAME}.tmp.pub" "${NAME}.pub"
> +
> +    # This sync does double duty: Ensuring that the data in the temporary
> +    # private key file is on disk before the rename, and ensuring that the
> +    # public key rename is completed before the private key rename, since we
> +    # switch on the existence of the private key to trigger key generation.
> +    # This does mean it is possible for the public key to exist, but be garbage
> +    # but this is OK because in that case the private key won't exist and the
> +    # keys will be regenerated.
> +    #
> +    # In the event that sync understands arguments that limit what it tries to
> +    # fsync(), we provided them. If it does not, it will simply call sync()
> +    # which is just as well
> +    sync "${NAME}.pub" "$DIR" "${NAME}.tmp"
> +
> +    mv -f "${NAME}.tmp" "${NAME}"

No need for "-f" here as ${NAME} is known not to exist.

> +fi
> +
> diff --git a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> index 148e6ad..23fd351 100644
> --- a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> +++ b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> @@ -1,22 +1,23 @@
>  [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
> +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
> +ExecStart=@BASE_BINDIR@/sync
>  Type=oneshot
>  RemainAfterExit=yes
> diff --git a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> index 5b96745..c1fcda4 100644
> --- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> +++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.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,15 @@ 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' \
> +               -e 's,@BASE_BINDIR@,${base_bindir},g' \
> +               ${D}${sysconfdir}/init.d/sshd
> +
> +       install -d ${D}${libexecdir}/${BPN}
> +       install -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}
>  }
>
>  do_install_ptest () {
> --
> 2.9.3
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

[PATCH v6] openssh: Atomically generate host keys

[Joshua Watt]

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     | 22 ++++----------
 .../openssh/openssh/sshd-check-key                 | 35 ++++++++++++++++++++++
 .../openssh/openssh/sshdgenkeys.service            | 25 ++++++++--------
 meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  9 ++++++
 4 files changed, 62 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..e02c479 100644
--- a/meta/recipes-connectivity/openssh/openssh/init
+++ b/meta/recipes-connectivity/openssh/openssh/init
@@ -45,23 +45,11 @@ 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
+    @BASE_BINDIR@/sync
 }
 
 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..3afdb8b
--- /dev/null
+++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
@@ -0,0 +1,35 @@
+#! /bin/sh
+NAME="$1"
+TYPE="$2"
+
+if [ -z "$NAME" ] || [ -z "$TYPE" ]; then
+    echo "Usage: $0 NAME TYPE"
+    exit 1
+fi
+
+
+if [ ! -f "$NAME" ]; then
+    DIR="$(dirname "$NAME")"
+
+    echo "  generating ssh $TYPE key..."
+    ssh-keygen -q -f "${NAME}.tmp" -N '' -t $TYPE
+
+    # Move (Atomically rename) files
+    mv -f "${NAME}.tmp.pub" "${NAME}.pub"
+
+    # This sync does double duty: Ensuring that the data in the temporary
+    # private key file is on disk before the rename, and ensuring that the
+    # public key rename is completed before the private key rename, since we
+    # switch on the existence of the private key to trigger key generation.
+    # This does mean it is possible for the public key to exist, but be garbage
+    # but this is OK because in that case the private key won't exist and the
+    # keys will be regenerated.
+    #
+    # In the event that sync understands arguments that limit what it tries to
+    # fsync(), we provided them. If it does not, it will simply call sync()
+    # which is just as well
+    sync "${NAME}.pub" "$DIR" "${NAME}.tmp"
+
+    mv "${NAME}.tmp" "$NAME"
+fi
+
diff --git a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
index 148e6ad..23fd351 100644
--- a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
+++ b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
@@ -1,22 +1,23 @@
 [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
+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
+ExecStart=@BASE_BINDIR@/sync
 Type=oneshot
 RemainAfterExit=yes
diff --git a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
index 5b96745..c1fcda4 100644
--- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
+++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.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,15 @@ 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' \
+		-e 's,@BASE_BINDIR@,${base_bindir},g' \
+		${D}${sysconfdir}/init.d/sshd
+
+	install -d ${D}${libexecdir}/${BPN}
+	install -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}
 }
 
 do_install_ptest () {
-- 
2.9.4

Re: [PATCH v6] openssh: Atomically generate host keys

[Otavio Salvador]

On Tue, May 30, 2017 at 11:34 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>

It does make a lot of sense. The only consideration I have is that
this might be a simple rework on the recipe:

       install -d ${D}${libexecdir}/${BPN}
       install -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}

can be replaced with:

       install -Dm 0755 ${WORKDIR}/sshd-check-key
${D}${libexecdir}/${BPN}/sshd-check-key

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

[PATCH v7] openssh: Atomically generate host keys

[Joshua Watt]

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     | 22 ++++----------
 .../openssh/openssh/sshd-check-key                 | 35 ++++++++++++++++++++++
 .../openssh/openssh/sshdgenkeys.service            | 25 ++++++++--------
 meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  8 +++++
 4 files changed, 61 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..e02c479 100644
--- a/meta/recipes-connectivity/openssh/openssh/init
+++ b/meta/recipes-connectivity/openssh/openssh/init
@@ -45,23 +45,11 @@ 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
+    @BASE_BINDIR@/sync
 }
 
 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..3afdb8b
--- /dev/null
+++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
@@ -0,0 +1,35 @@
+#! /bin/sh
+NAME="$1"
+TYPE="$2"
+
+if [ -z "$NAME" ] || [ -z "$TYPE" ]; then
+    echo "Usage: $0 NAME TYPE"
+    exit 1
+fi
+
+
+if [ ! -f "$NAME" ]; then
+    DIR="$(dirname "$NAME")"
+
+    echo "  generating ssh $TYPE key..."
+    ssh-keygen -q -f "${NAME}.tmp" -N '' -t $TYPE
+
+    # Move (Atomically rename) files
+    mv -f "${NAME}.tmp.pub" "${NAME}.pub"
+
+    # This sync does double duty: Ensuring that the data in the temporary
+    # private key file is on disk before the rename, and ensuring that the
+    # public key rename is completed before the private key rename, since we
+    # switch on the existence of the private key to trigger key generation.
+    # This does mean it is possible for the public key to exist, but be garbage
+    # but this is OK because in that case the private key won't exist and the
+    # keys will be regenerated.
+    #
+    # In the event that sync understands arguments that limit what it tries to
+    # fsync(), we provided them. If it does not, it will simply call sync()
+    # which is just as well
+    sync "${NAME}.pub" "$DIR" "${NAME}.tmp"
+
+    mv "${NAME}.tmp" "$NAME"
+fi
+
diff --git a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
index 148e6ad..23fd351 100644
--- a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
+++ b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
@@ -1,22 +1,23 @@
 [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
+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
+ExecStart=@BASE_BINDIR@/sync
 Type=oneshot
 RemainAfterExit=yes
diff --git a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
index 5b96745..ec4b55f 100644
--- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
+++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.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' \
+		-e 's,@BASE_BINDIR@,${base_bindir},g' \
+		${D}${sysconfdir}/init.d/sshd
+
+	install -D -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}
 }
 
 do_install_ptest () {
-- 
2.9.4

Re: [PATCH v7] openssh: Atomically generate host keys

[Joshua Watt]

On Wed, May 31, 2017 at 10:05 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     | 22 ++++----------
>  .../openssh/openssh/sshd-check-key                 | 35 ++++++++++++++++++++++
>  .../openssh/openssh/sshdgenkeys.service            | 25 ++++++++--------
>  meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  8 +++++
>  4 files changed, 61 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..e02c479 100644
> --- a/meta/recipes-connectivity/openssh/openssh/init
> +++ b/meta/recipes-connectivity/openssh/openssh/init
> @@ -45,23 +45,11 @@ 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
> +    @BASE_BINDIR@/sync
>  }
>
>  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..3afdb8b
> --- /dev/null
> +++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
> @@ -0,0 +1,35 @@
> +#! /bin/sh
> +NAME="$1"
> +TYPE="$2"
> +
> +if [ -z "$NAME" ] || [ -z "$TYPE" ]; then
> +    echo "Usage: $0 NAME TYPE"
> +    exit 1
> +fi
> +
> +
> +if [ ! -f "$NAME" ]; then
> +    DIR="$(dirname "$NAME")"
> +
> +    echo "  generating ssh $TYPE key..."
> +    ssh-keygen -q -f "${NAME}.tmp" -N '' -t $TYPE
> +
> +    # Move (Atomically rename) files
> +    mv -f "${NAME}.tmp.pub" "${NAME}.pub"
> +
> +    # This sync does double duty: Ensuring that the data in the temporary
> +    # private key file is on disk before the rename, and ensuring that the
> +    # public key rename is completed before the private key rename, since we
> +    # switch on the existence of the private key to trigger key generation.
> +    # This does mean it is possible for the public key to exist, but be garbage
> +    # but this is OK because in that case the private key won't exist and the
> +    # keys will be regenerated.
> +    #
> +    # In the event that sync understands arguments that limit what it tries to
> +    # fsync(), we provided them. If it does not, it will simply call sync()
> +    # which is just as well
> +    sync "${NAME}.pub" "$DIR" "${NAME}.tmp"
> +
> +    mv "${NAME}.tmp" "$NAME"
> +fi
> +
> diff --git a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> index 148e6ad..23fd351 100644
> --- a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> +++ b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> @@ -1,22 +1,23 @@
>  [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
> +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
> +ExecStart=@BASE_BINDIR@/sync
>  Type=oneshot
>  RemainAfterExit=yes
> diff --git a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> index 5b96745..ec4b55f 100644
> --- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> +++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.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' \
> +               -e 's,@BASE_BINDIR@,${base_bindir},g' \
> +               ${D}${sysconfdir}/init.d/sshd
> +
> +       install -D -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}
>  }
>
>  do_install_ptest () {
> --
> 2.9.4
>

Ping?

Re: [PATCH v7] openssh: Atomically generate host keys

[Joshua Watt]

On Tue, 2017-06-06 at 22:30 -0500, Joshua Watt wrote:
> On Wed, May 31, 2017 at 10:05 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     | 22 ++++----
> > ------
> >  .../openssh/openssh/sshd-check-key                 | 35
> > ++++++++++++++++++++++
> >  .../openssh/openssh/sshdgenkeys.service            | 25 ++++++++
> > --------
> >  meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  8 +++++
> >  4 files changed, 61 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..e02c479 100644
> > --- a/meta/recipes-connectivity/openssh/openssh/init
> > +++ b/meta/recipes-connectivity/openssh/openssh/init
> > @@ -45,23 +45,11 @@ 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
> > +    @BASE_BINDIR@/sync
> >  }
> > 
> >  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..3afdb8b
> > --- /dev/null
> > +++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
> > @@ -0,0 +1,35 @@
> > +#! /bin/sh
> > +NAME="$1"
> > +TYPE="$2"
> > +
> > +if [ -z "$NAME" ] || [ -z "$TYPE" ]; then
> > +    echo "Usage: $0 NAME TYPE"
> > +    exit 1
> > +fi
> > +
> > +
> > +if [ ! -f "$NAME" ]; then
> > +    DIR="$(dirname "$NAME")"
> > +
> > +    echo "  generating ssh $TYPE key..."
> > +    ssh-keygen -q -f "${NAME}.tmp" -N '' -t $TYPE
> > +
> > +    # Move (Atomically rename) files
> > +    mv -f "${NAME}.tmp.pub" "${NAME}.pub"
> > +
> > +    # This sync does double duty: Ensuring that the data in the
> > temporary
> > +    # private key file is on disk before the rename, and ensuring
> > that the
> > +    # public key rename is completed before the private key
> > rename, since we
> > +    # switch on the existence of the private key to trigger key
> > generation.
> > +    # This does mean it is possible for the public key to exist,
> > but be garbage
> > +    # but this is OK because in that case the private key won't
> > exist and the
> > +    # keys will be regenerated.
> > +    #
> > +    # In the event that sync understands arguments that limit what
> > it tries to
> > +    # fsync(), we provided them. If it does not, it will simply
> > call sync()
> > +    # which is just as well
> > +    sync "${NAME}.pub" "$DIR" "${NAME}.tmp"
> > +
> > +    mv "${NAME}.tmp" "$NAME"
> > +fi
> > +
> > diff --git a/meta/recipes-
> > connectivity/openssh/openssh/sshdgenkeys.service b/meta/recipes-
> > connectivity/openssh/openssh/sshdgenkeys.service
> > index 148e6ad..23fd351 100644
> > --- a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> > +++ b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> > @@ -1,22 +1,23 @@
> >  [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
> > +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
> > +ExecStart=@BASE_BINDIR@/sync
> >  Type=oneshot
> >  RemainAfterExit=yes
> > diff --git a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> > b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> > index 5b96745..ec4b55f 100644
> > --- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> > +++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> > @@ -25,6 +25,7 @@ SRC_URI = "http://ftp.openbsd.org/pub/OpenBSD/Ope
> > nSSH/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' \
> > +               -e 's,@BASE_BINDIR@,${base_bindir},g' \
> > +               ${D}${sysconfdir}/init.d/sshd
> > +
> > +       install -D -m 0755 ${WORKDIR}/sshd-check-key
> > ${D}${libexecdir}/${BPN}
> >  }
> > 
> >  do_install_ptest () {
> > --
> > 2.9.4
> > 
> 
> Ping?

Re: [PATCH v7] openssh: Atomically generate host keys

[Joshua Watt]

On Tue, 2017-06-06 at 22:30 -0500, Joshua Watt wrote:
> On Wed, May 31, 2017 at 10:05 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     | 22 ++++----
> > ------
> >  .../openssh/openssh/sshd-check-key                 | 35
> > ++++++++++++++++++++++
> >  .../openssh/openssh/sshdgenkeys.service            | 25 ++++++++
> > --------
> >  meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  8 +++++
> >  4 files changed, 61 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..e02c479 100644
> > --- a/meta/recipes-connectivity/openssh/openssh/init
> > +++ b/meta/recipes-connectivity/openssh/openssh/init
> > @@ -45,23 +45,11 @@ 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
> > +    @BASE_BINDIR@/sync
> >  }
> > 
> >  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..3afdb8b
> > --- /dev/null
> > +++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
> > @@ -0,0 +1,35 @@
> > +#! /bin/sh
> > +NAME="$1"
> > +TYPE="$2"
> > +
> > +if [ -z "$NAME" ] || [ -z "$TYPE" ]; then
> > +    echo "Usage: $0 NAME TYPE"
> > +    exit 1
> > +fi
> > +
> > +
> > +if [ ! -f "$NAME" ]; then
> > +    DIR="$(dirname "$NAME")"
> > +
> > +    echo "  generating ssh $TYPE key..."
> > +    ssh-keygen -q -f "${NAME}.tmp" -N '' -t $TYPE
> > +
> > +    # Move (Atomically rename) files
> > +    mv -f "${NAME}.tmp.pub" "${NAME}.pub"
> > +
> > +    # This sync does double duty: Ensuring that the data in the
> > temporary
> > +    # private key file is on disk before the rename, and ensuring
> > that the
> > +    # public key rename is completed before the private key
> > rename, since we
> > +    # switch on the existence of the private key to trigger key
> > generation.
> > +    # This does mean it is possible for the public key to exist,
> > but be garbage
> > +    # but this is OK because in that case the private key won't
> > exist and the
> > +    # keys will be regenerated.
> > +    #
> > +    # In the event that sync understands arguments that limit what
> > it tries to
> > +    # fsync(), we provided them. If it does not, it will simply
> > call sync()
> > +    # which is just as well
> > +    sync "${NAME}.pub" "$DIR" "${NAME}.tmp"
> > +
> > +    mv "${NAME}.tmp" "$NAME"
> > +fi
> > +
> > diff --git a/meta/recipes-
> > connectivity/openssh/openssh/sshdgenkeys.service b/meta/recipes-
> > connectivity/openssh/openssh/sshdgenkeys.service
> > index 148e6ad..23fd351 100644
> > --- a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> > +++ b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
> > @@ -1,22 +1,23 @@
> >  [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
> > +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
> > +ExecStart=@BASE_BINDIR@/sync
> >  Type=oneshot
> >  RemainAfterExit=yes
> > diff --git a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> > b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> > index 5b96745..ec4b55f 100644
> > --- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> > +++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> > @@ -25,6 +25,7 @@ SRC_URI = "http://ftp.openbsd.org/pub/OpenBSD/Ope
> > nSSH/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' \
> > +               -e 's,@BASE_BINDIR@,${base_bindir},g' \
> > +               ${D}${sysconfdir}/init.d/sshd
> > +
> > +       install -D -m 0755 ${WORKDIR}/sshd-check-key
> > ${D}${libexecdir}/${BPN}
> >  }
> > 
> >  do_install_ptest () {
> > --
> > 2.9.4
> > 
> 
> Ping?

Ping? Am I missing something to get this merged?

[PATCH v8] openssh: Atomically generate host keys

[Joshua Watt]

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     | 24 +++--------------
 .../openssh/openssh/sshd-check-key                 | 30 ++++++++++++++++++++++
 .../openssh/openssh/sshdgenkeys.service            | 16 +++---------
 meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  8 ++++++
 4 files changed, 46 insertions(+), 32 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 386628a..acb35c3 100644
--- a/meta/recipes-connectivity/openssh/openssh/init
+++ b/meta/recipes-connectivity/openssh/openssh/init
@@ -80,26 +80,10 @@ check_keys() {
 	[ -z "${HOST_KEY_ED25519}" ] && HOST_KEY_ED25519=$SYSCONFDIR/ssh_host_ed25519_key
 
 	# create keys if necessary
-	if [ ! -f $HOST_KEY_RSA ]; then
-		echo "  generating ssh RSA key..."
-		mkdir -p $(dirname $HOST_KEY_RSA)
-		ssh-keygen -q -f $HOST_KEY_RSA -N '' -t rsa
-	fi
-	if [ ! -f $HOST_KEY_ECDSA ]; then
-		echo "  generating ssh ECDSA key..."
-		mkdir -p $(dirname $HOST_KEY_ECDSA)
-		ssh-keygen -q -f $HOST_KEY_ECDSA -N '' -t ecdsa
-	fi
-	if [ ! -f $HOST_KEY_DSA ]; then
-		echo "  generating ssh DSA key..."
-		mkdir -p $(dirname $HOST_KEY_DSA)
-		ssh-keygen -q -f $HOST_KEY_DSA -N '' -t dsa
-	fi
-	if [ ! -f $HOST_KEY_ED25519 ]; then
-		echo "  generating ssh ED25519 key..."
-		mkdir -p $(dirname $HOST_KEY_ED25519)
-		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..4999af2
--- /dev/null
+++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
@@ -0,0 +1,30 @@
+#! /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
+    mkdir -p "$(dirname "$NAME")"
+
+    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.5p1.bb b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
index 5b96745..ede8823 100644
--- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
+++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.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 ${D}${libexecdir}/${BPN}
+	install -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}
 }
 
 do_install_ptest () {
-- 
2.9.4

Re: [PATCH v8] openssh: Atomically generate host keys

[Joshua Watt]

On Tue, Jun 13, 2017 at 10:31 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     | 24 +++--------------
>  .../openssh/openssh/sshd-check-key                 | 30 ++++++++++++++++++++++
>  .../openssh/openssh/sshdgenkeys.service            | 16 +++---------
>  meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  8 ++++++
>  4 files changed, 46 insertions(+), 32 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 386628a..acb35c3 100644
> --- a/meta/recipes-connectivity/openssh/openssh/init
> +++ b/meta/recipes-connectivity/openssh/openssh/init
> @@ -80,26 +80,10 @@ check_keys() {
>         [ -z "${HOST_KEY_ED25519}" ] && HOST_KEY_ED25519=$SYSCONFDIR/ssh_host_ed25519_key
>
>         # create keys if necessary
> -       if [ ! -f $HOST_KEY_RSA ]; then
> -               echo "  generating ssh RSA key..."
> -               mkdir -p $(dirname $HOST_KEY_RSA)
> -               ssh-keygen -q -f $HOST_KEY_RSA -N '' -t rsa
> -       fi
> -       if [ ! -f $HOST_KEY_ECDSA ]; then
> -               echo "  generating ssh ECDSA key..."
> -               mkdir -p $(dirname $HOST_KEY_ECDSA)
> -               ssh-keygen -q -f $HOST_KEY_ECDSA -N '' -t ecdsa
> -       fi
> -       if [ ! -f $HOST_KEY_DSA ]; then
> -               echo "  generating ssh DSA key..."
> -               mkdir -p $(dirname $HOST_KEY_DSA)
> -               ssh-keygen -q -f $HOST_KEY_DSA -N '' -t dsa
> -       fi
> -       if [ ! -f $HOST_KEY_ED25519 ]; then
> -               echo "  generating ssh ED25519 key..."
> -               mkdir -p $(dirname $HOST_KEY_ED25519)
> -               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..4999af2
> --- /dev/null
> +++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
> @@ -0,0 +1,30 @@
> +#! /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
> +    mkdir -p "$(dirname "$NAME")"
> +
> +    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.5p1.bb b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> index 5b96745..ede8823 100644
> --- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
> +++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.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 ${D}${libexecdir}/${BPN}
> +       install -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}
>  }
>
>  do_install_ptest () {
> --
> 2.9.4
>

Argh. Nevermind. This was a rebase of the wrong patch version. I
shouldn't be trying this so late.

[PATCH v9] openssh: Atomically generate host keys

[Joshua Watt]

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     | 25 +++------------
 .../openssh/openssh/sshd-check-key                 | 37 ++++++++++++++++++++++
 .../openssh/openssh/sshdgenkeys.service            | 25 ++++++++-------
 meta/recipes-connectivity/openssh/openssh_7.5p1.bb |  8 +++++
 4 files changed, 63 insertions(+), 32 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 386628a..2832e67 100644
--- a/meta/recipes-connectivity/openssh/openssh/init
+++ b/meta/recipes-connectivity/openssh/openssh/init
@@ -80,26 +80,11 @@ check_keys() {
 	[ -z "${HOST_KEY_ED25519}" ] && HOST_KEY_ED25519=$SYSCONFDIR/ssh_host_ed25519_key
 
 	# create keys if necessary
-	if [ ! -f $HOST_KEY_RSA ]; then
-		echo "  generating ssh RSA key..."
-		mkdir -p $(dirname $HOST_KEY_RSA)
-		ssh-keygen -q -f $HOST_KEY_RSA -N '' -t rsa
-	fi
-	if [ ! -f $HOST_KEY_ECDSA ]; then
-		echo "  generating ssh ECDSA key..."
-		mkdir -p $(dirname $HOST_KEY_ECDSA)
-		ssh-keygen -q -f $HOST_KEY_ECDSA -N '' -t ecdsa
-	fi
-	if [ ! -f $HOST_KEY_DSA ]; then
-		echo "  generating ssh DSA key..."
-		mkdir -p $(dirname $HOST_KEY_DSA)
-		ssh-keygen -q -f $HOST_KEY_DSA -N '' -t dsa
-	fi
-	if [ ! -f $HOST_KEY_ED25519 ]; then
-		echo "  generating ssh ED25519 key..."
-		mkdir -p $(dirname $HOST_KEY_ED25519)
-		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
+	@BASE_BINDIR@/sync
 }
 
 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..56c500d
--- /dev/null
+++ b/meta/recipes-connectivity/openssh/openssh/sshd-check-key
@@ -0,0 +1,37 @@
+#! /bin/sh
+NAME="$1"
+TYPE="$2"
+
+if [ -z "$NAME" ] || [ -z "$TYPE" ]; then
+    echo "Usage: $0 NAME TYPE"
+    exit 1
+fi
+
+
+if [ ! -f "$NAME" ]; then
+    DIR="$(dirname "$NAME")"
+
+    mkdir -p "$DIR"
+
+    echo "  generating ssh $TYPE key..."
+    ssh-keygen -q -f "${NAME}.tmp" -N '' -t $TYPE
+
+    # Move (Atomically rename) files
+    mv -f "${NAME}.tmp.pub" "${NAME}.pub"
+
+    # This sync does double duty: Ensuring that the data in the temporary
+    # private key file is on disk before the rename, and ensuring that the
+    # public key rename is completed before the private key rename, since we
+    # switch on the existence of the private key to trigger key generation.
+    # This does mean it is possible for the public key to exist, but be garbage
+    # but this is OK because in that case the private key won't exist and the
+    # keys will be regenerated.
+    #
+    # In the event that sync understands arguments that limit what it tries to
+    # fsync(), we provided them. If it does not, it will simply call sync()
+    # which is just as well
+    sync "${NAME}.pub" "$DIR" "${NAME}.tmp"
+
+    mv "${NAME}.tmp" "$NAME"
+fi
+
diff --git a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
index 148e6ad..23fd351 100644
--- a/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
+++ b/meta/recipes-connectivity/openssh/openssh/sshdgenkeys.service
@@ -1,22 +1,23 @@
 [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
+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
+ExecStart=@BASE_BINDIR@/sync
 Type=oneshot
 RemainAfterExit=yes
diff --git a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb b/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
index 5b96745..cdca7ee 100644
--- a/meta/recipes-connectivity/openssh/openssh_7.5p1.bb
+++ b/meta/recipes-connectivity/openssh/openssh_7.5p1.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' \
+		-e 's,@BASE_BINDIR@,${base_bindir},g' \
+		${D}${sysconfdir}/init.d/sshd
+
+	install -D -m 0755 ${WORKDIR}/sshd-check-key ${D}${libexecdir}/${BPN}/sshd-check-key
 }
 
 do_install_ptest () {
-- 
2.9.4

Re: [PATCH] openssh: Atomically generate host keys

[Ulrich Ölmann]

On Tue, May 23, 2017 at 03:37:16PM +0100, Burton, Ross wrote:
> On 7 May 2017 at 02:33, Joshua Watt <jpewhacker@gmail.com> wrote:
> > 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
> >
> 
> Can you not continue to use ConditionPathExists to only run this unit if it
> needs to run?  You can prepend the argument with | to make them logical OR
> instead of logical AND, if I'm reading this documentation correctly.

Am I right that if we have a read-write mounted root-FS with already existing
keys in /etc/ssh the service unit will nevertheless be started on _every_ boot
now as the files which are checked for existance in /var/run/ssh are missing?

Best regards
Ulrich
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] openssh: Atomically generate host keys

[Joshua Watt]

On Tue, 2017-06-20 at 10:52 +0200, Ulrich Ölmann wrote:
> On Tue, May 23, 2017 at 03:37:16PM +0100, Burton, Ross wrote:
> > On 7 May 2017 at 02:33, Joshua Watt <jpewhacker@gmail.com> wrote:
> > > 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
> > > 
> > 
> > Can you not continue to use ConditionPathExists to only run this
> > unit if it
> > needs to run?  You can prepend the argument with | to make them
> > logical OR
> > instead of logical AND, if I'm reading this documentation
> > correctly.
> 
> Am I right that if we have a read-write mounted root-FS with already
> existing
> keys in /etc/ssh the service unit will nevertheless be started on
> _every_ boot
> now as the files which are checked for existance in /var/run/ssh are
> missing?

Yes. The service is actually run when the first ssh connection is made
(not at boot time), but it will do so on the first connection every
boot cycle. I don't know a way to do a complex conditional in systemd,
so this does the superset and makes sshd-check-key figure out if the
key actually needs generating or not. Perhaps there is a better way to
do this with the systemd conditional syntax that I am not aware of?
Ideally, the conditional checks in the systemd unit would be able to
use the SYSCONFDIR from /etc/default/ssh, but I'm not sure if that is
possible.

> 
> Best regards
> Ulrich
> -- 
> Pengutronix
> e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.d
> e/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-
> 0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-
> 5555 |

[PATCH v10] openssh: Atomically generate host keys

[Joshua Watt]

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.

[YOCTO #11671]

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 .../openssh/openssh/sshd_check_keys                | 42 +++++++++++++++++-----
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/meta/recipes-connectivity/openssh/openssh/sshd_check_keys b/meta/recipes-connectivity/openssh/openssh/sshd_check_keys
index f5bba53..5463b1a 100644
--- a/meta/recipes-connectivity/openssh/openssh/sshd_check_keys
+++ b/meta/recipes-connectivity/openssh/openssh/sshd_check_keys
@@ -1,5 +1,35 @@
 #! /bin/sh
 
+generate_key() {
+    local FILE=$1
+    local TYPE=$2
+    local DIR="$(dirname "$FILE")"
+
+    mkdir -p "$DIR"
+    ssh-keygen -q -f "${FILE}.tmp" -N '' -t $TYPE
+
+    # Atomically rename file public key
+    mv -f "${FILE}.tmp.pub" "${FILE}.pub"
+
+    # This sync does double duty: Ensuring that the data in the temporary
+    # private key file is on disk before the rename, and ensuring that the
+    # public key rename is completed before the private key rename, since we
+    # switch on the existence of the private key to trigger key generation.
+    # This does mean it is possible for the public key to exist, but be garbage
+    # but this is OK because in that case the private key won't exist and the
+    # keys will be regenerated.
+    #
+    # In the event that sync understands arguments that limit what it tries to
+    # fsync(), we provided them. If it does not, it will simply call sync()
+    # which is just as well
+    sync "${FILE}.pub" "$DIR" "${FILE}.tmp"
+
+    mv "${FILE}.tmp" "$FILE"
+
+    # sync to ensure the atomic rename is committed
+    sync "$DIR"
+}
+
 # /etc/default/ssh may set SYSCONFDIR and SSHD_OPTS
 if test -f /etc/default/ssh; then
     . /etc/default/ssh
@@ -43,22 +73,18 @@ HOST_KEY_ED25519=$(grep ^HostKey "${sshd_config}" | grep _ed25519_ | tail -1 | a
 # create keys if necessary
 if [ ! -f $HOST_KEY_RSA ]; then
     echo "  generating ssh RSA key..."
-    mkdir -p $(dirname $HOST_KEY_RSA)
-    ssh-keygen -q -f $HOST_KEY_RSA -N '' -t rsa
+    generate_key $HOST_KEY_RSA rsa
 fi
 if [ ! -f $HOST_KEY_ECDSA ]; then
     echo "  generating ssh ECDSA key..."
-    mkdir -p $(dirname $HOST_KEY_ECDSA)
-    ssh-keygen -q -f $HOST_KEY_ECDSA -N '' -t ecdsa
+    generate_key $HOST_KEY_ECDSA ecdsa
 fi
 if [ ! -f $HOST_KEY_DSA ]; then
     echo "  generating ssh DSA key..."
-    mkdir -p $(dirname $HOST_KEY_DSA)
-    ssh-keygen -q -f $HOST_KEY_DSA -N '' -t dsa
+    generate_key $HOST_KEY_DSA dsa
 fi
 if [ ! -f $HOST_KEY_ED25519 ]; then
     echo "  generating ssh ED25519 key..."
-    mkdir -p $(dirname $HOST_KEY_ED25519)
-    ssh-keygen -q -f $HOST_KEY_ED25519 -N '' -t ed25519
+    generate_key $HOST_KEY_ED25519 ed25519
 fi
 
-- 
2.9.4

✗ patchtest: failure for openssh: Atomically generate host keys (rev10)

[Patchwork]

== Series Details ==

Series: openssh: Atomically generate host keys (rev10)
Revision: 10
URL   : https://patchwork.openembedded.org/series/6626/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             Series does not apply on top of target branch [test_series_merge_on_head] 
  Suggested fix    Rebase your series on top of targeted branch
  Targeted branch  master (currently at b1c4661742)



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

[PATCH v11] openssh: Atomically generate host keys

[Joshua Watt]

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.

[YOCTO #11671]

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 .../openssh/openssh/sshd_check_keys                | 42 +++++++++++++++++-----
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/meta/recipes-connectivity/openssh/openssh/sshd_check_keys b/meta/recipes-connectivity/openssh/openssh/sshd_check_keys
index f5bba53ca31..5463b1a4cb1 100644
--- a/meta/recipes-connectivity/openssh/openssh/sshd_check_keys
+++ b/meta/recipes-connectivity/openssh/openssh/sshd_check_keys
@@ -1,5 +1,35 @@
 #! /bin/sh
 
+generate_key() {
+    local FILE=$1
+    local TYPE=$2
+    local DIR="$(dirname "$FILE")"
+
+    mkdir -p "$DIR"
+    ssh-keygen -q -f "${FILE}.tmp" -N '' -t $TYPE
+
+    # Atomically rename file public key
+    mv -f "${FILE}.tmp.pub" "${FILE}.pub"
+
+    # This sync does double duty: Ensuring that the data in the temporary
+    # private key file is on disk before the rename, and ensuring that the
+    # public key rename is completed before the private key rename, since we
+    # switch on the existence of the private key to trigger key generation.
+    # This does mean it is possible for the public key to exist, but be garbage
+    # but this is OK because in that case the private key won't exist and the
+    # keys will be regenerated.
+    #
+    # In the event that sync understands arguments that limit what it tries to
+    # fsync(), we provided them. If it does not, it will simply call sync()
+    # which is just as well
+    sync "${FILE}.pub" "$DIR" "${FILE}.tmp"
+
+    mv "${FILE}.tmp" "$FILE"
+
+    # sync to ensure the atomic rename is committed
+    sync "$DIR"
+}
+
 # /etc/default/ssh may set SYSCONFDIR and SSHD_OPTS
 if test -f /etc/default/ssh; then
     . /etc/default/ssh
@@ -43,22 +73,18 @@ HOST_KEY_ED25519=$(grep ^HostKey "${sshd_config}" | grep _ed25519_ | tail -1 | a
 # create keys if necessary
 if [ ! -f $HOST_KEY_RSA ]; then
     echo "  generating ssh RSA key..."
-    mkdir -p $(dirname $HOST_KEY_RSA)
-    ssh-keygen -q -f $HOST_KEY_RSA -N '' -t rsa
+    generate_key $HOST_KEY_RSA rsa
 fi
 if [ ! -f $HOST_KEY_ECDSA ]; then
     echo "  generating ssh ECDSA key..."
-    mkdir -p $(dirname $HOST_KEY_ECDSA)
-    ssh-keygen -q -f $HOST_KEY_ECDSA -N '' -t ecdsa
+    generate_key $HOST_KEY_ECDSA ecdsa
 fi
 if [ ! -f $HOST_KEY_DSA ]; then
     echo "  generating ssh DSA key..."
-    mkdir -p $(dirname $HOST_KEY_DSA)
-    ssh-keygen -q -f $HOST_KEY_DSA -N '' -t dsa
+    generate_key $HOST_KEY_DSA dsa
 fi
 if [ ! -f $HOST_KEY_ED25519 ]; then
     echo "  generating ssh ED25519 key..."
-    mkdir -p $(dirname $HOST_KEY_ED25519)
-    ssh-keygen -q -f $HOST_KEY_ED25519 -N '' -t ed25519
+    generate_key $HOST_KEY_ED25519 ed25519
 fi
 
-- 
2.13.5