From 4460df713e48899ba6154a975f828d4505fe3c96 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 24 Jun 2026 03:12:49 +0100 Subject: [PATCH] Fix FBP negotiated packet version handling --- docs/test-harness-design.md | 17 +++++----- docs/testing.md | 16 ++++----- docs/v1x-codex-changelog.md | 3 ++ hblink.py | 4 +-- tests/test_deterministic_harness.py | 52 +++++++++++++++++++++++++++++ tests/test_udp_blackbox_harness.py | 1 - 6 files changed, 73 insertions(+), 20 deletions(-) diff --git a/docs/test-harness-design.md b/docs/test-harness-design.md index 71eb6d0..37a6354 100644 --- a/docs/test-harness-design.md +++ b/docs/test-harness-design.md @@ -445,13 +445,11 @@ test venv; otherwise a temporary venv is used for the scenario. with unrelated HBP-to-HBP routing. - Version-negotiation bugs visible over UDP: BCVE downgrade, unsupported version or invalid hash must not mutate the configured outbound behavior, and v4 - packet fixtures characterize the historical v4 metadata layout. -- Known protocol-version issues can be carried as expected-failure black-box - tests until runtime behavior is changed: unsupported embedded `DMRE` versions - are currently not rejected, and the v4 send layout currently carries the - module default version byte instead of the configured `PROTO_VER` value. v4 - is historical/deprecation context, not a desired long-term compatibility - target. + packet fixtures characterize the historical v4 metadata layout. Configured + v4 outbound `DMRE` packets must carry the negotiated session version byte, + while BCVE advertises the local maximum supported version so peers can + upgrade. v4 is historical/deprecation context, not a desired long-term + compatibility target. - Protocol-refusal bugs visible over UDP: signed v1 OBP packets on a v5-configured link should produce BCVE and should not leak to HBP targets. v1 itself remains supported as an open OBP interop protocol, especially for @@ -638,8 +636,9 @@ Assertions should be grouped by intent: and valid BCST STUN blocks OpenBridge traffic in both directions. - UDP black-box FBP version tests verify that BCVE downgrade, unsupported version and invalid hash do not change outbound packet version, and that - historical v4 packet fixtures currently route using the older metadata layout. - This v4 coverage is characterization/deprecation context. + historical v4 packet fixtures route using the older metadata layout with the + negotiated session version byte. This v4 coverage is + characterization/deprecation context. - UDP black-box OBP-v1 refusal tests verify that a signed v1 packet received on a v5-configured link receives BCVE and does not route onward. diff --git a/docs/testing.md b/docs/testing.md index a320b80..f87f14d 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -215,8 +215,9 @@ routing in both directions, startup acceptance of slot/route-specific timer directives, FBP keepalive/version control setup, BCKA gating of enhanced HBP-to-FBP forwarding, BCSQ source-quench suppression of HBP-to-FBP forwarding, rejection of invalid BCSQ, BCVE downgrade/unsupported/invalid-version handling, -current FBP v5 packet handling, historical FBP v4 characterization, and signed -v1 OBP packet refusal on a v5-configured link. v1 remains an important open OBP +current FBP v5 packet handling, historical FBP v4 characterization including +negotiated outbound packet version bytes, and signed v1 OBP packet refusal on a +v5-configured link. v1 remains an important open OBP interop protocol for external network bridge instances, primarily through `bridge.py`; the `bridge_master.py` UDP test here only verifies that a v5-configured FBP link refuses v1 traffic. They also cover rejection of FBP @@ -238,12 +239,11 @@ stale FBP timestamps and max-hop FBP packets; these must not leak traffic to HBP targets, and stale/max-hop FBP packets must produce a source-quench response. Selected malformed packet tests also assert subprocess warning logs. -Two UDP tests are marked as expected failures because they document current -protocol-version issues rather than fixed behavior: unsupported embedded `DMRE` -packet versions are not yet rejected, and the historical v4 send layout -currently carries the module default version byte instead of the configured -`PROTO_VER` value. v4 is characterization/deprecation context, not a long-term -protocol contract. +Protocol-version regression coverage verifies unsupported embedded `DMRE` +packet versions are rejected without poisoning the session version, and the +historical v4 send layout carries the negotiated session version byte. BCVE +advertises the local maximum supported version. v4 is +characterization/deprecation context, not a long-term protocol contract. UDP integration tests are opt-in: diff --git a/docs/v1x-codex-changelog.md b/docs/v1x-codex-changelog.md index 221eb37..d0c338f 100644 --- a/docs/v1x-codex-changelog.md +++ b/docs/v1x-codex-changelog.md @@ -78,6 +78,9 @@ peer being quenched, including dial-a-TG reflector TGs. - Made STUN/BCST handling consistent as a broad FBP traffic gate. - Preserved protocol-version-sensitive FBP/OBP metadata layout. +- Fixed outbound FBP `DMRE` version bytes to use the negotiated OpenBridge + session version rather than the module default version constant. BCVE remains + an advertisement of the local maximum supported version. - Added tests for FBP keepalive gating, wrong network ID, bad hashes, stale timestamps, max-hop handling, v4 characterization and v1 refusal on v5 links. diff --git a/hblink.py b/hblink.py index a7c5f50..7921c92 100755 --- a/hblink.py +++ b/hblink.py @@ -184,7 +184,7 @@ class OPENBRIDGE(DatagramProtocol): if _packet[:3] == DMR and self._config['TARGET_IP']: if 'VER' in self._config and self._config['VER'] > 4: - _ver = VER.to_bytes(1,'big') + _ver = self._config['VER'].to_bytes(1,'big') _packet = b''.join([DMRE,_packet[4:11], self._CONFIG['GLOBAL']['SERVER_ID'],_packet[15:],_ber,_rssi,_ver,time_ns().to_bytes(8,'big'), _source_server, _source_rptr, _hops]) _h = blake2b(key=self._config['PASSPHRASE'], digest_size=16) _h.update(_packet) @@ -193,7 +193,7 @@ class OPENBRIDGE(DatagramProtocol): self.transport.write(_packet, (self._config['TARGET_IP'], self._config['TARGET_PORT'])) elif 'VER' in self._config and self._config['VER'] == 4: - _ver = VER.to_bytes(1,'big') + _ver = self._config['VER'].to_bytes(1,'big') _packet = b''.join([DMRE,_packet[4:11], self._CONFIG['GLOBAL']['SERVER_ID'],_packet[15:],_ber,_rssi,_ver,time_ns().to_bytes(8,'big'), _source_server, _hops]) _h = blake2b(key=self._config['PASSPHRASE'], digest_size=16) _h.update(_packet) diff --git a/tests/test_deterministic_harness.py b/tests/test_deterministic_harness.py index abb61f8..ffafd54 100644 --- a/tests/test_deterministic_harness.py +++ b/tests/test_deterministic_harness.py @@ -2026,6 +2026,58 @@ DEFAULT_DIAL_TS2: 92 self.assertEqual(config["SYSTEMS"]["OBP-1"]["VER"], 5) self.assertEqual(scenario.capture.for_system("OBP-1"), []) + def test_openbridge_send_system_uses_configured_v4_packet_version(self): + config = minimal_config(()) + add_openbridge_system(config, "OBP-1", network_id=3001) + config["SYSTEMS"]["OBP-1"]["VER"] = 4 + config["SYSTEMS"]["OBP-1"]["TARGET_PORT"] = 62031 + + with DeterministicScenario(config=config) as scenario: + system = scenario.bm.systems["OBP-1"] + + scenario.bm.OPENBRIDGE.send_system(system, PacketSpec(peer_id=3001).data()[:53]) + + packet, sockaddr = scenario.transports["OBP-1"].writes[-1] + + self.assertEqual(sockaddr, ("127.0.0.1", 62031)) + self.assertEqual(packet[:4], b"DMRE") + self.assertEqual(len(packet), 85) + self.assertEqual(packet[55], 4) + + def test_openbridge_send_system_uses_configured_v5_packet_version(self): + config = minimal_config(()) + add_openbridge_system(config, "OBP-1", network_id=3001) + config["SYSTEMS"]["OBP-1"]["VER"] = 5 + config["SYSTEMS"]["OBP-1"]["TARGET_PORT"] = 62031 + + with DeterministicScenario(config=config) as scenario: + system = scenario.bm.systems["OBP-1"] + + scenario.bm.OPENBRIDGE.send_system(system, PacketSpec(peer_id=3001).data()[:53]) + + packet, sockaddr = scenario.transports["OBP-1"].writes[-1] + + self.assertEqual(sockaddr, ("127.0.0.1", 62031)) + self.assertEqual(packet[:4], b"DMRE") + self.assertEqual(len(packet), 89) + self.assertEqual(packet[55], 5) + + def test_openbridge_send_bcve_advertises_max_supported_version(self): + config = minimal_config(()) + add_openbridge_system(config, "OBP-1", network_id=3001) + config["SYSTEMS"]["OBP-1"]["VER"] = 4 + config["SYSTEMS"]["OBP-1"]["ENHANCED_OBP"] = True + config["SYSTEMS"]["OBP-1"]["TARGET_PORT"] = 62031 + + with DeterministicScenario(config=config) as scenario: + scenario.bm.systems["OBP-1"].send_bcve() + + packet, sockaddr = scenario.transports["OBP-1"].writes[-1] + + self.assertEqual(sockaddr, ("127.0.0.1", 62031)) + self.assertEqual(packet[:4], b"BCVE") + self.assertEqual(packet[4], 5) + def test_hbp_master_parser_discards_truncated_dmrd_from_connected_peer(self): config = minimal_config(("MASTER-A",)) diff --git a/tests/test_udp_blackbox_harness.py b/tests/test_udp_blackbox_harness.py index 4759612..2be1f92 100644 --- a/tests/test_udp_blackbox_harness.py +++ b/tests/test_udp_blackbox_harness.py @@ -980,7 +980,6 @@ class UdpBlackBoxHarnessTest(unittest.TestCase): self.assertEqual(len(captured.packet), 85) self.assertEqual(captured.fields["source_rptr"], b"\x00\x00\x00\x00") - @unittest.expectedFailure def test_fbp_configured_proto_v4_outbound_packet_carries_v4_version_byte(self): require_udp_integration_enabled()