Skip to content

fix(vpn/ipsec): bound shutdown when charon-nm ignores SIGTERM - #250

Open
sofianebeloucif wants to merge 1 commit into
cloud-gouv:mainfrom
sofianebeloucif:fix/charon-nm-shutdown
Open

sofianebeloucif wants to merge 1 commit into
cloud-gouv:mainfrom
sofianebeloucif:fix/charon-nm-shutdown

Conversation

@sofianebeloucif

@sofianebeloucif sofianebeloucif commented Aug 28, 2026

Copy link
Copy Markdown

Summary

Fixes #205. A hung charon-nm process (strongSwan's NetworkManager
IPsec plugin) can block system shutdown/reboot for a long time.

charon-nm does not always handle SIGTERM correctly, in particular
when a VPN connection attempt hasn't fully completed. When that
happens it keeps running inside NetworkManager's cgroup even after
NetworkManager.service itself reports as stopped (its Type=dbus
completion condition -- losing its D-Bus name -- doesn't require its
whole cgroup to be empty).

Setting NetworkManager.service's own TimeoutStopSec is not
enough on its own: since the unit is already considered stopped as
soon as its main process exits, that per-unit timer never gets a
chance to force-kill the leftover charon-nm. What actually hangs
shutdown/reboot is a different, later mechanism: once every unit has
been stopped, systemd's low-level systemd-shutdown does one final
SIGTERM-then-SIGKILL sweep over any processes still around, and the
wait before that SIGKILL is governed by DefaultTimeoutStopSec
(system-wide default: 90s), not any per-unit TimeoutStopSec.

This PR bounds both:

  • keeps NetworkManager.service's own TimeoutStopSec (bounds
    systemctl stop NetworkManager.service in isolation, documents
    intent);
  • lowers the system-wide DefaultTimeoutStopSec, which is what
    actually bounds the real-world hang.

Testing

Added tests/vpn-ipsec-shutdown.nix, a NixOS VM test that:

  1. asserts the mitigation is applied (TimeoutStopUSec on
    NetworkManager.service and the system-wide default are both
    10s);
  2. simulates a charon-nm process stuck ignoring SIGTERM inside
    NetworkManager's cgroup;
  3. triggers a real machine shutdown and asserts it completes well
    within the fixed timeout, instead of hanging for the default 90s.

Verified locally: shutdown takes ~23s with the fix, vs. ~103s with
only the per-unit TimeoutStopSec (no system-wide default), vs.
hanging on the default 90s+ with neither.

Note: I used Claude Code to help write this PR description and comments.

`charon-nm` (strongSwan's NetworkManager plugin subprocess) does not
always handle SIGTERM correctly, in particular when a VPN connection
attempt hasn't fully completed. When that happens it keeps running
inside NetworkManager's cgroup even after NetworkManager.service
itself reports as stopped (its `Type=dbus` completion condition --
losing its D-Bus name -- doesn't require its whole cgroup to be
empty).

Setting NetworkManager.service's own TimeoutStopSec is not enough:
since the unit is already considered stopped as soon as its main
process exits, that per-unit timer never gets a chance to force-kill
the leftover charon-nm. What actually hangs shutdown/reboot is a
different, later mechanism: once every unit has been stopped,
systemd's low-level systemd-shutdown does one final SIGTERM-then-
SIGKILL sweep over any processes still around, and the wait before
that SIGKILL is governed by DefaultTimeoutStopSec (system-wide
default: 90s), not any per-unit TimeoutStopSec.

Bound both: keep NetworkManager.service's own TimeoutStopSec (bounds
`systemctl stop NetworkManager.service` in isolation, documents
intent) and lower the system-wide DefaultTimeoutStopSec, which is
what actually bounds the real-world hang.

Add a regression test that simulates a charon-nm process stuck
ignoring SIGTERM in NetworkManager's cgroup and asserts a full
machine shutdown completes well within the fixed timeout instead of
the default 90s.

Closes cloud-gouv#205
@rlahfa-dinum

Copy link
Copy Markdown
Contributor

Is there a reason why you closed #249 which contained the disclosure that you used Claude Code to generate the PR?

@sofianebeloucif

sofianebeloucif commented Aug 29, 2026

Copy link
Copy Markdown
Author

I used Claude Code to help write the PR description and comments for this. I hesitated to mention it because I was worried it would make people take the work behind it less seriously. Closing #249 was me acting on that worry, not a good call in hindsight. Looking through the repo's history afterward (e.g. #234, #235), disclosed Claude Code usage clearly hasn't been a problem here. Reopened cleanly as #250, disclosed. Sorry for the noise.

@jdauphant-dinum

Copy link
Copy Markdown
Contributor

You can use AI but you have to read, understand and review it.

# "NetworkManager.service: Unit process N (charon-nm) remains running after unit
# stopped." This is the same underlying strongSwan bug as the suspend/resume
# workaround above; it's tracked upstream (e.g.
# https://bugs.archlinux.org/task/68116) and we don't have the resources to fix

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we have resources to fix strongSwan itself, wouldn't it be better to fix this upstream or have an issue reference to the bug?

# https://bugs.archlinux.org/task/68116) and we don't have the resources to fix
# strongSwan itself.
#
# Setting NetworkManager.service's own TimeoutStopSec is not sufficient here:

@rlahfa-dinum rlahfa-dinum Sep 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change has a way more invasive consequence, what justify to bound shutdown/reboot just for the sake of charon-nm shutdown, especially if there might be workloads that requires the natural shutdown/reboot timeouts that could lose data otherwise?

#
# SPDX-License-Identifier: MIT

# Regression test for https://github.com/cloud-gouv/securix/issues/205:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a mock test, wouldn't it be better to reproduce the actual strongSwan hung in this test and use it for upstream? I don't think this tests interesting things otherwise.

@@ -0,0 +1,120 @@
# SPDX-FileCopyrightText: 2025 Antoine Eiche <aei.ext@hackcyom.com>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you intended to be copyright owner as you authored the file

@rlahfa-dinum rlahfa-dinum added status: awaiting-author Blocked on author's actions A/networking Networking, VPN, firewalls, etc. labels Sep 2, 2026

This branch has not been deployed

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

Labels

A/networking Networking, VPN, firewalls, etc. status: awaiting-author Blocked on author's actions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Charon-nm block system shutdown

3 participants