[GitHub] [flink] pnowojski commented on a change in pull request #10845: [FLINK-15355][plugins]Classloader restricts access to parent to whitelist.

2020-01-16 Thread GitBox
pnowojski commented on a change in pull request #10845: 
[FLINK-15355][plugins]Classloader restricts access to parent to whitelist.
URL: https://github.com/apache/flink/pull/10845#discussion_r367276414
 
 

 ##
 File path: 
flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java
 ##
 @@ -121,7 +121,7 @@
public static final ConfigOption 
PLUGIN_ALWAYS_PARENT_FIRST_LOADER_PATTERNS = ConfigOptions
.key("plugin.classloader.parent-first-patterns.default")
.stringType()
-   
.defaultValue("java.;scala.;org.apache.flink.;javax.annotation.;org.slf4j;org.apache.log4j;org.apache"
 +
+   
.defaultValue("java.;javax.;scala.;org.apache.flink.;org.slf4j;org.apache.log4j;org.apache"
 +
 
 Review comment:
   revert?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [flink] pnowojski commented on a change in pull request #10845: [FLINK-15355][plugins]Classloader restricts access to parent to whitelist.

2020-01-16 Thread GitBox
pnowojski commented on a change in pull request #10845: 
[FLINK-15355][plugins]Classloader restricts access to parent to whitelist.
URL: https://github.com/apache/flink/pull/10845#discussion_r367279519
 
 

 ##
 File path: 
flink-core/src/main/java/org/apache/flink/core/plugin/PluginLoader.java
 ##
 @@ -100,4 +105,97 @@ public P next() {
}
}
 
+   /**
+* Loads all classes from the plugin jar except for explicitly 
white-listed packages (org.apache.flink, logging).
+*
+* No class/resource in the system class loader (everything in lib/) 
can be seen in the plugin except those
+* starting with a whitelist prefix.
+*/
+   private static final class PluginClassLoader extends URLClassLoader {
+   private final ClassLoader flinkClassLoader;
+
+   private final String[] allowedFlinkPackages;
+
+   private final String[] allowedResourcePrefixes;
+
+   PluginClassLoader(URL[] pluginResourceURLs, ClassLoader 
flinkClassLoader, String[] allowedFlinkPackages) {
+   super(pluginResourceURLs, null);
+   this.flinkClassLoader = flinkClassLoader;
+   this.allowedFlinkPackages = allowedFlinkPackages;
+   allowedResourcePrefixes = 
Arrays.stream(allowedFlinkPackages)
+   .map(packageName -> packageName.replace('.', 
'/'))
+   .toArray(String[]::new);
+   }
+
+   @Override
+   protected Class loadClass(final String name, final boolean 
resolve) throws ClassNotFoundException {
+   synchronized (getClassLoadingLock(name)) {
+   final Class loadedClass = 
findLoadedClass(name);
+   if (loadedClass != null) {
+   return resolveIfNeeded(resolve, 
loadedClass);
+   }
+
+   if (isAllowedFlinkClass(name)) {
+   try {
+   return resolveIfNeeded(resolve, 
flinkClassLoader.loadClass(name));
+   } catch (ClassNotFoundException e) {
+   // fallback to resolving it in 
this classloader
+   // for cases where the plugin 
uses org.apache.flink namespace
+   }
+   }
+
+   return super.loadClass(name, resolve);
+   }
+   }
+
+   private Class resolveIfNeeded(final boolean resolve, final 
Class loadedClass) {
+   if (resolve) {
+   resolveClass(loadedClass);
+   }
+   return loadedClass;
+   }
+
+   @Override
+   public URL getResource(final String name) {
+   if (isAllowedFlinkResource(name)) {
+   return flinkClassLoader.getResource(name);
+   }
+
+   URL urlClassLoaderResource = findResource(name);
+
+   if (urlClassLoaderResource != null) {
+   return urlClassLoaderResource;
+   }
+
+   return null;
 
 Review comment:
   ```
   return super.getResource(name);
   ```
   ?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [flink] pnowojski commented on a change in pull request #10845: [FLINK-15355][plugins]Classloader restricts access to parent to whitelist.

2020-01-16 Thread GitBox
pnowojski commented on a change in pull request #10845: 
[FLINK-15355][plugins]Classloader restricts access to parent to whitelist.
URL: https://github.com/apache/flink/pull/10845#discussion_r367279915
 
 

 ##
 File path: 
flink-tests/src/test/java/org/apache/flink/test/plugin/PluginLoaderTest.java
 ##
 @@ -21,6 +21,7 @@
 import org.apache.flink.core.plugin.PluginDescriptor;
 import org.apache.flink.core.plugin.PluginLoader;
 import org.apache.flink.test.plugin.jar.plugina.TestServiceA;
+import org.apache.flink.test.plugin.spi.TestSpi;
 
 Review comment:
   Why have all the tests changed in this commit?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services