url/homelable
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #295 from Pouzor/fix/scanner-two-stage-version-scan

Browse Source 
fix(scanner): decouple port discovery from version detection (#277)
This commit is contained in:
Pouzor - Rémy Jardient
2026-07-18 00:50:46 +02:00
committed by GitHub
parent 587f7f3e61 6f01d5550d
commit 5b0c064e68
3 changed files with 191 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/app/core/config.py
+5
View File
@@ -52,6 +52,11 @@ class Settings(BaseSettings):
# Scanner
scanner_ranges: list[str] = ["192.168.1.0/24"]
# Phase-2 version-detection (-sV) host timeout, seconds. Bounds the version
# pass so a stalling TLS port (e.g. Proxmox 8006) can't hang it. Discovered
# ports survive a timeout regardless; raise this on slow/overlay networks.
scanner_version_host_timeout: int = 60
# Deep scan — persisted defaults (overridable per-scan from the scan dialog).
# http_ranges: extra nmap port ranges, opt-in, no default. Probe + TLS off by default.
scanner_http_ranges: list[str] = []
backend/app/services/scanner.py
+64 -27
View File
@@ -14,6 +14,7 @@ from typing import Any
from sqlalchemy import or_, select
from sqlalchemy.ext.asyncio import AsyncSession
from app.core.config import settings
from app.db.models import Node, PendingDevice, ScanRun
from app.services.discovery_sources import add_source
from app.services.fingerprint import fingerprint_ports, suggest_node_type
@@ -240,6 +241,14 @@ def _nmap_scan_single(host_dict: dict[str, Any], port_spec: str = _EXTRA_PORTS)
"""
Phase 2 — single-IP port scan with service detection.
Runs in a thread (blocking). Returns the host dict enriched with open_ports.
Two decoupled nmap passes (issue #277):
Pass A — port discovery only (``--open``, no ``-sV``). Fast and reliable;
its open ports are authoritative and are never discarded.
Pass B — version detection (``-sV``) scoped to the ports Pass A found,
bounded by ``--host-timeout``. Best-effort: if it times out or
fails (e.g. a TLS port stalls plaintext probes), the Pass A ports
are kept with empty banners instead of the whole host being lost.
"""
ip = host_dict["ip"]
logger.info("[Phase 2] Scanning %s ...", ip)
@@ -248,48 +257,76 @@ def _nmap_scan_single(host_dict: dict[str, Any], port_spec: str = _EXTRA_PORTS)
logger.warning("[Phase 2] nmap not available, skipping %s", ip)
return host_dict
is_root = os.geteuid() == 0
if is_root:
# SYN scan + version detection (fastest, most accurate)
scan_args = f"-sS -sV --open -T4 -Pn --host-timeout 60s -p {port_spec}"
else:
# TCP connect scan (-sT) — no raw sockets needed, works without root.
# nmap auto-selects -sT without root but being explicit avoids edge cases.
scan_args = f"-sT -sV --open -T4 -Pn --host-timeout 60s -p {port_spec}"
# -sS (SYN) needs root; -sT (connect) works without it. nmap auto-selects
# -sT without root but being explicit avoids edge cases.
scan_type = "-sS" if os.geteuid() == 0 else "-sT"
logger.debug("[Phase 2] %s args: %s", ip, scan_args)
nm = nmap.PortScanner()
# --- Pass A: port discovery (no -sV, no host-timeout) ---
discovery_args = f"{scan_type} --open -T4 -Pn -p {port_spec}"
logger.debug("[Phase 2] %s discovery args: %s", ip, discovery_args)
nm_disc = nmap.PortScanner()
try:
nm.scan(hosts=ip, arguments=scan_args)
nm_disc.scan(hosts=ip, arguments=discovery_args)
except Exception as exc:
logger.warning("[Phase 2] nmap FAILED for %s (%s: %s) — skipping port scan", ip, type(exc).__name__, exc)
logger.warning("[Phase 2] nmap discovery FAILED for %s (%s: %s) — skipping port scan",
ip, type(exc).__name__, exc)
return host_dict
all_scanned = nm.all_hosts()
logger.debug("[Phase 2] %s — nmap returned %d host(s) in results", ip, len(all_scanned))
if ip not in all_scanned:
if ip not in nm_disc.all_hosts():
logger.info("[Phase 2] %s — no open ports found (all closed/filtered or nmap had no results)", ip)
return host_dict
open_ports = []
for proto in nm[ip].all_protocols():
for port, info in nm[ip][proto].items():
for proto in nm_disc[ip].all_protocols():
for port, info in nm_disc[ip][proto].items():
if info["state"] == "open":
banner = (info.get("product", "") + " " + info.get("version", "")).strip()
open_ports.append({"port": port, "protocol": proto, "banner": banner})
open_ports.append({"port": port, "protocol": proto, "banner": ""})
if open_ports:
port_summary = ", ".join(
f"{p['port']}/{p['protocol']} ({p['banner'] or 'unknown'})" for p in open_ports
)
logger.info("[Phase 2] %s — %d open port(s): %s", ip, len(open_ports), port_summary)
else:
if not open_ports:
logger.info("[Phase 2] %s — 0 open ports detected", ip)
host_dict["open_ports"] = []
if not host_dict["mac"]:
host_dict["mac"] = nm_disc[ip].get("addresses", {}).get("mac")
return host_dict
# --- Pass B: version detection on the discovered ports (best-effort) ---
port_list = ",".join(str(p["port"]) for p in open_ports)
timeout = settings.scanner_version_host_timeout
version_args = f"{scan_type} -sV -T4 -Pn --host-timeout {timeout}s -p {port_list}"
logger.debug("[Phase 2] %s version args: %s", ip, version_args)
nm_ver = nmap.PortScanner()
banners: dict[tuple[str, int], str] = {}
ver_os = None
ver_mac = None
try:
nm_ver.scan(hosts=ip, arguments=version_args)
if ip in nm_ver.all_hosts():
for proto in nm_ver[ip].all_protocols():
for port, info in nm_ver[ip][proto].items():
banner = (info.get("product", "") + " " + info.get("version", "")).strip()
if banner:
banners[(proto, port)] = banner
ver_os = _extract_os(nm_ver, ip)
ver_mac = nm_ver[ip].get("addresses", {}).get("mac")
else:
logger.info("[Phase 2] %s — version detection returned no results, keeping %d port(s) without banners",
ip, len(open_ports))
except Exception as exc:
logger.info("[Phase 2] %s — version detection failed (%s: %s), keeping %d port(s) without banners",
ip, type(exc).__name__, exc, len(open_ports))
for p in open_ports:
p["banner"] = banners.get((p["protocol"], p["port"]), "")
port_summary = ", ".join(
f"{p['port']}/{p['protocol']} ({p['banner'] or 'unknown'})" for p in open_ports
)
logger.info("[Phase 2] %s — %d open port(s): %s", ip, len(open_ports), port_summary)
host_dict["open_ports"] = open_ports
if not host_dict["mac"]:
host_dict["mac"] = nm[ip].get("addresses", {}).get("mac")
host_dict["os"] = _extract_os(nm, ip)
host_dict["mac"] = ver_mac or nm_disc[ip].get("addresses", {}).get("mac")
host_dict["os"] = ver_os
return host_dict
backend/tests/test_scanner.py
+122
View File
@@ -253,6 +253,128 @@ def test_nmap_scan_single_returns_host_unchanged_when_no_results():
assert result["mac"] == "34:94:54:aa:bb:cc" # preserved from Phase 1
# ---------------------------------------------------------------------------
# _nmap_scan_single — two-pass discovery/version split (issue #277)
# ---------------------------------------------------------------------------
def _fake_scanner(ip, port_info, mac=None):
"""Mock nmap.PortScanner whose results contain `ip` with tcp `port_info`."""
host = MagicMock()
host.all_protocols.return_value = ["tcp"]
host.__getitem__ = MagicMock(return_value=port_info)
host.get.return_value = {"mac": mac} if mac else {}
nm = MagicMock()
nm.all_hosts.return_value = [ip]
nm.__getitem__ = MagicMock(return_value=host)
return nm
def _empty_scanner():
nm = MagicMock()
nm.all_hosts.return_value = []
return nm
def _failing_scanner(exc=Exception("host timeout")):
nm = MagicMock()
nm.scan.side_effect = exc
return nm
def test_nmap_scan_single_two_pass_merges_banners():
"""Pass A discovers ports; Pass B enriches them with -sV banners."""
from app.services.scanner import _nmap_scan_single
host = {"ip": "192.168.1.10", "hostname": None, "mac": None, "os": None, "open_ports": []}
disc = _fake_scanner("192.168.1.10", {22: {"state": "open"}, 8006: {"state": "open"}})
ver = _fake_scanner("192.168.1.10", {
22: {"state": "open", "product": "OpenSSH", "version": "9.0"},
8006: {"state": "open", "product": "", "version": ""},
})
with patch("app.services.scanner.nmap.PortScanner", side_effect=[disc, ver]), \
patch("app.services.scanner.os.geteuid", return_value=0), \
patch("app.services.scanner._extract_os", return_value=None):
result = _nmap_scan_single(host)
banners = {p["port"]: p["banner"] for p in result["open_ports"]}
assert banners == {22: "OpenSSH 9.0", 8006: ""}
# Pass A: discovery only, no -sV / host-timeout. Pass B: -sV, bounded.
disc_args = disc.scan.call_args.kwargs["arguments"]
ver_args = ver.scan.call_args.kwargs["arguments"]
assert "-sV" not in disc_args and "--host-timeout" not in disc_args
assert "-sV" in ver_args and "--host-timeout 60s" in ver_args
assert ver_args.endswith("-p 22,8006") # version pass scoped to found ports
def test_nmap_scan_single_keeps_ports_when_version_pass_fails():
"""Regression #277: a stalling version pass must not drop discovered ports."""
from app.services.scanner import _nmap_scan_single
host = {"ip": "192.168.100.3", "hostname": None, "mac": None, "os": None, "open_ports": []}
disc = _fake_scanner("192.168.100.3", {22: {"state": "open"}, 8006: {"state": "open"}})
ver = _failing_scanner() # -sV blows past --host-timeout on the TLS port
with patch("app.services.scanner.nmap.PortScanner", side_effect=[disc, ver]), \
patch("app.services.scanner.os.geteuid", return_value=0):
result = _nmap_scan_single(host)
ports = {p["port"] for p in result["open_ports"]}
assert ports == {22, 8006} # both survive despite the version failure
assert all(p["banner"] == "" for p in result["open_ports"])
def test_nmap_scan_single_keeps_ports_when_version_pass_empty():
"""Version pass returns no results for the host — keep the discovered ports."""
from app.services.scanner import _nmap_scan_single
host = {"ip": "192.168.1.11", "hostname": None, "mac": None, "os": None, "open_ports": []}
disc = _fake_scanner("192.168.1.11", {443: {"state": "open"}})
ver = _empty_scanner()
with patch("app.services.scanner.nmap.PortScanner", side_effect=[disc, ver]), \
patch("app.services.scanner.os.geteuid", return_value=0):
result = _nmap_scan_single(host)
assert [p["port"] for p in result["open_ports"]] == [443]
assert result["open_ports"][0]["banner"] == ""
def test_nmap_scan_single_no_open_ports_skips_version_pass():
"""Host reachable but nothing open — no version pass, empty ports."""
from app.services.scanner import _nmap_scan_single
host = {"ip": "192.168.1.12", "hostname": None, "mac": None, "os": None, "open_ports": []}
disc = _fake_scanner("192.168.1.12", {80: {"state": "filtered"}})
# Only one PortScanner instance may be created (Pass B must be skipped);
# a second would raise StopIteration from side_effect.
with patch("app.services.scanner.nmap.PortScanner", side_effect=[disc]), \
patch("app.services.scanner.os.geteuid", return_value=0):
result = _nmap_scan_single(host)
assert result["open_ports"] == []
def test_nmap_scan_single_non_root_uses_connect_scan():
"""Without root, both passes use -sT (connect) instead of -sS (SYN)."""
from app.services.scanner import _nmap_scan_single
host = {"ip": "192.168.1.13", "hostname": None, "mac": None, "os": None, "open_ports": []}
disc = _fake_scanner("192.168.1.13", {80: {"state": "open"}})
ver = _fake_scanner("192.168.1.13", {80: {"state": "open", "product": "nginx", "version": "1.24"}})
with patch("app.services.scanner.nmap.PortScanner", side_effect=[disc, ver]), \
patch("app.services.scanner.os.geteuid", return_value=1000), \
patch("app.services.scanner._extract_os", return_value=None):
result = _nmap_scan_single(host)
assert disc.scan.call_args.kwargs["arguments"].startswith("-sT")
assert ver.scan.call_args.kwargs["arguments"].startswith("-sT")
assert result["open_ports"][0]["banner"] == "nginx 1.24"
# ---------------------------------------------------------------------------
# _nmap_scan
# ---------------------------------------------------------------------------