Reject multiple sessions with the same host key

SSH host key is supposed to be unique among devices. If there is an
attempt to establish a session with the same key, terminate the new
session.

Since that rules out the possibility of multiple remotes for a key,
refactor the code to use a ConcurrentMap instead of a Multimap --
removing the need for any locking.

As we really want to use CallHomeProtocolSessionContext in logging,
make it implement a toString() method and clean it up a bit.

JIRA: NETCONF-653
Change-Id: I2d78b9aa34ee4b16d0c6d2ed2ac172942c575851
Signed-off-by: Manoj Chokka <manoj.chokka@verizon.com>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/netconf/callhome-protocol/src/main/java/org/opendaylight/netconf/callhome/protocol/CallHomeProtocolSessionContext.java b/netconf/callhome-protocol/src/main/java/org/opendaylight/netconf/callhome/protocol/CallHomeProtocolSessionContext.java
index 3814e9d68324d2fd54ffe2ff005865ac87cc0a8a..4aff6d3c7940f9d4dfa495c9dc87cfb31a4e08ce 100644 (file)
--- a/netconf/callhome-protocol/src/main/java/org/opendaylight/netconf/callhome/protocol/CallHomeProtocolSessionContext.java
+++ b/netconf/callhome-protocol/src/main/java/org/opendaylight/netconf/callhome/protocol/CallHomeProtocolSessionContext.java
@@ -42,4 +42,9 @@ public interface CallHomeProtocolSessionContext {
      * @return Version string provided by remote server.
      */
     String getRemoteServerVersion();
+
+    /**
+     * Terminate this session.
+     */
+    void terminate();
 }

diff --git a/netconf/callhome-protocol/src/main/java/org/opendaylight/netconf/callhome/protocol/CallHomeSessionContext.java b/netconf/callhome-protocol/src/main/java/org/opendaylight/netconf/callhome/protocol/CallHomeSessionContext.java
index f517d8a64c9aded40634f46dbc1467071dbe2a1a..cf064152d1fc78bc3d13172e20743f7e5097a675 100644 (file)
--- a/netconf/callhome-protocol/src/main/java/org/opendaylight/netconf/callhome/protocol/CallHomeSessionContext.java
+++ b/netconf/callhome-protocol/src/main/java/org/opendaylight/netconf/callhome/protocol/CallHomeSessionContext.java
@@ -94,6 +94,12 @@ class CallHomeSessionContext implements CallHomeProtocolSessionContext {
         };
     }
 
+    @Override
+    public void terminate() {
+        sshSession.close(false);
+        removeSelf();
+    }
+
     private void channelOpenFailed(final Throwable throwable) {
         LOG.error("Unable to open netconf subsystem, disconnecting.", throwable);
         sshSession.close(false);

diff --git a/netconf/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountDispatcher.java b/netconf/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountDispatcher.java
index e8e1925962d037907daff5a2ece2941aed183940..b73cddfda47cca9000e0f427c2f59108b517b166 100644 (file)
--- a/netconf/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountDispatcher.java
+++ b/netconf/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountDispatcher.java
@@ -110,10 +110,12 @@ public class CallHomeMountDispatcher implements NetconfClientDispatcher, CallHom
                                          final CallHomeChannelActivator activator) {
         final CallHomeMountSessionContext deviceContext =
                 getSessionManager().createSession(session, activator, onCloseHandler);
-        final NodeId nodeId = deviceContext.getId();
-        final Node configNode = deviceContext.getConfigNode();
-        LOG.info("Provisioning fake config {}", configNode);
-        topology.connectNode(nodeId, configNode);
+        if (deviceContext != null) {
+            final NodeId nodeId = deviceContext.getId();
+            final Node configNode = deviceContext.getConfigNode();
+            LOG.info("Provisioning fake config {}", configNode);
+            topology.connectNode(nodeId, configNode);
+        }
     }
 
     public CallHomeMountSessionManager getSessionManager() {

diff --git a/netconf/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountSessionContext.java b/netconf/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountSessionContext.java
index e371eb1113412423f9595015fc1a6d9a79bb20c2..c9f0db72007af0bf856174e5d33e87a524977c3f 100644 (file)
--- a/netconf/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountSessionContext.java
+++ b/netconf/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountSessionContext.java
@@ -9,10 +9,9 @@ package org.opendaylight.netconf.callhome.mount;
 
 import static java.util.Objects.requireNonNull;
 
+import com.google.common.base.MoreObjects;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import io.netty.util.concurrent.Promise;
-import java.net.InetSocketAddress;
-import java.security.PublicKey;
 import org.opendaylight.netconf.api.NetconfMessage;
 import org.opendaylight.netconf.api.NetconfTerminationReason;
 import org.opendaylight.netconf.callhome.protocol.CallHomeChannelActivator;
@@ -27,8 +26,10 @@ import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeBuilder;
 
+// Non-final to allow mocking
 class CallHomeMountSessionContext {
-    public interface CloseCallback {
+    @FunctionalInterface
+    interface CloseCallback {
         void onClosed(CallHomeMountSessionContext deviceContext);
     }
 
@@ -49,27 +50,32 @@ class CallHomeMountSessionContext {
         this.onClose = requireNonNull(callback, "callback");
     }
 
+    CallHomeProtocolSessionContext getProtocol() {
+        return protocol;
+    }
+
     NodeId getId() {
         return nodeId;
     }
 
-    public ContextKey getContextKey() {
+    ContextKey getContextKey() {
         return key;
     }
 
     Node getConfigNode() {
-        NodeBuilder builder = new NodeBuilder();
-        return builder.setNodeId(getId()).addAugmentation(NetconfNode.class, configNetconfNode()).build();
-    }
-
-    private NetconfNode configNetconfNode() {
-        NetconfNodeBuilder node = new NetconfNodeBuilder();
-        node.setHost(new Host(key.getIpAddress()));
-        node.setPort(key.getPort());
-        node.setTcpOnly(Boolean.FALSE);
-        node.setCredentials(new LoginPasswordBuilder().setUsername("ommited").setPassword("ommited").build());
-        node.setSchemaless(Boolean.FALSE);
-        return node.build();
+        return new NodeBuilder()
+                .setNodeId(getId())
+                .addAugmentation(NetconfNode.class, new NetconfNodeBuilder()
+                    .setHost(new Host(key.getIpAddress()))
+                    .setPort(key.getPort())
+                    .setTcpOnly(Boolean.FALSE)
+                    .setCredentials(new LoginPasswordBuilder()
+                        .setUsername("ommited")
+                        .setPassword("ommited")
+                        .build())
+                    .setSchemaless(Boolean.FALSE)
+                    .build())
+                .build();
     }
 
     @SuppressWarnings("unchecked")
@@ -77,7 +83,14 @@ class CallHomeMountSessionContext {
         return (Promise<V>) activator.activate(wrap(sessionListener));
     }
 
-    @SuppressWarnings("deprecation")
+    @Override
+    public String toString() {
+        return MoreObjects.toStringHelper(this)
+                .add("address", protocol.getRemoteAddress())
+                .add("hostKey", protocol.getRemoteServerKey())
+                .toString();
+    }
+
     private NetconfClientSessionListener wrap(final NetconfClientSessionListener delegate) {
         return new NetconfClientSessionListener() {
             @Override
@@ -115,12 +128,4 @@ class CallHomeMountSessionContext {
     private void removeSelf() {
         onClose.onClosed(this);
     }
-
-    InetSocketAddress getRemoteAddress() {
-        return protocol.getRemoteAddress();
-    }
-
-    PublicKey getRemoteServerKey() {
-        return protocol.getRemoteServerKey();
-    }
 }

diff --git a/netconf/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountSessionManager.java b/netconf/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountSessionManager.java
index 80e82f5e9f91be4b9b71ea695b9a928331b17f3e..ae34495bbaf768a60339e3a9b0fe6a1d0dd18a4b 100644 (file)
--- a/netconf/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountSessionManager.java
+++ b/netconf/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountSessionManager.java
@@ -7,55 +7,65 @@
  */
 package org.opendaylight.netconf.callhome.mount;
 
-import com.google.common.collect.Multimap;
-import com.google.common.collect.MultimapBuilder;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.security.PublicKey;
-import java.util.Collection;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
-import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.netconf.callhome.mount.CallHomeMountSessionContext.CloseCallback;
 import org.opendaylight.netconf.callhome.protocol.CallHomeChannelActivator;
 import org.opendaylight.netconf.callhome.protocol.CallHomeProtocolSessionContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class CallHomeMountSessionManager implements CallHomeMountSessionContext.CloseCallback {
+    private static final Logger LOG = LoggerFactory.getLogger(CallHomeMountSessionManager.class);
 
     private final ConcurrentMap<SocketAddress, CallHomeMountSessionContext> contextByAddress =
         new ConcurrentHashMap<>();
-    private final Multimap<PublicKey, CallHomeMountSessionContext> contextByPublicKey = MultimapBuilder.hashKeys()
-        .hashSetValues().build();
+    private final ConcurrentMap<PublicKey, CallHomeMountSessionContext> contextByPublicKey = new ConcurrentHashMap<>();
 
-    public @Nullable CallHomeMountSessionContext getByAddress(InetSocketAddress remoteAddr) {
+    @Nullable CallHomeMountSessionContext getByAddress(final InetSocketAddress remoteAddr) {
         return contextByAddress.get(remoteAddr);
     }
 
+    CallHomeMountSessionContext createSession(final CallHomeProtocolSessionContext session,
+            final CallHomeChannelActivator activator, final CloseCallback onCloseHandler) {
+        final CallHomeMountSessionContext deviceContext = new CallHomeMountSessionContext(session.getSessionName(),
+            session, activator, devCtxt -> onClosed(devCtxt, onCloseHandler));
 
-    public @NonNull Collection<CallHomeMountSessionContext> getByPublicKey(PublicKey publicKey) {
-        return contextByPublicKey.get(publicKey);
-    }
-
-    CallHomeMountSessionContext createSession(CallHomeProtocolSessionContext session,
-                                              CallHomeChannelActivator activator, final CloseCallback onCloseHandler) {
-
-        String name = session.getSessionName();
-        CallHomeMountSessionContext deviceContext = new CallHomeMountSessionContext(name,
-                session, activator, devCtxt -> {
-            CallHomeMountSessionManager.this.onClosed(devCtxt);
-            onCloseHandler.onClosed(devCtxt);
-        });
-
-        contextByAddress.put(deviceContext.getRemoteAddress(), deviceContext);
-        contextByPublicKey.put(deviceContext.getRemoteServerKey(), deviceContext);
+        final PublicKey remoteKey = session.getRemoteServerKey();
+        final CallHomeMountSessionContext existing = contextByPublicKey.putIfAbsent(remoteKey, deviceContext);
+        if (existing != null) {
+            // Check if the sshkey of the incoming netconf server is present. If present return null, else store the
+            // session. The sshkey is the uniqueness of the callhome sessions not the uniqueid/devicename.
+            LOG.error("SSH Host Key {} is associated with existing session {}, closing session {}", remoteKey, existing,
+                session);
+            session.terminate();
+            return null;
+        }
 
+        final InetSocketAddress remoteAddress = session.getRemoteAddress();
+        final CallHomeMountSessionContext prev = contextByAddress.put(remoteAddress, deviceContext);
+        if (prev != null) {
+            LOG.warn("Remote {} replaced context {} with {}", remoteAddress, prev, deviceContext);
+        }
         return deviceContext;
     }
 
     @Override
-    public synchronized void onClosed(CallHomeMountSessionContext deviceContext) {
-        contextByAddress.remove(deviceContext.getRemoteAddress());
-        contextByPublicKey.remove(deviceContext.getRemoteServerKey(), deviceContext);
+    public void onClosed(final CallHomeMountSessionContext deviceContext) {
+        final CallHomeProtocolSessionContext session = deviceContext.getProtocol();
+        contextByAddress.remove(session.getRemoteAddress(), deviceContext);
+        contextByPublicKey.remove(session.getRemoteServerKey(), deviceContext);
+    }
+
+    @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD",
+            justification = "https://github.com/spotbugs/spotbugs/issues/811")
+    private void onClosed(final CallHomeMountSessionContext deviceContext, final CloseCallback onCloseHandler) {
+        onClosed(deviceContext);
+        onCloseHandler.onClosed(deviceContext);
     }
 }