Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential for systemd dependency cycles #63

Closed
sagepe opened this issue Jan 21, 2022 · 1 comment · Fixed by #64
Closed

Potential for systemd dependency cycles #63

sagepe opened this issue Jan 21, 2022 · 1 comment · Fixed by #64
Labels
bug Something isn't working

Comments

@sagepe
Copy link
Contributor

sagepe commented Jan 21, 2022

We see occasional systemd dependency cycles on instances which run cloud-init that result in the ipset start job being deleted at system startup, delaying the firewall configuration until Puppet has run post-boot.

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: 6.26.0
  • Ruby: 2.5.9p229
  • Distribution: Debian 11
  • Module version: 2.1.0

How to reproduce (e.g Puppet code you use)

We set firewall_service => 'netfilter-persistent.service',

Our server instances are provisioned with cloud-init.

What are you seeing

We see a systemd dependency cycle that sometimes results in the ipset start job being deleted, knocking on to the iptables-persistent service failing as the rules use ipsets. This means that the server instance firewalls aren't configured at boot until Puppet runs subsequently and sorts it all out, leaving a vulnerable window of time.

What behaviour did you expect instead

No dependency cycles and a firewall before the networking comes up.

Output log

network-pre.target: Job ipset.service/start deleted to break ordering cycle starting with network-pre.target/start

Any additional information you'd like to impart

As it stands, the service unit file shipped with this module sets:

Before=network-pre.target iptables.service ip6tables.service
Wants=network-pre.target

It has no setting for DefaultDependencies which means it gets the defaults, one of which is After=sysinit.target.

This is good enough to get the ipsets configured before networking is started in many cases... until you introduce another unit that wants to run after networking, but before sysinit.target. You then end up with dependency cycles which sometimes result in the ipset job being deleted, which might mean no firewall if that relies upon ipsets (which ours does via puppetlabs-firewall module and the iptables-persistent service).

One example of a service that clashes in this way is cloud-init at some of our providers, which has some constraints over when the various stages need to run, and often has DefaultDependencies=no, After=networking.service and Before=sysinit.target to meet them.

I understand that this issue is going to be somewhat environment specific, but given that it seems to me that you will always want your ipsets configured early in the boot process and before networking, I think the implicit dependency of After=sysinit.target should be dropped by setting DefaultDependencies=no. The systemd documentation linked above says "Only services involved with early boot or late system shutdown should disable this option" - which seems to be the case here.

To work around this, we've added a drop-in file with the following (similar to what's in the netfilter-persistent.service unit - the other settings ensure some of the other settings implied by DefaultDependencies are handled sensibly):

[Unit]
DefaultDependencies=no
After=systemd-modules-load.service local-fs.target
Wants=systemd-modules-load.service local-fs.target
Before=shutdown.target
Conflicts=shutdown.target

This appears to work well for us and I suspect that these settings could go into the main unit file safely, or at least are worth documenting as something to keep in mind.

Happy to send a PR to this effect if you agree, but I thought I'd open this issue first to allow for discussion.

@bastelfreak
Copy link
Member

Hi @sagepe , thanks for the nice writeup! I think adding your changes to the default unit is a good idea. can you provide a PR for it?

sagepe added a commit to mysociety/puppet-ipset that referenced this issue Feb 25, 2022
The current ipset service unit sets `Before=network-pre.target` as it is
needed before the network to help ensure that the firewall is up and
running before anyone might actually try to connect. It has no setting
for `DefaultDependencies` which means it takes the defaults, one of
which is `After=sysinit.target`.

There are some cases where this can cause a dependency cycle with other
units that want to start early in the boot process between
`networking.service` and `sysinit.target` (for example, cloud-init).
This results in one of the offending units being removed and risks
startup continuing without ipsets being configured, potentially leaving
a firewall open and a host at risk.

The ipset service can safely be run before `sysinit.target` by setting
`DefaultDependencies=no` with a couple of additional dependencies to
handle some of the other implied settings that are removed as a result.

Fixes voxpupuli#63
sagepe added a commit to mysociety/puppet-ipset that referenced this issue Nov 13, 2022
The current ipset service unit sets `Before=network-pre.target` as it is
needed before the network to help ensure that the firewall is up and
running before anyone might actually try to connect. It has no setting
for `DefaultDependencies` which means it takes the defaults, one of
which is `After=sysinit.target`.

There are some cases where this can cause a dependency cycle with other
units that want to start early in the boot process between
`networking.service` and `sysinit.target` (for example, cloud-init).
This results in one of the offending units being removed and risks
startup continuing without ipsets being configured, potentially leaving
a firewall open and a host at risk.

The ipset service can safely be run before `sysinit.target` by setting
`DefaultDependencies=no` with a couple of additional dependencies to
handle some of the other implied settings that are removed as a result.

Fixes voxpupuli#63
@jhoblitt jhoblitt added the bug Something isn't working label Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants