You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by rm...@apache.org on 2019/12/05 05:07:15 UTC

[lucene-solr] branch branch_8x updated: SOLR-14015: remove blanket filesystem read access from solr-tests.policy

This is an automated email from the ASF dual-hosted git repository.

rmuir pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new 5325c29  SOLR-14015: remove blanket filesystem read access from solr-tests.policy
5325c29 is described below

commit 5325c29cf9c122978333d6ff029940319cce71cc
Author: Robert Muir <rm...@apache.org>
AuthorDate: Wed Dec 4 23:16:19 2019 -0500

    SOLR-14015: remove blanket filesystem read access from solr-tests.policy
    
    Restrict this to only minimal paths like lucene. It is the defense for directory traversal attacks.
    It will also help find bad bugs where things are reading filesystem in the wrong locations.
---
 lucene/common-build.xml                            |  2 ++
 .../apache/lucene/util/TestSecurityManager.java    | 24 ++++++++++++++++++++
 lucene/tools/junit4/solr-tests.policy              | 26 ++++++++++++++++++++--
 .../test/org/apache/solr/cloud/rule/RulesTest.java |  9 +++++---
 .../org/apache/solr/util/TestSystemIdResolver.java |  1 +
 5 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/lucene/common-build.xml b/lucene/common-build.xml
index fe17239..2e45f62 100644
--- a/lucene/common-build.xml
+++ b/lucene/common-build.xml
@@ -1174,6 +1174,8 @@
 
             <!-- Restrict access to certain Java features and install security manager: -->
             <sysproperty key="common.dir" file="${common.dir}" />
+            <sysproperty key="common-solr.dir" file="${common.dir}/../solr" />
+            <sysproperty key="ant.library.dir" file="${ant.library.dir}" />
             <sysproperty key="clover.db.dir" file="${clover.db.dir}" />
             <syspropertyset>
                 <propertyref prefix="java.security.manager"/>
diff --git a/lucene/test-framework/src/java/org/apache/lucene/util/TestSecurityManager.java b/lucene/test-framework/src/java/org/apache/lucene/util/TestSecurityManager.java
index 70539cd..ee2e382 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/util/TestSecurityManager.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/util/TestSecurityManager.java
@@ -106,6 +106,30 @@ public final class TestSecurityManager extends SecurityManager {
 
   /**
    * {@inheritDoc}
+   * <p>This method implements hacks to workaround hadoop's garbage FileUtil code
+   */
+  @Override
+  public void checkRead(String file) {
+    for (StackTraceElement element : Thread.currentThread().getStackTrace()) {
+      // hadoop "createPermissionsDiagnosisString" method doesn't handle securityexception and fails completely.
+      // it insists on climbing up full directory tree!
+      // so, lie to it, and tell it we will happily read, so it does not crash.
+      if ("org.apache.hadoop.hdfs.MiniDFSCluster".equals(element.getClassName()) &&
+          "createPermissionsDiagnosisString".equals(element.getMethodName())) {
+        return;
+      }
+      // hadoop "canRead" method doesn't handle securityexception and fails completely.
+      // so, lie to it, and tell it we will happily read, so it does not crash.
+      if ("org.apache.hadoop.fs.FileUtil".equals(element.getClassName()) &&
+          "canRead".equals(element.getMethodName())) {
+        return;
+      }
+    }
+    super.checkRead(file);
+  }
+
+  /**
+   * {@inheritDoc}
    * <p>This method inspects the stack trace and checks who is calling
    * {@link System#exit(int)} and similar methods
    * @throws SecurityException if the caller of this method is not the test runner itself.
diff --git a/lucene/tools/junit4/solr-tests.policy b/lucene/tools/junit4/solr-tests.policy
index c9bb1f2..c3cd0e8 100644
--- a/lucene/tools/junit4/solr-tests.policy
+++ b/lucene/tools/junit4/solr-tests.policy
@@ -18,8 +18,14 @@
 // Policy file for solr tests. Please keep minimal and avoid wildcards.
 
 grant {
-  // permissions for file access, write access only to sandbox:
-  permission java.io.FilePermission "<<ALL FILES>>", "read";
+  // contain read access to only what we need:
+  // 3rd party jar resources (where symlinks are not supported), test-files/ resources
+  permission java.io.FilePermission "${common.dir}${/}-", "read";
+  permission java.io.FilePermission "${common-solr.dir}${/}-", "read";
+  // 3rd party jar resources (where symlinks are supported)
+  permission java.io.FilePermission "${user.home}${/}.ivy2${/}cache${/}-", "read";
+  // system jar resources
+  permission java.io.FilePermission "${java.home}${/}-", "read";
   permission java.io.FilePermission "${junit4.childvm.cwd}", "read";
   permission java.io.FilePermission "${junit4.childvm.cwd}${/}temp", "read,write,delete";
   permission java.io.FilePermission "${junit4.childvm.cwd}${/}temp${/}-", "read,write,delete";
@@ -27,6 +33,22 @@ grant {
   permission java.io.FilePermission "${junit4.tempDir}${/}*", "read,write,delete";
   permission java.io.FilePermission "${clover.db.dir}${/}-", "read,write,delete";
   permission java.io.FilePermission "${tests.linedocsfile}", "read";
+  // hadoop
+  permission java.io.FilePermission "${ant.library.dir}${/}-", "read";
+  permission java.io.FilePermission "${user.home}${/}.ant${/}lib${/}-", "read";
+  permission java.io.FilePermission "${user.home}${/}hadoop-metrics2.properties", "read";
+  permission java.io.FilePermission "${user.home}${/}hadoop-metrics2-namenode.properties", "read";
+  // kerberos
+  permission java.io.FilePermission "${user.home}${/}.java.login.config", "read";
+  // SolrTestCaseJ4 explicitly uses these
+  permission java.io.FilePermission "/dev/./urandom", "read";
+  permission java.io.FilePermission "/dev/random", "read";
+  // DirectoryFactoryTest messes with these (wtf?)
+  permission java.io.FilePermission "/tmp/inst1/conf/solrcore.properties", "read";
+  permission java.io.FilePermission "/path/to/myinst/conf/solrcore.properties", "read";
+  // TestConfigSets messes with these (wtf?)
+  permission java.io.FilePermission "/path/to/solr/home/lib", "read";
+
   permission java.nio.file.LinkPermission "hard";
   
   // all possibilities of accepting/binding connections on localhost with ports >=1024:
diff --git a/solr/core/src/test/org/apache/solr/cloud/rule/RulesTest.java b/solr/core/src/test/org/apache/solr/cloud/rule/RulesTest.java
index afe6acd..ee20fcf 100644
--- a/solr/core/src/test/org/apache/solr/cloud/rule/RulesTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/rule/RulesTest.java
@@ -18,6 +18,7 @@ package org.apache.solr.cloud.rule;
 
 import java.lang.invoke.MethodHandles;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.HashSet;
 import java.util.List;
@@ -82,8 +83,9 @@ public class RulesTest extends SolrCloudTestCase {
                  5, cluster.getJettySolrRunners().size());
     
     final long minGB = (random().nextBoolean() ? 1 : 0);
+    final Path toTest = Paths.get("").toAbsolutePath();
     assumeTrue("doIntegrationTest needs minGB="+minGB+" usable disk space",
-        ImplicitSnitch.getUsableSpaceInGB(Paths.get("/")) > minGB);
+        ImplicitSnitch.getUsableSpaceInGB(toTest) > minGB);
 
     String rulesColl = "rulesColl";
     CollectionAdminRequest.createCollectionWithImplicitRouter(rulesColl, "conf", "shard1", 2)
@@ -323,13 +325,14 @@ public class RulesTest extends SolrCloudTestCase {
 
   @Test
   public void testModifyColl() throws Exception {
+    final Path toTest = Paths.get("").toAbsolutePath();
 
     final long minGB1 = (random().nextBoolean() ? 1 : 0);
     final long minGB2 = 5;
     assumeTrue("testModifyColl needs minGB1="+minGB1+" usable disk space",
-        ImplicitSnitch.getUsableSpaceInGB(Paths.get("/")) > minGB1);
+        ImplicitSnitch.getUsableSpaceInGB(toTest) > minGB1);
     assumeTrue("testModifyColl needs minGB2="+minGB2+" usable disk space",
-        ImplicitSnitch.getUsableSpaceInGB(Paths.get("/")) > minGB2);
+        ImplicitSnitch.getUsableSpaceInGB(toTest) > minGB2);
 
     String rulesColl = "modifyColl";
     CollectionAdminRequest.createCollection(rulesColl, "conf", 1, 2)
diff --git a/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java b/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java
index 89ef180..b1d585b 100644
--- a/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java
+++ b/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java
@@ -87,6 +87,7 @@ public class TestSystemIdResolver extends SolrTestCaseJ4 {
         resolver.resolveEntity(null, null, "solrres:/solrconfig.xml", path);
       });
       assertTrue(ioe.getMessage().startsWith("Can't find resource")
+          || ioe.getMessage().contains("access denied")
           || ioe.getMessage().contains("is outside resource loader dir"));
     }
   }