JDK24: Permanently Disable the Security Manager #20625

theresa-m · 2024-11-18T21:40:35Z

Throw an error on initialization if java.security.manager attempts to add a security manager
configure System.setSecurityManager to always throw an UnsupportedOperationException
System.getSecurityManager will always return null since a security manager can't be set
Update java.security.* javadocs
Exclude unused helper methods in java.security.* including native code
Disable functional tests that rely on a security manager

Related: #20563

theresa-m · 2024-11-18T21:46:53Z

test/functional/cmdLineTests/J9security/playlist.xml

+		<disables>
+			<disable>
+				<comment>https://github.com/eclipse-openj9/openj9/issues/20563</comment>
+				<version>24+</version>


@llxia is this the preferred way to disable tests from running in future versions? The security tests should run for JDK 11-23 only. I've mostly seen the block used in temporarily disabled tests so I wanted to check since this would be a permanent change.

disable is for temporary excludes. In this case, we should set <version>[11, 23]</version> . Example code: https://github.com/adoptium/TKG/blob/master/examples/jdkVersion/playlist.xml#L39

tajila · 2024-11-20T15:30:28Z

@JasonFengJ9 Please review these changes

JasonFengJ9

The natives related to checkPermission() can be ifdef out for JDK24+ such as

openj9/runtime/jcl/common/java_lang_Class.cpp

Line 1406 in 48709bf

    
           Java_java_security_AccessController_getAccSnapshot(JNIEnv* env, jclass jsAccessController, jint startingFrame, jboolean forDoPrivilegedWithCombiner)

JasonFengJ9 · 2024-11-20T19:21:58Z

jcl/src/java.base/share/classes/java/lang/System.java

@@ -1265,6 +1265,10 @@ static void checkTmpDir() {

 /*[IF JAVA_SPEC_VERSION >= 9]*/
 static void initSecurityManager(ClassLoader applicationClassLoader) {


initSecurityManager() can be removed at

openj9/jcl/src/java.base/share/classes/java/lang/ClassLoader.java

Line 297 in 48709bf

System.initSecurityManager(applicationClassLoader);

This is still needed since initSecurityManager is used to detect settings of the java.security.manager property.

initSecurityManager() reads the system property java.security.manager, and sets throwUOEFromSetSM which can be skipped within setSecurityManager().
System.initSecurityManager(applicationClassLoader) seems not needed for JDK24+.

It will still be needed to throw an exception on startup for illegal java.security.manager manager settings triggered by throwErrorOnInit .

jcl/src/java.base/share/classes/java/lang/System.java

JasonFengJ9 · 2024-11-20T19:30:35Z

jcl/src/java.base/share/classes/java/security/AccessControlContext.java

@@ -727,10 +727,15 @@ private boolean debugHelper(Permission perm) {
 *
 * @param       perm java.security.Permission
 *                  the permission to check
+/*[IF JAVA_SPEC_VERSION >= 24]


The API description should be changed for JDK24 such as Throws AccessControlException.

jcl/src/java.base/share/classes/java/security/AccessController.java

test/functional/testVars.mk

.../functional/cmdLineTests/shareClassTests/DataHelperTests/DataHelperTests_SecurityManager.xml

test/functional/cmdLineTests/shareClassTests/DataHelperTests/playlist.xml

test/functional/Java8andUp/src/org/openj9/test/attachAPI/TestAttachAPI.java

pshipton · 2024-11-20T20:40:37Z

FYI #20655

Signed-off-by: Theresa Mammarella <[email protected]>

to a different job and exclude from 24+. Signed-off-by: Theresa Mammarella <[email protected]>

Signed-off-by: Theresa Mammarella <[email protected]>

JasonFengJ9 · 2024-11-22T21:50:05Z

jcl/src/java.base/share/classes/java/lang/System.java

 	} else if ("disallow".equals(javaSecurityManager)) { //$NON-NLS-1$
 		/*[IF JAVA_SPEC_VERSION > 11]*/
 		throwUOEFromSetSM = true;
 		/*[ELSE] JAVA_SPEC_VERSION > 11 */
 		/* Do nothing. */
 		/*[ENDIF] JAVA_SPEC_VERSION > 11 */
 	} else {
+		/*[IF JAVA_SPEC_VERSION >= 24]*/
+		throwErrorOnInit = true;
+		/*[ELSE] JAVA_SPEC_VERSION >= 24 */
 		/*[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$


These WARNING messages shouldn't be printed for JDK24+.

They are excluded already (in the /*[ELSE] JAVA_SPEC_VERSION >= 24 */) block.

JasonFengJ9 · 2024-11-22T21:52:57Z

jcl/src/java.base/share/classes/java/lang/System.java

+		/*[MSG "K0637", "A command line option has attempted to allow or enable the Security Manager. Enabling a Security Manager is not supported."]*/
+		throw new Error(Msg.getString("K0637")); //$NON-NLS-1$
+	}
+	/*[ENDIF] JAVA_SPEC_VERSION >= 24 */


I think throwErrorOnInit isn't needed, the Error can be moved around Line 1278 whenever java.security.manager is detected.

I liked the variable since the -Djava.security.manager=disallow case should not throw an exception.

JasonFengJ9 · 2024-11-22T21:54:36Z

jcl/src/java.base/share/classes/com/ibm/oti/util/ExternalMessages-MasterIndex.properties

@@ -355,6 +355,7 @@ K0633="{0} is not a valid AccessMode."
 K0634="{0}.{1}({2}) is static. Expected instance field."
 K0635="{0}.{1}({2}) is non-static. Expected static field."
 K0636="No such field: {0}.{1}({2})"
+K0637="A command line option has attempted to allow or enable the Security Manager. Enabling a Security Manager is not supported."


K0637 has been taken.

Thanks for catching this.

JasonFengJ9

In addition, all AccessController.doPrivileged_XXX usages can be removed for JDK24+ such as

openj9/jcl/src/java.base/share/classes/java/lang/ref/Reference.java

Line 70 in 3da9d2f

    
           static final boolean ENABLED = AccessController.doPrivileged(new PrivilegedAction<Boolean>() {

Yeah, there are lots of them.

JasonFengJ9 · 2024-11-24T23:46:18Z

jcl/src/java.base/share/classes/java/security/AccessController.java

@@ -49,25 +51,25 @@ public final class AccessController {
 		initializeInternal();
 	}

+	private static native void initializeInternal();


This native can be removed for JDK24+, I expect J9JavaVM->doPrivilegedMethodID_XXX are not needed along with

openj9/runtime/jcl/common/acccont.c

Line 28 in 3da9d2f

jboolean JNICALL Java_java_security_AccessController_initializeInternal(JNIEnv *env, jclass thisClz)

I have excluded these and am running a personal build to see if there are any impacted tests.

Signed-off-by: Theresa Mammarella <[email protected]>

- Throw an error on initialization if java.security.manager attempts to add a security manager - configure setSecurityManager to always throw an UnsupportedOperationException - getSecurityManager will always return null since a security manager can't be set Signed-off-by: Theresa Mammarella <[email protected]>

theresa-m · 2024-11-25T18:28:57Z

In addition, all AccessController.doPrivileged_XXX usages can be removed for JDK24+ such as

openj9/jcl/src/java.base/share/classes/java/lang/ref/Reference.java

Line 70 in 3da9d2f

static final boolean ENABLED = AccessController.doPrivileged(new PrivilegedAction<Boolean>() {

Yeah, there are lots of them.

Do you mind if I do this in a second pull request? This change set is already getting large.

JasonFengJ9 · 2024-11-25T18:51:55Z

In addition, all AccessController.doPrivileged_XXX usages can be removed for JDK24+
Yeah, there are lots of them.

Do you mind if I do this in a second pull request? This change set is already getting large.

Sounds good.

theresa-m added comp:vm comp:test jdk24 labels Nov 18, 2024

theresa-m force-pushed the fix_20563 branch from 2b81790 to 409e084 Compare November 18, 2024 21:44

theresa-m commented Nov 18, 2024

View reviewed changes

theresa-m force-pushed the fix_20563 branch 3 times, most recently from 888855d to 7ab4eda Compare November 19, 2024 16:24

theresa-m marked this pull request as ready for review November 20, 2024 15:14

theresa-m requested a review from tajila November 20, 2024 15:14

JasonFengJ9 reviewed Nov 20, 2024

View reviewed changes

theresa-m added 6 commits November 22, 2024 16:18

Set security manager only for JDK 18-23

5f46f88

Signed-off-by: Theresa Mammarella <[email protected]>

Disable cmdLineTester_J9securityTests for 24+

3a5072d

Signed-off-by: Theresa Mammarella <[email protected]>

Disable cmdLineTester_ProxyFieldAccess for 24+

d1238a7

Signed-off-by: Theresa Mammarella <[email protected]>

Split DataHelperTests reliant on the security manager

7e5cffa

to a different job and exclude from 24+. Signed-off-by: Theresa Mammarella <[email protected]>

Update criu_nonPortable SecurityManager error for 24+

a2dfc42

Signed-off-by: Theresa Mammarella <[email protected]>

Don't fail on TestAttachAPI SecurityException for 24

7492ec2

Signed-off-by: Theresa Mammarella <[email protected]>

theresa-m force-pushed the fix_20563 branch 2 times, most recently from fce9383 to fc9e6eb Compare November 22, 2024 21:28

JasonFengJ9 reviewed Nov 22, 2024

View reviewed changes

JasonFengJ9 reviewed Nov 25, 2024

View reviewed changes

theresa-m force-pushed the fix_20563 branch 2 times, most recently from 725ab4c to 72168cc Compare November 25, 2024 18:22

theresa-m added 2 commits November 25, 2024 13:25

java.security comment update and exclude helper methods

3219249

Signed-off-by: Theresa Mammarella <[email protected]>

theresa-m force-pushed the fix_20563 branch from 72168cc to c9cabc6 Compare November 25, 2024 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK24: Permanently Disable the Security Manager #20625

JDK24: Permanently Disable the Security Manager #20625

theresa-m commented Nov 18, 2024 •

edited

Loading

theresa-m Nov 18, 2024 •

edited

Loading

llxia Nov 19, 2024 •

edited

Loading

tajila commented Nov 20, 2024

JasonFengJ9 left a comment

JasonFengJ9 Nov 20, 2024

theresa-m Nov 20, 2024

JasonFengJ9 Nov 21, 2024

theresa-m Nov 22, 2024

JasonFengJ9 Nov 20, 2024

pshipton commented Nov 20, 2024

JasonFengJ9 Nov 22, 2024

theresa-m Nov 25, 2024

JasonFengJ9 Nov 22, 2024

theresa-m Nov 25, 2024

JasonFengJ9 Nov 22, 2024

theresa-m Nov 25, 2024

JasonFengJ9 left a comment

JasonFengJ9 Nov 24, 2024

theresa-m Nov 25, 2024

theresa-m commented Nov 25, 2024

JasonFengJ9 commented Nov 25, 2024

		@@ -1265,6 +1265,10 @@ static void checkTmpDir() {

		/[IF JAVA_SPEC_VERSION >= 9]/
		static void initSecurityManager(ClassLoader applicationClassLoader) {

JDK24: Permanently Disable the Security Manager #20625

Are you sure you want to change the base?

JDK24: Permanently Disable the Security Manager #20625

Conversation

theresa-m commented Nov 18, 2024 • edited Loading

theresa-m Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

llxia Nov 19, 2024 • edited Loading

Choose a reason for hiding this comment

tajila commented Nov 20, 2024

JasonFengJ9 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pshipton commented Nov 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JasonFengJ9 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theresa-m commented Nov 25, 2024

JasonFengJ9 commented Nov 25, 2024

theresa-m commented Nov 18, 2024 •

edited

Loading

theresa-m Nov 18, 2024 •

edited

Loading

llxia Nov 19, 2024 •

edited

Loading