You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2018/08/26 05:51:46 UTC

[4/6] mina-sshd git commit: [SSHD-839] Use correct bound address to unbind local tunnel endpoint

[SSHD-839] Use correct bound address to unbind local tunnel endpoint


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/463658a9
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/463658a9
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/463658a9

Branch: refs/heads/master
Commit: 463658a945a75251d455da35374fbde0402877c8
Parents: aa7547b
Author: Goldstein Lyor <ly...@cb4.com>
Authored: Wed Aug 22 14:10:25 2018 +0300
Committer: Goldstein Lyor <ly...@cb4.com>
Committed: Sun Aug 26 08:50:42 2018 +0300

----------------------------------------------------------------------
 .../common/forward/DefaultForwardingFilter.java | 60 +++++++++++++-------
 .../sshd/common/util/net/SshdSocketAddress.java |  9 +++
 2 files changed, 47 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/463658a9/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwardingFilter.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwardingFilter.java b/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwardingFilter.java
index 3e87278..42fdce0 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwardingFilter.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwardingFilter.java
@@ -95,7 +95,10 @@ public class DefaultForwardingFilter
     private final ConnectionService service;
     private final IoHandlerFactory socksProxyIoHandlerFactory = () -> new SocksProxy(getConnectionService());
     private final Session sessionInstance;
+    private final Object localLock = new Object();
     private final Map<Integer, SshdSocketAddress> localToRemote = new TreeMap<>(Comparator.naturalOrder());
+    private final Map<Integer, InetSocketAddress> boundLocals = new TreeMap<>(Comparator.naturalOrder());
+
     private final Map<Integer, SshdSocketAddress> remoteToLocal = new TreeMap<>(Comparator.naturalOrder());
     private final Map<Integer, SocksProxy> dynamicLocal = new TreeMap<>(Comparator.naturalOrder());
     private final Set<LocalForwardingEntry> localForwards = new HashSet<>();
@@ -191,23 +194,29 @@ public class DefaultForwardingFilter
             throw new IllegalStateException("TcpipForwarder is closing");
         }
 
-        InetSocketAddress bound;
+        InetSocketAddress bound = null;
         int port;
         signalEstablishingExplicitTunnel(local, remote, true);
         try {
             bound = doBind(local, staticIoHandlerFactory);
             port = bound.getPort();
-            SshdSocketAddress prev;
-            synchronized (localToRemote) {
-                prev = localToRemote.put(port, remote);
-            }
+            synchronized (localLock) {
+                SshdSocketAddress prevRemote = localToRemote.get(port);
+                if (prevRemote != null) {
+                    throw new IOException("Multiple local port forwarding addressing on port=" + port + ": current=" + remote + ", previous=" + prevRemote);
+                }
 
-            if (prev != null) {
-                throw new IOException("Multiple local port forwarding bindings on port=" + port + ": current=" + remote + ", previous=" + prev);
+                InetSocketAddress prevBound = boundLocals.get(port);
+                if (prevBound != null) {
+                    throw new IOException("Multiple local port forwarding bindings on port=" + port + ": current=" + bound + ", previous=" + prevBound);
+                }
+
+                localToRemote.put(port, remote);
+                boundLocals.put(port, bound);
             }
         } catch (IOException | RuntimeException e) {
             try {
-                stopLocalPortForwarding(local);
+                unbindLocalForwarding(local, remote, bound);
             } catch (IOException | RuntimeException err) {
                 e.addSuppressed(err);
             }
@@ -232,29 +241,36 @@ public class DefaultForwardingFilter
     public synchronized void stopLocalPortForwarding(SshdSocketAddress local) throws IOException {
         Objects.requireNonNull(local, "Local address is null");
 
-        SshdSocketAddress bound;
+        SshdSocketAddress remote;
+        InetSocketAddress bound;
         int port = local.getPort();
-        synchronized (localToRemote) {
-            bound = localToRemote.remove(port);
+        synchronized (localLock) {
+            remote = localToRemote.remove(port);
+            bound = boundLocals.remove(port);
         }
 
+        unbindLocalForwarding(local, remote, bound);
+    }
+
+    protected void unbindLocalForwarding(SshdSocketAddress local, SshdSocketAddress remote, InetSocketAddress bound) throws IOException {
         if ((bound != null) && (acceptor != null)) {
             if (log.isDebugEnabled()) {
-                log.debug("stopLocalPortForwarding(" + local + ") unbind " + bound);
+                log.debug("unbindLocalForwarding(" + local + " => " + remote + ") unbind " + bound);
             }
 
-            signalTearingDownExplicitTunnel(bound, true);
+            SshdSocketAddress boundAddress = new SshdSocketAddress(bound);
+            signalTearingDownExplicitTunnel(boundAddress, true);
             try {
-                acceptor.unbind(bound.toInetSocketAddress());
+                acceptor.unbind(bound);
             } catch (RuntimeException e) {
-                signalTornDownExplicitTunnel(bound, true, e);
+                signalTornDownExplicitTunnel(boundAddress, true, e);
                 throw e;
             }
 
-            signalTornDownExplicitTunnel(bound, true, null);
+            signalTornDownExplicitTunnel(boundAddress, true, null);
         } else {
             if (log.isDebugEnabled()) {
-                log.debug("stopLocalPortForwarding(" + local + ") no mapping/acceptor for " + bound);
+                log.debug("unbindLocalForwarding(" + local + " => " + remote + ") no mapping/acceptor for " + bound);
             }
         }
     }
@@ -284,14 +300,14 @@ public class DefaultForwardingFilter
             }
             port = (remotePort == 0) ? result.getInt() : remote.getPort();
             // TODO: Is it really safe to only store the local address after the request ?
-            SshdSocketAddress prev;
             synchronized (remoteToLocal) {
-                prev = remoteToLocal.put(port, local);
+                SshdSocketAddress prev = remoteToLocal.get(port);
+                if (prev != null) {
+                    throw new IOException("Multiple remote port forwarding bindings on port=" + port + ": current=" + remote + ", previous=" + prev);
+                }
+                remoteToLocal.put(port, local);
             }
 
-            if (prev != null) {
-                throw new IOException("Multiple remote port forwarding bindings on port=" + port + ": current=" + remote + ", previous=" + prev);
-            }
         } catch (IOException | RuntimeException e) {
             try {
                 stopRemotePortForwarding(remote);

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/463658a9/sshd-core/src/main/java/org/apache/sshd/common/util/net/SshdSocketAddress.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/util/net/SshdSocketAddress.java b/sshd-core/src/main/java/org/apache/sshd/common/util/net/SshdSocketAddress.java
index 3a40d20..b465d14 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/util/net/SshdSocketAddress.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/util/net/SshdSocketAddress.java
@@ -144,6 +144,15 @@ public class SshdSocketAddress extends SocketAddress {
         this(IPV4_ANYADDR, port);
     }
 
+    public SshdSocketAddress(InetSocketAddress addr) {
+        Objects.requireNonNull(addr, "No address provided");
+
+        String host = addr.getHostString();
+        hostName = GenericUtils.isEmpty(host) ? IPV4_ANYADDR : host;
+        port = addr.getPort();
+        ValidateUtils.checkTrue(port >= 0, "Port must be >= 0: %d", port);
+    }
+
     public SshdSocketAddress(String hostName, int port) {
         Objects.requireNonNull(hostName, "Host name may not be null");
         this.hostName = GenericUtils.isEmpty(hostName) ? IPV4_ANYADDR : hostName;