From b468a298816a61f52785d80f4272db4109c1c511 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Tue, 19 Aug 2014 18:21:07 +0200 Subject: [PATCH] Improve Socks5 Bytestreams - determine all local IPv4 and IPv6 addresses - prevent loopback addresses from appearing as streamhost Some unit tests where changed because they assumed that a host only has one local address. But nowadays hosts often have more, at least because they are IPv4 and IPv6 multi-homed. --- .../socks5/Socks5BytestreamManager.java | 42 +++++++----- .../bytestreams/socks5/Socks5Proxy.java | 65 +++++++++++++------ .../bytestreams/socks5/packet/Bytestream.java | 5 +- .../socks5/Socks5ByteStreamManagerTest.java | 5 +- .../bytestreams/socks5/Socks5ProxyTest.java | 10 ++- 5 files changed, 82 insertions(+), 45 deletions(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java index 06ad8bfc7..d70848e73 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java @@ -652,24 +652,34 @@ public final class Socks5BytestreamManager implements BytestreamManager { // get local proxy singleton Socks5Proxy socks5Server = Socks5Proxy.getSocks5Proxy(); - if (socks5Server.isRunning()) { - List addresses = socks5Server.getLocalAddresses(); - int port = socks5Server.getPort(); - - if (addresses.size() >= 1) { - List streamHosts = new ArrayList(); - for (String address : addresses) { - StreamHost streamHost = new StreamHost(this.connection.getUser(), address); - streamHost.setPort(port); - streamHosts.add(streamHost); - } - return streamHosts; - } - + if (!socks5Server.isRunning()) { + // server is not running + return null; } + List addresses = socks5Server.getLocalAddresses(); + if (addresses.isEmpty()) { + // local address could not be determined + return null; + } + final int port = socks5Server.getPort(); - // server is not running or local address could not be determined - return null; + List streamHosts = new ArrayList(); + outerloop: for (String address : addresses) { + // Prevent loopback addresses from appearing as streamhost + final String[] loopbackAddresses = { "127.0.0.1", "0:0:0:0:0:0:0:1" }; + for (String loopbackAddress : loopbackAddresses) { + // Use 'startsWith' here since IPv6 addresses may have scope ID, + // ie. the part after the '%' sign. + if (address.startsWith(loopbackAddress)) { + continue outerloop; + } + } + StreamHost streamHost = new StreamHost(this.connection.getUser(), + address); + streamHost.setPort(port); + streamHosts.add(streamHost); + } + return streamHosts; } /** diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5Proxy.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5Proxy.java index 855cba222..152922300 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5Proxy.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5Proxy.java @@ -20,12 +20,14 @@ import java.io.DataInputStream; import java.io.DataOutputStream; import java.io.IOException; import java.net.InetAddress; +import java.net.NetworkInterface; import java.net.ServerSocket; import java.net.Socket; import java.net.SocketException; -import java.net.UnknownHostException; -import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; +import java.util.Enumeration; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -47,7 +49,7 @@ import org.jivesoftware.smack.SmackException; *

* If your application is running on a machine with multiple network interfaces or if you want to * provide your public address in case you are behind a NAT router, invoke - * {@link #addLocalAddress(String)} or {@link #replaceLocalAddresses(List)} to modify the list of + * {@link #addLocalAddress(String)} or {@link #replaceLocalAddresses(Collection)} to modify the list of * local network addresses used for outgoing SOCKS5 Bytestream requests. *

* The local SOCKS5 proxy server refuses all connections except the ones that are explicitly allowed @@ -95,7 +97,7 @@ public class Socks5Proxy { /* list of digests connections should be stored */ private final List allowedConnections = Collections.synchronizedList(new LinkedList()); - private final Set localAddresses = Collections.synchronizedSet(new LinkedHashSet()); + private final Set localAddresses = new LinkedHashSet(4); /** * Private constructor. @@ -103,14 +105,27 @@ public class Socks5Proxy { private Socks5Proxy() { this.serverProcess = new Socks5ServerProcess(); - // add default local address + Enumeration networkInterfaces; try { - this.localAddresses.add(InetAddress.getLocalHost().getHostAddress()); + networkInterfaces = NetworkInterface.getNetworkInterfaces(); + } catch (SocketException e) { + throw new IllegalStateException(e); } - catch (UnknownHostException e) { - // do nothing + Set localHostAddresses = new HashSet(); + for (NetworkInterface networkInterface : Collections.list(networkInterfaces)) { + // We can't use NetworkInterface.getInterfaceAddresses here, which + // would return a List instead the deprecated Enumeration, because + // it's Android API 9 and Smack currently uses 8. Change that when + // we raise Smack's minimum Android API. + Enumeration inetAddresses = networkInterface.getInetAddresses(); + for (InetAddress address : Collections.list(inetAddresses)) { + localHostAddresses.add(address.getHostAddress()); + } } - + if (localHostAddresses.isEmpty()) { + throw new IllegalStateException("Could not determine any local host address"); + } + replaceLocalAddresses(localHostAddresses); } /** @@ -243,15 +258,17 @@ public class Socks5Proxy { *

* Note that the list of addresses initially contains the address returned by * InetAddress.getLocalHost().getHostAddress(). You can replace the list of - * addresses by invoking {@link #replaceLocalAddresses(List)}. + * addresses by invoking {@link #replaceLocalAddresses(Collection)}. * * @param address the local network address to add */ public void addLocalAddress(String address) { if (address == null) { - throw new IllegalArgumentException("address may not be null"); + return; + } + synchronized (localAddresses) { + this.localAddresses.add(address); } - this.localAddresses.add(address); } /** @@ -259,19 +276,24 @@ public class Socks5Proxy { * longer be used of outgoing SOCKS5 Bytestream requests. * * @param address the local network address to remove + * @return true if the address was removed. */ - public void removeLocalAddress(String address) { - this.localAddresses.remove(address); + public boolean removeLocalAddress(String address) { + synchronized(localAddresses) { + return localAddresses.remove(address); + } } /** - * Returns an unmodifiable list of the local network addresses that will be used for streamhost + * Returns an set of the local network addresses that will be used for streamhost * candidates of outgoing SOCKS5 Bytestream requests. * - * @return unmodifiable list of the local network addresses + * @return set of the local network addresses */ public List getLocalAddresses() { - return Collections.unmodifiableList(new ArrayList(this.localAddresses)); + synchronized (localAddresses) { + return new LinkedList(localAddresses); + } } /** @@ -284,13 +306,14 @@ public class Socks5Proxy { * * @param addresses the new list of local network addresses */ - public void replaceLocalAddresses(List addresses) { + public void replaceLocalAddresses(Collection addresses) { if (addresses == null) { throw new IllegalArgumentException("list must not be null"); } - this.localAddresses.clear(); - this.localAddresses.addAll(addresses); - + synchronized(localAddresses) { + localAddresses.clear(); + localAddresses.addAll(addresses); + } } /** diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/packet/Bytestream.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/packet/Bytestream.java index 4f9b70584..9d8879c6f 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/packet/Bytestream.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/packet/Bytestream.java @@ -17,7 +17,6 @@ package org.jivesoftware.smackx.bytestreams.socks5.packet; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.List; @@ -145,8 +144,8 @@ public class Bytestream extends IQ { * * @return Returns the list of stream hosts contained in the packet. */ - public Collection getStreamHosts() { - return Collections.unmodifiableCollection(streamHosts); + public List getStreamHosts() { + return Collections.unmodifiableList(streamHosts); } /** diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java index 17ecee4b6..1a06159e5 100644 --- a/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java @@ -799,10 +799,9 @@ public class Socks5ByteStreamManagerTest { public void verify(Bytestream request, Bytestream response) { assertEquals(response.getSessionID(), request.getSessionID()); - assertEquals(2, request.getStreamHosts().size()); - StreamHost streamHost1 = (StreamHost) request.getStreamHosts().toArray()[0]; + StreamHost streamHost1 = request.getStreamHosts().get(0); assertEquals(response.getUsedHost().getJID(), streamHost1.getJID()); - StreamHost streamHost2 = (StreamHost) request.getStreamHosts().toArray()[1]; + StreamHost streamHost2 = request.getStreamHosts().get(request.getStreamHosts().size() - 1); assertEquals(response.getUsedHost().getJID(), streamHost2.getJID()); assertEquals("localAddress", streamHost2.getAddress()); } diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ProxyTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ProxyTest.java index 5e2a2deed..3a2cb5134 100644 --- a/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ProxyTest.java +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ProxyTest.java @@ -30,6 +30,7 @@ import java.net.Socket; import java.net.SocketException; import java.net.UnknownHostException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import org.junit.After; @@ -144,7 +145,13 @@ public class Socks5ProxyTest { proxy.addLocalAddress("same"); proxy.addLocalAddress("same"); - assertEquals(2, proxy.getLocalAddresses().size()); + int sameCount = 0; + for(String localAddress : proxy.getLocalAddresses()) { + if ("same".equals(localAddress)) { + sameCount++; + } + } + assertEquals(1, sameCount); } /** @@ -297,7 +304,6 @@ public class Socks5ProxyTest { proxy.start(); assertTrue(proxy.isRunning()); - String digest = new String(new byte[] { (byte) 0xAA }); // add digest to allow connection