T8099: Update strongswan to 6.0.6#1222
Conversation
* Remove systemd-dev from dependencies * Add `set -e` to build_cmd. Strongswan depends on `systemd` and `systemd-dev`, installing both fails with: ``` The following packages have unmet dependencies: systemd-dev : Breaks: systemd (< 253-2~) but 252.39-1~deb12u2 is to be installed E: Unable to correct problems, you have held broken packages. ``` `systemd-dev` contains pkg-config files for systemd and udev, but current `systemd` package installs identical `/usr/share/pkgconfig/systemd.pc` - maybe that's why packages cannot be installed simultaneously. So remove it from dependencies. Though the package failed to build, `./build.py` returned 0 as `build_cmd` has two build commands one after another without any checks of exit value of first one that builds strongswan. Added `set -e` so that any failure in any of commands results to build failure.
* Upgrade to 6.0.6 * Update 30-strongswan-configs.chroot to not change /etc/strongswan.d/charon.conf as the file is part of package strongswan-charon that should not be installed * Rebase all patches * Enable ML-KEM for Post Quantum
📝 WalkthroughWalkthroughStrongSwan is upgraded from 5.9.11-2 to 6.0.6-1 with five patches. The upgrade simplifies plugin disabling, adds IKE SA address override hints for precise initiation matching, extends VICI with certificate data and SA state-change events, disables unused Debian options, and enables ML-KEM post-quantum cryptography. ChangesStrongSwan 6.0.6 Upgrade with Address Overrides and VICI Enhancements
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
scripts/package-build/strongswan/patches/strongswan/0001-charon-add-optional-source-and-remote-overrides-for-.patch (2)
243-250:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject malformed
my-host/other-hostvalues.In
src/libcharon/plugins/vici/vici_control.c,host_create_from_string()is unchecked. If parsing fails, the override is silently ignored and the request falls back to the unqualified peer config, which makes a typo in--source/--remotelook like a valid initiate.Suggested fix
if (my_host_str) { my_host = host_create_from_string(my_host_str, 0); + if (!my_host) + { + msg = send_reply(this, "invalid my-host '%s'", my_host_str); + goto ret; + } } if (other_host_str) { other_host = host_create_from_string(other_host_str, 0); + if (!other_host) + { + msg = send_reply(this, "invalid other-host '%s'", other_host_str); + goto ret; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/package-build/strongswan/patches/strongswan/0001-charon-add-optional-source-and-remote-overrides-for-.patch` around lines 243 - 250, Null return from host_create_from_string() is currently ignored causing malformed --source/--remote to be treated as absent; update the vici_control.c handling of my_host_str and other_host_str so you check the return value of host_create_from_string(my_host_str, 0) and host_create_from_string(other_host_str, 0), and if either returns NULL reject the VICI request (set an error response / return an error code), logging a clear message about the malformed my-host/other-host input (include the original string). Ensure you reference and modify the block handling my_host/my_host_str and other_host/other_host_str in src/libcharon/plugins/vici/vici_control.c and do not silently fall back to the unqualified peer config.
361-367:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't replace configured IKE ports with hardcoded UDP/500.
In
src/libcharon/sa/ike_sa_manager.c, overrides that arrive without an explicit port are forced toIKEV2_UDP_PORT, and Lines 395-398 then push those hosts onto the selected/new SA. That breaksmy-host/other-hostinitiation for peers that use non-defaultlocal_portorremote_port: the address override silently discards the ports frompeer_cfg.src/libcharon/sa/trap_manager.calready preserves the configured ports before callingcheckout_by_config(), so this path should do the same.Suggested fix
+ ike_cfg_t *ike_cfg = peer_cfg->get_ike_cfg(peer_cfg); + if (my_host && my_host->get_port(my_host) == 0) { - my_host->set_port(my_host, IKEV2_UDP_PORT); + my_host->set_port(my_host, ike_cfg->get_my_port(ike_cfg)); } if (other_host && other_host->get_port(other_host) == 0) { - other_host->set_port(other_host, IKEV2_UDP_PORT); + other_host->set_port(other_host, ike_cfg->get_other_port(ike_cfg)); }Also applies to: 395-398
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/package-build/strongswan/patches/strongswan/0001-charon-add-optional-source-and-remote-overrides-for-.patch` around lines 361 - 367, The patch is overwriting configured non-default ports by unconditionally setting IKEV2_UDP_PORT when my_host->get_port(...) or other_host->get_port(...) is 0; instead preserve any ports already configured by peer_cfg (as trap_manager.c does) and do not force the default UDP port here. In src/libcharon/sa/ike_sa_manager.c, remove or change the set_port(my_host, IKEV2_UDP_PORT) / set_port(other_host, IKEV2_UDP_PORT) logic so that you only apply the IKEV2_UDP_PORT default when there is truly no port configured anywhere (i.e., get_port(...) == 0 and there is no previously stored/configured port on the selected/new SA), otherwise leave the existing port intact; reference my_host, other_host, get_port, set_port, IKEV2_UDP_PORT and ensure behavior matches checkout_by_config()/trap_manager.c which preserves configured ports.scripts/package-build/strongswan/patches/strongswan/0002-vici-send-certificates-for-ike-sa-events.patch (1)
42-44:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winIterate all auth_cfg entries before deciding certificate is absent.
Line 43 and Line 64 only inspect the first
auth_cfg. In multi-round auth,AUTH_RULE_SUBJECT_CERTcan be in a later entry, solocal-cert-data/remote-cert-datamay be silently omitted.Proposed fix
- if (enumerator->enumerate(enumerator, &auth_cfg)) + while (enumerator->enumerate(enumerator, &auth_cfg)) { certificate_t *cert = auth_cfg->get(auth_cfg, AUTH_RULE_SUBJECT_CERT); chunk_t encoding; if (cert && cert->get_encoding(cert, CERT_ASN1_DER, &encoding)) { b->add(b, VICI_KEY_VALUE, "local-cert-data", encoding); free(encoding.ptr); + break; } }- if (enumerator->enumerate(enumerator, &auth_cfg)) + while (enumerator->enumerate(enumerator, &auth_cfg)) { certificate_t *cert = auth_cfg->get(auth_cfg, AUTH_RULE_SUBJECT_CERT); chunk_t encoding; if (cert && cert->get_encoding(cert, CERT_ASN1_DER, &encoding)) { b->add(b, VICI_KEY_VALUE, "remote-cert-data", encoding); free(encoding.ptr); + break; } }Also applies to: 63-65
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/package-build/strongswan/patches/strongswan/0002-vici-send-certificates-for-ike-sa-events.patch` around lines 42 - 44, The current code calls ike_sa->create_auth_cfg_enumerator(...) and then checks only the first auth_cfg via a single if (enumerator->enumerate(enumerator, &auth_cfg)) which misses AUTH_RULE_SUBJECT_CERT entries in later auth rounds; change the logic to loop over all entries from the enumerator (e.g., while (enumerator->enumerate(enumerator, &auth_cfg)) { ... }) and inspect each auth_cfg for AUTH_RULE_SUBJECT_CERT, setting local-cert-data/remote-cert-data when found and breaking early if both are discovered; apply the same loop fix to both places that use enumerate (the blocks referencing enumerator/enumerate/auth_cfg around the AUTH_RULE_SUBJECT_CERT checks) and ensure the enumerator is properly destroyed/released after the loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/package-build/strongswan/package.toml`:
- Line 9: pre_build_hook currently uses a brittle sed pattern "/ systemd-dev /"
that can miss tokens like "systemd-dev,"; update the pre_build_hook in
package.toml to a more robust sed invocation that removes the systemd-dev token
regardless of surrounding punctuation or line position (e.g., use a
word-boundary-aware pattern such as sed -i -E
's/(^|[[:space:],;])systemd-dev([[:space:],;]|$)/\1\2/g' debian/control) so the
dependency is reliably stripped; edit the pre_build_hook entry (the string
assigned to pre_build_hook) to replace the old sed command with the improved
pattern.
---
Outside diff comments:
In
`@scripts/package-build/strongswan/patches/strongswan/0001-charon-add-optional-source-and-remote-overrides-for-.patch`:
- Around line 243-250: Null return from host_create_from_string() is currently
ignored causing malformed --source/--remote to be treated as absent; update the
vici_control.c handling of my_host_str and other_host_str so you check the
return value of host_create_from_string(my_host_str, 0) and
host_create_from_string(other_host_str, 0), and if either returns NULL reject
the VICI request (set an error response / return an error code), logging a clear
message about the malformed my-host/other-host input (include the original
string). Ensure you reference and modify the block handling my_host/my_host_str
and other_host/other_host_str in src/libcharon/plugins/vici/vici_control.c and
do not silently fall back to the unqualified peer config.
- Around line 361-367: The patch is overwriting configured non-default ports by
unconditionally setting IKEV2_UDP_PORT when my_host->get_port(...) or
other_host->get_port(...) is 0; instead preserve any ports already configured by
peer_cfg (as trap_manager.c does) and do not force the default UDP port here. In
src/libcharon/sa/ike_sa_manager.c, remove or change the set_port(my_host,
IKEV2_UDP_PORT) / set_port(other_host, IKEV2_UDP_PORT) logic so that you only
apply the IKEV2_UDP_PORT default when there is truly no port configured anywhere
(i.e., get_port(...) == 0 and there is no previously stored/configured port on
the selected/new SA), otherwise leave the existing port intact; reference
my_host, other_host, get_port, set_port, IKEV2_UDP_PORT and ensure behavior
matches checkout_by_config()/trap_manager.c which preserves configured ports.
In
`@scripts/package-build/strongswan/patches/strongswan/0002-vici-send-certificates-for-ike-sa-events.patch`:
- Around line 42-44: The current code calls
ike_sa->create_auth_cfg_enumerator(...) and then checks only the first auth_cfg
via a single if (enumerator->enumerate(enumerator, &auth_cfg)) which misses
AUTH_RULE_SUBJECT_CERT entries in later auth rounds; change the logic to loop
over all entries from the enumerator (e.g., while
(enumerator->enumerate(enumerator, &auth_cfg)) { ... }) and inspect each
auth_cfg for AUTH_RULE_SUBJECT_CERT, setting local-cert-data/remote-cert-data
when found and breaking early if both are discovered; apply the same loop fix to
both places that use enumerate (the blocks referencing
enumerator/enumerate/auth_cfg around the AUTH_RULE_SUBJECT_CERT checks) and
ensure the enumerator is properly destroyed/released after the loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 6f06ee0a-6223-4e7f-9ae3-3a4f00b907b7
📒 Files selected for processing (7)
data/live-build-config/hooks/live/30-strongswan-configs.chrootscripts/package-build/strongswan/package.tomlscripts/package-build/strongswan/patches/strongswan/0001-charon-add-optional-source-and-remote-overrides-for-.patchscripts/package-build/strongswan/patches/strongswan/0002-vici-send-certificates-for-ike-sa-events.patchscripts/package-build/strongswan/patches/strongswan/0003-vici-add-support-for-individual-sa-state-changes.patchscripts/package-build/strongswan/patches/strongswan/0004-VyOS-disable-options-enabled-by-Debian-that-are-unus.patchscripts/package-build/strongswan/patches/strongswan/0005-T8099-enable-Module-Lattice-based-crypto-ML-KEM.patch
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
ansible/ansible(manual)
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build_iso
- GitHub Check: codeql-analysis-call / Analyze (python)
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🧰 Additional context used
🔍 Remote MCP Context7
Summary of Relevant Context for PR Review
StrongSwan 6.0.6 Release Information
StrongSwan 6.0.6 fixes seven vulnerabilities and comes with several other improvements and fixes. Key security fixes in this version include:
- Seven vulnerabilities were found and fixed through improvements in AI-assisted security analysis.
- The ML-KEM API used in the wolfssl plugin has been adapted for wolfSSL 5.8.0.
- Modular plugin loading is now configured through
charon-cmd.confandcharon-nm.confconfig files in/etc/strongswan.d.
ML-KEM Post-Quantum Cryptography
ML-KEM is a module-lattice-based key encapsulation mechanism standardized in FIPS 203, enabling quantum-resistant key exchange over insecure channels. Specific details relevant to the PR:
- ML-KEM specifies three parameter sets: ML-KEM-512, ML-KEM-768, and ML-KEM-1024, in order of increasing security strength and decreasing performance.
- Before standardization, ML-KEM was known as CRYSTALS-Kyber, and NIST selected it as the primary standard for general encryption because of its potent combination of security and speed.
- StrongSwan's ML-KEM implementation supports the ML-KEM (Module-Lattice-based crypto) plugin, supporting ML-KEM via various crypto backends like Botan, wolfSSL, OpenSSL, and AWS-LC.
VyOS Integration
The swanctl system uses the VICI (Versatile IKE Configuration Interface) to communicate with the charon daemon and represents the current recommended approach for configuring strongSwan. This is relevant to the PR's patches that extend VICI with new features for certificate handling and SA state change monitoring.
Build and Dependency Changes
The PR correctly addresses the systemd-dev dependency issue: The Debian strongSwan 6.0.0-1 release handles removal of plugins and updates build configurations for the modern strongSwan version., [::web_search::]
🔇 Additional comments (7)
data/live-build-config/hooks/live/30-strongswan-configs.chroot (1)
12-12: LGTM!Also applies to: 20-20
scripts/package-build/strongswan/package.toml (1)
3-3: LGTM!Also applies to: 13-13, 17-17
scripts/package-build/strongswan/patches/strongswan/0004-VyOS-disable-options-enabled-by-Debian-that-are-unus.patch (1)
25-25: LGTM!Also applies to: 183-190, 199-200, 208-208, 214-214, 222-222, 227-227, 235-235, 240-240, 265-270
scripts/package-build/strongswan/patches/strongswan/0002-vici-send-certificates-for-ike-sa-events.patch (1)
21-23: LGTM!Also applies to: 84-85, 93-95, 102-107, 115-117, 124-126, 133-135, 142-144
scripts/package-build/strongswan/patches/strongswan/0003-vici-add-support-for-individual-sa-state-changes.patch (1)
20-35: LGTM!Also applies to: 42-79, 88-142, 147-155
scripts/package-build/strongswan/patches/strongswan/0005-T8099-enable-Module-Lattice-based-crypto-ML-KEM.patch (2)
19-19: LGTM!Also applies to: 27-27, 35-35, 43-43
55-57: Verify the backend contract behind--enable-ml.Line 57 only enables strongSwan’s
mlplugin.ML_KEM_*will still stay absent at runtime if the selected crypto backend in this package set/rootfs does not expose ML-KEM, so please confirm that contract alongside the smoketest expectation.Source: MCP tools
| # systemd now contains pkg-config files and systemd-dev is not needed | ||
| # this changes debian/control so the change is needed before mk-build-deps | ||
| # call so cannot be included in normal patches | ||
| pre_build_hook = "sed -i '/ systemd-dev /d' debian/control" |
There was a problem hiding this comment.
Brittle sed match may fail to remove systemd-dev.
Line 9 uses / systemd-dev /, which misses common control-file tokens like systemd-dev,. If it misses, the original dependency conflict remains and the build fix is ineffective.
Suggested fix
-pre_build_hook = "sed -i '/ systemd-dev /d' debian/control"
+pre_build_hook = "sed -Ei '/\\bsystemd-dev\\b/d' debian/control"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pre_build_hook = "sed -i '/ systemd-dev /d' debian/control" | |
| pre_build_hook = "sed -Ei '/\\bsystemd-dev\\b/d' debian/control" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/package-build/strongswan/package.toml` at line 9, pre_build_hook
currently uses a brittle sed pattern "/ systemd-dev /" that can miss tokens like
"systemd-dev,"; update the pre_build_hook in package.toml to a more robust sed
invocation that removes the systemd-dev token regardless of surrounding
punctuation or line position (e.g., use a word-boundary-aware pattern such as
sed -i -E 's/(^|[[:space:],;])systemd-dev([[:space:],;]|$)/\1\2/g'
debian/control) so the dependency is reliably stripped; edit the pre_build_hook
entry (the string assigned to pre_build_hook) to replace the old sed command
with the improved pattern.
|
CI integration 👍 passed! Details
|
2 participants
Change summary
Types of changes
Related Task(s)
Related PR(s)
How to test / Smoketest result
/usr/libexec/vyos/tests/smoke/cli/test_vpn_ipsec.pyChecklist: