-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[proxy/vmess] Fix UDP over TCP fragmentation in VMESS + zero security #2337
Conversation
Codecov ReportBase: 39.21% // Head: 39.16% // Decreases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #2337 +/- ##
==========================================
- Coverage 39.21% 39.16% -0.05%
==========================================
Files 617 618 +1
Lines 36859 36910 +51
==========================================
+ Hits 14454 14457 +3
- Misses 20816 20864 +48
Partials 1589 1589
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Since this is a draft PR introducing I think usage of
By processing
Above thoughts requires a major refactoring of dispatcher app, which is exactly what I've done after prior DNS updates. By taking |
Firstly, thanks for your work on this. I will review and merge its dependent PR first and have a look if there is a nice way to improve this pull request. As for your suggestion on moving the packet addr too dispatcher, I would like to have more discussion before advise you to proceed:
|
what about implement |
I recommend merging this PR, although it may be broken, this feature is not working properly. |
It has been open 120 days with no activity. Remove stale label or comment or this will be closed in 5 days |
It has been open 120 days with no activity. Remove stale label or comment or this will be closed in 5 days |
To make things easier, this PR is only scoped in VMESS zero security.
UDP proxying for VMESS zero is problematic, because VMESS zero does not attach any header to UDP-over-TCP traffic, causing source UDP packets to be fragmented into multiple packets, or merged into one packet.
Socks5 prefixed UDP packet with destination header, Trojan prefixed UDP packet with destination and length header. Considering
packetaddr
already does the job to prefix UDP packet with destination header, this PR implements a similar utility calledpacketstream
, to make UDP packets correctly proxied on a stream by prefixing a length header.When VMESS zero security is proxying UDP traffic, the
PacketStreamWriter
andPacketStreamReader
will be used to correctly encode and recover UDP packets from the TCP stream.This PR relies on #2336.
This PR is a draft, because I replaced the writer and reader globally for VMESS zero security, which is a breaking change for old servers and clients. I think it could be solved by introducing options or version number somewhere in the request header, but recently I don't have much time in proposing a mature solution, so I just enabled globally and adapted in both my clients and servers, and leave this draft for community to come up with a best compatible solution.