diff --git a/custom_components/pfsense/brand/icon.png b/custom_components/pfsense/brand/icon.png deleted file mode 100644 index e69de29..0000000 diff --git a/custom_components/pfsense/config_flow.py b/custom_components/pfsense/config_flow.py index 5723188..82a04b5 100644 --- a/custom_components/pfsense/config_flow.py +++ b/custom_components/pfsense/config_flow.py @@ -24,9 +24,11 @@ CONF_DEVICE_TRACKER_ENABLED, CONF_DEVICE_TRACKER_SCAN_INTERVAL, CONF_DEVICES, + CONF_RULE_SWITCH_KILL_STATES, DEFAULT_DEVICE_TRACKER_CONSIDER_HOME, DEFAULT_DEVICE_TRACKER_ENABLED, DEFAULT_DEVICE_TRACKER_SCAN_INTERVAL, + DEFAULT_RULE_SWITCH_KILL_STATES, DEFAULT_SCAN_INTERVAL, DEFAULT_VERIFY_SSL, DOMAIN, @@ -235,6 +237,13 @@ async def async_step_init(self, user_input=None): DEFAULT_DEVICE_TRACKER_CONSIDER_HOME, ), ): vol.All(vol.Coerce(int), vol.Clamp(min=0, max=600)), + vol.Optional( + CONF_RULE_SWITCH_KILL_STATES, + default=opts.get( + CONF_RULE_SWITCH_KILL_STATES, + DEFAULT_RULE_SWITCH_KILL_STATES, + ), + ): bool, } return self.async_show_form(step_id="init", data_schema=vol.Schema(base_schema)) diff --git a/custom_components/pfsense/const.py b/custom_components/pfsense/const.py index b6f80ef..d0cdd92 100644 --- a/custom_components/pfsense/const.py +++ b/custom_components/pfsense/const.py @@ -56,6 +56,9 @@ CONF_DEVICES = "devices" +CONF_RULE_SWITCH_KILL_STATES = "rule_switch_kill_states" +DEFAULT_RULE_SWITCH_KILL_STATES = False + COUNT = "count" BYTES_RECEIVED = "bytes_received" diff --git a/custom_components/pfsense/pypfsense/__init__.py b/custom_components/pfsense/pypfsense/__init__.py index 416b50d..7aa09e5 100644 --- a/custom_components/pfsense/pypfsense/__init__.py +++ b/custom_components/pfsense/pypfsense/__init__.py @@ -13,6 +13,7 @@ from __future__ import annotations import asyncio +import ipaddress import logging import re from typing import Any @@ -343,6 +344,16 @@ async def get_nat_outbound_rules(self) -> list[dict]: data = await self._get("/firewall/nat/outbound/mappings") return data or [] + # pfSense's own webConfigurator stores content-less config elements + # (````) that the REST API surfaces as empty strings. + # A PATCH re-validates the whole rule object, so those empty-but-required + # fields make an otherwise unrelated toggle fail with + # ``FIELD_EMPTY_NOT_ALLOWED``. Re-send them with the value pfSense would + # have defaulted to, which is a no-op for the rule's behaviour. + _RULE_REQUIRED_DEFAULTS = { + "/firewall/rule": {"statetype": "keep state"}, + } + async def _set_rule_disabled( self, path: str, rules: list[dict], match_key: str, match_value, disabled: bool ) -> None: @@ -351,10 +362,12 @@ async def _set_rule_disabled( continue if bool(rule.get("disabled")) == disabled: return + payload = {"id": rule["id"], "disabled": disabled} + for field, fallback in self._RULE_REQUIRED_DEFAULTS.get(path, {}).items(): + if not rule.get(field): + payload[field] = fallback async with self._write_lock: - await self._request( - "PATCH", path, payload={"id": rule["id"], "disabled": disabled} - ) + await self._request("PATCH", path, payload=payload) await self._apply("firewall") return @@ -493,6 +506,60 @@ async def kill_states(self, source: str, destination: str | None = None) -> None cmd += f" -k {_shq(destination)}" await self.exec_command(cmd) + async def kill_states_for_rule(self, rule: dict) -> None: + """Drop state-table entries for the hosts/networks a rule matches on. + + Used when a rule switch is toggled so existing connections don't keep + flowing under the old ruleset. REST-only, best effort: this uses the + ``DELETE /firewall/states`` prefix filter, so rule endpoints it can't + turn into an IPv4 host / octet-aligned CIDR prefix -- ``any``, + ``(self)``, interface macros (``wan:ip``), negated aliases, non + octet-aligned networks (e.g. ``/25``), IPv6 -- are skipped rather than + falling back to ``pfctl``. + """ + prefixes: set[str] = set() + for net in await self._rule_match_networks(rule): + prefix = _states_prefix(net) + if prefix: + prefixes.add(prefix) + if not prefixes: + return + + async with self._write_lock: + for prefix in prefixes: + for field in ("source", "destination"): + try: + await self._request( + "DELETE", + "/firewall/states", + params={f"{field}__startswith": prefix, "limit": 0}, + ) + except PfSenseAPIError: + pass + + async def _rule_match_networks(self, rule: dict) -> list: + """Concrete ``ip_network`` objects for a rule's source + destination.""" + out: list = [] + aliases: list[dict] | None = None + for side in ("source", "destination"): + value = rule.get(side) + if not isinstance(value, str): + continue + value = value.strip() + if not value or value.startswith("!") or value in ("any", "(self)"): + continue + if ":" in value: # interface address macros, e.g. ``wan:ip`` + continue + net = _as_network(value) + if net is not None: + out.append(net) + continue + # Otherwise treat it as an alias name and expand it. + if aliases is None: + aliases = await self._get("/firewall/aliases") or [] + out.extend(_expand_alias_networks(aliases, value)) + return out + # ------------------------------------------------------- system control async def system_reboot(self, type: str = "normal") -> None: @@ -535,6 +602,51 @@ def _shq(value: str) -> str: return "'" + str(value).replace("'", "'\\''") + "'" +def _as_network(value: str): + """Parse ``value`` as an IPv4/IPv6 host or CIDR, or ``None``.""" + try: + return ipaddress.ip_network(value, strict=False) + except ValueError: + return None + + +def _states_prefix(net) -> str | None: + """``str`` that ``firewall/state`` source/destination values start with for + every address in ``net``, or ``None`` if ``net`` can't be expressed that way. + + States render endpoints as ``ip:port`` (IPv4) so a host becomes ``"ip:"`` + and an octet-aligned network becomes its leading octets plus a dot. + """ + if net.version != 4: + return None + if net.prefixlen == 32: + return f"{net.network_address}:" + if net.prefixlen in (8, 16, 24): + octets = str(net.network_address).split(".") + return ".".join(octets[: net.prefixlen // 8]) + "." + return None + + +def _expand_alias_networks(aliases: list[dict], name: str, _depth: int = 3) -> list: + """Flatten a host/network alias (following nested aliases) to networks.""" + if _depth <= 0: + return [] + target = next((a for a in aliases if a.get("name") == name), None) + if target is None or target.get("type") not in ("host", "network"): + return [] + found: list = [] + for entry in target.get("address") or []: + entry = str(entry).strip() + if not entry or entry.startswith("!"): + continue + net = _as_network(entry) + if net is not None: + found.append(net) + else: # a nested alias reference + found.extend(_expand_alias_networks(aliases, entry, _depth - 1)) + return found + + def _flatten_params(params: dict | None) -> dict | None: """aiohttp needs str values; drop ``None`` and stringify the rest.""" if not params: diff --git a/custom_components/pfsense/strings.json b/custom_components/pfsense/strings.json index f3a224d..71fd5ab 100644 --- a/custom_components/pfsense/strings.json +++ b/custom_components/pfsense/strings.json @@ -42,7 +42,8 @@ "scan_interval": "Scan Interval (seconds)", "device_tracker_enabled": "Enable Device Tracker", "device_tracker_scan_interval": "Device Tracker Scan Interval (seconds)", - "device_tracker_consider_home": "Device Tracker Consider Home (seconds)" + "device_tracker_consider_home": "Device Tracker Consider Home (seconds)", + "rule_switch_kill_states": "Reset matching states when a rule switch is toggled" } }, "device_tracker": { diff --git a/custom_components/pfsense/switch.py b/custom_components/pfsense/switch.py index 3d08354..ffb9ef6 100644 --- a/custom_components/pfsense/switch.py +++ b/custom_components/pfsense/switch.py @@ -15,7 +15,12 @@ from homeassistant.util import slugify from . import CoordinatorEntityManager, PfSenseEntity -from .const import COORDINATOR, DOMAIN +from .const import ( + CONF_RULE_SWITCH_KILL_STATES, + COORDINATOR, + DEFAULT_RULE_SWITCH_KILL_STATES, + DOMAIN, +) _LOGGER = logging.getLogger(__name__) @@ -175,6 +180,19 @@ def is_on(self): def extra_state_attributes(self): return None + async def _maybe_kill_rule_states(self, rule): + """Flush the state table for a rule's hosts after a toggle, if enabled.""" + if not rule: + return + if not self.config_entry.options.get( + CONF_RULE_SWITCH_KILL_STATES, DEFAULT_RULE_SWITCH_KILL_STATES + ): + return + try: + await self._get_pfsense_client().kill_states_for_rule(rule) + except Exception: # best effort - never fail the toggle over this + _LOGGER.warning("failed to kill states for toggled rule", exc_info=True) + class PfSenseFilterSwitch(PfSenseSwitch): def _pfsense_get_tracker(self): @@ -211,6 +229,7 @@ async def async_turn_on(self, **kwargs): tracker = self._pfsense_get_tracker() client = self._get_pfsense_client() await client.enable_filter_rule_by_tracker(tracker) + await self._maybe_kill_rule_states(rule) await self.coordinator.async_refresh() async def async_turn_off(self, **kwargs): @@ -221,6 +240,7 @@ async def async_turn_off(self, **kwargs): tracker = self._pfsense_get_tracker() client = self._get_pfsense_client() await client.disable_filter_rule_by_tracker(tracker) + await self._maybe_kill_rule_states(rule) await self.coordinator.async_refresh() @@ -274,6 +294,7 @@ async def async_turn_on(self, **kwargs): method = client.enable_nat_outbound_rule_by_created_time await method(tracker) + await self._maybe_kill_rule_states(rule) await self.coordinator.async_refresh() async def async_turn_off(self, **kwargs): @@ -290,6 +311,7 @@ async def async_turn_off(self, **kwargs): method = client.disable_nat_outbound_rule_by_created_time await method(tracker) + await self._maybe_kill_rule_states(rule) await self.coordinator.async_refresh() diff --git a/custom_components/pfsense/translations/en.json b/custom_components/pfsense/translations/en.json index f3a224d..71fd5ab 100644 --- a/custom_components/pfsense/translations/en.json +++ b/custom_components/pfsense/translations/en.json @@ -42,7 +42,8 @@ "scan_interval": "Scan Interval (seconds)", "device_tracker_enabled": "Enable Device Tracker", "device_tracker_scan_interval": "Device Tracker Scan Interval (seconds)", - "device_tracker_consider_home": "Device Tracker Consider Home (seconds)" + "device_tracker_consider_home": "Device Tracker Consider Home (seconds)", + "rule_switch_kill_states": "Reset matching states when a rule switch is toggled" } }, "device_tracker": { diff --git a/tests/test_pypfsense.py b/tests/test_pypfsense.py index 2c5fc59..2d6f5c2 100644 --- a/tests/test_pypfsense.py +++ b/tests/test_pypfsense.py @@ -1,5 +1,7 @@ """Unit tests for the async pfSense REST API v2 client.""" +import re + import aiohttp import pytest from aioresponses import aioresponses @@ -110,21 +112,70 @@ async def test_carp_status_reduces_to_bool(client): assert await client.get_carp_status() is False +def _patch_body(m): + return next( + r for (method, url), reqs in m.requests.items() + for r in reqs if method == "PATCH" + ).kwargs["json"] + + async def test_disable_filter_rule_patches_then_applies(client): rules = [ {"id": 4, "tracker": 111, "disabled": False, "descr": "r"}, - {"id": 5, "tracker": 222, "disabled": False, "descr": "r2"}, + {"id": 5, "tracker": 222, "disabled": False, "descr": "r2", + "statetype": "keep state"}, ] with aioresponses() as m: m.get(f"{API}/firewall/rules", payload=_envelope(rules)) m.patch(f"{API}/firewall/rule", payload=_envelope({"id": 5, "disabled": True})) m.post(f"{API}/firewall/apply", payload=_envelope({"applied": True})) await client.disable_filter_rule_by_tracker(222) - req = next( - r for (method, url), reqs in m.requests.items() - for r in reqs if method == "PATCH" + assert _patch_body(m) == {"id": 5, "disabled": True} + + +async def test_disable_filter_rule_backfills_empty_statetype(client): + # pfSense's GUI writes ````; a bare disabled PATCH then + # fails FIELD_EMPTY_NOT_ALLOWED, so the client re-sends the default. + rules = [{"id": 5, "tracker": 222, "disabled": False, "statetype": ""}] + with aioresponses() as m: + m.get(f"{API}/firewall/rules", payload=_envelope(rules)) + m.patch(f"{API}/firewall/rule", payload=_envelope({"id": 5, "disabled": True})) + m.post(f"{API}/firewall/apply", payload=_envelope({"applied": True})) + await client.disable_filter_rule_by_tracker(222) + assert _patch_body(m) == { + "id": 5, + "disabled": True, + "statetype": "keep state", + } + + +async def test_kill_states_for_rule_resolves_alias_to_prefix(client): + rule = {"source": "kids", "destination": "any"} + aliases = [{"name": "kids", "type": "network", "address": ["10.0.10.0/24"]}] + with aioresponses() as m: + m.get(f"{API}/firewall/aliases", payload=_envelope(aliases)) + m.delete( + re.compile(rf"{re.escape(API)}/firewall/states.*"), + payload=_envelope([]), + repeat=True, ) - assert req.kwargs["json"] == {"id": 5, "disabled": True} + await client.kill_states_for_rule(rule) + deletes = [ + str(url) + for (method, url), reqs in m.requests.items() + if method == "DELETE" + for _ in reqs + ] + assert any("source__startswith=10.0.10." in u for u in deletes) + assert any("destination__startswith=10.0.10." in u for u in deletes) + + +async def test_kill_states_for_rule_skips_unresolvable_endpoints(client): + # ``any`` / ``(self)`` / a /25 network have no usable prefix -> no request. + rule = {"source": "any", "destination": "(self)"} + with aioresponses() as m: + await client.kill_states_for_rule(rule) + assert m.requests == {} async def test_build_telemetry_shape():