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

Use Java 9 FilePermission model (#26302) 

This commit makes the security code aware of the Java 9 FilePermission changes (see #21534) and allows us to remove the `jdk.io.permissionsUseCanonicalPath` system property.
This commit is contained in:
Yannick Welsch 2017-08-22 11:22:00 +09:30 committed by GitHub
parent bdefcbdcd6
commit 3d8feff66e
9 changed files with 146 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy
View File

@ -549,8 +549,6 @@ class BuildPlugin implements Plugin<Project> {
systemProperty 'tests.artifact', project.name
systemProperty 'tests.task', path
systemProperty 'tests.security.manager', 'true'
// Breaking change in JDK-9, revert to JDK-8 behavior for now, see https://github.com/elastic/elasticsearch/issues/21534
systemProperty 'jdk.io.permissionsUseCanonicalPath', 'true'
systemProperty 'jna.nosys', 'true'
// default test sysprop values
systemProperty 'tests.ifNoTests', 'fail'

3
buildSrc/src/main/resources/forbidden/jdk-signatures.txt
View File

@ -107,3 +107,6 @@ java.util.Collections#EMPTY_MAP
java.util.Collections#EMPTY_SET
java.util.Collections#shuffle(java.util.List) @ Use java.util.Collections#shuffle(java.util.List, java.util.Random) with a reproducible source of randomness
@defaultMessage Avoid creating FilePermission objects directly, but use FilePermissionUtils instead
java.io.FilePermission#<init>(java.lang.String,java.lang.String)

86
core/src/main/java/org/elasticsearch/bootstrap/FilePermissionUtils.java Normal file
View File

@ -0,0 +1,86 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch 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.elasticsearch.bootstrap;
import org.elasticsearch.common.SuppressForbidden;
import java.io.FilePermission;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.Permissions;
public class FilePermissionUtils {
/** no instantiation */
private FilePermissionUtils() {}
private static final boolean VERSION_IS_AT_LEAST_JAVA_9 = JavaVersion.current().compareTo(JavaVersion.parse("9")) >= 0;
/**
* Add access to single file path
* @param policy current policy to add permissions to
* @param path the path itself
* @param permissions set of file permissions to grant to the path
*/
@SuppressForbidden(reason = "only place where creating Java-9 compatible FilePermission objects is possible")
public static void addSingleFilePath(Permissions policy, Path path, String permissions) throws IOException {
policy.add(new FilePermission(path.toString(), permissions));
if (VERSION_IS_AT_LEAST_JAVA_9 && Files.exists(path)) {
// Java 9 FilePermission model requires this due to the removal of pathname canonicalization,
// see also https://github.com/elastic/elasticsearch/issues/21534
Path realPath = path.toRealPath();
if (path.toString().equals(realPath.toString()) == false) {
policy.add(new FilePermission(realPath.toString(), permissions));
}
}
}
/**
* Add access to path (and all files underneath it); this also creates the directory if it does not exist.
*
* @param policy current policy to add permissions to
* @param configurationName the configuration name associated with the path (for error messages only)
* @param path the path itself
* @param permissions set of file permissions to grant to the path
*/
@SuppressForbidden(reason = "only place where creating Java-9 compatible FilePermission objects is possible")
public static void addDirectoryPath(Permissions policy, String configurationName, Path path, String permissions) throws IOException {
// paths may not exist yet, this also checks accessibility
try {
Security.ensureDirectoryExists(path);
} catch (IOException e) {
throw new IllegalStateException("Unable to access '" + configurationName + "' (" + path + ")", e);
}
// add each path twice: once for itself, again for files underneath it
policy.add(new FilePermission(path.toString(), permissions));
policy.add(new FilePermission(path.toString() + path.getFileSystem().getSeparator() + "-", permissions));
if (VERSION_IS_AT_LEAST_JAVA_9) {
// Java 9 FilePermission model requires this due to the removal of pathname canonicalization,
// see also https://github.com/elastic/elasticsearch/issues/21534
Path realPath = path.toRealPath();
if (path.toString().equals(realPath.toString()) == false) {
policy.add(new FilePermission(realPath.toString(), permissions));
policy.add(new FilePermission(realPath.toString() + realPath.getFileSystem().getSeparator() + "-", permissions));
}
}
}
}

57
core/src/main/java/org/elasticsearch/bootstrap/Security.java
View File

@ -23,14 +23,12 @@ import org.elasticsearch.SecureSM;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.network.NetworkModule;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.http.HttpTransportSettings;
import org.elasticsearch.plugins.PluginInfo;
import org.elasticsearch.transport.TcpTransport;
import java.io.FilePermission;
import java.io.IOException;
import java.net.SocketPermission;
import java.net.URISyntaxException;
@ -52,6 +50,9 @@ import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
import static org.elasticsearch.bootstrap.FilePermissionUtils.addDirectoryPath;
import static org.elasticsearch.bootstrap.FilePermissionUtils.addSingleFilePath;
/**
* Initializes SecurityManager with necessary permissions.
* <br>
@ -240,10 +241,10 @@ final class Security {
throw new RuntimeException(e);
}
// resource itself
policy.add(new FilePermission(path.toString(), "read,readlink"));
// classes underneath
if (Files.isDirectory(path)) {
policy.add(new FilePermission(path.toString() + path.getFileSystem().getSeparator() + "-", "read,readlink"));
addDirectoryPath(policy, "class.path", path, "read,readlink");
} else {
addSingleFilePath(policy, path, "read,readlink");
}
}
}
@ -251,22 +252,23 @@ final class Security {
/**
* Adds access to all configurable paths.
*/
static void addFilePermissions(Permissions policy, Environment environment) {
static void addFilePermissions(Permissions policy, Environment environment) throws IOException {
// read-only dirs
addPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.binFile(), "read,readlink");
addPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.libFile(), "read,readlink");
addPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.modulesFile(), "read,readlink");
addPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.pluginsFile(), "read,readlink");
addPath(policy, "path.conf'", environment.configFile(), "read,readlink");
addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.binFile(), "read,readlink");
addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.libFile(), "read,readlink");
addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.modulesFile(), "read,readlink");
addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.pluginsFile(), "read,readlink");
addDirectoryPath(policy, "path.conf'", environment.configFile(), "read,readlink");
// read-write dirs
addPath(policy, "java.io.tmpdir", environment.tmpFile(), "read,readlink,write,delete");
addPath(policy, Environment.PATH_LOGS_SETTING.getKey(), environment.logsFile(), "read,readlink,write,delete");
addDirectoryPath(policy, "java.io.tmpdir", environment.tmpFile(), "read,readlink,write,delete");
addDirectoryPath(policy, Environment.PATH_LOGS_SETTING.getKey(), environment.logsFile(), "read,readlink,write,delete");
if (environment.sharedDataFile() != null) {
addPath(policy, Environment.PATH_SHARED_DATA_SETTING.getKey(), environment.sharedDataFile(), "read,readlink,write,delete");
addDirectoryPath(policy, Environment.PATH_SHARED_DATA_SETTING.getKey(), environment.sharedDataFile(),
"read,readlink,write,delete");
}
final Set<Path> dataFilesPaths = new HashSet<>();
for (Path path : environment.dataFiles()) {
addPath(policy, Environment.PATH_DATA_SETTING.getKey(), path, "read,readlink,write,delete");
addDirectoryPath(policy, Environment.PATH_DATA_SETTING.getKey(), path, "read,readlink,write,delete");
/*
* We have to do this after adding the path because a side effect of that is that the directory is created; the Path#toRealPath
* invocation will fail if the directory does not already exist. We use Path#toRealPath to follow symlinks and handle issues
@ -282,11 +284,11 @@ final class Security {
}
}
for (Path path : environment.repoFiles()) {
addPath(policy, Environment.PATH_REPO_SETTING.getKey(), path, "read,readlink,write,delete");
addDirectoryPath(policy, Environment.PATH_REPO_SETTING.getKey(), path, "read,readlink,write,delete");
}
if (environment.pidFile() != null) {
// we just need permission to remove the file if its elsewhere.
policy.add(new FilePermission(environment.pidFile().toString(), "delete"));
addSingleFilePath(policy, environment.pidFile(), "delete");
}
}
@ -367,27 +369,6 @@ final class Security {
policy.add(new SocketPermission("*:" + portRange, "listen,resolve"));
}
/**
* Add access to path (and all files underneath it); this also creates the directory if it does not exist.
*
* @param policy current policy to add permissions to
* @param configurationName the configuration name associated with the path (for error messages only)
* @param path the path itself
* @param permissions set of file permissions to grant to the path
*/
static void addPath(Permissions policy, String configurationName, Path path, String permissions) {
// paths may not exist yet, this also checks accessibility
try {
ensureDirectoryExists(path);
} catch (IOException e) {
throw new IllegalStateException("Unable to access '" + configurationName + "' (" + path + ")", e);
}
// add each path twice: once for itself, again for files underneath it
policy.add(new FilePermission(path.toString(), permissions));
policy.add(new FilePermission(path.toString() + path.getFileSystem().getSeparator() + "-", permissions));
}
/**
* Ensures configured directory {@code path} exists.
* @throws IOException if {@code path} exists, but is not a directory, not accessible, or broken symbolic link.

3
distribution/src/main/resources/config/jvm.options
View File

@ -63,9 +63,6 @@
# exceptions because stack traces are important for debugging
-XX:-OmitStackTraceInFastThrow
# use old-style file permissions on JDK9
-Djdk.io.permissionsUseCanonicalPath=true
# flags to configure Netty
-Dio.netty.noUnsafe=true
-Dio.netty.noKeySetOptimization=true

46
plugins/ingest-attachment/src/main/java/org/elasticsearch/ingest/attachment/TikaImpl.java
View File

@ -27,17 +27,19 @@ import org.apache.tika.parser.AutoDetectParser;
import org.apache.tika.parser.Parser;
import org.apache.tika.parser.ParserDecorator;
import org.elasticsearch.SpecialPermission;
import org.elasticsearch.bootstrap.FilePermissionUtils;
import org.elasticsearch.bootstrap.JarHell;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.io.PathUtils;
import java.io.ByteArrayInputStream;
import java.io.FilePermission;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.lang.reflect.ReflectPermission;
import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.AccessControlContext;
import java.security.AccessController;
@ -119,27 +121,32 @@ final class TikaImpl {
// compute some minimal permissions for parsers. they only get r/w access to the java temp directory,
// the ability to load some resources from JARs, and read sysprops
@SuppressForbidden(reason = "adds access to tmp directory")
static PermissionCollection getRestrictedPermissions() {
Permissions perms = new Permissions();
// property/env access needed for parsing
perms.add(new PropertyPermission("*", "read"));
perms.add(new RuntimePermission("getenv.TIKA_CONFIG"));
// add permissions for resource access:
// classpath
addReadPermissions(perms, JarHell.parseClassPath());
// plugin jars
if (TikaImpl.class.getClassLoader() instanceof URLClassLoader) {
URL[] urls = ((URLClassLoader)TikaImpl.class.getClassLoader()).getURLs();
Set<URL> set = new LinkedHashSet<>(Arrays.asList(urls));
if (set.size() != urls.length) {
throw new AssertionError("duplicate jars: " + Arrays.toString(urls));
try {
// add permissions for resource access:
// classpath
addReadPermissions(perms, JarHell.parseClassPath());
// plugin jars
if (TikaImpl.class.getClassLoader() instanceof URLClassLoader) {
URL[] urls = ((URLClassLoader)TikaImpl.class.getClassLoader()).getURLs();
Set<URL> set = new LinkedHashSet<>(Arrays.asList(urls));
if (set.size() != urls.length) {
throw new AssertionError("duplicate jars: " + Arrays.toString(urls));
}
addReadPermissions(perms, set);
}
addReadPermissions(perms, set);
// jvm's java.io.tmpdir (needs read/write)
FilePermissionUtils.addDirectoryPath(perms, "java.io.tmpdir",
PathUtils.get(System.getProperty("java.io.tmpdir")), "read,readlink,write,delete");
} catch (IOException e) {
throw new UncheckedIOException(e);
}
// jvm's java.io.tmpdir (needs read/write)
perms.add(new FilePermission(System.getProperty("java.io.tmpdir") + System.getProperty("file.separator") + "-",
"read,readlink,write,delete"));
// current hacks needed for POI/PDFbox issues:
perms.add(new SecurityPermission("putProviderProperty.BC"));
perms.add(new SecurityPermission("insertProvider"));
@ -152,14 +159,15 @@ final class TikaImpl {
// add resources to (what is typically) a jar, but might not be (e.g. in tests/IDE)
@SuppressForbidden(reason = "adds access to jar resources")
static void addReadPermissions(Permissions perms, Set<URL> resources) {
static void addReadPermissions(Permissions perms, Set<URL> resources) throws IOException {
try {
for (URL url : resources) {
Path path = PathUtils.get(url.toURI());
// resource itself
perms.add(new FilePermission(path.toString(), "read,readlink"));
// classes underneath
perms.add(new FilePermission(path.toString() + System.getProperty("file.separator") + "-", "read,readlink"));
if (Files.isDirectory(path)) {
FilePermissionUtils.addDirectoryPath(perms, "class.path", path, "read,readlink");
} else {
FilePermissionUtils.addSingleFilePath(perms, path, "read,readlink");
}
}
} catch (URISyntaxException bogus) {
throw new RuntimeException(bogus);

3
qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/ESPolicyUnitTests.java
View File

@ -19,6 +19,7 @@
package org.elasticsearch.bootstrap;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.test.ESTestCase;
import java.io.FilePermission;
@ -44,6 +45,7 @@ public class ESPolicyUnitTests extends ESTestCase {
* even though ProtectionDomain's ctor javadocs might make you think
* that the policy won't be consulted.
*/
@SuppressForbidden(reason = "to create FilePermission object")
public void testNullCodeSource() throws Exception {
assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
// create a policy with AllPermission
@ -61,6 +63,7 @@ public class ESPolicyUnitTests extends ESTestCase {
* <p>
* its unclear when/if this happens, see https://bugs.openjdk.java.net/browse/JDK-8129972
*/
@SuppressForbidden(reason = "to create FilePermission object")
public void testNullLocation() throws Exception {
assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
PermissionCollection noPermissions = new Permissions();

6
qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilSecurityTests.java
View File

@ -22,7 +22,6 @@ package org.elasticsearch.bootstrap;
import org.apache.lucene.util.Constants;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
@ -73,6 +72,7 @@ public class EvilSecurityTests extends ESTestCase {
/** test generated permissions for all configured paths */
@SuppressWarnings("deprecation") // needs to check settings for deprecated path
@SuppressForbidden(reason = "to create FilePermission object")
public void testEnvironmentPaths() throws Exception {
Path path = createTempDir();
// make a fake ES home and ensure we only grant permissions to that.
@ -217,7 +217,7 @@ public class EvilSecurityTests extends ESTestCase {
assumeNoException("test cannot create symbolic links with security manager enabled", e);
}
Permissions permissions = new Permissions();
Security.addPath(permissions, "testing", link, "read");
FilePermissionUtils.addDirectoryPath(permissions, "testing", link, "read");
assertExactPermissions(new FilePermission(link.toString(), "read"), permissions);
assertExactPermissions(new FilePermission(link.resolve("foo").toString(), "read"), permissions);
assertExactPermissions(new FilePermission(target.toString(), "read"), permissions);
@ -227,6 +227,7 @@ public class EvilSecurityTests extends ESTestCase {
/**
* checks exact file permissions, meaning those and only those for that path.
*/
@SuppressForbidden(reason = "to create FilePermission object")
static void assertExactPermissions(FilePermission expected, PermissionCollection actual) {
String target = expected.getName(); // see javadocs
Set<String> permissionSet = asSet(expected.getActions().split(","));
@ -246,6 +247,7 @@ public class EvilSecurityTests extends ESTestCase {
/**
* checks that this path has no permissions
*/
@SuppressForbidden(reason = "to create FilePermission object")
static void assertNoPermissions(Path path, PermissionCollection actual) {
String target = path.toString();
assertFalse(actual.implies(new FilePermission(target, "read")));

8
test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java
View File

@ -102,19 +102,19 @@ public class BootstrapForTesting {
Permissions perms = new Permissions();
Security.addClasspathPermissions(perms);
// java.io.tmpdir
Security.addPath(perms, "java.io.tmpdir", javaTmpDir, "read,readlink,write,delete");
FilePermissionUtils.addDirectoryPath(perms, "java.io.tmpdir", javaTmpDir, "read,readlink,write,delete");
// custom test config file
if (Strings.hasLength(System.getProperty("tests.config"))) {
perms.add(new FilePermission(System.getProperty("tests.config"), "read,readlink"));
FilePermissionUtils.addSingleFilePath(perms, PathUtils.get(System.getProperty("tests.config")), "read,readlink");
}
// jacoco coverage output file
final boolean testsCoverage =
Booleans.parseBoolean(System.getProperty("tests.coverage", "false"));
if (testsCoverage) {
Path coverageDir = PathUtils.get(System.getProperty("tests.coverage.dir"));
perms.add(new FilePermission(coverageDir.resolve("jacoco.exec").toString(), "read,write"));
FilePermissionUtils.addSingleFilePath(perms, coverageDir.resolve("jacoco.exec"), "read,write");
// in case we get fancy and use the -integration goals later:
perms.add(new FilePermission(coverageDir.resolve("jacoco-it.exec").toString(), "read,write"));
FilePermissionUtils.addSingleFilePath(perms, coverageDir.resolve("jacoco-it.exec"), "read,write");
}
// intellij hack: intellij test runner wants setIO and will
// screw up all test logging without it!