Skip to content

Commit

Permalink
Stop restricting read access to the file system
Browse files Browse the repository at this point in the history
This is just too complicated to get right (on all platforms).
  • Loading branch information
veithen committed May 27, 2024
1 parent bb0cd25 commit e02d96d
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 254 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# hermetic-maven-plugin

This Maven plugin configures the Maven build so that tests are executed hermetically, i.e. without access to remote resources and files outside of the project directory. To achieve this the plugin generates a Java 2 security policy and configures a custom security manager. By default the corresponding command line arguments are added to the `argLine` property so that they are picked up by [maven-surefire-plugin](http://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#argLine).
This Maven plugin configures the Maven build so that tests are executed hermetically, i.e. without access to remote resources. To achieve this the plugin generates a Java 2 security policy and configures a custom security manager. By default the corresponding command line arguments are added to the `argLine` property so that they are picked up by [maven-surefire-plugin](http://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#argLine).

## Making maven-invoker-plugin executions hermetic

Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

<groupId>com.github.veithen.maven</groupId>
<artifactId>hermetic-maven-plugin</artifactId>
<version>0.7.2-SNAPSHOT</version>
<version>0.8.0-SNAPSHOT</version>
<packaging>maven-plugin</packaging>

<url>https://github.com/veithen/hermetic-maven-plugin</url>
Expand Down
41 changes: 0 additions & 41 deletions src/it/cross-project/child/pom.xml

This file was deleted.

30 changes: 0 additions & 30 deletions src/it/cross-project/child/src/test/java/PermissionTest.java

This file was deleted.

55 changes: 0 additions & 55 deletions src/it/cross-project/pom.xml

This file was deleted.

Empty file removed src/it/cross-project/somefile.txt
Empty file.
7 changes: 0 additions & 7 deletions src/it/simple/src/test/java/PermissionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,6 @@ public void testLocalURLAccess() throws Exception {
}
}

@Test
public void testFileSystemAccess() throws Exception {
assertThrows(
SecurityException.class,
() -> new File(System.getProperty("user.home"), "somefile").listFiles());
}

@Test
public void testUserHomeDirectoryCheck() throws Exception {
new File(System.getProperty("user.home")).isDirectory();
Expand Down
114 changes: 13 additions & 101 deletions src/main/java/com/github/veithen/maven/hermetic/GeneratePolicyMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Properties;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.apache.maven.artifact.Artifact;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.plugin.AbstractMojo;
import org.apache.maven.plugin.MojoExecution;
Expand All @@ -62,17 +58,6 @@
defaultPhase = LifecyclePhase.GENERATE_TEST_RESOURCES,
threadSafe = true)
public final class GeneratePolicyMojo extends AbstractMojo {
private static final String[] defaultSafeMethods = {
"org.apache.tools.ant.types.Path.addExisting",
"org.apache.tools.ant.util.JavaEnvUtils.getJdkExecutable",
};

private static final String[] miscFiles = {
// The Maven version distributed with RHEL uses a modified Apache HttpClient that reads this
// file.
"/usr/share/publicsuffix/effective_tld_names.dat",
};

@Parameter(property = "project", readonly = true, required = true)
private MavenProject project;

Expand All @@ -99,9 +84,6 @@ public final class GeneratePolicyMojo extends AbstractMojo {
@Parameter(defaultValue = "false", required = true)
private boolean allowExec;

@Parameter(defaultValue = "false", required = true)
private boolean allowCrossProjectAccess;

@Parameter(defaultValue = "argLine", required = true)
private String property;

Expand All @@ -111,16 +93,6 @@ public final class GeneratePolicyMojo extends AbstractMojo {
@Parameter(defaultValue = "false", required = true)
private boolean generatePolicyOnly;

private static boolean isDescendant(File dir, File path) {
do {
if (path.equals(dir)) {
return true;
}
path = path.getParentFile();
} while (path != null);
return false;
}

@Override
public void execute() throws MojoExecutionException, MojoFailureException {
Log log = getLog();
Expand All @@ -129,79 +101,11 @@ public void execute() throws MojoExecutionException, MojoFailureException {
return;
}

File projectDir = project.getBasedir();
if (allowCrossProjectAccess) {
File dir = projectDir;
while ((dir = dir.getParentFile()) != null) {
if (new File(dir, "pom.xml").exists()) {
projectDir = dir;
}
}
}

outputFile.getParentFile().mkdirs();
try (Writer out = new OutputStreamWriter(new FileOutputStream(outputFile), "utf-8")) {
PolicyWriter writer = new PolicyWriter(out, log);
writer.start();

// If the JAVA_HOME environment variable is set to a symlink, then the java.home system
// property will point to the target of the symlink, so we need to use the original
// JAVA_HOME value.
String javaHomeString = System.getenv("JAVA_HOME");
if (javaHomeString == null) {
javaHomeString = System.getProperty("java.home");
}
File javaHome = new File(javaHomeString);
File jdkHome = javaHome.getName().equals("jre") ? javaHome.getParentFile() : javaHome;
if (log.isDebugEnabled()) {
log.debug("JDK home is " + jdkHome);
}
writer.generateDirPermissions(jdkHome, Integer.MAX_VALUE, false);
String extDirs = System.getProperty("java.ext.dirs");
if (extDirs != null) {
List<File> dirs =
Stream.of(extDirs.split(Pattern.quote(File.pathSeparator)))
.map(File::new)
.filter(dir -> !isDescendant(jdkHome, dir))
.collect(Collectors.toList());
for (File dir : dirs) {
writer.generateDirPermissions(dir, 1, false);
}
}
writer.generateDirPermissions(
new File(System.getProperty("maven.home")), Integer.MAX_VALUE, false);
writer.generateDirPermissions(
new File(session.getSettings().getLocalRepository()), 0, false);
writer.generateDirPermissions(projectDir, 0, false);
writer.writePermission(
new FilePermission(
session.getRequest().getUserToolchainsFile().getAbsolutePath(),
"read"));
for (MavenProject project : session.getProjects()) {
File file = project.getArtifact().getFile();
if (file != null) {
writer.writePermission(new FilePermission(file.getAbsolutePath(), "read"));
}
for (Artifact attachedArtifact : project.getAttachedArtifacts()) {
writer.writePermission(
new FilePermission(
attachedArtifact.getFile().getAbsolutePath(), "read"));
}
}
for (String dir :
new String[] {
project.getBuild().getDirectory(), System.getProperty("java.io.tmpdir")
}) {
writer.generateDirPermissions(new File(dir), 0, true);
}
// Some code (like maven-bundle-plugin) uses File#isDirectory() on the home directory.
// Allow this, but don't allow access to other files.
writer.writePermission(new FilePermission(System.getProperty("user.home"), "read"));
for (String file : miscFiles) {
if (new File(file).exists()) {
writer.writePermission(new FilePermission(file, "read"));
}
}
writer.writePermission(
new SocketPermission("localhost", "connect,listen,accept,resolve"));
for (NetworkInterface iface :
Expand All @@ -215,8 +119,18 @@ public void execute() throws MojoExecutionException, MojoFailureException {
}
}
}
if (allowExec) {
writer.writePermission(new FilePermission("<<ALL FILES>>", "execute"));
writer.writePermission(
new FilePermission(
"<<ALL FILES>>",
allowExec ? "read,readlink,execute" : "read,readlink"));
for (String dir :
new String[] {
project.getBuild().getDirectory(), System.getProperty("java.io.tmpdir")
}) {
writer.writePermission(new FilePermission(dir, "read,readlink,write"));
writer.writePermission(
new FilePermission(
new File(dir, "-").toString(), "read,readlink,write,delete"));
}
writer.end();
} catch (IOException ex) {
Expand Down Expand Up @@ -259,11 +173,9 @@ public void execute() throws MojoExecutionException, MojoFailureException {

args.add("-Xbootclasspath/a:" + securityManagerJarFile.toString());
args.add("-Djava.security.manager=com.github.veithen.hermetic.HermeticSecurityManager");
List<String> allSafeMethods = new ArrayList<>(Arrays.asList(defaultSafeMethods));
if (safeMethods != null) {
allSafeMethods.addAll(Arrays.asList(safeMethods));
args.add("-Dhermetic.safeMethods=" + String.join(",", Arrays.asList(safeMethods)));
}
args.add("-Dhermetic.safeMethods=" + String.join(",", allSafeMethods));
}
// "==" sets the policy instead of adding additional permissions.
args.add("-Djava.security.policy==" + outputFile.getAbsolutePath().replace('\\', '/'));
Expand Down
18 changes: 0 additions & 18 deletions src/main/java/com/github/veithen/maven/hermetic/PolicyWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
*/
package com.github.veithen.maven.hermetic;

import java.io.File;
import java.io.FilePermission;
import java.io.IOException;
import java.io.Writer;
import java.security.Permission;
Expand Down Expand Up @@ -70,22 +68,6 @@ void writePermission(Permission permission) throws IOException {
actions.isEmpty() ? null : actions);
}

void generateDirPermissions(File dir, int maxDepth, boolean allowWrite) throws IOException {
String rootActions = allowWrite ? "read,readlink,write" : "read,readlink";
String actions = allowWrite ? "read,readlink,write,delete" : "read,readlink";
for (PathSpec pathSpec :
PathUtil.enumeratePaths(dir.getAbsoluteFile().toPath(), maxDepth)) {
writePermission(
new FilePermission(
pathSpec.path().toString(),
pathSpec.depth() == 0 ? rootActions : actions));
if (pathSpec.directory()) {
writePermission(
new FilePermission(pathSpec.path().resolve("-").toString(), actions));
}
}
}

void end() throws IOException {
out.write("};\n");
}
Expand Down

0 comments on commit e02d96d

Please sign in to comment.