* [ptxdist] [RFC] rework openssh/rc-once
@ 2020-08-17 14:45 Christian Hermann
2020-08-17 14:45 ` [ptxdist] [PATCH 1/1] openssh/rc-once: build all supported hostkeys, simplify Christian Hermann
0 siblings, 1 reply; 6+ messages in thread
From: Christian Hermann @ 2020-08-17 14:45 UTC (permalink / raw)
To: ptxdist
Hi list,
as of now, shipping the sshd_config file with default "HostKey" lines
results in only the eds25519 hostkey being built when rc-once is run.
This patch makes use of sshd's own configuration parsing capabilities to
determine which key types are implicit defaults.
Beware of the FIXME in the script, which is up for discussion as well of
course.
Kind regards
_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ptxdist] [PATCH 1/1] openssh/rc-once: build all supported hostkeys, simplify
2020-08-17 14:45 [ptxdist] [RFC] rework openssh/rc-once Christian Hermann
@ 2020-08-17 14:45 ` Christian Hermann
2020-08-18 6:57 ` Michael Olbrich
0 siblings, 1 reply; 6+ messages in thread
From: Christian Hermann @ 2020-08-17 14:45 UTC (permalink / raw)
To: ptxdist
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.
2. Simplify key generation logic
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:]')"
+
+ keygen_args=
+ case "$keytype" in
+ rsa) keygen_args="-b 4096" ;;
+ esac
+
+ echo "Creating $prettykeytype key"
+ rm -f "$keyfile"
+ 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?
+ if [ -z "$hostkeys" ]; then
+ fallback="/etc/ssh/ssh_host_ed25519_key"
+ echo "HostKey $fallback" >>/etc/ssh/sshd_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"
+ done
}
if ! create_keys; then
- echo "Generating SSH keys failed!"
+ echo "Generating SSH keys failed!" >&2
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ptxdist] [PATCH 1/1] openssh/rc-once: build all supported hostkeys, simplify
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
0 siblings, 2 replies; 6+ messages in thread
From: Michael Olbrich @ 2020-08-18 6:57 UTC (permalink / raw)
To: ptxdist
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ptxdist] [PATCH 1/1] openssh/rc-once: build all supported hostkeys, simplify
2020-08-18 6:57 ` Michael Olbrich
@ 2020-08-18 11:35 ` Christian Hermann
2020-08-18 14:02 ` Christian Hermann
1 sibling, 0 replies; 6+ messages in thread
From: Christian Hermann @ 2020-08-18 11:35 UTC (permalink / raw)
To: ptxdist
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.
will do, thanks.
>
>> 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...
Yeah, testing this with busybox was on my list as well...
>
>> +
>> + 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.
indeed. I'm inclined to just `set -e` on top then
>
>> + 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.
It doesn't make a difference of course, i just considered it good style
to print errors to stderr.
it's your call in the end
>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ptxdist] [PATCH 1/1] openssh/rc-once: build all supported hostkeys, simplify
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
1 sibling, 1 reply; 6+ messages in thread
From: Christian Hermann @ 2020-08-18 14:02 UTC (permalink / raw)
To: ptxdist
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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ptxdist] [PATCH 1/1] openssh/rc-once: build all supported hostkeys, simplify
2020-08-18 14:02 ` Christian Hermann
@ 2020-08-18 15:38 ` Michael Olbrich
0 siblings, 0 replies; 6+ messages in thread
From: Michael Olbrich @ 2020-08-18 15:38 UTC (permalink / raw)
To: ptxdist
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-18 15:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox