Skip to content

fix(tokio): refuse SETUP paths on URI bindings and keep iroh keys private on release (backport #7206, #7213) - #7389

Merged
kixelated merged 2 commits into
releasefrom
backport/setup-path-iroh-key
Oct 30, 2026
Merged

kixelated merged 2 commits into
releasefrom
backport/setup-path-iroh-key

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Problem

release has two security bugs that moq-dev/moq#7206 and moq-dev/moq#7213 fixed on main:

  • SETUP path on URI bindings. On WebTransport and WebSocket, Request::path()/query() used the SETUP Path whenever it was non-empty and fell back to the request URI only otherwise. A client could dial /room?jwt=… but send /admin?jwt=… in SETUP, and Tokio would authorize the SETUP path. This affects moq-lite-05+ and every moq-transport draft.
  • iroh key permissions. The persisted --iroh-secret file was written with the default umask, typically readable by everyone, and was read through symlinks.

Tracking quest: quest/m0/backport-security-fixes.md, claimed in moq-dev/moq#7381.

Approach

This is a hand port, not a cherry-pick. release has no moq_net::transport::Address, which #7206 depends on, and no moq-tokio/src/secret.rs.

  • SETUP path. moq-tokio's private Setup::accept takes a uri flag, set when the request carries a URL: WebTransport over noq or iroh, and WebSocket. If that SETUP carries a non-empty path, the session is closed with ProtocolViolation and the request is dropped. Request::path()/query() now read only the URL when there is one, as on main. The io_uring relay listener already authorized on the URL; it now also refuses a WebTransport SETUP path, matching main's Tokio and io_uring refusal. Raw QUIC, TCP, and Unix bindings still take the path from SETUP.
  • iroh key. fix(tokio): keep persisted iroh keys private #7213's load_or_create is ported unchanged as a private function in iroh.rs, along with its four tests. A new file is created exclusively with mode 0600. An existing file must be a regular non-symlink file that group and others cannot read. The relay config doc gains fix(tokio): keep persisted iroh keys private #7213's paragraph.

Choices made without asking (listed for review):

  1. An empty SETUP Path is accepted on URI bindings. main refuses presence even when empty. release's public Handshake::path() returns "" for both absent and empty, and telling them apart would add moq-net API, which a patch backport must not. An empty path never overrides the URI any more, so it cannot change what gets authorized.
  2. The regression test lives in moq-tokio, not in a relay integration test. release has no tests/integration/refusals.rs harness. websocket_refuses_setup_path drives moq-lite-06, moqt-18, and moqt-22 over an in-memory WebSocket. It checks that non-empty paths are refused and the client session closes, and that an absent or empty path falls back to the URL. Without the fix it fails.
  3. The test does not assert close code 3. On release's qmux 0.5.3 the client sees a bare Closed here instead of the application close code; main's test runs on qmux 0.6+. The server side does return ProtocolViolation, and the test asserts that.

Impact

  • Public API: none. cargo semver-checks -p moq-tokio --baseline-rev origin/release reports no semver update required. Only private items changed.
  • Wire: on WebTransport and WebSocket, a non-empty SETUP Path now closes the session with a protocol violation (code 3).
  • iroh: an existing --iroh-secret file that is group- or world-readable is now refused at startup. Operators need chmod 600, as the doc says.
  • Versions: none bumped. The maintainer picks them by hand per CONTRIBUTING.md.

Validation

  • nix develop -c just ci check origin/release: passed (after just fix formatting).
  • nix develop -c just ci test origin/release: passed. Rust: 3253 tests passed, 7 skipped. That includes websocket_refuses_setup_path and the four persisted_secret_* tests.
  • cargo semver-checks -p moq-tokio --baseline-rev origin/release: no semver update required.
  • With the refusal turned off, websocket_refuses_setup_path fails on qmux-01.moq-lite-06 accepted SETUP path Some("/room?jwt=url-token").

Review

  • Reviewer: Codex, gpt-6-astra, medium effort (quest review --base origin/release --deep), round 1.
  • Head reviewed: 354da95.
  • Findings: none.

Follow-ups

  • moq-dev/moq#7366 (relay SETUP-path dot-segment refusal) is still open on main. It is not in this quest's Plan; once it merges, it may need its own release backport.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 2 commits October 30, 2026 06:27
…vate on release

Backports #7206 and #7213 onto release.

- WebTransport and WebSocket take the path and query from the request URI only;
  a non-empty SETUP path is refused with a protocol violation (Tokio and the
  io_uring relay listener) instead of being authorized in its place.
- The persisted iroh secret is created exclusively with mode 0600, and an
  existing file that is a symlink, non-regular, or group/other readable is
  refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-30T14:03:02.361434Z 354da95 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 354da9523d05b5c2bf6e77b7174441d98ad1af52.

No actionable findings in this release backport. The Tokio and uring paths reject non-empty SETUP paths on URI-bearing bindings before authorization, while raw/stream bindings retain their in-band path and query. The key-file change uses exclusive creation and validates the opened existing file rather than following a final symlink; Unix private-file checks and the accompanying migration documentation are consistent.

Direction: the focused backport is reasonable. Retain the stated absent/empty SETUP-path compatibility behavior; this is not a claim that every forbidden parameter-presence case is rejected.

Verification limits: independent static review of all four changed files and surrounding binding/dependency context only. No tests, feature builds, live transport checks, or Windows reparse-point checks were executed.

(Written by OpenAI)

@kixelated

Copy link
Copy Markdown
Collaborator Author

Landing summary for head 354da9523d05b5c2bf6e77b7174441d98ad1af52 on release:

  • Changes: this backports moq-dev/moq#7206. Tokio and io_uring now refuse a non-empty SETUP Path on WebTransport and WebSocket bindings, and Request::path()/query() read only the URL there. It also backports moq-dev/moq#7213: the --iroh-secret file is created with 0600, and an existing file that is a symlink or readable by group or others is refused.
  • Reviews: the local Codex round on this head had no findings. The OpenAI review and the Codex connector had none either. CI is green (20/20).
  • Decisions: on 2026-10-30 the maintainer approved merging this now. No version bump and no release cut here. One combined patch release comes later, after moq-dev/moq#7298 and the dot-segment/auth-redirect backport land.
  • Kept as listed in the body: an empty SETUP Path is still accepted on URI bindings, to avoid new moq-net API. The regression test lives in moq-tokio. Close code 3 is asserted on the server side only.

(Written by Claude Opus 5.5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant