From 64adc9e42c888295a6ab0b23a6890fe4f25e52ff Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 6 Sep 2017 09:30:13 -0700 Subject: [PATCH 1/5] Internal: Change policy file system properties to not contain jar versions Security manager policy files contains grants for specific codebases, where a codebase is a jar file. We use a system property containing the name of the jar file to resolve the jar file location when parsing the policy file. However, this means the version of the jars must be modified when versions of dependencies change. This is particularly messy for elasticsearch, where we now have a dependency on the rest client, and need to support both a snapshot version for testing and non snapshot for release. This commit removes the version portion of the jar file name. The implications are we will no longer need to edit policy files when dependencies change, and do not need any special handling for snapshot vs release builds and their internal policy files in the core elasticsearch jar. --- .../org/elasticsearch/bootstrap/Security.java | 16 +++++++++------- .../org/elasticsearch/bootstrap/security.policy | 6 +++--- .../bootstrap/test-framework.policy | 16 ++++++++-------- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Security.java b/core/src/main/java/org/elasticsearch/bootstrap/Security.java index e5c326b8ee174..347ef6ecfb5a9 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Security.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Security.java @@ -43,12 +43,15 @@ import java.security.Permissions; import java.security.Policy; import java.security.URIParameter; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Pattern; import static org.elasticsearch.bootstrap.FilePermissionUtils.addDirectoryPath; import static org.elasticsearch.bootstrap.FilePermissionUtils.addSingleFilePath; @@ -191,13 +194,16 @@ static Map getPluginPermissions(Environment environment) throws I @SuppressForbidden(reason = "accesses fully qualified URLs to configure security") static Policy readPolicy(URL policyFile, Set codebases) { try { + List shortNames = new ArrayList<>(); try { // set codebase properties for (URL url : codebases) { - String shortName = PathUtils.get(url.toURI()).getFileName().toString(); - if (shortName.endsWith(".jar") == false) { + String fileName = PathUtils.get(url.toURI()).getFileName().toString(); + if (fileName.endsWith(".jar") == false) { continue; // tests :( } + String shortName = fileName.replaceFirst("-\\d+\\.\\d+.*\\.jar", ".jar"); + shortNames.add(shortName); String previous = System.setProperty("codebase." + shortName, url.toString()); if (previous != null) { throw new IllegalStateException("codebase property already set: " + shortName + "->" + previous); @@ -206,11 +212,7 @@ static Policy readPolicy(URL policyFile, Set codebases) { return Policy.getInstance("JavaPolicy", new URIParameter(policyFile.toURI())); } finally { // clear codebase properties - for (URL url : codebases) { - String shortName = PathUtils.get(url.toURI()).getFileName().toString(); - if (shortName.endsWith(".jar") == false) { - continue; // tests :( - } + for (String shortName : shortNames) { System.clearProperty("codebase." + shortName); } } diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy index b1c2aab11aeca..a61fa22b4d9ae 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy @@ -24,14 +24,14 @@ //// SecurityManager impl: //// Must have all permissions to properly perform access checks -grant codeBase "${codebase.securesm-1.1.jar}" { +grant codeBase "${codebase.securesm.jar}" { permission java.security.AllPermission; }; //// Very special jar permissions: //// These are dangerous permissions that we don't want to grant to everything. -grant codeBase "${codebase.lucene-core-7.0.0-snapshot-d94a5f0.jar}" { +grant codeBase "${codebase.lucene-core.jar}" { // needed to allow MMapDirectory's "unmap hack" (die unmap hack, die) // java 8 package permission java.lang.RuntimePermission "accessClassInPackage.sun.misc"; @@ -42,7 +42,7 @@ grant codeBase "${codebase.lucene-core-7.0.0-snapshot-d94a5f0.jar}" { permission java.lang.RuntimePermission "accessDeclaredMembers"; }; -grant codeBase "${codebase.lucene-misc-7.0.0-snapshot-d94a5f0.jar}" { +grant codeBase "${codebase.lucene-misc.jar}" { // needed to allow shard shrinking to use hard-links if possible via lucenes HardlinkCopyDirectoryWrapper permission java.nio.file.LinkPermission "hard"; }; diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy b/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy index 79cb42214ddd3..9029faed54094 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy @@ -21,7 +21,7 @@ //// These are mock objects and test management that we allow test framework libs //// to provide on our behalf. But tests themselves cannot do this stuff! -grant codeBase "${codebase.securemock-1.2.jar}" { +grant codeBase "${codebase.securemock.jar}" { // needed to access ReflectionFactory (see below) permission java.lang.RuntimePermission "accessClassInPackage.sun.reflect"; // needed for reflection in ibm jdk @@ -33,7 +33,7 @@ grant codeBase "${codebase.securemock-1.2.jar}" { permission java.lang.reflect.ReflectPermission "suppressAccessChecks"; }; -grant codeBase "${codebase.lucene-test-framework-7.0.0-snapshot-d94a5f0.jar}" { +grant codeBase "${codebase.lucene-test-framework.jar}" { // needed by RamUsageTester permission java.lang.reflect.ReflectPermission "suppressAccessChecks"; // needed for testing hardlinks in StoreRecoveryTests since we install MockFS @@ -42,7 +42,7 @@ grant codeBase "${codebase.lucene-test-framework-7.0.0-snapshot-d94a5f0.jar}" { permission java.lang.RuntimePermission "accessDeclaredMembers"; }; -grant codeBase "${codebase.randomizedtesting-runner-2.5.2.jar}" { +grant codeBase "${codebase.randomizedtesting-runner.jar}" { // optionally needed for access to private test methods (e.g. beforeClass) permission java.lang.reflect.ReflectPermission "suppressAccessChecks"; // needed to fail tests on uncaught exceptions from other threads @@ -53,29 +53,29 @@ grant codeBase "${codebase.randomizedtesting-runner-2.5.2.jar}" { permission java.lang.RuntimePermission "accessDeclaredMembers"; }; -grant codeBase "${codebase.junit-4.12.jar}" { +grant codeBase "${codebase.junit.jar}" { // needed for TestClass creation permission java.lang.RuntimePermission "accessDeclaredMembers"; }; -grant codeBase "${codebase.mocksocket-1.2.jar}" { +grant codeBase "${codebase.mocksocket.jar}" { // mocksocket makes and accepts socket connections permission java.net.SocketPermission "*", "accept,connect"; }; -grant codeBase "${codebase.elasticsearch-rest-client-7.0.0-alpha1-SNAPSHOT.jar}" { +grant codeBase "${codebase.elasticsearch-rest-client.jar}" { // rest makes socket connections for rest tests permission java.net.SocketPermission "*", "connect"; // rest client uses system properties which gets the default proxy permission java.net.NetPermission "getProxySelector"; }; -grant codeBase "${codebase.httpcore-nio-4.4.5.jar}" { +grant codeBase "${codebase.httpcore-nio.jar}" { // httpcore makes socket connections for rest tests permission java.net.SocketPermission "*", "connect"; }; -grant codeBase "${codebase.httpasyncclient-4.1.2.jar}" { +grant codeBase "${codebase.httpasyncclient.jar}" { // httpasyncclient makes socket connections for rest tests permission java.net.SocketPermission "*", "connect"; // rest client uses system properties which gets the default proxy From 68a84d9002c70c475aac35fe370e821b1e776ce0 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 6 Sep 2017 14:57:46 -0700 Subject: [PATCH 2/5] alternative --- .../org/elasticsearch/bootstrap/Security.java | 28 +++++++++++++------ .../elasticsearch/bootstrap/security.policy | 6 ++-- .../bootstrap/test-framework.policy | 16 +++++------ .../plugin-metadata/plugin-security.policy | 2 +- 4 files changed, 31 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Security.java b/core/src/main/java/org/elasticsearch/bootstrap/Security.java index 347ef6ecfb5a9..314f5bdcf3e78 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Security.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Security.java @@ -19,7 +19,9 @@ package org.elasticsearch.bootstrap; +import org.elasticsearch.Build; import org.elasticsearch.SecureSM; +import org.elasticsearch.Version; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.network.NetworkModule; @@ -51,7 +53,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.regex.Pattern; import static org.elasticsearch.bootstrap.FilePermissionUtils.addDirectoryPath; import static org.elasticsearch.bootstrap.FilePermissionUtils.addSingleFilePath; @@ -194,17 +195,26 @@ static Map getPluginPermissions(Environment environment) throws I @SuppressForbidden(reason = "accesses fully qualified URLs to configure security") static Policy readPolicy(URL policyFile, Set codebases) { try { - List shortNames = new ArrayList<>(); + List propertiesSet = new ArrayList<>(); try { // set codebase properties for (URL url : codebases) { - String fileName = PathUtils.get(url.toURI()).getFileName().toString(); - if (fileName.endsWith(".jar") == false) { + String shortName = PathUtils.get(url.toURI()).getFileName().toString(); + if (shortName.endsWith(".jar") == false) { continue; // tests :( } - String shortName = fileName.replaceFirst("-\\d+\\.\\d+.*\\.jar", ".jar"); - shortNames.add(shortName); - String previous = System.setProperty("codebase." + shortName, url.toString()); + if (shortName.startsWith("elasticsearch-rest-client")) { + final String esVersion = Version.CURRENT + (Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : ""); + System.setProperty("es.version", esVersion); + propertiesSet.add("es.version"); + final String urlAsString = url.toString(); + final int index = urlAsString.indexOf("-" + System.getProperty("es.version") + ".jar"); + assert index >= 0; + shortName = urlAsString.substring(0, index); + } + String property = "codebase." + shortName; + propertiesSet.add(property); + String previous = System.setProperty(property, url.toString()); if (previous != null) { throw new IllegalStateException("codebase property already set: " + shortName + "->" + previous); } @@ -212,8 +222,8 @@ static Policy readPolicy(URL policyFile, Set codebases) { return Policy.getInstance("JavaPolicy", new URIParameter(policyFile.toURI())); } finally { // clear codebase properties - for (String shortName : shortNames) { - System.clearProperty("codebase." + shortName); + for (String property : propertiesSet) { + System.clearProperty(property); } } } catch (NoSuchAlgorithmException | URISyntaxException e) { diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy index a61fa22b4d9ae..b1c2aab11aeca 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy @@ -24,14 +24,14 @@ //// SecurityManager impl: //// Must have all permissions to properly perform access checks -grant codeBase "${codebase.securesm.jar}" { +grant codeBase "${codebase.securesm-1.1.jar}" { permission java.security.AllPermission; }; //// Very special jar permissions: //// These are dangerous permissions that we don't want to grant to everything. -grant codeBase "${codebase.lucene-core.jar}" { +grant codeBase "${codebase.lucene-core-7.0.0-snapshot-d94a5f0.jar}" { // needed to allow MMapDirectory's "unmap hack" (die unmap hack, die) // java 8 package permission java.lang.RuntimePermission "accessClassInPackage.sun.misc"; @@ -42,7 +42,7 @@ grant codeBase "${codebase.lucene-core.jar}" { permission java.lang.RuntimePermission "accessDeclaredMembers"; }; -grant codeBase "${codebase.lucene-misc.jar}" { +grant codeBase "${codebase.lucene-misc-7.0.0-snapshot-d94a5f0.jar}" { // needed to allow shard shrinking to use hard-links if possible via lucenes HardlinkCopyDirectoryWrapper permission java.nio.file.LinkPermission "hard"; }; diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy b/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy index 9029faed54094..840f182adab73 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy @@ -21,7 +21,7 @@ //// These are mock objects and test management that we allow test framework libs //// to provide on our behalf. But tests themselves cannot do this stuff! -grant codeBase "${codebase.securemock.jar}" { +grant codeBase "${codebase.securemock-1.2.jar}" { // needed to access ReflectionFactory (see below) permission java.lang.RuntimePermission "accessClassInPackage.sun.reflect"; // needed for reflection in ibm jdk @@ -33,7 +33,7 @@ grant codeBase "${codebase.securemock.jar}" { permission java.lang.reflect.ReflectPermission "suppressAccessChecks"; }; -grant codeBase "${codebase.lucene-test-framework.jar}" { +grant codeBase "${codebase.lucene-test-framework-7.0.0-snapshot-d94a5f0.jar}" { // needed by RamUsageTester permission java.lang.reflect.ReflectPermission "suppressAccessChecks"; // needed for testing hardlinks in StoreRecoveryTests since we install MockFS @@ -42,7 +42,7 @@ grant codeBase "${codebase.lucene-test-framework.jar}" { permission java.lang.RuntimePermission "accessDeclaredMembers"; }; -grant codeBase "${codebase.randomizedtesting-runner.jar}" { +grant codeBase "${codebase.randomizedtesting-runner-2.5.2.jar}" { // optionally needed for access to private test methods (e.g. beforeClass) permission java.lang.reflect.ReflectPermission "suppressAccessChecks"; // needed to fail tests on uncaught exceptions from other threads @@ -53,29 +53,29 @@ grant codeBase "${codebase.randomizedtesting-runner.jar}" { permission java.lang.RuntimePermission "accessDeclaredMembers"; }; -grant codeBase "${codebase.junit.jar}" { +grant codeBase "${codebase.junit-4.12.jar}" { // needed for TestClass creation permission java.lang.RuntimePermission "accessDeclaredMembers"; }; -grant codeBase "${codebase.mocksocket.jar}" { +grant codeBase "${codebase.mocksocket-1.2.jar}" { // mocksocket makes and accepts socket connections permission java.net.SocketPermission "*", "accept,connect"; }; -grant codeBase "${codebase.elasticsearch-rest-client.jar}" { +grant codeBase "${codebase.elasticsearch-rest-client}-${es.version}.jar" { // rest makes socket connections for rest tests permission java.net.SocketPermission "*", "connect"; // rest client uses system properties which gets the default proxy permission java.net.NetPermission "getProxySelector"; }; -grant codeBase "${codebase.httpcore-nio.jar}" { +grant codeBase "${codebase.httpcore-nio-4.4.5.jar}" { // httpcore makes socket connections for rest tests permission java.net.SocketPermission "*", "connect"; }; -grant codeBase "${codebase.httpasyncclient.jar}" { +grant codeBase "${codebase.httpasyncclient-4.1.2.jar}" { // httpasyncclient makes socket connections for rest tests permission java.net.SocketPermission "*", "connect"; // rest client uses system properties which gets the default proxy diff --git a/modules/reindex/src/main/plugin-metadata/plugin-security.policy b/modules/reindex/src/main/plugin-metadata/plugin-security.policy index 39c1d77277169..8f63708cbbe48 100644 --- a/modules/reindex/src/main/plugin-metadata/plugin-security.policy +++ b/modules/reindex/src/main/plugin-metadata/plugin-security.policy @@ -22,7 +22,7 @@ grant { permission java.net.SocketPermission "*", "connect"; }; -grant codeBase "${codebase.elasticsearch-rest-client-7.0.0-alpha1-SNAPSHOT.jar}" { +grant codeBase "${codebase.elasticsearch-rest-client}-${es.version}.jar" { // rest client uses system properties which gets the default proxy permission java.net.NetPermission "getProxySelector"; }; From 896c351bea63e86fcc8d66d8fc50a3c3fce3b458 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 6 Sep 2017 15:41:50 -0700 Subject: [PATCH 3/5] remove es.version --- .../java/org/elasticsearch/bootstrap/Security.java | 10 +++++----- .../org/elasticsearch/bootstrap/test-framework.policy | 2 +- .../src/main/plugin-metadata/plugin-security.policy | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Security.java b/core/src/main/java/org/elasticsearch/bootstrap/Security.java index 314f5bdcf3e78..67400b5d03802 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Security.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Security.java @@ -205,12 +205,11 @@ static Policy readPolicy(URL policyFile, Set codebases) { } if (shortName.startsWith("elasticsearch-rest-client")) { final String esVersion = Version.CURRENT + (Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : ""); - System.setProperty("es.version", esVersion); - propertiesSet.add("es.version"); - final String urlAsString = url.toString(); - final int index = urlAsString.indexOf("-" + System.getProperty("es.version") + ".jar"); + final int index = shortName.indexOf("-" + esVersion + ".jar"); assert index >= 0; - shortName = urlAsString.substring(0, index); + String restClientAlias = "codebase." + shortName.substring(0, index); + propertiesSet.add(restClientAlias); + System.setProperty(restClientAlias, url.toString()); } String property = "codebase." + shortName; propertiesSet.add(property); @@ -223,6 +222,7 @@ static Policy readPolicy(URL policyFile, Set codebases) { } finally { // clear codebase properties for (String property : propertiesSet) { + System.out.println("Unsetting " + property); System.clearProperty(property); } } diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy b/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy index 840f182adab73..5b94f28254e58 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy @@ -63,7 +63,7 @@ grant codeBase "${codebase.mocksocket-1.2.jar}" { permission java.net.SocketPermission "*", "accept,connect"; }; -grant codeBase "${codebase.elasticsearch-rest-client}-${es.version}.jar" { +grant codeBase "${codebase.elasticsearch-rest-client}" { // rest makes socket connections for rest tests permission java.net.SocketPermission "*", "connect"; // rest client uses system properties which gets the default proxy diff --git a/modules/reindex/src/main/plugin-metadata/plugin-security.policy b/modules/reindex/src/main/plugin-metadata/plugin-security.policy index 8f63708cbbe48..70fb51b845ce1 100644 --- a/modules/reindex/src/main/plugin-metadata/plugin-security.policy +++ b/modules/reindex/src/main/plugin-metadata/plugin-security.policy @@ -22,7 +22,7 @@ grant { permission java.net.SocketPermission "*", "connect"; }; -grant codeBase "${codebase.elasticsearch-rest-client}-${es.version}.jar" { +grant codeBase "${codebase.elasticsearch-rest-client}" { // rest client uses system properties which gets the default proxy permission java.net.NetPermission "getProxySelector"; }; From f04ad17485233f8452bcedca23d692d1da6abb80 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 6 Sep 2017 15:58:41 -0700 Subject: [PATCH 4/5] iter --- .../src/main/java/org/elasticsearch/bootstrap/Security.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Security.java b/core/src/main/java/org/elasticsearch/bootstrap/Security.java index 67400b5d03802..ed8df7f2e4b5d 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Security.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Security.java @@ -203,15 +203,15 @@ static Policy readPolicy(URL policyFile, Set codebases) { if (shortName.endsWith(".jar") == false) { continue; // tests :( } + String property = "codebase." + shortName; if (shortName.startsWith("elasticsearch-rest-client")) { final String esVersion = Version.CURRENT + (Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : ""); - final int index = shortName.indexOf("-" + esVersion + ".jar"); + final int index = property.indexOf("-" + esVersion + ".jar"); assert index >= 0; - String restClientAlias = "codebase." + shortName.substring(0, index); + String restClientAlias = property.substring(0, index); propertiesSet.add(restClientAlias); System.setProperty(restClientAlias, url.toString()); } - String property = "codebase." + shortName; propertiesSet.add(property); String previous = System.setProperty(property, url.toString()); if (previous != null) { From cebf67a971a75ae5f3712cbe4ddac9fb5241868f Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 6 Sep 2017 17:52:11 -0700 Subject: [PATCH 5/5] add comment and remove debug --- .../src/main/java/org/elasticsearch/bootstrap/Security.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Security.java b/core/src/main/java/org/elasticsearch/bootstrap/Security.java index ed8df7f2e4b5d..a1ce20a0e27c8 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Security.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Security.java @@ -205,6 +205,11 @@ static Policy readPolicy(URL policyFile, Set codebases) { } String property = "codebase." + shortName; if (shortName.startsWith("elasticsearch-rest-client")) { + // The rest client is currently the only example where we have an elasticsearch built artifact + // which needs special permissions in policy files when used. This temporary solution is to + // pass in an extra system property that omits the -version.jar suffix the other properties have. + // That allows the snapshots to reference snapshot builds of the client, and release builds to + // referenced release builds of the client, all with the same grant statements. final String esVersion = Version.CURRENT + (Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : ""); final int index = property.indexOf("-" + esVersion + ".jar"); assert index >= 0; @@ -222,7 +227,6 @@ static Policy readPolicy(URL policyFile, Set codebases) { } finally { // clear codebase properties for (String property : propertiesSet) { - System.out.println("Unsetting " + property); System.clearProperty(property); } }