Skip to content

Commit 57356f8

Browse files
committed
Fix console proxy idle timeout and noVNC session handling
1 parent 9f96c9d commit 57356f8

3 files changed

Lines changed: 142 additions & 103 deletions

File tree

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

Lines changed: 98 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,15 @@
3636

3737
import org.apache.commons.lang3.ArrayUtils;
3838
import org.apache.commons.lang3.StringUtils;
39+
import org.apache.logging.log4j.LogManager;
40+
import org.apache.logging.log4j.Logger;
3941
import org.apache.logging.log4j.core.config.Configurator;
4042
import org.eclipse.jetty.websocket.api.Session;
4143

4244
import com.cloud.utils.PropertiesUtil;
4345
import com.google.gson.Gson;
4446
import com.sun.net.httpserver.HttpServer;
4547

46-
import org.apache.logging.log4j.LogManager;
47-
import org.apache.logging.log4j.Logger;
48-
4948
/**
5049
*
5150
* ConsoleProxy, singleton class that manages overall activities in console proxy process. To make legacy code work, we still
@@ -82,7 +81,7 @@ public class ConsoleProxy {
8281
static boolean standaloneStart = false;
8382

8483
static String encryptorPassword = "Dummy";
85-
static final String[] skipProperties = new String[]{"certificate", "cacertificate", "keystore_password", "privatekey"};
84+
static final String[] skipProperties = new String[] {"certificate", "cacertificate", "keystore_password", "privatekey"};
8685

8786
static Set<String> allowedSessions = new HashSet<>();
8887

@@ -93,11 +92,13 @@ public static void addAllowedSession(String sessionUuid) {
9392
private static void configLog4j() {
9493
final ClassLoader loader = Thread.currentThread().getContextClassLoader();
9594
URL configUrl = loader.getResource("/conf/log4j-cloud.xml");
96-
if (configUrl == null)
95+
if (configUrl == null) {
9796
configUrl = ClassLoader.getSystemResource("log4j-cloud.xml");
97+
}
9898

99-
if (configUrl == null)
99+
if (configUrl == null) {
100100
configUrl = ClassLoader.getSystemResource("conf/log4j-cloud.xml");
101+
}
101102

102103
if (configUrl != null) {
103104
try {
@@ -114,28 +115,28 @@ private static void configLog4j() {
114115
} catch (URISyntaxException e) {
115116
System.out.println("Unable to convert log4j configuration Url to URI");
116117
}
117-
// DOMConfigurator.configure(configUrl);
118118
} else {
119119
System.out.println("Configure log4j with default properties");
120120
}
121121
}
122122

123123
private static void configProxy(Properties conf) {
124124
LOGGER.info("Configure console proxy...");
125-
for (Object key : conf.keySet()) {
126-
LOGGER.info("Property " + (String)key + ": " + conf.getProperty((String)key));
127-
if (!ArrayUtils.contains(skipProperties, key)) {
128-
LOGGER.info("Property " + (String)key + ": " + conf.getProperty((String)key));
125+
if (conf != null) {
126+
for (Object key : conf.keySet()) {
127+
if (!ArrayUtils.contains(skipProperties, key)) {
128+
LOGGER.info("Property " + (String) key + ": " + conf.getProperty((String) key));
129+
}
129130
}
130131
}
131132

132-
String s = conf.getProperty("consoleproxy.httpListenPort");
133+
String s = conf != null ? conf.getProperty("consoleproxy.httpListenPort") : null;
133134
if (s != null) {
134135
httpListenPort = Integer.parseInt(s);
135136
LOGGER.info("Setting httpListenPort=" + s);
136137
}
137138

138-
s = conf.getProperty("premium");
139+
s = conf != null ? conf.getProperty("premium") : null;
139140
if (s != null && s.equalsIgnoreCase("true")) {
140141
LOGGER.info("Premium setting will override settings from consoleproxy.properties, listen at port 443");
141142
httpListenPort = 443;
@@ -144,25 +145,25 @@ private static void configProxy(Properties conf) {
144145
factoryClzName = ConsoleProxyBaseServerFactoryImpl.class.getName();
145146
}
146147

147-
s = conf.getProperty("consoleproxy.httpCmdListenPort");
148+
s = conf != null ? conf.getProperty("consoleproxy.httpCmdListenPort") : null;
148149
if (s != null) {
149150
httpCmdListenPort = Integer.parseInt(s);
150151
LOGGER.info("Setting httpCmdListenPort=" + s);
151152
}
152153

153-
s = conf.getProperty("consoleproxy.reconnectMaxRetry");
154+
s = conf != null ? conf.getProperty("consoleproxy.reconnectMaxRetry") : null;
154155
if (s != null) {
155156
reconnectMaxRetry = Integer.parseInt(s);
156157
LOGGER.info("Setting reconnectMaxRetry=" + reconnectMaxRetry);
157158
}
158159

159-
s = conf.getProperty("consoleproxy.readTimeoutSeconds");
160+
s = conf != null ? conf.getProperty("consoleproxy.readTimeoutSeconds") : null;
160161
if (s != null) {
161162
readTimeoutSeconds = Integer.parseInt(s);
162163
LOGGER.info("Setting readTimeoutSeconds=" + readTimeoutSeconds);
163164
}
164165

165-
s = conf.getProperty("consoleproxy.defaultBufferSize");
166+
s = conf != null ? conf.getProperty("consoleproxy.defaultBufferSize") : null;
166167
if (s != null) {
167168
defaultBufferSize = Integer.parseInt(s);
168169
LOGGER.info("Setting defaultBufferSize=" + defaultBufferSize);
@@ -173,7 +174,7 @@ public static ConsoleProxyServerFactory getHttpServerFactory() {
173174
try {
174175
Class<?> clz = Class.forName(factoryClzName);
175176
try {
176-
ConsoleProxyServerFactory factory = (ConsoleProxyServerFactory)clz.newInstance();
177+
ConsoleProxyServerFactory factory = (ConsoleProxyServerFactory) clz.newInstance();
177178
factory.init(ConsoleProxy.ksBits, ConsoleProxy.ksPassword);
178179
return factory;
179180
} catch (InstantiationException e) {
@@ -197,11 +198,11 @@ public static ConsoleProxyAuthenticationResult authenticateConsoleAccess(Console
197198
authResult.setHost(param.getClientHostAddress());
198199
authResult.setPort(param.getClientHostPort());
199200

200-
if (org.apache.commons.lang3.StringUtils.isNotBlank(param.getExtraSecurityToken())) {
201+
if (StringUtils.isNotBlank(param.getExtraSecurityToken())) {
201202
String extraToken = param.getExtraSecurityToken();
202203
String clientProvidedToken = param.getClientProvidedExtraSecurityToken();
203-
LOGGER.debug(String.format("Extra security validation for the console access, provided %s " +
204-
"to validate against %s", clientProvidedToken, extraToken));
204+
LOGGER.debug(String.format("Extra security validation for the console access, provided %s to validate against %s",
205+
clientProvidedToken, extraToken));
205206

206207
if (!extraToken.equals(clientProvidedToken)) {
207208
LOGGER.error("The provided extra token does not match the expected value for this console endpoint");
@@ -233,20 +234,21 @@ public static ConsoleProxyAuthenticationResult authenticateConsoleAccess(Console
233234
Object result;
234235
try {
235236
result =
236-
authMethod.invoke(ConsoleProxy.context, param.getClientHostAddress(), String.valueOf(param.getClientHostPort()), param.getClientTag(),
237-
param.getClientHostPassword(), param.getTicket(), reauthentication, param.getSessionUuid());
237+
authMethod.invoke(ConsoleProxy.context, param.getClientHostAddress(), String.valueOf(param.getClientHostPort()),
238+
param.getClientTag(), param.getClientHostPassword(), param.getTicket(), reauthentication,
239+
param.getSessionUuid(), param.getClientIp());
238240
} catch (IllegalAccessException e) {
239-
LOGGER.error("Unable to invoke authenticateConsoleAccess due to IllegalAccessException" + " for vm: " + param.getClientTag(), e);
241+
LOGGER.error("Unable to invoke authenticateConsoleAccess due to IllegalAccessException for vm: " + param.getClientTag(), e);
240242
authResult.setSuccess(false);
241243
return authResult;
242244
} catch (InvocationTargetException e) {
243-
LOGGER.error("Unable to invoke authenticateConsoleAccess due to InvocationTargetException " + " for vm: " + param.getClientTag(), e);
245+
LOGGER.error("Unable to invoke authenticateConsoleAccess due to InvocationTargetException for vm: " + param.getClientTag(), e);
244246
authResult.setSuccess(false);
245247
return authResult;
246248
}
247249

248250
if (result != null && result instanceof String) {
249-
authResult = new Gson().fromJson((String)result, ConsoleProxyAuthenticationResult.class);
251+
authResult = new Gson().fromJson((String) result, ConsoleProxyAuthenticationResult.class);
250252
} else {
251253
LOGGER.error("Invalid authentication return object " + result + " for vm: " + param.getClientTag() + ", decline the access");
252254
authResult.setSuccess(false);
@@ -286,7 +288,8 @@ public static void ensureRoute(String address) {
286288
}
287289
}
288290

289-
public static void startWithContext(Properties conf, Object context, byte[] ksBits, String ksPassword, String password, Boolean isSourceIpCheckEnabled) {
291+
public static void startWithContext(Properties conf, Object context, byte[] ksBits, String ksPassword,
292+
String password, Boolean isSourceIpCheckEnabled) {
290293
setEncryptorPassword(password);
291294
configLog4j();
292295
LOGGER.info("Start console proxy with context");
@@ -308,7 +311,7 @@ public static void startWithContext(Properties conf, Object context, byte[] ksBi
308311
final ClassLoader loader = Thread.currentThread().getContextClassLoader();
309312
Class<?> contextClazz = loader.loadClass("com.cloud.agent.resource.consoleproxy.ConsoleProxyResource");
310313
authMethod = contextClazz.getDeclaredMethod("authenticateConsoleAccess", String.class, String.class,
311-
String.class, String.class, String.class, Boolean.class, String.class);
314+
String.class, String.class, String.class, Boolean.class, String.class, String.class);
312315
reportMethod = contextClazz.getDeclaredMethod("reportLoadInfo", String.class);
313316
ensureRouteMethod = contextClazz.getDeclaredMethod("ensureRoute", String.class);
314317
} catch (SecurityException e) {
@@ -326,34 +329,40 @@ public static void startWithContext(Properties conf, Object context, byte[] ksBi
326329
Properties props = new Properties();
327330
if (confs == null) {
328331
final File file = PropertiesUtil.findConfigFile("consoleproxy.properties");
329-
if (file == null)
332+
if (file == null) {
330333
LOGGER.info("Can't load consoleproxy.properties from classpath, will use default configuration");
331-
else
334+
} else {
332335
try {
333336
confs = new FileInputStream(file);
334337
} catch (FileNotFoundException e) {
335338
LOGGER.info("Ignoring file not found exception and using defaults");
336339
}
340+
}
337341
}
338342
if (confs != null) {
339343
try {
340344
props.load(confs);
341345

346+
if (conf == null) {
347+
conf = new Properties();
348+
}
342349
for (Object key : props.keySet()) {
343350
// give properties passed via context high priority, treat properties from consoleproxy.properties
344351
// as default values
345-
if (conf.get(key) == null)
352+
if (conf.get(key) == null) {
346353
conf.put(key, props.get(key));
354+
}
347355
}
348356
} catch (Exception e) {
349357
LOGGER.error(e.toString(), e);
358+
} finally {
359+
try {
360+
confs.close();
361+
} catch (IOException e) {
362+
LOGGER.error("Failed to close consoleproxy.properties : " + e.toString(), e);
363+
}
350364
}
351365
}
352-
try {
353-
confs.close();
354-
} catch (IOException e) {
355-
LOGGER.error("Failed to close consolepropxy.properties : " + e.toString(), e);
356-
}
357366

358367
start(conf);
359368
}
@@ -474,8 +483,8 @@ public static ConsoleProxyClient getVncViewer(ConsoleProxyClientParam param) thr
474483
LOGGER.info("The rfb thread died, reinitializing the viewer " + viewer);
475484
viewer.initClient(param);
476485
} else if (!param.getClientHostPassword().equals(viewer.getClientHostPassword())) {
477-
LOGGER.warn("Bad sid detected(VNC port may be reused). sid in session: " + viewer.getClientHostPassword() + ", sid in request: " +
478-
param.getClientHostPassword());
486+
LOGGER.warn("Bad sid detected(VNC port may be reused). sid in session: " + viewer.getClientHostPassword() +
487+
", sid in request: " + param.getClientHostPassword());
479488
viewer.initClient(param);
480489
}
481490
}
@@ -484,8 +493,9 @@ public static ConsoleProxyClient getVncViewer(ConsoleProxyClientParam param) thr
484493
ConsoleProxyClientStatsCollector statsCollector = getStatsCollector();
485494
String loadInfo = statsCollector.getStatsReport();
486495
reportLoadInfo(loadInfo);
487-
if (LOGGER.isDebugEnabled())
496+
if (LOGGER.isDebugEnabled()) {
488497
LOGGER.debug("Report load change : " + loadInfo);
498+
}
489499
}
490500

491501
return viewer;
@@ -509,13 +519,15 @@ public static ConsoleProxyClient getAjaxVncViewer(ConsoleProxyClientParam param,
509519
// protected against malicious attack by modifying URL content
510520
if (ajaxSession != null) {
511521
long ajaxSessionIdFromUrl = Long.parseLong(ajaxSession);
512-
if (ajaxSessionIdFromUrl != viewer.getAjaxSessionId())
522+
if (ajaxSessionIdFromUrl != viewer.getAjaxSessionId()) {
513523
throw new AuthenticationException("Cannot use the existing viewer " + viewer + ": modified AJAX session id");
524+
}
514525
}
515526

516-
if (param.getClientHostPassword() == null || param.getClientHostPassword().isEmpty() ||
517-
!param.getClientHostPassword().equals(viewer.getClientHostPassword()))
527+
if (param.getClientHostPassword() == null || param.getClientHostPassword().isEmpty()
528+
|| !param.getClientHostPassword().equals(viewer.getClientHostPassword())) {
518529
throw new AuthenticationException("Cannot use the existing viewer " + viewer + ": bad sid");
530+
}
519531

520532
if (!viewer.isFrontEndAlive()) {
521533

@@ -529,8 +541,9 @@ public static ConsoleProxyClient getAjaxVncViewer(ConsoleProxyClientParam param,
529541
ConsoleProxyClientStatsCollector statsCollector = getStatsCollector();
530542
String loadInfo = statsCollector.getStatsReport();
531543
reportLoadInfo(loadInfo);
532-
if (LOGGER.isDebugEnabled())
544+
if (LOGGER.isDebugEnabled()) {
533545
LOGGER.debug("Report load change : " + loadInfo);
546+
}
534547
}
535548
return viewer;
536549
}
@@ -566,9 +579,11 @@ public static void authenticationExternally(ConsoleProxyClientParam param) throw
566579
ConsoleProxyAuthenticationResult authResult = authenticateConsoleAccess(param, false);
567580

568581
if (authResult == null || !authResult.isSuccess()) {
569-
LOGGER.warn("External authenticator failed authentication request for vm " + param.getClientTag() + " with sid " + param.getClientHostPassword());
582+
LOGGER.warn("External authenticator failed authentication request for vm " + param.getClientTag()
583+
+ " with sid " + param.getClientHostPassword());
570584

571-
throw new AuthenticationException("External authenticator failed request for vm " + param.getClientTag() + " with sid " + param.getClientHostPassword());
585+
throw new AuthenticationException("External authenticator failed request for vm " + param.getClientTag()
586+
+ " with sid " + param.getClientHostPassword());
572587
}
573588
}
574589

@@ -596,50 +611,55 @@ public void execute(Runnable r) {
596611
}
597612

598613
public static ConsoleProxyNoVncClient getNoVncViewer(ConsoleProxyClientParam param, String ajaxSession,
599-
Session session) throws AuthenticationException {
614+
Session session) throws AuthenticationException {
600615
boolean reportLoadChange = false;
601616
String clientKey = param.getClientMapKey();
602-
synchronized (connectionMap) {
603-
ConsoleProxyClient viewer = connectionMap.get(clientKey);
604-
if (viewer == null || viewer.getClass() != ConsoleProxyNoVncClient.class) {
605-
authenticationExternally(param);
606-
viewer = new ConsoleProxyNoVncClient(session);
607-
viewer.initClient(param);
617+
LOGGER.debug("Getting NoVNC viewer for {}. Client tag: {}. session UUID: {}",
618+
clientKey, param.getClientTag(), param.getSessionUuid());
619+
synchronized (connectionMap) {
620+
ConsoleProxyClient viewer = connectionMap.get(clientKey);
621+
if (viewer == null || viewer.getClass() != ConsoleProxyNoVncClient.class) {
622+
authenticationExternally(param);
623+
viewer = new ConsoleProxyNoVncClient(session);
624+
viewer.initClient(param);
625+
626+
connectionMap.put(clientKey, viewer);
627+
reportLoadChange = true;
628+
} else {
629+
if (param.getClientHostPassword() == null || param.getClientHostPassword().isEmpty()
630+
|| !param.getClientHostPassword().equals(viewer.getClientHostPassword())) {
631+
throw new AuthenticationException("Cannot use the existing viewer " + viewer + ": bad sid");
632+
}
608633

609-
connectionMap.put(clientKey, viewer);
610-
reportLoadChange = true;
611-
} else {
612-
if (param.getClientHostPassword() == null || param.getClientHostPassword().isEmpty() ||
613-
!param.getClientHostPassword().equals(viewer.getClientHostPassword()))
614-
throw new AuthenticationException("Cannot use the existing viewer " + viewer + ": bad sid");
634+
try {
635+
authenticationExternally(param);
636+
} catch (Exception e) {
637+
LOGGER.error("Authentication failed for param: " + param);
638+
return null;
639+
}
640+
LOGGER.info("Initializing new novnc client and disconnecting existing session");
641+
try {
642+
((ConsoleProxyNoVncClient) viewer).getSession().disconnect();
643+
} catch (IOException e) {
644+
LOGGER.error("Exception while disconnect session of novnc viewer object: " + viewer, e);
645+
}
646+
removeViewer(viewer);
647+
viewer = new ConsoleProxyNoVncClient(session);
648+
viewer.initClient(param);
649+
connectionMap.put(clientKey, viewer);
650+
reportLoadChange = true;
651+
}
615652

616-
try {
617-
authenticationExternally(param);
618-
} catch (Exception e) {
619-
LOGGER.error("Authentication failed for param: " + param);
620-
return null;
621-
}
622-
LOGGER.info("Initializing new novnc client and disconnecting existing session");
623-
try {
624-
((ConsoleProxyNoVncClient)viewer).getSession().disconnect();
625-
} catch (IOException e) {
626-
LOGGER.error("Exception while disconnect session of novnc viewer object: " + viewer, e);
627-
}
628-
removeViewer(viewer);
629-
viewer = new ConsoleProxyNoVncClient(session);
630-
viewer.initClient(param);
631-
connectionMap.put(clientKey, viewer);
632-
reportLoadChange = true;
633-
}
634653

635654
if (reportLoadChange) {
636655
ConsoleProxyClientStatsCollector statsCollector = getStatsCollector();
637656
String loadInfo = statsCollector.getStatsReport();
638657
reportLoadInfo(loadInfo);
639-
if (LOGGER.isDebugEnabled())
658+
if (LOGGER.isDebugEnabled()) {
640659
LOGGER.debug("Report load change : " + loadInfo);
660+
}
641661
}
642-
return (ConsoleProxyNoVncClient)viewer;
662+
return (ConsoleProxyNoVncClient) viewer;
643663
}
644664
}
645665
}

0 commit comments

Comments
 (0)