GUACAMOLE-641: Canonicalize individual, tokenized components of secret names rather than the whole name.

In the event that a secret name is structured, such as the URL-like
notation used by Keeper Secrets Manager, canonicalizing/encoding the
entire name could result in the name itself becoming invalid. Only the
portions that come from tokens should be canonicalized.
This commit is contained in:
Michael Jumper
2022-01-21 16:34:23 -08:00
parent 16cb9ed69b
commit 786430612e
3 changed files with 31 additions and 14 deletions

View File

@@ -66,8 +66,8 @@ public class AzureKeyVaultSecretService extends CachedVaultSecretService {
* not allowed by Azure Key Vault, replacing them with a single dash. * not allowed by Azure Key Vault, replacing them with a single dash.
*/ */
@Override @Override
public String canonicalize(String name) { public String canonicalize(String nameComponent) {
Matcher disallowed = DISALLOWED_CHARACTERS.matcher(name); Matcher disallowed = DISALLOWED_CHARACTERS.matcher(nameComponent);
return disallowed.replaceAll("-"); return disallowed.replaceAll("-");
} }

View File

@@ -29,25 +29,25 @@ public interface VaultSecretService {
/** /**
* Translates an arbitrary string, which may contain characters not allowed * Translates an arbitrary string, which may contain characters not allowed
* by the vault implementation, into a string which is a valid secret name. * by the vault implementation, into a string which is valid within a
* The type of transformation performed on the string, if any, will depend * secret name. The type of transformation performed on the string, if any,
* on the specific requirements of the vault provider. * will depend on the specific requirements of the vault provider.
* *
* NOTE: It is critical that this transformation is deterministic and * NOTE: It is critical that this transformation is deterministic and
* reasonably predictable for users. If an implementation must apply a * reasonably predictable for users. If an implementation must apply a
* transformation to secret names, that transformation needs to be * transformation to secret names, that transformation needs to be
* documented. * documented.
* *
* @param name * @param nameComponent
* An arbitrary string intended for use as a secret name, but which may * An arbitrary string intended for use within a secret name, but which
* contain characters not allowed by the vault implementation. * may contain characters not allowed by the vault implementation.
* *
* @return * @return
* A name containing essentially the same content as the provided * A string containing essentially the same content as the provided
* string, but transformed deterministically such that it is acceptable * string, but transformed deterministically such that it is acceptable
* as a secret name by the vault provider. * as a component of a secret name by the vault provider.
*/ */
String canonicalize(String name); String canonicalize(String nameComponent);
/** /**
* Returns a Future which eventually completes with the value of the secret * Returns a Future which eventually completes with the value of the secret

View File

@@ -147,14 +147,31 @@ public class VaultUserContext extends TokenInjectingUserContext {
/** /**
* Creates a new TokenFilter instance with token values set for all tokens * Creates a new TokenFilter instance with token values set for all tokens
* which are not specific to connections or connection groups. Currently, * which are not specific to connections or connection groups. Currently,
* this is only the vault-specific username token ("USERNAME"). * this is only the vault-specific username token ("USERNAME"). Each token
* stored within the returned TokenFilter via setToken() will be
* automatically canonicalized for use within secret names.
* *
* @return * @return
* A new TokenFilter instance with token values set for all tokens * A new TokenFilter instance with token values set for all tokens
* which are not specific to connections or connection groups. * which are not specific to connections or connection groups.
*/ */
private TokenFilter createFilter() { private TokenFilter createFilter() {
TokenFilter filter = new TokenFilter();
// Create filter that automatically canonicalizes all token values
TokenFilter filter = new TokenFilter() {
@Override
public void setToken(String name, String value) {
super.setToken(name, secretService.canonicalize(value));
}
@Override
public void setTokens(Map<String, String> tokens) {
tokens.entrySet().forEach((entry) -> setToken(entry.getKey(), entry.getValue()));
}
};
filter.setToken(USERNAME_TOKEN, self().getIdentifier()); filter.setToken(USERNAME_TOKEN, self().getIdentifier());
return filter; return filter;
} }
@@ -196,7 +213,7 @@ public class VaultUserContext extends TokenInjectingUserContext {
// secrets which cannot be translated // secrets which cannot be translated
String secretName; String secretName;
try { try {
secretName = secretService.canonicalize(filter.filterStrict(entry.getValue())); secretName = filter.filterStrict(entry.getValue());
} }
catch (GuacamoleTokenUndefinedException e) { catch (GuacamoleTokenUndefinedException e) {
logger.debug("Secret for token \"{}\" will not be retrieved. " logger.debug("Secret for token \"{}\" will not be retrieved. "