diff --git a/gns3server/controller/link.py b/gns3server/controller/link.py index 14ff447ea..76011b9f2 100644 --- a/gns3server/controller/link.py +++ b/gns3server/controller/link.py @@ -23,6 +23,7 @@ import html from .controller_error import ControllerError, ControllerNotFoundError from gns3server.agent.web_wireshark.manager import WebWiresharkManager from gns3server.config import Config +from gns3server.utils.packet_filter_validation import validate_all_filters, FilterValidationError import logging @@ -47,7 +48,7 @@ FILTERS = [ "name": "Delay", "description": "Delay packets in milliseconds. You can add jitter in milliseconds (+/-) of the delay", "parameters": [ - {"name": "Latency", "minimum": 0, "maximum": 32767, "unit": "ms", "type": "int"}, + {"name": "Latency", "minimum": 1, "maximum": 32767, "unit": "ms", "type": "int"}, {"name": "Jitter (-/+)", "minimum": 0, "maximum": 32767, "unit": "ms", "type": "int"}, ], }, @@ -161,6 +162,12 @@ class Link: if len(values) != 0 and values[0] != 0 and values[0] != "": new_filters[filter] = values + # Validate filter parameters before applying + try: + validate_all_filters(new_filters) + except FilterValidationError as e: + raise ControllerError(f"Invalid packet filter parameters: {str(e)}") + if new_filters != self.filters: self._filters = new_filters if self._created: @@ -576,7 +583,7 @@ class Link: "suspend": self._suspended, "show_filters_icon": getattr(self, '_show_filters_icon', True), } - return { + result = { "nodes": res, "link_id": self._id, "project_id": self._project.id, @@ -591,3 +598,4 @@ class Link: "wireshark": self._wireshark, "show_filters_icon": getattr(self, '_show_filters_icon', True), } + return result diff --git a/gns3server/controller/project.py b/gns3server/controller/project.py index d9c30a703..ae063e4aa 100644 --- a/gns3server/controller/project.py +++ b/gns3server/controller/project.py @@ -1205,7 +1205,13 @@ class Project: continue link = await self.add_link(link_id=link_data["link_id"]) if "filters" in link_data: - await link.update_filters(link_data["filters"]) + try: + await link.update_filters(link_data["filters"]) + except ControllerError as e: + log.warning( + "Dropping invalid filters on link %s: %s", + link_data.get("link_id"), e + ) if "link_style" in link_data: await link.update_link_style(link_data["link_style"]) if "show_filters_icon" in link_data: diff --git a/gns3server/utils/packet_filter_validation.py b/gns3server/utils/packet_filter_validation.py new file mode 100644 index 000000000..0bd36fc5e --- /dev/null +++ b/gns3server/utils/packet_filter_validation.py @@ -0,0 +1,191 @@ +""" +Packet filter parameter validation utilities. +""" + +import logging +import subprocess +from typing import Dict, List, Any, Optional + +log = logging.getLogger(__name__) + + +class FilterValidationError(Exception): + """Raised when packet filter parameters fail validation.""" + pass + + +def validate_bpf_syntax(bpf_expression: str) -> Dict[str, Optional[str]]: + """ + Validate BPF filter expression syntax using tcpdump. + + Uses `tcpdump -d` to compile the BPF expression into filter instructions. + This calls pcap_compile() internally (same as ubridge) but does not + capture traffic, so it returns immediately for both valid and invalid + expressions. + + Args: + bpf_expression: BPF filter expression to validate + + Returns: + dict with 'valid' (bool) and 'error' (str or None) keys + """ + try: + result = subprocess.run( + ["tcpdump", "-d", bpf_expression], + capture_output=True, + text=True, + ) + + if result.returncode != 0: + # Extract meaningful error from tcpdump's stderr + # Skip "Warning: assuming Ethernet" lines, keep only error lines + error_lines = [] + for line in result.stderr.split("\n"): + line = line.strip() + if line and not line.startswith("Warning:"): + # Strip "tcpdump: " prefix + for prefix in ["tcpdump: "]: + if line.startswith(prefix): + line = line[len(prefix):] + error_lines.append(line) + error_msg = " ".join(error_lines) if error_lines else "Invalid BPF expression" + log.warning("BPF syntax validation failed: %s", error_msg) + return {"valid": False, "error": error_msg} + + log.info("BPF syntax validation passed") + return {"valid": True, "error": None} + + except FileNotFoundError: + log.warning( + "tcpdump not found, skipping BPF syntax validation. " + "Install tcpdump to enable BPF validation." + ) + return {"valid": True, "error": None} + + except Exception as e: + log.error("Unexpected error during BPF validation: %s", e) + return {"valid": False, "error": f"BPF validation error: {str(e)}"} + + +def validate_filter_parameters(filter_type: str, values: List[Any]) -> None: + """ + Validate packet filter parameters. + + Args: + filter_type: Type of packet filter + values: List of parameter values + + Raises: + FilterValidationError: If parameters are invalid + """ + + # Define validation rules based on ubridge implementation + VALIDATION_RULES = { + "frequency_drop": { + "params_count": 1, + "ranges": [(-1, 32767)], # min, max + "names": ["Frequency"], + "units": ["th packet"] + }, + "packet_loss": { + "params_count": 1, + "ranges": [(0, 100)], + "names": ["Chance"], + "units": ["%"] + }, + "delay": { + "params_count": 2, # latency, jitter + "ranges": [(1, 32767), (0, 32767)], # ubridge rejects latency <= 0 + "names": ["Latency", "Jitter"], + "units": ["ms", "ms"] + }, + "corrupt": { + "params_count": 1, + "ranges": [(0, 100)], + "names": ["Chance"], + "units": ["%"] + }, + "bpf": { + "params_count": 1, + "is_text": True, + "names": ["Filters"] + } + } + + if filter_type not in VALIDATION_RULES: + raise FilterValidationError(f"Unknown filter type: {filter_type}") + + rules = VALIDATION_RULES[filter_type] + + # Check parameter count + if len(values) != rules["params_count"]: + raise FilterValidationError( + f"{filter_type} expects {rules['params_count']} parameter(s), got {len(values)}" + ) + + # Validate each parameter + for i, value in enumerate(values): + if rules.get("is_text"): + # Text validation (BPF) + if not isinstance(value, str): + raise FilterValidationError( + f"{filter_type} parameter {rules['names'][i]} must be a string" + ) + + # Validate BPF syntax using tshark (same method as gns3_copilot) + # The value may be a multi-line string; each line becomes a + # separate ubridge filter. Validate each line individually. + value = value.strip() + if value: + lines = value.split("\n") + for line_num, line in enumerate(lines): + line = line.strip() + if not line: + continue + bpf_result = validate_bpf_syntax(line) + if not bpf_result["valid"]: + raise FilterValidationError( + f"{filter_type} parameter {rules['names'][i]} line {line_num + 1} " + f"has invalid syntax: {bpf_result['error']}" + ) + else: + # Integer parameter validation + try: + if isinstance(value, str): + value = value.strip() + int_value = int(value) + else: + int_value = int(value) + except (ValueError, TypeError): + raise FilterValidationError( + f"{filter_type} parameter {rules['names'][i]} must be an integer, got: {value}" + ) + + # Range validation + min_val, max_val = rules["ranges"][i] + if int_value < min_val or int_value > max_val: + raise FilterValidationError( + f"{filter_type} parameter {rules['names'][i]} must be between " + f"{min_val} and {max_val} {rules['units'][i]}, got: {int_value}" + ) + + +def validate_all_filters(filters: Dict[str, List[Any]]) -> None: + """ + Validate all packet filters. + + Args: + filters: Dictionary mapping filter types to their values + + Raises: + FilterValidationError: If any filter is invalid + """ + + if not filters: + return + + for filter_type, values in filters.items(): + if not values or (isinstance(values, list) and len(values) == 0): + continue + + validate_filter_parameters(filter_type, values) \ No newline at end of file diff --git a/tests/api/routes/controller/test_links.py b/tests/api/routes/controller/test_links.py index 5068fc8ea..bc50e053c 100644 --- a/tests/api/routes/controller/test_links.py +++ b/tests/api/routes/controller/test_links.py @@ -63,10 +63,10 @@ class TestLinkRoutes: node1, node2 = nodes filters = { - "latency": [10], + "delay": [10, 0], "frequency_drop": [50] } - + with asyncio_patch("gns3server.controller.udp_link.UDPLink.create") as mock: response = await client.post(app.url_path_for("create_link", project_id=project.id), json={ "nodes": [ @@ -88,7 +88,7 @@ class TestLinkRoutes: ], "filters": filters }) - + assert mock.called assert response.status_code == status.HTTP_201_CREATED assert response.json()["link_id"] is not None @@ -250,10 +250,10 @@ class TestLinkRoutes: ) -> None: filters = { - "latency": [10], + "delay": [10, 0], "frequency_drop": [50] } - + node1, node2 = nodes with asyncio_patch("gns3server.controller.udp_link.UDPLink.create") as mock: response = await client.post(app.url_path_for("create_link", project_id=project.id), json={ @@ -315,7 +315,7 @@ class TestLinkRoutes: ) -> None: filters = { - "latency": [10], + "delay": [10, 0], "frequency_drop": [50] } diff --git a/tests/controller/test_udp_link.py b/tests/controller/test_udp_link.py index 22a7bb50f..fb03947c9 100644 --- a/tests/controller/test_udp_link.py +++ b/tests/controller/test_udp_link.py @@ -46,7 +46,7 @@ async def test_create(project): link = UDPLink(project) await link.add_node(node1, 0, 4) - await link.update_filters({"latency": [10]}) + await link.update_filters({"delay": [10, 0]}) async def compute1_callback(path, data={}, **kwargs): """ @@ -77,7 +77,7 @@ async def test_create(project): "rhost": "192.168.1.2", "rport": 2048, "type": "nio_udp", - "filters": {"latency": [10]}, + "filters": {"delay": [10, 0]}, "suspend": False, }, timeout=120) @@ -313,7 +313,7 @@ async def test_update(project): link = UDPLink(project) await link.add_node(node1, 0, 4) - await link.update_filters({"latency": [10]}) + await link.update_filters({"delay": [10, 0]}) async def compute1_callback(path, data={}, **kwargs): """ @@ -345,7 +345,7 @@ async def test_update(project): "rport": 2048, "type": "nio_udp", "suspend": False, - "filters": {"latency": [10]} + "filters": {"delay": [10, 0]} }, timeout=120) compute2.post.assert_any_call("/projects/{}/vpcs/nodes/{}/adapters/3/ports/1/nio".format(project.id, node2.id), data={ @@ -358,7 +358,7 @@ async def test_update(project): }, timeout=120) assert link.created - await link.update_filters({"drop": [5], "bpf": ["icmp[icmptype] == 8"]}) + await link.update_filters({"frequency_drop": [5], "bpf": ["icmp[icmptype] == 8"]}) compute1.put.assert_any_call("/projects/{}/vpcs/nodes/{}/adapters/0/ports/4/nio".format(project.id, node1.id), data={ "lport": 1024, "rhost": "192.168.1.2", @@ -366,7 +366,7 @@ async def test_update(project): "type": "nio_udp", "suspend": False, "filters": { - "drop": [5], + "frequency_drop": [5], "bpf": ["icmp[icmptype] == 8"] } }, timeout=120) @@ -392,7 +392,7 @@ async def test_update_suspend(project): link = UDPLink(project) await link.add_node(node1, 0, 4) - await link.update_filters({"latency": [10]}) + await link.update_filters({"frequency_drop": [-1]}) await link.update_suspend(True) async def compute1_callback(path, data={}, **kwargs): diff --git a/tests/utils/test_packet_filter_validation.py b/tests/utils/test_packet_filter_validation.py new file mode 100644 index 000000000..3a9de41bd --- /dev/null +++ b/tests/utils/test_packet_filter_validation.py @@ -0,0 +1,163 @@ +""" +Unit tests for packet filter validation. +""" + +import pytest +from gns3server.utils.packet_filter_validation import ( + validate_filter_parameters, + validate_all_filters, + FilterValidationError +) + + +class TestPacketFilterValidation: + """Test packet filter parameter validation.""" + + def test_frequency_drop_valid(self): + """Test valid frequency drop parameters.""" + # Valid range: -1 to 32767 + validate_filter_parameters("frequency_drop", [-1]) + validate_filter_parameters("frequency_drop", [1]) + validate_filter_parameters("frequency_drop", [100]) + validate_filter_parameters("frequency_drop", [32767]) + + def test_frequency_drop_invalid(self): + """Test invalid frequency drop parameters.""" + # Too low + with pytest.raises(FilterValidationError, match="between -1 and 32767"): + validate_filter_parameters("frequency_drop", [-2]) + + # Too high + with pytest.raises(FilterValidationError, match="between -1 and 32767"): + validate_filter_parameters("frequency_drop", [32768]) + + # Wrong type + with pytest.raises(FilterValidationError, match="must be an integer"): + validate_filter_parameters("frequency_drop", ["invalid"]) + + def test_packet_loss_valid(self): + """Test valid packet loss parameters.""" + # Valid range: 0-100% + validate_filter_parameters("packet_loss", [0]) + validate_filter_parameters("packet_loss", [50]) + validate_filter_parameters("packet_loss", [100]) + + def test_packet_loss_invalid(self): + """Test invalid packet loss parameters.""" + # Negative + with pytest.raises(FilterValidationError, match="between 0 and 100"): + validate_filter_parameters("packet_loss", [-1]) + + # Over 100% + with pytest.raises(FilterValidationError, match="between 0 and 100"): + validate_filter_parameters("packet_loss", [101]) + + def test_delay_valid(self): + """Test valid delay parameters.""" + # Valid range: 1-32767ms latency, 0-32767ms jitter + validate_filter_parameters("delay", [1, 0]) + validate_filter_parameters("delay", [100, 50]) + validate_filter_parameters("delay", [32767, 32767]) + + def test_delay_invalid(self): + """Test invalid delay parameters.""" + # Zero latency (ubridge rejects latency <= 0) + with pytest.raises(FilterValidationError, match="between 1 and 32767"): + validate_filter_parameters("delay", [0, 0]) + + # Negative latency + with pytest.raises(FilterValidationError, match="between 1 and 32767"): + validate_filter_parameters("delay", [-1, 0]) + + # Over max + with pytest.raises(FilterValidationError, match="between 1 and 32767"): + validate_filter_parameters("delay", [32768, 0]) + + # Negative jitter + with pytest.raises(FilterValidationError, match="between 0 and 32767"): + validate_filter_parameters("delay", [100, -1]) + + def test_corrupt_valid(self): + """Test valid corrupt parameters.""" + # Valid range: 0-100% + validate_filter_parameters("corrupt", [0]) + validate_filter_parameters("corrupt", [50]) + validate_filter_parameters("corrupt", [100]) + + def test_corrupt_invalid(self): + """Test invalid corrupt parameters.""" + # Over 100% + with pytest.raises(FilterValidationError, match="between 0 and 100"): + validate_filter_parameters("corrupt", [101]) + + def test_bpf_valid(self): + """Test valid BPF parameters.""" + validate_filter_parameters("bpf", ["tcp port 80"]) + validate_filter_parameters("bpf", ["tcp and not port 22"]) + validate_filter_parameters("bpf", [""]) # Empty is valid + validate_filter_parameters("bpf", ["host 192.168.1.1 and port 443"]) + + def test_bpf_multi_line_valid(self): + """Test valid multi-line BPF expressions.""" + validate_filter_parameters("bpf", ["tcp port 80\nnot arp"]) + validate_filter_parameters("bpf", ["tcp and not port 22\nhost 192.168.1.1\nicmp"]) + + def test_bpf_multi_line_invalid(self): + """Test multi-line BPF with invalid line.""" + with pytest.raises(FilterValidationError) as excinfo: + validate_filter_parameters("bpf", ["tcp port 80\ninvalid!!!"]) + err = str(excinfo.value).lower() + assert "syntax error" in err + + def test_bpf_invalid(self): + """Test invalid BPF parameters.""" + # Wrong type + with pytest.raises(FilterValidationError, match="must be a string"): + validate_filter_parameters("bpf", [123]) + + # Invalid BPF syntax + with pytest.raises(FilterValidationError) as excinfo: + validate_filter_parameters("bpf", ["tcp port"]) # Missing port number + assert "syntax error" in str(excinfo.value).lower() + + def test_parameter_count_mismatch(self): + """Test wrong number of parameters.""" + # frequency_drop expects 1 parameter + with pytest.raises(FilterValidationError, match="expects 1 parameter"): + validate_filter_parameters("frequency_drop", []) + + with pytest.raises(FilterValidationError, match="expects 1 parameter"): + validate_filter_parameters("frequency_drop", [1, 2]) + + # delay expects 2 parameters + with pytest.raises(FilterValidationError, match="expects 2 parameter"): + validate_filter_parameters("delay", [100]) + + def test_string_to_int_conversion(self): + """Test string to integer conversion.""" + # Should work with string numbers + validate_filter_parameters("frequency_drop", ["10"]) + validate_filter_parameters("packet_loss", ["50"]) + validate_filter_parameters("delay", ["100", "50"]) + + def test_validate_all_filters(self): + """Test validating multiple filters at once.""" + filters = { + "frequency_drop": [10], + "delay": [100, 50] + } + validate_all_filters(filters) # Should not raise + + def test_validate_all_filters_with_invalid(self): + """Test validate_all_filters with invalid filter.""" + filters = { + "frequency_drop": [10], + "packet_loss": [150] # Invalid: over 100% + } + with pytest.raises(FilterValidationError): + validate_all_filters(filters) + + def test_unknown_filter_type(self): + """Test unknown filter type.""" + with pytest.raises(FilterValidationError, match="Unknown filter type"): + validate_filter_parameters("unknown_filter", [1]) \ No newline at end of file