From 6f01d5550d2defe7984623a31f0b0b96aadc915d Mon Sep 17 00:00:00 2001 From: Pouzor Date: Sat, 18 Jul 2026 00:35:27 +0200 Subject: [PATCH] fix(scanner): decouple port discovery from version detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit nmap -sV sends plaintext probes to TLS ports (e.g. Proxmox 8006). They stall until the host crosses --host-timeout, at which point nmap skips the whole host and discards ports it already found — a reachable host is returned with 0 open ports (issue #277). Split Phase 2 into two passes: Pass A (-sS/-sT --open) discovers ports fast with no version detection; its ports are authoritative. Pass B (-sV --host-timeout, scoped to the found ports) enriches banners best-effort — on timeout/failure the discovered ports are kept with empty banners instead of losing the host. Healthy hosts still get full-intensity -sV banners (no quality drop). Version host-timeout is now configurable via SCANNER_VERSION_HOST_TIMEOUT (default 60s). ha-relevant: yes --- backend/app/core/config.py | 5 ++ backend/app/services/scanner.py | 91 +++++++++++++++++------- backend/tests/test_scanner.py | 122 ++++++++++++++++++++++++++++++++ 3 files changed, 191 insertions(+), 27 deletions(-) diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 12b85b7..7c24d01 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -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] = [] diff --git a/backend/app/services/scanner.py b/backend/app/services/scanner.py index b616f93..cb70362 100644 --- a/backend/app/services/scanner.py +++ b/backend/app/services/scanner.py @@ -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 diff --git a/backend/tests/test_scanner.py b/backend/tests/test_scanner.py index 2b296ef..a34832b 100644 --- a/backend/tests/test_scanner.py +++ b/backend/tests/test_scanner.py @@ -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 # ---------------------------------------------------------------------------