You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by jw...@apache.org on 2016/07/04 19:24:11 UTC
[2/2] groovy git commit: Fix Security Tests (closes #363)
Fix Security Tests (closes #363)
Adjust policy file settings for new gradle build and ensure
test support class uses privileged access where appropriate.
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/e21d6226
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/e21d6226
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/e21d6226
Branch: refs/heads/master
Commit: e21d6226b9a4cc411fb0e7d24cf7b6023edc9002
Parents: 440f279
Author: John Wagenleitner <jw...@apache.org>
Authored: Mon Jul 4 11:52:11 2016 -0700
Committer: John Wagenleitner <jw...@apache.org>
Committed: Mon Jul 4 11:52:11 2016 -0700
----------------------------------------------------------------------
security/groovy.policy | 98 +++++++++++++-------
src/test/groovy/security/SecurityTest.java | 27 +++---
.../groovy/security/SecurityTestSupport.java | 46 +++++----
3 files changed, 108 insertions(+), 63 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/e21d6226/security/groovy.policy
----------------------------------------------------------------------
diff --git a/security/groovy.policy b/security/groovy.policy
index 82bc047..3d7d077 100644
--- a/security/groovy.policy
+++ b/security/groovy.policy
@@ -42,27 +42,28 @@ keystore "file:${user.dir}/security/groovykeys";
* would all be in a set of jars. If these jars are signed, the grants could be strengthened to
* by adding a signedBy clause to the grant.
*/
-
-//spg: Looks like james switched the output destination to bin from target/classes
-//in his 7/13/04 update to the eclipse .classpath. This permission allows the
-//Security tests to run properly in eclipse (they were unaffected when running from maven)
+
+// Eclipse default output directory to allow running security tests in that IDE
grant codeBase "file:${user.dir}/bin/-" {
permission java.security.AllPermission;
};
-grant codeBase "file:${user.dir}/target/classes/-" {
+// IntelliJ IDEA default output directory to allow running security tests in that IDE
+grant codeBase "file:${user.dir}/out/-" {
permission java.security.AllPermission;
};
-grant codeBase "file:${user.dir}/target/dist/-" {
+// Gradle build output directory
+grant codeBase "file:${user.dir}/target/-" {
permission java.security.AllPermission;
};
-grant codeBase "file:${user.dir}/target/instrumented-classes/-" {
+grant codeBase "file:${user.dir}/subprojects/groovy-test/target/-" {
permission java.security.AllPermission;
};
-grant codeBase "file:${user.dir}/target/classes/test/-" {
+// Gradle dependency cache
+grant codeBase "file:${user.home}/.gradle/-" {
permission java.security.AllPermission;
};
@@ -91,15 +92,6 @@ grant codeBase "file:${gradle.junit}" {
};
/*
- * When running from ant, this codebase is required.
- * If not running from ant, the codesource will not be found, but will not cause an error.
- * TODO: narrow this down to specific jars
- */
-grant codeBase "file:${user.dir}/target/lib/-" {
- permission java.security.AllPermission;
-};
-
-/*
* When running the junit plugin from within eclipse, this codebase is required.
* If not running from eclipse, the codesource will not be found, but will not cause an error.
*/
@@ -120,6 +112,9 @@ grant codeBase "file:${user.dir}/../../plugins/-" {
*/
grant codeBase "file:/groovy/shell" {
permission java.lang.RuntimePermission "accessDeclaredMembers";
+
+ // For StreamClassloaderInScriptTest.groovy test
+ permission java.io.FilePermission "${user.dir}${/}blahblah.dat", "read,write,delete";
};
/*
@@ -128,7 +123,6 @@ grant codeBase "file:/groovy/shell" {
*/
grant codeBase "file:/groovy/script" {
permission java.lang.RuntimePermission "accessDeclaredMembers";
- //permission java.util.PropertyPermission "groovy.output.dir", "read";
};
/*
@@ -143,7 +137,7 @@ grant codeBase "file:/groovy/testSupport" {
/*
* ================= SecurityTest codebases ===============
* The following grants are for individual security test cases where the
- * codebase is explicity specified (e.g. the script is a raw string rather
+ * codebase is explicitly specified (e.g. the script is a raw string rather
* than read from a file). Note that even though the codebases conform to
* the file URL syntax, they are not physical files: they represent virtual
* codebases.
@@ -151,7 +145,7 @@ grant codeBase "file:/groovy/testSupport" {
* expected to be missing by the test case. It is here as a comment for
* information.
*/
-
+
//Since a codebase is a URL, we can make use of the natural hierarchy permission implications
//to grant a 'global' accessDeclaredPermission here.
grant codeBase "file:/groovy/security/-" {
@@ -182,15 +176,11 @@ grant codeBase "file:${user.dir}/src/test/-" {
// introspection. The groovy code could potentially be changed to eliminate this requirement
// by adding privileged operations in many places.
permission java.lang.RuntimePermission "accessDeclaredMembers";
-
- // SHOULD NOT HAVE TO DO THIS! - Level.getLocalizedName() should have a doPrivileged block.
- // Any test that logs, will encounter this (although it will be masked by the method
- permission java.lang.RuntimePermission "accessClassInPackage.sun.util.logging.resources";
};
grant codeBase "file:${user.dir}/src/test/groovy/security/forbiddenCodeBase.gvy" {
//The following grant is commented out so that the test case will throw an AccessControlException
- permission groovy.security.GroovyCodeSourcePermission "groovy/security/forbiddenCodeBase";
+ //permission groovy.security.GroovyCodeSourcePermission "/groovy/security/forbiddenCodeBase";
};
// Grant permission to .groovy files extracted from a signed jar that has been signedBy "Groovy"
@@ -200,38 +190,74 @@ grant signedBy "Groovy" {
};
grant codeBase "file:${user.dir}/src/test/groovy/bugs/BadScriptNameBug.groovy" {
- permission java.util.PropertyPermission "java.class.path", "read";
- permission java.io.FilePermission "<<ALL FILES>>", "read";
permission java.lang.RuntimePermission "createClassLoader";
};
grant codeBase "file:${user.dir}/src/test/groovy/ClosureMethodTest.groovy" {
- permission java.io.FilePermission "${user.dir}${/}src${/}test${/}-", "read";
- permission java.util.PropertyPermission "file.encoding", "read";
-};
-
-grant codeBase "file:${user.dir}/src/test/groovy/GroovyMethodsTest.groovy" {
- permission java.util.PropertyPermission "*", "read,write";
- //spg 2006-02-09 added ALL FILES execute because a test case
- //invokes Runtime.exec(). This is OK and permission should be granted here.
- permission java.io.FilePermission "<<ALL FILES>>", "execute";
+ permission java.lang.RuntimePermission "getClassLoader";
};
grant codeBase "file:${user.dir}/src/test/groovy/ClosureWithDefaultParamTest.groovy" {
permission java.io.FilePermission "src${/}test${/}-", "read";
+
+ // Required because test calls File.eachLine which eventually invokes
+ // groovy.util.CharsetToolkit#getDefaultSystemCharset which does not wrap
+ // the property access in a privileged block.
permission java.util.PropertyPermission "file.encoding", "read";
};
grant codeBase "file:${user.dir}/src/test/groovy/bugs/ConstructorBug.groovy" {
permission java.lang.RuntimePermission "createClassLoader";
+
+ // Required because GroovyCodeSource calls to File#getCanonicalPath (at least on the Windows FileSystem)
+ permission java.util.PropertyPermission "user.dir", "read";
+
permission java.io.FilePermission "src${/}test${/}groovy${/}bugs${/}TestBase.groovy", "read";
permission java.io.FilePermission "src${/}test${/}groovy${/}bugs${/}TestDerived.groovy", "read";
+ permission java.io.FilePermission "${user.dir}${/}target${/}classes${/}test${/}groovy${/}bugs${/}TestBase.class", "read";
+ permission java.io.FilePermission "${user.dir}${/}target${/}classes${/}test${/}groovy${/}bugs${/}TestDerived.class", "read";
};
grant codeBase "file:${user.dir}/src/test/groovy/script/ScriptTest.groovy" {
permission java.io.FilePermission "src${/}test${/}groovy${/}-", "read";
+ permission groovy.security.GroovyCodeSourcePermission "/groovy/shell";
+
+ // Required because GroovyCodeSource calls to File#getCanonicalPath (at least on the Windows FileSystem)
+ permission java.util.PropertyPermission "user.dir", "read";
+
+ // Required by MethodTestScript.groovy run as a script
+ permission java.lang.RuntimePermission "accessClassInPackage.sun.reflect";
+
+ // Required by StreamClassloaderInScriptTest.groovy run as a script
+ permission java.io.FilePermission "${user.dir}${/}blahblah.dat", "read,write,delete";
+
+ // Required for JUnit to report errors for scripts that are junit test cases
+ permission java.util.PropertyPermission "user.home", "read";
+ permission java.io.FilePermission "${user.home}${/}junit.properties", "read";
};
grant codeBase "file:${user.dir}/src/test/groovy/script/EvalInScript.groovy" {
permission java.io.FilePermission "src${/}test${/}groovy${/}script${/}HelloWorld.groovy", "read";
+ permission groovy.security.GroovyCodeSourcePermission "/groovy/shell";
+
+ // Required because GroovyCodeSource calls to File#getCanonicalPath (at least on the Windows FileSystem)
+ permission java.util.PropertyPermission "user.dir", "read";
+};
+
+grant codeBase "file:${user.dir}/src/test/groovy/script/MethodTestScript.groovy" {
+ permission java.lang.RuntimePermission "accessClassInPackage.sun.reflect";
+};
+
+grant codeBase "file:${user.dir}/src/test/groovy/script/StreamClassloaderInScriptTest.groovy" {
+ permission groovy.security.GroovyCodeSourcePermission "/groovy/shell";
+ permission java.io.FilePermission "${user.dir}${/}blahblah.dat", "read,write,delete";
+};
+
+grant {
+ // Required to load Global AST Transforms
+ permission java.io.FilePermission "${user.dir}${/}target${/}resources${/}main${/}META-INF${/}services${/}-", "read";
+ permission java.io.FilePermission "${user.dir}${/}target${/}libs${/}-", "read";
+
+ // Required for StreamClassloaderInScriptTest
+ permission java.io.FilePermission "${java.home}${/}-", "read";
};
http://git-wip-us.apache.org/repos/asf/groovy/blob/e21d6226/src/test/groovy/security/SecurityTest.java
----------------------------------------------------------------------
diff --git a/src/test/groovy/security/SecurityTest.java b/src/test/groovy/security/SecurityTest.java
index c9f5d15..eda8e91 100644
--- a/src/test/groovy/security/SecurityTest.java
+++ b/src/test/groovy/security/SecurityTest.java
@@ -20,11 +20,12 @@ package groovy.security;
import groovy.lang.GroovyCodeSource;
import org.codehaus.groovy.control.CompilationFailedException;
-import org.junit.Ignore;
import java.io.File;
import java.io.IOException;
import java.net.URL;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
import java.security.Security;
import java.util.PropertyPermission;
@@ -33,13 +34,9 @@ import java.util.PropertyPermission;
* behavior (e.g. ensuring that GroovyCodeSources may only be created for which proper permissions exist).
* Other tests run .groovy scripts under a secure environment and ensure that the proper permissions
* are required for success.
- * <p>
- * Todo: find out why the marked tests are environment specific and why security tests are not
- * running on the build server.
*
* @author Steve Goetze
*/
-@Ignore("does not work at the moment, please fix me")
public class SecurityTest extends SecurityTestSupport {
public void testForbiddenProperty() {
@@ -63,7 +60,16 @@ public class SecurityTest extends SecurityTestSupport {
//Check that the Security package.access control works.
public void testPackageAccess() {
String script = "new javax.print.PrintException();";
- Security.setProperty("package.access", "javax.print");
+ // Use our privileged access in order to prevent checks lower in the call stack. Otherwise we would have
+ // to grant access to IDE unit test runners and unit test libs. We only care about testing the call stack
+ // higher upstream from this point of execution.
+ AccessController.doPrivileged(new PrivilegedAction<Void>() {
+ @Override
+ public Void run() {
+ Security.setProperty("package.access", "javax.print");
+ return null;
+ }
+ });
//This should throw an ACE because its codeBase does not allow access to javax.print
assertExecute(script, "/groovy/security/javax/print/deny", new RuntimePermission("accessClassInPackage.javax.print"));
//This should not throw an ACE because groovy.policy grants the codeBase access to javax.print
@@ -78,11 +84,6 @@ public class SecurityTest extends SecurityTestSupport {
assertExecute(new File("src/test/groovy/ClosureMethodTest.groovy"), null);
}
- public void testGroovyMethodsTest_FAILS() {
- if (notYetImplemented()) return;
- assertExecute(new File("src/test/groovy/GroovyMethodsTest.groovy"), null);
- }
-
public void testClosureWithDefaultParamTest() {
assertExecute(new File("src/test/groovy/ClosureWithDefaultParamTest.groovy"), null);
}
@@ -91,6 +92,10 @@ public class SecurityTest extends SecurityTestSupport {
assertExecute(new File("src/test/groovy/script/ScriptTest.groovy"), null);
}
+ public void testConstructorBug() {
+ assertExecute(new File("src/test/groovy/bugs/ConstructorBug.groovy"), null);
+ }
+
//Mailing list post by Richard Hensley reporting a CodeSource bug. A GroovyCodeSource created
//with a URL was causing an NPE.
public void testCodeSource() throws IOException, CompilationFailedException {
http://git-wip-us.apache.org/repos/asf/groovy/blob/e21d6226/src/test/groovy/security/SecurityTestSupport.java
----------------------------------------------------------------------
diff --git a/src/test/groovy/security/SecurityTestSupport.java b/src/test/groovy/security/SecurityTestSupport.java
index bb63a7b..5a161e8 100644
--- a/src/test/groovy/security/SecurityTestSupport.java
+++ b/src/test/groovy/security/SecurityTestSupport.java
@@ -29,7 +29,6 @@ import junit.framework.TestResult;
import junit.framework.TestSuite;
import junit.textui.ResultPrinter;
import org.codehaus.groovy.runtime.InvokerHelper;
-import org.junit.Ignore;
import java.io.File;
import java.io.PrintStream;
@@ -40,8 +39,7 @@ import java.util.Enumeration;
/**
* @author Steve Goetze
*/
-@Ignore("base class for security tests")
-public class SecurityTestSupport extends GroovyTestCase {
+public abstract class SecurityTestSupport extends GroovyTestCase {
private static final String POLICY_FILE = "security/groovy.policy";
private static int counter = 0;
private static boolean securityDisabled;
@@ -262,17 +260,26 @@ public class SecurityTestSupport extends GroovyTestCase {
* is non-null, then this invocation expects an AccessControlException with missingPermission
* as the reason. If missingPermission is null, the script is expected to execute successfully.
*/
- protected void assertExecute(File file, Permission missingPermission) {
+ protected void assertExecute(final File file, final Permission missingPermission) {
if (!isSecurityAvailable()) {
return;
}
- GroovyCodeSource gcs = null;
- try {
- gcs = new GroovyCodeSource(file);
- } catch (IOException fnfe) {
- fail(fnfe.toString());
- }
- parseAndExecute(gcs, missingPermission);
+ // Use our privileged access in order to prevent checks lower in the call stack. Otherwise we would have
+ // to grant access to IDE unit test runners and unit test libs. We only care about testing the call stack
+ // higher upstream from this point of execution.
+ AccessController.doPrivileged(new PrivilegedAction<Void>() {
+ @Override
+ public Void run() {
+ GroovyCodeSource gcs = null;
+ try {
+ gcs = new GroovyCodeSource(file);
+ } catch (IOException fnfe) {
+ fail(fnfe.toString());
+ }
+ parseAndExecute(gcs, missingPermission);
+ return null;
+ }
+ });
}
/*
@@ -280,13 +287,20 @@ public class SecurityTestSupport extends GroovyTestCase {
* is non-null, then this invocation expects an AccessControlException with missingPermission
* as the reason. If missingPermission is null, the script is expected to execute successfully.
*/
- protected void assertExecute(String scriptStr, String codeBase, Permission missingPermission) {
+ protected void assertExecute(final String scriptStr, String codeBase, final Permission missingPermission) {
if (!isSecurityAvailable()) {
return;
}
- if (codeBase == null) {
- codeBase = "/groovy/security/test";
- }
- parseAndExecute(new GroovyCodeSource(scriptStr, generateClassName(), codeBase), missingPermission);
+ final String effectiveCodeBase = (codeBase != null) ? codeBase : "/groovy/security/test";
+ // Use our privileged access in order to prevent checks lower in the call stack. Otherwise we would have
+ // to grant access to IDE unit test runners and unit test libs. We only care about testing the call stack
+ // higher upstream from this point of execution.
+ AccessController.doPrivileged(new PrivilegedAction<Void>() {
+ @Override
+ public Void run() {
+ parseAndExecute(new GroovyCodeSource(scriptStr, generateClassName(), effectiveCodeBase), missingPermission);
+ return null;
+ }
+ });
}
}