From 075f02da930c934ddb2ef67185c7567dad39a6f2 Mon Sep 17 00:00:00 2001 From: Babneet Singh Date: Thu, 20 Jan 2022 11:14:06 -0500 Subject: [PATCH 1/3] Update java.security.manager behaviour 1. Consolidate security manager startup code invoked from ClassLoader.initializeClassLoaders in System.initSecurityManager (new method). 2. As per the Java doc, the java.security.manager property should only be read at startup. Example: If this property is initialized with "disallow" when the JVM is started, then a security manager will not be set at startup and cannot be set dynamically. Currently, this property is read in System.setSecurityManager. If a user changes this property after startup, then System.setSecurityManager's behaviour will change after startup. This behaviour does not follow the Java doc and reference implementation. To fix this issue, the java.security.manager is only read once at startup in System.initSecurityManager. 3. In JDK18, System.setSecurityManager's run-time behaviour changes with respect to the java.security.manager property. Previously, if the property was null, the run-time behaviour was success or throws SecurityException. In JDK18, the new behaviour is to throw UnsupportedOperationException if the property is null. This matches the behaviour when the property is set to "disallow". Fixes: https://github.com/eclipse-openj9/openj9/issues/14092 Fixes: https://github.com/eclipse-openj9/openj9/issues/14094 Signed-off-by: Babneet Singh --- .../share/classes/java/lang/ClassLoader.java | 26 +--------- .../share/classes/java/lang/System.java | 48 +++++++++++++++++-- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/jcl/src/java.base/share/classes/java/lang/ClassLoader.java b/jcl/src/java.base/share/classes/java/lang/ClassLoader.java index 246fe6aae8b..1255babefa7 100644 --- a/jcl/src/java.base/share/classes/java/lang/ClassLoader.java +++ b/jcl/src/java.base/share/classes/java/lang/ClassLoader.java @@ -286,31 +286,7 @@ static final void initializeClassLoaders() { } /*[ENDIF] JAVA_SPEC_VERSION >= 10 */ jdk.internal.misc.VM.initLevel(2); - String javaSecurityManager = System.internalGetProperties().getProperty("java.security.manager"); //$NON-NLS-1$ - if ((javaSecurityManager != null) - /*[IF JAVA_SPEC_VERSION >= 12]*/ - /* See the SecurityManager javadoc for details about special tokens. */ - && !javaSecurityManager.equals("disallow") //$NON-NLS-1$ /* special token to disallow SecurityManager */ - && !javaSecurityManager.equals("allow") //$NON-NLS-1$ /* special token to allow SecurityManager */ - /*[ENDIF] JAVA_SPEC_VERSION >= 12 */ - ) { - /*[IF JAVA_SPEC_VERSION >= 17]*/ - System.initialErr.println("WARNING: A command line option has enabled the Security Manager"); //$NON-NLS-1$ - System.initialErr.println("WARNING: The Security Manager is deprecated and will be removed in a future release"); //$NON-NLS-1$ - /*[ENDIF] JAVA_SPEC_VERSION >= 17 */ - if (javaSecurityManager.isEmpty() || "default".equals(javaSecurityManager)) { //$NON-NLS-1$ - System.setSecurityManager(new SecurityManager()); - } else { - try { - Constructor constructor = Class.forName(javaSecurityManager, true, applicationClassLoader).getConstructor(); - constructor.setAccessible(true); - System.setSecurityManager((SecurityManager)constructor.newInstance()); - } catch (Throwable e) { - /*[MSG "K0631", "JVM can't set custom SecurityManager due to {0}"]*/ - throw new Error(com.ibm.oti.util.Msg.getString("K0631", e.toString()), e); //$NON-NLS-1$ - } - } - } + System.initSecurityManager(applicationClassLoader); jdk.internal.misc.VM.initLevel(3); /*[ELSE]*/ applicationClassLoader = sun.misc.Launcher.getLauncher().getClassLoader(); diff --git a/jcl/src/java.base/share/classes/java/lang/System.java b/jcl/src/java.base/share/classes/java/lang/System.java index 377a25f3a53..06b96827948 100644 --- a/jcl/src/java.base/share/classes/java/lang/System.java +++ b/jcl/src/java.base/share/classes/java/lang/System.java @@ -31,6 +31,7 @@ import java.util.PropertyPermission; import java.security.*; import java.lang.reflect.Method; +import java.lang.reflect.Constructor; /*[IF Sidecar19-SE]*/ import jdk.internal.misc.Unsafe; @@ -91,6 +92,14 @@ public final class System { private static Map, Object> setSMCallers; /*[ENDIF] JAVA_SPEC_VERSION >= 17 */ + /*[IF JAVA_SPEC_VERSION > 11] */ + /** + * setSecurityManager() should throw UnsupportedOperationException + * if throwUOEFromSetSM is set to true. + */ + private static boolean throwUOEFromSetSM; + /*[ENDIF] JAVA_SPEC_VERSION > 11 */ + //Get a ref to the Runtime instance for faster lookup private static final Runtime RUNTIME = Runtime.getRuntime(); /** @@ -1051,6 +1060,39 @@ public static void setProperties(Properties p) { } } +static void initSecurityManager(ClassLoader applicationClassLoader) { + String javaSecurityManager = internalGetProperties().getProperty("java.security.manager"); //$NON-NLS-1$ + /*[IF JAVA_SPEC_VERSION > 11]*/ + if ("allow".equals(javaSecurityManager)) { + /* Do nothing. */ + } else if ("disallow".equals(javaSecurityManager) //$NON-NLS-1$ + /*[IF JAVA_SPEC_VERSION >= 18]*/ + || (null == javaSecurityManager) + /*[ENDIF] JAVA_SPEC_VERSION >= 18 */ + ) { + throwUOEFromSetSM = true; + } else + /*[ENDIF] JAVA_SPEC_VERSION > 11 */ + if (null != javaSecurityManager) { + /*[IF JAVA_SPEC_VERSION >= 17]*/ + initialErr.println("WARNING: A command line option has enabled the Security Manager"); //$NON-NLS-1$ + initialErr.println("WARNING: The Security Manager is deprecated and will be removed in a future release"); //$NON-NLS-1$ + /*[ENDIF] JAVA_SPEC_VERSION >= 17 */ + if (javaSecurityManager.isEmpty() || "default".equals(javaSecurityManager)) { //$NON-NLS-1$ + setSecurityManager(new SecurityManager()); + } else { + try { + Constructor constructor = Class.forName(javaSecurityManager, true, applicationClassLoader).getConstructor(); + constructor.setAccessible(true); + setSecurityManager((SecurityManager)constructor.newInstance()); + } catch (Throwable e) { + /*[MSG "K0631", "JVM can't set custom SecurityManager due to {0}"]*/ + throw new Error(com.ibm.oti.util.Msg.getString("K0631", e.toString()), e); //$NON-NLS-1$ + } + } + } +} + /** * Sets the active security manager. Note that once * the security manager has been set, it can not be @@ -1076,12 +1118,12 @@ public static void setSecurityManager(final SecurityManager s) { @SuppressWarnings("removal") final SecurityManager currentSecurity = security; - /*[IF JAVA_SPEC_VERSION >= 12]*/ - if ("disallow".equals(systemProperties.getProperty("java.security.manager"))) { //$NON-NLS-1$ //$NON-NLS-2$ + /*[IF JAVA_SPEC_VERSION > 11]*/ + if (throwUOEFromSetSM) { /*[MSG "K0B00", "The Security Manager is deprecated and will be removed in a future release"]*/ throw new UnsupportedOperationException(com.ibm.oti.util.Msg.getString("K0B00")); //$NON-NLS-1$ } - /*[ENDIF] JAVA_SPEC_VERSION >= 12 */ + /*[ENDIF] JAVA_SPEC_VERSION > 11 */ /*[IF JAVA_SPEC_VERSION >= 17] */ Class callerClz = getCallerClass(); From bd6425ff57a0477ee1a9ec74a9a4d5cd443901eb Mon Sep 17 00:00:00 2001 From: Babneet Singh Date: Wed, 2 Feb 2022 16:34:00 -0500 Subject: [PATCH 2/3] Set java.security.manager property for tests in JDK18+ Tests which use System.setSecurityManager need to explicitly set the java.security.manager property in JDK18+. Impacted Java8andUp tests: - JCL_TEST_Java-Security - JCL_TEST_Java-Lang - JCL_TEST_Java-Lang-Invoke - JCL_TEST_Java-Lang_ClassLoader - JCL_TEST_Java-Internals - JCL_TEST_IBM-VM - JCL_TEST_Java-Lang-Ref Impacted JLM_Tests: - JLM_Tests_IBMinternal - TestMemoryMXBean Impacted Jsr292 tests: - jsr292Test - jsr292BootstrapTest Impacted cmdLineTests: - J9security - proxyFieldAccess Signed-off-by: Babneet Singh --- test/functional/JLM_Tests/playlist.xml | 7 +++-- .../Java8andUp/java8andUpSettings.mk | 4 ++- test/functional/Java8andUp/playlist.xml | 10 +++---- test/functional/Jsr292/playlist.xml | 10 +++---- test/functional/Jsr292/variables.mk | 5 ++-- .../cmdLineTests/J9security/playlist.xml | 5 ++-- .../proxyFieldAccess/playlist.xml | 20 +++++++++---- test/functional/variables.mk | 30 +++++++++++++++++++ 8 files changed, 68 insertions(+), 23 deletions(-) create mode 100644 test/functional/variables.mk diff --git a/test/functional/JLM_Tests/playlist.xml b/test/functional/JLM_Tests/playlist.xml index a537fec8e9f..9739a45bc3d 100644 --- a/test/functional/JLM_Tests/playlist.xml +++ b/test/functional/JLM_Tests/playlist.xml @@ -1,6 +1,6 @@ + $(TEST_ROOT)$(D)functional$(D)variables.mk JLM_Tests_interface_SE80 @@ -64,7 +65,7 @@ NoOptions -XX:+HeapManagementMXBeanCompatibility - $(JAVA_COMMAND) $(JVM_OPTIONS) \ + $(JAVA_COMMAND) $(JAVA_SECURITY_MANAGER) $(JVM_OPTIONS) \ --add-exports=jdk.management/com.ibm.lang.management.internal=ALL-UNNAMED --add-exports=java.management/com.ibm.java.lang.management.internal=ALL-UNNAMED \ -XX:SharedCacheHardLimit=16m -Xscmx1m -Xshareclasses:name=testJLM,reset \ -cp $(Q)$(RESOURCES_DIR)$(P)$(TESTNG)$(P)$(TEST_RESROOT)$(D)jlm_tests.jar$(Q) \ @@ -190,7 +191,7 @@ NoOptions -XX:+HeapManagementMXBeanCompatibility - $(JAVA_COMMAND) $(JVM_OPTIONS) \ + $(JAVA_COMMAND) $(JAVA_SECURITY_MANAGER) $(JVM_OPTIONS) \ --add-exports=jdk.management/com.ibm.lang.management.internal=ALL-UNNAMED --add-exports=java.management/com.ibm.java.lang.management.internal=ALL-UNNAMED \ -XX:SharedCacheHardLimit=16m -Xscmx1m -Xshareclasses:name=testJLM,reset \ -cp $(Q)$(RESOURCES_DIR)$(P)$(TESTNG)$(P)$(TEST_RESROOT)$(D)jlm_tests.jar$(Q) \ diff --git a/test/functional/Java8andUp/java8andUpSettings.mk b/test/functional/Java8andUp/java8andUpSettings.mk index f98a2e6c964..a4cd3cf91a2 100644 --- a/test/functional/Java8andUp/java8andUpSettings.mk +++ b/test/functional/Java8andUp/java8andUpSettings.mk @@ -1,5 +1,5 @@ ############################################################################## -# Copyright (c) 2018, 2021 IBM Corp. and others +# Copyright (c) 2018, 2022 IBM Corp. and others # # This program and the accompanying materials are made available under # the terms of the Eclipse Public License 2.0 which accompanies this @@ -20,6 +20,8 @@ # SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 ############################################################################## +include $(TEST_ROOT)$(D)functional$(D)variables.mk + ADD_MODULE_JAVA_SE_EE= # java.se.ee should only used for jdk 9 and 10 # if JDK_VERSION is 9 10 diff --git a/test/functional/Java8andUp/playlist.xml b/test/functional/Java8andUp/playlist.xml index c1f5217facf..ea6dd365b02 100644 --- a/test/functional/Java8andUp/playlist.xml +++ b/test/functional/Java8andUp/playlist.xml @@ -1,6 +1,6 @@ + $(TEST_ROOT)$(D)functional$(D)variables.mk cmdLineTester_J9securityTests_SE80 @@ -55,7 +56,7 @@ $(JAVA_COMMAND) $(JVM_OPTIONS) -Xdump \ -DMODULE_ADDEXPORTS_OPTION=$(Q)--add-exports$(Q) -DMODULE_ADDEXPORTS_VALUE=$(Q)java.base/jdk.internal.org.objectweb.asm=ALL-UNNAMED$(Q) \ -DTESTJ9SECURITYJARPATH=$(Q)$(TEST_RESROOT)$(D)$(Q) -DRESJAR=$(CMDLINETESTER_RESJAR) \ - -DEXE=$(SQ)$(JAVA_COMMAND) $(JVM_OPTIONS) -Xdump$(SQ) -jar $(CMDLINETESTER_JAR) -config $(Q)$(TEST_RESROOT)$(D)j9SecurityTest.xml$(Q) \ + -DEXE=$(SQ)$(JAVA_COMMAND) $(JAVA_SECURITY_MANAGER) $(JVM_OPTIONS) -Xdump$(SQ) -jar $(CMDLINETESTER_JAR) -config $(Q)$(TEST_RESROOT)$(D)j9SecurityTest.xml$(Q) \ -explainExcludes -nonZeroExitWhenError; \ $(TEST_STATUS) diff --git a/test/functional/cmdLineTests/proxyFieldAccess/playlist.xml b/test/functional/cmdLineTests/proxyFieldAccess/playlist.xml index 5b863e5e387..90ca4cacc78 100644 --- a/test/functional/cmdLineTests/proxyFieldAccess/playlist.xml +++ b/test/functional/cmdLineTests/proxyFieldAccess/playlist.xml @@ -1,6 +1,6 @@ + $(TEST_ROOT)$(D)functional$(D)variables.mk cmdLineTester_ProxyFieldAccess_SE80 NoOptions - $(JAVA_COMMAND) $(JVM_OPTIONS) -Xdump -DTESTPROXYFIELDACCESSJAR=$(Q)$(TEST_RESROOT)$(D)testproxyfieldaccess.jar$(Q) -DTESTDIR=$(Q)$(TEST_RESROOT)$(Q) -DRESJAR=$(CMDLINETESTER_RESJAR) -DEXE=$(SQ)$(JAVA_COMMAND) $(JVM_OPTIONS) -Xdump$(SQ) -jar $(CMDLINETESTER_JAR) -config $(Q)$(TEST_RESROOT)$(D)testproxyfieldaccess.xml$(Q) -explainExcludes -nonZeroExitWhenError -xids all,$(PLATFORM),$(VARIATION) $(CMDLINETESTER_XLIST); \ - ${TEST_STATUS} + $(JAVA_COMMAND) $(JVM_OPTIONS) -Xdump \ + -DTESTPROXYFIELDACCESSJAR=$(Q)$(TEST_RESROOT)$(D)testproxyfieldaccess.jar$(Q) \ + -DTESTDIR=$(Q)$(TEST_RESROOT)$(Q) -DRESJAR=$(CMDLINETESTER_RESJAR) \ + -DEXE=$(SQ)$(JAVA_COMMAND) $(JVM_OPTIONS) -Xdump$(SQ) -jar $(CMDLINETESTER_JAR) -config $(Q)$(TEST_RESROOT)$(D)testproxyfieldaccess.xml$(Q) \ + -explainExcludes -nonZeroExitWhenError -xids all,$(PLATFORM),$(VARIATION) $(CMDLINETESTER_XLIST); \ + ${TEST_STATUS} sanity @@ -42,13 +47,18 @@ 8 + cmdLineTester_ProxyFieldAccess NoOptions - $(JAVA_COMMAND) $(JVM_OPTIONS) -Xdump -DTESTPROXYFIELDACCESSJAR=$(Q)$(TEST_RESROOT)$(D)testproxyfieldaccess.jar$(Q) -DTESTDIR=$(Q)$(TEST_RESROOT)$(Q) -DRESJAR=$(CMDLINETESTER_RESJAR) -DEXE=$(SQ)$(JAVA_COMMAND) $(JVM_OPTIONS) --add-opens=java.base/java.lang=ALL-UNNAMED --add-reads java.base=jdk.unsupported --add-opens=java.base/java.lang=ALL-UNNAMED -Xdump$(SQ) -jar $(CMDLINETESTER_JAR) -config $(Q)$(TEST_RESROOT)$(D)testproxyfieldaccess.xml$(Q) -explainExcludes -nonZeroExitWhenError -xids all,$(PLATFORM),$(VARIATION) $(CMDLINETESTER_XLIST); \ - ${TEST_STATUS} + $(JAVA_COMMAND) $(JVM_OPTIONS) -Xdump \ + -DTESTPROXYFIELDACCESSJAR=$(Q)$(TEST_RESROOT)$(D)testproxyfieldaccess.jar$(Q) \ + -DTESTDIR=$(Q)$(TEST_RESROOT)$(Q) -DRESJAR=$(CMDLINETESTER_RESJAR) \ + -DEXE=$(SQ)$(JAVA_COMMAND) $(JAVA_SECURITY_MANAGER) $(JVM_OPTIONS) --add-opens=java.base/java.lang=ALL-UNNAMED --add-reads java.base=jdk.unsupported --add-opens=java.base/java.lang=ALL-UNNAMED -Xdump$(SQ) -jar $(CMDLINETESTER_JAR) -config $(Q)$(TEST_RESROOT)$(D)testproxyfieldaccess.xml$(Q) \ + -explainExcludes -nonZeroExitWhenError -xids all,$(PLATFORM),$(VARIATION) $(CMDLINETESTER_XLIST); \ + ${TEST_STATUS} sanity diff --git a/test/functional/variables.mk b/test/functional/variables.mk new file mode 100644 index 00000000000..efa5f571d2d --- /dev/null +++ b/test/functional/variables.mk @@ -0,0 +1,30 @@ +############################################################################## +# Copyright (c) 2022, 2022 IBM Corp. and others +# +# This program and the accompanying materials are made available under +# the terms of the Eclipse Public License 2.0 which accompanies this +# distribution and is available at https://www.eclipse.org/legal/epl-2.0/ +# or the Apache License, Version 2.0 which accompanies this distribution and +# is available at https://www.apache.org/licenses/LICENSE-2.0. +# +# This Source Code may also be made available under the following +# Secondary Licenses when the conditions for such availability set +# forth in the Eclipse Public License, v. 2.0 are satisfied: GNU +# General Public License, version 2 with the GNU Classpath +# Exception [1] and GNU General Public License, version 2 with the +# OpenJDK Assembly Exception [2]. +# +# [1] https://www.gnu.org/software/classpath/license.html +# [2] http://openjdk.java.net/legal/assembly-exception.html +# +# SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +############################################################################## + +# In JDK18+, java.security.manager == null behaves as -Djava.security.manager=disallow. +# In JDK17-, java.security.manager == null behaves as -Djava.security.manager=allow. +# For OpenJ9 tests to work as expected, -Djava.security.manager=allow behaviour is +# needed in JDK18+. +JAVA_SECURITY_MANAGER= +ifeq ($(filter 8 9 10 11 12 13 14 15 16 17, $(JDK_VERSION)),) + JAVA_SECURITY_MANAGER=-Djava.security.manager=allow +endif From c0e4f3af7f552b01260102ac4afa3736d3512e6b Mon Sep 17 00:00:00 2001 From: Babneet Singh Date: Wed, 2 Feb 2022 09:32:00 -0500 Subject: [PATCH 3/3] Explicitly handle null arguments in System.setSecurityManager setSecurityManager should do nothing if the input argument is null and no security manager has been established. Before, this was being enforced implicitly. The implicit check no longer works. There is error handling code and code to print a warning message, which needs to avoided in the above "do nothing" scenario. So, an explicit null check has been added to enforce the "no nothing" scenario. Signed-off-by: Babneet Singh --- jcl/src/java.base/share/classes/java/lang/System.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/jcl/src/java.base/share/classes/java/lang/System.java b/jcl/src/java.base/share/classes/java/lang/System.java index 06b96827948..fdf9236a1c2 100644 --- a/jcl/src/java.base/share/classes/java/lang/System.java +++ b/jcl/src/java.base/share/classes/java/lang/System.java @@ -1118,6 +1118,11 @@ public static void setSecurityManager(final SecurityManager s) { @SuppressWarnings("removal") final SecurityManager currentSecurity = security; + if ((currentSecurity == null) && (s == null)) { + /* Return if the input argument is null and no security manager has been established. */ + return; + } + /*[IF JAVA_SPEC_VERSION > 11]*/ if (throwUOEFromSetSM) { /*[MSG "K0B00", "The Security Manager is deprecated and will be removed in a future release"]*/