Merge pull request #10936 from rmuir/eight_point_three

simplify securitymanager init

src/main/java/org/elasticsearch/bootstrap/Security.java

@@ -19,16 +19,19 @@
 
 package org.elasticsearch.bootstrap;
 
-import com.google.common.io.ByteStreams;
-import org.apache.lucene.util.IOUtils;
 import org.apache.lucene.util.StringHelper;
 import org.elasticsearch.env.Environment;
 
 import java.io.*;
-import java.nio.charset.StandardCharsets;
+import java.net.URI;
 import java.nio.file.Files;
-import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
+import java.security.Permission;
+import java.security.PermissionCollection;
+import java.security.Permissions;
+import java.security.Policy;
+import java.security.ProtectionDomain;
+import java.security.URIParameter;
 
 /** 
  * Initializes securitymanager with necessary permissions.
@@ -45,70 +48,74 @@ class Security {
      * Initializes securitymanager for the environment
      * Can only happen once!
      */
-    static void configure(Environment environment) throws IOException {
-        // init lucene random seed. it will use /dev/urandom where available.
+    static void configure(Environment environment) throws Exception {
+        // init lucene random seed. it will use /dev/urandom where available:
         StringHelper.randomId();
-        InputStream config = Security.class.getResourceAsStream(POLICY_RESOURCE);
-        if (config == null) {
-            throw new NoSuchFileException(POLICY_RESOURCE);
-        }
-        Path newConfig = processTemplate(config, environment);
-        System.setProperty("java.security.policy", newConfig.toString());
-        System.setSecurityManager(new SecurityManager());
-        IOUtils.deleteFilesIgnoringExceptions(newConfig); // TODO: maybe log something if it fails?
-    }
-   
-    // package-private for testing
-    static Path processTemplate(InputStream template, Environment environment) throws IOException {
-        Path processed = Files.createTempFile(null, null);
-        try (OutputStream output = new BufferedOutputStream(Files.newOutputStream(processed))) {
-            // copy the template as-is.
-            try (InputStream in = new BufferedInputStream(template)) {
-                ByteStreams.copy(in, output);
-            }
 
-            //  all policy files are UTF-8:
-            //  https://docs.oracle.com/javase/7/docs/technotes/guides/security/PolicyFiles.html
-            try (Writer writer = new OutputStreamWriter(output, StandardCharsets.UTF_8)) {
-                writer.write(System.lineSeparator());
-                writer.write("grant {");
-                writer.write(System.lineSeparator());
+        // enable security policy: union of template and environment-based paths.
+        URI template = Security.class.getResource(POLICY_RESOURCE).toURI();
+        Policy.setPolicy(new ESPolicy(template, createPermissions(environment)));
 
-                // add permissions for all configured paths.
-                // TODO: improve test infra so we can reduce permissions where read/write
-                // is not really needed...
-                addPath(writer, environment.homeFile(), "read,readlink,write,delete");
-                addPath(writer, environment.configFile(), "read,readlink,write,delete");
-                addPath(writer, environment.logsFile(), "read,readlink,write,delete");
-                addPath(writer, environment.pluginsFile(), "read,readlink,write,delete");
-                for (Path path : environment.dataFiles()) {
-                    addPath(writer, path, "read,readlink,write,delete");
-                }
-                for (Path path : environment.dataWithClusterFiles()) {
-                    addPath(writer, path, "read,readlink,write,delete");
-                }
+        // enable security manager
+        System.setSecurityManager(new SecurityManager());
 
-                writer.write("};");
-                writer.write(System.lineSeparator());
-            }
+        // do some basic tests
+        selfTest();
+    }
+
+    /** returns dynamic Permissions to configured paths */
+    static Permissions createPermissions(Environment environment) throws IOException {
+        // TODO: improve test infra so we can reduce permissions where read/write
+        // is not really needed...
+        Permissions policy = new Permissions();
+        addPath(policy, environment.homeFile(), "read,readlink,write,delete");
+        addPath(policy, environment.configFile(), "read,readlink,write,delete");
+        addPath(policy, environment.logsFile(), "read,readlink,write,delete");
+        addPath(policy, environment.pluginsFile(), "read,readlink,write,delete");
+        for (Path path : environment.dataFiles()) {
+            addPath(policy, path, "read,readlink,write,delete");
+        }
+        for (Path path : environment.dataWithClusterFiles()) {
+            addPath(policy, path, "read,readlink,write,delete");
         }
-        return processed;
+
+        return policy;
     }
     
-    static void addPath(Writer writer, Path path, String permissions) throws IOException {
+    /** Add access to path (and all files underneath it */
+    static void addPath(Permissions policy, Path path, String permissions) throws IOException {
         // paths may not exist yet
         Files.createDirectories(path);
         // add each path twice: once for itself, again for files underneath it
-        writer.write("permission java.io.FilePermission \"" + encode(path) + "\", \"" + permissions + "\";");
-        writer.write(System.lineSeparator());
-        writer.write("permission java.io.FilePermission \"" + encode(path) + "${/}-\", \"" + permissions + "\";");
-        writer.write(System.lineSeparator());
+        policy.add(new FilePermission(path.toString(), permissions));
+        policy.add(new FilePermission(path.toString() + path.getFileSystem().getSeparator() + "-", permissions));
     }
-    
-    // Any backslashes in paths must be escaped, because it is the escape character when parsing.
-    // See "Note Regarding File Path Specifications on Windows Systems".
-    // https://docs.oracle.com/javase/7/docs/technotes/guides/security/PolicyFiles.html
-    static String encode(Path path) {
-        return path.toString().replace("\\", "\\\\");
+
+    /** Simple checks that everything is ok */
+    static void selfTest() {
+        // check we can manipulate temporary files
+        try {
+            Files.delete(Files.createTempFile(null, null));
+        } catch (IOException ignored) {
+            // potentially virus scanner
+        } catch (SecurityException problem) {
+            throw new SecurityException("Security misconfiguration: cannot access java.io.tmpdir", problem);
+        }
+    }
+
+    /** custom policy for union of static and dynamic permissions */
+    static class ESPolicy extends Policy {
+        final Policy template;
+        final PermissionCollection dynamic;
+
+        ESPolicy(URI template, PermissionCollection dynamic) throws Exception {
+            this.template = Policy.getInstance("JavaPolicy", new URIParameter(template));
+            this.dynamic = dynamic;
+        }
+
+        @Override
+        public boolean implies(ProtectionDomain domain, Permission permission) {
+            return template.implies(domain, permission) || dynamic.implies(permission);
+        }
     }
 }

src/main/resources/org/elasticsearch/bootstrap/security.policy

@@ -34,7 +34,7 @@ grant {
   // project base directory
   permission java.io.FilePermission "${project.basedir}${/}target${/}-", "read";
   // read permission for lib sigar
-  permission java.io.FilePermission "${project.basedir}${/}lib/sigar{/}-", "read";
+  permission java.io.FilePermission "${project.basedir}${/}lib${/}sigar${/}-", "read";
   // mvn custom ./m2/repository for dependency jars
   permission java.io.FilePermission "${m2.repository}${/}-", "read";
 
@@ -84,10 +84,6 @@ grant {
   // needed for natives calls
   permission java.lang.RuntimePermission "loadLibrary.*";
 
-  // needed for testing access rules etc
-  permission java.lang.RuntimePermission "createSecurityManager";
-  permission java.security.SecurityPermission "createPolicy.JavaPolicy";
-
   // reflection hacks:
   // needed for Striped64 (what is this doing), also enables unmap hack
   permission java.lang.RuntimePermission "accessClassInPackage.sun.misc";

src/test/java/org/elasticsearch/bootstrap/SecurityTests.java

@@ -24,12 +24,9 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.test.ElasticsearchTestCase;
 
-import java.io.ByteArrayInputStream;
 import java.io.FilePermission;
 import java.nio.file.Path;
-import java.security.Policy;
-import java.security.ProtectionDomain;
-import java.security.URIParameter;
+import java.security.Permissions;
 
 public class SecurityTests extends ElasticsearchTestCase {
     
@@ -42,17 +39,15 @@ public class SecurityTests extends ElasticsearchTestCase {
         settingsBuilder.put("path.home", esHome.toString());
         Settings settings = settingsBuilder.build();
 
-        Environment environment = new Environment(settings);        
-        Path policyFile = Security.processTemplate(new ByteArrayInputStream(new byte[0]), environment);
+        Environment environment = new Environment(settings);
+        Permissions permissions = Security.createPermissions(environment);
       
-        ProtectionDomain domain = getClass().getProtectionDomain();
-        Policy policy = Policy.getInstance("JavaPolicy", new URIParameter(policyFile.toUri()));
         // the fake es home
-        assertTrue(policy.implies(domain, new FilePermission(esHome.toString(), "read")));
+        assertTrue(permissions.implies(new FilePermission(esHome.toString(), "read")));
         // its parent
-        assertFalse(policy.implies(domain, new FilePermission(path.toString(), "read")));
+        assertFalse(permissions.implies(new FilePermission(path.toString(), "read")));
         // some other sibling
-        assertFalse(policy.implies(domain, new FilePermission(path.resolve("other").toString(), "read")));
+        assertFalse(permissions.implies(new FilePermission(path.resolve("other").toString(), "read")));
     }
 
     /** test generated permissions for all configured paths */
@@ -67,29 +62,26 @@ public class SecurityTests extends ElasticsearchTestCase {
         settingsBuilder.put("path.logs", path.resolve("logs").toString());
         Settings settings = settingsBuilder.build();
 
-        Environment environment = new Environment(settings);        
-        Path policyFile = Security.processTemplate(new ByteArrayInputStream(new byte[0]), environment);
-     
-        ProtectionDomain domain = getClass().getProtectionDomain();
-        Policy policy = Policy.getInstance("JavaPolicy", new URIParameter(policyFile.toUri()));
+        Environment environment = new Environment(settings);
+        Permissions permissions = Security.createPermissions(environment);
 
         // check that all directories got permissions:
         // homefile: this is needed unless we break out rules for "lib" dir.
         // TODO: make read-only
-        assertTrue(policy.implies(domain, new FilePermission(environment.homeFile().toString(), "read,readlink,write,delete")));
+        assertTrue(permissions.implies(new FilePermission(environment.homeFile().toString(), "read,readlink,write,delete")));
         // config file
         // TODO: make read-only
-        assertTrue(policy.implies(domain, new FilePermission(environment.configFile().toString(), "read,readlink,write,delete")));
+        assertTrue(permissions.implies(new FilePermission(environment.configFile().toString(), "read,readlink,write,delete")));
         // plugins: r/w, TODO: can this be minimized?
-        assertTrue(policy.implies(domain, new FilePermission(environment.pluginsFile().toString(), "read,readlink,write,delete")));
+        assertTrue(permissions.implies(new FilePermission(environment.pluginsFile().toString(), "read,readlink,write,delete")));
         // data paths: r/w
         for (Path dataPath : environment.dataFiles()) {
-            assertTrue(policy.implies(domain, new FilePermission(dataPath.toString(), "read,readlink,write,delete")));
+            assertTrue(permissions.implies(new FilePermission(dataPath.toString(), "read,readlink,write,delete")));
         }
         for (Path dataPath : environment.dataWithClusterFiles()) {
-            assertTrue(policy.implies(domain, new FilePermission(dataPath.toString(), "read,readlink,write,delete")));
+            assertTrue(permissions.implies(new FilePermission(dataPath.toString(), "read,readlink,write,delete")));
         }
         // logs: r/w
-        assertTrue(policy.implies(domain, new FilePermission(environment.logsFile().toString(), "read,readlink,write,delete")));
+        assertTrue(permissions.implies(new FilePermission(environment.logsFile().toString(), "read,readlink,write,delete")));
     }
 }