From 0597358dde292d739809cd7426e0b55921372ccf Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 22 Oct 2021 18:28:59 -0700 Subject: [PATCH] GUACAMOLE-956: Decouple tunnel UUID from HTTP tunnel session identification. --- .../src/main/webapp/modules/Tunnel.js | 30 ++- .../servlet/GuacamoleHTTPTunnelMap.java | 53 ++--- .../servlet/GuacamoleHTTPTunnelServlet.java | 196 ++++++++++++------ 3 files changed, 189 insertions(+), 90 deletions(-) diff --git a/guacamole-common-js/src/main/webapp/modules/Tunnel.js b/guacamole-common-js/src/main/webapp/modules/Tunnel.js index 49129888e..eef12b58c 100644 --- a/guacamole-common-js/src/main/webapp/modules/Tunnel.js +++ b/guacamole-common-js/src/main/webapp/modules/Tunnel.js @@ -309,6 +309,25 @@ Guacamole.HTTPTunnel = function(tunnelURL, crossDomain, extraTunnelHeaders) { */ var extraHeaders = extraTunnelHeaders || {}; + /** + * The name of the HTTP header containing the session token specific to the + * HTTP tunnel implementation. + * + * @private + * @constant + * @type {string} + */ + var TUNNEL_TOKEN_HEADER = 'Guacamole-Tunnel-Token'; + + /** + * The session token currently assigned to this HTTP tunnel. All distinct + * HTTP tunnel connections will have their own dedicated session token. + * + * @private + * @type {string} + */ + var tunnelSessionToken = null; + /** * Adds the configured additional headers to the given request. * @@ -453,6 +472,7 @@ Guacamole.HTTPTunnel = function(tunnelURL, crossDomain, extraTunnelHeaders) { message_xmlhttprequest.withCredentials = withCredentials; addExtraHeaders(message_xmlhttprequest, extraHeaders); message_xmlhttprequest.setRequestHeader("Content-type", "application/octet-stream"); + message_xmlhttprequest.setRequestHeader(TUNNEL_TOKEN_HEADER, tunnelSessionToken); // Once response received, send next queued event. message_xmlhttprequest.onreadystatechange = function() { @@ -697,6 +717,7 @@ Guacamole.HTTPTunnel = function(tunnelURL, crossDomain, extraTunnelHeaders) { // Make request, increment request ID var xmlhttprequest = new XMLHttpRequest(); xmlhttprequest.open("GET", TUNNEL_READ + tunnel.uuid + ":" + (request_id++)); + xmlhttprequest.setRequestHeader(TUNNEL_TOKEN_HEADER, tunnelSessionToken); xmlhttprequest.withCredentials = withCredentials; addExtraHeaders(xmlhttprequest, extraHeaders); xmlhttprequest.send(null); @@ -728,8 +749,15 @@ Guacamole.HTTPTunnel = function(tunnelURL, crossDomain, extraTunnelHeaders) { reset_timeout(); - // Get UUID from response + // Get UUID and HTTP-specific tunnel session token from response tunnel.setUUID(connect_xmlhttprequest.responseText); + tunnelSessionToken = connect_xmlhttprequest.getResponseHeader(TUNNEL_TOKEN_HEADER); + + // Fail connect attempt if token is not successfully assigned + if (!tunnelSessionToken) { + close_tunnel(new Guacamole.Status(Guacamole.Status.Code.UPSTREAM_NOT_FOUND)); + return; + } // Mark as open tunnel.setState(Guacamole.Tunnel.State.OPEN); diff --git a/guacamole-common/src/main/java/org/apache/guacamole/servlet/GuacamoleHTTPTunnelMap.java b/guacamole-common/src/main/java/org/apache/guacamole/servlet/GuacamoleHTTPTunnelMap.java index eaa2fcf1c..918ca206b 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/servlet/GuacamoleHTTPTunnelMap.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/servlet/GuacamoleHTTPTunnelMap.java @@ -58,7 +58,8 @@ class GuacamoleHTTPTunnelMap { private final ScheduledExecutorService executor = Executors.newScheduledThreadPool(1); /** - * Map of all tunnels that are using HTTP, indexed by tunnel UUID. + * Map of all tunnels that are using HTTP, indexed by their tunnel-specific + * session tokens. */ private final ConcurrentMap tunnelMap = new ConcurrentHashMap(); @@ -141,22 +142,22 @@ class GuacamoleHTTPTunnelMap { } /** - * Returns the GuacamoleTunnel having the given UUID, wrapped within a - * GuacamoleHTTPTunnel. If the no tunnel having the given UUID is - * available, null is returned. + * Returns the GuacamoleTunnel associated with the given tunnel-specific + * session token, wrapped within a GuacamoleHTTPTunnel. If the no tunnel + * is associated with the given token, null is returned. * - * @param uuid - * The UUID of the tunnel to retrieve. + * @param tunnelSessionToken + * The tunnel-specific session token of the HTTP tunnel to retrieve. * * @return - * The GuacamoleTunnel having the given UUID, wrapped within a - * GuacamoleHTTPTunnel, if such a tunnel exists, or null if there is no - * such tunnel. + * The GuacamoleTunnel associated with the given tunnel-specific + * session token, wrapped within a GuacamoleHTTPTunnel, if such a + * tunnel exists, or null if there is no such tunnel. */ - public GuacamoleHTTPTunnel get(String uuid) { + public GuacamoleHTTPTunnel get(String tunnelSessionToken) { // Update the last access time - GuacamoleHTTPTunnel tunnel = tunnelMap.get(uuid); + GuacamoleHTTPTunnel tunnel = tunnelMap.get(tunnelSessionToken); if (tunnel != null) tunnel.access(); @@ -169,32 +170,34 @@ class GuacamoleHTTPTunnelMap { * Registers that a new connection has been established using HTTP via the * given GuacamoleTunnel. * - * @param uuid - * The UUID of the tunnel being added (registered). + * @param tunnelSessionToken + * The tunnel-specific session token of the HTTP tunnel being added + * (registered). * * @param tunnel * The GuacamoleTunnel being registered, its associated connection * having just been established via HTTP. */ - public void put(String uuid, GuacamoleTunnel tunnel) { - tunnelMap.put(uuid, new GuacamoleHTTPTunnel(tunnel)); + public void put(String tunnelSessionToken, GuacamoleTunnel tunnel) { + tunnelMap.put(tunnelSessionToken, new GuacamoleHTTPTunnel(tunnel)); } /** - * Removes the GuacamoleTunnel having the given UUID, if such a tunnel - * exists. The original tunnel is returned wrapped within a - * GuacamoleHTTPTunnel. + * Removes the GuacamoleTunnel associated with the given tunnel-specific + * session token, if such a tunnel exists. The original tunnel is returned + * wrapped within a GuacamoleHTTPTunnel. * - * @param uuid - * The UUID of the tunnel to remove (deregister). + * @param tunnelSessionToken + * The tunnel-specific session token of the HTTP tunnel to remove + * (deregister). * * @return - * The GuacamoleTunnel having the given UUID, if such a tunnel exists, - * wrapped within a GuacamoleHTTPTunnel, or null if no such tunnel - * exists and no removal was performed. + * The GuacamoleTunnel having the given tunnel-specific session token, + * if such a tunnel exists, wrapped within a GuacamoleHTTPTunnel, or + * null if no such tunnel exists and no removal was performed. */ - public GuacamoleHTTPTunnel remove(String uuid) { - return tunnelMap.remove(uuid); + public GuacamoleHTTPTunnel remove(String tunnelSessionToken) { + return tunnelMap.remove(tunnelSessionToken); } /** diff --git a/guacamole-common/src/main/java/org/apache/guacamole/servlet/GuacamoleHTTPTunnelServlet.java b/guacamole-common/src/main/java/org/apache/guacamole/servlet/GuacamoleHTTPTunnelServlet.java index be2da1312..492076933 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/servlet/GuacamoleHTTPTunnelServlet.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/servlet/GuacamoleHTTPTunnelServlet.java @@ -25,6 +25,8 @@ import java.io.InputStreamReader; import java.io.OutputStreamWriter; import java.io.Reader; import java.io.Writer; +import java.security.SecureRandom; +import java.util.Base64; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -37,7 +39,6 @@ import org.apache.guacamole.GuacamoleServerException; import org.apache.guacamole.io.GuacamoleReader; import org.apache.guacamole.io.GuacamoleWriter; import org.apache.guacamole.net.GuacamoleTunnel; -import org.apache.guacamole.protocol.GuacamoleStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,10 +54,17 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet { private final Logger logger = LoggerFactory.getLogger(GuacamoleHTTPTunnelServlet.class); /** - * Map of absolutely all active tunnels using HTTP, indexed by tunnel UUID. + * Map of absolutely all active tunnels using HTTP, indexed by tunnel + * session token. */ private final GuacamoleHTTPTunnelMap tunnels = new GuacamoleHTTPTunnelMap(); + /** + * The name of the HTTP header that contains the tunnel-specific session + * token identifying each active and distinct HTTP tunnel connection. + */ + private static final String TUNNEL_TOKEN_HEADER_NAME = "Guacamole-Tunnel-Token"; + /** * The prefix of the query string which denotes a tunnel read operation. */ @@ -68,29 +76,64 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet { private static final String WRITE_PREFIX = "write:"; /** - * The length of the read prefix, in characters. + * Instance of SecureRandom for generating the session token specific to + * each distinct HTTP tunnel connection. */ - private static final int READ_PREFIX_LENGTH = READ_PREFIX.length(); + private final SecureRandom secureRandom = new SecureRandom(); /** - * The length of the write prefix, in characters. + * Instance of Base64.Encoder for encoding random session tokens as + * strings. */ - private static final int WRITE_PREFIX_LENGTH = WRITE_PREFIX.length(); + private final Base64.Encoder encoder = Base64.getEncoder(); /** - * The length of every tunnel UUID, in characters. + * Generates a new, securely-random session token that may be used to + * represent the ongoing communication session of a distinct HTTP tunnel + * connection. + * + * @return + * A new, securely-random session token. */ - private static final int UUID_LENGTH = 36; + protected String generateToken() { + byte[] bytes = new byte[33]; + secureRandom.nextBytes(bytes); + return encoder.encodeToString(bytes); + } /** * Registers the given tunnel such that future read/write requests to that * tunnel will be properly directed. * + * @deprecated + * This function has been deprecated in favor of {@link #registerTunnel(java.lang.String, org.apache.guacamole.net.GuacamoleTunnel)}, + * which decouples identification of HTTP tunnel sessions from the + * tunnel UUID. + * * @param tunnel * The tunnel to register. */ + @Deprecated protected void registerTunnel(GuacamoleTunnel tunnel) { - tunnels.put(tunnel.getUUID().toString(), tunnel); + registerTunnel(tunnel.getUUID().toString(), tunnel); + } + + /** + * Registers the given HTTP tunnel such that future read/write requests + * including the given tunnel-specific session token will be properly + * directed. The session token must be unpredictable (securely-random) and + * unique across all active HTTP tunnels. It is recommended that each HTTP + * tunnel session token be obtained through calling {@link #generateToken()}. + * + * @param tunnelSessionToken + * The tunnel-specific session token to associate with the HTTP tunnel + * being registered. + * + * @param tunnel + * The tunnel to register. + */ + protected void registerTunnel(String tunnelSessionToken, GuacamoleTunnel tunnel) { + tunnels.put(tunnelSessionToken, tunnel); logger.debug("Registered tunnel \"{}\".", tunnel.getUUID()); } @@ -98,33 +141,56 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet { * Deregisters the given tunnel such that future read/write requests to * that tunnel will be rejected. * + * @deprecated + * This function has been deprecated in favor of {@link #deregisterTunnel(java.lang.String)}, + * which decouples identification of HTTP tunnel sessions from the + * tunnel UUID. + * * @param tunnel * The tunnel to deregister. */ + @Deprecated protected void deregisterTunnel(GuacamoleTunnel tunnel) { - tunnels.remove(tunnel.getUUID().toString()); - logger.debug("Deregistered tunnel \"{}\".", tunnel.getUUID()); + deregisterTunnel(tunnel.getUUID().toString()); } /** - * Returns the tunnel with the given UUID, if it has been registered with - * registerTunnel() and not yet deregistered with deregisterTunnel(). + * Deregisters the HTTP tunnel associated with the given tunnel-specific + * session token such that future read/write requests to that tunnel will + * be rejected. Each HTTP tunnel must be associated with a session token + * unique to that tunnel via a call {@link #registerTunnel(java.lang.String, org.apache.guacamole.net.GuacamoleTunnel)}. + * + * @param tunnelSessionToken + * The tunnel-specific session token associated with the HTTP tunnel + * being deregistered. + */ + protected void deregisterTunnel(String tunnelSessionToken) { + GuacamoleTunnel tunnel = tunnels.remove(tunnelSessionToken); + if (tunnel != null) + logger.debug("Deregistered tunnel \"{}\".", tunnel.getUUID()); + } + + /** + * Returns the tunnel associated with the given tunnel-specific session + * token, if it has been registered with {@link #registerTunnel(java.lang.String, org.apache.guacamole.net.GuacamoleTunnel)} + * and not yet deregistered with {@link #deregisterTunnel(java.lang.String)}. * - * @param tunnelUUID - * The UUID of registered tunnel. + * @param tunnelSessionToken + * The tunnel-specific session token associated with the HTTP tunnel to + * be retrieved. * * @return - * The tunnel corresponding to the given UUID. + * The tunnel corresponding to the given session token. * * @throws GuacamoleException * If the requested tunnel does not exist because it has not yet been * registered or it has been deregistered. */ - protected GuacamoleTunnel getTunnel(String tunnelUUID) + protected GuacamoleTunnel getTunnel(String tunnelSessionToken) throws GuacamoleException { // Pull tunnel from map - GuacamoleTunnel tunnel = tunnels.get(tunnelUUID); + GuacamoleTunnel tunnel = tunnels.get(tunnelSessionToken); if (tunnel == null) throw new GuacamoleResourceNotFoundException("No such tunnel."); @@ -209,52 +275,54 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet { if (query == null) throw new GuacamoleClientException("No query string provided."); - // If connect operation, call doConnect() and return tunnel UUID - // in response. + // If connect operation, call doConnect() and return tunnel + // session token and UUID in response if (query.equals("connect")) { GuacamoleTunnel tunnel = doConnect(request); - if (tunnel != null) { + if (tunnel == null) + throw new GuacamoleResourceNotFoundException("No tunnel created."); - // Register newly-created tunnel - registerTunnel(tunnel); + // Register newly-created tunnel + String tunnelSessionToken = generateToken(); + registerTunnel(tunnelSessionToken, tunnel); - try { - // Ensure buggy browsers do not cache response - response.setHeader("Cache-Control", "no-cache"); - - // Send UUID to client - response.getWriter().print(tunnel.getUUID().toString()); - } - catch (IOException e) { - throw new GuacamoleServerException(e); - } + try { + // Ensure buggy browsers do not cache response + response.setHeader("Cache-Control", "no-cache"); + // Include tunnel session token for future requests + response.setHeader(TUNNEL_TOKEN_HEADER_NAME, tunnelSessionToken); + + // Send UUID to client + response.getWriter().print(tunnel.getUUID().toString()); + } + catch (IOException e) { + throw new GuacamoleServerException(e); } - // Failed to connect - else - throw new GuacamoleResourceNotFoundException("No tunnel created."); + // Connection successful + return; } - // If read operation, call doRead() with tunnel UUID, ignoring any - // characters following the tunnel UUID. - else if (query.startsWith(READ_PREFIX)) - doRead(request, response, query.substring( - READ_PREFIX_LENGTH, - READ_PREFIX_LENGTH + UUID_LENGTH)); + // Pull tunnel-specific session token from request + String tunnelSessionToken = request.getHeader(TUNNEL_TOKEN_HEADER_NAME); + if (tunnelSessionToken == null) + throw new GuacamoleClientException("The HTTP tunnel session " + + "token is required for all requests after " + + "connecting."); - // If write operation, call doWrite() with tunnel UUID, ignoring any - // characters following the tunnel UUID. + // Dispatch valid tunnel read/write operations + if (query.startsWith(READ_PREFIX)) + doRead(request, response, tunnelSessionToken); else if (query.startsWith(WRITE_PREFIX)) - doWrite(request, response, query.substring( - WRITE_PREFIX_LENGTH, - WRITE_PREFIX_LENGTH + UUID_LENGTH)); + doWrite(request, response, tunnelSessionToken); // Otherwise, invalid operation else throw new GuacamoleClientException("Invalid tunnel operation: " + query); + } // Catch any thrown guacamole exception and attempt to pass within the @@ -308,20 +376,20 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet { * Any data to be sent to the client in response to the write request * should be written to the response body of this HttpServletResponse. * - * @param tunnelUUID - * The UUID of the tunnel to read from, as specified in the write - * request. This tunnel must have been created by a previous call to - * doConnect(). + * @param tunnelSessionToken + * The tunnel-specific session token of the HTTP tunnel to read from, + * as specified in the read request. This tunnel must have been created + * by a previous call to doConnect(). * * @throws GuacamoleException * If an error occurs while handling the read request. */ protected void doRead(HttpServletRequest request, - HttpServletResponse response, String tunnelUUID) + HttpServletResponse response, String tunnelSessionToken) throws GuacamoleException { // Get tunnel, ensure tunnel exists - GuacamoleTunnel tunnel = getTunnel(tunnelUUID); + GuacamoleTunnel tunnel = getTunnel(tunnelSessionToken); // Ensure tunnel is open if (!tunnel.isOpen()) @@ -371,7 +439,7 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet { // Close tunnel immediately upon EOF if (message == null) { - deregisterTunnel(tunnel); + deregisterTunnel(tunnelSessionToken); tunnel.close(); } @@ -385,7 +453,7 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet { catch (GuacamoleConnectionClosedException e) { // Deregister and close - deregisterTunnel(tunnel); + deregisterTunnel(tunnelSessionToken); tunnel.close(); // End-of-instructions marker @@ -398,7 +466,7 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet { catch (GuacamoleException e) { // Deregister and close - deregisterTunnel(tunnel); + deregisterTunnel(tunnelSessionToken); tunnel.close(); throw e; @@ -416,7 +484,7 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet { logger.debug("Error writing to servlet output stream", e); // Deregister and close - deregisterTunnel(tunnel); + deregisterTunnel(tunnelSessionToken); tunnel.close(); } @@ -439,19 +507,19 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet { * @param response * The HttpServletResponse associated with the write request received. * - * @param tunnelUUID - * The UUID of the tunnel to write to, as specified in the write - * request. This tunnel must have been created by a previous call to - * doConnect(). + * @param tunnelSessionToken + * The tunnel-specific session token of the HTTP tunnel to write to, + * as specified in the write request. This tunnel must have been created + * by a previous call to doConnect(). * * @throws GuacamoleException * If an error occurs while handling the write request. */ protected void doWrite(HttpServletRequest request, - HttpServletResponse response, String tunnelUUID) + HttpServletResponse response, String tunnelSessionToken) throws GuacamoleException { - GuacamoleTunnel tunnel = getTunnel(tunnelUUID); + GuacamoleTunnel tunnel = getTunnel(tunnelSessionToken); // We still need to set the content type to avoid the default of // text/html, as such a content type would cause some browsers to @@ -498,7 +566,7 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet { catch (IOException e) { // Deregister and close - deregisterTunnel(tunnel); + deregisterTunnel(tunnelSessionToken); tunnel.close(); throw new GuacamoleServerException("I/O Error sending data to server: " + e.getMessage(), e);