Added allowed url property to URLContextClassLoaderFactory

core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java

@@ -47,6 +47,7 @@ public static synchronized void initContextFactory(AccumuloConfiguration conf) {
         LOG.info("Using default {}, which is subject to change in a future release",
             ContextClassLoaderFactory.class.getName());
         FACTORY = new URLContextClassLoaderFactory();
+        FACTORY.init(() -> new ConfigurationImpl(conf));
       } else {
         // load user's selected implementation and provide it with the service environment
         try {

core/src/main/java/org/apache/accumulo/core/classloader/URLContextClassLoaderFactory.java

@@ -18,13 +18,17 @@
  */
 package org.apache.accumulo.core.classloader;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import java.io.UncheckedIOException;
 import java.net.MalformedURLException;
-import java.net.URI;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.util.Arrays;
+import java.util.regex.Pattern;
 
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.spi.common.ContextClassLoaderEnvironment;
 import org.apache.accumulo.core.spi.common.ContextClassLoaderFactory;
 import org.apache.accumulo.core.util.cache.Caches;
 import org.apache.accumulo.core.util.cache.Caches.CacheName;
@@ -42,15 +46,35 @@ public class URLContextClassLoaderFactory implements ContextClassLoaderFactory {
 
   private static final Logger LOG = LoggerFactory.getLogger(URLContextClassLoaderFactory.class);
 
+  public static final String URL_PATTERN_PROPERTY =
+      Property.GENERAL_ARBITRARY_PROP_PREFIX + "factory.class.loader.url.allowed.patterns";
+
   // Cache the class loaders for re-use
   // WeakReferences are used so that the class loaders can be cleaned up when no longer needed
   // Classes that are loaded contain a reference to the class loader used to load them
   // so the class loader will be garbage collected when no more classes are loaded that reference it
   private final Cache<String,URLClassLoader> classloaders =
       Caches.getInstance().createNewBuilder(CacheName.CLASSLOADERS, true).weakValues().build();
 
+  private volatile Pattern urlPattern = null;
+
   @Override
-  public ClassLoader getClassLoader(String context) {
+  public void init(ContextClassLoaderEnvironment env) {
+    String urlPatternProperty = env.getConfiguration().get(URL_PATTERN_PROPERTY);
+    if (urlPatternProperty == null) {
+      LOG.warn("Property " + URL_PATTERN_PROPERTY + " not set, no contexts are allowed");
+    } else {
+      urlPattern = Pattern.compile(urlPatternProperty);
+    }
+  }
+
+  @Override
+  public ClassLoader getClassLoader(String context) throws ContextClassLoaderException {
+    if (urlPattern == null) {
+      throw new ContextClassLoaderException(
+          "Property " + URL_PATTERN_PROPERTY + " not set, no contexts are allowed");
+    }
+
     if (context == null) {
       throw new IllegalArgumentException("Unknown context");
     }
@@ -59,7 +83,11 @@ public ClassLoader getClassLoader(String context) {
       LOG.debug("Creating URLClassLoader for context, uris: {}", context);
       return new URLClassLoader(Arrays.stream(context.split(",")).map(p -> {
         try {
-          return URI.create(p).toURL();
+          URL url = new URL(p);
+          checkArgument(urlPattern.matcher(url.toExternalForm()).matches(),
+              "Context %s URL (%s) not allowed by pattern (%s)", context, url.toExternalForm(),
+              urlPattern.pattern());
+          return url;
         } catch (MalformedURLException e) {
           throw new UncheckedIOException(e);
         }

core/src/test/java/org/apache/accumulo/core/classloader/ContextClassLoaderFactoryTest.java

@@ -19,7 +19,10 @@
 package org.apache.accumulo.core.classloader;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.nio.file.Files;
@@ -37,14 +40,16 @@
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
-@SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "paths not set by user input")
+@SuppressFBWarnings(value = {"PATH_TRAVERSAL_IN", "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"},
+    justification = "paths not set by user input")
 public class ContextClassLoaderFactoryTest extends WithTestNames {
 
   @TempDir
   private static Path tempFolder;
 
   private URL uri1;
   private URL uri2;
+  private String tempFolderPattern;
 
   @BeforeEach
   public void setup() throws Exception {
@@ -70,6 +75,41 @@ public void setup() throws Exception {
         propsFile2.toFile());
     uri2 = propsFile2.toUri().toURL();
 
+    tempFolderPattern = tempFolder.toUri().toURL().toExternalForm() + ".*";
+  }
+
+  @Test
+  public void urlContextPatternNotSet() {
+    ConfigurationCopy cc = new ConfigurationCopy();
+    cc.set(Property.GENERAL_CONTEXT_CLASSLOADER_FACTORY.getKey(),
+        URLContextClassLoaderFactory.class.getName());
+    ClassLoaderUtil.resetContextFactoryForTests();
+    ClassLoaderUtil.initContextFactory(cc);
+    ContextClassLoaderException ex = assertThrows(ContextClassLoaderException.class, () -> {
+      @SuppressWarnings("unused")
+      URLClassLoader classloader =
+          (URLClassLoader) ClassLoaderUtil.getContextFactory().getClassLoader(uri1.toString());
+    });
+    assertEquals(
+        "Error getting classloader for context: Property general.custom.factory.class.loader.url.allowed.patterns not set, no contexts are allowed",
+        ex.getMessage());
+  }
+
+  @Test
+  public void urlContextPatternDoesNotMath() throws MalformedURLException {
+    ConfigurationCopy cc = new ConfigurationCopy();
+    cc.set(Property.GENERAL_CONTEXT_CLASSLOADER_FACTORY.getKey(),
+        URLContextClassLoaderFactory.class.getName());
+    cc.set(URLContextClassLoaderFactory.URL_PATTERN_PROPERTY,
+        new URL("file:///path/to/unknown/folder/.*").toExternalForm());
+    ClassLoaderUtil.resetContextFactoryForTests();
+    ClassLoaderUtil.initContextFactory(cc);
+    IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> {
+      @SuppressWarnings("unused")
+      URLClassLoader classloader =
+          (URLClassLoader) ClassLoaderUtil.getContextFactory().getClassLoader(uri1.toString());
+    });
+    assertTrue(ex.getMessage().contains(" not allowed by pattern "));
   }
 
   @Test
@@ -78,6 +118,7 @@ public void differentContexts() throws ContextClassLoaderException {
     ConfigurationCopy cc = new ConfigurationCopy();
     cc.set(Property.GENERAL_CONTEXT_CLASSLOADER_FACTORY.getKey(),
         URLContextClassLoaderFactory.class.getName());
+    cc.set(URLContextClassLoaderFactory.URL_PATTERN_PROPERTY, tempFolderPattern);
     ClassLoaderUtil.resetContextFactoryForTests();
     ClassLoaderUtil.initContextFactory(cc);
 

minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterClasspathTest.java

@@ -29,6 +29,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 
+import org.apache.accumulo.core.classloader.URLContextClassLoaderFactory;
 import org.apache.accumulo.core.client.Accumulo;
 import org.apache.accumulo.core.client.AccumuloClient;
 import org.apache.accumulo.core.client.BatchWriter;
@@ -76,6 +77,8 @@ public static void setupMiniCluster() throws Exception {
     config.setZooKeeperPort(0);
     HashMap<String,String> site = new HashMap<>();
     site.put(Property.TSERV_WAL_MAX_SIZE.getKey(), "1G");
+    site.put(URLContextClassLoaderFactory.URL_PATTERN_PROPERTY,
+        jarFile.toURI().toURL().toExternalForm());
     config.setSiteConfig(site);
     accumulo = new MiniAccumuloCluster(config);
     accumulo.start();
@@ -95,8 +98,8 @@ public void testPerTableClasspath() throws Exception {
       final String tableName = testName();
 
       var ntc = new NewTableConfiguration();
-      ntc.setProperties(
-          Map.of(Property.TABLE_CLASSLOADER_CONTEXT.getKey(), jarFile.toURI().toString()));
+      ntc.setProperties(Map.of(Property.TABLE_CLASSLOADER_CONTEXT.getKey(),
+          jarFile.toURI().toURL().toExternalForm()));
       ntc.attachIterator(
           new IteratorSetting(100, "foocensor", "org.apache.accumulo.test.FooFilter"));
 

test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java

@@ -31,6 +31,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.EnumSet;
 import java.util.List;
@@ -39,6 +40,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 
+import org.apache.accumulo.core.classloader.URLContextClassLoaderFactory;
 import org.apache.accumulo.core.client.Accumulo;
 import org.apache.accumulo.core.client.AccumuloClient;
 import org.apache.accumulo.core.client.IteratorSetting;
@@ -95,6 +97,13 @@ public static void after() throws Exception {
   @Override
   public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreSite) {
     ExternalCompactionTestUtils.configureMiniCluster(cfg, coreSite);
+    try {
+      cfg.setProperty(URLContextClassLoaderFactory.URL_PATTERN_PROPERTY,
+          new URL("file:" + cfg.getDir().toString() + "/accumulo/classpath/.*").toExternalForm());
+    } catch (MalformedURLException e) {
+      throw new RuntimeException(e);
+    }
+
     // After 1 failure start backing off by 5s.
     // After 3 failures, terminate the Compactor
     cfg.setProperty(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD, "1");
@@ -220,7 +229,7 @@ public void testClassLoaderContextErrorKillsCompactor() throws Exception {
 
       // Set the context on the table
       client.tableOperations().setProperty(table1, Property.TABLE_CLASSLOADER_CONTEXT.getKey(),
-          dst.toUri().toString());
+          dst.toUri().toURL().toExternalForm());
 
       final IteratorSetting cfg =
           new IteratorSetting(101, "FooFilter", "org.apache.accumulo.test.FooFilter");

test/src/main/java/org/apache/accumulo/test/functional/ScannerContextIT.java

@@ -25,11 +25,14 @@
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
 import java.io.IOException;
+import java.net.MalformedURLException;
+import java.net.URL;
 import java.time.Duration;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.Map.Entry;
 
+import org.apache.accumulo.core.classloader.URLContextClassLoaderFactory;
 import org.apache.accumulo.core.client.Accumulo;
 import org.apache.accumulo.core.client.AccumuloClient;
 import org.apache.accumulo.core.client.BatchScanner;
@@ -44,7 +47,9 @@
 import org.apache.accumulo.core.data.Value;
 import org.apache.accumulo.core.security.Authorizations;
 import org.apache.accumulo.miniclusterImpl.MiniAccumuloClusterImpl;
+import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl;
 import org.apache.accumulo.test.harness.AccumuloClusterHarness;
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.junit.jupiter.api.BeforeEach;
@@ -64,6 +69,16 @@ protected Duration defaultTimeout() {
     return Duration.ofMinutes(2);
   }
 
+  @Override
+  public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration hadoopCoreSite) {
+    try {
+      cfg.setProperty(URLContextClassLoaderFactory.URL_PATTERN_PROPERTY,
+          new URL(CONTEXT_DIR + "/.*").toExternalForm());
+    } catch (MalformedURLException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
   @BeforeEach
   public void checkCluster() throws Exception {
     assumeTrue(getClusterType() == ClusterType.MINI);
@@ -131,7 +146,7 @@ public void testScanContextOverridesTableContext() throws Exception {
       // that contains nothing. The ScanContextIT context will point to the test iterators jar
       String tableContextProperty = Property.TABLE_CLASSLOADER_CONTEXT.getKey();
       String tableContextDir = "file://" + System.getProperty("user.dir") + "/target";
-      String tableContextClasspath = tableContextDir + "/TestFoo.jar";
+      String tableContextClasspath = new URL(tableContextDir + "/TestFoo.jar").toExternalForm();
 
       // Set the ScanContextIT context on the namespace
       c.namespaceOperations().setProperty(Namespace.DEFAULT.name(), tableContextProperty, CONTEXT);
@@ -194,7 +209,7 @@ public void testOneScannerDoesntInterfereWithAnother() throws Exception {
         IteratorSetting cfg = new IteratorSetting(21, "reverse",
             "org.apache.accumulo.test.functional.ValueReversingIterator");
         one.addScanIterator(cfg);
-        one.setClassLoaderContext(CONTEXT);
+        one.setClassLoaderContext(new URL(CONTEXT).toExternalForm());
 
         Iterator<Entry<Key,Value>> iterator = one.iterator();
         for (int i = 0; i < ITERATIONS; i++) {
@@ -236,7 +251,7 @@ public void testClearContext() throws Exception {
         IteratorSetting cfg = new IteratorSetting(21, "reverse",
             "org.apache.accumulo.test.functional.ValueReversingIterator");
         one.addScanIterator(cfg);
-        one.setClassLoaderContext(CONTEXT);
+        one.setClassLoaderContext(new URL(CONTEXT).toExternalForm());
 
         Iterator<Entry<Key,Value>> iterator = one.iterator();
         for (int i = 0; i < ITERATIONS; i++) {
@@ -264,7 +279,7 @@ private void scanCheck(AccumuloClient c, String tableName, IteratorSetting cfg,
       String expected) throws Exception {
     try (Scanner bs = c.createScanner(tableName, Authorizations.EMPTY)) {
       if (context != null) {
-        bs.setClassLoaderContext(context);
+        bs.setClassLoaderContext(new URL(context).toExternalForm());
       }
       if (cfg != null) {
         bs.addScanIterator(cfg);
@@ -284,7 +299,7 @@ private void batchCheck(AccumuloClient c, String tableName, IteratorSetting cfg,
     try (BatchScanner bs = c.createBatchScanner(tableName)) {
       bs.setRanges(Collections.singleton(new Range()));
       if (context != null) {
-        bs.setClassLoaderContext(context);
+        bs.setClassLoaderContext(new URL(context).toExternalForm());
       }
       if (cfg != null) {
         bs.addScanIterator(cfg);

test/src/main/java/org/apache/accumulo/test/shell/ShellServerIT.java

@@ -35,6 +35,8 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.net.MalformedURLException;
+import java.net.URL;
 import java.nio.file.Files;
 import java.time.Duration;
 import java.util.ArrayList;
@@ -51,6 +53,7 @@
 import java.util.regex.Pattern;
 
 import org.apache.accumulo.core.Constants;
+import org.apache.accumulo.core.classloader.URLContextClassLoaderFactory;
 import org.apache.accumulo.core.client.Accumulo;
 import org.apache.accumulo.core.client.AccumuloClient;
 import org.apache.accumulo.core.client.AccumuloException;
@@ -129,6 +132,13 @@ public class ShellServerIT extends SharedMiniClusterBase {
   private static class ShellServerITConfigCallback implements MiniClusterConfigurationCallback {
     @Override
     public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreSite) {
+      try {
+        cfg.setProperty(URLContextClassLoaderFactory.URL_PATTERN_PROPERTY,
+            new URL("file://" + System.getProperty("user.dir") + "/target/.*").toExternalForm());
+      } catch (MalformedURLException e) {
+        throw new RuntimeException(e);
+      }
+
       // Only one tserver to avoid race conditions on ZK propagation (auths and configuration)
       cfg.getClusterServerConfiguration().setNumDefaultTabletServers(1);
       // Set the min span to 0 so we will definitely get all the traces back. See ACCUMULO-4365
@@ -1756,7 +1766,8 @@ public void verifyPerTableClasspath(final String table, final File fooConstraint
 
     File fooFilterJar = initJar("/org/apache/accumulo/test/FooFilter.jar", "FooFilter", rootPath);
 
-    String context = fooFilterJar.toURI() + "," + fooConstraintJar.toURI();
+    String context = fooFilterJar.toURI().toURL().toExternalForm() + ","
+        + fooConstraintJar.toURI().toURL().toExternalForm();
 
     ts.exec("createtable " + table, true);
     ts.exec(
@@ -1959,10 +1970,11 @@ public void scansWithClassLoaderContext() throws IOException {
     make10();
     setupFakeContextPath();
 
+    String fakeCtx = new URL(FAKE_CONTEXT).toExternalForm();
     result = ts.exec("config -t " + tableName + " -s " + Property.TABLE_CLASSLOADER_CONTEXT.getKey()
-        + "=" + FAKE_CONTEXT);
+        + "=" + fakeCtx);
     assertEquals("root@miniInstance " + tableName + "> config -t " + tableName + " -s "
-        + Property.TABLE_CLASSLOADER_CONTEXT.getKey() + "=" + FAKE_CONTEXT + "\n", result);
+        + Property.TABLE_CLASSLOADER_CONTEXT.getKey() + "=" + fakeCtx + "\n", result);
 
     result = ts.exec("setshelliter -pn baz -n reverse -p 21 -class " + VALUE_REVERSING_ITERATOR);
     assertTrue(result.contains("The iterator class does not implement OptionDescriber"));
@@ -1991,25 +2003,26 @@ public void scansWithClassLoaderContext() throws IOException {
 
     // Override the table classloader context with the REAL implementation of
     // ValueReversingIterator, which does reverse the value.
-    result = ts.exec("scan -pn baz -b row1 -e row1 -cc " + REAL_CONTEXT);
+    String realCtx = new URL(REAL_CONTEXT).toExternalForm();
+    result = ts.exec("scan -pn baz -b row1 -e row1 -cc " + realCtx);
     assertEquals(2, result.split("\n").length);
     assertTrue(result.contains("eulav"));
     assertFalse(result.contains("value"));
-    result = ts.exec("scan -pn baz -b row3 -e row5 -cc " + REAL_CONTEXT);
+    result = ts.exec("scan -pn baz -b row3 -e row5 -cc " + realCtx);
     assertEquals(4, result.split("\n").length);
     assertTrue(result.contains("eulav"));
     assertFalse(result.contains("value"));
-    result = ts.exec("scan -pn baz -r row3 -cc " + REAL_CONTEXT);
+    result = ts.exec("scan -pn baz -r row3 -cc " + realCtx);
     assertEquals(2, result.split("\n").length);
     assertTrue(result.contains("eulav"));
     assertFalse(result.contains("value"));
-    result = ts.exec("scan -pn baz -b row: -cc " + REAL_CONTEXT);
+    result = ts.exec("scan -pn baz -b row: -cc " + realCtx);
     assertEquals(1, result.split("\n").length);
-    result = ts.exec("scan -pn baz -b row -cc " + REAL_CONTEXT);
+    result = ts.exec("scan -pn baz -b row -cc " + realCtx);
     assertEquals(11, result.split("\n").length);
     assertTrue(result.contains("eulav"));
     assertFalse(result.contains("value"));
-    result = ts.exec("scan -pn baz -e row: -cc " + REAL_CONTEXT);
+    result = ts.exec("scan -pn baz -e row: -cc " + realCtx);
     assertEquals(11, result.split("\n").length);
     assertTrue(result.contains("eulav"));
     assertFalse(result.contains("value"));