On Tue, Aug 18, 2020 at 04:02:53PM +0200, Christian Hermann wrote: > After playing some more, it seems to be not such a good idea after-all. > > `sshd -T` fails if none of the (implicitly or explicitly) configured > keys are available. This makes it unsuitable for rc-once where no keys > should be present > > ``` > [root@host ~]# rm /etc/ssh/ssh*key* > [root@host ~]# sshd -T > Unable to load host key: /etc/ssh/ssh_host_ed25519_key > sshd: no hostkeys available -- exiting. > ``` > > What i instead like to use on systems powerful enough is `ssh-keygen -A` > right before sshd is started, which of course isn't suitable for every > platform (it build all possible keys, including RSA keys which take very > long incapable hardware). > > So guess I need to retire this RFC.
We could still do the second part: Loop over the found keys instead of trying a fixed list and skip any that are not configured. Michael > Thanks for your review Michael > > On 18.08.20 08:57, Michael Olbrich wrote: > > 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] > >> > > > > _______________________________________________ > 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]
