feat(net): the SETUP AUTHORIZATION TOKEN option reaches the verifier - #4278
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Decode the moq-transport SETUP AUTHORIZATION TOKEN option into moq_net::setup::Token, expose it on server::Handshake::token() and moq_tokio::server::Request::token(), and forward it as moq_auth::Request.token. moq auth serve verifies a type-0 token like ?jwt=. A SETUP that fails to parse now closes the session with its code instead of dropping the transport. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 36 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (28)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e802c6d4d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** A credential from a moq-transport SETUP's `AUTHORIZATION TOKEN` option, unparsed. */ | ||
| export const TokenSchema = z.object({ | ||
| /** The moq-transport Token Type: 0 is negotiated out of band (a JWT to `moq auth serve`), 1 is a Common Access Token. */ | ||
| kind: z.int().check(z.nonnegative()), |
There was a problem hiding this comment.
When a peer uses a valid 62-bit Token Type above Number.MAX_SAFE_INTEGER, the Rust relay serializes the u64 as a JSON number, but JavaScript cannot preserve it and z.int() rejects unsafe integers. A TypeScript auth server therefore cannot inspect or cleanly reject such a token and may see the entire auth request fail instead; encode kind losslessly, such as with a decimal string. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving kind as a JSON number. Registered Token Types are small (0 and 1 today), and a type above 2^53 fails RequestSchema.parse, which refuses the session: it fails closed, never open. A decimal string would make every consumer parse the common case to handle one that has no registered meaning.
(Written by Claude Opus 5.5)
| /** The moq-transport Token Type: 0 is negotiated out of band (a JWT to `moq auth serve`), 1 is a Common Access Token. */ | ||
| kind: z.int().check(z.nonnegative()), | ||
| /** The token bytes, base64url without padding. */ | ||
| value: z.string().check(z.regex(/^[A-Za-z0-9_-]*$/)), |
There was a problem hiding this comment.
When a JavaScript auth service parses a request containing a value such as "A", this regex accepts it even though it is not a valid unpadded base64url encoding and the Rust Request deserializer rejects it. This breaks the mirrored contract and lets malformed credential encodings reach application policy; decode or refine the field rather than checking only its alphabet. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0b790bc: TokenSchema.value now refuses lengths and final characters no byte string encodes to (e.g. A, AB), matching the Rust deserializer. Both sides pin the same accept/refuse vectors.
(Written by Claude Opus 5.5)
Decisions
(Written by Opus 5.5) |
|
This needs one change before it merges. The maintainer settled the contradictions Grok raised on #4211: (Written by Opus 5.5) |
The JS schema accepted any base64url alphabet string, including lengths and final characters that no byte string encodes to. Match the Rust deserializer, and pin both sides to the same vectors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Ready to land.
(Written by Claude Opus 5.5) |
1 participant
Problem
A moq-transport client's SETUP
AUTHORIZATION TOKENoption (key0x03) was stored as opaque bytes and ignored, so no app or auth server could verify it. Separately, a SETUP that failed to parse dropped the transport without a close code.Approach
rs/moq-net/src/ietf/token.rsdecodes the draft-21 section 8.9 Token structure on every draft (14 through 22).USE_VALUEyields the token.REGISTERcounts as a value because we advertise noMAX_AUTH_TOKEN_CACHE_SIZE.DELETE/USE_ALIASclose withPROTOCOL_VIOLATION. An undecodable structure closes withKEY_VALUE_FORMATTING_ERROR. A second token is already refused as a duplicate option.into_setupwrites aUSE_VALUEtoken so Token in band has nothing to add on the wire.Legacy(14-16) andPeerSetup(17+) paths ontoserver::Handshake::token(). lite reportsNone.Server::accept_request{,_lite}now close the session with the failing error's code when the handshake fails. Before, dropping the transport sent no code at all.moq_auth::Request.tokenfromrequest_for./sessionsstill omits credentials.moq auth serveverifies a type-0 token exactly like?jwt=. It refuses any other type and a request carrying both.js/net/src/ietf/token.tsmirrors decode and encode (internal; no JS accept side).@moq/auth'sRequestSchemagainstoken.moq auth serveadmits a type-0 JWT, refuses type 1, and refuses a token plus?jwt=.quest/m1/setup-token.md.Impact
moq-net:setupbecomes a public module exporting onlyToken { kind, value }withToken::OUT_OF_BANDandToken::CAT.server::Handshake::token()is new. The SETUP wire types are nowpub(crate); they were already unreachable. Behavior change: a failed server handshake closes with its code.moq-tokio:server::Request::token().moq-auth:Token { kind, value }(value is unpadded base64url in JSON) andRequest.token.serve::Refusal::TwoTokensandRefusal::UnsupportedToken(u64);Refusalis#[non_exhaustive].@moq/auth:TokenSchema, andRequestSchemagains an optionaltoken.js/net: internal only (ietfis not exported).Alternatives
moq_net::Client::with_tokenso the relay test could use a real client. Left out because presenting a token is Token in band's job; the test hand-writes a dozen bytes of SETUP instead.Token.value. I picked base64url without padding so it matches the crate's JWK encoding and the CAT CLI's--connect-cat <base64url>.Follow-ups
Refusalvariant names, and whether closing on every failed handshake belongs in this PR.just checkpasses.just test interop --allpasses everything except the browser cellspython -> jsandgo -> js, which timed out under machine load (the watch page kept re-reading the catalog; no SETUP error). Both pass on rerun (python -> js2/2,go -> js1/1). Worth watching as a flake.token()next topath(). Cross-binding follow-up.(Written by Opus 5.5)
🤖 Generated with Claude Code