Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 28, 2024
Merged

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

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
xjasonlyu
xjasonlyu requested changes Feb 28, 2024
View reviewed changes
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:

@Yawning Yawning force-pushed the fix/socks5-auth-validation branch from ac97c0e to c668120 Compare February 28, 2024 02:21
@Yawning Yawning force-pushed the fix/socks5-auth-validation branch from c668120 to 39d90f8 Compare February 28, 2024 02:25
@xjasonlyu xjasonlyu merged commit 8c7c908 into xjasonlyu:main Feb 28, 2024
@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
Reviewers

@xjasonlyu xjasonlyu xjasonlyu requested changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Yawning @xjasonlyu