From 4a1527b1d4311bbf9d76468141dc68d02a90efa4 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 20 Jan 2019 13:26:44 -0800 Subject: [PATCH 01/12] GUACAMOLE-524: Provide bridge implementations of both old and new versions of connect() for sake of compatibility. --- .../guacamole/net/auth/Connectable.java | 52 ++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connectable.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connectable.java index 39b12f98f..c597fe6bc 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connectable.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connectable.java @@ -19,6 +19,7 @@ package org.apache.guacamole.net.auth; +import java.util.Collections; import java.util.Map; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.net.GuacamoleTunnel; @@ -29,6 +30,37 @@ import org.apache.guacamole.protocol.GuacamoleClientInformation; */ public interface Connectable { + /** + * Establishes a connection to guacd using the information associated with + * this object. The connection will be provided the given client + * information. + * + * @deprecated + * This function has been deprecated in favor of + * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation, java.util.Map)}, + * which allows for connection parameter tokens to be injected and + * applied by cooperating extensions, replacing the functionality + * previously provided through the {@link org.apache.guacamole.token.StandardTokens} + * class. It continues to be defined on this interface for + * compatibility. New implementations should instead implement + * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation, java.util.Map)}. + * + * @param info + * Information associated with the connecting client. + * + * @return + * A fully-established GuacamoleTunnel. + * + * @throws GuacamoleException + * If an error occurs while connecting to guacd, or if permission to + * connect is denied. + */ + @Deprecated + default GuacamoleTunnel connect(GuacamoleClientInformation info) + throws GuacamoleException { + return this.connect(info, Collections.emptyMap()); + } + /** * Establishes a connection to guacd using the information associated with * this object. The connection will be provided the given client @@ -36,6 +68,17 @@ public interface Connectable { * apply the given tokens when configuring the connection, such as with a * {@link org.apache.guacamole.token.TokenFilter}. * + *

A default implementation which invokes the old, deprecated + * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation)} + * is provided solely for compatibility with extensions which implement only + * the deprecated function. This default implementation is useful only + * for extensions relying on the deprecated function and will be removed + * when the deprecated function is removed. + * + *

New implementations should always implement this function + * in favor of the deprecated + * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation)}. + * * @see Parameter Tokens * * @param info @@ -54,8 +97,13 @@ public interface Connectable { * If an error occurs while connecting to guacd, or if permission to * connect is denied. */ - public GuacamoleTunnel connect(GuacamoleClientInformation info, - Map tokens) throws GuacamoleException; + default GuacamoleTunnel connect(GuacamoleClientInformation info, + Map tokens) throws GuacamoleException { + + // Allow old implementations of Connectable to continue to work + return this.connect(info); + + } /** * Returns the number of active connections associated with this object. From b6383879201f49f570db8b4fc603e6f70ca45cfb Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 21 Jan 2019 17:38:21 -0800 Subject: [PATCH 02/12] GUACAMOLE-524: Provide distinct, documented, internal access to raw GuacamoleConfiguration within SimpleConnection. While raw, internal access to the GuacamoleConfiguration was originally present in older versions of SimpleConnection, this access was undocumented and could result in unexpected behavior if the default constructor was used, getConfiguration() was overridden, or setConfiguration() was called. --- .../net/auth/simple/SimpleConnection.java | 37 ++++++++++++++----- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java index 2dec26a94..adbcaed90 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java @@ -52,7 +52,7 @@ public class SimpleConnection extends AbstractConnection { /** * Backing configuration, containing all sensitive information. */ - private GuacamoleConfiguration config; + private GuacamoleConfiguration fullConfig; /** * Creates a completely uninitialized SimpleConnection. @@ -71,19 +71,36 @@ public class SimpleConnection extends AbstractConnection { */ public SimpleConnection(String name, String identifier, GuacamoleConfiguration config) { - - // Set name - setName(name); - // Set identifier - setIdentifier(identifier); + super.setName(name); + super.setIdentifier(identifier); + super.setConfiguration(config); - // Set config - setConfiguration(config); - this.config = config; + this.fullConfig = config; } + /** + * Returns the GuacamoleConfiguration describing how to connect to this + * connection. Unlike the GuacamoleConfiguration returned by + * {@link #getConfiguration()}, which may omit or tokenize information, + * the GuacamoleConfiguration returned by this function contains the full + * configuration to be used to establish the connection. + * + * @return + * The full GuacamoleConfiguration describing how to connect to this + * connection, without any information omitted or tokenized. + */ + protected GuacamoleConfiguration getFullConfiguration() { + return fullConfig; + } + + @Override + public void setConfiguration(GuacamoleConfiguration config) { + super.setConfiguration(config); + this.fullConfig = config; + } + @Override public int getActiveConnections() { return 0; @@ -112,7 +129,7 @@ public class SimpleConnection extends AbstractConnection { int port = proxyConfig.getPort(); // Apply tokens to config parameters - GuacamoleConfiguration filteredConfig = new GuacamoleConfiguration(config); + GuacamoleConfiguration filteredConfig = new GuacamoleConfiguration(getFullConfiguration()); new TokenFilter(tokens).filterValues(filteredConfig.getParameters()); GuacamoleSocket socket; From 7e67dde75155ab88af4570bcfeb3a94175093fc9 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 21 Jan 2019 17:42:47 -0800 Subject: [PATCH 03/12] GUACAMOLE-524: Leverage thread-local storage to allow overriding the deprecated connect() function to have the expected effect within subclasses of SimpleConnection. --- .../net/auth/simple/SimpleConnection.java | 45 +++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java index adbcaed90..3713b6b77 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java @@ -55,7 +55,27 @@ public class SimpleConnection extends AbstractConnection { private GuacamoleConfiguration fullConfig; /** - * Creates a completely uninitialized SimpleConnection. + * The tokens which should apply strictly to the next call to + * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation)}. + * This storage is intended as a temporary bridge allowing the old version + * of connect() to be overridden while still resulting in the same behavior + * as older versions of SimpleConnection. This storage should be + * removed once support for the old, deprecated connect() is removed. + */ + private final ThreadLocal> currentTokens = + new ThreadLocal>() { + + @Override + protected Map initialValue() { + return Collections.emptyMap(); + } + + }; + + /** + * Creates a completely uninitialized SimpleConnection. The name, + * identifier, and configuration of this SimpleConnection must eventually + * be set before the SimpleConnection may be used. */ public SimpleConnection() { } @@ -117,8 +137,9 @@ public class SimpleConnection extends AbstractConnection { } @Override - public GuacamoleTunnel connect(GuacamoleClientInformation info, - Map tokens) throws GuacamoleException { + @SuppressWarnings("deprecation") + public GuacamoleTunnel connect(GuacamoleClientInformation info) + throws GuacamoleException { // Retrieve proxy configuration from environment Environment environment = new LocalEnvironment(); @@ -130,7 +151,7 @@ public class SimpleConnection extends AbstractConnection { // Apply tokens to config parameters GuacamoleConfiguration filteredConfig = new GuacamoleConfiguration(getFullConfiguration()); - new TokenFilter(tokens).filterValues(filteredConfig.getParameters()); + new TokenFilter(currentTokens.get()).filterValues(filteredConfig.getParameters()); GuacamoleSocket socket; @@ -160,6 +181,22 @@ public class SimpleConnection extends AbstractConnection { } return new SimpleGuacamoleTunnel(socket); + + } + + @Override + public GuacamoleTunnel connect(GuacamoleClientInformation info, + Map tokens) throws GuacamoleException { + + // Make received tokens available within the legacy connect() strictly + // in context of the current connect() call + try { + currentTokens.set(tokens); + return connect(info); + } + finally { + currentTokens.remove(); + } } From 6be722ed9de6fc79620a432e43326262721cad53 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 21 Jan 2019 19:55:33 -0800 Subject: [PATCH 04/12] GUACAMOLE-524: Require usages of SimpleConnection to explicitly request automatic interpretation of parameter tokens. Do not enable by default. Previous implementations of SimpleConnection did not interpret parameter tokens automatically. Adding that behavior now could have security implications for downstream users of the class if parameter values may unexpectedly contain substrings which would be interpreted as tokens, particularly if parameter values are built from untrusted input. --- .../quickconnect/QuickConnectDirectory.java | 2 +- .../simple/SimpleAuthenticationProvider.java | 2 +- .../net/auth/simple/SimpleConnection.java | 80 +++++++++++++++++-- .../net/auth/simple/SimpleUserContext.java | 34 +++++++- 4 files changed, 107 insertions(+), 11 deletions(-) diff --git a/extensions/guacamole-auth-quickconnect/src/main/java/org/apache/guacamole/auth/quickconnect/QuickConnectDirectory.java b/extensions/guacamole-auth-quickconnect/src/main/java/org/apache/guacamole/auth/quickconnect/QuickConnectDirectory.java index 37b07ba33..cec0432b2 100644 --- a/extensions/guacamole-auth-quickconnect/src/main/java/org/apache/guacamole/auth/quickconnect/QuickConnectDirectory.java +++ b/extensions/guacamole-auth-quickconnect/src/main/java/org/apache/guacamole/auth/quickconnect/QuickConnectDirectory.java @@ -107,7 +107,7 @@ public class QuickConnectDirectory extends SimpleDirectory { String name = QCParser.getName(config); // Create a new connection and set the parent identifier. - Connection connection = new SimpleConnection(name, newConnectionId, config); + Connection connection = new SimpleConnection(name, newConnectionId, config, true); connection.setParentIdentifier(QuickConnectUserContext.ROOT_IDENTIFIER); // Place the object in this directory. diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleAuthenticationProvider.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleAuthenticationProvider.java index 6d08d99ac..202181ab9 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleAuthenticationProvider.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleAuthenticationProvider.java @@ -167,7 +167,7 @@ public abstract class SimpleAuthenticationProvider return null; // Return user context restricted to authorized configs - return new SimpleUserContext(this, authenticatedUser.getIdentifier(), configs); + return new SimpleUserContext(this, authenticatedUser.getIdentifier(), configs, true); } diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java index 3713b6b77..cb9dcde55 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java @@ -54,6 +54,13 @@ public class SimpleConnection extends AbstractConnection { */ private GuacamoleConfiguration fullConfig; + /** + * Whether parameter tokens in the underlying GuacamoleConfiguration should + * be automatically applied upon connecting. If false, parameter tokens + * will not be interpreted at all. + */ + private final boolean interpretTokens; + /** * The tokens which should apply strictly to the next call to * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation)}. @@ -75,28 +82,81 @@ public class SimpleConnection extends AbstractConnection { /** * Creates a completely uninitialized SimpleConnection. The name, * identifier, and configuration of this SimpleConnection must eventually - * be set before the SimpleConnection may be used. + * be set before the SimpleConnection may be used. Parameter tokens within + * the GuacamoleConfiguration eventually supplied with + * {@link #setConfiguration(org.apache.guacamole.protocol.GuacamoleConfiguration)} + * will not be interpreted. */ public SimpleConnection() { + this(false); + } + + /** + * Creates a completely uninitialized SimpleConnection. The name, + * identifier, and configuration of this SimpleConnection must eventually + * be set before the SimpleConnection may be used. Parameter tokens within + * the GuacamoleConfiguration eventually supplied with + * {@link #setConfiguration(org.apache.guacamole.protocol.GuacamoleConfiguration)} + * will not be interpreted unless explicitly requested with + * {@link #setInterpretTokens(boolean)}. + * + * @param interpretTokens + * Whether parameter tokens in the underlying GuacamoleConfiguration + * should be automatically applied upon connecting. If false, parameter + * tokens will not be interpreted at all. + */ + public SimpleConnection(boolean interpretTokens) { + this.interpretTokens = interpretTokens; } /** * Creates a new SimpleConnection having the given identifier and - * GuacamoleConfiguration. + * GuacamoleConfiguration. Parameter tokens within the + * GuacamoleConfiguration will not be interpreted unless explicitly + * requested with {@link #setInterpretTokens(boolean)}. * - * @param name The name to associate with this connection. - * @param identifier The identifier to associate with this connection. - * @param config The configuration describing how to connect to this - * connection. + * @param name + * The name to associate with this connection. + * + * @param identifier + * The identifier to associate with this connection. + * + * @param config + * The configuration describing how to connect to this connection. */ public SimpleConnection(String name, String identifier, GuacamoleConfiguration config) { + this(name, identifier, config, false); + } + + /** + * Creates a new SimpleConnection having the given identifier and + * GuacamoleConfiguration. Parameter tokens will be interpreted if + * explicitly requested. + * + * @param name + * The name to associate with this connection. + * + * @param identifier + * The identifier to associate with this connection. + * + * @param config + * The configuration describing how to connect to this connection. + * + * @param interpretTokens + * Whether parameter tokens in the underlying GuacamoleConfiguration + * should be automatically applied upon connecting. If false, parameter + * tokens will not be interpreted at all. + */ + public SimpleConnection(String name, String identifier, + GuacamoleConfiguration config, boolean interpretTokens) { super.setName(name); super.setIdentifier(identifier); super.setConfiguration(config); this.fullConfig = config; + this.interpretTokens = interpretTokens; } @@ -191,8 +251,14 @@ public class SimpleConnection extends AbstractConnection { // Make received tokens available within the legacy connect() strictly // in context of the current connect() call try { - currentTokens.set(tokens); + + // Automatically filter configurations only if explicitly + // configured to do so + if (interpretTokens) + currentTokens.set(tokens); + return connect(info); + } finally { currentTokens.remove(); diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleUserContext.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleUserContext.java index 03e94fbac..a1dce55af 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleUserContext.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleUserContext.java @@ -59,7 +59,8 @@ public class SimpleUserContext extends AbstractUserContext { * Creates a new SimpleUserContext which provides access to only those * configurations within the given Map. The username is set to the * ANONYMOUS_IDENTIFIER defined by AuthenticatedUser, effectively declaring - * the current user as anonymous. + * the current user as anonymous. Parameter tokens within the given + * GuacamoleConfigurations will not be interpreted. * * @param authProvider * The AuthenticationProvider creating this UserContext. @@ -76,6 +77,8 @@ public class SimpleUserContext extends AbstractUserContext { /** * Creates a new SimpleUserContext for the user with the given username * which provides access to only those configurations within the given Map. + * Parameter tokens within the given GuacamoleConfigurations will not be + * interpreted. * * @param authProvider * The AuthenticationProvider creating this UserContext. @@ -89,6 +92,33 @@ public class SimpleUserContext extends AbstractUserContext { */ public SimpleUserContext(AuthenticationProvider authProvider, String username, Map configs) { + this(authProvider, username, configs, false); + } + + /** + * Creates a new SimpleUserContext for the user with the given username + * which provides access to only those configurations within the given Map. + * Parameter tokens within the given GuacamoleConfigurations will be + * interpreted if explicitly requested. + * + * @param authProvider + * The AuthenticationProvider creating this UserContext. + * + * @param username + * The username of the user associated with this UserContext. + * + * @param configs + * A Map of all configurations for which the user associated with + * this UserContext has read access. + * + * @param interpretTokens + * Whether parameter tokens in the underlying GuacamoleConfigurations + * should be automatically applied upon connecting. If false, parameter + * tokens will not be interpreted at all. + */ + public SimpleUserContext(AuthenticationProvider authProvider, + String username, Map configs, + boolean interpretTokens) { // Produce map of connections from given configs Map connections = new ConcurrentHashMap(configs.size()); @@ -99,7 +129,7 @@ public class SimpleUserContext extends AbstractUserContext { GuacamoleConfiguration config = configEntry.getValue(); // Add as simple connection - Connection connection = new SimpleConnection(identifier, identifier, config); + Connection connection = new SimpleConnection(identifier, identifier, config, true); connection.setParentIdentifier(DEFAULT_ROOT_CONNECTION_GROUP); connections.put(identifier, connection); From 49cd4e5531d7d45b106cf5fc3d89e4bbfb953e75 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 21 Jan 2019 22:03:05 -0800 Subject: [PATCH 05/12] GUACAMOLE-524: Clarify and document SimpleConnection implementation. --- .../net/auth/simple/SimpleConnection.java | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java index cb9dcde55..de218b24d 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java @@ -41,10 +41,10 @@ import org.apache.guacamole.protocol.GuacamoleConfiguration; import org.apache.guacamole.token.TokenFilter; /** - * An extremely basic Connection implementation. The underlying connection to - * guacd is established using the configuration information provided in - * guacamole.properties. Parameter tokens provided to connect() are - * automatically applied. Tracking of active connections and connection history + * A Connection implementation which establishes the underlying connection to + * guacd using the configuration information provided in guacamole.properties. + * Parameter tokens provided to connect() are automatically applied if + * explicitly requested. Tracking of active connections and connection history * is not provided. */ public class SimpleConnection extends AbstractConnection { @@ -162,10 +162,11 @@ public class SimpleConnection extends AbstractConnection { /** * Returns the GuacamoleConfiguration describing how to connect to this - * connection. Unlike the GuacamoleConfiguration returned by - * {@link #getConfiguration()}, which may omit or tokenize information, - * the GuacamoleConfiguration returned by this function contains the full - * configuration to be used to establish the connection. + * connection. Unlike {@link #getConfiguration()}, which is allowed to omit + * or tokenize information, the GuacamoleConfiguration returned by this + * function will always be the full configuration to be used to establish + * the connection, as provided when this SimpleConnection was created or via + * {@link #setConfiguration(org.apache.guacamole.protocol.GuacamoleConfiguration)}. * * @return * The full GuacamoleConfiguration describing how to connect to this @@ -244,6 +245,19 @@ public class SimpleConnection extends AbstractConnection { } + /** + * {@inheritDoc} + * + *

This implementation will connect using the GuacamoleConfiguration + * returned by {@link #getFullConfiguration()}, honoring the + * "guacd-hostname", "guacd-port", and "guacd-ssl" properties set within + * guacamole.properties. Parameter tokens will be taken into account if + * the SimpleConnection was explicitly requested to do so when created. + * + *

Implementations requiring more complex behavior should consider using + * the {@link AbstractConnection} base class or implementing + * {@link Connection} directly. + */ @Override public GuacamoleTunnel connect(GuacamoleClientInformation info, Map tokens) throws GuacamoleException { From 9d74d9911e06ebd5f92fba16936cf3a0ac57a190 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 22 Jan 2019 00:28:44 -0800 Subject: [PATCH 06/12] GUACAMOLE-524: Correct JavaDoc references to previous iteration of API rework. --- .../apache/guacamole/net/auth/simple/SimpleConnection.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java index de218b24d..bf62e8bf6 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java @@ -97,8 +97,7 @@ public class SimpleConnection extends AbstractConnection { * be set before the SimpleConnection may be used. Parameter tokens within * the GuacamoleConfiguration eventually supplied with * {@link #setConfiguration(org.apache.guacamole.protocol.GuacamoleConfiguration)} - * will not be interpreted unless explicitly requested with - * {@link #setInterpretTokens(boolean)}. + * will not be interpreted unless explicitly requested. * * @param interpretTokens * Whether parameter tokens in the underlying GuacamoleConfiguration @@ -113,7 +112,7 @@ public class SimpleConnection extends AbstractConnection { * Creates a new SimpleConnection having the given identifier and * GuacamoleConfiguration. Parameter tokens within the * GuacamoleConfiguration will not be interpreted unless explicitly - * requested with {@link #setInterpretTokens(boolean)}. + * requested. * * @param name * The name to associate with this connection. @@ -256,7 +255,7 @@ public class SimpleConnection extends AbstractConnection { * *

Implementations requiring more complex behavior should consider using * the {@link AbstractConnection} base class or implementing - * {@link Connection} directly. + * {@link org.apache.guacamole.net.auth.Connection} directly. */ @Override public GuacamoleTunnel connect(GuacamoleClientInformation info, From 05553ec918d4477299504188d1ffb43e306fa4b4 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 22 Jan 2019 12:24:25 -0800 Subject: [PATCH 07/12] GUACAMOLE-524: Internally replace Connectable with an ABI-compatible version. Defining an ABI-compatible version of Connectable at the guacamole-ext level is problematic as concrete implementations of the interface will suddenly compile despite having no implementation of connect() at all. We can instead rely on the web application to ensure binary compatibility, leaving guacamole-ext to define the interface that new code should use. --- .../guacamole/net/auth/Connectable.java | 57 ++------- .../net/auth/simple/SimpleConnection.java | 28 ++++- .../guacamole/net/auth/Connectable.java | 109 ++++++++++++++++++ 3 files changed, 144 insertions(+), 50 deletions(-) create mode 100644 guacamole/src/main/java/org/apache/guacamole/net/auth/Connectable.java diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connectable.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connectable.java index c597fe6bc..407ad7ad5 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connectable.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connectable.java @@ -19,7 +19,6 @@ package org.apache.guacamole.net.auth; -import java.util.Collections; import java.util.Map; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.net.GuacamoleTunnel; @@ -30,36 +29,14 @@ import org.apache.guacamole.protocol.GuacamoleClientInformation; */ public interface Connectable { - /** - * Establishes a connection to guacd using the information associated with - * this object. The connection will be provided the given client - * information. - * - * @deprecated - * This function has been deprecated in favor of - * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation, java.util.Map)}, - * which allows for connection parameter tokens to be injected and - * applied by cooperating extensions, replacing the functionality - * previously provided through the {@link org.apache.guacamole.token.StandardTokens} - * class. It continues to be defined on this interface for - * compatibility. New implementations should instead implement - * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation, java.util.Map)}. - * - * @param info - * Information associated with the connecting client. - * - * @return - * A fully-established GuacamoleTunnel. - * - * @throws GuacamoleException - * If an error occurs while connecting to guacd, or if permission to - * connect is denied. + /* + * IMPORTANT: + * ---------- + * The web application (guacamole) defines its own version of this + * interface containing defaults which allow backwards compatibility with + * 1.0.0. Any changes to this interface MUST be properly reflected in that + * copy of the interface such that they are binary compatible. */ - @Deprecated - default GuacamoleTunnel connect(GuacamoleClientInformation info) - throws GuacamoleException { - return this.connect(info, Collections.emptyMap()); - } /** * Establishes a connection to guacd using the information associated with @@ -68,17 +45,6 @@ public interface Connectable { * apply the given tokens when configuring the connection, such as with a * {@link org.apache.guacamole.token.TokenFilter}. * - *

A default implementation which invokes the old, deprecated - * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation)} - * is provided solely for compatibility with extensions which implement only - * the deprecated function. This default implementation is useful only - * for extensions relying on the deprecated function and will be removed - * when the deprecated function is removed. - * - *

New implementations should always implement this function - * in favor of the deprecated - * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation)}. - * * @see Parameter Tokens * * @param info @@ -97,13 +63,8 @@ public interface Connectable { * If an error occurs while connecting to guacd, or if permission to * connect is denied. */ - default GuacamoleTunnel connect(GuacamoleClientInformation info, - Map tokens) throws GuacamoleException { - - // Allow old implementations of Connectable to continue to work - return this.connect(info); - - } + public GuacamoleTunnel connect(GuacamoleClientInformation info, + Map tokens) throws GuacamoleException; /** * Returns the number of active connections associated with this object. diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java index bf62e8bf6..26cc7dc03 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java @@ -196,8 +196,32 @@ public class SimpleConnection extends AbstractConnection { // Do nothing - there are no attributes } - @Override - @SuppressWarnings("deprecation") + /** + * Establishes a connection to guacd using the information associated with + * this object. The connection will be provided the given client + * information. + * + *

This definition is the legacy connect() definition from 1.0.0 and + * older. It is redefined here for the sake of ABI compatibility with + * 1.0.0 but is no longer defined within the + * {@link org.apache.guacamole.net.auth.Connectable} interface. + * + * @deprecated + * This definition exists solely for binary compatibility. It should + * never be used by new code. New implementations should instead use + * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation, java.util.Map)}. + * + * @param info + * Information associated with the connecting client. + * + * @return + * A fully-established GuacamoleTunnel. + * + * @throws GuacamoleException + * If an error occurs while connecting to guacd, or if permission to + * connect is denied. + */ + @Deprecated public GuacamoleTunnel connect(GuacamoleClientInformation info) throws GuacamoleException { diff --git a/guacamole/src/main/java/org/apache/guacamole/net/auth/Connectable.java b/guacamole/src/main/java/org/apache/guacamole/net/auth/Connectable.java new file mode 100644 index 000000000..849d49a13 --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/net/auth/Connectable.java @@ -0,0 +1,109 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.net.auth; + +import java.util.Collections; +import java.util.Map; +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.net.GuacamoleTunnel; +import org.apache.guacamole.protocol.GuacamoleClientInformation; + +/** + * Internal, ABI-compatible version of the Connectable interface from + * guacamole-ext which defines fallback defaults for older versions of the API. + * As this interface will take precedence in the servlet container's + * classloader over the definition from guacamole-ext, this allows backwards + * compatibility with the 1.0.0 API while keeping the actual API definition + * within guacamole-ext strict. + * + *

For this to work, this interface definition MUST be 100% + * ABI-compatible with the Connectable interface defined by guacamole-ext in + * 1.0.0 and onward. + */ +public interface Connectable { + + /** + * Establishes a connection to guacd using the information associated with + * this object. The connection will be provided the given client + * information. + * + *

This definition is the legacy connect() definition from 1.0.0 and + * older. It is redefined here for the sake of ABI compatibility with + * 1.0.0 but is no longer defined within guacamole-ext. + * + * @deprecated + * This definition exists solely for binary compatibility. It should + * never be used by new code. New implementations should instead use + * the current version of connect() as defined by guacamole-ext. + * + * @param info + * Information associated with the connecting client. + * + * @return + * A fully-established GuacamoleTunnel. + * + * @throws GuacamoleException + * If an error occurs while connecting to guacd, or if permission to + * connect is denied. + */ + @Deprecated + default GuacamoleTunnel connect(GuacamoleClientInformation info) + throws GuacamoleException { + + // Pass through usages of the old API to the new API + return this.connect(info, Collections.emptyMap()); + + } + + /** + * {@inheritDoc} + * + *

This definition is the current version of connect() as defined by + * guacamole-ext. + * + *

A default implementation which invokes the old, deprecated + * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation)} + * is provided solely for compatibility with extensions which implement only + * the old version of this function. This default implementation is useful + * only for extensions relying on the older API and will be removed when + * support for that version of the API is removed. + * + * @see + * The definition of getActiveConnections() in the current version of + * the Connectable interface, as defined by guacamole-ext. + */ + default GuacamoleTunnel connect(GuacamoleClientInformation info, + Map tokens) throws GuacamoleException { + + // Allow old implementations of Connectable to continue to work + return this.connect(info); + + } + + /** + * {@inheritDoc} + * + * @see + * The definition of getActiveConnections() in the current version of + * the Connectable interface, as defined by guacamole-ext. + */ + int getActiveConnections(); + +} From 2580068aa4306306d161d3e69964f591889d323e Mon Sep 17 00:00:00 2001 From: Joel Best Date: Tue, 22 Jan 2019 16:57:09 -0500 Subject: [PATCH 08/12] GUACAMOLE-704: Add ldap-follow-referrals setting for Docker containers --- guacamole-docker/bin/start.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/guacamole-docker/bin/start.sh b/guacamole-docker/bin/start.sh index 33dc35a14..23078329f 100755 --- a/guacamole-docker/bin/start.sh +++ b/guacamole-docker/bin/start.sh @@ -321,6 +321,10 @@ END "ldap-user-search-filter" \ "$LDAP_USER_SEARCH_FILTER" + set_optional_property \ + "ldap-follow-referrals" \ + "$LDAP_FOLLOW_REFERRALS" + # Add required .jar files to GUACAMOLE_EXT ln -s /opt/guacamole/ldap/guacamole-auth-*.jar "$GUACAMOLE_EXT" From a28b3c393a7ee701c52680ff47a5f823cbbd05f7 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 22 Jan 2019 14:25:27 -0800 Subject: [PATCH 09/12] GUACAMOLE-524: Do not interpret tokens in SimpleUserContext if interpretTokens is false. --- .../org/apache/guacamole/net/auth/simple/SimpleUserContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleUserContext.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleUserContext.java index a1dce55af..2cefee2e7 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleUserContext.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleUserContext.java @@ -129,7 +129,7 @@ public class SimpleUserContext extends AbstractUserContext { GuacamoleConfiguration config = configEntry.getValue(); // Add as simple connection - Connection connection = new SimpleConnection(identifier, identifier, config, true); + Connection connection = new SimpleConnection(identifier, identifier, config, interpretTokens); connection.setParentIdentifier(DEFAULT_ROOT_CONNECTION_GROUP); connections.put(identifier, connection); From feecb6301f6ec4c1c560b54a752b5090b1393d1c Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 22 Jan 2019 15:46:43 -0800 Subject: [PATCH 10/12] GUACAMOLE-524: Declare deprecation of old connect() within guacamole-ext. --- .../guacamole/net/auth/Connectable.java | 32 +++++++++++++++++++ .../net/auth/simple/SimpleConnection.java | 26 +-------------- .../guacamole/net/auth/Connectable.java | 2 +- 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connectable.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connectable.java index 407ad7ad5..2f3326fdc 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connectable.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connectable.java @@ -19,6 +19,7 @@ package org.apache.guacamole.net.auth; +import java.util.Collections; import java.util.Map; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.net.GuacamoleTunnel; @@ -38,6 +39,37 @@ public interface Connectable { * copy of the interface such that they are binary compatible. */ + /** + * Establishes a connection to guacd using the information associated with + * this object. The connection will be provided the given client + * information. + * + * @deprecated + * This function has been deprecated in favor of + * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation, java.util.Map)}, + * which allows for connection parameter tokens to be injected and + * applied by cooperating extensions, replacing the functionality + * previously provided through the {@link org.apache.guacamole.token.StandardTokens} + * class. It continues to be defined on this interface for + * compatibility. New implementations should instead implement + * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation, java.util.Map)}. + * + * @param info + * Information associated with the connecting client. + * + * @return + * A fully-established GuacamoleTunnel. + * + * @throws GuacamoleException + * If an error occurs while connecting to guacd, or if permission to + * connect is denied. + */ + @Deprecated + default GuacamoleTunnel connect(GuacamoleClientInformation info) + throws GuacamoleException { + return this.connect(info, Collections.emptyMap()); + } + /** * Establishes a connection to guacd using the information associated with * this object. The connection will be provided the given client diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java index 26cc7dc03..2934cbe9f 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java @@ -196,31 +196,7 @@ public class SimpleConnection extends AbstractConnection { // Do nothing - there are no attributes } - /** - * Establishes a connection to guacd using the information associated with - * this object. The connection will be provided the given client - * information. - * - *

This definition is the legacy connect() definition from 1.0.0 and - * older. It is redefined here for the sake of ABI compatibility with - * 1.0.0 but is no longer defined within the - * {@link org.apache.guacamole.net.auth.Connectable} interface. - * - * @deprecated - * This definition exists solely for binary compatibility. It should - * never be used by new code. New implementations should instead use - * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation, java.util.Map)}. - * - * @param info - * Information associated with the connecting client. - * - * @return - * A fully-established GuacamoleTunnel. - * - * @throws GuacamoleException - * If an error occurs while connecting to guacd, or if permission to - * connect is denied. - */ + @Override @Deprecated public GuacamoleTunnel connect(GuacamoleClientInformation info) throws GuacamoleException { diff --git a/guacamole/src/main/java/org/apache/guacamole/net/auth/Connectable.java b/guacamole/src/main/java/org/apache/guacamole/net/auth/Connectable.java index 849d49a13..e09baa1d8 100644 --- a/guacamole/src/main/java/org/apache/guacamole/net/auth/Connectable.java +++ b/guacamole/src/main/java/org/apache/guacamole/net/auth/Connectable.java @@ -46,7 +46,7 @@ public interface Connectable { * *

This definition is the legacy connect() definition from 1.0.0 and * older. It is redefined here for the sake of ABI compatibility with - * 1.0.0 but is no longer defined within guacamole-ext. + * 1.0.0 but is deprecated within guacamole-ext. * * @deprecated * This definition exists solely for binary compatibility. It should From bcbac1fb57b4ed9fe8750cdc44895f1ac9d7e8ad Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 22 Jan 2019 15:49:16 -0800 Subject: [PATCH 11/12] GUACAMOLE-524: Ensure all guacamole-ext classes implementing connect() use the old connect() as their basis. Overriding the old connect() will not have the expected effect otherwise. --- .../net/auth/DelegatingConnection.java | 38 +++++++++++++++++- .../net/auth/DelegatingConnectionGroup.java | 39 ++++++++++++++++++- .../auth/simple/SimpleConnectionGroup.java | 13 +++++-- 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingConnection.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingConnection.java index b80e8683a..95b6e9326 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingConnection.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingConnection.java @@ -19,6 +19,7 @@ package org.apache.guacamole.net.auth; +import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Map; @@ -39,6 +40,24 @@ public class DelegatingConnection implements Connection { */ private final Connection connection; + /** + * The tokens which should apply strictly to the next call to + * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation)}. + * This storage is intended as a temporary bridge allowing the old version + * of connect() to be overridden while still resulting in the same behavior + * as older versions of DelegatingConnection. This storage should be + * removed once support for the old, deprecated connect() is removed. + */ + private final ThreadLocal> currentTokens = + new ThreadLocal>() { + + @Override + protected Map initialValue() { + return Collections.emptyMap(); + } + + }; + /** * Wraps the given Connection such that all function calls against this * DelegatingConnection will be delegated to it. @@ -127,10 +146,27 @@ public class DelegatingConnection implements Connection { return connection.getSharingProfileIdentifiers(); } + @Override + @Deprecated + public GuacamoleTunnel connect(GuacamoleClientInformation info) + throws GuacamoleException { + return connection.connect(info, currentTokens.get()); + } + @Override public GuacamoleTunnel connect(GuacamoleClientInformation info, Map tokens) throws GuacamoleException { - return connection.connect(info, tokens); + + // Make received tokens available within the legacy connect() strictly + // in context of the current connect() call + try { + currentTokens.set(tokens); + return connect(info); + } + finally { + currentTokens.remove(); + } + } @Override diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingConnectionGroup.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingConnectionGroup.java index 1d958bdb2..5af6eb13e 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingConnectionGroup.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingConnectionGroup.java @@ -19,6 +19,7 @@ package org.apache.guacamole.net.auth; +import java.util.Collections; import java.util.Map; import java.util.Set; import org.apache.guacamole.GuacamoleException; @@ -36,6 +37,25 @@ public class DelegatingConnectionGroup implements ConnectionGroup { */ private final ConnectionGroup connectionGroup; + /** + * The tokens which should apply strictly to the next call to + * {@link #connect(org.apache.guacamole.protocol.GuacamoleClientInformation)}. + * This storage is intended as a temporary bridge allowing the old version + * of connect() to be overridden while still resulting in the same behavior + * as older versions of DelegatingConnectionGroup. This storage + * should be removed once support for the old, deprecated connect() is + * removed. + */ + private final ThreadLocal> currentTokens = + new ThreadLocal>() { + + @Override + protected Map initialValue() { + return Collections.emptyMap(); + } + + }; + /** * Wraps the given ConnectionGroup such that all function calls against this * DelegatingConnectionGroup will be delegated to it. @@ -118,10 +138,27 @@ public class DelegatingConnectionGroup implements ConnectionGroup { connectionGroup.setAttributes(attributes); } + @Override + @Deprecated + public GuacamoleTunnel connect(GuacamoleClientInformation info) + throws GuacamoleException { + return connectionGroup.connect(info, currentTokens.get()); + } + @Override public GuacamoleTunnel connect(GuacamoleClientInformation info, Map tokens) throws GuacamoleException { - return connectionGroup.connect(info, tokens); + + // Make received tokens available within the legacy connect() strictly + // in context of the current connect() call + try { + currentTokens.set(tokens); + return connect(info); + } + finally { + currentTokens.remove(); + } + } @Override diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnectionGroup.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnectionGroup.java index a077eb312..b2f7de0ca 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnectionGroup.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnectionGroup.java @@ -47,7 +47,7 @@ public class SimpleConnectionGroup extends AbstractConnectionGroup { * The identifiers of all connection groups in this group. */ private final Set connectionGroupIdentifiers; - + /** * Creates a new SimpleConnectionGroup having the given name and identifier * which will expose the given contents. @@ -109,9 +109,16 @@ public class SimpleConnectionGroup extends AbstractConnectionGroup { } @Override - public GuacamoleTunnel connect(GuacamoleClientInformation info, - Map tokens) throws GuacamoleException { + @Deprecated + public GuacamoleTunnel connect(GuacamoleClientInformation info) + throws GuacamoleException { throw new GuacamoleSecurityException("Permission denied."); } + @Override + public GuacamoleTunnel connect(GuacamoleClientInformation info, + Map tokens) throws GuacamoleException { + return connect(info); + } + } From 1df127be108f3e8a230bbdd8820a1bbb2b82d3a1 Mon Sep 17 00:00:00 2001 From: Joel Best Date: Tue, 22 Jan 2019 19:16:57 -0500 Subject: [PATCH 12/12] GUACAMOLE-704: Add ldap-follow-referrals setting for Docker containers --- guacamole-docker/bin/start.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guacamole-docker/bin/start.sh b/guacamole-docker/bin/start.sh index 23078329f..08bac3a3f 100755 --- a/guacamole-docker/bin/start.sh +++ b/guacamole-docker/bin/start.sh @@ -321,7 +321,7 @@ END "ldap-user-search-filter" \ "$LDAP_USER_SEARCH_FILTER" - set_optional_property \ + set_optional_property \ "ldap-follow-referrals" \ "$LDAP_FOLLOW_REFERRALS"