From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1k83hQ-0006LC-Mi for ptxdist@pengutronix.de; Tue, 18 Aug 2020 17:38:56 +0200 Received: from mol by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1k83hQ-0006Gg-At for ptxdist@pengutronix.de; Tue, 18 Aug 2020 17:38:56 +0200 Date: Tue, 18 Aug 2020 17:38:56 +0200 From: Michael Olbrich Message-ID: <20200818153856.GD13376@pengutronix.de> References: <20200817144502.11265-1-christian.hermann@hytera.de> <20200817144502.11265-2-christian.hermann@hytera.de> <20200818065735.GG8684@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Subject: Re: [ptxdist] [PATCH 1/1] openssh/rc-once: build all supported hostkeys, simplify List-Id: PTXdist Development Mailing List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: ptxdist@pengutronix.de Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ptxdist-bounces@pengutronix.de Sender: "ptxdist" To: ptxdist@pengutronix.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 > >> --- > >> 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