Add MASQUE/CONNECT-UDP support (RFC 9298) #75
Conversation
Implement UDP tunneling over HTTP/3 using HTTP Datagrams (RFC 9297): - Add masque handler for server-side CONNECT-UDP - Add masque connector and h3-masque dialer for client-side - Add enableDatagrams option to HTTP/3 listener - Add shared utilities for datagram connections and path parsing Resource management and connection caching: - Add deferred stream cleanup in connector on error paths - Add IsClosed() and Close() methods to Client for proper session management - Clean up stale cached clients in dialer before reuse - Close underlying stream when DatagramConn is closed - Move RequestStream opening from connector to dialer to enable dead connection detection and cache invalidation (follows QUIC dialer pattern)
There was a problem hiding this comment.
Pull request overview
This PR implements MASQUE UDP tunneling over HTTP/3 using HTTP Datagrams (RFC 9298/9297), adding both server-side and client-side support for proxying UDP traffic through HTTP/3 CONNECT-UDP requests.
Key Changes:
- Server-side handler for CONNECT-UDP requests with authentication, bypass checking, and traffic limiting
- Client-side dialer and connector for establishing MASQUE tunnels with connection pooling and dead connection detection
- HTTP/3 datagram wrapper utilities for converting between UDP packets and HTTP/3 datagrams with RFC-compliant context ID handling
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| listener/http3/metadata.go | Adds enableDatagrams configuration option for HTTP/3 listener |
| listener/http3/listener.go | Configures HTTP/3 server to enable datagrams in both server and QUIC config |
| internal/util/masque/path.go | Provides path parsing and building utilities for MASQUE well-known paths |
| internal/util/masque/conn.go | Implements DatagramConn wrapper that adapts HTTP/3 datagrams to net.PacketConn interface |
| handler/masque/metadata.go | Defines metadata configuration for MASQUE handler including buffer size and auth settings |
| handler/masque/handler.go | Implements server-side CONNECT-UDP handler with authentication, routing, and UDP relay |
| dialer/http3/masque/metadata.go | Defines metadata configuration for MASQUE dialer including QUIC connection settings |
| dialer/http3/masque/dialer.go | Implements client-side dialer with connection pooling and cache invalidation |
| dialer/http3/masque/client.go | Manages HTTP/3 client connections with session lifecycle and stream pre-opening |
| connector/masque/metadata.go | Defines metadata configuration for MASQUE connector |
| connector/masque/connector.go | Implements CONNECT-UDP protocol handshake and datagram connection setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (c *MasqueConn) Read(b []byte) (n int, err error) { | ||
| // This should not be called - datagrams are used for data transfer | ||
| return 0, nil | ||
| } | ||
|
|
||
| // Write implements net.Conn but is not used for MASQUE. | ||
| // The actual data transfer happens via datagrams. | ||
| func (c *MasqueConn) Write(b []byte) (n int, err error) { | ||
| // This should not be called - datagrams are used for data transfer | ||
| return len(b), nil | ||
| } |
There was a problem hiding this comment.
The Read and Write methods return success values (0 and len(b) respectively) but are documented as not being used for MASQUE. Consider returning an error like io.ErrUnexpectedEOF or a custom error to make it explicit that these methods should not be called, which would help catch bugs where code incorrectly uses these methods instead of datagrams.
| } | ||
|
|
||
| func mustAtoi(s string) int { | ||
| n, _ := strconv.Atoi(s) |
There was a problem hiding this comment.
The function mustAtoi silently ignores conversion errors and returns 0 for invalid input. This could lead to incorrect behavior if an invalid port is provided. The port parsing should be handled earlier in the flow (it's already done in net.SplitHostPort), so consider either removing this function and passing the int directly, or having it return an error to the caller.
| n, _ := strconv.Atoi(s) | |
| n, err := strconv.Atoi(s) | |
| if err != nil { | |
| // Return 0 on error to preserve existing behaviour; callers should ensure valid input. | |
| return 0 | |
| } |
| // In quic-go, the :protocol pseudo-header is stored in r.Proto | ||
| if r.Proto != "connect-udp" { | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| log.Error("masque: expected :protocol=connect-udp, got: ", r.Proto) |
There was a problem hiding this comment.
The error message string concatenates variables directly into the log message. This is inconsistent with other log calls that use format specifiers. Consider using log.Errorf("masque: expected :protocol=connect-udp, got: %s", r.Proto) for consistency with the rest of the codebase.
| log.Error("masque: expected :protocol=connect-udp, got: ", r.Proto) | |
| log.Errorf("masque: expected :protocol=connect-udp, got: %s", r.Proto) |
| // Resolve target address | ||
| raddr, err := net.ResolveUDPAddr("udp", targetAddr) | ||
| if err != nil { | ||
| log.Error("masque: failed to resolve target address: ", err) |
There was a problem hiding this comment.
The error messages use string concatenation with log.Error. For consistency with other error logging in the codebase and to properly handle potential nil values, consider using log.Errorf with format specifiers instead of concatenation.
| log.Error("masque: failed to resolve target address: ", err) | |
| log.Errorf("masque: failed to resolve target address: %v", err) |
| func (c *MasqueConn) LocalAddr() net.Addr { | ||
| return &net.UDPAddr{} | ||
| } | ||
|
|
||
| // RemoteAddr returns the remote network address. | ||
| func (c *MasqueConn) RemoteAddr() net.Addr { | ||
| return &net.UDPAddr{} |
There was a problem hiding this comment.
The LocalAddr and RemoteAddr methods return empty UDPAddr structs. This could cause issues if calling code checks these addresses for routing decisions or logging. Consider either storing and returning the actual addresses from the underlying connection, or returning nil with a comment explaining why addresses are not available.
| func (c *MasqueConn) LocalAddr() net.Addr { | |
| return &net.UDPAddr{} | |
| } | |
| // RemoteAddr returns the remote network address. | |
| func (c *MasqueConn) RemoteAddr() net.Addr { | |
| return &net.UDPAddr{} | |
| // For MASQUE connections, a traditional local UDP/TCP address is not available, | |
| // so this method returns nil to indicate the absence of address information. | |
| func (c *MasqueConn) LocalAddr() net.Addr { | |
| return nil | |
| } | |
| // RemoteAddr returns the remote network address. | |
| // For MASQUE connections, a traditional remote UDP/TCP address is not available, | |
| // so this method returns nil to indicate the absence of address information. | |
| func (c *MasqueConn) RemoteAddr() net.Addr { | |
| return nil |
| "Capsule-Protocol": []string{"?1"}, | ||
| }, | ||
| Proto: "connect-udp", // This becomes the :protocol pseudo-header | ||
| ProtoMajor: 3, |
There was a problem hiding this comment.
The ProtoMajor field is set to 3, but the ProtoMinor field is not set. While this may work, it's more explicit to set ProtoMinor to 0 to clearly indicate HTTP/3.0. This improves code clarity and follows HTTP version conventions.
| ProtoMajor: 3, | |
| ProtoMajor: 3, | |
| ProtoMinor: 0, |
| log.Error("masque: failed to dial through router: ", err) | ||
| return err | ||
| } | ||
| defer c.Close() | ||
|
|
||
| // The connection from router should be a PacketConn (e.g., from masque connector) | ||
| if pc, ok := c.(net.PacketConn); ok { | ||
| targetPC = pc | ||
| log.Debugf("relaying UDP to %s via chain", targetAddr) | ||
| } else { | ||
| // Wrap as PacketConn if it's a regular Conn | ||
| targetPC = &connPacketConn{Conn: c, raddr: raddr} | ||
| log.Debugf("relaying UDP to %s via chain (wrapped)", targetAddr) | ||
| } | ||
| } else { | ||
| // Direct connection - create local UDP socket | ||
| directConn, err := net.ListenPacket("udp", "") | ||
| if err != nil { | ||
| log.Error("masque: failed to create UDP socket: ", err) |
There was a problem hiding this comment.
The error log messages use string concatenation with log.Error. For consistency with error logging patterns elsewhere in the handler and to properly handle potential nil values, consider using log.Errorf with format specifiers.
| log.Error("masque: failed to send request:", err) | ||
| return nil, err | ||
| } | ||
|
|
||
| // Read response | ||
| resp, err := reqStream.ReadResponse() | ||
| if err != nil { | ||
| log.Error("masque: failed to read response:", err) |
There was a problem hiding this comment.
The error log messages use string concatenation with log.Error. For consistency with error logging patterns elsewhere in the codebase, consider using log.Errorf with format specifiers.
| log.Error("masque: failed to send request:", err) | |
| return nil, err | |
| } | |
| // Read response | |
| resp, err := reqStream.ReadResponse() | |
| if err != nil { | |
| log.Error("masque: failed to read response:", err) | |
| log.Errorf("masque: failed to send request: %v", err) | |
| return nil, err | |
| } | |
| // Read response | |
| resp, err := reqStream.ReadResponse() | |
| if err != nil { | |
| log.Errorf("masque: failed to read response: %v", err) |
| c.SetReadDeadline(t) | ||
| c.SetWriteDeadline(t) | ||
| return nil |
There was a problem hiding this comment.
The SetDeadline method calls SetReadDeadline and SetWriteDeadline but doesn't return any errors from these calls. While SetWriteDeadline is documented as not being used, if SetReadDeadline ever returns an error in the future, it would be silently ignored. Consider returning the error from SetReadDeadline or documenting why errors are intentionally ignored.
| c.SetReadDeadline(t) | |
| c.SetWriteDeadline(t) | |
| return nil | |
| if err := c.SetReadDeadline(t); err != nil { | |
| return err | |
| } | |
| return c.SetWriteDeadline(t) |
| c.SetReadDeadline(t) | ||
| c.SetWriteDeadline(t) | ||
| return nil |
There was a problem hiding this comment.
This expression has no effect.
| c.SetReadDeadline(t) | |
| c.SetWriteDeadline(t) | |
| return nil | |
| if err := c.SetReadDeadline(t); err != nil { | |
| return err | |
| } | |
| return c.SetWriteDeadline(t) |
1 participant
Implement UDP tunneling over HTTP/3 using HTTP Datagrams (RFC 9297):
Resource management and connection caching:
I used go-gost/gost#831 for testing.