Re: [PATCH] sshd-regen-keys: Fix sshd deadlock on boot

[Claudius Heine <claudius.heine.ext@siemens.com>]

Hi,

On 13/12/2018 10.48, Harald Seiler wrote:
> Hello Claudius,
> 
> On Thu, 2018-12-13 at 10:41 +0100, Claudius Heine wrote:
>> Hi Harald,
>>
>> On 13/12/2018 09.56, Harald Seiler wrote:
>>> Currently, when sshd-regen-keys runs dpkg-reconfigure, this
>>> will lead to a call to `systemctl restart ssh`.  This call blocks
>>> forever because of course the sshd-regen-keys unit, which is a
>>> dependency of sshd, hasn't finished at this point and can't do so
>>> because it is waiting as well.
>>>
>>> To circumvent this deadlock, this commit changes sshd-regen-keys'
>>> behavior so sshd is first disabled and only reenabled after the
>>> job is done.
>>>
>>> Signed-off-by: Harald Seiler <hws@denx.de>
>>> ---
>>>    .../sshd-regen-keys/files/sshd-regen-keys.service     |  2 +-
>>>    .../sshd-regen-keys/files/sshd-regen-keys.sh          | 19 +++++++++++++++++++
>>>    .../sshd-regen-keys/sshd-regen-keys_0.1.bb            |  7 +++++--
>>>    3 files changed, 25 insertions(+), 3 deletions(-)
>>>    create mode 100644 meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.sh
>>>
>>> diff --git a/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.service b/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.service
>>> index 3b8231f..a05e1a9 100644
>>> --- a/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.service
>>> +++ b/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.service
>>> @@ -10,7 +10,7 @@ ConditionPathIsReadWrite=/etc
>>>    Type=oneshot
>>>    RemainAfterExit=yes
>>>    Environment=DEBIAN_FRONTEND=noninteractive
>>> -ExecStart=/bin/sh -c "rm -v /etc/ssh/ssh_host_*_key*; dpkg-reconfigure openssh-server"
>>> +ExecStart=/usr/sbin/sshd-regen-keys.sh
>>>    ExecStartPost=-/bin/systemctl disable sshd-regen-keys.service
>>>    StandardOutput=syslog
>>>    StandardError=syslog
>>> diff --git a/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.sh b/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.sh
>>> new file mode 100644
>>> index 0000000..294e8fa
>>> --- /dev/null
>>> +++ b/meta/recipes-support/sshd-regen-keys/files/sshd-regen-keys.sh
>>> @@ -0,0 +1,19 @@
>>> +#!/usr/bin/env sh
>>> +
>>> +echo -n "SSH server is "
>>> +if systemctl is-enabled ssh; then
>>> +    SSHD_ENABLED="true"
>>> +    systemctl disable --no-reload ssh
>>> +fi
>>> +
>>> +echo "Removing keys ..."
>>> +rm -v /etc/ssh/ssh_host_*_key*
>>> +
>>> +echo "Regenerating keys ..."
>>> +dpkg-reconfigure openssh-server
>>
>> Since this is part of 'meta', does it make sense to make the package
>> name+service file name configurable from the bitbake configuration or is
>> that too much trouble.
>>
> 
> I don't quite understand what you mean, can you please
> elaborate on that?

Basically if those names should be configurable from the isar 
distro/multiconfig etc. E.g. what happens if I decided to use some 
openssh replacement or a different/future debian based distribution?

IIUC ideally `meta` should be distribution independent.

So if that is wanted then we would need to create those files via some 
template mechanism, e.g. envsubst or just sed.

But since sshd-regen-keys already depends on those elsewhere, that point 
might just be out of scope of this patch. So I let you decide. :)

> 
>>> +
>>> +if test -n $SSHD_ENABLED; then
>>> +    echo "Reenabling ssh server ..."
>>> +    systemctl enable --no-reload ssh
>>> +    systemctl start --no-block ssh
>>
>> Should the service be stopped before? Or in other words: Does it make
>> sense to differentiate between sshd enabled and started in this script?
> 
> ssh.service is guaranteed to be stopped because it requires
> sshd-regen-keys.service to be done before starting.

Ok. Make sense. Then its impossible or very difficult to figure out if 
sshd was started but disabled before your script was entered (in which 
case you would have to start it but not enable it).

Claudius

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de