modules/enterprise/binding/pom.xml | 2 modules/enterprise/binding/src/main/java/org/rhq/bindings/SandboxedScriptEngine.java | 3 modules/enterprise/binding/src/main/java/org/rhq/bindings/ScriptEngineFactory.java | 60 +++++++++- modules/enterprise/binding/src/test/java/org/rhq/bindings/ScriptEngineTest.java | 9 + modules/enterprise/server/itests/src/test/java/org/rhq/enterprise/client/security/test/JndiAccessTest.java | 4 modules/enterprise/server/itests/src/test/resources/log4j.xml | 4 modules/enterprise/server/plugins/alert-cli/src/main/java/org/rhq/enterprise/server/plugins/alertCli/CliSender.java | 9 - 7 files changed, 77 insertions(+), 14 deletions(-)
New commits: commit 7d14f7484ad758be037694e3599654e4fcb11a88 Author: Lukas Krejci lkrejci@redhat.com Date: Thu Jan 12 16:45:50 2012 +0100
Increasing the log level back to WARN now that the tests are working again.
diff --git a/modules/enterprise/server/itests/src/test/resources/log4j.xml b/modules/enterprise/server/itests/src/test/resources/log4j.xml index 1e37497..bc65329 100644 --- a/modules/enterprise/server/itests/src/test/resources/log4j.xml +++ b/modules/enterprise/server/itests/src/test/resources/log4j.xml @@ -19,7 +19,7 @@ <appender name="CONSOLE" class="org.apache.log4j.ConsoleAppender"> <errorHandler class="org.jboss.logging.util.OnlyOnceErrorHandler"/> <param name="Target" value="System.out"/> - <param name="Threshold" value="INFO"/> + <param name="Threshold" value="WARN"/>
<layout class="org.apache.log4j.PatternLayout"> <!-- The default pattern: Date Priority [Category] Messagen --> @@ -32,7 +32,7 @@
<appender name="FILE" class="org.apache.log4j.RollingFileAppender"> <param name="File" value="target/server-jar-test.log"/> - <param name="Threshold" value="INFO"/> + <param name="Threshold" value="WARN"/> <param name="Append" value="false"/>
<layout class="org.apache.log4j.PatternLayout">
commit 759b64ee6b76525650ebece39459bf9abe5c7dd4 Author: Lukas Krejci lkrejci@redhat.com Date: Thu Jan 12 16:45:20 2012 +0100
Making the script security work with Java 6u27 and up. Due to CVE-2011-3544 the Rhino script engine changed the way it applies security permissions which broke our original sandboxing approach.
diff --git a/modules/enterprise/binding/src/main/java/org/rhq/bindings/SandboxedScriptEngine.java b/modules/enterprise/binding/src/main/java/org/rhq/bindings/SandboxedScriptEngine.java index 7b34c6d..f994f09 100644 --- a/modules/enterprise/binding/src/main/java/org/rhq/bindings/SandboxedScriptEngine.java +++ b/modules/enterprise/binding/src/main/java/org/rhq/bindings/SandboxedScriptEngine.java @@ -40,6 +40,9 @@ import javax.script.ScriptEngineFactory; import javax.script.ScriptException;
/** + * <b>DO NOT USE THIS CLASS DIRECTLY!!!!</b> Use {@link org.rhq.bindings.ScriptEngineFactory#getSecuredScriptEngine(String, org.rhq.bindings.util.PackageFinder, StandardBindings, PermissionCollection)} + * method instead for a reliably secured script engine. + * <p> * This is a decorator class for any other {@link ScriptEngine} implementation * that runs any of the eval methods with the defined set of {@link Permission}s. * <p> diff --git a/modules/enterprise/binding/src/main/java/org/rhq/bindings/ScriptEngineFactory.java b/modules/enterprise/binding/src/main/java/org/rhq/bindings/ScriptEngineFactory.java index 168d2fb..5834922 100644 --- a/modules/enterprise/binding/src/main/java/org/rhq/bindings/ScriptEngineFactory.java +++ b/modules/enterprise/binding/src/main/java/org/rhq/bindings/ScriptEngineFactory.java @@ -25,6 +25,15 @@ import java.beans.Introspector; import java.beans.MethodDescriptor; import java.io.IOException; import java.lang.reflect.Method; +import java.net.URL; +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.CodeSource; +import java.security.PermissionCollection; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import java.security.ProtectionDomain; +import java.security.cert.Certificate; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -79,7 +88,7 @@ public class ScriptEngineFactory { public static ScriptEngine getScriptEngine(String language, PackageFinder packageFinder, StandardBindings bindings) throws ScriptException, IOException { ScriptEngineInitializer initializer = getInitializer(language); - + if (initializer == null) { return null; } @@ -92,6 +101,55 @@ public class ScriptEngineFactory {
return engine; } + + /** + * This method is similar to the {@link #getScriptEngine(String, PackageFinder, StandardBindings)} method + * but additionally applies a security wrapper on the returned script engine so that the scripts execute + * with the provided java permissions. + * + * @see #getScriptEngine(String, PackageFinder, StandardBindings) + */ + public static ScriptEngine getSecuredScriptEngine(final String language, final PackageFinder packageFinder, final StandardBindings bindings, final PermissionCollection permissions) throws ScriptException, IOException { + CodeSource src = new CodeSource(new URL("http://rhq-project.org/scripting"), (Certificate[]) null); + ProtectionDomain scriptDomain = new ProtectionDomain(src, permissions); + AccessControlContext ctx = new AccessControlContext(new ProtectionDomain[] { scriptDomain }); + try { + return AccessController.doPrivileged(new PrivilegedExceptionAction<ScriptEngine>() { + @Override + public ScriptEngine run() throws Exception { + //This might seem a bit excessive but is necessary due to the + //change in security handling in the rhino script engine + //that occured in Java6u27 (due to a CVE desribed here: + //https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2011-3544) + + //In Java 6u26 and earlier, it was enough to wrap a script engine + //in the sandbox and everything would work. + + //Java 6u27 introduced new behavior where the rhino script engine + //remembers the access control context with which it has been + //constructed and combines that with the callers protection domain + //when a script is executed. Because this class has all perms and + //all the code in RHQ that called ScriptEngine.eval* also + //had all perms, the scripts would never be sandboxed even if the call + //was pushed through the SandboxedScriptEngine. + + //This means that the below wrapping is necessary for the security + //to work in java6 pre u27 while the surrounding privileged block + //is necessary for the security to be applied in java6 u27 and later. + return new SandboxedScriptEngine(getScriptEngine(language, packageFinder, bindings), permissions); + } + }, ctx); + } catch (PrivilegedActionException e) { + Throwable cause = e.getCause(); + if (cause instanceof IOException) { + throw (IOException) cause; + } else if (cause instanceof ScriptException) { + throw (ScriptException) cause; + } else { + throw new ScriptException(e); + } + } + }
/** * Injects the values provided in the bindings into the {@link ScriptContext#ENGINE_SCOPE engine scope} diff --git a/modules/enterprise/binding/src/test/java/org/rhq/bindings/ScriptEngineTest.java b/modules/enterprise/binding/src/test/java/org/rhq/bindings/ScriptEngineTest.java index 69874e4..15184c1 100644 --- a/modules/enterprise/binding/src/test/java/org/rhq/bindings/ScriptEngineTest.java +++ b/modules/enterprise/binding/src/test/java/org/rhq/bindings/ScriptEngineTest.java @@ -40,6 +40,7 @@ import org.rhq.bindings.util.PackageFinder; * * @author Lukas Krejci */ +@Test public class ScriptEngineTest {
private static StandardBindings EMPTY_BINDINGS = new StandardBindings(new PrintWriter(System.out), new FakeRhqFacade()); @@ -52,9 +53,7 @@ public class ScriptEngineTest {
@Test public void testSandbox() throws ScriptException, IOException { - ScriptEngine engine = getScriptEngine(); - - SandboxedScriptEngine sandbox = new SandboxedScriptEngine(engine, new StandardScriptPermissions()); + ScriptEngine sandbox = getSecuredScriptEngine();
try { sandbox.eval("java.lang.System.exit(1);"); @@ -90,6 +89,10 @@ public class ScriptEngineTest { return ScriptEngineFactory.getScriptEngine("JavaScript", new PackageFinder(Collections.<File>emptyList()), EMPTY_BINDINGS); }
+ private ScriptEngine getSecuredScriptEngine() throws ScriptException, IOException { + return ScriptEngineFactory.getSecuredScriptEngine("JavaScript", new PackageFinder(Collections.<File>emptyList()), EMPTY_BINDINGS, new StandardScriptPermissions()); + } + private void assertSecurityExceptionPresent(Throwable t) { boolean ok = false; while (t != null) { diff --git a/modules/enterprise/server/itests/src/test/java/org/rhq/enterprise/client/security/test/JndiAccessTest.java b/modules/enterprise/server/itests/src/test/java/org/rhq/enterprise/client/security/test/JndiAccessTest.java index ae848a8..de0d912 100644 --- a/modules/enterprise/server/itests/src/test/java/org/rhq/enterprise/client/security/test/JndiAccessTest.java +++ b/modules/enterprise/server/itests/src/test/java/org/rhq/enterprise/client/security/test/JndiAccessTest.java @@ -181,12 +181,10 @@ public class JndiAccessTest extends AbstractEJB3Test {
private ScriptEngine getEngine(Subject subject) throws ScriptException, IOException { StandardBindings bindings = new StandardBindings(new PrintWriter(System.out), new LocalClient(subject)); - ScriptEngine engine = ScriptEngineFactory.getScriptEngine("JavaScript", new PackageFinder(Collections.<File>emptyList()), bindings);
PermissionCollection perms = new StandardScriptPermissions(); - perms.add(new SerializablePermission("enableSubclassImplementation"));
- return new SandboxedScriptEngine(engine, perms); + return ScriptEngineFactory.getSecuredScriptEngine("JavaScript", new PackageFinder(Collections.<File>emptyList()), bindings, perms); }
private static void checkIsDesiredSecurityException(ScriptException e) { diff --git a/modules/enterprise/server/plugins/alert-cli/src/main/java/org/rhq/enterprise/server/plugins/alertCli/CliSender.java b/modules/enterprise/server/plugins/alert-cli/src/main/java/org/rhq/enterprise/server/plugins/alertCli/CliSender.java index 3aab6a2..43090c7 100644 --- a/modules/enterprise/server/plugins/alert-cli/src/main/java/org/rhq/enterprise/server/plugins/alertCli/CliSender.java +++ b/modules/enterprise/server/plugins/alert-cli/src/main/java/org/rhq/enterprise/server/plugins/alertCli/CliSender.java @@ -127,8 +127,6 @@ public class CliSender extends AlertSender<CliComponent> {
engine = getScriptEngine(alert, scriptOut, config);
- final SandboxedScriptEngine sandbox = new SandboxedScriptEngine(engine, new StandardScriptPermissions()); - InputStream packageBits = getPackageBits(config.packageId, config.repoId);
reader = new BufferedReader(new InputStreamReader(packageBits)); @@ -137,10 +135,11 @@ public class CliSender extends AlertSender<CliComponent> {
final ExceptionHolder exceptionHolder = new ExceptionHolder();
+ final ScriptEngine e = engine; Thread scriptRunner = new Thread(new Runnable() { public void run() { try { - sandbox.eval(rdr); + e.eval(rdr); } catch (ScriptException e) { exceptionHolder.scriptException = e; } @@ -414,8 +413,8 @@ public class CliSender extends AlertSender<CliComponent> { ScriptEngine engine = SCRIPT_ENGINES.poll();
if (engine == null) { - engine = ScriptEngineFactory.getScriptEngine(ENGINE_NAME, new PackageFinder(Collections - .<File> emptyList()), bindings); + engine = ScriptEngineFactory.getSecuredScriptEngine(ENGINE_NAME, new PackageFinder(Collections + .<File> emptyList()), bindings, new StandardScriptPermissions()); } else { ScriptEngineFactory.injectStandardBindings(engine, bindings, true); }
commit 40247b6c476b0a68476480fe5bca118c4db93489 Author: Lukas Krejci lkrejci@redhat.com Date: Thu Jan 12 16:28:10 2012 +0100
making sure that the tests that try to exit the jvm won't make the test suite appear to have succeeded.
diff --git a/modules/enterprise/binding/pom.xml b/modules/enterprise/binding/pom.xml index 0a9dd46..52def8e 100644 --- a/modules/enterprise/binding/pom.xml +++ b/modules/enterprise/binding/pom.xml @@ -209,6 +209,8 @@ <!-- <argLine>-Xdebug -Xnoagent -Djava.compiler=NONE -Xrunjdwp:transport=dt_socket,address=8787,server=y,suspend=y</argLine> --> <argLine>-Djava.security.manager -Djava.security.policy==${project.build.testOutputDirectory}/allow-all.policy</argLine> + <!-- This is important, because some of the tests try to exit the JVM. --> + <failIfNoTests>true</failIfNoTests> </configuration> </plugin>
rhq-commits@lists.fedorahosted.org