import static com.google.common.base.Preconditions.checkArgument;
import static java.util.Objects.requireNonNull;
+import com.google.common.annotations.VisibleForTesting;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import io.netty.channel.EventLoopGroup;
import io.netty.util.concurrent.GlobalEventExecutor;
class CallHomeSessionContext implements CallHomeProtocolSessionContext {
private static final Logger LOG = LoggerFactory.getLogger(CallHomeSessionContext.class);
- static final Session.AttributeKey<CallHomeSessionContext> SESSION_KEY = new Session.AttributeKey<>();
-
private static final String NETCONF = "netconf";
+ @VisibleForTesting
+ static final Session.AttributeKey<CallHomeSessionContext> SESSION_KEY = new Session.AttributeKey<>();
+
private final ClientSession sshSession;
private final CallHomeAuthorization authorization;
private final Factory factory;
checkArgument(this.authorization.isServerAllowed(), "Server was not allowed.");
this.factory = requireNonNull(factory);
this.sshSession = requireNonNull(sshSession);
- this.sshSession.setAttribute(SESSION_KEY, this);
this.remoteAddress = (InetSocketAddress) this.sshSession.getIoSession().getRemoteAddress();
serverKey = this.sshSession.getServerKey();
}
+ final void associate() {
+ sshSession.setAttribute(SESSION_KEY, this);
+ }
+
static CallHomeSessionContext getFrom(final ClientSession sshSession) {
return sshSession.getAttribute(SESSION_KEY);
}
}
static class Factory {
-
+ private final ConcurrentMap<String, CallHomeSessionContext> sessions = new ConcurrentHashMap<>();
private final EventLoopGroup nettyGroup;
private final NetconfClientSessionNegotiatorFactory negotiatorFactory;
private final CallHomeNetconfSubsystemListener subsystemListener;
- private final ConcurrentMap<String, CallHomeSessionContext> sessions = new ConcurrentHashMap<>();
Factory(final EventLoopGroup nettyGroup, final NetconfClientSessionNegotiatorFactory negotiatorFactory,
final CallHomeNetconfSubsystemListener subsystemListener) {
- this.nettyGroup = requireNonNull(nettyGroup, "nettyGroup");
- this.negotiatorFactory = requireNonNull(negotiatorFactory, "negotiatorFactory");
+ this.nettyGroup = requireNonNull(nettyGroup);
+ this.negotiatorFactory = requireNonNull(negotiatorFactory);
this.subsystemListener = requireNonNull(subsystemListener);
}
- void remove(final CallHomeSessionContext session) {
- sessions.remove(session.getSessionId(), session);
- }
-
ReverseSshChannelInitializer getChannelInitializer(final NetconfClientSessionListener listener) {
return ReverseSshChannelInitializer.create(negotiatorFactory, listener);
}
return subsystemListener;
}
+ EventLoopGroup getNettyGroup() {
+ return nettyGroup;
+ }
+
@Nullable CallHomeSessionContext createIfNotExists(final ClientSession sshSession,
final CallHomeAuthorization authorization, final SocketAddress remoteAddress) {
- CallHomeSessionContext session = new CallHomeSessionContext(sshSession, authorization,
- remoteAddress, this);
- CallHomeSessionContext preexisting = sessions.putIfAbsent(session.getSessionId(), session);
- // If preexisting is null - session does not exist, so we can safely create new one, otherwise we return
- // null and incoming connection will be rejected.
- return preexisting == null ? session : null;
+ final var newSession = new CallHomeSessionContext(sshSession, authorization, remoteAddress, this);
+ final var existing = sessions.putIfAbsent(newSession.getSessionId(), newSession);
+ if (existing == null) {
+ // There was no mapping, but now there is. Associate the the context with the session.
+ newSession.associate();
+ return newSession;
+ }
+
+ // We already have a mapping, do not create a new one. But also check if the current session matches
+ // the one stored in the session. This can happen during rekeying.
+ return existing == CallHomeSessionContext.getFrom(sshSession) ? existing : null;
}
- EventLoopGroup getNettyGroup() {
- return nettyGroup;
+ void remove(final CallHomeSessionContext session) {
+ sessions.remove(session.getSessionId(), session);
}
}
}
LOG.debug("SSH session {} event {}", session, event);
switch (event) {
case KeyEstablished:
- doAuth(clientSession);
+ // Case of key re-exchange - if session is once authenticated, it does not need to be made again
+ if (!clientSession.isAuthenticated()) {
+ doAuth(clientSession);
+ }
break;
case Authenticated:
CallHomeSessionContext.getFrom(clientSession).openNetconfChannel();
public boolean verifyServerKey(final ClientSession sshClientSession, final SocketAddress remoteAddress,
final PublicKey serverKey) {
final CallHomeAuthorization authorization = authProvider.provideAuth(remoteAddress, serverKey);
- // server is not authorized
if (!authorization.isServerAllowed()) {
+ // server is not authorized
LOG.info("Incoming session {} was rejected by Authorization Provider.", sshClientSession);
return false;
}
- CallHomeSessionContext session = sessionFactory.createIfNotExists(
- sshClientSession, authorization, remoteAddress);
- // Session was created, session with same name does not exists
- if (session != null) {
- return true;
+
+ if (sessionFactory.createIfNotExists(sshClientSession, authorization, remoteAddress) == null) {
+ // Session was not created, session with same name exists
+ LOG.info("Incoming session {} was rejected. Session with same name {} is already active.", sshClientSession,
+ authorization.getSessionName());
+ return false;
}
- // Session was not created, session with same name exists
- LOG.info("Incoming session {} was rejected. Session with same name {} is already active.",
- sshClientSession, authorization.getSessionName());
- return false;
+
+ // Session was created, session with same name does not exist
+ LOG.debug("Incoming session {} was successfully verified.", sshClientSession);
+ return true;
}
public void bind() throws IOException {
*/
package org.opendaylight.netconf.callhome.protocol;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyString;
@Test
public void theContextShouldBeSettableAndRetrievableAsASessionAttribute() {
- // redo instance below because previous constructor happened too early to capture behavior
+ // when
instance = realFactory.createIfNotExists(mockSession, mockAuth, address);
+ // then
+ assertNotNull(instance);
+ verify(mockSession, times(1)).setAttribute(CallHomeSessionContext.SESSION_KEY, instance);
+ verify(mockSession, times(0)).getAttribute(any());
+
// when
CallHomeSessionContext.getFrom(mockSession);
// then
- verify(mockSession, times(1)).setAttribute(CallHomeSessionContext.SESSION_KEY, instance);
verify(mockSession, times(1)).getAttribute(CallHomeSessionContext.SESSION_KEY);
}
@Test
@Ignore
+ // FIXME: enable this test
public void failureToOpenTheChannelShouldCauseTheSessionToClose() {
// given
instance = realFactory.createIfNotExists(mockSession, mockAuth, address);
-
OpenFuture mockFuture = mock(OpenFuture.class);
Mockito.doReturn(false).when(mockFuture).isOpened();
Mockito.doReturn(new RuntimeException("test")).when(mockFuture).getException();
// You'll see an error message logged to the console - it is expected.
verify(mockSession, times(1)).close(anyBoolean());
}
+
+ @Test
+ public void theContextConstructorShouldNotModifySession() {
+ instance = new CallHomeSessionContext(mockSession, mockAuth, address, realFactory);
+ verify(mockSession, times(0)).setAttribute(any(), any());
+ assertNull(CallHomeSessionContext.getFrom(mockSession));
+ }
}
final PublicKey serverKey = mock(PublicKey.class);
doReturn(serverKey).when(mockSession).getServerKey();
- SessionListener listener = instance.createSessionListener();
+ final SessionListener listener = instance.createSessionListener();
doReturn(mockAuthFuture).when(mockContext).authorize();
+ doReturn(false).when(mockSession).isAuthenticated();
// when
listener.sessionEvent(mockSession, evt[pass]);
// then
@Test
public void verificationOfTheServerKeyShouldBeSuccessfulForServerIsAllowed() {
// given
-
ClientSessionImpl mockClientSession = mock(ClientSessionImpl.class);
Mockito.doReturn("test").when(mockClientSession).toString();
SocketAddress mockSocketAddr = mock(SocketAddress.class);
Mockito.doReturn(true).when(mockAuth).isServerAllowed();
Mockito.doReturn("some-session-name").when(mockAuth).getSessionName();
-
Mockito.doReturn(mockAuth).when(mockCallHomeAuthProv).provideAuth(mockSocketAddr, mockPublicKey);
-
Mockito.doReturn(null).when(mockFactory).createIfNotExists(mockClientSession, mockAuth, mockSocketAddr);
// expect
- instance.verifyServerKey(mockClientSession, mockSocketAddr, mockPublicKey);
+ assertFalse(instance.verifyServerKey(mockClientSession, mockSocketAddr, mockPublicKey));
}
@Test
Code Review / netconf.git / commitdiff