fix(nestsclient): reject inbound Subscribe whose broadcast doesn't match — moq-lite Lite-03 subscribe.rs

The publisher-side dispatcher in handleInboundBidi previously matched inbound Subscribe bidis on `track` only, never checking whether the requested `broadcast` field matched the suffix our publisher claimed on this session. A peer (or buggy relay) could open a Subscribe under broadcast="otherPubkey" track="audio/data" and we would route OUR audio frames to them — the dispatcher only filtered on track. Production never bit because moq-rs's relay routes Subscribe messages to the specific publisher whose broadcast matches the requested path upstream of us, so an off-target broadcast never landed on our inbound bidi pump. But a peer-to-peer connection without a relay in between would have been able to siphon our audio under any broadcast string they invented. The fix: validate `sub.broadcast == publisher.suffix` (both already path-normalised by the codec) before track matching. On mismatch, reply `SubscribeDrop(errorCode=BROADCAST_DOES_NOT_EXIST, reasonPhrase="<requested> not published on this session (we publish <ours>)")` and FIN. New error code `MoqLiteSubscribeDropCode.BROADCAST_DOES_NOT_EXIST = 0x05L` mirrors the IETF MoQ-transport `TRACK_NAMESPACE_DOES_NOT_EXIST` semantic so a watcher can tell apart "wrong room" from "wrong rendition". Regression test: `MoqLiteSessionTest.publisher_replies_subscribeDrop_when_broadcast_does_not_match`. Audit doc: nestsClient/plans/2026-05-09-moq-lite-rfc-compliance.md (M5). https://claude.ai/code/session_012TGfo99Ugz7fcCPv85a8Tq
2026-05-09 13:52:47 +00:00
parent 6c2d7efccb
commit 3dcee2a4e3
3 changed files with 94 additions and 0 deletions
@@ -156,6 +156,20 @@ object MoqLiteSubscribeDropCode {
     * `com.vitorpamplona.nestsclient.moq.ErrorCode.TRACK_DOES_NOT_EXIST`.
     */
    const val TRACK_DOES_NOT_EXIST: Long = 0x04L
+
+    /**
+     * The publisher does not serve this broadcast at all. Sent for a
+     * subscribe whose `broadcast` field doesn't match the suffix we
+     * published under on this session. Distinct from
+     * [TRACK_DOES_NOT_EXIST] (which means "we publish this broadcast
+     * but not under that track name") so the watcher can tell apart
+     * "wrong room" from "wrong rendition". Mirrors the IETF
+     * `com.vitorpamplona.nestsclient.moq.ErrorCode.TRACK_NAMESPACE_DOES_NOT_EXIST`
+     * conceptually — moq-lite's flatter path model collapses
+     * namespace/track into broadcast/track, but the same "not found
+     * at this level" semantic applies.
+     */
+    const val BROADCAST_DOES_NOT_EXIST: Long = 0x05L
 }

 /**
@@ -880,6 +880,43 @@ class MoqLiteSession internal constructor(
                        MoqLiteControlType.Subscribe -> {
                            val subPayload = buffer.readSizePrefixed() ?: return@collect
                            val sub = MoqLiteCodec.decodeSubscribe(subPayload)
+                            // Validate the requested broadcast matches our
+                            // session's publisher suffix BEFORE matching
+                            // tracks — a peer subscribing under the wrong
+                            // broadcast must not be able to siphon our audio
+                            // by guessing a track name. Both sides have
+                            // already path-normalised at the codec wire
+                            // boundary (`MoqLiteCodec.decodeSubscribe` calls
+                            // `MoqLitePath.normalize`), so a direct equality
+                            // is sufficient. Reject with
+                            // BROADCAST_DOES_NOT_EXIST so the peer sees a
+                            // typed error code rather than a silent FIN.
+                            // Publishers in `publishersSnapshot` all share
+                            // the same suffix (enforced by
+                            // [MoqLiteSession.publish]), so checking the
+                            // first is equivalent to checking all.
+                            val ourSuffix = announcePublisher.suffix
+                            if (sub.broadcast != ourSuffix) {
+                                Log.w("NestTx") {
+                                    "SUBSCRIBE inbound id=${sub.id} broadcast='${sub.broadcast}' does not match " +
+                                        "publisher.suffix='$ourSuffix' — replying SubscribeDrop"
+                                }
+                                runCatching {
+                                    bidi.write(
+                                        MoqLiteCodec.encodeSubscribeDrop(
+                                            MoqLiteSubscribeDrop(
+                                                errorCode = MoqLiteSubscribeDropCode.BROADCAST_DOES_NOT_EXIST,
+                                                reasonPhrase =
+                                                    "broadcast '${sub.broadcast}' is not published on this session " +
+                                                        "(we publish '$ourSuffix')",
+                                            ),
+                                        ),
+                                    )
+                                    bidi.finish()
+                                }
+                                dispatched = true
+                                return@collect
+                            }
                            // Find the publisher that claims this track. With the
                            // single-track-per-publisher model, only one match is possible.
                            val targetPublisher = publishersSnapshot.firstOrNull { it.track == sub.track }
@@ -570,6 +570,49 @@ class MoqLiteSessionTest {
            session.close()
        }

+    @Test
+    fun publisher_replies_subscribeDrop_when_broadcast_does_not_match() =
+        runBlocking {
+            // Lite-03 audit M5: when the relay opens a Subscribe bidi
+            // whose `broadcast` field doesn't match our publisher's
+            // suffix, the publisher MUST reply SubscribeDrop with the
+            // BROADCAST_DOES_NOT_EXIST code rather than route OUR audio
+            // to the wrong subscriber. Pre-fix we matched on `track`
+            // only and would happily send Opus frames to a peer who
+            // subscribed under any broadcast string they liked.
+            val (clientSide, serverSide) = FakeWebTransport.pair()
+            val session = MoqLiteSession.client(clientSide, pumpScope)
+
+            val publisher = session.publish(broadcastSuffix = "speakerPubkey", track = "audio/data")
+
+            val subBidi = serverSide.openBidiStream()
+            subBidi.write(Varint.encode(MoqLiteControlType.Subscribe.code))
+            subBidi.write(
+                MoqLiteCodec.encodeSubscribe(
+                    MoqLiteSubscribe(
+                        id = 11L,
+                        broadcast = "wrongPubkey",
+                        track = "audio/data",
+                        priority = 0x80,
+                        ordered = true,
+                        maxLatencyMillis = 0L,
+                        startGroup = null,
+                        endGroup = null,
+                    ),
+                ),
+            )
+
+            val ackChunk = withTimeout(2_000) { subBidi.incoming().first() }
+            val resp = MoqLiteCodec.decodeSubscribeResponse(ackChunk)
+            val dropped = resp as MoqLiteCodec.SubscribeResponse.Dropped
+            assertEquals(MoqLiteSubscribeDropCode.BROADCAST_DOES_NOT_EXIST, dropped.drop.errorCode)
+            kotlin.test.assertContains(dropped.drop.reasonPhrase, "wrongPubkey")
+            kotlin.test.assertContains(dropped.drop.reasonPhrase, "speakerPubkey")
+
+            publisher.close()
+            session.close()
+        }
+
    @Test
    fun publisher_replies_subscribeDrop_when_track_is_not_published() =
        runBlocking {