Skip to content
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

Fix: socks5 usernames and passwords can BOTH be up to 255 bytes #343

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

Yawning
Copy link
Contributor

@Yawning Yawning commented Feb 28, 2024

This should be rather self-explanatory. The existing authentication routine limits the entire username/password request to 255 bytes, however that limit applies to the UNAME/PASSWD fields individually. My PR also enforces the RFC specified minimum of 1 byte.

https://github.com/xjasonlyu/tun2socks/blob/main/transport/socks5/rfc1929.txt#L40

2.  Initial negotiation

   Once the SOCKS V5 server has started, and the client has selected the
   Username/Password Authentication protocol, the Username/Password
   subnegotiation begins.  This begins with the client producing a
   Username/Password request:

           +----+------+----------+------+----------+
           |VER | ULEN |  UNAME   | PLEN |  PASSWD  |
           +----+------+----------+------+----------+
           | 1  |  1   | 1 to 255 |  1   | 1 to 255 |
           +----+------+----------+------+----------+

@xjasonlyu xjasonlyu self-requested a review February 28, 2024 02:11
@xjasonlyu xjasonlyu added the enhancement New feature or request label Feb 28, 2024
Copy link
Owner

@xjasonlyu xjasonlyu left a comment

Choose a reason for hiding this comment

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

Thanks for your PR!

Just some minor stuff:

transport/socks5/socks5.go Outdated Show resolved Hide resolved
@xjasonlyu xjasonlyu merged commit 8c7c908 into xjasonlyu:main Feb 28, 2024
1 check passed
@xjasonlyu
Copy link
Owner

Merged, thanks!

@Yawning Yawning deleted the fix/socks5-auth-validation branch February 28, 2024 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants