From a2a173b77093f60af67e527da47c0ee4b93eadc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Mon, 27 Jul 2026 09:54:28 +0200 Subject: [PATCH 01/10] feat(license): deliver and transport VPP licenses over the debug channel Checkpoint of Sub-fase B (vpp licensing): apply_vpp_plugin_conf copies conf/.license alongside the installed plugin config (bundle delivery). New webserver/vpp_license_debug.py answers 0x48/0x49/0x4A at the Python level in debug_websocket.py, ahead of the is_connected gate, mirroring the same wire contract the Arduino firmware already speaks. Includes a path traversal guard on license writes and pytest coverage for path parity, delivery, and non-resurrection. Co-Authored-By: Claude Sonnet 5 --- .../pytest/plugins/test_vpp_license_debug.py | 135 ++++++++++++++++ .../plugins/test_vpp_license_delivery.py | 89 ++++++++++ webserver/debug_websocket.py | 23 ++- webserver/plcapp_management.py | 13 ++ webserver/vpp_license_debug.py | 152 ++++++++++++++++++ 5 files changed, 406 insertions(+), 6 deletions(-) create mode 100644 tests/pytest/plugins/test_vpp_license_debug.py create mode 100644 tests/pytest/plugins/test_vpp_license_delivery.py create mode 100644 webserver/vpp_license_debug.py diff --git a/tests/pytest/plugins/test_vpp_license_debug.py b/tests/pytest/plugins/test_vpp_license_debug.py new file mode 100644 index 00000000..8f7b2638 --- /dev/null +++ b/tests/pytest/plugins/test_vpp_license_debug.py @@ -0,0 +1,135 @@ +"""Tests for the webserver-level VPP license debug FCs (0x48/0x49/0x4A). + +Covers the raw-PDU responses the editor's modbus-pdu.ts parsers expect, the raw +(non-hex-decoded) anchor semantics that must match the .so (D70d), and the 0x49 +write / 0x4A read round-trip landing on the .license sibling of the plugin +config (same path the bundle + the .so use). +""" +import os + +import pytest + +lic = pytest.importorskip( + "webserver.vpp_license_debug", + reason="runtime webserver package not importable (no venv)", +) + + +def _hex(data: bytes) -> str: + return " ".join(f"{b:02X}" for b in data) + + +def _install_plugin(tmp_path, monkeypatch): + """Fake one installed VPP plugin whose config_path lives under a temp cwd.""" + cwd = tmp_path / "runtime" + (cwd / "build" / "vpp").mkdir(parents=True) + monkeypatch.chdir(cwd) + (cwd / "vpp_plugins.conf").write_text("dummy\n") + config_path = str(cwd / "build" / "vpp" / "rpi_gpio.json") + + class _P: + name = "rpi_gpio" + config_path = None + + def __init__(self, cp): + self.config_path = cp + + class _Conf: + plugins = [_P(config_path)] + + monkeypatch.setattr(lic.PluginsConfiguration, "from_file", classmethod(lambda cls, _p: _Conf())) + return config_path + + +def test_is_license_command(): + assert lic.is_license_command("48") + assert lic.is_license_command("49 00 62") + assert lic.is_license_command("4A") + assert not lic.is_license_command("41 00 00") + assert not lic.is_license_command("") + + +def test_get_board_id_returns_raw_ascii_anchor(tmp_path, monkeypatch): + # Mimic /proc/device-tree/serial-number: ASCII hex + trailing NUL. + anchor_file = tmp_path / "serial-number" + anchor_file.write_bytes(b"8625807b0a83ae7d\x00") + monkeypatch.setattr(lic, "ANCHOR_PATH", str(anchor_file)) + + resp = lic.handle_license_command("48") + parts = resp.split() + assert parts[0] == "48" + assert parts[1] == "7E" # SUCCESS + assert parts[2] == "10" # 16 bytes + body = bytes(int(p, 16) for p in parts[3:]) + # RAW ascii, NUL stripped -- NOT hex-decoded (the .so hashes exactly these bytes). + assert body == b"8625807b0a83ae7d" + + +def test_get_board_id_missing_anchor_is_empty_success(tmp_path, monkeypatch): + # No anchor -> SUCCESS with id_len=0 (matches Arduino D57), not an error byte. + monkeypatch.setattr(lic, "ANCHOR_PATH", str(tmp_path / "nope")) + assert lic.handle_license_command("48") == "48 7E 00" + + +def test_write_refuses_path_traversal(tmp_path, monkeypatch): + # A forged vpp_plugins.conf whose config_path escapes the runtime root must + # NOT let 0x49 write outside it (defense-in-depth; mirrors apply_vpp_plugin_conf). + cwd = tmp_path / "runtime" + cwd.mkdir() + monkeypatch.chdir(cwd) + (cwd / "vpp_plugins.conf").write_text("dummy\n") + escaping = str(tmp_path / "outside" / "evil.json") # sibling of cwd -> escapes root + + class _P: + name = "x" + config_path = escaping + + class _Conf: + plugins = [_P()] + + monkeypatch.setattr(lic.PluginsConfiguration, "from_file", classmethod(lambda cls, _p: _Conf())) + + cmd = _hex(bytes([0x49, 0x00, 0x62]) + bytes(98)) + assert lic.handle_license_command(cmd) == "49 85" # refused -> LIC_UNSUPPORTED + assert not os.path.exists(tmp_path / "outside" / "evil.license") + + +def test_read_license_empty_when_no_conf(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) # no vpp_plugins.conf here + assert lic.handle_license_command("4A") == "4A 83" # LIC_EMPTY + + +def test_write_then_read_roundtrip(tmp_path, monkeypatch): + config_path = _install_plugin(tmp_path, monkeypatch) + blob = bytes([0x4F, 0x50, 0x4C, 0x43]) + bytes(range(1, 95)) # 98 bytes + assert len(blob) == 98 + + cmd = _hex(bytes([0x49, 0x00, 0x62]) + blob) # [0x49][len=98 u16BE][blob] + assert lic.handle_license_command(cmd) == "49 7E" # SUCCESS + + expected_path = config_path[:-5] + ".license" + assert os.path.exists(expected_path) + assert os.path.getsize(expected_path) == 98 + + read = lic.handle_license_command("4A") + parts = read.split() + assert parts[0] == "4A" and parts[1] == "7E" + assert parts[2] == "00" and parts[3] == "62" # len 98, u16BE + assert bytes(int(p, 16) for p in parts[4:]) == blob + + +def test_write_wrong_size_is_corrupt(tmp_path, monkeypatch): + _install_plugin(tmp_path, monkeypatch) + cmd = _hex(bytes([0x49, 0x00, 0x04]) + b"\x01\x02\x03\x04") # not 98 + assert lic.handle_license_command(cmd) == "49 84" # LIC_CORRUPT + + +def test_write_without_installed_plugin_is_unsupported(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) # no vpp_plugins.conf + blob = bytes(98) + cmd = _hex(bytes([0x49, 0x00, 0x62]) + blob) + assert lic.handle_license_command(cmd) == "49 85" # LIC_UNSUPPORTED + + +def test_non_license_fc_passes_through(): + assert lic.handle_license_command("41 00 00 00 01") is None diff --git a/tests/pytest/plugins/test_vpp_license_delivery.py b/tests/pytest/plugins/test_vpp_license_delivery.py new file mode 100644 index 00000000..efdbc01a --- /dev/null +++ b/tests/pytest/plugins/test_vpp_license_delivery.py @@ -0,0 +1,89 @@ +"""Tests for VPP device-license delivery via apply_vpp_plugin_conf. + +A licensed VPP's activated blob rides in the upload as conf/.license and +must land next to the plugin config at the sibling path the .so derives from its +config_path (drop a trailing ".json", append ".license"). If the runtime and the +plugin disagree on that path, the .so never finds the license and falls to demo. + +The parity test is pure (no runtime deps). The integration test imports the real +apply_vpp_plugin_conf and is skipped where the webserver package can't import +(e.g. a dev box without the runtime venv); CI with the venv runs it. +""" +import os +import shutil + +import pytest + + +def _plugin_license_path(config_path: str) -> str: + """Mirror of derive_license_path() in the licensed rpi_plugin.c: the .so's + view of where its license lives, given its config path.""" + base = config_path[:-5] if config_path.endswith(".json") else config_path + return base + ".license" + + +def _runtime_license_dest(dest_config: str) -> str: + """Mirror of the delivery rule in apply_vpp_plugin_conf (kept identical).""" + base = dest_config[:-5] if dest_config.endswith(".json") else dest_config + return base + ".license" + + +@pytest.mark.parametrize( + "config_path", + [ + "/opt/runtime/build/vpp/rpi_gpio.json", + "build/vpp/rpi_gpio.json", + "rpi_gpio", # no extension + "a/b.c/rpi_gpio.json", + ], +) +def test_delivery_path_matches_plugin_derivation(config_path): + """The runtime must deliver the .license to exactly the path the plugin reads.""" + assert _runtime_license_dest(config_path) == _plugin_license_path(config_path) + + +def test_apply_vpp_plugin_conf_delivers_license(tmp_path, monkeypatch): + """Integration: a conf/.license in the upload is copied to the sibling + of the plugin's config_path; absence leaves no license (device -> demo).""" + mgmt = pytest.importorskip( + "webserver.plcapp_management", + reason="runtime webserver package not importable (no venv)", + ) + + # Fake a single native plugin whose config_path lives under the temp cwd. + cwd = tmp_path / "runtime" + (cwd).mkdir() + monkeypatch.chdir(cwd) + config_path = str(cwd / "build" / "vpp" / "rpi_gpio.json") + + class _P: + name = "rpi_gpio" + + def __init__(self, cp): + self.config_path = cp + + class _Conf: + plugins = [_P(config_path)] + + monkeypatch.setattr(mgmt.PluginsConfiguration, "from_file", classmethod(lambda cls, _p: _Conf())) + monkeypatch.setattr(mgmt.build_state, "log", lambda *_a, **_k: None, raising=False) + + # Build the uploaded generated_dir: vpp_plugins.conf + conf/{json,license}. + gen = tmp_path / "generated" + (gen / "conf").mkdir(parents=True) + (gen / "vpp_plugins.conf").write_text("dummy\n") + (gen / "conf" / "rpi_gpio.json").write_text("{}\n") + (gen / "conf" / "rpi_gpio.license").write_bytes(b"\x4f\x50\x4c\x43" + b"\x00" * 94) # 98-byte blob + + mgmt.apply_vpp_plugin_conf(str(gen)) + + expected = config_path[:-5] + ".license" + assert os.path.exists(expected), "license blob not delivered to the plugin's sibling path" + assert os.path.getsize(expected) == 98 + + # Second pass without a .license in the upload must not resurrect a stale one + # from the same source (delivery only copies what the upload carries). + os.remove(expected) + (gen / "conf" / "rpi_gpio.license").unlink() + mgmt.apply_vpp_plugin_conf(str(gen)) + assert not os.path.exists(expected) diff --git a/webserver/debug_websocket.py b/webserver/debug_websocket.py index 655f10e8..af9a17fe 100644 --- a/webserver/debug_websocket.py +++ b/webserver/debug_websocket.py @@ -11,6 +11,7 @@ from flask_socketio import SocketIO, emit from webserver.logger import get_logger +from webserver.vpp_license_debug import handle_license_command logger, _ = get_logger("debug_ws", use_buffer=True) @@ -109,6 +110,22 @@ def handle_debug_command(data): Returns debug response in same hex format """ try: + command_hex = data.get("command", "") + if not command_hex: + logger.warning("Empty debug command received") + emit("debug_response", {"success": False, "error": "Empty command"}) + return + + # License function codes (0x48/0x49/0x4A) operate on host files + # (/proc anchor + conf/.license) and are resolved here in + # Python (D70a) BEFORE the unix-socket gate below, so device + # activation works even while the PLC/core is stopped. + license_response = handle_license_command(command_hex) + if license_response is not None: + logger.debug("License FC handled locally: %s -> %s", command_hex, license_response) + emit("debug_response", {"success": True, "data": license_response}) + return + if not _unix_client or not _unix_client.is_connected(): logger.error("Unix socket not connected") emit( @@ -117,12 +134,6 @@ def handle_debug_command(data): ) return - command_hex = data.get("command", "") - if not command_hex: - logger.warning("Empty debug command received") - emit("debug_response", {"success": False, "error": "Empty command"}) - return - logger.debug("Debug command received: %s", command_hex) unix_command = f"DEBUG:{command_hex}\n" diff --git a/webserver/plcapp_management.py b/webserver/plcapp_management.py index b2476897..9d072747 100644 --- a/webserver/plcapp_management.py +++ b/webserver/plcapp_management.py @@ -316,6 +316,19 @@ def apply_vpp_plugin_conf(generated_dir: str = "core/generated") -> None: os.makedirs(os.path.dirname(dest_config), exist_ok=True) shutil.copy2(src_config, dest_config) build_state.log(f"[INFO] VPP: copied {p.name}.json to {dest_config}\n") + + # Deliver the optional device license blob alongside the config, at + # the sibling path the licensed plugin derives from its config path + # (drop a trailing ".json", append ".license" -- must mirror + # derive_license_path() in the plugin exactly, or the .so reads the + # wrong path and falls to demo). Present only for a licensed VPP whose + # device was activated; absent for free VPPs or demo devices. + src_license = os.path.join(conf_dir, f"{p.name}.license") + if os.path.exists(src_license): + base = dest_config[:-5] if dest_config.endswith(".json") else dest_config + dest_license = base + ".license" + shutil.copy2(src_license, dest_license) + build_state.log(f"[INFO] VPP: copied {p.name}.license to {dest_license}\n") else: # No VPP in this upload — remove any stale vpp_plugins.conf so # the plugin loader does not attempt to load old VPP drivers. diff --git a/webserver/vpp_license_debug.py b/webserver/vpp_license_debug.py new file mode 100644 index 00000000..d6ed0dd7 --- /dev/null +++ b/webserver/vpp_license_debug.py @@ -0,0 +1,152 @@ +"""VPP device-license debug function codes (0x48/0x49/0x4A), resolved at the +webserver level (D70a). + +On runtime-v4 the licensing anchor and the license blob are HOST FILES +(``/proc/device-tree/serial-number`` and ``conf/.license``), not plugin +memory, so these function codes are answered here in Python instead of the +realtime C core: it needs no core rebuild, works while the PLC is stopped +(resolves the chicken-and-egg of activating before a program runs), and reuses +the same license-path derivation as the bundle delivery in +``apply_vpp_plugin_conf``. + +This lets the editor speak ONE license protocol over any transport (D70c): the +exact Modbus PDU it uses on Arduino, carried by the debug WebSocket. The frame is +raw Modbus PDU (no MBAP, no CRC), byte-identical to the editor's ``modbus-pdu.ts``: + + 0x48 get-board-id : req [0x48] resp [0x48][status][id_len:u8][id...] + 0x49 write-license: req [0x49][len:u16BE][blob] resp [0x49][status] + 0x4A read-license : req [0x4A] resp [0x4A][status][len:u16BE][blob] + +Anchor bytes are returned RAW (ASCII, trailing NUL/whitespace stripped) so the +editor derives the SAME device_id the .so does (D70d); no hex-decoding. +""" +import os +from typing import Optional + +from webserver.plugin_config_model import PluginsConfiguration + +# License function codes (mirror simulator/types.ts + firmware modbus_types.h). +FC_GET_BOARD_ID = 0x48 +FC_WRITE_LICENSE = 0x49 +FC_READ_LICENSE = 0x4A +_LICENSE_FCS = (FC_GET_BOARD_ID, FC_WRITE_LICENSE, FC_READ_LICENSE) + +# Status bytes (shared with the Arduino firmware / editor). +ST_SUCCESS = 0x7E +ST_LIC_EMPTY = 0x83 +ST_LIC_CORRUPT = 0x84 +ST_LIC_UNSUPPORTED = 0x85 + +ANCHOR_PATH = "/proc/device-tree/serial-number" +VPP_CONF = "vpp_plugins.conf" +LIC_BLOB_SIZE = 98 + + +def is_license_command(command_hex: str) -> bool: + """True when the PDU's first byte is a license function code.""" + data = _bytes_from_hex(command_hex) + return bool(data) and data[0] in _LICENSE_FCS + + +def _bytes_from_hex(command_hex: str) -> bytes: + try: + return bytes(int(tok, 16) for tok in command_hex.split()) + except ValueError: + return b"" + + +def _hex_from_bytes(data: bytes) -> str: + # Uppercase 2-digit, space-joined -- the format the editor's + # hexSpacedToBytes/bytesToHexSpaced round-trips. + return " ".join(f"{b:02X}" for b in data) + + +def _read_anchor() -> bytes: + try: + with open(ANCHOR_PATH, "rb") as handle: + raw = handle.read() + except OSError: + return b"" + # Strip trailing NUL / whitespace -- MUST match derive in rpi_plugin.c + # (the device-tree serial is NUL-terminated). + return raw.rstrip(b"\x00\r\n\t ") + + +def _license_path() -> Optional[str]: + """The ``.license`` sibling of the installed licensed plugin's config_path, + mirroring ``apply_vpp_plugin_conf``/derive_license_path so 0x49 and the + bundle write the SAME file the .so reads. None when no VPP plugin config is + installed yet (no upload). Multi-plugin disambiguation by vppId is a future + extension (the PDU carries no plugin id); the common case is one VPP plugin. + """ + if not os.path.exists(VPP_CONF): + return None + try: + conf = PluginsConfiguration.from_file(VPP_CONF) + except Exception: + return None + candidates = [p for p in conf.plugins if getattr(p, "config_path", None)] + if not candidates: + return None + config_path = candidates[0].config_path + base = config_path[:-5] if config_path.endswith(".json") else config_path + path = base + ".license" + # Anti-traversal (mirror the guard apply_vpp_plugin_conf applies to the same + # config_path): never read/write outside the runtime root, even if a forged + # vpp_plugins.conf carries an escaping config_path. 0x49 writes 98 bytes as + # root, so refuse an escaping target rather than trust the conf. + runtime_root = os.path.abspath(".") + if not os.path.abspath(path).startswith(runtime_root + os.sep): + return None + return path + + +def handle_license_command(command_hex: str) -> Optional[str]: + """Resolve a license function code and return the response as spaced hex. + + Returns ``None`` when the command is not a license FC, so the caller forwards + it to the C core as before. Never raises for a well-formed license FC. + """ + data = _bytes_from_hex(command_hex) + if not data or data[0] not in _LICENSE_FCS: + return None + fc = data[0] + + if fc == FC_GET_BOARD_ID: + anchor = _read_anchor() + if not anchor: + # Match the Arduino firmware (D57): no id -> SUCCESS with id_len=0, so + # the editor sees a clean empty id (outcome no-id), not an error byte. + return _hex_from_bytes(bytes([fc, ST_SUCCESS, 0])) + length = min(len(anchor), 255) + return _hex_from_bytes(bytes([fc, ST_SUCCESS, length]) + anchor[:length]) + + if fc == FC_READ_LICENSE: + path = _license_path() + if not path or not os.path.exists(path): + return _hex_from_bytes(bytes([fc, ST_LIC_EMPTY])) + with open(path, "rb") as handle: + blob = handle.read() + if len(blob) != LIC_BLOB_SIZE: + return _hex_from_bytes(bytes([fc, ST_LIC_CORRUPT])) + length = len(blob) + header = bytes([fc, ST_SUCCESS, (length >> 8) & 0xFF, length & 0xFF]) + return _hex_from_bytes(header + blob) + + if fc == FC_WRITE_LICENSE: + # [0x49][len:u16BE][blob...] + if len(data) < 3: + return _hex_from_bytes(bytes([fc, ST_LIC_CORRUPT])) + length = (data[1] << 8) | data[2] + blob = data[3 : 3 + length] + if len(blob) != length or length != LIC_BLOB_SIZE: + return _hex_from_bytes(bytes([fc, ST_LIC_CORRUPT])) + path = _license_path() + if not path: + return _hex_from_bytes(bytes([fc, ST_LIC_UNSUPPORTED])) + os.makedirs(os.path.dirname(path) or ".", exist_ok=True) + with open(path, "wb") as handle: + handle.write(blob) + return _hex_from_bytes(bytes([fc, ST_SUCCESS])) + + return None From 48bc68868357944e4adbdc931b75340bba99ed62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Mon, 27 Jul 2026 10:10:47 +0200 Subject: [PATCH 02/10] fix(license): strengthen anti-traversal guard, dedup license-path derivation apply_vpp_plugin_conf's guard only checked os.path.abspath(dest_config) .startswith(runtime_root) -- a sibling directory that merely shares the root as a string prefix (e.g. runtime_root "/opt/runtime" vs. an escaping "/opt/runtime-evil/x") would wrongly pass. Anchor on runtime_root + os.sep instead, mirroring the guard vpp_license_debug.py's 0x49 handler already had. Extracted derive_license_path/resolve_license_path into vpp_license_debug.py so both the bundle-delivery path (apply_vpp_plugin_conf) and the debug-channel path (0x49) share one derivation instead of two independently-maintained copies. Co-Authored-By: Claude Sonnet 5 --- webserver/plcapp_management.py | 22 +++++++++------ webserver/vpp_license_debug.py | 50 ++++++++++++++++++++++++---------- 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/webserver/plcapp_management.py b/webserver/plcapp_management.py index 9d072747..87dc8a45 100644 --- a/webserver/plcapp_management.py +++ b/webserver/plcapp_management.py @@ -12,6 +12,7 @@ from webserver.runtimemanager import RuntimeManager from webserver.logger import get_logger, LogParser from webserver.plugin_config_model import PluginsConfiguration, PluginConfig, PluginType +from webserver.vpp_license_debug import derive_license_path logger, _ = get_logger("runtime", use_buffer=True) @@ -309,8 +310,13 @@ def apply_vpp_plugin_conf(generated_dir: str = "core/generated") -> None: build_state.log(f"[WARNING] VPP: conf/{p.name}.json not found in upload, skipping\n") continue dest_config = os.path.normpath(p.config_path) - # Guard against path traversal in editor-generated vpp_plugins.conf - if not os.path.abspath(dest_config).startswith(runtime_root): + # Guard against path traversal in editor-generated vpp_plugins.conf. + # `+ os.sep` matters: a bare prefix check would wrongly accept a + # sibling directory that merely shares runtime_root as a string + # prefix (e.g. runtime_root "/opt/runtime" vs. an escaping + # "/opt/runtime-evil/x") -- anchoring on the separator requires the + # escaping path to actually be a child of the runtime directory. + if not os.path.abspath(dest_config).startswith(runtime_root + os.sep): build_state.log(f"[WARNING] VPP: config_path '{p.config_path}' escapes runtime root, skipping\n") continue os.makedirs(os.path.dirname(dest_config), exist_ok=True) @@ -319,14 +325,14 @@ def apply_vpp_plugin_conf(generated_dir: str = "core/generated") -> None: # Deliver the optional device license blob alongside the config, at # the sibling path the licensed plugin derives from its config path - # (drop a trailing ".json", append ".license" -- must mirror - # derive_license_path() in the plugin exactly, or the .so reads the - # wrong path and falls to demo). Present only for a licensed VPP whose - # device was activated; absent for free VPPs or demo devices. + # (derive_license_path, shared with vpp_license_debug.py's 0x49 + # handler so both write the SAME file the .so reads). Present only + # for a licensed VPP whose device was activated; absent for free + # VPPs or demo devices. dest_config already passed the traversal + # guard above, so no need to re-check its .license sibling. src_license = os.path.join(conf_dir, f"{p.name}.license") if os.path.exists(src_license): - base = dest_config[:-5] if dest_config.endswith(".json") else dest_config - dest_license = base + ".license" + dest_license = derive_license_path(dest_config) shutil.copy2(src_license, dest_license) build_state.log(f"[INFO] VPP: copied {p.name}.license to {dest_license}\n") else: diff --git a/webserver/vpp_license_debug.py b/webserver/vpp_license_debug.py index d6ed0dd7..874ca29f 100644 --- a/webserver/vpp_license_debug.py +++ b/webserver/vpp_license_debug.py @@ -72,12 +72,42 @@ def _read_anchor() -> bytes: return raw.rstrip(b"\x00\r\n\t ") +def derive_license_path(config_path: str) -> str: + """The ``.license`` sibling of a plugin's config_path: drop a trailing + ``.json``, append ``.license``. MUST mirror ``derive_license_path()`` in + the plugin's C source (rpi_plugin.c) exactly, or the .so reads the wrong + path and falls back to demo. + """ + base = config_path[:-5] if config_path.endswith(".json") else config_path + return base + ".license" + + +def resolve_license_path(config_path: str, runtime_root: Optional[str] = None) -> Optional[str]: + """``derive_license_path()`` plus the anti-traversal guard: never resolve + to a path outside the runtime root, even if a forged ``vpp_plugins.conf`` + carries an escaping ``config_path``. 0x49 writes 98 bytes as root, and + ``apply_vpp_plugin_conf`` copies an upload-supplied blob, so both refuse an + escaping target rather than trust the conf. Returns None when it escapes. + + A bare ``.startswith(root)`` is not enough: a sibling directory that merely + shares the root as a *string* prefix (e.g. root ``/opt/runtime`` vs. an + escaping ``/opt/runtime-evil/x``) would wrongly pass. Anchoring on + ``root + os.sep`` requires the escaping path to actually be a child of the + root directory, not just share its name as a prefix. + """ + root = os.path.abspath(runtime_root) if runtime_root else os.path.abspath(".") + path = derive_license_path(config_path) + if not os.path.abspath(path).startswith(root + os.sep): + return None + return path + + def _license_path() -> Optional[str]: """The ``.license`` sibling of the installed licensed plugin's config_path, - mirroring ``apply_vpp_plugin_conf``/derive_license_path so 0x49 and the - bundle write the SAME file the .so reads. None when no VPP plugin config is - installed yet (no upload). Multi-plugin disambiguation by vppId is a future - extension (the PDU carries no plugin id); the common case is one VPP plugin. + mirroring ``apply_vpp_plugin_conf`` so 0x49 and the bundle write the SAME + file the .so reads. None when no VPP plugin config is installed yet (no + upload). Multi-plugin disambiguation by vppId is a future extension (the + PDU carries no plugin id); the common case is one VPP plugin. """ if not os.path.exists(VPP_CONF): return None @@ -88,17 +118,7 @@ def _license_path() -> Optional[str]: candidates = [p for p in conf.plugins if getattr(p, "config_path", None)] if not candidates: return None - config_path = candidates[0].config_path - base = config_path[:-5] if config_path.endswith(".json") else config_path - path = base + ".license" - # Anti-traversal (mirror the guard apply_vpp_plugin_conf applies to the same - # config_path): never read/write outside the runtime root, even if a forged - # vpp_plugins.conf carries an escaping config_path. 0x49 writes 98 bytes as - # root, so refuse an escaping target rather than trust the conf. - runtime_root = os.path.abspath(".") - if not os.path.abspath(path).startswith(runtime_root + os.sep): - return None - return path + return resolve_license_path(candidates[0].config_path) def handle_license_command(command_hex: str) -> Optional[str]: From b48e7155914639d59b5ad45af438d3fe0f57de1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Mon, 27 Jul 2026 13:04:25 +0200 Subject: [PATCH 03/10] fix(vpp): resolve symlinks in the containment guard and match the C derivation Follow-up to ec995c2, from its code review. - os.path.abspath normalises lexically and does NOT follow symlinks, so the guard did not deliver what its docstring promised: any link out of the tree (a deploy link, a volume mounted under build/) let an innocent-looking relative config_path resolve outside the runtime root while still reporting success. Verified with a directory junction: abspath reports the target inside the root, realpath resolves it outside. Both write paths now share one is_inside_root() helper built on realpath + commonpath. commonpath also fixes a fail-closed edge: a root of "/" made the separator-anchored check compare against "//", which nothing starts with, silently refusing every license write for a process whose cwd is /. - Apply the same helper to safe_extract's guard, which still had the exact prefix bug ec995c2 fixed 170 lines below it. analyze_zip already rejects ".." before that point, so this is defence in depth -- but it is the same bug class in the same file. - derive_license_path now mirrors rpi_plugin.c exactly. The C strips the extension only when len > strlen(".json"), so a config_path of literally ".json" keeps it; endswith stripped it, and the runtime would have written a file the .so never reads. Empty in, empty out, as the C does; an empty derivation is refused rather than resolved against the cwd. - Regression tests for the guard. The existing traversal test used a sibling sharing no string prefix with the root, so the OLD buggy check rejected it too -- it could not tell the fixed guard from the broken one. The new cases pin what actually distinguishes them: the prefix sibling, the symlink escape, a not-yet-created nested path, and the "/" root. - The parity test now calls the real derive_license_path instead of a third hand-written copy of it, and covers the two inputs where C and Python had actually diverged. Comparing two transcriptions would stay green while the shipped function drifted. --- .../pytest/plugins/test_vpp_license_debug.py | 78 +++++++++++++++++++ .../plugins/test_vpp_license_delivery.py | 32 ++++++-- webserver/plcapp_management.py | 24 +++--- webserver/vpp_license_debug.py | 65 +++++++++++++--- 4 files changed, 173 insertions(+), 26 deletions(-) diff --git a/tests/pytest/plugins/test_vpp_license_debug.py b/tests/pytest/plugins/test_vpp_license_debug.py index 8f7b2638..578d3721 100644 --- a/tests/pytest/plugins/test_vpp_license_debug.py +++ b/tests/pytest/plugins/test_vpp_license_debug.py @@ -133,3 +133,81 @@ def test_write_without_installed_plugin_is_unsupported(tmp_path, monkeypatch): def test_non_license_fc_passes_through(): assert lic.handle_license_command("41 00 00 00 01") is None + + +# -------------------------------------------------------------------------- +# Containment guard (is_inside_root) +# +# The pre-existing traversal test above uses a sibling that shares NO string +# prefix with the root, so the old buggy `startswith(root)` check rejected it +# too -- it could not tell the fixed guard from the broken one. These pin the +# two cases that actually distinguish them. +# -------------------------------------------------------------------------- + + +def test_rejects_sibling_that_shares_the_root_as_a_string_prefix(tmp_path): + """`/opt/runtime-evil/x` must not pass a root of `/opt/runtime`. + + This is the exact input the bare `.startswith(root)` guard accepted. + """ + root = tmp_path / "runtime" + root.mkdir() + (tmp_path / "runtime-evil").mkdir() + escaping = str(tmp_path / "runtime-evil" / "payload.json") + + assert lic.is_inside_root(escaping, str(root)) is False + # ...and the string-prefix check it replaced would have said yes: + assert os.path.abspath(escaping).startswith(os.path.abspath(str(root))) + + +def test_rejects_a_path_that_escapes_through_a_symlink(tmp_path): + """A link out of the tree beats a lexical abspath check. + + `abspath` normalises text only; a path whose parent is a symlink pointing + outside the root resolves outside it while still *looking* contained. + """ + root = tmp_path / "runtime" + root.mkdir() + outside = tmp_path / "elsewhere" + outside.mkdir() + link = root / "build" + try: + link.symlink_to(outside, target_is_directory=True) + except (OSError, NotImplementedError): + pytest.skip("symlink creation not permitted on this host") + + target = str(link / "payload.json") + + assert lic.is_inside_root(target, str(root)) is False + # The lexical check this replaced sees a path squarely inside the root: + assert os.path.abspath(target).startswith(os.path.abspath(str(root)) + os.sep) + + +def test_accepts_a_nested_path_that_does_not_exist_yet(tmp_path): + """A `.license` is written before it exists; containment must still hold.""" + root = tmp_path / "runtime" + (root / "core").mkdir(parents=True) + + assert lic.is_inside_root(str(root / "core" / "vpp.license"), str(root)) is True + + +def test_a_filesystem_root_does_not_refuse_everything(tmp_path): + """Guards against the `"/" + os.sep == "//"` degenerate case. + + A separator-anchored prefix check refuses every path when the root is `/`, + silently breaking all license writes for a process whose cwd is `/`. + """ + root = os.path.abspath(os.sep) + + assert lic.is_inside_root(str(tmp_path / "anything.license"), root) is True + + +def test_resolve_license_path_returns_none_for_a_prefix_sibling(tmp_path): + root = tmp_path / "runtime" + root.mkdir() + (tmp_path / "runtime-evil").mkdir() + + escaping = str(tmp_path / "runtime-evil" / "plugin.json") + + assert lic.resolve_license_path(escaping, str(root)) is None + assert lic.resolve_license_path(str(root / "plugin.json"), str(root)) == str(root / "plugin.license") diff --git a/tests/pytest/plugins/test_vpp_license_delivery.py b/tests/pytest/plugins/test_vpp_license_delivery.py index efdbc01a..827bd6a2 100644 --- a/tests/pytest/plugins/test_vpp_license_delivery.py +++ b/tests/pytest/plugins/test_vpp_license_delivery.py @@ -14,18 +14,37 @@ import pytest +_lic = pytest.importorskip( + "webserver.vpp_license_debug", + reason="runtime webserver package not importable (no venv)", +) + def _plugin_license_path(config_path: str) -> str: """Mirror of derive_license_path() in the licensed rpi_plugin.c: the .so's - view of where its license lives, given its config path.""" - base = config_path[:-5] if config_path.endswith(".json") else config_path + view of where its license lives, given its config path. + + This is the ONLY hand-written copy in the test -- it stands in for the C, + which we cannot call from here, so it must track rpi_plugin.c line by line: + empty in, empty out; and the extension is stripped only when the path is + strictly LONGER than ".json" (`len > elen` in the C), so a path of exactly + ".json" keeps it. + """ + if not config_path: + return "" + ext = ".json" + base = config_path[: -len(ext)] if len(config_path) > len(ext) and config_path.endswith(ext) else config_path return base + ".license" def _runtime_license_dest(dest_config: str) -> str: - """Mirror of the delivery rule in apply_vpp_plugin_conf (kept identical).""" - base = dest_config[:-5] if dest_config.endswith(".json") else dest_config - return base + ".license" + """The runtime's real derivation -- NOT a copy. + + A second hand-written mirror here would make this a comparison of two + transcriptions: it would stay green even if the shipped function drifted, + which is precisely the drift the test exists to catch. + """ + return _lic.derive_license_path(dest_config) @pytest.mark.parametrize( @@ -35,6 +54,9 @@ def _runtime_license_dest(dest_config: str) -> str: "build/vpp/rpi_gpio.json", "rpi_gpio", # no extension "a/b.c/rpi_gpio.json", + ".json", # exactly the extension: the C's `len > elen` keeps it + "", # empty in, empty out + "rpi_gpio.JSON", # case-sensitive on both sides: kept, not stripped ], ) def test_delivery_path_matches_plugin_derivation(config_path): diff --git a/webserver/plcapp_management.py b/webserver/plcapp_management.py index 87dc8a45..b4af160e 100644 --- a/webserver/plcapp_management.py +++ b/webserver/plcapp_management.py @@ -12,7 +12,7 @@ from webserver.runtimemanager import RuntimeManager from webserver.logger import get_logger, LogParser from webserver.plugin_config_model import PluginsConfiguration, PluginConfig, PluginType -from webserver.vpp_license_debug import derive_license_path +from webserver.vpp_license_debug import derive_license_path, is_inside_root logger, _ = get_logger("runtime", use_buffer=True) @@ -145,8 +145,14 @@ def safe_extract(zip_path, dest_dir, valid_files): out_path = os.path.join(dest_dir, filename) out_path = os.path.abspath(out_path) - # Ensure extraction stays inside destination - if not out_path.startswith(os.path.abspath(dest_dir)): + # Ensure extraction stays inside destination. Same containment rule + # as the VPP config copy below: a bare prefix check accepts a + # sibling sharing dest_dir as a string prefix (dest_dir + # "core/generated" vs. an entry resolving to "core/generatedX/..."), + # and it ignores symlinks entirely. analyze_zip() already rejects + # entries containing ".." before we get here, so this is defence in + # depth -- but it is the same bug class, so it gets the same fix. + if not is_inside_root(out_path, dest_dir): # logger.warning("Skipping suspicious path: %s", filename) continue @@ -311,12 +317,12 @@ def apply_vpp_plugin_conf(generated_dir: str = "core/generated") -> None: continue dest_config = os.path.normpath(p.config_path) # Guard against path traversal in editor-generated vpp_plugins.conf. - # `+ os.sep` matters: a bare prefix check would wrongly accept a - # sibling directory that merely shares runtime_root as a string - # prefix (e.g. runtime_root "/opt/runtime" vs. an escaping - # "/opt/runtime-evil/x") -- anchoring on the separator requires the - # escaping path to actually be a child of the runtime directory. - if not os.path.abspath(dest_config).startswith(runtime_root + os.sep): + # Shares one containment definition with the 0x49 write path (see + # is_inside_root): rejects a sibling that merely shares runtime_root + # as a string prefix, AND resolves symlinks, which a lexical + # abspath check does not -- a link out of the tree would otherwise + # let an innocent-looking relative path write outside the root. + if not is_inside_root(dest_config, runtime_root): build_state.log(f"[WARNING] VPP: config_path '{p.config_path}' escapes runtime root, skipping\n") continue os.makedirs(os.path.dirname(dest_config), exist_ok=True) diff --git a/webserver/vpp_license_debug.py b/webserver/vpp_license_debug.py index 874ca29f..5fb44d3d 100644 --- a/webserver/vpp_license_debug.py +++ b/webserver/vpp_license_debug.py @@ -77,27 +77,68 @@ def derive_license_path(config_path: str) -> str: ``.json``, append ``.license``. MUST mirror ``derive_license_path()`` in the plugin's C source (rpi_plugin.c) exactly, or the .so reads the wrong path and falls back to demo. + + Two details are copied from the C rather than written the idiomatic way, + because "exactly" is the whole contract: + + * The C strips the extension only when ``len > strlen(".json")``, so a + config_path of literally ``".json"`` keeps it and becomes + ``".json.license"``. A plain ``endswith`` would strip it and yield + ``".license"`` -- the runtime would write one file and the .so read + another. + * The C leaves ``out`` empty for a NULL/empty config_path; an empty string + in, an empty string out. """ - base = config_path[:-5] if config_path.endswith(".json") else config_path + if not config_path: + return "" + ext = ".json" + base = config_path[: -len(ext)] if len(config_path) > len(ext) and config_path.endswith(ext) else config_path return base + ".license" +def is_inside_root(path: str, runtime_root: Optional[str] = None) -> bool: + """True when ``path`` really resolves to a location under the runtime root. + + Shared by every write path that trusts an editor-supplied ``config_path`` + (0x49 writes 98 bytes as root; ``apply_vpp_plugin_conf`` copies an + upload-supplied blob), so all of them agree on one definition of + "contained". + + Two traps this avoids: + + * A bare ``.startswith(root)`` accepts a sibling that merely shares the + root as a *string* prefix -- root ``/opt/runtime`` vs. an escaping + ``/opt/runtime-evil/x``. + * ``abspath`` normalises lexically only, so it does NOT follow symlinks. If + any component inside the root is a link out (a deploy link, a data volume + mounted under ``build/``), a perfectly innocent-looking relative path + resolves outside the root and the guard still reports success. + ``realpath`` resolves the links; on a path that does not exist yet it + resolves the existing prefix and appends the remainder, which is exactly + what a not-yet-written ``.license`` needs. + + ``commonpath`` is used instead of a separator-anchored prefix so that a + root of ``/`` (``abspath`` yields ``"/"``, and ``"/" + os.sep`` is ``"//"``, + which nothing starts with) does not silently refuse every write. + """ + root = os.path.realpath(runtime_root) if runtime_root else os.path.realpath(".") + try: + return os.path.commonpath([root, os.path.realpath(path)]) == root + except ValueError: + # Raised for paths that share no root at all (different drives on + # Windows, or a mix of absolute and relative that cannot be compared). + return False + + def resolve_license_path(config_path: str, runtime_root: Optional[str] = None) -> Optional[str]: """``derive_license_path()`` plus the anti-traversal guard: never resolve to a path outside the runtime root, even if a forged ``vpp_plugins.conf`` - carries an escaping ``config_path``. 0x49 writes 98 bytes as root, and - ``apply_vpp_plugin_conf`` copies an upload-supplied blob, so both refuse an - escaping target rather than trust the conf. Returns None when it escapes. - - A bare ``.startswith(root)`` is not enough: a sibling directory that merely - shares the root as a *string* prefix (e.g. root ``/opt/runtime`` vs. an - escaping ``/opt/runtime-evil/x``) would wrongly pass. Anchoring on - ``root + os.sep`` requires the escaping path to actually be a child of the - root directory, not just share its name as a prefix. + carries an escaping ``config_path``. Returns None when it escapes. """ - root = os.path.abspath(runtime_root) if runtime_root else os.path.abspath(".") path = derive_license_path(config_path) - if not os.path.abspath(path).startswith(root + os.sep): + # An empty derivation (empty config_path) would resolve to the cwd, which + # IS inside the root -- refuse it rather than let it through as a target. + if not path or not is_inside_root(path, runtime_root): return None return path From 2da6c26b2779ae448505d3adfd17f23dd7639d4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Tue, 28 Jul 2026 13:50:42 +0200 Subject: [PATCH 04/10] feat(vpp): verify the package signature before compiling an uploaded plugin A licensed VPP ships the closed enforcement objects (license_core.o, license_gate.o) and a link-only Makefile inside the user's upload, and scripts/compile.sh runs that Makefile as root. Nothing in that path proved where those bytes came from: adding a three-line license_gate.c that always answers "licensed" and letting the uploaded Makefile rebuild license_gate.o from it was enough to run a licensed VPP in full mode. Measured, not assumed -- against the pre-change code the stub compiles and links, and compile.sh reports success. openplc-packages already signs every .vpp with Ed25519 and emits a signature.json holding a sha256 per packaged file; the editor already carries the matching public key. The machinery existed and simply was not wired end to end. This wires it: - webserver/vpp_package_signature.py verifies the Ed25519 signature over the WHOLE canonical payload, then compares hashes only for the files that actually travelled. A filtered slice is not what was signed and could never verify. Canonicalization mirrors openplc-packages/scripts/lib/package-signing.ts byte for byte; the risk that it does not is pinned by a known-answer vector copied out of a real signed .vpp, which no self-signed fixture could catch. - webserver/app.py gates the upload between safe_extract and apply_vpp_plugin_conf. The order is the point: a refused plugin never gets a vpp_plugins.conf installed, never gets its config or license blob copied into the runtime root, and never reaches make. - scripts/compile.sh refuses to run the uploaded Makefile unless a verification seal is present AND its tree digest still matches the tree on disk. That covers a direct invocation of the script and the window between the gate and make. checksum.sha256 is documented as the recompilation cache it always was; it is self-attestation and must never be read as integrity again. - core/src/drivers/vpp_plugin_seal.c re-checks the .so's sha256 immediately before dlopen. The upload gate can only speak for the plugin's inputs -- the object is linked on the device after the package was signed -- so without this an object dropped into build/vpp/ after the compile would load with no provenance at all. - Containment for the path that actually gets dlopen'ed. vpp_plugins.conf arrives verbatim from the upload and only config_path was ever validated, so a forged conf could name any .so on the box. plcapp_management.py now requires every VPP plugin path to resolve inside build/vpp/, and plugin_config.c rejects absolute and ".." paths in the upload-supplied config so containment does not rest on Python alone. Policy for unsigned uploads lives in exactly one function, vpp_package_signature.signature_required(): an upload carrying vpp_plugin/ must be signed, a plain PLC program is untouched. Existing users and editors built before the sidecar existed keep working. Scope, written down so it is not rediscovered: this closes the licensing bypass, not the RCE. Arbitrary native code as root via core/generated/*.cpp is untouched and is a privilege question, not a signature one. The verifier also lives inside a runtime the owner can recompile, so this raises the cost of the cheap attack rather than forming a cryptographic barrier. Refs proposal #38 (option C plus the path containment of 1.7), security-audit-2026-07-28.md finding 1. --- core/src/CMakeLists.txt | 1 + core/src/drivers/plugin_config.c | 82 +++- core/src/drivers/plugin_config.h | 19 + core/src/drivers/plugin_driver.c | 23 +- core/src/drivers/vpp_plugin_seal.c | 318 ++++++++++++ core/src/drivers/vpp_plugin_seal.h | 59 +++ requirements.txt | 6 + scripts/compile.sh | 117 ++++- .../plugins/test_vpp_license_delivery.py | 9 +- .../plugins/test_vpp_plugin_signature.py | 460 ++++++++++++++++++ webserver/app.py | 28 ++ webserver/plcapp_management.py | 66 ++- webserver/vpp_package_signature.py | 427 ++++++++++++++++ 13 files changed, 1607 insertions(+), 8 deletions(-) create mode 100644 core/src/drivers/vpp_plugin_seal.c create mode 100644 core/src/drivers/vpp_plugin_seal.h create mode 100644 tests/pytest/plugins/test_vpp_plugin_signature.py create mode 100644 webserver/vpp_package_signature.py diff --git a/core/src/CMakeLists.txt b/core/src/CMakeLists.txt index 70f5c286..8093dab0 100644 --- a/core/src/CMakeLists.txt +++ b/core/src/CMakeLists.txt @@ -50,6 +50,7 @@ add_executable(plc_main ${CMAKE_SOURCE_DIR}/core/src/plc_app/scan_cycle_manager.c ${CMAKE_SOURCE_DIR}/core/src/drivers/plugin_driver.c ${CMAKE_SOURCE_DIR}/core/src/drivers/plugin_config.c + ${CMAKE_SOURCE_DIR}/core/src/drivers/vpp_plugin_seal.c ${CMAKE_SOURCE_DIR}/core/src/plc_app/unix_socket.c ${CMAKE_SOURCE_DIR}/core/src/plc_app/debug_handler.c ${CMAKE_SOURCE_DIR}/core/src/plc_app/client_tcp_udp.c diff --git a/core/src/drivers/plugin_config.c b/core/src/drivers/plugin_config.c index d115e7be..6b75daa8 100644 --- a/core/src/drivers/plugin_config.c +++ b/core/src/drivers/plugin_config.c @@ -21,7 +21,65 @@ static void remove_newline(char *str) } } -int parse_plugin_config(const char *config_file, plugin_config_t *configs, int max_configs) +/** + * Reject a plugin path that could point outside the runtime tree. + * + * The `path` field of a plugin config is handed straight to dlopen (VPP) or + * used as a Python module location, so a config the runtime did not write is + * arbitrary-code selection. vpp_plugins.conf IS such a config: it arrives + * verbatim inside the user's upload. The Python side contains it too + * (webserver/plcapp_management.py validate_vpp_plugins_conf); this check is + * here so containment does not depend on one language alone. + * + * @param require_contained 0 to only reject `..` traversal, 1 to also reject + * absolute paths. Config files the runtime itself owns (plugins.conf) + * pass 0: an operator with a hand-written absolute path there is not + * the threat, and refusing it would break working installations. Only + * the upload-supplied config is parsed with 1. + * @return 1 when the path is acceptable, 0 when it must be rejected. + */ +static int plugin_path_is_acceptable(const char *path, int require_contained) +{ + if (!path || path[0] == '\0') + { + return 0; + } + + /* Any ".." component escapes, in every config, with no legitimate use. */ + const char *cursor = path; + while (*cursor != '\0') + { + if (cursor[0] == '.' && cursor[1] == '.' && + (cursor[2] == '\0' || cursor[2] == '/' || cursor[2] == '\\')) + { + /* Only a ".." that starts a component counts, so a file legally + * named "libfoo..so" is not rejected. */ + if (cursor == path || cursor[-1] == '/' || cursor[-1] == '\\') + { + return 0; + } + } + cursor++; + } + + if (require_contained) + { + if (path[0] == '/' || path[0] == '\\') + { + return 0; + } + /* Windows-style drive prefix ("C:\..."), reachable on the Cygwin build. */ + if (path[1] == ':') + { + return 0; + } + } + + return 1; +} + +static int parse_plugin_config_internal(const char *config_file, plugin_config_t *configs, + int max_configs, int require_contained) { FILE *file = fopen(config_file, "r"); if (!file) @@ -57,6 +115,17 @@ int parse_plugin_config(const char *config_file, plugin_config_t *configs, int m configs[config_count].path[sizeof(configs[config_count].path) - 1] = '\0'; remove_newline(configs[config_count].path); + /* Containment: drop the whole entry rather than load from a path that + * escapes the runtime tree. Skipping the entry (instead of aborting the + * parse) keeps one bad line from disabling every other plugin. */ + if (!plugin_path_is_acceptable(configs[config_count].path, require_contained)) + { + log_error("[PLUGIN] rejected plugin '%s' from %s: path '%s' is not contained in the " + "runtime tree", + configs[config_count].name, config_file, configs[config_count].path); + continue; + } + // Parsing enabled token = strtok(NULL, ","); if (!token) @@ -110,3 +179,14 @@ int parse_plugin_config(const char *config_file, plugin_config_t *configs, int m fclose(file); return config_count; } + +int parse_plugin_config(const char *config_file, plugin_config_t *configs, int max_configs) +{ + return parse_plugin_config_internal(config_file, configs, max_configs, 0); +} + +int parse_plugin_config_contained(const char *config_file, plugin_config_t *configs, + int max_configs) +{ + return parse_plugin_config_internal(config_file, configs, max_configs, 1); +} diff --git a/core/src/drivers/plugin_config.h b/core/src/drivers/plugin_config.h index bb134377..a95fb23a 100644 --- a/core/src/drivers/plugin_config.h +++ b/core/src/drivers/plugin_config.h @@ -14,6 +14,25 @@ typedef struct char venv_path[MAX_PLUGIN_PATH_LEN]; // Path to virtual environment } plugin_config_t; +/** + * Parse a plugin config the runtime owns (plugins.conf). + * + * Entries whose `path` contains a ".." component are rejected and skipped; + * absolute paths are allowed, because an operator may legitimately point a + * hand-written plugins.conf at one. + */ int parse_plugin_config(const char *config_file, plugin_config_t *configs, int max_configs); +/** + * Parse a plugin config that came from an upload (vpp_plugins.conf). + * + * As above, plus absolute (and Windows drive-prefixed) paths are rejected: the + * `path` field of this file is chosen by whoever produced the upload and is fed + * to dlopen, so it must stay inside the runtime tree. Mirrors the Python-side + * containment in webserver/plcapp_management.py so neither side is the only + * thing standing between an upload and dlopen. + */ +int parse_plugin_config_contained(const char *config_file, plugin_config_t *configs, + int max_configs); + #endif // PLUGIN_CONFIG_H diff --git a/core/src/drivers/plugin_driver.c b/core/src/drivers/plugin_driver.c index 8cac385a..a1e7e15f 100644 --- a/core/src/drivers/plugin_driver.c +++ b/core/src/drivers/plugin_driver.c @@ -23,6 +23,7 @@ #include "../plc_app/utils/utils.h" #include "plugin_config.h" #include "plugin_driver.h" +#include "vpp_plugin_seal.h" #include #include #include @@ -523,7 +524,10 @@ int plugin_driver_append_config(plugin_driver_t *driver, const char *config_file } plugin_config_t configs[MAX_PLUGINS]; - int config_count = parse_plugin_config(config_file, configs, MAX_PLUGINS); + /* This config file comes from the user's upload (the editor writes it and + * webserver/plcapp_management.py copies it verbatim), so its paths are + * parsed under containment: no "..", no absolute paths. */ + int config_count = parse_plugin_config_contained(config_file, configs, MAX_PLUGINS); if (config_count < 0) { return -1; @@ -1341,6 +1345,23 @@ int native_plugin_get_symbols(plugin_instance_t *plugin) return -1; } + /* Last metre before execution: a VPP plugin .so must match the hash + * scripts/compile.sh sealed after building it from a signature-verified + * upload. The upload gate proved the plugin's INPUTS came from a signed + * package; it cannot speak for the .so, which is linked here afterwards. + * Without this check an object dropped into build/vpp/ after the compile + * would be dlopen'ed with no provenance at all. + * + * Built-in plugins from plugins.conf are produced by the runtime's own + * CMake build and are not sealed -- vpp_plugin_seal_required() scopes the + * check to objects that resolve inside build/vpp/. */ + if (vpp_plugin_seal_required(plugin->config.path) && + vpp_plugin_seal_verify(plugin->config.path) != 0) + { + free(native_bundle); + return -1; + } + // Load the shared library void *handle = dlopen(plugin->config.path, RTLD_LOCAL | RTLD_NOW); if (!handle) diff --git a/core/src/drivers/vpp_plugin_seal.c b/core/src/drivers/vpp_plugin_seal.c new file mode 100644 index 00000000..3cc286b1 --- /dev/null +++ b/core/src/drivers/vpp_plugin_seal.c @@ -0,0 +1,318 @@ +#include "vpp_plugin_seal.h" +#include "../plc_app/utils/log.h" + +#include +#include +#include +#include +#include + +#ifndef PATH_MAX +#define PATH_MAX 4096 +#endif + +#define VPP_BUILD_SUBDIR "build/vpp" +#define VPP_SEAL_FILE VPP_BUILD_SUBDIR "/vpp_plugin.seal" +#define SHA256_HEX_LEN 64 +#define SHA256_BLOCK_LEN 64 + +/* ------------------------------------------------------------------------- */ +/* SHA-256 (FIPS 180-4). */ +/* */ +/* Written out here because the runtime links no crypto library: CMakeLists */ +/* has no OpenSSL, and pulling one in for a 32-byte digest would add a build */ +/* dependency to every target we ship to. This is the standard construction */ +/* with no shortcuts; it hashes a file in 64-byte blocks so a multi-megabyte */ +/* .so does not have to be resident. */ +/* ------------------------------------------------------------------------- */ + +typedef struct +{ + uint32_t state[8]; + uint64_t bit_len; + uint8_t buffer[SHA256_BLOCK_LEN]; + size_t buffer_len; +} sha256_ctx_t; + +static const uint32_t SHA256_K[64] = { + 0x428a2f98u, 0x71374491u, 0xb5c0fbcfu, 0xe9b5dba5u, 0x3956c25bu, 0x59f111f1u, 0x923f82a4u, + 0xab1c5ed5u, 0xd807aa98u, 0x12835b01u, 0x243185beu, 0x550c7dc3u, 0x72be5d74u, 0x80deb1feu, + 0x9bdc06a7u, 0xc19bf174u, 0xe49b69c1u, 0xefbe4786u, 0x0fc19dc6u, 0x240ca1ccu, 0x2de92c6fu, + 0x4a7484aau, 0x5cb0a9dcu, 0x76f988dau, 0x983e5152u, 0xa831c66du, 0xb00327c8u, 0xbf597fc7u, + 0xc6e00bf3u, 0xd5a79147u, 0x06ca6351u, 0x14292967u, 0x27b70a85u, 0x2e1b2138u, 0x4d2c6dfcu, + 0x53380d13u, 0x650a7354u, 0x766a0abbu, 0x81c2c92eu, 0x92722c85u, 0xa2bfe8a1u, 0xa81a664bu, + 0xc24b8b70u, 0xc76c51a3u, 0xd192e819u, 0xd6990624u, 0xf40e3585u, 0x106aa070u, 0x19a4c116u, + 0x1e376c08u, 0x2748774cu, 0x34b0bcb5u, 0x391c0cb3u, 0x4ed8aa4au, 0x5b9cca4fu, 0x682e6ff3u, + 0x748f82eeu, 0x78a5636fu, 0x84c87814u, 0x8cc70208u, 0x90befffau, 0xa4506cebu, 0xbef9a3f7u, + 0xc67178f2u}; + +static uint32_t rotr32(uint32_t value, unsigned int bits) +{ + return (value >> bits) | (value << (32u - bits)); +} + +static void sha256_init(sha256_ctx_t *ctx) +{ + ctx->state[0] = 0x6a09e667u; + ctx->state[1] = 0xbb67ae85u; + ctx->state[2] = 0x3c6ef372u; + ctx->state[3] = 0xa54ff53au; + ctx->state[4] = 0x510e527fu; + ctx->state[5] = 0x9b05688cu; + ctx->state[6] = 0x1f83d9abu; + ctx->state[7] = 0x5be0cd19u; + ctx->bit_len = 0; + ctx->buffer_len = 0; +} + +static void sha256_compress(sha256_ctx_t *ctx, const uint8_t *block) +{ + uint32_t w[64]; + for (int i = 0; i < 16; i++) + { + w[i] = ((uint32_t)block[i * 4] << 24) | ((uint32_t)block[i * 4 + 1] << 16) | + ((uint32_t)block[i * 4 + 2] << 8) | (uint32_t)block[i * 4 + 3]; + } + for (int i = 16; i < 64; i++) + { + uint32_t s0 = rotr32(w[i - 15], 7) ^ rotr32(w[i - 15], 18) ^ (w[i - 15] >> 3); + uint32_t s1 = rotr32(w[i - 2], 17) ^ rotr32(w[i - 2], 19) ^ (w[i - 2] >> 10); + w[i] = w[i - 16] + s0 + w[i - 7] + s1; + } + + uint32_t a = ctx->state[0]; + uint32_t b = ctx->state[1]; + uint32_t c = ctx->state[2]; + uint32_t d = ctx->state[3]; + uint32_t e = ctx->state[4]; + uint32_t f = ctx->state[5]; + uint32_t g = ctx->state[6]; + uint32_t h = ctx->state[7]; + + for (int i = 0; i < 64; i++) + { + uint32_t s1 = rotr32(e, 6) ^ rotr32(e, 11) ^ rotr32(e, 25); + uint32_t ch = (e & f) ^ ((~e) & g); + uint32_t temp1 = h + s1 + ch + SHA256_K[i] + w[i]; + uint32_t s0 = rotr32(a, 2) ^ rotr32(a, 13) ^ rotr32(a, 22); + uint32_t maj = (a & b) ^ (a & c) ^ (b & c); + uint32_t temp2 = s0 + maj; + + h = g; + g = f; + f = e; + e = d + temp1; + d = c; + c = b; + b = a; + a = temp1 + temp2; + } + + ctx->state[0] += a; + ctx->state[1] += b; + ctx->state[2] += c; + ctx->state[3] += d; + ctx->state[4] += e; + ctx->state[5] += f; + ctx->state[6] += g; + ctx->state[7] += h; +} + +static void sha256_update(sha256_ctx_t *ctx, const uint8_t *data, size_t len) +{ + for (size_t i = 0; i < len; i++) + { + ctx->buffer[ctx->buffer_len++] = data[i]; + if (ctx->buffer_len == SHA256_BLOCK_LEN) + { + sha256_compress(ctx, ctx->buffer); + ctx->bit_len += 512; + ctx->buffer_len = 0; + } + } +} + +static void sha256_final_hex(sha256_ctx_t *ctx, char *out_hex) +{ + size_t i = ctx->buffer_len; + ctx->bit_len += (uint64_t)ctx->buffer_len * 8u; + + ctx->buffer[i++] = 0x80; + if (i > 56) + { + while (i < SHA256_BLOCK_LEN) + { + ctx->buffer[i++] = 0x00; + } + sha256_compress(ctx, ctx->buffer); + i = 0; + } + while (i < 56) + { + ctx->buffer[i++] = 0x00; + } + for (int b = 7; b >= 0; b--) + { + ctx->buffer[i++] = (uint8_t)((ctx->bit_len >> (b * 8)) & 0xffu); + } + sha256_compress(ctx, ctx->buffer); + + static const char hex[] = "0123456789abcdef"; + for (int w = 0; w < 8; w++) + { + for (int b = 3; b >= 0; b--) + { + uint8_t byte = (uint8_t)((ctx->state[w] >> (b * 8)) & 0xffu); + *out_hex++ = hex[byte >> 4]; + *out_hex++ = hex[byte & 0x0fu]; + } + } + *out_hex = '\0'; +} + +int vpp_plugin_seal_sha256_file(const char *path, char *out_hex) +{ + if (!path || !out_hex) + { + return -1; + } + + FILE *file = fopen(path, "rb"); + if (!file) + { + return -1; + } + + sha256_ctx_t ctx; + sha256_init(&ctx); + + uint8_t chunk[4096]; + size_t read_len; + while ((read_len = fread(chunk, 1, sizeof(chunk), file)) > 0) + { + sha256_update(&ctx, chunk, read_len); + } + + int failed = ferror(file) ? -1 : 0; + fclose(file); + if (failed != 0) + { + return -1; + } + + sha256_final_hex(&ctx, out_hex); + return 0; +} + +/* ------------------------------------------------------------------------- */ +/* Seal lookup */ +/* ------------------------------------------------------------------------- */ + +/** Basename of @p path, i.e. everything after the last '/'. Never NULL. */ +static const char *path_basename(const char *path) +{ + const char *slash = strrchr(path, '/'); + return slash ? slash + 1 : path; +} + +int vpp_plugin_seal_required(const char *path) +{ + if (!path || path[0] == '\0') + { + return 0; + } + + /* Compare RESOLVED paths, not the literal string: the config may say + * ./build/vpp/libx.so, build/vpp/libx.so, or an absolute path, and a + * symlink anywhere in the chain would defeat a textual match. When the + * file does not exist yet realpath fails -- fall back to a textual test so + * a missing object is still treated as VPP (and therefore still refused + * below) instead of being waved through as a built-in. */ + char resolved_path[PATH_MAX]; + char resolved_vpp[PATH_MAX]; + if (realpath(path, resolved_path) && realpath(VPP_BUILD_SUBDIR, resolved_vpp)) + { + size_t len = strlen(resolved_vpp); + if (strncmp(resolved_path, resolved_vpp, len) == 0 && + (resolved_path[len] == '/' || resolved_path[len] == '\0')) + { + return 1; + } + return 0; + } + + return strstr(path, VPP_BUILD_SUBDIR "/") != NULL ? 1 : 0; +} + +int vpp_plugin_seal_verify(const char *path) +{ + if (!path || path[0] == '\0') + { + return -1; + } + + char actual_hex[SHA256_HEX_LEN + 1]; + if (vpp_plugin_seal_sha256_file(path, actual_hex) != 0) + { + log_error("[PLUGIN] cannot hash VPP plugin '%s' for its integrity seal", path); + return -1; + } + + FILE *seal = fopen(VPP_SEAL_FILE, "r"); + if (!seal) + { + log_error("[PLUGIN] no VPP plugin seal at %s -- refusing to load '%s'. The plugin was " + "not produced by a verified upload; re-upload the program from the editor.", + VPP_SEAL_FILE, path); + return -1; + } + + const char *wanted_name = path_basename(path); + char line[512]; + int found = 0; + int matched = 0; + + /* Seal lines are "<64 hex> ", written by scripts/compile.sh. */ + while (fgets(line, sizeof(line), seal)) + { + if (line[0] == '#' || line[0] == '\n' || line[0] == '\r') + { + continue; + } + + char sealed_hex[SHA256_HEX_LEN + 1]; + char sealed_name[256]; + if (sscanf(line, "%64s %255s", sealed_hex, sealed_name) != 2) + { + continue; + } + if (strcmp(sealed_name, wanted_name) != 0) + { + continue; + } + + found = 1; + matched = (strncmp(sealed_hex, actual_hex, SHA256_HEX_LEN) == 0); + break; + } + + fclose(seal); + + if (!found) + { + log_error("[PLUGIN] VPP plugin '%s' has no entry in %s -- refusing to load it. Only " + "objects built from a signature-verified upload are sealed.", + path, VPP_SEAL_FILE); + return -1; + } + + if (!matched) + { + log_error("[PLUGIN] VPP plugin '%s' does not match its sealed hash -- refusing to load " + "it. The .so changed after the verified build.", + path); + return -1; + } + + return 0; +} diff --git a/core/src/drivers/vpp_plugin_seal.h b/core/src/drivers/vpp_plugin_seal.h new file mode 100644 index 00000000..bca655b9 --- /dev/null +++ b/core/src/drivers/vpp_plugin_seal.h @@ -0,0 +1,59 @@ +/** + * @file vpp_plugin_seal.h + * @brief Last-metre integrity check for VPP plugin shared objects. + * + * The trust anchor for a VPP plugin is the package's Ed25519 signature, which + * the webserver verifies on the upload path before anything is compiled + * (webserver/vpp_package_signature.py). That covers the INPUTS: the prebuilt + * vendor objects and the link-only Makefile. + * + * It cannot cover the OUTPUT. The .so is linked on the device, after the + * signature was made, so its bytes are unknown to whoever signed. Instead + * scripts/compile.sh records the sha256 of every .so it produced from a + * verified tree into build/vpp/vpp_plugin.seal, and this module re-checks that + * hash immediately before dlopen. Without it, an object swapped into + * build/vpp/ AFTER the compile would load unchecked, and verifying the upload + * would have proved nothing about the code actually executed. + * + * Scope, deliberately narrow: only objects that resolve inside build/vpp/ are + * sealed. Built-in plugins listed in plugins.conf are produced by the + * runtime's own CMake build, are not user-supplied, and are left alone. + * + * Not a defence against someone with root and a text editor -- the seal is + * unkeyed, and a runtime the user recompiles can have this call removed. It + * raises the cost of the cheap attack (drop a .so into build/vpp/) to that of + * rebuilding the runtime. + */ + +#ifndef VPP_PLUGIN_SEAL_H +#define VPP_PLUGIN_SEAL_H + +/** + * @brief Whether @p path must carry a seal (i.e. resolves inside build/vpp/). + * + * @param path Plugin path exactly as it appears in the plugin config. + * @return 1 when the path is a VPP build artefact, 0 otherwise. + */ +int vpp_plugin_seal_required(const char *path); + +/** + * @brief Verify @p path against build/vpp/vpp_plugin.seal. + * + * Fails closed: a missing seal file, a missing entry for this object, an + * unreadable object, or a hash mismatch all return non-zero. + * + * @param path Plugin path as it appears in the plugin config. + * @return 0 when the object matches its sealed hash, non-zero otherwise. + */ +int vpp_plugin_seal_verify(const char *path); + +/** + * @brief sha256 of a file, written as 64 lower-case hex chars plus NUL. + * + * @param path File to hash. + * @param out_hex Buffer of at least 65 bytes. + * @return 0 on success, non-zero when the file cannot be read. + */ +int vpp_plugin_seal_sha256_file(const char *path, char *out_hex); + +#endif /* VPP_PLUGIN_SEAL_H */ diff --git a/requirements.txt b/requirements.txt index aae4fd93..0f3826cd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,6 +5,12 @@ flask_sqlalchemy Flask-SocketIO PyJWT python-dotenv +# Ed25519 verification of the VPP package signature on the upload path +# (webserver/vpp_package_signature.py). Imported lazily and only while handling +# an upload, so a runtime that has not re-run install.sh still boots and still +# accepts plain PLC programs -- only VPP uploads are refused, with a message +# naming this package. +cryptography pytest pytest-flask pre-commit diff --git a/scripts/compile.sh b/scripts/compile.sh index 90609b74..46ce520f 100755 --- a/scripts/compile.sh +++ b/scripts/compile.sh @@ -109,12 +109,20 @@ make -j"$JOBS" -f scripts/Makefile.strucpp # into BUILD_PATH (next to new_libplc.so) so the runtime's plugin # loader picks it up under the same lookup rules as built-ins. # -# Checksum cache: skip recompilation when the source hasn't changed -# AND a previous build's .so is still present. Saves ~20-60 s per -# upload on the slowest targets. +# checksum.sha256 is a RECOMPILATION CACHE KEY, NOT AN INTEGRITY CHECK. +# It is written by the editor over the files the editor itself just copied, +# travels inside the same upload as those files, and is only ever compared +# against a copy of itself saved by a previous build. It proves nothing about +# where the plugin came from and must never be read as if it did. +# +# What DOES gate this block is the verification seal below: the webserver +# verified the VPP package's Ed25519 signature over these exact bytes before +# calling this script (webserver/vpp_package_signature.py). Without a seal +# that still matches the tree on disk, no uploaded Makefile runs. # ----------------------------------------------------------------------- VPP_PLUGIN_DIR="$GENERATED_DIR/vpp_plugin" VPP_CHECKSUM_FILE="$VPP_PLUGIN_DIR/checksum.sha256" +VPP_VERIFIED_SEAL="$GENERATED_DIR/vpp_plugin.verified" # VPP outputs land in a dedicated subdir of BUILD_PATH so the cleanup # glob below can scope itself to VPP-only artefacts. If a future built-in # plugin ships as a .so dropped into BUILD_PATH directly, the old @@ -122,8 +130,90 @@ VPP_CHECKSUM_FILE="$VPP_PLUGIN_DIR/checksum.sha256" # without a vpp_plugin subtree present. VPP_OUTPUT_DIR="$BUILD_PATH/vpp" VPP_CACHED_CHECKSUM="$VPP_OUTPUT_DIR/checksum.sha256" +# Seal the loader checks before dlopen (core/src/drivers/vpp_plugin_seal.c). +VPP_OBJECT_SEAL="$VPP_OUTPUT_DIR/vpp_plugin.seal" + +# sha256 of a file, hex only. sha256sum (coreutils) on Linux targets, shasum on +# hosts that ship the Perl tool instead. No tool means no verification, and a +# security gate that cannot run must not be silently skipped -- so the caller +# fails the build instead. +sha256_hex() { + if command -v sha256sum > /dev/null 2>&1; then + sha256sum "$1" | cut -d' ' -f1 + elif command -v shasum > /dev/null 2>&1; then + shasum -a 256 "$1" | cut -d' ' -f1 + else + return 1 + fi +} + +# Digest over a whole directory tree: sha256 of " \n" lines +# for every regular file, sorted by path. Must stay byte-identical to +# webserver/vpp_package_signature.py's tree_digest(), which is what produced +# the value in the seal -- hence LC_ALL=C for byte-order sort, POSIX relative +# paths, and exactly two spaces. +vpp_tree_digest() { + local dir="$1" f inner + ( + cd "$dir" || exit 1 + find . -type f -print | sed 's|^\./||' | LC_ALL=C sort | while IFS= read -r f; do + inner=$(sha256_hex "$f") || exit 1 + printf '%s %s\n' "$inner" "$f" + done + ) | { if command -v sha256sum > /dev/null 2>&1; then sha256sum; else shasum -a 256; fi; } | cut -d' ' -f1 +} + +# Refuse to build an unverified plugin tree. +# +# The webserver already verified the package signature before calling this +# script, and this seal is how that verdict reaches us. It exists for the case +# where compile.sh is invoked directly (by hand, by a systemd unit, by a future +# caller that forgets the gate) and for the window between the gate and `make`: +# the digest is recomputed here, so a file swapped in after verification does +# not build either. +# +# The seal is unkeyed and therefore forgeable by anyone who can already write +# into core/generated/ -- but that is a shell, not the upload endpoint this +# defends. It is an interlock, not a trust anchor. +check_vpp_verification_seal() { + if [ ! -f "$VPP_VERIFIED_SEAL" ]; then + echo "[ERROR] Refusing to build the uploaded VPP plugin: no verification seal at" >&2 + echo " $VPP_VERIFIED_SEAL." >&2 + echo " The plugin tree in $VPP_PLUGIN_DIR was never checked against a signed" >&2 + echo " VPP package. Upload the program through the runtime's /api/upload-file" >&2 + echo " endpoint, which verifies the package signature first." >&2 + return 1 + fi + + local sealed actual + sealed=$(awk '/^treeDigest /{print $2; exit}' "$VPP_VERIFIED_SEAL") + if [ -z "$sealed" ]; then + echo "[ERROR] Verification seal $VPP_VERIFIED_SEAL has no treeDigest." >&2 + return 1 + fi + + if ! actual=$(vpp_tree_digest "$VPP_PLUGIN_DIR") || [ -z "$actual" ]; then + echo "[ERROR] Cannot compute the VPP plugin digest (no sha256sum/shasum on PATH)." >&2 + echo " Install coreutils; the plugin integrity check cannot be skipped." >&2 + return 1 + fi + + if [ "$sealed" != "$actual" ]; then + echo "[ERROR] The VPP plugin tree changed after it was verified." >&2 + echo " sealed: $sealed" >&2 + echo " on disk: $actual" >&2 + return 1 + fi + + echo "[INFO] VPP plugin verified against the signed package (digest ${actual:0:12}...)" + return 0 +} if [ -d "$VPP_PLUGIN_DIR" ] && [ -f "$VPP_PLUGIN_DIR/Makefile" ]; then + # Before mkdir, before the cache check, before make: nothing from the + # upload is acted on until the seal matches. + check_vpp_verification_seal || exit 3 + NEEDS_COMPILE=1 mkdir -p "$VPP_OUTPUT_DIR" @@ -144,11 +234,32 @@ if [ -d "$VPP_PLUGIN_DIR" ] && [ -f "$VPP_PLUGIN_DIR/Makefile" ]; then OUTPUT_DIR="$(pwd)/$VPP_OUTPUT_DIR" \ RUNTIME_ROOT="$(pwd)" + # Save the uploader's checksum file as the cache key for the next + # upload. Cache only -- see the header comment above: this is not, and + # never was, an integrity record. if [ -f "$VPP_CHECKSUM_FILE" ]; then cp "$VPP_CHECKSUM_FILE" "$VPP_CACHED_CHECKSUM" fi echo "[INFO] VPP plugin compiled successfully" fi + + # Record the sha256 of every .so this verified build produced, so the + # plugin loader can refuse an object swapped in AFTER the compile + # (core/src/drivers/vpp_plugin_seal.c, checked immediately before dlopen). + # Written on the cache-hit path too: the cached .so belongs to this same + # verified tree, and a runtime upgraded onto an existing build/vpp/ would + # otherwise have no seal at all and refuse to load a legitimate plugin. + : > "$VPP_OBJECT_SEAL" + for so in "$VPP_OUTPUT_DIR"/lib*_plugin.so; do + [ -f "$so" ] || continue + if ! so_hash=$(sha256_hex "$so"); then + echo "[ERROR] Cannot hash $so (no sha256sum/shasum on PATH)." >&2 + rm -f "$VPP_OBJECT_SEAL" + exit 3 + fi + printf '%s %s\n' "$so_hash" "$(basename "$so")" >> "$VPP_OBJECT_SEAL" + echo "[INFO] Sealed $(basename "$so") (${so_hash:0:12}...)" + done else # No VPP plugin in this upload — clean up the entire VPP output dir # so a stale .so doesn't get picked up by the loader. Scoping the rm diff --git a/tests/pytest/plugins/test_vpp_license_delivery.py b/tests/pytest/plugins/test_vpp_license_delivery.py index 827bd6a2..68092a80 100644 --- a/tests/pytest/plugins/test_vpp_license_delivery.py +++ b/tests/pytest/plugins/test_vpp_license_delivery.py @@ -78,14 +78,19 @@ def test_apply_vpp_plugin_conf_delivers_license(tmp_path, monkeypatch): monkeypatch.chdir(cwd) config_path = str(cwd / "build" / "vpp" / "rpi_gpio.json") + # `path` is not decoration: apply_vpp_plugin_conf now runs the uploaded conf + # through validate_vpp_plugins_conf first, which requires every VPP plugin's + # .so to resolve inside build/vpp/ (see test_vpp_plugin_signature.py). A fake + # without it would only prove the fake is out of date. class _P: name = "rpi_gpio" - def __init__(self, cp): + def __init__(self, cp, so): self.config_path = cp + self.path = so class _Conf: - plugins = [_P(config_path)] + plugins = [_P(config_path, str(cwd / "build" / "vpp" / "librpi_gpio_plugin.so"))] monkeypatch.setattr(mgmt.PluginsConfiguration, "from_file", classmethod(lambda cls, _p: _Conf())) monkeypatch.setattr(mgmt.build_state, "log", lambda *_a, **_k: None, raising=False) diff --git a/tests/pytest/plugins/test_vpp_plugin_signature.py b/tests/pytest/plugins/test_vpp_plugin_signature.py new file mode 100644 index 00000000..cb2910c9 --- /dev/null +++ b/tests/pytest/plugins/test_vpp_plugin_signature.py @@ -0,0 +1,460 @@ +"""Tests for the VPP package-signature gate on the upload path. + +Two kinds of test, on purpose: + +1. **A known-answer vector taken from a REAL signed package.** ``REAL_SIGNATURE`` + below is the verbatim ``signature.json`` of + ``com.openplc.raspberry-pi-licensed-1.0.0.vpp``, produced by the + openplc-packages signing pipeline, and the key it names is the one shipped in + webserver/vpp_package_signature.py. It exists because the single real risk in + this design is that the runtime's canonicalization does not reproduce the + signer's byte for byte -- and a payload I sign myself with my own + canonicalization would agree with my own mistake and stay green. This vector + cannot: the bytes were signed by code in another repo, in another language. + +2. **Plumbing tests, signed with a throwaway key.** Everything downstream of the + signature check -- which files must be present, which must not, how the + uploaded ``vpp_plugin/`` maps onto the signed package paths -- needs payloads + this test controls, so it generates a key and trusts it for the duration. + These are safe to self-sign precisely because test 1 pins the format. +""" +import base64 +import hashlib +import json +import os +import shutil +import tempfile + +import pytest + +_sig = pytest.importorskip( + "webserver.vpp_package_signature", + reason="runtime webserver package not importable (no venv)", +) + +# Skipped rather than failed where the dependency is absent: the module is +# designed to refuse VPP uploads with an actionable message in that state, and +# that behaviour is asserted separately below without needing the library. +_crypto = pytest.importorskip("cryptography", reason="cryptography not installed") + +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey # noqa: E402 +from cryptography.hazmat.primitives.serialization import Encoding, PublicFormat # noqa: E402 + +# --------------------------------------------------------------------------- +# 1. Known-answer vector from a real .vpp +# --------------------------------------------------------------------------- +# Copied byte for byte out of dist/com.openplc.raspberry-pi-licensed-1.0.0.vpp. +# Do not "tidy" it: the point is that these exact values, canonicalized by this +# repo's implementation, are what the openplc-packages private key signed. +REAL_SIGNATURE = json.loads( + """ +{ + "formatVersion": "1.0", + "alg": "ed25519", + "keyId": "openplc-2026", + "packageId": "com.openplc.raspberry-pi-licensed", + "version": "1.0.0", + "signedAt": "2026-07-22T10:59:20.069Z", + "files": { + "assets/boards/raspberry-pi.png": "e94cd058c3ae18c92931d024e558702805d6435044201befb21bda08bad00464", + "assets/logo.png": "e94cd058c3ae18c92931d024e558702805d6435044201befb21bda08bad00464", + "hal/runtime-v4/plugin/config_template.json": "574e353a5b75185f0777bbb5f722fc6c6cc7058fca0b08fa072fe06a3dda6112", + "hal/runtime-v4/plugin/license_core.o": "edbf26abe1e83b445dda08687398cf2bdcd7bf43318cb57057a905a70aac30c8", + "hal/runtime-v4/plugin/license_gate.o": "6054c4d4116db7e73c391e82cc02baabd1eb054a8a806e74d23ade4023cbb759", + "hal/runtime-v4/plugin/Makefile": "057d85d4e27743b76767dae2071b920357da3f6a9848fd4e9d899563925f49c3", + "hal/runtime-v4/plugin/rpi_config.o": "6de869b44f25ede6bb9dfb446a788394a65f2c231d141c051bad8d0e4ab66538", + "hal/runtime-v4/plugin/rpi_gpio.o": "aff4d91f9fd1507d343d71edffd38f08ba3c667c67fa5f89b2df7dae1326ab18", + "hal/runtime-v4/plugin/rpi_plugin.o": "a978cd597f7c54ae3c2dbbc4b2349ed0abfb62f811bb9cea6d04b5ac4f4c2d16", + "hal/runtime-v4/plugin/sha256.o": "f39be9f90d088106e848112c5706bbcd0f30ede54c0b35439c057e6610d6b786", + "hal/runtime-v4/plugin/uECC.o": "db574e6d107d4acccb22ed124fd9d05f8ba9ec752cee29c071782643c0c30ec1", + "manifest.json": "8db0dad34bdd356a2eaac67221a93e2eec2c2ba99559b3f44c37b4ba5bdbb947" + }, + "signature": "/GsBcebrbwUpRdPHzkU0lhYyiHYsOtccqcqoqju+27jJN+VZ+0T1QzeArJOOuTb+oBUjxa26FPEpLei+qm7WAQ==" +} +""" +) + + +def test_real_package_signature_verifies(): + """The shipped trusted key verifies a signature made by the real pipeline. + + If canonicalize() ever drifts from openplc-packages/scripts/lib/ + package-signing.ts:54 -- key ordering, separators, escaping, anything -- + this fails, and it is the only test that can catch that. + """ + files, error = _sig._verify_signature_file(REAL_SIGNATURE) + assert error == "", error + assert files["hal/runtime-v4/plugin/license_gate.o"] == ( + "6054c4d4116db7e73c391e82cc02baabd1eb054a8a806e74d23ade4023cbb759" + ) + + +def test_real_package_signature_rejects_a_single_flipped_hash(): + """Same vector with one hash changed: the signature must no longer verify. + + Guards against the failure mode where the signature step is accidentally + turned into a no-op and everything passes. + """ + doctored = json.loads(json.dumps(REAL_SIGNATURE)) + doctored["files"]["hal/runtime-v4/plugin/license_gate.o"] = "00" * 32 + _files, error = _sig._verify_signature_file(doctored) + assert "does not verify" in error + + +def test_real_package_signature_rejects_an_injected_payload_field(): + """An extra key is canonicalized INTO the payload, so it breaks the + signature -- it is not silently ignored.""" + doctored = json.loads(json.dumps(REAL_SIGNATURE)) + doctored["notes"] = "harmless" + _files, error = _sig._verify_signature_file(doctored) + assert "does not verify" in error + + +def test_untrusted_key_is_refused(): + doctored = json.loads(json.dumps(REAL_SIGNATURE)) + doctored["keyId"] = "attacker-2026" + _files, error = _sig._verify_signature_file(doctored) + assert "untrusted key" in error + + +# --------------------------------------------------------------------------- +# 2. Plumbing, with a throwaway signing key +# --------------------------------------------------------------------------- +PLUGIN_DIR_REL = "hal/runtime-v4/plugin" + +# The plugin payload of a licensed VPP, in miniature: the enforcement objects, +# the link-only Makefile that names them, and the config template the editor +# strips before upload. +PACKAGE_FILES = { + "manifest.json": b'{"id":"com.test.board"}\n', + f"{PLUGIN_DIR_REL}/Makefile": b"VENDOR_OBJECTS := plugin.o license_core.o license_gate.o\n", + f"{PLUGIN_DIR_REL}/plugin.o": b"\x7fELF plugin object\n", + f"{PLUGIN_DIR_REL}/license_core.o": b"\x7fELF license core\n", + f"{PLUGIN_DIR_REL}/license_gate.o": b"\x7fELF license gate\n", + f"{PLUGIN_DIR_REL}/config_template.json": b'{"plugin_name":"testplug"}\n', + f"{PLUGIN_DIR_REL}/nested/extra.o": b"\x7fELF nested\n", +} + + +class _Signer: + """A throwaway Ed25519 key, trusted only inside a `with` block.""" + + def __init__(self, key_id="test-key"): + self.key_id = key_id + self._key = Ed25519PrivateKey.generate() + + def __enter__(self): + self._pem = ( + self._key.public_key() + .public_bytes(Encoding.PEM, PublicFormat.SubjectPublicKeyInfo) + .decode("utf-8") + ) + _sig.TRUSTED_PACKAGE_KEYS[self.key_id] = self._pem + return self + + def __exit__(self, *_exc): + _sig.TRUSTED_PACKAGE_KEYS.pop(self.key_id, None) + return False + + def sign_package(self, files: dict[str, bytes]) -> dict: + payload = { + "formatVersion": "1.0", + "alg": "ed25519", + "keyId": self.key_id, + "packageId": "com.test.board", + "version": "1.0.0", + "signedAt": "2026-07-28T00:00:00.000Z", + "files": {rel: hashlib.sha256(data).hexdigest() for rel, data in files.items()}, + } + signature = self._key.sign(_sig.canonicalize(payload).encode("utf-8")) + return {**payload, "signature": base64.b64encode(signature).decode("ascii")} + + +def _build_upload(root: str, signer: _Signer, files=None, plugin_dir=PLUGIN_DIR_REL) -> str: + """Reproduce what the editor puts in the upload: the plugin directory's + contents (minus the files it excludes), its checksum.sha256, and the + forwarded package signature.""" + files = PACKAGE_FILES if files is None else files + os.makedirs(root, exist_ok=True) + plugin_out = os.path.join(root, "vpp_plugin") + prefix = plugin_dir + "/" + for rel, data in files.items(): + if not rel.startswith(prefix): + continue + inner = rel[len(prefix) :] + if os.path.basename(inner) in _sig.EDITOR_EXCLUDED_BASENAMES: + continue + out = os.path.join(plugin_out, inner) + os.makedirs(os.path.dirname(out), exist_ok=True) + with open(out, "wb") as handle: + handle.write(data) + # The editor's cache key. Content is irrelevant here; presence is what the + # gate has to tolerate, since it cannot be covered by the signature. + with open(os.path.join(plugin_out, "checksum.sha256"), "w") as handle: + handle.write("deadbeef\n") + with open(os.path.join(root, "vpp_signature.json"), "w", encoding="utf-8") as handle: + json.dump({"pluginDir": plugin_dir, "package": signer.sign_package(files)}, handle) + return root + + +def _tmp(): + return tempfile.mkdtemp(prefix="vpp-sig-test-") + + +def test_signed_untouched_upload_is_accepted(): + """The inverse of every test below, and the one that keeps this gate from + quietly becoming a blanket refusal nobody notices.""" + root = _tmp() + try: + with _Signer() as signer: + _build_upload(root, signer) + result = _sig.verify_uploaded_vpp_plugin(root) + assert result.ok, result.error + assert result.package_id == "com.test.board" + assert result.tree_digest and len(result.tree_digest) == 64 + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_plain_program_without_vpp_plugin_is_untouched(): + """Policy: only uploads carrying vpp_plugin/ need a signature. Every + existing user and every older editor keeps working.""" + root = _tmp() + try: + with open(os.path.join(root, "generated.hpp"), "w") as handle: + handle.write("// program\n") + result = _sig.verify_uploaded_vpp_plugin(root) + assert result.ok + assert result.tree_digest is None + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_injected_license_gate_source_is_refused(): + """The audit's attack, first half: drop in a license_gate.c that always + answers "licensed". It has no signed hash, so the upload dies here.""" + root = _tmp() + try: + with _Signer() as signer: + _build_upload(root, signer) + with open(os.path.join(root, "vpp_plugin", "license_gate.c"), "w") as handle: + handle.write("int license_gate_actuation_allowed(void) { return 1; }\n") + result = _sig.verify_uploaded_vpp_plugin(root) + assert not result.ok + assert "not covered by the package signature" in result.error + assert "license_gate.c" in result.error + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_edited_makefile_is_refused(): + """The audit's attack, second half: point VENDOR_OBJECTS at the stub.""" + root = _tmp() + try: + with _Signer() as signer: + _build_upload(root, signer) + makefile = os.path.join(root, "vpp_plugin", "Makefile") + with open(makefile, "ab") as handle: + handle.write(b"license_gate.o: license_gate.c\n\t$(CC) -fPIC -c -o $@ $<\n") + result = _sig.verify_uploaded_vpp_plugin(root) + assert not result.ok + assert "does not match the package signature" in result.error + assert "Makefile" in result.error + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_swapped_prebuilt_object_is_refused(): + root = _tmp() + try: + with _Signer() as signer: + _build_upload(root, signer) + with open(os.path.join(root, "vpp_plugin", "license_gate.o"), "wb") as handle: + handle.write(b"\x7fELF attacker gate\n") + result = _sig.verify_uploaded_vpp_plugin(root) + assert not result.ok + assert "license_gate.o" in result.error + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_dropped_enforcement_object_is_refused(): + """Deleting a signed object must fail HERE, not merely at link time: not + every package's Makefile names its objects explicitly.""" + root = _tmp() + try: + with _Signer() as signer: + _build_upload(root, signer) + os.remove(os.path.join(root, "vpp_plugin", "license_core.o")) + result = _sig.verify_uploaded_vpp_plugin(root) + assert not result.ok + assert "missing from the upload" in result.error + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_editor_excluded_files_may_be_absent(): + """config_template.json is signed in the package but the editor drops it, + so its absence must not be read as a dropped object.""" + root = _tmp() + try: + with _Signer() as signer: + _build_upload(root, signer) + assert not os.path.exists(os.path.join(root, "vpp_plugin", "config_template.json")) + result = _sig.verify_uploaded_vpp_plugin(root) + assert result.ok, result.error + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_unsigned_upload_with_vpp_plugin_is_refused(): + root = _tmp() + try: + with _Signer() as signer: + _build_upload(root, signer) + os.remove(os.path.join(root, "vpp_signature.json")) + result = _sig.verify_uploaded_vpp_plugin(root) + assert not result.ok + assert "no usable package signature" in result.error + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_signature_from_a_different_package_is_refused(): + """Reusing a legitimate signature.json from another package does not help: + the hashes do not match the objects that travelled.""" + root = _tmp() + try: + with _Signer() as signer: + _build_upload(root, signer) + other = {**PACKAGE_FILES, f"{PLUGIN_DIR_REL}/license_gate.o": b"different bytes\n"} + with open(os.path.join(root, "vpp_signature.json"), "w", encoding="utf-8") as handle: + json.dump({"pluginDir": PLUGIN_DIR_REL, "package": signer.sign_package(other)}, handle) + result = _sig.verify_uploaded_vpp_plugin(root) + assert not result.ok + assert "license_gate.o" in result.error + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_plugin_dir_cannot_traverse(): + root = _tmp() + try: + with _Signer() as signer: + _build_upload(root, signer) + with open(os.path.join(root, "vpp_signature.json"), "w", encoding="utf-8") as handle: + json.dump( + {"pluginDir": "hal/../../etc", "package": signer.sign_package(PACKAGE_FILES)}, + handle, + ) + result = _sig.verify_uploaded_vpp_plugin(root) + assert not result.ok + assert "invalid pluginDir" in result.error + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_wrong_plugin_dir_cannot_launder_files(): + """pluginDir is unsigned routing information, so prove it cannot be used to + make unsigned bytes verify: pointing it at a subtree the uploaded files do + not belong to fails, rather than matching some other signed hash.""" + root = _tmp() + try: + with _Signer() as signer: + _build_upload(root, signer) + with open(os.path.join(root, "vpp_signature.json"), "w", encoding="utf-8") as handle: + json.dump( + {"pluginDir": "hal/runtime-v4", "package": signer.sign_package(PACKAGE_FILES)}, + handle, + ) + result = _sig.verify_uploaded_vpp_plugin(root) + assert not result.ok + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_seal_is_written_only_for_a_verified_plugin(): + """scripts/compile.sh keys off this file, so an accepted plain program must + not produce one and an accepted plugin must.""" + root = _tmp() + try: + with _Signer() as signer: + _build_upload(root, signer) + result = _sig.verify_uploaded_vpp_plugin(root) + _sig.write_verification_seal(root, result) + seal = os.path.join(root, _sig.VERIFICATION_SEAL_NAME) + assert os.path.exists(seal) + with open(seal, encoding="utf-8") as handle: + body = handle.read() + assert f"treeDigest {result.tree_digest}" in body + + plain = _tmp() + try: + _sig.write_verification_seal(plain, _sig.verify_uploaded_vpp_plugin(plain)) + assert not os.path.exists(os.path.join(plain, _sig.VERIFICATION_SEAL_NAME)) + finally: + shutil.rmtree(plain, ignore_errors=True) + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_tree_digest_tracks_content_and_names(): + """The digest compile.sh recomputes must change for a content edit AND for + a rename, or a swap between the gate and `make` would slip through.""" + root = _tmp() + try: + os.makedirs(os.path.join(root, "d")) + with open(os.path.join(root, "d", "a"), "wb") as handle: + handle.write(b"one") + first = _sig.tree_digest(root) + with open(os.path.join(root, "d", "a"), "wb") as handle: + handle.write(b"two") + assert _sig.tree_digest(root) != first + os.rename(os.path.join(root, "d", "a"), os.path.join(root, "d", "b")) + renamed = _sig.tree_digest(root) + with open(os.path.join(root, "d", "b"), "wb") as handle: + handle.write(b"two") + assert _sig.tree_digest(root) == renamed + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_signature_policy_is_one_decision(): + """The unsigned-upload policy lives in exactly one function. If someone + changes it, this is the test that has to be updated with it.""" + assert _sig.signature_required(has_vpp_plugin=True) is True + assert _sig.signature_required(has_vpp_plugin=False) is False + + +# --------------------------------------------------------------------------- +# 3. Containment of the loaded path (the .so the runtime actually dlopens) +# --------------------------------------------------------------------------- +def test_vpp_plugins_conf_containment(): + """A verified plugin tree still means nothing if vpp_plugins.conf can point + `path` at any .so on the box. Every entry must resolve inside build/vpp/.""" + mgmt = pytest.importorskip( + "webserver.plcapp_management", + reason="runtime webserver package not importable (no venv)", + ) + root = _tmp() + try: + os.makedirs(os.path.join(root, "build", "vpp")) + cases = [ + ("good,./build/vpp/libx_plugin.so,1,1,./build/vpp/x.json,\n", True), + ("evil,/tmp/evil.so,1,1,./build/vpp/x.json,\n", False), + ("evil,../../tmp/evil.so,1,1,./build/vpp/x.json,\n", False), + # Inside the runtime root but outside build/vpp/: still unverified. + ("evil,./build/libplc.so,1,1,./build/vpp/x.json,\n", False), + ("evil,./core/generated/vpp_plugin/x.so,1,1,./build/vpp/x.json,\n", False), + # Escaping config_path, which the pre-existing per-file guard also + # caught -- now the whole conf is refused instead of one line. + ("evil,./build/vpp/libx_plugin.so,1,1,/etc/cron.d/runme,\n", False), + ] + for line, expect_ok in cases: + conf = os.path.join(root, "vpp_plugins.conf") + with open(conf, "w") as handle: + handle.write(line) + ok, reason = mgmt.validate_vpp_plugins_conf(conf, root, "build/vpp") + assert ok is expect_ok, f"{line.strip()} -> ok={ok} reason={reason}" + finally: + shutil.rmtree(root, ignore_errors=True) diff --git a/webserver/app.py b/webserver/app.py index a05505e5..9c3d65c6 100644 --- a/webserver/app.py +++ b/webserver/app.py @@ -45,6 +45,10 @@ restapi_bp, ) from webserver.runtimemanager import RuntimeManager +from webserver.vpp_package_signature import ( + verify_uploaded_vpp_plugin, + write_verification_seal, +) logger, _ = get_logger("logger", use_buffer=True) @@ -284,6 +288,30 @@ def handle_upload_file(data: dict) -> dict: safe_extract(zip_file, extract_dir, valid_files) + # Verify the VPP package signature BEFORE anything from this upload is + # copied into the runtime root and before any Makefile from it runs. + # + # The order matters and is the reason this sits here rather than in + # compile.sh: a licensed VPP ships the closed enforcement objects and a + # link-only Makefile inside the upload, and compile.sh runs that + # Makefile as root. Refusing here means an unsigned or tampered plugin + # never gets a vpp_plugins.conf installed, never gets its config or + # license blob copied out, and never reaches `make`. + # + # Policy for uploads without a signature lives in exactly one place -- + # vpp_package_signature.signature_required(). Today: only uploads that + # carry a vpp_plugin/ directory must be signed, so plain PLC programs + # from any editor keep working untouched. + verification = verify_uploaded_vpp_plugin(extract_dir) + if not verification.ok: + build_state.status = BuildStatus.FAILED + build_state.log(f"[ERROR] {verification.error}\n") + return { + "UploadFileFail": verification.error, + "CompilationStatus": build_state.status.name, + } + write_verification_seal(extract_dir, verification) + # Apply VPP plugin conf from upload (copy if present, delete if not) apply_vpp_plugin_conf(extract_dir) diff --git a/webserver/plcapp_management.py b/webserver/plcapp_management.py index b4af160e..0a00fc51 100644 --- a/webserver/plcapp_management.py +++ b/webserver/plcapp_management.py @@ -271,6 +271,58 @@ def _wait_for_plc_idle(runtime_manager: RuntimeManager, timeout_s: float) -> boo return False +def validate_vpp_plugins_conf(conf_path: str, runtime_root: str, vpp_build_dir: str) -> tuple[bool, str]: + """Containment check for an upload-supplied ``vpp_plugins.conf``. + + The ``path`` field of this file is what the C plugin loader passes straight + to ``dlopen`` (``core/src/drivers/plugin_driver.c``). It arrives verbatim + from the upload, and until this existed only ``config_path`` was checked -- + so a forged conf could name ANY .so on the filesystem, including one the + attacker left there by an unrelated route, and verifying the plugin the + build produced would have proved nothing about the object actually loaded. + + Two rules, and the whole file is refused if either is broken (rather than + dropping the offending line): a conf that tries to escape is not a conf we + want to partially honour, and leaving the rest installed would silently + load a subset of what the editor intended. + + 1. ``path`` must resolve inside the runtime root -- same + ``is_inside_root`` definition (symlink-resolving) every other write path + here uses. + 2. ``path`` must resolve inside ``build/vpp/``. That is the only directory + compile.sh writes VPP artefacts into, and the only one whose contents + the compile-time seal covers, so anything outside it is by definition + unverified. + + The C side repeats rule 1's spirit in ``parse_plugin_config_contained`` -- + on purpose, so containment does not depend on Python alone. + """ + plugins_conf = PluginsConfiguration.from_file(conf_path) + vpp_root = os.path.abspath(os.path.join(runtime_root, vpp_build_dir)) + + def against_root(candidate: str) -> str: + """Resolve a conf path the way the C loader will: relative entries are + relative to the runtime root (which is the loader's cwd). Resolving + against the process cwd instead would make the guard depend on where + the caller happened to be.""" + return candidate if os.path.isabs(candidate) else os.path.join(runtime_root, candidate) + + for p in plugins_conf.plugins: + if not p.path: + return False, f"plugin '{p.name}' has an empty path" + plugin_path = against_root(p.path) + if not is_inside_root(plugin_path, runtime_root): + return False, f"plugin '{p.name}' path '{p.path}' escapes the runtime root" + if not is_inside_root(plugin_path, vpp_root): + return False, ( + f"plugin '{p.name}' path '{p.path}' is outside {vpp_build_dir}/ " + "(VPP plugins may only load objects built by this upload)" + ) + if p.config_path and not is_inside_root(against_root(p.config_path), runtime_root): + return False, f"plugin '{p.name}' config_path '{p.config_path}' escapes the runtime root" + return True, "" + + def apply_vpp_plugin_conf(generated_dir: str = "core/generated") -> None: """Apply or remove the VPP plugin configuration for this upload. @@ -296,6 +348,19 @@ def apply_vpp_plugin_conf(generated_dir: str = "core/generated") -> None: uploaded_conf = os.path.join(generated_dir, "vpp_plugins.conf") if os.path.exists(uploaded_conf): + runtime_root = os.path.abspath(".") + + # Containment BEFORE the copy: once this file is in the runtime root the + # C loader will dlopen whatever `path` says, so an escaping entry has to + # be stopped while it is still just a file in core/generated/. + contained, reason = validate_vpp_plugins_conf(uploaded_conf, runtime_root, VPP_BUILD_DIR) + if not contained: + build_state.log(f"[ERROR] VPP: refusing vpp_plugins.conf from upload: {reason}\n") + if os.path.exists(VPP_CONF_DEST): + os.remove(VPP_CONF_DEST) + build_state.log("[INFO] VPP: removed previous vpp_plugins.conf\n") + return + # Copy vpp_plugins.conf to runtime root shutil.copy2(uploaded_conf, VPP_CONF_DEST) build_state.log(f"[INFO] VPP: installed vpp_plugins.conf from upload\n") @@ -307,7 +372,6 @@ def apply_vpp_plugin_conf(generated_dir: str = "core/generated") -> None: # a separate destination. conf_dir = os.path.join(generated_dir, "conf") vpp_conf_plugins = PluginsConfiguration.from_file(VPP_CONF_DEST) - runtime_root = os.path.abspath(".") for p in vpp_conf_plugins.plugins: if not p.config_path: continue diff --git a/webserver/vpp_package_signature.py b/webserver/vpp_package_signature.py new file mode 100644 index 00000000..36b41dd7 --- /dev/null +++ b/webserver/vpp_package_signature.py @@ -0,0 +1,427 @@ +"""Ed25519 verification of the VPP package signature that rides in an upload. + +Why this exists +--------------- +A licensed VPP ships the closed enforcement objects (``license_core.o``, +``license_gate.o``) plus a link-only ``Makefile`` INSIDE the user's upload, and +``scripts/compile.sh`` runs that Makefile as root. Nothing in that path proved +where those bytes came from: swapping an object, or adding a three-line +``license_gate.c`` stub that always returns "licensed" and listing it in the +Makefile's ``VENDOR_OBJECTS``, was enough to run a licensed VPP in FULL mode. + +The signing machinery already existed and was simply not wired end to end: +``openplc-packages`` signs every ``.vpp`` with Ed25519 and emits a +``signature.json`` holding a sha256 per packaged file, and the editor already +carries the matching public key. What was missing was (a) forwarding that file +to the runtime and (b) checking it here, BEFORE any ``make`` runs. + +The contract, byte for byte +--------------------------- +Mirrors ``openplc-packages/scripts/lib/package-signing.ts`` and the editor's +``src/backend/shared/utils/vpp/verify-package-signature.ts``. All three MUST +agree on: + +1. **Payload** — every key of ``signature.json`` except ``signature``. Extra + keys are NOT ignored: they are canonicalized in, so a doctored file fails. +2. **Canonicalization** — recursive, key-sorted JSON with no whitespace. This + is the exact byte string Ed25519 covers (see :func:`canonicalize`). +3. **File hashing** — sha256 over the raw bytes, lower-case hex. + +What is verified here, and what is deliberately not +--------------------------------------------------- +The signature covers the WHOLE package, but the upload only carries the plugin +directory. So: the Ed25519 signature is checked over the ENTIRE payload (never +a filtered slice -- a slice is not what was signed and would never verify), and +then the file hashes are compared only for the files that actually travelled, +mapped back to their package-relative paths through ``pluginDir``. + +``pluginDir`` itself is unsigned routing information. That is safe by +construction: it only selects WHICH signed subtree the uploaded bytes are +compared against, and every subtree of the package was signed by us. It cannot +be pointed at anything that would let unsigned bytes through. + +Trust model (write this down, do not rediscover it) +--------------------------------------------------- +The verifier lives inside an open-source runtime the user can recompile, so a +determined owner can delete this gate and reinstall. That is accepted: the goal +is to turn "edit a Makefile inside a tarball" (minutes, low skill) into "fork +and reinstall the runtime" (visible, does not survive an upgrade). It is NOT a +cryptographic barrier against the device owner. +""" + +from dataclasses import dataclass +from hashlib import sha256 +from typing import Final, Optional +import json +import os + +# --------------------------------------------------------------------------- +# Trust anchor +# --------------------------------------------------------------------------- +# keyId -> PEM Ed25519 public key. Mirrors, byte for byte, the editor's +# src/backend/shared/utils/vpp/trusted-keys.ts:20. The map (rather than a +# single constant) is what makes rotation possible: ship both keys, retire the +# old one once no supported package still depends on it. The private halves +# live only in the openplc-packages signing pipeline (CI secret). +TRUSTED_PACKAGE_KEYS: Final[dict[str, str]] = { + "openplc-2026": ( + "-----BEGIN PUBLIC KEY-----\n" + "MCowBQYDK2VwAyEABdweEuJAfYG923RkmZLYsmonLvCcgVtgpJ7mngbRJQk=\n" + "-----END PUBLIC KEY-----\n" + ), +} + +# --------------------------------------------------------------------------- +# Upload layout — the editor side of the contract +# --------------------------------------------------------------------------- +# Written by openplc-editor's CompilerModule.handleVendorPluginPackaging. +SIGNATURE_SIDECAR_NAME: Final[str] = "vpp_signature.json" +VPP_PLUGIN_DIR_NAME: Final[str] = "vpp_plugin" +# Seal consumed by scripts/compile.sh so a direct invocation of the script +# cannot build an unverified plugin tree. +VERIFICATION_SEAL_NAME: Final[str] = "vpp_plugin.verified" + +# Files the editor deliberately drops when it copies the package's plugin +# directory into vpp_plugin/ (see compiler-module.ts EXCLUDE_FILES). They are +# signed in the package but never travel, so their absence is expected and +# must not be read as a dropped object. Matched by basename at any depth, +# exactly as the editor matches them. +EDITOR_EXCLUDED_BASENAMES: Final[frozenset[str]] = frozenset( + {"config_template.json", "requirements.txt"} +) + +# Files the editor GENERATES into vpp_plugin/ and which therefore cannot be +# covered by the package signature. Top-level only. checksum.sha256 is the +# recompilation cache key (scripts/compile.sh) -- it is not integrity, and it +# is not compiled or linked into anything. +EDITOR_GENERATED_FILES: Final[frozenset[str]] = frozenset({"checksum.sha256"}) + + +# --------------------------------------------------------------------------- +# SINGLE POINT OF POLICY — what happens to an upload without a signature +# --------------------------------------------------------------------------- +def signature_required(has_vpp_plugin: bool) -> bool: + """Whether this upload must carry a valid package signature. + + THIS FUNCTION IS THE ONLY PLACE THE POLICY LIVES. Changing the answer for + a whole class of uploads is a one-line edit here; do not spread the + decision into callers. + + Current policy, and the trade-off it buys: + + * An upload that carries a ``vpp_plugin/`` directory MUST be signed. That + directory is the attack surface -- it is the only content the runtime + compiles with a Makefile that came from the upload itself. + * A plain PLC program (no ``vpp_plugin/``) is untouched. Every existing + user, and every editor built before the sidecar existed, keeps working. + + What this deliberately does NOT do: require a signature for all uploads. + That would be strictly stronger (it would also cover the ``core/generated/ + *.cpp`` path, which is arbitrary native code compiled as root) but it + would refuse every upload from every editor in the field today, and the + C++ path is the product -- ``c_blocks_code.cpp`` is the user's own C slot, + which nobody can sign for them. Raising the bar there is a privilege + question (run the compiler unprivileged), not a signature question. + + The residual gap while this stays as it is: an attacker who wants native + code as root does not need ``vpp_plugin/`` at all. This gate closes the + LICENSING bypass, not the RCE. + """ + return has_vpp_plugin + + +@dataclass(frozen=True) +class VerificationResult: + """Outcome of the upload gate. ``ok`` False means: refuse the upload.""" + + ok: bool + #: Operator-facing reason; empty when ``ok``. Written verbatim into the + #: upload response and the build log, so it must say what to DO. + error: str = "" + #: Set when a signature was actually verified (None for a plain program). + package_id: Optional[str] = None + #: sha256 over the verified vpp_plugin/ tree; the seal compile.sh checks. + tree_digest: Optional[str] = None + + +def canonicalize(value: object) -> str: + """Recursive, key-sorted JSON with no extra whitespace. + + Must produce the same bytes as ``canonicalize`` in + openplc-packages/scripts/lib/package-signing.ts:54. Two details are not + incidental: + + * ``ensure_ascii=False`` -- JS ``JSON.stringify`` emits non-ASCII + characters raw; Python escapes them by default, which would change the + signed bytes for any package with a non-ASCII path. + * Numbers are the one shape where the two languages can still disagree + (JS renders ``1.0`` as ``1``). Every field of a real payload is a + string, so this never fires in practice, and if it ever did the + mismatch fails CLOSED (bad signature) rather than open. + """ + if value is None or isinstance(value, (str, bool, int, float)): + return json.dumps(value, ensure_ascii=False) + if isinstance(value, (list, tuple)): + return "[" + ",".join(canonicalize(v) for v in value) + "]" + if isinstance(value, dict): + keys = sorted(value.keys()) + return "{" + ",".join(f"{json.dumps(k, ensure_ascii=False)}:{canonicalize(value[k])}" for k in keys) + "}" + raise TypeError(f"cannot canonicalize {type(value).__name__}") + + +def _verify_ed25519(message: bytes, signature: bytes, public_key_pem: str) -> bool: + """Ed25519 verify. Any failure -- including a missing dependency -- raises. + + ``cryptography`` is listed in requirements.txt. It is imported lazily and + inside the request path only, so a runtime that has not re-run install.sh + still boots and still serves plain programs; only VPP uploads are refused, + with a message that names the missing package. + """ + try: + from cryptography.exceptions import InvalidSignature + from cryptography.hazmat.primitives.serialization import load_pem_public_key + except ImportError as exc: # pragma: no cover - depends on the install + raise RuntimeError( + "python package 'cryptography' is not installed, so the VPP package " + "signature cannot be verified; re-run install.sh (or " + "pip install -r requirements.txt) on this runtime" + ) from exc + + key = load_pem_public_key(public_key_pem.encode("utf-8")) + verify = getattr(key, "verify", None) + if verify is None: + raise RuntimeError("trusted key is not an Ed25519 public key") + try: + verify(signature, message) + except InvalidSignature: + return False + return True + + +def sha256_file(path: str) -> str: + """sha256 hex of a file's raw bytes, read in chunks (objects are MBs).""" + digest = sha256() + with open(path, "rb") as handle: + for chunk in iter(lambda: handle.read(65536), b""): + digest.update(chunk) + return digest.hexdigest() + + +def list_tree_files(root: str) -> list[str]: + """Every regular file under ``root`` as sorted POSIX-relative paths. + + Symlinks are reported as-is rather than followed: the caller rejects them, + because a link is not a regular file and hashing its target would let an + upload pass verification on bytes that are not in the upload. + """ + out: list[str] = [] + for current, dirnames, filenames in os.walk(root, followlinks=False): + dirnames.sort() + for name in sorted(filenames): + full = os.path.join(current, name) + rel = os.path.relpath(full, root).replace(os.sep, "/") + out.append(rel) + return sorted(out) + + +def tree_digest(root: str) -> str: + """Digest over a whole directory: sha256 of " \\n" lines. + + Same shape scripts/compile.sh recomputes when it checks the verification + seal, so the two cannot drift: sorted POSIX-relative paths, two spaces, + trailing newline, then sha256 of that listing. + """ + outer = sha256() + for rel in list_tree_files(root): + outer.update(f"{sha256_file(os.path.join(root, rel))} {rel}\n".encode("utf-8")) + return outer.hexdigest() + + +def _parse_sidecar(path: str) -> tuple[dict, str, str]: + """Read the editor's sidecar. Returns (signature_file, plugin_dir, error).""" + try: + with open(path, "r", encoding="utf-8") as handle: + raw = json.load(handle) + except (OSError, ValueError) as exc: + return {}, "", f"{SIGNATURE_SIDECAR_NAME} is missing or unreadable ({exc})" + + if not isinstance(raw, dict): + return {}, "", f"{SIGNATURE_SIDECAR_NAME} is malformed (not an object)" + + package = raw.get("package") + plugin_dir = raw.get("pluginDir") + if not isinstance(package, dict) or not isinstance(plugin_dir, str) or not plugin_dir: + return {}, "", f"{SIGNATURE_SIDECAR_NAME} is malformed (missing 'package' or 'pluginDir')" + + # pluginDir only selects which signed subtree to compare against, but it is + # still string-concatenated into signed paths -- keep it a plain relative + # POSIX path so it cannot be used to walk anywhere unexpected. + normalized = plugin_dir.strip("/") + if not normalized or normalized.startswith("/") or ".." in normalized.split("/") or "\\" in normalized: + return {}, "", f"{SIGNATURE_SIDECAR_NAME} has an invalid pluginDir: {plugin_dir!r}" + + return package, normalized, "" + + +def _verify_signature_file(signature_file: dict) -> tuple[dict, str]: + """Check the detached Ed25519 signature. Returns (files_map, error).""" + signature_b64 = signature_file.get("signature") + if not isinstance(signature_b64, str) or not signature_b64: + return {}, "package signature.json is malformed (no signature)" + + # The payload is EVERYTHING except `signature` -- including keys we do not + # know about. That is what the signing side hashed, and it is why an + # attacker cannot smuggle extra fields past the check. + payload = {k: v for k, v in signature_file.items() if k != "signature"} + + for field in ("formatVersion", "alg", "keyId", "packageId", "version", "signedAt"): + if not isinstance(payload.get(field), str): + return {}, f"package signature.json is malformed (bad '{field}')" + files = payload.get("files") + if not isinstance(files, dict) or any(not isinstance(v, str) for v in files.values()): + return {}, "package signature.json is malformed (bad 'files' map)" + + if payload["alg"] != "ed25519": + return {}, f"unsupported package signature algorithm: {payload['alg']}" + + public_key_pem = TRUSTED_PACKAGE_KEYS.get(payload["keyId"]) + if not public_key_pem: + return {}, f"package is signed by an untrusted key: {payload['keyId']}" + + from base64 import b64decode + + try: + signature = b64decode(signature_b64, validate=True) + except ValueError: + return {}, "package signature is not valid base64" + + message = canonicalize(payload).encode("utf-8") + try: + valid = _verify_ed25519(message, signature, public_key_pem) + except RuntimeError as exc: + return {}, str(exc) + if not valid: + return {}, "package signature does not verify (tampered package or wrong key)" + + return files, "" + + +def verify_uploaded_vpp_plugin(generated_dir: str) -> VerificationResult: + """The upload gate. Call BEFORE anything is copied out of ``generated_dir``. + + Refusing here (rather than in compile.sh) is deliberate: nothing from the + upload -- not vpp_plugins.conf, not the per-plugin JSON, not the license + blob -- reaches the runtime root until this returns ok. + """ + plugin_dir = os.path.join(generated_dir, VPP_PLUGIN_DIR_NAME) + has_vpp_plugin = os.path.isdir(plugin_dir) + + if not signature_required(has_vpp_plugin): + return VerificationResult(ok=True) + + sidecar_path = os.path.join(generated_dir, SIGNATURE_SIDECAR_NAME) + signature_file, signed_prefix, error = _parse_sidecar(sidecar_path) + if error: + return VerificationResult( + ok=False, + error=( + f"this upload contains a VPP plugin but no usable package signature: {error}. " + "Re-install the VPP package from a signed .vpp and upload again from an " + "editor that forwards the package signature." + ), + ) + + signed_files, error = _verify_signature_file(signature_file) + if error: + return VerificationResult(ok=False, error=f"VPP package signature rejected: {error}") + + prefix = signed_prefix + "/" + + # 1) Everything that travelled must be signed, with the signed bytes. + # This is the half that stops the audit's attack: an injected + # license_gate.c has no signed hash, and an edited Makefile has the + # wrong one. + try: + present = list_tree_files(plugin_dir) + except OSError as exc: + return VerificationResult(ok=False, error=f"VPP plugin directory is unreadable: {exc}") + + for rel in present: + full = os.path.join(plugin_dir, rel) + if os.path.islink(full) or not os.path.isfile(full): + return VerificationResult( + ok=False, + error=f"VPP plugin directory contains a non-regular file: vpp_plugin/{rel}", + ) + if rel in EDITOR_GENERATED_FILES: + continue + expected = signed_files.get(prefix + rel) + if expected is None: + return VerificationResult( + ok=False, + error=( + f"VPP plugin file is not covered by the package signature: vpp_plugin/{rel}. " + "The uploaded plugin does not match the signed .vpp package." + ), + ) + if sha256_file(full) != expected: + return VerificationResult( + ok=False, + error=( + f"VPP plugin file does not match the package signature: vpp_plugin/{rel}. " + "The uploaded plugin was modified after the package was signed." + ), + ) + + # 2) Nothing signed may be missing either. Without this, dropping an + # enforcement object would still be refused only indirectly (the signed + # Makefile would fail to link it), which depends on that Makefile + # naming the object explicitly. Not every package's will. + present_set = set(present) + for signed_path in signed_files: + if not signed_path.startswith(prefix): + continue + rel = signed_path[len(prefix) :] + if os.path.basename(rel) in EDITOR_EXCLUDED_BASENAMES: + continue # the editor drops these on purpose; see the constant + if rel not in present_set: + return VerificationResult( + ok=False, + error=( + f"VPP plugin file is missing from the upload: vpp_plugin/{rel}. " + "The uploaded plugin is not the signed .vpp package." + ), + ) + + return VerificationResult( + ok=True, + package_id=str(signature_file.get("packageId", "")), + tree_digest=tree_digest(plugin_dir), + ) + + +def write_verification_seal(generated_dir: str, result: VerificationResult) -> None: + """Record that the gate ran and over which bytes. + + scripts/compile.sh refuses to run the uploaded Makefile without this seal + AND without recomputing the same tree digest, so invoking compile.sh + directly cannot skip the gate, and a file swapped between the gate and + ``make`` is caught. The seal is an interlock, not a trust anchor: it is + unkeyed, so anyone who can already write into core/generated/ can forge + it -- but anyone who can do that has a shell, and the upload path (the + thing being defended) cannot. + """ + if result.tree_digest is None: + return + seal_path = os.path.join(generated_dir, VERIFICATION_SEAL_NAME) + lines = [ + "# OpenPLC VPP plugin verification seal.", + "# Written by the webserver upload gate (webserver/vpp_package_signature.py)", + "# after the package Ed25519 signature verified against the bytes below.", + "# Consumed by scripts/compile.sh. Not a signature -- see the docstring.", + f"packageId {result.package_id or ''}", + f"treeDigest {result.tree_digest}", + "", + ] + with open(seal_path, "w", encoding="utf-8") as handle: + handle.write("\n".join(lines)) From 60265109774d6e7cee099aa3ab5ea196e26467f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Tue, 28 Jul 2026 21:48:38 +0200 Subject: [PATCH 05/10] test(vpp): pin the python/bash tree digest contract with a cross-language test webserver/vpp_package_signature.py's tree_digest() and scripts/compile.sh's vpp_tree_digest() must agree byte for byte: compile.sh recomputes the digest from the seal python wrote and refuses to build any VPP upload where the two disagree. That agreement was checked by hand once and never fixed by test, so any future edit to either side's sort order, hash line format, or encoding could silently break every legitimate VPP upload in the field. Add tests/pytest/plugins/test_vpp_tree_digest_cross_language.py, which extracts the real sha256_hex/vpp_tree_digest function bodies out of scripts/compile.sh by source text (rather than reimplementing them) and runs them under bash against the same directory tree_digest() hashes in-process, covering nested directories, mixed case, hyphen/underscore collisions, and ASCII-vs-natural sort ordering. Verified this catches real divergence: reversing the bash sort order, and separately narrowing the python line separator to one space, both turned the new test red; reverting either restores green. --- .../test_vpp_tree_digest_cross_language.py | 241 ++++++++++++++++++ 1 file changed, 241 insertions(+) create mode 100644 tests/pytest/plugins/test_vpp_tree_digest_cross_language.py diff --git a/tests/pytest/plugins/test_vpp_tree_digest_cross_language.py b/tests/pytest/plugins/test_vpp_tree_digest_cross_language.py new file mode 100644 index 00000000..6aa22e34 --- /dev/null +++ b/tests/pytest/plugins/test_vpp_tree_digest_cross_language.py @@ -0,0 +1,241 @@ +"""Cross-language contract test for the VPP plugin tree digest. + +Why this exists +--------------- +The upload gate (``webserver/vpp_package_signature.py``) computes a sha256 +tree digest over the verified ``vpp_plugin/`` directory and writes it into +the verification seal. ``scripts/compile.sh`` recomputes that SAME digest, +in bash, right before it lets the uploaded Makefile run +(``check_vpp_verification_seal`` -> ``vpp_tree_digest``), and refuses the +build if the two disagree. + +Nobody fixed by test that the two computations actually agree. They were +compared by hand once, for one tree, during the work that added the +signature gate (a nested, mixed-case, hyphen/underscore tree; both sides +produced ``64772944...6bf5c``). Every line of either function is free to +drift after that -- a change to the sort locale, the hash line format, the +path separator, anything -- and the FIRST symptom would be every legitimate +VPP upload refusing to compile in the field. That is an availability +failure, not a security one, but it is just as silent: nothing today would +catch it before a user hits it. + +How this test stays honest +--------------------------- +It does NOT re-implement ``vpp_tree_digest`` in Python and compare that +reimplementation to itself -- that would only prove this file agrees with +its own assumptions about what compile.sh does. Instead it extracts the +REAL ``sha256_hex`` and ``vpp_tree_digest`` function bodies out of +``scripts/compile.sh`` by source text and executes them, unmodified, under +bash. If either function's source changes, this test runs the new text, +not a stale copy. + +``scripts/compile.sh`` cannot be ``source``d directly: it is a full build +script (``set -euo pipefail``, then immediately runs ``check_required_files`` +and the real compiler invocation) with no include-guard around the two +digest functions. Extracting just those two function definitions is the +only way to exercise the real bash code without also running a build. + +Windows-checkout note: this repo has no ``.gitattributes``, so +``scripts/compile.sh`` is CRLF on a Windows checkout while every ``.py`` +file is LF (see tasks #50/#58). The extracted function text has its ``\r`` +stripped before being handed to bash for exactly that reason -- this is +normalizing line endings for execution, not changing what the function +does. +""" +import os +import re +import shutil +import stat +import subprocess +import tempfile + +import pytest + +_sig = pytest.importorskip( + "webserver.vpp_package_signature", + reason="runtime webserver package not importable (no venv)", +) + +_BASH = shutil.which("bash") +_COMPILE_SH = os.path.normpath( + os.path.join(os.path.dirname(__file__), "..", "..", "..", "scripts", "compile.sh") +) + + +def _extract_bash_function(source: str, name: str) -> str: + """Pull ``name() { ... }`` out of ``source`` by exact text, closing brace + on its own line at column 0 (true of both functions in compile.sh today). + + Raises rather than silently returning an empty string: a test that + "passes" because it extracted nothing and ran nothing would be worse + than not existing. + """ + pattern = re.compile( + r"^" + re.escape(name) + r"\(\)\s*\{$.*?^\}$", + re.MULTILINE | re.DOTALL, + ) + match = pattern.search(source) + if not match: + raise AssertionError( + f"could not find function {name!r} in {_COMPILE_SH} -- the " + "extraction pattern in this test no longer matches the real " + "source, which means this test is not exercising real code" + ) + return match.group(0) + + +def _bash_tree_digest(directory: str) -> str: + """Run the REAL ``vpp_tree_digest`` (plus its ``sha256_hex`` helper) + extracted from scripts/compile.sh, over ``directory``.""" + with open(_COMPILE_SH, "r", encoding="utf-8", newline=None) as handle: + source = handle.read() + # newline=None already gives universal-newline translation, but be + # explicit about the CRLF checkout: strip any stray \r before this text + # is interpreted by bash, which would otherwise choke on it or (worse) + # silently fold it into a path/hash string. + sha256_hex_fn = _extract_bash_function(source, "sha256_hex").replace("\r", "") + vpp_tree_digest_fn = _extract_bash_function(source, "vpp_tree_digest").replace("\r", "") + + script = "\n".join( + [ + "#!/usr/bin/env bash", + "set -euo pipefail", + sha256_hex_fn, + vpp_tree_digest_fn, + 'vpp_tree_digest "$1"', + "", + ] + ) + + fd, script_path = tempfile.mkstemp(prefix="vpp-tree-digest-", suffix=".sh") + try: + with os.fdopen(fd, "w", encoding="utf-8", newline="\n") as handle: + handle.write(script) + os.chmod(script_path, os.stat(script_path).st_mode | stat.S_IEXEC) + result = subprocess.run( + [_BASH, script_path, directory], + capture_output=True, + text=True, + check=False, + ) + finally: + os.remove(script_path) + + assert result.returncode == 0, ( + f"vpp_tree_digest (bash) exited {result.returncode}\n" + f"stdout: {result.stdout!r}\nstderr: {result.stderr!r}" + ) + digest = result.stdout.strip() + assert len(digest) == 64 and all(c in "0123456789abcdef" for c in digest), ( + f"vpp_tree_digest (bash) did not print a sha256 hex digest: {result.stdout!r}" + ) + return digest + + +def _write_tree(root: str, files: dict) -> None: + for rel, data in files.items(): + out = os.path.join(root, rel) + os.makedirs(os.path.dirname(out), exist_ok=True) + with open(out, "wb") as handle: + handle.write(data) + + +pytestmark = pytest.mark.skipif(_BASH is None, reason="bash not found on PATH") + + +# --------------------------------------------------------------------------- +# The edge cases: everything the task called out as a place the two +# implementations' sort/hash could visibly diverge if either side drifted. +# --------------------------------------------------------------------------- +EDGE_CASE_TREE = { + # Nested directories, depth > 1. + "hal/runtime-v4/plugin/license_core.o": b"\x7fELF license core\n", + "hal/runtime-v4/plugin/nested/deep/leaf.o": b"deep nested leaf\n", + # Mixed case: same name, different case, must sort as distinct, separate + # entries (uppercase ASCII sorts before lowercase in a byte-order sort). + "Config.json": b'{"case":"upper"}\n', + "config.json": b'{"case":"lower"}\n', + # Hyphen vs underscore vs neither, sharing a long common prefix. Byte + # values: '-' (0x2D) < '.' (0x2E) < '/' (0x2F) < '_' (0x5F) < letters. + "license-core-extra.o": b"hyphen variant\n", + "license_core_extra.o": b"underscore variant\n", + "licensecoreextra.o": b"no separator variant\n", + # A bare file whose name is a directory-name prefix of another entry's + # directory, so a flat full-path sort and a component-wise sort could, in + # principle, disagree about where the bare file lands relative to the + # directory's contents. + "foo.txt": b"bare file named foo.txt\n", + "foo/y.txt": b"file inside foo/\n", + "foo-bar/x.txt": b"file inside foo-bar/\n", + # ASCII-vs-natural-order collision: byte/ASCII sort puts file1 < file10 < + # file2 (compares the '1' before the '0'), which is NOT numeric order. + # Both implementations must agree on the SAME (ASCII) order, not on what + # a human would consider "natural". + "series/file1.txt": b"one\n", + "series/file2.txt": b"two\n", + "series/file10.txt": b"ten\n", + # Case-mixed directory alongside a same-named-but-cased sibling. + "Assets/logo.png": b"asset upper dir\n", + "assets/logo.png": b"asset lower dir\n", +} + + +def test_python_and_bash_tree_digest_agree_on_edge_case_tree(): + """The contract this whole file exists to pin: sha256_hex(python) == + sha256_hex(bash) over the exact same tree, covering nested depth, + mixed case, hyphen/underscore, and ASCII-vs-natural sort collisions.""" + root = tempfile.mkdtemp(prefix="vpp-tree-digest-cross-lang-") + try: + _write_tree(root, EDGE_CASE_TREE) + python_digest = _sig.tree_digest(root) + bash_digest = _bash_tree_digest(root) + assert python_digest == bash_digest, ( + f"tree_digest (python) = {python_digest}\n" + f"vpp_tree_digest (bash) = {bash_digest}\n" + "These MUST agree byte for byte -- scripts/compile.sh recomputes " + "this digest and refuses to build any upload where it disagrees " + "with the seal python wrote, so a real divergence here means " + "every legitimate VPP upload stops compiling." + ) + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_python_and_bash_tree_digest_agree_on_empty_tree(): + """Zero files is the degenerate case: both sides must hash the same + (empty) byte stream rather than, say, one of them erroring out.""" + root = tempfile.mkdtemp(prefix="vpp-tree-digest-cross-lang-empty-") + try: + assert _sig.tree_digest(root) == _bash_tree_digest(root) + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_python_and_bash_tree_digest_agree_on_single_deep_file(): + """A minimal nested case in isolation, so a failure here narrows the + problem down to path handling rather than sort order among many files.""" + root = tempfile.mkdtemp(prefix="vpp-tree-digest-cross-lang-deep-") + try: + _write_tree(root, {"a/b/c/d/leaf-file_name.o": b"single deep file\n"}) + assert _sig.tree_digest(root) == _bash_tree_digest(root) + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_python_and_bash_tree_digest_both_change_on_rename(): + """Companion to test_tree_digest_tracks_content_and_names in + test_vpp_plugin_signature.py, but checked in BOTH languages: a rename + that leaves content untouched must still move the digest on the bash + side too, or a swap between the gate and `make` could rename a file + around a still-matching seal.""" + root = tempfile.mkdtemp(prefix="vpp-tree-digest-cross-lang-rename-") + try: + _write_tree(root, {"d/a": b"same bytes\n"}) + before_py, before_sh = _sig.tree_digest(root), _bash_tree_digest(root) + os.rename(os.path.join(root, "d", "a"), os.path.join(root, "d", "b")) + after_py, after_sh = _sig.tree_digest(root), _bash_tree_digest(root) + assert before_py != after_py + assert before_sh != after_sh + assert after_py == after_sh + finally: + shutil.rmtree(root, ignore_errors=True) From e49382b86ad9ce4fde7183d95fbd20541e250ce3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Thu, 30 Jul 2026 13:58:43 +0200 Subject: [PATCH 06/10] fix(vpp): validate the license blob on read and write, and write it atomically 0x4A tested the LENGTH only, so a 98-byte file that does not verify -- an SD card cloned from another Pi, corrupted flash, a torn write -- answered SUCCESS. The editor reads that as "magic + crc32 verified", reports Licensed and returns before asking the backend for a fresh license, so the one automatic repair path never runs precisely because the editor trusts the blob, while license_core refuses it and the plugin drops to demo and stops actuating 15 minutes later. The same file on an ESP32 answers 0x83/0x84 and the editor recovers by itself. Emit the same status bytes the bare-metal store already emits, with the same checks in the same order (license_store.h, license_store_esp32.cpp): wrong size -> 0x84, magic absent -> 0x83, crc mismatch -> 0x84. No new ABI: the editor already treats corrupt/empty as "recover this device". zlib.crc32 IS CRC-32/ISO-HDLC from the stdlib, so there is no second derivation to drift. 0x49 gets the same function, before it touches the filesystem: a blob that would read back as EMPTY/CORRUPT must not replace one that is already there. Signature and device binding stay where they belong -- license_core is the only verifier; the runtime only transports. Also on 0x49: tmp + fsync + os.replace. open(path, "wb") truncates before writing, so ENOSPC or a power cut mid-activation destroyed the PREVIOUS, VALID license. An activation that fails must not leave the device worse off. Anchor normalization follows the C, which is canonical because it is the side that decides whether the license verifies: stop stripping TAB (rpi_plugin.c never did, while both comments claimed byte-identity -- an anchor ending in 0x09 derived a different deviceId on each side, so the purchased license simply never worked), and refuse an anchor above the 64-byte ceiling the C reads instead of framing up to 255 bytes that derive an identity the verifier cannot reproduce. Filesystem failures map to the status bytes the store already defines (IO_ERROR -> 0x82, TOO_LARGE -> 0x81) instead of raising -- a read-only SD card is the number-one failure mode of an industrial Pi, and an exception here reached the editor as a transport error indistinguishable from a dropped link, against this function's own "never raises" contract. Document the multi-plugin behaviour truthfully and WARN on more than one candidate. Saying "multi-plugin is unsupported" would be false: candidates filters on having a config_path, NOT on being licensable, so a single licensable VPP is enough for this to bite -- one free VPP ahead of it in vpp_plugins.conf and the license lands on the free plugin's sibling. Blob validation does not catch it either: the blob is valid, just in the wrong file. Tests: the round-trip test used to hand 0x49 98 bytes with a deliberately WRONG crc and assert SUCCESS -- it pinned the defect in place instead of catching it. It now uses the real signed golden blob from license-core/test, so the runtime is asserted against an artifact another implementation produced. Co-Authored-By: Claude Opus 5 --- .../pytest/plugins/test_vpp_license_debug.py | 369 +++++++++++++++++- webserver/vpp_license_debug.py | 258 ++++++++++-- 2 files changed, 599 insertions(+), 28 deletions(-) diff --git a/tests/pytest/plugins/test_vpp_license_debug.py b/tests/pytest/plugins/test_vpp_license_debug.py index 578d3721..4e666b02 100644 --- a/tests/pytest/plugins/test_vpp_license_debug.py +++ b/tests/pytest/plugins/test_vpp_license_debug.py @@ -4,8 +4,13 @@ (non-hex-decoded) anchor semantics that must match the .so (D70d), and the 0x49 write / 0x4A read round-trip landing on the .license sibling of the plugin config (same path the bundle + the .so use). + +Blob integrity is exercised against the REAL signed golden blob rather than a +made-up 98 bytes: see ``_GOLDEN_BLOB_HEX``. """ +import logging import os +import zlib import pytest @@ -19,6 +24,48 @@ def _hex(data: bytes) -> str: return " ".join(f"{b:02X}" for b in data) +# The real signed 98-byte license blob, copied verbatim from +# openplc-packages/license-core/test/license-golden-signed.json ("blobHex") -- +# the same vector license_core's host test and the editor/backend unit tests use +# (anchor 00b18ced -> deviceId 659a3520540f803625ddc34081e893d3, product +# 29a17c7c2486d355). Using it here means these tests assert the runtime against +# an artifact produced by ANOTHER implementation, not against bytes this file +# made up: if the magic or the crc32 range ever drifts on either side, this +# literal stops validating. +_GOLDEN_BLOB_HEX = ( + "4f504c430100659a3520540f803625ddc34081e893d329a17c7c2486d355" + "fbff79f73b679ce59fa93304507867e82d7b41b93acd98274dc48531299e" + "2215b1ae72881573bd800ba73a5c7731d13224772c90df806051d42d78b5" + "c97a046a2ae3a72d" +) + + +def _golden_blob() -> bytes: + blob = bytes.fromhex(_GOLDEN_BLOB_HEX) + assert len(blob) == 98 + return blob + + +def _blob_with_bad_crc() -> bytes: + """The golden blob with one payload byte flipped and the stored crc left + alone: right size, right magic, crc no longer covers the content. This is + what a torn write or a flipped flash cell produces.""" + blob = bytearray(_golden_blob()) + blob[40] ^= 0xFF + assert zlib.crc32(bytes(blob[:94])) != int.from_bytes(blob[94:98], "little") + return bytes(blob) + + +def _blob_without_magic() -> bytes: + """98 bytes whose first 4 are not `4F 50 4C 43`, with the crc recomputed so + that ONLY the magic is wrong -- otherwise this case would be + indistinguishable from the bad-crc one.""" + blob = bytearray(_golden_blob()) + blob[0:4] = b"\x00\x00\x00\x00" + blob[94:98] = zlib.crc32(bytes(blob[:94])).to_bytes(4, "little") + return bytes(blob) + + def _install_plugin(tmp_path, monkeypatch): """Fake one installed VPP plugin whose config_path lives under a temp cwd.""" cwd = tmp_path / "runtime" @@ -74,6 +121,11 @@ def test_get_board_id_missing_anchor_is_empty_success(tmp_path, monkeypatch): def test_write_refuses_path_traversal(tmp_path, monkeypatch): # A forged vpp_plugins.conf whose config_path escapes the runtime root must # NOT let 0x49 write outside it (defense-in-depth; mirrors apply_vpp_plugin_conf). + # + # The blob is the VALID golden one on purpose: with blob validation in front + # of the path resolution, an invalid blob would be refused before the guard + # was ever reached, and this test could no longer tell a working guard from a + # missing one. cwd = tmp_path / "runtime" cwd.mkdir() monkeypatch.chdir(cwd) @@ -89,7 +141,7 @@ class _Conf: monkeypatch.setattr(lic.PluginsConfiguration, "from_file", classmethod(lambda cls, _p: _Conf())) - cmd = _hex(bytes([0x49, 0x00, 0x62]) + bytes(98)) + cmd = _hex(bytes([0x49, 0x00, 0x62]) + _golden_blob()) assert lic.handle_license_command(cmd) == "49 85" # refused -> LIC_UNSUPPORTED assert not os.path.exists(tmp_path / "outside" / "evil.license") @@ -100,9 +152,16 @@ def test_read_license_empty_when_no_conf(tmp_path, monkeypatch): def test_write_then_read_roundtrip(tmp_path, monkeypatch): + """A blob that really validates round-trips as SUCCESS. + + This test used to hand 0x49 `magic + bytes(range(1,95))` -- 98 bytes with a + deliberately WRONG crc32 -- and assert SUCCESS on the read back. It pinned + the very defect that made a Pi with an unverifiable license report "Licensed" + to the editor: it locked the bug in place instead of catching it. The blob + here is the real signed golden one, so SUCCESS now means what it says. + """ config_path = _install_plugin(tmp_path, monkeypatch) - blob = bytes([0x4F, 0x50, 0x4C, 0x43]) + bytes(range(1, 95)) # 98 bytes - assert len(blob) == 98 + blob = _golden_blob() cmd = _hex(bytes([0x49, 0x00, 0x62]) + blob) # [0x49][len=98 u16BE][blob] assert lic.handle_license_command(cmd) == "49 7E" # SUCCESS @@ -126,15 +185,315 @@ def test_write_wrong_size_is_corrupt(tmp_path, monkeypatch): def test_write_without_installed_plugin_is_unsupported(tmp_path, monkeypatch): monkeypatch.chdir(tmp_path) # no vpp_plugins.conf - blob = bytes(98) - cmd = _hex(bytes([0x49, 0x00, 0x62]) + blob) + # Valid blob, so UNSUPPORTED can only come from the missing plugin config. + cmd = _hex(bytes([0x49, 0x00, 0x62]) + _golden_blob()) assert lic.handle_license_command(cmd) == "49 85" # LIC_UNSUPPORTED +# -------------------------------------------------------------------------- +# Blob integrity, both directions +# +# 0x4A used to test the LENGTH only. A 98-byte file that does not verify -- an +# SD card cloned from another Pi, corrupted flash, a torn write -- answered +# `4A 7E`, which the editor reads as "magic + crc32 verified", so it reported +# "Licensed" and returned BEFORE asking the backend for a fresh license. The one +# automatic repair path never ran precisely because the editor trusted the blob, +# while license_core refused it and the plugin dropped to demo. The same file on +# an ESP32 answers 0x83/0x84 and the editor recovers automatically. +# +# 0x49 wrote whatever it was handed, so 98 bytes of junk destroyed a valid +# license and answered SUCCESS. +# -------------------------------------------------------------------------- + + +def test_read_reports_corrupt_when_the_crc_does_not_verify(tmp_path, monkeypatch): + config_path = _install_plugin(tmp_path, monkeypatch) + with open(config_path[:-5] + ".license", "wb") as handle: + handle.write(_blob_with_bad_crc()) + + assert lic.handle_license_command("4A") == "4A 84" # LIC_CORRUPT, not SUCCESS + + +def test_read_reports_empty_when_the_magic_is_absent(tmp_path, monkeypatch): + config_path = _install_plugin(tmp_path, monkeypatch) + with open(config_path[:-5] + ".license", "wb") as handle: + handle.write(_blob_without_magic()) + + assert lic.handle_license_command("4A") == "4A 83" # LIC_EMPTY + + +def test_read_reports_empty_for_a_zeroed_license_file(tmp_path, monkeypatch): + """98 zero bytes is what a wiped or freshly-truncated file looks like; the + old length-only check answered `4A 7E 00 62` + 98 zeros.""" + config_path = _install_plugin(tmp_path, monkeypatch) + with open(config_path[:-5] + ".license", "wb") as handle: + handle.write(bytes(98)) + + assert lic.handle_license_command("4A") == "4A 83" # LIC_EMPTY (no magic) + + +def test_read_reports_corrupt_for_a_wrong_sized_license_file(tmp_path, monkeypatch): + config_path = _install_plugin(tmp_path, monkeypatch) + with open(config_path[:-5] + ".license", "wb") as handle: + handle.write(_golden_blob()[:97]) + + assert lic.handle_license_command("4A") == "4A 84" # LIC_CORRUPT + + +def test_write_refuses_a_blob_whose_crc_does_not_verify(tmp_path, monkeypatch): + _install_plugin(tmp_path, monkeypatch) + cmd = _hex(bytes([0x49, 0x00, 0x62]) + _blob_with_bad_crc()) + assert lic.handle_license_command(cmd) == "49 84" # LIC_CORRUPT + + +def test_write_refuses_a_blob_without_the_magic(tmp_path, monkeypatch): + _install_plugin(tmp_path, monkeypatch) + cmd = _hex(bytes([0x49, 0x00, 0x62]) + _blob_without_magic()) + assert lic.handle_license_command(cmd) == "49 83" # LIC_EMPTY + + +def test_a_refused_write_leaves_the_previous_license_untouched(tmp_path, monkeypatch): + """The destructive half: junk sent to 0x49 must not overwrite a good license. + + Observed before this change: a valid license followed by 98 bytes of junk + answered `49 7E` and the file became 98 zeros -- a remote, persistent + downgrade of a licensed device to demo. + """ + config_path = _install_plugin(tmp_path, monkeypatch) + license_path = config_path[:-5] + ".license" + good = _golden_blob() + assert lic.handle_license_command(_hex(bytes([0x49, 0x00, 0x62]) + good)) == "49 7E" + + junk = bytes(98) + assert lic.handle_license_command(_hex(bytes([0x49, 0x00, 0x62]) + junk)) == "49 83" + + with open(license_path, "rb") as handle: + assert handle.read() == good + assert lic.handle_license_command("4A").startswith("4A 7E 00 62") + + def test_non_license_fc_passes_through(): assert lic.handle_license_command("41 00 00 00 01") is None +# -------------------------------------------------------------------------- +# Atomicity and I/O failures +# -------------------------------------------------------------------------- + + +def test_a_failed_rename_leaves_the_previous_license_intact_and_no_debris(tmp_path, monkeypatch): + """The torn-write case: the write fails AFTER the new bytes are on disk. + + `open(path, "wb")` truncates before writing, so ENOSPC / a power cut / the + process dying mid-activation destroyed the PREVIOUS, VALID license and left + the device in demo on the next start. With tmp + replace the old license is + still the one at `path`, and the temporary is cleaned up. + """ + config_path = _install_plugin(tmp_path, monkeypatch) + license_path = config_path[:-5] + ".license" + good = _golden_blob() + assert lic.handle_license_command(_hex(bytes([0x49, 0x00, 0x62]) + good)) == "49 7E" + + def _boom(*_args, **_kwargs): + raise OSError(28, "No space left on device") + + monkeypatch.setattr(lic.os, "replace", _boom) + + other = bytearray(good) + other[6] ^= 0x01 # a different, still-valid-looking device_id + other[94:98] = zlib.crc32(bytes(other[:94])).to_bytes(4, "little") + resp = lic.handle_license_command(_hex(bytes([0x49, 0x00, 0x62]) + bytes(other))) + + assert resp == "49 82" # IO_ERROR, not an exception and not SUCCESS + with open(license_path, "rb") as handle: + assert handle.read() == good + leftovers = [p for p in os.listdir(os.path.dirname(license_path)) if p.endswith(".tmp")] + assert leftovers == [] + + +def test_write_maps_a_failing_filesystem_to_io_error(tmp_path, monkeypatch): + """A read-only SD card is the number-one failure mode of an industrial Pi. + + It used to propagate PermissionError out of handle_license_command -- against + that function's own "never raises for a well-formed license FC" contract -- + and the WebSocket layer flattened it into `{"success": false, "error": ...}`, + a shape the editor's PDU parser never sees as a status byte. Bare metal + answers 0x82 for exactly this. + """ + _install_plugin(tmp_path, monkeypatch) + + def _read_only(*_args, **_kwargs): + raise PermissionError(13, "Read-only file system") + + monkeypatch.setattr(lic.tempfile, "mkstemp", _read_only) + + cmd = _hex(bytes([0x49, 0x00, 0x62]) + _golden_blob()) + assert lic.handle_license_command(cmd) == "49 82" # IO_ERROR + + +def test_read_maps_an_unreadable_license_to_io_error(tmp_path, monkeypatch): + config_path = _install_plugin(tmp_path, monkeypatch) + license_path = config_path[:-5] + ".license" + with open(license_path, "wb") as handle: + handle.write(_golden_blob()) + + real_open = open + + def _fake_open(path, *args, **kwargs): + if str(path) == license_path: + raise PermissionError(13, "Permission denied") + return real_open(path, *args, **kwargs) + + monkeypatch.setattr("builtins.open", _fake_open) + + assert lic.handle_license_command("4A") == "4A 82" # IO_ERROR + + +# -------------------------------------------------------------------------- +# Anchor normalization (0x48) +# +# The C is canonical: rpi_plugin.c is the side that decides whether the license +# verifies. Byte-for-byte parity with the real C source is pinned separately, by +# test_vpp_anchor_cross_language.py; these are the wire-level consequences. +# -------------------------------------------------------------------------- + + +def test_anchor_keeps_a_trailing_tab(tmp_path, monkeypatch): + """TAB is NOT in the C's strip list, so it must not be in ours either. + + An anchor ending in 0x09 stripped here but not there derives a different + device_id on each side: sha256("openplc-dev-v1|" + "8625807b0a83ae7d\\t")[:16] + is ac07623afa23c771..., while the .so computes 7146518f9842adac... The + purchased license would simply never work, with nothing in any log. + """ + anchor_file = tmp_path / "serial-number" + anchor_file.write_bytes(b"8625807b0a83ae7d\t\x00") + monkeypatch.setattr(lic, "ANCHOR_PATH", str(anchor_file)) + + parts = lic.handle_license_command("48").split() + assert parts[:2] == ["48", "7E"] + assert parts[2] == "11" # 17 bytes: the TAB is still there + assert bytes(int(p, 16) for p in parts[3:]) == b"8625807b0a83ae7d\t" + + +def test_anchor_strip_set_is_exactly_the_four_bytes_the_c_strips(): + assert lic.ANCHOR_STRIP_BYTES == b"\x00\r\n " + assert b"\t" not in lic.ANCHOR_STRIP_BYTES + + +def test_anchor_at_the_ceiling_is_served(tmp_path, monkeypatch): + anchor_file = tmp_path / "serial-number" + anchor_file.write_bytes(b"a" * 64 + b"\x00") + monkeypatch.setattr(lic, "ANCHOR_PATH", str(anchor_file)) + + parts = lic.handle_license_command("48").split() + assert parts[:3] == ["48", "7E", "40"] # 0x40 == 64 + assert bytes(int(p, 16) for p in parts[3:]) == b"a" * 64 + + +def test_anchor_over_the_ceiling_is_refused_not_truncated(tmp_path, monkeypatch): + """rpi_plugin.c reads `uint8_t anchor[64]`, so the verifier never sees more. + + Sending 65+ bytes on the wire would have the editor hash all of them and the + .so hash the first 64 -> DEVICE_MISMATCH -> demo, with a license bought + against an identity that can never validate. Refuse instead; never truncate + silently, in either direction. + """ + anchor_file = tmp_path / "serial-number" + anchor_file.write_bytes(b"b" * 65) + monkeypatch.setattr(lic, "ANCHOR_PATH", str(anchor_file)) + + assert lic.handle_license_command("48") == "48 81" # TOO_LARGE, no id bytes + + +def test_anchor_padding_past_the_ceiling_still_serves_the_stripped_value(tmp_path, monkeypatch): + """A file longer than 64 bytes whose tail is all strippable is NOT an error: + the C reads 64 bytes and strips the padding out of them, landing on exactly + the same value this side computes from the whole file.""" + anchor_file = tmp_path / "serial-number" + anchor_file.write_bytes(b"c" * 20 + b"\x00" * 60) + monkeypatch.setattr(lic, "ANCHOR_PATH", str(anchor_file)) + + parts = lic.handle_license_command("48").split() + assert parts[:3] == ["48", "7E", "14"] # 20 bytes + assert bytes(int(p, 16) for p in parts[3:]) == b"c" * 20 + + +# -------------------------------------------------------------------------- +# Multi-plugin: documented behaviour + a trace in the log +# -------------------------------------------------------------------------- + + +def test_more_than_one_candidate_writes_the_first_and_warns(tmp_path, monkeypatch): + """0x49 acts on the FIRST plugin carrying a config_path. + + `candidates` filters on having a config_path, NOT on being licensable, so a + single licensable VPP is enough for this to bite: one free VPP ahead of it in + vpp_plugins.conf and the license lands on the free plugin's sibling, while + 0x4A reads that same wrong file back and reports SUCCESS. Disambiguating for + real is out of scope by decision; a WARN is what makes the day it bites + leave a trace. + """ + cwd = tmp_path / "runtime" + (cwd / "build" / "vpp").mkdir(parents=True) + monkeypatch.chdir(cwd) + (cwd / "vpp_plugins.conf").write_text("dummy\n") + free_config = str(cwd / "build" / "vpp" / "free_gpio.json") + paid_config = str(cwd / "build" / "vpp" / "paid_rpi.json") + + class _P: + def __init__(self, name, cp): + self.name = name + self.config_path = cp + + class _Conf: + plugins = [_P("free_gpio", free_config), _P("paid_rpi", paid_config)] + + monkeypatch.setattr(lic.PluginsConfiguration, "from_file", classmethod(lambda cls, _p: _Conf())) + + records = [] + + class _Capture(logging.Handler): + def emit(self, record): + records.append(record) + + handler = _Capture() + lic.logger.addHandler(handler) + try: + cmd = _hex(bytes([0x49, 0x00, 0x62]) + _golden_blob()) + assert lic.handle_license_command(cmd) == "49 7E" + finally: + lic.logger.removeHandler(handler) + + # Documented behaviour: first candidate wins, the second gets nothing. + assert os.path.exists(free_config[:-5] + ".license") + assert not os.path.exists(paid_config[:-5] + ".license") + # ...and it is not silent. + warnings = [r for r in records if r.levelno >= logging.WARNING] + assert warnings, "more than one candidate must WARN" + assert "free_gpio" in warnings[0].getMessage() + + +def test_a_single_candidate_does_not_warn(tmp_path, monkeypatch): + _install_plugin(tmp_path, monkeypatch) + + records = [] + + class _Capture(logging.Handler): + def emit(self, record): + records.append(record) + + handler = _Capture() + lic.logger.addHandler(handler) + try: + cmd = _hex(bytes([0x49, 0x00, 0x62]) + _golden_blob()) + assert lic.handle_license_command(cmd) == "49 7E" + finally: + lic.logger.removeHandler(handler) + + assert [r for r in records if r.levelno >= logging.WARNING] == [] + + # -------------------------------------------------------------------------- # Containment guard (is_inside_root) # diff --git a/webserver/vpp_license_debug.py b/webserver/vpp_license_debug.py index 5fb44d3d..12c4c89a 100644 --- a/webserver/vpp_license_debug.py +++ b/webserver/vpp_license_debug.py @@ -17,14 +17,25 @@ 0x49 write-license: req [0x49][len:u16BE][blob] resp [0x49][status] 0x4A read-license : req [0x4A] resp [0x4A][status][len:u16BE][blob] -Anchor bytes are returned RAW (ASCII, trailing NUL/whitespace stripped) so the -editor derives the SAME device_id the .so does (D70d); no hex-decoding. +Anchor bytes are returned RAW (ASCII, trailing NUL/CR/LF/space stripped) so the +editor derives the SAME device_id the .so does (D70d); no hex-decoding. The +canonical normalization is the C one (``rpi_plugin.c``) because the C is what +decides whether a license verifies -- see ``ANCHOR_STRIP_BYTES`` below. + +Blob integrity is checked on BOTH directions (0x49 and 0x4A) with the same +function, mirroring what ``license_store_read`` validates on bare metal so the +two targets answer the same bytes for the same file. """ import os +import tempfile +import zlib from typing import Optional +from webserver.logger import get_logger from webserver.plugin_config_model import PluginsConfiguration +logger, _ = get_logger("vpp_license", use_buffer=True) + # License function codes (mirror simulator/types.ts + firmware modbus_types.h). FC_GET_BOARD_ID = 0x48 FC_WRITE_LICENSE = 0x49 @@ -33,6 +44,12 @@ # Status bytes (shared with the Arduino firmware / editor). ST_SUCCESS = 0x7E +# 0x81/0x82 are MB_DEBUG_ERROR_OUT_OF_BOUNDS / MB_DEBUG_ERROR_OUT_OF_MEMORY, +# which license_store.h:44-49 REUSES for LIC_STORE_TOO_LARGE / LIC_STORE_IO_ERROR. +# The bare-metal store already answers these two and the editor already parses +# them (modbus-pdu.ts statusError), so emitting them here adds no ABI. +ST_LIC_TOO_LARGE = 0x81 +ST_LIC_IO_ERROR = 0x82 ST_LIC_EMPTY = 0x83 ST_LIC_CORRUPT = 0x84 ST_LIC_UNSUPPORTED = 0x85 @@ -41,6 +58,28 @@ VPP_CONF = "vpp_plugins.conf" LIC_BLOB_SIZE = 98 +# Bytes stripped from the END of the raw anchor: NUL, CR, LF and SPACE -- and +# ONLY those four, because that is the list in rpi_plugin.c:103-107, and the C +# is canonical (it is the side that decides whether the license verifies). +# TAB used to be in this list and never was in the C one, while both comments +# claimed byte-identity: an anchor ending in 0x09 derived a DIFFERENT device_id +# here than on the .so, so the purchased license silently never worked. Do NOT +# add bytes "for safety" -- every byte in this set changes the device_id. +# Parity is pinned by tests/pytest/plugins/test_vpp_anchor_cross_language.py, +# which executes the real C. +ANCHOR_STRIP_BYTES = b"\x00\r\n " +# rpi_plugin.c:99 reads the anchor into `uint8_t anchor[64]`, so the .so never +# sees more than 64 bytes. Refuse a longer anchor instead of putting bytes on +# the wire that would derive a device_id the .so cannot reproduce (it would +# hash the first 64; the editor would hash all of them -> DEVICE_MISMATCH -> +# demo). Never truncate silently. +ANCHOR_MAX_BYTES = 64 + +# Blob layout (contract/firmware/license_blob.h, license-blob.ts): LE u32 magic +# at 0, CRC-32/ISO-HDLC over bytes [0..93] stored as a LE u32 at 94. +LIC_MAGIC_LE = 0x434C504F # bytes 4F 50 4C 43 +LIC_OFF_CRC32 = 94 + def is_license_command(command_hex: str) -> bool: """True when the PDU's first byte is a license function code.""" @@ -67,9 +106,44 @@ def _read_anchor() -> bytes: raw = handle.read() except OSError: return b"" - # Strip trailing NUL / whitespace -- MUST match derive in rpi_plugin.c - # (the device-tree serial is NUL-terminated). - return raw.rstrip(b"\x00\r\n\t ") + # Strip trailing NUL / CR / LF / SPACE -- exactly the four bytes + # rpi_plugin.c strips, no more (the device-tree serial is NUL-terminated). + return raw.rstrip(ANCHOR_STRIP_BYTES) + + +def validate_license_blob(blob: bytes) -> Optional[int]: + """``None`` when the blob is one the closed verifier could accept; otherwise + the status byte to answer with. + + Same checks, same order, same status bytes as ``license_store_read`` on bare + metal (``license_store.h:24-26``, ``license_store_esp32.cpp:59-68``): wrong + size -> CORRUPT, magic absent -> EMPTY, crc32 mismatch -> CORRUPT. + + Why this matters on the Linux path specifically: 0x4A used to test the + length ONLY, so a 98-byte file that does not verify (an SD card cloned from + another Pi, a corrupted flash, a torn write) answered SUCCESS. The editor + reads SUCCESS as "a valid license blob is present and intact (magic + crc32)" + and returns 'already-licensed' BEFORE talking to the backend -- so the one + automatic repair path (recover a fresh license) never ran, precisely because + the editor believed the blob was good, while license_core refused it and the + plugin dropped to demo and stopped actuating 15 minutes later. + + This is NOT a verdict on the license: signature and device binding are the + closed license-core's job and it is the only verifier (the runtime only + transports). These are the checks that need no key and no anchor, which is + exactly the set the bare-metal store already performs. + + ``zlib.crc32`` IS CRC-32/ISO-HDLC from the stdlib -- not a reimplementation + of ours, so there is no second derivation to drift. + """ + if len(blob) != LIC_BLOB_SIZE: + return ST_LIC_CORRUPT + if int.from_bytes(blob[0:4], "little") != LIC_MAGIC_LE: + return ST_LIC_EMPTY + stored = int.from_bytes(blob[LIC_OFF_CRC32 : LIC_OFF_CRC32 + 4], "little") + if zlib.crc32(blob[:LIC_OFF_CRC32]) != stored: + return ST_LIC_CORRUPT + return None def derive_license_path(config_path: str) -> str: @@ -144,11 +218,30 @@ def resolve_license_path(config_path: str, runtime_root: Optional[str] = None) - def _license_path() -> Optional[str]: - """The ``.license`` sibling of the installed licensed plugin's config_path, - mirroring ``apply_vpp_plugin_conf`` so 0x49 and the bundle write the SAME - file the .so reads. None when no VPP plugin config is installed yet (no - upload). Multi-plugin disambiguation by vppId is a future extension (the - PDU carries no plugin id); the common case is one VPP plugin. + """The ``.license`` sibling of the FIRST installed plugin that carries a + ``config_path``, mirroring ``apply_vpp_plugin_conf`` so 0x49 and the bundle + write the SAME file the .so reads. None when no VPP plugin config is + installed yet (no upload). + + KNOWN LIMITATION -- documented, deliberately not fixed here. The PDU carries + no plugin id, so 0x49 and 0x4A always act on ``candidates[0]``. Note what + ``candidates`` filters on: having a ``config_path``, NOT being licensable. + So this bites with a SINGLE licensable VPP installed -- one FREE VPP ahead of + it in ``vpp_plugins.conf`` is enough for the license to land on the free + plugin's ``.license``, after which 0x4A reads that same wrong file back and + reports SUCCESS while the licensed plugin stays in demo and stops actuating. + **Install the licensable VPP alone, or first in the list.** + + Note that the blob validation above does NOT catch this case: the blob is + perfectly valid, it is just in the wrong file. + + Disambiguating for real is out of scope by decision: it needs either a plugin + id on the wire (the PDU is frozen) or ``licensable``/``vppId`` on + ``PluginConfig``, which carries neither -- so the runtime has no + ``name``->``vppId`` mapping and cannot even compare the blob's ``product_id``. + Failing closed on >1 candidate was rejected too: it would block a device that + legitimately runs a free VPP alongside a paid one. What is left is a WARN, so + that the day this bites leaves a trace in the log. """ if not os.path.exists(VPP_CONF): return None @@ -159,14 +252,90 @@ def _license_path() -> Optional[str]: candidates = [p for p in conf.plugins if getattr(p, "config_path", None)] if not candidates: return None + if len(candidates) > 1: + logger.warning( + "vpp_plugins.conf lists %d plugins with a config_path; license FCs " + "(0x49/0x4A) act on the FIRST one (%s). If the licensable VPP is not " + "that one, its license lands on the wrong file and 0x4A still reports " + "SUCCESS while the plugin runs in demo. Install the licensable VPP " + "alone, or first in the list. Candidates: %s", + len(candidates), + getattr(candidates[0], "name", "?"), + [getattr(p, "name", "?") for p in candidates], + ) return resolve_license_path(candidates[0].config_path) +def _fsync_directory(directory: str) -> None: + """Best-effort fsync of ``directory`` so the RENAME itself is durable. + + Without it the replaced name can still be lost to a power cut even though + the file contents were fsynced. Not available on every platform (Windows + refuses to open a directory), and a failure here does not undo a rename that + already succeeded -- so every error is swallowed on purpose. + """ + try: + fd = os.open(directory, os.O_RDONLY) + except OSError: + return + try: + os.fsync(fd) + except OSError: + pass + finally: + os.close(fd) + + +def _write_license_atomically(path: str, blob: bytes) -> None: + """Write ``blob`` to ``path`` so that a failure NEVER destroys the license + that is already there. Raises ``OSError`` for the caller to map. + + ``open(path, "wb")`` truncates before writing, so ENOSPC, a power cut or the + process dying mid-activation would leave a partial file -- destroying the + PREVIOUS, VALID license and dropping the device to demo on the next start. + An activation that fails must not leave the device worse than it was. + + A temporary sibling plus ``os.replace`` makes the swap atomic within the + filesystem; the ``fsync`` before it is what makes the bytes durable rather + than merely visible. ``mkstemp`` (not a fixed ``.tmp`` name) so two writers + cannot interleave into the same temporary and then rename the mess over a + good license. + """ + directory = os.path.dirname(path) or "." + os.makedirs(directory, exist_ok=True) + fd, tmp_path = tempfile.mkstemp(dir=directory, prefix=".license-", suffix=".tmp") + try: + with os.fdopen(fd, "wb") as handle: + handle.write(blob) + handle.flush() + os.fsync(handle.fileno()) + # mkstemp creates 0600; the license is public signed data and the bundle + # delivery path writes it under the default mask -- keep the two alike so + # the .so reads the same file either way. + os.chmod(tmp_path, 0o644) + os.replace(tmp_path, path) + except OSError: + # Leave no debris behind, and leave the previous license untouched. + try: + os.unlink(tmp_path) + except OSError: + pass + raise + _fsync_directory(directory) + + def handle_license_command(command_hex: str) -> Optional[str]: """Resolve a license function code and return the response as spaced hex. Returns ``None`` when the command is not a license FC, so the caller forwards - it to the C core as before. Never raises for a well-formed license FC. + it to the C core as before. + + Never raises for a well-formed license FC -- and now actually keeps that + promise: filesystem failures are mapped to the status bytes the bare-metal + store already answers (IO_ERROR -> 0x82, TOO_LARGE -> 0x81) instead of + propagating an exception that the WebSocket layer would flatten into a + ``{"success": false}`` envelope, which the editor's PDU parser never sees as + a status byte at all. """ data = _bytes_from_hex(command_hex) if not data or data[0] not in _LICENSE_FCS: @@ -179,19 +348,47 @@ def handle_license_command(command_hex: str) -> Optional[str]: # Match the Arduino firmware (D57): no id -> SUCCESS with id_len=0, so # the editor sees a clean empty id (outcome no-id), not an error byte. return _hex_from_bytes(bytes([fc, ST_SUCCESS, 0])) - length = min(len(anchor), 255) - return _hex_from_bytes(bytes([fc, ST_SUCCESS, length]) + anchor[:length]) + if len(anchor) > ANCHOR_MAX_BYTES: + # REFUSE. The .so only ever reads 64 bytes, so anything longer would + # make the editor derive a device_id the verifier cannot reproduce -- + # the license would be bought against an identity that never + # validates. An error byte is recoverable; a wrong device_id is not. + logger.error( + "Anchor at %s is %d bytes after normalization, over the %d-byte " + "ceiling the license verifier reads; refusing 0x48 rather than " + "returning bytes that derive a device_id the verifier cannot " + "reproduce.", + ANCHOR_PATH, + len(anchor), + ANCHOR_MAX_BYTES, + ) + return _hex_from_bytes(bytes([fc, ST_LIC_TOO_LARGE])) + return _hex_from_bytes(bytes([fc, ST_SUCCESS, len(anchor)]) + anchor) if fc == FC_READ_LICENSE: path = _license_path() if not path or not os.path.exists(path): return _hex_from_bytes(bytes([fc, ST_LIC_EMPTY])) - with open(path, "rb") as handle: - blob = handle.read() - if len(blob) != LIC_BLOB_SIZE: - return _hex_from_bytes(bytes([fc, ST_LIC_CORRUPT])) - length = len(blob) - header = bytes([fc, ST_SUCCESS, (length >> 8) & 0xFF, length & 0xFF]) + try: + with open(path, "rb") as handle: + blob = handle.read() + except OSError as exc: + # A read-only or failing SD card is the number-one failure mode of an + # industrial Pi. It has a status byte on bare metal (IO_ERROR -> + # 0x82); raising here instead would surface as a generic transport + # error the editor cannot tell from a dropped link. + logger.error("0x4A could not read %s: %s", path, exc) + return _hex_from_bytes(bytes([fc, ST_LIC_IO_ERROR])) + bad_status = validate_license_blob(blob) + if bad_status is not None: + logger.warning( + "0x4A: license at %s failed validation (%d bytes) -> status 0x%02X", + path, + len(blob), + bad_status, + ) + return _hex_from_bytes(bytes([fc, bad_status])) + header = bytes([fc, ST_SUCCESS, (LIC_BLOB_SIZE >> 8) & 0xFF, LIC_BLOB_SIZE & 0xFF]) return _hex_from_bytes(header + blob) if fc == FC_WRITE_LICENSE: @@ -200,14 +397,29 @@ def handle_license_command(command_hex: str) -> Optional[str]: return _hex_from_bytes(bytes([fc, ST_LIC_CORRUPT])) length = (data[1] << 8) | data[2] blob = data[3 : 3 + length] - if len(blob) != length or length != LIC_BLOB_SIZE: + if len(blob) != length: return _hex_from_bytes(bytes([fc, ST_LIC_CORRUPT])) + # Validate BEFORE touching the filesystem, with the same function 0x4A + # uses: a blob that would read back as EMPTY/CORRUPT must never replace a + # license that is already there. The size check that used to live here is + # the first check inside validate_license_blob, and answers the same + # 0x84. + bad_status = validate_license_blob(blob) + if bad_status is not None: + logger.warning( + "0x49 refused a %d-byte blob that does not validate -> status 0x%02X", + len(blob), + bad_status, + ) + return _hex_from_bytes(bytes([fc, bad_status])) path = _license_path() if not path: return _hex_from_bytes(bytes([fc, ST_LIC_UNSUPPORTED])) - os.makedirs(os.path.dirname(path) or ".", exist_ok=True) - with open(path, "wb") as handle: - handle.write(blob) + try: + _write_license_atomically(path, blob) + except OSError as exc: + logger.error("0x49 could not write %s: %s", path, exc) + return _hex_from_bytes(bytes([fc, ST_LIC_IO_ERROR])) return _hex_from_bytes(bytes([fc, ST_SUCCESS])) return None From 13758d8a9f8476a75246c4f16a6e26bc95827168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Thu, 30 Jul 2026 13:58:58 +0200 Subject: [PATCH 07/10] test(vpp): pin the anchor normalization against the real C, not a transcription The anchor is the pre-image of the licensing identity: the editor hashes the bytes 0x48 returns into the deviceId a license is signed for, and rpi_plugin.c hashes its own normalization of the same file into what license_core compares against. Both sides carried a comment asserting byte-identity with the other and neither had it. This does not re-implement the C in Python and compare that to itself, which is the weakness the delivery test admits to in its own docstring. It extracts the REAL strip set, the REAL anchor[64] declaration, the REAL read_file_bytes and the REAL normalization loop out of rpi_plugin.c by source text, then (a) asserts the Python constants equal the C ones -- no compiler needed, so a TAB creeping back into the strip set fails anywhere -- and (b) compiles the extracted C unmodified and compares its output byte for byte with _read_anchor() over an edge-case table: the measured Pi 5 shape, trailing TAB, a TAB behind a space, mixed NUL/CR/LF/space tails, interior NULs, degenerate tails, exactly-at-ceiling, and padding past the ceiling. Same approach as the tree-digest test. The one case where the two CANNOT agree is asserted as a refusal: above the C's 64-byte buffer the C truncates, so nothing goes on the wire at all. The C source lives in the sibling openplc-packages repository, so this file skips on a runtime-only checkout (OPENPLC_PACKAGES_DIR overrides the lookup). That is a real gap of the same kind as the symlink case, and the mirror of this test belongs in openplc-packages, where the source is always present. Co-Authored-By: Claude Opus 5 --- .../plugins/test_vpp_anchor_cross_language.py | 309 ++++++++++++++++++ 1 file changed, 309 insertions(+) create mode 100644 tests/pytest/plugins/test_vpp_anchor_cross_language.py diff --git a/tests/pytest/plugins/test_vpp_anchor_cross_language.py b/tests/pytest/plugins/test_vpp_anchor_cross_language.py new file mode 100644 index 00000000..5ac65c65 --- /dev/null +++ b/tests/pytest/plugins/test_vpp_anchor_cross_language.py @@ -0,0 +1,309 @@ +"""Cross-language contract test for the licensing ANCHOR normalization. + +Why this exists +--------------- +``webserver/vpp_license_debug.py`` normalizes the raw hardware anchor before +putting it on the wire for FC 0x48, and the editor hashes exactly those bytes +into the ``deviceId`` a license is signed for. The plugin's C +(``rpi_plugin.c::license_gate_bringup``) normalizes the SAME file, on the SAME +board, and hands its result to the closed ``license_core``, which is the side +that decides whether the license verifies. The C is therefore canonical. + +Both sides carried a comment claiming byte-identity with the other, and both were +wrong: the Python stripped a trailing TAB and the C never did, so an anchor +ending in 0x09 derived a different ``deviceId`` on each side -- +``sha256("openplc-dev-v1|" + "8625807b0a83ae7d\\t")[:16]`` is +``ac07623afa23c771...`` where the C computes ``7146518f9842adac...``. Nothing +would log, nothing would fail: the customer pays and the license simply never +works. The C also reads into ``uint8_t anchor[64]`` and silently truncates, +while the Python read the whole file and framed up to 255 bytes on the wire. + +How this test stays honest +-------------------------- +It does NOT re-implement the C normalization in Python and compare that against +itself -- that would only prove this file agrees with its own assumptions. There +is already one test in this suite that does the weaker thing on purpose and says +so (``test_vpp_license_delivery.py``'s hand transcription of +``derive_license_path``). This file instead: + +1. extracts the REAL strip set and the REAL buffer size out of the C source by + source text, and asserts the Python constants equal them; and +2. extracts the REAL ``read_file_bytes`` function and the REAL anchor block + (declaration, read, and normalization loop), compiles them unmodified, and + compares the bytes the C produces with the bytes ``_read_anchor()`` produces, + over the same files. + +If either side's source changes, this test runs the NEW text, not a stale copy. + +Where the C lives +----------------- +The plugin C source is in the sibling ``openplc-packages`` repository, not in +this one, so this test resolves it and SKIPS when it cannot be found. Point +``OPENPLC_PACKAGES_DIR`` at a checkout to run it from elsewhere. This is a real +limitation -- on a runtime-only CI checkout this file skips, exactly like the +symlink case in ``test_vpp_license_debug.py`` -- and the mirror of this test +belongs in ``openplc-packages``, where the C source is always present. +""" + +import os +import re +import shutil +import subprocess +import tempfile + +import pytest + +lic = pytest.importorskip( + "webserver.vpp_license_debug", + reason="runtime webserver package not importable (no venv)", +) + +_REPO_ROOT = os.path.normpath(os.path.join(os.path.dirname(__file__), "..", "..", "..")) +_RPI_PLUGIN_REL = os.path.join( + "packages", + "com.openplc.raspberry-pi-licensed", + "hal", + "runtime-v4", + "source", + "rpi_plugin.c", +) + + +def _find_rpi_plugin_source(): + """The licensed Pi plugin's C source, or None. + + Checked in order: an explicit OPENPLC_PACKAGES_DIR, then a sibling + ``openplc-packages`` checkout next to this repository. + """ + roots = [] + env_root = os.environ.get("OPENPLC_PACKAGES_DIR") + if env_root: + roots.append(env_root) + roots.append(os.path.join(os.path.dirname(_REPO_ROOT), "openplc-packages")) + for root in roots: + candidate = os.path.join(root, _RPI_PLUGIN_REL) + if os.path.isfile(candidate): + return candidate + return None + + +_RPI_PLUGIN_C = _find_rpi_plugin_source() +_CC = next((c for c in ("cc", "gcc", "clang") if shutil.which(c)), None) + +pytestmark = pytest.mark.skipif( + _RPI_PLUGIN_C is None, + reason=( + "rpi_plugin.c not found: this test needs the sibling openplc-packages " + "checkout (or OPENPLC_PACKAGES_DIR) because the canonical anchor " + "normalization lives there, not in this repo" + ), +) + + +def _c_source() -> str: + # newline=None gives universal-newline translation; strip any stray \r on + # top of it, because this repo has no .gitattributes and a Windows checkout + # can hand us CRLF where the extraction patterns expect bare \n (tasks + # #50/#58). Normalizing line endings for matching does not change what the + # C does. + with open(_RPI_PLUGIN_C, "r", encoding="utf-8", newline=None) as handle: + return handle.read().replace("\r", "") + + +def _extract(pattern: str, what: str) -> str: + """Pull a chunk of real C out of the source by exact text. + + Raises rather than returning an empty string: a test that "passes" because + it extracted nothing and compared nothing would be worse than no test. + """ + match = re.compile(pattern, re.MULTILINE | re.DOTALL).search(_c_source()) + if not match: + raise AssertionError( + f"could not locate {what} in {_RPI_PLUGIN_C} -- the extraction " + "pattern in this test no longer matches the real source, which " + "means this test is not exercising real code" + ) + return match.group(0) + + +_ANCHOR_BLOCK_PATTERN = r"^ uint8_t anchor\[\d+\];$.*?^ \}$" +_READ_FILE_BYTES_PATTERN = r"^static long read_file_bytes\(.*?^\}$" + + +# --------------------------------------------------------------------------- +# 1. The constants, read out of the real C source. No compiler needed. +# --------------------------------------------------------------------------- + + +def test_python_strips_exactly_the_bytes_the_c_strips(): + """The strip set is the whole contract: one extra byte on either side moves + the deviceId and the purchased license stops matching the hardware.""" + block = _extract(_ANCHOR_BLOCK_PATTERN, "the anchor normalization block") + # Every character literal compared against anchor[alen - 1] in the real loop. + comparisons = re.findall(r"anchor\[alen - 1\] == '((?:\\.|[^'\\])+)'", block) + assert comparisons, f"no strip comparisons found in the extracted C:\n{block}" + + escapes = {"\\0": b"\x00", "\\n": b"\n", "\\r": b"\r", "\\t": b"\t", " ": b" "} + c_strip_set = set() + for literal in comparisons: + assert literal in escapes, f"unhandled C character literal {literal!r}" + c_strip_set.add(escapes[literal]) + + assert c_strip_set == {bytes([b]) for b in lic.ANCHOR_STRIP_BYTES}, ( + f"C strips {sorted(c_strip_set)}, Python strips " + f"{sorted(bytes([b]) for b in lic.ANCHOR_STRIP_BYTES)}. These MUST be " + "the same four bytes -- any difference changes the derived deviceId and " + "the license signed for this board stops verifying, silently." + ) + + +def test_python_anchor_ceiling_matches_the_c_buffer(): + block = _extract(_ANCHOR_BLOCK_PATTERN, "the anchor normalization block") + size = int(re.search(r"uint8_t anchor\[(\d+)\];", block).group(1)) + assert size == lic.ANCHOR_MAX_BYTES, ( + f"the C reads the anchor into uint8_t anchor[{size}] but this runtime " + f"caps at {lic.ANCHOR_MAX_BYTES}. The C never sees more than its buffer, " + "so anything above the cap must be refused here, not truncated." + ) + + +# --------------------------------------------------------------------------- +# 2. The behaviour, by executing the real C. Needs a compiler. +# --------------------------------------------------------------------------- + +_HARNESS = """\ +#include +#include +#include +#include + +/* The anchor block below reads RPI_ANCHOR_PATH; point it at argv[1] so the same + real code can be run against a temp file instead of /proc. */ +#define RPI_ANCHOR_PATH argv[1] + +%(read_file_bytes)s + +int main(int argc, char **argv) +{ + if (argc < 2) return 2; +%(anchor_block)s + for (long i = 0; i < alen; i++) printf("%%02x", anchor[i]); + printf("\\n"); + return 0; +} +""" + + +def _build_c_normalizer(workdir: str) -> str: + source = _HARNESS % { + "read_file_bytes": _extract(_READ_FILE_BYTES_PATTERN, "read_file_bytes()"), + "anchor_block": _extract(_ANCHOR_BLOCK_PATTERN, "the anchor normalization block"), + } + src_path = os.path.join(workdir, "anchor_harness.c") + with open(src_path, "w", encoding="utf-8", newline="\n") as handle: + handle.write(source) + exe_path = os.path.join(workdir, "anchor_harness") + result = subprocess.run( + [_CC, "-std=c99", "-Wall", "-o", exe_path, src_path], + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, ( + "the extracted C did not compile -- the extraction is picking up text " + f"that is not the real function/block:\n{result.stdout}\n{result.stderr}\n" + f"--- generated source ---\n{source}" + ) + return exe_path + + +def _c_normalize(exe_path: str, workdir: str, raw: bytes) -> bytes: + anchor_file = os.path.join(workdir, "serial-number") + with open(anchor_file, "wb") as handle: + handle.write(raw) + result = subprocess.run([exe_path, anchor_file], capture_output=True, text=True, check=False) + assert result.returncode == 0, f"C harness exited {result.returncode}: {result.stderr}" + return bytes.fromhex(result.stdout.strip()) + + +def _python_normalize(monkeypatch, workdir: str, raw: bytes) -> bytes: + anchor_file = os.path.join(workdir, "serial-number-py") + with open(anchor_file, "wb") as handle: + handle.write(raw) + monkeypatch.setattr(lic, "ANCHOR_PATH", anchor_file) + return lic._read_anchor() + + +# Raw anchor files, and why each one is here. +PARITY_CASES = { + # The Pi 5 shape actually measured on hardware: ASCII hex + trailing NUL. + "pi5_serial_with_nul": b"8625807b0a83ae7d\x00", + # The case that was PROVEN divergent: a trailing TAB the C keeps. + "trailing_tab": b"8625807b0a83ae7d\t", + "trailing_tab_then_nul": b"8625807b0a83ae7d\t\x00", + # A TAB behind a space: the loop must stop at the TAB, so the space stays. + "space_then_tab": b"8625807b0a83ae7d \t", + # All four strippable bytes, in a mixed tail. + "mixed_trailing_whitespace": b"8625807b0a83ae7d \r\n\x00", + # Interior NULs must survive; only the tail is stripped. + "interior_nul": b"abc\x00def\x00", + # Degenerate tails. + "all_nul": b"\x00\x00\x00\x00", + "empty": b"", + "single_byte": b"Z", + # Exactly at the C buffer size, with and without padding. + "exactly_at_ceiling": b"a" * 64, + "at_ceiling_plus_padding": b"b" * 20 + b"\x00" * 60, + "padding_only_past_ceiling": b"c" * 63 + b"\x00" * 200, + # Non-ASCII bytes: neither side may interpret or transcode them. + "high_bytes": bytes([0x00, 0xB1, 0x8C, 0xED, 0x00]), +} + + +@pytest.mark.skipif(_CC is None, reason="no C compiler (cc/gcc/clang) on PATH") +@pytest.mark.parametrize("case", sorted(PARITY_CASES)) +def test_python_and_c_normalize_the_anchor_identically(case, monkeypatch): + raw = PARITY_CASES[case] + workdir = tempfile.mkdtemp(prefix="vpp-anchor-cross-lang-") + try: + exe = _build_c_normalizer(workdir) + from_c = _c_normalize(exe, workdir, raw) + from_python = _python_normalize(monkeypatch, workdir, raw) + assert from_python == from_c, ( + f"case {case!r}: python -> {from_python!r}, C -> {from_c!r}. " + "These MUST be identical: the editor hashes the python bytes into " + "the deviceId a license is signed for, and license_core hashes the " + "C bytes to decide whether that license is valid." + ) + finally: + shutil.rmtree(workdir, ignore_errors=True) + + +@pytest.mark.skipif(_CC is None, reason="no C compiler (cc/gcc/clang) on PATH") +def test_an_anchor_longer_than_the_c_buffer_is_refused_rather_than_diverging(monkeypatch): + """The one case where the two sides CANNOT agree, and what we do about it. + + The C reads 64 bytes and hashes those; a longer anchor would have this side + hash more, so the two deviceIds differ by construction. Rather than serve + bytes that derive an identity the verifier can never reproduce, 0x48 refuses + with TOO_LARGE (0x81) -- an error the editor surfaces, instead of a license + bought against a deviceId that will never validate. + """ + raw = b"d" * 100 + workdir = tempfile.mkdtemp(prefix="vpp-anchor-cross-lang-big-") + try: + exe = _build_c_normalizer(workdir) + from_c = _c_normalize(exe, workdir, raw) + assert len(from_c) == lic.ANCHOR_MAX_BYTES # the C silently truncates + + anchor_file = os.path.join(workdir, "serial-number-py") + with open(anchor_file, "wb") as handle: + handle.write(raw) + monkeypatch.setattr(lic, "ANCHOR_PATH", anchor_file) + + # The bytes really do diverge... + assert lic._read_anchor() != from_c + # ...so nothing is put on the wire. + assert lic.handle_license_command("48") == "48 81" + finally: + shutil.rmtree(workdir, ignore_errors=True) From 87de838833f6ad7f725838a4296a34e081c2b834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Thu, 30 Jul 2026 13:59:16 +0200 Subject: [PATCH 08/10] fix(license): re-authenticate every debug command and gate the license FCs on role The debug channel is a trust boundary: whoever reaches it reads the hardware anchor and writes the license blob. 0x48 hands out the pre-image the licensing identity and the possession key are derived from, offline and forever -- the anchor does not rotate, so revoking an account afterwards takes nothing back. Two things did not match that status. The JWT was verified only on connect. An open socket kept answering commands after its token expired (15 minutes by default; the config sets no JWT_ACCESS_TOKEN_EXPIRES) and after /logout, because the blacklist is only consulted inside verify_jwt_in_request. The token is now captured per connection and the full pipeline -- signature, expiry, revocation, user lookup -- runs on every command. It costs an HMAC and a set lookup. The license FCs were @jwt_required() with no role check, so a plain `user` account could read the anchor of any board and overwrite its license. They now require admin, through the role mechanism the REST API already has (the User model's is_admin(), resolved by the JWT user_lookup_loader). The gate is scoped to 0x48/0x49/0x4A: a `user` can still debug variables. Not in scope, and deliberately still open: the bare-metal side of the same channel has no authentication at all, and authenticating Modbus TCP is a protocol project rather than a fix. Tests: nothing touched this file before -- the whole suite stayed green with the guard removed. Four properties are pinned now, each verified by mutation: connect requires a valid token, every command is re-authenticated (revoking mid -session cuts access off), the license FCs require admin, and the D70a ordering invariant that they resolve BEFORE the is_connected gate, which is what lets a device be activated while the PLC is stopped. Co-Authored-By: Claude Opus 5 --- .../restapi/test_debug_websocket_auth.py | 263 ++++++++++++++++++ webserver/debug_websocket.py | 76 ++++- 2 files changed, 337 insertions(+), 2 deletions(-) create mode 100644 tests/pytest/restapi/test_debug_websocket_auth.py diff --git a/tests/pytest/restapi/test_debug_websocket_auth.py b/tests/pytest/restapi/test_debug_websocket_auth.py new file mode 100644 index 00000000..48f3b2ee --- /dev/null +++ b/tests/pytest/restapi/test_debug_websocket_auth.py @@ -0,0 +1,263 @@ +"""Authentication and authorization on the debug WebSocket. + +Why this file exists +-------------------- +Nothing used to touch ``webserver/debug_websocket.py`` -- deleting the +``if not token: return False`` guard left the ENTIRE suite green while handing +the hardware anchor to anonymous callers. That guard is load-bearing: the anchor +is the pre-image the licensing identity and the possession key are derived from, +so whoever reads it can prove possession of that board forever, offline, and the +anchor never rotates. This channel is a trust boundary, and these tests are what +say so in executable form. + +Three properties are pinned here: + +1. **Connect requires a valid token** -- the deleted-guard case. +2. **Every COMMAND is re-authenticated**, not just the connect. An already-open + socket must stop working once its token is revoked or expires; the connect-time + verdict is not inherited for the life of the connection. +3. **The license FCs require the admin role.** ``@jwt_required()`` never looks at + the role, so a plain ``user`` account could read the anchor of any board and + overwrite its license. + +Plus the D70a ordering invariant, which nothing asserted: the license FCs are +resolved BEFORE the ``is_connected`` gate, so activation works while the PLC is +stopped -- which is the whole reason they live in the webserver. + +It lives next to the REST API fixtures because the debug socket is mounted on the +same Flask app (``app_restapi``) and depends on its JWT manager, ``User`` model +and roles. Those are exactly what ``conftest.py`` here builds. +""" + +import pytest + +flask_socketio = pytest.importorskip( + "flask_socketio", reason="flask_socketio not installed (no venv)" +) + +from conftest import auth, create_user, token_for # noqa: E402 + +from webserver import debug_websocket as dws # noqa: E402 +from webserver import vpp_license_debug as lic # noqa: E402 + +_NAMESPACE = "/api/debug" +_ANCHOR = b"8625807b0a83ae7d" + + +class _FakeUnixClient: + """Stands in for the C core's unix socket.""" + + def __init__(self, connected=True, response="DEBUG:41 00 01"): + self.connected = connected + self.response = response + self.sent = [] + + def is_connected(self): + return self.connected + + def send_and_receive(self, command, timeout=None): + self.sent.append(command) + return self.response + + +@pytest.fixture() +def socketio(app): + """The debug WebSocket wired onto the REST API app. + + ``init_debug_websocket`` is called per test because it is what installs the + handlers on a fresh SocketIO instance; the module-level ``_unix_client`` and + ``_session_tokens`` it owns are reset here so tests cannot leak into each + other. + """ + dws._session_tokens.clear() + sio = dws.init_debug_websocket(app, _FakeUnixClient()) + yield sio + dws._session_tokens.clear() + + +@pytest.fixture() +def anchor(monkeypatch, tmp_path): + """A fake /proc/device-tree/serial-number, so 0x48 has something to leak.""" + anchor_file = tmp_path / "serial-number" + anchor_file.write_bytes(_ANCHOR + b"\x00") + monkeypatch.setattr(lic, "ANCHOR_PATH", str(anchor_file)) + return _ANCHOR + + +def _connect(socketio, app, token=None): + kwargs = {"namespace": _NAMESPACE} + if token is not None: + kwargs["auth"] = {"token": token} + return socketio.test_client(app, **kwargs) + + +def _command(client, command_hex): + client.get_received(_NAMESPACE) # drain the "connected" event + client.emit("debug_command", {"command": command_hex}, namespace=_NAMESPACE) + received = client.get_received(_NAMESPACE) + assert received, "no debug_response emitted" + return received[-1]["args"][0] + + +def _user_token(client_http, admin_token, username="tech", password="tech-pass"): + resp = create_user(client_http, username, password, token=admin_token, role="user") + assert resp.status_code == 201, resp.get_json() + assert resp.get_json()["role"] == "user" + return token_for(client_http, username, password) + + +# --------------------------------------------------------------------------- +# 1. Connect +# --------------------------------------------------------------------------- + + +def test_connect_without_a_token_is_refused(socketio, app, admin_token): + ws = _connect(socketio, app, token=None) + assert not ws.is_connected(_NAMESPACE) + + +def test_connect_with_a_garbage_token_is_refused(socketio, app, admin_token): + ws = _connect(socketio, app, token="not-a-jwt") + assert not ws.is_connected(_NAMESPACE) + + +def test_connect_with_a_valid_admin_token_is_accepted(socketio, app, admin_token): + ws = _connect(socketio, app, token=admin_token) + assert ws.is_connected(_NAMESPACE) + + +# --------------------------------------------------------------------------- +# 2. Every command is re-authenticated +# --------------------------------------------------------------------------- + + +def test_a_revoked_token_stops_working_on_an_already_open_socket( + socketio, app, client, admin_token, anchor +): + """/logout must actually end the session's access to the anchor. + + The blacklist is only consulted by verify_jwt_in_request, so a socket that + authenticated once and never again kept serving 0x48 after logout. Expiry + goes through the very same call. + """ + ws = _connect(socketio, app, token=admin_token) + assert _command(ws, "48")["success"] is True + + assert client.post("/api/logout", headers=auth(admin_token)).status_code == 200 + + refused = _command(ws, "48") + assert refused["success"] is False + assert refused["error"] == "Unauthorized" + assert "data" not in refused + + +def test_a_command_on_a_session_with_no_captured_token_is_refused( + socketio, app, admin_token, anchor +): + """Belt and braces: if the per-connection token is gone, so is access. + + Nothing may fall back to "the socket is open, therefore it is authorized". + """ + ws = _connect(socketio, app, token=admin_token) + dws._session_tokens.clear() + + refused = _command(ws, "48") + assert refused["success"] is False + assert refused["error"] == "Unauthorized" + + +def test_disconnect_drops_the_captured_token(socketio, app, admin_token): + ws = _connect(socketio, app, token=admin_token) + assert len(dws._session_tokens) == 1 + ws.disconnect(namespace=_NAMESPACE) + assert dws._session_tokens == {} + + +def test_a_non_license_command_also_requires_a_live_token(socketio, app, client, admin_token): + ws = _connect(socketio, app, token=admin_token) + assert _command(ws, "41 00 00")["success"] is True + + assert client.post("/api/logout", headers=auth(admin_token)).status_code == 200 + + assert _command(ws, "41 00 00")["error"] == "Unauthorized" + + +# --------------------------------------------------------------------------- +# 3. The license FCs require the admin role +# --------------------------------------------------------------------------- + + +def test_an_admin_can_read_the_anchor(socketio, app, admin_token, anchor): + ws = _connect(socketio, app, token=admin_token) + response = _command(ws, "48") + + assert response["success"] is True + parts = response["data"].split() + assert parts[:3] == ["48", "7E", "10"] + assert bytes(int(p, 16) for p in parts[3:]) == anchor + + +@pytest.mark.parametrize("command_hex", ["48", "49 00 62" + " 00" * 98, "4A"]) +def test_a_non_admin_cannot_use_the_license_fcs( + socketio, app, client, admin_token, anchor, command_hex +): + """Role `user` must not read the anchor nor write a license. + + Reading it once is enough to derive that board's licensing identity and its + possession key offline, forever -- the anchor does not rotate, so revoking the + account afterwards takes nothing back. + """ + user_token = _user_token(client, admin_token) + ws = _connect(socketio, app, token=user_token) + assert ws.is_connected(_NAMESPACE) # a `user` may still debug variables + + refused = _command(ws, command_hex) + assert refused["success"] is False + assert refused["error"] == "Admin privileges required" + assert "data" not in refused + assert _ANCHOR.hex() not in str(refused) + assert _ANCHOR.decode() not in str(refused) + + +def test_a_non_admin_can_still_run_ordinary_debug_commands(socketio, app, client, admin_token): + """The role gate is scoped to the license FCs -- it is not a general lockout, + which is what would make it get reverted.""" + user_token = _user_token(client, admin_token) + ws = _connect(socketio, app, token=user_token) + + response = _command(ws, "41 00 00") + assert response["success"] is True + assert response["data"] == "41 00 01" + + +# --------------------------------------------------------------------------- +# 4. D70a: the license FCs resolve BEFORE the is_connected gate +# --------------------------------------------------------------------------- + + +def test_license_fcs_are_answered_while_the_core_is_stopped(socketio, app, admin_token, anchor): + """The whole point of resolving 0x48/0x49/0x4A in the webserver. + + Activation has to work before any program runs (the chicken-and-egg the + docstring in vpp_license_debug describes), so a license FC must not go + through the unix-socket gate. Nothing asserted this ordering before, and it + is one `return` away from silently regressing to "connect the PLC first". + """ + dws._unix_client = _FakeUnixClient(connected=False) + ws = _connect(socketio, app, token=admin_token) + + response = _command(ws, "48") + assert response["success"] is True + assert response["data"].startswith("48 7E") + # ...and it never reached the core. + assert dws._unix_client.sent == [] + + +def test_a_non_license_command_still_needs_the_core(socketio, app, admin_token): + """The other half of the ordering: only the license FCs skip the gate.""" + dws._unix_client = _FakeUnixClient(connected=False) + ws = _connect(socketio, app, token=admin_token) + + response = _command(ws, "41 00 00") + assert response["success"] is False + assert response["error"] == "Runtime not connected" diff --git a/webserver/debug_websocket.py b/webserver/debug_websocket.py index af9a17fe..5271f988 100644 --- a/webserver/debug_websocket.py +++ b/webserver/debug_websocket.py @@ -7,17 +7,64 @@ """ from flask import request -from flask_jwt_extended import verify_jwt_in_request +from flask_jwt_extended import current_user, verify_jwt_in_request from flask_socketio import SocketIO, emit from webserver.logger import get_logger -from webserver.vpp_license_debug import handle_license_command +from webserver.vpp_license_debug import handle_license_command, is_license_command logger, _ = get_logger("debug_ws", use_buffer=True) _socketio = None # pylint: disable=invalid-name _unix_client = None # pylint: disable=invalid-name +# Token captured per connection, so every COMMAND can be re-authenticated rather +# than trusting the one check done at connect time. Keyed by socket id; dropped +# on disconnect. +_session_tokens: dict = {} + + +def _reverify_session_token() -> bool: + """Re-run the FULL authentication pipeline for the current command. + + The connect handler authenticates once. Without this, an open socket keeps + answering commands after its token expires (15 minutes by default -- the + config sets no JWT_ACCESS_TOKEN_EXPIRES) and after /logout revokes it (the + blacklist is only consulted by verify_jwt_in_request). That matters here + because the license FCs read the hardware anchor and write the license blob: + this channel is a trust boundary, so "authenticated once, ever" is not + enough. Re-verifying per command is cheap -- an HMAC and a set lookup. + """ + token = _session_tokens.get(request.sid) + if not token: + logger.warning("Debug command on a session with no captured token") + return False + try: + # Same pipeline as @jwt_required(): expiry, signature, blacklist and the + # user lookup that current_user depends on. + request.environ["HTTP_AUTHORIZATION"] = f"Bearer {token}" + verify_jwt_in_request() + return True + except Exception as e: + logger.warning("Debug command rejected, token no longer valid: %s", e) + return False + + +def _current_user_is_admin() -> bool: + """True when the re-verified token belongs to an admin account. + + Uses the role mechanism that already exists in the REST API (the User model's + ``is_admin()``, resolved through the JWT user_lookup_loader) -- @jwt_required + alone does not look at the role, so a plain ``user`` account could read the + anchor of any board and overwrite its license. Must be called AFTER + _reverify_session_token(), which is what populates ``current_user``. + """ + user = current_user + if not user: + return False + checker = getattr(user, "is_admin", None) + return bool(checker and checker()) + def init_debug_websocket(app, unix_client_instance): """ @@ -84,6 +131,10 @@ def handle_connect(auth): request.environ["HTTP_AUTHORIZATION"] = f"Bearer {token}" verify_jwt_in_request() + # Kept so every command can re-run the same check (expiry, revocation + # and role), instead of the session inheriting this one verdict. + _session_tokens[request.sid] = token + logger.info("Debug WebSocket connected") emit("connected", {"status": "ok"}) return True @@ -95,6 +146,7 @@ def handle_connect(auth): @_socketio.on("disconnect", namespace="/api/debug") def handle_disconnect(): """Handle WebSocket disconnection""" + _session_tokens.pop(request.sid, None) logger.info("Debug WebSocket disconnected") @_socketio.on("debug_command", namespace="/api/debug") @@ -116,6 +168,26 @@ def handle_debug_command(data): emit("debug_response", {"success": False, "error": "Empty command"}) return + # Re-authenticate EVERY command, not just the connect. See + # _reverify_session_token: an expired or revoked token must stop + # working on a socket that is already open. + if not _reverify_session_token(): + emit("debug_response", {"success": False, "error": "Unauthorized"}) + return + + # The license FCs are a trust boundary of their own: 0x48 hands out + # the raw anchor (from which the licensing identity and the + # possession key are derived, offline and forever) and 0x49 writes + # the license blob. Require the admin role for them -- @jwt_required + # alone never looks at the role. + if is_license_command(command_hex) and not _current_user_is_admin(): + logger.warning("License FC refused for a non-admin account: %s", command_hex) + emit( + "debug_response", + {"success": False, "error": "Admin privileges required"}, + ) + return + # License function codes (0x48/0x49/0x4A) operate on host files # (/proc anchor + conf/.license) and are resolved here in # Python (D70a) BEFORE the unix-socket gate below, so device From db4402bdcb622100768664a3fd572de4bcd852bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Wed, 19 Aug 2026 20:33:53 +0200 Subject: [PATCH 09/10] test(vpp): follow the anchor normalization into license_platform.c (ADR-0003) The canonical C this test pins moved while the branch sat unmerged: the anchor read + strip loop left rpi_plugin.c::license_gate_bringup and now lives verbatim in the closed core's license_platform.c (__linux__ branch), with the buffer ceiling as LIC_ANCHOR_MAX in license_platform.h. The old extraction patterns matched nothing real anymore, which is exactly the failure mode the test warns about in its own docstring. Repointing it made it STRONGER: license_platform.c ships a compile-time override seam for its anchor path (LIC_LINUX_ANCHOR_PATH, put there for the packages host tests), so instead of regex-extracting code blocks into a harness, the test now compiles the REAL translation unit unmodified and drives it through that seam. The constants checks read the strip set out of the .c and the ceiling out of the .h by source text, as before. Same parity table, same refusal case; 16/16 locally, and the whole runtime suite (161) green on the rebase. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015uUH3ZL5ehreMUf2dtanWD --- .../plugins/test_vpp_anchor_cross_language.py | 239 +++++++++--------- 1 file changed, 123 insertions(+), 116 deletions(-) diff --git a/tests/pytest/plugins/test_vpp_anchor_cross_language.py b/tests/pytest/plugins/test_vpp_anchor_cross_language.py index 5ac65c65..2b2224f8 100644 --- a/tests/pytest/plugins/test_vpp_anchor_cross_language.py +++ b/tests/pytest/plugins/test_vpp_anchor_cross_language.py @@ -4,19 +4,24 @@ --------------- ``webserver/vpp_license_debug.py`` normalizes the raw hardware anchor before putting it on the wire for FC 0x48, and the editor hashes exactly those bytes -into the ``deviceId`` a license is signed for. The plugin's C -(``rpi_plugin.c::license_gate_bringup``) normalizes the SAME file, on the SAME -board, and hands its result to the closed ``license_core``, which is the side -that decides whether the license verifies. The C is therefore canonical. - -Both sides carried a comment claiming byte-identity with the other, and both were -wrong: the Python stripped a trailing TAB and the C never did, so an anchor -ending in 0x09 derived a different ``deviceId`` on each side -- +into the ``deviceId`` a license is signed for. The closed core's +``license_platform.c`` (``__linux__`` branch) normalizes the SAME file, on the +SAME board, and hands its result to ``license_core``, which is the side that +decides whether the license verifies. The C is therefore canonical. + +That C used to live in the open plugin (``rpi_plugin.c::license_gate_bringup``) +and moved into the closed core verbatim under ADR-0003 -- the packages repo +marks the logic "A CONTRACT, NOT AN IMPLEMENTATION DETAIL" for exactly the +reason this test exists: both sides once carried a comment claiming +byte-identity with the other, and both were wrong. The Python stripped a +trailing TAB and the C never did, so an anchor ending in 0x09 derived a +different ``deviceId`` on each side -- ``sha256("openplc-dev-v1|" + "8625807b0a83ae7d\\t")[:16]`` is ``ac07623afa23c771...`` where the C computes ``7146518f9842adac...``. Nothing would log, nothing would fail: the customer pays and the license simply never -works. The C also reads into ``uint8_t anchor[64]`` and silently truncates, -while the Python read the whole file and framed up to 255 bytes on the wire. +works. The C also reads into a ``LIC_ANCHOR_MAX``-byte buffer and silently +truncates, while the Python read the whole file and framed up to 255 bytes on +the wire. How this test stays honest -------------------------- @@ -26,23 +31,25 @@ so (``test_vpp_license_delivery.py``'s hand transcription of ``derive_license_path``). This file instead: -1. extracts the REAL strip set and the REAL buffer size out of the C source by - source text, and asserts the Python constants equal them; and -2. extracts the REAL ``read_file_bytes`` function and the REAL anchor block - (declaration, read, and normalization loop), compiles them unmodified, and - compares the bytes the C produces with the bytes ``_read_anchor()`` produces, - over the same files. +1. extracts the REAL strip set out of ``license_platform.c`` and the REAL + buffer ceiling out of ``license_platform.h`` by source text, and asserts the + Python constants equal them; and +2. compiles ``license_platform.c`` ITSELF -- whole and unmodified, through the + ``LIC_LINUX_ANCHOR_PATH`` override seam the file provides for its own host + tests -- and compares the bytes the real C produces with the bytes + ``_read_anchor()`` produces, over the same files. If either side's source changes, this test runs the NEW text, not a stale copy. Where the C lives ----------------- -The plugin C source is in the sibling ``openplc-packages`` repository, not in -this one, so this test resolves it and SKIPS when it cannot be found. Point -``OPENPLC_PACKAGES_DIR`` at a checkout to run it from elsewhere. This is a real -limitation -- on a runtime-only CI checkout this file skips, exactly like the -symlink case in ``test_vpp_license_debug.py`` -- and the mirror of this test -belongs in ``openplc-packages``, where the C source is always present. +``license-core/src/license_platform.{c,h}`` in the sibling ``openplc-packages`` +repository, not in this one, so this test resolves it and SKIPS when it cannot +be found. Point ``OPENPLC_PACKAGES_DIR`` at a checkout to run it from +elsewhere. This is a real limitation -- on a runtime-only CI checkout this file +skips, exactly like the symlink case in ``test_vpp_license_debug.py`` -- and +the mirror of this test belongs in ``openplc-packages``, where the C source is +always present. """ import os @@ -59,18 +66,12 @@ ) _REPO_ROOT = os.path.normpath(os.path.join(os.path.dirname(__file__), "..", "..", "..")) -_RPI_PLUGIN_REL = os.path.join( - "packages", - "com.openplc.raspberry-pi-licensed", - "hal", - "runtime-v4", - "source", - "rpi_plugin.c", -) +_PLATFORM_C_REL = os.path.join("license-core", "src", "license_platform.c") +_PLATFORM_H_REL = os.path.join("license-core", "src", "license_platform.h") -def _find_rpi_plugin_source(): - """The licensed Pi plugin's C source, or None. +def _find_packages_root(): + """The openplc-packages checkout holding license_platform.{c,h}, or None. Checked in order: an explicit OPENPLC_PACKAGES_DIR, then a sibling ``openplc-packages`` checkout next to this repository. @@ -81,55 +82,36 @@ def _find_rpi_plugin_source(): roots.append(env_root) roots.append(os.path.join(os.path.dirname(_REPO_ROOT), "openplc-packages")) for root in roots: - candidate = os.path.join(root, _RPI_PLUGIN_REL) - if os.path.isfile(candidate): - return candidate + if os.path.isfile(os.path.join(root, _PLATFORM_C_REL)) and os.path.isfile( + os.path.join(root, _PLATFORM_H_REL) + ): + return root return None -_RPI_PLUGIN_C = _find_rpi_plugin_source() +_PACKAGES_ROOT = _find_packages_root() _CC = next((c for c in ("cc", "gcc", "clang") if shutil.which(c)), None) pytestmark = pytest.mark.skipif( - _RPI_PLUGIN_C is None, + _PACKAGES_ROOT is None, reason=( - "rpi_plugin.c not found: this test needs the sibling openplc-packages " - "checkout (or OPENPLC_PACKAGES_DIR) because the canonical anchor " - "normalization lives there, not in this repo" + "license_platform.{c,h} not found: this test needs the sibling " + "openplc-packages checkout (or OPENPLC_PACKAGES_DIR) because the " + "canonical anchor normalization lives there, not in this repo" ), ) -def _c_source() -> str: +def _read_source(rel_path: str) -> str: # newline=None gives universal-newline translation; strip any stray \r on - # top of it, because this repo has no .gitattributes and a Windows checkout - # can hand us CRLF where the extraction patterns expect bare \n (tasks - # #50/#58). Normalizing line endings for matching does not change what the - # C does. - with open(_RPI_PLUGIN_C, "r", encoding="utf-8", newline=None) as handle: + # top of it, because a Windows checkout can hand us CRLF where the + # extraction patterns expect bare \n (tasks #50/#58). Normalizing line + # endings for matching does not change what the C does. + path = os.path.join(_PACKAGES_ROOT, rel_path) + with open(path, "r", encoding="utf-8", newline=None) as handle: return handle.read().replace("\r", "") -def _extract(pattern: str, what: str) -> str: - """Pull a chunk of real C out of the source by exact text. - - Raises rather than returning an empty string: a test that "passes" because - it extracted nothing and compared nothing would be worse than no test. - """ - match = re.compile(pattern, re.MULTILINE | re.DOTALL).search(_c_source()) - if not match: - raise AssertionError( - f"could not locate {what} in {_RPI_PLUGIN_C} -- the extraction " - "pattern in this test no longer matches the real source, which " - "means this test is not exercising real code" - ) - return match.group(0) - - -_ANCHOR_BLOCK_PATTERN = r"^ uint8_t anchor\[\d+\];$.*?^ \}$" -_READ_FILE_BYTES_PATTERN = r"^static long read_file_bytes\(.*?^\}$" - - # --------------------------------------------------------------------------- # 1. The constants, read out of the real C source. No compiler needed. # --------------------------------------------------------------------------- @@ -138,10 +120,16 @@ def _extract(pattern: str, what: str) -> str: def test_python_strips_exactly_the_bytes_the_c_strips(): """The strip set is the whole contract: one extra byte on either side moves the deviceId and the purchased license stops matching the hardware.""" - block = _extract(_ANCHOR_BLOCK_PATTERN, "the anchor normalization block") - # Every character literal compared against anchor[alen - 1] in the real loop. - comparisons = re.findall(r"anchor\[alen - 1\] == '((?:\\.|[^'\\])+)'", block) - assert comparisons, f"no strip comparisons found in the extracted C:\n{block}" + source = _read_source(_PLATFORM_C_REL) + # Every character literal compared against out[n - 1u] in the real + # normalization loop (the __linux__ branch is the only place that shape + # appears in the file). + comparisons = re.findall(r"out\[n - 1u\] == '((?:\\.|[^'\\])+)'", source) + assert comparisons, ( + f"no strip comparisons found in {_PLATFORM_C_REL} -- the extraction " + "pattern no longer matches the real source, which means this test is " + "not exercising real code" + ) escapes = {"\\0": b"\x00", "\\n": b"\n", "\\r": b"\r", "\\t": b"\t", " ": b" "} c_strip_set = set() @@ -158,70 +146,88 @@ def test_python_strips_exactly_the_bytes_the_c_strips(): def test_python_anchor_ceiling_matches_the_c_buffer(): - block = _extract(_ANCHOR_BLOCK_PATTERN, "the anchor normalization block") - size = int(re.search(r"uint8_t anchor\[(\d+)\];", block).group(1)) + header = _read_source(_PLATFORM_H_REL) + match = re.search(r"#define LIC_ANCHOR_MAX (\d+)u?\b", header) + assert match, ( + f"LIC_ANCHOR_MAX not found in {_PLATFORM_H_REL} -- the ceiling moved " + "and this test must follow it" + ) + size = int(match.group(1)) assert size == lic.ANCHOR_MAX_BYTES, ( - f"the C reads the anchor into uint8_t anchor[{size}] but this runtime " - f"caps at {lic.ANCHOR_MAX_BYTES}. The C never sees more than its buffer, " - "so anything above the cap must be refused here, not truncated." + f"the C reads the anchor into a {size}-byte buffer (LIC_ANCHOR_MAX) but " + f"this runtime caps at {lic.ANCHOR_MAX_BYTES}. The C never sees more " + "than its buffer, so anything above the cap must be refused here, not " + "truncated." ) # --------------------------------------------------------------------------- # 2. The behaviour, by executing the real C. Needs a compiler. +# +# license_platform.c ships its own test seam: LIC_LINUX_ANCHOR_PATH overrides +# the /proc/device-tree path at compile time (the packages host tests use the +# same seam). So the whole file compiles UNMODIFIED, pointed at a temp file, +# and a three-line main() prints what license_platform_anchor() returned -- +# no extraction, no transcription, the real translation unit end to end. # --------------------------------------------------------------------------- -_HARNESS = """\ +_HARNESS_MAIN = """\ #include -#include -#include -#include -/* The anchor block below reads RPI_ANCHOR_PATH; point it at argv[1] so the same - real code can be run against a temp file instead of /proc. */ -#define RPI_ANCHOR_PATH argv[1] +#include "license_platform.h" -%(read_file_bytes)s - -int main(int argc, char **argv) +int main(void) { - if (argc < 2) return 2; -%(anchor_block)s - for (long i = 0; i < alen; i++) printf("%%02x", anchor[i]); + uint8_t buf[LIC_ANCHOR_MAX]; + size_t n = license_platform_anchor(buf, sizeof buf); + for (size_t i = 0; i < n; i++) printf("%02x", buf[i]); printf("\\n"); return 0; } """ -def _build_c_normalizer(workdir: str) -> str: - source = _HARNESS % { - "read_file_bytes": _extract(_READ_FILE_BYTES_PATTERN, "read_file_bytes()"), - "anchor_block": _extract(_ANCHOR_BLOCK_PATTERN, "the anchor normalization block"), - } - src_path = os.path.join(workdir, "anchor_harness.c") - with open(src_path, "w", encoding="utf-8", newline="\n") as handle: - handle.write(source) +def _build_c_normalizer(workdir: str) -> tuple: + """Compile the REAL license_platform.c against a temp anchor file. + + Returns (exe_path, anchor_file): the binary reads `anchor_file` on every + run, so each parity case just rewrites that file and re-invokes. + """ + anchor_file = os.path.join(workdir, "serial-number") + main_path = os.path.join(workdir, "anchor_harness_main.c") + with open(main_path, "w", encoding="utf-8", newline="\n") as handle: + handle.write(_HARNESS_MAIN) + src_dir = os.path.join(_PACKAGES_ROOT, "license-core", "src") + platform_c = os.path.join(_PACKAGES_ROOT, _PLATFORM_C_REL) exe_path = os.path.join(workdir, "anchor_harness") result = subprocess.run( - [_CC, "-std=c99", "-Wall", "-o", exe_path, src_path], + [ + _CC, + "-std=c11", + "-Wall", + "-I", + src_dir, + f'-DLIC_LINUX_ANCHOR_PATH="{anchor_file}"', + platform_c, + main_path, + "-o", + exe_path, + ], capture_output=True, text=True, check=False, ) assert result.returncode == 0, ( - "the extracted C did not compile -- the extraction is picking up text " - f"that is not the real function/block:\n{result.stdout}\n{result.stderr}\n" - f"--- generated source ---\n{source}" + "the real license_platform.c did not compile against the harness -- " + f"either the seam or the header contract changed:\n{result.stdout}\n{result.stderr}" ) - return exe_path + return exe_path, anchor_file -def _c_normalize(exe_path: str, workdir: str, raw: bytes) -> bytes: - anchor_file = os.path.join(workdir, "serial-number") +def _c_normalize(exe_path: str, anchor_file: str, raw: bytes) -> bytes: with open(anchor_file, "wb") as handle: handle.write(raw) - result = subprocess.run([exe_path, anchor_file], capture_output=True, text=True, check=False) + result = subprocess.run([exe_path], capture_output=True, text=True, check=False) assert result.returncode == 0, f"C harness exited {result.returncode}: {result.stderr}" return bytes.fromhex(result.stdout.strip()) @@ -266,8 +272,8 @@ def test_python_and_c_normalize_the_anchor_identically(case, monkeypatch): raw = PARITY_CASES[case] workdir = tempfile.mkdtemp(prefix="vpp-anchor-cross-lang-") try: - exe = _build_c_normalizer(workdir) - from_c = _c_normalize(exe, workdir, raw) + exe, anchor_file = _build_c_normalizer(workdir) + from_c = _c_normalize(exe, anchor_file, raw) from_python = _python_normalize(monkeypatch, workdir, raw) assert from_python == from_c, ( f"case {case!r}: python -> {from_python!r}, C -> {from_c!r}. " @@ -283,23 +289,24 @@ def test_python_and_c_normalize_the_anchor_identically(case, monkeypatch): def test_an_anchor_longer_than_the_c_buffer_is_refused_rather_than_diverging(monkeypatch): """The one case where the two sides CANNOT agree, and what we do about it. - The C reads 64 bytes and hashes those; a longer anchor would have this side - hash more, so the two deviceIds differ by construction. Rather than serve - bytes that derive an identity the verifier can never reproduce, 0x48 refuses - with TOO_LARGE (0x81) -- an error the editor surfaces, instead of a license - bought against a deviceId that will never validate. + The C reads LIC_ANCHOR_MAX bytes and hashes those; a longer anchor would + have this side hash more, so the two deviceIds differ by construction. + Rather than serve bytes that derive an identity the verifier can never + reproduce, 0x48 refuses with TOO_LARGE (0x81) -- an error the editor + surfaces, instead of a license bought against a deviceId that will never + validate. """ raw = b"d" * 100 workdir = tempfile.mkdtemp(prefix="vpp-anchor-cross-lang-big-") try: - exe = _build_c_normalizer(workdir) - from_c = _c_normalize(exe, workdir, raw) + exe, anchor_file = _build_c_normalizer(workdir) + from_c = _c_normalize(exe, anchor_file, raw) assert len(from_c) == lic.ANCHOR_MAX_BYTES # the C silently truncates - anchor_file = os.path.join(workdir, "serial-number-py") - with open(anchor_file, "wb") as handle: + py_anchor_file = os.path.join(workdir, "serial-number-py") + with open(py_anchor_file, "wb") as handle: handle.write(raw) - monkeypatch.setattr(lic, "ANCHOR_PATH", anchor_file) + monkeypatch.setattr(lic, "ANCHOR_PATH", py_anchor_file) # The bytes really do diverge... assert lic._read_anchor() != from_c From c96b3748724f43ee969855753fe124eee9529e9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Thu, 20 Aug 2026 17:49:28 +0200 Subject: [PATCH 10/10] =?UTF-8?q?fix(license):=20correctness=20round=20?= =?UTF-8?q?=E2=80=94=20token=20renewal,=20honest=20anchor,=20seal=20that?= =?UTF-8?q?=20seals?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review 2026-08-20 (Thiago, read across #169/#1023/#681), findings R1-R5: - R1: expiry is now distinguishable (token_expired) and renewable: a 'reauth' event runs the fresh token through the FULL verification pipeline and swaps the session token — the per-command re-verify stops being a 15-minute fuse on every debug/licensing session. Editor half: openplc-editor 349919137 (candidate reads the token manager at create(); held channel gets reauth pushed on refresh). - R2: an unreadable anchor answers LIC_UNSUPPORTED, never SUCCESS/len=0 — every anchor-less host used to derive the SAME deviceId and a purchase bound to it could never validate on the .so. - R3: the object seal is written ONLY for objects this run compiled; a checksum cache-hit stands only when the existing seal vouches for the .so on disk, otherwise the tree (just signature-verified) is recompiled — a re-upload can no longer bless a swapped object, and an upgraded runtime with no seal rebuilds instead of blessing unknown bytes. - R4: vpp_tree_digest propagates hashing failures (pipefail; the old '|| exit 1' exited a pipeline-stage subshell and produced a digest of a PARTIAL listing reported as tampering) and sha256_hex strips the escape marker GNU sha256sum prefixes for exotic filenames. Empty tree hashes as empty input, matching the python side. - R5: the webserver now rejects absolute plugin paths like the C loader always did — a contained-absolute conf was accepted at upload and silently dropped at parse, the plugin never loading with no error anywhere. Full pytest 161 passed (delivery fixture moved to the relative path the editor actually emits; empty-anchor expectation flipped to 0x85). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015uUH3ZL5ehreMUf2dtanWD --- scripts/compile.sh | 58 +++++++++++++++---- .../pytest/plugins/test_vpp_license_debug.py | 6 +- .../plugins/test_vpp_license_delivery.py | 2 +- webserver/debug_websocket.py | 41 ++++++++++++- webserver/plcapp_management.py | 13 ++++- webserver/vpp_license_debug.py | 11 +++- 6 files changed, 111 insertions(+), 20 deletions(-) diff --git a/scripts/compile.sh b/scripts/compile.sh index 46ce520f..412c6b15 100755 --- a/scripts/compile.sh +++ b/scripts/compile.sh @@ -139,9 +139,9 @@ VPP_OBJECT_SEAL="$VPP_OUTPUT_DIR/vpp_plugin.seal" # fails the build instead. sha256_hex() { if command -v sha256sum > /dev/null 2>&1; then - sha256sum "$1" | cut -d' ' -f1 + sha256sum "$1" | awk '{ sub(/^\\/, "", $1); print $1; exit }' elif command -v shasum > /dev/null 2>&1; then - shasum -a 256 "$1" | cut -d' ' -f1 + shasum -a 256 "$1" | awk '{ sub(/^\\/, "", $1); print $1; exit }' else return 1 fi @@ -153,14 +153,24 @@ sha256_hex() { # the value in the seal -- hence LC_ALL=C for byte-order sort, POSIX relative # paths, and exactly two spaces. vpp_tree_digest() { - local dir="$1" f inner - ( + local dir="$1" listing + # '|| exit 1' used to exit the while's SUBSHELL (a pipeline stage); the + # function's status was cut's (0), so a hashing failure emitted a digest + # of a PARTIAL listing -- reported as "tree changed after it was + # verified", accusing an intact package of tampering when the real cause + # was a missing tool (review 2026-08-20, R4). pipefail makes the inner + # failure the pipeline's, and the caller sees it. + listing=$( cd "$dir" || exit 1 + set -o pipefail find . -type f -print | sed 's|^\./||' | LC_ALL=C sort | while IFS= read -r f; do - inner=$(sha256_hex "$f") || exit 1 + inner=$(sha256_hex "$f") || { echo "[ERROR] cannot hash $dir/$f" >&2; exit 1; } printf '%s %s\n' "$inner" "$f" done - ) | { if command -v sha256sum > /dev/null 2>&1; then sha256sum; else shasum -a 256; fi; } | cut -d' ' -f1 + ) || return 1 + # An empty tree hashes as EMPTY INPUT (python's tree_digest of no files); + # printf '%s\n' of an empty capture would inject one newline. + { [ -n "$listing" ] && printf '%s\n' "$listing" || printf ''; } | { if command -v sha256sum > /dev/null 2>&1; then sha256sum; else shasum -a 256; fi; } | awk '{ sub(/^\\/, "", $1); print $1; exit }' } # Refuse to build an unverified plugin tree. @@ -209,6 +219,20 @@ check_vpp_verification_seal() { return 0 } +# Every lib*_plugin.so on disk must hash to a line in the seal (review +# 2026-08-20, R3). Decides whether a checksum cache hit may skip the +# compile: objects the seal does not vouch for force a rebuild. +vpp_object_seal_matches() { + [ -f "$VPP_OBJECT_SEAL" ] || return 1 + local so so_hash + for so in "$VPP_OUTPUT_DIR"/lib*_plugin.so; do + [ -f "$so" ] || continue + so_hash=$(sha256_hex "$so") || return 1 + grep -Fxq "$so_hash $(basename "$so")" "$VPP_OBJECT_SEAL" || return 1 + done + return 0 +} + if [ -d "$VPP_PLUGIN_DIR" ] && [ -f "$VPP_PLUGIN_DIR/Makefile" ]; then # Before mkdir, before the cache check, before make: nothing from the # upload is acted on until the seal matches. @@ -220,8 +244,18 @@ if [ -d "$VPP_PLUGIN_DIR" ] && [ -f "$VPP_PLUGIN_DIR/Makefile" ]; then if [ -f "$VPP_CHECKSUM_FILE" ] && [ -f "$VPP_CACHED_CHECKSUM" ]; then if diff -q "$VPP_CHECKSUM_FILE" "$VPP_CACHED_CHECKSUM" > /dev/null 2>&1; then if ls "$VPP_OUTPUT_DIR"/lib*_plugin.so 1>/dev/null 2>&1; then - echo "[INFO] VPP plugin source unchanged (checksum match), skipping recompilation" - NEEDS_COMPILE=0 + # A cache hit only stands when the SEAL vouches for the + # objects on disk (review 2026-08-20, R3): re-blessing + # whatever sits in build/vpp/ converted a detected tamper + # into a permanent pass on the next re-upload, and an + # upgraded runtime with no seal must REBUILD from the + # just-verified tree, never bless unknown bytes. + if vpp_object_seal_matches; then + echo "[INFO] VPP plugin source unchanged (checksum match), skipping recompilation" + NEEDS_COMPILE=0 + else + echo "[INFO] Object seal missing or stale for cached .so -- recompiling from the verified tree" + fi fi fi fi @@ -246,9 +280,10 @@ if [ -d "$VPP_PLUGIN_DIR" ] && [ -f "$VPP_PLUGIN_DIR/Makefile" ]; then # Record the sha256 of every .so this verified build produced, so the # plugin loader can refuse an object swapped in AFTER the compile # (core/src/drivers/vpp_plugin_seal.c, checked immediately before dlopen). - # Written on the cache-hit path too: the cached .so belongs to this same - # verified tree, and a runtime upgraded onto an existing build/vpp/ would - # otherwise have no seal at all and refuse to load a legitimate plugin. + # ONLY when this run compiled (review 2026-08-20, R3): sealing on the + # cache-hit path blessed whatever bytes sat in build/vpp/; the upgrade- + # without-seal case is served by the forced recompile above. + if [ "$NEEDS_COMPILE" -eq 1 ]; then : > "$VPP_OBJECT_SEAL" for so in "$VPP_OUTPUT_DIR"/lib*_plugin.so; do [ -f "$so" ] || continue @@ -260,6 +295,7 @@ if [ -d "$VPP_PLUGIN_DIR" ] && [ -f "$VPP_PLUGIN_DIR/Makefile" ]; then printf '%s %s\n' "$so_hash" "$(basename "$so")" >> "$VPP_OBJECT_SEAL" echo "[INFO] Sealed $(basename "$so") (${so_hash:0:12}...)" done + fi else # No VPP plugin in this upload — clean up the entire VPP output dir # so a stale .so doesn't get picked up by the loader. Scoping the rm diff --git a/tests/pytest/plugins/test_vpp_license_debug.py b/tests/pytest/plugins/test_vpp_license_debug.py index 4e666b02..4c5d1195 100644 --- a/tests/pytest/plugins/test_vpp_license_debug.py +++ b/tests/pytest/plugins/test_vpp_license_debug.py @@ -113,9 +113,11 @@ def test_get_board_id_returns_raw_ascii_anchor(tmp_path, monkeypatch): def test_get_board_id_missing_anchor_is_empty_success(tmp_path, monkeypatch): - # No anchor -> SUCCESS with id_len=0 (matches Arduino D57), not an error byte. + # No anchor -> LIC_UNSUPPORTED (review 2026-08-20, R2): on this medium 0x48 + # is ONLY the licensing anchor, and SUCCESS/0 made every anchor-less host + # derive the SAME deviceId -- a purchase bound to it never validated. monkeypatch.setattr(lic, "ANCHOR_PATH", str(tmp_path / "nope")) - assert lic.handle_license_command("48") == "48 7E 00" + assert lic.handle_license_command("48") == "48 85" def test_write_refuses_path_traversal(tmp_path, monkeypatch): diff --git a/tests/pytest/plugins/test_vpp_license_delivery.py b/tests/pytest/plugins/test_vpp_license_delivery.py index 68092a80..17eabd58 100644 --- a/tests/pytest/plugins/test_vpp_license_delivery.py +++ b/tests/pytest/plugins/test_vpp_license_delivery.py @@ -90,7 +90,7 @@ def __init__(self, cp, so): self.path = so class _Conf: - plugins = [_P(config_path, str(cwd / "build" / "vpp" / "librpi_gpio_plugin.so"))] + plugins = [_P(config_path, "./build/vpp/librpi_gpio_plugin.so")] monkeypatch.setattr(mgmt.PluginsConfiguration, "from_file", classmethod(lambda cls, _p: _Conf())) monkeypatch.setattr(mgmt.build_state, "log", lambda *_a, **_k: None, raising=False) diff --git a/webserver/debug_websocket.py b/webserver/debug_websocket.py index 5271f988..1998971f 100644 --- a/webserver/debug_websocket.py +++ b/webserver/debug_websocket.py @@ -8,6 +8,7 @@ from flask import request from flask_jwt_extended import current_user, verify_jwt_in_request +from jwt import ExpiredSignatureError from flask_socketio import SocketIO, emit from webserver.logger import get_logger @@ -45,6 +46,13 @@ def _reverify_session_token() -> bool: request.environ["HTTP_AUTHORIZATION"] = f"Bearer {token}" verify_jwt_in_request() return True + except ExpiredSignatureError: + # Distinguishable on purpose (review 2026-08-20, R1): expiry is the ONE + # failure the client can repair -- its token manager re-logins and + # pushes the fresh token through the 'reauth' event below. Everything + # else (revocation, tamper) stays a generic refusal. + logger.info("Debug command deferred: session token expired (client may reauth)") + return "expired" except Exception as e: logger.warning("Debug command rejected, token no longer valid: %s", e) return False @@ -143,6 +151,31 @@ def handle_connect(auth): logger.warning("Debug WebSocket auth failed: %s", e) return False + @_socketio.on("reauth", namespace="/api/debug") + def handle_reauth(data): + """Swap this session's token for a fresh one, after FULL verification. + + The per-command re-verification (above) makes the connect-time token a + 15-minute fuse; this is the renewal path (review 2026-08-20, R1). The + new token goes through the same pipeline as every command -- signature, + expiry, revocation, user lookup -- so reauth can never LOWER the bar, + only extend a session that could re-login anyway. + """ + token = data.get("token", "") if isinstance(data, dict) else "" + if not token: + emit("reauth_result", {"success": False, "error": "no token"}) + return + try: + request.environ["HTTP_AUTHORIZATION"] = f"Bearer {token}" + verify_jwt_in_request() + except Exception as e: + logger.warning("reauth rejected: %s", e) + emit("reauth_result", {"success": False, "error": "Unauthorized"}) + return + _session_tokens[request.sid] = token + logger.info("Debug session token renewed via reauth") + emit("reauth_result", {"success": True}) + @_socketio.on("disconnect", namespace="/api/debug") def handle_disconnect(): """Handle WebSocket disconnection""" @@ -171,8 +204,12 @@ def handle_debug_command(data): # Re-authenticate EVERY command, not just the connect. See # _reverify_session_token: an expired or revoked token must stop # working on a socket that is already open. - if not _reverify_session_token(): - emit("debug_response", {"success": False, "error": "Unauthorized"}) + verdict = _reverify_session_token() + if verdict is not True: + emit( + "debug_response", + {"success": False, "error": "token_expired" if verdict == "expired" else "Unauthorized"}, + ) return # The license FCs are a trust boundary of their own: 0x48 hands out diff --git a/webserver/plcapp_management.py b/webserver/plcapp_management.py index 0a00fc51..549d4bb5 100644 --- a/webserver/plcapp_management.py +++ b/webserver/plcapp_management.py @@ -305,11 +305,22 @@ def against_root(candidate: str) -> str: relative to the runtime root (which is the loader's cwd). Resolving against the process cwd instead would make the guard depend on where the caller happened to be.""" - return candidate if os.path.isabs(candidate) else os.path.join(runtime_root, candidate) + return os.path.join(runtime_root, candidate) for p in plugins_conf.plugins: if not p.path: return False, f"plugin '{p.name}' has an empty path" + if os.path.isabs(p.path): + # The C loader (plugin_config.c, require_contained=1) rejects EVERY + # absolute path; tolerating a contained absolute here produced a + # conf accepted by the upload and silently dropped at parse time -- + # the VPP never loaded and nothing said why (review 2026-08-20, + # R5). The two guards must agree, and the editor only ever emits + # relative paths, so nothing legitimate breaks. + return False, ( + f"plugin '{p.name}' path '{p.path}' is absolute -- plugin paths " + f"must be relative to the runtime root (./build/vpp/...)" + ) plugin_path = against_root(p.path) if not is_inside_root(plugin_path, runtime_root): return False, f"plugin '{p.name}' path '{p.path}' escapes the runtime root" diff --git a/webserver/vpp_license_debug.py b/webserver/vpp_license_debug.py index 12c4c89a..fb52fa93 100644 --- a/webserver/vpp_license_debug.py +++ b/webserver/vpp_license_debug.py @@ -345,9 +345,14 @@ def handle_license_command(command_hex: str) -> Optional[str]: if fc == FC_GET_BOARD_ID: anchor = _read_anchor() if not anchor: - # Match the Arduino firmware (D57): no id -> SUCCESS with id_len=0, so - # the editor sees a clean empty id (outcome no-id), not an error byte. - return _hex_from_bytes(bytes([fc, ST_SUCCESS, 0])) + # NOT the Arduino convention (review 2026-08-20, R2). On this medium + # 0x48 is ONLY the licensing anchor -- SUCCESS with id_len=0 made the + # editor hash an EMPTY pre-image, so every anchor-less host (x86 box, + # container, unmounted /proc/device-tree) derived the SAME deviceId, + # a purchase bound to it never validated on the .so, and the buyer + # got a 2-hour demo forever. UNSUPPORTED is the truth: this device + # has no hardware anchor to license against. + return _hex_from_bytes(bytes([fc, ST_LIC_UNSUPPORTED])) if len(anchor) > ANCHOR_MAX_BYTES: # REFUSE. The .so only ever reads 64 bytes, so anything longer would # make the editor derive a device_id the verifier cannot reproduce --