From 2ed5d74bc61e362a2f62908a6fa6c2a32f7fd997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20P=C3=A9rez?= Date: Mon, 21 Sep 2026 01:58:28 -0300 Subject: [PATCH] feat(obp): warn when two bridges share a passphrase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Control frames carry no NETWORK_ID, so three things can tell two OPENBRIDGE bridges apart: a legacy port of their own, a passphrase of their own, or a source address that matches what one of them is configured with. Any one is enough. Sharing the fan-in port and a passphrase leaves only the address, and a frame from an address none of them knows then goes to whichever bridge was registered first — the misattribution behind #79 and #87. Nothing said so. The validator checks duplicate NETWORK_IDs and duplicate legacy ports, but treats PASSPHRASE as just another string. openbridge_passphrase_collisions() groups the enabled OPENBRIDGE systems that share one, and returns the names only, never the secret. It surfaces as a warn finding in --doctor and as a line at startup next to the fan-in summary. Kept advisory on purpose: a shared passphrase works as long as every peer address is distinct and current, so refusing to start would stop servers that are fine today. --- .../application/proxy/deployment.py | 29 +++++++++++++++ src/adn_server/infrastructure/doctor.py | 11 ++++++ .../infrastructure/proxy/obp_runtime.py | 13 ++++++- tests/infrastructure/test_doctor.py | 19 ++++++++++ tests/infrastructure/test_obp_proxy.py | 35 +++++++++++++++++++ 5 files changed, 106 insertions(+), 1 deletion(-) diff --git a/src/adn_server/application/proxy/deployment.py b/src/adn_server/application/proxy/deployment.py index ef1d1b1..65fd395 100644 --- a/src/adn_server/application/proxy/deployment.py +++ b/src/adn_server/application/proxy/deployment.py @@ -79,6 +79,35 @@ def config_has_enabled_openbridge(config: dict[str, Any]) -> bool: ) +def openbridge_passphrase_collisions(config: dict[str, Any]) -> list[list[str]]: + """Enabled OPENBRIDGE systems grouped by a passphrase more than one of them uses. + + Control frames carry no NETWORK_ID, so on the shared fan-in port the passphrase + is what tells two bridges apart when the source address does not. The groups are + returned, never the passphrase. + """ + systems = config.get("SYSTEMS", {}) + if not isinstance(systems, dict): + return [] + by_passphrase: dict[bytes, list[str]] = {} + for name, sys_cfg in systems.items(): + if not isinstance(sys_cfg, dict) or sys_cfg.get("MODE") != "OPENBRIDGE": + continue + if not sys_cfg.get("ENABLED", True): + continue + passphrase = sys_cfg.get("PASSPHRASE") or b"" + if isinstance(passphrase, str): + passphrase = passphrase.encode("utf-8") + passphrase = bytes(passphrase).strip().rstrip(b"\x00") + if not passphrase: + continue + by_passphrase.setdefault(passphrase, []).append(str(name)) + return sorted( + (sorted(names) for names in by_passphrase.values() if len(names) > 1), + key=lambda names: names[0], + ) + + def obp_proxy_enabled(config: dict[str, Any]) -> bool: """True when OBP proxy manages inbound UDP (default on for OPENBRIDGE configs).""" block = _obp_proxy_block(config) diff --git a/src/adn_server/infrastructure/doctor.py b/src/adn_server/infrastructure/doctor.py index 78a11c5..0b0e243 100644 --- a/src/adn_server/infrastructure/doctor.py +++ b/src/adn_server/infrastructure/doctor.py @@ -35,6 +35,7 @@ from adn_server.application.proxy.deployment import ( normalize_proxy_target, obp_bridge_legacy_listen_port, obp_proxy_enabled, + openbridge_passphrase_collisions, proxy_target_system, ) from adn_server.domain.errors import ConfigError @@ -231,6 +232,16 @@ def collect_findings( else: findings.append(Finding("warn", "systems", f"{name}: unknown MODE={mode}")) + for shared in openbridge_passphrase_collisions(config): + findings.append( + Finding( + "warn", + "peer", + f"{', '.join(shared)}: same PASSPHRASE — a control frame from an address " + "none of them is configured with goes to whichever is registered first", + ) + ) + if not echo: reports = config.get("REPORTS", {}) if reports.get("REPORT", True): diff --git a/src/adn_server/infrastructure/proxy/obp_runtime.py b/src/adn_server/infrastructure/proxy/obp_runtime.py index b784160..cbb2abc 100644 --- a/src/adn_server/infrastructure/proxy/obp_runtime.py +++ b/src/adn_server/infrastructure/proxy/obp_runtime.py @@ -28,7 +28,11 @@ from typing import Any from twisted.internet import reactor -from adn_server.application.proxy.deployment import obp_bridge_legacy_listen_port, obp_proxy_enabled +from adn_server.application.proxy.deployment import ( + obp_bridge_legacy_listen_port, + obp_proxy_enabled, + openbridge_passphrase_collisions, +) from adn_server.domain.mesh_session import mesh_sessions from adn_server.infrastructure.proxy.obp_config import obp_proxy_settings from adn_server.infrastructure.proxy.obp_fanin import ( @@ -226,6 +230,13 @@ def start_obp_proxy_service( ) if bridge_count == 0: logger.warning("(OBP_PROXY) No enabled OPENBRIDGE systems registered") + for shared in openbridge_passphrase_collisions(config): + logger.warning( + "(OBP_PROXY) %s share one PASSPHRASE: a control frame from an address none of " + "them is configured with goes to whichever is registered first — give each " + "bridge its own passphrase", + ", ".join(shared), + ) return state diff --git a/tests/infrastructure/test_doctor.py b/tests/infrastructure/test_doctor.py index d0eac06..664477e 100644 --- a/tests/infrastructure/test_doctor.py +++ b/tests/infrastructure/test_doctor.py @@ -91,6 +91,25 @@ def test_collect_findings_peer_mesh_protocol() -> None: assert any("MESH_PROTOCOL=dmre_v5" in m for m in peer_msgs) +def test_collect_findings_warns_when_two_bridges_share_a_passphrase() -> None: + """Without it nothing tells the operator their control frames are only being + told apart by source address. The passphrase itself must not be printed.""" + config = { + "GLOBAL": {"SERVER_ID": 7301}, + "REPORTS": {"REPORT": False}, + "SYSTEMS": { + "OBP-A": {"MODE": "OPENBRIDGE", "ENABLED": True, "NETWORK_ID": 1, "PASSPHRASE": "shared"}, + "OBP-B": {"MODE": "OPENBRIDGE", "ENABLED": True, "NETWORK_ID": 2, "PASSPHRASE": "shared"}, + }, + } + findings = collect_findings(config, project_root=".", config_path="cfg.yaml") + shared = [f for f in findings if "same PASSPHRASE" in f.message] + assert len(shared) == 1 + assert shared[0].level == "warn" + assert "OBP-A, OBP-B" in shared[0].message + assert not any("shared" in f.message for f in findings) + + def test_collect_findings_obp_per_bridge_migration() -> None: """7301-style: OBP-CL2 on fan-in 62032; another bridge keeps legacy 62999.""" config = { diff --git a/tests/infrastructure/test_obp_proxy.py b/tests/infrastructure/test_obp_proxy.py index 49a5e2c..34a262e 100644 --- a/tests/infrastructure/test_obp_proxy.py +++ b/tests/infrastructure/test_obp_proxy.py @@ -34,6 +34,7 @@ from adn_server.application.proxy.deployment import ( obp_bridge_legacy_listen_port, obp_proxy_bind_legacy_ports, obp_proxy_enabled, + openbridge_passphrase_collisions, ) from adn_server.domain import bytes_4 from adn_server.domain.errors import ConfigError @@ -286,6 +287,40 @@ def test_validate_obp_proxy_duplicate_network_id() -> None: assert "NETWORK_ID" in str(exc.value) +def test_shared_passphrase_between_bridges_is_reported() -> None: + """Control frames carry no NETWORK_ID, so a shared passphrase leaves only the + source address to tell two bridges apart. str and bytes are the same secret.""" + config = _obp_config() + config["SYSTEMS"]["OBP-EU"] = { + **config["SYSTEMS"]["OBP-CL"], + "PORT": 62045, + "NETWORK_ID": 73045, + "PASSPHRASE": b"test-passphrase", + } + assert openbridge_passphrase_collisions(config) == [["OBP-CL", "OBP-EU"]] + + +def test_distinct_passphrases_are_not_reported() -> None: + config = _obp_config() + config["SYSTEMS"]["OBP-EU"] = { + **config["SYSTEMS"]["OBP-CL"], + "PORT": 62045, + "NETWORK_ID": 73045, + "PASSPHRASE": "another-one", + } + assert openbridge_passphrase_collisions(config) == [] + + +def test_only_enabled_openbridge_systems_count_as_a_collision() -> None: + """A disabled bridge, a MASTER and an empty passphrase are not a clash.""" + config = _obp_config() + config["SYSTEMS"]["OBP-OFF"] = {**config["SYSTEMS"]["OBP-CL"], "ENABLED": False, "NETWORK_ID": 73046} + config["SYSTEMS"]["HOTSPOT"]["PASSPHRASE"] = "test-passphrase" + config["SYSTEMS"]["OBP-BLANK"] = {**config["SYSTEMS"]["OBP-CL"], "NETWORK_ID": 73047, "PASSPHRASE": ""} + config["SYSTEMS"]["OBP-BLANK2"] = {**config["SYSTEMS"]["OBP-CL"], "NETWORK_ID": 73048, "PASSPHRASE": ""} + assert openbridge_passphrase_collisions(config) == [] + + def test_validate_obp_proxy_migrated_bridge_port_matches_listen() -> None: config = _obp_config() config["OBP_PROXY"]["LISTEN_PORT"] = 62044