From b4e1e3e85368ccd3c35d7ab22425d6e7ee3b67a6 Mon Sep 17 00:00:00 2001 From: Mike Degatano Date: Tue, 23 Aug 2022 07:57:16 -0400 Subject: [PATCH] Identify and handle dhcp issues (#3806) * Identify and handle dhcp issues * Change test from DHCP to Connection Problem --- supervisor/dbus/const.py | 18 ++++ supervisor/dbus/network/configuration.py | 4 +- supervisor/dbus/network/connection.py | 15 +++ supervisor/host/network.py | 24 +++-- .../resolution/checks/network_interface.py | 63 +++++++++++++ supervisor/resolution/const.py | 1 + ...op_NetworkManager_Connection_Active_1.json | 2 +- tests/host/test_network.py | 41 +++++++- .../check/test_check_network_interface.py | 94 +++++++++++++++++++ 9 files changed, 251 insertions(+), 11 deletions(-) create mode 100644 supervisor/resolution/checks/network_interface.py create mode 100644 tests/resolution/check/test_check_network_interface.py diff --git a/supervisor/dbus/const.py b/supervisor/dbus/const.py index 7c7b50e11..e13a908e7 100644 --- a/supervisor/dbus/const.py +++ b/supervisor/dbus/const.py @@ -112,6 +112,7 @@ DBUS_ATTR_RESOLV_CONF_MODE = "ResolvConfMode" DBUS_ATTR_RCMANAGER = "RcManager" DBUS_ATTR_SSID = "Ssid" DBUS_ATTR_STATE = "State" +DBUS_ATTR_STATE_FLAGS = "StateFlags" DBUS_ATTR_STATIC_HOSTNAME = "StaticHostname" DBUS_ATTR_STATIC_OPERATING_SYSTEM_CPE_NAME = "OperatingSystemCPEName" DBUS_ATTR_STRENGTH = "Strength" @@ -161,6 +162,23 @@ class ConnectionStateType(int, Enum): DEACTIVATED = 4 +class ConnectionStateFlags(int, Enum): + """Connection state flags. + + https://developer-old.gnome.org/NetworkManager/stable/nm-dbus-types.html#NMActivationStateFlags + """ + + NONE = 0 + IS_MASTER = 0x1 + IS_SLAVE = 0x2 + LAYER2_READY = 0x4 + IP4_READY = 0x8 + IP6_READY = 0x10 + MASTER_HAS_SLAVES = 0x20 + LIFETIME_BOUND_TO_PROFILE_VISIBILITY = 0x40 + EXTERNAL = 0x80 + + class ConnectivityState(int, Enum): """Network connectvity. diff --git a/supervisor/dbus/network/configuration.py b/supervisor/dbus/network/configuration.py index 704679400..65415a06d 100644 --- a/supervisor/dbus/network/configuration.py +++ b/supervisor/dbus/network/configuration.py @@ -8,8 +8,8 @@ import attr class IpConfiguration: """NetworkSettingsIPConfig object for Network Manager.""" - gateway: IPv6Address | IPv6Address | None = attr.ib() - nameservers: list[IPv6Address | IPv6Address] = attr.ib() + gateway: IPv4Address | IPv6Address | None = attr.ib() + nameservers: list[IPv4Address | IPv6Address] = attr.ib() address: list[IPv4Interface | IPv6Interface] = attr.ib() diff --git a/supervisor/dbus/network/connection.py b/supervisor/dbus/network/connection.py index 316b132d2..107eeb094 100644 --- a/supervisor/dbus/network/connection.py +++ b/supervisor/dbus/network/connection.py @@ -13,6 +13,7 @@ from ..const import ( DBUS_ATTR_NAMESERVER_DATA, DBUS_ATTR_NAMESERVERS, DBUS_ATTR_STATE, + DBUS_ATTR_STATE_FLAGS, DBUS_ATTR_TYPE, DBUS_ATTR_UUID, DBUS_IFACE_CONNECTION_ACTIVE, @@ -20,6 +21,7 @@ from ..const import ( DBUS_IFACE_IP6CONFIG, DBUS_NAME_NM, DBUS_OBJECT_BASE, + ConnectionStateFlags, ConnectionStateType, ) from ..interface import DBusInterfaceProxy @@ -40,6 +42,7 @@ class NetworkConnection(DBusInterfaceProxy): self._ipv4: IpConfiguration | None = None self._ipv6: IpConfiguration | None = None + self._state_flags: set[ConnectionStateFlags] = {ConnectionStateFlags.NONE} @property def id(self) -> str: @@ -61,6 +64,11 @@ class NetworkConnection(DBusInterfaceProxy): """Return the state of the connection.""" return self.properties[DBUS_ATTR_STATE] + @property + def state_flags(self) -> set[ConnectionStateFlags]: + """Return state flags of the connection.""" + return self._state_flags + @property def setting_object(self) -> int: """Return the connection object path.""" @@ -86,6 +94,13 @@ class NetworkConnection(DBusInterfaceProxy): """Update connection information.""" self.properties = await self.dbus.get_properties(DBUS_IFACE_CONNECTION_ACTIVE) + # State Flags + self._state_flags = { + flag + for flag in ConnectionStateFlags + if flag.value & self.properties[DBUS_ATTR_STATE_FLAGS] + } or {ConnectionStateFlags.NONE} + # IPv4 if self.properties[DBUS_ATTR_IP4CONFIG] != DBUS_OBJECT_BASE: ip4 = await DBus.connect(DBUS_NAME_NM, self.properties[DBUS_ATTR_IP4CONFIG]) diff --git a/supervisor/host/network.py b/supervisor/host/network.py index 9e7a575c6..c1fb928ca 100644 --- a/supervisor/host/network.py +++ b/supervisor/host/network.py @@ -31,6 +31,7 @@ from ..exceptions import ( ) from ..jobs.const import JobCondition from ..jobs.decorator import Job +from ..resolution.checks.network_interface import CheckNetworkInterface from .const import AuthMethod, InterfaceMethod, InterfaceType, WifiMode _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -109,8 +110,9 @@ class NetworkManager(CoreSysAttributes): # Apply current settings on each interface so OS can update any out of date defaults interfaces = [ - Interface.from_dbus_interface(self.sys_dbus.network.interfaces[i]) - for i in self.sys_dbus.network.interfaces + Interface.from_dbus_interface(interface) + for interface in self.sys_dbus.network.interfaces.values() + if not CheckNetworkInterface.check_interface(interface) ] with suppress(HostNetworkNotFound): await asyncio.gather( @@ -345,6 +347,16 @@ class Interface: @staticmethod def from_dbus_interface(inet: NetworkInterface) -> Interface: """Concert a dbus interface into normal Interface.""" + ipv4_method = ( + Interface._map_nm_method(inet.settings.ipv4.method) + if inet.settings and inet.settings.ipv4 + else InterfaceMethod.DISABLED + ) + ipv6_method = ( + Interface._map_nm_method(inet.settings.ipv6.method) + if inet.settings and inet.settings.ipv6 + else InterfaceMethod.DISABLED + ) return Interface( inet.name, inet.settings is not None, @@ -352,21 +364,21 @@ class Interface: inet.primary, Interface._map_nm_type(inet.type), IpConfig( - Interface._map_nm_method(inet.settings.ipv4.method), + ipv4_method, inet.connection.ipv4.address, inet.connection.ipv4.gateway, inet.connection.ipv4.nameservers, ) if inet.connection and inet.connection.ipv4 - else IpConfig(InterfaceMethod.DISABLED, [], None, []), + else IpConfig(ipv4_method, [], None, []), IpConfig( - Interface._map_nm_method(inet.settings.ipv6.method), + ipv6_method, inet.connection.ipv6.address, inet.connection.ipv6.gateway, inet.connection.ipv6.nameservers, ) if inet.connection and inet.connection.ipv6 - else IpConfig(InterfaceMethod.DISABLED, [], None, []), + else IpConfig(ipv6_method, [], None, []), Interface._map_nm_wifi(inet), Interface._map_nm_vlan(inet), ) diff --git a/supervisor/resolution/checks/network_interface.py b/supervisor/resolution/checks/network_interface.py new file mode 100644 index 000000000..4e9e21191 --- /dev/null +++ b/supervisor/resolution/checks/network_interface.py @@ -0,0 +1,63 @@ +"""Helpers to check core security.""" +from ...const import CoreState +from ...coresys import CoreSys +from ...dbus.const import ConnectionStateFlags, ConnectionStateType +from ...dbus.network.interface import NetworkInterface +from ..const import ContextType, IssueType +from .base import CheckBase + + +def setup(coresys: CoreSys) -> CheckBase: + """Check setup function.""" + return CheckNetworkInterface(coresys) + + +class CheckNetworkInterface(CheckBase): + """CheckNetworkInterface class for check.""" + + async def run_check(self) -> None: + """Run check if not affected by issue.""" + for interface in self.sys_dbus.network.interfaces.values(): + if CheckNetworkInterface.check_interface(interface): + self.sys_resolution.create_issue( + IssueType.NETWORK_CONNECTION_PROBLEM, + ContextType.SYSTEM, + interface.name, + ) + + async def approve_check(self, reference: str | None = None) -> bool: + """Approve check if it is affected by issue.""" + if not reference: + return False + + interface = self.sys_dbus.network.interfaces.get(reference) + + return interface and CheckNetworkInterface.check_interface(interface) + + @staticmethod + def check_interface(interface: NetworkInterface) -> bool: + """Return true if a managed, connected interface has an issue.""" + if not (interface.managed and interface.connection): + return False + + return not ( + interface.connection.state + in [ConnectionStateType.ACTIVATED, ConnectionStateType.ACTIVATING] + and ConnectionStateFlags.IP4_READY in interface.connection.state_flags + and ConnectionStateFlags.IP6_READY in interface.connection.state_flags + ) + + @property + def issue(self) -> IssueType: + """Return a IssueType enum.""" + return IssueType.NETWORK_CONNECTION_PROBLEM + + @property + def context(self) -> ContextType: + """Return a ContextType enum.""" + return ContextType.SYSTEM + + @property + def states(self) -> list[CoreState]: + """Return a list of valid states when this check can run.""" + return [CoreState.RUNNING, CoreState.STARTUP] diff --git a/supervisor/resolution/const.py b/supervisor/resolution/const.py index 72b7b7dc3..702b29d24 100644 --- a/supervisor/resolution/const.py +++ b/supervisor/resolution/const.py @@ -73,6 +73,7 @@ class IssueType(str, Enum): FATAL_ERROR = "fatal_error" FREE_SPACE = "free_space" MISSING_IMAGE = "missing_image" + NETWORK_CONNECTION_PROBLEM = "network_connection_problem" PWNED = "pwned" SECURITY = "security" TRUST = "trust" diff --git a/tests/fixtures/org_freedesktop_NetworkManager_Connection_Active_1.json b/tests/fixtures/org_freedesktop_NetworkManager_Connection_Active_1.json index 84e5590eb..bdbd00b79 100644 --- a/tests/fixtures/org_freedesktop_NetworkManager_Connection_Active_1.json +++ b/tests/fixtures/org_freedesktop_NetworkManager_Connection_Active_1.json @@ -6,7 +6,7 @@ "Type": "802-3-ethernet", "Devices": ["/org/freedesktop/NetworkManager/Devices/1"], "State": 2, - "StateFlags": 12, + "StateFlags": 92, "Default": true, "Ip4Config": "/org/freedesktop/NetworkManager/IP4Config/1", "Dhcp4Config": "/org/freedesktop/NetworkManager/DHCP4Config/1", diff --git a/tests/host/test_network.py b/tests/host/test_network.py index bdfc1d4b2..ef348433a 100644 --- a/tests/host/test_network.py +++ b/tests/host/test_network.py @@ -1,8 +1,9 @@ """Test network manager.""" -from unittest.mock import Mock, patch +from ipaddress import IPv4Address, IPv6Address +from unittest.mock import Mock, PropertyMock, patch from supervisor.coresys import CoreSys -from supervisor.dbus.const import InterfaceMethod +from supervisor.dbus.const import ConnectionStateFlags, InterfaceMethod from supervisor.host.const import InterfaceType from supervisor.host.network import Interface, IpConfig @@ -24,6 +25,14 @@ async def test_load(coresys: CoreSys): assert len(coresys.host.network.interfaces) == 2 assert coresys.host.network.interfaces[0].name == "eth0" assert coresys.host.network.interfaces[0].enabled is True + assert coresys.host.network.interfaces[0].ipv4.method == InterfaceMethod.AUTO + assert coresys.host.network.interfaces[0].ipv4.gateway == IPv4Address( + "192.168.2.1" + ) + assert coresys.host.network.interfaces[0].ipv6.method == InterfaceMethod.AUTO + assert coresys.host.network.interfaces[0].ipv6.gateway == IPv6Address( + "fe80::da58:d7ff:fe00:9c69" + ) assert coresys.host.network.interfaces[1].name == "wlan0" assert coresys.host.network.interfaces[1].enabled is False @@ -56,3 +65,31 @@ async def test_load_with_disabled_methods(coresys: CoreSys): await coresys.host.network.load() activate_connection.assert_not_called() + + +async def test_load_with_network_connection_issues(coresys: CoreSys): + """Test load does not update interfaces with network connection issues.""" + with patch( + "supervisor.dbus.network.connection.NetworkConnection.state_flags", + new=PropertyMock(return_value={ConnectionStateFlags.IP6_READY}), + ), patch( + "supervisor.dbus.network.connection.NetworkConnection.ipv4", + new=PropertyMock(return_value=None), + ), patch.object( + coresys.host.sys_dbus.network, + "activate_connection", + new=Mock(wraps=coresys.host.sys_dbus.network.activate_connection), + ) as activate_connection: + await coresys.host.network.load() + + activate_connection.assert_not_called() + + assert len(coresys.host.network.interfaces) == 2 + assert coresys.host.network.interfaces[0].name == "eth0" + assert coresys.host.network.interfaces[0].enabled is True + assert coresys.host.network.interfaces[0].ipv4.method == InterfaceMethod.AUTO + assert coresys.host.network.interfaces[0].ipv4.gateway is None + assert coresys.host.network.interfaces[0].ipv6.method == InterfaceMethod.AUTO + assert coresys.host.network.interfaces[0].ipv6.gateway == IPv6Address( + "fe80::da58:d7ff:fe00:9c69" + ) diff --git a/tests/resolution/check/test_check_network_interface.py b/tests/resolution/check/test_check_network_interface.py new file mode 100644 index 000000000..5776ee7fd --- /dev/null +++ b/tests/resolution/check/test_check_network_interface.py @@ -0,0 +1,94 @@ +"""Test Check Network Interface.""" +from unittest.mock import PropertyMock, patch + +import pytest + +from supervisor.const import CoreState +from supervisor.coresys import CoreSys +from supervisor.dbus.const import ConnectionStateFlags +from supervisor.resolution.checks.network_interface import CheckNetworkInterface +from supervisor.resolution.const import ContextType, IssueType +from supervisor.resolution.data import Issue + + +async def test_base(coresys: CoreSys): + """Test check basics.""" + network_interface = CheckNetworkInterface(coresys) + assert network_interface.slug == "network_interface" + assert network_interface.enabled + + +@pytest.mark.parametrize( + "state_flags", + [ + {ConnectionStateFlags.IP4_READY}, + {ConnectionStateFlags.IP6_READY}, + {ConnectionStateFlags.NONE}, + ], +) +async def test_check(coresys: CoreSys, state_flags: set[ConnectionStateFlags]): + """Test check.""" + network_interface = CheckNetworkInterface(coresys) + coresys.core.state = CoreState.RUNNING + + assert len(coresys.resolution.issues) == 0 + + await network_interface.run_check() + assert len(coresys.resolution.issues) == 0 + + with patch( + "supervisor.dbus.network.connection.NetworkConnection.state_flags", + new=PropertyMock(return_value=state_flags), + ): + await network_interface.run_check() + + assert coresys.resolution.issues == [ + Issue(IssueType.NETWORK_CONNECTION_PROBLEM, ContextType.SYSTEM, "eth0") + ] + + +@pytest.mark.parametrize( + "state_flags", + [ + {ConnectionStateFlags.IP4_READY}, + {ConnectionStateFlags.IP6_READY}, + {ConnectionStateFlags.NONE}, + ], +) +async def test_approve(coresys: CoreSys, state_flags: set[ConnectionStateFlags]): + """Test check.""" + network_interface = CheckNetworkInterface(coresys) + coresys.core.state = CoreState.RUNNING + + assert not await network_interface.approve_check("eth0") + + with patch( + "supervisor.dbus.network.connection.NetworkConnection.state_flags", + new=PropertyMock(return_value=state_flags), + ): + assert await network_interface.approve_check("eth0") + + +async def test_did_run(coresys: CoreSys): + """Test that the check ran as expected.""" + network_interface = CheckNetworkInterface(coresys) + should_run = network_interface.states + should_not_run = [state for state in CoreState if state not in should_run] + assert len(should_run) != 0 + assert len(should_not_run) != 0 + + with patch( + "supervisor.resolution.checks.network_interface.CheckNetworkInterface.run_check", + return_value=None, + ) as check: + for state in should_run: + coresys.core.state = state + await network_interface() + check.assert_called_once() + check.reset_mock() + + for state in should_not_run: + coresys.core.state = state + await network_interface() + check.assert_not_called() + check.reset_mock()