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

Disable Security Manager Tests #14412

Open
babsingh opened this issue Feb 1, 2022 · 17 comments
Open

Disable Security Manager Tests #14412

babsingh opened this issue Feb 1, 2022 · 17 comments

Comments

@babsingh
Copy link
Contributor

babsingh commented Feb 1, 2022

This issue applies to tests that utilize System.setSecurityManager.

JDK18

In JDK18, the security manager is disallowed by default when the java.security.manager system property is not specified. Currently, the security manager tests in OpenJ9 cannot be selectively disabled. To keep the security manager tests correctly functioning, -Djava.security.manager=allow needs to be specified while running these tests with OpenJ9 and other implementations.

As per adoptium/TKG#263 (comment), only the OpenJ9 repo has tests which use System.setSecurityManager. The second commit in #14329 sets the java.security.manager property for the impacted tests in OpenJ9.

Related:

Future JDK

In a future JDK, the security manager will be removed. So, the security manager tests in OpenJ9 will need to be disabled.

If the compilation of the tests need to be disabled, then they would need to be separated from their current environment and run standalone. This may be challenging and will require substantially more time.

Disabling the tests during runtime may be easier. This can be achieved without separating the tests. org.junit.Assume.assumeTrue allows a test to be ignored based upon a condition. In this case, the condition would return FALSE if a security manager is not allowed based upon the JDK version and java.security.manager system property.

Impacted Tests

In OpenJ9, the impacted security manager tests are:

    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

For system tests, only the framework which launches system tests is impacted: stf.load/src/stf.load/net/adoptopenjdk/loadTest/LoadTest.java. SecurityManager.checkExit is used to prevent tests from exiting the process. Currently, there is no replacement for SecurityManager.checkExit. This issue is being tracked in https://bugs.openjdk.java.net/browse/JDK-8199704. The goal is to add an ability to intercept or disable System::exit.

How to identify classes and methods dependent on the Security Manager?

This is a useful resource: https://openjdk.java.net/jeps/411. There is a section titled: Deprecate APIs for removal. It lists the classes and methods, which will be terminally deprecated and retained. This information can be used in segregation of tests, specifically to identify classes and methods which are strongly dependent on the security manager.

@babsingh
Copy link
Contributor Author

babsingh commented Feb 1, 2022

@babsingh
Copy link
Contributor Author

babsingh commented Feb 1, 2022

@pshipton when the security manager is removed, do we need to disable compilation of those tests? or disabling the tests during runtime should be sufficient?

@pshipton
Copy link
Member

pshipton commented Feb 1, 2022

I expect the tests won't compile on the new jdk since there won't be any java.lang.SecurityManager class available.

@babsingh
Copy link
Contributor Author

babsingh commented Feb 1, 2022

only the OpenJ9 repo has tests which use System.setSecurityManager

The above statement is wrong. The system tests do use the Security Manager. Initially, I only looked at https://github.com/adoptium/aqa-systemtest. The system tests use code from https://github.com/adoptium/STF which utilizes the Security Manager.

Personal build: https://hyc-runtimes-jenkins.swg-devops.com/view/OpenJ9%20-%20Personal/job/Pipeline-Build-Test-Personal/11686/

With JDK18's java.security.manager behaviour, the following exception is thrown:

DLT stderr Exception in thread "main" java.lang.UnsupportedOperationException: The Security Manager is deprecated and will be removed in a future release
DLT stderr 	at java.base/java.lang.System.setSecurityManager(System.java:802)
DLT stderr 	at net.adoptopenjdk.loadTest.LoadTest.overrideSecurityManager(LoadTest.java:135)
DLT stderr 	at net.adoptopenjdk.loadTest.LoadTest.main(LoadTest.java:114)

Requested help from @Mesbah-Alam to figure the best approach to handle the below failures. Current options: either add -Djava.security.manager=allow or remove the use of the SecurityManager class.

56 failing sanity.system test targets


Test Name Duration Age
2 - - DaaLoadTest_daa2_5m_0 6 sec
4 - - TestJlmRemoteClassAuth_1 18 sec
6 - - TestJlmRemoteNotifierProxyAuth_1 18 sec
7 - - TestJlmRemoteThreadAuth_1 17 sec
8 - - TestJlmRemoteThreadNoAuth_1 17 sec
11 - - TestIBMJlmRemoteClassAuth_1 16 sec
12 - - TestIBMJlmRemoteMemoryAuth_1 16 sec
13 - - TestIBMJlmRemoteMemoryNoAuth_0 17 sec
14 - - TestIBMJlmRemoteMemoryNoAuth_1 6.4 sec
15 - - LambdaLoadTest_J9_5m_0 5.4 sec
16 - - LambdaLoadTest_CS_5m_0 4.8 sec
17 - - ParallelStreamsLoadTest_J9_0 5.3 sec
18 - - MathLoadTest_autosimd_5m_0 4.5 sec
19 - - MathLoadTest_autosimd_5m_1 5.5 sec
20 - - MathLoadTest_bigdecimal_5m_0 4.6 sec
22 - - MathLoadTest_bigdecimal_CS_5m_0 4.9 sec
23 - - MauveSingleInvocLoad_J9_5m_0 8.3 sec
24 - - MauveSingleInvocLoad_J9_5m_1 6.7 sec
55 - - CLLoad_0 4.5 sec
2 - - DaaLoadTest_daa1_5m_0 4.5 sec
3 - - DaaLoadTest_daa2_5m_1 3.9 sec
4 - - DaaLoadTest_daa3_5m_0 4.7 sec
5 - - DaaLoadTest_daa3_CS_5m_0 4 sec
6 - - TestJlmRemoteClassNoAuth_1 6.9 sec
7 - - TestJlmRemoteMemoryAuth_1 16 sec
9 - - TestJlmRemoteMemoryNoAuth_1 16 sec
10 - - TestJlmRemoteNotifierProxyAuth_0 15 sec
12 - - TestJlmRemoteThreadNoAuth_0 5.7 sec
13 - - TestIBMJlmRemoteClassNoAuth_1 15 sec
14 - - TestIBMJlmRemoteMemoryAuth_0 16 sec
15 - - LambdaLoadTest_J9_5m_1 3.8 sec
16 - - ParallelStreamsLoadTest_J9_1 3.7 sec
18 - - MathLoadTest_bigdecimal_5m_1 4.5 sec
19 - - MauveSingleThrdLoad_J9_5m_1 5.9 sec
20 - - MauveMultiThrdLoad_5m_0 5.3 sec
63 - - ClassLoadingTest_5m_1 4.5 sec
65 - - NioLoadTest_5m_1 4.7 sec
2 - - DaaLoadTest_daa1_5m_1 4.4 sec
3 - - DaaLoadTest_daa3_5m_1 4.4 sec
4 - - DaaLoadTest_daa1_CS_5m_0 4.9 sec
5 - - DaaLoadTest_daa2_CS_5m_0 5.6 sec
8 - - TestJlmRemoteClassAuth_0 16 sec
10 - - TestJlmRemoteClassNoAuth_0 6.1 sec
12 - - TestJlmRemoteMemoryAuth_0 16 sec
13 - - TestJlmRemoteMemoryNoAuth_0 6.2 sec
15 - - TestJlmRemoteThreadAuth_0 16 sec
17 - - TestIBMJlmRemoteClassAuth_0 17 sec
18 - - TestIBMJlmRemoteClassNoAuth_0 17 sec
19 - - ParallelStreamsLoadTest_CS_0 4.9 sec
20 - - MathLoadTest_autosimd_CS_5m_0 4.8 sec
21 - - MauveSingleThrdLoad_J9_5m_0 8.3 sec
22 - - MauveMultiThrdLoad_5m_1 7.7 sec
59 - - CLLoad_1 5.7 sec
64 - - ClassLoadingTest_5m_0 4.4 sec
65 - - NioLoadTest_5m_0 4.4 sec
67 - - ClassLoadingTest_CS_5m_0 4.9 sec

29 failing extended.system test targets


Test Name Duration Age
2 - - DaaLoadTest_all_CS_5m_0 4.3 sec
3 - - MiniMix_5m_0 5.3 sec
5 - - MiniMix_aot_5m_0 8.5 sec
6 - - ConcurrentLoadTest_5m_1 4.5 sec
7 - - DBBLoadTest_5m_1 4.7 sec
9 - - LockingLoadTest_1 4.7 sec
11 - - UtilLoadTest_5m_1 4.6 sec
13 - - HCRLateAttachWorkload_previewEnabled_1 5.3 sec
16 - - SharedClassesWorkload_0 5.1 sec
2 - - DaaLoadTest_all_5m_0 4.3 sec
3 - - DaaLoadTest_all_5m_1 3.3 sec
4 - - MathLoadTest_all_5m_0 3.6 sec
5 - - MathLoadTest_all_5m_1 4.4 sec
6 - - MathLoadTest_all_CS_5m_0 4 sec
7 - - HCRLateAttachWorkload_previewEnabled_0 4.7 sec
8 - - HeapHogLoadTest_5m_0 4.7 sec
9 - - ObjectTreeLoadTest_5m_0 4.4 sec
10 - - SharedClassesAPI_1 13 sec
3 - - MiniMix_5m_1 7.9 sec
4 - - ConcurrentLoadTest_5m_0 4.3 sec
6 - - DBBLoadTest_5m_0 4.3 sec
7 - - LangLoadTest_5m_0 4.6 sec
8 - - LangLoadTest_5m_1 5.5 sec
10 - - LockingLoadTest_0 4.4 sec
11 - - UtilLoadTest_5m_0 4.4 sec
14 - - HeapHogLoadTest_5m_1 4.5 sec
15 - - ObjectTreeLoadTest_5m_1 4.5 sec
17 - - SharedClassesAPI_0 14 sec
18 - - SharedClassesWorkload_1 6.1 sec

@Mesbah-Alam
Copy link
Contributor

DLT stderr 	at net.adoptopenjdk.loadTest.LoadTest.overrideSecurityManager(LoadTest.java:135)

Yes, all load tests in STF based system tests will invoke LoadTests.java and thus invoke overrideSecurityManager(). From the comment in the code, it was added there to "block System.exit attempts" - that'd probably be made by the third party Mauve tests that system tests run. We need to find a replacement solution to do the same; we may add -Djava.security.manager=allow for JDK18 as a temporary solution.

@pshipton
Copy link
Member

pshipton commented Feb 2, 2022

For jdk18 using -Djava.security.manager=allow is the permanent solution. I'm guessing sometime between jdk19 and jdk21 (and I'm leaning towards jdk19) the java.lang.SecurityManager will be removed. -Djava.security.manager=allow won't work and code that uses SecurityManager won't compile. We should consider how we are going to handle that when it happens.

The other one to think about is https://openjdk.java.net/jeps/421, finalization will be removed as some point as well. Not sure if any system tests are using it. I'm guessing finalization may be disabled by default in jdk19 and removed in jdk20.

@JasonFengJ9
Copy link
Member

I'm guessing sometime between jdk19 and jdk21 (and I'm leaning towards jdk19) the java.lang.SecurityManager will be removed.

If that's a correct prediction, disabling Security Manager tests (build & run) can start from JDK18 instead of setting -Djava.security.manager=allow for non-LTS versions before its actual removal. The tests for JDK17 and earlier LTS levels should be retained.

JEP 421 (Finalization removal) will have similar requirements as well, but I think it is in much smaller scale comparing to how much security manager tests present.

babsingh added a commit to babsingh/aqa-tests that referenced this issue Feb 2, 2022
In JDK18+, java.security.manager == null behaves as
-Djava.security.manager=disallow.

In JDK17-, java.security.manager == null behaves as
-Djava.security.manager=allow.

In case of system tests, the base infra (STF) which is used to launch tests
utilizes the security manager in
net.adoptopenjdk.loadTest.LoadTest.overrideSecurityManager. For system tests
to work as expected, -Djava.security.manager=allow behaviour is needed in
JDK18+.

Related: eclipse-openj9/openj9#14412

Signed-off-by: Babneet Singh <[email protected]>
babsingh added a commit to babsingh/aqa-tests that referenced this issue Feb 2, 2022
In JDK18+, java.security.manager == null behaves as
-Djava.security.manager=disallow.

In JDK17-, java.security.manager == null behaves as
-Djava.security.manager=allow.

In case of system tests, the base infra (STF) which is used to launch tests
utilizes the security manager in
net.adoptopenjdk.loadTest.LoadTest.overrideSecurityManager. For system tests
to work as expected, -Djava.security.manager=allow behaviour is needed in
JDK18+.

Related: eclipse-openj9/openj9#14412

Signed-off-by: Babneet Singh <[email protected]>
@babsingh
Copy link
Contributor Author

babsingh commented Feb 2, 2022

disabling Security Manager tests (build & run) can start from JDK18 instead of setting -Djava.security.manager=allow for non-LTS versions before its actual removal

There are OpenJDK and other non-openj9 tests related to the security manager for JDK18. Disabling security manager tests in OpenJ9 instead of setting -Djava.security.manager=allow may prevent us to pass those non-openj9 tests which are not frequently run.

+1 to begin segregating/isolating security manager tests so they can be disabled once the security manager is removed in a future JDK. Segregating security manager tests will be challenging since it is widely used in functional and system tests. Also, isolating some tests may be difficult because they are deeply dependent on their current test environment. I don't think it is feasible to achieve this for the JDK18 release.

babsingh added a commit to babsingh/aqa-tests that referenced this issue Feb 2, 2022
In JDK18+, java.security.manager == null behaves as
-Djava.security.manager=disallow.

In JDK17-, java.security.manager == null behaves as
-Djava.security.manager=allow.

In case of system tests, the base infra (STF) which is used to launch tests
utilizes the security manager in
net.adoptopenjdk.loadTest.LoadTest.overrideSecurityManager. For system tests
to work as expected, -Djava.security.manager=allow behaviour is needed in
JDK18+.

Related: eclipse-openj9/openj9#14412

Signed-off-by: Babneet Singh <[email protected]>
@JasonFengJ9
Copy link
Member

Agreed that there are large number of security manager tests and it might take a while to fix them all. I think the task can be divided, and addressed in separated PRs while applying the workaround to keep JDK18 test green.

To co-ordinate the effort, I propose this issue serves a placeholder for disabling individual security manager test, no separated issue required. Ideally, before starting working on a test, its name/location is posted here to avoid duplicated work.

I can start w/ https://github.com/eclipse-openj9/openj9/tree/master/test/functional/cmdLineTests/J9security.

@babsingh
Copy link
Contributor Author

babsingh commented Feb 3, 2022

Added the list of all impacted tests in the main description: #14412 (comment).

sophia-guo pushed a commit to adoptium/aqa-tests that referenced this issue Feb 4, 2022
In JDK18+, java.security.manager == null behaves as
-Djava.security.manager=disallow.

In JDK17-, java.security.manager == null behaves as
-Djava.security.manager=allow.

In case of system tests, the base infra (STF) which is used to launch tests
utilizes the security manager in
net.adoptopenjdk.loadTest.LoadTest.overrideSecurityManager. For system tests
to work as expected, -Djava.security.manager=allow behaviour is needed in
JDK18+.

Related: eclipse-openj9/openj9#14412

Signed-off-by: Babneet Singh <[email protected]>
@EricYangIBM
Copy link
Contributor

I will start with JCL_TEST_Java-Security and potentially work on the other Java8andUp tests

@EricYangIBM
Copy link
Contributor

When I ran JCL_Test locally I found that JCL_TEST_Java-Internals and JCL_TEST_Java-Lang-Ref didn't fail (the others did with the UnsupportedOperationException)

@babsingh
Copy link
Contributor Author

babsingh commented Feb 7, 2022

re #14412 (comment):

JCL_TEST_Java-Internals

has Test_J9VMInternals which depends on Test_J9VMInternalsImpl. In Test_J9VMInternalsImpl, test_checkPackageAccess invokes System.setSecurityManager. This test should technically fail when UnsupportedOperationException is thrown from setSecurityManager. Since it allows throws Throwable, it exits cleanly for the UnsupportedOperationException without invoking any of the Assert.fails. This behaviour should be corrected.

JCL_TEST_Java-Lang-Ref

has Test_ReferenceQueue which depends on Support_ExtendedTestEnvironment. In Support_ExtendedTestEnvironment, setSecurityManager invokes System.setSecurityManager. It should be verified if Support_ExtendedTestEnvironment.setSecurityManager is being used. If it is unused, we can probably get rid of it.

We won't be able to compile the above tests once the security manager is disabled. Even though the above tests do not fail, we will still need to segregate them.

@babsingh
Copy link
Contributor Author

This is a useful resource: https://openjdk.java.net/jeps/411. There is a section titled: Deprecate APIs for removal. It lists the classes and methods, which will be terminally deprecated and retained. This information can be used in segregation of tests. Specifically to identify classes and methods which are strongly dependent on the security manager.

@babsingh
Copy link
Contributor Author

Update: For system tests, only the framework which launches system tests is impacted: stf.load/src/stf.load/net/adoptopenjdk/loadTest/LoadTest.java. SecurityManager.checkExit is used to prevent tests from exiting the process. Currently, there is no replacement for SecurityManager.checkExit. This issue is being tracked in https://bugs.openjdk.java.net/browse/JDK-8199704. The goal is to add an ability to intercept or disable System::exit. fyi @Mesbah-Alam

@cjjdespres
Copy link
Contributor

@babsingh Out of curiosity, did the failures in your comment all look like

DLT stderr Exception in thread "main" java.lang.UnsupportedOperationException: The Security Manager is deprecated and will be removed in a future release
DLT stderr 	at java.base/java.lang.System.setSecurityManager(System.java:802)
DLT stderr 	at net.adoptopenjdk.loadTest.LoadTest.overrideSecurityManager(LoadTest.java:135)
DLT stderr 	at net.adoptopenjdk.loadTest.LoadTest.main(LoadTest.java:114)

as you mentioned? There were a lot of recent JITServer test failures with substantial overlap with that list, and I wanted to confirm whether or not that was coincidence. (None of the JITServer test failures looked like that, of course).

@babsingh
Copy link
Contributor Author

did the failures in #14412 (comment) all look like as you mentioned?

@cjjdespres Yes, all failures had UnsupportedOperationException when System.setSecurityManager was invoked with the security manager disabled. None of the failures in #14538 are related to the security manager.

pshipton pushed a commit to pshipton/openj9 that referenced this issue Feb 28, 2022
SecurityManager will be removed in a future jdk. Separate tests that
rely on it so that they can be easily disabled in the future.

Issue: eclipse-openj9#14412
Signed-off-by: Eric Yang <[email protected]>
EricYangIBM added a commit to EricYangIBM/openj9 that referenced this issue Mar 1, 2022
SecurityManager will be removed in a future jdk. Separate tests that
rely on it so that they can be easily disabled in the future.

Issue: eclipse-openj9#14412
Signed-off-by: Eric Yang <[email protected]>
pshipton pushed a commit to pshipton/openj9 that referenced this issue Mar 3, 2022
SecurityManager will be removed in a future jdk. Separate tests that
rely on it so that they can be easily disabled in the future.

Issue: eclipse-openj9#14412
Signed-off-by: Eric Yang <[email protected]>
pshipton pushed a commit to pshipton/openj9 that referenced this issue Mar 3, 2022
SecurityManager will be removed in a future jdk. Separate tests that
rely on it so that they can be easily disabled in the future.

Issue: eclipse-openj9#14412
Signed-off-by: Eric Yang <[email protected]>
EricYangIBM added a commit to EricYangIBM/openj9 that referenced this issue Mar 4, 2022
SecurityManager will be removed in a future jdk. Separate tests that
rely on it so that they can be easily disabled in the future.

Issue: eclipse-openj9#14412
Signed-off-by: Eric Yang <[email protected]>
EricYangIBM added a commit to EricYangIBM/openj9 that referenced this issue Mar 4, 2022
SecurityManager will be removed in a future jdk. Separate tests that
rely on it so that they can be easily disabled in the future.

Issue: eclipse-openj9#14412
Signed-off-by: Eric Yang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants