Skip to content

Commit

Permalink
Mark 'users' as a secured file, and add a secured permission for LDAP…
Browse files Browse the repository at this point in the history
… role mappings (#108767)
  • Loading branch information
thecoop committed Jun 13, 2024
1 parent f5405a4 commit b36df9d
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 20 deletions.
36 changes: 27 additions & 9 deletions server/src/main/java/org/elasticsearch/watcher/FileWatcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,17 @@
import org.apache.logging.log4j.Logger;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.hash.MessageDigests;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.util.CollectionUtils;

import java.io.IOException;
import java.io.InputStream;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
import java.security.AccessControlException;
import java.util.Arrays;
import java.util.stream.StreamSupport;

/**
* File resources watcher
Expand Down Expand Up @@ -114,6 +116,22 @@ void onDirectoryDeleted() {}
void onFileDeleted() {}
}

protected boolean fileExists(Path path) {
return Files.exists(path);
}

protected BasicFileAttributes readAttributes(Path path) throws IOException {
return Files.readAttributes(path, BasicFileAttributes.class);
}

protected InputStream newInputStream(Path path) throws IOException {
return Files.newInputStream(path);
}

protected DirectoryStream<Path> listFiles(Path path) throws IOException {
return Files.newDirectoryStream(path);
}

private class FileObserver extends Observer {
private long length;
private long lastModified;
Expand All @@ -131,10 +149,10 @@ public void checkAndNotify() throws IOException {
long prevLastModified = lastModified;
byte[] prevDigest = digest;

exists = Files.exists(path);
exists = fileExists(path);
// TODO we might use the new NIO2 API to get real notification?
if (exists) {
BasicFileAttributes attributes = Files.readAttributes(path, BasicFileAttributes.class);
BasicFileAttributes attributes = readAttributes(path);
isDirectory = attributes.isDirectory();
if (isDirectory) {
length = 0;
Expand Down Expand Up @@ -202,7 +220,7 @@ public void checkAndNotify() throws IOException {
}

private byte[] calculateDigest() {
try (var in = Files.newInputStream(path)) {
try (var in = newInputStream(path)) {
return MessageDigests.digest(in, MessageDigests.md5());
} catch (IOException e) {
logger.warn(
Expand All @@ -215,9 +233,9 @@ private byte[] calculateDigest() {
}

private void init(boolean initial) throws IOException {
exists = Files.exists(path);
exists = fileExists(path);
if (exists) {
BasicFileAttributes attributes = Files.readAttributes(path, BasicFileAttributes.class);
BasicFileAttributes attributes = readAttributes(path);
isDirectory = attributes.isDirectory();
if (isDirectory) {
onDirectoryCreated(initial);
Expand Down Expand Up @@ -245,9 +263,9 @@ private Observer createChild(Path file, boolean initial) throws IOException {
}

private Path[] listFiles() throws IOException {
final Path[] files = FileSystemUtils.files(path);
Arrays.sort(files);
return files;
try (var dirs = FileWatcher.this.listFiles(path)) {
return StreamSupport.stream(dirs.spliterator(), false).sorted().toArray(Path[]::new);
}
}

private Observer[] listChildren(boolean initial) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.security;

import org.elasticsearch.watcher.FileWatcher;

import java.io.IOException;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
import java.security.PrivilegedAction;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;

import static java.security.AccessController.doPrivileged;

/**
* Extension of {@code FileWatcher} that does privileged calls to IO.
* <p>
* This class exists so that the calls into the IO methods get here first in the security stackwalk,
* enabling us to use doPrivileged to ensure we have access. If we don't do this, the code location
* that is doing the accessing is not the one that is granted the SecuredFileAccessPermission,
* so the check in ESPolicy fails.
*/
public class PrivilegedFileWatcher extends FileWatcher {

public PrivilegedFileWatcher(Path path) {
super(path);
}

@Override
protected boolean fileExists(Path path) {
return doPrivileged((PrivilegedAction<Boolean>) () -> Files.exists(path));
}

@Override
protected BasicFileAttributes readAttributes(Path path) throws IOException {
try {
return doPrivileged(
(PrivilegedExceptionAction<BasicFileAttributes>) () -> Files.readAttributes(path, BasicFileAttributes.class)
);
} catch (PrivilegedActionException e) {
throw new IOException(e);
}
}

@Override
protected DirectoryStream<Path> listFiles(Path path) throws IOException {
try {
return doPrivileged((PrivilegedExceptionAction<DirectoryStream<Path>>) () -> Files.newDirectoryStream(path));
} catch (PrivilegedActionException e) {
throw new IOException(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.logging.DeprecationCategory;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.network.NetworkModule;
import org.elasticsearch.common.network.NetworkService;
import org.elasticsearch.common.settings.ClusterSettings;
Expand Down Expand Up @@ -412,6 +414,9 @@
import java.io.Closeable;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.PrivilegedAction;
import java.security.Provider;
import java.time.Clock;
import java.util.ArrayList;
Expand All @@ -436,6 +441,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static java.security.AccessController.doPrivileged;
import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonList;
Expand Down Expand Up @@ -697,6 +703,29 @@ protected List<ReloadableSecurityComponent> getReloadableSecurityComponents() {
return this.reloadableComponents.get();
}

/*
* Copied from XPackPlugin.resolveConfigFile so we don't go to a different codesource
* and so fail the secured file permission check on the users file.
* If there's a secured permission granted on this file (which there should be),
* ES has already checked the file is actually in the config directory
*/
public static Path resolveSecuredConfigFile(Environment env, String file) {
Path config = env.configFile().resolve(file);
if (doPrivileged((PrivilegedAction<Boolean>) () -> Files.exists(config)) == false) {
Path legacyConfig = env.configFile().resolve("x-pack").resolve(file);
if (doPrivileged((PrivilegedAction<Boolean>) () -> Files.exists(legacyConfig))) {
DeprecationLogger.getLogger(XPackPlugin.class)
.warn(
DeprecationCategory.OTHER,
"config_file_path",
"Config file [" + file + "] is in a deprecated location. Move from " + legacyConfig + " to " + config
);
return legacyConfig;
}
}
return config;
}

@Override
public Collection<?> createComponents(PluginServices services) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import org.elasticsearch.env.Environment;
import org.elasticsearch.watcher.FileWatcher;
import org.elasticsearch.watcher.ResourceWatcherService;
import org.elasticsearch.xpack.core.XPackPlugin;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
Expand All @@ -25,20 +24,24 @@
import org.elasticsearch.xpack.core.security.support.Validation;
import org.elasticsearch.xpack.core.security.support.Validation.Users;
import org.elasticsearch.xpack.core.security.user.User;
import org.elasticsearch.xpack.security.PrivilegedFileWatcher;
import org.elasticsearch.xpack.security.Security;
import org.elasticsearch.xpack.security.support.FileReloadListener;
import org.elasticsearch.xpack.security.support.SecurityFiles;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.PrivilegedAction;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.CopyOnWriteArrayList;

import static java.security.AccessController.doPrivileged;
import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableMap;
import static org.elasticsearch.core.Strings.format;
Expand All @@ -58,9 +61,9 @@ public FileUserPasswdStore(RealmConfig config, ResourceWatcherService watcherSer
FileUserPasswdStore(RealmConfig config, ResourceWatcherService watcherService, Runnable listener) {
file = resolveFile(config.env());
settings = config.settings();
users = parseFileLenient(file, logger, settings);
users = doPrivileged((PrivilegedAction<Map<String, char[]>>) () -> parseFileLenient(file, logger, settings));
listeners = new CopyOnWriteArrayList<>(Collections.singletonList(listener));
FileWatcher watcher = new FileWatcher(file.getParent());
FileWatcher watcher = new PrivilegedFileWatcher(file.getParent());
watcher.addListener(new FileReloadListener(file, this::tryReload));
try {
watcherService.add(watcher, ResourceWatcherService.Frequency.HIGH);
Expand Down Expand Up @@ -93,7 +96,7 @@ public boolean userExists(String username) {
}

public static Path resolveFile(Environment env) {
return XPackPlugin.resolveConfigFile(env, "users");
return Security.resolveSecuredConfigFile(env, "users");
}

/**
Expand Down Expand Up @@ -179,7 +182,7 @@ void notifyRefresh() {

private void tryReload() {
final Map<String, char[]> previousUsers = users;
users = parseFileLenient(file, logger, settings);
users = doPrivileged((PrivilegedAction<Map<String, char[]>>) () -> parseFileLenient(file, logger, settings));

if (Maps.deepEquals(previousUsers, users) == false) {
logger.info("users file [{}] changed. updating users...", file.toAbsolutePath());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@
import org.elasticsearch.watcher.FileChangesListener;
import org.elasticsearch.watcher.FileWatcher;
import org.elasticsearch.watcher.ResourceWatcherService;
import org.elasticsearch.xpack.core.XPackPlugin;
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
import org.elasticsearch.xpack.core.security.authc.support.DnRoleMapperSettings;
import org.elasticsearch.xpack.security.PrivilegedFileWatcher;
import org.elasticsearch.xpack.security.Security;
import org.elasticsearch.xpack.security.authc.support.mapper.AbstractRoleMapperClearRealmCache;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.PrivilegedAction;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -36,6 +38,7 @@
import java.util.Set;
import java.util.stream.Collectors;

import static java.security.AccessController.doPrivileged;
import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableMap;
import static org.elasticsearch.core.Strings.format;
Expand All @@ -58,8 +61,10 @@ public DnRoleMapper(RealmConfig config, ResourceWatcherService watcherService) {
this.config = config;
useUnmappedGroupsAsRoles = config.getSetting(DnRoleMapperSettings.USE_UNMAPPED_GROUPS_AS_ROLES_SETTING);
file = resolveFile(config);
dnRoles = parseFileLenient(file, logger, config.type(), config.name());
FileWatcher watcher = new FileWatcher(file.getParent());
dnRoles = doPrivileged(
(PrivilegedAction<Map<String, List<String>>>) () -> parseFileLenient(file, logger, config.type(), config.name())
);
FileWatcher watcher = new PrivilegedFileWatcher(file.getParent());
watcher.addListener(new FileListener());
try {
watcherService.add(watcher, ResourceWatcherService.Frequency.HIGH);
Expand All @@ -70,7 +75,7 @@ public DnRoleMapper(RealmConfig config, ResourceWatcherService watcherService) {

public static Path resolveFile(RealmConfig realmConfig) {
String location = realmConfig.getSetting(DnRoleMapperSettings.ROLE_MAPPING_FILE_SETTING);
return XPackPlugin.resolveConfigFile(realmConfig.env(), location);
return Security.resolveSecuredConfigFile(realmConfig.env(), location);
}

/**
Expand Down Expand Up @@ -233,7 +238,9 @@ public void onFileDeleted(Path file) {
public void onFileChanged(Path file) {
if (file.equals(DnRoleMapper.this.file)) {
final Map<String, List<String>> previousDnRoles = dnRoles;
dnRoles = parseFileLenient(file, logger, config.type(), config.name());
dnRoles = doPrivileged(
(PrivilegedAction<Map<String, List<String>>>) () -> parseFileLenient(file, logger, config.type(), config.name())
);

if (previousDnRoles.equals(dnRoles) == false) {
logger.info(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import org.elasticsearch.xpack.core.security.authc.support.DnRoleMapperSettings;

import java.nio.file.Path;
import java.security.AccessController;
import java.security.PrivilegedAction;

/**
* A BootstrapCheck that {@link DnRoleMapper} files exist and are valid (valid YAML and valid DNs)
Expand All @@ -31,7 +33,15 @@ public class RoleMappingFileBootstrapCheck implements BootstrapCheck {
@Override
public BootstrapCheckResult check(BootstrapContext context) {
try {
DnRoleMapper.parseFile(path, LogManager.getLogger(getClass()), realmConfig.type(), realmConfig.name(), true);
AccessController.doPrivileged(
(PrivilegedAction<?>) () -> DnRoleMapper.parseFile(
path,
LogManager.getLogger(getClass()),
realmConfig.type(),
realmConfig.name(),
true
)
);
return BootstrapCheckResult.success();
} catch (Exception e) {
return BootstrapCheckResult.failure(e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
grant {
permission java.lang.RuntimePermission "setFactory";

// secure the users file from other things (current and legacy locations)
permission org.elasticsearch.SecuredConfigFileAccessPermission "users";
permission org.elasticsearch.SecuredConfigFileAccessPermission "x-pack/users";
// other security files specified by settings
permission org.elasticsearch.SecuredConfigFileSettingAccessPermission "xpack.security.authc.realms.ldap.*.files.role_mapping";

// needed for SAML
permission java.util.PropertyPermission "org.apache.xml.security.ignoreLineBreaks", "read,write";

Expand Down

0 comments on commit b36df9d

Please sign in to comment.