mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-10-22 10:52:21 +03:00
Improve packet filter validation: use tcpdump, handle multi-line BPF, safe project load
Changes: - Replace tshark BPF validation with tcpdump -d (calls pcap_compile internally like ubridge, returns instantly without waiting for traffic) - Support multi-line BPF expressions: split on newlines and validate each line individually - Always validate, never save invalid filters on error - Drop invalid filters during project load with warning (prevents old topologies with bad filters from failing to open) - Simplify test cases (no longer depend on tshark availability)
This commit is contained in:
parent
87f38b1507
commit
7bcb96a368
@ -583,7 +583,7 @@ class Link:
|
|||||||
"suspend": self._suspended,
|
"suspend": self._suspended,
|
||||||
"show_filters_icon": getattr(self, '_show_filters_icon', True),
|
"show_filters_icon": getattr(self, '_show_filters_icon', True),
|
||||||
}
|
}
|
||||||
return {
|
result = {
|
||||||
"nodes": res,
|
"nodes": res,
|
||||||
"link_id": self._id,
|
"link_id": self._id,
|
||||||
"project_id": self._project.id,
|
"project_id": self._project.id,
|
||||||
@ -598,3 +598,4 @@ class Link:
|
|||||||
"wireshark": self._wireshark,
|
"wireshark": self._wireshark,
|
||||||
"show_filters_icon": getattr(self, '_show_filters_icon', True),
|
"show_filters_icon": getattr(self, '_show_filters_icon', True),
|
||||||
}
|
}
|
||||||
|
return result
|
||||||
|
|||||||
@ -1205,7 +1205,13 @@ class Project:
|
|||||||
continue
|
continue
|
||||||
link = await self.add_link(link_id=link_data["link_id"])
|
link = await self.add_link(link_id=link_data["link_id"])
|
||||||
if "filters" in link_data:
|
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:
|
if "link_style" in link_data:
|
||||||
await link.update_link_style(link_data["link_style"])
|
await link.update_link_style(link_data["link_style"])
|
||||||
if "show_filters_icon" in link_data:
|
if "show_filters_icon" in link_data:
|
||||||
|
|||||||
@ -16,12 +16,12 @@ class FilterValidationError(Exception):
|
|||||||
|
|
||||||
def validate_bpf_syntax(bpf_expression: str) -> Dict[str, Optional[str]]:
|
def validate_bpf_syntax(bpf_expression: str) -> Dict[str, Optional[str]]:
|
||||||
"""
|
"""
|
||||||
Validate BPF filter expression syntax using tshark.
|
Validate BPF filter expression syntax using tcpdump.
|
||||||
|
|
||||||
This uses the same approach as gns3_copilot's packet filter tool:
|
Uses `tcpdump -d` to compile the BPF expression into filter instructions.
|
||||||
- Run tshark with the BPF expression on loopback interface
|
This calls pcap_compile() internally (same as ubridge) but does not
|
||||||
- Check for "Invalid" in output indicating syntax errors
|
capture traffic, so it returns immediately for both valid and invalid
|
||||||
- Timeout is expected behavior (tshark waits for traffic)
|
expressions.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
bpf_expression: BPF filter expression to validate
|
bpf_expression: BPF filter expression to validate
|
||||||
@ -30,51 +30,35 @@ def validate_bpf_syntax(bpf_expression: str) -> Dict[str, Optional[str]]:
|
|||||||
dict with 'valid' (bool) and 'error' (str or None) keys
|
dict with 'valid' (bool) and 'error' (str or None) keys
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
# Use tshark to validate BPF syntax with 1 second timeout
|
|
||||||
# Use -i lo (loopback) to avoid "(null)" interface in error messages
|
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
["tshark", "-f", bpf_expression, "-i", "lo"],
|
["tcpdump", "-d", bpf_expression],
|
||||||
timeout=1,
|
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Check if output contains "Invalid" indicating syntax error
|
if result.returncode != 0:
|
||||||
if "Invalid" in result.stdout or "Invalid" in result.stderr:
|
# Extract meaningful error from tcpdump's stderr
|
||||||
|
# Skip "Warning: assuming Ethernet" lines, keep only error lines
|
||||||
error_lines = []
|
error_lines = []
|
||||||
if "Invalid" in result.stderr:
|
for line in result.stderr.split("\n"):
|
||||||
error_lines.extend(
|
line = line.strip()
|
||||||
line for line in result.stderr.split("\n") if "Invalid" in line
|
if line and not line.startswith("Warning:"):
|
||||||
)
|
# Strip "tcpdump: " prefix
|
||||||
if "Invalid" in result.stdout:
|
for prefix in ["tcpdump: "]:
|
||||||
error_lines.extend(
|
if line.startswith(prefix):
|
||||||
line for line in result.stdout.split("\n") if "Invalid" in line
|
line = line[len(prefix):]
|
||||||
)
|
error_lines.append(line)
|
||||||
|
error_msg = " ".join(error_lines) if error_lines else "Invalid BPF expression"
|
||||||
# Strip interface suffix for cleaner error
|
|
||||||
error_msg_parts = []
|
|
||||||
for line in error_lines:
|
|
||||||
clean = line.split(" for interface")[0].strip()
|
|
||||||
if clean:
|
|
||||||
error_msg_parts.append(clean)
|
|
||||||
error_msg = " ".join(error_msg_parts) if error_msg_parts else "Invalid BPF syntax"
|
|
||||||
log.warning("BPF syntax validation failed: %s", error_msg)
|
log.warning("BPF syntax validation failed: %s", error_msg)
|
||||||
return {"valid": False, "error": error_msg}
|
return {"valid": False, "error": error_msg}
|
||||||
|
|
||||||
log.info("BPF syntax validation passed")
|
log.info("BPF syntax validation passed")
|
||||||
return {"valid": True, "error": None}
|
return {"valid": True, "error": None}
|
||||||
|
|
||||||
except subprocess.TimeoutExpired:
|
|
||||||
# Timeout is expected behavior - tshark waits for traffic
|
|
||||||
# No "Invalid" in output means syntax is correct
|
|
||||||
log.info("BPF syntax validation passed (timeout expected)")
|
|
||||||
return {"valid": True, "error": None}
|
|
||||||
|
|
||||||
except FileNotFoundError:
|
except FileNotFoundError:
|
||||||
# tshark not installed - skip validation
|
|
||||||
log.warning(
|
log.warning(
|
||||||
"tshark not found, skipping BPF syntax validation. "
|
"tcpdump not found, skipping BPF syntax validation. "
|
||||||
"Install tshark to enable BPF validation."
|
"Install tcpdump to enable BPF validation."
|
||||||
)
|
)
|
||||||
return {"valid": True, "error": None}
|
return {"valid": True, "error": None}
|
||||||
|
|
||||||
@ -149,13 +133,21 @@ def validate_filter_parameters(filter_type: str, values: List[Any]) -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
# Validate BPF syntax using tshark (same method as gns3_copilot)
|
# 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()
|
value = value.strip()
|
||||||
if value: # Only validate non-empty BPF expressions
|
if value:
|
||||||
bpf_result = validate_bpf_syntax(value)
|
lines = value.split("\n")
|
||||||
if not bpf_result["valid"]:
|
for line_num, line in enumerate(lines):
|
||||||
raise FilterValidationError(
|
line = line.strip()
|
||||||
f"{filter_type} parameter {rules['names'][i]} has invalid syntax: {bpf_result['error']}"
|
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:
|
else:
|
||||||
# Integer parameter validation
|
# Integer parameter validation
|
||||||
try:
|
try:
|
||||||
|
|||||||
@ -93,26 +93,28 @@ class TestPacketFilterValidation:
|
|||||||
validate_filter_parameters("bpf", [""]) # Empty is valid
|
validate_filter_parameters("bpf", [""]) # Empty is valid
|
||||||
validate_filter_parameters("bpf", ["host 192.168.1.1 and port 443"])
|
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):
|
def test_bpf_invalid(self):
|
||||||
"""Test invalid BPF parameters."""
|
"""Test invalid BPF parameters."""
|
||||||
# Wrong type
|
# Wrong type
|
||||||
with pytest.raises(FilterValidationError, match="must be a string"):
|
with pytest.raises(FilterValidationError, match="must be a string"):
|
||||||
validate_filter_parameters("bpf", [123])
|
validate_filter_parameters("bpf", [123])
|
||||||
|
|
||||||
# Invalid BPF syntax (requires tshark)
|
# Invalid BPF syntax
|
||||||
try:
|
with pytest.raises(FilterValidationError) as excinfo:
|
||||||
validate_filter_parameters("bpf", ["tcp port"]) # Missing port number
|
validate_filter_parameters("bpf", ["tcp port"]) # Missing port number
|
||||||
# If tshark is not available, this might pass
|
assert "syntax error" in str(excinfo.value).lower()
|
||||||
import subprocess
|
|
||||||
subprocess.run(["which", "tshark"], capture_output=True)
|
|
||||||
# If we get here, tshark exists, so validation should have failed
|
|
||||||
pytest.fail("Expected BPF validation to fail for invalid syntax")
|
|
||||||
except FilterValidationError as e:
|
|
||||||
# Expected: BPF syntax error
|
|
||||||
assert "invalid syntax" in str(e).lower() or "Invalid capture filter" in str(e)
|
|
||||||
except (FileNotFoundError, subprocess.CalledProcessError):
|
|
||||||
# tshark not installed, skip this test
|
|
||||||
pytest.skip("tshark not installed, skipping BPF syntax validation test")
|
|
||||||
|
|
||||||
def test_parameter_count_mismatch(self):
|
def test_parameter_count_mismatch(self):
|
||||||
"""Test wrong number of parameters."""
|
"""Test wrong number of parameters."""
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user