diff --git a/CHANGELOG.md b/CHANGELOG.md index dd44625..4cab226 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the truthful number from the unrounded value. The ratio is now rounded to three decimals, which keeps the tenth of a percent those surfaces display. +### Security + +- **A hostname that merely started with `127.` skipped the token requirement.** + Whether `serve.host` may be bound without a `serve.token` is decided by + `is_loopback_host`, which accepted any spelling beginning with `127.` — and + that also matches *names*: `127.corp.example` is a perfectly legal hostname + that resolves wherever its owner points it, so configuring it exposed the + dashboard (server addresses, login users, forwarded ports) to a public + address with no token, which is the one thing `ensure_bindable` exists to + prevent. The address is now parsed with `ipaddress` and judged by + `is_loopback`, with `localhost` the only accepted name — and a v4-mapped + address (`::ffff:127.0.0.1`) is judged by the IPv4 address it stands for, + since `IPv6Address.is_loopback` only learned those forms in 3.12 (the CI + matrix had 3.11 demanding a token for the same spelling). Spellings the OS + accepts but that parser rejects — the shorthand `127.1`, the absolute form + `localhost.` — now count as exposed, so the remaining error is "demanded a + token it did not need" rather than "skipped the token it did need". +- **Request lines reached the log verbatim.** `log_message` — and therefore + `log_error`, which funnels through it — forwarded the raw request line into + the log record. `BaseHTTPRequestHandler` decodes that line as latin-1, so a + client can put ESC, NUL, DEL, C1 bytes or bidi overrides in it: enough to make + `ponte serve`'s log claim something that never happened, or to make a terminal + tailing it render output it never received. Untrusted text is now neutralised + before logging, each non-printable character becoming a visible `?` (kept + rather than deleted, so an attempt leaves a trace instead of vanishing), and + the line is capped at 500 characters so a 64 KiB request line cannot become a + 64 KiB log line. + ### Changed - **Every SSH path now builds its connection flags in one place.** The tunnel, diff --git a/README.md b/README.md index ebf08da..3f80e81 100644 --- a/README.md +++ b/README.md @@ -178,6 +178,13 @@ refresh = 5 # dashboard auto-refresh, seconds # ipv6 hosts are fine too: host = "::1" ``` +Loopback is decided by *parsing* the address, never by matching its spelling: a +name such as `127.corp.example` resolves wherever its owner points it, so it +counts as exposed and needs a token. Shorthand the OS would accept but the +parser does not (`127.1`, or the absolute form `localhost.`) also counts as +exposed — the error message says so, and `127.0.0.1` is what the default uses +anyway. + Clients then pass `?token=...` (handy for scrapers) or `Authorization: Bearer ...`. All four endpoints are read-only, re-read the daemon status per request and send `Cache-Control: no-store`, so a page can never show a stale "healthy" for a @@ -477,6 +484,11 @@ refresh = 5 # 看板自动刷新秒数 # 也支持 IPv6：host = "::1" ``` +回环与否是**解析**地址得出的，不靠拼写匹配：像 `127.corp.example` 这样的名字 +解析到哪里由它的所有者决定，所以它算“对外”，需要令牌。操作系统接受、但解析器 +不认的简写（`127.1`，或绝对形式 `localhost.`）同样算“对外”——报错信息里会说明， +而默认值本来就是 `127.0.0.1`。 + 客户端用 `?token=...`（脚本/采集器方便）或 `Authorization: Bearer ...`。 四个接口全是只读、每次请求都重新读取守护进程状态，并带 `Cache-Control: no-store`——所以页面不会拿旧的“健康”去骗一个已经挂了的隧道。 diff --git a/ponte/config.py b/ponte/config.py index 5b7563c..34ac654 100644 --- a/ponte/config.py +++ b/ponte/config.py @@ -23,6 +23,7 @@ from __future__ import annotations +import ipaddress import logging import os import re @@ -1362,9 +1363,34 @@ def is_loopback_host(host: str) -> bool: exposed. That includes ``""``, which ``http.server`` reads as *all interfaces*, so an odd spelling cannot sneak past the token requirement in :func:`ensure_bindable`. + + The address is *parsed* rather than pattern-matched. A ``"127."`` prefix + test looks equivalent to ``127.0.0.0/8`` but is not: it also accepts + **names**, and ``127.corp.example`` is a perfectly legal hostname that + resolves wherever its owner points it. This function is the only thing + between the dashboard (server addresses, login users, forwarded ports) and + the network, so a name that merely starts with ``127.`` must not be able to + skip the token. Spellings the OS would accept but this parser rejects — the + shorthand ``127.1``, the absolute form ``localhost.``, ``::`` for that + matter — therefore count as exposed: erring towards demanding a token. """ - name = host.strip().strip("[]").lower() - return name in ("localhost", "::1") or name.startswith("127.") + name = host.strip() + if name.startswith("[") and name.endswith("]"): + name = name[1:-1] + try: + address = ipaddress.ip_address(name) + except ValueError: + return name.lower() == "localhost" + if isinstance(address, ipaddress.IPv6Address): + mapped = address.ipv4_mapped + if mapped is not None: + # ``::ffff:127.0.0.1`` *is* the IPv4 loopback, but + # ``IPv6Address.is_loopback`` only learned about v4-mapped forms in + # 3.12 — the CI matrix measured 3.11 calling this spelling exposed, + # which would have demanded a token on one interpreter and not on + # another. Judging the address it stands for keeps the gate fixed. + return mapped.is_loopback + return address.is_loopback def ensure_bindable(host: str, token: str) -> None: diff --git a/ponte/serve.py b/ponte/serve.py index 755a530..e8ad87a 100644 --- a/ponte/serve.py +++ b/ponte/serve.py @@ -1374,6 +1374,32 @@ def __init__( super().__init__(address, PonteRequestHandler) +#: Longest log line ponte writes for one request. The request line arrives +#: straight off the socket, so its length is the client's choice — http.server +#: will read up to 64 KiB of it, and a log line that long is a flood. +_LOG_TEXT_LIMIT = 500 + + +def _sanitize_log(text: str) -> str: + """Neutralise characters that could forge a log line, and cap the length. + + ``BaseHTTPRequestHandler`` hands us the request line as it came off the + socket (decoded as latin-1, so every byte above 0x7f becomes a character), + and :meth:`PonteRequestHandler.log_message` funnels ``log_error`` here too. + That text can therefore carry ESC sequences, NUL, DEL, C1 bytes or bidi + overrides — enough to make a log file claim something it never saw, or a + terminal render something it never received. ``str.isprintable`` is ``False`` + for exactly those classes (``Cc``/``Cf``/``Zl``/``Zp``) while keeping + ordinary spaces, so each offender becomes a visible ``?`` instead of an + invisible instruction. The cap keeps a 64 KiB request line from becoming a + 64 KiB log line. + """ + cleaned = "".join(char if char.isprintable() else "?" for char in text) + if len(cleaned) <= _LOG_TEXT_LIMIT: + return cleaned + return cleaned[:_LOG_TEXT_LIMIT] + "…" + + class PonteRequestHandler(BaseHTTPRequestHandler): """Serves the four read-only endpoints; one instance per connection.""" @@ -1389,8 +1415,13 @@ def _state(self) -> PonteHTTPServer: return cast(PonteHTTPServer, self.server) def log_message(self, format: str, *args: Any) -> None: # noqa: A002 - """Route the stdlib's stderr chatter into the ponte logger.""" - logger.debug("%s %s", self.address_string(), format % args) + """Route the stdlib's stderr chatter into the ponte logger. + + Everything logged for a request — including the raw request line, and + whatever :meth:`log_error` passes on — goes through :func:`_sanitize_log` + first, because none of it is ours. + """ + logger.debug("%s %s", self.address_string(), _sanitize_log(format % args)) # -- methods ----------------------------------------------------------- diff --git a/tests/test_config.py b/tests/test_config.py index 1f4adc6..ad6bdc8 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1039,14 +1039,22 @@ def test_serve_unknown_key_warns(tmp_path) -> None: [ ("127.0.0.1", True), ("127.1.2.3", True), + ("127.255.255.255", True), + ("[127.0.0.1]", True), + (" 127.0.0.1 ", True), ("localhost", True), ("LOCALHOST", True), ("[::1]", True), + ("::ffff:127.0.0.1", True), ("", False), ("0.0.0.0", False), ("::", False), + ("[::]", False), ("192.168.1.5", False), ("example.com", False), + ("notlocalhost", False), + ("127.1", False), + ("localhost.", False), ], ) def test_is_loopback_host(host, expected) -> None: @@ -1054,10 +1062,35 @@ def test_is_loopback_host(host, expected) -> None: ``""`` 特别重要：``http.server`` 把空地址当成 *所有网卡*，所以它绝不是 回环地址，不能因为“看起来是空的”就放行。 + + 最后两条是故意的窄：``127.1``（操作系统接受的简写）与 ``localhost.`` + （绝对形式）都真的指向回环，但解析器不认，于是要令牌——判错的方向必须 + 是“多要一个令牌”，而不是“少要一个”。 """ assert is_loopback_host(host) is expected +@pytest.mark.parametrize( + "host", + [ + "127.corp.example", + "127.0.0.1.example.com", + "127.0.0.1.", + "127.attacker.tld", + ], +) +def test_is_loopback_host_rejects_names_that_merely_start_with_127(host) -> None: + """前缀匹配会放行**主机名**，而名字解析到哪里由域名所有者决定。 + + ``is_loopback_host`` 是 ``ensure_bindable`` 唯一的判据，也就是看板（服务器 + 地址、登录用户、转发端口）与网络之间的全部防线：放行一个能解析到公网的名字 + 等于无令牌把内网拓扑绑出去。 + """ + assert is_loopback_host(host) is False + with pytest.raises(ConfigValidationError): + ensure_bindable(host, "") + + def test_ensure_bindable_treats_an_empty_host_as_exposed() -> None: with pytest.raises(ConfigValidationError, match="token"): ensure_bindable("", "") diff --git a/tests/test_serve.py b/tests/test_serve.py index 9329d4f..eb1acdb 100644 --- a/tests/test_serve.py +++ b/tests/test_serve.py @@ -11,7 +11,9 @@ import contextlib import json +import logging import re +import socket import threading import urllib.error import urllib.request @@ -22,6 +24,8 @@ from ponte import __version__ from ponte.config import ConfigValidationError, ensure_bindable from ponte.serve import ( + _LOG_TEXT_LIMIT, + _sanitize_log, create_server, dashboard_html, health_response, @@ -598,6 +602,39 @@ def test_dashboard_waits_quietly_for_the_first_check() -> None: assert "等待首次检查" in page +# --------------------------------------------------------------------------- +# Log lines +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + ("GET /healthz HTTP/1.1", "GET /healthz HTTP/1.1"), + ("two words", "two words"), + ("esc\x1b[31mred", "esc?[31mred"), + ("nul\x00del\x7f", "nul?del?"), + ("bidi\u202eforged", "bidi?forged"), + ("line\u2028break", "line?break"), + ("tab\tsplit", "tab?split"), + ], +) +def test_sanitize_log_neutralises_forging_characters(raw, expected) -> None: + """可打印字符（含普通空格）原样保留，其余换成可见的 ``?``。 + + 换成 ``?`` 而不是直接删除：删掉会让 ``FAKE-LOG`` 变成 ``FAKELOG``，把一次 + 尝试性注入从日志里抹掉；留着可读痕迹才能看出有人在试。 + """ + assert _sanitize_log(raw) == expected + + +def test_sanitize_log_caps_a_hostile_request_line() -> None: + """``http.server`` 肯读 64 KiB 的请求行，日志行不能跟着它长。""" + capped = _sanitize_log("A" * 64_000) + assert len(capped) == _LOG_TEXT_LIMIT + 1 # +1 是末尾的省略号 + assert capped.endswith("…") + + # --------------------------------------------------------------------------- # End to end: a real server, real HTTP # --------------------------------------------------------------------------- @@ -629,6 +666,34 @@ def _get(url: str, *, headers: dict[str, str] | None = None, method: str = "GET" return error.code, error.headers, error.read().decode("utf-8") +def _raw_request(base: str, request_line: bytes) -> None: + """Speak HTTP by hand: *urllib* refuses to send control characters.""" + host, _, port = base.removeprefix("http://").partition(":") + with socket.create_connection((host, int(port)), timeout=5) as sock: + sock.sendall(request_line + b"\r\nHost: 127.0.0.1\r\nConnection: close\r\n\r\n") + with contextlib.suppress(OSError, TimeoutError): + while sock.recv(65536): + pass + + +def test_request_lines_cannot_forge_log_lines(caplog) -> None: + """请求行是攻击者可控的，日志则是给人看的——两者不能直接相接。 + + 走裸套接字是必须的（``urllib`` 不会替我们发控制字符，而真实攻击者会）： + ESC 能伪造终端输出，NUL/DEL 能骗过日志查看器，而 latin-1 解码还会把 UTF-8 + 字节变成 C1 控制字符。 + """ + hostile = b"GET /x\x1b[31mFORGED\x00\x7f\xc2\x80 HTTP/1.1" + with caplog.at_level(logging.DEBUG, logger="ponte.serve"): + with _running_server(lambda: _payload()) as base: + _raw_request(base, hostile) + + logged = [record.getMessage() for record in caplog.records if record.name == "ponte.serve"] + assert any("FORGED" in message for message in logged), "整条消息不该被丢弃" + for message in logged: + assert message.isprintable(), message + + def test_end_to_end_endpoints_answer() -> None: with _running_server(lambda: _payload()) as base: status, headers, body = _get(base + "/")