From 06d321fe5dde0cfebee17ef6d21f8267a078ddff Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Tue, 27 Sep 2022 17:09:25 +0000 Subject: [PATCH] GUCAMOLE-1656: Do not expose the KSM config blob through the REST API. --- .../ksm/KsmAuthenticationProviderModule.java | 8 + .../vault/ksm/conf/KsmAttributeService.java | 215 ++++++++++++++ .../vault/ksm/secret/KsmSecretService.java | 19 +- .../vault/ksm/user/KsmConnection.java | 5 +- .../vault/ksm/user/KsmConnectionGroup.java | 49 ++-- .../vault/ksm/user/KsmDirectory.java | 93 ++++++ .../vault/ksm/user/KsmDirectoryService.java | 269 ++++-------------- .../guacamole/vault/ksm/user/KsmUser.java | 74 ++++- .../vault/ksm/user/KsmUserFactory.java | 40 +++ 9 files changed, 519 insertions(+), 253 deletions(-) create mode 100644 extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectory.java create mode 100644 extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUserFactory.java diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/KsmAuthenticationProviderModule.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/KsmAuthenticationProviderModule.java index faf22aa4e..b9d38da93 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/KsmAuthenticationProviderModule.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/KsmAuthenticationProviderModule.java @@ -24,7 +24,10 @@ import org.apache.guacamole.vault.VaultAuthenticationProviderModule; import org.apache.guacamole.vault.ksm.conf.KsmAttributeService; import org.apache.guacamole.vault.ksm.conf.KsmConfigurationService; import org.apache.guacamole.vault.ksm.secret.KsmSecretService; +import org.apache.guacamole.vault.ksm.user.KsmConnectionGroup; import org.apache.guacamole.vault.ksm.user.KsmDirectoryService; +import org.apache.guacamole.vault.ksm.user.KsmUserFactory; +import org.apache.guacamole.vault.ksm.user.KsmUser; import org.apache.guacamole.vault.conf.VaultAttributeService; import org.apache.guacamole.vault.conf.VaultConfigurationService; import org.apache.guacamole.vault.ksm.secret.KsmClient; @@ -67,6 +70,11 @@ public class KsmAuthenticationProviderModule install(new FactoryModuleBuilder() .implement(KsmClient.class, KsmClient.class) .build(KsmClientFactory.class)); + + // Bind factory for creating KsmUsers + install(new FactoryModuleBuilder() + .implement(KsmUser.class, KsmUser.class) + .build(KsmUserFactory.class)); } } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java index 24958208d..c1d3b95d5 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java @@ -19,20 +19,30 @@ package org.apache.guacamole.vault.ksm.conf; +import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.Base64; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.form.BooleanField; import org.apache.guacamole.form.Form; import org.apache.guacamole.form.TextField; +import org.apache.guacamole.language.TranslatableGuacamoleClientException; import org.apache.guacamole.vault.conf.VaultAttributeService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.inject.Inject; import com.google.inject.Singleton; +import com.keepersecurity.secretsManager.core.InMemoryStorage; +import com.keepersecurity.secretsManager.core.SecretsManager; +import com.keepersecurity.secretsManager.core.SecretsManagerOptions; /** * A service that exposes KSM-specific attributes, allowing setting KSM @@ -52,12 +62,41 @@ public class KsmAttributeService implements VaultAttributeService { @Inject private KsmConfigurationService configurationService; + /** + * A singleton ObjectMapper for converting a Map to a JSON string when + * generating a base64-encoded JSON KSM config blob. + */ + private static final ObjectMapper objectMapper = new ObjectMapper(); + + /** + * All expected fields in the KSM configuration JSON blob. + */ + private static final List EXPECTED_KSM_FIELDS = ( + Collections.unmodifiableList(Arrays.asList( + SecretsManager.KEY_HOSTNAME, + SecretsManager.KEY_CLIENT_ID, + SecretsManager.KEY_PRIVATE_KEY, + SecretsManager.KEY_CLIENT_KEY, + SecretsManager.KEY_APP_KEY, + SecretsManager.KEY_OWNER_PUBLIC_KEY, + SecretsManager.KEY_SERVER_PUBIC_KEY_ID + ))); + /** * The name of the attribute which can contain a KSM configuration blob * associated with either a connection group or user. */ public static final String KSM_CONFIGURATION_ATTRIBUTE = "ksm-config"; + /** + * The KSM configuration attribute contains sensitive information, so it + * should not be exposed through the directory. Instead, if a value is + * set on the attributes of an object, the following value will be exposed + * in its place, and correspondingly the underlying value will not be + * changed if this value is provided to an update call. + */ + public static final String KSM_ATTRIBUTE_PLACEHOLDER_VALUE = "**********"; + /** * All attributes related to configuring the KSM vault on a * per-connection-group or per-user basis. @@ -133,5 +172,181 @@ public class KsmAttributeService implements VaultAttributeService { } } + /** + * Sanitize the value of the provided KSM config attribute. If the provided + * config value is non-empty, it will be replaced with the placeholder + * value to avoid leaking sensitive information. If the value is empty, it + * will be replaced by `null`. + * + * @param ksmAttributeValue + * The KSM configuration attribute value to sanitize. + * + * @return + * The sanitized KSM configuration attribute value, stripped of any + * sensitive information. + */ + public static String sanitizeKsmAttributeValue(String ksmAttributeValue) { + + // Any non-empty values may contain sensitive information, and should + // be replaced by the safe placeholder value + if (ksmAttributeValue != null && !ksmAttributeValue.trim().isEmpty()) + return KSM_ATTRIBUTE_PLACEHOLDER_VALUE; + + // If the configuration value is empty, expose a null value + else + return null; + + } + + /** + * Return true if the provided input is a valid base64-encoded string, + * false otherwise. + * + * @param input + * The string to check if base-64 encoded. + * + * @return + * true if the provided input is a valid base64-encoded string, + * false otherwise. + */ + private static boolean isBase64(String input) { + + try { + Base64.getDecoder().decode(input); + return true; + } catch (IllegalArgumentException e) { + return false; + } + } + + /** + * Given a map of attribute values, check for the presence of the + * KSM_CONFIGURATION_ATTRIBUTE attribute. If it's set, check if it's a valid + * KSM one-time token. If so, attempt to translate it to a base-64-encoded + * json KSM config blob. If it's already a KSM config blob, validate it as + * config blob. If either validation fails, a GuacamoleException will be thrown. + * The processed attribute values will be returned. + * + * @param attributes + * The attributes for which the KSM configuration attribute + * parsing/validation should be performed. + * + * @throws GuacamoleException + * If the KSM_CONFIGURATION_ATTRIBUTE is set, but fails to validate as + * either a KSM one-time-token, or a KSM base64-encoded JSON config blob. + */ + public Map processAttributes( + Map attributes) throws GuacamoleException { + + // Get the value of the KSM config attribute in the provided map + String ksmConfigValue = attributes.get( + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE); + + // If the placeholder value was provided, do not update the attribute + if (KsmAttributeService.KSM_ATTRIBUTE_PLACEHOLDER_VALUE.equals(ksmConfigValue)) { + + // Remove the attribute from the map so it won't be updated + attributes = new HashMap<>(attributes); + attributes.remove(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE); + + } + + // Check if the attribute is set to a non-empty value + else if (ksmConfigValue != null && !ksmConfigValue.trim().isEmpty()) { + + // If it's already base64-encoded, it's a KSM configuration blob, + // so validate it immediately + if (isBase64(ksmConfigValue)) { + + // Attempt to validate the config as a base64-econded KSM config blob + try { + KsmConfig.parseKsmConfig(ksmConfigValue); + + // If it validates, the entity can be left alone - it's already valid + return attributes; + } + + catch (GuacamoleException exception) { + + // If the parsing attempt fails, throw a translatable error for display + // on the frontend + throw new TranslatableGuacamoleClientException( + "Invalid base64-encoded JSON KSM config provided for " + + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE + " attribute", + "CONNECTION_GROUP_ATTRIBUTES.ERROR_INVALID_KSM_CONFIG_BLOB", + exception); + } + } + + // It wasn't a valid base64-encoded string, it should be a one-time token, so + // attempt to validat it as such, and if valid, update the attribute to the + // base64 config blob generated by the token + try { + + // Create an initially empty storage to be populated using the one-time token + InMemoryStorage storage = new InMemoryStorage(); + + // Populate the in-memory storage using the one-time-token + SecretsManager.initializeStorage(storage, ksmConfigValue, null); + + // Create an options object using the values we extracted from the one-time token + SecretsManagerOptions options = new SecretsManagerOptions( + storage, null, + configurationService.getAllowUnverifiedCertificate()); + + // Attempt to fetch secrets using the options we created. This will both validate + // that the configuration works, and potentially populate missing fields that the + // initializeStorage() call did not set. + SecretsManager.getSecrets(options); + + // Create a map to store the extracted values from the KSM storage + Map configMap = new HashMap<>(); + + // Go through all the expected fields, extract from the KSM storage, + // and write to the newly created map + EXPECTED_KSM_FIELDS.forEach(configKey -> { + + // Only write the value into the new map if non-null + String value = storage.getString(configKey); + if (value != null) + configMap.put(configKey, value); + + }); + + // JSON-encode the value, and then base64 encode that to get the format + // that KSM would expect + String jsonString = objectMapper.writeValueAsString(configMap); + String base64EncodedJson = Base64.getEncoder().encodeToString( + jsonString.getBytes(StandardCharsets.UTF_8)); + + // Finally, try to parse the newly generated token as a KSM config. If this + // works, the config should be fully functional + KsmConfig.parseKsmConfig(base64EncodedJson); + + // Make a copy of the existing attributes, modifying just the value for + // KSM_CONFIGURATION_ATTRIBUTE + attributes = new HashMap<>(attributes); + attributes.put( + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, base64EncodedJson); + + } + + // The KSM SDK only throws raw Exceptions, so we can't be more specific + catch (Exception exception) { + + // If the parsing attempt fails, throw a translatable error for display + // on the frontend + throw new TranslatableGuacamoleClientException( + "Invalid one-time KSM token provided for " + + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE + " attribute", + "CONNECTION_GROUP_ATTRIBUTES.ERROR_INVALID_KSM_ONE_TIME_TOKEN", + exception); + } + } + + return attributes; + + } + } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java index 59c1450bf..515c1fc3d 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java @@ -47,12 +47,14 @@ import org.apache.guacamole.net.auth.Connectable; import org.apache.guacamole.net.auth.Connection; import org.apache.guacamole.net.auth.ConnectionGroup; import org.apache.guacamole.net.auth.Directory; +import org.apache.guacamole.net.auth.User; import org.apache.guacamole.net.auth.UserContext; import org.apache.guacamole.protocol.GuacamoleConfiguration; import org.apache.guacamole.token.TokenFilter; import org.apache.guacamole.vault.ksm.GuacamoleExceptionSupplier; import org.apache.guacamole.vault.ksm.conf.KsmAttributeService; import org.apache.guacamole.vault.ksm.conf.KsmConfigurationService; +import org.apache.guacamole.vault.ksm.user.KsmDirectory; import org.apache.guacamole.vault.secret.VaultSecretService; import org.apache.guacamole.vault.secret.WindowsUsername; import org.slf4j.Logger; @@ -299,7 +301,12 @@ public class KsmSecretService implements VaultSecretService { Set observedIdentifiers = new HashSet<>(); observedIdentifiers.add(parentIdentifier); - Directory connectionGroupDirectory = userContext.getConnectionGroupDirectory(); + // Use the unwrapped connection group directory to avoid KSM config + // value sanitization + Directory connectionGroupDirectory = ( + (KsmDirectory) userContext.getConnectionGroupDirectory() + ).getUnderlyingDirectory(); + while (true) { // Fetch the parent group, if one exists @@ -378,10 +385,16 @@ public class KsmSecretService implements VaultSecretService { // If user KSM configs are enabled globally, and for the given connectable, // return the user-specific KSM config, if one exists - if (confService.getAllowUserConfig() && isKsmUserConfigEnabled(connectable)) + if (confService.getAllowUserConfig() && isKsmUserConfigEnabled(connectable)) { - return userContext.self().getAttributes().get( + // Get the underlying user, to avoid the KSM config sanitization + User self = ( + ((KsmDirectory) userContext.getUserDirectory()) + .getUnderlyingDirectory().get(userContext.self().getIdentifier())); + + return self.getAttributes().get( KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE); + } // If user-specific KSM config is disabled globally or for the given diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java index 24d7dd062..3981486fe 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java @@ -62,10 +62,9 @@ public class KsmConnection extends DelegatingConnection { // Make a copy of the existing map Map attributes = Maps.newHashMap(super.getAttributes()); - // Add the configuration attribute - attributes.putIfAbsent(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null); + // Add the user-config-enabled configuration attribute + attributes.putIfAbsent(KsmAttributeService.KSM_USER_CONFIG_ENABLED_ATTRIBUTE, null); return attributes; - } } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnectionGroup.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnectionGroup.java index 175e4dd5f..68d445771 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnectionGroup.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnectionGroup.java @@ -30,34 +30,31 @@ import com.google.common.collect.Maps; /** * A KSM-specific connection group implementation that always exposes * the KSM_CONFIGURATION_ATTRIBUTE attribute, even when no value is set. - * This ensures that the attribute will always show up in the UI, even - * for connection groups that don't already have it set. + * The value of the attribute will be sanitized if non-empty. This ensures + * that the attribute will always show up in the UI, even for connection + * groups that don't already have it set, and that any sensitive information + * in the attribute value will not be exposed. */ public class KsmConnectionGroup extends DelegatingConnectionGroup { /** - * Create a new KsmConnectionGroup instance, wrapping the provided - * ConnectionGroup. + * Create a new KsmConnectionGroup wrapping the provided ConnectionGroup record. * * @param connectionGroup - * The ConnectionGroup instance to wrap. + * The ConnectionGroup record to wrap. */ - public KsmConnectionGroup(ConnectionGroup connectionGroup) { - - // Wrap the provided connection group + KsmConnectionGroup(ConnectionGroup connectionGroup) { super(connectionGroup); } - @Override - public Map getAttributes() { - - // Make a copy of the existing map - Map attributes = Maps.newHashMap(super.getAttributes()); - - // Add the configuration attribute - attributes.putIfAbsent(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null); - return attributes; - + /** + * Return the underlying wrapped connection group record. + * + * @return + * The wrapped connection group record. + */ + ConnectionGroup getUnderlyingConnectionGroup() { + return getDelegateConnectionGroup(); } /** @@ -70,4 +67,20 @@ import com.google.common.collect.Maps; return getDelegateConnectionGroup(); } + @Override + public Map getAttributes() { + + // Make a copy of the existing map + Map attributes = Maps.newHashMap(super.getAttributes()); + + // Sanitize the KSM configuration attribute, and ensure the attribute + // is always present + attributes.put( + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, + KsmAttributeService.sanitizeKsmAttributeValue( + attributes.get(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE))); + + return attributes; + } + } \ No newline at end of file diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectory.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectory.java new file mode 100644 index 000000000..5cfcca947 --- /dev/null +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectory.java @@ -0,0 +1,93 @@ +/* + * 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.vault.ksm.user; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.net.auth.DelegatingDirectory; +import org.apache.guacamole.net.auth.Directory; +import org.apache.guacamole.net.auth.Identifiable; + +/** + * A KSM-specific version of DecoratingDirectory that exposes the underlying + * directory for when it's needed. + */ +public abstract class KsmDirectory + extends DelegatingDirectory { + + /** + * Create a new KsmDirectory, delegating to the provided directory. + * + * @param directory + * The directory to delegate to. + */ + public KsmDirectory(Directory directory) { + super(directory); + } + + /** + * Returns the underlying directory that this DecoratingDirectory is + * delegating to. + * + * @return + * The underlying directory. + */ + public Directory getUnderlyingDirectory() { + return getDelegateDirectory(); + } + + /** + * Process and return a potentially-modified version of the object + * with the same identifier in the wrapped directory. + * + * @param object + * The object from the underlying directory. + * + * @return + * A potentially-modified version of the object with the same + * identifier in the wrapped directory. + */ + protected abstract ObjectType wrap(ObjectType object); + + @Override + public ObjectType get(String identifier) throws GuacamoleException { + + // Process and return the object from the wrapped directory + return wrap(super.get(identifier)); + + } + + @Override + public Collection getAll(Collection identifiers) + throws GuacamoleException { + + // Process and return each object from the wrapped directory + return super.getAll(identifiers).stream().map( + superObject -> wrap(superObject) + ).collect(Collectors.toList()); + + } + +} diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java index 6ce734601..5203a06dd 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java @@ -19,32 +19,17 @@ package org.apache.guacamole.vault.ksm.user; -import java.nio.charset.StandardCharsets; -import java.util.Arrays; -import java.util.Base64; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.language.TranslatableGuacamoleClientException; -import org.apache.guacamole.net.auth.Attributes; import org.apache.guacamole.net.auth.Connection; import org.apache.guacamole.net.auth.ConnectionGroup; import org.apache.guacamole.net.auth.DecoratingDirectory; +import org.apache.guacamole.net.auth.DelegatingDirectory; import org.apache.guacamole.net.auth.Directory; import org.apache.guacamole.net.auth.User; import org.apache.guacamole.vault.ksm.conf.KsmAttributeService; -import org.apache.guacamole.vault.ksm.conf.KsmConfig; -import org.apache.guacamole.vault.ksm.conf.KsmConfigurationService; import org.apache.guacamole.vault.user.VaultDirectoryService; -import com.fasterxml.jackson.databind.ObjectMapper; import com.google.inject.Inject; -import com.keepersecurity.secretsManager.core.InMemoryStorage; -import com.keepersecurity.secretsManager.core.SecretsManager; -import com.keepersecurity.secretsManager.core.SecretsManagerOptions; /** * A KSM-specific vault directory service that wraps the connection group directory @@ -54,175 +39,17 @@ import com.keepersecurity.secretsManager.core.SecretsManagerOptions; public class KsmDirectoryService extends VaultDirectoryService { /** - * Service for retrieving KSM configuration details. + * A factory for constructing new KsmUser instances. */ @Inject - private KsmConfigurationService configurationService; + private KsmUserFactory ksmUserFactory; /** - * A singleton ObjectMapper for converting a Map to a JSON string when - * generating a base64-encoded JSON KSM config blob. + * Service for retrieving any custom attributes defined for the + * current vault implementation and processing of said attributes. */ - private static final ObjectMapper objectMapper = new ObjectMapper(); - - /** - * All expected fields in the KSM configuration JSON blob. - */ - private static final List EXPECTED_KSM_FIELDS = ( - Collections.unmodifiableList(Arrays.asList( - SecretsManager.KEY_HOSTNAME, - SecretsManager.KEY_CLIENT_ID, - SecretsManager.KEY_PRIVATE_KEY, - SecretsManager.KEY_CLIENT_KEY, - SecretsManager.KEY_APP_KEY, - SecretsManager.KEY_OWNER_PUBLIC_KEY, - SecretsManager.KEY_SERVER_PUBIC_KEY_ID - ))); - - /** - * Return true if the provided input is a valid base64-encoded string, - * false otherwise. - * - * @param input - * The string to check if base-64 encoded. - * - * @return - * true if the provided input is a valid base64-encoded string, - * false otherwise. - */ - private static boolean isBase64(String input) { - - try { - Base64.getDecoder().decode(input); - return true; - } catch (IllegalArgumentException e) { - return false; - } - } - - /** - * Given an attributes-enabled entity, check for the presence of the - * KSM_CONFIGURATION_ATTRIBUTE attribute. If it's set, check if it's a valid - * KSM one-time token. If so, attempt to translate it to a base-64-encoded - * json KSM config blob, and set it back to the provided entity. - * If it's already a KSM config blob, validate it as config blob. If either - * validation fails, a GuacamoleException will be thrown. - * - * @param entity - * The attributes-enabled entity for which the KSM configuration - * attribute parsing/validation should be performed. - * - * @throws GuacamoleException - * If the KSM_CONFIGURATION_ATTRIBUTE is set, but fails to validate as - * either a KSM one-time-token, or a KSM base64-encoded JSON config blob. - */ - public void processAttributes(Attributes entity) throws GuacamoleException { - - // By default, if the KSM config attribute isn't being set, pass the - // provided attributes through without any changes - Map attributes = entity.getAttributes(); - - // Get the value of the KSM config attribute in the provided map - String ksmConfigValue = attributes.get( - KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE); - - // Check if the attribute is set to a non-empty value - if (ksmConfigValue != null && !ksmConfigValue.trim().isEmpty()) { - - // If it's already base64-encoded, it's a KSM configuration blob, - // so validate it immediately - if (isBase64(ksmConfigValue)) { - - // Attempt to validate the config as a base64-econded KSM config blob - try { - KsmConfig.parseKsmConfig(ksmConfigValue); - - // If it validates, the entity can be left alone - it's already valid - return; - } - - catch (GuacamoleException exception) { - - // If the parsing attempt fails, throw a translatable error for display - // on the frontend - throw new TranslatableGuacamoleClientException( - "Invalid base64-encoded JSON KSM config provided for " - + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE + " attribute", - "CONNECTION_GROUP_ATTRIBUTES.ERROR_INVALID_KSM_CONFIG_BLOB", - exception); - } - } - - // It wasn't a valid base64-encoded string, it should be a one-time token, so - // attempt to validat it as such, and if valid, update the attribute to the - // base64 config blob generated by the token - try { - - // Create an initially empty storage to be populated using the one-time token - InMemoryStorage storage = new InMemoryStorage(); - - // Populate the in-memory storage using the one-time-token - SecretsManager.initializeStorage(storage, ksmConfigValue, null); - - // Create an options object using the values we extracted from the one-time token - SecretsManagerOptions options = new SecretsManagerOptions( - storage, null, - configurationService.getAllowUnverifiedCertificate()); - - // Attempt to fetch secrets using the options we created. This will both validate - // that the configuration works, and potentially populate missing fields that the - // initializeStorage() call did not set. - SecretsManager.getSecrets(options); - - // Create a map to store the extracted values from the KSM storage - Map configMap = new HashMap<>(); - - // Go through all the expected fields, extract from the KSM storage, - // and write to the newly created map - EXPECTED_KSM_FIELDS.forEach(configKey -> { - - // Only write the value into the new map if non-null - String value = storage.getString(configKey); - if (value != null) - configMap.put(configKey, value); - - }); - - // JSON-encode the value, and then base64 encode that to get the format - // that KSM would expect - String jsonString = objectMapper.writeValueAsString(configMap); - String base64EncodedJson = Base64.getEncoder().encodeToString( - jsonString.getBytes(StandardCharsets.UTF_8)); - - // Finally, try to parse the newly generated token as a KSM config. If this - // works, the config should be fully functional - KsmConfig.parseKsmConfig(base64EncodedJson); - - // Make a copy of the existing attributes, modifying just the value for - // KSM_CONFIGURATION_ATTRIBUTE - attributes = new HashMap<>(attributes); - attributes.put( - KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, base64EncodedJson); - - // Set the newly updated attributes back to the original object - entity.setAttributes(attributes); - - } - - // The KSM SDK only throws raw Exceptions, so we can't be more specific - catch (Exception exception) { - - // If the parsing attempt fails, throw a translatable error for display - // on the frontend - throw new TranslatableGuacamoleClientException( - "Invalid one-time KSM token provided for " - + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE + " attribute", - "CONNECTION_GROUP_ATTRIBUTES.ERROR_INVALID_KSM_ONE_TIME_TOKEN", - exception); - } - } - - } + @Inject + private KsmAttributeService attributeService; @Override public Directory getConnectionDirectory( @@ -259,40 +86,45 @@ public class KsmDirectoryService extends VaultDirectoryService { // validate KSM configurations, and translate one-time-tokens, if possible, // as well as ensuring that all ConnectionGroups returned include the // KSM_CONFIGURATION_ATTRIBUTE attribute, so it will be available in the UI. - return new DecoratingDirectory(underlyingDirectory) { + // The value of the KSM_CONFIGURATION_ATTRIBUTE will be sanitized if set. + return new KsmDirectory(underlyingDirectory) { @Override public void add(ConnectionGroup connectionGroup) throws GuacamoleException { - // Check for the KSM config attribute and translate the one-time token - // if possible before adding - processAttributes(connectionGroup); + // Process attribute values before saving + connectionGroup.setAttributes( + attributeService.processAttributes( + connectionGroup.getAttributes())); + super.add(connectionGroup); } @Override public void update(ConnectionGroup connectionGroup) throws GuacamoleException { - // Check for the KSM config attribute and translate the one-time token - // if possible before updating - processAttributes(connectionGroup); + // Unwrap the existing ConnectionGroup + if (connectionGroup instanceof KsmConnectionGroup) + connectionGroup = ((KsmConnectionGroup) connectionGroup).getUnderlyingConnectionGroup(); + + // Process attribute values before saving + connectionGroup.setAttributes( + attributeService.processAttributes( + connectionGroup.getAttributes())); + super.update(connectionGroup); - } - - @Override - protected ConnectionGroup decorate(ConnectionGroup connectionGroup) throws GuacamoleException { - - // Wrap the existing connection group in a KsmConnection to ensure the presence of the - // KSM_CONFIGURATION_ATTRIBUTE attribute - return new KsmConnectionGroup(connectionGroup); } @Override - protected ConnectionGroup undecorate(ConnectionGroup connectionGroup) throws GuacamoleException { + protected ConnectionGroup wrap(ConnectionGroup object) { - // Return the underlying connection group that the KsmConnectionGroup wraps - return ((KsmConnectionGroup) connectionGroup).getUnderlyConnectionGroup(); + // Do not process the ConnectionGroup further if it does not exist + if (object == null) + return null; + + // Sanitize values when a ConnectionGroup is fetched from the directory + return new KsmConnectionGroup(object); } @@ -307,40 +139,47 @@ public class KsmDirectoryService extends VaultDirectoryService { // validate KSM configurations, and translate one-time-tokens, if possible // Additionally, this directory will will decorate all users with a // KsmUser wrapper to ensure that all defined KSM fields will be exposed - // in the user attributes. - return new DecoratingDirectory(underlyingDirectory) { + // in the user attributes. The value of the KSM_CONFIGURATION_ATTRIBUTE + // will be sanitized if set. + return new KsmDirectory(underlyingDirectory) { @Override public void add(User user) throws GuacamoleException { - // Check for the KSM config attribute and translate the one-time token - // if possible before adding - processAttributes(user); + // Process attribute values before saving + user.setAttributes( + attributeService.processAttributes( + user.getAttributes())); + super.add(user); } @Override public void update(User user) throws GuacamoleException { - // Check for the KSM config attribute and translate the one-time token - // if possible before updating - processAttributes(user); + // Unwrap the existing user + if (user instanceof KsmUser) + user = ((KsmUser) user).getUnderlyingUser(); + + // Process attribute values before saving + user.setAttributes( + attributeService.processAttributes( + user.getAttributes())); + super.update(user); + } @Override - protected User decorate(User user) throws GuacamoleException { + protected User wrap(User object) { - // Wrap in a KsmUser class to ensure that all defined KSM fields will be - // present - return new KsmUser(user); - } + // Do not process the user further if it does not exist + if (object == null) + return null; - @Override - protected User undecorate(User user) throws GuacamoleException { + // Sanitize values when a user is fetched from the directory + return ksmUserFactory.create(object); - // Unwrap the KsmUser - return ((KsmUser) user).getUnderlyingUser(); } }; diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java index dc2877564..9c002eb4d 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java @@ -21,27 +21,49 @@ package org.apache.guacamole.vault.ksm.user; import java.util.Map; -import org.apache.guacamole.net.auth.DelegatingUser; +import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.net.auth.User; +import org.apache.guacamole.net.auth.DelegatingUser; import org.apache.guacamole.vault.ksm.conf.KsmAttributeService; +import org.apache.guacamole.vault.ksm.conf.KsmConfigurationService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.collect.Maps; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; +import com.google.inject.assistedinject.AssistedInject; -/** - * A User that explicitly adds a blank entry for any defined - * KSM user attributes. - */ -public class KsmUser extends DelegatingUser { + /** + * A KSM-specific user implementation that exposes the + * KSM_CONFIGURATION_ATTRIBUTE attribute even if no value is set. but only + * if user-specific KSM configuration is enabled. The value of the attribute + * will be sanitized if non-empty. This ensures that the attribute will always + * show up in the UI when the feature is enabled, even for users that don't + * already have it set, and that any sensitive information in the attribute + * value will not be exposed. + */ + public class KsmUser extends DelegatingUser { /** - * Create a new Vault user wrapping the provided User record. Any - * KSM-specific attribute forms will have empty values automatically - * populated when getAttributes() is called. + * Logger for this class. + */ + private static final Logger logger = LoggerFactory.getLogger(KsmUser.class); + + /** + * Service for retrieving KSM configuration details. + */ + @Inject + private KsmConfigurationService configurationService; + + /** + * Create a new Ksmuser wrapping the provided User record. * * @param user - * The user record to wrap. + * The User record to wrap. */ - KsmUser(User user) { + @AssistedInject + KsmUser(@Assisted User user) { super(user); } @@ -61,9 +83,33 @@ public class KsmUser extends DelegatingUser { // Make a copy of the existing map Map attributes = Maps.newHashMap(super.getAttributes()); - // Add the configuration attribute - attributes.putIfAbsent(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null); + // Figure out if user-level KSM config is enabled + boolean userKsmConfigEnabled = false; + try { + userKsmConfigEnabled = configurationService.getAllowUserConfig(); + } catch (GuacamoleException e) { + + logger.warn( + "Disabling user KSM config due to exception: {}" + , e.getMessage()); + logger.debug("Error looking up if user KSM config is enabled.", e); + + } + + // If user-specific KSM configuration is not enabled, do not expose the + // attribute at all + if (!userKsmConfigEnabled) + attributes.remove(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE); + + else + // Sanitize the KSM configuration attribute, and ensure the attribute + // is always present + attributes.put( + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, + KsmAttributeService.sanitizeKsmAttributeValue( + attributes.get(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE))); + return attributes; } -} + } \ No newline at end of file diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUserFactory.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUserFactory.java new file mode 100644 index 000000000..9aafbb622 --- /dev/null +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUserFactory.java @@ -0,0 +1,40 @@ +/* + * 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.vault.ksm.user; + +import org.apache.guacamole.net.auth.User; + +/** + * Factory for creating KSM-specific users, which wrap an underlying User. + */ +public interface KsmUserFactory { + + /** + * Returns a new instance of a KsmUser, wrapping the provided underlying User. + * + * @param user + * The underlying User that should be wrapped. + * + * @return + * A new instance of a KsmUser, wrapping the provided underlying User. + */ + KsmUser create(User user); + +}