New substitution for setSecurityManager

Author: vjovanov

docs/reference-manual/native-image/Compatibility.md

@@ -43,7 +43,8 @@ Note that `invokedynamic` use cases generated by `javac` for, for example, Java
 
 ### Security Manager
 
-Native Image will not allow setting `-Djava.security.manager` to anything but `"disallow"` at image build time or image run time.
+Native Image will produce images as if `-Djava.security.manager` was set to `disallow`. 
+At image run time, calls to `java.lang.System#setSecurityManager` exit the program with error code `255` if `-Djava.security.manager` is set to anything but `disallow` at program startup.
 
 ## Features That May Operate Differently in a Native Image
 

substratevm/CHANGELOG.md

@@ -14,7 +14,7 @@ This changelog summarizes major changes to GraalVM Native Image.
 * (GR-47937) Make the lambda-class name format in Native-Image consistent with the JDK name format.
 * (GR-45651) Methods, fields and constructors of `Object`, primitive classes and array classes are now registered by default for reflection.
 * (GR-45651) The Native Image agent now tracks calls to `ClassLoader.findSystemClass`, `ObjectInputStream.resolveClass` and `Bundles.of`, and registers resource bundles as bundle name-locale pairs.
-* (GR-49807) Before this change the function `System#setSecurityManager` was always halting program execution with a VM error. This was inconvenient as the VM error prints an uncomprehensible error message and prevents further continuation of the program. For cases where the program is expected to throw an exception when  `System#setSecurityManager` is called, execution on Native Image was not possible. Now, `System#setSecurityManager` throws an `java.lang.UnsupportedOperationException` by default. If the property `java.security.manager` is set to `allow` the program will print a user-readable stack trace and exit with code `99`.
+* (GR-49807) Before this change the function `System#setSecurityManager` was always halting program execution with a VM error. This was inconvenient as the VM error prints an uncomprehensible error message and prevents further continuation of the program. For cases where the program is expected to throw an exception when  `System#setSecurityManager` is called, execution on Native Image was not possible. Now, `System#setSecurityManager` throws an `java.lang.UnsupportedOperationException` by default. If the property `java.security.manager` is set to `allow` the program will print a user-readable stack trace and exit with code `99`. Value of `java.security.manager` that would be set at build time is completely ignored at run time.
 
 ## GraalVM for JDK 21 (Internal Version 23.1.0)
 * (GR-35746) Lower the default aligned chunk size from 1 MB to 512 KB for the serial and epsilon GCs, reducing memory usage and image size in many cases.

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/JavaMainWrapper.java

@@ -35,6 +35,7 @@
 import java.util.List;
 import java.util.function.BooleanSupplier;
 
+import jdk.graal.compiler.word.Word;
 import org.graalvm.nativeimage.CurrentIsolate;
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.Isolate;
@@ -80,8 +81,6 @@
 import com.oracle.svm.util.ClassUtil;
 import com.oracle.svm.util.ReflectionUtil;
 
-import jdk.graal.compiler.word.Word;
-
 @InternalVMMethod
 public class JavaMainWrapper {
     /*
@@ -225,13 +224,6 @@ private static int runCore0() {
             // Ensure that native code using JNI_GetCreatedJavaVMs finds this isolate.
             JNIJavaVMList.addJavaVM(JNIFunctionTables.singleton().getGlobalJavaVM());
 
-            String smp = System.getProperty("java.security.manager");
-            if (smp != null && !"disallow".equals(smp)) {
-                System.err.println("Error: Property '-Djava.security.manager' is set. SecurityManager is not supported by Native Image. Please unset this property. " +
-                                "Exiting the program to prevent misinterpretation of the set SecurityManager.");
-                System.exit(-1);
-            }
-
             /*
              * Invoke the application's main method. Invoking the main method via a method handle
              * preserves exceptions, while invoking the main method via reflection would wrap

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java

@@ -1228,8 +1228,8 @@ private void checkPackageAccess(SecurityManager sm, ClassLoader ccl, boolean che
     }
 
     /**
-     * Never called, but consider reachable by a that happens bug in an interaction between PE
-     * and @Delete in analysis.
+     * GR-49983: Never called and partially evaluated, but fails if removed. Can be removed when
+     * GR-49983 is fixed.
      */
     @KeepOriginal
     @SuppressWarnings({"deprecation", "unused"})

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JavaLangSubstitutions.java

@@ -44,6 +44,7 @@
 import java.util.stream.Stream;
 
 import org.graalvm.nativeimage.ImageSingletons;
+import org.graalvm.nativeimage.LogHandler;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
 import org.graalvm.nativeimage.hosted.FieldValueTransformer;
@@ -66,12 +67,14 @@
 import com.oracle.svm.core.hub.DynamicHub;
 import com.oracle.svm.core.jdk.JavaLangSubstitutions.ClassValueSupport;
 import com.oracle.svm.core.monitor.MonitorSupport;
+import com.oracle.svm.core.option.HostedOptionKey;
 import com.oracle.svm.core.snippets.SubstrateForeignCallTarget;
 import com.oracle.svm.core.thread.JavaThreads;
 import com.oracle.svm.core.thread.VMOperation;
 import com.oracle.svm.core.util.VMError;
 import com.oracle.svm.util.ReflectionUtil;
 
+import jdk.graal.compiler.options.Option;
 import jdk.graal.compiler.replacements.nodes.BinaryMathIntrinsicNode;
 import jdk.graal.compiler.replacements.nodes.BinaryMathIntrinsicNode.BinaryOperation;
 import jdk.graal.compiler.replacements.nodes.UnaryMathIntrinsicNode;
@@ -342,21 +345,6 @@ final class Target_java_lang_System {
     @Alias private static PrintStream err;
     @Alias private static InputStream in;
 
-    /**
-     * This substitution can be removed when the warning for setting `-Djava.security.manager=allow`
-     * in the image build is converted into an error.
-     */
-    @Substitute
-    private static boolean allowSecurityManager() {
-        if (SystemPropertiesSupport.singleton().isSecurityManagerAllowed()) {
-            /* Fail fatally in case someone tried to set the SM at build time */
-            System.err.println("Error: Property '-Djava.security.manager' was set at build time. Security manager is not supported by native image. Please unset this property. " +
-                            "Exiting the program to prevent misinterpretation of the set SecurityManager.");
-            System.exit(-1);
-        }
-        return false;
-    }
-
     @Substitute
     private static void setIn(InputStream is) {
         in = is;
@@ -417,6 +405,41 @@ private static String getProperty(String key, String def) {
 
     @Alias
     private static native void checkKey(String key);
+
+    /**
+     * Force System.Never in case it was set at build time via the `-Djava.security.manager=allow`
+     * passed to the image builder.
+     */
+    @Alias @RecomputeFieldValue(kind = Kind.FromAlias, isFinal = true) //
+    private static int allowSecurityManager = 1;
+
+    @Substitute
+    @TargetElement(onlyWith = JavaLangSubstitutions.UseSecurityManagerPropertyAtRuntime.class)
+    private static void setSecurityManager(SecurityManager s) {
+        /* We read properties interpreted at isolate creation as that is what happens on the JVM */
+        String smp = SystemPropertiesSupport.singleton().getSavedProperties().get("java.security.manager");
+        if (smp != null && !smp.equals("disallow")) {
+            /*
+             * The strict failure is needed as the security precaution: In case a user does not read
+             * our documentation, uses this deprecated API marked for removal, and passes
+             * "-Djava.security.manager=allow" at runtime, and accidentally catches the
+             * UnsupportedOperationException, we don't want to compromise their security.
+             */
+            System.err.println("""
+                            Fatal error: Property '-Djava.security.manager' is set, but SecurityManager is not supported by Native Image. Please unset this property.
+                            Exiting the program to prevent misinterpretation of the set SecurityManager at:""");
+
+            for (var traceElement : new UnsupportedOperationException().getStackTrace()) {
+                System.err.println("\tat " + traceElement);
+            }
+
+            /* bypasses possible filters on System.exit */
+            ImageSingletons.lookup(LogHandler.class).fatalError();
+        }
+
+        throw new UnsupportedOperationException(
+                        "The Security Manager is deprecated and will be removed in a future release");
+    }
 }
 
 final class NotAArch64 implements BooleanSupplier {
@@ -622,6 +645,9 @@ public static Enumeration<URL> findResources(String name) {
     // Checkstyle: resume
 }
 
+/**
+ * This substitution should not be needed: GR-49983.
+ */
 @TargetClass(value = jdk.internal.logger.LoggerFinderLoader.class)
 final class Target_jdk_internal_logger_LoggerFinderLoader {
     // Checkstyle: stop
@@ -660,6 +686,18 @@ public Object transform(Object receiver, Object originalValue) {
 /** Dummy class to have a class with the file's name. */
 public final class JavaLangSubstitutions {
 
+    public static class UseSecurityManagerPropertyAtRuntime implements BooleanSupplier {
+        public static class Options {
+            @Option(help = "Used only for testing as exiting the program shadows other working tests, please do not use in production.")//
+            public static final HostedOptionKey<Boolean> TestingSecurityViolationUseSecurityManagerPropertyAtRuntime = new HostedOptionKey<>(true);
+        }
+
+        @Override
+        public boolean getAsBoolean() {
+            return Options.TestingSecurityViolationUseSecurityManagerPropertyAtRuntime.getValue();
+        }
+    }
+
     public static final class StringUtil {
         /**
          * Returns a character from a string at {@code index} position based on the encoding format.

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/SystemPropertiesSupport.java

@@ -30,6 +30,7 @@
 import java.util.Properties;
 import java.util.function.Supplier;
 
+import jdk.graal.compiler.api.replacements.Fold;
 import org.graalvm.nativeimage.ImageInfo;
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.Platform;
@@ -40,8 +41,6 @@
 import com.oracle.svm.core.config.ConfigurationValues;
 import com.oracle.svm.core.util.VMError;
 
-import jdk.graal.compiler.api.replacements.Fold;
-
 /**
  * This class maintains the system properties at run time.
  *
@@ -90,8 +89,6 @@ public abstract class SystemPropertiesSupport implements RuntimeSystemProperties
     private final String hostOS = System.getProperty("os.name");
     // needed as fallback for platforms that don't implement osNameValue
 
-    private final boolean securityManagerAllowed = System.getProperty("java.security.manager") != null && !"disallow".equals(System.getProperty("java.security.manager"));
-
     private volatile boolean fullyInitialized;
 
     @Fold
@@ -319,9 +316,4 @@ protected String osNameValue() {
     }
 
     protected abstract String osVersionValue();
-
-    @Fold
-    public boolean isSecurityManagerAllowed() {
-        return securityManagerAllowed;
-    }
 }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGeneratorRunner.java

@@ -132,13 +132,6 @@ public void run() {
                 throw UserError.abort("Unknown options: %s", String.join(" ", remainingArguments));
             }
 
-            var smp = System.getProperty("java.security.manager");
-            if (smp != null && !"disallow".equals(smp)) {
-                /* Warning will be promoted to an `UserError.abort` in future builds. */
-                LogUtils.warning("The flag '-Djava.security.manager' was passed to the image builder which is not allowed." +
-                                " Native Image does not allow setting the security manager at runtime so this feature must be disabled. In the future release this warning will become an error.");
-            }
-
             Integer checkDependencies = SubstrateOptions.CheckBootModuleDependencies.getValue(imageClassLoader.classLoaderSupport.getParsedHostedOptions());
             if (checkDependencies > 0) {
                 checkBootModuleDependencies(checkDependencies > 1);