You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by us...@apache.org on 2013/09/21 16:58:32 UTC
svn commit: r1525248 - in /lucene/dev/branches/branch_4x: ./ solr/
solr/contrib/ solr/contrib/velocity/src/java/org/apache/solr/response/
solr/core/ solr/core/src/java/org/apache/solr/cloud/
solr/core/src/java/org/apache/solr/core/ solr/core/src/test/o...
Author: uschindler
Date: Sat Sep 21 14:58:31 2013
New Revision: 1525248
URL: http://svn.apache.org/r1525248
Log:
Merged revision(s) 1525246 from lucene/dev/trunk:
SOLR-4882: Restrict SolrResourceLoader to only allow access to resource files below the instance dir
Modified:
lucene/dev/branches/branch_4x/ (props changed)
lucene/dev/branches/branch_4x/solr/ (props changed)
lucene/dev/branches/branch_4x/solr/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_4x/solr/contrib/ (props changed)
lucene/dev/branches/branch_4x/solr/contrib/velocity/src/java/org/apache/solr/response/VelocityResponseWriter.java
lucene/dev/branches/branch_4x/solr/core/ (props changed)
lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java
lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java
lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/schema/PrimitiveFieldTypeTest.java
lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java
Modified: lucene/dev/branches/branch_4x/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/CHANGES.txt?rev=1525248&r1=1525247&r2=1525248&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/CHANGES.txt (original)
+++ lucene/dev/branches/branch_4x/solr/CHANGES.txt Sat Sep 21 14:58:31 2013
@@ -39,6 +39,17 @@ New Features
* SOLR-5167: Add support for AnalyzingInfixSuggester (AnalyzingInfixLookupFactory).
(Areek Zillur, Varun Thacker via Robert Muir)
+Security
+----------------------
+
+* SOLR-4882: SolrResourceLoader was restricted to only allow access to resource
+ files below the instance dir. The reason for this is security related: Some
+ Solr components allow to pass in resource paths via REST parameters
+ (e.g. XSL stylesheets, velocity templates,...) and load them via resource
+ loader. For backwards compatibility, this security feature can be disabled
+ by a new system property: solr.allow.unsafe.resourceloading=true
+ (Uwe Schindler)
+
Other Changes
----------------------
Modified: lucene/dev/branches/branch_4x/solr/contrib/velocity/src/java/org/apache/solr/response/VelocityResponseWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/contrib/velocity/src/java/org/apache/solr/response/VelocityResponseWriter.java?rev=1525248&r1=1525247&r2=1525248&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/contrib/velocity/src/java/org/apache/solr/response/VelocityResponseWriter.java (original)
+++ lucene/dev/branches/branch_4x/solr/contrib/velocity/src/java/org/apache/solr/response/VelocityResponseWriter.java Sat Sep 21 14:58:31 2013
@@ -67,7 +67,6 @@ public class VelocityResponseWriter impl
} catch (ClassCastException e) {
// known edge case where QueryResponse's extraction assumes "response" is a SolrDocumentList
// (AnalysisRequestHandler emits a "response")
- e.printStackTrace();
rsp = new SolrResponseBase();
rsp.setResponse(parsedResponse);
}
@@ -121,25 +120,7 @@ public class VelocityResponseWriter impl
SolrVelocityResourceLoader resourceLoader =
new SolrVelocityResourceLoader(request.getCore().getSolrConfig().getResourceLoader());
engine.setProperty("solr.resource.loader.instance", resourceLoader);
-
- File fileResourceLoaderBaseDir = null;
- try {
- String template_root = request.getParams().get("v.base_dir");
- fileResourceLoaderBaseDir = new File(request.getCore().getResourceLoader().getConfigDir(), "velocity");
- if (template_root != null) {
- fileResourceLoaderBaseDir = new File(template_root);
- }
- } catch (SolrException e) {
- // no worries... probably in ZooKeeper mode and getConfigDir() isn't available, so we'll just ignore omit
- // the file system resource loader
- }
-
- if (fileResourceLoaderBaseDir != null) {
- engine.setProperty(RuntimeConstants.FILE_RESOURCE_LOADER_PATH, fileResourceLoaderBaseDir.getAbsolutePath());
- engine.setProperty(RuntimeConstants.RESOURCE_LOADER, "params,file,solr");
- } else {
- engine.setProperty(RuntimeConstants.RESOURCE_LOADER, "params,solr");
- }
+ engine.setProperty(RuntimeConstants.RESOURCE_LOADER, "params,solr");
// TODO: Externalize Velocity properties
String propFile = request.getParams().get("v.properties");
Modified: lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java?rev=1525248&r1=1525247&r2=1525248&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java Sat Sep 21 14:58:31 2013
@@ -18,6 +18,7 @@ package org.apache.solr.cloud;
*/
import java.io.ByteArrayInputStream;
+import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.util.List;
@@ -75,7 +76,7 @@ public class ZkSolrResourceLoader extend
String file = collectionZkPath + "/" + resource;
try {
if (zkController.pathExists(file)) {
- byte[] bytes = zkController.getZkClient().getData(collectionZkPath + "/" + resource, null, null, true);
+ byte[] bytes = zkController.getZkClient().getData(file, null, null, true);
return new ByteArrayInputStream(bytes);
}
} catch (Exception e) {
@@ -83,7 +84,7 @@ public class ZkSolrResourceLoader extend
}
try {
// delegate to the class loader (looking into $INSTANCE_DIR/lib jars)
- is = classLoader.getResourceAsStream(resource);
+ is = classLoader.getResourceAsStream(resource.replace(File.separatorChar, '/'));
} catch (Exception e) {
throw new IOException("Error opening " + resource, e);
}
Modified: lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java?rev=1525248&r1=1525247&r2=1525248&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java Sat Sep 21 14:58:31 2013
@@ -55,6 +55,7 @@ import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.Constructor;
import java.net.MalformedURLException;
+import java.net.URI;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.charset.CharacterCodingException;
@@ -250,7 +251,7 @@ public class SolrResourceLoader implemen
}
public String getConfigDir() {
- return instanceDir + "conf/";
+ return instanceDir + "conf" + File.separator;
}
public String getDataDir() {
@@ -299,27 +300,46 @@ public class SolrResourceLoader implemen
public InputStream openResource(String resource) throws IOException {
InputStream is=null;
try {
- File f0 = new File(resource);
- File f = f0;
+ File f0 = new File(resource), f = f0;
if (!f.isAbsolute()) {
// try $CWD/$configDir/$resource
- f = new File(getConfigDir() + resource);
+ f = new File(getConfigDir() + resource).getAbsoluteFile();
}
- if (f.isFile() && f.canRead()) {
+ boolean found = f.isFile() && f.canRead();
+ if (!found) { // no success with $CWD/$configDir/$resource
+ f = f0.getAbsoluteFile();
+ found = f.isFile() && f.canRead();
+ }
+ // check that we don't escape instance dir
+ if (found) {
+ if (!Boolean.parseBoolean(System.getProperty("solr.allow.unsafe.resourceloading", "false"))) {
+ final URI instanceURI = new File(getInstanceDir()).getAbsoluteFile().toURI().normalize();
+ final URI fileURI = f.toURI().normalize();
+ if (instanceURI.relativize(fileURI) == fileURI) {
+ // no URI relativize possible, so they don't share same base folder
+ throw new IOException("For security reasons, SolrResourceLoader cannot load files from outside the instance's directory: " + f +
+ "; if you want to override this safety feature and you are sure about the consequences, you can pass the system property "+
+ "-Dsolr.allow.unsafe.resourceloading=true to your JVM");
+ }
+ }
+ // relativize() returned a relative, new URI, so we are fine!
return new FileInputStream(f);
- } else if (f != f0) { // no success with $CWD/$configDir/$resource
- if (f0.isFile() && f0.canRead())
- return new FileInputStream(f0);
- }
- // delegate to the class loader (looking into $INSTANCE_DIR/lib jars)
- is = classLoader.getResourceAsStream(resource);
- if (is == null)
- is = classLoader.getResourceAsStream(getConfigDir() + resource);
+ }
+ // Delegate to the class loader (looking into $INSTANCE_DIR/lib jars).
+ // We need a ClassLoader-compatible (forward-slashes) path here!
+ is = classLoader.getResourceAsStream(resource.replace(File.separatorChar, '/'));
+ // This is a hack just for tests (it is not done in ZKResourceLoader)!
+ // -> the getConfigDir's path must not be absolute!
+ if (is == null && System.getProperty("jetty.testMode") != null && !new File(getConfigDir()).isAbsolute()) {
+ is = classLoader.getResourceAsStream((getConfigDir() + resource).replace(File.separatorChar, '/'));
+ }
+ } catch (IOException ioe) {
+ throw ioe;
} catch (Exception e) {
throw new IOException("Error opening " + resource, e);
}
if (is==null) {
- throw new IOException("Can't find resource '" + resource + "' in classpath or '" + getConfigDir() + "', cwd="+System.getProperty("user.dir"));
+ throw new IOException("Can't find resource '" + resource + "' in classpath or '" + new File(getConfigDir()).getAbsolutePath() + "'");
}
return is;
}
Modified: lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java?rev=1525248&r1=1525247&r2=1525248&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java Sat Sep 21 14:58:31 2013
@@ -20,10 +20,10 @@ package org.apache.solr.core;
import junit.framework.Assert;
import org.apache.lucene.util.Constants;
-import org.apache.lucene.util.LuceneTestCase;
import org.apache.lucene.analysis.core.KeywordTokenizerFactory;
import org.apache.lucene.analysis.ngram.NGramFilterFactory;
import org.apache.lucene.util._TestUtil;
+import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.SolrException;
import org.apache.solr.handler.admin.LukeRequestHandler;
import org.apache.solr.handler.component.FacetComponent;
@@ -34,6 +34,7 @@ import org.apache.solr.util.plugin.SolrC
import java.io.File;
import java.io.FileFilter;
import java.io.FileOutputStream;
+import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.CharacterCodingException;
import java.util.Arrays;
@@ -42,23 +43,46 @@ import java.util.List;
import java.util.jar.JarEntry;
import java.util.jar.JarOutputStream;
-public class ResourceLoaderTest extends LuceneTestCase
+public class ResourceLoaderTest extends SolrTestCaseJ4
{
public void testInstanceDir() throws Exception {
SolrResourceLoader loader = new SolrResourceLoader(null);
String instDir = loader.getInstanceDir();
assertTrue(instDir + " is not equal to " + "solr/", instDir.equals("solr/") == true);
+ loader.close();
loader = new SolrResourceLoader("solr");
instDir = loader.getInstanceDir();
assertTrue(instDir + " is not equal to " + "solr/", instDir.equals("solr" + File.separator) == true);
+ loader.close();
+ }
+
+ public void testEscapeInstanceDir() throws Exception {
+ File temp = _TestUtil.getTempDir("testEscapeInstanceDir");
+ try {
+ temp.mkdirs();
+ new File(temp, "dummy.txt").createNewFile();
+ File instanceDir = new File(temp, "instance");
+ instanceDir.mkdir();
+ new File(instanceDir, "conf").mkdir();
+ SolrResourceLoader loader = new SolrResourceLoader(instanceDir.getAbsolutePath());
+ try {
+ loader.openResource("../../dummy.txt").close();
+ fail();
+ } catch (IOException ioe) {
+ assertTrue(ioe.getMessage().startsWith("For security reasons, SolrResourceLoader"));
+ }
+ loader.close();
+ } finally {
+ _TestUtil.rmDir(temp);
+ }
}
- public void testAwareCompatibility()
+ public void testAwareCompatibility() throws Exception
{
SolrResourceLoader loader = new SolrResourceLoader( "." );
- Class clazz = ResourceLoaderAware.class;
+ Class<?> clazz = ResourceLoaderAware.class;
// Check ResourceLoaderAware valid objects
loader.assertAwareCompatibility( clazz, new NGramFilterFactory(new HashMap<String,String>()) );
loader.assertAwareCompatibility( clazz, new KeywordTokenizerFactory(new HashMap<String,String>()) );
@@ -98,6 +122,8 @@ public class ResourceLoaderTest extends
}
catch( SolrException ex ) { } // OK
}
+
+ loader.close();
}
public void testBOMMarkers() throws Exception {
@@ -124,6 +150,8 @@ public class ResourceLoaderTest extends
List<String> lines = loader.getLines(fileWithBom);
assertEquals(1, lines.size());
assertEquals("BOMsAreEvil", lines.get(0));
+
+ loader.close();
}
public void testWrongEncoding() throws Exception {
@@ -131,11 +159,12 @@ public class ResourceLoaderTest extends
SolrResourceLoader loader = new SolrResourceLoader("solr/collection1");
// ensure we get our exception
try {
- List<String> lines = loader.getLines(wrongEncoding);
+ loader.getLines(wrongEncoding);
fail();
} catch (SolrException expected) {
assertTrue(expected.getCause() instanceof CharacterCodingException);
}
+ loader.close();
}
public void testClassLoaderLibs() throws Exception {
Modified: lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/schema/PrimitiveFieldTypeTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/schema/PrimitiveFieldTypeTest.java?rev=1525248&r1=1525247&r2=1525248&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/schema/PrimitiveFieldTypeTest.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/schema/PrimitiveFieldTypeTest.java Sat Sep 21 14:58:31 2013
@@ -43,10 +43,17 @@ public class PrimitiveFieldTypeTest exte
System.setProperty("enable.update.log", "false"); // schema12 doesn't support _version_
System.setProperty("solr.test.sys.prop1", "propone");
System.setProperty("solr.test.sys.prop2", "proptwo");
+ System.setProperty("solr.allow.unsafe.resourceloading", "true");
initMap = new HashMap<String,String>();
config = new SolrConfig(new SolrResourceLoader("solr/collection1"), testConfHome + "solrconfig.xml", null);
}
+
+ @Override
+ public void tearDown() throws Exception {
+ System.clearProperty("solr.allow.unsafe.resourceloading");
+ super.tearDown();
+ }
@SuppressWarnings("deprecation")
@Test
Modified: lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java?rev=1525248&r1=1525247&r2=1525248&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java Sat Sep 21 14:58:31 2013
@@ -28,6 +28,16 @@ import org.apache.commons.io.IOUtils;
public class TestSystemIdResolver extends LuceneTestCase {
+ public void setUp() throws Exception {
+ super.setUp();
+ System.setProperty("solr.allow.unsafe.resourceloading", "true");
+ }
+
+ public void tearDown() throws Exception {
+ System.clearProperty("solr.allow.unsafe.resourceloading");
+ super.tearDown();
+ }
+
private void assertEntityResolving(SystemIdResolver resolver, String expectedSystemId, String base, String systemId) throws Exception {
final InputSource is = resolver.resolveEntity(null, null, base, systemId);
try {