RFR: 8338411: Implement JEP 486: Permanently Disable the Security Manager [v4]

Mon Oct 28 20:14:55 UTC 2024

On Mon, 28 Oct 2024 12:29:07 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> This is the implementation of JEP 486: Permanently Disable the Security Manager. See [JEP 486](https://openjdk.org/jeps/486) for more details. The [CSR](https://bugs.openjdk.org/browse/JDK-8338412) describes in detail the main changes in the JEP and also includes an apidiff of the specification changes.
>> 
>> NOTE: the majority (~95%) of the changes in this PR are test updates (removal/modifications) and API specification changes, the latter mostly to remove `@throws SecurityException`. The remaining changes are primarily the removal of the `SecurityManager`, `Policy`, `AccessController` and other Security Manager API implementations. There is very little new code.
>> 
>> The code changes can be broken down into roughly the following categories:
>> 
>> 1. Degrading the behavior of Security Manager APIs to either throw Exceptions by default or provide an execution environment that disallows access to all resources by default.
>> 2. Changing hundreds of methods and constructors to no longer throw a `SecurityException` if a Security Manager was enabled. They will operate as they did in JDK 23 with no Security Manager enabled.
>> 3. Changing the `java` command to exit with a fatal error if a Security Manager is enabled.
>> 4. Removing the hotspot native code for the privileged stack walk and the inherited access control context. The remaining hotspot code and tests related to the Security Manager will be removed immediately after integration - see [JDK-8341916](https://bugs.openjdk.org/browse/JDK-8341916).
>> 5. Removing or modifying hundreds of tests. Many tests that tested Security Manager behavior are no longer relevant and thus have been removed or modified.
>> 
>> There are a handful of Security Manager related tests that are failing and are at the end of the `test/jdk/ProblemList.txt`, `test/langtools/ProblemList.txt` and `test/hotspot/jtreg/ProblemList.txt` files - these will be removed or separate bugs will be filed before integrating this PR. 
>> 
>> Inside the JDK, we have retained calls to `SecurityManager::getSecurityManager` and `AccessController::doPrivileged` for now, as these methods have been degraded to behave the same as they did in JDK 23 with no Security Manager enabled. After we integrate this JEP, those calls will be removed in each area (client-libs, core-libs, security, etc).
>> 
>> I don't expect each reviewer to review all the code changes in this JEP. Rather, I advise that you only focus on the changes for the area (client-libs, core-libs, net, ...
>
> Sean Mullan has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 175 commits:
> 
>  - Merge remote-tracking branch 'jdk-sandbox/jep486' into JDK-8338411
>  - Specify that params passed to getPermissions and implies are ignored and
>    implies always returns false.
>  - Change deprecated annotations to api notes on getPolicy and setPolicy.
>  - clientlibs: Updated Problemlist
>    Deleted javax/swing/JPopupMenu/6694823/bug6694823.java entry since it was determined that it is not a JDK bug but expected behavior on windows and linux platform.
>  - clientlibs: Deleted JPopupMenu tests
>    The following tests are deleted as they don't hold value without SM
>    test/jdk/javax/swing/JPopupMenu/6691503/bug6691503.java - It tests that the popup are
>    always-on-top for apps which doesn't hold value after SM removal.
>    
>    test/jdk/javax/swing/JPopupMenu/6694823/bug6694823.java - Tests whether popup can overlap taskbar.
>    Works differently on macOS and windows & linux but this is the expected behavior.
>    Details in JDK-8342012. Not a functional issue.
>  - clientlibs: GetSoundBankSecurityException.java renamed to EmptySoundBankTest.java
>  - clientlibs: GetSoundBankSecurityException.java renamed to EmptySoundBankTest.java
>    test renamed, test summary updated
>  - Restore note for implementers in src/java.prefs/share/classes/java/util/prefs/AbstractPreferences.java
>  - Change "SecurityManager" to "Security Manager". Add some missing code and linkplain tags.
>  - Add api note to class description that permission checking is not supported and remove text about getting permissions from system policy. In getPermissions(), change "granted to the given codesource" to "for the codesource".
>  - ... and 165 more: https://git.openjdk.org/jdk/compare/1476f6c4...e490f698

Reviewed all tests under test/jaxp/javax/xml/jaxp.
A few imports moved around unnecessarily but otherwise looks fine.

test/jaxp/javax/xml/jaxp/functional/org/w3c/dom/ptests/NodeTest.java line 53:

> 51: import org.w3c.dom.Node;
> 52: import org.w3c.dom.NodeList;
> 53: import static jaxp.library.JAXPTestUtilities.USER_DIR;

The import change was unnecessary.

test/jaxp/javax/xml/jaxp/unittest/sax/Bug7057778Test.java line 48:

> 46: import org.xml.sax.XMLReader;
> 47: import org.xml.sax.ext.DefaultHandler2;
> 48: import static jaxp.library.JAXPTestUtilities.USER_DIR;

Keep imports JAXPTestUtilities imports together.

test/jaxp/javax/xml/jaxp/unittest/stream/XMLEventReaderTest/EventReaderTest.java line 32:

> 30: import javax.xml.stream.XMLStreamException;
> 31: import javax.xml.stream.events.StartDocument;
> 32: import static org.testng.Assert.assertEquals;

Import change is unnecessary.

test/jaxp/javax/xml/jaxp/unittest/validation/Bug6925531Test.java line 52:

> 50:             + "        targetNamespace='jaxp13_test'\n"
> 51:             + "        elementFormDefault='qualified'>\n"
> 52:             + "    <element name='test' type='string'/>\n"

Would be a good use for multi-line strings.   """ """
But not worth changing.

test/jaxp/javax/xml/jaxp/unittest/xpath/XPathPrecedingTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.

no change necessary

-------------

PR Review: https://git.openjdk.org/jdk/pull/21498#pullrequestreview-2399985101
PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1819611468
PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1819667734
PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1819672433
PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1819646448
PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1819624631