From 6fa1b1a3d0a084e0245508e50cc9da3f07c3d9d7 Mon Sep 17 00:00:00 2001 From: Pouzor Date: Thu, 3 Sep 2026 23:07:02 +0200 Subject: [PATCH] fix(scan): never send an HTTP GET to a raw printing port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A printer treats whatever bytes land on TCP/9100 as a print job, so our probe's "GET / HTTP/1.1" came out of the tray as a page of plaintext. Two paths reached it, and neither could be configured away — 9100 is hard-coded in the scanner's port list, and user ranges only ever add to that list: - the deep-scan HTTP probe, once per scan; - the status checker, every 60 s for as long as the node exists, which is the one that turns a scan annoyance into a ream of paper. Both already had a non-HTTP port denylist; 515 and 9100-9107 were simply missing from it. nmap ships `Exclude T:9100-9107` for this exact reason, which is why the -sV pass never triggered it and only our own probes did. Node Exporter also lives on 9100 and loses its status check as a result. A grey dot is the cheaper mistake. Its fingerprint entry is port-match only, so discovery and labelling are unaffected. Fixes #404 ha-relevant: yes --- backend/app/services/http_probe.py | 11 ++++++++- backend/app/services/status_checker.py | 9 +++++++- backend/tests/test_http_probe.py | 25 +++++++++++++++++++++ backend/tests/test_status_checker.py | 31 ++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/backend/app/services/http_probe.py b/backend/app/services/http_probe.py index 554de4d..cc109ec 100644 --- a/backend/app/services/http_probe.py +++ b/backend/app/services/http_probe.py @@ -25,7 +25,16 @@ _MAX_BODY_BYTES = 64 * 1024 _TITLE_RE = re.compile(r"]*>(.*?)", re.IGNORECASE | re.DOTALL) _PROBE_TIMEOUT = 3.0 # Ports we never bother probing over HTTP (not web services). -_NON_HTTP_PORTS = frozenset({22, 21, 23, 25, 53, 110, 143, 161, 162, 179, 445, 3306, 5432, 6379}) +# +# 515 and 9100-9107 are raw printing (LPD / JetDirect). Those are not merely +# uninteresting: a printer takes whatever bytes land on 9100 as a print job, so +# a GET line makes it spit out a page of plaintext. Never send them anything. +# nmap already ships `Exclude T:9100-9107` for the same reason, which is why +# the -sV pass never triggered this — only our own probe did. +_PRINTER_PORTS = frozenset({515, *range(9100, 9108)}) +_NON_HTTP_PORTS = frozenset({ + 22, 21, 23, 25, 53, 110, 143, 161, 162, 179, 445, 3306, 5432, 6379, +}) | _PRINTER_PORTS async def _read_capped(resp: httpx.Response) -> str: diff --git a/backend/app/services/status_checker.py b/backend/app/services/status_checker.py index 21dbab1..76b9bf0 100644 --- a/backend/app/services/status_checker.py +++ b/backend/app/services/status_checker.py @@ -134,10 +134,17 @@ async def _tcp_connect(host: str, port: int) -> bool: # grey (unknown) rather than going red. An open TCP socket doesn't prove the # service is healthy, and a closed one flaps red misleadingly (e.g. SSH on a # box that simply firewalls 22). Only HTTP(S)-reachable services are checked. +# +# 515 and 9100-9107 are raw printing (LPD / JetDirect), and they are here for a +# stronger reason than the rest: a printer treats any bytes on 9100 as a print +# job, so a GET line makes it print a page — every 60 s, for as long as the node +# exists. Node Exporter also lives on 9100 and loses its status check because of +# this; a grey dot is the cheaper mistake. +_PRINTER_PORTS = frozenset({515, *range(9100, 9108)}) _NON_HTTP_PORTS = frozenset({ 22, 21, 23, 25, 465, 587, 53, 110, 143, 993, 995, 389, 636, 445, 514, 1433, 3306, 5432, 5672, 6379, 9092, 11211, 27017, 27018, -}) +}) | _PRINTER_PORTS _HTTPS_PORTS = frozenset({443, 8443}) diff --git a/backend/tests/test_http_probe.py b/backend/tests/test_http_probe.py index e7c171f..2f0817b 100644 --- a/backend/tests/test_http_probe.py +++ b/backend/tests/test_http_probe.py @@ -131,6 +131,31 @@ async def test_probe_port_skips_non_http_ports(): assert factory.requests == [] +@pytest.mark.parametrize("port", [515, 9100, 9101, 9107]) +@pytest.mark.asyncio +async def test_probe_port_never_touches_a_raw_printing_port(port): + """Issue #404: a printer prints whatever bytes reach 9100, GET line included.""" + ctx, factory = _patch_client(lambda req: _response("nope")) + with ctx: + result = await probe_port("10.0.0.5", port) + assert result is None + assert factory.requests == [] + + +@pytest.mark.asyncio +async def test_probe_open_ports_leaves_a_printer_port_alone(): + """The batch path must skip it too, without dropping the port from the result.""" + ports = [{"port": 8096, "protocol": "tcp"}, {"port": 9100, "protocol": "tcp"}] + ctx, factory = _patch_client(lambda req: _response("Jellyfin")) + with ctx: + result = await probe_open_ports("10.0.0.5", ports) + + by_port = {p["port"]: p for p in result} + assert by_port[9100]["http_signals"] is None + assert by_port[8096]["http_signals"]["title"] == "Jellyfin" + assert all(":9100" not in str(r.url) for r in factory.requests) + + @pytest.mark.asyncio async def test_probe_port_verify_tls_flag_passed(): ctx, factory = _patch_client(lambda req: _response("X")) diff --git a/backend/tests/test_status_checker.py b/backend/tests/test_status_checker.py index 42a3724..dca69c1 100644 --- a/backend/tests/test_status_checker.py +++ b/backend/tests/test_status_checker.py @@ -450,6 +450,37 @@ async def test_check_service_non_http_port_is_unknown(): mock_http.assert_not_called() +@pytest.mark.parametrize("port", [515, 9100, 9101, 9107]) +@pytest.mark.asyncio +async def test_check_service_never_touches_a_raw_printing_port(port): + """Issue #404: a GET to JetDirect/LPD makes the printer print it. + + The status checker runs every 60 s, so a printer approved as a node used to + print one page a minute, forever. These ports get no request at all. + """ + svc = {"port": port, "protocol": "tcp", "service_name": "jetdirect"} + with patch("app.services.status_checker._tcp_connect", new_callable=AsyncMock) as mock_tcp, \ + patch("app.services.status_checker._http_get", new_callable=AsyncMock) as mock_http: + result = await check_service(svc, "10.0.0.1") + assert result == "unknown" + mock_tcp.assert_not_called() + mock_http.assert_not_called() + + +@pytest.mark.asyncio +async def test_check_service_printer_port_named_http_is_still_skipped(): + """A service_name saying "http" must not talk the checker onto 9100. + + Fingerprinting labels 9100 "Node Exporter", and an override can put any + name there — the port decides, not the label. + """ + svc = {"port": 9100, "protocol": "tcp", "service_name": "http-node-exporter"} + with patch("app.services.status_checker._http_get", new_callable=AsyncMock) as mock_http: + result = await check_service(svc, "10.0.0.1") + assert result == "unknown" + mock_http.assert_not_called() + + @pytest.mark.asyncio async def test_check_service_ssh_port_22_is_unknown(): """SSH (port 22) is never checked — keep it grey, not red/green."""