mailarchive of the ptxdist mailing list
 help / color / mirror / Atom feed
From: Michael Olbrich <m.olbrich@pengutronix.de>
To: ptxdist@pengutronix.de
Subject: Re: [ptxdist] [PATCH 1/1] openssh/rc-once: build all supported hostkeys, simplify
Date: Tue, 18 Aug 2020 17:38:56 +0200	[thread overview]
Message-ID: <20200818153856.GD13376@pengutronix.de> (raw)
In-Reply-To: <b5482022-ed65-ac96-b8d4-0c60014efa74@hytera.de>

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

      reply	other threads:[~2020-08-18 15:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-17 14:45 [ptxdist] [RFC] rework openssh/rc-once Christian Hermann
2020-08-17 14:45 ` [ptxdist] [PATCH 1/1] openssh/rc-once: build all supported hostkeys, simplify Christian Hermann
2020-08-18  6:57   ` Michael Olbrich
2020-08-18 11:35     ` Christian Hermann
2020-08-18 14:02     ` Christian Hermann
2020-08-18 15:38       ` Michael Olbrich [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200818153856.GD13376@pengutronix.de \
    --to=m.olbrich@pengutronix.de \
    --cc=ptxdist@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox