mirror of
https://github.com/pyMC-dev/pyMC_Repeater.git
synced 2026-10-31 04:01:59 +01:00
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.
This commit is contained in:
1 parent
369c6142b6
commit
e6d1a43d86
2 files changed
+75
-1
No files matched your search
@@ -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
|
||||
|
||||
@@ -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={})
|
||||
|
||||
Reference in new issue
Block a user