Java 18 - do not set SecurityManager if unsupported

[BenjaminAmos]

Contains

This pull request reverts ca91326 and re-implements it properly using a runtime check. It ensures that the game does not set a custom SecurityManager when running on Java 18 or above.

How to test

• Run the game using Java 18 (or later)
• Ensure that the game doesn't crash on start-up (it should reach at least the main menu)

[skaldarnar]

@BenjaminAmos do I need #5109 to be able to test this to support newer Java, or how did you do it locally?

[BenjaminAmos]

It looks like I might have increased the gradle version to 7.2 locally. I forgot about that. #5109 should make this work as well.

[skaldarnar]

I tried to test this with #5109 and Java 20 (since I cannot find a Java 18 or 19 via SDKMan), but then I run into the issue that Kotlin and Java 20 don't get along yet 🙈 Why can't things be easy 🙄

I suspect this to just work, and will give it another try running the game with Java 11 - if that still works, I guess we can merge and move forward to sort out Gradle, Java, and Kotlin versions along the path. 👍

[skaldarnar]

Again, did not test with Java 18 or higher due to the issues with Gradle, but the game still starts up fine 👍

[soloturn]

kotlin-1.9.0 is released: https://kotlinlang.org/docs/whatsnew19.html which supports java-20, gradle-8.1 supports gradle as well, but cannot run itself yet with java-20.

[soloturn]

@skaldarnar @jdrueckert you want to merge this one please? it prints a warning what parameter to use, which will be valid for java-21 as well, so sufficient for a couple of years.

here some details on why security manager got removed, and how otherwise to address it (attention, they try to sell you snyk...)
https://snyk.io/blog/securitymanager-removed-java/

some key parts extracted:

Ultimately, all deprecated code should be removed or updated since it isn't well-maintained and can present a security risk. You can use jdeprscan to find all deprecated code in your class files.

The only true out-of-the-box alternative to the SecurityManager is the JDK's Security Library, and developers must acquaint themselves with Oracle's Secure Coding Guidelines for Java SE. Ultimately, writing secure code from the get-go is the first step to minimizing your project's vulnerabilities.

from oracles secure coding guidelines:

... the security manager does not and cannot provide protection against issues such as side-channel attacks or lower level problems such as Row hammer, nor can it guarantee complete intra-process isolation. Separate processes (JVMs) should be used to isolate untrusted code from trusted code with sensitive information. Utilizing lower level isolation mechanisms available from operating systems or containers is also recommended.

[soloturn]

@BenjaminAmos @jdrueckert @skaldarnar java-21 is out, tested if it still is good, and it is. can we please merge this?

[BenjaminAmos]

I think this just got overlooked. It happens sometimes.

I'm not able to merge this until the tests pass (this one specifically), even if I wanted to. I might have to rebase these changes to force a tests re-run so that GitHub makes it mergable again. The changes should not affect normal execution before Java 18, so I think I could merge it safely.

[soloturn]

I'm not able to merge this until the tests pass (this one specifically), even if I wanted to. I might have to rebase these changes to force a tests re-run so that GitHub makes it mergable again. The changes should not affect normal execution before Java 18, so I think I could merge it safely.

i cannot see the failing test link, is it because i am not project member?

[Cervator]

Hey @soloturn - interestingly it does look like if I load the PR in an incognito browser I can't see the checks on the main PR page, I have to go to the checks page (https://github.com/MovingBlocks/Terasology/pull/5107/checks) to find them. Huh! I had no idea. That's pretty weird. It looks like this to a maintainer (in dark mode anyway):

I just tried adding you to a "Contributors" team we used to add everybody who contributed to - started doing that years ago before GitHub made some further UI tweaks. Does that let you see the thing in the screenshot? If not maybe it needs a push-access team or so.

Let me see if a rebuild in Jenkins will help, our CI was a little erratic for a while but should be better now. The branch may still need a quick update sometime.

[Cervator]

After the rebuild it looks a bit different, but that may again be more about the CI infrastructure changes. Checking Jenkins directly only shows one failing test, however: https://jenkins.terasology.io/job/Terasology/job/engine/view/change-requests/job/PR-5107/1/testReport/junit/org.terasology.engine.integrationenvironment/ExampleTest/Integration_Tests___testClientConnection__/

Being that it is "ExampleTest" I'm not too worried about it, the latest build from develop also failed on that test: https://jenkins.terasology.io/job/Terasology/job/engine/job/develop/3/

Still curious if you can see the check results now on the main PR page @soloturn :-)

The change itself looks reasonable despite the area of security historically having been a bit of a can of worms to deal with. But yeah if this doesn't affect anything below Java 18 it sounds safe enough 👍

[soloturn]

now i can see the test result here: https://jenkins.terasology.io/job/Terasology/job/engine/job/develop/3/testReport/junit/org.terasology.engine.integrationenvironment/ExampleTest/Integration_Tests___testClientConnection__/

with the stack trace and error message:
java.lang.RuntimeException: Could not connect client org.terasology.engine.core.TerasologyEngine@768d49d5 to local host - timeout.