Merge pull request #2754 from yueguobin/feature/packet-filter-validation

Add packet filter parameter validation to prevent ubridge errors
This commit is contained in:
Jeremy Grossmann 2026-05-30 23:01:17 +02:00 committed by GitHub
commit cc416f20d5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 384 additions and 16 deletions

View File

@ -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

View File

@ -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:

View File

@ -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)

View File

@ -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]
}

View File

@ -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):

View File

@ -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])