You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by "adb014 (via GitHub)" <gi...@apache.org> on 2026/05/21 11:02:40 UTC
[PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
adb014 opened a new pull request, #1214:
URL: https://github.com/apache/guacamole-client/pull/1214
This PR was first based on #1143 but its moved a fair way on since then, I have however stolen their JIRA ticket. The main differences are
- It supports both OpenBao and Hashicorp Vault, so this PR also address #1116
- It uses the same idea as #1116 to have the tokens represented by their paths in the Vault
- This extension requires read access of /sys/mounts in the Vault to obtan the list of secret engine mount paths and their types. This allows Guacamole to know how to treat the mount path in the token without additional user input.
- Uses the spring-vault modules to talk to the Vault
- It adds support for both KV_1 and KV_2 Key/Values secret engines
- It adds support of the LDAP secret engine for static, dynamic and service accounts
- It adds support of the SSH secret engine including both SSH one-time passwords and signed User certificates
- It adds supports for the databse secret engine, allowing Guacamole itself to obtain its username, password and if the database id 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)
- Prepare an addition to guacamole-manual curently included in this commit in the docs/ sub-directory of the extension
As this PR is fully documented in the[ markdown/jinja](https://github.com/adb014/guacamole-client/blob/openbao/extensions/guacamole-vault/modules/guacamole-vault-openbao/docs/openbao.md.j2) I won't do in to details of the working of this extension, but rather discuss the problems I encountered
**Simplified token names**:
The KSM extension uses token names like KEEPER_SERVER_USERNAME as only a single Keeper secret record with relatively well controls key values is used. THis extension allows basically any mount path of teh secret engine and path to the secret record within the Vault and so multiple secret records. Its therefore difficult to see how the KSM token naming method could be projected on to a Vault.
As in #1116 I chose to expose the Vault paths directly in the tokens, allowing the complete freedom
**Token paths**:
In most cases the token name to use will be the path to the Vault secret record post-pending with the secret value to extract. This has two issues
1. The Vault itself doesn't create the SSH certificates so the secret names "unsigned", "public" and "private" are imposed by Guacamole
2. The LDAP service accounts are checked out via and endpoint "/check-out". I chose to drop the "/check-out" in the token path in the extension
**Secret caching**:
When obtaining a secret record from the Vault, the secrets in the record are associated between themselves. For exemple a dynamic LDAP account has a single use username and password. The structure of the base vault extension assumes a function getValue that returns a single secret value. We therefore need to cache the values for a single record to keep the association between common values.
In the getTokens method the VaultSecretService, this is not a problem as all tokens for a single connection are resolved at once and we can set a UUID as a key in the cache to ensure common values are kept together. It also prevents session stealing by timing issues in concurrent connections. For the secrets looked up for guacamole.propertes.vlt that basically only concern secrets without any user context this is no problem. The issue is with tokens looked up in vault-token-mapping.yml. These secrets are looked up individually, but they have a user context. To avoid secret stealing and try to keep common secrets together I used a cache key combining GUAC_USERNAME, USERNAME and the path to the Vault secret record.
By the nature of Vault tokens where the password rotation at each usage, this cache should only be very short term, in the order of a few seconds. I used caffiene for the cache
**Spring-Vault** :
Due to the Tomcat 9 dependency of Guacamole I was forced to use a version of spring-vault compatible with Java-11, so I'm left using version 2.3.4 of spring-vault, when spring-vault is currently working on version 4.x. This came with many issues, the main one being that the LifecycleAwareSessionManager class of spring-vault in version 2.3.4 is pretty much broken, so I had to reimplement my own SessionManager. The use of spring-vault in this pull-request is therefore more of a promise to be able to simplify the extension when Guacamole migrates to a newer Tomcat
Spring-vault also pulled in a number of other dependencies (spring-framework, micrometre, jspecify) that you might not want to pull in.
**SSH ed25519**:
The SSH certificate signing reqires Guacamole to be able to generate temporty SSH certificates. I used apach SSHD for this. The Java 11 versions of apache SSHD don't include ed25519 directly and I had to include i2p as a crypto provider for ed25519
**LDAP Service accounts**:
The LDAP service accounts in the Vault must be checked out and checked back in again after use. This presented two problems for this extension.
1. With the extension the GuacamoleTunnel is not yet opened, and so I have no unique connection ID for the extension. I addressed this by assuming that the TunnelConnectEvent will be very rapidly after the call to getTokens in the extension, and setting the connection UUID in the TunnelConnectEvent.
2. The TunnelCloseEvent is a bit random when and if it is called. I wrote the code to check-in the service account if a TunnelCloseEvent is recieved, but as a back-up I fixed the TTL of the checked out accounts to 2 hours to ensure the Vault automatically checks in these accounts. In Guacamole the service accounts check in information is just dropped after 2 hours. If we can get more reliable TunnelCloseEvents on the termination of the sessions, the code as is should just work
**Non renewable or expired token**
Even in recent spring-vault implementations of LifecycleAwareSessionManager, an expired or non renewable token will cause the SessionManager to just stop. I'd like to use VaultAgent for complex authentification methods via a token sink file. This essentially means that a reauthentication via a call to the function CLientAuthentication.login() of spring-vault should be used if the token is expired or non-renewable. I implemented this in my SessionManager
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on code in PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#discussion_r3282329161
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/src/main/java/org/apache/guacamole/vault/openbao/secret/OpenBaoClient.java:
##########
@@ -0,0 +1,644 @@
+/*
+ * 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.openbao.secret;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.RemovalCause;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.net.event.TunnelCloseEvent;
+import org.apache.guacamole.net.event.TunnelConnectEvent;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.vault.openbao.conf.OpenBaoConfigurationService;
+import org.apache.guacamole.vault.openbao.vault.FileTokenAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthenticationOptions;
+import org.apache.guacamole.vault.openbao.vault.TtlAwareSessionManager;
+import org.springframework.http.client.SimpleClientHttpRequestFactory;
+import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
+import org.springframework.vault.authentication.ClientAuthentication;
+import org.springframework.vault.authentication.TokenAuthentication;
+import org.springframework.vault.client.VaultEndpoint;
+import org.springframework.vault.core.VaultKeyValueOperations;
+import org.springframework.vault.core.VaultTemplate;
+import org.springframework.vault.VaultException;
+import org.springframework.vault.support.VaultResponse;
+import org.springframework.web.client.RestTemplate;
+import org.springframework.web.util.DefaultUriBuilderFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+public class OpenBaoClient {
+ /**
+ * Logger for this class.
+ */
+ private static final Logger logger = LoggerFactory.getLogger(OpenBaoClient.class);
+
+ /**
+ * Service for retrieving OpenBao configuration.
+ */
+ private OpenBaoConfigurationService configService;
+
+ /**
+ * A singleton ObjectMapper for converting a Map to a JSON string when
+ * returning a complex token.
+ */
+ private static final ObjectMapper objectMapper = new ObjectMapper();
+
+ /**
+ * The prefix of the Guacamole token to resolve on the vault server.
+ */
+ static final String VAULT_TOKEN_PREFIX = "vault://";
+
+ /**
+ * Cache of secrets recently fetched
+ */
+ private Cache<String, Map<String, Object>> cache;
+
+ /**
+ * Vault template that will be used with all of the mount paths
+ */
+ private VaultTemplate vaultTemplate;
+
+ /*
+ * Ttl aware token manager for automatic token renewal
+ */
+ private TtlAwareSessionManager sessionManager;
+
+ /**
+ * Vault client authentication object
+ */
+ private ClientAuthentication authentication;
+
+ /**
+ * A HashMap of the checked out LDAP Sessions
+ */
+ private final Map<String, LDAPSessionInfo> ldapSessions = new HashMap<>();
+
+ /**
+ * A task scheduler for remove terminated checked out LDAP Sessions
+ */
+ ThreadPoolTaskScheduler scheduler;
+
+ /**
+ * Constructor allowing early injection of configuration and initialization
+ * to start the token renewal process as early as possible
+ *
+ * @param configService
+ * The injected configuration service
+ */
+ public OpenBaoClient(OpenBaoConfigurationService configService) {
+ this.configService = configService;
+
+ try {
+ VaultEndpoint endpoint = VaultEndpoint.from(configService.getVaultUri().resolve("v1"));
+
+ SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory();
+ try {
+ requestFactory.setConnectTimeout(configService.getConnectionTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint connection timeout: " + e.getMessage());
+ }
+ try {
+ requestFactory.setReadTimeout(configService.getRequestTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint request timeout: " + e.getMessage());
+ }
+
+ RestTemplate restTemplate = new RestTemplate(requestFactory);
+ restTemplate.setUriTemplateHandler(new DefaultUriBuilderFactory(
+ configService.getVaultUri().resolve("v1").toString()));
+
+ if (configService.getVaultToken() != null) {
+ if (isTokenReadableFile(configService.getVaultToken())) {
+ logger.debug("File Token : {}", configService.getVaultToken());
+ this.authentication =
+ new FileTokenAuthentication(configService.getVaultToken());
+ }
+ else {
+ logger.debug("Token : {}", configService.getVaultToken());
+ this.authentication = new TokenAuthentication(configService.getVaultToken());
+ }
+ }
+ else if (configService.getVaultUsername() != null && configService.getVaultPassword() != null) {
+ UsernamePasswordAuthenticationOptions options =
+ UsernamePasswordAuthenticationOptions.builder()
+ .username(configService.getVaultUsername())
+ .password(configService.getVaultPassword())
+ .build();
+
+ logger.debug("Username/Password : {}, {}", configService.getVaultUsername(),
+ configService.getVaultPassword());
Review Comment:
I'd thought about that, but for testing it is needed. A compromise would be to delete the logger of "secret" values everywhere, but keep the overall output in the getValue and getTokens methods of OpenBaoSecretService but commented rather than behind lgger.debug
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#issuecomment-4510103347
PS: The build failure for this PR is due to the changes in generate-license-files.sh a few months ago. Basically just have to rename the README files to LICENSE, remove the version from the directory names and reformat the LICENSE files to be the same as the other LICENSE files...
I'll rebase this PR, so it builds with the latest Guacamole
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 closed pull request #1214: GUACAMOLE-2196: Openbao/Hashicorp Vault extension
URL: https://github.com/apache/guacamole-client/pull/1214
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on code in PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#discussion_r3282433698
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/src/main/java/org/apache/guacamole/vault/openbao/secret/OpenBaoClient.java:
##########
@@ -0,0 +1,644 @@
+/*
+ * 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.openbao.secret;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.RemovalCause;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.net.event.TunnelCloseEvent;
+import org.apache.guacamole.net.event.TunnelConnectEvent;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.vault.openbao.conf.OpenBaoConfigurationService;
+import org.apache.guacamole.vault.openbao.vault.FileTokenAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthenticationOptions;
+import org.apache.guacamole.vault.openbao.vault.TtlAwareSessionManager;
+import org.springframework.http.client.SimpleClientHttpRequestFactory;
+import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
+import org.springframework.vault.authentication.ClientAuthentication;
+import org.springframework.vault.authentication.TokenAuthentication;
+import org.springframework.vault.client.VaultEndpoint;
+import org.springframework.vault.core.VaultKeyValueOperations;
+import org.springframework.vault.core.VaultTemplate;
+import org.springframework.vault.VaultException;
+import org.springframework.vault.support.VaultResponse;
+import org.springframework.web.client.RestTemplate;
+import org.springframework.web.util.DefaultUriBuilderFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+public class OpenBaoClient {
+ /**
+ * Logger for this class.
+ */
+ private static final Logger logger = LoggerFactory.getLogger(OpenBaoClient.class);
+
+ /**
+ * Service for retrieving OpenBao configuration.
+ */
+ private OpenBaoConfigurationService configService;
+
+ /**
+ * A singleton ObjectMapper for converting a Map to a JSON string when
+ * returning a complex token.
+ */
+ private static final ObjectMapper objectMapper = new ObjectMapper();
+
+ /**
+ * The prefix of the Guacamole token to resolve on the vault server.
+ */
+ static final String VAULT_TOKEN_PREFIX = "vault://";
+
+ /**
+ * Cache of secrets recently fetched
+ */
+ private Cache<String, Map<String, Object>> cache;
+
+ /**
+ * Vault template that will be used with all of the mount paths
+ */
+ private VaultTemplate vaultTemplate;
+
+ /*
+ * Ttl aware token manager for automatic token renewal
+ */
+ private TtlAwareSessionManager sessionManager;
+
+ /**
+ * Vault client authentication object
+ */
+ private ClientAuthentication authentication;
+
+ /**
+ * A HashMap of the checked out LDAP Sessions
+ */
+ private final Map<String, LDAPSessionInfo> ldapSessions = new HashMap<>();
+
+ /**
+ * A task scheduler for remove terminated checked out LDAP Sessions
+ */
+ ThreadPoolTaskScheduler scheduler;
+
+ /**
+ * Constructor allowing early injection of configuration and initialization
+ * to start the token renewal process as early as possible
+ *
+ * @param configService
+ * The injected configuration service
+ */
+ public OpenBaoClient(OpenBaoConfigurationService configService) {
+ this.configService = configService;
+
+ try {
+ VaultEndpoint endpoint = VaultEndpoint.from(configService.getVaultUri().resolve("v1"));
+
+ SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory();
+ try {
+ requestFactory.setConnectTimeout(configService.getConnectionTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint connection timeout: " + e.getMessage());
+ }
+ try {
+ requestFactory.setReadTimeout(configService.getRequestTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint request timeout: " + e.getMessage());
+ }
+
+ RestTemplate restTemplate = new RestTemplate(requestFactory);
+ restTemplate.setUriTemplateHandler(new DefaultUriBuilderFactory(
+ configService.getVaultUri().resolve("v1").toString()));
+
+ if (configService.getVaultToken() != null) {
+ if (isTokenReadableFile(configService.getVaultToken())) {
+ logger.debug("File Token : {}", configService.getVaultToken());
+ this.authentication =
+ new FileTokenAuthentication(configService.getVaultToken());
+ }
+ else {
+ logger.debug("Token : {}", configService.getVaultToken());
+ this.authentication = new TokenAuthentication(configService.getVaultToken());
+ }
+ }
+ else if (configService.getVaultUsername() != null && configService.getVaultPassword() != null) {
+ UsernamePasswordAuthenticationOptions options =
+ UsernamePasswordAuthenticationOptions.builder()
+ .username(configService.getVaultUsername())
+ .password(configService.getVaultPassword())
+ .build();
+
+ logger.debug("Username/Password : {}, {}", configService.getVaultUsername(),
+ configService.getVaultPassword());
+ this.authentication =
+ new UsernamePasswordAuthentication(options, endpoint, restTemplate);
+ }
+ else {
+ throw new GuacamoleException("Either a vault token or Username/Password must be supplied");
+ }
+
+ // Create a task scheduler for our token renewal
+ ThreadPoolTaskScheduler taskscheduler = new ThreadPoolTaskScheduler();
+ taskscheduler.setPoolSize(1);
+ taskscheduler.setThreadNamePrefix("vault-renewal-");
+ taskscheduler.initialize();
+
+ // Session manager to automatically renew tokens before expiration
+ this.sessionManager = new TtlAwareSessionManager(this.authentication,
+ restTemplate, taskscheduler, configService.getTokenRenewalDelay());
+
+ this.vaultTemplate = new VaultTemplate(endpoint, requestFactory, this.sessionManager);
+ }
+ catch (Exception e) {
+ logger.error("Error initializing Vault client: {}", e.getMessage());
+ }
+
+ // Initialize the cache with maximum size of 1MB, and cache expiry with
+ // forced cleanup
+ // FIXME I'd really like to do something like "Array.fill(v, '\0');" in
+ // the removal listener to ensure that passwords are no longer in memory.
+ // However, both spring-core-vault and Guacamole store these values
+ // elsewhere as immutable String values, So even if I stored them in the
+ // cache as char[] copies of the password would be elsewhere as String
+ // values in memory.. A VaultConverter function could deal with the
+ // spring-vault-core part of the problem, but not Gaucamole.
+ try {
+ cache = Caffeine.newBuilder()
+ .expireAfterWrite(Duration.ofMillis(configService.getVaultCacheLifetime()))
+ .maximumSize(1_000_000)
+ .build();
+ }
+ catch (GuacamoleException e) {
+ logger.error("Can't setup OpenBao cache");
+ }
+ }
+
+ /**
+ * Function to detect if the the token is in fact a readable file
+ * rather than a token string
+ *
+ * @param token
+ * The string with the token returned from configService
+ *
+ * @return
+ * True is the token is a readable file
+ */
+ private static boolean isTokenReadableFile(String token) {
+ try {
+ Path path = Paths.get(token);
+ return Files.isRegularFile(path) && Files.isReadable(path);
+ }
+ catch (Exception e) {
+ return false;
+ }
+ }
+
+ /**
+ * Lists the valid mount paths and their type
+ *
+ * @return
+ * The map of the mount path and with the value being the type.
+ * The type can be ssh, ldap, database, kv_1 or kv_2
+ */
+ public Map<String, Object> listMountPaths() {
+
+ Map<String, Object> cacheMountPaths = cache.getIfPresent("sys/mounts");
+ if (cacheMountPaths != null) {
+ return cacheMountPaths;
+ }
+
+ VaultResponse response = vaultTemplate.read("sys/mounts");
+
+ if (response == null || response.getData() == null) {
+ return Collections.emptyMap();
+ }
+
+ Map<String, Object> mounts = response.getData();
+ Map<String, Object> mountPaths = new HashMap<>();
+
+ mounts.forEach((mountPath, mountInfoObj) -> {
+ if (mountInfoObj instanceof Map<?, ?>) {
+ Map<?,?> mountInfo = (Map<?,?>) mountInfoObj;
+ Object type = mountInfo.get("type");
+ if (type instanceof String) {
+ if (type.equals("ssh") || type.equals("database") || type.equals("ldap")) {
+ mountPaths.put(mountPath, type);
+ }
+ else if (type.equals("kv")) {
+ // Need to detect if type 1 or type 2 Key/Value engine
+ if (mountInfo.get("options") instanceof Map<?, ?>) {
+ Map<?,?> options = (Map<?,?>) mountInfo.get("options");
+ Object version = options.get("version");
+ if (version instanceof String) {
+ if (version.equals("2")) {
+ mountPaths.put(mountPath, "kv_2");
+ }
+ else {
+ mountPaths.put(mountPath, "kv_1");
+ }
+ }
+ }
+ else {
+ // No options, assume kv_1
+ mountPaths.put(mountPath, "kv_1");
+ }
+ }
+ }
+ }
+ });
+
+ cache.put("sys/mounts", mountPaths);
+
+ return mountPaths;
+ }
+
+ /**
+ * Contains information about the checked out LDAP sessions
+ */
+ private static class LDAPSessionInfo {
+ final String checkInPath;
+ final String username;
+ final Boolean initialized;
+ final Instant created;
+
+ public LDAPSessionInfo(String checkInPath, String username) {
+ this.checkInPath = checkInPath;
+ this.username = username;
+ this.initialized = false;
+ this.created = Instant.now();
+ }
+
+ public LDAPSessionInfo(String checkInPath, String username, Boolean initialized) {
+ this.checkInPath = checkInPath;
+ this.username = username;
+ this.initialized = initialized;
+ this.created = Instant.now();
+ }
+ }
+
+ /**
+ * The LDAP session interface checks out session that then can not be
+ * used till they are checked in. In getTokens we have the problem that
+ * we don't have access to the tunnel ID and so can't identify it.
+ * Guacamole is also not guarenteed to generate a TunnelCloseEvent.
+ *
+ * So this function is fragile and relies on the fact that the TunnelConnectEvent
+ * will be running a few tens of milliseconds after the getTokens command to
+ * limit the risk of confusing two connection. There is still a small risk
+ * of error here.
+ *
+ * @param event
+ * A TunnelConnectEvent or TunnelCloseEvent
+ */
+ public void connectLdapSession(Object event) {
+ if (event instanceof TunnelConnectEvent) {
+ LDAPSessionInfo session = ldapSessions.get("checkin");
+ if (session != null) {
+ String id = ((TunnelConnectEvent) event).getTunnel().getUUID().toString();
+ logger.debug("Storing connection ID: {}", id);
+ ldapSessions.put(id, new LDAPSessionInfo(session.checkInPath, session.username, true));
+ ldapSessions.remove("checkin");
+ }
+ }
+ else {
+ String id = ((TunnelCloseEvent) event).getTunnel().getUUID().toString();
+ LDAPSessionInfo session = ldapSessions.get(id);
+ if (session != null && session.initialized) {
+ // FIXME : We don't always receive the TunnelCloseEvent
+ // So this checkin function only kinda works
+ logger.debug("Removing stored LDAP session: {}", id);
+ vaultTemplate.write(session.checkInPath, Map.of("service_account_names", session.username));
+ ldapSessions.remove(id);
+ }
+ }
+
+ // Do some clean up of the active LDAP sessions. If a session hasn't been
+ // checked in after 2 hours, just drop it from the hashMap. As the TTL of
+ // the vault is already 2 hours don't need to check it in. Don't really
+ // care if the value hang around in our hashmap so don't need a dedicated
+ // task for this
+ ldapSessions.entrySet().removeIf(e -> Instant.now().isAfter(e.getValue().created.plusSeconds(7200)));
+ }
+
+ /**
+ * Retrieves a value from a vault by its path. It first parses the
+ * leading mount path from the token, ensures it is valid and uses
+ * a supported secret engine. It then passes off the rest of the
+ * processing to a method dedicated to each secret engine.
+ *
+ * @param token
+ * The Guacamole token to look up in OpenBao.
+ *
+ * @param username
+ * The connection username, that must be non null for SSH certificate
+ * generation.
+ *
+ * @param key
+ * A pseudo-unique key to use to stored cached secrets, to keep secrets
+ * associated with the same connection together, even if they vault token
+ * itself is not unique.
+ *
+ * @return
+ * The value associated with the token.
+ *
+ * @throws GuacamoleException
+ * If the secret cannot be retrieved from the Vault.
+ */
+ public String getValue(String token, String username, String key) throws GuacamoleException {
+ try {
+ if (! token.startsWith(VAULT_TOKEN_PREFIX)) {
+ throw new GuacamoleException("Invalid token Vault token: " + token);
+ }
+
+ // Detect and validate the mount path
+ Map<String, Object> mountpaths = listMountPaths();
+ String mountPath = null;
+ String type = null;
+ for (String _path : mountpaths.keySet()) {
+ if (token.startsWith(VAULT_TOKEN_PREFIX + _path) &&
+ (mountPath == null || _path.length() > mountPath.length())) {
+ mountPath = _path;
+ type = (String) mountpaths.get(_path);
+ }
+ }
+ if (mountPath == null || mountPath.isEmpty()) {
+ throw new GuacamoleException("The Vault mount path of the token is invalid: " + token);
+ }
+
+ // Find last slash to isolate the secret value in the record
+ int lastSlashIndex = token.lastIndexOf('/');
+ if (lastSlashIndex == -1)
+ lastSlashIndex = VAULT_TOKEN_PREFIX.length();
+
+ String path = token.substring(VAULT_TOKEN_PREFIX.length() + mountPath.length(), lastSlashIndex);
+ String secret = token.substring(lastSlashIndex + 1);
+
+ logger.debug("MountPath {}, path {}, secret {}, type {}", mountPath, path, secret, type);
+
+ Map<String, Object> cacheResponse = (Map<String, Object>) cache.getIfPresent(key);
+
+ Object raw;
+ if (cacheResponse != null) {
+ raw = cacheResponse.get(secret);
+ }
+ else {
+ Map<String, Object> response;
+
+ switch (type) {
+ case "ssh":
+ response = getValueSSH(mountPath, path, username);
+ break;
+ case "ldap":
+ response = getValueLDAP(mountPath, path);
+ break;
+ case "database":
+ response = getValueDB(mountPath, path);
+ break;
+ case "kv_1":
+ response = getValueKV(mountPath, path, VaultKeyValueOperations.KeyValueBackend.KV_1);
+ break;
+ case "kv_2":
+ response = getValueKV(mountPath, path, VaultKeyValueOperations.KeyValueBackend.KV_2);
+ break;
+ default:
+ throw new GuacamoleException("Unknown secret engine for the token: " + token);
+ }
+
+ cache.put(key, response);
+ raw = response.get(secret);
+ }
+
+ if (raw == null) {
+ throw new VaultException("Secret '" + secret + "' not found on the path '" + mountPath + path + "'");
+ }
+ else if (raw instanceof String || raw instanceof Number || raw instanceof Boolean) {
+ return String.valueOf(raw);
+ }
+ else {
+ try {
+ // Stored JSON value.. Probably not usable, but return as a string
+ return objectMapper.writeValueAsString(raw);
+ }
+ catch (JsonProcessingException e) {
+ throw new GuacamoleException("Error json parsing returned secret: ", e);
+ }
+ }
+ }
+ catch (VaultException e) {
+ logger.error("Failed to retrieve secret from the vault : {}", e.getMessage());
+ throw new GuacamoleServerException("Failed to retrieve secret from Vault Server : " + e.getMessage());
+ }
+
+ }
+
+ /**
+ * Retrieves a value from a key-value secret engine of a vault.
+ *
+ * @param mountPath
+ * The mountPath of the key-value secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record
+ *
+ * @param type
+ * The type of key-value store. Either "kv_1" or "kv_2"
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueKV(String mountPath, String path, VaultKeyValueOperations.KeyValueBackend type) throws GuacamoleException {
+ VaultKeyValueOperations kvOperations = vaultTemplate.opsForKeyValue(mountPath, type);
+
+ // Get the values on the path and cache them
+ VaultResponse response = kvOperations.get(path);
+
+ if (response == null || response.getData() == null)
+ {
+ throw new GuacamoleServerException("Value not found in Vault for path: " + mountPath + path);
+ }
+
+ return response.getData();
+ }
+
+ /**
+ * Retrieves a an ssh one-time password or signed SSH certificate
+ *
+ * @param mountPath
+ * The mountPath of the SSH secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record representing the SSH role
+ *
+ * @param username
+ * The connection username that must be non null for SSH certificate
+ * generation.
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueSSH(String mountPath, String path, String username) throws GuacamoleException {
+ if (path.startsWith("creds/")) {
+ if (username == null || username.isEmpty()) {
+ throw new GuacamoleException("The username can not be empty for SSH signed certificates");
+ }
+
+ VaultResponse response =
+ vaultTemplate.write(mountPath + path, Map.of("ip", "0.0.0.0"));
+
+ if (response == null || response.getData() == null) {
+ throw new GuacamoleException("No response from Vault SSH engine");
+ }
+
+ return response.getData();
+ }
+ else if (path.startsWith("sign/")) {
+ OpenBaoSshKeys sshKeys = new OpenBaoSshKeys(configService.getSshType());
+ Map<String, Object> request = Map.of(
+ "public_key", sshKeys.publicSsh,
+ "valid_principals", username,
+ "extensions", Map.of("permit-pty", ""),
+ "ttl", configService.getSshConnectionTimeout());
+
+ VaultResponse vaultResponse =
+ vaultTemplate.write(mountPath + path, request);
+
+ if (vaultResponse == null || vaultResponse.getData() == null) {
+ throw new GuacamoleException("No response from Vault SSH engine");
+ }
+
+ String signedCert = (String) vaultResponse.getData().get("signed_key");
+
+ if (signedCert == null) {
+ throw new GuacamoleException("Vault did not return a signed SSH certificate");
+ }
+
+ return Map.of("private", sshKeys.privateSshPem,
+ "public", signedCert,
+ "unsigned", sshKeys.publicSsh);
+ }
+ else {
+ throw new GuacamoleException("Unknown SSH type on path: " + mountPath + path);
+ }
+ }
+
+ /**
+ * Retrieves a username or password from an LDAP secret engine. The type of
+ * account supported might be static, dynamic or service accounts
+ *
+ * @param mountPath
+ * The mountPath of the LDAP secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record representing the LDAP role
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueLDAP(String mountPath, String path) throws GuacamoleException {
+ VaultResponse response;
+ if (path.startsWith("static") || path.startsWith("creds/")) {
+ response = vaultTemplate.read(mountPath + path);
+ }
+ else if (path.startsWith("library/")) {
+ response = vaultTemplate.write(mountPath + path + "/check-out", Map.of("ttl", "2h"));
+ }
+ else {
+ throw new GuacamoleException("Unknown LDAP type on path: " + mountPath +"/" + path);
+ }
+
+ if (response == null || response.getData() == null) {
+ throw new GuacamoleException("No response from LDAP secrets engine");
+ }
+
+ Map<String, Object> retval = response.getData();
+ if (path.startsWith("library/")) {
+ String username = String.valueOf(retval.get("service_account_name"));
+ retval.put("username", username);
+
+ // Register a listener to check-in the account for a TunnelClose Event
+ logger.info("Caching session : {}", username);
+ LDAPSessionInfo info = new LDAPSessionInfo(mountPath + path + "/check-in", username);
+ ldapSessions.put("checkin", info);
+ }
+
+ return retval;
+ }
+
+ /**
+ * Retrieves a username or password from a Databse secret engine.
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on code in PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#discussion_r3282430259
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/src/main/java/org/apache/guacamole/vault/openbao/secret/OpenBaoClient.java:
##########
@@ -0,0 +1,644 @@
+/*
+ * 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.openbao.secret;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.RemovalCause;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.net.event.TunnelCloseEvent;
+import org.apache.guacamole.net.event.TunnelConnectEvent;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.vault.openbao.conf.OpenBaoConfigurationService;
+import org.apache.guacamole.vault.openbao.vault.FileTokenAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthenticationOptions;
+import org.apache.guacamole.vault.openbao.vault.TtlAwareSessionManager;
+import org.springframework.http.client.SimpleClientHttpRequestFactory;
+import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
+import org.springframework.vault.authentication.ClientAuthentication;
+import org.springframework.vault.authentication.TokenAuthentication;
+import org.springframework.vault.client.VaultEndpoint;
+import org.springframework.vault.core.VaultKeyValueOperations;
+import org.springframework.vault.core.VaultTemplate;
+import org.springframework.vault.VaultException;
+import org.springframework.vault.support.VaultResponse;
+import org.springframework.web.client.RestTemplate;
+import org.springframework.web.util.DefaultUriBuilderFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+public class OpenBaoClient {
+ /**
+ * Logger for this class.
+ */
+ private static final Logger logger = LoggerFactory.getLogger(OpenBaoClient.class);
+
+ /**
+ * Service for retrieving OpenBao configuration.
+ */
+ private OpenBaoConfigurationService configService;
+
+ /**
+ * A singleton ObjectMapper for converting a Map to a JSON string when
+ * returning a complex token.
+ */
+ private static final ObjectMapper objectMapper = new ObjectMapper();
+
+ /**
+ * The prefix of the Guacamole token to resolve on the vault server.
+ */
+ static final String VAULT_TOKEN_PREFIX = "vault://";
+
+ /**
+ * Cache of secrets recently fetched
+ */
+ private Cache<String, Map<String, Object>> cache;
+
+ /**
+ * Vault template that will be used with all of the mount paths
+ */
+ private VaultTemplate vaultTemplate;
+
+ /*
+ * Ttl aware token manager for automatic token renewal
+ */
+ private TtlAwareSessionManager sessionManager;
+
+ /**
+ * Vault client authentication object
+ */
+ private ClientAuthentication authentication;
+
+ /**
+ * A HashMap of the checked out LDAP Sessions
+ */
+ private final Map<String, LDAPSessionInfo> ldapSessions = new HashMap<>();
+
+ /**
+ * A task scheduler for remove terminated checked out LDAP Sessions
+ */
+ ThreadPoolTaskScheduler scheduler;
+
+ /**
+ * Constructor allowing early injection of configuration and initialization
+ * to start the token renewal process as early as possible
+ *
+ * @param configService
+ * The injected configuration service
+ */
+ public OpenBaoClient(OpenBaoConfigurationService configService) {
+ this.configService = configService;
+
+ try {
+ VaultEndpoint endpoint = VaultEndpoint.from(configService.getVaultUri().resolve("v1"));
+
+ SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory();
+ try {
+ requestFactory.setConnectTimeout(configService.getConnectionTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint connection timeout: " + e.getMessage());
+ }
+ try {
+ requestFactory.setReadTimeout(configService.getRequestTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint request timeout: " + e.getMessage());
+ }
+
+ RestTemplate restTemplate = new RestTemplate(requestFactory);
+ restTemplate.setUriTemplateHandler(new DefaultUriBuilderFactory(
+ configService.getVaultUri().resolve("v1").toString()));
+
+ if (configService.getVaultToken() != null) {
+ if (isTokenReadableFile(configService.getVaultToken())) {
+ logger.debug("File Token : {}", configService.getVaultToken());
+ this.authentication =
+ new FileTokenAuthentication(configService.getVaultToken());
+ }
+ else {
+ logger.debug("Token : {}", configService.getVaultToken());
+ this.authentication = new TokenAuthentication(configService.getVaultToken());
+ }
+ }
+ else if (configService.getVaultUsername() != null && configService.getVaultPassword() != null) {
+ UsernamePasswordAuthenticationOptions options =
+ UsernamePasswordAuthenticationOptions.builder()
+ .username(configService.getVaultUsername())
+ .password(configService.getVaultPassword())
+ .build();
+
+ logger.debug("Username/Password : {}, {}", configService.getVaultUsername(),
+ configService.getVaultPassword());
+ this.authentication =
+ new UsernamePasswordAuthentication(options, endpoint, restTemplate);
+ }
+ else {
+ throw new GuacamoleException("Either a vault token or Username/Password must be supplied");
+ }
+
+ // Create a task scheduler for our token renewal
+ ThreadPoolTaskScheduler taskscheduler = new ThreadPoolTaskScheduler();
+ taskscheduler.setPoolSize(1);
+ taskscheduler.setThreadNamePrefix("vault-renewal-");
+ taskscheduler.initialize();
+
+ // Session manager to automatically renew tokens before expiration
+ this.sessionManager = new TtlAwareSessionManager(this.authentication,
+ restTemplate, taskscheduler, configService.getTokenRenewalDelay());
+
+ this.vaultTemplate = new VaultTemplate(endpoint, requestFactory, this.sessionManager);
+ }
+ catch (Exception e) {
+ logger.error("Error initializing Vault client: {}", e.getMessage());
+ }
+
+ // Initialize the cache with maximum size of 1MB, and cache expiry with
+ // forced cleanup
+ // FIXME I'd really like to do something like "Array.fill(v, '\0');" in
+ // the removal listener to ensure that passwords are no longer in memory.
+ // However, both spring-core-vault and Guacamole store these values
+ // elsewhere as immutable String values, So even if I stored them in the
+ // cache as char[] copies of the password would be elsewhere as String
+ // values in memory.. A VaultConverter function could deal with the
+ // spring-vault-core part of the problem, but not Gaucamole.
+ try {
+ cache = Caffeine.newBuilder()
+ .expireAfterWrite(Duration.ofMillis(configService.getVaultCacheLifetime()))
+ .maximumSize(1_000_000)
+ .build();
+ }
+ catch (GuacamoleException e) {
+ logger.error("Can't setup OpenBao cache");
+ }
+ }
+
+ /**
+ * Function to detect if the the token is in fact a readable file
+ * rather than a token string
+ *
+ * @param token
+ * The string with the token returned from configService
+ *
+ * @return
+ * True is the token is a readable file
+ */
+ private static boolean isTokenReadableFile(String token) {
+ try {
+ Path path = Paths.get(token);
+ return Files.isRegularFile(path) && Files.isReadable(path);
+ }
+ catch (Exception e) {
+ return false;
+ }
+ }
+
+ /**
+ * Lists the valid mount paths and their type
+ *
+ * @return
+ * The map of the mount path and with the value being the type.
+ * The type can be ssh, ldap, database, kv_1 or kv_2
+ */
+ public Map<String, Object> listMountPaths() {
+
+ Map<String, Object> cacheMountPaths = cache.getIfPresent("sys/mounts");
+ if (cacheMountPaths != null) {
+ return cacheMountPaths;
+ }
+
+ VaultResponse response = vaultTemplate.read("sys/mounts");
+
+ if (response == null || response.getData() == null) {
+ return Collections.emptyMap();
+ }
+
+ Map<String, Object> mounts = response.getData();
+ Map<String, Object> mountPaths = new HashMap<>();
+
+ mounts.forEach((mountPath, mountInfoObj) -> {
+ if (mountInfoObj instanceof Map<?, ?>) {
+ Map<?,?> mountInfo = (Map<?,?>) mountInfoObj;
+ Object type = mountInfo.get("type");
+ if (type instanceof String) {
+ if (type.equals("ssh") || type.equals("database") || type.equals("ldap")) {
+ mountPaths.put(mountPath, type);
+ }
+ else if (type.equals("kv")) {
+ // Need to detect if type 1 or type 2 Key/Value engine
+ if (mountInfo.get("options") instanceof Map<?, ?>) {
+ Map<?,?> options = (Map<?,?>) mountInfo.get("options");
+ Object version = options.get("version");
+ if (version instanceof String) {
+ if (version.equals("2")) {
+ mountPaths.put(mountPath, "kv_2");
+ }
+ else {
+ mountPaths.put(mountPath, "kv_1");
+ }
+ }
+ }
+ else {
+ // No options, assume kv_1
+ mountPaths.put(mountPath, "kv_1");
+ }
+ }
+ }
+ }
+ });
+
+ cache.put("sys/mounts", mountPaths);
+
+ return mountPaths;
+ }
+
+ /**
+ * Contains information about the checked out LDAP sessions
+ */
+ private static class LDAPSessionInfo {
+ final String checkInPath;
+ final String username;
+ final Boolean initialized;
+ final Instant created;
+
+ public LDAPSessionInfo(String checkInPath, String username) {
+ this.checkInPath = checkInPath;
+ this.username = username;
+ this.initialized = false;
+ this.created = Instant.now();
+ }
+
+ public LDAPSessionInfo(String checkInPath, String username, Boolean initialized) {
+ this.checkInPath = checkInPath;
+ this.username = username;
+ this.initialized = initialized;
+ this.created = Instant.now();
+ }
+ }
+
+ /**
+ * The LDAP session interface checks out session that then can not be
+ * used till they are checked in. In getTokens we have the problem that
+ * we don't have access to the tunnel ID and so can't identify it.
+ * Guacamole is also not guarenteed to generate a TunnelCloseEvent.
+ *
+ * So this function is fragile and relies on the fact that the TunnelConnectEvent
+ * will be running a few tens of milliseconds after the getTokens command to
+ * limit the risk of confusing two connection. There is still a small risk
+ * of error here.
+ *
+ * @param event
+ * A TunnelConnectEvent or TunnelCloseEvent
+ */
+ public void connectLdapSession(Object event) {
+ if (event instanceof TunnelConnectEvent) {
+ LDAPSessionInfo session = ldapSessions.get("checkin");
+ if (session != null) {
+ String id = ((TunnelConnectEvent) event).getTunnel().getUUID().toString();
+ logger.debug("Storing connection ID: {}", id);
+ ldapSessions.put(id, new LDAPSessionInfo(session.checkInPath, session.username, true));
+ ldapSessions.remove("checkin");
+ }
+ }
+ else {
+ String id = ((TunnelCloseEvent) event).getTunnel().getUUID().toString();
+ LDAPSessionInfo session = ldapSessions.get(id);
+ if (session != null && session.initialized) {
+ // FIXME : We don't always receive the TunnelCloseEvent
+ // So this checkin function only kinda works
+ logger.debug("Removing stored LDAP session: {}", id);
+ vaultTemplate.write(session.checkInPath, Map.of("service_account_names", session.username));
+ ldapSessions.remove(id);
+ }
+ }
+
+ // Do some clean up of the active LDAP sessions. If a session hasn't been
+ // checked in after 2 hours, just drop it from the hashMap. As the TTL of
+ // the vault is already 2 hours don't need to check it in. Don't really
+ // care if the value hang around in our hashmap so don't need a dedicated
+ // task for this
+ ldapSessions.entrySet().removeIf(e -> Instant.now().isAfter(e.getValue().created.plusSeconds(7200)));
+ }
+
+ /**
+ * Retrieves a value from a vault by its path. It first parses the
+ * leading mount path from the token, ensures it is valid and uses
+ * a supported secret engine. It then passes off the rest of the
+ * processing to a method dedicated to each secret engine.
+ *
+ * @param token
+ * The Guacamole token to look up in OpenBao.
+ *
+ * @param username
+ * The connection username, that must be non null for SSH certificate
+ * generation.
+ *
+ * @param key
+ * A pseudo-unique key to use to stored cached secrets, to keep secrets
+ * associated with the same connection together, even if they vault token
+ * itself is not unique.
+ *
+ * @return
+ * The value associated with the token.
+ *
+ * @throws GuacamoleException
+ * If the secret cannot be retrieved from the Vault.
+ */
+ public String getValue(String token, String username, String key) throws GuacamoleException {
+ try {
+ if (! token.startsWith(VAULT_TOKEN_PREFIX)) {
+ throw new GuacamoleException("Invalid token Vault token: " + token);
+ }
+
+ // Detect and validate the mount path
+ Map<String, Object> mountpaths = listMountPaths();
+ String mountPath = null;
+ String type = null;
+ for (String _path : mountpaths.keySet()) {
+ if (token.startsWith(VAULT_TOKEN_PREFIX + _path) &&
+ (mountPath == null || _path.length() > mountPath.length())) {
+ mountPath = _path;
+ type = (String) mountpaths.get(_path);
+ }
+ }
+ if (mountPath == null || mountPath.isEmpty()) {
+ throw new GuacamoleException("The Vault mount path of the token is invalid: " + token);
+ }
+
+ // Find last slash to isolate the secret value in the record
+ int lastSlashIndex = token.lastIndexOf('/');
+ if (lastSlashIndex == -1)
+ lastSlashIndex = VAULT_TOKEN_PREFIX.length();
+
+ String path = token.substring(VAULT_TOKEN_PREFIX.length() + mountPath.length(), lastSlashIndex);
+ String secret = token.substring(lastSlashIndex + 1);
+
+ logger.debug("MountPath {}, path {}, secret {}, type {}", mountPath, path, secret, type);
+
+ Map<String, Object> cacheResponse = (Map<String, Object>) cache.getIfPresent(key);
+
+ Object raw;
+ if (cacheResponse != null) {
+ raw = cacheResponse.get(secret);
+ }
+ else {
+ Map<String, Object> response;
+
+ switch (type) {
+ case "ssh":
+ response = getValueSSH(mountPath, path, username);
+ break;
+ case "ldap":
+ response = getValueLDAP(mountPath, path);
+ break;
+ case "database":
+ response = getValueDB(mountPath, path);
+ break;
+ case "kv_1":
+ response = getValueKV(mountPath, path, VaultKeyValueOperations.KeyValueBackend.KV_1);
+ break;
+ case "kv_2":
+ response = getValueKV(mountPath, path, VaultKeyValueOperations.KeyValueBackend.KV_2);
+ break;
+ default:
+ throw new GuacamoleException("Unknown secret engine for the token: " + token);
+ }
+
+ cache.put(key, response);
+ raw = response.get(secret);
+ }
+
+ if (raw == null) {
+ throw new VaultException("Secret '" + secret + "' not found on the path '" + mountPath + path + "'");
+ }
+ else if (raw instanceof String || raw instanceof Number || raw instanceof Boolean) {
+ return String.valueOf(raw);
+ }
+ else {
+ try {
+ // Stored JSON value.. Probably not usable, but return as a string
+ return objectMapper.writeValueAsString(raw);
+ }
+ catch (JsonProcessingException e) {
+ throw new GuacamoleException("Error json parsing returned secret: ", e);
+ }
+ }
+ }
+ catch (VaultException e) {
+ logger.error("Failed to retrieve secret from the vault : {}", e.getMessage());
+ throw new GuacamoleServerException("Failed to retrieve secret from Vault Server : " + e.getMessage());
+ }
+
+ }
+
+ /**
+ * Retrieves a value from a key-value secret engine of a vault.
+ *
+ * @param mountPath
+ * The mountPath of the key-value secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record
+ *
+ * @param type
+ * The type of key-value store. Either "kv_1" or "kv_2"
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueKV(String mountPath, String path, VaultKeyValueOperations.KeyValueBackend type) throws GuacamoleException {
+ VaultKeyValueOperations kvOperations = vaultTemplate.opsForKeyValue(mountPath, type);
+
+ // Get the values on the path and cache them
+ VaultResponse response = kvOperations.get(path);
+
+ if (response == null || response.getData() == null)
+ {
+ throw new GuacamoleServerException("Value not found in Vault for path: " + mountPath + path);
+ }
+
+ return response.getData();
+ }
+
+ /**
+ * Retrieves a an ssh one-time password or signed SSH certificate
+ *
+ * @param mountPath
+ * The mountPath of the SSH secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record representing the SSH role
+ *
+ * @param username
+ * The connection username that must be non null for SSH certificate
+ * generation.
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueSSH(String mountPath, String path, String username) throws GuacamoleException {
+ if (path.startsWith("creds/")) {
+ if (username == null || username.isEmpty()) {
+ throw new GuacamoleException("The username can not be empty for SSH signed certificates");
+ }
+
+ VaultResponse response =
+ vaultTemplate.write(mountPath + path, Map.of("ip", "0.0.0.0"));
+
+ if (response == null || response.getData() == null) {
+ throw new GuacamoleException("No response from Vault SSH engine");
+ }
+
+ return response.getData();
+ }
+ else if (path.startsWith("sign/")) {
+ OpenBaoSshKeys sshKeys = new OpenBaoSshKeys(configService.getSshType());
+ Map<String, Object> request = Map.of(
+ "public_key", sshKeys.publicSsh,
+ "valid_principals", username,
+ "extensions", Map.of("permit-pty", ""),
+ "ttl", configService.getSshConnectionTimeout());
+
+ VaultResponse vaultResponse =
+ vaultTemplate.write(mountPath + path, request);
+
+ if (vaultResponse == null || vaultResponse.getData() == null) {
+ throw new GuacamoleException("No response from Vault SSH engine");
+ }
+
+ String signedCert = (String) vaultResponse.getData().get("signed_key");
+
+ if (signedCert == null) {
+ throw new GuacamoleException("Vault did not return a signed SSH certificate");
+ }
+
+ return Map.of("private", sshKeys.privateSshPem,
+ "public", signedCert,
+ "unsigned", sshKeys.publicSsh);
+ }
+ else {
+ throw new GuacamoleException("Unknown SSH type on path: " + mountPath + path);
+ }
+ }
+
+ /**
+ * Retrieves a username or password from an LDAP secret engine. The type of
+ * account supported might be static, dynamic or service accounts
+ *
+ * @param mountPath
+ * The mountPath of the LDAP secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record representing the LDAP role
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueLDAP(String mountPath, String path) throws GuacamoleException {
+ VaultResponse response;
+ if (path.startsWith("static") || path.startsWith("creds/")) {
+ response = vaultTemplate.read(mountPath + path);
+ }
+ else if (path.startsWith("library/")) {
+ response = vaultTemplate.write(mountPath + path + "/check-out", Map.of("ttl", "2h"));
+ }
+ else {
+ throw new GuacamoleException("Unknown LDAP type on path: " + mountPath +"/" + path);
+ }
+
+ if (response == null || response.getData() == null) {
+ throw new GuacamoleException("No response from LDAP secrets engine");
+ }
+
+ Map<String, Object> retval = response.getData();
+ if (path.startsWith("library/")) {
+ String username = String.valueOf(retval.get("service_account_name"));
+ retval.put("username", username);
+
+ // Register a listener to check-in the account for a TunnelClose Event
+ logger.info("Caching session : {}", username);
+ LDAPSessionInfo info = new LDAPSessionInfo(mountPath + path + "/check-in", username);
+ ldapSessions.put("checkin", info);
+ }
+
+ return retval;
+ }
+
+ /**
+ * Retrieves a username or password from a Databse secret engine.
+ * Before version 1.10 the data could only have username/password
+ * After there can be static preconfigured fields that the vault tokens
+ * might access.
+ *
+ * @param mountPath
+ * The mountPath of the database secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record representing the LDAP role
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueDB(String mountPath, String path) throws GuacamoleException {
+ VaultResponse response = vaultTemplate.read(mountPath + path);
+
+ if (response == null || response.getData() == null) {
+ throw new GuacamoleException("No response from Databse secrets engine");
Review Comment:
At least it shows I wrote the code and not Cluade ;-). Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#issuecomment-4568113949
> @corentin-soriano can you hold off on reviewing this PR? Looking again at #1116 I kind of understand why the author wrote the code as he did, and there is one clear avantage of the code in #1116 for the concurrency in the http requests. Basically I see two possibilities
>
> 1. We only allow a single vault configured at boot time as in this PR. In that case the class responsible for talking to the vault is a singleton as here. I’d still like to cherry pick the concurrency code from the vault client of [GUACAMOLE-2137: Introduce HashiCorp Vault token handler (based on the KSM module) #1116](https://github.com/apache/guacamole-client/pull/1116) in that case to add to [GUACAMOLE-2196: Openbao/Hashicorp Vault extension  #1214](https://github.com/apache/guacamole-client/pull/1214)
> 2. We want to allow per connection group vaults. I initially thought this was a bad idea, particularly on a per user basis, and as Guacamole has access to all of the configured vaults, it’s not a security issue from threats of compromission of [guacamole-client](https://issues.apache.org/jira/browse/GUACAMOLE-client) itself. However, if the global vault has very limited permissions on the vault (or not configured at all) and the permissions of the vault role used for each connection group is limited to a different set of secrets. Doing this reduces the chance of exposing secrets from one connection group to another accidentally, so there is an advantage
>
> What I’m thinking of doing, starting from #1116 is
>
> 1. remove the user vault configuration as I really can’t see a use case and in case of a typo in a connection token by an administrator, it could be used to connect to a host outside the allowed connection group. For example imagine the connection group had the token vault://kv1/sevrer/user/hostname, where server is incorrectly spelt and won’t be found in the global vault. The user could then inject their own value for this.
> 2. Replace the base64 encoded json configuration with real individual configuration parameters
> 3. Replace the token naming scheme with the one I used here as closer to a standard URL format
> 4. Use my cache keying scheme to limit risk of secret stealing and allow paired tokens (think public/private ssh certificates that go together) to be kept together
> 5. Take my OpenBaoClient class and SessionManager, and a few other ancillary classes and use them to replace the only the http client in vault client of [GUACAMOLE-2137: Introduce HashiCorp Vault token handler (based on the KSM module)Â #1116](https://github.com/apache/guacamole-client/pull/1116) keeping all the concurrency code.
> 6. Rebase against latest main to ensure build still works
> 7. Do two new PRs for [guacamole-client](https://issues.apache.org/jira/browse/GUACAMOLE-client) and [guacamole-manual](https://issues.apache.org/jira/browse/GUACAMOLE-manual)
>
> This should end up with a PR functionally equivalent to #1214 but allowing per connection group vaults, with a minimum of work. At that point up to you and the other maintainers to decide if per connection group vaults are a good idea or not.
@corentin-soriano Frankly after working with #1116, I'm going to close this pull request and create another pull request that combines #1116, #1147 and #1214. To be continued in my next pull request
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on code in PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#discussion_r3282422613
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/src/main/java/org/apache/guacamole/vault/openbao/secret/OpenBaoClient.java:
##########
@@ -0,0 +1,644 @@
+/*
+ * 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.openbao.secret;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.RemovalCause;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.net.event.TunnelCloseEvent;
+import org.apache.guacamole.net.event.TunnelConnectEvent;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.vault.openbao.conf.OpenBaoConfigurationService;
+import org.apache.guacamole.vault.openbao.vault.FileTokenAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthenticationOptions;
+import org.apache.guacamole.vault.openbao.vault.TtlAwareSessionManager;
+import org.springframework.http.client.SimpleClientHttpRequestFactory;
+import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
+import org.springframework.vault.authentication.ClientAuthentication;
+import org.springframework.vault.authentication.TokenAuthentication;
+import org.springframework.vault.client.VaultEndpoint;
+import org.springframework.vault.core.VaultKeyValueOperations;
+import org.springframework.vault.core.VaultTemplate;
+import org.springframework.vault.VaultException;
+import org.springframework.vault.support.VaultResponse;
+import org.springframework.web.client.RestTemplate;
+import org.springframework.web.util.DefaultUriBuilderFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+public class OpenBaoClient {
+ /**
+ * Logger for this class.
+ */
+ private static final Logger logger = LoggerFactory.getLogger(OpenBaoClient.class);
+
+ /**
+ * Service for retrieving OpenBao configuration.
+ */
+ private OpenBaoConfigurationService configService;
+
+ /**
+ * A singleton ObjectMapper for converting a Map to a JSON string when
+ * returning a complex token.
+ */
+ private static final ObjectMapper objectMapper = new ObjectMapper();
+
+ /**
+ * The prefix of the Guacamole token to resolve on the vault server.
+ */
+ static final String VAULT_TOKEN_PREFIX = "vault://";
+
+ /**
+ * Cache of secrets recently fetched
+ */
+ private Cache<String, Map<String, Object>> cache;
+
+ /**
+ * Vault template that will be used with all of the mount paths
+ */
+ private VaultTemplate vaultTemplate;
+
+ /*
+ * Ttl aware token manager for automatic token renewal
+ */
+ private TtlAwareSessionManager sessionManager;
+
+ /**
+ * Vault client authentication object
+ */
+ private ClientAuthentication authentication;
+
+ /**
+ * A HashMap of the checked out LDAP Sessions
+ */
+ private final Map<String, LDAPSessionInfo> ldapSessions = new HashMap<>();
+
+ /**
+ * A task scheduler for remove terminated checked out LDAP Sessions
+ */
+ ThreadPoolTaskScheduler scheduler;
+
+ /**
+ * Constructor allowing early injection of configuration and initialization
+ * to start the token renewal process as early as possible
+ *
+ * @param configService
+ * The injected configuration service
+ */
+ public OpenBaoClient(OpenBaoConfigurationService configService) {
+ this.configService = configService;
+
+ try {
+ VaultEndpoint endpoint = VaultEndpoint.from(configService.getVaultUri().resolve("v1"));
+
+ SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory();
+ try {
+ requestFactory.setConnectTimeout(configService.getConnectionTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint connection timeout: " + e.getMessage());
+ }
+ try {
+ requestFactory.setReadTimeout(configService.getRequestTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint request timeout: " + e.getMessage());
+ }
+
+ RestTemplate restTemplate = new RestTemplate(requestFactory);
+ restTemplate.setUriTemplateHandler(new DefaultUriBuilderFactory(
+ configService.getVaultUri().resolve("v1").toString()));
+
+ if (configService.getVaultToken() != null) {
+ if (isTokenReadableFile(configService.getVaultToken())) {
+ logger.debug("File Token : {}", configService.getVaultToken());
+ this.authentication =
+ new FileTokenAuthentication(configService.getVaultToken());
+ }
+ else {
+ logger.debug("Token : {}", configService.getVaultToken());
+ this.authentication = new TokenAuthentication(configService.getVaultToken());
+ }
+ }
+ else if (configService.getVaultUsername() != null && configService.getVaultPassword() != null) {
+ UsernamePasswordAuthenticationOptions options =
+ UsernamePasswordAuthenticationOptions.builder()
+ .username(configService.getVaultUsername())
+ .password(configService.getVaultPassword())
+ .build();
+
+ logger.debug("Username/Password : {}, {}", configService.getVaultUsername(),
+ configService.getVaultPassword());
Review Comment:
Ok, I've removed or commentted all places where secret values are logged even at debug level
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on code in PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#discussion_r3288169454
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/docs/openbao.md.j2:
##########
Review Comment:
Ok, I deleted the documentation in the PR and created a seperate PR ( [#292](/apache/guacamole-manual/pull/292)) for guacamole-manual
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on code in PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#discussion_r3282297204
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/docs/openbao.md.j2:
##########
Review Comment:
> Wouldn't this documentation be better suited to the [guacamole-manual](https://github.com/apache/guacamole-manual) project?
Yes, but I included here just till the PR is finalized, included or rejected. The documentation is already in a format ready for guacamole-manual (or at least I hope so)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#issuecomment-4525362338
@corentin-soriano can you hold off on reviewing this PR? Looking again at #1116 I kind of understand why the author wrote the code as he did, and there is one clear avantage of the code in #1116 for the concurrency in the http requests. Basically I see two possibilities
1. We only allow a single vault configured at boot time as in this PR. In that case the class responsible for talking to the vault is a singleton as here. I’d still like to cherry pick the concurrency code from the vault client of #1116 in that case to add to #1214
2. We want to allow per connection group vaults. I initially thought this was a bad idea, particularly on a per user basis, and as Guacamole has access to all of the configured vaults, it’s not a security issue from threats of compromission of guacamole-client itself. However, if the global vault has very limited permissions on the vault (or not configured at all) and the permissions of the vault role used for each connection group is limited to a different set of secrets. Doing this reduces the chance of exposing secrets from one connection group to another accidentally, so there is an advantage
What I’m thinking of doing, starting from #1116 is
1. remove the user vault configuration as I really can’t see a use case and in case of a typo in a connection token by an administrator, it could be used to connect to a host outside the allowed connection group. For example imagine the connection group had the token vault://kv1/sevrer/user/hostname, where server is incorrectly spelt and won’t be found in the global vault. The user could then inject their own value for this.
2. Replace the base64 encoded json configuration with real individual configuration parameters
3. Replace the token naming scheme with the one I used here as closer to a standard URL format
4. Use my cache keying scheme to limit risk of secret stealing and allow paired tokens (think public/private ssh certificates that go together) to be kept together
5. Take my OpenBaoClient class and SessionManager, and a few other ancillary classes and use them to replace the only the http client in vault client of #1116 keeping all the concurrency code.
6. Rebase against latest main to ensure build still works
7. Do two new PRs for guacamole-client and guacamole-manual
This should end up with a PR functionally equivalent to #1214 but allowing per connection group vaults, with a minimum of work. At that point up to you and the other maintainers to decide if per connection group vaults are a good idea or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "corentin-soriano (via GitHub)" <gi...@apache.org>.
corentin-soriano commented on code in PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#discussion_r3281277409
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/docs/include/openbao.properties.in:
##########
@@ -0,0 +1,6 @@
+# The URI of the OpenBao or Hashicorp Vault server to use.
+vault-uri: http://localhost:8200
+
+# The authentication token to use to access the vault
+vault-token: s.IPzVb3b5dThhjIrBX245szisjTQcylwSjMEeyJqcidaH8Hf
Review Comment:
Is this a token that was committed here by mistake?
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/src/main/java/org/apache/guacamole/vault/openbao/secret/OpenBaoClient.java:
##########
@@ -0,0 +1,644 @@
+/*
+ * 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.openbao.secret;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.RemovalCause;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.net.event.TunnelCloseEvent;
+import org.apache.guacamole.net.event.TunnelConnectEvent;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.vault.openbao.conf.OpenBaoConfigurationService;
+import org.apache.guacamole.vault.openbao.vault.FileTokenAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthenticationOptions;
+import org.apache.guacamole.vault.openbao.vault.TtlAwareSessionManager;
+import org.springframework.http.client.SimpleClientHttpRequestFactory;
+import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
+import org.springframework.vault.authentication.ClientAuthentication;
+import org.springframework.vault.authentication.TokenAuthentication;
+import org.springframework.vault.client.VaultEndpoint;
+import org.springframework.vault.core.VaultKeyValueOperations;
+import org.springframework.vault.core.VaultTemplate;
+import org.springframework.vault.VaultException;
+import org.springframework.vault.support.VaultResponse;
+import org.springframework.web.client.RestTemplate;
+import org.springframework.web.util.DefaultUriBuilderFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+public class OpenBaoClient {
+ /**
+ * Logger for this class.
+ */
+ private static final Logger logger = LoggerFactory.getLogger(OpenBaoClient.class);
+
+ /**
+ * Service for retrieving OpenBao configuration.
+ */
+ private OpenBaoConfigurationService configService;
+
+ /**
+ * A singleton ObjectMapper for converting a Map to a JSON string when
+ * returning a complex token.
+ */
+ private static final ObjectMapper objectMapper = new ObjectMapper();
+
+ /**
+ * The prefix of the Guacamole token to resolve on the vault server.
+ */
+ static final String VAULT_TOKEN_PREFIX = "vault://";
+
+ /**
+ * Cache of secrets recently fetched
+ */
+ private Cache<String, Map<String, Object>> cache;
+
+ /**
+ * Vault template that will be used with all of the mount paths
+ */
+ private VaultTemplate vaultTemplate;
+
+ /*
+ * Ttl aware token manager for automatic token renewal
+ */
+ private TtlAwareSessionManager sessionManager;
+
+ /**
+ * Vault client authentication object
+ */
+ private ClientAuthentication authentication;
+
+ /**
+ * A HashMap of the checked out LDAP Sessions
+ */
+ private final Map<String, LDAPSessionInfo> ldapSessions = new HashMap<>();
+
+ /**
+ * A task scheduler for remove terminated checked out LDAP Sessions
+ */
+ ThreadPoolTaskScheduler scheduler;
+
+ /**
+ * Constructor allowing early injection of configuration and initialization
+ * to start the token renewal process as early as possible
+ *
+ * @param configService
+ * The injected configuration service
+ */
+ public OpenBaoClient(OpenBaoConfigurationService configService) {
+ this.configService = configService;
+
+ try {
+ VaultEndpoint endpoint = VaultEndpoint.from(configService.getVaultUri().resolve("v1"));
+
+ SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory();
+ try {
+ requestFactory.setConnectTimeout(configService.getConnectionTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint connection timeout: " + e.getMessage());
+ }
+ try {
+ requestFactory.setReadTimeout(configService.getRequestTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint request timeout: " + e.getMessage());
+ }
+
+ RestTemplate restTemplate = new RestTemplate(requestFactory);
+ restTemplate.setUriTemplateHandler(new DefaultUriBuilderFactory(
+ configService.getVaultUri().resolve("v1").toString()));
+
+ if (configService.getVaultToken() != null) {
+ if (isTokenReadableFile(configService.getVaultToken())) {
+ logger.debug("File Token : {}", configService.getVaultToken());
+ this.authentication =
+ new FileTokenAuthentication(configService.getVaultToken());
+ }
+ else {
+ logger.debug("Token : {}", configService.getVaultToken());
+ this.authentication = new TokenAuthentication(configService.getVaultToken());
+ }
+ }
+ else if (configService.getVaultUsername() != null && configService.getVaultPassword() != null) {
+ UsernamePasswordAuthenticationOptions options =
+ UsernamePasswordAuthenticationOptions.builder()
+ .username(configService.getVaultUsername())
+ .password(configService.getVaultPassword())
+ .build();
+
+ logger.debug("Username/Password : {}, {}", configService.getVaultUsername(),
+ configService.getVaultPassword());
+ this.authentication =
+ new UsernamePasswordAuthentication(options, endpoint, restTemplate);
+ }
+ else {
+ throw new GuacamoleException("Either a vault token or Username/Password must be supplied");
+ }
+
+ // Create a task scheduler for our token renewal
+ ThreadPoolTaskScheduler taskscheduler = new ThreadPoolTaskScheduler();
+ taskscheduler.setPoolSize(1);
+ taskscheduler.setThreadNamePrefix("vault-renewal-");
+ taskscheduler.initialize();
+
+ // Session manager to automatically renew tokens before expiration
+ this.sessionManager = new TtlAwareSessionManager(this.authentication,
+ restTemplate, taskscheduler, configService.getTokenRenewalDelay());
+
+ this.vaultTemplate = new VaultTemplate(endpoint, requestFactory, this.sessionManager);
+ }
+ catch (Exception e) {
+ logger.error("Error initializing Vault client: {}", e.getMessage());
+ }
+
+ // Initialize the cache with maximum size of 1MB, and cache expiry with
+ // forced cleanup
+ // FIXME I'd really like to do something like "Array.fill(v, '\0');" in
+ // the removal listener to ensure that passwords are no longer in memory.
+ // However, both spring-core-vault and Guacamole store these values
+ // elsewhere as immutable String values, So even if I stored them in the
+ // cache as char[] copies of the password would be elsewhere as String
+ // values in memory.. A VaultConverter function could deal with the
+ // spring-vault-core part of the problem, but not Gaucamole.
+ try {
+ cache = Caffeine.newBuilder()
+ .expireAfterWrite(Duration.ofMillis(configService.getVaultCacheLifetime()))
+ .maximumSize(1_000_000)
+ .build();
+ }
+ catch (GuacamoleException e) {
+ logger.error("Can't setup OpenBao cache");
+ }
+ }
+
+ /**
+ * Function to detect if the the token is in fact a readable file
+ * rather than a token string
+ *
+ * @param token
+ * The string with the token returned from configService
+ *
+ * @return
+ * True is the token is a readable file
+ */
+ private static boolean isTokenReadableFile(String token) {
+ try {
+ Path path = Paths.get(token);
+ return Files.isRegularFile(path) && Files.isReadable(path);
+ }
+ catch (Exception e) {
+ return false;
+ }
+ }
+
+ /**
+ * Lists the valid mount paths and their type
+ *
+ * @return
+ * The map of the mount path and with the value being the type.
+ * The type can be ssh, ldap, database, kv_1 or kv_2
+ */
+ public Map<String, Object> listMountPaths() {
+
+ Map<String, Object> cacheMountPaths = cache.getIfPresent("sys/mounts");
+ if (cacheMountPaths != null) {
+ return cacheMountPaths;
+ }
+
+ VaultResponse response = vaultTemplate.read("sys/mounts");
+
+ if (response == null || response.getData() == null) {
+ return Collections.emptyMap();
+ }
+
+ Map<String, Object> mounts = response.getData();
+ Map<String, Object> mountPaths = new HashMap<>();
+
+ mounts.forEach((mountPath, mountInfoObj) -> {
+ if (mountInfoObj instanceof Map<?, ?>) {
+ Map<?,?> mountInfo = (Map<?,?>) mountInfoObj;
+ Object type = mountInfo.get("type");
+ if (type instanceof String) {
+ if (type.equals("ssh") || type.equals("database") || type.equals("ldap")) {
+ mountPaths.put(mountPath, type);
+ }
+ else if (type.equals("kv")) {
+ // Need to detect if type 1 or type 2 Key/Value engine
+ if (mountInfo.get("options") instanceof Map<?, ?>) {
+ Map<?,?> options = (Map<?,?>) mountInfo.get("options");
+ Object version = options.get("version");
+ if (version instanceof String) {
+ if (version.equals("2")) {
+ mountPaths.put(mountPath, "kv_2");
+ }
+ else {
+ mountPaths.put(mountPath, "kv_1");
+ }
+ }
+ }
+ else {
+ // No options, assume kv_1
+ mountPaths.put(mountPath, "kv_1");
+ }
+ }
+ }
+ }
+ });
+
+ cache.put("sys/mounts", mountPaths);
+
+ return mountPaths;
+ }
+
+ /**
+ * Contains information about the checked out LDAP sessions
+ */
+ private static class LDAPSessionInfo {
+ final String checkInPath;
+ final String username;
+ final Boolean initialized;
+ final Instant created;
+
+ public LDAPSessionInfo(String checkInPath, String username) {
+ this.checkInPath = checkInPath;
+ this.username = username;
+ this.initialized = false;
+ this.created = Instant.now();
+ }
+
+ public LDAPSessionInfo(String checkInPath, String username, Boolean initialized) {
+ this.checkInPath = checkInPath;
+ this.username = username;
+ this.initialized = initialized;
+ this.created = Instant.now();
+ }
+ }
+
+ /**
+ * The LDAP session interface checks out session that then can not be
+ * used till they are checked in. In getTokens we have the problem that
+ * we don't have access to the tunnel ID and so can't identify it.
+ * Guacamole is also not guarenteed to generate a TunnelCloseEvent.
+ *
+ * So this function is fragile and relies on the fact that the TunnelConnectEvent
+ * will be running a few tens of milliseconds after the getTokens command to
+ * limit the risk of confusing two connection. There is still a small risk
+ * of error here.
+ *
+ * @param event
+ * A TunnelConnectEvent or TunnelCloseEvent
+ */
+ public void connectLdapSession(Object event) {
+ if (event instanceof TunnelConnectEvent) {
+ LDAPSessionInfo session = ldapSessions.get("checkin");
+ if (session != null) {
+ String id = ((TunnelConnectEvent) event).getTunnel().getUUID().toString();
+ logger.debug("Storing connection ID: {}", id);
+ ldapSessions.put(id, new LDAPSessionInfo(session.checkInPath, session.username, true));
+ ldapSessions.remove("checkin");
+ }
+ }
+ else {
+ String id = ((TunnelCloseEvent) event).getTunnel().getUUID().toString();
+ LDAPSessionInfo session = ldapSessions.get(id);
+ if (session != null && session.initialized) {
+ // FIXME : We don't always receive the TunnelCloseEvent
+ // So this checkin function only kinda works
+ logger.debug("Removing stored LDAP session: {}", id);
+ vaultTemplate.write(session.checkInPath, Map.of("service_account_names", session.username));
+ ldapSessions.remove(id);
+ }
+ }
+
+ // Do some clean up of the active LDAP sessions. If a session hasn't been
+ // checked in after 2 hours, just drop it from the hashMap. As the TTL of
+ // the vault is already 2 hours don't need to check it in. Don't really
+ // care if the value hang around in our hashmap so don't need a dedicated
+ // task for this
+ ldapSessions.entrySet().removeIf(e -> Instant.now().isAfter(e.getValue().created.plusSeconds(7200)));
+ }
+
+ /**
+ * Retrieves a value from a vault by its path. It first parses the
+ * leading mount path from the token, ensures it is valid and uses
+ * a supported secret engine. It then passes off the rest of the
+ * processing to a method dedicated to each secret engine.
+ *
+ * @param token
+ * The Guacamole token to look up in OpenBao.
+ *
+ * @param username
+ * The connection username, that must be non null for SSH certificate
+ * generation.
+ *
+ * @param key
+ * A pseudo-unique key to use to stored cached secrets, to keep secrets
+ * associated with the same connection together, even if they vault token
+ * itself is not unique.
+ *
+ * @return
+ * The value associated with the token.
+ *
+ * @throws GuacamoleException
+ * If the secret cannot be retrieved from the Vault.
+ */
+ public String getValue(String token, String username, String key) throws GuacamoleException {
+ try {
+ if (! token.startsWith(VAULT_TOKEN_PREFIX)) {
+ throw new GuacamoleException("Invalid token Vault token: " + token);
+ }
+
+ // Detect and validate the mount path
+ Map<String, Object> mountpaths = listMountPaths();
+ String mountPath = null;
+ String type = null;
+ for (String _path : mountpaths.keySet()) {
+ if (token.startsWith(VAULT_TOKEN_PREFIX + _path) &&
+ (mountPath == null || _path.length() > mountPath.length())) {
+ mountPath = _path;
+ type = (String) mountpaths.get(_path);
+ }
+ }
+ if (mountPath == null || mountPath.isEmpty()) {
+ throw new GuacamoleException("The Vault mount path of the token is invalid: " + token);
+ }
+
+ // Find last slash to isolate the secret value in the record
+ int lastSlashIndex = token.lastIndexOf('/');
+ if (lastSlashIndex == -1)
+ lastSlashIndex = VAULT_TOKEN_PREFIX.length();
+
+ String path = token.substring(VAULT_TOKEN_PREFIX.length() + mountPath.length(), lastSlashIndex);
+ String secret = token.substring(lastSlashIndex + 1);
+
+ logger.debug("MountPath {}, path {}, secret {}, type {}", mountPath, path, secret, type);
+
+ Map<String, Object> cacheResponse = (Map<String, Object>) cache.getIfPresent(key);
+
+ Object raw;
+ if (cacheResponse != null) {
+ raw = cacheResponse.get(secret);
+ }
+ else {
+ Map<String, Object> response;
+
+ switch (type) {
+ case "ssh":
+ response = getValueSSH(mountPath, path, username);
+ break;
+ case "ldap":
+ response = getValueLDAP(mountPath, path);
+ break;
+ case "database":
+ response = getValueDB(mountPath, path);
+ break;
+ case "kv_1":
+ response = getValueKV(mountPath, path, VaultKeyValueOperations.KeyValueBackend.KV_1);
+ break;
+ case "kv_2":
+ response = getValueKV(mountPath, path, VaultKeyValueOperations.KeyValueBackend.KV_2);
+ break;
+ default:
+ throw new GuacamoleException("Unknown secret engine for the token: " + token);
+ }
+
+ cache.put(key, response);
+ raw = response.get(secret);
+ }
+
+ if (raw == null) {
+ throw new VaultException("Secret '" + secret + "' not found on the path '" + mountPath + path + "'");
+ }
+ else if (raw instanceof String || raw instanceof Number || raw instanceof Boolean) {
+ return String.valueOf(raw);
+ }
+ else {
+ try {
+ // Stored JSON value.. Probably not usable, but return as a string
+ return objectMapper.writeValueAsString(raw);
+ }
+ catch (JsonProcessingException e) {
+ throw new GuacamoleException("Error json parsing returned secret: ", e);
+ }
+ }
+ }
+ catch (VaultException e) {
+ logger.error("Failed to retrieve secret from the vault : {}", e.getMessage());
+ throw new GuacamoleServerException("Failed to retrieve secret from Vault Server : " + e.getMessage());
+ }
+
+ }
+
+ /**
+ * Retrieves a value from a key-value secret engine of a vault.
+ *
+ * @param mountPath
+ * The mountPath of the key-value secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record
+ *
+ * @param type
+ * The type of key-value store. Either "kv_1" or "kv_2"
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueKV(String mountPath, String path, VaultKeyValueOperations.KeyValueBackend type) throws GuacamoleException {
+ VaultKeyValueOperations kvOperations = vaultTemplate.opsForKeyValue(mountPath, type);
+
+ // Get the values on the path and cache them
+ VaultResponse response = kvOperations.get(path);
+
+ if (response == null || response.getData() == null)
+ {
+ throw new GuacamoleServerException("Value not found in Vault for path: " + mountPath + path);
+ }
+
+ return response.getData();
+ }
+
+ /**
+ * Retrieves a an ssh one-time password or signed SSH certificate
+ *
+ * @param mountPath
+ * The mountPath of the SSH secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record representing the SSH role
+ *
+ * @param username
+ * The connection username that must be non null for SSH certificate
+ * generation.
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueSSH(String mountPath, String path, String username) throws GuacamoleException {
+ if (path.startsWith("creds/")) {
+ if (username == null || username.isEmpty()) {
+ throw new GuacamoleException("The username can not be empty for SSH signed certificates");
+ }
+
+ VaultResponse response =
+ vaultTemplate.write(mountPath + path, Map.of("ip", "0.0.0.0"));
+
+ if (response == null || response.getData() == null) {
+ throw new GuacamoleException("No response from Vault SSH engine");
+ }
+
+ return response.getData();
+ }
+ else if (path.startsWith("sign/")) {
+ OpenBaoSshKeys sshKeys = new OpenBaoSshKeys(configService.getSshType());
+ Map<String, Object> request = Map.of(
+ "public_key", sshKeys.publicSsh,
+ "valid_principals", username,
+ "extensions", Map.of("permit-pty", ""),
+ "ttl", configService.getSshConnectionTimeout());
+
+ VaultResponse vaultResponse =
+ vaultTemplate.write(mountPath + path, request);
+
+ if (vaultResponse == null || vaultResponse.getData() == null) {
+ throw new GuacamoleException("No response from Vault SSH engine");
+ }
+
+ String signedCert = (String) vaultResponse.getData().get("signed_key");
+
+ if (signedCert == null) {
+ throw new GuacamoleException("Vault did not return a signed SSH certificate");
+ }
+
+ return Map.of("private", sshKeys.privateSshPem,
+ "public", signedCert,
+ "unsigned", sshKeys.publicSsh);
+ }
+ else {
+ throw new GuacamoleException("Unknown SSH type on path: " + mountPath + path);
+ }
+ }
+
+ /**
+ * Retrieves a username or password from an LDAP secret engine. The type of
+ * account supported might be static, dynamic or service accounts
+ *
+ * @param mountPath
+ * The mountPath of the LDAP secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record representing the LDAP role
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueLDAP(String mountPath, String path) throws GuacamoleException {
+ VaultResponse response;
+ if (path.startsWith("static") || path.startsWith("creds/")) {
+ response = vaultTemplate.read(mountPath + path);
+ }
+ else if (path.startsWith("library/")) {
+ response = vaultTemplate.write(mountPath + path + "/check-out", Map.of("ttl", "2h"));
+ }
+ else {
+ throw new GuacamoleException("Unknown LDAP type on path: " + mountPath +"/" + path);
+ }
+
+ if (response == null || response.getData() == null) {
+ throw new GuacamoleException("No response from LDAP secrets engine");
+ }
+
+ Map<String, Object> retval = response.getData();
+ if (path.startsWith("library/")) {
+ String username = String.valueOf(retval.get("service_account_name"));
+ retval.put("username", username);
+
+ // Register a listener to check-in the account for a TunnelClose Event
+ logger.info("Caching session : {}", username);
+ LDAPSessionInfo info = new LDAPSessionInfo(mountPath + path + "/check-in", username);
+ ldapSessions.put("checkin", info);
+ }
+
+ return retval;
+ }
+
+ /**
+ * Retrieves a username or password from a Databse secret engine.
+ * Before version 1.10 the data could only have username/password
+ * After there can be static preconfigured fields that the vault tokens
+ * might access.
+ *
+ * @param mountPath
+ * The mountPath of the database secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record representing the LDAP role
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueDB(String mountPath, String path) throws GuacamoleException {
+ VaultResponse response = vaultTemplate.read(mountPath + path);
+
+ if (response == null || response.getData() == null) {
+ throw new GuacamoleException("No response from Databse secrets engine");
Review Comment:
Typo in `Database`
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/src/main/java/org/apache/guacamole/vault/openbao/secret/OpenBaoClient.java:
##########
@@ -0,0 +1,644 @@
+/*
+ * 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.openbao.secret;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.RemovalCause;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.net.event.TunnelCloseEvent;
+import org.apache.guacamole.net.event.TunnelConnectEvent;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.vault.openbao.conf.OpenBaoConfigurationService;
+import org.apache.guacamole.vault.openbao.vault.FileTokenAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthenticationOptions;
+import org.apache.guacamole.vault.openbao.vault.TtlAwareSessionManager;
+import org.springframework.http.client.SimpleClientHttpRequestFactory;
+import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
+import org.springframework.vault.authentication.ClientAuthentication;
+import org.springframework.vault.authentication.TokenAuthentication;
+import org.springframework.vault.client.VaultEndpoint;
+import org.springframework.vault.core.VaultKeyValueOperations;
+import org.springframework.vault.core.VaultTemplate;
+import org.springframework.vault.VaultException;
+import org.springframework.vault.support.VaultResponse;
+import org.springframework.web.client.RestTemplate;
+import org.springframework.web.util.DefaultUriBuilderFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+public class OpenBaoClient {
+ /**
+ * Logger for this class.
+ */
+ private static final Logger logger = LoggerFactory.getLogger(OpenBaoClient.class);
+
+ /**
+ * Service for retrieving OpenBao configuration.
+ */
+ private OpenBaoConfigurationService configService;
+
+ /**
+ * A singleton ObjectMapper for converting a Map to a JSON string when
+ * returning a complex token.
+ */
+ private static final ObjectMapper objectMapper = new ObjectMapper();
+
+ /**
+ * The prefix of the Guacamole token to resolve on the vault server.
+ */
+ static final String VAULT_TOKEN_PREFIX = "vault://";
+
+ /**
+ * Cache of secrets recently fetched
+ */
+ private Cache<String, Map<String, Object>> cache;
+
+ /**
+ * Vault template that will be used with all of the mount paths
+ */
+ private VaultTemplate vaultTemplate;
+
+ /*
+ * Ttl aware token manager for automatic token renewal
+ */
+ private TtlAwareSessionManager sessionManager;
+
+ /**
+ * Vault client authentication object
+ */
+ private ClientAuthentication authentication;
+
+ /**
+ * A HashMap of the checked out LDAP Sessions
+ */
+ private final Map<String, LDAPSessionInfo> ldapSessions = new HashMap<>();
+
+ /**
+ * A task scheduler for remove terminated checked out LDAP Sessions
+ */
+ ThreadPoolTaskScheduler scheduler;
+
+ /**
+ * Constructor allowing early injection of configuration and initialization
+ * to start the token renewal process as early as possible
+ *
+ * @param configService
+ * The injected configuration service
+ */
+ public OpenBaoClient(OpenBaoConfigurationService configService) {
+ this.configService = configService;
+
+ try {
+ VaultEndpoint endpoint = VaultEndpoint.from(configService.getVaultUri().resolve("v1"));
+
+ SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory();
+ try {
+ requestFactory.setConnectTimeout(configService.getConnectionTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint connection timeout: " + e.getMessage());
+ }
+ try {
+ requestFactory.setReadTimeout(configService.getRequestTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint request timeout: " + e.getMessage());
+ }
+
+ RestTemplate restTemplate = new RestTemplate(requestFactory);
+ restTemplate.setUriTemplateHandler(new DefaultUriBuilderFactory(
+ configService.getVaultUri().resolve("v1").toString()));
+
+ if (configService.getVaultToken() != null) {
+ if (isTokenReadableFile(configService.getVaultToken())) {
+ logger.debug("File Token : {}", configService.getVaultToken());
+ this.authentication =
+ new FileTokenAuthentication(configService.getVaultToken());
+ }
+ else {
+ logger.debug("Token : {}", configService.getVaultToken());
+ this.authentication = new TokenAuthentication(configService.getVaultToken());
+ }
+ }
+ else if (configService.getVaultUsername() != null && configService.getVaultPassword() != null) {
+ UsernamePasswordAuthenticationOptions options =
+ UsernamePasswordAuthenticationOptions.builder()
+ .username(configService.getVaultUsername())
+ .password(configService.getVaultPassword())
+ .build();
+
+ logger.debug("Username/Password : {}, {}", configService.getVaultUsername(),
+ configService.getVaultPassword());
+ this.authentication =
+ new UsernamePasswordAuthentication(options, endpoint, restTemplate);
+ }
+ else {
+ throw new GuacamoleException("Either a vault token or Username/Password must be supplied");
+ }
+
+ // Create a task scheduler for our token renewal
+ ThreadPoolTaskScheduler taskscheduler = new ThreadPoolTaskScheduler();
+ taskscheduler.setPoolSize(1);
+ taskscheduler.setThreadNamePrefix("vault-renewal-");
+ taskscheduler.initialize();
+
+ // Session manager to automatically renew tokens before expiration
+ this.sessionManager = new TtlAwareSessionManager(this.authentication,
+ restTemplate, taskscheduler, configService.getTokenRenewalDelay());
+
+ this.vaultTemplate = new VaultTemplate(endpoint, requestFactory, this.sessionManager);
+ }
+ catch (Exception e) {
+ logger.error("Error initializing Vault client: {}", e.getMessage());
+ }
+
+ // Initialize the cache with maximum size of 1MB, and cache expiry with
+ // forced cleanup
+ // FIXME I'd really like to do something like "Array.fill(v, '\0');" in
+ // the removal listener to ensure that passwords are no longer in memory.
+ // However, both spring-core-vault and Guacamole store these values
+ // elsewhere as immutable String values, So even if I stored them in the
+ // cache as char[] copies of the password would be elsewhere as String
+ // values in memory.. A VaultConverter function could deal with the
+ // spring-vault-core part of the problem, but not Gaucamole.
+ try {
+ cache = Caffeine.newBuilder()
+ .expireAfterWrite(Duration.ofMillis(configService.getVaultCacheLifetime()))
+ .maximumSize(1_000_000)
+ .build();
+ }
+ catch (GuacamoleException e) {
+ logger.error("Can't setup OpenBao cache");
+ }
+ }
+
+ /**
+ * Function to detect if the the token is in fact a readable file
+ * rather than a token string
+ *
+ * @param token
+ * The string with the token returned from configService
+ *
+ * @return
+ * True is the token is a readable file
+ */
+ private static boolean isTokenReadableFile(String token) {
+ try {
+ Path path = Paths.get(token);
+ return Files.isRegularFile(path) && Files.isReadable(path);
+ }
+ catch (Exception e) {
+ return false;
+ }
+ }
+
+ /**
+ * Lists the valid mount paths and their type
+ *
+ * @return
+ * The map of the mount path and with the value being the type.
+ * The type can be ssh, ldap, database, kv_1 or kv_2
+ */
+ public Map<String, Object> listMountPaths() {
+
+ Map<String, Object> cacheMountPaths = cache.getIfPresent("sys/mounts");
+ if (cacheMountPaths != null) {
+ return cacheMountPaths;
+ }
+
+ VaultResponse response = vaultTemplate.read("sys/mounts");
+
+ if (response == null || response.getData() == null) {
+ return Collections.emptyMap();
+ }
+
+ Map<String, Object> mounts = response.getData();
+ Map<String, Object> mountPaths = new HashMap<>();
+
+ mounts.forEach((mountPath, mountInfoObj) -> {
+ if (mountInfoObj instanceof Map<?, ?>) {
+ Map<?,?> mountInfo = (Map<?,?>) mountInfoObj;
+ Object type = mountInfo.get("type");
+ if (type instanceof String) {
+ if (type.equals("ssh") || type.equals("database") || type.equals("ldap")) {
+ mountPaths.put(mountPath, type);
+ }
+ else if (type.equals("kv")) {
+ // Need to detect if type 1 or type 2 Key/Value engine
+ if (mountInfo.get("options") instanceof Map<?, ?>) {
+ Map<?,?> options = (Map<?,?>) mountInfo.get("options");
+ Object version = options.get("version");
+ if (version instanceof String) {
+ if (version.equals("2")) {
+ mountPaths.put(mountPath, "kv_2");
+ }
+ else {
+ mountPaths.put(mountPath, "kv_1");
+ }
+ }
+ }
+ else {
+ // No options, assume kv_1
+ mountPaths.put(mountPath, "kv_1");
+ }
+ }
+ }
+ }
+ });
+
+ cache.put("sys/mounts", mountPaths);
+
+ return mountPaths;
+ }
+
+ /**
+ * Contains information about the checked out LDAP sessions
+ */
+ private static class LDAPSessionInfo {
+ final String checkInPath;
+ final String username;
+ final Boolean initialized;
+ final Instant created;
+
+ public LDAPSessionInfo(String checkInPath, String username) {
+ this.checkInPath = checkInPath;
+ this.username = username;
+ this.initialized = false;
+ this.created = Instant.now();
+ }
+
+ public LDAPSessionInfo(String checkInPath, String username, Boolean initialized) {
+ this.checkInPath = checkInPath;
+ this.username = username;
+ this.initialized = initialized;
+ this.created = Instant.now();
+ }
+ }
+
+ /**
+ * The LDAP session interface checks out session that then can not be
+ * used till they are checked in. In getTokens we have the problem that
+ * we don't have access to the tunnel ID and so can't identify it.
+ * Guacamole is also not guarenteed to generate a TunnelCloseEvent.
+ *
+ * So this function is fragile and relies on the fact that the TunnelConnectEvent
+ * will be running a few tens of milliseconds after the getTokens command to
+ * limit the risk of confusing two connection. There is still a small risk
+ * of error here.
+ *
+ * @param event
+ * A TunnelConnectEvent or TunnelCloseEvent
+ */
+ public void connectLdapSession(Object event) {
+ if (event instanceof TunnelConnectEvent) {
+ LDAPSessionInfo session = ldapSessions.get("checkin");
+ if (session != null) {
+ String id = ((TunnelConnectEvent) event).getTunnel().getUUID().toString();
+ logger.debug("Storing connection ID: {}", id);
+ ldapSessions.put(id, new LDAPSessionInfo(session.checkInPath, session.username, true));
+ ldapSessions.remove("checkin");
+ }
+ }
+ else {
+ String id = ((TunnelCloseEvent) event).getTunnel().getUUID().toString();
+ LDAPSessionInfo session = ldapSessions.get(id);
+ if (session != null && session.initialized) {
+ // FIXME : We don't always receive the TunnelCloseEvent
+ // So this checkin function only kinda works
+ logger.debug("Removing stored LDAP session: {}", id);
+ vaultTemplate.write(session.checkInPath, Map.of("service_account_names", session.username));
+ ldapSessions.remove(id);
+ }
+ }
+
+ // Do some clean up of the active LDAP sessions. If a session hasn't been
+ // checked in after 2 hours, just drop it from the hashMap. As the TTL of
+ // the vault is already 2 hours don't need to check it in. Don't really
+ // care if the value hang around in our hashmap so don't need a dedicated
+ // task for this
+ ldapSessions.entrySet().removeIf(e -> Instant.now().isAfter(e.getValue().created.plusSeconds(7200)));
+ }
+
+ /**
+ * Retrieves a value from a vault by its path. It first parses the
+ * leading mount path from the token, ensures it is valid and uses
+ * a supported secret engine. It then passes off the rest of the
+ * processing to a method dedicated to each secret engine.
+ *
+ * @param token
+ * The Guacamole token to look up in OpenBao.
+ *
+ * @param username
+ * The connection username, that must be non null for SSH certificate
+ * generation.
+ *
+ * @param key
+ * A pseudo-unique key to use to stored cached secrets, to keep secrets
+ * associated with the same connection together, even if they vault token
+ * itself is not unique.
+ *
+ * @return
+ * The value associated with the token.
+ *
+ * @throws GuacamoleException
+ * If the secret cannot be retrieved from the Vault.
+ */
+ public String getValue(String token, String username, String key) throws GuacamoleException {
+ try {
+ if (! token.startsWith(VAULT_TOKEN_PREFIX)) {
+ throw new GuacamoleException("Invalid token Vault token: " + token);
+ }
+
+ // Detect and validate the mount path
+ Map<String, Object> mountpaths = listMountPaths();
+ String mountPath = null;
+ String type = null;
+ for (String _path : mountpaths.keySet()) {
+ if (token.startsWith(VAULT_TOKEN_PREFIX + _path) &&
+ (mountPath == null || _path.length() > mountPath.length())) {
+ mountPath = _path;
+ type = (String) mountpaths.get(_path);
+ }
+ }
+ if (mountPath == null || mountPath.isEmpty()) {
+ throw new GuacamoleException("The Vault mount path of the token is invalid: " + token);
+ }
+
+ // Find last slash to isolate the secret value in the record
+ int lastSlashIndex = token.lastIndexOf('/');
+ if (lastSlashIndex == -1)
+ lastSlashIndex = VAULT_TOKEN_PREFIX.length();
+
+ String path = token.substring(VAULT_TOKEN_PREFIX.length() + mountPath.length(), lastSlashIndex);
+ String secret = token.substring(lastSlashIndex + 1);
+
+ logger.debug("MountPath {}, path {}, secret {}, type {}", mountPath, path, secret, type);
+
+ Map<String, Object> cacheResponse = (Map<String, Object>) cache.getIfPresent(key);
+
+ Object raw;
+ if (cacheResponse != null) {
+ raw = cacheResponse.get(secret);
+ }
+ else {
+ Map<String, Object> response;
+
+ switch (type) {
+ case "ssh":
+ response = getValueSSH(mountPath, path, username);
+ break;
+ case "ldap":
+ response = getValueLDAP(mountPath, path);
+ break;
+ case "database":
+ response = getValueDB(mountPath, path);
+ break;
+ case "kv_1":
+ response = getValueKV(mountPath, path, VaultKeyValueOperations.KeyValueBackend.KV_1);
+ break;
+ case "kv_2":
+ response = getValueKV(mountPath, path, VaultKeyValueOperations.KeyValueBackend.KV_2);
+ break;
+ default:
+ throw new GuacamoleException("Unknown secret engine for the token: " + token);
+ }
+
+ cache.put(key, response);
+ raw = response.get(secret);
+ }
+
+ if (raw == null) {
+ throw new VaultException("Secret '" + secret + "' not found on the path '" + mountPath + path + "'");
+ }
+ else if (raw instanceof String || raw instanceof Number || raw instanceof Boolean) {
+ return String.valueOf(raw);
+ }
+ else {
+ try {
+ // Stored JSON value.. Probably not usable, but return as a string
+ return objectMapper.writeValueAsString(raw);
+ }
+ catch (JsonProcessingException e) {
+ throw new GuacamoleException("Error json parsing returned secret: ", e);
+ }
+ }
+ }
+ catch (VaultException e) {
+ logger.error("Failed to retrieve secret from the vault : {}", e.getMessage());
+ throw new GuacamoleServerException("Failed to retrieve secret from Vault Server : " + e.getMessage());
+ }
+
+ }
+
+ /**
+ * Retrieves a value from a key-value secret engine of a vault.
+ *
+ * @param mountPath
+ * The mountPath of the key-value secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record
+ *
+ * @param type
+ * The type of key-value store. Either "kv_1" or "kv_2"
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueKV(String mountPath, String path, VaultKeyValueOperations.KeyValueBackend type) throws GuacamoleException {
+ VaultKeyValueOperations kvOperations = vaultTemplate.opsForKeyValue(mountPath, type);
+
+ // Get the values on the path and cache them
+ VaultResponse response = kvOperations.get(path);
+
+ if (response == null || response.getData() == null)
+ {
+ throw new GuacamoleServerException("Value not found in Vault for path: " + mountPath + path);
+ }
+
+ return response.getData();
+ }
+
+ /**
+ * Retrieves a an ssh one-time password or signed SSH certificate
+ *
+ * @param mountPath
+ * The mountPath of the SSH secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record representing the SSH role
+ *
+ * @param username
+ * The connection username that must be non null for SSH certificate
+ * generation.
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueSSH(String mountPath, String path, String username) throws GuacamoleException {
+ if (path.startsWith("creds/")) {
+ if (username == null || username.isEmpty()) {
+ throw new GuacamoleException("The username can not be empty for SSH signed certificates");
+ }
+
+ VaultResponse response =
+ vaultTemplate.write(mountPath + path, Map.of("ip", "0.0.0.0"));
+
+ if (response == null || response.getData() == null) {
+ throw new GuacamoleException("No response from Vault SSH engine");
+ }
+
+ return response.getData();
+ }
+ else if (path.startsWith("sign/")) {
+ OpenBaoSshKeys sshKeys = new OpenBaoSshKeys(configService.getSshType());
+ Map<String, Object> request = Map.of(
+ "public_key", sshKeys.publicSsh,
+ "valid_principals", username,
+ "extensions", Map.of("permit-pty", ""),
+ "ttl", configService.getSshConnectionTimeout());
+
+ VaultResponse vaultResponse =
+ vaultTemplate.write(mountPath + path, request);
+
+ if (vaultResponse == null || vaultResponse.getData() == null) {
+ throw new GuacamoleException("No response from Vault SSH engine");
+ }
+
+ String signedCert = (String) vaultResponse.getData().get("signed_key");
+
+ if (signedCert == null) {
+ throw new GuacamoleException("Vault did not return a signed SSH certificate");
+ }
+
+ return Map.of("private", sshKeys.privateSshPem,
+ "public", signedCert,
+ "unsigned", sshKeys.publicSsh);
+ }
+ else {
+ throw new GuacamoleException("Unknown SSH type on path: " + mountPath + path);
+ }
+ }
+
+ /**
+ * Retrieves a username or password from an LDAP secret engine. The type of
+ * account supported might be static, dynamic or service accounts
+ *
+ * @param mountPath
+ * The mountPath of the LDAP secret engine on the vault server.
+ *
+ * @param path
+ * The path of the secret record representing the LDAP role
+ *
+ * @return
+ * The values associated with the path.
+ *
+ * @throws GuacamoleException
+ * If the secrets cannot be retrieved from the Vault.
+ */
+ private Map<String, Object> getValueLDAP(String mountPath, String path) throws GuacamoleException {
+ VaultResponse response;
+ if (path.startsWith("static") || path.startsWith("creds/")) {
+ response = vaultTemplate.read(mountPath + path);
+ }
+ else if (path.startsWith("library/")) {
+ response = vaultTemplate.write(mountPath + path + "/check-out", Map.of("ttl", "2h"));
+ }
+ else {
+ throw new GuacamoleException("Unknown LDAP type on path: " + mountPath +"/" + path);
+ }
+
+ if (response == null || response.getData() == null) {
+ throw new GuacamoleException("No response from LDAP secrets engine");
+ }
+
+ Map<String, Object> retval = response.getData();
+ if (path.startsWith("library/")) {
+ String username = String.valueOf(retval.get("service_account_name"));
+ retval.put("username", username);
+
+ // Register a listener to check-in the account for a TunnelClose Event
+ logger.info("Caching session : {}", username);
+ LDAPSessionInfo info = new LDAPSessionInfo(mountPath + path + "/check-in", username);
+ ldapSessions.put("checkin", info);
+ }
+
+ return retval;
+ }
+
+ /**
+ * Retrieves a username or password from a Databse secret engine.
Review Comment:
Typo in `Database`
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/src/main/java/org/apache/guacamole/vault/openbao/secret/OpenBaoSecretService.java:
##########
@@ -0,0 +1,319 @@
+/*
+ * 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.openbao.secret;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Future;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.UUID;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.net.auth.Connectable;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.UserContext;
+import org.apache.guacamole.protocol.GuacamoleConfiguration;
+import org.apache.guacamole.token.TokenFilter;
+import org.apache.guacamole.vault.openbao.secret.OpenBaoClient;
+import org.apache.guacamole.vault.openbao.secret.OpenBaoClientProvider;
+import org.apache.guacamole.vault.secret.VaultSecretService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * OpenBao implementation of VaultSecretService.
+ * Retrieves secrets from OpenBao based on parameters of the logged-in user.
+ */
+@Singleton
+public class OpenBaoSecretService implements VaultSecretService {
+
+ /**
+ * Logger for this class.
+ */
+ private static final Logger logger = LoggerFactory.getLogger(OpenBaoSecretService.class);
+
+ /**
+ * Client for communicating with OpenBao.
+ */
+ private final Provider<OpenBaoClient> openBaoClientProvider;
+
+ /**
+ * Constructor that loads when the service is created, forcing early
+ * start of Vault token renewal. Inject OpenBaoClient via Provider
+ * to avoid circular Guice dependency
+ *
+ * @param openBaoClientProvider
+ * A Provider for the OpenBaoClient singleton
+ */
+ @Inject
+ public OpenBaoSecretService(Provider<OpenBaoClient> openBaoClientProvider) {
+ this.openBaoClientProvider = openBaoClientProvider;
+ logger.debug("OpenBaoSecretService initialized");
+ }
+
+ /**
+ * Get a Guice cached copy of the OpenBaoClient Singleton
+ *
+ * @return
+ * A singleton OpenBaoClient instance
+ */
+ private OpenBaoClient client() {
+ return openBaoClientProvider.get();
+ }
+
+ /**
+ * As vault notation is essentially a URL, encode all components
+ * using standard URL escaping.
+ *
+ * @param nameComponent
+ * The token to be canonicalized
+ *
+ * @return
+ * The canonicalized token
+ */
+ @Override
+ public String canonicalize(String nameComponent) {
+ try {
+
+ //
Review Comment:
`//` without associated comment
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/src/main/java/org/apache/guacamole/vault/openbao/secret/OpenBaoClient.java:
##########
@@ -0,0 +1,644 @@
+/*
+ * 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.openbao.secret;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.RemovalCause;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.net.event.TunnelCloseEvent;
+import org.apache.guacamole.net.event.TunnelConnectEvent;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.vault.openbao.conf.OpenBaoConfigurationService;
+import org.apache.guacamole.vault.openbao.vault.FileTokenAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthentication;
+import org.apache.guacamole.vault.openbao.vault.UsernamePasswordAuthenticationOptions;
+import org.apache.guacamole.vault.openbao.vault.TtlAwareSessionManager;
+import org.springframework.http.client.SimpleClientHttpRequestFactory;
+import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
+import org.springframework.vault.authentication.ClientAuthentication;
+import org.springframework.vault.authentication.TokenAuthentication;
+import org.springframework.vault.client.VaultEndpoint;
+import org.springframework.vault.core.VaultKeyValueOperations;
+import org.springframework.vault.core.VaultTemplate;
+import org.springframework.vault.VaultException;
+import org.springframework.vault.support.VaultResponse;
+import org.springframework.web.client.RestTemplate;
+import org.springframework.web.util.DefaultUriBuilderFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+public class OpenBaoClient {
+ /**
+ * Logger for this class.
+ */
+ private static final Logger logger = LoggerFactory.getLogger(OpenBaoClient.class);
+
+ /**
+ * Service for retrieving OpenBao configuration.
+ */
+ private OpenBaoConfigurationService configService;
+
+ /**
+ * A singleton ObjectMapper for converting a Map to a JSON string when
+ * returning a complex token.
+ */
+ private static final ObjectMapper objectMapper = new ObjectMapper();
+
+ /**
+ * The prefix of the Guacamole token to resolve on the vault server.
+ */
+ static final String VAULT_TOKEN_PREFIX = "vault://";
+
+ /**
+ * Cache of secrets recently fetched
+ */
+ private Cache<String, Map<String, Object>> cache;
+
+ /**
+ * Vault template that will be used with all of the mount paths
+ */
+ private VaultTemplate vaultTemplate;
+
+ /*
+ * Ttl aware token manager for automatic token renewal
+ */
+ private TtlAwareSessionManager sessionManager;
+
+ /**
+ * Vault client authentication object
+ */
+ private ClientAuthentication authentication;
+
+ /**
+ * A HashMap of the checked out LDAP Sessions
+ */
+ private final Map<String, LDAPSessionInfo> ldapSessions = new HashMap<>();
+
+ /**
+ * A task scheduler for remove terminated checked out LDAP Sessions
+ */
+ ThreadPoolTaskScheduler scheduler;
+
+ /**
+ * Constructor allowing early injection of configuration and initialization
+ * to start the token renewal process as early as possible
+ *
+ * @param configService
+ * The injected configuration service
+ */
+ public OpenBaoClient(OpenBaoConfigurationService configService) {
+ this.configService = configService;
+
+ try {
+ VaultEndpoint endpoint = VaultEndpoint.from(configService.getVaultUri().resolve("v1"));
+
+ SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory();
+ try {
+ requestFactory.setConnectTimeout(configService.getConnectionTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint connection timeout: " + e.getMessage());
+ }
+ try {
+ requestFactory.setReadTimeout(configService.getRequestTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Using default vault endpoint request timeout: " + e.getMessage());
+ }
+
+ RestTemplate restTemplate = new RestTemplate(requestFactory);
+ restTemplate.setUriTemplateHandler(new DefaultUriBuilderFactory(
+ configService.getVaultUri().resolve("v1").toString()));
+
+ if (configService.getVaultToken() != null) {
+ if (isTokenReadableFile(configService.getVaultToken())) {
+ logger.debug("File Token : {}", configService.getVaultToken());
+ this.authentication =
+ new FileTokenAuthentication(configService.getVaultToken());
+ }
+ else {
+ logger.debug("Token : {}", configService.getVaultToken());
+ this.authentication = new TokenAuthentication(configService.getVaultToken());
+ }
+ }
+ else if (configService.getVaultUsername() != null && configService.getVaultPassword() != null) {
+ UsernamePasswordAuthenticationOptions options =
+ UsernamePasswordAuthenticationOptions.builder()
+ .username(configService.getVaultUsername())
+ .password(configService.getVaultPassword())
+ .build();
+
+ logger.debug("Username/Password : {}, {}", configService.getVaultUsername(),
+ configService.getVaultPassword());
Review Comment:
I'm not sure we should display a password in plain text in the logs, even at the debug level.
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/docs/openbao.md.j2:
##########
Review Comment:
Wouldn't this documentation be better suited to the [guacamole-manual](https://github.com/apache/guacamole-manual) project?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on code in PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#discussion_r3282406358
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/src/main/java/org/apache/guacamole/vault/openbao/secret/OpenBaoSecretService.java:
##########
@@ -0,0 +1,319 @@
+/*
+ * 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.openbao.secret;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Future;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.UUID;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.net.auth.Connectable;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.UserContext;
+import org.apache.guacamole.protocol.GuacamoleConfiguration;
+import org.apache.guacamole.token.TokenFilter;
+import org.apache.guacamole.vault.openbao.secret.OpenBaoClient;
+import org.apache.guacamole.vault.openbao.secret.OpenBaoClientProvider;
+import org.apache.guacamole.vault.secret.VaultSecretService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * OpenBao implementation of VaultSecretService.
+ * Retrieves secrets from OpenBao based on parameters of the logged-in user.
+ */
+@Singleton
+public class OpenBaoSecretService implements VaultSecretService {
+
+ /**
+ * Logger for this class.
+ */
+ private static final Logger logger = LoggerFactory.getLogger(OpenBaoSecretService.class);
+
+ /**
+ * Client for communicating with OpenBao.
+ */
+ private final Provider<OpenBaoClient> openBaoClientProvider;
+
+ /**
+ * Constructor that loads when the service is created, forcing early
+ * start of Vault token renewal. Inject OpenBaoClient via Provider
+ * to avoid circular Guice dependency
+ *
+ * @param openBaoClientProvider
+ * A Provider for the OpenBaoClient singleton
+ */
+ @Inject
+ public OpenBaoSecretService(Provider<OpenBaoClient> openBaoClientProvider) {
+ this.openBaoClientProvider = openBaoClientProvider;
+ logger.debug("OpenBaoSecretService initialized");
+ }
+
+ /**
+ * Get a Guice cached copy of the OpenBaoClient Singleton
+ *
+ * @return
+ * A singleton OpenBaoClient instance
+ */
+ private OpenBaoClient client() {
+ return openBaoClientProvider.get();
+ }
+
+ /**
+ * As vault notation is essentially a URL, encode all components
+ * using standard URL escaping.
+ *
+ * @param nameComponent
+ * The token to be canonicalized
+ *
+ * @return
+ * The canonicalized token
+ */
+ @Override
+ public String canonicalize(String nameComponent) {
+ try {
+
+ //
Review Comment:
Opps.. Removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#issuecomment-4513047860
Latest commit removes the need from Guacamole to have access to the Vault path "/sys/mounts" and uses the path-help API instead. The path "sys/mounts" give information and metadata on all secret engine mount paths in the Vault, which is possibly a security risk depending on your threat model, whereas the path-help functionality only responses for paths that Guacamole has the permission to access.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on code in PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#discussion_r3282310537
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/docs/include/openbao.properties.in:
##########
@@ -0,0 +1,6 @@
+# The URI of the OpenBao or Hashicorp Vault server to use.
+vault-uri: http://localhost:8200
+
+# The authentication token to use to access the vault
+vault-token: s.IPzVb3b5dThhjIrBX245szisjTQcylwSjMEeyJqcidaH8Hf
Review Comment:
Try decoding with a base62 decode and you'll see the value is "DidYouReallyThinkThisWasARealToken?"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on code in PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#discussion_r3282446815
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/docs/openbao.md.j2:
##########
Review Comment:
I'd like to keep the docuemntation here for now , so everything is in one place. The last thing I'll do before the merge is to remove it and port it to guacamole-manual. Is that enough to resolve this ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2196: Openbao/Hashicorp Vault extension [guacamole-client]
Posted by "adb014 (via GitHub)" <gi...@apache.org>.
adb014 commented on code in PR #1214:
URL: https://github.com/apache/guacamole-client/pull/1214#discussion_r3282459684
##########
extensions/guacamole-vault/modules/guacamole-vault-openbao/docs/openbao.md.j2:
##########
Review Comment:
In the doc/ directory I also have a test script "openbao.sh" that I use to setup a test openbao server and printout a test plan with all the secrets needed to test all of the ways the tokens can be included in Guacamole . SHould this alos be removed ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org