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 <[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...
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
>> [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]
