Hello,
I would like to work on bug 8028776: Java socks proxy to resolve DNS remotely,
and did an analysis of the current java.net code.
After calling url.openConnection(proxy).getInputStream(), DNS resolution takes
place when a new InetSocketAddress is created in NetworkClient.java lines
176-184:
if (connectTimeout >= 0) {
s.connect(new InetSocketAddress(server, port), connectTimeout);
} else {
if (defaultConnectTimeout > 0) {
s.connect(new InetSocketAddress(server, port), defaultConnectTimeout);
} else {
s.connect(new InetSocketAddress(server, port));
}
}
As you can see, in three cases a new InetSocketAddress is created, and this
constructor resolves the domain name.
My First idea was to get an unresolved InetSocketAddress when SOCKS was used:
InetSocketAddress targetAddress = proxy.type() == Proxy.Type.SOCKS ?
InetSocketAddress.createUnresolved(server, port) :
new InetSocketAddress(server, port);
if (connectTimeout >= 0) {
s.connect(targetAddress, connectTimeout);
} else {
if (defaultConnectTimeout > 0) {
s.connect(targetAddress, defaultConnectTimeout);
} else {
s.connect(targetAddress);
}
}
Then, SocksSocketImpl just had to send the domain name for unresolved
endpoints. Luckily, this has already been implemented in SocksSocketImpl.java,
lines 416-421:
if (epoint.isUnresolved()) {
out.write(DOMAIN_NAME);
out.write(epoint.getHostName().length());
out.write(epoint.getHostName().getBytes(StandardCharsets.ISO_8859_1));
out.write((epoint.getPort() >> 8) & 0xff);
out.write((epoint.getPort() >> 0) & 0xff);
While this works flawlessly for SOCKS v5 proxies, this won't work for SOCKS v4.
Proxying DNS is not possible with SOCKS v4, so DNS should still be resolved for
SOCKS v4. Unfortunately, there are several caveats:
* While the SocksProxy class has a protocolVersion property, this property
cannot be read at the location of the change above, because URL.openConnection
creates a new Proxy of type ApplicationProxy. According to the comment, this is
as a security measure. This copy is not a full copy, because it lacks the
SocksProxy protocolVersion property.
* SocksSocketImpl.useV4 (which determines whether to use v4 or v5) does not
only check the proxy type (which AFAIK never is SocksProxy), but also
DefaultProxySelector.socksProxyVersion() == 4 (line 74). While convenient,
there could be a difference between the socksProxyVersion in the SocksProxy
object that was originally created, and the version in the system property
(even though the SocksProxy property cannot currently be set programmatically).
* The change in NetworkClient.java above could use the same trick, to check
DefaultProxySelector.socksProxyVersion(). However, lines 397-406 prevent this.
Starting at line 389:
// This is SOCKS V5
out.write(PROTO_VERS);
out.write(2);
out.write(NO_AUTH);
out.write(USER_PASSW);
out.flush();
byte[] data = new byte[2];
int i = readSocksReply(in, data, deadlineMillis);
if (i != 2 || ((int)data[0]) != PROTO_VERS) {
// Maybe it's not a V5 sever after all
// Let's try V4 before we give up
// SOCKS Protocol version 4 doesn't know how to deal with
// DOMAIN type of addresses (unresolved addresses here)
if (epoint.isUnresolved())
throw new UnknownHostException(epoint.toString());
connectV4(in, out, epoint, deadlineMillis);
return;
}
So even if SOCKS v5 is configured, SocksSocketImpl tries v4 if v5 fails, and
then v4 will always fail because the address is unresolved. The fallback from
v5 to v4 breaks the first idea.
Before I found out that the proxy object was changed into an ApplicationProxy
object, I thought about adding a 'proxyDns' property to the SocksProxy class.
When this property would be explicitly set to true, proxying DNS is expected,
so SOCKS v5 is mandatory. In this case, it wouldn't matter if the fallback
would throw an exception. However, the SocksProxy information disappears in the
ApplicationProxy copy, and as far as I see, there is no easy way to create a
SocksProxy with the right version and a new property from code, without using
system properties and the ProxySelector. Maybe a ProxyFactory could help out,
but such a class does not exist.
Currently, the version of SOCKS can only be set in system property
socksProxyVersion. We could introduce a new system property socksProxyDns,
which tells NetworkClient to proxy DNS when using SOCKS, like this:
private static final String SOCKS_PROXY_DNS = "socksProxyDns";
public static boolean socksProxyDns() {
Boolean val = AccessController.doPrivileged(
new PrivilegedAction<Boolean>() {
@Override public Boolean run() {
return NetProperties.getBoolean(SOCKS_PROXY_DNS);
}
});
return val == null ? false : val;
}
Again, NetworkClient.java lines 176-184, now checking the new property:
InetSocketAddress targetAddress = proxy.type() == Proxy.Type.SOCKS &&
socksProxyDns() ?
InetSocketAddress.createUnresolved(server, port) :
new InetSocketAddress(server, port);
if (connectTimeout >= 0) {
s.connect(targetAddress, connectTimeout);
} else {
if (defaultConnectTimeout > 0) {
s.connect(targetAddress, defaultConnectTimeout);
} else {
s.connect(targetAddress);
}
}
This works, and compatibility-wise, I think this is almost risk free. But from
an application programmer's point of view, I'd prefer a way to specify the
SOCKS version and DNS proxying in type safe code instead of a system property.
What would be the preferred solution? What can I do next?
I already prepared some jtreg tests to test the proxy (SOCKS protocol is very
easy to test). However, I'd like to guarantee that no local DNS query is made
when SOCKS v5 with DNS proxying is enabled, and that a local DNS query is made
when DNS proxying is not enabled. Unfortunately, I didn't find a clean way to
replace the default NameService implementation. I tested this with a
NetworkService class implementing an InvocationHandler, which I assigned to the
static nameService property of InetAddress. Even though this currently works,
it may not work in a future release:
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access to field java.net.InetAddress.nameService
WARNING: Please consider reporting this to the maintainers of this class
WARNING: Use --illegal-access=warn to enable warnings of further illegal
reflective access operations
WARNING: All illegal access operations will be denied in a future release
Does anyone have any suggestions to replace the NameService in test code?
-Josh
