GUACAMOLE-2137: Introduce HashiCorp Vault token handler (based on the KSM module) - #1116
GUACAMOLE-2137: Introduce HashiCorp Vault token handler (based on the KSM module)#1116TdlQ wants to merge 2 commits into
Conversation
corentin-soriano
left a comment
There was a problem hiding this comment.
I think you need to add guacamole-vault/hv in the map_extensions to resolve the CI build error:
#20 143.2 Mapped: guacamole-vault/ksm -> KSM_
..........
#20 144.3 ERROR: Unmapped extension: /opt/guacamole/extensions/guacamole-vault/hv/guacamole-vault-hv.jar
After this line:
|
Hi, can you provide an example of the hv-config property please ? |
To generate one of those configs in the shell, install "jq". export HV_CONFIG_B64=$(jq -n --arg url "https://your-vault-only..com" |
|
Shouldn't This merge with the PR #1143 as Hashicorp and OpenBao use essentially the same API |
adb014
left a comment
There was a problem hiding this comment.
I'm not a member of this projet but a user wanting openboa/hv support, so I'd like to see an openbao/hv jar file in a future version of Guacamole. You can take or leave my comments as you want :-)
| /** | ||
| * The name of the attribute which can contain a HV configuration blob | ||
| * associated with either a connection group or user. | ||
| */ | ||
| public static final String HV_CONFIGURATION_ATTRIBUTE = "hv-config"; |
There was a problem hiding this comment.
Wouldn't it be better to use roleID and secretID that use an opaque base64 encoded json blob containing the token ?
| import java.net.http.HttpClient; | ||
| import java.net.http.HttpRequest; | ||
| import java.net.http.HttpResponse; |
There was a problem hiding this comment.
Wouldn't it be better to use org.springframework.vault and delegate all of http communication and reuse and concurrency issues to spring ? See my propsed code block in #1143
|
|
||
| /** | ||
| * API version. | ||
| */ | ||
| static final String HASHICORP_VAULT_HTTP_VERSION = "/v1/"; |
There was a problem hiding this comment.
If allowing both Hashicorp and OpenBao to use the same jar file, this should allow /v2/ as well
| * The maximum amount of time that an entry will be stored in the cache | ||
| * before being refreshed, in milliseconds. | ||
| */ | ||
| private long cacheLifetime; |
There was a problem hiding this comment.
If delegated to org.springframework.vault, cache is withn the spring framework. I also has an issue with caching values within Guacamole in that highly sensitive data might end up swapped to disk and not in memory and more easily attackable. If caching is used, need to think about how to avoid cache leaks
There was a problem hiding this comment.
One idea to avoid cache leaks is to use a weak cache like
com.github.benmanes.caffeine.cache.Caffeine;
com.github.benmanes.caffeine.cache.Cache;
...
private final Cache<String, Object> cachedSecrets =
Caffeine.newBuilder().weakKeys().build();The keys and values are garbage collected under memory pressure, meaning there will be more cache misses. But better that than a cache leak.
| super(TOKEN_MAPPING_FILENAME, PROPERTIES_FILENAME); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Should probably always return false here. Hashicorp is not really designed to allow users to store their own secrets in it. Yes its technically possible to create a mount path for a user and therir secrets and a dedicated token for this mount path, it practical terms its too painful to do it really... Removing this will simplify the code enormously
| /** | ||
| * A map of base-64 encoded JSON HV config blobs to associated HV client instances. | ||
| * A distinct HV client will exist for every HV config. | ||
| */ | ||
| private final ConcurrentMap<String, HvClient> hvClientMap = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
This is only needed to allow users to have their own secrets stored in hashcorp on a dedicated mount path with a dedicated token.. Probably not needed, allowing the code to be very much simplified
There was a problem hiding this comment.
If this are no user attributes and we don't allow different vaults, mount pathes, etc on a per connection basis. As far as I can see this enitire is not needed.
BTW, I tried your module on a 1.6.0 guacmole client and the forms defined in this file didn't appear in the Admin UI. Is this expected ?
|
I’ve been looking at the today and I like your scheme that you have used. But I’d like to see it completed with other secret engines than just the key-value secret engine. It seems to me only the KV, LDAP and SSH secret engines make sense for guacamole connections, but if we want to store guacamole’s database secret in a vault we’ll also need the database engine as well. I propose including the engine to use in the prefix and removing HASHI from the prefix to make it shorter after adding the engine and agnostic wrt to openbao. So 4 prefixes VAULT_KV: for the key-value engine So for a static-role in the ldap engine the token to use would be For the ssh secret engine using certificates we’ll need to create the ssh keys in guacamole and use the secret engine to sign them. The account needs to exist on the ssh host so the username should be taken from the connection username or a key-value otherwise. However, no path or value is needed for these tokens. So I’d propose a generic token like For SSH one-time passwords they are created for a particular username and hostname, that can be recovered from the connection information itself. Only the password token makes sense in that case which might be returned with the token The database tokens are a bit like the KV tokens with seperate username and password values. So the token in that case would be There are many edge cases in implementing all of these tokens types. I’ll take a try at it based on your code and the other PR for openbao, but testing it extensively will be difficult |
|
Hi there, I'm interessed by Hashicorp Vault support. We found all the ssh keys & windows passwords in the SQL database in clear text. We want to be sure theses information could not easily read. |
That’s a Vault configuration issue and nothing to do with Guacamole. If the Vault is using a database for storage it always stores “secrets” encrypted. It might store some metadata non encrypted however. By “ssh keys” do you mean static keys stored in a key-value secret engine of the Vault or temporary signed keys using the vault ssh secret engine? If the second, look at #1214 |
…apache#1214 - It supports both OpenBao and Hashicorp Vault - Uses url like tokens of the form "vault://<mount>/<path>/<secret>" - Uses the path-help function of the vault to determine the vault type - Uses spring-vault to communicate with the vault - Adds support for both KV_1 and KV_2 Key-Value secret engines - Adds support for LDAP secret engine and static, dynamic and service accounts - Adds support of the SSH secret engine including both SSH one-time passwords and signed user certificates - Adds supports for the database secret engine, allowing Guacamole itself to obtain its username, password and if the database is configured with additional static values, the URI of the database server and the database itself - Adds the possiblity of including sub-tokens with the Vault tokens (ex: vault://kv1/users{GUAC_USERNAME}/password) - Gets the connectionGroup and User fallback functions in apache#1116 to actually work - Doesn't use a Base64 configuration string, but real Guacamole configuration options - Doesn't use a sanitize function on TextFields of the Form, but rather PasswordField types
…apache#1214 - It supports both OpenBao and Hashicorp Vault - Uses url like tokens of the form "vault://<mount>/<path>/<secret>" - Uses the path-help function of the vault to determine the vault type - Uses spring-vault to communicate with the vault - Adds support for both KV_1 and KV_2 Key-Value secret engines - Adds support for LDAP secret engine and static, dynamic and service accounts - Adds support of the SSH secret engine including both SSH one-time passwords and signed user certificates - Adds supports for the database secret engine, allowing Guacamole itself to obtain its username, password and if the database is configured with additional static values, the URI of the database server and the database itself - Adds the possiblity of including sub-tokens with the Vault tokens (ex: vault://kv1/users{GUAC_USERNAME}/password) - Gets the connectionGroup and User fallback functions in apache#1116 to actually work - Doesn't use a Base64 configuration string, but real Guacamole configuration options - Doesn't use a sanitize function on TextFields of the Form, but rather PasswordField types
This PR introduces a new module to handle HashiCorp Vault tokens. It is heavily inspired by and reuses a significant amount of code from the existing KSM module.
The main goal is to provide a dedicated, lightweight solution for fetching secrets from HashiCorp Vault for use in Guacamole connection parameters. This allows for replacing static credentials with dynamic, centrally managed secrets.
Key Features & Implementation Details
Notable Differences and Design Choices (vs KSM)