Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#286] disable SecurityManager starting with Java 21 #287

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,22 @@
<compilerArg>-Xlint:all</compilerArg>
</compilerArgs>
</configuration>
<executions>
<execution>
<id>compile-java21</id>
<phase>compile</phase>
<goals>
<goal>compile</goal>
</goals>
<configuration>
<compileSourceRoots>
<compileSourceroot>${project.basedir}/src/main/java21</compileSourceroot>
</compileSourceRoots>
<!-- cannot use multireleaseOutput, because we are creating a Java 8 class file -->
<outputDirectory>${project.build.outputDirectory}/META-INF/versions/21</outputDirectory>
keeganwitt marked this conversation as resolved.
Show resolved Hide resolved
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.codehaus.plexus</groupId>
Expand Down Expand Up @@ -343,6 +359,13 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<version>3.4.2</version>
<configuration>
<archive>
<manifestEntries>
<Multi-Release>true</Multi-Release>
</manifestEntries>
</archive>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
Expand Down
22 changes: 6 additions & 16 deletions src/main/java/org/codehaus/gmavenplus/mojo/ShellMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@
import org.apache.maven.plugins.annotations.Parameter;
import org.apache.maven.plugins.annotations.ResolutionScope;
import org.codehaus.gmavenplus.model.internal.Version;
import org.codehaus.gmavenplus.util.DefaultSecurityManagerSetter;
import org.codehaus.gmavenplus.util.NoExitSecurityManager;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.net.MalformedURLException;
import org.codehaus.gmavenplus.util.SecurityManagerSetter;

import static org.codehaus.gmavenplus.mojo.ExecuteMojo.GROOVY_4_0_0_RC_1;
import static org.codehaus.gmavenplus.util.ReflectionUtils.findConstructor;
Expand Down Expand Up @@ -91,16 +93,10 @@ public void execute() throws MojoExecutionException {
}

if (groovyVersionSupportsAction()) {
final SecurityManager defaultSecurityManager = System.getSecurityManager();
final SecurityManagerSetter securityManagerSetter = new DefaultSecurityManagerSetter(getLog(), this.allowSystemExits);

try {
if (!allowSystemExits) {
getLog().warn("JEP 411 deprecated Security Manager in Java 17 for removal. Therefore `allowSystemExits` is also deprecated for removal.");
try {
System.setSecurityManager(new NoExitSecurityManager());
} catch (UnsupportedOperationException e) {
getLog().warn("Attempted to use Security Manager in a JVM where it's disabled by default. You might try `-Djava.security.manager=allow` to override this.");
}
}
securityManagerSetter.setNoExitSecurityManager();

// get classes we need with reflection
Class<?> shellClass = classWrangler.getClass(groovyAtLeast(GROOVY_4_0_0_ALPHA1) ? "org.apache.groovy.groovysh.Groovysh" : "org.codehaus.groovy.tools.shell.Groovysh");
Expand All @@ -127,13 +123,7 @@ public void execute() throws MojoExecutionException {
} catch (InstantiationException e) {
throw new MojoExecutionException("Error occurred while instantiating a Groovy class from classpath.", e);
} finally {
if (!allowSystemExits) {
try {
System.setSecurityManager(defaultSecurityManager);
} catch (UnsupportedOperationException e) {
getLog().warn("Attempted to use Security Manager in a JVM where it's disabled by default. You might try `-Djava.security.manager=allow` to override this.");
}
}
securityManagerSetter.revertToPreviousSecurityManager();
}
} else {
getLog().error("Your Groovy version (" + classWrangler.getGroovyVersionString() + ") doesn't support running a shell. The minimum version of Groovy required is " + minGroovyVersion + ". Skipping shell startup.");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.codehaus.gmavenplus.util;

import org.apache.maven.plugin.logging.Log;

public abstract class AbstractSecurityManagerSetter implements SecurityManagerSetter{

private final boolean allowSystemExits;

private final Log log;

public AbstractSecurityManagerSetter(final Log log, final boolean allowSystemExits) {
this.log = log;
this.allowSystemExits = allowSystemExits;
}

protected boolean getAllowSystemExits() {
return allowSystemExits;
}

protected Log getLog() {
return log;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package org.codehaus.gmavenplus.util;

import java.util.concurrent.atomic.AtomicReference;
import org.apache.maven.plugin.logging.Log;

public class DefaultSecurityManagerSetter extends AbstractSecurityManagerSetter {

private final AtomicReference<SecurityManager> previousSecurityManager = new AtomicReference<>();

public DefaultSecurityManagerSetter(Log log, final boolean allowSystemExits) {
super(log, allowSystemExits);
}

@Override
public void setNoExitSecurityManager() {
if (this.getAllowSystemExits()) {
return;
}

this.previousSecurityManager.set(System.getSecurityManager());

getLog().warn("Setting a security manager is deprecated. Running this build with Java 21 or newer might result in different behaviour.");
System.setSecurityManager(new NoExitSecurityManager());
}

@Override
public void revertToPreviousSecurityManager() {
if (this.getAllowSystemExits()) {
return;
}

this.getPreviousSecurityManager().getAndUpdate((previousSecurityManager -> {
if (previousSecurityManager == null) {
return null;
}

System.setSecurityManager(previousSecurityManager);

return null;
}));
}

protected AtomicReference<SecurityManager> getPreviousSecurityManager() {
return previousSecurityManager;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package org.codehaus.gmavenplus.util;

/**
* Sets a custom security manager and returns the previous instance.
* Can also revert to the previous security Manager.
* <p>
* Implementation notice: For Java 21 and above,
* the implementation must be a no-op and issue a warning.
*
*/
public interface SecurityManagerSetter {

/**
* For Java until 20, this method should set the given Security manager if property {@code allowSystemExits} is set.
*/
void setNoExitSecurityManager();

void revertToPreviousSecurityManager();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package org.codehaus.gmavenplus.util;

import java.util.concurrent.atomic.AtomicReference;
import org.apache.maven.plugin.logging.Log;

public class DefaultSecurityManagerSetter extends AbstractSecurityManagerSetter {

public DefaultSecurityManagerSetter(Log log, final boolean allowSystemExits) {
super(log, allowSystemExits);
}

@Override
public void setNoExitSecurityManager() {
if (this.getAllowSystemExits()) {
return;
}

getLog().warn("Setting a security manager is not supported starting with Java 21.");
}

@Override
public void revertToPreviousSecurityManager() {
if (this.getAllowSystemExits()) {
return;
}

getLog().warn("Setting a security manager is not supported starting with Java 21.");

}
}
Loading