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

[Jan Kiszka <jan.kiszka@siemens.com>]

On 13.12.18 11:03, [ext] Claudius Heine wrote:
> 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. :)
> 

I agree on the general goal but I think we could be more relaxed at this stage 
/wrt optional support packages like this one. Eventually, we can sort out also 
these kind of dependencies but we will also need proper test cases for such 
abstractions which we lack at this point.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux