GUACAMOLE-1623: Extract domain field directly from the vault, or split out of username.

This commit is contained in:
James Muehlner
2022-06-15 22:44:32 +00:00
parent 8822db781e
commit 647cfa6a0c
8 changed files with 381 additions and 22 deletions

View File

@@ -59,6 +59,13 @@
<artifactId>jackson-dataformat-yaml</artifactId> <artifactId>jackson-dataformat-yaml</artifactId>
</dependency> </dependency>
<!-- JUnit -->
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<!-- Guice --> <!-- Guice -->
<dependency> <dependency>
<groupId>com.google.inject</groupId> <groupId>com.google.inject</groupId>

View File

@@ -55,7 +55,7 @@ public abstract class VaultConfigurationService {
@Inject @Inject
private VaultSecretService secretService; private VaultSecretService secretService;
/** /**
* ObjectMapper for deserializing YAML. * ObjectMapper for deserializing YAML.
*/ */
@@ -127,7 +127,7 @@ public abstract class VaultConfigurationService {
return Collections.emptyMap(); return Collections.emptyMap();
return mapping; return mapping;
} }
// Fail if YAML is invalid/unreadable // Fail if YAML is invalid/unreadable
@@ -169,7 +169,7 @@ public abstract class VaultConfigurationService {
String secretName = super.getProperty(name); String secretName = super.getProperty(name);
if (secretName == null) if (secretName == null)
return null; return null;
return secretService.getValue(secretName).get(); return secretService.getValue(secretName).get();
} }
@@ -177,7 +177,7 @@ public abstract class VaultConfigurationService {
if (e.getCause() instanceof GuacamoleException) if (e.getCause() instanceof GuacamoleException)
throw (GuacamoleException) e; throw (GuacamoleException) e;
throw new GuacamoleServerException(String.format("Property " throw new GuacamoleServerException(String.format("Property "
+ "\"%s\" could not be retrieved from the vault.", name), e); + "\"%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;
} }

View File

@@ -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(
"(?<domain>[^@\\\\]+)\\\\(?<username>[^@\\\\]+)");
/**
* 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(
"(?<username>[^@\\\\]+)@(?<domain>[^@\\\\]+)");
/**
* 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);
}
}

View File

@@ -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<String> 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());
});
}
}

View File

@@ -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 * Creates a new KsmConfigurationService which reads the configuration
* from "ksm-token-mapping.yml" and properties from * from "ksm-token-mapping.yml" and properties from
@@ -105,6 +117,11 @@ public class KsmConfigurationService extends VaultConfigurationService {
return environment.getProperty(ALLOW_UNVERIFIED_CERT, false); 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 * Returns the options required to authenticate with Keeper Secrets Manager
* when retrieving secrets. These options are read from the contents of * when retrieving secrets. These options are read from the contents of

View File

@@ -22,7 +22,6 @@ package org.apache.guacamole.vault.ksm.secret;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import com.keepersecurity.secretsManager.core.Hosts; import com.keepersecurity.secretsManager.core.Hosts;
import com.keepersecurity.secretsManager.core.KeeperFile;
import com.keepersecurity.secretsManager.core.KeeperRecord; import com.keepersecurity.secretsManager.core.KeeperRecord;
import com.keepersecurity.secretsManager.core.KeeperSecrets; import com.keepersecurity.secretsManager.core.KeeperSecrets;
import com.keepersecurity.secretsManager.core.Login; import com.keepersecurity.secretsManager.core.Login;
@@ -119,7 +118,7 @@ public class KsmClient {
* {@link #cacheLock} acquired appropriately. * {@link #cacheLock} acquired appropriately.
*/ */
private KeeperSecrets cachedSecrets = null; private KeeperSecrets cachedSecrets = null;
/** /**
* All records retrieved from Keeper Secrets Manager, where each key is the * All records retrieved from Keeper Secrets Manager, where each key is the
* UID of the corresponding record. The contents of this Map are * UID of the corresponding record. The contents of this Map are
@@ -216,7 +215,7 @@ public class KsmClient {
// Store all secrets within cache // Store all secrets within cache
cachedSecrets = secrets; cachedSecrets = secrets;
// Clear unambiguous cache of all records by UID // Clear unambiguous cache of all records by UID
cachedRecordsByUid.clear(); cachedRecordsByUid.clear();
@@ -398,7 +397,7 @@ public class KsmClient {
* The record associated with the given username, or null if there is * The record associated with the given username, or null if there is
* no such record or multiple such records. * no such record or multiple such records.
* *
* @throws GuacamoleException * @throws GuacamoleException
* If an error occurs that prevents the record from being retrieved. * If an error occurs that prevents the record from being retrieved.
*/ */
public KeeperRecord getRecordByLogin(String username) throws GuacamoleException { public KeeperRecord getRecordByLogin(String username) throws GuacamoleException {

View File

@@ -41,12 +41,20 @@ import java.util.function.Function;
import java.util.regex.Matcher; import java.util.regex.Matcher;
import java.util.regex.Pattern; import java.util.regex.Pattern;
/** /**
* Service for automatically parsing out secrets and data from Keeper records. * Service for automatically parsing out secrets and data from Keeper records.
*/ */
@Singleton @Singleton
public class KsmRecordService { 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 * Regular expression which matches the labels of custom fields containing
* hostnames/addresses. * hostnames/addresses.
@@ -143,13 +151,13 @@ public class KsmRecordService {
* empty or contains multiple values, null is returned. Note that null will * empty or contains multiple values, null is returned. Note that null will
* also be returned if the mapping transformation returns null for the * also be returned if the mapping transformation returns null for the
* single value stored in the list. * single value stored in the list.
* *
* @param <T> * @param <T>
* The type of object stored in the list. * The type of object stored in the list.
* *
* @param <R> * @param <R>
* The type of object to return. * The type of object to return.
* *
* @param values * @param values
* The list to retrieve a single value from. * The list to retrieve a single value from.
* *
@@ -168,7 +176,7 @@ public class KsmRecordService {
return null; return null;
return mapper.apply(value); return mapper.apply(value);
} }
/** /**
@@ -271,7 +279,7 @@ public class KsmRecordService {
* multiple such fields, null is returned. Both standard and custom fields * multiple such fields, null is returned. Both standard and custom fields
* are searched. As standard fields do not have labels, any given label * are searched. As standard fields do not have labels, any given label
* pattern is ignored for standard fields. * pattern is ignored for standard fields.
* *
* @param <T> * @param <T>
* The type of field to return. * The type of field to return.
* *
@@ -339,7 +347,7 @@ public class KsmRecordService {
return null; return null;
foundFile = file; foundFile = file;
} }
return foundFile; return foundFile;
@@ -362,7 +370,7 @@ public class KsmRecordService {
if (file == null) if (file == null)
return CompletableFuture.completedFuture(null); return CompletableFuture.completedFuture(null);
return CompletableFuture.supplyAsync(() -> { return CompletableFuture.supplyAsync(() -> {
return new String(SecretsManager.downloadFile(file), StandardCharsets.UTF_8); 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<KeeperRecordField> 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 * Returns the password associated with the given record. Both standard and
* custom fields are searched. Only "Password" and "Hidden" field types are * 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: // a pair of custom hidden fields for the private key and passphrase:
// the standard password field of the "Login" record refers to the // the standard password field of the "Login" record refers to the
// user's own password, if any, not the passphrase of their key) // user's own password, if any, not the passphrase of their key)
// Use password "private key" custom field as fallback ... // Use password "private key" custom field as fallback ...
Password passwordField = getField(custom, Password.class, PASSPHRASE_LABEL_PATTERN); Password passwordField = getField(custom, Password.class, PASSPHRASE_LABEL_PATTERN);
if (passwordField != null) if (passwordField != null)
@@ -567,7 +607,7 @@ public class KsmRecordService {
return getSingleStringValue(hiddenField.getValue()); return getSingleStringValue(hiddenField.getValue());
return null; return null;
} }
} }

View File

@@ -32,7 +32,9 @@ import java.util.concurrent.Future;
import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleException;
import org.apache.guacamole.protocol.GuacamoleConfiguration; import org.apache.guacamole.protocol.GuacamoleConfiguration;
import org.apache.guacamole.token.TokenFilter; 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.VaultSecretService;
import org.apache.guacamole.vault.secret.WindowsUsername;
/** /**
* Service which retrieves secrets from Keeper Secrets Manager. * Service which retrieves secrets from Keeper Secrets Manager.
@@ -52,6 +54,12 @@ public class KsmSecretService implements VaultSecretService {
@Inject @Inject
private KsmRecordService recordService; private KsmRecordService recordService;
/**
* Service for retrieving configuration information.
*/
@Inject
private KsmConfigurationService confService;
@Override @Override
public String canonicalize(String nameComponent) { public String canonicalize(String nameComponent) {
try { try {
@@ -86,17 +94,48 @@ public class KsmSecretService implements VaultSecretService {
* @param record * @param record
* The record to retrieve secrets from when generating tokens. This may * The record to retrieve secrets from when generating tokens. This may
* be null. * be null.
*
* @throws GuacamoleException
* If configuration details in guacamole.properties cannot be parsed.
*/ */
private void addRecordTokens(Map<String, Future<String>> tokens, String prefix, private void addRecordTokens(Map<String, Future<String>> tokens, String prefix,
KeeperRecord record) { KeeperRecord record) throws GuacamoleException {
if (record == null) if (record == null)
return; 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 // Username of server-related record
String username = recordService.getUsername(record); String username = recordService.getUsername(record);
if (username != null) if (username != null) {
tokens.put(prefix + "USERNAME", CompletableFuture.completedFuture(username));
// 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 // Password of server-related record
String password = recordService.getPassword(record); String password = recordService.getPassword(record);
@@ -113,7 +152,7 @@ public class KsmSecretService implements VaultSecretService {
tokens.put(prefix + "KEY", privateKey); tokens.put(prefix + "KEY", privateKey);
} }
@Override @Override
public Map<String, Future<String>> getTokens(GuacamoleConfiguration config, public Map<String, Future<String>> getTokens(GuacamoleConfiguration config,
TokenFilter filter) throws GuacamoleException { TokenFilter filter) throws GuacamoleException {
@@ -135,7 +174,7 @@ public class KsmSecretService implements VaultSecretService {
// Tokens specific to RDP // Tokens specific to RDP
if ("rdp".equals(config.getProtocol())) { if ("rdp".equals(config.getProtocol())) {
// Retrieve and define gateway server-specific tokens, if any // Retrieve and define gateway server-specific tokens, if any
String gatewayHostname = parameters.get("gateway-hostname"); String gatewayHostname = parameters.get("gateway-hostname");
if (gatewayHostname != null && !gatewayHostname.isEmpty()) if (gatewayHostname != null && !gatewayHostname.isEmpty())