From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from dude02.hi.pengutronix.de ([2001:67c:670:100:1d::28]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1k7vYt-0004mJ-C7 for ptxdist@pengutronix.de; Tue, 18 Aug 2020 08:57:35 +0200 Received: from mol by dude02.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1k7vYt-0006CH-2n for ptxdist@pengutronix.de; Tue, 18 Aug 2020 08:57:35 +0200 Date: Tue, 18 Aug 2020 08:57:35 +0200 From: Michael Olbrich Message-ID: <20200818065735.GG8684@pengutronix.de> References: <20200817144502.11265-1-christian.hermann@hytera.de> <20200817144502.11265-2-christian.hermann@hytera.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200817144502.11265-2-christian.hermann@hytera.de> 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 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 > -- 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