feat(mumble): ATAK VX voice integration — auth, channel/group sync, private-call ACLs#8
Open
TX-RX wants to merge 22 commits into
Open
feat(mumble): ATAK VX voice integration — auth, channel/group sync, private-call ACLs#8TX-RX wants to merge 22 commits into
TX-RX wants to merge 22 commits into
Conversation
…nforcement
## 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 `---<uuid>` 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 `---<uuid>`
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 `---<uuid>` 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Without this, the OTS install's locked-down ACL model (revoke @ALL on group channels, grant access only to <groupname> 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/<groupname>/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.
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).
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).
…in 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.
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.
…T users 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.
… 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).
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.
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.
…r 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.
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).
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.
- 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.
…ionRefused 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.
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) <noreply@anthropic.com>
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDzrz7kU1P2yNcPqv9oBUS
…ching) 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDzrz7kU1P2yNcPqv9oBUS
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
2 tasks
- resolve_identity: extract presented-name normalization into a pure _candidate_callsigns() helper (behavior-preserving) and add a .strip() on the ---<uid> suffix split, so a callsign portion carrying a stray leading/trailing space (e.g. "ANVIL ---<uid>") still matches TRIM()'d EUD rows. Mirrors upstream brian7704#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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDzrz7kU1P2yNcPqv9oBUS
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDzrz7kU1P2yNcPqv9oBUS
Owner
Author
Addressing the CI findings (commit 8239db3)Went through each scanner finding on its merits rather than blanket-suppressing. Split into real fixes vs false positives: Real fixes
False positives (annotated, no behavior change)
Lint — Not silenced: the callsign/cert identity-binding gap (callsign auth doesn't verify the presented client cert belongs to the claimed identity) is a real finding and remains open for a follow-up — it is deliberately not suppressed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full Mumble voice integration for the ATAK VX plugin, combining the authentication/group-management work and the private-call ACL / direction-enforcement work into one branch.
mumble_ice_app.pyis byte-identical to what is running in production ontak.texas-tak.com(OpenTAKServer 1.7.11);mumble_authenticator.pymatches production behavior plus one additive hardening (stray-space handling on the Vx callsign suffix) and now ships with unit tests — verified this session.This supersedes the two WIP branches
feature/mumble-authentication-and-group-managementandfeature/mumble-vx-private-call-acl: the server actually ran the union of them (auth from one, ice_app from the other), so they're merged here into a single, production-matching, cleanly-mergeable unit rather than two overlapping PRs.Authentication (
mumble_authenticator.py)resolve_identity()lookup chain: OTS username → EUD callsign exact → callsign with---<uuid>VX suffix stripped → underscore↔space substitution → cert-CN fallback.TRIM()so a stale leading/trailing space on the EUD row no longer breaks Mumble auth (real failure: ATAK publishescallsign="Name ", the plugin trims before building its username, the two never match). Seedocs/mumble-callsign-cleanup.md._candidate_callsigns()helper, with a.strip()on the---<uid>suffix split so a callsign carrying a stray leading/trailing space (e.g."ANVIL ---<uid>") still matches. Mirrors upstream Fix Mumble auth lookup for ATAK Vx plugin usernames brian7704/OpenTAKServer#338 while keeping our broader lookup chain (underscore→space, cert-CN fallback).tests/test_mumble_authenticator.py) covering_candidate_callsigns,mumble_identity, and theresolve_identitylookup chain (pytest.importorskip("Ice"), since the module loads Murmur's Ice slice at import).@adminfor OTS administrators.Channel / group management
scheduled_jobs.py,group_api.py).Private-call ACLs & enforcement (
mumble_ice_app.py)@alldeny, member allow,@adminfull,@authsub-grant via inheritance), self-healing on each sync.MakeTempChannelgrant + creator-admin preservation on temps.DirectionEnforcementCallbackwith in-memory suppress tracking; admins skipped (callinggetStateon admin sessions blocks Ice ~30s).connected=FalseonConnectionRefused), tightened timeouts/thread-pool/retry,OTS_ICE_SECRETpulled from config.Notes
main(the fork's collab branch). Rebased onto currentmain; main's Hungarian translation is preserved (the WIP branches' apparent deletion was an artifact of their older base, not intentional).OTS_ICE_SECRETdefaults to""(config-driven, no value committed).TAK PRIVATE - <hex>); no real callsigns/coordinates/UIDs.🤖 Generated with Claude Code
https://claude.ai/code/session_01TDzrz7kU1P2yNcPqv9oBUS