Re: [ptxdist] [PATCH 1/1] openssh/rc-once: build all supported hostkeys, simplify

[Michael Olbrich <m.olbrich@pengutronix.de>]

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 <christian.hermann@hytera.de>
> ---
>  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
> ptxdist@pengutronix.de
> To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de
> 

-- 
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
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de