fix(scanner): decouple port discovery from version detection
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
This commit is contained in:
@@ -52,6 +52,11 @@ class Settings(BaseSettings):
|
|||||||
# Scanner
|
# Scanner
|
||||||
scanner_ranges: list[str] = ["192.168.1.0/24"]
|
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).
|
# 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.
|
# http_ranges: extra nmap port ranges, opt-in, no default. Probe + TLS off by default.
|
||||||
scanner_http_ranges: list[str] = []
|
scanner_http_ranges: list[str] = []
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ from typing import Any
|
|||||||
from sqlalchemy import or_, select
|
from sqlalchemy import or_, select
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
|
from app.core.config import settings
|
||||||
from app.db.models import Node, PendingDevice, ScanRun
|
from app.db.models import Node, PendingDevice, ScanRun
|
||||||
from app.services.discovery_sources import add_source
|
from app.services.discovery_sources import add_source
|
||||||
from app.services.fingerprint import fingerprint_ports, suggest_node_type
|
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.
|
Phase 2 — single-IP port scan with service detection.
|
||||||
Runs in a thread (blocking). Returns the host dict enriched with open_ports.
|
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"]
|
ip = host_dict["ip"]
|
||||||
logger.info("[Phase 2] Scanning %s ...", 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)
|
logger.warning("[Phase 2] nmap not available, skipping %s", ip)
|
||||||
return host_dict
|
return host_dict
|
||||||
|
|
||||||
is_root = os.geteuid() == 0
|
# -sS (SYN) needs root; -sT (connect) works without it. nmap auto-selects
|
||||||
if is_root:
|
# -sT without root but being explicit avoids edge cases.
|
||||||
# SYN scan + version detection (fastest, most accurate)
|
scan_type = "-sS" if os.geteuid() == 0 else "-sT"
|
||||||
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}"
|
|
||||||
|
|
||||||
logger.debug("[Phase 2] %s args: %s", ip, scan_args)
|
# --- Pass A: port discovery (no -sV, no host-timeout) ---
|
||||||
nm = nmap.PortScanner()
|
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:
|
try:
|
||||||
nm.scan(hosts=ip, arguments=scan_args)
|
nm_disc.scan(hosts=ip, arguments=discovery_args)
|
||||||
except Exception as exc:
|
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
|
return host_dict
|
||||||
|
|
||||||
all_scanned = nm.all_hosts()
|
if ip not in nm_disc.all_hosts():
|
||||||
logger.debug("[Phase 2] %s — nmap returned %d host(s) in results", ip, len(all_scanned))
|
|
||||||
if ip not in all_scanned:
|
|
||||||
logger.info("[Phase 2] %s — no open ports found (all closed/filtered or nmap had no results)", ip)
|
logger.info("[Phase 2] %s — no open ports found (all closed/filtered or nmap had no results)", ip)
|
||||||
return host_dict
|
return host_dict
|
||||||
|
|
||||||
open_ports = []
|
open_ports = []
|
||||||
for proto in nm[ip].all_protocols():
|
for proto in nm_disc[ip].all_protocols():
|
||||||
for port, info in nm[ip][proto].items():
|
for port, info in nm_disc[ip][proto].items():
|
||||||
if info["state"] == "open":
|
if info["state"] == "open":
|
||||||
banner = (info.get("product", "") + " " + info.get("version", "")).strip()
|
open_ports.append({"port": port, "protocol": proto, "banner": ""})
|
||||||
open_ports.append({"port": port, "protocol": proto, "banner": banner})
|
|
||||||
|
|
||||||
if open_ports:
|
if not 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:
|
|
||||||
logger.info("[Phase 2] %s — 0 open ports detected", ip)
|
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
|
host_dict["open_ports"] = open_ports
|
||||||
if not host_dict["mac"]:
|
if not host_dict["mac"]:
|
||||||
host_dict["mac"] = nm[ip].get("addresses", {}).get("mac")
|
host_dict["mac"] = ver_mac or nm_disc[ip].get("addresses", {}).get("mac")
|
||||||
host_dict["os"] = _extract_os(nm, ip)
|
host_dict["os"] = ver_os
|
||||||
return host_dict
|
return host_dict
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -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
|
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
|
# _nmap_scan
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user