honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Security: reduce memory usage of DnRoleMapper (#34250) 

The `DnRoleMapper` class is used to map distinguished names of groups
and users to role names. This mapper builds in an internal map that
maps from a `com.unboundid.ldap.sdk.DN` to a `Set<String>`. In cases
where a lot of distinct DNs are mapped to roles, this can consume quite
a bit of memory. The majority of the memory is consumed by the DN
object. For example, a 94 character DN that has 9 relative DNs (RDN)
will retain 4KB of memory, whereas the String itself consumes less than
250 bytes.

In order to reduce memory usage, we can map from a normalized DN string
to a List of roles. The normalized string is actually how the DN class
determines equality with another DN and we can drop the overhead of
needing to keep all of the other objects in memory. Additionally the
use of a List provides memory savings as each HashSet is backed by a
HashMap, which consumes a great deal more memory than an appropriately
sized ArrayList. The uniqueness we get from a Set is maintained by
first building a set when parsing the file and then converting to a
list upon completion.

Closes #34237
This commit is contained in:
Jay Modi 2018-10-03 09:30:57 -06:00 committed by GitHub
parent f09190c14d
commit 3c1fdc9fc0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 26 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapper.java
View File

@ -8,14 +8,17 @@ package org.elasticsearch.xpack.security.authc.support;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.stream.Collectors;
import com.unboundid.ldap.sdk.DN;
import com.unboundid.ldap.sdk.LDAPException;
@ -51,7 +54,7 @@ public class DnRoleMapper implements UserRoleMapper {
private final Path file;
private final boolean useUnmappedGroupsAsRoles;
private final CopyOnWriteArrayList<Runnable> listeners = new CopyOnWriteArrayList<>();
private volatile Map<DN, Set<String>> dnRoles;
private volatile Map<String, List<String>> dnRoles;
public DnRoleMapper(RealmConfig config, ResourceWatcherService watcherService) {
this.config = config;
@ -87,7 +90,7 @@ public class DnRoleMapper implements UserRoleMapper {
* logging the error and skipping/removing all mappings. This is aligned with how we handle other auto-loaded files
* in security.
*/
public static Map<DN, Set<String>> parseFileLenient(Path path, Logger logger, String realmType, String realmName) {
public static Map<String, List<String>> parseFileLenient(Path path, Logger logger, String realmType, String realmName) {
try {
return parseFile(path, logger, realmType, realmName, false);
} catch (Exception e) {
@ -98,7 +101,7 @@ public class DnRoleMapper implements UserRoleMapper {
}
}
public static Map<DN, Set<String>> parseFile(Path path, Logger logger, String realmType, String realmName, boolean strict) {
public static Map<String, List<String>> parseFile(Path path, Logger logger, String realmType, String realmName, boolean strict) {
logger.trace("reading realm [{}/{}] role mappings file [{}]...", realmType, realmName, path.toAbsolutePath());
@ -149,7 +152,10 @@ public class DnRoleMapper implements UserRoleMapper {
logger.debug("[{}] role mappings found in file [{}] for realm [{}/{}]", dnToRoles.size(), path.toAbsolutePath(), realmType,
realmName);
return unmodifiableMap(dnToRoles);
Map<String, List<String>> normalizedMap = dnToRoles.entrySet().stream().collect(Collectors.toMap(
entry -> entry.getKey().toNormalizedString(),
entry -> Collections.unmodifiableList(new ArrayList<>(entry.getValue()))));
return unmodifiableMap(normalizedMap);
} catch (IOException | SettingsException e) {
throw new ElasticsearchException("could not read realm [" + realmType + "/" + realmName + "] role mappings file [" +
path.toAbsolutePath() + "]", e);
@ -176,8 +182,9 @@ public class DnRoleMapper implements UserRoleMapper {
Set<String> roles = new HashSet<>();
for (String groupDnString : groupDns) {
DN groupDn = dn(groupDnString);
if (dnRoles.containsKey(groupDn)) {
roles.addAll(dnRoles.get(groupDn));
String normalizedGroupDn = groupDn.toNormalizedString();
if (dnRoles.containsKey(normalizedGroupDn)) {
roles.addAll(dnRoles.get(normalizedGroupDn));
} else if (useUnmappedGroupsAsRoles) {
roles.add(relativeName(groupDn));
}
@ -187,14 +194,14 @@ public class DnRoleMapper implements UserRoleMapper {
groupDns, file.getFileName(), config.type(), config.name());
}
DN userDn = dn(userDnString);
Set<String> rolesMappedToUserDn = dnRoles.get(userDn);
String normalizedUserDn = dn(userDnString).toNormalizedString();
List<String> rolesMappedToUserDn = dnRoles.get(normalizedUserDn);
if (rolesMappedToUserDn != null) {
roles.addAll(rolesMappedToUserDn);
}
if (logger.isDebugEnabled()) {
logger.debug("the roles [{}], are mapped from the user [{}] using file [{}] for realm [{}/{}]",
(rolesMappedToUserDn == null) ? Collections.emptySet() : rolesMappedToUserDn, userDnString, file.getFileName(),
(rolesMappedToUserDn == null) ? Collections.emptySet() : rolesMappedToUserDn, normalizedUserDn, file.getFileName(),
config.type(), config.name());
}
return roles;

20
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapperTests.java
View File

@ -200,27 +200,27 @@ public class DnRoleMapperTests extends ESTestCase {
public void testParseFile() throws Exception {
Path file = getDataPath("role_mapping.yml");
Logger logger = CapturingLogger.newCapturingLogger(Level.INFO, null);
Map<DN, Set<String>> mappings = DnRoleMapper.parseFile(file, logger, "_type", "_name", false);
Map<String, List<String>> mappings = DnRoleMapper.parseFile(file, logger, "_type", "_name", false);
assertThat(mappings, notNullValue());
assertThat(mappings.size(), is(3));
DN dn = new DN("cn=avengers,ou=marvel,o=superheros");
assertThat(mappings, hasKey(dn));
Set<String> roles = mappings.get(dn);
assertThat(mappings, hasKey(dn.toNormalizedString()));
List<String> roles = mappings.get(dn.toNormalizedString());
assertThat(roles, notNullValue());
assertThat(roles, hasSize(2));
assertThat(roles, containsInAnyOrder("security", "avenger"));
dn = new DN("cn=shield,ou=marvel,o=superheros");
assertThat(mappings, hasKey(dn));
roles = mappings.get(dn);
assertThat(mappings, hasKey(dn.toNormalizedString()));
roles = mappings.get(dn.toNormalizedString());
assertThat(roles, notNullValue());
assertThat(roles, hasSize(1));
assertThat(roles, contains("security"));
dn = new DN("cn=Horatio Hornblower,ou=people,o=sevenSeas");
assertThat(mappings, hasKey(dn));
roles = mappings.get(dn);
assertThat(mappings, hasKey(dn.toNormalizedString()));
roles = mappings.get(dn.toNormalizedString());
assertThat(roles, notNullValue());
assertThat(roles, hasSize(1));
assertThat(roles, contains("avenger"));
@ -230,7 +230,7 @@ public class DnRoleMapperTests extends ESTestCase {
Path file = createTempDir().resolve("foo.yaml");
Files.createFile(file);
Logger logger = CapturingLogger.newCapturingLogger(Level.DEBUG, null);
Map<DN, Set<String>> mappings = DnRoleMapper.parseFile(file, logger, "_type", "_name", false);
Map<String, List<String>> mappings = DnRoleMapper.parseFile(file, logger, "_type", "_name", false);
assertThat(mappings, notNullValue());
assertThat(mappings.isEmpty(), is(true));
List<String> events = CapturingLogger.output(logger.getName(), Level.DEBUG);
@ -242,7 +242,7 @@ public class DnRoleMapperTests extends ESTestCase {
public void testParseFile_WhenFileDoesNotExist() throws Exception {
Path file = createTempDir().resolve(randomAlphaOfLength(10));
Logger logger = CapturingLogger.newCapturingLogger(Level.INFO, null);
Map<DN, Set<String>> mappings = DnRoleMapper.parseFile(file, logger, "_type", "_name", false);
Map<String, List<String>> mappings = DnRoleMapper.parseFile(file, logger, "_type", "_name", false);
assertThat(mappings, notNullValue());
assertThat(mappings.isEmpty(), is(true));
@ -272,7 +272,7 @@ public class DnRoleMapperTests extends ESTestCase {
// writing in utf_16 should cause a parsing error as we try to read the file in utf_8
Files.write(file, Collections.singletonList("aldlfkjldjdflkjd"), StandardCharsets.UTF_16);
Logger logger = CapturingLogger.newCapturingLogger(Level.INFO, null);
Map<DN, Set<String>> mappings = DnRoleMapper.parseFileLenient(file, logger, "_type", "_name");
Map<String, List<String>> mappings = DnRoleMapper.parseFileLenient(file, logger, "_type", "_name");
assertThat(mappings, notNullValue());
assertThat(mappings.isEmpty(), is(true));
List<String> events = CapturingLogger.output(logger.getName(), Level.ERROR);