Skip to content

Add sshd-keygen disable drop-in conf#1028

Merged
TheRealFalcon merged 3 commits into
canonical:mainfrom
raharper:fix/disable-sshd-keygen-if-cloud-init-runs
Oct 12, 2021
Merged

Add sshd-keygen disable drop-in conf#1028
TheRealFalcon merged 3 commits into
canonical:mainfrom
raharper:fix/disable-sshd-keygen-if-cloud-init-runs

Conversation

@raharper
Copy link
Copy Markdown
Collaborator

Proposed Commit Message

Inhibit sshd-keygen@.service if cloud-init is active

In some cloud-init enabled images the sshd-keygen@.service
may race with cloud-init and prevent ssh host keys from being
generated or generating host keys twice slowing boot and  consuming
additional entropy during boot.  This drop-in unit adds a condition to
the sshd-keygen@.service which prevents running if cloud-init is active.

Additional Context

Test Steps

  1. boot centos/rhel based cloud-init image
  2. check systemctl status sshd-keygen@ecdsa to see if service ran (it should run)
  3. install/upgrade cloud-init package from this PR
  4. cloud-init clean --logs ; rm -f /etc/ssh/ssh_host*; reboot
  5. check systemctl status sshd-keygen@ecdsa to see if service ran (it should NOT run)

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

@esposem
Copy link
Copy Markdown
Contributor

esposem commented Sep 21, 2021

Thank you for the PR!
I tried to backport this fix on rhel, but RPM generation fails with the following error

Checking for unpackaged file(s): /usr/lib/rpm/check-files /builddir/build/BUILDROOT/cloud-init-21.1-8.el9.esposem202109210940.noarch
RPM build errors:
error: Installed (but unpackaged) file(s) found:
   /etc/systemd/system/sshd-keygen@.service.d/disable-sshd-keygen-if-cloud-init-active.conf
    File listed twice: /usr/libexec/cloud-init/ds-identify
    File listed twice: /usr/share/doc/cloud-init
    Installed (but unpackaged) file(s) found:
   /etc/systemd/system/sshd-keygen@.service.d/disable-sshd-keygen-if-cloud-init-active.conf
Child return code was: 1
EXCEPTION: [Error()]
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/mockbuild/trace_decorator.py", line 93, in trace
    result = func(*args, **kw)
  File "/usr/lib/python3.6/site-packages/mockbuild/util.py", line 789, in do_with_status
    raise exception.Error("Command failed: \n # %s\n%s" % (command, output), child.returncode)
mockbuild.exception.Error: Command failed: 
 # bash --login -c /usr/bin/rpmbuild -bb --target noarch --nodeps /builddir/build/SPECS/cloud-init.spec

I am not an expert on these things, but when trying to do the downstream fix I got this error when I forgot to add the file to %files.

Also, another thing worth mentioning is that when I tried to pack the drop-in downstream (as you initially suggested) I just had to modify the rhel-specific cloud-init.spec file (not setup.py), and use RPM_BUILD_ROOT/%{_unitdir} (which I suppose maps to 'systemdsystemunitdir': '/lib/systemd/system'), otherwise it would not have permission to move the file in /etc/systemd/system.

@raharper
Copy link
Copy Markdown
Collaborator Author

@esposem Thanks for testing that out. I suspect downstreams will need to manually patch this in since they're using cloud-init releases which don't include the file yet.

I haven't yet adjusted the upstream rpm build spec (we use cloud-init/package/ let me do that now.

@raharper raharper force-pushed the fix/disable-sshd-keygen-if-cloud-init-runs branch from 25e9904 to f820ed9 Compare September 21, 2021 21:38
@esposem
Copy link
Copy Markdown
Contributor

esposem commented Sep 23, 2021

Uhm still the same problem and same error, the RHEL rpm does not build.

I suspect downstreams will need to manually patch this in since they're using cloud-init releases which don't include the file yet.

So from what I understand I can pack the fix downstream only, but then there should be no upstream PR.
If instead we want the fix upstream, then this fix should work also when backporting this commit downstream (new file included).

Since we are going with the latter option, we need to make this fix work :)

@esposem
Copy link
Copy Markdown
Contributor

esposem commented Sep 23, 2021

@raharper or @TheRealFalcon can you please test this patch on top of the current PR commit? I can't push on this PR. This works on RHEL (also tested that the file is effectively added and works), and allows me (on fedora) to generate a cloudinit RPM by doing make rpm. I do not know what happens on other distros, though.

0001-adjust-as-done-downstream.patch.txt

@TheRealFalcon
Copy link
Copy Markdown
Contributor

What I see looks good to me, but @raharper has more knowledge of this area. I'd like his input on this.

@raharper
Copy link
Copy Markdown
Collaborator Author

raharper commented Oct 6, 2021

Sorry for not replying sooner.

So from what I understand I can pack the fix downstream only, but then there should be no upstream PR.

Not quite. There will be an upstream patch that works for all of the distros we build from. Our upstream spec is in the repo and we build that. Downstreams typically have their own spec file and this PR may not apply directly as downstream has different build requirements than upstream does.

Uhm still the same problem and same error, the RHEL rpm does not build.

If you compare your cloud-init.spec file with https://github.com/canonical/cloud-init/blob/main/packages/redhat/cloud-init.spec.in are they same or do they differ? I suspect they differ. The PR as it is now does build with upstream. We build with:

% cd cloud-init
% ./tools/run-container --package --source-package centos/8
...
% ls -altr cloud-init*22*el8*
-rw-r--r-- 1 rharper rharper 1145292 Sep 21 15:59 cloud-init-21.3+22.g136261b6-1.el8.noarch.rpm
-rw-r--r-- 1 rharper rharper 1361306 Sep 21 15:59 cloud-init-21.3+22.g136261b6-1.el8.src.rpm
% rpm -qpi cloud-init-21.3+22.g136261b6-1.el8.src.rpm 
Name        : cloud-init
Version     : 21.3+22.g136261b6
Release     : 1.el8
Architecture: noarch
Install Date: (not installed)
Group       : System Environment/Base
Size        : 1358652
License     : Dual-licesed GPLv3 or Apache 2.0
Signature   : (none)
Source RPM  : (none)
Build Date  : Tue 21 Sep 2021 15:59:28 CDT
Build Host  : ci-optimum-beetle
Relocations : (not relocatable)
URL         : http://launchpad.net/cloud-init
Summary     : Cloud instance init scripts
Description :
Cloud-init is a set of init scripts for cloud instances.  Cloud instances
need special scripts to run during initialization to retrieve and install
ssh keys and to let the user run various scripts.
% rpm -qpl cloud-init-21.3+22.g136261b6-1.el8.noarch.rpm | grep sshd-keygen
/etc/systemd/system/sshd-keygen@.service.d/disable-sshd-keygen-if-cloud-init-active.conf

Upstream needs to package this fix into the cloud-init tarball that the python setup.py generates as that's the release input for building from source.

@esposem
Copy link
Copy Markdown
Contributor

esposem commented Oct 11, 2021

If you compare your cloud-init.spec file with https://github.com/canonical/cloud-init/blob/main/packages/redhat/cloud-init.spec.in are they same or do they differ? I suspect they differ.

Yes, it differs.
Ok, from my side I think this PR can go ahead, we will take care of downstream. Thank you!

@TheRealFalcon TheRealFalcon merged commit b3e31ba into canonical:main Oct 12, 2021
@xiaoge1001
Copy link
Copy Markdown
Contributor

[root@localhost ~]# systemctl status cloud-init
○ cloud-init.service - Initial cloud-init job (metadata service crawler)
Loaded: loaded (/usr/lib/systemd/system/cloud-init.service; enabled; vendor preset: disabled)
Active: inactive (dead)

Feb 04 15:20:21 localhost cloud-init[1909]: ci-info: ... ...
... ...

cloud-init is in inactive, /etc/ssh/ssh_host_rsa_key generation is also affected. I can reproduce it. That should be a problem.

@holmanb
Copy link
Copy Markdown
Member

holmanb commented Feb 4, 2023

@xiaoge1001 please file a bug report on launchpad if you believe there to be an issue related to this pull request.

@xiaoge1001
Copy link
Copy Markdown
Contributor

@holmanb I've reported the problem.
https://bugs.launchpad.net/cloud-init/+bug/2004632

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants