diff --git a/doc/bin/relay/auth.md b/doc/bin/relay/auth.md index e507a47e84..8e84fde839 100644 --- a/doc/bin/relay/auth.md +++ b/doc/bin/relay/auth.md @@ -64,9 +64,15 @@ The client and relay snapshot each accepted grant's deadline on a monotonic clock, so later polls, outages, and wall-clock adjustments do not restart it. **Revalidate and outage.** On the cadence the relay POSTs `revalidate` with the -same request. A grant applies: a changed `root` or `mounts`, or one that no longer covers -what the session holds closes it with `Unauthorized` (the live session is not -resized in place), and so does a flipped `peer`; a changed `tier` keeps the +same request. A grant applies: its patterns resize the live session in place, +narrower or wider, so a moderation decision lands on the session it targets and +can be lifted the same way. Subscriptions and fetches outside a narrower grant +reset with `Unauthorized`, the broadcasts it published outside abort, and +everything else keeps flowing; a wider grant brings those paths back, up to what +the session was admitted with. A one-shot HTTP `/fetch` ends on a narrower grant +instead. A +changed `root` or `mounts` closes the session with `Unauthorized`, and so does a +flipped `peer`; a changed `tier` keeps the session and moves its stats: its presence counts under the new tier from then on, as does each group and subscription it starts afterwards, while one already in flight finishes where it began. A 401 or 403 closes the session now, as does a 2xx diff --git a/doc/lib/rs/moq-net.md b/doc/lib/rs/moq-net.md index 7b5e828c9d..d16b3d1571 100644 --- a/doc/lib/rs/moq-net.md +++ b/doc/lib/rs/moq-net.md @@ -115,6 +115,29 @@ shrinks withdraws the announcements and cancels the subscriptions it no longer covers, and leaves the session up. On moq-lite each cancelled stream resets with `StreamError::Unauthorized`. +`session.auth().authorize(&grant)` authorizes the peer for `grant` on a live +session; calling it again re-authorizes, narrower or wider. It works on every +version, whether or not the peer speaks AUTH: the session enforces it itself, and +never past its origin handles. A narrower grant ends what falls outside at once: announcements to the peer +retract, its new requests are refused, its subscriptions and fetches reset with +`Unauthorized`, and the broadcasts it published abort, so local readers see +`Unauthorized` too. A wider one brings back what the old grant held back: +announcements are made again, the peer's own announcements return to the origin, +and new requests are accepted. The rest of the session carries on. When the +session answers the peer's connection credential itself, the peer is sent the +grant. + +```rust +// Deafen alice's audio: the peer may still subscribe to her video. +let video: Pattern = "room/alice/video/**".parse()?; +session.auth().authorize(&auth::Grant { + subscribe: video.into(), + ..auth::Grant::all() +}); +// Undeafen. +session.auth().authorize(&auth::Grant::all()); +``` + ## Patterns `Pattern` describes a set of paths; `Patterns` is a union reduced by diff --git a/js/net/src/lite/publisher.test.ts b/js/net/src/lite/publisher.test.ts index 2466166cd2..d21a481d2c 100644 --- a/js/net/src/lite/publisher.test.ts +++ b/js/net/src/lite/publisher.test.ts @@ -1,5 +1,8 @@ import { expect, mock, spyOn, test } from "bun:test"; import { Signal } from "@moq/signals"; +import type { Grant } from "../auth.ts"; +import { Producer as BroadcastProducer } from "../broadcast.ts"; +import { StreamCode, toStreamCode } from "../error.ts"; import { Producer as GroupProducer } from "../group.ts"; import { randomHop } from "../hop.ts"; import { hooks } from "../internal.ts"; @@ -1641,3 +1644,94 @@ test("lite draft-05: a group that goes stale while its stream opens writes nothi origin.close(); } }); + +function grantOf(publish: string): Grant { + return { publish: new Path.Patterns([Path.Pattern.subtree(publish)]), subscribe: new Path.Patterns([]) }; +} + +// The resets carrying our UNAUTHORIZED code, by message. +function unauthorizedResets(resets: { mock: { calls: unknown[][] } }): string[] { + return resets.mock.calls + .map(([reason]) => reason as Error) + .filter((reason) => toStreamCode(reason) === StreamCode.Unauthorized) + .map((reason) => reason.message); +} + +// The grant watch is armed before the first check, so a shrink that lands while the broadcast +// is still resolving resets the subscription instead of being missed for good. +test("lite draft-06: a grant that shrinks while the broadcast resolves resets the subscription", async () => { + const pair = createMockTransportPair(ALPN_06); + const origin = new OriginProducer(); + const handle = origin.dynamic(Path.from("live")); + const requests = handle.requested(); + const grant = new Signal(grantOf("live")); + const publisher = new Publisher(pair.server, Version.DRAFT_06, randomHop(), origin.consume(), grant); + + const client = await Stream.open(pair.client); + const server = await Stream.accept(pair.server); + if (!server) throw new Error("publisher never accepted the subscribe stream"); + + const resets = spyOn(Writer.prototype, "reset"); + const produced = new BroadcastProducer(); + try { + const msg = new Subscribe({ id: 0n, broadcast: Path.from("live/cam"), track: "video", priority: 0 }); + const serving = publisher.runSubscribe(msg, server); + + // The subscription is parked resolving the broadcast when the grant shrinks. + const request = await requests.next(); + if (request.done) throw new Error("the handler never saw the request"); + grant.set(grantOf("other")); + await Promise.resolve(); + request.value.accept(produced); + await serving; + + expect(unauthorizedResets(resets)).toContain("unauthorized: live/cam"); + } finally { + resets.mockRestore(); + produced.close(); + handle.close(); + publisher.close(); + client.close(); + origin.close(); + } +}); + +// A fetch holds its grant watch until the last frame: a group can stay open as long as its +// track, so a check at accept alone would keep serving after a shrink. +test("lite draft-06: a grant that shrinks mid-fetch resets the fetch", async () => { + const pair = createMockTransportPair(ALPN_06); + const origin = new OriginProducer(); + const grant = new Signal(grantOf("live")); + const publisher = new Publisher(pair.server, Version.DRAFT_06, randomHop(), origin.consume(), grant); + + const broadcast = publish(origin, Path.from("live")); + const track = broadcast.createTrack("video"); + // Left open, so the fetch is still serving when the grant shrinks. + const group = new GroupProducer(0); + group.writeString("first"); + track.writeGroup(group); + + const client = await Stream.open(pair.client); + const server = await Stream.accept(pair.server); + if (!server) throw new Error("publisher never accepted the fetch stream"); + + const resets = spyOn(Writer.prototype, "reset"); + try { + const msg = new Fetch({ broadcast: Path.from("live"), track: "video", priority: 0, group: 0 }); + const serving = publisher.runFetch(msg, server); + // The first frame is on the wire before the grant shrinks. + await client.reader.u8(); + + grant.set(grantOf("other")); + await serving; + + expect(unauthorizedResets(resets)).toContain("unauthorized: live"); + } finally { + resets.mockRestore(); + group.close(); + publisher.close(); + client.close(); + broadcast.close(); + origin.close(); + } +}); diff --git a/js/net/src/lite/publisher.ts b/js/net/src/lite/publisher.ts index 39d20c108a..f8c379424e 100644 --- a/js/net/src/lite/publisher.ts +++ b/js/net/src/lite/publisher.ts @@ -574,10 +574,46 @@ export class Publisher { * @internal */ async runSubscribe(msg: Subscribe, stream: Stream) { - // Serve only what our grant lets us publish, and stop once it no longer does. + // Serve only what our grant lets us publish, and stop once it no longer does. The + // watch is armed before the first check and held to the end, so a shrink while the + // broadcast resolves is never missed. + const revoked = unauthorized(msg.broadcast); + let serving: track.Subscriber | undefined; + const watch = this.#watch(msg.broadcast, () => { + console.debug(`publish revoked: broadcast=${msg.broadcast} track=${msg.track}`); + serving?.close(revoked); + stream.abort(revoked); + }); + try { + await this.#serveSubscribe(msg, stream, watch, (track) => { + serving = track; + }); + } finally { + watch.dispose(); + } + } + + // Watch whether our grant still lets us publish `broadcast`, calling `onRevoke` once + // it does not. Arm it before the request's first check. + #watch(broadcast: Path.Valid, onRevoke: () => void): { revoked: () => boolean; dispose: () => void } { + let revoked = false; + const dispose = + this.#grant?.subscribe(() => { + if (revoked || !this.#denied(broadcast)) return; + revoked = true; + onRevoke(); + }) ?? (() => {}); + return { revoked: () => revoked || this.#denied(broadcast), dispose }; + } + + async #serveSubscribe( + msg: Subscribe, + stream: Stream, + watch: { revoked: () => boolean }, + serving: (track: track.Subscriber) => void, + ) { // Checked before resolving, so a denied request never reaches the origin. - const denied = () => this.#denied(msg.broadcast); - if (denied()) { + if (watch.revoked()) { stream.writer.reset(unauthorized(msg.broadcast)); return; } @@ -591,6 +627,8 @@ export class Publisher { stream.writer.reset(error(err)); return; } + // Revoked while the broadcast resolved: the watch already reset the stream. + if (watch.revoked()) return; if (!front) { console.debug(`publish unknown: broadcast=${msg.broadcast}`); stream.writer.reset(new NotFound(`broadcast ${msg.broadcast}`)); @@ -606,6 +644,7 @@ export class Publisher { end: endGroup === undefined ? undefined : { excluded: endGroup }, }, }); + serving(track); positionCursor(track, this.version, msg.startGroup); hooks.replaceGroups(track, { end: endGroup === undefined ? undefined : { excluded: endGroup } }); @@ -615,14 +654,6 @@ export class Publisher { let datagrams = Promise.resolve(); let controls: SubscriptionControls | undefined; - const revoked = unauthorized(msg.broadcast); - const disposeGrant = this.#grant?.subscribe(() => { - if (!denied()) return; - console.debug(`publish revoked: broadcast=${msg.broadcast} track=${track.name}`); - track.close(revoked); - stream.abort(revoked); - }); - try { let timescale: Timescale = Timescale.MILLI; @@ -696,8 +727,6 @@ export class Publisher { track.close(e); stream.abort(e); await Promise.all([datagrams, controls?.decoding]); - } finally { - disposeGrant?.(); } } @@ -711,7 +740,31 @@ export class Publisher { stream.writer.reset(new Error("fetch requires moq-lite-05 or newer")); return; } - if (this.#denied(msg.broadcast)) { + // Like a subscription, the fetch holds its watch until the last frame: a group can stay + // open as long as its track, so a check at accept alone would keep serving after a shrink. + const revoked = unauthorized(msg.broadcast); + let fetched: group.Consumer | undefined; + const watch = this.#watch(msg.broadcast, () => { + console.debug(`fetch revoked: broadcast=${msg.broadcast} track=${msg.track} group=${msg.group}`); + fetched?.close(revoked); + stream.abort(revoked); + }); + try { + await this.#serveFetch(msg, stream, watch, (group) => { + fetched = group; + }); + } finally { + watch.dispose(); + } + } + + async #serveFetch( + msg: Fetch, + stream: Stream, + watch: { revoked: () => boolean }, + serving: (group: group.Consumer) => void, + ) { + if (watch.revoked()) { stream.writer.reset(unauthorized(msg.broadcast)); return; } @@ -730,6 +783,8 @@ export class Publisher { stream.writer.reset(new NotFound(`broadcast ${msg.broadcast}`)); return; } + // Revoked while the broadcast resolved: the watch already reset the stream. + if (watch.revoked()) return; // The subscriber opened this stream, so its send order only ranked the request. Rank the // response here, on the same scale as the group streams it competes with. @@ -741,6 +796,12 @@ export class Publisher { // come off the same front, so the metadata and the frames are one generation. const info = await this.#resolveTrackInfo(front, msg.track); group = await wireOf(front).fetchGroup(msg.track, msg.group, { priority: msg.priority }); + serving(group); + // Revoked while the group resolved: the watch already reset the stream. + if (watch.revoked()) { + group.close(unauthorized(msg.broadcast)); + return; + } await this.#runFetchGroup(group, stream.writer, { timescale: Timescale(info.timescale), start: msg.startFrame, diff --git a/js/net/src/lite/subscriber.test.ts b/js/net/src/lite/subscriber.test.ts index f9dec8b901..53499109c0 100644 --- a/js/net/src/lite/subscriber.test.ts +++ b/js/net/src/lite/subscriber.test.ts @@ -1,5 +1,6 @@ import { expect, spyOn, test } from "bun:test"; import { Signal } from "@moq/signals"; +import type { Grant } from "../auth.ts"; import type { Probe as ProbeStats } from "../connection/stats.ts"; import { error, reason, StreamCode, StreamError } from "../error.ts"; import { HopSchema, isAnonymous, MAX_HOPS, Route, UNKNOWN_HOP } from "../hop.ts"; @@ -803,3 +804,33 @@ test("a fetch started after the subscriber closes rejects without opening a stre expectCut(err, undefined); expect(streams.length).toBe(0); }); + +// The grant watch is armed before the first check, so a shrink while TRACK_INFO is still in +// flight refuses the subscription rather than opening a SUBSCRIBE the grant no longer covers. +test("a grant that shrinks while the subscription sets up refuses it", async () => { + const { quic, streams } = fakeSession(); + const scoped = (prefix: string): Grant => ({ + publish: new Path.Patterns([]), + subscribe: new Path.Patterns([Path.Pattern.subtree(prefix)]), + }); + const grant = new Signal(scoped("room")); + const subscriber = new Subscriber(quic, Version.DRAFT_05, HopSchema.parse(1n), undefined, undefined, grant); + + const track = subscriber.consume(Path.from("room/cam")).track("video").subscribe().ordered(); + const next = track.nextGroup().catch((err: unknown) => err); + + // Parked on TRACK_INFO when the grant shrinks. + await drainUntil(() => streams.length === 1); + await streams[0].reading; + grant.set(scoped("other")); + await answerTrackInfo(streams[0]); + + const err = await next; + expect(err).toBeInstanceOf(StreamError); + expect((err as StreamError).code).toBe(StreamCode.Unauthorized); + // Nothing past the TRACK stream reached the wire. + expect(streams.length).toBe(1); + + track.close(); + subscriber.close(); +}); diff --git a/js/net/src/lite/subscriber.ts b/js/net/src/lite/subscriber.ts index b395d16491..860c4aa6d0 100644 --- a/js/net/src/lite/subscriber.ts +++ b/js/net/src/lite/subscriber.ts @@ -80,6 +80,15 @@ function supportsTrackStream(version: Version): boolean { } } +// What a subscription has set up so far, so a revocation (or a timeout) can reach it at +// any stage. +interface SubscribeSetup { + stream?: Stream; + producer?: track.Producer; + // Set once our grant stops covering the broadcast. + revoked?: Error; +} + interface SubscribeEntry { // The write side: incoming GROUP streams are routed here. The application reads // the matching track.Subscriber it got from broadcast.Consumer.subscribe. @@ -533,10 +542,29 @@ export class Subscriber { return; } const refused = unauthorized(broadcast); - if (this.#denied(broadcast)) { - request.reject(refused); - return; + // Armed before the first check and held to the end, so a shrink while the + // subscription sets up is never missed: it resets whatever reached the wire. + const state: SubscribeSetup = {}; + const disposeGrant = this.#grant?.subscribe(() => { + if (state.revoked || !this.#denied(broadcast)) return; + state.revoked = refused; + console.debug(`subscribe revoked: id=${id} broadcast=${broadcast} track=${request.name}`); + state.producer?.close(refused); + state.stream?.abort(refused); + }); + try { + if (this.#denied(broadcast)) { + request.reject(refused); + return; + } + await this.#serveSubscribe(id, broadcast, request, state); + } finally { + disposeGrant?.(); } + } + + async #serveSubscribe(id: bigint, broadcast: Path.Valid, request: track.Request, state: SubscribeSetup) { + const subscription = request.subscription; // `timescale` stays undefined until TRACK_INFO (or, on older drafts, // implicit defaults) resolves it; runGroup blocks on it before decoding. @@ -557,7 +585,6 @@ export class Subscriber { // Open the stream under a timeout. The stream handle flows back via `state` // so the timeout path can abort it if it finishes opening after the deadline. - const state: { stream?: Stream } = {}; const setup = this.#openSubscribe(state, msg, request, id, timescale); let opened: { stream: Stream; entry: SubscribeEntry }; @@ -570,8 +597,10 @@ export class Subscriber { console.debug(`subscribe ok: id=${id} broadcast=${broadcast} track=${request.name}`); } catch (err) { // The setup outlived its deadline waiting for the first response: a control - // timeout, not content that arrived late. - const e = err instanceof TimeoutError ? controlTimeout(err) : await sessionCause(this.#quic, err); + // timeout, not content that arrived late. A revocation says so instead. + const e = + state.revoked ?? + (err instanceof TimeoutError ? controlTimeout(err) : await sessionCause(this.#quic, err)); request.reject(e); this.#subscribes.delete(id); console.warn(`subscribe error: id=${id} broadcast=${broadcast} track=${request.name} error=${reason(e)}`); @@ -587,13 +616,6 @@ export class Subscriber { const { stream, entry } = opened; const producer = entry.track; - // Losing the grant ends the subscription, leaving the session alone. - const disposeGrant = this.#grant?.subscribe(() => { - if (!this.#denied(broadcast)) return; - console.debug(`subscribe revoked: id=${id} broadcast=${broadcast} track=${request.name}`); - producer.close(refused); - stream.abort(refused); - }); try { // Watch for subscription changes and send SUBSCRIBE_UPDATE. Lite01/Lite02 // don't carry SUBSCRIBE_UPDATE on the wire, so skip the watcher there @@ -641,7 +663,6 @@ export class Subscriber { console.warn(`subscribe error: id=${id} broadcast=${broadcast} track=${request.name} error=${reason(e)}`); stream.abort(e); } finally { - disposeGrant?.(); this.#subscribes.delete(id); } } @@ -656,7 +677,7 @@ export class Subscriber { // SUBSCRIBE is accepted implicitly (no SUBSCRIBE_OK). Older drafts carry no // per-track properties, so they resolve to defaults and just drain SUBSCRIBE_OK. async #openSubscribe( - state: { stream?: Stream }, + state: SubscribeSetup, msg: Subscribe, request: track.Request, id: bigint, @@ -668,6 +689,7 @@ export class Subscriber { if (supportsTrackStream(this.version)) { // Fetch the immutable properties once via the TRACK stream. const info = await this.#trackInfo(msg.broadcast, msg.track); + if (state.revoked) throw state.revoked; producer = request.accept(this.#toModelInfo(info)); timescale.set(info.timescale); } else { @@ -693,8 +715,14 @@ export class Subscriber { requested: msg.startGroup, }; this.#subscribes.set(id, entry); + state.producer = producer; state.stream = await Stream.open(this.#quic); + // Revoked while the stream opened: nothing reached the wire yet, so stop here. + if (state.revoked) { + state.stream.abort(state.revoked); + throw state.revoked; + } await state.stream.writer.u53(StreamId.Subscribe); await msg.encode(state.stream.writer, this.version); diff --git a/quest/m1/auth/README.md b/quest/m1/auth/README.md index f8e909b7d1..02e427c048 100644 --- a/quest/m1/auth/README.md +++ b/quest/m1/auth/README.md @@ -39,9 +39,8 @@ Decisions settled while planning, recorded so review does not relitigate them: the union and cancels publications and subscriptions that lose authorization. Other authorized work continues on the same session. An empty union leaves the session connected with no access, so it can accept a fresh token. - [Origin narrowing](/quest/m1/auth/narrowing.md) owns the common resize - operation; relay token handling requires it rather than shipping a temporary - close-on-shrink policy. + `auth::Handle::authorize` is the common resize operation; relay token handling + uses it rather than shipping a temporary close-on-shrink policy. - **A public grant contains publish patterns, subscribe patterns, and an expiry**, in the presenter's own root; the presenter never sees the relay-side root, and every token in a union shares the connection's root. Unscoped @@ -92,9 +91,6 @@ existing lite-06 ALPN. - [AUTH endings](/quest/m1/auth/error-codes.md) - an out-of-range AUTH_ERROR code is refused, and both sides settle and recompute grants when a stream ends - [AUTH violations](/quest/m1/auth/violations.md) - every AUTH protocol violation closes the session in Rust and JS, lite and IETF - [JS fetch grant watch](/quest/m1/auth/js-fetch-watch.md) - a JS `fetchGroup` ends `Unauthorized` when its path leaves the grant -- [Origin narrowing](/quest/m1/auth/narrowing.md) - a live grant narrows in - place: subscriptions outside it reset, publishes outside it abort, and relay - revalidation stops closing the session - [Relay tokens](/quest/m1/auth/relay-refresh.md) - the relay verifies tokens sent in band, unions their grants, and cancels only work that loses access - [Request tokens](/quest/m1/auth/request-token.md) - an `AUTHORIZATION diff --git a/quest/m1/auth/error-codes.md b/quest/m1/auth/error-codes.md index 71881c1f51..b1bd9e518d 100644 --- a/quest/m1/auth/error-codes.md +++ b/quest/m1/auth/error-codes.md @@ -30,8 +30,4 @@ are closed, so every way an AUTH stream ends reports what actually happened: it. - One regression test per item, each failing without its fix. -The fifth deferred item, a grant recheck after async origin resolution, -belongs to [Origin narrowing](/quest/m1/auth/narrowing.md) with the other -watcher races. - Public API: none. Wire: none. diff --git a/quest/m1/auth/js-fetch-watch.md b/quest/m1/auth/js-fetch-watch.md index 4362cbd6ad..68015c86b5 100644 --- a/quest/m1/auth/js-fetch-watch.md +++ b/quest/m1/auth/js-fetch-watch.md @@ -8,7 +8,7 @@ session's grant ends with `Unauthorized` instead of finishing. ## Plan -Found by [Origin narrowing](/quest/m1/auth/narrowing.md) (#4560), which gave +Found by #4560, which gave every other JS request a watch armed before its first await. Follow the same pattern, and add a regression test where the grant changes during setup and mid-fetch. diff --git a/quest/m1/auth/narrowing.md b/quest/m1/auth/narrowing.md deleted file mode 100644 index 2771cec520..0000000000 --- a/quest/m1/auth/narrowing.md +++ /dev/null @@ -1,65 +0,0 @@ -# [M] A live origin grant narrows in place - -## Goal - -A `moq-net` session's grant can be replaced by a narrower pattern union -while the session is live: announce cursors retract the patterns that fell -outside, resolution refuses them, subscriptions under them reset with -`UNAUTHORIZED`, and broadcasts the session published under them abort. The -relay uses it on revalidation so a moderation decision (a deafened user's -audio path) takes effect on the session it targets, which a forked client -cannot bypass. This is the per-subscriber exclusion -[#2714](https://github.com/moq-dev/moq/issues/2714) asked for; the scope and -match half already landed in #3672 and #3746. - -A narrowing always succeeds. A changed root still closes the session. - -## Plan - -- Enforce per session, not per handout. The line's session gate for received - grants (`auth::Gate`, the lite publisher resetting subscriptions it denies, - `regrant` retracting announces no longer covered, and the IETF equivalent) - is the same mechanism pointed at the relay's own grant. Reuse it rather than - building a second one. Readers that bypass the session publisher (HTTP - `/fetch` holds its own lease and closes on it; moq-hls and WHEP enforce no - per-user grant today) are out of scope. -- #3971 built narrowing alone and was closed: it refused whenever anything - live sat in the removed part, which is exactly when a grant narrows, and it - kept the ceiling in a side map. Scopes today are copied by value into every - handle and cursor (`OriginScope` in `rs/moq-net/src/model/origin.rs`), so - narrowing needs shared state. If handles need a tree to share it, keep the - ceiling on the grant node itself (an `Arc` shared by clones and children). -- Close the revocation races Codex found on - [#4179](https://github.com/moq-dev/moq/pull/4179), which the same watcher - mechanism owns: an in-flight lite FETCH keeps serving after its path is - revoked (Rust and JS check only at accept, and Rust's dropped fetches reset - `CANCELLED`, not `UNAUTHORIZED`); a Rust lite subscription still in - `Establish` when the grant shrinks is dropped with `CANCELLED` instead of - aborted `UNAUTHORIZED`; and the JS lite publisher and subscriber arm their - grant watchers only after an await (`demand()`, `#openSubscribe`), and - `Getter.subscribe` does not replay, so a shrink during setup is missed for - good. Every request holds one watcher from its first check to its end, and - rechecks when the watcher is armed. -- Publish side: routes and broadcasts the session published outside the - narrowed grant abort, so consumers see `Unauthorized` just as the subscribe - side does. Draining is not an option: a group may stay open as long as its - track. -- Relay revalidation uses it: a re-checked grant with the same root narrows - the session instead of closing it (`Lease::ended` reporting "grant - narrowed" in `rs/moq-relay/src/auth.rs`), on every accept path: native, - io_uring, and `moq --listen` share `connection::supervise`, while WebSocket - has its own copy of the loop. -- Prove the deafen case end to end: subscribe under a room prefix, narrow - with a grant that excludes that audio path, and assert the subscription - resets with `UNAUTHORIZED` and no further objects arrive, while a sibling - path under the same prefix keeps flowing. -- Benchmark the narrowing and per-session gate cost in - `rs/moq-net/benches/origin.rs`, swept over routes and subscribers, with a - session that never narrows as the baseline. - -Public API: additive on moq-net and on moq-relay's revalidate path. Wire: -none; narrowed subscriptions reset with the existing UNAUTHORIZED code. - -## Closes - -- [#2714](https://github.com/moq-dev/moq/issues/2714) - close this issue when the quest finishes diff --git a/quest/m1/auth/relay-refresh.md b/quest/m1/auth/relay-refresh.md index 4d03299fb2..8176895768 100644 --- a/quest/m1/auth/relay-refresh.md +++ b/quest/m1/auth/relay-refresh.md @@ -30,27 +30,28 @@ expiry, an expiry that leaves the union intact ends only that token, and sessions reject a non-empty token as `Unsupported`. - Union: the connection holds the set of accepted leases. Every one must carry the admitted root, else `AUTH_ERROR { Unauthorized }` naming the - root. The session's origin handles are rebuilt through `Cluster::publisher` - and `Cluster::subscriber` from the union of the set's publish and subscribe - prefixes, intersected with the role the client declared at SETUP so a + root. The peer is authorized for the union of the set's publish and subscribe + patterns, intersected with the role the client declared at SETUP so a publish-only session never starts receiving announcements because a later - token happened to carry subscribe prefixes, and swapped in whenever the set - grows; keep it behind one function, and a shrinking union goes through - [Origin narrowing](/quest/m1/auth/narrowing.md) rather than a close. + token happened to carry subscribe prefixes, re-authorized through + `auth::Handle::authorize` whenever the union changes, grown or shrunk; keep it + behind one function. A grant never reaches past the session's origin handles, + so scope those to the admitted root and role rather than to the first + token's patterns, and nothing is rebuilt or swapped in. - Expiry: the deadline today is the admitted lease's `closed()` inside one `tokio::select!` arm. Select on one lease per token in the set instead, and on any firing recompute the union without it: unchanged means `AUTH_ERROR { Expired }` on that token's stream and the session - continues, shrunk narrows the origin and resets each stream that lost - access with `UNAUTHORIZED`. A token + continues, shrunk re-authorizes the smaller union, which resets each stream that + lost access with `UNAUTHORIZED`. A token withdrawn by the client (its stream closed or reset) recomputes the union the same way with nothing written back, since the stream is gone: unchanged - means the session continues, shrunk narrows the same way. Each lease revalidates on + means the session continues, shrunk re-authorizes the same way. Each lease revalidates on its own cadence, and every re-check that changes the grant, a lower `expires` or a different `publish` or `subscribe`, writes the complete replacement grant as an update AUTH_OK on that token's stream and recomputes the union from the latest grant of every token, so the client can present a replacement in time and a narrowed token never keeps its old - scope; a shrunk union narrows as above. + scope; a changed union re-authorizes as above. - Refusals map as connect-time ones do and never expose a status: every refusal `Client::attach` returns for a new token, a `403`, an empty or invalid grant, an unparseable body, or a `5xx`, is @@ -72,14 +73,10 @@ expiry, an expiry that leaves the union intact ends only that token, and - Tests: a second token with a later `exp` and the same scope keeps the session past the first token's expiry, with only the first stream ending; an expired or invalid token is refused and nothing changes; a token with a - different root is refused naming it; a token adding a prefix rebuilds the - origin scope and the prefix is now solicited; withdrawing the only token + different root is refused naming it; a token adding a prefix widens the + grant and the prefix is now solicited; withdrawing the only token covering a prefix resets that prefix's streams with `UNAUTHORIZED` and the session stays connected; an mTLS session refuses a token; a proxy-mode outage on re-check keeps the token. Additive. - -## Required - -- [Origin narrowing](/quest/m1/auth/narrowing.md) - the live re-scope a shrinking token union needs, so no temporary close-on-shrink policy ships diff --git a/quest/m1/auth/request-token.md b/quest/m1/auth/request-token.md index fa93750186..a22f24e113 100644 --- a/quest/m1/auth/request-token.md +++ b/quest/m1/auth/request-token.md @@ -40,7 +40,7 @@ on the unknown key, and the legacy drafts silently ignore it. until it lapses. When a request's grant expires or is revoked, that request alone ends with `EXPIRED_AUTH_TOKEN` or `UNAUTHORIZED`. A session grant that shrinks cancels the requests it covered, as for any request, through - [Origin narrowing](/quest/m1/auth/narrowing.md). + `auth::Handle::authorize`. - Relay: each such request gets its own lease from a per-request call on `moq_auth::Client`, not the `Client::attach` that [Relay tokens](/quest/m1/auth/relay-refresh.md) builds. `attach` connects with the diff --git a/rs/moq-net/benches/origin.rs b/rs/moq-net/benches/origin.rs index 62692ebd9d..b78bfe0813 100644 --- a/rs/moq-net/benches/origin.rs +++ b/rs/moq-net/benches/origin.rs @@ -8,13 +8,22 @@ //! down to its prefix and beneath it, and each recomputes its best route from //! the entries at that prefix alone. //! +//! `origin/narrow*` put a session in front of the origin, since a narrowing is +//! enforced by the session serving it: every subscription holds a gate on the +//! session's grant, and a narrowing wakes each one. The baseline is a session +//! that never narrows. +//! //! Run with `cargo bench -p moq-net --bench origin`. +#[path = "../tests/support/mod.rs"] +mod support; + use std::time::Duration; use criterion::{BenchmarkId, Criterion, Throughput, criterion_group, criterion_main}; use futures::FutureExt; -use moq_net::{Hop, Hops, Pattern, Patterns, Timestamp, announce, broadcast, kio, origin}; +use moq_net::{Hop, Hops, Pattern, Patterns, Timestamp, announce, auth, broadcast, kio, origin, track}; +use support::harness::{MockConnectOptions, MockPair, connect_mock}; /// `(publishers, subscribers)` shapes for the fan-out benchmarks. const SHAPES: [(usize, usize); 3] = [(100, 10), (1_000, 100), (1_000, 1_000)]; @@ -423,6 +432,152 @@ fn bench_handoff(c: &mut Criterion) { group.finish(); } +/// `(routes, subscribers)` shapes for the narrowing benchmarks: [`SHAPES`] plus a +/// row that grows the routes alone, so each axis shows its own slope. +const NARROW_SHAPES: [(usize, usize); 4] = [(100, 10), (1_000, 10), (1_000, 100), (1_000, 1_000)]; + +/// A relay serving `routes` broadcasts to one client session subscribed to +/// `subscribers` of them. The first subscription is the one a narrowing to +/// `room/**` ends: its broadcast sits at `muted` instead. +struct Watching { + pair: MockPair, + tracks: Vec, + subscriptions: Vec, + _broadcasts: Vec, + _origins: [origin::Producer; 2], +} + +fn watched_path(index: usize) -> String { + match index { + 0 => "muted".to_string(), + index => format!("room/{index}"), + } +} + +async fn watching(routes: usize, subscribers: usize) -> Watching { + let spawn = |hop| { + let (producer, driver) = origin::Producer::new(origin::Config::new(Hop::new(hop).unwrap())); + tokio::spawn(support::harness::run(driver)); + producer + }; + let relay = spawn(1); + let received = spawn(2); + + let mut broadcasts = Vec::new(); + let mut tracks = Vec::new(); + for index in 0..routes { + let broadcast = relay.publish(watched_path(index), origin::Route::default()).unwrap(); + tracks.push(broadcast.create_track("video", None).unwrap()); + broadcasts.push(broadcast); + } + + let mut options = MockConnectOptions::new("moq-lite-06".parse().unwrap()); + options.server_publish = Some(relay.consume()); + options.client_subscribe = Some(received.clone()); + let pair = connect_mock(options).await; + + let mut subscriptions = Vec::new(); + for index in 0..subscribers { + let remote = received.consume().routed_broadcast(watched_path(index)).await.unwrap(); + let subscription = remote.track("video").unwrap().subscribe(None).await.unwrap(); + subscriptions.push(subscription); + } + // One round, so every subscription is live end to end before anything is timed. + let mut watching = Watching { + pair, + tracks, + subscriptions, + _broadcasts: broadcasts, + _origins: [relay, received], + }; + watching.round().await; + watching +} + +impl Watching { + /// Every subscribed track writes a group, and every subscription reads it. + async fn round(&mut self) { + for track in &self.tracks[..self.subscriptions.len()] { + let mut group = track.append_group().unwrap(); + group.write_frame(Timestamp::ZERO, b"frame".as_ref()).unwrap(); + group.finish().unwrap(); + } + for subscription in &mut self.subscriptions { + subscription.recv_group().await.unwrap().expect("track ended"); + } + } +} + +fn runtime() -> tokio::runtime::Runtime { + tokio::runtime::Builder::new_current_thread() + .enable_time() + .build() + .unwrap() +} + +/// Everything under `room/`, which leaves out only the `muted` subscription. +fn room() -> auth::Grant { + auth::Grant { + subscribe: Pattern::subtree("room").unwrap().into(), + ..auth::Grant::all() + } +} + +/// Steady-state delivery through a session's gates: one round across every +/// subscription, on a session that never narrows and on one narrowed to a grant +/// that still covers them all. Neither may depend on `routes`. +fn bench_narrow_delivery(c: &mut Criterion) { + let mut group = c.benchmark_group("origin/narrow_delivery"); + let rt = runtime(); + for (routes, subscribers) in NARROW_SHAPES { + group.throughput(Throughput::Elements(subscribers as u64)); + for narrowed in [false, true] { + let label = if narrowed { "narrowed" } else { "never" }; + let id = BenchmarkId::new(label, format!("{routes}r_{subscribers}s")); + group.bench_function(id, |b| { + let mut watching = rt.block_on(watching(routes, subscribers)); + if narrowed { + // Covers every subscription but the muted one, which is not subscribed here. + let mut grant = room(); + grant.subscribe.insert(Pattern::literal("muted").unwrap()); + watching.pair.server.auth().authorize(&grant); + } + b.iter(|| rt.block_on(watching.round())); + }); + } + } + group.finish(); +} + +/// One narrowing on a live session: from `narrow` until the subscription it +/// excludes resets. It wakes every subscription's gate and re-checks every route +/// the session announced, so it may grow with the session's own `subscribers` +/// and `routes`, never with anything else on the origin. +fn bench_narrow(c: &mut Criterion) { + let mut group = c.benchmark_group("origin/narrow"); + group.sample_size(10); + group.sampling_mode(criterion::SamplingMode::Flat); + let rt = runtime(); + for (routes, subscribers) in NARROW_SHAPES { + let id = BenchmarkId::from_parameter(format!("{routes}r_{subscribers}s")); + group.bench_function(id, |b| { + b.iter_custom(|iters| { + let mut elapsed = Duration::ZERO; + for _ in 0..iters { + let mut watching = rt.block_on(watching(routes, subscribers)); + let start = std::time::Instant::now(); + watching.pair.server.auth().authorize(&room()); + let ended = rt.block_on(watching.subscriptions[0].recv_group()); + elapsed += start.elapsed(); + assert!(ended.is_err(), "the muted subscription outlived the narrowing"); + } + elapsed + }); + }); + } + group.finish(); +} + criterion_group!( benches, bench_announce, @@ -433,6 +588,8 @@ criterion_group!( bench_serve_idle, bench_subscribe, bench_request, - bench_handoff + bench_handoff, + bench_narrow_delivery, + bench_narrow ); criterion_main!(benches); diff --git a/rs/moq-net/src/auth.rs b/rs/moq-net/src/auth.rs index 7ea4acf06a..6403a3ac79 100644 --- a/rs/moq-net/src/auth.rs +++ b/rs/moq-net/src/auth.rs @@ -10,7 +10,9 @@ //! The other direction, answering the peer's tokens, is automatic: the session //! grants what its own origin handles allow. An application that verifies tokens //! itself takes [`requests`](Handle::requests) before running the session's -//! driver, and then answers every token the peer presents. +//! driver, and then answers every token the peer presents. Either way, +//! [`authorize`](Handle::authorize) re-authorizes the peer on a live session, narrower +//! or wider, on every version, whether or not it speaks AUTH. //! //! moq-transport draft-17+ carries the same exchange when both sides negotiate the //! MoQ Auth extension. Older versions, and peers that do not negotiate it, carry no @@ -62,22 +64,37 @@ impl Grant { } } - fn patterns(&self, direction: Direction) -> &Patterns { - match direction { - Direction::Publish => &self.publish, - Direction::Subscribe => &self.subscribe, - } - } - /// Fold `other` into this grant: the paths either allows, lapsing at the /// earlier expiry, which is when the union next shrinks. fn union(&mut self, other: &Self) { self.publish.extend(other.publish.iter().cloned()); self.subscribe.extend(other.subscribe.iter().cloned()); - self.expires = match (self.expires, other.expires) { - (Some(a), Some(b)) => Some(a.min(b)), - (a, b) => a.or(b), + self.expires = earliest(self.expires, other.expires); + } + + /// The paths both grants allow, lapsing at the earlier expiry. + /// + /// Fails closed: an intersection too large to hold grants nothing, never more. + fn intersect(&self, other: &Self) -> Self { + let both = |a: &Patterns, b: &Patterns| { + a.intersect(b).unwrap_or_else(|err| { + tracing::warn!(%err, "grant intersection too large; granting nothing"); + Patterns::new() + }) }; + Self { + publish: both(&self.publish, &other.publish), + subscribe: both(&self.subscribe, &other.subscribe), + expires: earliest(self.expires, other.expires), + } + } +} + +/// The earlier of two optional deadlines, where `None` is never. +fn earliest(a: Option, b: Option) -> Option { + match (a, b) { + (Some(a), Some(b)) => Some(a.min(b)), + (a, b) => a.or(b), } } @@ -97,8 +114,12 @@ pub(crate) struct State { union: Option, /// The peer replied to some token, so the union is known even when empty. replied: bool, - /// Bumped whenever the union changes, so the session's per-stream gates can - /// skip re-matching paths on every wakeup. + /// The most this side lets the peer do, in the peer's terms: `None` (only the + /// origin handles bound it) until [`Handle::authorize`] sets it; the grant it was + /// last called with. + limit: Option, + /// Bumped whenever the union or the limit changes, so the session's per-stream + /// gates can skip re-matching paths on every wakeup. epoch: u64, acceptor: Acceptor, } @@ -150,6 +171,71 @@ impl State { fn closed(&self) -> Option { self.closed.clone() } + + /// The union's and the limit's patterns for `direction`, each `None` while + /// unrestricted. + fn parts(&self, direction: Direction) -> (Option<&Patterns>, Option<&Patterns>) { + // The peer's grant names what we may do; the limit names what the peer may, + // so what we send is what the peer may subscribe to, and the other way around. + match direction { + Direction::Publish => ( + self.union.as_ref().map(|union| &union.publish), + self.limit.as_ref().map(|limit| &limit.subscribe), + ), + Direction::Subscribe => ( + self.union.as_ref().map(|union| &union.subscribe), + self.limit.as_ref().map(|limit| &limit.publish), + ), + } + } + + /// What `direction` allows right now. + fn permit(&self, direction: Direction) -> Permit { + let (granted, limit) = self.parts(direction); + Permit { + granted: granted.cloned(), + limit: limit.cloned(), + } + } + + /// Whether `direction` allows `path` right now, without copying the patterns. + fn allows(&self, direction: Direction, path: &str) -> bool { + let (granted, limit) = self.parts(direction); + granted.is_none_or(|granted| granted.matches(path)) && limit.is_none_or(|limit| limit.matches(path)) + } +} + +/// What one direction of a session allows: the grant the peer gave this side (the +/// union of its tokens) and the limit this side set on the peer, each `None` +/// while unrestricted. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub(crate) struct Permit { + granted: Option, + limit: Option, +} + +impl Permit { + /// Whether both allow `path`. + pub(crate) fn matches(&self, path: &str) -> bool { + self.granted.as_ref().is_none_or(|granted| granted.matches(path)) && self.within_limit(path) + } + + /// Whether the limit alone allows `path`: for what the peer offers us, which our + /// own grant does not decide. + pub(crate) fn within_limit(&self, path: &str) -> bool { + self.limit.as_ref().is_none_or(|limit| limit.matches(path)) + } + + /// Whether this allows everything `other` does, judged part by part, so a `false` + /// may be spurious but a `true` never is. + pub(crate) fn covers(&self, other: &Self) -> bool { + let covers = |this: &Option, other: &Option| match (this, other) { + (None, _) => true, + (Some(_), None) => false, + (Some(this), Some(other)) => this.covers(other), + }; + covers(&self.granted, &other.granted) && covers(&self.limit, &other.limit) + } } /// The session's auth handle, returned by [`Session::auth`](crate::Session::auth). @@ -354,26 +440,72 @@ impl Handle { Poll::Ready(()) } + /// Authorize the peer for `grant` on this session, in the session's own paths. + /// Calling it again re-authorizes, narrower or wider, replacing the last grant. The + /// session's origin handles still bound it, so it never grants more than they allow. + /// + /// A narrower grant ends what falls outside at once, with [`Error::Unauthorized`] + /// where it has a reader: announcements to the peer retract, its new requests are + /// refused, its subscriptions reset, and the broadcasts it published abort. A wider + /// one brings back what the old grant held back: announcements to the peer are made + /// again, and so are the peer's announcements it withheld from the origin, and new + /// requests are accepted. The rest of the session carries on. It works on every + /// version, since the session enforces it without the peer's help. + /// + /// When the session answers the peer's connection credential itself, the peer is + /// told the grant, within what the origin handles allow, `expires` included. An application answering + /// tokens through [`requests`](Self::requests) updates its own [`Issued`] grants. + pub fn authorize(&self, grant: &Grant) { + let mut state = self.state.lock(); + if state.limit.as_ref() != Some(grant) { + state.limit = Some(grant.clone()); + state.epoch += 1; + } + } + /// The union once it changed since `epoch` last saw it, advancing `epoch`. It /// only ever changes into `Some`: no answer yet is the initial state. pub(crate) fn poll_union(&self, epoch: &mut u64, waiter: &kio::Waiter) -> Poll> { - let seen = *epoch; - let state = ready_or!(self.state.poll(waiter, |state| match state.epoch != seen { - true => Poll::Ready(()), - false => Poll::Pending, - })); + let state = ready_or!(self.poll_epoch(*epoch, waiter)); *epoch = state.epoch; Poll::Ready(state.union.clone()) } - /// Whether the union allows `path` right now. A union that is still `None` (no - /// answer yet, or a version without AUTH) allows everything. + /// What `direction` allows once it may have changed since `epoch` last saw it, + /// advancing `epoch`. + pub(crate) fn poll_permit(&self, direction: Direction, epoch: &mut u64, waiter: &kio::Waiter) -> Poll { + let state = ready_or!(self.poll_epoch(*epoch, waiter)); + *epoch = state.epoch; + Poll::Ready(state.permit(direction)) + } + + /// The limit once it may have changed since `epoch` last saw it, advancing `epoch`. + pub(crate) fn poll_limit(&self, epoch: &mut u64, waiter: &kio::Waiter) -> Poll> { + let state = ready_or!(self.poll_epoch(*epoch, waiter)); + *epoch = state.epoch; + Poll::Ready(state.limit.clone()) + } + + fn poll_epoch(&self, seen: u64, waiter: &kio::Waiter) -> Poll> { + self.state.poll(waiter, |state| match state.epoch != seen { + true => Poll::Ready(()), + false => Poll::Pending, + }) + } + + /// Whether `direction` allows `path` right now: the union (allowing everything + /// while it is `None`, before an answer or on a version without AUTH) and the + /// limit both. pub(crate) fn allows(&self, direction: Direction, path: &str) -> bool { + self.state.read().allows(direction, path) + } + + /// Whether the limit alone allows `direction` at `path`: for a route the peer + /// offers us, which our own grant does not decide. + pub(crate) fn within_limit(&self, direction: Direction, path: &str) -> bool { let state = self.state.read(); - state - .union - .as_ref() - .is_none_or(|union| union.patterns(direction).matches(path)) + let (_, limit) = state.parts(direction); + limit.is_none_or(|limit| limit.matches(path)) } /// The peer turned out not to negotiate AUTH: fail every token as unsupported and @@ -707,18 +839,21 @@ impl Drop for Issued { } } -/// Which half of a grant a [`Gate`] checks. +/// Which way media flows, from this side's view. #[derive(Clone, Copy)] pub(crate) enum Direction { + /// This side sends to the peer: our grant's `publish`, and the limit's `subscribe`. Publish, + /// The peer sends to this side: our grant's `subscribe`, and the limit's `publish`. Subscribe, } -/// Watches whether the union still allows one path, for a long-lived stream that -/// must end once it does not. +/// Watches whether the session still allows one path, for a request that must end +/// once it does not. /// -/// Cheap to poll on every wakeup: the path is only re-matched when the union -/// changes. +/// Create it at the request's first check and hold it to the end, so a change +/// landing in between is never missed. Cheap to poll on every wakeup: the path is +/// only re-matched when the union or the limit changes. pub(crate) struct Gate { handle: Handle, path: crate::PathOwned, @@ -736,20 +871,59 @@ impl Gate { } } - /// Resolve once the union no longer allows the path. A union that is still - /// `None` (no answer yet, or a version without AUTH) allows everything. + /// Resolve once the session no longer allows the path. A union that is still + /// `None` (no answer yet, or a version without AUTH) and a session never limited + /// allow everything. pub(crate) fn poll_denied(&mut self, waiter: &kio::Waiter) -> Poll<()> { loop { - let Some(union) = ready_or!(self.handle.poll_union(&mut self.epoch, waiter)) else { - continue; - }; - if !union.patterns(self.direction).matches(self.path.as_str()) { + let permit = ready_or!(self.handle.poll_permit(self.direction, &mut self.epoch, waiter)); + if !permit.matches(self.path.as_str()) { return Poll::Ready(()); } } } } +/// The default acceptor's answer to the peer's connection credential: the grant +/// the session's origin handles allow, within the limit as it changes. +pub(crate) struct DefaultGrant { + handle: Handle, + base: Grant, + epoch: u64, + limit: Option, + /// The grant last handed out, so an unrelated wakeup sends nothing. + sent: Option, +} + +impl DefaultGrant { + pub(crate) fn new(handle: Handle, base: Grant) -> Self { + let limit = handle.state.read().limit.clone(); + Self { + handle, + base, + epoch: 0, + limit, + sent: None, + } + } + + /// The grant to send next: the first, then each one a new limit changes. + pub(crate) fn poll(&mut self, waiter: &kio::Waiter) -> Poll { + while let Poll::Ready(limit) = self.handle.poll_limit(&mut self.epoch, waiter) { + self.limit = limit; + } + let grant = match &self.limit { + Some(limit) => self.base.intersect(limit), + None => self.base.clone(), + }; + if self.sent.as_ref() == Some(&grant) { + return Poll::Pending; + } + self.sent = Some(grant.clone()); + Poll::Ready(grant) + } +} + /// The close reason naming a broadcast published outside the grant. [`Error`] carries /// no payload, so the path travels here, in the session's own terms. pub(crate) fn unauthorized_reason(path: &crate::Path) -> String { @@ -814,3 +988,85 @@ impl Enforce { } } } + +#[cfg(test)] +mod tests { + use super::*; + + fn patterns(prefixes: &[&str]) -> Patterns { + prefixes + .iter() + .map(|prefix| crate::Pattern::subtree(prefix).unwrap()) + .collect() + } + + fn grant(publish: &[&str], subscribe: &[&str]) -> Grant { + Grant { + publish: patterns(publish), + subscribe: patterns(subscribe), + expires: None, + } + } + + /// A limit replaces the last one, narrower or wider, and applies to a version + /// without AUTH too. + #[test] + fn a_limit_replaces_the_last() { + let handle = Handle::new(false); + assert!(handle.allows(Direction::Publish, "room/alice/audio")); + + handle.authorize(&grant(&["room/bob"], &["room/alice"])); + // What we send is what the peer may subscribe to, and the other way around. + assert!(handle.allows(Direction::Publish, "room/alice/audio")); + assert!(!handle.allows(Direction::Publish, "room/bob/cam")); + assert!(handle.allows(Direction::Subscribe, "room/bob/cam")); + assert!(!handle.allows(Direction::Subscribe, "room/alice/audio")); + + handle.authorize(&grant(&[""], &["room/alice/video"])); + assert!(!handle.allows(Direction::Publish, "room/alice/audio")); + assert!(handle.allows(Direction::Publish, "room/alice/video")); + assert!(handle.allows(Direction::Subscribe, "other")); + + handle.authorize(&Grant::all()); + assert!(handle.allows(Direction::Publish, "room/alice/audio")); + assert!(handle.allows(Direction::Subscribe, "other")); + } + + /// Our own grant does not decide what the peer may offer us, only the limit does. + #[test] + fn the_limit_alone_decides_what_the_peer_offers() { + let handle = Handle::new(true); + let token = handle.present(Bytes::new(), true).unwrap(); + handle.granted(0, grant(&[], &[])); + assert!(!handle.allows(Direction::Subscribe, "room/bob/cam")); + assert!(handle.within_limit(Direction::Subscribe, "room/bob/cam")); + + handle.authorize(&grant(&["room/alice"], &[])); + assert!(!handle.within_limit(Direction::Subscribe, "room/bob/cam")); + assert!(handle.within_limit(Direction::Subscribe, "room/alice/cam")); + drop(token); + } + + /// The default acceptor answers with its base grant, then with each grant a new + /// limit changes, narrower or wider, and stays quiet on a change that leaves it alone. + #[test] + fn the_default_grant_follows_the_limit() { + let handle = Handle::new(true); + let mut default = DefaultGrant::new(handle.clone(), grant(&["room"], &["room"])); + let waiter = kio::Waiter::noop(); + + assert_eq!(default.poll(&waiter), Poll::Ready(grant(&["room"], &["room"]))); + assert!(default.poll(&waiter).is_pending()); + + handle.authorize(&grant(&["room"], &["room/alice"])); + assert_eq!(default.poll(&waiter), Poll::Ready(grant(&["room"], &["room/alice"]))); + + // Wider again, but never past what the origin handles allow. + handle.authorize(&Grant::all()); + assert_eq!(default.poll(&waiter), Poll::Ready(grant(&["room"], &["room"]))); + + // A limit that leaves the grant as it is: nothing to tell the peer. + handle.authorize(&grant(&[""], &[""])); + assert!(default.poll(&waiter).is_pending()); + } +} diff --git a/rs/moq-net/src/client.rs b/rs/moq-net/src/client.rs index b28df340f4..91cafad15f 100644 --- a/rs/moq-net/src/client.rs +++ b/rs/moq-net/src/client.rs @@ -408,6 +408,8 @@ impl Client { }; let stream = stream.with_version(v); + // Draft 14-16 carry no AUTH, but the session is still limited through this handle. + let auth = crate::auth::Handle::new(false); // Draft 14-16: the path rode in the bidi SETUP above, not the uni one. let (protocol, goaway) = ietf::start(ietf::Config { runtime: runtime.clone(), @@ -423,14 +425,9 @@ impl Client { path: None, peer_setup_stream: None, peer_declared: Some(peer_declared), - auth: crate::auth::Handle::new(false), + auth: auth.clone(), })?; - ( - None, - crate::driver::Protocol::Ietf(protocol), - goaway, - crate::auth::Handle::new(false), - ) + (None, crate::driver::Protocol::Ietf(protocol), goaway, auth) } }; diff --git a/rs/moq-net/src/ietf/auth.rs b/rs/moq-net/src/ietf/auth.rs index 1e15970faa..cf7e591ba7 100644 --- a/rs/moq-net/src/ietf/auth.rs +++ b/rs/moq-net/src/ietf/auth.rs @@ -395,6 +395,7 @@ impl Serve { version: Version, ) { let issue = Issue::shared(); + let mut default = None; match self.handle.acceptor() { Some(requests) => { // A closed queue hands the request back, and dropping it refuses the token. @@ -412,10 +413,11 @@ impl Serve { } return; } - None => issue.lock().outbox.push_back(Reply::Grant(self.peer_grant)), + // Re-sent whenever a new limit changes it. + None => default = Some(crate::auth::DefaultGrant::new(self.handle.clone(), self.peer_grant)), } - let err = match serve_issue(&self.runtime, &issue, &mut stream, version).await { + let err = match serve_issue(&self.runtime, &issue, default.as_mut(), &mut stream, version).await { Ok(()) => Error::Cancel, Err(err) => { match &err { @@ -431,10 +433,12 @@ impl Serve { } } -/// Write the acceptor's replies until either side ends the token. +/// Write the acceptor's replies until either side ends the token. `default` feeds +/// the default acceptor's grant into the outbox as it changes. async fn serve_issue( runtime: &crate::time::Clock, issue: &kio::Shared, + mut default: Option<&mut crate::auth::DefaultGrant>, stream: &mut Stream, version: Version, ) -> Result<(), Error> { @@ -449,6 +453,11 @@ async fn serve_issue( if let Poll::Ready(res) = stream.reader.poll_closed(&mut cx) { return Poll::Ready(Next::Withdrawn(res)); } + if let Some(default) = default.as_deref_mut() + && let Poll::Ready(grant) = default.poll(waiter) + { + issue.lock().outbox.push_back(Reply::Grant(grant)); + } match issue.poll(waiter, |issue| match issue.outbox.is_empty() && !issue.done { true => Poll::Pending, false => Poll::Ready(()), diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 466c0563ca..32aafccdc8 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -201,9 +201,9 @@ struct Namespaces { /// The open PUBLISH_NAMESPACE request carrying each advertised namespace. Empty when /// the entries ride a SUBSCRIBE_NAMESPACE stream inline. requests: HashMap>, - /// What our grant lets us publish (MoQ Auth), `None` while unknown. - permit: Option, - /// The grant union's epoch last applied to `permit`. + /// What we may advertise: our grant (MoQ Auth) and the ceiling on the peer. + permit: crate::auth::Permit, + /// The auth epoch last applied to `permit`. epoch: u64, } @@ -214,14 +214,14 @@ impl Namespaces { target, watched: HashMap::new(), requests: HashMap::new(), - permit: None, + permit: Default::default(), epoch: 0, } } - /// Whether our grant lets us advertise `path`. An unknown grant allows everything. + /// Whether we may advertise `path`. An unknown grant allows everything. fn permitted(&self, path: &crate::Path) -> bool { - self.permit.as_ref().is_none_or(|permit| permit.matches(path.as_str())) + self.permit.matches(path.as_str()) } } @@ -233,8 +233,8 @@ enum NamespaceEvent { Update(Option), /// The retry sleep fired: re-offer whatever the peer should be holding and isn't. Retry, - /// Our grant changed (MoQ Auth): re-check every namespace against it. - Regrant(Option), + /// Our grant (MoQ Auth) or the ceiling changed: re-check every namespace against it. + Regrant(crate::auth::Permit), } #[derive(Clone)] @@ -1944,8 +1944,8 @@ where } // A grant change applies before the next update, so a namespace it no // longer covers is withdrawn rather than re-sent. - if let Poll::Ready(union) = self.auth.poll_union(epoch, waiter) { - return Poll::Ready(NamespaceEvent::Regrant(union)); + if let Poll::Ready(permit) = self.auth.poll_permit(crate::auth::Direction::Publish, epoch, waiter) { + return Poll::Ready(NamespaceEvent::Regrant(permit)); } if let Poll::Ready(update) = announced.poll_next(waiter) { return Poll::Ready(NamespaceEvent::Update(update)); @@ -1979,8 +1979,8 @@ where self.sync_namespace(&mut ns, &suffix, &path).await?; } } - NamespaceEvent::Regrant(union) => { - ns.permit = union.map(|grant| grant.publish); + NamespaceEvent::Regrant(permit) => { + ns.permit = permit; let suffixes: Vec = ns.watched.keys().cloned().collect(); for suffix in suffixes { let path = prefix.join(&suffix); diff --git a/rs/moq-net/src/ietf/session.rs b/rs/moq-net/src/ietf/session.rs index 496f954165..45141d585d 100644 --- a/rs/moq-net/src/ietf/session.rs +++ b/rs/moq-net/src/ietf/session.rs @@ -154,6 +154,7 @@ where let control = Control::new(request_id_max, client); let adapter = ControlStreamAdapter::new(session.clone(), control.clone(), version); + // No AUTH on these drafts, but a limit still reaches both halves. let publisher = Publisher::new( runtime.clone(), adapter.clone(), @@ -162,7 +163,8 @@ where peer_hop, peer_setup.clone(), version, - ); + ) + .with_auth(auth.clone()); let (tasks, mut task_set) = TaskSet::new(); let subscriber = Subscriber::new( runtime.clone(), @@ -176,7 +178,8 @@ where version, tasks.clone(), goaway.going_away.clone(), - ); + ) + .with_auth(auth.clone()); // GOAWAY send task: draft-14-16 carry GOAWAY on the shared control // stream. Parked on the drain trigger; races the transport close so diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index 12df60fb20..24afc208bb 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -379,8 +379,13 @@ struct BroadcastState { route: crate::origin::Route, // The served route: dropping it (and the serve task's clone) retracts the - // route and rejects its queued requests. - dynamic: crate::origin::Dynamic, + // route and rejects its queued requests. `None` while the session's limit holds + // it back, though the peer still advertises it. + dynamic: Option, + + // Bumped each time the route attaches, so a serve task outlived by a limit that + // took the route away and gave it back ends rather than serve beside the new one. + generation: u64, // active number of PUBLISH_NAMESPACE messages. count: usize, @@ -500,12 +505,65 @@ where } } - /// Bound what we subscribe to by the grant this session's tokens earn (MoQ Auth). + /// Bound what we subscribe to by the grant this session's tokens earn (MoQ Auth), + /// and what the peer may publish to us by the session's limit, as either changes. pub fn with_auth(mut self, auth: crate::auth::Handle) -> Self { self.auth = auth; + let this = self.clone(); + self.tasks.push(async move { this.run_limit().await }); self } + /// Follow the session's limit, attaching every withheld namespace a new limit + /// covers. Each route's own serve task holds itself back when a limit no longer + /// covers it. + async fn run_limit(&self) { + let mut epoch = 0; + loop { + let permit = kio::wait(|waiter| { + self.auth + .poll_permit(crate::auth::Direction::Subscribe, &mut epoch, waiter) + }) + .await; + let mut state = self.state.lock(); + let mut attached = Vec::new(); + for (path, entry) in state.broadcasts.iter_mut() { + let allowed = permit.within_limit(path.as_str()); + match &entry.dynamic { + None if allowed => { + let mut route = entry.route.clone(); + if self.going_away.is_set() { + route.cost = crate::origin::Cost::DRAIN; + } + let Ok(dynamic) = self.origin.dynamic(path, route) else { + continue; + }; + tracing::info!(route = %self.origin.absolute(path), "namespace authorized again"); + entry.dynamic = Some(dynamic); + entry.generation += 1; + attached.push((path.clone(), entry.generation)); + } + _ => {} + } + } + drop(state); + for (path, generation) in attached { + self.serve_route(path, generation); + } + } + } + + /// Serve the requests beneath one attached namespace on its own task. + fn serve_route(&self, path: PathOwned, generation: u64) { + let this = self.clone(); + self.tasks.push(async move { + // stop_announce is the authoritative remover: it drops the entry + // (retracting the route) once the announce refcount hits zero, + // which is what makes run_route exit, as does a limit holding it back. + this.run_route(path, generation).await; + }); + } + /// End every active subscription with the error that ended the session. pub fn abort(&self, err: &Error) { self.state.lock().abort(err); @@ -1357,29 +1415,38 @@ where // tracks keep flowing. let entry = entry.into_mut(); entry.route = route.clone(); - entry.dynamic.update(route)?; + if let Some(dynamic) = &entry.dynamic { + dynamic.update(route)?; + } Ok(()) } Entry::Vacant(entry) => { + // Outside the session's limit: held, not refused, so a wider limit can + // attach it while the peer still advertises it. + if !self.auth.within_limit(crate::auth::Direction::Subscribe, path.as_str()) { + tracing::debug!(route = %self.origin.absolute(&path), "withholding announce outside the limit"); + entry.insert(BroadcastState { + route, + dynamic: None, + generation: 0, + count: 1, + sources: HashMap::new(), + }); + return Ok(()); + } // Propagates Error::Unauthorized if the namespace is out of scope. let dynamic = self.origin.dynamic(&path, route.clone())?; entry.insert(BroadcastState { route, - dynamic, + dynamic: Some(dynamic), + generation: 0, count: 1, sources: HashMap::new(), }); tracing::debug!(route = %self.origin.absolute(&path), "announce"); - - let this = self.clone(); - self.tasks.push(async move { - // stop_announce is the authoritative remover: it drops the entry - // (retracting the route) once the announce refcount hits zero, - // which is what makes run_route exit. - this.run_route(path).await; - }); + self.serve_route(path, 0); Ok(()) } @@ -1426,9 +1493,10 @@ where /// per requested path and serve its track requests until the route is /// retracted or the session dies. Tracks in flight at a retraction run to /// their own end. - async fn run_route(&self, path: PathOwned) { + async fn run_route(&self, path: PathOwned, generation: u64) { let mut broadcasts = TaskSet::owned(); let mut closed_session = self.session.clone(); + let mut epoch = 0; loop { let next = broadcasts .drive(|waiter| { @@ -1443,11 +1511,32 @@ where if self.going_away.poll(waiter).is_ready() { self.drain_route(&path); } + // A limit that no longer covers the namespace holds the route back as a + // retraction would, closing its sources. Tracks in flight end on their + // own gates, with `Unauthorized`. + let mut excluded = false; + while let Poll::Ready(permit) = + self.auth + .poll_permit(crate::auth::Direction::Subscribe, &mut epoch, waiter) + { + excluded = !permit.within_limit(path.as_str()); + } // The route lives in the entry: stop_announce removing it retracts // the route, and this loop ends with it. let mut state = self.state.lock(); - match state.broadcasts.get_mut(&path) { - Some(entry) => entry.dynamic.poll_requested_broadcast(waiter).map(Some), + let Some(entry) = state.broadcasts.get_mut(&path) else { + return Poll::Ready(None); + }; + if entry.generation != generation { + return Poll::Ready(None); + } + if excluded && entry.dynamic.is_some() { + tracing::info!(route = %self.origin.absolute(&path), "namespace no longer authorized"); + entry.dynamic = None; + entry.sources.clear(); + } + match &mut entry.dynamic { + Some(dynamic) => dynamic.poll_requested_broadcast(waiter).map(Some), None => Poll::Ready(None), } }) @@ -1510,7 +1599,9 @@ where return; } entry.route.cost = crate::origin::Cost::DRAIN; - let _ = entry.dynamic.update(entry.route.clone()); + if let Some(dynamic) = &entry.dynamic { + let _ = dynamic.update(entry.route.clone()); + } } async fn run_broadcast(&self, path: Path<'_>, mut broadcast: broadcast::Dynamic) -> Result<(), Error> { diff --git a/rs/moq-net/src/lite/publisher.rs b/rs/moq-net/src/lite/publisher.rs index 7b659074ef..706564b19a 100644 --- a/rs/moq-net/src/lite/publisher.rs +++ b/rs/moq-net/src/lite/publisher.rs @@ -115,6 +115,12 @@ fn position_cursor(track: &mut track::Subscriber, version: Version, start_group: } impl Shared { + /// Watches whether the session still lets us serve `broadcast` to the peer: our + /// grant, and the ceiling on what the peer may subscribe to. + fn gate(&self, broadcast: &crate::Path) -> crate::auth::Gate { + crate::auth::Gate::new(self.auth.clone(), broadcast.to_owned(), crate::auth::Direction::Publish) + } + /// The origin to resolve a peer-requested broadcast from: excludes routes /// through the peer, so a subscription is never served data that flowed /// through the subscriber. The identity is the one the peer declared in its @@ -379,6 +385,9 @@ struct AuthServe { /// Shared with the app's [`crate::auth::Request`] / [`crate::auth::Issued`], or /// filled in by the default acceptor. issue: Option>, + /// The default acceptor's grant for the connection's credential, re-sent as a + /// new limit changes it. `None` when the app answers. + default: Option, /// Our side is finished: FIN sent, waiting for the acknowledgement. finished: bool, } @@ -393,6 +402,7 @@ impl AuthServe { shared, stream: Some(stream), issue: None, + default: None, finished: false, }) } @@ -440,8 +450,10 @@ impl AuthServe { // the presenter, the same as a peer that predates AUTH. None if !msg.token.is_empty() => return Poll::Ready(Err(Error::Unsupported)), None => { - let grant = self.shared.peer_grant.clone(); - issue.lock().outbox.push_back(crate::auth::Reply::Grant(grant)); + self.default = Some(crate::auth::DefaultGrant::new( + self.shared.auth.clone(), + self.shared.peer_grant.clone(), + )); } } self.issue = Some(issue.clone()); @@ -470,6 +482,12 @@ impl AuthServe { continue; } + if let Some(default) = &mut self.default + && let Poll::Ready(grant) = default.poll(waiter) + { + issue.lock().outbox.push_back(crate::auth::Reply::Grant(grant)); + } + let mut state = match issue.poll(waiter, |issue| match issue.outbox.is_empty() && !issue.done { true => Poll::Pending, false => Poll::Ready(()), @@ -810,9 +828,7 @@ impl AnnounceServe { }; let announced = origin.announced(); let mut run = AnnounceRun::new(prefix, self.shared.self_origin, self.shared.version); - if self.shared.version.has_auth() { - run.auth = Some(self.shared.auth.clone()); - } + run.auth = Some(self.shared.auth.clone()); self.state = AnnounceState::Run { origin, announced, run }; } } @@ -834,12 +850,12 @@ struct AnnounceRun { // prefix. live: HashMap, phase: AnnouncePhase, - // Our grant, on versions with AUTH: only what it lets us publish is announced, - // and a shrink withdraws what it no longer covers. + // Our grant and the ceiling on the peer: only what both let us publish is + // announced, and a shrink withdraws what they no longer cover. auth: Option, epoch: u64, - // What the union lets us publish, `None` until the peer first answers. - permit: Option, + // What we may announce right now. + permit: crate::auth::Permit, } /// What the peer holds for one advertised suffix. @@ -871,29 +887,26 @@ impl AnnounceRun { phase: AnnouncePhase::Init, auth: None, epoch: 0, - permit: None, + permit: Default::default(), } } - /// Whether our grant lets us announce `suffix` (relative to the requested prefix). + /// Whether we may announce `suffix` (relative to the requested prefix). fn permitted(&self, suffix: &crate::Path) -> bool { - self.permit - .as_ref() - .is_none_or(|permit| permit.matches(self.prefix.join(suffix).as_str())) + self.permit.matches(self.prefix.join(suffix).as_str()) } - /// Apply a change to our grant: withdraw what it no longer covers and, when it - /// grew, re-read the origin to announce what it now does. + /// Apply a change to what we may announce: withdraw what it no longer covers and, + /// when it grew, re-read the origin to announce what it now does. fn regrant( &mut self, stream: &mut Stream, origin: &origin::Consumer, announced: &mut announce::Consumer, - permit: crate::Patterns, + permit: crate::auth::Permit, ) -> Result<(), Error> { - // `None` allowed everything, so any grant only shrinks it. - let grew = self.permit.as_ref().is_some_and(|old| !old.covers(&permit)); - self.permit = Some(permit); + let grew = !self.permit.covers(&permit); + self.permit = permit; if !grew { let revoked: Vec<_> = self @@ -1076,7 +1089,7 @@ impl AnnounceRun { ) -> Result<(), Error> { match self.version { Version::Lite01 | Version::Lite02 => { - let mut init = Vec::new(); + let mut init: Vec<(crate::PathOwned, Hops, crate::origin::Cost)> = Vec::new(); // Send ANNOUNCE_INIT as the first message with all currently active routes. // We use `try_next()` to synchronously get the initial updates. @@ -1085,21 +1098,30 @@ impl AnnounceRun { let suffix = update.prefix; if update.kind.is_active() { - if self.outgoing(&update.route, &absolute).is_none() { + if !self.permitted(&suffix) { continue; } + let Some((hops, cost)) = self.outgoing(&update.route, &absolute) else { + continue; + }; tracing::debug!(route = %absolute, "announce"); - if !init.contains(&suffix) { - init.push(suffix); + if !init.iter().any(|(p, ..)| p == &suffix) { + init.push((suffix, hops, cost)); } } else { // A potential race: a just-announced route already retracted. tracing::debug!(route = %absolute, "unannounce"); - init.retain(|p| p != &suffix); + init.retain(|(p, ..)| p != &suffix); } } - let announce_init = lite::AnnounceInit { suffixes: init }; + // The peer holds these now, so a later retraction or a narrower grant ends them. + let mut suffixes = Vec::with_capacity(init.len()); + for (suffix, hops, cost) in init { + self.live.insert(suffix.clone(), Advertised { id: None, hops, cost }); + suffixes.push(suffix); + } + let announce_init = lite::AnnounceInit { suffixes }; stream.writer.buffer(&announce_init)?; } _ if self.version.has_announce_ok() => { @@ -1164,8 +1186,9 @@ impl AnnounceRun { // advertises something it would withdraw, or abort over, a moment later. if let Some(auth) = &self.auth { ready!(auth.poll_setup_answered(waiter)); - if let Poll::Ready(union) = auth.poll_union(&mut self.epoch, waiter) { - self.permit = union.map(|grant| grant.publish); + if let Poll::Ready(permit) = auth.poll_permit(crate::auth::Direction::Publish, &mut self.epoch, waiter) + { + self.permit = permit; } } self.init(stream, origin, announced)?; @@ -1183,10 +1206,10 @@ impl AnnounceRun { // A grant change applies before the next update, so a route it no longer // covers is withdrawn rather than re-sent. if let Some(auth) = &self.auth - && let Poll::Ready(Some(grant)) = auth.poll_union(&mut self.epoch, waiter) - && self.permit.as_ref() != Some(&grant.publish) + && let Poll::Ready(permit) = auth.poll_permit(crate::auth::Direction::Publish, &mut self.epoch, waiter) + && self.permit != permit { - self.regrant(stream, origin, announced, grant.publish)?; + self.regrant(stream, origin, announced, permit)?; continue; } @@ -1228,6 +1251,8 @@ struct TrackInfoServe { shared: Arc>, stream: Option>, state: TrackInfoState, + /// Ends the request once the session stops allowing its broadcast. + gate: Option, // Log context, filled in after the decode. absolute: crate::PathOwned, track: String, @@ -1264,6 +1289,7 @@ impl TrackInfoServe { shared, stream: Some(stream), state: TrackInfoState::Decode, + gate: None, absolute: Default::default(), track: Default::default(), }) @@ -1283,6 +1309,9 @@ impl TrackInfoServe { | Error::Transport(_) => { tracing::debug!(broadcast = %self.absolute, track = %self.track, "track info cancelled") } + Error::Unauthorized => { + tracing::debug!(broadcast = %self.absolute, track = %self.track, "track info unauthorized") + } err => { tracing::warn!(broadcast = %self.absolute, track = %self.track, %err, "track info error") } @@ -1294,6 +1323,11 @@ impl TrackInfoServe { } fn poll_serve(&mut self, waiter: &kio::Waiter) -> Poll> { + if let Some(gate) = &mut self.gate + && gate.poll_denied(waiter).is_ready() + { + return Poll::Ready(Err(Error::Unauthorized)); + } loop { // While the lookup runs the requester sends nothing, so its send side closing, // by FIN or reset, means it stopped waiting. Drop the lookup then, so the @@ -1318,12 +1352,11 @@ impl TrackInfoServe { self.track = msg.track.to_string(); tracing::debug!(broadcast = %self.absolute, track = %self.track, "track info requested"); // Checked before anything is resolved, like a subscription. - let allowed = self - .shared - .auth - .allows(crate::auth::Direction::Publish, msg.broadcast.as_str()); + let mut gate = self.shared.gate(&msg.broadcast); + let denied = gate.poll_denied(waiter).is_ready(); + self.gate = Some(gate); self.state = TrackInfoState::Hop { msg }; - if !allowed { + if denied { return Poll::Ready(Err(Error::Unauthorized)); } } @@ -1476,22 +1509,15 @@ impl SubscribeServe { self.track = msg.track.to_string(); tracing::info!(id = self.id, broadcast = %self.absolute, track = %self.track, "subscribed started"); - if self.shared.version.has_auth() { - let mut gate = crate::auth::Gate::new( - self.shared.auth.clone(), - msg.broadcast.to_owned(), - crate::auth::Direction::Publish, - ); - // Checked before anything is resolved, so a broadcast our grant - // does not cover is never served, not just cut off later. - if gate.poll_denied(waiter).is_ready() { - self.state = SubscribeState::Hop { msg }; - return Poll::Ready(Err(Error::Unauthorized)); - } - self.gate = Some(gate); - } - + // Checked before anything is resolved, so a broadcast the session does + // not allow is never served, not just cut off later. + let mut gate = self.shared.gate(&msg.broadcast); + let denied = gate.poll_denied(waiter).is_ready(); + self.gate = Some(gate); self.state = SubscribeState::Hop { msg }; + if denied { + return Poll::Ready(Err(Error::Unauthorized)); + } } SubscribeState::Hop { .. } => { // We just received a subscribe for this exact path, so by definition the @@ -1628,6 +1654,8 @@ struct FetchServe { shared: Arc>, stream: Option>, state: FetchState, + /// Ends the fetch, even mid-group, once the session stops allowing its broadcast. + gate: Option, // Log context, filled in after the decode. absolute: crate::PathOwned, track: String, @@ -1681,6 +1709,7 @@ impl FetchServe { shared, stream: Some(stream), state: FetchState::Decode, + gate: None, absolute: Default::default(), track: Default::default(), group: 0, @@ -1703,6 +1732,9 @@ impl FetchServe { | Error::Transport(_) => { tracing::info!(broadcast = %self.absolute, track = %self.track, group = %self.group, "fetch cancelled") } + Error::Unauthorized => { + tracing::info!(broadcast = %self.absolute, track = %self.track, group = %self.group, "fetch unauthorized") + } err => { tracing::warn!(broadcast = %self.absolute, track = %self.track, group = %self.group, %err, "fetch error") } @@ -1714,6 +1746,11 @@ impl FetchServe { } fn poll_serve(&mut self, waiter: &kio::Waiter) -> Poll> { + if let Some(gate) = &mut self.gate + && gate.poll_denied(waiter).is_ready() + { + return Poll::Ready(Err(Error::Unauthorized)); + } loop { match &mut self.state { FetchState::Decode => { @@ -1726,13 +1763,13 @@ impl FetchServe { self.group = msg.group; tracing::info!(broadcast = %self.absolute, track = %self.track, group = %self.group, "fetch started"); - // Checked before anything is resolved, like a subscription. - let allowed = self - .shared - .auth - .allows(crate::auth::Direction::Publish, msg.broadcast.as_str()); + // Checked before anything is resolved, like a subscription, and held + // until the last frame. + let mut gate = self.shared.gate(&msg.broadcast); + let denied = gate.poll_denied(waiter).is_ready(); + self.gate = Some(gate); self.state = FetchState::Hop { msg }; - if !allowed { + if denied { return Poll::Ready(Err(Error::Unauthorized)); } } diff --git a/rs/moq-net/src/lite/subscriber.rs b/rs/moq-net/src/lite/subscriber.rs index 28354e7e73..06f72efcf7 100644 --- a/rs/moq-net/src/lite/subscriber.rs +++ b/rs/moq-net/src/lite/subscriber.rs @@ -326,13 +326,7 @@ impl Subscriber { // outside our scope, so don't serve it. Reflections are already // filtered above. let route = self.announced_route(hops, cost, link_cost, responder_origin); - let Ok(dynamic) = self.origin.dynamic(&path, route.clone()) else { - return Ok(false); - }; - - announced.attach(path, route, dynamic); - - Ok(true) + Ok(announced.offer(self, path, route)) } /// The route to announce for a prefix this peer advertised, charging our @@ -424,13 +418,7 @@ impl Subscriber { return Ok(true); } - let Ok(dynamic) = self.origin.dynamic(&path, metadata.clone()) else { - announced.declined(path); - return Ok(false); - }; - announced.attach(path, metadata, dynamic); - - Ok(true) + Ok(announced.offer(self, path, metadata)) } /// Remove a subscription, releasing the session's handle on its producer. @@ -1173,6 +1161,8 @@ struct PrefixRun { // path, and lite-07 bases name it too. Tracked even for announces we drop // locally (reflected loops), since the sender doesn't know we dropped them. decoder: lite::AnnounceDecoder, + /// The auth epoch last applied, so a new limit holds back or brings back routes. + epoch: u64, } impl AnnouncePrefix { @@ -1259,6 +1249,7 @@ impl AnnouncePrefix { link_cost, announced: Announced::default(), decoder: lite::AnnounceDecoder::default(), + epoch: 0, }; // Lite01/02 send the initial set as one ANNOUNCE_INIT message, so they @@ -1299,6 +1290,16 @@ impl AnnouncePrefix { if self.subscriber.going_away.poll(waiter).is_ready() { run.announced.drain(); } + // A new limit holds back what the peer may no longer publish to us, + // and brings back what it may again. The peer still holds each + // advertisement either way. + while let Poll::Ready(permit) = + self.subscriber + .auth + .poll_permit(crate::auth::Direction::Subscribe, &mut run.epoch, waiter) + { + run.announced.limit(&permit, &self.subscriber); + } loop { match stream.reader.poll_decode_maybe::(&mut cx) { Poll::Ready(Ok(Some(announce))) => { @@ -1557,10 +1558,11 @@ mod tests { .subscribe(None) .await .unwrap(); + let gate = serve.gate(); let mut running = TrackServeRun { serve, state: TrackRunState::Serve(serving), - gate: None, + gate, }; kio::wait(|waiter| kio::Task::poll(&mut running, waiter)).await; @@ -1608,10 +1610,11 @@ mod tests { .subscribe(None) .await .unwrap(); + let gate = serve.gate(); let mut running = TrackServeRun { serve, state: TrackRunState::Serve(serving), - gate: None, + gate, }; assert!( kio::Task::poll(&mut running, &kio::Waiter::noop()).is_ready(), @@ -1905,6 +1908,63 @@ mod tests { assert!(wire.is_empty(), "a second SUBSCRIBE trailed the first"); } + /// A narrower limit landing while the SUBSCRIBE is still opening resets it with + /// UNAUTHORIZED, like a live one, and releases its id. + #[tokio::test] + async fn a_narrower_limit_resets_a_subscribe_still_opening() { + // Writes park, so the SUBSCRIBE is on an open stream but not yet flushed. + let gate = kio::Producer::new(false); + let session = SinkSession::gated_bi(gate.consume()); + let auth = crate::auth::Handle::new(false); + + let origin = origin::Config::new(crate::Hop::new(1).unwrap()).produce(); + let subscriber = Subscriber::new(SubscriberConfig { + runtime: crate::time::Clock::tokio(), + session: session.clone(), + origin, + recv_bandwidth: None, + version: VERSION, + peer_setup: Default::default(), + peer_hop: None, + cost: None, + going_away: Default::default(), + auth: auth.clone(), + }); + let subscribes = subscriber.subscribes.clone(); + let serve = TrackServe { + subscriber, + path: Path::new("room/host").to_owned(), + name: "audio".to_string(), + }; + + let broadcast = crate::broadcast::Info::new().produce(); + let request = broadcast.reserve_track("audio").unwrap(); + let mut serving = ServeLoop::new(&serve, request, Default::default(), Some(Timescale::default())); + let establish = serve.prepare_establish( + &mut serving.serving, + Subscription::default(), + Some(Timescale::default()), + ); + serving.mode = ServeMode::Establish(establish); + let gate_run = serve.gate(); + let mut running = TrackServeRun { + serve, + state: TrackRunState::Serve(serving), + gate: gate_run, + }; + + assert!(kio::Task::poll(&mut running, &kio::Waiter::noop()).is_pending()); + assert_eq!(session.log.bi_opens(), 1, "the SUBSCRIBE stream is open"); + + auth.authorize(&crate::auth::Grant::default()); + assert!(kio::Task::poll(&mut running, &kio::Waiter::noop()).is_ready()); + + let unauthorized = crate::StreamError::Unauthorized.to_code(); + assert_eq!(session.log.resets(), vec![unauthorized]); + assert!(subscribes.lock().is_empty(), "the id outlived its subscription"); + drop(gate); + } + /// Everything a `handle_subscription` test needs to stay alive for the call. struct Harness { serve: TrackServe, @@ -3019,13 +3079,24 @@ enum Sub { /// owns its path and announce id until it retracts or restarts it. #[derive(Default)] struct Announced { - routes: HashMap>, + routes: HashMap, /// Attached routes whose request queue woke since the last serve pass. The /// driver wakes for every group the session carries, so a pass must cost what /// was requested, not every route the peer announced. ready: kio::Queue, } +/// What we made of one advertisement the peer holds. +enum Held { + /// Not in the origin for good: a reflection, or outside our origin's scope. + Declined, + /// Outside the session's limit: the route as advertised, attached once a + /// limit allows it. + Withheld(crate::origin::Route), + /// In the origin, serving requests. + Attached(Box), +} + impl Announced { fn contains(&self, path: &PathOwned) -> bool { self.routes.contains_key(path) @@ -3040,12 +3111,42 @@ impl Announced { }); let route = AnnouncedRoute::new(route, dynamic, wake); route.waker.wake_by_ref(); - self.routes.insert(path, Some(route)); + self.routes.insert(path, Held::Attached(Box::new(route))); + } + + /// Put the peer's route into the origin, so paths under it resolve through this + /// session on demand, or hold it back when the session's limit does not cover it. + /// Returns whether it attached. + fn offer( + &mut self, + subscriber: &Subscriber, + path: PathOwned, + route: crate::origin::Route, + ) -> bool { + if !subscriber + .auth + .within_limit(crate::auth::Direction::Subscribe, path.as_str()) + { + tracing::debug!(route = %subscriber.log_path(&path), "withholding announce outside the limit"); + self.routes.insert(path, Held::Withheld(route)); + return false; + } + // An error means the prefix is outside our origin's scope, so don't serve it. + match subscriber.origin.dynamic(&path, route.clone()) { + Ok(dynamic) => { + self.attach(path, route, dynamic); + true + } + Err(_) => { + self.declined(path); + false + } + } } fn declined(&mut self, path: PathOwned) { // Dropping a replaced route closes its sources. - self.routes.insert(path, None); + self.routes.insert(path, Held::Declined); } /// Record an advertisement before deciding what to do with it. @@ -3059,17 +3160,20 @@ impl Announced { /// with [`Self::contains`]. Overwriting an attached route is [`Self::declined`]'s job. fn reserve(&mut self, path: PathOwned) { debug_assert!(!self.routes.contains_key(&path), "reserved a prefix already advertised"); - self.routes.insert(path, None); + self.routes.insert(path, Held::Declined); } fn attached(&mut self, path: &PathOwned) -> Option<&mut AnnouncedRoute> { - self.routes.get_mut(path)?.as_mut() + match self.routes.get_mut(path)? { + Held::Attached(route) => Some(route.as_mut()), + _ => None, + } } /// Retire this session's advertisement without invalidating another live /// session from the same peer. Dropping its sources closes their requests. fn withdraw(&mut self, path: &PathOwned) { - if let Some(Some(entry)) = self.routes.remove(path) { + if let Some(Held::Attached(entry)) = self.routes.remove(path) { entry.dynamic.withdrawn(); } } @@ -3081,7 +3185,7 @@ impl Announced { let root = subscriber.origin.root().to_owned(); while let Poll::Ready(Ok(path)) = self.ready.poll_pop(waiter) { // A route retired since it woke has nothing left to serve. - let Some(Some(entry)) = self.routes.get_mut(&path) else { + let Some(Held::Attached(entry)) = self.routes.get_mut(&path) else { continue; }; // Cleared before polling, so a request landing mid-pass queues the route again. @@ -3106,10 +3210,44 @@ impl Announced { } } + /// Apply a new limit: hold back every attached route it no longer covers, closing + /// its sources (tracks in flight end on their own gates, with `Unauthorized`), and + /// attach every withheld one it now does. + fn limit(&mut self, permit: &crate::auth::Permit, subscriber: &Subscriber) { + let changed: Vec = self + .routes + .iter() + .filter(|(path, held)| match held { + Held::Attached(_) => !permit.within_limit(path.as_str()), + Held::Withheld(_) => permit.within_limit(path.as_str()), + Held::Declined => false, + }) + .map(|(path, _)| path.clone()) + .collect(); + for path in changed { + match self.routes.remove(&path) { + Some(Held::Attached(entry)) => { + tracing::info!(route = %subscriber.log_path(&path), "announce no longer authorized"); + self.routes.insert(path, Held::Withheld(entry.route.clone())); + } + Some(Held::Withheld(mut route)) => { + tracing::info!(route = %subscriber.log_path(&path), "announce authorized again"); + if subscriber.going_away.is_set() { + route.cost = crate::origin::Cost::DRAIN; + } + self.offer(subscriber, path, route); + } + _ => unreachable!("only attached and withheld routes change"), + } + } + } + /// Re-price every attached route to a draining cost (the peer sent a GOAWAY). fn drain(&mut self) { - for entry in self.routes.values_mut().flatten() { - entry.drain(); + for held in self.routes.values_mut() { + if let Held::Attached(entry) = held { + entry.drain(); + } } } } @@ -3211,6 +3349,16 @@ struct TrackServe { } impl TrackServe { + /// Watches whether the session still lets us receive this broadcast from the peer: + /// our grant, and the ceiling on what the peer may publish. + fn gate(&self) -> crate::auth::Gate { + crate::auth::Gate::new( + self.subscriber.auth.clone(), + self.path.clone(), + crate::auth::Direction::Subscribe, + ) + } + fn widen_frame_bounds(&self, subscription: &mut Subscription) { if self.subscriber.version.has_frame_bounds() { return; @@ -3551,6 +3699,14 @@ impl Establish { } } + /// Give up on the SUBSCRIBE, resetting it with `err` if it reached the wire. + fn abort(self, err: &Error) { + self.serve.subscriber.remove_subscribe(self.id); + if let EstablishState::Send { stream } | EstablishState::WaitOk { stream } = self.state { + stream.writer.abort(err); + } + } + fn activate(&self, stream: Stream) -> SubStream { SubStream { stream, @@ -3571,8 +3727,8 @@ impl Establish { struct TrackServeRun { serve: TrackServe, state: TrackRunState, - /// Cancels the track once our grant stops covering its broadcast. - gate: Option, + /// Cancels the track once the session stops allowing its broadcast. + gate: crate::auth::Gate, } // A state machine's enum is its storage: one transient instance per stream, so the @@ -3603,18 +3759,12 @@ impl TrackServeRun { let info = track::Info::default().with_max_age(serve.subscriber.origin.default_max_age()); TrackRunState::Serve(ServeLoop::new(&serve, request, info, None)) }; - let gate = serve.subscriber.version.has_auth().then(|| { - crate::auth::Gate::new( - serve.subscriber.auth.clone(), - serve.path.clone(), - crate::auth::Direction::Subscribe, - ) - }); + let gate = serve.gate(); Self { serve, state, gate } } - /// Our grant no longer covers the broadcast: end the track and cancel the - /// upstream subscription, leaving the rest of the session alone. + /// The session no longer allows the broadcast: end the track and cancel the + /// upstream subscription and fetches, leaving the rest of the session alone. fn revoke(&mut self) { tracing::info!(broadcast = %self.serve.subscriber.log_path(&self.serve.path), track = %self.serve.name, "subscription no longer authorized"); match std::mem::replace(&mut self.state, TrackRunState::Done) { @@ -3626,11 +3776,15 @@ impl TrackServeRun { TrackRunState::Serve(serve_loop) => { let _ = serve_loop.serving.abort(Error::Unauthorized); // Reset rather than finish, so the publisher reads a revocation instead of - // a routine unsubscribe. + // a routine unsubscribe. A SUBSCRIBE still opening is on the wire too. if let Sub::Active(active) = serve_loop.sub { self.serve.subscriber.remove_subscribe(active.id); active.stream.writer.abort(&Error::Unauthorized); } + if let ServeMode::Establish(establish) = serve_loop.mode { + establish.abort(&Error::Unauthorized); + } + // Each fetch in flight resets itself as it drops, seeing the same denial. } TrackRunState::Done => {} } @@ -3639,9 +3793,7 @@ impl TrackServeRun { impl kio::Task for TrackServeRun { fn poll(&mut self, waiter: &kio::Waiter) -> Poll<()> { - if let Some(gate) = &mut self.gate - && gate.poll_denied(waiter).is_ready() - { + if self.gate.poll_denied(waiter).is_ready() { self.revoke(); return Poll::Ready(()); } @@ -4171,6 +4323,45 @@ impl FetchServeRun { } } +impl Drop for FetchServeRun { + fn drop(&mut self) { + // Dropped because the session stopped allowing the broadcast: reset with that + // reason, so neither the publisher nor the waiting reader reads a routine cancel. + // Any other drop keeps the default cancel. + if matches!(self.state, FetchRunState::Done) + || self + .serve + .subscriber + .auth + .allows(crate::auth::Direction::Subscribe, self.serve.path.as_str()) + { + return; + } + let err = Error::Unauthorized; + match std::mem::replace(&mut self.state, FetchRunState::Done) { + FetchRunState::Open { request } => { + if let Some(request) = request { + request.reject(err); + } + } + FetchRunState::Send { request, stream, .. } | FetchRunState::Answer { request, stream, .. } => { + stream.writer.abort(&err); + if let Some(request) = request { + request.reject(err); + } + } + FetchRunState::Ingest { + mut stream, producer, .. + } => { + stream.reader.abort(&err); + stream.writer.abort(&err); + let _ = producer.abort(err); + } + FetchRunState::Done => {} + } + } +} + impl kio::Task for FetchServeRun { fn poll(&mut self, waiter: &kio::Waiter) -> Poll<()> { let mut cx = std::task::Context::from_waker(waiter.waker()); diff --git a/rs/moq-net/tests/auth.rs b/rs/moq-net/tests/auth.rs index e828124af6..1a4711209e 100644 --- a/rs/moq-net/tests/auth.rs +++ b/rs/moq-net/tests/auth.rs @@ -136,15 +136,22 @@ async fn granted(session: &Session) -> Grant { async fn wait_announced(origin: &origin::Consumer, path: &str, active: bool) { let mut announced = origin.announced(); let mut live = std::collections::HashSet::new(); - loop { - if live.contains(path) == active { - return; - } - let update = announced.next().await.expect("origin closed"); + let apply = |live: &mut std::collections::HashSet, update: moq_net::announce::Update| { match update.kind.is_active() { true => live.insert(update.prefix.to_string()), false => live.remove(update.prefix.as_str()), }; + }; + // Take in the replay first, so a retraction is judged against what is announced now + // rather than against an empty start. + while let Some(update) = futures::FutureExt::now_or_never(announced.next()).flatten() { + apply(&mut live, update); + } + loop { + if live.contains(path) == active { + return; + } + apply(&mut live, announced.next().await.expect("origin closed")); } } @@ -163,6 +170,7 @@ struct Pair { client: Session, server: Session, client_transport: MockSession, + server_transport: MockSession, requests: Option, } @@ -187,6 +195,7 @@ async fn connect(opts: Options) -> Pair { } let observe = client_transport.clone(); + let observe_server = server_transport.clone(); let client_fut = async { let (session, driver) = client.connect(now(), client_transport).await.expect("client handshake"); tokio::spawn(run(driver)); @@ -211,6 +220,7 @@ async fn connect(opts: Options) -> Pair { client, server, client_transport: observe, + server_transport: observe_server, requests, } } @@ -969,3 +979,341 @@ async fn nothing_outside_the_grant_reaches_the_peer(version: &'static str) { .await .expect("timed out"); } + +/// Run each limit case on every version family: the session enforces its limit itself, +/// so a peer without AUTH (or one that ignores it) cannot keep what it lost. +macro_rules! limit_cases { + ($($case:ident),* $(,)?) => { + mod limit_lite_05 { + $(#[tokio::test] async fn $case() { super::$case("moq-lite-05").await })* + } + mod limit_lite_06 { + $(#[tokio::test] async fn $case() { super::$case(super::LITE_06).await })* + } + mod limit_moqt_16 { + $(#[tokio::test] async fn $case() { super::$case("moq-transport-16").await })* + } + mod limit_moqt_17 { + $(#[tokio::test] async fn $case() { super::$case(super::MOQT_17).await })* + } + }; +} + +limit_cases!( + a_narrowing_deafens_one_path, + a_narrowing_aborts_what_the_peer_published, + a_widening_brings_back_a_deafened_path, + a_widening_brings_back_what_the_peer_published, +); + +#[tokio::test] +async fn lite_05_narrowing_resets_a_fetch_in_flight() { + a_narrowing_resets_a_fetch_in_flight("moq-lite-05").await +} + +#[tokio::test] +async fn lite_06_narrowing_resets_a_fetch_in_flight() { + a_narrowing_resets_a_fetch_in_flight(LITE_06).await +} + +/// Whether `version` exchanges AUTH, so the peer also hears of a narrowing. +fn speaks_auth(version: &str) -> bool { + matches!(version, LITE_06 | MOQT_17 | MOQT_22) +} + +/// A revocation as the reader sees it: the local gate's own error, or the peer's +/// UNAUTHORIZED reset relayed across the splice. +fn unauthorized(err: &Error) -> bool { + matches!(err, Error::Unauthorized | Error::Stream(StreamError::Unauthorized)) +} + +/// Read groups until the subscription ends, returning how it ended. +async fn ended(sub: &mut moq_net::track::Subscriber) -> Error { + loop { + match sub.recv_group().await { + Ok(Some(_)) => continue, + Ok(None) => panic!("subscription finished instead of ending"), + Err(err) => return err, + } + } +} + +/// The deafen case: narrowing a live session away from one audio path resets that +/// subscription and retracts its announcement, while a sibling under the same prefix +/// keeps flowing and the session stays up. +async fn a_narrowing_deafens_one_path(version: &'static str) { + within(async { + let ts = |ms| moq_net::Timestamp::from_millis(ms).unwrap(); + let prefs = || moq_net::track::Subscription::default().with_max_age(Duration::from_secs(10)); + + let relay = produce_origin(1); + let audio = relay.create_broadcast("room/alice/audio").unwrap(); + let audio_track = audio.create_track("opus", None).unwrap(); + audio.announce(Default::default()).unwrap(); + let video = relay.create_broadcast("room/alice/video").unwrap(); + let video_track = video.create_track("h264", None).unwrap(); + video.announce(Default::default()).unwrap(); + + let received = produce_origin(3); + let pair = connect(Options { + version: Some(version), + client_subscribe: Some(received.clone()), + server_publish: Some(relay.scope("", &patterns(&["room"])).unwrap()), + ..Default::default() + }) + .await; + + let mut group = audio_track.append_group().unwrap(); + group.write_frame(ts(0), b"a".as_ref()).unwrap(); + let mut group = video_track.append_group().unwrap(); + group.write_frame(ts(0), b"v".as_ref()).unwrap(); + + let remote = received.consume().routed_broadcast("room/alice/audio").await.unwrap(); + let mut audio_sub = remote.track("opus").unwrap().subscribe(prefs()).await.unwrap(); + audio_sub.recv_group().await.unwrap().unwrap(); + let remote = received.consume().routed_broadcast("room/alice/video").await.unwrap(); + let mut video_sub = remote.track("h264").unwrap().subscribe(prefs()).await.unwrap(); + video_sub.recv_group().await.unwrap().unwrap(); + + pair.server.auth().authorize(&grant(&[], &["room/alice/video"])); + + let err = ended(&mut audio_sub).await; + assert!(unauthorized(&err), "{err:?}"); + wait_announced(&received.consume(), "room/alice/audio", false).await; + + // The sibling keeps flowing. + let mut group = video_track.append_group().unwrap(); + group.write_frame(ts(1), b"v".as_ref()).unwrap(); + let group = video_sub.recv_group().await.unwrap().expect("video still flows"); + assert_eq!(group.sequence, 1); + + // The relay enforced it, not the client: moq-lite resets the subscription with + // UNAUTHORIZED, and neither side closed the session. + if version.starts_with("moq-lite") { + let resets = pair.server_transport.resets(); + assert!(resets.contains(&StreamError::Unauthorized.to_code()), "{resets:x?}"); + } + assert_eq!(pair.client_transport.close_reason(), None); + + // A peer that speaks AUTH is told what it may still subscribe to. + if speaks_auth(version) { + let narrowed = wait_for(pair.client.auth().grant(), |grant| { + grant + .as_ref() + .is_some_and(|grant| grant.subscribe == patterns(&["room/alice/video"])) + }) + .await; + assert_eq!(narrowed, Some(grant(&[], &["room/alice/video"]))); + } + }) + .await + .expect("timed out"); +} + +/// Narrowing what the peer may publish aborts the broadcasts it published outside, +/// so the relay's own readers see `Unauthorized`, and retracts their routes, while +/// what it may still publish keeps flowing. +async fn a_narrowing_aborts_what_the_peer_published(version: &'static str) { + within(async { + let ts = |ms| moq_net::Timestamp::from_millis(ms).unwrap(); + let prefs = || moq_net::track::Subscription::default().with_max_age(Duration::from_secs(10)); + + let client_origin = produce_origin(2); + let mic = client_origin.create_broadcast("room/bob/mic").unwrap(); + let mic_track = mic.create_track("opus", None).unwrap(); + mic.announce(Default::default()).unwrap(); + let cam = client_origin.create_broadcast("room/bob/cam").unwrap(); + let cam_track = cam.create_track("h264", None).unwrap(); + cam.announce(Default::default()).unwrap(); + + let relay = produce_origin(1); + let pair = connect(Options { + version: Some(version), + client_publish: Some(client_origin.clone()), + server_subscribe: Some(relay.clone()), + ..Default::default() + }) + .await; + + let mut group = mic_track.append_group().unwrap(); + group.write_frame(ts(0), b"m".as_ref()).unwrap(); + let mut group = cam_track.append_group().unwrap(); + group.write_frame(ts(0), b"c".as_ref()).unwrap(); + + let remote = relay.consume().routed_broadcast("room/bob/mic").await.unwrap(); + let mut mic_sub = remote.track("opus").unwrap().subscribe(prefs()).await.unwrap(); + mic_sub.recv_group().await.unwrap().unwrap(); + let remote = relay.consume().routed_broadcast("room/bob/cam").await.unwrap(); + let mut cam_sub = remote.track("h264").unwrap().subscribe(prefs()).await.unwrap(); + cam_sub.recv_group().await.unwrap().unwrap(); + + pair.server.auth().authorize(&grant(&["room/bob/cam"], &[])); + + let err = ended(&mut mic_sub).await; + assert!(unauthorized(&err), "{err:?}"); + wait_announced(&relay.consume(), "room/bob/mic", false).await; + + let mut group = cam_track.append_group().unwrap(); + group.write_frame(ts(1), b"c".as_ref()).unwrap(); + let group = cam_sub.recv_group().await.unwrap().expect("cam still flows"); + assert_eq!(group.sequence, 1); + + // A narrowing is not a publication outside the grant: the client stays up. + assert_eq!(pair.client_transport.close_reason(), None); + }) + .await + .expect("timed out"); +} + +/// A fetch still streaming its group when the grant narrows away from it is reset with +/// UNAUTHORIZED by the publisher, not left to finish. +async fn a_narrowing_resets_a_fetch_in_flight(version: &'static str) { + within(async { + let ts = |ms| moq_net::Timestamp::from_millis(ms).unwrap(); + + let relay = produce_origin(1); + let broadcast = relay.create_broadcast("room/x").unwrap(); + let track = broadcast.create_track("video", None).unwrap(); + broadcast.announce(Default::default()).unwrap(); + // The group stays open, so the fetch is still in flight when the grant narrows. + let mut group = track.append_group().unwrap(); + group.write_frame(ts(0), b"first".as_ref()).unwrap(); + + let received = produce_origin(3); + let pair = connect(Options { + version: Some(version), + client_subscribe: Some(received.clone()), + server_publish: Some(relay.clone()), + ..Default::default() + }) + .await; + + let remote = received.consume().routed_broadcast("room/x").await.unwrap(); + let mut fetched = remote.track("video").unwrap().fetch_group(0, None).await.unwrap(); + let frame = fetched.read_frame().await.unwrap().expect("first frame"); + assert_eq!(frame.payload.as_ref(), b"first"); + + pair.server.auth().authorize(&grant(&[], &["room/y"])); + + let err = loop { + match fetched.read_frame().await { + Ok(Some(_)) => continue, + Ok(None) => panic!("fetch finished instead of ending"), + Err(err) => break err, + } + }; + assert!(unauthorized(&err), "{err:?}"); + let resets = pair.server_transport.resets(); + assert!(resets.contains(&StreamError::Unauthorized.to_code()), "{resets:x?}"); + drop(group); + }) + .await + .expect("timed out"); +} + +/// Read groups until one at `sequence` or later arrives. +async fn recv_through(sub: &mut moq_net::track::Subscriber, sequence: u64) { + loop { + let group = sub.recv_group().await.unwrap().expect("track ended"); + if group.sequence >= sequence { + return; + } + } +} + +/// Widening the limit after a narrowing brings the deafened path back: it is announced +/// again and a new subscription to it flows. +async fn a_widening_brings_back_a_deafened_path(version: &'static str) { + within(async { + let ts = |ms| moq_net::Timestamp::from_millis(ms).unwrap(); + let prefs = || moq_net::track::Subscription::default().with_max_age(Duration::from_secs(10)); + + let relay = produce_origin(1); + let audio = relay.create_broadcast("room/alice/audio").unwrap(); + let audio_track = audio.create_track("opus", None).unwrap(); + audio.announce(Default::default()).unwrap(); + + let received = produce_origin(3); + let pair = connect(Options { + version: Some(version), + client_subscribe: Some(received.clone()), + server_publish: Some(relay.scope("", &patterns(&["room"])).unwrap()), + ..Default::default() + }) + .await; + + let mut group = audio_track.append_group().unwrap(); + group.write_frame(ts(0), b"a".as_ref()).unwrap(); + let remote = received.consume().routed_broadcast("room/alice/audio").await.unwrap(); + let mut sub = remote.track("opus").unwrap().subscribe(prefs()).await.unwrap(); + sub.recv_group().await.unwrap().unwrap(); + + pair.server.auth().authorize(&grant(&[], &["room/alice/video"])); + let err = ended(&mut sub).await; + assert!(unauthorized(&err), "{err:?}"); + wait_announced(&received.consume(), "room/alice/audio", false).await; + drop((sub, remote)); + + pair.server.auth().authorize(&grant(&[], &["room"])); + wait_announced(&received.consume(), "room/alice/audio", true).await; + let remote = received.consume().routed_broadcast("room/alice/audio").await.unwrap(); + let mut sub = remote.track("opus").unwrap().subscribe(prefs()).await.unwrap(); + let mut group = audio_track.append_group().unwrap(); + group.write_frame(ts(1), b"a".as_ref()).unwrap(); + recv_through(&mut sub, 1).await; + + // A peer that speaks AUTH is told it may subscribe again. + if speaks_auth(version) { + let widened = wait_for(pair.client.auth().grant(), |grant| { + grant + .as_ref() + .is_some_and(|grant| grant.subscribe == patterns(&["room"])) + }) + .await; + assert_eq!(widened, Some(grant(&[], &["room"]))); + } + assert_eq!(pair.client_transport.close_reason(), None); + }) + .await + .expect("timed out"); +} + +/// Widening the limit after a narrowing brings back what the peer published: its route +/// is in the origin again and the relay's own readers can subscribe to it. +async fn a_widening_brings_back_what_the_peer_published(version: &'static str) { + within(async { + let ts = |ms| moq_net::Timestamp::from_millis(ms).unwrap(); + let prefs = || moq_net::track::Subscription::default().with_max_age(Duration::from_secs(10)); + + let client_origin = produce_origin(2); + let mic = client_origin.create_broadcast("room/bob/mic").unwrap(); + let mic_track = mic.create_track("opus", None).unwrap(); + mic.announce(Default::default()).unwrap(); + + let relay = produce_origin(1); + let pair = connect(Options { + version: Some(version), + client_publish: Some(client_origin.clone()), + server_subscribe: Some(relay.clone()), + ..Default::default() + }) + .await; + wait_announced(&relay.consume(), "room/bob/mic", true).await; + + pair.server.auth().authorize(&grant(&["room/bob/cam"], &[])); + wait_announced(&relay.consume(), "room/bob/mic", false).await; + + pair.server.auth().authorize(&grant(&["room"], &[])); + wait_announced(&relay.consume(), "room/bob/mic", true).await; + let remote = relay.consume().routed_broadcast("room/bob/mic").await.unwrap(); + let mut sub = remote.track("opus").unwrap().subscribe(prefs()).await.unwrap(); + let mut group = mic_track.append_group().unwrap(); + group.write_frame(ts(0), b"m".as_ref()).unwrap(); + recv_through(&mut sub, 0).await; + + // Neither the narrowing nor the widening is a publication outside the grant. + assert_eq!(pair.client_transport.close_reason(), None); + }) + .await + .expect("timed out"); +} diff --git a/rs/moq-relay/src/auth.rs b/rs/moq-relay/src/auth.rs index c124948c87..f2cd315070 100644 --- a/rs/moq-relay/src/auth.rs +++ b/rs/moq-relay/src/auth.rs @@ -412,8 +412,7 @@ impl Token { } /// Whether `other` still covers everything this token scopes: the same root and - /// mounts, and every grant still held. A narrower re-check closes the session until - /// pattern scopes can resize it in place. + /// mounts, and every grant still held. pub(crate) fn covered_by(&self, other: &Self) -> bool { self.root == other.root && self.mounts == other.mounts @@ -432,6 +431,8 @@ impl Token { pub struct Lease { consumer: lease::Consumer, token: Token, + /// The live session a re-check re-authorizes in place, once attached. + session: Option, /// When the grant runs out, enforced here whoever drives the lease: a fixed /// grant has no driver, and an auth server's may be mid-outage. Fixed on tokio's /// clock when the grant arrives, so re-polling [`ended`](Self::ended) never @@ -450,17 +451,26 @@ impl Lease { token: Token::new(path, &grant), expires: grant.deadline(), consumer, + session: None, stats: Default::default(), } } + /// Re-authorize `session` in place when a re-check changes what the grant allows, + /// narrower or wider. Without it a narrower grant ends the lease. + pub(crate) fn authorizing(mut self, session: &moq_net::Session) -> Self { + self.session = Some(session.auth()); + self + } + /// Attach the session's stats context, so a re-checked tier retags it live. pub fn with_stats(mut self, stats: moq_net::stats::Session) -> Self { self.stats = stats; self } - /// The scope the session was admitted under. + /// The scope the session holds: what it was admitted under, with the patterns of + /// the latest re-check that resized it. pub fn token(&self) -> &Token { &self.token } @@ -473,11 +483,14 @@ impl Lease { /// Wait for the lease to stop covering the session: the grant expired, was /// revoked, or was re-checked into one that no longer covers the token. /// - /// A changed root or mounts, or a narrower grant, ends it: origin handles cannot yet narrow - /// a live scope in place (tracked by `quest/m1/auth/narrowing.md`). A flipped - /// `peer` ends it too, since the routes it already announced would be - /// misreported as entering here or from a peer. A changed tier keeps the - /// session and moves its [stats](Self::with_stats) to the new tier. + /// A grant with the same root and mounts re-authorizes an attached session for its + /// patterns, narrower or wider, and keeps going; the origin handles the session was + /// admitted with still bound it, so it never reaches past them. Without an attached + /// session a narrower grant ends the lease and a wider one changes nothing. + /// A changed root or mounts ends it, and so does a flipped `peer`, since the + /// routes it already announced would be misreported as entering here or from a + /// peer. A changed tier keeps the session and moves its + /// [stats](Self::with_stats) to the new tier. pub async fn ended(&mut self) -> lease::Reason { loop { let expire = async { @@ -501,8 +514,25 @@ impl Lease { if fresh.peer != self.token.peer { return "peer changed".into(); } - if !self.token.covered_by(&fresh) { - return "grant narrowed".into(); + if fresh.publish != self.token.publish || fresh.subscribe != self.token.subscribe { + match &self.session { + Some(session) => { + tracing::info!( + publish = %join(&fresh.publish), + subscribe = %join(&fresh.subscribe), + "grant changed; re-authorizing the session", + ); + session.authorize(&moq_net::auth::Grant { + publish: fresh.publish.clone(), + subscribe: fresh.subscribe.clone(), + expires: None, + }); + self.token.publish = fresh.publish; + self.token.subscribe = fresh.subscribe; + } + None if !self.token.covered_by(&fresh) => return "grant narrowed".into(), + None => {} + } } if fresh.tier != self.token.tier { tracing::info!(from = %self.token.tier, to = %fresh.tier, "tier changed"); @@ -526,6 +556,15 @@ impl Lease { } } +/// Patterns as one comma-separated line, for logs. +fn join(patterns: &Patterns) -> String { + patterns + .iter() + .map(|pattern| pattern.as_str()) + .collect::>() + .join(",") +} + enum Decider { Server(moq_auth::Client), /// The public rules, as a token with an empty root. diff --git a/rs/moq-relay/src/connection.rs b/rs/moq-relay/src/connection.rs index 8d5e460c84..e195650745 100644 --- a/rs/moq-relay/src/connection.rs +++ b/rs/moq-relay/src/connection.rs @@ -275,20 +275,21 @@ pub(crate) fn authorize( /// the internal listener re-checks this lease; `None` for a session that is /// not in the table. /// -/// The lease is the decider's live word on the grant: when it stops covering -/// the session ([`auth::Lease::ended`]) the session closes with the reason, and -/// the session's own close is reported back through the lease as the `end` event. -/// Either way, a relay shutdown drains the session with a GOAWAY instead of -/// cutting it off. +/// The lease is the decider's live word on the grant: a re-check re-authorizes the +/// session in place, narrower or wider, and when it stops covering the session +/// ([`auth::Lease::ended`]) the session closes with the reason. The session's own +/// close is reported back through the lease as the `end` event. Either way, a relay +/// shutdown drains the session with a GOAWAY instead of cutting it off. /// /// The session handle is `Send + Sync` whatever transport carries it, so this /// runs on the shared runtime even for sessions a pinned QUIC worker drives. pub async fn supervise( session: moq_net::Session, - mut lease: auth::Lease, + lease: auth::Lease, mut shutdown: crate::shutdown::Observer, registration: Option, ) -> anyhow::Result<()> { + let mut lease = lease.authorizing(&session); loop { let nudged = async { match ®istration { diff --git a/rs/moq-relay/src/websocket.rs b/rs/moq-relay/src/websocket.rs index 6093d0ba95..f67f2a5cda 100644 --- a/rs/moq-relay/src/websocket.rs +++ b/rs/moq-relay/src/websocket.rs @@ -127,7 +127,7 @@ struct SessionInputs { async fn handle_socket( socket: T, session: SessionInputs, - mut lease: auth::Lease, + lease: auth::Lease, pending: Option<(crate::session::Registry, moq_auth::Request)>, ) -> anyhow::Result<()> where @@ -197,6 +197,7 @@ where // The handshake is done, so this is a MoQ session now: only now can a push // be serviced, and only now does the session appear in the live table. let registration = pending.map(|(sessions, request)| sessions.register(request)); + let mut lease = lease.authorizing(&session); loop { let nudged = async { diff --git a/rs/moq-relay/tests/auth_lifetime.rs b/rs/moq-relay/tests/auth_lifetime.rs index 12d305d6c3..941cd3da27 100644 --- a/rs/moq-relay/tests/auth_lifetime.rs +++ b/rs/moq-relay/tests/auth_lifetime.rs @@ -4,8 +4,9 @@ //! or QUIC) or its axum WebSocket path (`serve_ws` over `ws://`), points it at a //! scripted auth server, connects a publisher and a subscriber, confirms media //! flows, then asserts the relay follows the server's word: a re-check that moves -//! the tier retags the live session's stats, a narrower grant or a refusal closes it, -//! and every close reports `end` with what it moved. +//! the tier retags the live session's stats, a changed grant re-authorizes it in +//! place, a moved root or a refusal closes it, and every close reports `end` with +//! what it moved. //! The last tests swap the server for an in-process decider answering //! `Admissions`, and prove the lease it drives reaches the session the same way, //! including an outage that keeps it until `expires`, on Tokio's paused clock. @@ -556,9 +557,11 @@ async fn a_moved_tier_retags_the_live_session() { relay.abort(); } -/// A narrower grant closes the session on the next re-check, over TCP and WebSocket. +/// A re-check resizes the live session in place, over TCP and WebSocket: a narrower +/// grant resets the deafened path with `Unauthorized` while a sibling under the same +/// prefix keeps flowing, a wider one brings the path back, and neither session closes. #[tokio::test] -async fn a_narrower_grant_closes_live_sessions() { +async fn a_rechecked_grant_resizes_live_sessions() { for scheme in ["tcp", "ws"] { let script = Script::new(grant(Duration::from_secs(3600))); let auth = build_auth(script.spawn().await); @@ -566,18 +569,154 @@ async fn a_narrower_grant_closes_live_sessions() { "tcp" => spawn_relay(auth).await, _ => spawn_ws_relay(auth).await, }; - let (pub_session, sub_session) = connect_and_round_trip(&room_url(scheme, port)).await; + let url = room_url(scheme, port); + + let pub_origin = moq_tokio::origin::spawn(); + let mut tracks = Vec::new(); + for path in ["alice/audio", "alice/video"] { + let broadcast = pub_origin.create_broadcast(path).expect("create broadcast"); + let track = broadcast.create_track("media", None).expect("create track"); + broadcast.announce(Default::default()).expect("announce broadcast"); + tracks.push((broadcast, track)); + } + let pub_session = tokio::time::timeout( + TIMEOUT, + client() + .with_publisher(pub_origin.consume()) + .with_reconnect(false) + .connect(url.clone()) + .established(), + ) + .await + .expect("publisher connect timeout") + .expect("publisher connect failed"); + + let sub_origin = moq_tokio::origin::spawn(); + let sub_consumer = sub_origin.consume(); + let sub_session = tokio::time::timeout( + TIMEOUT, + client() + .with_subscriber(sub_origin) + .with_reconnect(false) + .connect(url.clone()) + .established(), + ) + .await + .expect("subscriber connect timeout") + .expect("subscriber connect failed"); + + let mut subs = Vec::new(); + for path in ["alice/audio", "alice/video"] { + let broadcast = tokio::time::timeout(TIMEOUT, sub_consumer.routed_broadcast(path)) + .await + .expect("announcement timeout") + .expect("announced broadcast resolves"); + subs.push( + broadcast + .track("media") + .unwrap() + .subscribe(None) + .await + .expect("subscribe"), + ); + } + let send = |index: usize, sequence: u64| { + let mut group = tracks[index].1.append_group().expect("append group"); + group + .write_frame(moq_net::Timestamp::ZERO, b"media".as_ref()) + .expect("write frame"); + group.finish().expect("finish group"); + assert_eq!(group.sequence, sequence); + }; + for (index, sub) in subs.iter_mut().enumerate() { + send(index, 0); + tokio::time::timeout(TIMEOUT, sub.recv_group()) + .await + .expect("recv_group timeout") + .expect("recv_group failed") + .expect("track closed prematurely"); + } + // Deafen alice's audio: everything else stays as admitted. let mut narrow = grant(Duration::from_secs(3600)); - narrow.publish = ["nobody/**".parse().unwrap()].into_iter().collect(); + narrow.subscribe = ["alice/video/**".parse().unwrap()].into_iter().collect(); script.on_revalidate(Answer::Grant(narrow)); - assert_closed(pub_session, Duration::from_secs(5), &format!("{scheme} publisher")).await; - assert_closed(sub_session, Duration::from_secs(5), &format!("{scheme} subscriber")).await; + let err = loop { + match tokio::time::timeout(TIMEOUT, subs[0].recv_group()) + .await + .expect("the deafened track never ended") + { + Ok(Some(_)) => continue, + Ok(None) => panic!("{scheme}: the deafened track finished instead of ending"), + Err(err) => break err, + } + }; + assert!( + matches!( + err, + moq_net::Error::Unauthorized | moq_net::Error::Stream(moq_net::StreamError::Unauthorized) + ), + "{scheme}: {err:?}" + ); + + send(1, 1); + let group = tokio::time::timeout(TIMEOUT, subs[1].recv_group()) + .await + .expect("recv_group timeout") + .expect("recv_group failed") + .expect("the sibling track closed"); + assert_eq!(group.sequence, 1, "{scheme}"); + + // Undeafen: the path is announced again and a new subscription flows. + script.on_revalidate(Answer::Grant(grant(Duration::from_secs(3600)))); + let broadcast = tokio::time::timeout(TIMEOUT, sub_consumer.routed_broadcast("alice/audio")) + .await + .expect("the undeafened path was never announced again") + .expect("announced broadcast resolves"); + let mut audio = broadcast + .track("media") + .unwrap() + .subscribe(None) + .await + .expect("subscribe"); + send(0, 1); + let group = tokio::time::timeout(TIMEOUT, audio.recv_group()) + .await + .expect("recv_group timeout") + .expect("recv_group failed") + .expect("the undeafened track closed"); + assert!(group.sequence >= 1, "{scheme}"); + + for (session, what) in [(pub_session, "publisher"), (sub_session, "subscriber")] { + assert!( + tokio::time::timeout(Duration::from_millis(200), session.closed()) + .await + .is_err(), + "{scheme}: a resize must not close the {what}" + ); + } relay.abort(); } } +/// A re-check that moves the root still closes the session: nothing it holds is +/// named the same way under the new one. +#[tokio::test] +async fn a_moved_root_closes_live_sessions() { + let script = Script::new(grant(Duration::from_secs(3600))); + let (port, relay) = spawn_relay(build_auth(script.spawn().await)).await; + let (pub_session, sub_session) = connect_and_round_trip(&room_url("tcp", port)).await; + + let mut moved = grant(Duration::from_secs(3600)); + moved.root = Some("elsewhere".into()); + script.on_revalidate(Answer::Grant(moved)); + + assert_closed(pub_session, Duration::from_secs(5), "publisher").await; + assert_closed(sub_session, Duration::from_secs(5), "subscriber").await; + relay.abort(); +} + /// A refusal on re-check closes the session, and the `end` says why. /// 403, 401, and an empty grant are all a no. #[tokio::test]