From e6d1a43d86133540fd36c47a97b421c52e6eb5b4 Mon Sep 17 00:00:00 2001 From: agessaman Date: Fri, 4 Sep 2026 21:52:12 -0700 Subject: [PATCH] fix(room): a guest may read the room, not post to it simple_room_server::onPeerDataRecv takes the `(client->permissions & PERM_ACL_ROLE_MASK) == PERM_ACL_GUEST` branch for a PLAIN message and stores nothing. openHop called add_post for anyone who authenticated, so knowing the guest password was enough to write to a room: add_post caps length and rate-limits per client, but never looks at the role. Also warns once per message when openhop_core does not publish txt_type, so the open text-type gate is visible in the log rather than silent. --- repeater/handler_helpers/text.py | 33 +++++++++++++- ...test_handler_helpers_path_protocol_text.py | 43 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/repeater/handler_helpers/text.py b/repeater/handler_helpers/text.py index b181fe3..0c53b36 100644 --- a/repeater/handler_helpers/text.py +++ b/repeater/handler_helpers/text.py @@ -13,7 +13,7 @@ import time from openhop_core.node.handlers.text import TextMessageHandler from openhop_core.protocol import CryptoUtils, Identity -from .acl import is_admin_permissions +from .acl import PERM_ACL_GUEST, PERM_ACL_ROLE_MASK, is_admin_permissions from .mesh_cli import MeshCLI from .room_server import RoomServer @@ -41,6 +41,18 @@ TXT_TYPE_CLI_COMMAND = 0x03 SERVER_TXT_TYPES = frozenset((TXT_TYPE_PLAIN, TXT_TYPE_CLI_DATA, TXT_TYPE_CLI_COMMAND)) +def _is_guest_client(client) -> bool: + """True when ``client`` holds the GUEST role (firmware ``PERM_ACL_GUEST``). + + Falls back to the role bits for a client object with no ``is_guest`` + helper, so an older ACL shape still fails closed. + """ + is_guest = getattr(client, "is_guest", None) + if callable(is_guest): + return bool(is_guest()) + return (getattr(client, "permissions", 0) & PERM_ACL_ROLE_MASK) == PERM_ACL_GUEST + + class TextHelper: def __init__( self, @@ -311,6 +323,12 @@ class TextHelper: # A core older than the one that publishes ``txt_type`` reports # None; behave as before rather than dropping every message. txt_type = packet.decrypted.get("txt_type") + if txt_type is None: + logger.warning( + "TXT_MSG carried no txt_type: openhop_core is older than the " + "build that publishes it, so the text-type gate is open and a " + "signed post can reach the CLI. Upgrade openhop_core." + ) if txt_type is not None and txt_type not in SERVER_TXT_TYPES: logger.debug( f"[{identity_type}:{identity_name}] unsupported text type " @@ -378,6 +396,19 @@ class TextHelper: return # NOT a CLI command - store as regular room post + # + # A guest may read the room but not write to it: + # simple_room_server::onPeerDataRecv takes the + # `(client->permissions & PERM_ACL_ROLE_MASK) == PERM_ACL_GUEST` + # branch for a PLAIN message and stores nothing (and sends no + # ACK). Without this, anyone who can log in with the guest + # password can post to the room. + if sender_client is not None and _is_guest_client(sender_client): + logger.warning( + f"Room '{identity_name}': post denied from 0x{src_hash:02X} (guest)" + ) + return + try: # Get sender's full pubkey sender_pubkey = bytes([src_hash]) + b"\x00" * 31 # Default diff --git a/tests/test_handler_helpers_path_protocol_text.py b/tests/test_handler_helpers_path_protocol_text.py index 6c839b3..f6de021 100644 --- a/tests/test_handler_helpers_path_protocol_text.py +++ b/tests/test_handler_helpers_path_protocol_text.py @@ -564,6 +564,49 @@ async def test_text_helper_signed_plain_is_not_stored_as_a_room_post(): room.add_post.assert_awaited_once() +@pytest.mark.asyncio +@pytest.mark.parametrize( + "permissions,posts", + [ + (PERM_ACL_ADMIN, True), + (PERM_ACL_READ_WRITE, True), + (0, False), # PERM_ACL_GUEST -- read the room, do not write to it + ], +) +async def test_text_helper_room_post_denies_guests(permissions, posts): + """[fails pre-fix] A guest may read the room but not post to it. + + simple_room_server::onPeerDataRecv takes the + `(client->permissions & PERM_ACL_ROLE_MASK) == PERM_ACL_GUEST` branch for a + PLAIN message and stores nothing. openHop called add_post for anyone who + authenticated, so the guest password was enough to write to the room -- + add_post enforces a length cap and a rate limit, but no role. + """ + helper = TextHelper(identity_manager=MagicMock(), acl_dict={}) + room = MagicMock() + room.add_post = AsyncMock(return_value=True) + room.cli = None + helper.room_servers = {0x41: room} + helper.handlers = {0x41: {"name": "room", "type": "room_server", "identity": MagicMock()}} + helper._resolve_sender_client = MagicMock( + return_value=_FakeClient( + pubkey=bytes([0x21]) + b"x" * 31, + shared_secret=b"k" * 32, + permissions=permissions, + ) + ) + + await helper._on_message_received( + identity_name="room", + identity_type="room_server", + packet=_TxtPacket("hello room", 0), + dest_hash=0x41, + src_hash=0x21, + ) + + assert room.add_post.await_count == (1 if posts else 0) + + @pytest.mark.asyncio async def test_text_helper_process_text_packet_routes_or_forwards(): helper = TextHelper(identity_manager=MagicMock(), acl_dict={})