Skip to content

Fix hanging sessions for unauthorized connections#470

Merged
kixelated merged 1 commit intomainfrom
fix-auth-hanging
Jul 18, 2025
Merged

Fix hanging sessions for unauthorized connections#470
kixelated merged 1 commit intomainfrom
fix-auth-hanging

Conversation

@kixelated
Copy link
Collaborator

Summary

  • Fix authorization bug where sessions with no valid permissions would hang instead of being rejected
  • Add proper validation to reject connections when authentication is required but no token is provided
  • Add validation to reject sessions that have no publish or subscribe permissions

Problem

When connecting to a path that requires authentication (like hang/room before it was configured as public), sessions would hang indefinitely during the MoQ handshake instead of being properly rejected with an authorization error.

Root Cause

The issue was in rs/moq-relay/src/auth.rs where the validation logic would create a "dummy token" with no permissions for unauthorized connections, but then allow those connections to proceed to the MoQ session establishment phase. The server would then hang waiting for the client to initiate the MoQ handshake.

Solution

  • Check if authentication is required but no JWT token was provided → reject immediately
  • Check if a session ends up with no permissions at all → reject immediately
  • Ensure unauthorized connections are rejected at the WebTransport level instead of hanging during MoQ handshake

Test plan

  • Run just check to verify all tests pass
  • Verify the fix handles both missing authentication and zero-permission cases
  • Test connecting to an authenticated path without credentials confirms immediate rejection
  • Test that legitimate authenticated connections still work properly

🤖 Generated with Claude Code

When authentication is required but no token is provided, or when a session
has no permissions at all, properly reject the connection instead of hanging
during the MoQ handshake phase.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@claude
Copy link
Contributor

claude bot commented Jul 18, 2025

Claude finished @kixelated's task —— View job


Code Review - Security & Bug Analysis

  • Read the changed auth.rs file
  • Analyze the authorization fix implementation
  • Check for potential bugs and security issues
  • Provide review feedback

Analysis Summary

The fix addresses the hanging session issue by adding two validation checks in rs/moq-relay/src/auth.rs:218-234:

  1. Lines 218-221: Rejects connections when authentication is required but no JWT token is provided
  2. Lines 231-234: Rejects sessions that end up with no permissions

Security Assessment ✅

No security issues found. The changes improve security posture by:

  • Failing fast on unauthorized connections instead of allowing them to reach the handshake phase
  • Proper error handling without information leakage
  • Consistent with existing authentication logic

Bug Analysis ✅

No bugs identified. The implementation is sound:

  • Edge case handling: The constructor already ensures that public paths have at least one permission (lines 153-156), making the zero-permission check at line 232-234 a defensive validation
  • Error propagation: Proper use of anyhow::bail! for consistent error handling
  • Logic consistency: New validations align with existing authentication flow
  • Timing: Validation occurs at connection time, preventing hanging during MoQ handshake

Code Quality

The fix is minimal, targeted, and maintains consistency with the existing codebase patterns. Error messages are appropriately descriptive without revealing sensitive details.

Recommendation: Approve - This is a clean fix that resolves the hanging session issue without introducing security vulnerabilities or bugs.

@kixelated kixelated enabled auto-merge (squash) July 18, 2025 01:36
@kixelated kixelated merged commit 83249e9 into main Jul 18, 2025
3 checks passed
@kixelated kixelated deleted the fix-auth-hanging branch July 18, 2025 01:39
@moq-bot moq-bot bot mentioned this pull request Jul 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant