fix: make WireGuard killswitch bridge-aware#15
Open
jmpareja wants to merge 1 commit into
Open
Conversation
`get_physical_devices()` enumerated only ETHERNET/WIFI devices and did not exclude slaves of a bridge controller. On hosts where the primary uplink is a NetworkManager-managed Linux bridge (e.g. `br0` with `enp12s0` enslaved), the slave NIC is ACTIVATED but its connection profile has no IPv4 setting — its IP config lives on the bridge. The killswitch then called `config.get_num_routes()` on a `None` IPv4 setting and raised `AttributeError` on connect. Changes: - `get_physical_devices()` now also accepts `NM.DeviceType.BRIDGE` and excludes any device whose active connection has a controller (slave of a bond/bridge/team). Uses `get_controller()` with a fallback to the deprecated `get_master()` for older libnm. - `_add_ipv4_route` / `_remove_ipv4_routes` raise `GatewayNotFoundError` with a clear message when `get_setting_ip4_config()` returns `None`, instead of crashing with `AttributeError`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On hosts where the primary uplink is a NetworkManager-managed Linux bridge (e.g.
br0with a NIC enslaved), connecting to a WireGuard server with the killswitch enabled crashes with:NMClient.get_physical_devices()enumerates allETHERNET/WIFIdevices inACTIVATEDstate but does not skip slaves of a bridge/bond/team. The slave NIC isACTIVATED, yet its connection profile carries no IPv4 setting — IP config lives on the controller. The killswitch then dereferences aNoneNMSettingIP4Config.This PR makes the killswitch bridge-aware:
get_physical_devices()now also acceptsNM.DeviceType.BRIDGEand excludes any device whose active connection has a controller. Detection usesNMSettingConnection.get_controller()with a fallback to the deprecatedget_master()for older libnm builds._add_ipv4_routeand_remove_ipv4_routesraiseGatewayNotFoundErrorwith a descriptive message instead ofAttributeErrorwhenget_setting_ip4_config()returnsNone— defense in depth in case a device without IPv4 config still slips through.No existing behavior changes for hosts whose default route is on a plain ethernet/wifi NIC: such devices are not enslaved and continue to be picked up exactly as before.
Test plan
tests/.../wireguard/covering: bridge controller picked up, enslaved NIC skipped, plain ethernet still picked up,NoneIP4 setting raisesGatewayNotFoundError.br0(bridge) with a NIC enslaved — connection succeeds and routes are added/removed against the bridge.