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.

extensions/guacamole-auth-quickconnect/src/main/java/org/apache/guacamole/auth/quickconnect/QuickConnectDirectory.java

@@ -107,7 +107,7 @@ public class QuickConnectDirectory extends SimpleDirectory<Connection> {
         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.

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);
 
     }
 

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 name
-     * @param identifier The identifier to associate with this connection.
+     *     The name to associate with this connection.
-     * @param config The configuration describing how to connect to 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();

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<String, GuacamoleConfiguration> 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<String, GuacamoleConfiguration> configs,
+            boolean interpretTokens) {
 
         // Produce map of connections from given configs
         Map<String, Connection> connections = new ConcurrentHashMap<String, Connection>(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);