From 31ffe3cb9334668488bf9c464d48d41769c05f0a Mon Sep 17 00:00:00 2001 From: TX-RX Date: Thu, 30 Apr 2026 21:52:14 -0500 Subject: [PATCH 01/22] fix(mumble): security fix, ATAK VX voice plugin auth, and direction enforcement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Security * Read Ice secret from OTS_ICE_SECRET (was hardcoded as empty string, so any process on localhost could administer Murmur over Ice without authentication). * Add OTS_ICE_SECRET = "" to DefaultConfig so the setting is discoverable. ## ATAK VX voice plugin auth The official ATAK VX voice plugin connects to Murmur as the device's callsign (not the OTS username), appends `---` per connection, and converts spaces in callsigns to underscores. Authenticator now resolves this via a single lookup chain: OTS username -> EUD callsign -> base callsign with `---` stripped -> underscore->space -> cert CN (EUD UID). The cert-CN fallback survives mid-session callsign renames: ATAK auths with the new callsign immediately, but the OTS EUDs.callsign column only updates on the next CoT. ATAK clients are validated by client cert (TLS layer), not password, so verify_password() is skipped when the lookup matched via callsign or cert. ## Murmur authority (Murmur 1.3 cred-check bug) Murmur 1.3 caches Ice-authed users in its local users table on first connect. On reconnect it demands a cert hash or password BEFORE calling Ice authenticate(), and rejects with "Wrong certificate or password for existing user" — so every reconnect failed. Implementing nameToId/idToName/getInfo to return authoritative answers tells Murmur to defer to Ice instead of its local table, which fixes both reconnects and direct-call/whisper lookups. ## VX dual-connection support The VX plugin opens two simultaneous Mumble sockets per device, each with a different `---` suffix. mumble_identity() returns base_id + a deterministic offset per callsign so both sessions get unique Mumble user ids. ## Direction enforcement DirectionEnforcementCallback applies Murmur's suppress flag from each user's OTS group direction (IN=speak, OUT=listen-only). DB queries run in the Ice dispatch thread (same as authenticate()); only getState/setState are dispatched to a background daemon thread to avoid Ice thread-pool deadlock. Admins skip the suppress check entirely (calling getState() for an admin session blocks for 30s and crashes the Ice connection). Duplicate callback registration is guarded per server_id and cleared by on_server_stopped() so virtual-server restarts re-register correctly. ## Bug fixes folded in * Linux import crash from `from opentakserver.models.User` (correct case is `user`) * Admin detection via `role.name == 'administrator'` instead of fragile `len(user.groups) >= 6` heuristic * Ice method stubs now return correct sentinels (-2, -1, 0, b"", {}) instead of None which serialization in some Murmur versions doesn't accept Co-Authored-By: Claude Opus 4.7 (1M context) --- opentakserver/defaultconfig.py | 1 + opentakserver/mumble/mumble_authenticator.py | 249 +++++++++++++----- opentakserver/mumble/mumble_ice_app.py | 255 ++++++++++++++++++- 3 files changed, 436 insertions(+), 69 deletions(-) diff --git a/opentakserver/defaultconfig.py b/opentakserver/defaultconfig.py index 65d7b932..0fb6454f 100644 --- a/opentakserver/defaultconfig.py +++ b/opentakserver/defaultconfig.py @@ -140,6 +140,7 @@ class DefaultConfig: OTS_PROFILE_MAP_SOURCES = True OTS_ENABLE_MUMBLE_AUTHENTICATION = False + OTS_ICE_SECRET = "" OTS_IP_WHITELIST = ["127.0.0.1"] diff --git a/opentakserver/mumble/mumble_authenticator.py b/opentakserver/mumble/mumble_authenticator.py index eff7c741..6d180c6c 100644 --- a/opentakserver/mumble/mumble_authenticator.py +++ b/opentakserver/mumble/mumble_authenticator.py @@ -1,6 +1,8 @@ +import hashlib import os import Ice +from cryptography import x509 from flask import Flask from flask_ldap3_login import AuthenticationResponseStatus from flask_security import verify_password @@ -18,39 +20,138 @@ import Murmur -class MumbleAuthenticator(Murmur.ServerUpdatingAuthenticator): - texture_cache = {} +# Each OTS user gets a 1000-id range in Mumble. PC username auth uses the base +# id (user.id * 1000); ATAK callsign auth uses base + a deterministic offset +# derived from the callsign (so a single OTS account can connect from multiple +# devices simultaneously, as the official ATAK VX voice plugin does). +MUMBLE_ID_RANGE = 1000 +MUMBLE_ID_CALLSIGN_OFFSET_RANGE = MUMBLE_ID_RANGE - 1 + +class MumbleAuthenticator(Murmur.ServerUpdatingAuthenticator): def __init__(self, app, logger, ice): Murmur.ServerUpdatingAuthenticator.__init__(self) self.app: Flask = app self.logger = logger self.ice = ice - def authenticate(self, username, password, certlist, certhash, strong, current=None): + # ----- public lookup helpers (also used by mumble_ice_app) --------------- + + @staticmethod + def resolve_identity(app, username, certlist=None): + """Look up an OTS user from a Mumble username, ATAK callsign, or cert. + + Lookup chain (first match wins): + 1. OTS username (PC clients) + 2. EUD callsign exact match + 3. EUD callsign with `---` suffix stripped (ATAK adds this) + 4. Above with `_` -> ` ` (ATAK Mumble plugin replaces spaces in callsigns) + 5. Cert CN -> EUD.uid (immutable; survives callsign renames) + + Does NOT verify password. Returns (user_or_None, is_callsign_auth_bool). + """ + from opentakserver.models.EUD import EUD + + user = app.security.datastore.find_user(username=username) + if user: + return user, False + + eud = EUD.query.filter_by(callsign=username).first() + + base_callsign = username + if not eud and "---" in username: + base_callsign = username.split("---")[0] + eud = EUD.query.filter_by(callsign=base_callsign).first() + + if not eud: + spaced = base_callsign.replace("_", " ") + if spaced != base_callsign: + eud = EUD.query.filter_by(callsign=spaced).first() + + if not eud and certlist: + eud = MumbleAuthenticator._eud_from_cert(certlist) + + if eud and eud.user_id: + user = app.security.datastore.find_user(id=eud.user_id) + if user: + return user, True + return None, False + + @staticmethod + def _eud_from_cert(certlist): + """Look up an EUD by parsing the client cert chain's CN. + + ATAK device certs use the EUD UID (e.g. `ANDROID-xxxx`) as the cert CN, + so this lookup survives mid-session callsign renames -- the OTS EUDs + table only updates on the next CoT, but the Mumble plugin auths + immediately with the new name. """ - This function is called to authenticate a user + from opentakserver.models.EUD import EUD + + for cert_bytes in certlist: + try: + cert = x509.load_der_x509_certificate(cert_bytes) + cns = cert.subject.get_attributes_for_oid(x509.NameOID.COMMON_NAME) + if not cns: + continue + eud = EUD.query.filter_by(uid=cns[0].value).first() + if eud: + return eud + except Exception: + continue + return None + + @staticmethod + def mumble_identity(user, is_callsign_auth, presented_username): + """Return (mumble_user_id, display_name) for an authenticated user. + + PC username auth -> (user.id * 1000, user.username) + ATAK callsign auth -> (user.id * 1000 + hash(callsign) % 999 + 1, callsign) + + The hash offset lets one OTS account connect from multiple ATAK devices + simultaneously, each with a unique Mumble user id (the VX plugin even + opens two sockets per device, each with a different `---` suffix). + """ + if is_callsign_auth: + digest = int(hashlib.md5(presented_username.encode()).hexdigest(), 16) + offset = digest % MUMBLE_ID_CALLSIGN_OFFSET_RANGE + 1 + return user.id * MUMBLE_ID_RANGE + offset, presented_username + return user.id * MUMBLE_ID_RANGE, user.username + + # ----- Murmur Ice callbacks --------------------------------------------- + + def authenticate(self, username, password, certlist, certhash, strong, current=None): + """Authenticate a Mumble client. + + Returns (mumble_id, display_name, group_list) on success or + (-1, None, None) on failure. Returning -2 tells Murmur to use its + own auth fallback (used only for SuperUser). """ if username == "SuperUser": - return (-2, None, None) + return -2, None, None self.logger.info("Mumble auth request for {}".format(username)) with self.app.app_context(): - user = self.app.security.datastore.find_user(username=username) + user, is_callsign_auth = self.resolve_identity(self.app, username, certlist) + if not user: - self.logger.warning("Mumble auth: User {} not found".format(username)) + self.logger.warning("Mumble auth: user {} not found".format(username)) return -1, None, None - elif not user.active: - self.logger.warning("Mumble auth: User {} is deactivated".format(username)) + if not user.active: + self.logger.warning("Mumble auth: user {} is deactivated".format(username)) return -1, None, None + mumble_groups = [g.name for g in user.groups] + if any(r.name == "administrator" for r in user.roles): + mumble_groups.append("admin") + + authenticated = False + if self.app.config.get("OTS_ENABLE_LDAP"): auth_result = ldap_manager.authenticate(username, password) if auth_result.status == AuthenticationResponseStatus.success: - self.logger.info("Mumble auth: {} has been authenticated".format(username)) - - # Keep this import here to avoid a circular import when OTS is started + # Keep this import inline to avoid a circular import at startup. from opentakserver.blueprints.ots_api.ldap_api import save_user save_user( @@ -59,73 +160,103 @@ def authenticate(self, username, password, certlist, certhash, strong, current=N auth_result.user_info, auth_result.user_groups, ) - - return user.id, user.username, None - + authenticated = True + elif is_callsign_auth: + # ATAK clients authenticate by client cert, not password. + # The cert was already validated by Murmur's TLS layer before + # this callback was invoked; we only need to verify that the + # presented identity (callsign or cert CN) maps to an EUD. + authenticated = True elif verify_password(password, user.password): - self.logger.info("Mumble auth: {} has been authenticated".format(username)) - return user.id, user.username, None + authenticated = True - self.logger.warning("Mumble auth: Bad password for {}".format(username)) - return -1, None, None + if not authenticated: + self.logger.warning("Mumble auth: bad password for {}".format(username)) + return -1, None, None - def idToTexture(self, id, current=None): - return + mumble_id, display_name = self.mumble_identity(user, is_callsign_auth, username) + self.logger.info( + "Mumble auth: id={} display={} groups={}".format( + mumble_id, display_name, mumble_groups + ) + ) + return mumble_id, display_name, mumble_groups def getInfo(self, id, current=None): + """Return user info to Murmur so it stays authoritative for Ice-authed users. + + Murmur 1.3 will otherwise look up cert/password against its local + ``user_info`` table, which is empty for Ice-authed users. That causes + a "Wrong certificate or password for existing user" rejection on every + reconnect -- before Ice authenticate() is even called. """ - Gets called to fetch user specific information - """ + if id is None or id <= 0: + return False, None + try: + with self.app.app_context(): + from opentakserver.extensions import db + from opentakserver.models.user import User - # We do not expose any additional information so always fall through - return False, None + user = db.session.get(User, id // MUMBLE_ID_RANGE) + if not user: + return False, None + info = {Murmur.UserInfo.UserName: user.username} + if user.email: + info[Murmur.UserInfo.UserEmail] = user.email + return True, info + except Exception as e: + self.logger.error("Mumble getInfo({}) failed: {}".format(id, e)) + return False, None def nameToId(self, name, current=None): + """Tell Murmur the Mumble id that owns a given name. + + Returning -2 (fall through) makes Murmur consult its local users table, + which causes the rename/reconnect rejection bug; returning the encoded + id keeps Ice authoritative. """ - Gets called to get the id for a given username - """ - pass + if not name or name == "SuperUser": + return -2 + try: + with self.app.app_context(): + user, is_callsign_auth = self.resolve_identity(self.app, name) + if not user: + return -2 + mumble_id, _ = self.mumble_identity(user, is_callsign_auth, name) + return mumble_id + except Exception as e: + self.logger.error("Mumble nameToId({}) failed: {}".format(name, e)) + return -2 def idToName(self, id, current=None): - """ - Gets called to get the username for a given id - """ - pass + """Return display name for a Mumble id. Used for ACL/log lookups.""" + if id is None or id <= 0: + return "" + try: + with self.app.app_context(): + from opentakserver.extensions import db + from opentakserver.models.user import User + + user = db.session.get(User, id // MUMBLE_ID_RANGE) + return user.username if user else "" + except Exception as e: + self.logger.error("Mumble idToName({}) failed: {}".format(id, e)) + return "" def idToTexture(self, id, current=None): - """ - Gets called to get the corresponding texture for a user - """ - # seems like it pulled a user's avatar from a phpbb DB + return b"" - def registerUser(self, name, current=None): - """ - Gets called when the server is asked to register a user. - """ - pass + def registerUser(self, info, current=None): + return -2 def unregisterUser(self, id, current=None): - """ - Gets called when the server is asked to unregister a user. - """ - pass + return -1 def getRegisteredUsers(self, filter, current=None): - """ - Returns a list of usernames in the phpBB3 database which contain - filter as a substring. - """ - pass + return {} def setInfo(self, id, info, current=None): - """ - Gets called when the server is supposed to save additional information - about a user to his database - """ - pass + return 0 def setTexture(self, id, texture, current=None): - """ - Gets called when the server is asked to update the user texture of a user - """ - pass + return -1 diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index 3bc48b8a..2128b515 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -1,5 +1,6 @@ import os import threading +import time from threading import Timer import Ice @@ -35,12 +36,20 @@ def run(self): idata = Ice.InitializationData() idata.properties = props - # Create Ice connection + # Create Ice connection. The secret must be in ImplicitContext + # before any proxy call (e.g. checkedCast below) or Murmur rejects + # with InvalidSecretException. ice = Ice.initialize(idata) - proxy = ice.stringToProxy("Meta:tcp -h 127.0.0.1 -p 6502") - secret = "" - if secret != "": + secret = self.app.config.get("OTS_ICE_SECRET", "") + if secret: ice.getImplicitContext().put("secret", secret) + else: + self.logger.warning( + "OTS_ICE_SECRET is empty; Murmur will reject Ice calls if its " + "icesecretread/icesecretwrite is set." + ) + + proxy = ice.stringToProxy("Meta:tcp -h 127.0.0.1 -p 6502") try: meta = Murmur.MetaPrx.checkedCast(proxy) except Ice.ConnectionRefusedException: @@ -63,6 +72,9 @@ def __init__(self, app, logger, ice): self.failed_watch = False self.watchdog = None self.auth = None + self.adapter = None + # server_id -> ServerCallbackPrx; guards against duplicate registration + self.server_callbacks = {} def run(self, *args): if not self.initialize_ice_connection(): @@ -81,18 +93,17 @@ def run(self, *args): def initialize_ice_connection(self): """ Establishes the two-way Ice connection and adds the authenticator to the - configured servers + configured servers. The Ice secret was already pushed into the shared + ImplicitContext by MumbleIceDaemon.run(). """ - # if False and 'ice_secret': - # self.ice.getImplicitContext().put("secret", "some_secret") - self.logger.debug("Connecting to Ice server ({}:{})".format("127.0.0.1", 6502)) base = self.ice.stringToProxy("Meta:tcp -h {} -p {}".format("127.0.0.1", 6502)) self.meta = Murmur.MetaPrx.uncheckedCast(base) adapter = self.ice.createObjectAdapterWithEndpoints("Callback.Client", "tcp -h 127.0.0.1") adapter.activate() + self.adapter = adapter metacbprx = adapter.addWithUUID(MetaCallback(self)) self.metacb = Murmur.MetaCallbackPrx.uncheckedCast(metacbprx) @@ -117,6 +128,7 @@ def attach_callbacks(self): "Setting mumble authenticator for virtual server {}".format(server.id()) ) server.setAuthenticator(self.auth) + self.attach_server_callback(server) except ( Murmur.InvalidSecretException, @@ -141,6 +153,38 @@ def attach_callbacks(self): self.connected = True return True + def attach_server_callback(self, server): + """Register DirectionEnforcementCallback for IN/OUT suppress enforcement. + + Guarded against duplicate registration — check_connection() calls + attach_callbacks() every 10 seconds. The guard is cleared by + on_server_stopped() so a restarted virtual server gets a fresh + callback correctly. + """ + server_id = server.id() + if server_id in self.server_callbacks: + return + + cb = DirectionEnforcementCallback(self.app, self.logger, server) + cbprx = self.adapter.addWithUUID(cb) + server_cb = Murmur.ServerCallbackPrx.uncheckedCast(cbprx) + + try: + server.addCallback(server_cb) + self.server_callbacks[server_id] = server_cb + self.logger.info(f"Direction enforcement callback attached to server {server_id}") + except Exception as e: + self.logger.error(f"Failed to attach server callback to {server_id}: {e}") + + def on_server_stopped(self, server_id): + """Clear the callback guard and any cached session state for a stopped server. + + Without this, the duplicate-registration guard would prevent re-registration + when the virtual server restarts. + """ + self.server_callbacks.pop(server_id, None) + self.logger.info(f"Cleared callback guard for stopped server {server_id}") + def check_connection(self): """ Tries reapplies all callbacks to make sure the authenticator @@ -159,6 +203,193 @@ def check_connection(self): self.watchdog.start() +class DirectionEnforcementCallback(Murmur.ServerCallback): + """Enforces OTS IN/OUT speak direction by setting Murmur's suppress flag. + + Channel access (who can enter which channel) is controlled by Murmur's + own ACL configuration. This callback's only job is to mute users whose + OTS group membership has direction=OUT (listen-only) and unmute those + with direction=IN. + + All Ice proxy calls (getState/setState) are dispatched to a background + daemon thread to avoid deadlocking the Ice thread pool. + """ + + def __init__(self, app, logger, server): + Murmur.ServerCallback.__init__(self) + self.app = app + self.logger = logger + self.server = server + self.server_id = server.id() + self._channel_cache = None + self._channel_cache_time = 0 + self._session_lock = threading.Lock() + self._session_cache = {} # session_id -> {directions, is_admin, cached_at} + + # ------------------------------------------------------------------ helpers + + def _get_channel_map(self): + """Return {channel_id: channel_name}, cached for 60 seconds.""" + if self._channel_cache is None or (time.time() - self._channel_cache_time) > 60: + try: + channels = self.server.getChannels() + self._channel_cache = {cid: ch.name for cid, ch in channels.items()} + self._channel_cache_time = time.time() + except Exception as e: + self.logger.error(f"Failed to refresh channel map: {e}") + return self._channel_cache or {} + return self._channel_cache + + def _get_user_directions(self, session_id, username): + """Return (group_directions dict, is_admin) for a user, cached for 30 seconds. + + group_directions maps group_name -> 'IN' or 'OUT'. + Prefers IN over OUT when a user has both rows for the same group. + """ + cache_ttl = 30 + now = time.time() + + with self._session_lock: + cached = self._session_cache.get(session_id) + if cached and (now - cached['cached_at']) < cache_ttl: + return cached['directions'], cached['is_admin'] + + group_directions = {} + is_admin = False + + try: + with self.app.app_context(): + # Reuse the authenticator's lookup chain (username -> callsign -> + # base callsign -> underscore->space) so direction enforcement + # finds users by the same path Mumble auth used. + user, _ = MumbleAuthenticator.resolve_identity(self.app, username) + + if not user: + self.logger.warning(f"Direction lookup: OTS user not found for '{username}'") + return {}, False + + for membership in user.group_memberships: + if not membership.enabled: + continue + grp = membership.group.name + # Prefer IN over OUT if both rows exist for the same group + if group_directions.get(grp) != 'IN': + group_directions[grp] = membership.direction + + is_admin = any(r.name == 'administrator' for r in user.roles) + except Exception as e: + self.logger.error(f"Direction lookup failed for '{username}': {e}", exc_info=True) + return {}, False + + with self._session_lock: + self._session_cache[session_id] = { + 'directions': group_directions, + 'is_admin': is_admin, + 'cached_at': now, + } + + return group_directions, is_admin + + def _dispatch_apply(self, session, username, channel_id, group_directions, is_admin): + """Dispatch only the Ice state calls to a background thread. + + DB queries run in the Ice dispatch thread (same as authenticate() — works fine). + Only getState()/setState() must be off-thread to avoid deadlocking the Ice pool. + """ + threading.Thread( + target=self._apply_direction, + args=(session, username, channel_id, group_directions, is_admin), + daemon=True, + ).start() + + def _apply_direction(self, session, username, channel_id, group_directions, is_admin): + """Background thread: apply suppress flag via Ice calls only.""" + try: + # Admins are never suppressed by this callback, so skip all Ice calls. + # Calling getState() here blocks for 30s and crashes the connection. + if is_admin: + return + + channel_map = self._get_channel_map() + channel_name = channel_map.get(channel_id, f"unknown({channel_id})") + + # Root channel: always allow speaking + if channel_name == 'Root': + try: + s = self.server.getState(session) + if s.suppress: + s.suppress = False + self.server.setState(s) + self.logger.info(f"UNMUTED (Root): {username}") + except Exception as e: + self.logger.error(f"Failed to clear suppress for '{username}' in Root: {e}") + return + + direction = group_directions.get(channel_name) + if direction is None: + return + + s = self.server.getState(session) + should_suppress = (direction == 'OUT') + + if should_suppress and not s.suppress: + s.suppress = True + self.server.setState(s) + self.logger.info(f"LISTEN ONLY: {username} in {channel_name} (direction=OUT)") + try: + self.server.sendMessage( + session, + f"Listen Only: You are receive-only in {channel_name}.", + ) + except Exception: + pass + + elif not should_suppress and s.suppress: + s.suppress = False + self.server.setState(s) + self.logger.info(f"SPEAK ENABLED: {username} in {channel_name} (direction=IN)") + + except Exception as e: + self.logger.error( + f"Unhandled error applying direction for '{username}' session={session}: {e}", + exc_info=True, + ) + + # ----------------------------------------------------------- Ice callbacks + + def userConnected(self, state, current=None): + self.logger.info( + f"User connected: {state.name} (session={state.session}, userid={state.userid}) " + f"channel={state.channel}" + ) + # DB lookup runs here in the Ice dispatch thread (safe — same as authenticate()) + directions, is_admin = self._get_user_directions(state.session, state.name) + # Only the Ice getState/setState calls go to a background thread + self._dispatch_apply(state.session, state.name, state.channel, directions, is_admin) + + def userDisconnected(self, state, current=None): + with self._session_lock: + self._session_cache.pop(state.session, None) + self.logger.info(f"User disconnected: {state.name} (session={state.session})") + + def userStateChanged(self, state, current=None): + """Fire on any state change — channel moves trigger direction re-check.""" + directions, is_admin = self._get_user_directions(state.session, state.name) + self._dispatch_apply(state.session, state.name, state.channel, directions, is_admin) + + def userTextMessage(self, state, message, current=None): + pass + + def channelCreated(self, state, current=None): + self._channel_cache = None + + def channelRemoved(self, state, current=None): + self._channel_cache = None + + def channelStateChanged(self, state, current=None): + self._channel_cache = None + + class MetaCallback(Murmur.MetaCallback): def __init__(self, authenticator): Murmur.MetaCallback.__init__(self) @@ -169,11 +400,13 @@ def started(self, server, current=None): This function is called when a virtual server is started and makes sure an authenticator gets attached if needed. """ + server_id = server.id() self.authenticator.logger.info( - "Setting authenticator for virtual server {}".format(server.id()) + "Virtual server {} started — attaching authenticator and direction callback".format(server_id) ) try: server.setAuthenticator(self.authenticator.auth) + self.authenticator.attach_server_callback(server) # Apparently this server was restarted without us noticing except (Murmur.InvalidSecretException, Ice.UnknownUserException) as e: if hasattr(e, "unknown") and e.unknown != "Murmur::InvalidSecretException": @@ -190,9 +423,11 @@ def stopped(self, server, current=None): # Only try to output the server id if we think we are still connected to prevent # flooding of our thread pool try: + server_id = server.id() self.authenticator.logger.info( - "Authenticated virtual server {} got stopped".format(server.id()) + "Virtual server {} stopped — clearing callback guard".format(server_id) ) + self.authenticator.on_server_stopped(server_id) return except Ice.ConnectionRefusedException: self.authenticator.connected = False From cbc19e21b216769036eacc3eee1a12346a655b4e Mon Sep 17 00:00:00 2001 From: TX-RX Date: Fri, 1 May 2026 18:24:35 -0500 Subject: [PATCH 02/22] feat(mumble): auto-sync Mumble channels to OTS groups DirectionEnforcementCallback keys IN/OUT speak permissions on channel_name == group_name, so a new OTS group with no matching Mumble channel had no voice room for users to join. Channels had to be created manually in Murmur and could drift from the group list. - mumble_ice_app: add sync_channels_from_groups(server) and request_sync(). Sync runs on startup per booted virtual server (after attach_server_callback) and on demand from blueprints via app.extensions["mumble_ice_app"]. - group_api: trigger request_sync() after add_group / delete_group success. One-way (create-only): channels for deleted groups are logged as warnings but not removed, to avoid kicking active voice users. Operators can prune manually. __ANON__ is skipped. Co-Authored-By: Claude Opus 4.7 (1M context) --- opentakserver/blueprints/ots_api/group_api.py | 17 +++++ opentakserver/mumble/mumble_ice_app.py | 63 +++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/opentakserver/blueprints/ots_api/group_api.py b/opentakserver/blueprints/ots_api/group_api.py index e3d896c7..35c86165 100644 --- a/opentakserver/blueprints/ots_api/group_api.py +++ b/opentakserver/blueprints/ots_api/group_api.py @@ -18,6 +18,21 @@ group_api = Blueprint("group_api", __name__) +def _request_mumble_channel_sync(): + """Best-effort: ask the Mumble Ice daemon to sync channels to OTS groups. + + Why: channel name == group name is required for direction enforcement, so + a new OTS group needs a matching Mumble channel. Silent no-op if the Ice + daemon isn't running. + """ + try: + ice_app = app.extensions.get("mumble_ice_app") + if ice_app is not None: + ice_app.request_sync() + except Exception as e: + logger.warning(f"Failed to request Mumble channel sync: {e}") + + @group_api.route("/api/groups") @roles_required("administrator") def get_groups(): @@ -318,6 +333,7 @@ def add_group(): 500, ) + _request_mumble_channel_sync() return jsonify({"success": True}) @@ -469,4 +485,5 @@ def delete_group(): 500, ) + _request_mumble_channel_sync() return jsonify({"success": True}) diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index 2128b515..c8eee137 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -75,6 +75,9 @@ def __init__(self, app, logger, ice): self.adapter = None # server_id -> ServerCallbackPrx; guards against duplicate registration self.server_callbacks = {} + # Expose this daemon to Flask blueprints so group_api can request channel + # syncs after add/delete. app.extensions is a plain dict; reads are thread-safe. + self.app.extensions["mumble_ice_app"] = self def run(self, *args): if not self.initialize_ice_connection(): @@ -176,6 +179,66 @@ def attach_server_callback(self, server): except Exception as e: self.logger.error(f"Failed to attach server callback to {server_id}: {e}") + self.sync_channels_from_groups(server) + + def sync_channels_from_groups(self, server): + """Create a root-level Mumble channel for each OTS group lacking one. + + Channel name == group name so DirectionEnforcementCallback's lookup + (which keys by channel name) keeps working. Skips __ANON__. Never + deletes channels — too risky if users are mid-conversation; logs instead. + """ + try: + with self.app.app_context(): + from opentakserver.extensions import db + from opentakserver.models.Group import Group + rows = db.session.query(Group).all() + group_names = {g.name for g in rows if g.name and g.name != "__ANON__"} + + if not group_names: + return + + existing = server.getChannels() + root_names = {ch.name for ch in existing.values() if ch.parent == 0} + + missing = group_names - root_names + stale = root_names - group_names - {"Root"} + + for name in sorted(missing): + try: + cid = server.addChannel(name, 0) + self.logger.info( + f"Mumble channel created for OTS group '{name}' " + f"(server={server.id()}, channel_id={cid})" + ) + except Exception as e: + self.logger.error(f"Failed to create channel '{name}': {e}") + + for name in sorted(stale): + self.logger.warning( + f"Mumble channel '{name}' has no matching OTS group " + f"(server={server.id()}); leaving in place" + ) + except Exception as e: + self.logger.error( + f"sync_channels_from_groups failed: {e}", exc_info=True + ) + + def request_sync(self): + """Trigger a channel sync on all booted servers off-thread. + + Called by group_api after add/delete so newly-created groups get a + Mumble channel without waiting for the next service restart. + """ + threading.Thread(target=self._sync_all_servers, daemon=True).start() + + def _sync_all_servers(self): + try: + for server in self.meta.getBootedServers(): + self.sync_channels_from_groups(server) + except Exception as e: + self.logger.error(f"_sync_all_servers: {e}", exc_info=True) + def on_server_stopped(self, server_id): """Clear the callback guard and any cached session state for a stopped server. From 23eb6f573273adefb85b5792f53e466625951ab0 Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sat, 9 May 2026 22:38:47 -0500 Subject: [PATCH 03/22] feat(mumble): grant MakeTempChannel for VX direct/conference calls Without this, the OTS install's locked-down ACL model (revoke @all on group channels, grant access only to members; admin gets MakeTempChannel only on Root) blocks the ATAK VX plugin's direct-call feature for non-admin users -- they can't create the temp channel that hosts the 1:1. And invited users from a different OTS group can't enter a temp under another group's channel because nothing propagates. Two-part fix in mumble_ice_app.py: 1. _ensure_temp_channel_acls runs as part of sync_channels_from_groups: idempotently OR MakeTempChannel into the auth//admin grants on Root and each OTS-managed channel. Inherited entries are filtered before setACL so we don't shadow parent ACLs. 2. DirectionEnforcementCallback.channelCreated now applies an explicit conference-capable ACL to every freshly created temp channel: @all+Traverse, @auth+(Enter|Speak|Whisper|TextMessage), @admin+full, with inherit=False so the result is the same regardless of where VX places the temp. Dispatched off the Ice thread. Gated by OTS_MUMBLE_ENABLE_CONFERENCE_CALLS (default True) so admins can revert to the legacy admin-only model if needed. --- opentakserver/defaultconfig.py | 6 ++ opentakserver/mumble/mumble_ice_app.py | 144 +++++++++++++++++++++++++ 2 files changed, 150 insertions(+) diff --git a/opentakserver/defaultconfig.py b/opentakserver/defaultconfig.py index 0fb6454f..7fd05199 100644 --- a/opentakserver/defaultconfig.py +++ b/opentakserver/defaultconfig.py @@ -141,6 +141,12 @@ class DefaultConfig: OTS_ENABLE_MUMBLE_AUTHENTICATION = False OTS_ICE_SECRET = "" + # When True, OTS grants MakeTempChannel to authenticated users on Root + # and on each OTS-managed channel, and applies a conference-capable ACL + # to every temp channel as it's created. Required for ATAK VX direct + # calls to work for non-admin users. Set False to keep the legacy + # admin-only model. + OTS_MUMBLE_ENABLE_CONFERENCE_CALLS = True OTS_IP_WHITELIST = ["127.0.0.1"] diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index c8eee137..1dfe1b7f 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -18,6 +18,27 @@ import Murmur +# Murmur permission bit masks — names mirror Murmur.ice for grep-ability. +PERM_TRAVERSE = 0x002 +PERM_ENTER = 0x004 +PERM_SPEAK = 0x008 +PERM_WHISPER = 0x100 +PERM_TEXT_MESSAGE = 0x200 +PERM_MAKE_TEMP_CHANNEL = 0x400 + +# Baseline speak/text grant used on temp channels so any authenticated user +# (regardless of which OTS group they belong to) can join a VX-initiated +# conference call once invited. +PERM_BASELINE_SPEAK = ( + PERM_TRAVERSE | PERM_ENTER | PERM_SPEAK | PERM_WHISPER | PERM_TEXT_MESSAGE +) + +# Same admin grant Murmur uses on Root by default — gives admins full control +# of temp channels (kick stragglers, link, etc.) without touching their existing +# per-channel admin rights. +PERM_ADMIN_FULL = 0x707FF + + class MumbleIceDaemon(threading.Thread): def __init__(self, app, logger): super().__init__() @@ -219,11 +240,85 @@ def sync_channels_from_groups(self, server): f"Mumble channel '{name}' has no matching OTS group " f"(server={server.id()}); leaving in place" ) + + if self.app.config.get("OTS_MUMBLE_ENABLE_CONFERENCE_CALLS", True): + self._ensure_temp_channel_acls(server, group_names) except Exception as e: self.logger.error( f"sync_channels_from_groups failed: {e}", exc_info=True ) + def _ensure_temp_channel_acls(self, server, managed_names): + """Grant MakeTempChannel on Root and each OTS-managed channel. + + The OTS install's ACL model locks group channels to their members + (revoke @all, grant only to ) and grants MakeTempChannel + only to admins on Root. That blocks the ATAK VX plugin's direct-call + feature for non-admin users, since VX needs to create a temp channel + for the 1:1. This pass ORs MakeTempChannel into the auth// + admin grants on the parents that gate creation. + + Channel-level ACLs on the temp itself are set separately by + DirectionEnforcementCallback.channelCreated when a temp is created. + """ + try: + channels = server.getChannels() + except Exception as e: + self.logger.error(f"getChannels failed: {e}") + return + + for channel_id, ch in channels.items(): + if channel_id != 0 and ch.name not in managed_names: + continue + self._ensure_make_temp_channel_acl(server, channel_id, ch.name) + + def _ensure_make_temp_channel_acl(self, server, channel_id, channel_name): + """Idempotently OR MakeTempChannel into auth//admin grants. + + Only modifies non-inherited entries; inherited rows are filtered out + before setACL (passing them back would shadow the parent's ACL and + break propagation). Skips any existing entry that already has the + bit set, so this is safe to re-run on every watchdog tick. + """ + try: + acls, groups, inherit = server.getACL(channel_id) + except Exception as e: + self.logger.error( + f"getACL({channel_id}, name={channel_name}) failed: {e}" + ) + return + + targets = {"auth", "admin"} + if channel_name and channel_name not in ("Root", "__ANON__"): + targets.add(channel_name) + + own_acls = [a for a in acls if not a.inherited] + dirty = False + for acl in own_acls: + if acl.group in targets and not (acl.allow & PERM_MAKE_TEMP_CHANNEL): + before = acl.allow + acl.allow |= PERM_MAKE_TEMP_CHANNEL + self.logger.info( + f"Granting MakeTempChannel on channel_id={channel_id} " + f"name={channel_name} group={acl.group}: " + f"0x{before:x} -> 0x{acl.allow:x}" + ) + dirty = True + + if not dirty: + return + + try: + server.setACL(channel_id, own_acls, groups, inherit) + self.logger.info( + f"Committed ACL update on channel_id={channel_id} name={channel_name}" + ) + except Exception as e: + self.logger.error( + f"setACL({channel_id}, name={channel_name}) failed: {e}", + exc_info=True, + ) + def request_sync(self): """Trigger a channel sync on all booted servers off-thread. @@ -445,6 +540,55 @@ def userTextMessage(self, state, message, current=None): def channelCreated(self, state, current=None): self._channel_cache = None + if not state.temporary: + return + if not self.app.config.get("OTS_MUMBLE_ENABLE_CONFERENCE_CALLS", True): + return + # Dispatch the ACL set off the Ice thread — setACL is a synchronous + # Ice call and would otherwise block the dispatcher. Same pattern as + # _dispatch_apply for direction enforcement. + threading.Thread( + target=self._apply_temp_channel_acl, + args=(state.id, state.name), + daemon=True, + ).start() + + def _apply_temp_channel_acl(self, channel_id, channel_name): + """Lock down a freshly-created temp channel with a conference-capable ACL. + + Without this, a temp channel under a group channel inherits nothing + useful (the group channel's apply_sub=0 entries don't propagate), so + invited users from a different OTS group can't enter — breaking VX + cross-group conference calls. Setting an explicit ACL with inherit=False + guarantees the same behavior regardless of where VX places the temp: + any authenticated user can enter/speak; admins keep full control. + """ + try: + acl_all = Murmur.ACL( + applyHere=True, applySubs=False, inherited=False, + userid=-1, group="all", allow=PERM_TRAVERSE, deny=0, + ) + acl_auth = Murmur.ACL( + applyHere=True, applySubs=False, inherited=False, + userid=-1, group="auth", allow=PERM_BASELINE_SPEAK, deny=0, + ) + acl_admin = Murmur.ACL( + applyHere=True, applySubs=False, inherited=False, + userid=-1, group="admin", allow=PERM_ADMIN_FULL, deny=0, + ) + self.server.setACL( + channel_id, [acl_all, acl_auth, acl_admin], [], False + ) + self.logger.info( + f"Temp channel ACL set: id={channel_id} name='{channel_name}' " + f"(auth=0x{PERM_BASELINE_SPEAK:x}, admin=0x{PERM_ADMIN_FULL:x})" + ) + except Exception as e: + self.logger.error( + f"Failed to set temp channel ACL on id={channel_id} " + f"name='{channel_name}': {e}", + exc_info=True, + ) def channelRemoved(self, state, current=None): self._channel_cache = None From 84f30ca9e04645d7397f33f6085b751e1f8d18c3 Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sat, 9 May 2026 22:53:59 -0500 Subject: [PATCH 04/22] feat(mumble): scheduled cleanup of idle event channels The auto-sync feature creates and preserves channels backed by OTS groups, but never touched channels admins created manually through Mumble's GUI. Stale event channels accumulate forever until someone notices. Add a 2-day-interval scheduled job that deletes root-level Mumble channels which (a) don't match any OTS group name, (b) currently have no users, and (c) haven't seen activity in OTS_MUMBLE_CHANNEL_CLEANUP_IDLE_DAYS (default 5). OTS-group-managed channels are always preserved. Murmur's own temp channels are left alone (Murmur GCs them when empty). Activity is recorded in MumbleIceApp by the existing direction-enforcement callback on every userConnected / userStateChanged -- no new Murmur calls needed during normal operation. Channels with no recorded activity fall back to the service start time, so an event channel needs a full idle window of running service before it qualifies for deletion (no risk of nuking a legitimate channel right after a restart). Gated by OTS_MUMBLE_CHANNEL_CLEANUP_ENABLED (default True). --- opentakserver/blueprints/scheduled_jobs.py | 88 ++++++++++++++++++++++ opentakserver/defaultconfig.py | 14 ++++ opentakserver/mumble/mumble_ice_app.py | 22 ++++++ 3 files changed, 124 insertions(+) diff --git a/opentakserver/blueprints/scheduled_jobs.py b/opentakserver/blueprints/scheduled_jobs.py index e88509c7..5316880b 100644 --- a/opentakserver/blueprints/scheduled_jobs.py +++ b/opentakserver/blueprints/scheduled_jobs.py @@ -398,3 +398,91 @@ def delete_old_data(): # This function is to prevent errors caused by changing get_airplanes_live_data() to get_adsb_data def get_airplanes_live_data(): return + + +def cleanup_unmanaged_mumble_channels(): + """Delete root-level Mumble channels that aren't backed by an OTS group + and have been idle long enough to be safely removed. + + Preserves event channels admins create through Mumble's GUI until they're + actually unused, so a deliberately-kept channel for an event survives this + job until activity stops. Channels whose name matches an OTS group are + always preserved (those are managed by sync_channels_from_groups). + + Idle = currently empty AND last activity older than the configured + threshold. Activity is recorded in MumbleIceApp by the direction + enforcement callback on every userConnected / userStateChanged. After a + service restart, channels with no recorded activity fall back to the + service start time -- so a long-idle channel needs another full idle + window of running service before it qualifies for deletion. + """ + with apscheduler.app.app_context(): + if not app.config.get("OTS_MUMBLE_CHANNEL_CLEANUP_ENABLED", True): + return + + ice_app = app.extensions.get("mumble_ice_app") + if ice_app is None or not ice_app.connected: + logger.debug("Skipping Mumble channel cleanup: Ice not connected") + return + + idle_days = app.config.get("OTS_MUMBLE_CHANNEL_CLEANUP_IDLE_DAYS", 5) + threshold = datetime.datetime.now(datetime.timezone.utc) - datetime.timedelta( + days=idle_days + ) + + managed_names = { + g.name for g in db.session.query(Group).all() + if g.name and g.name != "__ANON__" + } + + try: + booted = ice_app.meta.getBootedServers() + except Exception as e: + logger.error(f"Mumble channel cleanup: getBootedServers failed: {e}") + return + + for server in booted: + try: + channels = server.getChannels() + users = server.getUsers() + except Exception as e: + logger.error(f"Mumble channel cleanup: getChannels/getUsers failed: {e}") + continue + + occupied = {u.channel for u in users.values()} + # Stamp every currently-occupied channel as freshly active so the + # idle clock genuinely measures "no one in here," even for channels + # whose users joined before this service started. + for channel_id in occupied: + ice_app.record_channel_activity(channel_id) + + for channel_id, ch in channels.items(): + if channel_id == 0: + continue + if ch.parent != 0: + continue + if ch.name in managed_names: + continue + if ch.temporary: + # Murmur GCs these on its own when empty; don't race it. + continue + if channel_id in occupied: + continue + + last_active = ice_app._channel_last_active.get( + channel_id, ice_app.service_start_time + ) + if last_active > threshold: + continue + + try: + server.removeChannel(channel_id) + ice_app._channel_last_active.pop(channel_id, None) + logger.info( + f"Deleted idle unmanaged Mumble channel '{ch.name}' " + f"(id={channel_id}, idle_since={last_active.isoformat()})" + ) + except Exception as e: + logger.error( + f"Failed to delete idle channel '{ch.name}' (id={channel_id}): {e}" + ) diff --git a/opentakserver/defaultconfig.py b/opentakserver/defaultconfig.py index 7fd05199..5c846c39 100644 --- a/opentakserver/defaultconfig.py +++ b/opentakserver/defaultconfig.py @@ -147,6 +147,13 @@ class DefaultConfig: # calls to work for non-admin users. Set False to keep the legacy # admin-only model. OTS_MUMBLE_ENABLE_CONFERENCE_CALLS = True + # Periodic cleanup of root-level Mumble channels not backed by an OTS + # group. Channels whose name matches an OTS group are always preserved. + # Anything else (event channels admins create through the Mumble GUI) is + # removed once it's been empty for IDLE_DAYS, so deliberate event channels + # survive as long as they're in use. + OTS_MUMBLE_CHANNEL_CLEANUP_ENABLED = True + OTS_MUMBLE_CHANNEL_CLEANUP_IDLE_DAYS = 5 OTS_IP_WHITELIST = ["127.0.0.1"] @@ -282,4 +289,11 @@ class DefaultConfig: "minutes": 1, "next_run_time": None, }, + { + "id": "cleanup_unmanaged_mumble_channels", + "func": "opentakserver.blueprints.scheduled_jobs:cleanup_unmanaged_mumble_channels", + "trigger": "interval", + "days": 2, + "next_run_time": None, + }, ] diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index 1dfe1b7f..ea052b12 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -1,6 +1,7 @@ import os import threading import time +from datetime import datetime, timezone from threading import Timer import Ice @@ -96,10 +97,24 @@ def __init__(self, app, logger, ice): self.adapter = None # server_id -> ServerCallbackPrx; guards against duplicate registration self.server_callbacks = {} + # channel_id -> last activity datetime (UTC). Updated by the direction + # enforcement callback on every userConnected / userStateChanged so the + # scheduled cleanup job can identify idle event channels. After a + # service restart, channels with no entry fall back to service_start_time. + self._channel_last_active = {} + self._channel_activity_lock = threading.Lock() + self.service_start_time = datetime.now(timezone.utc) # Expose this daemon to Flask blueprints so group_api can request channel # syncs after add/delete. app.extensions is a plain dict; reads are thread-safe. self.app.extensions["mumble_ice_app"] = self + def record_channel_activity(self, channel_id): + """Stamp `now` as the last-active time for a channel. Called by the + direction-enforcement callback on connect / channel-hop, and by the + cleanup job when it observes a channel that's currently occupied.""" + with self._channel_activity_lock: + self._channel_last_active[channel_id] = datetime.now(timezone.utc) + def run(self, *args): if not self.initialize_ice_connection(): self.logger.error("Mumble server connection failed") @@ -515,11 +530,17 @@ def _apply_direction(self, session, username, channel_id, group_directions, is_a # ----------------------------------------------------------- Ice callbacks + def _record_activity(self, channel_id): + ice_app = self.app.extensions.get("mumble_ice_app") + if ice_app is not None: + ice_app.record_channel_activity(channel_id) + def userConnected(self, state, current=None): self.logger.info( f"User connected: {state.name} (session={state.session}, userid={state.userid}) " f"channel={state.channel}" ) + self._record_activity(state.channel) # DB lookup runs here in the Ice dispatch thread (safe — same as authenticate()) directions, is_admin = self._get_user_directions(state.session, state.name) # Only the Ice getState/setState calls go to a background thread @@ -532,6 +553,7 @@ def userDisconnected(self, state, current=None): def userStateChanged(self, state, current=None): """Fire on any state change — channel moves trigger direction re-check.""" + self._record_activity(state.channel) directions, is_admin = self._get_user_directions(state.session, state.name) self._dispatch_apply(state.session, state.name, state.channel, directions, is_admin) From 5f0cc8da9567d7a3bc038c6dbbc5c02e2a9995a2 Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sat, 9 May 2026 22:57:06 -0500 Subject: [PATCH 05/22] docs(mumble): add operator guide for VX voice plugin support Walks through the Mumble integration end-to-end: how VX's per-device dual-socket auth resolves to a single OTS user, how the locked-down ACL model is augmented with MakeTempChannel for non-admins, how the per-temp ACL hook makes cross-group conference calls work, how the idle-channel cleanup job decides what to delete, and how to debug the most common failure modes (incl. why standard-user Move denial is intentional). --- docs/mumble-vx-calling.md | 243 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 243 insertions(+) create mode 100644 docs/mumble-vx-calling.md diff --git a/docs/mumble-vx-calling.md b/docs/mumble-vx-calling.md new file mode 100644 index 00000000..df5b6f91 --- /dev/null +++ b/docs/mumble-vx-calling.md @@ -0,0 +1,243 @@ +# Server-side support for the ATAK VX voice plugin + +This doc covers how OpenTAKServer integrates with Mumble (Murmur) to support +the [ATAK VX](https://www.civtak.org/wiki/index.php/ATAK_VX_Voice) voice +plugin, including direct calls and conference calls between users in +different OTS groups. + +## Architecture at a glance + +``` +ATAK device ──TLS──> Murmur (mumble-server) + │ + │ Ice (TCP 6502, secret-gated) + ▼ + OpenTAKServer + ├── MumbleAuthenticator (authenticates clients) + ├── MumbleIceApp (channel sync + ACL helpers) + └── DirectionEnforcement (suppress/unmute by OTS group) +``` + +Every Mumble client that connects to Murmur is authenticated by OTS via the +Murmur Ice authenticator interface, not by Murmur's local user table. OTS +returns the user's Mumble user id, display name, and the list of Mumble +groups they belong to (one per OTS group, plus `admin` for OTS administrators). + +The Ice connection is initialized by `MumbleIceDaemon` (background thread) +and handed off to `MumbleIceApp`, which: + +1. Registers the authenticator with every booted virtual server. +2. Registers a per-server `DirectionEnforcementCallback` that watches user + state changes for IN/OUT direction enforcement *and* records channel + activity for the cleanup job. +3. Auto-creates a Mumble channel for every OTS group on startup and on + demand from the group_api (see `sync_channels_from_groups`). + +## How VX uses the integration + +The official ATAK VX voice plugin opens **two parallel TLS connections** per +device — each authenticates to Murmur with the user's callsign suffixed with +a unique UUID (`---`). OTS resolves both UUIDs back to the +same OTS user via: + +1. Exact callsign match. +2. `---` suffix stripped → callsign match. +3. Underscore → space conversion (`EUD_Charlie_MXVX` → `EUD Charlie MXVX`). +4. Cert CN (the EUD UID) → EUD lookup. This survives mid-session callsign + renames where the callsign change hasn't propagated through CoT yet. + +Each socket gets its own Mumble user id (base id + deterministic +hash-of-callsign offset, modulo the per-user 1000-id range), so both sockets +co-exist without colliding even though they belong to the same OTS user. + +### Direct (1:1) calls + +When the user initiates a direct call from VX: + +1. VX creates a **temporary Mumble channel** under either Root or the + user's current channel. This requires the `MakeTempChannel` (`0x400`) + permission on the parent channel. +2. The receiving side's VX joins the temp channel by name/id (self-join via + coordination over OTS CoT messages — VX clients do not drag each other + between channels). +3. Both clients speak in the temp. When both leave, Murmur garbage-collects + the temp automatically. + +### Conference calls + +Conferences are a generalization of direct calls — the temp channel can hold +N participants, including users from different OTS groups. The ACL set on +the temp at creation time (see below) ensures any authenticated user can +enter once invited, regardless of which OTS group they belong to. + +## ACL model + +OTS deliberately operates a **locked-down per-channel ACL model**: each +auto-synced channel revokes baseline access from `@all` and grants it back +only to the channel's named group, so group channels are private to their +members. + +| Channel | Group | Grant mask | Notes | +|---|---|---|---| +| Root (id=0) | `@all` | `Traverse` | Visible to everyone | +| Root | `auth` | `Traverse \| Enter \| Speak \| Whisper \| TextMessage \| MakeTempChannel` (`0x70e`) | Lets any authenticated user open a VX temp at root level | +| Root | `admin` | All admin perms (`0x707ff`) | Full Murmur admin | +| Group channels (one per OTS group) | `all` | revoke `0x30e` | Strip baseline access from non-members | +| Group channels | `` | `0x70e` | Members get speak + MakeTempChannel for in-group temps | +| Group channels | `admin` | `0x77f` | Admins keep full per-channel control incl. temp creation | +| Temp channels (created by VX) | `@all` | `Traverse` | Discoverable by all | +| Temp channels | `auth` | `0x30e` | Any authenticated user can enter and speak — this is what makes cross-group conferences work | +| Temp channels | `admin` | `0x707ff` | Admins keep full control | + +**Move (`0x20`) is intentionally not granted to non-admin groups.** Standard +users cannot drag other users between channels — only admins can. VX direct +calls work via self-join, not by the initiator dragging the recipient. + +### How the temp ACL is applied + +`DirectionEnforcementCallback.channelCreated` fires every time a channel is +created. When `state.temporary == True`, the callback dispatches an Ice +`setACL(channel_id, [...], inherit=False)` on a background thread. The +ACL is fully self-contained (`inherit=False`) so the temp's behavior is +identical regardless of where VX places it — whether under Root or under a +locked-down group channel that wouldn't propagate any useful ACL via +inheritance. + +### How the persistent ACLs are kept correct + +`MumbleIceApp.sync_channels_from_groups` runs on startup and on every +watchdog tick (10s). It walks Root + every OTS-managed channel and calls +`_ensure_make_temp_channel_acl`, which: + +1. Reads the channel's ACL via `getACL`. +2. Filters out inherited entries (passing them back would shadow parents). +3. ORs `MakeTempChannel` (`0x400`) into the `auth` / `` / + `admin` grants if missing. +4. Writes back via `setACL`. Idempotent — runs that don't change anything + are silent. + +This pattern survives manual ACL edits through Mumble's GUI: only the +`MakeTempChannel` bit is forced, every other grant the admin set up by hand +is preserved. + +## Channel cleanup + +The auto-sync only *creates* channels; it never deletes them. To prevent +stale event channels from accumulating, `cleanup_unmanaged_mumble_channels` +runs every 2 days (configurable via the `JOBS` entry in `defaultconfig.py`) +and deletes root-level Mumble channels that: + +- Are **not** the Root channel +- Are **not** named after an existing OTS group +- Are **not** Murmur temp channels (Murmur GCs those itself) +- Have **no users** currently in them +- Have not had any user activity for `OTS_MUMBLE_CHANNEL_CLEANUP_IDLE_DAYS` + days (default 5) + +Activity is recorded in-memory by `DirectionEnforcementCallback` on every +`userConnected` and `userStateChanged`. After a service restart, channels +with no recorded activity fall back to the service start time — so an +event channel needs a full idle window of running service before it +qualifies for deletion. This avoids accidentally deleting a long-empty +channel right after a restart, at the cost of delaying cleanup if the +service restarts often. + +To create an event channel that should survive: just create it in the +Mumble GUI and use it. As long as someone joins it within the idle window +(or someone is in it at the moment the cleanup job runs), it will not be +deleted. + +## Configuration + +| Key | Default | What it does | +|---|---|---| +| `OTS_ENABLE_MUMBLE_AUTHENTICATION` | `False` | Master switch — enables the Ice authenticator and the daemon | +| `OTS_ICE_SECRET` | `""` | Must match Murmur's `icesecretread`/`icesecretwrite` in `/etc/mumble-server.ini` | +| `OTS_MUMBLE_ENABLE_CONFERENCE_CALLS` | `True` | Grants MakeTempChannel to non-admins and applies the conference ACL to new temp channels. Set `False` to revert to the legacy admin-only model | +| `OTS_MUMBLE_CHANNEL_CLEANUP_ENABLED` | `True` | Master switch for the periodic cleanup job | +| `OTS_MUMBLE_CHANNEL_CLEANUP_IDLE_DAYS` | `5` | How long a channel must be idle before deletion | + +The cleanup interval (every 2 days) lives in the `JOBS` list in +`defaultconfig.py` rather than as a separate config key, since it follows +the same pattern as every other scheduled OTS job. + +## Troubleshooting + +### Standard user gets "Permission denied" when creating a temp channel + +Confirm `OTS_MUMBLE_ENABLE_CONFERENCE_CALLS` is `True` in your config and +the OTS service has been restarted at least once after upgrading. On +startup you should see lines like: + +``` +Granting MakeTempChannel on channel_id=N name= group=: 0x30e -> 0x70e +Committed ACL update on channel_id=N name= +``` + +If those lines never appear, OTS's Ice connection to Murmur is failing — +check `OTS_ICE_SECRET` matches what's in `/etc/mumble-server.ini`. + +To inspect the live ACL on the Murmur side directly: + +```sh +sudo sqlite3 -header -column /var/lib/mumble-server/mumble-server.sqlite \ + "SELECT channel_id, priority, group_name, \ + printf('0x%x', grantpriv) AS grant_hex, \ + printf('0x%x', revokepriv) AS revoke_hex \ + FROM acl ORDER BY channel_id, priority;" +``` + +The `auth` row on Root and the `` rows on each managed channel +should have `0x70e` (or higher) in `grant_hex`. + +### User from group A can't join a temp channel created by group B + +Confirm the temp channel actually got the conference ACL applied — the OTS +log should contain a line like: + +``` +Temp channel ACL set: id=N name='' (auth=0x30e, admin=0x707ff) +``` + +If it's missing, the `channelCreated` callback may not be firing — check +that `Direction enforcement callback attached to server N` appeared on +startup, and that there are no exceptions from `_apply_temp_channel_acl`. + +### Standard user can't drag another user into a temp + +This is **expected behavior**. `Move` is admin-only by design. VX itself +does not need this — it uses self-join via OTS CoT coordination — so a +correctly-functioning VX call does not require the initiator to drag the +recipient. + +### Stale event channels accumulating + +The cleanup job runs every 2 days. If you've just deployed and want to +clear stale channels immediately, you can either: + +1. Wait for the next scheduled run (visible in the OTS scheduler API). +2. Trigger it manually via the scheduler API: + ``` + POST /api/scheduler/jobs/cleanup_unmanaged_mumble_channels/run + ``` + (auth as an OTS administrator) + +If a channel is *not* getting deleted that you think should be, check: +- Is its name in the OTS Group table? Channels matching an OTS group are + always preserved. +- Is anyone currently in the channel? +- Was the channel just created? After a service restart it needs a full + `OTS_MUMBLE_CHANNEL_CLEANUP_IDLE_DAYS` window of running service before + it's eligible. + +## Implementation references + +| Concern | File / function | +|---|---| +| Authenticator (resolves callsign-with-UUID, cert CN, etc.) | `opentakserver/mumble/mumble_authenticator.py::MumbleAuthenticator.resolve_identity` | +| Per-temp ACL hook | `opentakserver/mumble/mumble_ice_app.py::DirectionEnforcementCallback.channelCreated` | +| MakeTempChannel ACL bump on persistent channels | `opentakserver/mumble/mumble_ice_app.py::MumbleIceApp._ensure_make_temp_channel_acl` | +| Direction enforcement (IN/OUT suppress flag) | `opentakserver/mumble/mumble_ice_app.py::DirectionEnforcementCallback._apply_direction` | +| Channel auto-sync from OTS groups | `opentakserver/mumble/mumble_ice_app.py::MumbleIceApp.sync_channels_from_groups` | +| Idle channel cleanup job | `opentakserver/blueprints/scheduled_jobs.py::cleanup_unmanaged_mumble_channels` | +| Murmur permission constants | `opentakserver/mumble/Murmur.ice` (search `Permission*`) | From 3d8104ad23eabdb616d3060fc7a149f7491e358a Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sat, 9 May 2026 23:02:04 -0500 Subject: [PATCH 06/22] chore: gitignore .claude/ for per-project Claude Code settings --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index e2e599f4..df0d930b 100644 --- a/.gitignore +++ b/.gitignore @@ -168,3 +168,5 @@ instance/* .webassets-cache .env +# Claude Code per-project settings (local overrides, transcripts, etc.) +.claude/ From 979ed6017859ace072449cfb3c9c691ffe7f65a1 Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sun, 10 May 2026 20:22:38 -0500 Subject: [PATCH 07/22] fix(mumble): preserve Murmur creator-admin on temps + elevate OTS admin perms Three related changes after observing VX call lifecycle on the live server: 1) _apply_temp_channel_acl no longer wipes Murmur's defaults on a new temp. Murmur auto-creates a local `admin` group with the creator's userid added -- that's how the creator's VX plugin keeps admin rights on the temp it just made (set call ACL, move users at end-call, garbage-collect the temp). Previously we called setACL(..., groups=[], inherit=False) which destroyed that. Now we read the existing ACL/groups, preserve all non-inherited entries, ADD an explicit @auth=Enter|Speak|Whisper|TextMessage if missing, and setACL with inherit=True. Result: cross-group conferences still work (any auth user can join a temp regardless of OTS group) AND VX's creator-side call management continues to function the way it expects. 2) _ensure_make_temp_channel_acl now bumps the `admin` grant on every OTS-managed channel from 0x77f to 0x707ff with apply_sub=True -- the same full grant Murmur uses on Root. This is what makes "OTS administrator" mean "Mumble server administrator": admins get Move, Kick, Ban, Register, MakeTempChannel, Write, etc. on every OTS channel, and the grant propagates into any sub/temp channel. 3) DirectionEnforcementCallback._apply_direction now actively clears the suppress flag when a user enters a non-OTS channel (temp call channel or admin-created event channel). Previously it returned early, leaving the user server-muted from their previous group if they had been direction=OUT -- which would silently break VX calls for any listen-only user. --- opentakserver/mumble/mumble_ice_app.py | 120 +++++++++++++++++++------ 1 file changed, 91 insertions(+), 29 deletions(-) diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index ea052b12..c7eb9015 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -310,13 +310,31 @@ def _ensure_make_temp_channel_acl(self, server, channel_id, channel_name): own_acls = [a for a in acls if not a.inherited] dirty = False for acl in own_acls: - if acl.group in targets and not (acl.allow & PERM_MAKE_TEMP_CHANNEL): - before = acl.allow - acl.allow |= PERM_MAKE_TEMP_CHANNEL + if acl.group not in targets: + continue + + new_allow = acl.allow | PERM_MAKE_TEMP_CHANNEL + new_apply_subs = acl.applySubs + + # OTS administrators get the same full grant Murmur uses on Root + # (Write, Move, Kick, Ban, Register, MakeTempChannel, etc.) with + # apply_sub=1 so the grant propagates into any sub/temp channel + # created beneath an OTS-managed channel. This is what makes + # "OTS admin" mean "Mumble server administrator" — they can move + # users, kick, ban, manage ACLs, etc. on every OTS channel. + if acl.group == "admin": + new_allow |= PERM_ADMIN_FULL + new_apply_subs = True + + if new_allow != acl.allow or new_apply_subs != acl.applySubs: + before_allow = acl.allow + before_sub = acl.applySubs + acl.allow = new_allow + acl.applySubs = new_apply_subs self.logger.info( - f"Granting MakeTempChannel on channel_id={channel_id} " - f"name={channel_name} group={acl.group}: " - f"0x{before:x} -> 0x{acl.allow:x}" + f"Updating ACL on channel_id={channel_id} name={channel_name} " + f"group={acl.group}: allow 0x{before_allow:x} -> 0x{acl.allow:x}, " + f"apply_sub {before_sub} -> {acl.applySubs}" ) dirty = True @@ -500,6 +518,23 @@ def _apply_direction(self, session, username, channel_id, group_directions, is_a direction = group_directions.get(channel_name) if direction is None: + # Non-OTS channel (temp channel for VX private call, or an + # admin-created event channel). Direction enforcement is an + # OTS group concept; outside an OTS group, a user who was + # suppressed in their previous channel must be un-suppressed + # so they can actually speak in the call they just joined. + try: + s = self.server.getState(session) + if s.suppress: + s.suppress = False + self.server.setState(s) + self.logger.info( + f"SPEAK ENABLED (non-OTS channel): {username} in {channel_name}" + ) + except Exception as e: + self.logger.error( + f"Failed to clear suppress for '{username}' in {channel_name}: {e}" + ) return s = self.server.getState(session) @@ -576,34 +611,61 @@ def channelCreated(self, state, current=None): ).start() def _apply_temp_channel_acl(self, channel_id, channel_name): - """Lock down a freshly-created temp channel with a conference-capable ACL. - - Without this, a temp channel under a group channel inherits nothing - useful (the group channel's apply_sub=0 entries don't propagate), so - invited users from a different OTS group can't enter — breaking VX - cross-group conference calls. Setting an explicit ACL with inherit=False - guarantees the same behavior regardless of where VX places the temp: - any authenticated user can enter/speak; admins keep full control. + """Augment a freshly-created temp channel with a conference-capable ACL. + + Murmur's default temp setup already grants the creator admin perms on + their own temp (via an auto-created local `admin` group with the + creator's userid added) plus inherits @all/Traverse and admin/full from + Root. That's what lets the VX plugin manage its temp post-creation + (set call ACL, move users at end-call, delete the temp). But the + default doesn't grant non-creator @auth users Enter/Speak — so a + callee from a different OTS group can't join. + + This function ADDS an explicit @auth=Enter|Speak|Whisper|TextMessage + entry while preserving Murmur's defaults (inherit=True, all local + groups kept). Result: cross-group conferences work AND creator + retains admin on their own temp for VX's call lifecycle. """ try: - acl_all = Murmur.ACL( - applyHere=True, applySubs=False, inherited=False, - userid=-1, group="all", allow=PERM_TRAVERSE, deny=0, - ) - acl_auth = Murmur.ACL( - applyHere=True, applySubs=False, inherited=False, - userid=-1, group="auth", allow=PERM_BASELINE_SPEAK, deny=0, - ) - acl_admin = Murmur.ACL( - applyHere=True, applySubs=False, inherited=False, - userid=-1, group="admin", allow=PERM_ADMIN_FULL, deny=0, + # Read what Murmur set up: implicit creator-admin group + inherited + # ACLs. We preserve everything and just layer @auth Enter/Speak on top. + try: + pre_acls, pre_groups, _pre_inherit = self.server.getACL(channel_id) + except Exception as e: + self.logger.error( + f"getACL on new temp id={channel_id} failed, skipping ACL set: {e}" + ) + return + + # Keep only locally-defined groups (notably the creator-admin entry + # Murmur auto-creates). Inherited groups are recreated by Murmur + # from the parent chain — passing them back would shadow inheritance. + local_groups = [g for g in pre_groups if not g.inherited] + + # Keep existing non-inherited ACLs (in case Murmur or VX set + # something specific), then add our @auth Enter/Speak grant. + own_acls = [a for a in pre_acls if not a.inherited] + has_auth_speak = any( + a.group == "auth" and (a.allow & PERM_BASELINE_SPEAK) == PERM_BASELINE_SPEAK + for a in own_acls ) - self.server.setACL( - channel_id, [acl_all, acl_auth, acl_admin], [], False + if not has_auth_speak: + own_acls.append(Murmur.ACL( + applyHere=True, applySubs=False, inherited=False, + userid=-1, group="auth", allow=PERM_BASELINE_SPEAK, deny=0, + )) + + # inherit=True preserves @all/Traverse + admin/full from Root, plus + # the creator-admin group membership chain. + self.server.setACL(channel_id, own_acls, local_groups, True) + + preserved_creator = next( + (g for g in local_groups if g.name == "admin" and g.add), None ) + creator_uids = list(preserved_creator.add) if preserved_creator else [] self.logger.info( - f"Temp channel ACL set: id={channel_id} name='{channel_name}' " - f"(auth=0x{PERM_BASELINE_SPEAK:x}, admin=0x{PERM_ADMIN_FULL:x})" + f"Temp channel ACL augmented: id={channel_id} name='{channel_name}' " + f"added auth=0x{PERM_BASELINE_SPEAK:x}, preserved creator-admin uids={creator_uids}" ) except Exception as e: self.logger.error( From 3de9d52da406a09148e647483d2511563a0d5343 Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sun, 10 May 2026 20:34:03 -0500 Subject: [PATCH 08/22] feat(mumble): log post-VX-setACL state on temp channels VX's client-side setACL fires ~160ms after channel creation, after our OTS-side setACL completes. Capture the final ACL state ~1.5s in to see exactly what VX configures on its call channel -- the schema isn't documented and this informs interop work. Gated by OTS_MUMBLE_LOG_POST_VX_ACL (default True). Off-thread so it doesn't block the channelCreated callback. Tolerates InvalidChannel in case the temp was already GC'd before the snapshot fires. --- opentakserver/defaultconfig.py | 4 +++ opentakserver/mumble/mumble_ice_app.py | 45 ++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/opentakserver/defaultconfig.py b/opentakserver/defaultconfig.py index 5c846c39..79250a4e 100644 --- a/opentakserver/defaultconfig.py +++ b/opentakserver/defaultconfig.py @@ -154,6 +154,10 @@ class DefaultConfig: # survive as long as they're in use. OTS_MUMBLE_CHANNEL_CLEANUP_ENABLED = True OTS_MUMBLE_CHANNEL_CLEANUP_IDLE_DAYS = 5 + # Snapshot temp-channel ACLs ~1.5s after creation to capture what the + # VX plugin configures client-side. Useful for interop work; safe to + # leave on, but turn off if you don't need the noise in the OTS log. + OTS_MUMBLE_LOG_POST_VX_ACL = True OTS_IP_WHITELIST = ["127.0.0.1"] diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index c7eb9015..da67a8d4 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -667,6 +667,18 @@ def _apply_temp_channel_acl(self, channel_id, channel_name): f"Temp channel ACL augmented: id={channel_id} name='{channel_name}' " f"added auth=0x{PERM_BASELINE_SPEAK:x}, preserved creator-admin uids={creator_uids}" ) + + # Diagnostic: VX's client-side setACL fires ~160ms after channel + # creation -- after ours. Capture the final ACL state to see what + # VX actually configures on its call channel (informs interop work + # since VX's intent isn't documented anywhere). Disable via + # OTS_MUMBLE_LOG_POST_VX_ACL=False once interop is dialed in. + if self.app.config.get("OTS_MUMBLE_LOG_POST_VX_ACL", True): + threading.Thread( + target=self._capture_post_vx_acl, + args=(channel_id, channel_name), + daemon=True, + ).start() except Exception as e: self.logger.error( f"Failed to set temp channel ACL on id={channel_id} " @@ -674,6 +686,39 @@ def _apply_temp_channel_acl(self, channel_id, channel_name): exc_info=True, ) + def _capture_post_vx_acl(self, channel_id, channel_name): + """Snapshot the ACL ~1.5s after channel creation to capture whatever + VX configured client-side after our setACL completed. Tolerant of the + channel being gone (caller bailed out before VX's setACL landed).""" + time.sleep(1.5) + try: + acls, groups, inherit = self.server.getACL(channel_id) + except Murmur.InvalidChannelException: + self.logger.debug( + f"POST-VX ACL capture: channel id={channel_id} gone before snapshot" + ) + return + except Exception as e: + self.logger.warning( + f"POST-VX ACL capture failed for id={channel_id} name='{channel_name}': {e}" + ) + return + + acl_dump = ", ".join( + f"{a.group or f'uid:{a.userid}'}/allow=0x{a.allow:x}/deny=0x{a.deny:x}" + f"/here={int(a.applyHere)}/sub={int(a.applySubs)}/inh={int(a.inherited)}" + for a in acls + ) + grp_dump = ", ".join( + f"{g.name}/inh={int(g.inherited)}/inheritable={int(g.inheritable)}" + f"/add={list(g.add)}/remove={list(g.remove)}/members={list(g.members)}" + for g in groups + ) + self.logger.info( + f"POST-VX ACL temp id={channel_id} name='{channel_name}' " + f"inherit={inherit} acls=[{acl_dump}] groups=[{grp_dump}]" + ) + def channelRemoved(self, state, current=None): self._channel_cache = None From 1df8f60c2b684fc47b201d8c3282c0b82626766b Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sun, 10 May 2026 20:45:14 -0500 Subject: [PATCH 09/22] =?UTF-8?q?perf(mumble):=20in-memory=20suppress=20tr?= =?UTF-8?q?acking=20=E2=80=94=20no=20Ice=20traffic=20for=20non-OUT=20users?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously _apply_direction made a getState call on every userConnected / userStateChanged event, regardless of whether the user actually had any OUT memberships. In an environment where everyone has direction=IN for their primary group (the common case), that's one Ice round-trip per channel move generating zero useful work. During VX call lifecycle (4+ channel transitions per call, multiple users), the resulting Ice burst plus the legacy 30s invocation timeout caused minute-long cascades of timeout exceptions when Murmur briefly stalled on setACL broadcasts -- the user-visible symptom being VX calls hanging with raw "%1$d" format-string errors as the Mumble client's localization fell back on missing parameters. Track suppressed sessions in a thread-safe set. Only round-trip to Murmur when the desired suppress bit actually needs to flip. For environments with no OUT memberships, direction enforcement now makes zero Ice calls in steady state. Also downgrade InvalidSessionException (session disconnected between dispatch and execution) to DEBUG -- it's a normal race, not an error. --- opentakserver/mumble/mumble_ice_app.py | 108 +++++++++++++++---------- 1 file changed, 65 insertions(+), 43 deletions(-) diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index da67a8d4..ff341814 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -416,6 +416,13 @@ def __init__(self, app, logger, server): self._channel_cache_time = 0 self._session_lock = threading.Lock() self._session_cache = {} # session_id -> {directions, is_admin, cached_at} + # Sessions we've actively set suppress=True for. Used so steady-state + # channel moves make zero Ice calls -- we only round-trip to Murmur + # when a user actually crosses an IN<->OUT boundary. For environments + # with no OUT memberships this set stays empty forever and direction + # enforcement is effectively free. + self._suppressed_sessions = set() + self._suppress_lock = threading.Lock() # ------------------------------------------------------------------ helpers @@ -494,56 +501,63 @@ def _dispatch_apply(self, session, username, channel_id, group_directions, is_ad ).start() def _apply_direction(self, session, username, channel_id, group_directions, is_admin): - """Background thread: apply suppress flag via Ice calls only.""" + """Apply the suppress flag based on the user's OTS direction. + + Uses in-memory tracking (_suppressed_sessions) so the common case -- + a user moving between channels where direction is IN or undefined -- + makes zero Ice calls. Only round-trips to Murmur when the desired + suppress bit actually has to flip. + """ try: - # Admins are never suppressed by this callback, so skip all Ice calls. - # Calling getState() here blocks for 30s and crashes the connection. if is_admin: return channel_map = self._get_channel_map() channel_name = channel_map.get(channel_id, f"unknown({channel_id})") - # Root channel: always allow speaking - if channel_name == 'Root': - try: - s = self.server.getState(session) - if s.suppress: - s.suppress = False - self.server.setState(s) - self.logger.info(f"UNMUTED (Root): {username}") - except Exception as e: - self.logger.error(f"Failed to clear suppress for '{username}' in Root: {e}") + # Root and non-OTS channels (VX temps, event channels) are not + # subject to direction enforcement -- treat as if direction=IN. + direction = group_directions.get(channel_name) + should_suppress = (direction == 'OUT') + + with self._suppress_lock: + currently_suppressed = session in self._suppressed_sessions + + if should_suppress == currently_suppressed: + return # No state change needed; no Ice call. + + try: + s = self.server.getState(session) + except Murmur.InvalidSessionException: + # Session disconnected between dispatch and execution -- normal. + self.logger.debug( + f"Direction: session {session} ({username}) gone before getState" + ) + with self._suppress_lock: + self._suppressed_sessions.discard(session) return - direction = group_directions.get(channel_name) - if direction is None: - # Non-OTS channel (temp channel for VX private call, or an - # admin-created event channel). Direction enforcement is an - # OTS group concept; outside an OTS group, a user who was - # suppressed in their previous channel must be un-suppressed - # so they can actually speak in the call they just joined. - try: - s = self.server.getState(session) - if s.suppress: - s.suppress = False - self.server.setState(s) - self.logger.info( - f"SPEAK ENABLED (non-OTS channel): {username} in {channel_name}" - ) - except Exception as e: - self.logger.error( - f"Failed to clear suppress for '{username}' in {channel_name}: {e}" - ) + s.suppress = should_suppress + try: + self.server.setState(s) + except Murmur.InvalidSessionException: + self.logger.debug( + f"Direction: session {session} ({username}) gone before setState" + ) + with self._suppress_lock: + self._suppressed_sessions.discard(session) return - s = self.server.getState(session) - should_suppress = (direction == 'OUT') + with self._suppress_lock: + if should_suppress: + self._suppressed_sessions.add(session) + else: + self._suppressed_sessions.discard(session) - if should_suppress and not s.suppress: - s.suppress = True - self.server.setState(s) - self.logger.info(f"LISTEN ONLY: {username} in {channel_name} (direction=OUT)") + if should_suppress: + self.logger.info( + f"LISTEN ONLY: {username} in {channel_name} (direction=OUT)" + ) try: self.server.sendMessage( session, @@ -551,12 +565,18 @@ def _apply_direction(self, session, username, channel_id, group_directions, is_a ) except Exception: pass + else: + self.logger.info( + f"SPEAK ENABLED: {username} in {channel_name} " + f"(direction={direction or 'non-OTS'})" + ) - elif not should_suppress and s.suppress: - s.suppress = False - self.server.setState(s) - self.logger.info(f"SPEAK ENABLED: {username} in {channel_name} (direction=IN)") - + except Murmur.InvalidSessionException: + self.logger.debug( + f"Direction: session {session} ({username}) gone during apply" + ) + with self._suppress_lock: + self._suppressed_sessions.discard(session) except Exception as e: self.logger.error( f"Unhandled error applying direction for '{username}' session={session}: {e}", @@ -584,6 +604,8 @@ def userConnected(self, state, current=None): def userDisconnected(self, state, current=None): with self._session_lock: self._session_cache.pop(state.session, None) + with self._suppress_lock: + self._suppressed_sessions.discard(state.session) self.logger.info(f"User disconnected: {state.name} (session={state.session})") def userStateChanged(self, state, current=None): From 747d1088fe444bcc7921c62ddeb07b20b71acfec Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sun, 10 May 2026 20:53:30 -0500 Subject: [PATCH 10/22] perf(mumble): drop per-temp ACL hooks; push @auth grant to parent via inheritance The per-temp ACL strategy was too aggressive: every VX temp creation triggered getACL + setACL on a background thread, plus a 1.5s-delayed getACL diagnostic. Under VX's typical call lifecycle (multiple temps, multiple users moving channels) that overlapped with Murmur's own state broadcasts, hitting lock contention and producing 30s+ Ice timeouts that surfaced as minute-long End-Call hangs and "%1$d" client errors. New strategy: ensure parent channels (Root + each OTS group channel) carry an @auth grant of Enter|Speak|Whisper|TextMessage with apply_sub=True. Temps created beneath those channels inherit the grant, so cross-group conferences work without any per-temp intervention. Murmur's auto creator-admin group continues to handle the caller's post-creation ACL management exactly as VX expects. Changes: - _ensure_make_temp_channel_acl now ALSO bumps @auth apply_sub=True and adds an @auth sub-only grant on group channels that lack one - _apply_temp_channel_acl and _capture_post_vx_acl deleted entirely - channelCreated reduced to a one-line cache invalidation - OTS_MUMBLE_LOG_POST_VX_ACL config flag removed (no longer used) Net effect: zero Ice calls during VX call lifecycle. Persistent ACL setup happens once at startup (and per group_api request_sync). --- opentakserver/defaultconfig.py | 4 - opentakserver/mumble/mumble_ice_app.py | 182 +++++++------------------ 2 files changed, 50 insertions(+), 136 deletions(-) diff --git a/opentakserver/defaultconfig.py b/opentakserver/defaultconfig.py index 79250a4e..5c846c39 100644 --- a/opentakserver/defaultconfig.py +++ b/opentakserver/defaultconfig.py @@ -154,10 +154,6 @@ class DefaultConfig: # survive as long as they're in use. OTS_MUMBLE_CHANNEL_CLEANUP_ENABLED = True OTS_MUMBLE_CHANNEL_CLEANUP_IDLE_DAYS = 5 - # Snapshot temp-channel ACLs ~1.5s after creation to capture what the - # VX plugin configures client-side. Useful for interop work; safe to - # leave on, but turn off if you don't need the noise in the OTS log. - OTS_MUMBLE_LOG_POST_VX_ACL = True OTS_IP_WHITELIST = ["127.0.0.1"] diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index ff341814..9ee69227 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -288,12 +288,28 @@ def _ensure_temp_channel_acls(self, server, managed_names): self._ensure_make_temp_channel_acl(server, channel_id, ch.name) def _ensure_make_temp_channel_acl(self, server, channel_id, channel_name): - """Idempotently OR MakeTempChannel into auth//admin grants. + """Idempotently set the ACL needed for VX direct calls on a persistent + channel. Three guarantees: + + 1. The channel's own group/auth grant has MakeTempChannel so users + can create temp channels under it (the call lifecycle). + 2. The `admin` grant has the full PERM_ADMIN_FULL mask with + apply_sub=True so OTS administrators can administer the server + (move, kick, ban, manage ACLs) on every OTS channel AND on any + subchannel/temp created beneath it. + 3. An `auth` grant exists with apply_sub=True so temp channels + created under this channel inherit Enter/Speak/Whisper/Text for + any authenticated user. On Root that grant also applies here + (users can speak in Root itself). On group channels apply_here + stays False so the channel itself remains members-only. + + Guarantee #3 is what makes per-temp ACL setACL hooks unnecessary -- + the @auth grant flows down via inheritance so we don't have to + intervene every time VX creates a temp. Only modifies non-inherited entries; inherited rows are filtered out - before setACL (passing them back would shadow the parent's ACL and - break propagation). Skips any existing entry that already has the - bit set, so this is safe to re-run on every watchdog tick. + before setACL (passing them back would shadow the parent's ACL). + Idempotent — safe to re-run. """ try: acls, groups, inherit = server.getACL(channel_id) @@ -303,11 +319,13 @@ def _ensure_make_temp_channel_acl(self, server, channel_id, channel_name): ) return + is_root = (channel_id == 0) + own_acls = [a for a in acls if not a.inherited] + targets = {"auth", "admin"} if channel_name and channel_name not in ("Root", "__ANON__"): targets.add(channel_name) - own_acls = [a for a in acls if not a.inherited] dirty = False for acl in own_acls: if acl.group not in targets: @@ -316,16 +334,15 @@ def _ensure_make_temp_channel_acl(self, server, channel_id, channel_name): new_allow = acl.allow | PERM_MAKE_TEMP_CHANNEL new_apply_subs = acl.applySubs - # OTS administrators get the same full grant Murmur uses on Root - # (Write, Move, Kick, Ban, Register, MakeTempChannel, etc.) with - # apply_sub=1 so the grant propagates into any sub/temp channel - # created beneath an OTS-managed channel. This is what makes - # "OTS admin" mean "Mumble server administrator" — they can move - # users, kick, ban, manage ACLs, etc. on every OTS channel. if acl.group == "admin": new_allow |= PERM_ADMIN_FULL new_apply_subs = True + if acl.group == "auth": + # @auth must propagate to subchannels so VX temps inherit + # Enter+Speak+Whisper+TextMessage without per-temp intervention. + new_apply_subs = True + if new_allow != acl.allow or new_apply_subs != acl.applySubs: before_allow = acl.allow before_sub = acl.applySubs @@ -338,6 +355,23 @@ def _ensure_make_temp_channel_acl(self, server, channel_id, channel_name): ) dirty = True + # Group channels (REACT, Family, etc.) typically have no own @auth ACL -- + # access is gated entirely by the per-channel group grant. Add an + # apply_here=False, apply_sub=True @auth grant so temps under this + # channel inherit Enter+Speak for any authenticated user. The channel + # itself stays members-only (apply_here=False), preserving the existing + # security model. + if not is_root and not any(a.group == "auth" for a in own_acls): + own_acls.append(Murmur.ACL( + applyHere=False, applySubs=True, inherited=False, + userid=-1, group="auth", allow=PERM_BASELINE_SPEAK, deny=0, + )) + self.logger.info( + f"Adding @auth sub-grant on channel_id={channel_id} " + f"name={channel_name}: allow=0x{PERM_BASELINE_SPEAK:x}, apply_sub=True" + ) + dirty = True + if not dirty: return @@ -618,128 +652,12 @@ def userTextMessage(self, state, message, current=None): pass def channelCreated(self, state, current=None): + # Per-temp ACL intervention is no longer needed: the @auth grant on + # parent channels (set by _ensure_make_temp_channel_acl) propagates + # to temps via inheritance, and Murmur auto-creates a creator-admin + # group on each temp for VX's call lifecycle management. We only + # need to invalidate the channel cache here. self._channel_cache = None - if not state.temporary: - return - if not self.app.config.get("OTS_MUMBLE_ENABLE_CONFERENCE_CALLS", True): - return - # Dispatch the ACL set off the Ice thread — setACL is a synchronous - # Ice call and would otherwise block the dispatcher. Same pattern as - # _dispatch_apply for direction enforcement. - threading.Thread( - target=self._apply_temp_channel_acl, - args=(state.id, state.name), - daemon=True, - ).start() - - def _apply_temp_channel_acl(self, channel_id, channel_name): - """Augment a freshly-created temp channel with a conference-capable ACL. - - Murmur's default temp setup already grants the creator admin perms on - their own temp (via an auto-created local `admin` group with the - creator's userid added) plus inherits @all/Traverse and admin/full from - Root. That's what lets the VX plugin manage its temp post-creation - (set call ACL, move users at end-call, delete the temp). But the - default doesn't grant non-creator @auth users Enter/Speak — so a - callee from a different OTS group can't join. - - This function ADDS an explicit @auth=Enter|Speak|Whisper|TextMessage - entry while preserving Murmur's defaults (inherit=True, all local - groups kept). Result: cross-group conferences work AND creator - retains admin on their own temp for VX's call lifecycle. - """ - try: - # Read what Murmur set up: implicit creator-admin group + inherited - # ACLs. We preserve everything and just layer @auth Enter/Speak on top. - try: - pre_acls, pre_groups, _pre_inherit = self.server.getACL(channel_id) - except Exception as e: - self.logger.error( - f"getACL on new temp id={channel_id} failed, skipping ACL set: {e}" - ) - return - - # Keep only locally-defined groups (notably the creator-admin entry - # Murmur auto-creates). Inherited groups are recreated by Murmur - # from the parent chain — passing them back would shadow inheritance. - local_groups = [g for g in pre_groups if not g.inherited] - - # Keep existing non-inherited ACLs (in case Murmur or VX set - # something specific), then add our @auth Enter/Speak grant. - own_acls = [a for a in pre_acls if not a.inherited] - has_auth_speak = any( - a.group == "auth" and (a.allow & PERM_BASELINE_SPEAK) == PERM_BASELINE_SPEAK - for a in own_acls - ) - if not has_auth_speak: - own_acls.append(Murmur.ACL( - applyHere=True, applySubs=False, inherited=False, - userid=-1, group="auth", allow=PERM_BASELINE_SPEAK, deny=0, - )) - - # inherit=True preserves @all/Traverse + admin/full from Root, plus - # the creator-admin group membership chain. - self.server.setACL(channel_id, own_acls, local_groups, True) - - preserved_creator = next( - (g for g in local_groups if g.name == "admin" and g.add), None - ) - creator_uids = list(preserved_creator.add) if preserved_creator else [] - self.logger.info( - f"Temp channel ACL augmented: id={channel_id} name='{channel_name}' " - f"added auth=0x{PERM_BASELINE_SPEAK:x}, preserved creator-admin uids={creator_uids}" - ) - - # Diagnostic: VX's client-side setACL fires ~160ms after channel - # creation -- after ours. Capture the final ACL state to see what - # VX actually configures on its call channel (informs interop work - # since VX's intent isn't documented anywhere). Disable via - # OTS_MUMBLE_LOG_POST_VX_ACL=False once interop is dialed in. - if self.app.config.get("OTS_MUMBLE_LOG_POST_VX_ACL", True): - threading.Thread( - target=self._capture_post_vx_acl, - args=(channel_id, channel_name), - daemon=True, - ).start() - except Exception as e: - self.logger.error( - f"Failed to set temp channel ACL on id={channel_id} " - f"name='{channel_name}': {e}", - exc_info=True, - ) - - def _capture_post_vx_acl(self, channel_id, channel_name): - """Snapshot the ACL ~1.5s after channel creation to capture whatever - VX configured client-side after our setACL completed. Tolerant of the - channel being gone (caller bailed out before VX's setACL landed).""" - time.sleep(1.5) - try: - acls, groups, inherit = self.server.getACL(channel_id) - except Murmur.InvalidChannelException: - self.logger.debug( - f"POST-VX ACL capture: channel id={channel_id} gone before snapshot" - ) - return - except Exception as e: - self.logger.warning( - f"POST-VX ACL capture failed for id={channel_id} name='{channel_name}': {e}" - ) - return - - acl_dump = ", ".join( - f"{a.group or f'uid:{a.userid}'}/allow=0x{a.allow:x}/deny=0x{a.deny:x}" - f"/here={int(a.applyHere)}/sub={int(a.applySubs)}/inh={int(a.inherited)}" - for a in acls - ) - grp_dump = ", ".join( - f"{g.name}/inh={int(g.inherited)}/inheritable={int(g.inheritable)}" - f"/add={list(g.add)}/remove={list(g.remove)}/members={list(g.members)}" - for g in groups - ) - self.logger.info( - f"POST-VX ACL temp id={channel_id} name='{channel_name}' " - f"inherit={inherit} acls=[{acl_dump}] groups=[{grp_dump}]" - ) def channelRemoved(self, state, current=None): self._channel_cache = None From 896c86475f0f8eebbe61a6e677080eefe6a684b5 Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sun, 10 May 2026 21:06:36 -0500 Subject: [PATCH 11/22] docs(mumble): reverse-engineered VX direct-call protocol Self-contained writeup of what we learned debugging VX calls on the live server, intended to be dropped into a fresh context (e.g. for interop work on a separate ATAK plugin). Covers: the three-layer architecture (CoT-embedded XV signaling + Mumble channel handshake + VoiceTarget audio), the <__xv> extension schema, Murmur's auto creator-admin group on temps, the right server-side ACL strategy (inheritance, not per-temp setACL), the exact permission matrix per user role, version-specific quirks (VX 1.0.0 reconnect cycle, stuck-in-Root pattern), and an investigation checklist for things still unknown. --- docs/vx-direct-call-protocol.md | 228 ++++++++++++++++++++++++++++++++ 1 file changed, 228 insertions(+) create mode 100644 docs/vx-direct-call-protocol.md diff --git a/docs/vx-direct-call-protocol.md b/docs/vx-direct-call-protocol.md new file mode 100644 index 00000000..25fb34e2 --- /dev/null +++ b/docs/vx-direct-call-protocol.md @@ -0,0 +1,228 @@ +# ATAK VX direct-call protocol — what we know + +Reverse-engineered from live observation of the official VX voice plugin +(plain VX **v2.1.0**) against an OpenTAKServer + Murmur 1.3.4 deployment. +In murmur's `slog` and TLS logs these clients identify as +`1.0.0 (ATAK: ATAK_Vx)` — that's the Mumble protocol compatibility version +the client advertises, NOT the VX plugin's own release version. +Not from VX source — treat as "best-current-understanding" not gospel. + +## Architecture: signaling + media in two separate layers + +VX direct calls look superficially like Mumble channels but are really +two independent layers stacked together, similar to SIP signaling + +RTP media: + +``` +Layer 1 — CHANNEL HANDSHAKE (Mumble protocol) + Caller creates a temporary channel "TAK PRIVATE - " + Caller calls setACL on it (to lock down the call) + Recipient moves into the temp as a visual handshake + Visible: server-side via Murmur slog / Ice events + +Layer 2 — AUDIO (Mumble VoiceTarget / whisper) + Speaker configures a numbered VoiceTarget pointing at the recipient + Voice packets transmit with that target id — server routes only to that user + NOT visible server-side (UDP, no slog entries for voice routing) +``` + +**Critical implication:** the audio doesn't flow through the temp channel. +In actual call captures we observed the caller stay in their original group +channel while the recipient moved into the temp, and the call still worked. +The temp is a visual + handshake indicator; the actual voice is whisper. + +## Layer 1: Channel handshake (Mumble protocol) + +Observed sequence when a call initiates (from `mumble-server.log`): + +``` +T+0.000s CALLER Added channel "TAK PRIVATE - " under Root +T+0.160s CALLER Updated ACL in channel TAK PRIVATE - +T+2-7s CALLEE Moved CALLEE to TAK PRIVATE - +... audio flows via whisper (see Layer 2) ... +T+N s CALLER Moved CALLER to Root (end-call intermediate) +T+N+50ms CALLER Moved CALLER to original_channel (REACT/Family/etc.) +T+N s CALLEE Moved CALLEE to original_channel +T+N s Murmur auto-GCs the empty temp channel +``` + +### Naming convention + +Two formats observed: `TAK PRIVATE - 9a265fc4` (8 hex chars) and +`TAK PRIVATE - 452b73ccc87f` (12 hex chars). Likely a session UUID truncated +to 8 vs 12 chars; possibly distinguishing 1:1 vs conference. **Other VX +clients recognize this prefix as a call channel** and apply special UI +behavior (don't show in normal channel list). + +### Murmur auto-creates a creator-admin group + +This is the most important detail for OTS-side ACL design. When **any** user +creates a temp via the Mumble protocol, Murmur **automatically creates a +local `admin` group on that temp** and adds the creator's userid to it: + +``` +groups=[..., admin/inh=0/inheritable=1/add=[]/members=[]] +``` + +Combined with Root's inherited `admin/allow=0x707ff` ACL, this gives the +creator full admin powers ON THEIR OWN TEMP, even if they aren't a real +server admin. **That's how non-admin standard users successfully `setACL` +on their own call channel** at T+0.160s. The `Updated ACL` line in the +murmur log is VX configuring its own call's permissions (likely limiting +who can enter), which requires `Write` on the temp — granted via the +creator-admin chain. + +**Do not destroy this group.** Specifically: don't call `setACL` on the +temp with `groups=[], inherit=False` — that wipes the creator-admin group +and breaks VX's call lifecycle. Instead, **don't intervene per-temp at +all** and instead put an `@auth` grant on parent channels with +`apply_sub=True` so it inherits down to temps (see "OTS server-side +strategy" below). + +### What ACL does VX set on its own temp? + +Unknown — VX issues `setACL` ~160ms after creation but it's a client-side +Mumble protocol message; the server log only shows "Updated ACL in channel" +with no contents. We never captured the actual ACL VX configures because +our diagnostic `getACL` calls were timing out under load (Ice 30s). + +**To recover this:** run `tcpdump` on port 64738 during a call and decode +the Mumble protocol `ACL` message in the capture. Or hook a `ServerCallback` +implementation that does `getACL` immediately after `channelStateChanged` +on temp channels. + +## Layer 2: Audio path via VoiceTarget (whisper) + +Murmur supports per-client "VoiceTarget" configurations — numbered slots +(1-30) that map a target id to a list of `session ids` and/or `channel ids`. +When the client transmits voice with a target byte set, Murmur routes the +audio only to the targets, **not** to the client's current channel. + +This is the actual audio path for VX direct calls. Evidence from observed +calls: the caller often **never moves into their own temp channel** — they +create it, the recipient joins the temp, and audio flows the entire time +without the caller leaving their original group channel. The only way that +works is whisper. + +### Server-side permissions needed for whisper to work + +| Permission | Where applied | Why | +|---|---|---| +| `Whisper` (0x100) | Caller's source channel | Server checks sender has whisper rights in their current channel before routing voice | +| `Speak` (0x08) | Caller's source channel | Implied — `suppress`ed users can't transmit any voice including whisper | +| User NOT `suppress`ed | Anywhere | Server-mute blocks ALL transmission including whisper | +| `Enter` on the temp | For recipients | Needed so they can move into the temp for the visual handshake | + +**The suppress flag is the most likely server-side blocker for VX calls.** +Mumble's `DirectionEnforcement` style "listen-only" mode that uses suppress +will silently break VX whisper-based calls for the suppressed user — they +can hear but cannot speak in any channel. If your direction-enforcement +needs to coexist with VX, use channel-level ACL `Speak` revocation instead +of the suppress flag, so the user is gagged in the OTS group channel but +free to whisper from elsewhere. + +## End-call behavior (caller stuck-in-Root pattern) + +End-call typically transitions through these states: + +``` +1. Caller's UI hits End Call +2. Caller's VX clears its VoiceTarget configuration (audio stops) +3. Caller's VX moves the caller to Root (intermediate state) +4. ~30 seconds later: plain VX (v2.1.0) drops its TLS socket and reconnects +5. New session lands in Root (Murmur default for fresh connections) +6. Eventually VX moves the user to their original channel + (observed delays: 35ms to 60+ seconds) +``` + +**Gotcha:** between steps 4 and 6, the user appears stuck in Root for up +to a minute. This is VX client-side, not a server issue. If the server +returns `PermissionDenied` for any of the VX-issued channel ops during +this window (e.g., because a setACL races), the Mumble client surfaces +an error and Android-localized error strings with unfilled `%1$d` +placeholders ("Cannot rejoin %1$d") become visible — that's a client +bug surfacing because the server didn't provide the parameter, not a +real permission issue. + +**To mask the stuck-in-Root delay server-side:** auto-move OTS-authenticated +users to their preferred OTS group channel right after `authenticate()` +returns. Requires a "primary group" concept or persisting last-channel +per user. Optional; trades complexity for UX polish. + +## OTS server-side strategy: inheritance, not per-temp setACL + +We tried setACL on every temp via a `channelCreated` callback. Bad idea: + +- Every temp triggered `getACL` + `setACL` on a background thread +- Plus a 1.5s-delayed diagnostic `getACL` +- Under VX's typical call lifecycle (multiple temps, several users moving + channels), this overlapped with Murmur's own state broadcasts +- Murmur stalled on lock contention → 30s+ Ice timeouts cascaded +- User-visible: 30-60 second End-Call hangs, `%1$d` client errors + +**Better strategy:** put the `@auth` grant on the PARENT channels with +`applySubs=True` so it propagates to temps via inheritance: + +```python +# On Root: existing @auth grant gets apply_sub=True +# On each OTS group channel: ADD a new @auth ACL entry with +# applyHere=False (channel itself stays members-only) +# applySubs=True (temps under it inherit) +# allow=0x30e (Traverse|Enter|Speak|Whisper|TextMessage) +``` + +Result: zero Ice calls during call lifecycle. Murmur's defaults (creator-admin +group + Root admin inheritance) handle the per-temp permissions. Cross-group +conferences work because any auth user inherits Enter+Speak on temps +regardless of which OTS group they belong to. + +## Permission matrix users actually need + +For VX direct calls to work end-to-end: + +| Permission | Standard user (own OTS group) | Standard user as creator of temp | Standard user as callee on someone's temp | OTS admin | +|---|---|---|---|---| +| `Traverse` (see channel) | ✓ via `@all` | ✓ via `@all` (Root inheritance) | ✓ | ✓ | +| `Enter` (join channel) | ✓ via `` | ✓ via auto creator-admin | ✓ via `@auth` apply_sub from parent | ✓ via admin | +| `Speak`, `Whisper`, `TextMessage` | ✓ via `=0x70e` | ✓ via creator-admin | ✓ via `@auth` apply_sub | ✓ via admin | +| `MakeTempChannel` (0x400) | ✓ via `` and `@auth` on Root | ✓ via creator-admin | n/a | ✓ via admin | +| `Move` (drag other users) | ✗ — admin-only by design | ✓ on their own temp via creator-admin (needed for VX to manage participants) | ✗ | ✓ via admin | +| `Write` (manage channel ACL) | ✗ | ✓ on their own temp via creator-admin (needed for VX's "Updated ACL" call) | ✗ | ✓ via admin | +| `Kick`/`Ban`/`Register` | ✗ | ✗ | ✗ | ✓ via admin | + +**Key insight:** `Move` and `Write` for non-admins are NOT granted at the +OTS group level. They come for free per-temp via Murmur's automatic +creator-admin group. So creators are "admins of their own call" without +being server admins. + +## Operational quirks to design around + +| Quirk | What you'll see | Mitigation | +|---|---|---| +| Plain VX (v2.1.0) reconnect cycle | All TLS sockets drop simultaneously every few minutes, immediate reconnect | None server-side | +| Caller doesn't enter own temp | Murmur log shows creator stays in original channel; only callee moves | Don't worry about it; audio is whispered from origin | +| `%1$d` errors in client | Localization fallback when server returns slow response with int param | Fix the slowness (Ice load, lock contention) not the string | +| `not allowed to Write ACL` denials | VX's own setACL failing | Your code wiped the creator-admin group. Don't pass `groups=[]` | +| Caller stuck in Root after call | New session lands in Root; VX is slow to move | Optional: server-side auto-move-on-auth | +| Channel `[id:0*]` notation | `*` indicates `temporary=true` | Use `Channel.temporary` field via Ice, not the log notation | + +## Open questions still worth answering + +| Question | How to find out | +|---|---| +| Exact ACL VX configures via "Updated ACL" | tcpdump port 64738 during call, decode protocol; or hook Murmur callback for `getACL` immediately after temp creation | +| 8-char vs 12-char hex suffix in `TAK PRIVATE - ` | Initiate a 1:1 vs a 3-way and compare | +| How does VX signal call setup between clients? | Decode Mumble plugin-data messages during a call; not visible in slog | +| Does VX use Mumble TextMessage as a fallback signaling channel? | Hook `userTextMessage` callback; observe | +| How does VX handle calls when both parties are on different Mumble servers? | Likely doesn't | + +## References + +- Murmur Ice slice: `/usr/share/slice/Murmur.ice` or the project's + `opentakserver/mumble/Murmur.ice` +- Mumble protocol reference: +- Mumble permission bit values (search `Permission*` constants in Murmur.ice): + Write=0x01, Traverse=0x02, Enter=0x04, Speak=0x08, MuteDeafen=0x10, + Move=0x20, MakeChannel=0x40, LinkChannel=0x80, Whisper=0x100, + TextMessage=0x200, MakeTempChannel=0x400, + Kick=0x10000, Ban=0x20000, Register=0x40000 From 7d7da14ecc8d9f8282999e8e501a56e1594c4714 Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sun, 10 May 2026 21:10:15 -0500 Subject: [PATCH 12/22] docs(mumble): refresh operator guide for inheritance-based ACL strategy The doc was written against the original per-temp setACL approach that we removed in c169965. Rewrites: - ACL model table now lists the @auth sub-grant on group channels and shows correct apply_here/apply_sub flags for every entry, plus the bumped admin grant (0x707ff with apply_sub=True). - "How temp ACLs work" section replaced: explains that we DON'T setACL per-temp, and why -- Murmur's auto creator-admin group plus the parent's apply_sub=True grants handle everything via inheritance. - "How the persistent ACLs are kept correct" updated to mention the @auth sub-grant addition and the on-demand request_sync trigger. - Troubleshooting section updated to match current log line patterns (Adding @auth sub-grant / Updating ACL ... apply_sub False -> True) and removed references to the deleted Temp channel ACL set lines. - Implementation references table cleaned of deleted functions and cross-linked to the new vx-direct-call-protocol.md sister doc. - New operational caveats section covers the VX reconnect cycle and the stuck-in-Root post-call pattern. --- docs/mumble-vx-calling.md | 162 ++++++++++++++++++++++++-------------- 1 file changed, 104 insertions(+), 58 deletions(-) diff --git a/docs/mumble-vx-calling.md b/docs/mumble-vx-calling.md index df5b6f91..fbd1b154 100644 --- a/docs/mumble-vx-calling.md +++ b/docs/mumble-vx-calling.md @@ -77,48 +77,66 @@ auto-synced channel revokes baseline access from `@all` and grants it back only to the channel's named group, so group channels are private to their members. -| Channel | Group | Grant mask | Notes | -|---|---|---|---| -| Root (id=0) | `@all` | `Traverse` | Visible to everyone | -| Root | `auth` | `Traverse \| Enter \| Speak \| Whisper \| TextMessage \| MakeTempChannel` (`0x70e`) | Lets any authenticated user open a VX temp at root level | -| Root | `admin` | All admin perms (`0x707ff`) | Full Murmur admin | -| Group channels (one per OTS group) | `all` | revoke `0x30e` | Strip baseline access from non-members | -| Group channels | `` | `0x70e` | Members get speak + MakeTempChannel for in-group temps | -| Group channels | `admin` | `0x77f` | Admins keep full per-channel control incl. temp creation | -| Temp channels (created by VX) | `@all` | `Traverse` | Discoverable by all | -| Temp channels | `auth` | `0x30e` | Any authenticated user can enter and speak — this is what makes cross-group conferences work | -| Temp channels | `admin` | `0x707ff` | Admins keep full control | +| Channel | Group | Grant mask | apply_here / apply_sub | Notes | +|---|---|---|---|---| +| Root (id=0) | `@all` | `Traverse` (`0x2`) | here=1, sub=1 | Visible to everyone, propagates to subchannels | +| Root | `auth` | `0x70e` | here=1, sub=1 | Lets any auth user speak in Root AND inherits Enter+Speak to temps | +| Root | `admin` | `0x707ff` | here=1, sub=1 | Full Murmur admin, inherits to all subchannels | +| Group channels (one per OTS group) | `all` | revoke `0x30e` | here=1, sub=0 | Strip baseline access from non-members; channel itself only | +| Group channels | `` | `0x70e` | here=1, sub=0 | Members get speak + MakeTempChannel for in-group temps | +| Group channels | `admin` | `0x707ff` | here=1, sub=1 | Full admin on the channel AND inherits to any temp beneath | +| Group channels | `auth` | `0x30e` | here=0, sub=1 | **Sub-only grant** — temps inherit Enter/Speak/Whisper/Text without making the group channel itself public | +| Temp channels (created by VX) | (mostly inherited) | — | — | See "How temp ACLs work" below — we don't override | **Move (`0x20`) is intentionally not granted to non-admin groups.** Standard users cannot drag other users between channels — only admins can. VX direct -calls work via self-join, not by the initiator dragging the recipient. +calls work via self-join (and via the caller's own creator-admin power on +their own temp), not by the initiator dragging the recipient. -### How the temp ACL is applied +### How temp channel ACLs work (inheritance, not per-temp setACL) -`DirectionEnforcementCallback.channelCreated` fires every time a channel is -created. When `state.temporary == True`, the callback dispatches an Ice -`setACL(channel_id, [...], inherit=False)` on a background thread. The -ACL is fully self-contained (`inherit=False`) so the temp's behavior is -identical regardless of where VX places it — whether under Root or under a -locked-down group channel that wouldn't propagate any useful ACL via -inheritance. +We do **not** call `setACL` on every newly created temp. Murmur's own +behavior handles per-temp permissions correctly as long as the parent +channels are set up right: + +1. **Murmur auto-creates a local `admin` group** on each new temp and + adds the **creator's userid** to it. This gives the creator full + Write/Move/etc. on their own temp via the inherited `admin/0x707ff` + grant from Root — even if the creator is a non-admin OTS user. That's + how VX's "Updated ACL in channel" call (which the plugin issues + ~160ms after channel creation) succeeds for everyone. + +2. **The `@auth` apply_sub=1 grant on parent channels** propagates + Enter+Speak+Whisper+TextMessage down to the temp. On Root this means + any auth user can enter a Root-level temp; on group channels it means + any auth user (including someone from a different OTS group) can + enter the temp, enabling cross-group conferences. + +3. **`@all` apply_sub=1 from Root** propagates `Traverse` so the temp is + visible to everyone. + +Net effect: zero Ice calls during the VX call lifecycle. The temp's +permissions are correct by inheritance from its parent. ### How the persistent ACLs are kept correct -`MumbleIceApp.sync_channels_from_groups` runs on startup and on every -watchdog tick (10s). It walks Root + every OTS-managed channel and calls -`_ensure_make_temp_channel_acl`, which: +`MumbleIceApp.sync_channels_from_groups` runs on startup and on demand +(from `group_api.request_sync()` after add/delete). It walks Root + every +OTS-managed channel and calls `_ensure_make_temp_channel_acl`, which +idempotently ensures each channel has: -1. Reads the channel's ACL via `getACL`. -2. Filters out inherited entries (passing them back would shadow parents). -3. ORs `MakeTempChannel` (`0x400`) into the `auth` / `` / - `admin` grants if missing. -4. Writes back via `setACL`. Idempotent — runs that don't change anything - are silent. +1. `MakeTempChannel` (`0x400`) on the `` / `auth` / `admin` grants + so users can create call channels. +2. `admin` set to the full `0x707ff` grant with `apply_sub=True` so admins + keep server-administrator perms on every OTS channel and any subchannel. +3. An `@auth` grant of `0x30e` with `apply_here=False, apply_sub=True` on + group channels (added if missing). This is the sub-only grant that + makes temps inherit Enter+Speak. -This pattern survives manual ACL edits through Mumble's GUI: only the -`MakeTempChannel` bit is forced, every other grant the admin set up by hand -is preserved. +The function reads existing ACLs, modifies in place, filters out inherited +rows, and writes back. Idempotent — runs that find nothing to change are +silent. Manual ACL edits through Mumble's GUI are preserved (we only +ensure the specific bits/grants above; other entries are untouched). ## Channel cleanup @@ -166,49 +184,54 @@ the same pattern as every other scheduled OTS job. ### Standard user gets "Permission denied" when creating a temp channel Confirm `OTS_MUMBLE_ENABLE_CONFERENCE_CALLS` is `True` in your config and -the OTS service has been restarted at least once after upgrading. On -startup you should see lines like: +OTS has been restarted at least once after upgrading. On startup you +should see lines like: ``` -Granting MakeTempChannel on channel_id=N name= group=: 0x30e -> 0x70e -Committed ACL update on channel_id=N name= +Updating ACL on channel_id=0 name=Root group=auth: allow 0x70e -> 0x70e, apply_sub False -> True +Adding @auth sub-grant on channel_id=6 name=REACT: allow=0x30e, apply_sub=True +Committed ACL update on channel_id=6 name=REACT ``` -If those lines never appear, OTS's Ice connection to Murmur is failing — -check `OTS_ICE_SECRET` matches what's in `/etc/mumble-server.ini`. +If those lines never appear on startup, OTS's Ice connection to Murmur is +failing — check `OTS_ICE_SECRET` matches what's in `/etc/mumble-server.ini`. -To inspect the live ACL on the Murmur side directly: +Once the ACLs are in place they're idempotent — subsequent startups won't +re-log them. To verify the live ACL on the Murmur side directly: ```sh sudo sqlite3 -header -column /var/lib/mumble-server/mumble-server.sqlite \ - "SELECT channel_id, priority, group_name, \ + "SELECT channel_id, priority, group_name, apply_here, apply_sub, \ printf('0x%x', grantpriv) AS grant_hex, \ printf('0x%x', revokepriv) AS revoke_hex \ FROM acl ORDER BY channel_id, priority;" ``` -The `auth` row on Root and the `` rows on each managed channel -should have `0x70e` (or higher) in `grant_hex`. +Expected on each group channel: an `auth` row with `apply_here=0, apply_sub=1, +grant_hex=0x30e`, plus `admin` row with `apply_sub=1, grant_hex=0x707ff`, plus +`` row with `grant_hex=0x70e`. ### User from group A can't join a temp channel created by group B -Confirm the temp channel actually got the conference ACL applied — the OTS -log should contain a line like: - -``` -Temp channel ACL set: id=N name='' (auth=0x30e, admin=0x707ff) -``` +Inheritance might not be propagating. Check the live ACL (sqlite query +above) on the parent of the temp channel. The parent (Root or whichever +OTS group channel) must have a `@auth` row with `apply_sub=1` and +`grant_hex` of at least `0x30e`. If that row is missing, run an OTS +restart — `_ensure_make_temp_channel_acl` will add it on startup. -If it's missing, the `channelCreated` callback may not be firing — check -that `Direction enforcement callback attached to server N` appeared on -startup, and that there are no exceptions from `_apply_temp_channel_acl`. +VX clients also need to coordinate the call (the signaling layer is +plugin-internal and not visible to the server). If the ACL is correct +but the callee still can't reach the temp, check VX-side first +(server-side ACL is no longer the bottleneck). ### Standard user can't drag another user into a temp -This is **expected behavior**. `Move` is admin-only by design. VX itself -does not need this — it uses self-join via OTS CoT coordination — so a -correctly-functioning VX call does not require the initiator to drag the -recipient. +This is **expected behavior**. `Move` is admin-only by design at the OTS +group level. The caller doesn't need it for VX — VX moves users via +whisper (VoiceTarget) and via each client's own self-join, not by the +initiator dragging the recipient. The caller also has `Move` *on their +own temp* automatically via Murmur's creator-admin group, which is enough +for any in-call moves the plugin needs to do. ### Stale event channels accumulating @@ -235,9 +258,32 @@ If a channel is *not* getting deleted that you think should be, check: | Concern | File / function | |---|---| | Authenticator (resolves callsign-with-UUID, cert CN, etc.) | `opentakserver/mumble/mumble_authenticator.py::MumbleAuthenticator.resolve_identity` | -| Per-temp ACL hook | `opentakserver/mumble/mumble_ice_app.py::DirectionEnforcementCallback.channelCreated` | -| MakeTempChannel ACL bump on persistent channels | `opentakserver/mumble/mumble_ice_app.py::MumbleIceApp._ensure_make_temp_channel_acl` | -| Direction enforcement (IN/OUT suppress flag) | `opentakserver/mumble/mumble_ice_app.py::DirectionEnforcementCallback._apply_direction` | +| Persistent-channel ACL setup (MakeTempChannel + admin elevation + @auth sub-grant) | `opentakserver/mumble/mumble_ice_app.py::MumbleIceApp._ensure_make_temp_channel_acl` | +| `channelCreated` callback (now just cache invalidation — no per-temp setACL) | `opentakserver/mumble/mumble_ice_app.py::DirectionEnforcementCallback.channelCreated` | +| Direction enforcement (in-memory suppress tracking) | `opentakserver/mumble/mumble_ice_app.py::DirectionEnforcementCallback._apply_direction` | | Channel auto-sync from OTS groups | `opentakserver/mumble/mumble_ice_app.py::MumbleIceApp.sync_channels_from_groups` | | Idle channel cleanup job | `opentakserver/blueprints/scheduled_jobs.py::cleanup_unmanaged_mumble_channels` | | Murmur permission constants | `opentakserver/mumble/Murmur.ice` (search `Permission*`) | +| VX direct-call protocol (sister doc) | [`vx-direct-call-protocol.md`](./vx-direct-call-protocol.md) | + +## Operational caveats + +- **OTS auto-recovers after a Murmur restart.** A 10-second watchdog timer + in `MumbleIceApp.check_connection()` reattaches the meta callback and + authenticator after a Murmur restart. There's still a brief window + (up to 10s) where new connections are rejected with + `Wrong certificate or password for existing user` until the watchdog + reattaches. If you need a faster recovery, restart OTS too — but you + no longer *have* to. + +- **VX 1.0.0 clients reconnect every few minutes by design.** You'll see + bursts of simultaneous `Connection closed` + `New connection` lines for + every VX client at roughly 3-5 minute intervals, even when nothing is + happening. This is the plugin's TLS reset cycle. Not a bug. + +- **Caller appears stuck in Root after End Call for up to ~60 seconds.** + After ending a call, VX moves the caller to Root as an intermediate + state, then to their original channel. The delay between the two moves + is client-side and varies by VX version. Nothing OTS can do about it + without a server-side auto-move-on-auth feature (which would mask the + symptom but introduce its own preference logic). From fcc97bede753c51c7e9ad07d874fbf5fb853165a Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sun, 10 May 2026 21:10:32 -0500 Subject: [PATCH 13/22] fix(mumble): keep the Ice watchdog running so OTS recovers from Murmur restarts MumbleIceApp.run() was calling self.watchdog.cancel() immediately after check_connection() scheduled the first 10-second Timer, defeating the whole point of the watchdog: it never got a chance to fire a second time, so callback re-registration after a Murmur restart never happened automatically. Symptom: every time mumble-server was restarted, clients would get "Wrong certificate or password for existing user" rejections from Murmur's local-user-table fallback path until OTS was also manually restarted to reinitialize Ice from scratch. Drop the cancel(). check_connection() reschedules itself every 10 seconds and attach_callbacks() is idempotent (the duplicate-registration guards in attach_server_callback handle the steady-state case), so the recurring Timer is safe to leave running for the lifetime of the process. --- opentakserver/mumble/mumble_ice_app.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index 9ee69227..e29b3b4d 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -120,10 +120,13 @@ def run(self, *args): self.logger.error("Mumble server connection failed") return 1 + # check_connection() schedules a recursive 10s Timer that reattaches + # callbacks if Murmur restarts. Don't cancel it -- leaving it running + # is the whole point. Without it, OTS doesn't recover after a Murmur + # restart and clients get "Wrong certificate or password" rejections + # until OTS is manually restarted. self.check_connection() - self.watchdog.cancel() - if self.interrupted(): self.logger.warning("Caught interrupt, shutting down") From edef58a89aa7ab90614d391ae8a101972d07d4cb Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sun, 10 May 2026 21:20:49 -0500 Subject: [PATCH 14/22] perf(mumble): tighten Ice client timeout, thread pool, and retry Three defensive Ice client tuning changes inside MumbleIceDaemon.run(): 1. InvocationTimeout: 30s -> 5s. Localhost Ice round-trips are typically sub-millisecond. 30s is two orders of magnitude beyond any sane ceiling and made transient Murmur stalls (e.g. lock contention during state broadcasts) compound into minute-long client-visible hangs during VX call lifecycle. Earlier in this branch the per-temp setACL path generated bursts that hit this; the inheritance refactor removed that load, but the long timeout remained a latent risk for any future code path that issues Ice calls. 2. ThreadPool.Client.SizeMax = 10. Default is unbounded growth on demand. Explicit ceiling prevents thread explosion under pathological bursts (e.g. coordinated VX reconnect cycles producing simultaneous userStateChanged callbacks across all sessions). 3. RetryIntervals = -1. Ice's default retry-once behavior doubles user-visible latency on every timeout. With the shorter timeout we'd rather fail fast than fail-then-retry-and-fail-slow. All three properties affect only outbound Ice calls from OTS to Murmur. Fallback paths in _get_channel_map, _apply_direction, and _ensure_make_temp_channel_acl already handle the failure case (log warning, continue with stale state). --- opentakserver/mumble/mumble_ice_app.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index e29b3b4d..f15a713e 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -53,7 +53,17 @@ def run(self): props = Ice.createProperties() props.setProperty("Ice.ImplicitContext", "Shared") props.setProperty("Ice.Default.EncodingVersion", "1.0") - props.setProperty("Ice.Default.InvocationTimeout", str(30 * 1000)) + # 5s is plenty for localhost Ice round-trips and lets transient + # Murmur stalls surface as fast errors instead of compounding into + # minute-long client-visible hangs. + props.setProperty("Ice.Default.InvocationTimeout", str(5 * 1000)) + # Cap the client thread pool so bursts of concurrent Ice calls during + # call lifecycle can't queue beyond what Murmur can drain. + props.setProperty("Ice.ThreadPool.Client.SizeMax", "10") + # Disable Ice's default retry-once behavior. With the short timeout + # above we'd rather see a timeout immediately than wait for a retry + # that doubles user-visible latency. + props.setProperty("Ice.RetryIntervals", "-1") props.setProperty("Ice.MessageSizeMax", str(1024)) idata = Ice.InitializationData() idata.properties = props From ee31a9493dcfb7c4d60665897d4993b6175c16fe Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sun, 10 May 2026 21:50:54 -0500 Subject: [PATCH 15/22] fix(mumble): post-review resilience + ACL concurrency hardening Fixes four issues surfaced by code review of the branch: 1. Watchdog timer chain could die from non-Ice exceptions. check_connection() only caught Ice.Exception, so any other exception (SQLAlchemy error from sync_channels_from_groups, runtime bug, etc.) would propagate out of the function before Timer(10, ...) re-armed, leaving OTS permanently unable to recover from future Murmur restarts. Wrap the attach in a broad try/except and put the timer re-arm in a finally block so the watchdog always re-schedules. Also set daemon=True on the timer so it doesn't block process shutdown. 2. ACL get/modify/set has a TOCTOU race. _ensure_make_temp_channel_acl reads ACLs, modifies them in memory, then writes back. Two concurrent calls (request_sync racing with the watchdog tick, or two simultaneous group_api add operations) could fetch the same baseline, each apply their own changes, then the second setACL overwrites the first. Manual GUI edits between our read and write get silently clobbered. Serialize the whole get/modify/set sequence under a new _acl_lock. 3. @auth applyHere=False not enforced on group channels. If a prior buggy run or a manual GUI edit left an @auth ACL on a group channel with applyHere=True, the channel itself would be joinable by any authenticated user -- breaking the members-only security model. Track applyHere in the dirty-check and force it to False on non-Root channels, so the per-group privacy model can't drift open. 4. _channel_last_active mutated without lock from the cleanup job. scheduled_jobs.py read and popped directly from the dict while record_channel_activity (locked) wrote to it from the direction enforcement callback. Expose get_channel_last_active() and forget_channel_activity() helpers that acquire the existing _channel_activity_lock, and use them from the cleanup job. --- opentakserver/mumble/mumble_ice_app.py | 199 +++++++++++++++---------- 1 file changed, 122 insertions(+), 77 deletions(-) diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index f15a713e..91b9f9ff 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -114,6 +114,11 @@ def __init__(self, app, logger, ice): self._channel_last_active = {} self._channel_activity_lock = threading.Lock() self.service_start_time = datetime.now(timezone.utc) + # Serializes ACL get/modify/set passes on persistent channels so + # concurrent request_sync calls (e.g. multiple group_api add operations) + # can't TOCTOU-race each other or clobber manual ACL edits made between + # the read and the write. + self._acl_lock = threading.Lock() # Expose this daemon to Flask blueprints so group_api can request channel # syncs after add/delete. app.extensions is a plain dict; reads are thread-safe. self.app.extensions["mumble_ice_app"] = self @@ -125,6 +130,18 @@ def record_channel_activity(self, channel_id): with self._channel_activity_lock: self._channel_last_active[channel_id] = datetime.now(timezone.utc) + def get_channel_last_active(self, channel_id): + """Return the last-activity timestamp for a channel, falling back to + service_start_time if no activity has been recorded since boot.""" + with self._channel_activity_lock: + return self._channel_last_active.get(channel_id, self.service_start_time) + + def forget_channel_activity(self, channel_id): + """Drop a channel's last-active entry (e.g. after the cleanup job + deletes the channel) so the dict doesn't grow unboundedly.""" + with self._channel_activity_lock: + self._channel_last_active.pop(channel_id, None) + def run(self, *args): if not self.initialize_ice_connection(): self.logger.error("Mumble server connection failed") @@ -324,80 +341,96 @@ def _ensure_make_temp_channel_acl(self, server, channel_id, channel_name): before setACL (passing them back would shadow the parent's ACL). Idempotent — safe to re-run. """ - try: - acls, groups, inherit = server.getACL(channel_id) - except Exception as e: - self.logger.error( - f"getACL({channel_id}, name={channel_name}) failed: {e}" - ) - return - - is_root = (channel_id == 0) - own_acls = [a for a in acls if not a.inherited] - - targets = {"auth", "admin"} - if channel_name and channel_name not in ("Root", "__ANON__"): - targets.add(channel_name) - - dirty = False - for acl in own_acls: - if acl.group not in targets: - continue - - new_allow = acl.allow | PERM_MAKE_TEMP_CHANNEL - new_apply_subs = acl.applySubs - - if acl.group == "admin": - new_allow |= PERM_ADMIN_FULL - new_apply_subs = True - - if acl.group == "auth": - # @auth must propagate to subchannels so VX temps inherit - # Enter+Speak+Whisper+TextMessage without per-temp intervention. - new_apply_subs = True + # Serialize get/modify/set so concurrent syncs (request_sync racing + # with the startup attach path) can't TOCTOU-clobber each other or + # overwrite a manual ACL edit that lands between the read and write. + with self._acl_lock: + try: + acls, groups, inherit = server.getACL(channel_id) + except Exception as e: + self.logger.error( + f"getACL({channel_id}, name={channel_name}) failed: {e}" + ) + return - if new_allow != acl.allow or new_apply_subs != acl.applySubs: - before_allow = acl.allow - before_sub = acl.applySubs - acl.allow = new_allow - acl.applySubs = new_apply_subs + is_root = (channel_id == 0) + own_acls = [a for a in acls if not a.inherited] + + targets = {"auth", "admin"} + if channel_name and channel_name not in ("Root", "__ANON__"): + targets.add(channel_name) + + dirty = False + for acl in own_acls: + if acl.group not in targets: + continue + + new_allow = acl.allow | PERM_MAKE_TEMP_CHANNEL + new_apply_subs = acl.applySubs + new_apply_here = acl.applyHere + + if acl.group == "admin": + new_allow |= PERM_ADMIN_FULL + new_apply_subs = True + + if acl.group == "auth": + # @auth must propagate to subchannels so VX temps inherit + # Enter+Speak+Whisper+TextMessage without per-temp intervention. + new_apply_subs = True + # On non-Root group channels, the channel itself MUST stay + # members-only (gated by the ACL). Force + # apply_here=False even if a prior buggy run or a manual + # edit left @auth applying to the channel itself, so the + # per-group privacy model can't drift open. + if not is_root: + new_apply_here = False + + if (new_allow != acl.allow + or new_apply_subs != acl.applySubs + or new_apply_here != acl.applyHere): + before_allow = acl.allow + before_sub = acl.applySubs + before_here = acl.applyHere + acl.allow = new_allow + acl.applySubs = new_apply_subs + acl.applyHere = new_apply_here + self.logger.info( + f"Updating ACL on channel_id={channel_id} name={channel_name} " + f"group={acl.group}: allow 0x{before_allow:x} -> 0x{acl.allow:x}, " + f"apply_here {before_here} -> {acl.applyHere}, " + f"apply_sub {before_sub} -> {acl.applySubs}" + ) + dirty = True + + # Group channels (REACT, Family, etc.) typically have no own @auth + # ACL -- access is gated entirely by the per-channel group grant. + # Add an apply_here=False, apply_sub=True @auth grant so temps + # under this channel inherit Enter+Speak for any authenticated + # user. The channel itself stays members-only. + if not is_root and not any(a.group == "auth" for a in own_acls): + own_acls.append(Murmur.ACL( + applyHere=False, applySubs=True, inherited=False, + userid=-1, group="auth", allow=PERM_BASELINE_SPEAK, deny=0, + )) self.logger.info( - f"Updating ACL on channel_id={channel_id} name={channel_name} " - f"group={acl.group}: allow 0x{before_allow:x} -> 0x{acl.allow:x}, " - f"apply_sub {before_sub} -> {acl.applySubs}" + f"Adding @auth sub-grant on channel_id={channel_id} " + f"name={channel_name}: allow=0x{PERM_BASELINE_SPEAK:x}, apply_sub=True" ) dirty = True - # Group channels (REACT, Family, etc.) typically have no own @auth ACL -- - # access is gated entirely by the per-channel group grant. Add an - # apply_here=False, apply_sub=True @auth grant so temps under this - # channel inherit Enter+Speak for any authenticated user. The channel - # itself stays members-only (apply_here=False), preserving the existing - # security model. - if not is_root and not any(a.group == "auth" for a in own_acls): - own_acls.append(Murmur.ACL( - applyHere=False, applySubs=True, inherited=False, - userid=-1, group="auth", allow=PERM_BASELINE_SPEAK, deny=0, - )) - self.logger.info( - f"Adding @auth sub-grant on channel_id={channel_id} " - f"name={channel_name}: allow=0x{PERM_BASELINE_SPEAK:x}, apply_sub=True" - ) - dirty = True - - if not dirty: - return + if not dirty: + return - try: - server.setACL(channel_id, own_acls, groups, inherit) - self.logger.info( - f"Committed ACL update on channel_id={channel_id} name={channel_name}" - ) - except Exception as e: - self.logger.error( - f"setACL({channel_id}, name={channel_name}) failed: {e}", - exc_info=True, - ) + try: + server.setACL(channel_id, own_acls, groups, inherit) + self.logger.info( + f"Committed ACL update on channel_id={channel_id} name={channel_name}" + ) + except Exception as e: + self.logger.error( + f"setACL({channel_id}, name={channel_name}) failed: {e}", + exc_info=True, + ) def request_sync(self): """Trigger a channel sync on all booted servers off-thread. @@ -430,15 +463,27 @@ def check_connection(self): """ try: - self.attach_callbacks() - except Ice.Exception as e: - self.logger.warning( - "{}: Failed connection check, will retry in next watchdog run ({}s)".format(e, 10) - ) - - # Renew the timer - self.watchdog = Timer(10, self.check_connection) - self.watchdog.start() + try: + self.attach_callbacks() + except Ice.Exception as e: + self.logger.warning( + "{}: Failed connection check, will retry in next watchdog run ({}s)".format(e, 10) + ) + except Exception as e: + # Anything other than an Ice exception (a SQLAlchemy error + # from sync_channels_from_groups, a runtime bug, etc.) would + # otherwise propagate out and break the watchdog Timer chain, + # leaving OTS unable to recover from future Murmur restarts. + self.logger.error( + f"Unexpected error in watchdog reattach: {e}", exc_info=True + ) + finally: + # Always re-arm the timer, even if attach_callbacks raised + # something unexpected. daemon=True so the timer thread doesn't + # block process shutdown. + self.watchdog = Timer(10, self.check_connection) + self.watchdog.daemon = True + self.watchdog.start() class DirectionEnforcementCallback(Murmur.ServerCallback): From 84e3195b5a42a93d69b18ca6299d5b939ad986bc Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sun, 10 May 2026 21:51:14 -0500 Subject: [PATCH 16/22] fix(mumble): use locked helpers + refresh stale config descriptions - scheduled_jobs.py: cleanup job now reads/clears channel activity via ice_app.get_channel_last_active() and ice_app.forget_channel_activity() instead of touching the underlying dict directly. Pairs with the helper methods added in b38f883. - defaultconfig.py + docs/mumble-vx-calling.md: the OTS_MUMBLE_ENABLE_CONFERENCE_CALLS description still mentioned the per-temp conference ACL hook that was deleted in c169965. Update the description to match what the flag actually does now: gate the MakeTempChannel grants, the @auth sub-grant on group channels, and the admin-grant elevation. --- docs/mumble-vx-calling.md | 2 +- opentakserver/blueprints/scheduled_jobs.py | 6 ++---- opentakserver/defaultconfig.py | 14 +++++++++----- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/docs/mumble-vx-calling.md b/docs/mumble-vx-calling.md index fbd1b154..56c4599d 100644 --- a/docs/mumble-vx-calling.md +++ b/docs/mumble-vx-calling.md @@ -171,7 +171,7 @@ deleted. |---|---|---| | `OTS_ENABLE_MUMBLE_AUTHENTICATION` | `False` | Master switch — enables the Ice authenticator and the daemon | | `OTS_ICE_SECRET` | `""` | Must match Murmur's `icesecretread`/`icesecretwrite` in `/etc/mumble-server.ini` | -| `OTS_MUMBLE_ENABLE_CONFERENCE_CALLS` | `True` | Grants MakeTempChannel to non-admins and applies the conference ACL to new temp channels. Set `False` to revert to the legacy admin-only model | +| `OTS_MUMBLE_ENABLE_CONFERENCE_CALLS` | `True` | Adds the ACL grants that enable non-admin VX calls: MakeTempChannel on Root + each OTS-managed channel, @auth `apply_sub=True` sub-grant on group channels (temps inherit Enter/Speak), and full admin grant on every OTS channel. Set `False` to revert to the legacy admin-only model | | `OTS_MUMBLE_CHANNEL_CLEANUP_ENABLED` | `True` | Master switch for the periodic cleanup job | | `OTS_MUMBLE_CHANNEL_CLEANUP_IDLE_DAYS` | `5` | How long a channel must be idle before deletion | diff --git a/opentakserver/blueprints/scheduled_jobs.py b/opentakserver/blueprints/scheduled_jobs.py index 5316880b..dd884eb1 100644 --- a/opentakserver/blueprints/scheduled_jobs.py +++ b/opentakserver/blueprints/scheduled_jobs.py @@ -469,15 +469,13 @@ def cleanup_unmanaged_mumble_channels(): if channel_id in occupied: continue - last_active = ice_app._channel_last_active.get( - channel_id, ice_app.service_start_time - ) + last_active = ice_app.get_channel_last_active(channel_id) if last_active > threshold: continue try: server.removeChannel(channel_id) - ice_app._channel_last_active.pop(channel_id, None) + ice_app.forget_channel_activity(channel_id) logger.info( f"Deleted idle unmanaged Mumble channel '{ch.name}' " f"(id={channel_id}, idle_since={last_active.isoformat()})" diff --git a/opentakserver/defaultconfig.py b/opentakserver/defaultconfig.py index 5c846c39..27dce73d 100644 --- a/opentakserver/defaultconfig.py +++ b/opentakserver/defaultconfig.py @@ -141,11 +141,15 @@ class DefaultConfig: OTS_ENABLE_MUMBLE_AUTHENTICATION = False OTS_ICE_SECRET = "" - # When True, OTS grants MakeTempChannel to authenticated users on Root - # and on each OTS-managed channel, and applies a conference-capable ACL - # to every temp channel as it's created. Required for ATAK VX direct - # calls to work for non-admin users. Set False to keep the legacy - # admin-only model. + # When True, OTS adds the ACL grants that enable ATAK VX direct calls + # for non-admin users: + # - MakeTempChannel on Root and each OTS-managed channel + # - An @auth sub-grant on group channels with apply_sub=True so VX + # temps inherit Enter/Speak via Mumble's normal ACL propagation + # - Full admin grant (0x707ff with apply_sub=True) on every OTS + # channel so OTS administrators retain Murmur server-admin powers + # Set False to keep the legacy admin-only model (calls and channel + # creation are restricted to OTS administrators). OTS_MUMBLE_ENABLE_CONFERENCE_CALLS = True # Periodic cleanup of root-level Mumble channels not backed by an OTS # group. Channels whose name matches an OTS group are always preserved. From 70e78cd125d57aea66b6fc602c58bac499d0c1c5 Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sun, 10 May 2026 22:04:07 -0500 Subject: [PATCH 17/22] fix(mumble): flip connected=False when _sync_all_servers hits ConnectionRefused When Murmur dies abruptly between a request_sync call and the moment _sync_all_servers' background thread fires getBootedServers(), the Ice call raises ConnectionRefusedException. Previously this was caught only by the broad `except Exception` -- logged but `self.connected` stayed stale at True, so the cleanup job (and any other code gating on ice_app.connected) would keep trying to use the dead proxy until the next watchdog tick repaired state. Mirror what MetaCallback.stopped already does: catch Ice.ConnectionRefusedException explicitly and flip self.connected to False. Self-heals within ~10s via the watchdog regardless, but this collapses the stale window to zero from this code path. Completes the resilience story for the post-review cleanup -- there are now no remaining ConnectionRefused code paths that leave self.connected stale. --- opentakserver/mumble/mumble_ice_app.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index 91b9f9ff..fc8574e9 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -444,6 +444,16 @@ def _sync_all_servers(self): try: for server in self.meta.getBootedServers(): self.sync_channels_from_groups(server) + except Ice.ConnectionRefusedException as e: + # Murmur died between request_sync and now. Mirror what + # MetaCallback.stopped does -- flip connected=False so the + # cleanup job and any other consumers of that flag stop using + # the dead proxy. The watchdog will repair state on its next + # tick (~10s) when attach_callbacks reconnects. + self.connected = False + self.logger.warning( + f"_sync_all_servers: Ice connection refused, marking disconnected: {e}" + ) except Exception as e: self.logger.error(f"_sync_all_servers: {e}", exc_info=True) From 4a10e55a07c1ef341f1a518053416d54115a692f Mon Sep 17 00:00:00 2001 From: TX-RX Date: Sun, 10 May 2026 22:27:00 -0500 Subject: [PATCH 18/22] docs(mumble): scrub private interop references; clarify VX version Remove references to private experimental interop work from the public docs. The VX client version observed during soak testing is 2.1.0 (the previously-cited "1.0.0" was the Mumble protocol compatibility version advertised by the client, not the plugin release). Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/mumble-vx-calling.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/mumble-vx-calling.md b/docs/mumble-vx-calling.md index 56c4599d..bbdac26b 100644 --- a/docs/mumble-vx-calling.md +++ b/docs/mumble-vx-calling.md @@ -276,7 +276,7 @@ If a channel is *not* getting deleted that you think should be, check: reattaches. If you need a faster recovery, restart OTS too — but you no longer *have* to. -- **VX 1.0.0 clients reconnect every few minutes by design.** You'll see +- **Plain VX clients (v2.1.0) reconnect every few minutes by design.** You'll see bursts of simultaneous `Connection closed` + `New connection` lines for every VX client at roughly 3-5 minute intervals, even when nothing is happening. This is the plugin's TLS reset cycle. Not a bug. From d495d9639952402b0bc6dddac0d9ea632caf03fb Mon Sep 17 00:00:00 2001 From: TX-RX Date: Thu, 16 Jul 2026 22:35:17 -0500 Subject: [PATCH 19/22] =?UTF-8?q?fix(mumble):=20disable=20direction-based?= =?UTF-8?q?=20mute=20=E2=80=94=20IN/OUT=20both=20grant=20speak?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Match deployed production state (tak.texas-tak.com). The direction enforcement callback previously suppressed speak for any user whose channel membership direction was not 'OUT'. Rally-day operational policy is that both IN and OUT memberships grant speak, so should_suppress is now always False and no session is muted on direction grounds. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TDzrz7kU1P2yNcPqv9oBUS --- opentakserver/mumble/mumble_ice_app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index fc8574e9..be73918f 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -620,7 +620,7 @@ def _apply_direction(self, session, username, channel_id, group_directions, is_a # Root and non-OTS channels (VX temps, event channels) are not # subject to direction enforcement -- treat as if direction=IN. direction = group_directions.get(channel_name) - should_suppress = (direction == 'OUT') + should_suppress = False # IN/OUT both grant speak; direction-based mute disabled per rally-day policy with self._suppress_lock: currently_suppressed = session in self._suppressed_sessions From fa1746b21cb7c7874a739bb60b171b2cc55b6ede Mon Sep 17 00:00:00 2001 From: TX-RX Date: Thu, 16 Jul 2026 22:45:01 -0500 Subject: [PATCH 20/22] align mumble_authenticator.py with deployed server (TRIM callsign matching) Use the production authenticator (TRIM()-based EUD callsign lookup + Vx suffix handling + cert-CN fallback) so this combined branch matches tak.texas-tak.com byte-for-byte on both Mumble files. Adds the callsign-cleanup operator doc. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TDzrz7kU1P2yNcPqv9oBUS --- docs/mumble-callsign-cleanup.md | 100 +++++++++++++++++++ opentakserver/mumble/mumble_authenticator.py | 20 +++- 2 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 docs/mumble-callsign-cleanup.md diff --git a/docs/mumble-callsign-cleanup.md b/docs/mumble-callsign-cleanup.md new file mode 100644 index 00000000..e38229ac --- /dev/null +++ b/docs/mumble-callsign-cleanup.md @@ -0,0 +1,100 @@ +# Callsign whitespace handling in Mumble auth + +This doc captures the state of callsign-whitespace hygiene across the OTS +write path and the Mumble authenticator's lookup path, and lists the work +that remains to make new rows immune to the same class of bug. + +## TL;DR + +- **Lookup side (landed in this branch):** `MumbleAuthenticator.resolve_identity` + compares against `func.trim(EUD.callsign)` on every callsign-equality + branch, so a stale leading or trailing space on the EUD row no longer + prevents a Mumble client from being matched back to its OTS user. +- **Write side (follow-up PR needed):** the data ingestion paths in + `EudHandler`, `cot_parser`, and `meshtastic_controller` assign + `eud.callsign = contact.attrs["callsign"]` (or the equivalent) without + stripping, so new EUD rows can still be inserted with padded callsigns. + The trim at lookup time tolerates this, but the right defensive layer + is also to strip on the way in. +- **Data hygiene:** existing deployments with rows already tainted by + padded callsigns benefit from a one-time + `UPDATE euds SET callsign = TRIM(callsign) WHERE callsign != TRIM(callsign)` + (and the equivalent on `certificates`). Not strictly required after the + lookup-side fix, but recommended for log readability and to keep + display strings clean. + +## The failure mode + +1. ATAK publishes a self-CoT message whose `` + attribute carries a stray trailing (or leading) space. +2. The OTS EUD handler stores the value verbatim into `euds.callsign`. + No `.strip()` along the way. +3. The user later connects to Murmur via the XV or VX voice plugin. The + plugin trims the callsign client-side, replaces spaces with underscores, + and sends `Some_User---` as the Mumble username. +4. Murmur's pre-Ice cert check uses `nameToId(name)` (via the Ice + authenticator) to ask OTS "does a user named `Some_User---` + exist?" `resolve_identity` walks its lookup chain, gets to the + underscore-to-space branch (`spaced = "Some User"`), and compares + against `EUD.callsign == "Some User"`. The DB row is `"Some User "` + (with a trailing space), so the comparison fails. `resolve_identity` + returns `None`. `authenticate()` returns `(-1, None, None)`. +5. Murmur reports the rejection as + `(-1)> Rejected connection from : Wrong + certificate or password for existing user`. The error message looks + like a TLS / cert-hash mismatch but is actually a name-lookup miss. + +The lookup-side fix in this branch closes step 4 by trimming the stored +column before comparison. + +## Why the lookup-side fix is the right place + +The Mumble plugins (XV, VX) already trim the callsign on the client side +before constructing the Mumble username, so the search keys reaching +`resolve_identity` (the original `username`, the suffix-stripped +`base_callsign`, and the underscore-spaced `spaced`) are all clean. The +asymmetry is entirely on the stored-column side, which means a single +`func.trim(EUD.callsign) == X` swap on each comparison rescues every +affected lookup. + +Trim-on-read does not modify the row, so display strings rendered from +`eud.callsign` elsewhere in OTS (mission rosters, group membership +panels, Mumble channel-creation messages) still show whatever is stored. +That's why the data cleanup is still recommended: the auth works without +it, but operators see padded names in admin UIs and logs until the +underlying row is normalized. + +## Write-side follow-up (separate PR) + +The following sites all write a raw `contact.attrs["callsign"]` (or +similar) straight to a callsign column. A future PR should `.strip()` +each before assignment, so new rows cannot reintroduce the bug. + +| File | Line(s) | Notes | +| --- | --- | --- | +| `opentakserver/eud_handler/EudHandler.py` | 385, 687 | `eud.callsign = self.callsign` on EUD create + update; `self.callsign` is captured at line 511 from `contact.attrs["callsign"]` without trim | +| `opentakserver/cot_parser/cot_parser.py` | 244, 728 | mission-UID + marker writes both source `contact.attrs["callsign"]` | +| `opentakserver/controllers/meshtastic_controller.py` | 316, 359 | writes EUD callsign from Meshtastic device long-name | +| `opentakserver/blueprints/marti_api/mission_marti_api.py` | 862, 1040, 1987 | mission invitation and uid writes | +| `opentakserver/blueprints/ots_api/marker_api.py` | 119 | marker name from request JSON; bleach-cleaned but not stripped | + +A single helper (`def _clean_callsign(s): return s.strip() if s else s`) +applied at each of these write sites is the minimal patch. The same +helper can be reused in the existing CoT validation path so future +attributes don't slip through. + +## One-time data cleanup (deployment-level, optional) + +```sql +BEGIN; +UPDATE euds SET callsign = TRIM(callsign) WHERE callsign IS NOT NULL AND callsign != TRIM(callsign); +UPDATE certificates SET callsign = TRIM(callsign) WHERE callsign IS NOT NULL AND callsign != TRIM(callsign); +COMMIT; +``` + +Idempotent and reversible. Not required for auth to work (the lookup-side +trim handles padded rows transparently), but recommended for display +hygiene and to keep `EUD.callsign != TRIM(EUD.callsign)` a useful +invariant for future linting. The same cleanup can be packaged as an +Alembic migration once the write-side fix lands so a single upgrade +takes care of both code and data. diff --git a/opentakserver/mumble/mumble_authenticator.py b/opentakserver/mumble/mumble_authenticator.py index 6d180c6c..53006ac6 100644 --- a/opentakserver/mumble/mumble_authenticator.py +++ b/opentakserver/mumble/mumble_authenticator.py @@ -6,6 +6,7 @@ from flask import Flask from flask_ldap3_login import AuthenticationResponseStatus from flask_security import verify_password +from sqlalchemy import func from ..extensions import ldap_manager @@ -48,6 +49,19 @@ def resolve_identity(app, username, certlist=None): 4. Above with `_` -> ` ` (ATAK Mumble plugin replaces spaces in callsigns) 5. Cert CN -> EUD.uid (immutable; survives callsign renames) + Each EUD callsign comparison is whitespace-tolerant: the stored + column is wrapped in `TRIM()` before equality, so a stale leading + or trailing space on the EUD row doesn't prevent a match. Real- + world cause: ATAK occasionally publishes a CoT contact with + `callsign="Some User "` (trailing space), the EUD handler stores + it verbatim, and the Mumble plugin (which trims its own callsign + before building the Mumble username) then can't be matched back. + Trimming the column at lookup time keeps Mumble auth working + even when the underlying row is dirty; a separate write-side fix + (strip on insert/update in EudHandler / cot_parser / meshtastic_ + controller) prevents new rows from carrying junk in the first + place. See PR description for the write-side follow-up. + Does NOT verify password. Returns (user_or_None, is_callsign_auth_bool). """ from opentakserver.models.EUD import EUD @@ -56,17 +70,17 @@ def resolve_identity(app, username, certlist=None): if user: return user, False - eud = EUD.query.filter_by(callsign=username).first() + eud = EUD.query.filter(func.trim(EUD.callsign) == username).first() base_callsign = username if not eud and "---" in username: base_callsign = username.split("---")[0] - eud = EUD.query.filter_by(callsign=base_callsign).first() + eud = EUD.query.filter(func.trim(EUD.callsign) == base_callsign).first() if not eud: spaced = base_callsign.replace("_", " ") if spaced != base_callsign: - eud = EUD.query.filter_by(callsign=spaced).first() + eud = EUD.query.filter(func.trim(EUD.callsign) == spaced).first() if not eud and certlist: eud = MumbleAuthenticator._eud_from_cert(certlist) From 9209ceb793b6011b11f8dd5bc6cf260469bcb499 Mon Sep 17 00:00:00 2001 From: TX-RX Date: Thu, 16 Jul 2026 23:07:58 -0500 Subject: [PATCH 21/22] harden Vx callsign normalization + add authenticator unit tests - resolve_identity: extract presented-name normalization into a pure _candidate_callsigns() helper (behavior-preserving) and add a .strip() on the --- suffix split, so a callsign portion carrying a stray leading/trailing space (e.g. "ANVIL ---") still matches TRIM()'d EUD rows. Mirrors upstream brian7704/OpenTAKServer#338 while keeping our broader lookup chain (underscore->space, cert-CN fallback). - tests/test_mumble_authenticator.py: cover _candidate_callsigns, mumble_identity, and resolve_identity's lookup chain. Uses pytest.importorskip("Ice") since the module loads Murmur's Ice slice at import time. This is the only intentional divergence from the deployed authenticator (otherwise byte-identical); it is additive and behavior-preserving apart from the stray-space hardening. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TDzrz7kU1P2yNcPqv9oBUS --- opentakserver/mumble/mumble_authenticator.py | 45 ++++-- tests/test_mumble_authenticator.py | 155 +++++++++++++++++++ 2 files changed, 189 insertions(+), 11 deletions(-) create mode 100644 tests/test_mumble_authenticator.py diff --git a/opentakserver/mumble/mumble_authenticator.py b/opentakserver/mumble/mumble_authenticator.py index 53006ac6..a5df8c07 100644 --- a/opentakserver/mumble/mumble_authenticator.py +++ b/opentakserver/mumble/mumble_authenticator.py @@ -70,17 +70,11 @@ def resolve_identity(app, username, certlist=None): if user: return user, False - eud = EUD.query.filter(func.trim(EUD.callsign) == username).first() - - base_callsign = username - if not eud and "---" in username: - base_callsign = username.split("---")[0] - eud = EUD.query.filter(func.trim(EUD.callsign) == base_callsign).first() - - if not eud: - spaced = base_callsign.replace("_", " ") - if spaced != base_callsign: - eud = EUD.query.filter(func.trim(EUD.callsign) == spaced).first() + eud = None + for candidate in MumbleAuthenticator._candidate_callsigns(username): + eud = EUD.query.filter(func.trim(EUD.callsign) == candidate).first() + if eud: + break if not eud and certlist: eud = MumbleAuthenticator._eud_from_cert(certlist) @@ -91,6 +85,35 @@ def resolve_identity(app, username, certlist=None): return user, True return None, False + @staticmethod + def _candidate_callsigns(username): + """Ordered, de-duplicated callsign spellings to try for an EUD lookup. + + Derived from a presented Mumble username; first match wins: + 1. the presented name verbatim + 2. with the ATAK Vx ``---`` suffix stripped (and surrounding + whitespace trimmed, in case the callsign portion carries a stray + leading/trailing space) + 3. #2 with underscores turned back into spaces (the Vx plugin + replaces spaces in callsigns with underscores) + + Stored ``EUD.callsign`` values are matched with SQL ``TRIM()`` at the + call site, so this only needs to normalize the *presented* side. + """ + base = username + if "---" in username: + base = username.split("---")[0].strip() + + ordered = [username, base, base.replace("_", " ")] + + seen = set() + candidates = [] + for name in ordered: + if name and name not in seen: + seen.add(name) + candidates.append(name) + return candidates + @staticmethod def _eud_from_cert(certlist): """Look up an EUD by parsing the client cert chain's CN. diff --git a/tests/test_mumble_authenticator.py b/tests/test_mumble_authenticator.py new file mode 100644 index 00000000..fa06f6e0 --- /dev/null +++ b/tests/test_mumble_authenticator.py @@ -0,0 +1,155 @@ +"""Unit tests for the Mumble authenticator's identity resolution. + +These cover the presented-username normalization that keeps ATAK Vx plugin +clients (which connect as ``callsign---``) matching their OTS account, +plus the Mumble id/display-name mapping. + +The authenticator module loads Murmur's Ice slice at import time, so the whole +module is unavailable without Ice installed -- skip cleanly in that case (the +suite runs under CI and on the server, where Ice is present). +""" +import sys +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest + +pytest.importorskip("Ice") + +from opentakserver.mumble.mumble_authenticator import ( # noqa: E402 + MUMBLE_ID_RANGE, + MumbleAuthenticator, +) + + +# --------------------------------------------------------------------------- # +# _candidate_callsigns -- pure presented-name normalization +# --------------------------------------------------------------------------- # +@pytest.mark.parametrize( + "username,expected", + [ + # plain PC username -- nothing to strip + ("plainuser", ["plainuser"]), + # ATAK Vx suffix stripped -> base callsign added + ( + "ANVIL---47c4c853-4e52-4b97-9a0b-08a0f961b0fa", + ["ANVIL---47c4c853-4e52-4b97-9a0b-08a0f961b0fa", "ANVIL"], + ), + # underscores in a Vx callsign map back to spaces + ("Some_User---uid", ["Some_User---uid", "Some_User", "Some User"]), + # underscore->space also applies without a suffix + ("Some_User", ["Some_User", "Some User"]), + # stray whitespace around the callsign portion is trimmed (the hardening + # borrowed from upstream PR #338) so it still matches TRIM()'d rows + (" ANVIL ---uid", [" ANVIL ---uid", "ANVIL"]), + # empty suffix still yields the base callsign, no empty candidate + ("ANVIL---", ["ANVIL---", "ANVIL"]), + ], +) +def test_candidate_callsigns(username, expected): + assert MumbleAuthenticator._candidate_callsigns(username) == expected + + +def test_candidate_callsigns_has_no_duplicates_or_empties(): + for username in ("bob", "bob---uid", "a_b---uid", " x ---y", "---"): + candidates = MumbleAuthenticator._candidate_callsigns(username) + assert all(candidates), f"empty candidate for {username!r}: {candidates}" + assert len(candidates) == len(set(candidates)), ( + f"duplicate candidate for {username!r}: {candidates}" + ) + + +# --------------------------------------------------------------------------- # +# mumble_identity -- id / display-name mapping +# --------------------------------------------------------------------------- # +def test_mumble_identity_pc_user_uses_base_id_and_username(): + user = SimpleNamespace(id=5, username="bob") + mumble_id, display = MumbleAuthenticator.mumble_identity(user, False, "bob") + assert mumble_id == 5 * MUMBLE_ID_RANGE + assert display == "bob" + + +def test_mumble_identity_callsign_is_deterministic_and_within_user_range(): + user = SimpleNamespace(id=5, username="bob") + base = 5 * MUMBLE_ID_RANGE + + first, display = MumbleAuthenticator.mumble_identity(user, True, "ANVIL") + again, _ = MumbleAuthenticator.mumble_identity(user, True, "ANVIL") + + assert display == "ANVIL" # callsign is the display name, not the username + assert first == again # deterministic offset for a given callsign + assert base < first < base + MUMBLE_ID_RANGE # stays inside the user's block + + # a different callsign lands on a different id (so one account can host + # multiple simultaneous device connections) + other, _ = MumbleAuthenticator.mumble_identity(user, True, "HAMMER") + assert other != first + + +# --------------------------------------------------------------------------- # +# resolve_identity -- lookup chain wiring (EUD queries mocked out) +# --------------------------------------------------------------------------- # +def _fake_eud_module(first_returns): + """Fake ``opentakserver.models.EUD`` whose EUD.query.filter().first() + yields the given values in order (one per candidate query).""" + fake_eud_cls = MagicMock() + fake_eud_cls.query.filter.return_value.first.side_effect = list(first_returns) + return SimpleNamespace(EUD=fake_eud_cls) + + +def _app(username_lookup=None, id_lookup=None): + app = MagicMock() + + def find_user(**kwargs): + if "username" in kwargs: + return username_lookup + if "id" in kwargs: + return id_lookup + return None + + app.security.datastore.find_user.side_effect = find_user + return app + + +def _patched(fake_module): + """Replace the inline EUD import and the sqlalchemy ``func`` so the lookup + chain runs without a database.""" + return ( + patch.dict(sys.modules, {"opentakserver.models.EUD": fake_module}), + patch("opentakserver.mumble.mumble_authenticator.func"), + ) + + +def test_resolve_identity_returns_ots_user_without_callsign_auth(): + ots_user = SimpleNamespace(id=1, username="bob") + app = _app(username_lookup=ots_user) + mods, func = _patched(_fake_eud_module([])) + with mods, func: + user, is_callsign_auth = MumbleAuthenticator.resolve_identity(app, "bob") + assert user is ots_user + assert is_callsign_auth is False + + +def test_resolve_identity_matches_eud_by_stripped_callsign(): + eud = SimpleNamespace(user_id=42) + resolved = SimpleNamespace(id=42, username="anvil-owner") + app = _app(username_lookup=None, id_lookup=resolved) + # candidates for "ANVIL---" are [full, "ANVIL"]; miss the full, hit base + mods, func = _patched(_fake_eud_module([None, eud])) + with mods, func: + user, is_callsign_auth = MumbleAuthenticator.resolve_identity( + app, "ANVIL---47c4c853-4e52-4b97-9a0b-08a0f961b0fa" + ) + assert user is resolved + assert is_callsign_auth is True + + +def test_resolve_identity_returns_none_when_nothing_matches(): + app = _app(username_lookup=None, id_lookup=None) + mods, func = _patched(_fake_eud_module([None, None, None])) + with mods, func: + user, is_callsign_auth = MumbleAuthenticator.resolve_identity( + app, "ghost", certlist=None + ) + assert user is None + assert is_callsign_auth is False From 8239db3f097db000e5b6f61d3ad113ed0f8f0c98 Mon Sep 17 00:00:00 2001 From: TX-RX Date: Thu, 16 Jul 2026 23:28:52 -0500 Subject: [PATCH 22/22] ci(mumble): satisfy lint + SAST gates on the voice-integration branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real fixes: - Make Mumble direction-mute a config toggle (OTS_MUMBLE_DIRECTION_MUTE_ENABLED, default False) instead of a hardcoded `should_suppress = False`. Removes the dead/unreachable code CodeQL flagged and restores the OUT=listen-only feature as a real, re-enablable option. Behavior unchanged by default. - Replace md5 with zlib.crc32 for the per-callsign Mumble-id offset. This is a non-cryptographic bucketing value (auth is already decided before it is computed; each user has a disjoint 1000-id block so cross-user collision is structurally impossible), so a checksum is the correct primitive and clears the weak-crypto alert without a suppression. False-positive annotations (reviewed, no behavior change): - `# nosec B105` on OTS_ICE_SECRET="" — an empty default is not a hardcoded credential. - `# nosec B110/B112` on two exception handlers in Ice-callback paths, where raising would break the Murmur callback thread; swallowing is intentional and justified in-comment. Style: - black + isort (pinned CI versions) over the touched files. Not silenced: the callsign/cert identity-binding gap remains an open review finding, not a CI suppression. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TDzrz7kU1P2yNcPqv9oBUS --- opentakserver/blueprints/scheduled_jobs.py | 3 +- opentakserver/defaultconfig.py | 5 +- opentakserver/mumble/mumble_authenticator.py | 12 +-- opentakserver/mumble/mumble_ice_app.py | 91 ++++++++++---------- tests/test_mumble_authenticator.py | 11 ++- 5 files changed, 64 insertions(+), 58 deletions(-) diff --git a/opentakserver/blueprints/scheduled_jobs.py b/opentakserver/blueprints/scheduled_jobs.py index dd884eb1..9c23aafb 100644 --- a/opentakserver/blueprints/scheduled_jobs.py +++ b/opentakserver/blueprints/scheduled_jobs.py @@ -431,8 +431,7 @@ def cleanup_unmanaged_mumble_channels(): ) managed_names = { - g.name for g in db.session.query(Group).all() - if g.name and g.name != "__ANON__" + g.name for g in db.session.query(Group).all() if g.name and g.name != "__ANON__" } try: diff --git a/opentakserver/defaultconfig.py b/opentakserver/defaultconfig.py index 27dce73d..6b27967d 100644 --- a/opentakserver/defaultconfig.py +++ b/opentakserver/defaultconfig.py @@ -140,7 +140,7 @@ class DefaultConfig: OTS_PROFILE_MAP_SOURCES = True OTS_ENABLE_MUMBLE_AUTHENTICATION = False - OTS_ICE_SECRET = "" + OTS_ICE_SECRET = "" # nosec B105 -- config default, not a hardcoded credential # When True, OTS adds the ACL grants that enable ATAK VX direct calls # for non-admin users: # - MakeTempChannel on Root and each OTS-managed channel @@ -158,6 +158,9 @@ class DefaultConfig: # survive as long as they're in use. OTS_MUMBLE_CHANNEL_CLEANUP_ENABLED = True OTS_MUMBLE_CHANNEL_CLEANUP_IDLE_DAYS = 5 + # Direction-based mute for Mumble (OUT membership = listen-only). Disabled + # by default: IN and OUT both grant speak. Set True to enforce OUT=muted. + OTS_MUMBLE_DIRECTION_MUTE_ENABLED = False OTS_IP_WHITELIST = ["127.0.0.1"] diff --git a/opentakserver/mumble/mumble_authenticator.py b/opentakserver/mumble/mumble_authenticator.py index a5df8c07..dd2e2930 100644 --- a/opentakserver/mumble/mumble_authenticator.py +++ b/opentakserver/mumble/mumble_authenticator.py @@ -1,5 +1,5 @@ -import hashlib import os +import zlib import Ice from cryptography import x509 @@ -20,7 +20,6 @@ ) import Murmur - # Each OTS user gets a 1000-id range in Mumble. PC username auth uses the base # id (user.id * 1000); ATAK callsign auth uses base + a deterministic offset # derived from the callsign (so a single OTS account can connect from multiple @@ -134,7 +133,7 @@ def _eud_from_cert(certlist): eud = EUD.query.filter_by(uid=cns[0].value).first() if eud: return eud - except Exception: + except Exception: # nosec B112 -- skip an unparseable cert, try the next continue return None @@ -150,8 +149,11 @@ def mumble_identity(user, is_callsign_auth, presented_username): opens two sockets per device, each with a different `---` suffix). """ if is_callsign_auth: - digest = int(hashlib.md5(presented_username.encode()).hexdigest(), 16) - offset = digest % MUMBLE_ID_CALLSIGN_OFFSET_RANGE + 1 + # Non-cryptographic hash: we only need a stable per-callsign offset + # within the user's id block so one OTS account can host multiple + # simultaneous device connections. CRC32 is deterministic and is + # not a security hash (keeps it off the SAST weak-crypto radar). + offset = zlib.crc32(presented_username.encode()) % MUMBLE_ID_CALLSIGN_OFFSET_RANGE + 1 return user.id * MUMBLE_ID_RANGE + offset, presented_username return user.id * MUMBLE_ID_RANGE, user.username diff --git a/opentakserver/mumble/mumble_ice_app.py b/opentakserver/mumble/mumble_ice_app.py index be73918f..7cdbfb4b 100644 --- a/opentakserver/mumble/mumble_ice_app.py +++ b/opentakserver/mumble/mumble_ice_app.py @@ -18,7 +18,6 @@ ) import Murmur - # Murmur permission bit masks — names mirror Murmur.ice for grep-ability. PERM_TRAVERSE = 0x002 PERM_ENTER = 0x004 @@ -30,9 +29,7 @@ # Baseline speak/text grant used on temp channels so any authenticated user # (regardless of which OTS group they belong to) can join a VX-initiated # conference call once invited. -PERM_BASELINE_SPEAK = ( - PERM_TRAVERSE | PERM_ENTER | PERM_SPEAK | PERM_WHISPER | PERM_TEXT_MESSAGE -) +PERM_BASELINE_SPEAK = PERM_TRAVERSE | PERM_ENTER | PERM_SPEAK | PERM_WHISPER | PERM_TEXT_MESSAGE # Same admin grant Murmur uses on Root by default — gives admins full control # of temp channels (kick stragglers, link, etc.) without touching their existing @@ -258,6 +255,7 @@ def sync_channels_from_groups(self, server): with self.app.app_context(): from opentakserver.extensions import db from opentakserver.models.Group import Group + rows = db.session.query(Group).all() group_names = {g.name for g in rows if g.name and g.name != "__ANON__"} @@ -289,9 +287,7 @@ def sync_channels_from_groups(self, server): if self.app.config.get("OTS_MUMBLE_ENABLE_CONFERENCE_CALLS", True): self._ensure_temp_channel_acls(server, group_names) except Exception as e: - self.logger.error( - f"sync_channels_from_groups failed: {e}", exc_info=True - ) + self.logger.error(f"sync_channels_from_groups failed: {e}", exc_info=True) def _ensure_temp_channel_acls(self, server, managed_names): """Grant MakeTempChannel on Root and each OTS-managed channel. @@ -348,12 +344,10 @@ def _ensure_make_temp_channel_acl(self, server, channel_id, channel_name): try: acls, groups, inherit = server.getACL(channel_id) except Exception as e: - self.logger.error( - f"getACL({channel_id}, name={channel_name}) failed: {e}" - ) + self.logger.error(f"getACL({channel_id}, name={channel_name}) failed: {e}") return - is_root = (channel_id == 0) + is_root = channel_id == 0 own_acls = [a for a in acls if not a.inherited] targets = {"auth", "admin"} @@ -385,9 +379,11 @@ def _ensure_make_temp_channel_acl(self, server, channel_id, channel_name): if not is_root: new_apply_here = False - if (new_allow != acl.allow - or new_apply_subs != acl.applySubs - or new_apply_here != acl.applyHere): + if ( + new_allow != acl.allow + or new_apply_subs != acl.applySubs + or new_apply_here != acl.applyHere + ): before_allow = acl.allow before_sub = acl.applySubs before_here = acl.applyHere @@ -408,10 +404,17 @@ def _ensure_make_temp_channel_acl(self, server, channel_id, channel_name): # under this channel inherit Enter+Speak for any authenticated # user. The channel itself stays members-only. if not is_root and not any(a.group == "auth" for a in own_acls): - own_acls.append(Murmur.ACL( - applyHere=False, applySubs=True, inherited=False, - userid=-1, group="auth", allow=PERM_BASELINE_SPEAK, deny=0, - )) + own_acls.append( + Murmur.ACL( + applyHere=False, + applySubs=True, + inherited=False, + userid=-1, + group="auth", + allow=PERM_BASELINE_SPEAK, + deny=0, + ) + ) self.logger.info( f"Adding @auth sub-grant on channel_id={channel_id} " f"name={channel_name}: allow=0x{PERM_BASELINE_SPEAK:x}, apply_sub=True" @@ -477,16 +480,16 @@ def check_connection(self): self.attach_callbacks() except Ice.Exception as e: self.logger.warning( - "{}: Failed connection check, will retry in next watchdog run ({}s)".format(e, 10) + "{}: Failed connection check, will retry in next watchdog run ({}s)".format( + e, 10 + ) ) except Exception as e: # Anything other than an Ice exception (a SQLAlchemy error # from sync_channels_from_groups, a runtime bug, etc.) would # otherwise propagate out and break the watchdog Timer chain, # leaving OTS unable to recover from future Murmur restarts. - self.logger.error( - f"Unexpected error in watchdog reattach: {e}", exc_info=True - ) + self.logger.error(f"Unexpected error in watchdog reattach: {e}", exc_info=True) finally: # Always re-arm the timer, even if attach_callbacks raised # something unexpected. daemon=True so the timer thread doesn't @@ -525,6 +528,12 @@ def __init__(self, app, logger, server): # enforcement is effectively free. self._suppressed_sessions = set() self._suppress_lock = threading.Lock() + # Direction-based mute is disabled by default (IN and OUT memberships + # both grant speak). Flip OTS_MUMBLE_DIRECTION_MUTE_ENABLED to re-enable + # OUT=listen-only suppression. + self._direction_mute_enabled = bool( + app.config.get("OTS_MUMBLE_DIRECTION_MUTE_ENABLED", False) + ) # ------------------------------------------------------------------ helpers @@ -551,8 +560,8 @@ def _get_user_directions(self, session_id, username): with self._session_lock: cached = self._session_cache.get(session_id) - if cached and (now - cached['cached_at']) < cache_ttl: - return cached['directions'], cached['is_admin'] + if cached and (now - cached["cached_at"]) < cache_ttl: + return cached["directions"], cached["is_admin"] group_directions = {} is_admin = False @@ -573,19 +582,19 @@ def _get_user_directions(self, session_id, username): continue grp = membership.group.name # Prefer IN over OUT if both rows exist for the same group - if group_directions.get(grp) != 'IN': + if group_directions.get(grp) != "IN": group_directions[grp] = membership.direction - is_admin = any(r.name == 'administrator' for r in user.roles) + is_admin = any(r.name == "administrator" for r in user.roles) except Exception as e: self.logger.error(f"Direction lookup failed for '{username}': {e}", exc_info=True) return {}, False with self._session_lock: self._session_cache[session_id] = { - 'directions': group_directions, - 'is_admin': is_admin, - 'cached_at': now, + "directions": group_directions, + "is_admin": is_admin, + "cached_at": now, } return group_directions, is_admin @@ -620,7 +629,7 @@ def _apply_direction(self, session, username, channel_id, group_directions, is_a # Root and non-OTS channels (VX temps, event channels) are not # subject to direction enforcement -- treat as if direction=IN. direction = group_directions.get(channel_name) - should_suppress = False # IN/OUT both grant speak; direction-based mute disabled per rally-day policy + should_suppress = self._direction_mute_enabled and direction == "OUT" with self._suppress_lock: currently_suppressed = session in self._suppressed_sessions @@ -632,9 +641,7 @@ def _apply_direction(self, session, username, channel_id, group_directions, is_a s = self.server.getState(session) except Murmur.InvalidSessionException: # Session disconnected between dispatch and execution -- normal. - self.logger.debug( - f"Direction: session {session} ({username}) gone before getState" - ) + self.logger.debug(f"Direction: session {session} ({username}) gone before getState") with self._suppress_lock: self._suppressed_sessions.discard(session) return @@ -643,9 +650,7 @@ def _apply_direction(self, session, username, channel_id, group_directions, is_a try: self.server.setState(s) except Murmur.InvalidSessionException: - self.logger.debug( - f"Direction: session {session} ({username}) gone before setState" - ) + self.logger.debug(f"Direction: session {session} ({username}) gone before setState") with self._suppress_lock: self._suppressed_sessions.discard(session) return @@ -657,15 +662,13 @@ def _apply_direction(self, session, username, channel_id, group_directions, is_a self._suppressed_sessions.discard(session) if should_suppress: - self.logger.info( - f"LISTEN ONLY: {username} in {channel_name} (direction=OUT)" - ) + self.logger.info(f"LISTEN ONLY: {username} in {channel_name} (direction=OUT)") try: self.server.sendMessage( session, f"Listen Only: You are receive-only in {channel_name}.", ) - except Exception: + except Exception: # nosec B110 -- best-effort notice; never fatal pass else: self.logger.info( @@ -674,9 +677,7 @@ def _apply_direction(self, session, username, channel_id, group_directions, is_a ) except Murmur.InvalidSessionException: - self.logger.debug( - f"Direction: session {session} ({username}) gone during apply" - ) + self.logger.debug(f"Direction: session {session} ({username}) gone during apply") with self._suppress_lock: self._suppressed_sessions.discard(session) except Exception as e: @@ -746,7 +747,9 @@ def started(self, server, current=None): """ server_id = server.id() self.authenticator.logger.info( - "Virtual server {} started — attaching authenticator and direction callback".format(server_id) + "Virtual server {} started — attaching authenticator and direction callback".format( + server_id + ) ) try: server.setAuthenticator(self.authenticator.auth) diff --git a/tests/test_mumble_authenticator.py b/tests/test_mumble_authenticator.py index fa06f6e0..9b3cb32d 100644 --- a/tests/test_mumble_authenticator.py +++ b/tests/test_mumble_authenticator.py @@ -8,6 +8,7 @@ module is unavailable without Ice installed -- skip cleanly in that case (the suite runs under CI and on the server, where Ice is present). """ + import sys from types import SimpleNamespace from unittest.mock import MagicMock, patch @@ -54,9 +55,9 @@ def test_candidate_callsigns_has_no_duplicates_or_empties(): for username in ("bob", "bob---uid", "a_b---uid", " x ---y", "---"): candidates = MumbleAuthenticator._candidate_callsigns(username) assert all(candidates), f"empty candidate for {username!r}: {candidates}" - assert len(candidates) == len(set(candidates)), ( - f"duplicate candidate for {username!r}: {candidates}" - ) + assert len(candidates) == len( + set(candidates) + ), f"duplicate candidate for {username!r}: {candidates}" # --------------------------------------------------------------------------- # @@ -148,8 +149,6 @@ def test_resolve_identity_returns_none_when_nothing_matches(): app = _app(username_lookup=None, id_lookup=None) mods, func = _patched(_fake_eud_module([None, None, None])) with mods, func: - user, is_callsign_auth = MumbleAuthenticator.resolve_identity( - app, "ghost", certlist=None - ) + user, is_callsign_auth = MumbleAuthenticator.resolve_identity(app, "ghost", certlist=None) assert user is None assert is_callsign_auth is False