[Internal] 4.22.1.0 USSignal Console Proxy Issues

server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManager.java

@@ -84,6 +84,9 @@ public interface ConsoleProxyManager extends Manager, ConsoleProxyService {
     ConfigKey<Boolean> ConsoleProxyDisableRpFilter = new ConfigKey<>(Boolean.class, "consoleproxy.disable.rpfilter", "Console Proxy", "true",
             "disable rp_filter on console proxy VM public interface", true, ConfigKey.Scope.Zone, null);
 
+    ConfigKey<Long> ConsoleProxySessionReconnectionWindow = new ConfigKey<>(Long.class, "consoleproxy.session.reconnection.window", "Console Proxy", "0",
+            "Reconnection window (in milliseconds) for client IPs to the same session on console proxy VM", true, ConfigKey.Scope.Zone, null);
+
     ConfigKey<Integer> ConsoleProxyLaunchMax = new ConfigKey<>(Integer.class, "consoleproxy.launch.max", "Console Proxy", "10",
             "maximum number of console proxy instances per zone can be launched", false, ConfigKey.Scope.Zone, null);
 

server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java

@@ -1210,6 +1210,10 @@ public boolean finalizeVirtualMachineProfile(VirtualMachineProfile profile, Depl
         if (Boolean.TRUE.equals(disableRpFilter)) {
             buf.append(" disable_rp_filter=true");
         }
+        Long sessionReconnectionWindow = ConsoleProxySessionReconnectionWindow.valueIn(datacenterId);
+        if (sessionReconnectionWindow != null && sessionReconnectionWindow > 0) {
+            buf.append(" session_reconnection_window=").append(sessionReconnectionWindow);
+        }
 
         String msPublicKey = configurationDao.getValue("ssh.publickey");
         buf.append(" authorized_key=").append(VirtualMachineGuru.getEncodedMsPublicKey(msPublicKey));
@@ -1591,7 +1595,7 @@ public ConfigKey<?>[] getConfigKeys() {
         return new ConfigKey<?>[] {ConsoleProxySslEnabled, NoVncConsoleDefault, NoVncConsoleSourceIpCheckEnabled, ConsoleProxyServiceOffering,
                                    ConsoleProxyCapacityStandby, ConsoleProxyCapacityScanInterval, ConsoleProxyRestart, ConsoleProxyUrlDomain, ConsoleProxySessionMax, ConsoleProxySessionTimeout, ConsoleProxyDisableRpFilter, ConsoleProxyLaunchMax,
                                    ConsoleProxyManagementLastState, ConsoleProxyServiceManagementState, NoVncConsoleShowDot,
-                                   ConsoleProxyVmUserData};
+                                   ConsoleProxyVmUserData, ConsoleProxySessionReconnectionWindow};
     }
 
     protected ConsoleProxyStatus parseJsonToConsoleProxyStatus(String json) throws JsonParseException {

services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java

@@ -26,7 +26,6 @@
 import java.net.InetSocketAddress;
 import java.net.URISyntaxException;
 import java.net.URL;
-import java.util.HashSet;
 import java.util.Hashtable;
 import java.util.Map;
 import java.util.Properties;
@@ -83,12 +82,75 @@ public class ConsoleProxy {
     static String encryptorPassword = "Dummy";
     static final String[] skipProperties = new String[]{"certificate", "cacertificate", "keystore_password", "privatekey"};
 
-    static Set<String> allowedSessions = new HashSet<>();
+    static Set<String> allowedSessions = ConcurrentHashMap.newKeySet();
+    private static final Object allowedSessionsLock = new Object();
 
+    private static final Map<String, ReconnectGrant> sessionReconnectGrants = new ConcurrentHashMap<>();
+    private static long sessionReconnectionWindowMs = 0L;
+
+    // Invoked through reflection
     public static void addAllowedSession(String sessionUuid) {
         allowedSessions.add(sessionUuid);
     }
 
+    /**
+     * Grant the client IP a reconnection window of #{@link #sessionReconnectionWindowMs} ms to the same session UUID in case of a disconnection.
+     * The grant is bound to the client IP that was using the session so it cannot be redeemed by another client.
+     * @param sessionUuid session UUID to grant a reconnect window for
+     * @param clientIp source IP of the client the session was granted to
+     */
+    public static void grantReconnectWindowForSessionAndClientIp(String sessionUuid, String clientIp) {
+        if (sessionReconnectionWindowMs > 0) {
+            ReconnectGrant grant = new ReconnectGrant(System.currentTimeMillis() + sessionReconnectionWindowMs, clientIp);
+            sessionReconnectGrants.put(sessionUuid, grant);
+        }
+    }
+
+    /**
+     * True if the session UUID has been granted reconnection, within the reconnection window #{@link #sessionReconnectionWindowMs}.
+     */
+    private static boolean isSessionReconnectionGrantedForClientIp(String sessionUuid, String clientIp) {
+        ReconnectGrant grant = sessionReconnectGrants.remove(sessionUuid);
+        if (grant == null) {
+            LOGGER.debug("No reconnect grant found for session {} requested from IP {} (reconnection window: {}ms)",
+                    sessionUuid, clientIp, sessionReconnectionWindowMs);
+            return false;
+        }
+        if (grant.isExpired(System.currentTimeMillis())) {
+            LOGGER.debug("Rejecting reconnection for session {} as the reconnect window: {}ms is already expired",
+                    sessionUuid, sessionReconnectionWindowMs);
+            return false;
+        }
+        if (grant.clientIp != null && !grant.clientIp.equals(clientIp)) {
+            LOGGER.debug("Rejecting reconnection for session {} as it was requested from IP {} " +
+                    "but the session was granted to IP {}", sessionUuid, clientIp, grant.clientIp);
+            return false;
+        }
+        return true;
+    }
+
+    /**
+     * Drops expired, unclaimed reconnect grants so sessionReconnectGrants doesn't grow unbounded
+     * when a client never reconnects after a disconnection. Invoked periodically by {@link ConsoleProxyGCThread}.
+     */
+    static void cleanupExpiredReconnectGrants() {
+        sessionReconnectGrants.entrySet().removeIf(entry -> entry.getValue().isExpired(System.currentTimeMillis()));
+    }
+
+    private static final class ReconnectGrant {
+        final long expiryMillis;
+        final String clientIp;
+
+        ReconnectGrant(long expiryMillis, String clientIp) {
+            this.expiryMillis = expiryMillis;
+            this.clientIp = clientIp;
+        }
+
+        boolean isExpired(long now) {
+            return now >= expiryMillis;
+        }
+    }
+
     private static void configLog4j() {
         final ClassLoader loader = Thread.currentThread().getContextClassLoader();
         URL configUrl = loader.getResource("/conf/log4j-cloud.xml");
@@ -166,6 +228,12 @@ private static void configProxy(Properties conf) {
             defaultBufferSize = Integer.parseInt(s);
             LOGGER.info("Setting defaultBufferSize=" + defaultBufferSize);
         }
+
+        s = conf.getProperty("session_reconnection_window");
+        if (s != null) {
+            sessionReconnectionWindowMs = Long.parseLong(s);
+            LOGGER.info("Setting sessionReconnectionWindowMs=" + sessionReconnectionWindowMs);
+        }
     }
 
     public static ConsoleProxyServerFactory getHttpServerFactory() {
@@ -183,7 +251,7 @@ public static ConsoleProxyServerFactory getHttpServerFactory() {
                 return null;
             }
         } catch (ClassNotFoundException e) {
-            LOGGER.warn("Unable to find http server factory class: " + factoryClzName);
+            LOGGER.debug("Unable to find http server factory class: " + factoryClzName);
             return new ConsoleProxyBaseServerFactoryImpl();
         }
     }
@@ -209,13 +277,23 @@ public static ConsoleProxyAuthenticationResult authenticateConsoleAccess(Console
         }
 
         String sessionUuid = param.getSessionUuid();
-        if (allowedSessions.contains(sessionUuid)) {
-            LOGGER.debug("Acquiring the session " + sessionUuid + " not available for future use");
-            allowedSessions.remove(sessionUuid);
-        } else {
-            LOGGER.info("Session " + sessionUuid + " has already been used, cannot connect");
-            authResult.setSuccess(false);
-            return authResult;
+        LOGGER.debug("Authenticating console access [session: {}, clientIp: {}, reauthentication: {}, currently allowed sessions: {}]",
+                sessionUuid, param.getClientIp(), reauthentication, allowedSessions.size());
+        synchronized (allowedSessionsLock) {
+            if (allowedSessions.remove(sessionUuid)) {
+                LOGGER.debug("Acquiring the session {} from client IP {}", sessionUuid, param.getClientIp());
+            } else if (isSessionReconnectionGrantedForClientIp(sessionUuid, param.getClientIp())) {
+                LOGGER.info("Reconnecting the session {} after a dropped connection", sessionUuid);
+                return authResult;
+            } else {
+                LOGGER.debug("Rejecting console session {} from client IP {}: session is not in the allowed set (already " +
+                                "consumed, unknown, or expired) and no valid reconnect grant is available " +
+                                "(reconnection window: {}ms{})",
+                        sessionUuid, param.getClientIp(), sessionReconnectionWindowMs,
+                        sessionReconnectionWindowMs <= 0 ? ", reconnection disabled" : "");
+                authResult.setSuccess(false);
+                return authResult;
+            }
         }
 
         String websocketUrl = param.getWebsocketUrl();
@@ -250,7 +328,7 @@ public static ConsoleProxyAuthenticationResult authenticateConsoleAccess(Console
                 authResult.setSuccess(false);
             }
         } else {
-            LOGGER.warn("Private channel towards management server is not setup. Switch to offline mode and allow access to vm: " + param.getClientTag());
+            LOGGER.debug("Private channel towards management server is not setup. Switch to offline mode and allow access to vm: " + param.getClientTag());
         }
 
         return authResult;
@@ -266,7 +344,7 @@ public static void reportLoadInfo(String gsonLoadInfo) {
                 LOGGER.error("Unable to invoke reportLoadInfo due to " + e.getMessage());
             }
         } else {
-            LOGGER.warn("Private channel towards management server is not setup. Switch to offline mode and ignore load report");
+            LOGGER.debug("Private channel towards management server is not setup. Switch to offline mode and ignore load report");
         }
     }
 
@@ -280,7 +358,7 @@ public static void ensureRoute(String address) {
                 LOGGER.error("Unable to invoke ensureRoute due to " + e.getMessage());
             }
         } else {
-            LOGGER.warn("Unable to find ensureRoute method, console proxy agent is not up to date");
+            LOGGER.debug("Unable to find ensureRoute method, console proxy agent is not up to date");
         }
     }
 
@@ -472,7 +550,7 @@ public static ConsoleProxyClient getVncViewer(ConsoleProxyClientParam param) thr
                 LOGGER.info("The rfb thread died, reinitializing the viewer " + viewer);
                 viewer.initClient(param);
             } else if (!param.getClientHostPassword().equals(viewer.getClientHostPassword())) {
-                LOGGER.warn("Bad sid detected(VNC port may be reused). sid in session: " + viewer.getClientHostPassword() + ", sid in request: " +
+                LOGGER.debug("Bad sid detected(VNC port may be reused). sid in session: " + viewer.getClientHostPassword() + ", sid in request: " +
                         param.getClientHostPassword());
                 viewer.initClient(param);
             }
@@ -564,7 +642,7 @@ public static void authenticationExternally(ConsoleProxyClientParam param) throw
         ConsoleProxyAuthenticationResult authResult = authenticateConsoleAccess(param, false);
 
         if (authResult == null || !authResult.isSuccess()) {
-            LOGGER.warn("External authenticator failed authentication request for vm " + param.getClientTag() + " with sid " + param.getClientHostPassword());
+            LOGGER.debug("External authenticator failed authentication request for vm " + param.getClientTag() + " with sid " + param.getClientHostPassword());
 
             throw new AuthenticationException("External authenticator failed request for vm " + param.getClientTag() + " with sid " + param.getClientHostPassword());
         }
@@ -625,7 +703,7 @@ public static ConsoleProxyNoVncClient getNoVncViewer(ConsoleProxyClientParam par
                 } catch (IOException e) {
                     LOGGER.error("Exception while disconnect session of novnc viewer object: " + viewer, e);
                 }
-                removeViewer(viewer);
+                viewer.closeClient();
                 viewer = new ConsoleProxyNoVncClient(session);
                 viewer.initClient(param);
                 connectionMap.put(clientKey, viewer);

services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java

@@ -75,6 +75,7 @@ public void run() {
 
         while (true) {
             cleanupLogging();
+            ConsoleProxy.cleanupExpiredReconnectGrants();
             bReportLoad = false;
 
             if (logger.isDebugEnabled()) {

services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java

@@ -57,6 +57,8 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
             throws IOException, ServletException {
 
         if (this.getWebSocketFactory().isUpgradeRequest(request, response)) {
+            logger.debug("Received WebSocket upgrade request [path: {}, query: {}, remote IP: {}]",
+                    target, request.getQueryString(), request.getRemoteAddr());
             response.addHeader("Sec-WebSocket-Protocol", "binary");
             if (this.getWebSocketFactory().acceptWebSocket(request, response)) {
                 baseRequest.setHandled(true);
@@ -95,6 +97,9 @@ public void onConnect(final Session session) throws IOException, InterruptedExce
         String clientIp = session.getRemoteAddress().getAddress().getHostAddress();
         boolean sessionRequiresNewViewer = Boolean.parseBoolean(queryMap.get("sessionRequiresNewViewer"));
 
+        logger.info("WebSocket connect attempt [session UUID: {}, client IP: {}, host: {}, port: {}, sessionRequiresNewViewer: {}]",
+                sessionUuid, clientIp, host, portStr, sessionRequiresNewViewer);
+
         if (tag == null)
             tag = "";
 
@@ -175,12 +180,20 @@ private boolean checkSessionSourceIp(final Session session, final String sourceI
 
     @OnWebSocketClose
     public void onClose(Session session, int statusCode, String reason) throws IOException, InterruptedException {
-        String sessionSourceIp = session.getRemoteAddress().getAddress().getHostAddress();
-        logger.debug("Closing WebSocket session [source IP: {}, status code: {}].", sessionSourceIp, statusCode);
         if (viewer != null) {
-            ConsoleProxy.removeViewer(viewer);
+            viewer.closeClient();
+        }
+        String sessionSourceIp = getRemoteAddressSafely(session);
+        logger.debug("WebSocket session [source IP: {}, status code: {}, reason: {}] closed successfully.", sessionSourceIp, statusCode, reason);
+    }
+
+    private String getRemoteAddressSafely(Session session) {
+        try {
+            return session.getRemoteAddress().getAddress().getHostAddress();
+        } catch (Exception e) {
+            logger.debug("Failed to get remote address from WebSocket session", e);
+            return "unknown";
         }
-        logger.debug("WebSocket session [source IP: {}, status code: {}] closed successfully.", sessionSourceIp, statusCode);
     }
 
     @OnWebSocketFrame
@@ -191,6 +204,8 @@ public void onFrame(Frame f) throws IOException {
 
     @OnWebSocketError
     public void onError(Throwable cause) {
-        logger.error("Error on WebSocket [client ID: {}, session UUID: {}].", cause, viewer.getClientId(), viewer.getSessionUuid());
+        String clientId = viewer != null ? String.valueOf(viewer.getClientId()) : "unknown (no viewer created)";
+        String sessionUuid = viewer != null ? viewer.getSessionUuid() : "unknown";
+        logger.error("Error on WebSocket [client ID: {}, session UUID: {}]: {}", clientId, sessionUuid, cause.getMessage(), cause);
     }
 }

services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java

@@ -177,6 +177,7 @@ public void run() {
                         }
                     }
                     logger.info("Connection with client [{}] [IP: {}] is dead.", clientId, clientSourceIp);
+                    ConsoleProxy.grantReconnectWindowForSessionAndClientIp(sessionUuid, clientSourceIp);
                 } catch (IOException e) {
                     logger.error("Error on VNC client", e);
                 }
@@ -374,6 +375,9 @@ public void closeClient() {
         this.connectionAlive = false;
         // Clear buffer reference to allow GC when client disconnects
         this.readBuffer = null;
+        if (client != null) {
+            client.close();
+        }
         ConsoleProxy.removeViewer(this);
     }
 

services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java

@@ -129,6 +129,28 @@ public void proxyMsgOverWebSocketConnection(ByteBuffer msg) {
         }
     }
 
+    public void close() {
+        if (nioSocketConnection != null) {
+            nioSocketConnection.close();
+        }
+        if (webSocketReverseProxy != null) {
+            webSocketReverseProxy.close();
+        }
+        if (socket != null) {
+            try {
+                if (is != null) {
+                    is.close();
+                }
+                if (os != null) {
+                    os.close();
+                }
+                socket.close();
+            } catch (IOException e) {
+                logger.debug("Error closing socket: " + e.getMessage(), e);
+            }
+        }
+    }
+
     private void setTunnelSocketStreams() throws IOException {
         this.is = new DataInputStream(this.socket.getInputStream());
         this.os = new DataOutputStream(this.socket.getOutputStream());
@@ -391,13 +413,26 @@ public ByteBuffer handshakeProtocolVersion() {
         return verStr;
     }
 
+    protected static final long WAIT_FOR_NOVNC_TIMEOUT_MS = 30000;
+
     public void waitForNoVNCReply() {
-        int cycles = 0;
-        while (isWaitForNoVnc()) {
-            cycles++;
-        }
-        if (logger.isDebugEnabled()) {
-            logger.debug(String.format("Waited %d cycles for NoVnc", cycles));
+        long start = System.currentTimeMillis();
+        logger.debug("Waiting for a reply from the noVNC client during handshake");
+        synchronized (lock) {
+            while (waitForNoVnc) {
+                long remaining = WAIT_FOR_NOVNC_TIMEOUT_MS - (System.currentTimeMillis() - start);
+                if (remaining <= 0) {
+                    logger.debug("Timed out after {} ms waiting for a reply from the noVNC client during handshake", WAIT_FOR_NOVNC_TIMEOUT_MS);
+                    break;
+                }
+                try {
+                    lock.wait(remaining);
+                } catch (InterruptedException e) {
+                    Thread.currentThread().interrupt();
+                    logger.debug("Interrupted while waiting for a reply from the noVNC client during handshake", e);
+                    break;
+                }
+            }
         }
     }
 
@@ -446,6 +481,7 @@ public int handshakeSecurityType() {
     public void setWaitForNoVnc(boolean val) {
         synchronized (lock) {
             this.waitForNoVnc = val;
+            lock.notifyAll();
         }
     }