On Mon, Aug 17, 2020 at 04:45:02PM +0200, Christian Hermann wrote:
> 1. Ensure all supported hostkeys are generated, even if not configured
> explicitly.
> Parsing /etc/sshd_config is insufficient for determining which types of
> hostkeys are actually supported by the running instance of sshd.
> Ask `sshd -T` instead, as openssh uses sensible defaults if no custom
> 'HostKey' lines are present in /etc/ssh/sshd_config.

I like this part. But please split it into a separate patch to make it more
readable.

> 2. Simplify key generation logic

This needs some changes.

> Signed-off-by: Christian Hermann <[email protected]>
> ---
>  projectroot/etc/rc.once.d/openssh | 57 ++++++++++++++-----------------
>  1 file changed, 25 insertions(+), 32 deletions(-)
> 
> diff --git a/projectroot/etc/rc.once.d/openssh 
> b/projectroot/etc/rc.once.d/openssh
> index 66cfa06df..bc3634958 100644
> --- a/projectroot/etc/rc.once.d/openssh
> +++ b/projectroot/etc/rc.once.d/openssh
> @@ -3,47 +3,40 @@
>  PATH=/sbin:/bin:/usr/sbin:/usr/bin
>  
>  get_hostkeys() {
> -     [ -f /etc/ssh/sshd_config ] || return
> -     sed -n 's/^HostKey[ \t][ \t]*\(.*\)/\1/p' /etc/ssh/sshd_config
> -}
> -
> -host_keys_required() {
> -     hostkeys="$(get_hostkeys)"
> -     if [ "$hostkeys" ]; then
> -             echo "$hostkeys"
> -     else
> -             # No HostKey directives found, so we pick secure defaults
> -             echo /etc/ssh/ssh_host_ed25519_key
> -     fi
> +     sshd -T | sed -E -n -e 's/^hostkey[[:space:]]+(.*)/\1/p'
>  }
>  
>  create_key() {
> -     keytype="$1"
> -     prettykeytype="$(echo $_type | tr a-z A-Z)"
> -     shift
> -     hostkeys="$1"
> -     shift
> -
> -     file="/etc/ssh/ssh_host_${keytype}_key"
> -
> -     if echo "$hostkeys" | grep -x -F "$file" >/dev/null; then
> -             echo "Create $prettykeytype key; this may take some time ..."
> -             rm -f $file &&
> -             ssh-keygen -q -f "$file" -N '' -t "$keytype" "$@" || return
> -             echo "Created $prettykeytype key."
> -     fi
> +     keyfile="$1"
> +     keytype="$(echo "$keyfile" | sed -E -e 's/.*ssh_host_(.*)_key$/\1/')"
> +     prettykeytype="$(echo "$keytype" | tr '[:lower:]' '[:upper:]')"

So, I have a patch queued for mainline that removed the $prettykeytype
entirely. I came across a BSP that didn't have 'tr' at all. And ':lower:' /
':upper:' is worse, because that's a separate option in busybox...

> +
> +     keygen_args=
> +     case "$keytype" in
> +             rsa) keygen_args="-b 4096" ;;
> +     esac
> +
> +     echo "Creating $prettykeytype key"
> +     rm -f "$keyfile"

Keep the '&&'. If the file cannot be deleted, then calling ssh-keygen makes
no sense.

> +     ssh-keygen -q -f "$keyfile" -N "" -t "$keytype" $keygen_args
>  }
>  
>  create_keys() {
> -     hostkeys="$(host_keys_required)"
> +     hostkeys="$(get_hostkeys)"
> +
> +     # no hostkeys reported by sshd.  Try to provide a fallback
> +     #FIXME: if `sshd -T` fails, we very likely have bigger problems. shout 
> to stderr and exit instead?

Hmmm, I think, this requires some testing. What happens is if there are no
Keys configured in the config? Or a syntax error?

But in the end, I guess that sshd won't work anyways, so I think we should
just fail with an appropriate error message.

> +     if [ -z "$hostkeys" ]; then
> +             fallback="/etc/ssh/ssh_host_ed25519_key"
> +             echo "HostKey $fallback" >>/etc/ssh/sshd_config

Definitively not. Don't touch the config.

> +     fi
>  
> -     create_key "dsa" "$hostkeys" &&
> -     create_key "ecdsa" "$hostkeys" &&
> -     create_key "ed25519" "$hostkeys" &&
> -     create_key "rsa" "$hostkeys" -b 4096
> +     for keyfile in $hostkeys; do
> +             create_key "$keyfile"

                create_key "$keyfile" || return

We need to propagate the error here. Otherwise, only the last key will
produce an error.
If we cannot create a key, then something is really broken.

> +     done
>  }
>  
>  if ! create_keys; then
> -     echo "Generating SSH keys failed!"
> +     echo "Generating SSH keys failed!" >&2

I don't care either way, but does this make a difference? The message
should be visible on the console either way.

Michael

>       exit 1
>  fi
> -- 
> 2.28.0
> 
> 
> _______________________________________________
> ptxdist mailing list
> [email protected]
> To unsubscribe, send a mail with subject "unsubscribe" to 
> [email protected]
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
[email protected]
To unsubscribe, send a mail with subject "unsubscribe" to 
[email protected]

Reply via email to