diff --git a/extensions/guacamole-vault/modules/guacamole-vault-base/pom.xml b/extensions/guacamole-vault/modules/guacamole-vault-base/pom.xml index 96fe1180a..a8137b52f 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-base/pom.xml +++ b/extensions/guacamole-vault/modules/guacamole-vault-base/pom.xml @@ -59,6 +59,13 @@ jackson-dataformat-yaml + + + junit + junit + test + + com.google.inject diff --git a/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/conf/VaultConfigurationService.java b/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/conf/VaultConfigurationService.java index a666a7b97..490da6816 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/conf/VaultConfigurationService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/conf/VaultConfigurationService.java @@ -55,7 +55,7 @@ public abstract class VaultConfigurationService { @Inject private VaultSecretService secretService; - + /** * ObjectMapper for deserializing YAML. */ @@ -127,7 +127,7 @@ public abstract class VaultConfigurationService { return Collections.emptyMap(); return mapping; - + } // Fail if YAML is invalid/unreadable @@ -169,7 +169,7 @@ public abstract class VaultConfigurationService { String secretName = super.getProperty(name); if (secretName == null) return null; - + return secretService.getValue(secretName).get(); } @@ -177,7 +177,7 @@ public abstract class VaultConfigurationService { if (e.getCause() instanceof GuacamoleException) throw (GuacamoleException) e; - + throw new GuacamoleServerException(String.format("Property " + "\"%s\" could not be retrieved from the vault.", name), e); } @@ -187,4 +187,23 @@ public abstract class VaultConfigurationService { } + /** + * Return whether Windows domains should be split out from usernames when + * fetched from the vault. + * + * For example: "DOMAIN\\user" or "user@DOMAIN" should both + * be split into seperate username and domain tokens if this configuration + * is true. If false, no domain token should be created and the above values + * should be stored directly in the username token. + * + * @return + * true if windows domains should be split out from usernames, false + * otherwise. + * + * @throws GuacamoleException + * If the value specified within guacamole.properties cannot be + * parsed. + */ + public abstract boolean getSplitWindowsUsernames() throws GuacamoleException; + } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/secret/WindowsUsername.java b/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/secret/WindowsUsername.java new file mode 100644 index 000000000..36d8a1b05 --- /dev/null +++ b/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/secret/WindowsUsername.java @@ -0,0 +1,157 @@ +/* + * 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.secret; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import javax.annotation.Nonnull; + +/** + * A class representing a Windows username, which may optionally also include + * a domain. This class can be used to parse the username and domain out of a + * username from a vault. + */ +public class WindowsUsername { + + /** + * A pattern for matching a down-level logon name containing a Windows + * domain and username - e.g. domain\\user. For more information, see + * https://docs.microsoft.com/en-us/windows/win32/secauthn/user-name-formats#down-level-logon-name + */ + private static final Pattern DOWN_LEVEL_LOGON_NAME_PATTERN = Pattern.compile( + "(?[^@\\\\]+)\\\\(?[^@\\\\]+)"); + + /** + * A pattern for matching a user principal name containing a Windows + * domain and username - e.g. user@domain. For more information, see + * https://docs.microsoft.com/en-us/windows/win32/secauthn/user-name-formats#user-principal-name + */ + private static final Pattern USER_PRINCIPAL_NAME_PATTERN = Pattern.compile( + "(?[^@\\\\]+)@(?[^@\\\\]+)"); + + /** + * The username associated with the potential Windows domain/username + * value. If no domain is found, the username field will contain the + * entire value as read from the vault. + */ + private final String username; + + /** + * The dinaun associated with the potential Windows domain/username + * value. If no domain is found, this will be null. + */ + private final String domain; + + /** + * Create a WindowsUsername record with no associated domain. + * + * @param username + * The username, which should be the entire value as extracted + * from the vault. + */ + private WindowsUsername(@Nonnull String username) { + this.username = username; + this.domain = null; + } + + /** + * Create a WindowsUsername record with a username and a domain. + * + * @param username + * The username portion of the field value from the vault. + * + * @param domain + * The domain portion of the field value from the vault. + */ + private WindowsUsername( + @Nonnull String username, @Nonnull String domain) { + this.username = username; + this.domain = domain; + } + + /** + * Return the value of the username as extracted from the vault field. + * If the domain is null, this will be the entire field value. + * + * @return + * The username value as extracted from the vault field. + */ + public String getUsername() { + return username; + } + + /** + * Return the value of the domain as extracted from the vault field. + * If this is null, it means that no domain was found in the vault field. + * + * @return + * The domain value as extracted from the vault field. + */ + public String getDomain() { + return domain; + } + + /** + * Return true if a domain was found in the vault field, false otherwise. + * + * @return + * true if a domain was found in the vault field, false otherwise. + */ + public boolean hasDomain() { + return this.domain != null; + } + + /** + * Strip off a Windows domain from the provided username, if one is + * present. For example: "DOMAIN\\user" or "user@DOMAIN" will both + * be stripped to just "user". Note: neither the '@' or '\\' characters + * are valid in Windows usernames. + * + * @param vaultField + * The raw field value as retrieved from the vault. This might contain + * a Windows domain. + * + * @return + * The provided username with the Windows domain stripped off, if one + * is present. + */ + public static WindowsUsername splitWindowsUsernameFromDomain(String vaultField) { + + // If it's the down-level logon format, return the extracted username and domain + Matcher downLevelLogonMatcher = DOWN_LEVEL_LOGON_NAME_PATTERN.matcher(vaultField); + if (downLevelLogonMatcher.matches()) + return new WindowsUsername( + downLevelLogonMatcher.group("username"), + downLevelLogonMatcher.group("domain")); + + // If it's the user principal format, return the extracted username and domain + Matcher userPrincipalMatcher = USER_PRINCIPAL_NAME_PATTERN.matcher(vaultField); + if (userPrincipalMatcher.matches()) + return new WindowsUsername( + userPrincipalMatcher.group("username"), + userPrincipalMatcher.group("domain")); + + // If none of the expected formats matched, return the username with do domain + return new WindowsUsername(vaultField); + + } + +} \ No newline at end of file diff --git a/extensions/guacamole-vault/modules/guacamole-vault-base/src/test/java/org/apache/guacamole/vault/secret/WindowsUsernameTest.java b/extensions/guacamole-vault/modules/guacamole-vault-base/src/test/java/org/apache/guacamole/vault/secret/WindowsUsernameTest.java new file mode 100644 index 000000000..9a4c5c716 --- /dev/null +++ b/extensions/guacamole-vault/modules/guacamole-vault-base/src/test/java/org/apache/guacamole/vault/secret/WindowsUsernameTest.java @@ -0,0 +1,81 @@ +/* + * 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.secret; + +import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.List; + +/** + * Class to test the parsing functionality of the WindowsUsername class. + */ +public class WindowsUsernameTest { + + /** + * Verify that the splitWindowsUsernameFromDomain() method correctly strips Windows + * domains from provided usernames that include them, and does not modify + * usernames that do not have Windows domains. + */ + @Test + public void testSplitWindowsUsernameFromDomain() { + + WindowsUsername usernameAndDomain; + + // If no Windows domain is present in the provided field, the username should + // contain the entire field, and no domain should be returned + usernameAndDomain = WindowsUsername.splitWindowsUsernameFromDomain("bob"); + assertEquals(usernameAndDomain.getUsername(), "bob"); + assertFalse(usernameAndDomain.hasDomain()); + + // It should parse down-level logon name style domains + usernameAndDomain = WindowsUsername.splitWindowsUsernameFromDomain("localhost\\bob"); + assertEquals("bob", usernameAndDomain.getUsername(), "bob"); + assertTrue(usernameAndDomain.hasDomain()); + assertEquals("localhost", usernameAndDomain.getDomain()); + + // It should parse user principal name style domains + usernameAndDomain = WindowsUsername.splitWindowsUsernameFromDomain("bob@localhost"); + assertEquals("bob", usernameAndDomain.getUsername(), "bob"); + assertTrue(usernameAndDomain.hasDomain()); + assertEquals("localhost", usernameAndDomain.getDomain()); + + // It should not match if there are an invalid number of separators + List invalidSeparators = List.of( + "bob@local@host", "local\\host\\bob", + "bob\\local@host", "local@host\\bob"); + invalidSeparators.stream().forEach( + invalidSeparator -> { + + // An invalid number of separators means that the parse failed - + // there should be no detected domain, and the entire field value + // should be returned as the username + WindowsUsername parseOutput = + WindowsUsername.splitWindowsUsernameFromDomain(invalidSeparator); + assertFalse(parseOutput.hasDomain()); + assertEquals(invalidSeparator, parseOutput.getUsername()); + + }); + + } + +} diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java index 38bcaaef1..0bd3f40da 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java @@ -76,6 +76,18 @@ public class KsmConfigurationService extends VaultConfigurationService { } }; + /** + * Whether windows domains should be stripped off from usernames that are + * read from the KSM vault. + */ + private static final BooleanGuacamoleProperty STRIP_WINDOWS_DOMAINS = new BooleanGuacamoleProperty() { + + @Override + public String getName() { + return "ksm-strip-windows-domains"; + } + }; + /** * Creates a new KsmConfigurationService which reads the configuration * from "ksm-token-mapping.yml" and properties from @@ -105,6 +117,11 @@ public class KsmConfigurationService extends VaultConfigurationService { return environment.getProperty(ALLOW_UNVERIFIED_CERT, false); } + @Override + public boolean getSplitWindowsUsernames() throws GuacamoleException { + return environment.getProperty(STRIP_WINDOWS_DOMAINS, false); + } + /** * Returns the options required to authenticate with Keeper Secrets Manager * when retrieving secrets. These options are read from the contents of diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java index 2372dcb29..fa177014b 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java @@ -22,7 +22,6 @@ package org.apache.guacamole.vault.ksm.secret; import com.google.inject.Inject; import com.google.inject.Singleton; import com.keepersecurity.secretsManager.core.Hosts; -import com.keepersecurity.secretsManager.core.KeeperFile; import com.keepersecurity.secretsManager.core.KeeperRecord; import com.keepersecurity.secretsManager.core.KeeperSecrets; import com.keepersecurity.secretsManager.core.Login; @@ -119,7 +118,7 @@ public class KsmClient { * {@link #cacheLock} acquired appropriately. */ private KeeperSecrets cachedSecrets = null; - + /** * All records retrieved from Keeper Secrets Manager, where each key is the * UID of the corresponding record. The contents of this Map are @@ -216,7 +215,7 @@ public class KsmClient { // Store all secrets within cache cachedSecrets = secrets; - + // Clear unambiguous cache of all records by UID cachedRecordsByUid.clear(); @@ -398,7 +397,7 @@ public class KsmClient { * The record associated with the given username, or null if there is * no such record or multiple such records. * - * @throws GuacamoleException + * @throws GuacamoleException * If an error occurs that prevents the record from being retrieved. */ public KeeperRecord getRecordByLogin(String username) throws GuacamoleException { diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmRecordService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmRecordService.java index d67b5816a..a3d79b49e 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmRecordService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmRecordService.java @@ -41,12 +41,20 @@ import java.util.function.Function; import java.util.regex.Matcher; import java.util.regex.Pattern; + /** * Service for automatically parsing out secrets and data from Keeper records. */ @Singleton public class KsmRecordService { + /** + * Regular expression which matches the labels of custom fields containing + * domains. + */ + private static final Pattern DOMAIN_LABEL_PATTERN = + Pattern.compile("domain", Pattern.CASE_INSENSITIVE); + /** * Regular expression which matches the labels of custom fields containing * hostnames/addresses. @@ -143,13 +151,13 @@ public class KsmRecordService { * empty or contains multiple values, null is returned. Note that null will * also be returned if the mapping transformation returns null for the * single value stored in the list. - * + * * @param * The type of object stored in the list. - * + * * @param * The type of object to return. - * + * * @param values * The list to retrieve a single value from. * @@ -168,7 +176,7 @@ public class KsmRecordService { return null; return mapper.apply(value); - + } /** @@ -271,7 +279,7 @@ public class KsmRecordService { * multiple such fields, null is returned. Both standard and custom fields * are searched. As standard fields do not have labels, any given label * pattern is ignored for standard fields. - * + * * @param * The type of field to return. * @@ -339,7 +347,7 @@ public class KsmRecordService { return null; foundFile = file; - + } return foundFile; @@ -362,7 +370,7 @@ public class KsmRecordService { if (file == null) return CompletableFuture.completedFuture(null); - + return CompletableFuture.supplyAsync(() -> { return new String(SecretsManager.downloadFile(file), StandardCharsets.UTF_8); }); @@ -445,6 +453,38 @@ public class KsmRecordService { } + /** + * Returns the single domain associated with the given record. If the + * record has no associated domain, or multiple domains, null is + * returned. Domains are retrieved from "Text" and "Hidden" fields + * that have the label "domain" (case-insensitive). + * + * @param record + * The record to retrieve the domain from. + * + * @return + * The domain associated with the given record, or null if the record + * has no associated domain or multiple domains. + */ + public String getDomain(KeeperRecord record) { + + KeeperRecordData data = record.getData(); + List custom = data.getCustom(); + + // First check text "domain" custom field ... + Text textField = getField(custom, Text.class, DOMAIN_LABEL_PATTERN); + if (textField != null) + return getSingleStringValue(textField.getValue()); + + // ... or hidden "domain" custom field if that's not found + HiddenField hiddenField = getField(custom, HiddenField.class, DOMAIN_LABEL_PATTERN); + if (hiddenField != null) + return getSingleStringValue(hiddenField.getValue()); + + return null; + + } + /** * Returns the password associated with the given record. Both standard and * custom fields are searched. Only "Password" and "Hidden" field types are @@ -555,7 +595,7 @@ public class KsmRecordService { // a pair of custom hidden fields for the private key and passphrase: // the standard password field of the "Login" record refers to the // user's own password, if any, not the passphrase of their key) - + // Use password "private key" custom field as fallback ... Password passwordField = getField(custom, Password.class, PASSPHRASE_LABEL_PATTERN); if (passwordField != null) @@ -567,7 +607,7 @@ public class KsmRecordService { return getSingleStringValue(hiddenField.getValue()); return null; - + } } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java index 824f9e54e..21d7c91ac 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java @@ -32,7 +32,9 @@ import java.util.concurrent.Future; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.protocol.GuacamoleConfiguration; import org.apache.guacamole.token.TokenFilter; +import org.apache.guacamole.vault.ksm.conf.KsmConfigurationService; import org.apache.guacamole.vault.secret.VaultSecretService; +import org.apache.guacamole.vault.secret.WindowsUsername; /** * Service which retrieves secrets from Keeper Secrets Manager. @@ -52,6 +54,12 @@ public class KsmSecretService implements VaultSecretService { @Inject private KsmRecordService recordService; + /** + * Service for retrieving configuration information. + */ + @Inject + private KsmConfigurationService confService; + @Override public String canonicalize(String nameComponent) { try { @@ -86,17 +94,48 @@ public class KsmSecretService implements VaultSecretService { * @param record * The record to retrieve secrets from when generating tokens. This may * be null. + * + * @throws GuacamoleException + * If configuration details in guacamole.properties cannot be parsed. */ private void addRecordTokens(Map> tokens, String prefix, - KeeperRecord record) { + KeeperRecord record) throws GuacamoleException { if (record == null) return; + // Domain of server-related record + String domain = recordService.getDomain(record); + if (domain != null) + tokens.put(prefix + "DOMAIN", CompletableFuture.completedFuture(domain)); + // Username of server-related record String username = recordService.getUsername(record); - if (username != null) - tokens.put(prefix + "USERNAME", CompletableFuture.completedFuture(username)); + if (username != null) { + + // If the record had no directly defined domain, but there is a + // username, and the configuration is enabled to split Windows + // domains out of usernames, attempt to split the domain out now + if (domain == null && confService.getSplitWindowsUsernames()) { + WindowsUsername usernameAndDomain = + WindowsUsername.splitWindowsUsernameFromDomain(username); + + // Always store the username token + tokens.put(prefix + "USERNAME", CompletableFuture.completedFuture( + usernameAndDomain.getUsername())); + + // Only store the domain if one is detected + if (usernameAndDomain.hasDomain()) + tokens.put(prefix + "DOMAIN", CompletableFuture.completedFuture( + usernameAndDomain.getDomain())); + + } + + // If splitting is not enabled, store the whole value in the USERNAME token + else { + tokens.put(prefix + "USERNAME", CompletableFuture.completedFuture(username)); + } + } // Password of server-related record String password = recordService.getPassword(record); @@ -113,7 +152,7 @@ public class KsmSecretService implements VaultSecretService { tokens.put(prefix + "KEY", privateKey); } - + @Override public Map> getTokens(GuacamoleConfiguration config, TokenFilter filter) throws GuacamoleException { @@ -135,7 +174,7 @@ public class KsmSecretService implements VaultSecretService { // Tokens specific to RDP if ("rdp".equals(config.getProtocol())) { - + // Retrieve and define gateway server-specific tokens, if any String gatewayHostname = parameters.get("gateway-hostname"); if (gatewayHostname != null && !gatewayHostname.isEmpty())