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

Socks encoder/decoder do not always follow RFC regarding port #13426

Closed
laurentgo opened this issue Jun 5, 2023 · 2 comments · Fixed by #13427
Closed

Socks encoder/decoder do not always follow RFC regarding port #13426

laurentgo opened this issue Jun 5, 2023 · 2 comments · Fixed by #13427
Milestone

Comments

@laurentgo
Copy link
Contributor

Expected behavior

According to RFC1918 regarding Socks5 proxy protocol, destination port must be represented in network byte order. I didn't find a similar information for Socks4 proxy implementations but I would think the requirement is the same (and it seems that OpenJDK socks implementation follow that convention as well: https://github.com/openjdk/jdk/blob/5cd8af7622a93afb32f5f3fccdc453096992453c/test/jdk/java/net/Socks/SocksServer.java#L200)

Netty implementation should also obey the requirement, independently of the buffer internal order

Actual behavior

When an application uses byte buffers with little endian order, Netty socks encoder/decoder do not write the port information correctly, resulting in refused connection for example.

Steps to reproduce

0/ Setup a socks proxy (using ssh -D or mitmproxy)
1/ Write a small pipeline to connect to a remote system with a socks proxy handler
2/ Configure the pipeline to use an allocator to generate LE byte buffers
3/ Try and attempt to connect to the remote system

Minimal yet complete reproducer code (or URL to code)

Netty version

4.1.89

JVM version (e.g. java -version)

openjdk version "11.0.18" 2023-01-17
OpenJDK Runtime Environment Temurin-11.0.18+10 (build 11.0.18+10)
OpenJDK 64-Bit Server VM Temurin-11.0.18+10 (build 11.0.18+10, mixed mode)

OS version (e.g. uname -a)

Darwin xxx.local 22.5.0 Darwin Kernel Version 22.5.0: Mon Apr 24 20:53:19 PDT 2023; root:xnu-8796.121.2~5/RELEASE_ARM64_T6020 arm64

@normanmaurer
Copy link
Member

We love PRs

@laurentgo
Copy link
Contributor Author

And I love writing them :) I guess I'm trying to understand the existing unit tests used here and how I can create an EmbeddedChannel with the proper byte buffer ordering (Advice welcome)

@normanmaurer normanmaurer added this to the 4.1.94.Final milestone Jun 7, 2023
normanmaurer pushed a commit that referenced this issue Jun 9, 2023
…codecs (#13427)

Motivation:

Socks codecs do not always use network byte ordering when
encoding/decoding socks messages resulting in connection issues

Modification:

Use BE methods to properly read/write socks messages

Result:

Fixes #13426
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 a pull request may close this issue.

2 participants