From ffd027cc7deaaff0af70dfee4f0765cdce78d9e3 Mon Sep 17 00:00:00 2001 From: Martin Fidczuk Date: Mon, 15 Aug 2022 16:51:22 +0100 Subject: [PATCH] Use XMPP connection as local SOCKS5 address The default local address is often just "the first address found in the list of addresses read from the OS" and this might mean an internal IP address that cannot reach external servers. So wherever possible use the same IP address being used to connect to the XMPP server because this local address has a better chance of being suitable. This MR adds the above behaviour, and two UTs to test that we use the local XMPP connection IP when connected, and the previous behaviour when not. --- .../smack/bosh/XMPPBOSHConnection.java | 6 ++ .../jivesoftware/smack/XMPPConnection.java | 9 +++ .../ModularXmppClientToServerConnection.java | 6 ++ .../jivesoftware/smack/DummyConnection.java | 6 ++ .../socks5/Socks5BytestreamManager.java | 12 +++- .../socks5/Socks5ByteStreamManagerTest.java | 70 +++++++++++++++++++ .../smack/tcp/XMPPTCPConnection.java | 16 +++++ 7 files changed, 124 insertions(+), 1 deletion(-) diff --git a/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/XMPPBOSHConnection.java b/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/XMPPBOSHConnection.java index 8dff0e6e8..14d7c0e90 100644 --- a/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/XMPPBOSHConnection.java +++ b/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/XMPPBOSHConnection.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.io.PipedReader; import java.io.PipedWriter; import java.io.Writer; +import java.net.InetAddress; import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; @@ -314,6 +315,11 @@ public class XMPPBOSHConnection extends AbstractXMPPConnection { } } + @Override + public InetAddress getLocalAddress() { + return null; + } + @Override protected void shutdown() { instantShutdown(); diff --git a/smack-core/src/main/java/org/jivesoftware/smack/XMPPConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/XMPPConnection.java index 6b6c40334..0ef97910a 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/XMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/XMPPConnection.java @@ -16,6 +16,7 @@ */ package org.jivesoftware.smack; +import java.net.InetAddress; import java.util.concurrent.TimeUnit; import javax.xml.namespace.QName; @@ -160,6 +161,14 @@ public interface XMPPConnection { */ EntityFullJid getUser(); + /** + * Returns the local address currently in use for this connection, or null if + * this is invalid for the type of underlying connection. + * + * @return the local address currently in use for this connection + */ + InetAddress getLocalAddress(); + /** * Returns the stream ID for this connection, which is the value set by the server * when opening an XMPP stream. This value will be null if not connected to the server. diff --git a/smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnection.java index eb37ca60c..b24b00704 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnection.java @@ -17,6 +17,7 @@ package org.jivesoftware.smack.c2s; import java.io.IOException; +import java.net.InetAddress; import java.security.cert.CertificateException; import java.util.ArrayList; import java.util.Collection; @@ -1128,6 +1129,11 @@ public final class ModularXmppClientToServerConnection extends AbstractXMPPConne walkStateGraph(walkStateGraphContext); } + @Override + public InetAddress getLocalAddress() { + return null; + } + private Map getFilterStats() { Collection filters; synchronized (this) { diff --git a/smack-core/src/testFixtures/java/org/jivesoftware/smack/DummyConnection.java b/smack-core/src/testFixtures/java/org/jivesoftware/smack/DummyConnection.java index b85ce62b5..7e105d58b 100644 --- a/smack-core/src/testFixtures/java/org/jivesoftware/smack/DummyConnection.java +++ b/smack-core/src/testFixtures/java/org/jivesoftware/smack/DummyConnection.java @@ -17,6 +17,7 @@ package org.jivesoftware.smack; import java.io.IOException; +import java.net.InetAddress; import java.util.Date; import java.util.Random; import java.util.concurrent.BlockingQueue; @@ -139,6 +140,11 @@ public class DummyConnection extends AbstractXMPPConnection { } } + @Override + public InetAddress getLocalAddress() { + return null; + } + /** * Returns the number of packets that's sent through {@link #sendStanza(Stanza)} and * that has not been returned by {@link #getSentPacket()}. 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 66e221c49..673f7a989 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 @@ -656,13 +656,23 @@ public final class Socks5BytestreamManager extends Manager implements Bytestream */ public List getLocalStreamHost() { // Ensure that the local SOCKS5 proxy is running (if enabled). - Socks5Proxy.getSocks5Proxy(); + Socks5Proxy socks5Proxy = Socks5Proxy.getSocks5Proxy(); List streamHosts = new ArrayList<>(); XMPPConnection connection = connection(); EntityFullJid myJid = connection.getUser(); + // The default local address is often just 'the first address found in the + // list of addresses read from the OS' and this might mean an internal + // IP address that cannot reach external servers. So wherever possible + // use the same IP address being used to connect to the XMPP server + // because this local address has a better chance of being suitable. + InetAddress xmppLocalAddress = connection.getLocalAddress(); + if (xmppLocalAddress != null) { + socks5Proxy.replaceLocalAddresses(Collections.singletonList(xmppLocalAddress)); + } + for (Socks5Proxy socks5Server : Socks5Proxy.getRunningProxies()) { List addresses = socks5Server.getLocalAddresses(); if (addresses.isEmpty()) { 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 cfa59197a..00e721e5b 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 @@ -24,12 +24,16 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.net.Inet4Address; import java.net.InetAddress; import java.net.ServerSocket; +import java.net.UnknownHostException; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeoutException; @@ -1002,6 +1006,72 @@ public class Socks5ByteStreamManagerTest { protocol.verifyAll(); } + /** + * Invoking {@link Socks5BytestreamManager#getLocalStreamHost()} should return only a local address + * from XMPP connection when it is connected and has a socket with a bound non-localhost IP address. + * + * @throws InterruptedException if the calling thread was interrupted. + * @throws SmackException if Smack detected an exceptional situation. + * @throws XMPPErrorException if an XMPP protocol error was received. + */ + @Test + public void shouldUseXMPPConnectionLocalAddressWhenConnected() throws InterruptedException, XMPPErrorException, SmackException { + final Protocol protocol = new Protocol(); + final XMPPConnection connection = ConnectionUtils.createMockedConnection(protocol, initiatorJID); + + // prepare XMPP local address + Inet4Address xmppLocalAddress = mock(Inet4Address.class); + when(xmppLocalAddress.getHostAddress()).thenReturn("81.72.63.54"); + when(connection.getLocalAddress()).thenReturn(xmppLocalAddress); + + // get Socks5ByteStreamManager for connection + Socks5BytestreamManager byteStreamManager = Socks5BytestreamManager.getBytestreamManager(connection); + + List localStreamHost = byteStreamManager.getLocalStreamHost(); + + // must be only 1 stream host with XMPP local address IP + assertEquals(1, localStreamHost.size()); + assertEquals("81.72.63.54", localStreamHost.get(0).getAddress().toString()); + assertEquals(initiatorJID, localStreamHost.get(0).getJID()); + } + + /** + * Invoking {@link Socks5BytestreamManager#getLocalStreamHost()} should return all non-localhost + * local addresses when its XMPP connection's socket is null. + * + * @throws InterruptedException if the calling thread was interrupted. + * @throws SmackException if Smack detected an exceptional situation. + * @throws XMPPErrorException if an XMPP protocol error was received. + * @throws UnknownHostException if address cannot be resolved. + */ + @Test + public void shouldUseSocks5LocalAddressesWhenNotConnected() throws InterruptedException, XMPPErrorException, SmackException, UnknownHostException { + final Protocol protocol = new Protocol(); + final XMPPConnection connection = ConnectionUtils.createMockedConnection(protocol, initiatorJID); + + // No XMPP local address + when(connection.getLocalAddress()).thenReturn(null); + + // get Socks5ByteStreamManager for connection + Socks5BytestreamManager byteStreamManager = Socks5BytestreamManager.getBytestreamManager(connection); + + List localAddresses = new ArrayList<>(); + for (InetAddress inetAddress : Socks5Proxy.getSocks5Proxy().getLocalAddresses()) { + if (!inetAddress.isLoopbackAddress()) { + localAddresses.add(inetAddress); + } + } + + List localStreamHost = byteStreamManager.getLocalStreamHost(); + + // Must be the same addresses as in SOCKS5 proxy local address list (excluding loopback) + assertEquals(localAddresses.size(), localStreamHost.size()); + for (StreamHost streamHost : localStreamHost) { + assertTrue(localAddresses.contains(streamHost.getAddress().asInetAddress())); + assertEquals(initiatorJID, streamHost.getJID()); + } + } + private static void createResponses(Protocol protocol, String sessionID, Verification streamHostUsedVerification, Socks5TestProxy socks5TestProxy) throws XmppStringprepException { diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index e093ce1a6..38d2d3e55 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -1938,4 +1938,20 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { this.bundleAndDeferCallback = bundleAndDeferCallback; } + + /** + * Returns the local address currently in use for this connection. + * + * @return the local address + */ + @Override + public InetAddress getLocalAddress() { + final Socket socket = this.socket; + if (socket == null) return null; + + InetAddress localAddress = socket.getLocalAddress(); + if (localAddress.isAnyLocalAddress()) return null; + + return localAddress; + } }